fix: accept nullable email on /auth/users/upsert-oidc (SEC_REVIEW F33)

Some IdPs (or Entra app configurations that omit the email scope) do
not release the `email` claim. The UI's `OidcClaims->email` is `?string`
to reflect that, but `UserRepository::upsertOidc` typed `email` as a
non-null `string` and `AuthController::upsertOidc` rejected the request
with a 400 — blocking legitimate sign-ins for IdP setups that don't
release the claim, and risking a TypeError if a future change ever
piped the value through without the controller-level `''` guard.

`UserRepository::upsertOidc` now types `email` as `?string` and persists
`users.email = NULL` when absent. `AuthController::upsertOidc` reads
`email` via a new `nullableStr()` helper (missing / JSON-null / empty
string all collapse to `null`); `subject` and `display_name` remain
mandatory. The `user.created` / `user.role_changed` audit rows fall
back to `display_name` for `target_label` when `email` is null, so
SOC tooling still gets a human-readable identifier in either case.

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

api/src/Application/Auth/AuthController.php

@@ -41,11 +41,14 @@ final class AuthController
 
         $body = $this->json($request);
         $subject = self::str($body, 'subject');
-        $email = self::str($body, 'email');
+        // SEC_REVIEW F33: email is nullable — some IdPs (or Entra app
+        // configurations that omit the email scope) don't release it.
+        // Accept missing / null / empty as "no email released".
+        $email = self::nullableStr($body, 'email');
         $displayName = self::str($body, 'display_name');
         $groups = self::stringList($body, 'groups');
 
-        if ($subject === '' || $email === '' || $displayName === '') {
+        if ($subject === '' || $displayName === '') {
             return self::error($response, 400, 'invalid request body');
         }
 
@@ -67,6 +70,9 @@ final class AuthController
                 // SEC_REVIEW F5: account creation is a primary SOC event;
                 // emit a user.created row attributed to the service-token
                 // call (kind=system) with source IP from AuditContextMiddleware.
+                // SEC_REVIEW F33: fall back to displayName when the IdP
+                // didn't release `email`, so the audit row carries a
+                // human-readable label either way.
                 $this->audit->emitOrThrow(
                     AuditAction::USER_CREATED,
                     'user',
@@ -80,7 +86,7 @@ final class AuthController
                         'groups' => $groups,
                     ],
                     $auditCtx,
-                    $email,
+                    $email ?? $displayName,
                 );
             } elseif ($existing->role !== $user->role) {
                 // Role drift on subsequent OIDC login (group membership change).
@@ -95,7 +101,7 @@ final class AuthController
                         'groups' => $groups,
                     ],
                     $auditCtx,
-                    $email,
+                    $email ?? $displayName,
                 );
             }
 
@@ -265,6 +271,19 @@ final class AuthController
         return isset($body[$key]) && is_string($body[$key]) ? $body[$key] : '';
     }
 
+    /**
+     * @param array<string, mixed> $body
+     */
+    private static function nullableStr(array $body, string $key): ?string
+    {
+        if (!isset($body[$key]) || !is_string($body[$key])) {
+            return null;
+        }
+        $value = $body[$key];
+
+        return $value === '' ? null : $value;
+    }
+
     /**
      * @param array<string, mixed> $body
      * @return list<string>

api/src/Infrastructure/Auth/UserRepository.php

@@ -123,11 +123,18 @@ final class UserRepository
     }
 
     /**
+     * `$email` is nullable: SPEC §10 / SEC_REVIEW F33 — some IdPs do not
+     * release the `email` claim (configuration-side or scope-side), and the
+     * UI surfaces `?string` from `OidcClaims->email`. The boundary used to
+     * reject null emails with a 400 (and earlier still would have raised a
+     * TypeError on the way to the repository); now it persists `null` and
+     * lets the caller fall back to `displayName` for any user-facing label.
+     *
      * @param list<string> $groupIds
      */
     public function upsertOidc(
         string $subject,
-        string $email,
+        ?string $email,
         string $displayName,
         array $groupIds,
         Role $defaultRole,

api/tests/Integration/Auth/AuthEndpointsTest.php

@@ -437,6 +437,120 @@ final class AuthEndpointsTest extends AppTestCase
         self::assertSame('viewer', $second['role']);
     }
 
