Fix R01-N18: trust OIDC email only when issuer hasn't marked it unverified

The audit log uses `users.email` as the human-readable actor; in some
Entra tenant configurations the `preferred_username` claim is *not* a
verified email and can be controlled by the end user. The previous
fallback `claims.email ?? claims.preferred_username` therefore let an
attacker spoof their forensic identity (user identity is keyed by
`oid`/`sub` and is unaffected — only the displayed label was at risk).

New `App\Auth\OidcClaims::resolveEmail($claims, $oid)` is the single
decision point:

  1. `preferred_username` is never used as a fallback.
  2. `claims.email` with `email_verified === false` is rejected.
  3. Missing `email_verified` is treated as "no negative signal" so
     Entra v2.0 work/school tokens (which never emit the flag) keep
     working.
  4. Otherwise the trimmed `claims.email` is returned.
  5. When the email is missing, blank, or explicitly unverified the
     resolver returns `entra:<oid>` so the audit row still has a
     stable, non-spoofable actor.

`AuthController::callback` now calls the helper and treats `oid` as a
hard requirement (failure logged) before resolving the email, so the
resolver never has to invent a fallback for a missing oid.

Tests: new `tests/Auth/OidcClaimsTest.php` (10 cases) pins the rules:
verified-true / verified-absent / verified-false / preferred_username-
fallback-rejected / blank-email / whitespace-only / trim / no-claims-
at-all / non-bool-truthy / non-scalar-email. Suite: 252 / 683 (was
242 / 673).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

src/Auth/OidcClaims.php

@@ -0,0 +1,52 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Auth;
+
+/**
+ * Pure helpers for digesting the verified claims object returned by
+ * jumbojett/openid-connect-php. Kept separate from {@see OidcClient}
+ * (a thin factory) so the rules can be unit-tested without a real
+ * tenant.
+ */
+final class OidcClaims
+{
+    /**
+     * Resolve the audit-trail email for an OIDC user.
+     *
+     * R01-N18 — in some Entra tenant configurations `preferred_username`
+     * is NOT a verified email; it can be controlled by the end user. The
+     * audit log uses the email column as the human-readable actor, so an
+     * attacker could otherwise spoof their identity in the forensic
+     * trail. The user *identity* itself is keyed by `oid`/`sub` and is
+     * unaffected by this helper.
+     *
+     * Rules:
+     *  1. `preferred_username` is never used as a fallback — the only
+     *     trusted email source is `claims.email`.
+     *  2. If `claims.email_verified === false` we treat `claims.email`
+     *     as untrusted (the issuer explicitly told us so).
+     *  3. Entra v2.0 work/school accounts do not emit `email_verified`;
+     *     the `email` they ship is sourced from the directory and is
+     *     server-controlled. We treat a missing `email_verified` as
+     *     "no negative signal" and trust `email` in that case.
+     *  4. When the email is missing or explicitly unverified, fall
+     *     back to a documented immutable label `entra:<oid>` so the
+     *     audit row still has a stable, non-spoofable actor.
+     */
+    public static function resolveEmail(object $claims, string $oid): string
+    {
+        $rawEmail = (isset($claims->email) && is_scalar($claims->email))
+            ? trim((string) $claims->email)
+            : '';
+
+        $explicitlyUnverified = (($claims->email_verified ?? null) === false);
+
+        if ($rawEmail !== '' && !$explicitlyUnverified) {
+            return $rawEmail;
+        }
+
+        return 'entra:' . $oid;
+    }
+}

src/Controllers/AuthController.php

@@ -6,6 +6,7 @@ namespace App\Controllers;
 
 use App\Auth\BootstrapAdmin;
 use App\Auth\LocalAdmin;
+use App\Auth\OidcClaims;
 use App\Auth\OidcClient;
 use App\Auth\SessionGuard;
 use App\Http\Request;
@@ -77,15 +78,20 @@ final class AuthController
             return Response::redirect('/?auth_error=1');
         }
 
-        $oid   = (string) ($claims->oid   ?? $claims->sub ?? '');
-        $email = (string) ($claims->email ?? $claims->preferred_username ?? '');
-        $name  = (string) ($claims->name  ?? $email ?? 'user');
+        $oid = (string) ($claims->oid ?? $claims->sub ?? '');
 
         if ($oid === '') {
             $this->logFailure($req, 'missing_oid_or_sub_claim');
             return Response::redirect('/?auth_error=1');
         }
 
