fix: a signed-in session follows the account it belongs to

CLAUDE.md

@@ -482,7 +482,8 @@ It runs the other way too, and that is the more useful half: the API re-reads th
 request and refuses a disabled one, while the web trusted what login had put in the session — so
 disabling an account stopped its token at once and left its browser session working, and since the
 timeout is measured from the last request, the session being actively used is the one that never
-expires.
+expires. Revoking administrator rights, changing the group or tightening the profile had the same gap for
+the same reason; `Init` now rebuilds the session's user and profile from the row it already reads.
 
 **Take a rule you can see being enforced and go looking for its other door.** When you find the gap,
 put the check somewhere both doors reach — a shared base method, or the service under them — rather

src/Infrastructure/Adapter/In/Web/Init.php

@@ -57,7 +57,9 @@
 use SP\Domain\ItemPreset\Models\SessionTimeout;
 use SP\Domain\ItemPreset\Ports\ItemPresetInterface;
 use SP\Application\ItemPreset\Ports\ItemPresetService;
+use SP\Domain\User\Dtos\UserDto;
 use SP\Domain\User\Models\ProfileData;
+use SP\Domain\User\Models\User;
 use SP\Application\User\Ports\UserProfileService;
 use SP\Application\User\Ports\UserService;
 use SP\Domain\Core\Exceptions\NoSuchItemException;
@@ -286,10 +288,24 @@ public function initialize(string $controller): void
                 // the user on the login page. The controllers excluded here are the login form
                 // itself and the read-only lists behind the pickers; a disabled account is caught
                 // on the next page it asks for.
-                if ($this->context->isLoggedIn() && $this->isUserDisabled()) {
-                    logger('User disabled; ending session', 'INFO');
-
-                    SessionLifecycleHandler::restart();
+                //
+                // The same read then refreshes what the session holds about the user. Everything
+                // an authorisation decision reads — `isAdminApp`, `isAdminAcc`, the group, the
+                // profile — was copied into the session at login and never looked at again, so
+                // revoking somebody's administrator rights, moving them to another group or
+                // tightening their profile left the session they were using with the old ones for
+                // as long as they kept using it. The API rebuilds all of it from the row on every
+                // request (`Api::setupUser()`); this is the web asking the same question.
+                if ($this->context->isLoggedIn()) {
+                    $user = $this->readSignedInUser();
+
+                    if ($user?->isDisabled() === true) {
+                        logger('User disabled; ending session', 'INFO');
+
+                        SessionLifecycleHandler::restart();
+                    } elseif ($user !== null) {
+                        $this->refreshSignedInUser($user);
+                    }
                 }
             }
 
@@ -341,32 +357,48 @@ private function getUriFor(string $route): string
      * @throws SPException
      */
     /**
-     * Whether the signed-in user's account has since been disabled.
+     * The signed-in user as the database has them now, or null when that cannot be read.
      *
-     * Read from the database rather than from the session, which is the whole point: the session
-     * holds what was true at login.
-     *
-     * Only a positive answer ends a session. A read that fails — the database briefly unreachable,
-     * a row that does not come back — says nothing about the account, and this runs on every
-     * request of every session: answering "disabled" to a hiccup would sign out everybody at once
-     * and turn it into an outage. So the session stands, and the request goes on to fail on its
-     * own terms if it needed the user. An account that has been deleted rather than disabled is
-     * left to the ordinary session expiry for the same reason.
+     * A read that fails leaves the session alone: this runs on every request of every session, and
+     * treating "could not read the account" as a verdict would sign everybody out the moment the
+     * database hiccuped.
      */
-    private function isUserDisabled(): bool
+    private function readSignedInUser(): ?User
     {
         try {
-            // `=== true`, not the bare value: the getter is `?bool`, and under strict_types a null
-            // — a row whose flag was never set — is a TypeError out of a method declared `bool`,
-            // on every request of every session. An absent flag is not a disabled account.
-            return $this->userService->getById($this->context->getUserData()->id ?? 0)->isDisabled() === true;
+            return $this->userService->getById($this->context->getUserData()->id ?? 0);
         } catch (Throwable $e) {
             logger($e->getMessage());
 
-            return false;
+            return null;
         }
     }
 
