Forráskód Böngészése

fix: revalidate UI session against api periodically (SEC_REVIEW F36)

The UI's `AuthRequiredMiddleware` previously waved through any request
that had a `_user` row in the session, with no re-check against the api.
A user removed from an Entra admin group, deleted from the api, or
disabled via the F11 path kept their UI session — and its cached role
— until the 8 h idle or 24 h absolute timeout. The api itself (F11) is
the authoritative gate on every data call, so this didn't enable
privilege escalation, but it did mean the UI rendered admin nav, forms,
and pages the user could no longer use, and any "revoke this user
right now" lever an operator pulled was effectively a 5-to-30-minute
warning ahead of an 8-hour ride-out instead of an immediate kick.

The middleware now periodically re-fetches the user via
`GET /api/v1/admin/me`. Cadence is configurable with the new
`UI_SESSION_REVALIDATE_SECONDS` setting (default 300 = 5 min). On 403
(`user_disabled` / `unknown impersonated user`) or 404 the session is
cleared, an "access revoked" flash is set, and the user is bounced to
`/login`. On 200 with a changed role / display name / email the
cached UserContext is rewritten in place — `_authenticated_at` and
`_last_active` are preserved so the existing idle/absolute clocks keep
running off the original login. On api-unreachable, 401, 5xx, or any
other failure the request proceeds with the cached state and the
revalidation timestamp is rotated forward; an api outage cannot lock
every live session out and the api is still the authoritative gate on
the data call itself.

Implementation:

  - SessionManager: new `_revalidated_at` slot, `lastRevalidatedAt()`,
    `markRevalidated()`, and `updateUser()` (replaces the user row
    without touching the auth clocks). `setUser()` seeds
    `_revalidated_at = now()` on login so a fresh session starts the
    clock on the same edge as authenticated_at.
  - AuthRequiredMiddleware: now takes `AdminClient`, `LoggerInterface`,
    and the interval. Pre-F36 ("legacy") sessions without
    `_revalidated_at` are bootstrapped on first sight without an api
    call so existing rolling sessions don't stampede the api on deploy.
  - Container: factory registration so the int interval can be passed
    without DI/autowire confusion.
  - settings.php / .env.example: `UI_SESSION_REVALIDATE_SECONDS=300`.

Regression tests in
`ui/tests/Integration/Auth/SessionRevalidationTest.php` cover
within-interval skip (no api call), past-interval-role-unchanged
(timestamp moves forward, no session edit), past-interval-role-changed
(admin → viewer reflected immediately), the `user_disabled` and
unknown-user revocation kicks (302 to /login + flash + cleared
session), api-unreachable-keeps-session-alive, and the legacy-session
bootstrap.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa 4 napja
szülő
commit
2c3b65b469

+ 6 - 0
.env.example

@@ -119,3 +119,9 @@ LOCAL_ADMIN_PASSWORD_HASH=
 # JavaScript's Intl.DateTimeFormat picks something sensible if the
 # browser's preference isn't supported. Empty = browser-only.
 UI_LOCALE=
+
+# How often (seconds) the UI re-validates the cached user role/identity
+# against `GET /api/v1/admin/me`. Lower = faster propagation of Entra
+# group changes and explicit user-disable actions; higher = fewer api
+# calls per active user. Default 300 (5 min).
+UI_SESSION_REVALIDATE_SECONDS=300

+ 7 - 0
ui/config/settings.php

