[knittingcode]

Summary

Disabling a user (flowcrm:user:disable) now deletes every stored session of the user immediately, in the same transaction as the change and its audit event. Before, a session that was not used while the user was disabled became valid again after flowcrm:user:enable.

Motivation

ADR-019 states that "a password change or disabling the user ends them all" (the user's sessions), and the command prints "OK: the user is disabled; every session has ended." In fact the sessions ended only lazily: on each request the firewall compares the stored user with the current one, and disabled is part of that comparison but not of the credential fingerprint. Once the user was enabled again, an unused session matched again.

Reproduced on a production-like stack of main (fe64f7c): two sessions of one user, disable, enable, and the session that made no request in between answered GET /api/v1/me with 200 OK. The session rows stayed in the table after the disable (10 before, 10 after). Impact: if an operator disables a suspected-compromised account and later enables it without a password reset, an idle attacker session (under 30 minutes idle, under 8 hours since login) works again.

Implementation

• App\Identity\Application\UserSessions (new port): endAll(Uuid $userId).
• App\Identity\Infrastructure\Persistence\DbalUserSessions: DELETE FROM sessions WHERE position(convert_to(?, 'UTF8') IN sess_data) > 0 on the default connection, so it runs in the transaction of ManageUsers::disable.
  • The sessions table (PdoSessionHandler) has no user column. A stored session belongs to a user when its data holds the user's ID, which the serialized security token always does (SecurityUser::__serialize). Only logged-in sessions are stored, and each holds its own user's ID and random tokens.
  • A request holding a session row lock (LOCK_TRANSACTIONAL) is waited for, so its write cannot bring the row back afterwards.
• ManageUsers::disable calls it after User::disable() and before the audit record.
• ManageUsers::enable calls it too (second commit, 0d646e6). A login that was in flight during the disable can store its session after the rows were deleted. The per-request check ends that session while the user is disabled, but an enable before its first request would have revived it. No session can be valid when a user is enabled (a disabled user cannot log in), so deleting them again ends nothing legitimate. Raised in review (D, L-26-1).
• The existing per-request check of the disabled flag (SecurityUser::isEqualTo) stays, for a session stored after the disable (for example a login that was already past its checks).
• No schema change, no migration, no new privilege: flowcrm_app already has DELETE on sessions (config/database/runtime-privileges.yaml).

Testing performed

All runs used a disposable Compose project (flowcrm-f3dis, compose.yaml + compose.test.yaml, tmpfs database, torn down afterwards).

• Regression test first, on unchanged main code: AuthenticationTest::testReEnablingAUserDoesNotRestoreTheirSessions failed:

  1) App\Tests\Integration\Security\AuthenticationTest::testReEnablingAUserDoesNotRestoreTheirSessions
  a session from before the disable stays ended
  Failed asserting that 200 is identical to 401.
  Tests: 1, Assertions: 7, Failures: 1.
  

• After the fix:

  • AuthenticationTest|ProvisioningCommandTest|AuditTrailTest: OK (30 tests, 336 assertions).
  • Full integration suite: OK (271 tests, 2325 assertions).
  • bin/console app:database:verify-privileges --runtime: OK: the database roles and runtime privileges match config/database/runtime-privileges.yaml.
  • Negative controls (php tests/Mutation/run-controls.php): 40 of 40 negative controls detected. The two new ones:
    • session revived by enabling the user again -> detected: Tests: 1, Assertions: 7, Failures: 1.
    • disabling one user ends the sessions of others -> detected: Tests: 1, Assertions: 8, Failures: 1.
  • The existing control session survives disabling the user is still detected (Assertions: 6, Failures: 1). It would have gone undetected after the fix, because the disable test no longer reached the per-request check. The test now re-inserts the deleted row (a session stored after the disable) and asserts it ends on its next request.

• Lightweight checks (check-be.sh, the dev api image with this worktree's sources): unit OK (145 tests, 416 assertions), PHPStan [OK] No errors, PHP-CS-Fixer Found 0 of 171 files that can be fixed, Deptrac modules and layers Errors 0, architecture OK (23 tests, 24 assertions).

• Not run locally: the full tests/app/*.sh suites (stack, prodlike, frontend, scan). They run in CI.

• Follow-up commit 0d646e6 (enable also ends sessions), same disposable project:

  • Red first on 5b6897b: AuthenticationTest::testEnablingAUserEndsASessionStoredAfterTheDisable failed with the late session is not revived — Failed asserting that 200 is identical to 401. The test re-inserts the session row after the disable, then enables the user.
  • With the fix: AuthenticationTest OK (20 tests, 233 assertions); full integration suite OK (272 tests, 2333 assertions); verify-privileges --runtime OK; 41 of 41 negative controls detected. The new control a session stored after the disable revived by the enable was detected (Assertions: 6, Failures: 1). The existing session revived by enabling the user again control now searches the disable's call specifically (its old search string occurs twice) and is still detected.
  • Lightweight checks: unit OK (145 tests, 416 assertions), PHPStan no errors, CS-Fixer 0 fixable, Deptrac 0 errors, architecture OK (23 tests, 24 assertions).

Security considerations

• Closes a session-revival gap after an account disable. Login refusal for disabled users is unchanged.
• Scope: only rows whose data contains the disabled user's 36-character UUID are deleted. The new test asserts that another user's session row and its GET /api/v1/me are kept, and a negative control widening the DELETE is detected.
• Runs as the existing runtime role. No new privilege, table, column or configuration.
• Audit: unchanged. user.disabled is written in the same transaction. If the audit write fails, the transaction rolls back and the sessions stay.

Known limitations

• The deletion relies on the user ID being present in the stored session data (the serialized security token). A change to SecurityUser::__serialize that drops the ID must keep a user marker. The new test and control would fail in that case.
• Membership removal still ends no session. The tenant is resolved per request, so a removed member gets 404 on their next request. That is the documented behaviour and is not changed here.
• Password reset and own password change still end the other sessions lazily through the credential fingerprint. Those rows are left for garbage collection; this was already the behaviour and is not a revival risk.
• Docs: docs/dev-environment.md stated 28 controls (it was already out of date at 38); all three documented counts are now the real count. After merging main with fix(auth): match no account for an e-mail address with a NUL byte #28 (1e5e530) it is 43 (38 + fix(auth): match no account for an e-mail address with a NUL byte #28's 2 + these 3); locally 43 of 43 negative controls detected, integration OK (274 tests, 2381 assertions).