+    /**
+     * Replace the session's copy of the user and their profile with the current ones.
+     *
+     * The profile is read afresh too, since an administrator editing it changes what every holder
+     * may do. A profile that cannot be read keeps the one the session already has, for the same
+     * reason as above.
+     */
+    private function refreshSignedInUser(User $user): void
+    {
+        $userDto = UserDto::fromModel($user);
+
+        $this->context->setUserData($userDto);
+
+        try {
+            $this->context->setUserProfile(
+                $this->userProfileService
+                    ->getById($userDto->userProfileId ?? 0)
+                    ->hydrate(ProfileData::class) ?? new ProfileData()
+            );
+        } catch (Throwable $e) {
+            logger($e->getMessage());
+        }
+    }
+
+
     private function initUserSession(): void
     {
         $sessionContext = $this->requireSessionContext();

tests/Unit/Infrastructure/Adapter/In/Web/InitSessionTest.php

@@ -39,6 +39,8 @@
 use SP\Application\Config\Ports\ConfigFileService;
 use SP\Application\ItemPreset\Ports\ItemPresetService;
 use SP\Application\User\Ports\UserProfileService;
+use SP\Domain\User\Models\UserProfile;
+use SP\Domain\User\Models\ProfileData;
 use SP\Application\User\Ports\UserService;
 use SP\Domain\Core\Exceptions\NoSuchItemException;
 use SP\Domain\User\Models\User;
@@ -91,6 +93,7 @@ class InitSessionTest extends UnitaryTestCase
     private ItemPresetService|MockObject $itemPresetService;
     private SessionKeyService|MockObject $sessionKeyService;
     private MockObject|UserService $userService;
+    private ?UserProfileService $userProfileService = null;
     private Session $session;
 
     /**
@@ -302,7 +305,9 @@ public function testASessionWhoseAccountIsStillEnabledSurvives(): void
         $this->session->setUserData(new UserDto(id: 7, login: 'admin'));
 
         $this->userService = $this->createStub(UserService::class);
-        $this->userService->method('getById')->willReturn(new User(['id' => 7, 'isDisabled' => false]));
+        $this->userService->method('getById')->willReturn(
+            new User(['id' => 7, 'login' => 'admin', 'isDisabled' => false])
+        );
 
         $freshSession = $this->buildInitForAFreshSession($session = new Session());
 
@@ -312,6 +317,86 @@ public function testASessionWhoseAccountIsStillEnabledSurvives(): void
         self::assertSame('admin', $session->getUserData()->login);
     }
 
+    /**
+     * A session follows the account's privileges, not the ones it had at login.
+     *
+     * `isAdminApp` and the rest were copied into the session when the user signed in and never
+     * read again, so revoking somebody's administrator rights left the session they were using an
+     * administrator for as long as they kept using it — the timeout is measured from the last
+     * request. The API rebuilds the user from the row on every request; so does the web now.
+     *
+     * @throws Exception
+     */
+    public function testASessionLosesAdministratorRightsTheAccountNoLongerHas(): void
+    {
+        $this->givenAnInstalledInstance();
+        $this->session->setUserData(new UserDto(id: 7, login: 'admin', isAdminApp: true, isAdminAcc: true));
+
+        $this->userService = $this->createStub(UserService::class);
+        $this->userService->method('getById')->willReturn(
+            new User(['id' => 7, 'login' => 'admin', 'isAdminApp' => false, 'isAdminAcc' => false, 'userGroupId' => 3])
+        );
+
+        $init = $this->buildInitForAFreshSession($session = new Session());
+
+        $init->initialize(IndexController::class);
+
+        self::assertFalse($session->getUserData()->isAdminApp, 'the session kept a revoked administrator');
+        self::assertFalse($session->getUserData()->isAdminAcc);
+        self::assertSame(3, $session->getUserData()->userGroupId, 'the session kept the old group');
+    }
+
+    /**
+     * And the profile as it is now: tightening a profile changes what every holder may do.
+     *
+     * @throws Exception
+     */
+    public function testASessionFollowsTheProfileAsItIsNow(): void
+    {
+        $this->givenAnInstalledInstance();
+        $this->session->setUserData(new UserDto(id: 7, login: 'admin', userProfileId: 5));
+        $this->session->setUserProfile(new ProfileData(['accViewPass' => true]));
+
+        $this->userService = $this->createStub(UserService::class);
+        $this->userService->method('getById')->willReturn(new User(['id' => 7, 'login' => 'admin', 'userProfileId' => 5]));
+
+        $this->userProfileService = $this->createStub(UserProfileService::class);
+        $this->userProfileService->method('getById')->willReturn(
+            (new UserProfile(['id' => 5]))->dehydrate(new ProfileData(['accViewPass' => false]))
+        );
+
+        $init = $this->buildInitForAFreshSession($session = new Session());
+
+        $init->initialize(IndexController::class);
+
+        self::assertFalse($session->getUserProfile()?->isAccViewPass(), 'the session kept the old profile');
+    }
+
+    /**
+     * A profile that cannot be read keeps the one the session has, for the reason a failed user
+     * read does: this runs on every request, and a hiccup must not strip everybody's permissions.
+     *
+     * @throws Exception
+     */
+    public function testAProfileThatCannotBeReadKeepsTheOneTheSessionHas(): void
+    {
+        $this->givenAnInstalledInstance();
+        $this->session->setUserData(new UserDto(id: 7, login: 'admin', userProfileId: 5));
+        $this->session->setUserProfile(new ProfileData(['accViewPass' => true]));
+
+        $this->userService = $this->createStub(UserService::class);
+        $this->userService->method('getById')->willReturn(new User(['id' => 7, 'login' => 'admin', 'userProfileId' => 5]));
+
+        $this->userProfileService = $this->createStub(UserProfileService::class);
+        $this->userProfileService->method('getById')->willThrowException(new RuntimeException('down'));
+
+        $init = $this->buildInitForAFreshSession($session = new Session());
+
+        $init->initialize(IndexController::class);
+
+        self::assertTrue($session->getUserProfile()?->isAccViewPass());
+    }
+
     /**
      * A read that fails leaves the session alone, deliberately.
      *
@@ -406,7 +491,7 @@ private function buildInitForAFreshSession(Session $freshSession): Init
             $this->createStub(LanguageInterface::class),
             $this->itemPresetService,
             $databaseUtil,
-            $this->createStub(UserProfileService::class),
+            $this->userProfileService ?? $this->createStub(UserProfileService::class),
             $uriContext,
             $this->userService,
             $this->sessionKeyService