+        // R01-N18: never fall back to `preferred_username` (user-controlled
+        // in some Entra tenants) and reject `email` when the issuer marks
+        // it unverified. Identity is still keyed by `oid`; this only
+        // governs the human-readable label that ends up in the audit log.
+        $email = OidcClaims::resolveEmail($claims, $oid);
+        $name  = (string) ($claims->name ?? $email);
+
         // R01-N03: explicit env-bootstrap. The OIDC path no longer auto-
         // promotes the first user to land — an attacker with a valid
         // tenant account could otherwise win the race to /auth/callback.

tests/Auth/OidcClaimsTest.php

@@ -0,0 +1,144 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Auth;
+
+use App\Auth\OidcClaims;
+use App\Tests\TestCase;
+use stdClass;
+
+/**
+ * R01-N18 — pin the email-resolution rules. The function decides what
+ * string ends up in `users.email` and `audit_log.user_email` for OIDC
+ * users. Identity is keyed by `oid`, which is unaffected.
+ */
+final class OidcClaimsTest extends TestCase
+{
+    private const OID = '00000000-0000-0000-0000-000000000abc';
+
+    public function testTrustsEmailWhenExplicitlyVerified(): void
+    {
+        $c = new stdClass();
+        $c->email          = 'alice@example.com';
+        $c->email_verified = true;
+
+        self::assertSame(
+            'alice@example.com',
+            OidcClaims::resolveEmail($c, self::OID)
+        );
+    }
+
+    public function testTrustsEmailWhenVerifiedFlagIsAbsent(): void
+    {
+        // Entra v2.0 work/school accounts do not emit email_verified;
+        // the email comes from the directory and is server-controlled.
+        $c = new stdClass();
+        $c->email = 'alice@example.com';
+
+        self::assertSame(
+            'alice@example.com',
+            OidcClaims::resolveEmail($c, self::OID)
+        );
+    }
+
+    public function testFallsBackWhenIssuerExplicitlyUnverified(): void
+    {
+        $c = new stdClass();
+        $c->email          = 'spoofed@example.com';
+        $c->email_verified = false;
+
+        self::assertSame(
+            'entra:' . self::OID,
+            OidcClaims::resolveEmail($c, self::OID)
+        );
+    }
+
+    public function testNeverFallsBackToPreferredUsername(): void
+    {
+        // The actual hazard the finding flagged: preferred_username may
+        // be user-controlled. Even when email is missing entirely, the
+        // resolver must NOT pick it up.
+        $c = new stdClass();
+        $c->preferred_username = 'attacker@victim.example';
+
+        self::assertSame(
+            'entra:' . self::OID,
+            OidcClaims::resolveEmail($c, self::OID)
+        );
+    }
+
+    public function testEmptyEmailFallsBack(): void
+    {
+        $c = new stdClass();
+        $c->email          = '';
+        $c->email_verified = true;
+
+        self::assertSame(
+            'entra:' . self::OID,
+            OidcClaims::resolveEmail($c, self::OID)
+        );
+    }
+
+    public function testWhitespaceOnlyEmailFallsBack(): void
+    {
+        $c = new stdClass();
+        $c->email          = "   \t  ";
+        $c->email_verified = true;
+
+        self::assertSame(
+            'entra:' . self::OID,
+            OidcClaims::resolveEmail($c, self::OID)
+        );
+    }
+
+    public function testEmailIsTrimmed(): void
+    {
+        $c = new stdClass();
+        $c->email = '  alice@example.com  ';
+
+        self::assertSame(
+            'alice@example.com',
+            OidcClaims::resolveEmail($c, self::OID)
+        );
+    }
+
+    public function testNoClaimsAtAllFallsBack(): void
+    {
+        $c = new stdClass();
+
+        self::assertSame(
+            'entra:' . self::OID,
+            OidcClaims::resolveEmail($c, self::OID)
+        );
+    }
+
+    public function testTruthyButNonBooleanVerifiedStillTrusted(): void
+    {
+        // Per the OIDC core spec email_verified should be boolean. Some
+        // IdPs ship strings; we only treat literal `false` as a negative
+        // signal — anything else is "no negative signal" and we trust
+        // the email.
+        $c = new stdClass();
+        $c->email          = 'alice@example.com';
+        $c->email_verified = 'false'; // string, not bool false
+
+        self::assertSame(
+            'alice@example.com',
+            OidcClaims::resolveEmail($c, self::OID)
+        );
+    }
+
+    public function testNonScalarEmailTreatedAsMissing(): void
+    {
+        // Defence against unexpected JWT decoder shapes (e.g. nested
+        // objects, arrays). is_scalar() guards the cast.
+        $c = new stdClass();
+        $c->email = ['alice@example.com'];
+
+        self::assertSame(
+            'entra:' . self::OID,
+            OidcClaims::resolveEmail($c, self::OID)
+        );
+    }
+}