@@ -57,6 +57,13 @@ return [
     'session_idle_seconds' => (int) (getenv('SESSION_IDLE_SECONDS') ?: 28800),
     'session_absolute_seconds' => (int) (getenv('SESSION_ABSOLUTE_SECONDS') ?: 86400),
 
+    // SEC_REVIEW F36: how often the AuthRequired middleware re-checks the
+    // current user's role / disabled state against `GET /api/v1/admin/me`.
+    // Lower = faster propagation of group/role changes from Entra and
+    // explicit disable actions in the api; higher = fewer api calls per
+    // active user. Default 5 minutes.
+    'session_revalidate_seconds' => (int) (getenv('UI_SESSION_REVALIDATE_SECONDS') ?: 300),
+
     // Local-admin login throttle: file-backed JSON store on the container's
     // writable layer. Persists across FrankenPHP worker recycles and is
     // shared between workers; cleared by container restart (operator unlock

+ 19 - 1
ui/src/App/Container.php

@@ -89,6 +89,7 @@ final class Container
             'settings.local_admin_password_hash' => (string) ($settings['local_admin_password_hash'] ?? ''),
             'settings.session_idle' => (int) ($settings['session_idle_seconds'] ?? 28800),
             'settings.session_absolute' => (int) ($settings['session_absolute_seconds'] ?? 86400),
+            'settings.session_revalidate_seconds' => (int) ($settings['session_revalidate_seconds'] ?? 300),
             'settings.login_throttle_path' => (string) ($settings['login_throttle_path']
                 ?? sys_get_temp_dir() . '/irdb_login_throttle.json'),
             'settings.geoip_provider' => strtolower((string) ($settings['geoip_provider'] ?? 'dbip')),
@@ -199,7 +200,24 @@ final class Container
             SessionMiddleware::class => autowire(),
             CsrfMiddleware::class => autowire(),
             CspMiddleware::class => autowire(),
-            AuthRequiredMiddleware::class => autowire(),
+            AuthRequiredMiddleware::class => factory(static function (ContainerInterface $c): AuthRequiredMiddleware {
+                /** @var SessionManager $sessions */
+                $sessions = $c->get(SessionManager::class);
+                /** @var \Psr\Http\Message\ResponseFactoryInterface $rf */
+                $rf = $c->get(ResponseFactoryInterface::class);
+                /** @var AdminClient $admin */
+                $admin = $c->get(AdminClient::class);
+                /** @var LoggerInterface $logger */
+                $logger = $c->get(LoggerInterface::class);
+
+                return new AuthRequiredMiddleware(
+                    $sessions,
+                    $rf,
+                    $admin,
+                    $logger,
+                    (int) $c->get('settings.session_revalidate_seconds'),
+                );
+            }),
             TwigGlobalsMiddleware::class => factory(static function (ContainerInterface $c): TwigGlobalsMiddleware {
                 /** @var Twig $twig */
                 $twig = $c->get(Twig::class);

+ 40 - 2
ui/src/Auth/SessionManager.php

@@ -34,6 +34,7 @@ class SessionManager
     private const KEY_USER = '_user';
     private const KEY_LAST_ACTIVE = '_last_active';
     private const KEY_AUTH_AT = '_authenticated_at';
+    private const KEY_REVALIDATED_AT = '_revalidated_at';
     private const KEY_FLASH = '_flash';
     private const KEY_NEXT = '_next';
     private const KEY_OIDC = '_oidc';
@@ -101,10 +102,30 @@ class SessionManager
     }
 
     public function setUser(UserContext $user): void
+    {
+        $now = time();
+        $_SESSION[self::KEY_USER] = $user->toArray();
+        $_SESSION[self::KEY_LAST_ACTIVE] = $now;
+        $_SESSION[self::KEY_AUTH_AT] = $now;
+        // SEC_REVIEW F36: seed the revalidation clock on login. The
+        // AuthRequired middleware re-checks the user's role against the api
+        // every `revalidateAfterSeconds` so role changes / disablement /
+        // deletion in Entra or in the api propagate to live UI sessions
+        // without waiting for the 8h idle timeout.
+        $_SESSION[self::KEY_REVALIDATED_AT] = $now;
+    }
+
+    /**
+     * Update the cached user record without resetting the auth clocks.
+     * Called after a successful background revalidation when the api
+     * reports a different role / display name / email than the session
+     * had cached (SEC_REVIEW F36). Preserves `_authenticated_at` and
+     * `_last_active` so idle/absolute timeouts keep running off the
+     * original login.
+     */
+    public function updateUser(UserContext $user): void
     {
         $_SESSION[self::KEY_USER] = $user->toArray();
-        $_SESSION[self::KEY_LAST_ACTIVE] = time();
-        $_SESSION[self::KEY_AUTH_AT] = time();
     }
 
     public function getUser(): ?UserContext
@@ -117,6 +138,23 @@ class SessionManager
         return UserContext::fromArray($row);
     }
 
+    /**
+     * Last time the cached user record was revalidated against the api.
+     * Returns `null` for legacy/pre-feature sessions that predate F36 —
+     * the middleware bootstraps the timestamp on first sight.
+     */
+    public function lastRevalidatedAt(): ?int
+    {
+        $value = $_SESSION[self::KEY_REVALIDATED_AT] ?? null;
+
+        return is_int($value) ? $value : null;
+    }
+
+    public function markRevalidated(?int $now = null): void
+    {
+        $_SESSION[self::KEY_REVALIDATED_AT] = $now ?? time();
+    }
+
     public function refreshActivity(): void
     {
         if (isset($_SESSION[self::KEY_USER])) {

+ 124 - 10
ui/src/Http/AuthRequiredMiddleware.php

@@ -4,41 +4,155 @@ declare(strict_types=1);
 
 namespace App\Http;
 
+use App\ApiClient\AdminClient;
+use App\ApiClient\ApiAuthException;
+use App\ApiClient\ApiException;
+use App\ApiClient\ApiNotFoundException;
 use App\Auth\SessionManager;
+use App\Auth\UserContext;
 use Psr\Http\Message\ResponseFactoryInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
+use Psr\Log\LoggerInterface;
 
 /**
  * Gates `/app/*` routes. If no user is in the session, stash the
  * requested URL as `next` and 302 to `/login`. After a successful
  * login the controller pops `next` and redirects there.
+ *
+ * SEC_REVIEW F36: the cached UserContext (role, identity) is
+ * periodically re-checked against `GET /api/v1/admin/me`. If the api
+ * reports the user as disabled, deleted, or with a changed role, the
+ * UI session is brought back in line — disabled / unknown users are
+ * logged out immediately and a role downgrade in Entra propagates to
+ * the next protected request rather than waiting for the 8h idle
+ * timeout. The api itself is also the authoritative gate (every data
+ * call goes through `X-Acting-User-Id` and the api re-resolves the
+ * principal each time), so this middleware is mostly about avoiding
+ * the UI rendering nav items / forms the user can no longer use.
  */
 final class AuthRequiredMiddleware implements MiddlewareInterface
 {
     public function __construct(
         private readonly SessionManager $sessions,
         private readonly ResponseFactoryInterface $responseFactory,
+        private readonly AdminClient $admin,
+        private readonly LoggerInterface $logger,
+        private readonly int $revalidateAfterSeconds = 300,
     ) {
     }
 
     public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
     {
         $user = $this->sessions->getUser();
-        if ($user !== null) {
-            return $handler->handle($request);
+        if ($user === null) {
+            $uri = $request->getUri();
+            $path = $uri->getPath();
+            $query = $uri->getQuery();
+            $next = $query !== '' ? $path . '?' . $query : $path;
+            $this->sessions->setNext($next);
+
+            return $this->responseFactory
+                ->createResponse(302)
+                ->withHeader('Location', '/login');
+        }
+
+        if ($this->shouldRevalidate()) {
+            $revoked = $this->revalidate($user);
+            if ($revoked) {
+                $this->sessions->clear();
+                $this->sessions->flash('error', 'Your access was revoked. Please sign in again.');
+
+                return $this->responseFactory
+                    ->createResponse(302)
+                    ->withHeader('Location', '/login');
+            }
         }
 
-        $uri = $request->getUri();
-        $path = $uri->getPath();
-        $query = $uri->getQuery();
-        $next = $query !== '' ? $path . '?' . $query : $path;
-        $this->sessions->setNext($next);
+        return $handler->handle($request);
+    }
+
+    private function shouldRevalidate(): bool
+    {
+        $lastRev = $this->sessions->lastRevalidatedAt();
+        if ($lastRev === null) {
+            // Legacy/pre-F36 session — seed the clock and let the next
+            // request past the threshold be the first real revalidation.
+            $this->sessions->markRevalidated();
+
+            return false;
+        }
+
+        return (time() - $lastRev) >= $this->revalidateAfterSeconds;
+    }
+
+    /**
+     * @return bool `true` if the session must be cleared (user revoked).
+     */
+    private function revalidate(UserContext $user): bool
+    {
+        try {
+            $live = $this->admin->getMe($user->userId);
+        } catch (ApiAuthException $e) {
+            // 401 = ui_service_token deployment problem; 403 = this user
+            // is disabled / unknown. Only the 403 case revokes; a 401
+            // would lock every session out on a deployment slip-up.
+            if ($e->statusCode === 403) {
+                $this->logger->warning('session revalidation: user revoked by api', [
+                    'user_id' => $user->userId,
+                    'api_error' => $e->apiError,
+                ]);
+
+                return true;
+            }
+            $this->logger->error('session revalidation: api auth error, keeping session', [
+                'user_id' => $user->userId,
+                'status' => $e->statusCode,
+                'api_error' => $e->apiError,
+            ]);
+            $this->sessions->markRevalidated();
+
+            return false;
+        } catch (ApiNotFoundException $e) {
+            // Defensive: the api currently returns 403 (not 404) when the
+            // user record is missing. Treat 404 the same way for safety.
+            $this->logger->warning('session revalidation: user record missing', [
+                'user_id' => $user->userId,
+                'api_error' => $e->apiError,
+            ]);
+
+            return true;
+        } catch (ApiException $e) {
+            // Api unreachable / 5xx / unexpected. Don't lock everyone out
+            // on a backend blip; mark revalidated so we don't grind on it
+            // every request, and try again after the next interval.
+            $this->logger->warning('session revalidation: api unreachable, keeping session', [
+                'user_id' => $user->userId,
+                'error' => $e->getMessage(),
+            ]);
+            $this->sessions->markRevalidated();
+
+            return false;
+        }
+
+        $live_email = $live->email;
+        $changed = $live->role !== $user->role
+            || ($live->displayName !== '' && $live->displayName !== $user->displayName)
+            || $live_email !== $user->email;
+
+        if ($changed) {
+            $this->sessions->updateUser(new UserContext(
+                userId: $user->userId,
+                displayName: $live->displayName !== '' ? $live->displayName : $user->displayName,
+                role: $live->role,
+                email: $live_email,
+                source: $user->source,
+            ));
+        }
+        $this->sessions->markRevalidated();
 
-        return $this->responseFactory
-            ->createResponse(302)
-            ->withHeader('Location', '/login');
+        return false;
     }
 }

+ 189 - 0
ui/tests/Integration/Auth/SessionRevalidationTest.php

@@ -0,0 +1,189 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Integration\Auth;
+
+use App\Auth\UserContext;
+use App\Tests\Integration\Support\AppTestCase;
+
+/**
+ * SEC_REVIEW F36 — `AuthRequiredMiddleware` periodically revalidates the
+ * cached user against `GET /api/v1/admin/me`. A role change in Entra,
+ * an explicit disable in the api, or a deleted user record propagates
+ * to the live UI session at the next protected request that's past the
+ * revalidation window — no need to wait for the 8 h idle timeout.
+ */
+final class SessionRevalidationTest extends AppTestCase
+{
+    protected function setUp(): void
+    {
+        $this->bootApp();
+        // The first /app/* request in a process triggers session_start(),
+        // which clobbers $_SESSION values primed by the test. Hit a public
+        // route first so the session is already active before the test
+        // request fires (same workaround as SearchPageTest).
+        $this->request('GET', '/healthz');
+    }
+
+    public function testWithinIntervalSkipsRevalidation(): void
+    {
+        // Prime a fresh session with `_revalidated_at = now`. The middleware
+        // should NOT call the api on this request.
+        $this->seedSession(userId: 1, role: 'admin', revalidatedAt: time());
+        // Dashboard fires its own getDashboardStats() call; stub it.
+        $this->stubDashboardApiCalls();
+
+        $response = $this->request('GET', '/app/dashboard');
+
+        // We reached the dashboard handler — middleware did not bounce us
+        // back to /login or trigger a revalidation request to /admin/me.
+        self::assertNotSame('/login', $response->getHeaderLine('Location'));
+        // Exactly one api call: the dashboard stats. No /admin/me call.
+        $apiPaths = array_map(
+            static fn (array $entry): string => (string) $entry['request']->getUri()->getPath(),
+            $this->apiHistory,
+        );
+        self::assertNotContains('/api/v1/admin/me', $apiPaths);
+    }
+
+    public function testRevalidationKeepsSessionWhenRoleUnchanged(): void
+    {
+        // `_revalidated_at` set to 10 minutes ago → revalidation triggers.
+        $this->seedSession(userId: 1, role: 'admin', revalidatedAt: time() - 600);
+        $this->enqueueApiResponse(200, [
+            'user_id' => 1, 'role' => 'admin', 'email' => 'a@x',
+            'display_name' => 'Admin', 'is_local' => true,
+        ]);
+        // Dashboard page itself fires its own api calls — feed a permissive stub.
+        $this->stubDashboardApiCalls();
+
+        $response = $this->request('GET', '/app/dashboard');
+
+        self::assertNotSame(302, $response->getStatusCode());
+        self::assertSame('admin', $_SESSION['_user']['role']);
+        // `_revalidated_at` was rotated forward.
+        self::assertGreaterThanOrEqual(time() - 5, (int) $_SESSION['_revalidated_at']);
+    }
+
+    public function testRevalidationUpdatesSessionWhenRoleChanged(): void
+    {
+        $this->seedSession(userId: 1, role: 'admin', revalidatedAt: time() - 600);
+        // Api now reports the user as a viewer (was admin in session).
+        $this->enqueueApiResponse(200, [
+            'user_id' => 1, 'role' => 'viewer', 'email' => 'a@x',
+            'display_name' => 'Admin', 'is_local' => true,
+        ]);
+        $this->stubDashboardApiCalls();
+
+        $this->request('GET', '/app/dashboard');
+
+        self::assertSame('viewer', $_SESSION['_user']['role']);
+    }
+
+    public function testDisabledUserIsKickedToLogin(): void
+    {
+        // User disabled in api → ImpersonationMiddleware returns 403 user_disabled.
+        $this->seedSession(userId: 1, role: 'admin', revalidatedAt: time() - 600);
+        $this->enqueueApiResponse(403, ['error' => 'user_disabled']);
+
+        $response = $this->request('GET', '/app/dashboard');
+
+        self::assertSame(302, $response->getStatusCode());
+        self::assertSame('/login', $response->getHeaderLine('Location'));
+        self::assertArrayNotHasKey('_user', $_SESSION);
+        // The clear-and-flash-and-redirect is what the user sees.
+        $flash = $_SESSION['_flash'] ?? [];
+        self::assertNotEmpty($flash);
+        self::assertSame('error', $flash[0]['type']);
+        self::assertStringContainsString('revoked', $flash[0]['message']);
+    }
+
+    public function testDeletedUserIsKickedToLogin(): void
+    {
+        // Api reports 403 unknown impersonated user.
+        $this->seedSession(userId: 1, role: 'admin', revalidatedAt: time() - 600);
+        $this->enqueueApiResponse(403, ['error' => 'unknown impersonated user']);
+
+        $response = $this->request('GET', '/app/dashboard');
+
+        self::assertSame(302, $response->getStatusCode());
+        self::assertSame('/login', $response->getHeaderLine('Location'));
+        self::assertArrayNotHasKey('_user', $_SESSION);
+    }
+
+    public function testApiUnreachableKeepsSessionAlive(): void
+    {
+        // API down: an outage must NOT lock every session out. The middleware
+        // should log + tolerate, marking the session revalidated so it doesn't
+        // hammer the api on every request.
+        $this->seedSession(userId: 1, role: 'admin', revalidatedAt: time() - 600);
+        // Two ConnectExceptions because ApiClient retries once on connect.
+        $this->enqueueApiException(new \GuzzleHttp\Exception\ConnectException(
+            'down',
+            new \GuzzleHttp\Psr7\Request('GET', '/'),
+        ));
+        $this->enqueueApiException(new \GuzzleHttp\Exception\ConnectException(
+            'down',
+            new \GuzzleHttp\Psr7\Request('GET', '/'),
+        ));
+        $this->stubDashboardApiCalls();
+
+        $response = $this->request('GET', '/app/dashboard');
+
+        // Session preserved.
+        self::assertNotSame('/login', $response->getHeaderLine('Location'));
+        self::assertSame('admin', $_SESSION['_user']['role']);
+        // Marked revalidated even though the api was down.
+        self::assertGreaterThanOrEqual(time() - 5, (int) $_SESSION['_revalidated_at']);
+    }
+
+    public function testLegacySessionWithoutRevalidatedAtIsBootstrapped(): void
+    {
+        // Pre-F36 sessions don't have `_revalidated_at`. The middleware
+        // must bootstrap the timestamp instead of revalidating immediately
+        // (no api call, no test breakage on legacy fixtures).
+        $_SESSION['_user'] = (new UserContext(1, 'Admin', 'admin', null, UserContext::SOURCE_LOCAL))->toArray();
+        $_SESSION['_last_active'] = time();
+        $_SESSION['_authenticated_at'] = time();
+        // No _revalidated_at deliberately.
+        $this->stubDashboardApiCalls();
+
+        $this->request('GET', '/app/dashboard');
+
+        self::assertArrayHasKey('_revalidated_at', $_SESSION);
+        self::assertGreaterThanOrEqual(time() - 5, (int) $_SESSION['_revalidated_at']);
+    }
+
+    private function seedSession(int $userId, string $role, int $revalidatedAt): void
+    {
+        $_SESSION['_user'] = (new UserContext(
+            $userId,
+            'Admin',
+            $role,
+            'a@x',
+            UserContext::SOURCE_LOCAL,
+        ))->toArray();
+        $_SESSION['_last_active'] = time();
+        $_SESSION['_authenticated_at'] = time();
+        $_SESSION['_revalidated_at'] = $revalidatedAt;
+    }
+
+    /**
+     * Dashboard page fires its own api calls. Feed empty/200 stubs so the
+     * test focuses on the revalidation outcome rather than the page content.
+     */
+    private function stubDashboardApiCalls(): void
+    {
+        // Dashboard issues a getDashboardStats() call; queue a permissive
+        // empty payload. Tests that 302 away never reach the dashboard so
+        // unconsumed stubs don't cause failures.
+        $this->enqueueApiResponse(200, [
+            'totals' => [],
+            'top_categories' => [],
+            'top_countries' => [],
+            'last_7_days' => [],
+            'top_reporters_30d' => [],
+        ]);
+    }
+}