+    /**
+     * SEC_REVIEW F33 regression. `OidcClaims->email` is nullable — some
+     * IdPs (or Entra app configurations that omit the email scope) do
+     * not release the claim, and the UI forwards `email: null` in that
+     * case. The endpoint must accept the request, persist
+     * `users.email = NULL`, and emit `user.created` with `target_label`
+     * falling back to `display_name`.
+     */
+    public function testUpsertOidcAcceptsMissingEmail(): void
+    {
+        $token = $this->createToken(TokenKind::Service);
+        $headers = [
+            'Authorization' => 'Bearer ' . $token,
+            'Content-Type' => 'application/json',
+        ];
+
+        // No `email` key at all (key omitted).
+        $response = $this->request('POST', '/api/v1/auth/users/upsert-oidc', $headers, json_encode([
+            'subject' => 'no-email-sub',
+            'display_name' => 'No Email',
+            'groups' => [],
+        ]) ?: null);
+        self::assertSame(200, $response->getStatusCode());
+        $body = $this->decode($response);
+        self::assertNull($body['email']);
+        self::assertSame('No Email', $body['display_name']);
+
+        $row = $this->db->fetchAssociative(
+            'SELECT email, display_name FROM users WHERE id = :id',
+            ['id' => $body['user_id']],
+        );
+        self::assertIsArray($row);
+        self::assertNull($row['email'], 'email column must persist NULL when not released');
+        self::assertSame('No Email', $row['display_name']);
+
+        $audit = $this->db->fetchAssociative(
+            "SELECT target_label, details_json FROM audit_log WHERE action = 'user.created' ORDER BY id DESC LIMIT 1"
+        );
+        self::assertIsArray($audit);
+        self::assertSame('No Email', $audit['target_label'], 'target_label falls back to display_name when email null');
+        $details = json_decode((string) $audit['details_json'], true);
+        self::assertIsArray($details);
+        self::assertNull($details['email']);
+    }
+
+    public function testUpsertOidcAcceptsExplicitNullEmail(): void
+    {
+        $token = $this->createToken(TokenKind::Service);
+        $headers = [
+            'Authorization' => 'Bearer ' . $token,
+            'Content-Type' => 'application/json',
+        ];
+
+        // Explicit JSON null — what the UI sends when OidcClaims->email is null.
+        $response = $this->request('POST', '/api/v1/auth/users/upsert-oidc', $headers, json_encode([
+            'subject' => 'null-email-sub',
+            'email' => null,
+            'display_name' => 'Null Email',
+            'groups' => [],
+        ]) ?: null);
+        self::assertSame(200, $response->getStatusCode());
+        $body = $this->decode($response);
+        self::assertNull($body['email']);
+
+        $row = $this->db->fetchAssociative(
+            'SELECT email FROM users WHERE id = :id',
+            ['id' => $body['user_id']],
+        );
+        self::assertIsArray($row);
+        self::assertNull($row['email']);
+    }
+
+    /**
+     * SEC_REVIEW F33: subject and display_name remain required. Missing
+     * either still 400s — the relaxation only applies to `email`.
+     */
+    public function testUpsertOidcStillRejectsMissingSubject(): void
+    {
+        $token = $this->createToken(TokenKind::Service);
+        $response = $this->request(
+            'POST',
+            '/api/v1/auth/users/upsert-oidc',
+            [
+                'Authorization' => 'Bearer ' . $token,
+                'Content-Type' => 'application/json',
+            ],
+            json_encode([
+                'email' => 'x@example.com',
+                'display_name' => 'X',
+                'groups' => [],
+            ]) ?: null
+        );
+        self::assertSame(400, $response->getStatusCode());
+    }
+
+    public function testUpsertOidcStillRejectsMissingDisplayName(): void
+    {
+        $token = $this->createToken(TokenKind::Service);
+        $response = $this->request(
+            'POST',
+            '/api/v1/auth/users/upsert-oidc',
+            [
+                'Authorization' => 'Bearer ' . $token,
+                'Content-Type' => 'application/json',
+            ],
+            json_encode([
+                'subject' => 'x',
+                'email' => 'x@example.com',
+                'groups' => [],
+            ]) ?: null
+        );
+        self::assertSame(400, $response->getStatusCode());
+    }
+
     public function testUpsertOidcRejectsAdminToken(): void
     {
         // Even an admin token can't call /auth/* — those are service-only.