From e63b325e08fa72dcdb320d7404fe2c6ad6343b6e Mon Sep 17 00:00:00 2001 From: techmahedy Date: Thu, 5 Nov 2026 17:58:14 +0600 Subject: [PATCH 1/6] feat(session): add a database session driver and logout from other devices --- src/Phaseolies/Auth/Authable.php | 26 +- src/Phaseolies/Auth/Security/Authenticate.php | 201 ++++ .../Security/InteractsWithRememberCookie.php | 14 +- .../Console/Commands/SessionTableCommand.php | 99 ++ .../stubs/session/create_sessions_table.stub | 36 + .../Middleware/AuthenticateSession.php | 35 + .../Session/Contracts/UserSessionStore.php | 23 + src/Phaseolies/Session/SessionManager.php | 69 +- .../Session/Stores/DatabaseStore.php | 857 ++++++++++++++++++ tests/Auth/AuthenticateTest.php | 33 + tests/Auth/LogoutOtherDevicesTest.php | 535 +++++++++++ tests/Console/SessionTableCommandTest.php | 114 +++ tests/Session/DatabaseRotationTest.php | 63 ++ tests/Session/DatabaseStoreMysqlTest.php | 40 + tests/Session/DatabaseStorePgsqlTest.php | 39 + tests/Session/DatabaseStoreTest.php | 513 +++++++++++ tests/Session/RotationTest.php | 5 +- tests/Session/SessionManagerTest.php | 70 ++ 18 files changed, 2765 insertions(+), 7 deletions(-) create mode 100644 src/Phaseolies/Console/Commands/SessionTableCommand.php create mode 100644 src/Phaseolies/Console/Commands/stubs/session/create_sessions_table.stub create mode 100644 src/Phaseolies/Middleware/AuthenticateSession.php create mode 100644 src/Phaseolies/Session/Contracts/UserSessionStore.php create mode 100644 src/Phaseolies/Session/Stores/DatabaseStore.php create mode 100644 tests/Auth/LogoutOtherDevicesTest.php create mode 100644 tests/Console/SessionTableCommandTest.php create mode 100644 tests/Session/DatabaseRotationTest.php create mode 100644 tests/Session/DatabaseStoreMysqlTest.php create mode 100644 tests/Session/DatabaseStorePgsqlTest.php create mode 100644 tests/Session/DatabaseStoreTest.php diff --git a/src/Phaseolies/Auth/Authable.php b/src/Phaseolies/Auth/Authable.php index decf20bb..1e15830c 100644 --- a/src/Phaseolies/Auth/Authable.php +++ b/src/Phaseolies/Auth/Authable.php @@ -40,7 +40,17 @@ public function getAuthIdentifier(): int|string|null */ public function getAuthPassword(): ?string { - return $this->password; + return $this->{$this->getAuthPasswordName()}; + } + + /** + * Get the name of the attribute that holds the password hash. + * + * @return string + */ + public function getAuthPasswordName(): string + { + return 'password'; } /** @@ -50,7 +60,7 @@ public function getAuthPassword(): ?string */ public function getRememberToken(): ?string { - return $this->remember_token; + return $this->{$this->getRememberTokenName()}; } /** @@ -61,7 +71,17 @@ public function getRememberToken(): ?string */ public function setRememberToken(?string $value): void { - $this->remember_token = $value; + $this->{$this->getRememberTokenName()} = $value; + } + + /** + * Get the name of the attribute that holds the remember token. + * + * @return string + */ + public function getRememberTokenName(): string + { + return 'remember_token'; } /** diff --git a/src/Phaseolies/Auth/Security/Authenticate.php b/src/Phaseolies/Auth/Security/Authenticate.php index 4e413fd6..2f09a0d3 100644 --- a/src/Phaseolies/Auth/Security/Authenticate.php +++ b/src/Phaseolies/Auth/Security/Authenticate.php @@ -5,6 +5,9 @@ use Phaseolies\Support\Facades\Hash; use Phaseolies\Support\Facades\Crypt; use Phaseolies\Auth\Authable; +use Phaseolies\Session\Contracts\SessionStoreInterface; +use Phaseolies\Session\Contracts\UserSessionStore; +use Phaseolies\Session\SessionManager; class Authenticate { @@ -116,6 +119,16 @@ protected function getViaRememberKey(): string return 'auth_via_remember_' . $this->actorName; } + /** + * The session key holding the fingerprint of the password the session was signed in with. + * + * @return string + */ + protected function getPasswordFingerprintKey(): string + { + return 'password_hash_' . $this->actorName; + } + /** * The session key used to store a pending 2FA user ID for this actor. * @@ -358,11 +371,22 @@ public function logout(): void $user->save(); } + $this->endSession(); + } + + /** + * Sign this device out, leaving the user's other devices alone. + * + * @return void + */ + protected function endSession(): void + { $this->resolvedUser = null; $this->statelessUser = null; session()->forget($this->getSessionKey()); session()->forget($this->getViaRememberKey()); + session()->forget($this->getPasswordFingerprintKey()); // Only fully invalidate the session on the default actor so that other // actors (e.g. "admin") remain active when only the "web" actor logs out. @@ -376,6 +400,182 @@ public function logout(): void } } + /** + * Sign the user out of every other device, keeping this one signed in. + * + * @param string $password + * @return bool + */ + public function logoutOtherDevices(string $password): bool + { + $user = $this->user(); + + if ($user === null || !Hash::check($password, (string) $user->getAuthPassword())) { + return false; + } + + $changes = [$user->getAuthPasswordName() => Hash::make($password)]; + + $tokenName = $user->getRememberTokenName(); + + if (array_key_exists($tokenName, $user->getAttributes())) { + if (cookie()->has($this->getRememberCookieName())) { + $this->issueRememberToken($user); + } else { + $user->setRememberToken(null); + } + + $changes[$tokenName] = $user->getRememberToken(); + } + + $this->persistCredentials($user, $changes); + + $this->refreshSessionFingerprint(); + + $store = $this->sessionStore(); + $id = $user->getAuthIdentifier(); + + if ($id !== null && $this->tracksUsersIn($store)) { + /** @var UserSessionStore $store */ + $store->destroyUserSessions($id, session()->getId()); + } + + return true; + } + + /** + * Whether this device's session was signed in with the user's current password. + * + * @return bool + */ + public function isSessionCurrent(): bool + { + if (!session()->has($this->getSessionKey())) { + return true; + } + + $user = $this->user(); + + if ($user === null) { + return true; + } + + $stored = session($this->getPasswordFingerprintKey()); + + if (!is_string($stored)) { + $this->refreshSessionFingerprint(); + + return true; + } + + if (hash_equals($this->passwordFingerprint($user), $stored)) { + return true; + } + + $this->endSession(); + + return false; + } + + /** + * Record the current password of the signed-in user in this device's session. + * + * @return void + */ + public function refreshSessionFingerprint(): void + { + $user = $this->resolvedUser; + + if ($user === null || !session()->has($this->getSessionKey())) { + return; + } + + $fingerprint = $this->passwordFingerprint($user); + + if (session($this->getPasswordFingerprintKey()) !== $fingerprint) { + session()->put($this->getPasswordFingerprintKey(), $fingerprint); + } + } + + /** + * The other devices the user is signed in on, most recently active first. + * + * @return array + */ + public function sessions(): array + { + $user = $this->user(); + $store = $this->sessionStore(); + $id = $user?->getAuthIdentifier(); + + if ($id === null || !$this->tracksUsersIn($store)) { + return []; + } + + $current = session()->getId(); + + /** @var UserSessionStore $store */ + return array_map(static fn (array $session): array => [ + 'ip_address' => $session['ip_address'], + 'user_agent' => $session['user_agent'], + 'last_activity' => $session['last_activity'], + 'is_current' => hash_equals($session['id'], $current), + ], $store->userSessions($id)); + } + + /** + * Write credential columns straight to the user's row. + * + * @param Authable $user + * @param array $changes + * @return void + */ + protected function persistCredentials(Authable $user, array $changes): void + { + $user->newQuery() + ->where($user->getKeyName(), $user->getAuthIdentifier()) + ->update($changes); + + foreach ($changes as $attribute => $value) { + $user->{$attribute} = $value; + } + + $user->setOriginalAttributes(array_replace($user->getOriginalAttributes(), $changes)); + } + + /** + * A fingerprint of the user's password hash, safe to keep in the session. + * + * @param Authable $user + * @return string + */ + protected function passwordFingerprint(Authable $user): string + { + return hash_hmac('sha256', (string) $user->getAuthPassword(), (string) config('app.key', '')); + } + + /** + * The store the application's sessions are kept in. + * + * @return SessionStoreInterface + */ + protected function sessionStore(): SessionStoreInterface + { + return SessionManager::store(); + } + + /** + * Whether the store knows which sessions belong to this actor's users. + * + * @param SessionStoreInterface $store + * @return bool + */ + protected function tracksUsersIn(SessionStoreInterface $store): bool + { + return $store instanceof UserSessionStore + && $this->getSessionKey() === SessionManager::userSessionKey(); + } + /** * Set the authenticated user in the session. * @@ -386,6 +586,7 @@ private function setUser(Authable $user): void session()->regenerate(); session()->put($this->getSessionKey(), $user->getAuthIdentifier()); + session()->put($this->getPasswordFingerprintKey(), $this->passwordFingerprint($user)); $this->resolvedUser = $user; } diff --git a/src/Phaseolies/Auth/Security/InteractsWithRememberCookie.php b/src/Phaseolies/Auth/Security/InteractsWithRememberCookie.php index b3d29dc4..1df861e2 100644 --- a/src/Phaseolies/Auth/Security/InteractsWithRememberCookie.php +++ b/src/Phaseolies/Auth/Security/InteractsWithRememberCookie.php @@ -34,10 +34,22 @@ protected function getRememberCookieName(): string * @return void */ private function setRememberToken(Authable $user): void + { + $this->issueRememberToken($user); + + $user->save(); + } + + /** + * Give the user a new remember token and send it to this device, without saving the user. + * + * @param Authable $user + * @return void + */ + private function issueRememberToken(Authable $user): void { $token = bin2hex(random_bytes(32)); $user->setRememberToken(Hash::make($token)); - $user->save(); $id = $user->getAuthIdentifier(); $cookieValue = $id . '|' . $token . '|' . Hash::make($id . $token); diff --git a/src/Phaseolies/Console/Commands/SessionTableCommand.php b/src/Phaseolies/Console/Commands/SessionTableCommand.php new file mode 100644 index 00000000..5b5a2b7b --- /dev/null +++ b/src/Phaseolies/Console/Commands/SessionTableCommand.php @@ -0,0 +1,99 @@ +executeWithTiming(function (): int { + $table = (string) SessionManager::config()['table']; + + if (preg_match('/^[A-Za-z_][A-Za-z0-9_]*(\.[A-Za-z_][A-Za-z0-9_]*)?\z/', $table) !== 1) { + $this->displayError("Invalid session table name [{$table}]."); + + return Command::FAILURE; + } + + $directory = $this->migrationPath(); + $name = 'create_' . str_replace('.', '_', $table) . '_table'; + + if ($existing = $this->existingMigration($directory, $name)) { + $this->displayError('A migration for the session table already exists.'); + $this->line("📁 File: {$existing}"); + + return Command::FAILURE; + } + + if (!is_dir($directory) && !@mkdir($directory, 0755, true) && !is_dir($directory)) { + $this->displayError("Unable to create the migration directory [{$directory}]."); + + return Command::FAILURE; + } + + $file = $directory . '/' . date('Y_m_d_His') . '_' . $name . '.php'; + $stub = (string) file_get_contents(__DIR__ . '/stubs/session/create_sessions_table.stub'); + + file_put_contents($file, str_replace('{{ table }}', $table, $stub)); + + $this->displaySuccess('Session table migration created successfully.'); + $this->line("📁 File: {$file}"); + + return Command::SUCCESS; + }); + } + + /** + * Path of the application's migrations. + * + * @return string + */ + protected function migrationPath(): string + { + return base_path('schema/migrations'); + } + + /** + * An existing migration with the same name, if any. + * + * @param string $directory + * @param string $name + * @return string|null + */ + protected function existingMigration(string $directory, string $name): ?string + { + if (!is_dir($directory)) { + return null; + } + + foreach (new \FilesystemIterator($directory, \FilesystemIterator::SKIP_DOTS) as $file) { + if ($file->isFile() && str_ends_with($file->getFilename(), '_' . $name . '.php')) { + return $file->getPathname(); + } + } + + return null; + } +} diff --git a/src/Phaseolies/Console/Commands/stubs/session/create_sessions_table.stub b/src/Phaseolies/Console/Commands/stubs/session/create_sessions_table.stub new file mode 100644 index 00000000..3f78f945 --- /dev/null +++ b/src/Phaseolies/Console/Commands/stubs/session/create_sessions_table.stub @@ -0,0 +1,36 @@ +string('id', 128)->primary(); + // For UUID or ULID user keys, use: $table->string('user_id', 36)->nullable()->index(); + $table->unsignedBigInteger('user_id')->nullable()->index(); + $table->string('ip_address', 45)->nullable(); + $table->text('user_agent')->nullable(); + $table->longText('payload'); + $table->unsignedInteger('last_activity')->index(); + }); + } + + /** + * Reverse the migrations + * + * @return void + */ + public function down(): void + { + Schema::dropIfExists('{{ table }}'); + } +}; diff --git a/src/Phaseolies/Middleware/AuthenticateSession.php b/src/Phaseolies/Middleware/AuthenticateSession.php new file mode 100644 index 00000000..022f2f62 --- /dev/null +++ b/src/Phaseolies/Middleware/AuthenticateSession.php @@ -0,0 +1,35 @@ +actor($actor ?: null); + + // A stale session is signed out here; the request goes on as a guest, + // so the "auth" middleware of a protected route sends it to sign in. + $auth->isSessionCurrent(); + + $response = $next($request); + + // Keep this device signed in when this request changed the password. + $auth->refreshSessionFingerprint(); + + return $response; + } +} diff --git a/src/Phaseolies/Session/Contracts/UserSessionStore.php b/src/Phaseolies/Session/Contracts/UserSessionStore.php new file mode 100644 index 00000000..e304d85f --- /dev/null +++ b/src/Phaseolies/Session/Contracts/UserSessionStore.php @@ -0,0 +1,23 @@ + + */ + public function userSessions(int|string $userId): array; + + /** + * End every session of a user, except the one given. + * + * @param int|string $userId + * @param string|null $exceptId + * @return int Number of ended sessions + */ + public function destroyUserSessions(int|string $userId, ?string $exceptId = null): int; +} diff --git a/src/Phaseolies/Session/SessionManager.php b/src/Phaseolies/Session/SessionManager.php index 7e266a72..f9eb90b2 100644 --- a/src/Phaseolies/Session/SessionManager.php +++ b/src/Phaseolies/Session/SessionManager.php @@ -3,10 +3,13 @@ namespace Phaseolies\Session; use Closure; +use PDO; +use Phaseolies\Database\Database; use Phaseolies\Session\Contracts\SessionStoreInterface; use Phaseolies\Session\Exceptions\SessionException; use Phaseolies\Session\Stores\ArrayStore; use Phaseolies\Session\Stores\CookieStore; +use Phaseolies\Session\Stores\DatabaseStore; use Phaseolies\Session\Stores\FileStore; use Phaseolies\Support\Contracts\EncryptionInterface; @@ -22,6 +25,8 @@ class SessionManager 'lifetime' => 120, 'expire_on_close' => false, 'files' => null, + 'connection' => null, + 'table' => 'sessions', 'cookie' => 'doppar_session', 'path' => '/', 'domain' => null, @@ -179,7 +184,69 @@ public static function createStore(array $config): SessionStoreInterface 'file' => new FileStore($config['files'], $lifetime), 'cookie' => new CookieStore(app(EncryptionInterface::class), $lifetime), 'array' => new ArrayStore($lifetime), - default => throw new SessionException('Unsupported session driver. Supported drivers: file, cookie, array.'), + 'database' => new DatabaseStore( + self::connectionResolver($config['connection']), + (string) $config['table'], + $lifetime, + self::userResolver(), + self::clientResolver() + ), + default => throw new SessionException('Unsupported session driver. Supported drivers: file, cookie, database, array.'), + }; + } + + /** + * Resolve the database connection lazily, so nothing connects until a session is used. + * + * @param mixed $connection + * @return Closure(): PDO + */ + protected static function connectionResolver(mixed $connection): Closure + { + $name = is_string($connection) && $connection !== '' ? $connection : null; + + return static fn (): PDO => Database::getPdoInstance($name); + } + + /** + * Find the user a session belongs to: the one the default auth actor logged in. + * + * @return Closure(array): mixed|null + */ + public static function userResolver(): ?Closure + { + $key = self::userSessionKey(); + + return $key === null ? null : static fn (array $data): mixed => $data[$key] ?? null; + } + + /** + * The session key under which the default auth actor keeps the user id. + * + * @return string|null + */ + public static function userSessionKey(): ?string + { + $actor = (string) config('auth.default', 'web'); + $key = config("auth.actors.{$actor}.session_key"); + + return is_string($key) && $key !== '' ? $key : null; + } + + /** + * Describe the client a session is saved for. + * + * @return Closure(): array{ip_address: string|null, user_agent: string|null} + */ + protected static function clientResolver(): Closure + { + return static function (): array { + $request = app('request'); + + return [ + 'ip_address' => $request->ip(), + 'user_agent' => $request->userAgent(), + ]; }; } } diff --git a/src/Phaseolies/Session/Stores/DatabaseStore.php b/src/Phaseolies/Session/Stores/DatabaseStore.php new file mode 100644 index 00000000..cf76626f --- /dev/null +++ b/src/Phaseolies/Session/Stores/DatabaseStore.php @@ -0,0 +1,857 @@ + + */ + private array $statements = []; + + /** + * Last activity of the sessions read or written by this process. + * + * @var array + */ + private array $activity = []; + + /** + * Sessions this instance holds the lock of right now. + * + * @var array + */ + private array $held = []; + + /** + * Whether the missing lock support of the driver was already reported. + */ + private bool $reportedNoLocking = false; + + /** + * The quoted table name. + */ + private string $quotedTable = ''; + + /** + * The SQLite connection's own busy timeout, restored after taking a lock. + */ + private ?int $sqliteBusyTimeout = null; + + /** + * @param Closure(): PDO $connection Resolves the connection to use + * @param string $table + * @param int $lifetime Seconds + * @param Closure(array): (int|string|null)|null $userResolver + * @param Closure(): array{ip_address?: string|null, user_agent?: string|null}|null $metadataResolver + * @param float $lockTimeout + * @throws SessionException + */ + public function __construct( + protected Closure $connection, + protected string $table = 'sessions', + protected int $lifetime = 7200, + protected ?Closure $userResolver = null, + protected ?Closure $metadataResolver = null, + protected float $lockTimeout = 2.0 + ) { + if (preg_match('/^[A-Za-z_][A-Za-z0-9_]*(\.[A-Za-z_][A-Za-z0-9_]*)?\z/', $table) !== 1) { + throw new SessionException("Invalid session table name [{$table}]."); + } + } + + /** + * Load a session from the value of the session cookie + * + * @param string $cookieValue + * @return array{id: string, data: array}|null + * @throws SessionException + */ + public function read(string $cookieValue): ?array + { + if (!FileStore::isValidId($cookieValue)) { + return null; + } + + $row = $this->attempt('read', function () use ($cookieValue): array|false { + $statement = $this->execute( + "SELECT payload, last_activity FROM {$this->table()} WHERE id = ?", + [$cookieValue] + ); + + $row = $statement->fetch(PDO::FETCH_NUM); + $statement->closeCursor(); + + return $row; + }); + + if ($row === false) { + unset($this->activity[$cookieValue]); + + return null; + } + + [$payload, $lastActivity] = [(string) $row[0], (int) $row[1]]; + + if ($lastActivity + $this->lifetime < time()) { + // Guarded by the activity time, so a session another request has + // just refreshed is not deleted by this one. + $this->attempt('read', fn() => $this->execute( + "DELETE FROM {$this->table()} WHERE id = ? AND last_activity = ?", + [$cookieValue, $lastActivity] + )); + + unset($this->activity[$cookieValue]); + + return null; + } + + $data = $this->decode($payload); + + if ($data === null) { + return null; + } + + $this->remember($cookieValue, $lastActivity); + + return ['id' => $cookieValue, 'data' => $data]; + } + + /** + * Persist a session and return the value the session cookie must carry + * + * @param string $id + * @param array $data + * @return string + * @throws SessionException + */ + public function write(string $id, array $data): string + { + if (!FileStore::isValidId($id)) { + throw new SessionException('Invalid session id.'); + } + + $now = time(); + $metadata = $this->metadata(); + + $this->attempt('write', fn() => $this->upsert([ + $id, + $this->userOf($data), + $metadata['ip_address'], + $metadata['user_agent'], + base64_encode(serialize($data)), + $now, + ])); + + $this->remember($id, $now); + + return $id; + } + + /** + * Refresh the lifetime of a session without rewriting its data + * + * @param string $id + * @return void + * @throws SessionException + */ + public function touch(string $id): void + { + if (!FileStore::isValidId($id)) { + return; + } + + $now = time(); + $threshold = min(60, intdiv($this->lifetime, 10)); + + // Expiry only needs minute precision: skip the write while it is fresh. + if (isset($this->activity[$id]) && $now - $this->activity[$id] <= $threshold) { + return; + } + + $this->attempt('touch', fn() => $this->execute( + "UPDATE {$this->table()} SET last_activity = ? WHERE id = ? AND last_activity < ?", + [$now, $id, $now - $threshold] + )); + + $this->remember($id, $now); + } + + /** + * Remove a session + * + * @param string $id + * @return void + * @throws SessionException + */ + public function destroy(string $id): void + { + if (!FileStore::isValidId($id)) { + return; + } + + // Under the lock, so a request that is saving this session right now + // finishes first and then finds it gone instead of writing it back. + $this->synchronized($id, function () use ($id): void { + $this->attempt('destroy', fn() => $this->execute( + "DELETE FROM {$this->table()} WHERE id = ?", + [$id] + )); + }); + + unset($this->activity[$id]); + } + + /** + * Remove every expired session + * + * @param int $maxLifetime + * @return int + * @throws SessionException + */ + public function gc(int $maxLifetime): int + { + return $this->attempt('gc', fn() => $this->execute( + "DELETE FROM {$this->table()} WHERE last_activity <= ?", + [time() - $maxLifetime] + )->rowCount()); + } + + /** + * Remove expired sessions, doing only a bounded slice of the work + * + * @param int $maxLifetime + * @return int + * @throws SessionException + */ + public function sweep(int $maxLifetime): int + { + return $this->attempt('sweep', function () use ($maxLifetime): int { + $table = $this->table(); + $limit = self::SWEEP_LIMIT; + + $sql = $this->driver() === 'mysql' + ? "DELETE FROM {$table} WHERE last_activity <= ? LIMIT {$limit}" + : "DELETE FROM {$table} WHERE id IN (SELECT id FROM {$table} WHERE last_activity <= ? LIMIT {$limit})"; + + return $this->execute($sql, [time() - $maxLifetime])->rowCount(); + }); + } + + /** + * Remove every stored session + * + * @return int + * @throws SessionException + */ + public function flush(): int + { + $this->activity = []; + + return $this->attempt('flush', fn() => $this->execute("DELETE FROM {$this->table()}")->rowCount()); + } + + /** + * Whether the session data lives in the cookie instead of on the server + * + * @return bool + */ + public function isClientSide(): bool + { + return false; + } + + /** + * The live sessions of a user, most recently active first + * + * @param int|string $userId + * @return array + * @throws SessionException + */ + public function userSessions(int|string $userId): array + { + $rows = $this->attempt('list', function () use ($userId): array { + $statement = $this->execute( + "SELECT id, ip_address, user_agent, last_activity FROM {$this->table()} WHERE user_id = ? AND last_activity >= ? ORDER BY last_activity DESC", + [$userId, time() - $this->lifetime] + ); + + $rows = $statement->fetchAll(PDO::FETCH_ASSOC); + $statement->closeCursor(); + + return $rows; + }); + + return array_map(static fn(array $row): array => [ + 'id' => (string) $row['id'], + 'ip_address' => $row['ip_address'] === null ? null : (string) $row['ip_address'], + 'user_agent' => $row['user_agent'] === null ? null : (string) $row['user_agent'], + 'last_activity' => (int) $row['last_activity'], + ], $rows); + } + + /** + * End every session of a user, except the one given + * + * @param int|string $userId + * @param string|null $exceptId + * @return int + * @throws SessionException + */ + public function destroyUserSessions(int|string $userId, ?string $exceptId = null): int + { + $ids = $this->attempt('destroy', function () use ($userId): array { + $statement = $this->execute("SELECT id FROM {$this->table()} WHERE user_id = ?", [$userId]); + + $ids = $statement->fetchAll(PDO::FETCH_COLUMN); + $statement->closeCursor(); + + return $ids; + }); + + $ended = 0; + + // One by one, each under its own lock: a single bulk DELETE could race + // with a request of that device that is saving its session right now, + // which would write the session back after it was deleted. + foreach ($ids as $id) { + if ($id !== $exceptId) { + $this->destroy((string) $id); + $ended++; + } + } + + return $ended; + } + + /** + * Run a callback while no other request can save or destroy the same session + * + * @param string $id + * @param callable $callback + * @return mixed + */ + public function synchronized(string $id, callable $callback): mixed + { + if (!FileStore::isValidId($id) || isset($this->held[$id])) { + return $callback(); + } + + try { + $driver = $this->driver(); + } catch (\Throwable) { + return $callback(); + } + + return match ($driver) { + 'mysql' => $this->withMysqlLock($id, $callback), + 'pgsql' => $this->withPgsqlLock($id, $callback), + 'sqlite' => $this->withSqliteLock($id, $callback), + default => $this->withoutLock($driver, $callback), + }; + } + + /** + * Run a callback under a MySQL/MariaDB named lock. + * + * @param string $id + * @param callable $callback + * @return mixed + */ + protected function withMysqlLock(string $id, callable $callback): mixed + { + $name = 'doppar_session_' . $id; + + try { + $statement = $this->execute('SELECT GET_LOCK(?, ?)', [$name, (int) ceil($this->lockTimeout)]); + $acquired = $statement->fetchColumn(); + $statement->closeCursor(); + } catch (PDOException) { + $acquired = null; + } + + if ((string) $acquired !== '1') { + $this->reportLockTimeout(); + + return $callback(); + } + + $this->held[$id] = true; + + try { + return $callback(); + } finally { + unset($this->held[$id]); + + $this->quietly(function () use ($name): void { + $this->execute('SELECT RELEASE_LOCK(?)', [$name])->closeCursor(); + }); + } + } + + /** + * Run a callback under a PostgreSQL session-level advisory lock. + * + * @param string $id + * @param callable $callback + * @return mixed + */ + protected function withPgsqlLock(string $id, callable $callback): mixed + { + $keys = [self::int32(crc32(self::PGSQL_LOCK_NAMESPACE)), self::int32((int) hexdec(substr($id, 0, 8)))]; + $deadline = microtime(true) + $this->lockTimeout; + + while (true) { + try { + $statement = $this->execute('SELECT pg_try_advisory_lock(CAST(? AS integer), CAST(? AS integer))', $keys); + $acquired = $statement->fetchColumn(); + $statement->closeCursor(); + } catch (PDOException) { + $this->reportLockTimeout(); + + return $callback(); + } + + if (self::isTrue($acquired)) { + break; + } + + if (microtime(true) > $deadline) { + $this->reportLockTimeout(); + + return $callback(); + } + + usleep(5000); + } + + $this->held[$id] = true; + + try { + return $callback(); + } finally { + unset($this->held[$id]); + + $this->quietly(function () use ($keys): void { + $this->execute('SELECT pg_advisory_unlock(CAST(? AS integer), CAST(? AS integer))', $keys)->closeCursor(); + }); + } + } + + /** + * Run a callback inside an immediate (write-locking) SQLite transaction. + * + * @param string $id + * @param callable $callback + * @return mixed + */ + protected function withSqliteLock(string $id, callable $callback): mixed + { + $pdo = $this->pdo(); + + // A transaction of the application is already open on this connection: + // SQLite cannot nest one, and that transaction already holds the writes. + if ($pdo->inTransaction()) { + return $callback(); + } + + try { + // Wait for the write lock no longer than the lock timeout, then put + // the connection's own busy timeout back for the application. + $original = $this->sqliteBusyTimeout ??= (int) $pdo->query('PRAGMA busy_timeout')->fetchColumn(); + $pdo->exec('PRAGMA busy_timeout = ' . max(1, (int) round($this->lockTimeout * 1000))); + + try { + $pdo->exec('BEGIN IMMEDIATE'); + } finally { + $pdo->exec('PRAGMA busy_timeout = ' . $original); + } + } catch (PDOException) { + $this->reportLockTimeout(); + + return $callback(); + } + + $this->held[$id] = true; + + try { + $result = $callback(); + $pdo->exec('COMMIT'); + + return $result; + } catch (\Throwable $e) { + $this->quietly(fn() => $pdo->exec('ROLLBACK')); + + throw $e; + } finally { + unset($this->held[$id]); + } + } + + /** + * Run a callback without a lock, on a driver that offers none. + * + * @param string $driver + * @param callable $callback + * @return mixed + */ + protected function withoutLock(string $driver, callable $callback): mixed + { + if (!$this->reportedNoLocking) { + $this->reportedNoLocking = true; + + $this->report("Session locking is not supported for the [{$driver}] driver. Saving without a lock, so concurrent changes to a session may be lost."); + } + + return $callback(); + } + + /** + * Insert the session, or replace it when it exists. + * + * @param array $values id, user_id, ip_address, user_agent, payload, last_activity + * @return void + */ + protected function upsert(array $values): void + { + $table = $this->table(); + $columns = 'id, user_id, ip_address, user_agent, payload, last_activity'; + + switch ($this->driver()) { + case 'mysql': + $this->execute( + "INSERT INTO {$table} ({$columns}) VALUES (?, ?, ?, ?, ?, ?) ON DUPLICATE KEY UPDATE " + . 'user_id = VALUES(user_id), ip_address = VALUES(ip_address), user_agent = VALUES(user_agent), ' + . 'payload = VALUES(payload), last_activity = VALUES(last_activity)', + $values + ); + + return; + + case 'pgsql': + case 'sqlite': + $this->execute( + "INSERT INTO {$table} ({$columns}) VALUES (?, ?, ?, ?, ?, ?) ON CONFLICT (id) DO UPDATE SET " + . 'user_id = excluded.user_id, ip_address = excluded.ip_address, user_agent = excluded.user_agent, ' + . 'payload = excluded.payload, last_activity = excluded.last_activity', + $values + ); + + return; + } + + // Portable fallback for other drivers. + [$id, $userId, $ip, $agent, $payload, $time] = $values; + + $updated = $this->execute( + "UPDATE {$table} SET user_id = ?, ip_address = ?, user_agent = ?, payload = ?, last_activity = ? WHERE id = ?", + [$userId, $ip, $agent, $payload, $time, $id] + )->rowCount(); + + if ($updated === 0) { + $this->execute("INSERT INTO {$table} ({$columns}) VALUES (?, ?, ?, ?, ?, ?)", $values); + } + } + + /** + * Execute a prepared statement, reusing it for the same SQL. + * + * @param string $sql + * @param array $bindings + * @return PDOStatement + */ + protected function execute(string $sql, array $bindings = []): PDOStatement + { + $pdo = $this->pdo(); + $statement = $this->statements[$sql] ??= $pdo->prepare($sql); + + foreach (array_values($bindings) as $index => $value) { + $statement->bindValue($index + 1, $value, match (true) { + is_int($value) => PDO::PARAM_INT, + is_bool($value) => PDO::PARAM_BOOL, + $value === null => PDO::PARAM_NULL, + default => PDO::PARAM_STR, + }); + } + + $statement->execute(); + + return $statement; + } + + /** + * The connection, resetting the prepared statements when it changed. + * + * @return PDO + */ + protected function pdo(): PDO + { + $pdo = ($this->connection)(); + + if ($pdo !== $this->pdo) { + $this->pdo = $pdo; + $this->statements = []; + $this->driver = (string) $pdo->getAttribute(PDO::ATTR_DRIVER_NAME); + $this->quotedTable = ''; + $this->sqliteBusyTimeout = null; + } + + return $pdo; + } + + /** + * Name of the driver of the connection. + * + * @return string + */ + protected function driver(): string + { + $this->pdo(); + + return $this->driver; + } + + /** + * The table name, quoted for the driver. + * + * @return string + */ + protected function table(): string + { + if ($this->quotedTable !== '') { + return $this->quotedTable; + } + + $quote = $this->driver() === 'mysql' ? '`' : '"'; + + return $this->quotedTable = implode('.', array_map( + static fn(string $part): string => $quote . $part . $quote, + explode('.', $this->table) + )); + } + + /** + * Run a database operation, explaining a failure. + * + * @template T + * @param string $operation + * @param callable(): T $callback + * @return T + * @throws SessionException + */ + protected function attempt(string $operation, callable $callback): mixed + { + try { + return $callback(); + } catch (PDOException $e) { + throw new SessionException(sprintf( + 'Unable to %s the session in table [%s]: %s. If the table does not exist, run "php pool session:table" and migrate.', + $operation, + $this->table, + $e->getMessage() + ), 0, $e); + } + } + + /** + * Decode a stored payload, or null when it is not a session. + * + * @param string $payload + * @return array|null + */ + protected function decode(string $payload): ?array + { + $serialized = base64_decode($payload, true); + + if ($serialized === false || $serialized === '') { + return null; + } + + $data = @unserialize($serialized); + + return is_array($data) ? $data : null; + } + + /** + * The user a session belongs to. + * + * @param array $data + * @return int|string|null + */ + protected function userOf(array $data): int|string|null + { + if ($this->userResolver === null) { + return null; + } + + $user = ($this->userResolver)($data); + + return is_int($user) || (is_string($user) && $user !== '') ? $user : null; + } + + /** + * The client the session is being saved for. + * + * @return array{ip_address: string|null, user_agent: string|null} + */ + protected function metadata(): array + { + $metadata = []; + + if ($this->metadataResolver !== null) { + try { + $metadata = ($this->metadataResolver)(); + } catch (\Throwable) { + } + } + + $ip = $metadata['ip_address'] ?? null; + $agent = $metadata['user_agent'] ?? null; + + return [ + 'ip_address' => is_string($ip) && $ip !== '' ? substr($ip, 0, 45) : null, + 'user_agent' => is_string($agent) ? self::printable($agent, self::MAX_USER_AGENT) : null, + ]; + } + + /** + * Reduce client-supplied text to printable ASCII of a bounded length. + * + * @param string $value + * @param int $length + * @return string|null + */ + private static function printable(string $value, int $length): ?string + { + $value = substr((string) preg_replace('/[^\x20-\x7E]/', '?', $value), 0, $length); + + return $value === '' ? null : $value; + } + + /** + * Remember the last activity of a session, bounded for long-running workers. + * + * @param string $id + * @param int $time + * @return void + */ + protected function remember(string $id, int $time): void + { + if (count($this->activity) >= self::MAX_TRACKED && !isset($this->activity[$id])) { + $this->activity = []; + } + + $this->activity[$id] = $time; + } + + /** + * Run a cleanup step whose failure must not hide the original outcome. + * + * @param callable $callback + * @return void + */ + protected function quietly(callable $callback): void + { + try { + $callback(); + } catch (\Throwable) { + } + } + + /** + * Report that a session lock could not be taken. + * + * @return void + */ + protected function reportLockTimeout(): void + { + $this->report(sprintf( + 'Session lock was not acquired within %s seconds. Saving without a lock, so concurrent changes to this session may be lost.', + rtrim(rtrim(number_format($this->lockTimeout, 3, '.', ''), '0'), '.') + )); + } + + /** + * Report a condition that does not stop the request but should not go unnoticed. + * + * @param string $message + * @return void + */ + protected function report(string $message): void + { + try { + if (function_exists('warning')) { + warning('[Doppar] ' . $message); + } + } catch (\Throwable) { + } + } + + /** + * Whether a boolean column came back true: drivers return true, 1, "1" or "t". + * + * @param mixed $value + * @return bool + */ + private static function isTrue(mixed $value): bool + { + if (is_bool($value)) { + return $value; + } + + return in_array(is_scalar($value) ? strtolower((string) $value) : '', ['1', 't', 'true'], true); + } + + /** + * Interpret an unsigned 32-bit value as a signed PostgreSQL integer. + * + * @param int $value + * @return int + */ + private static function int32(int $value): int + { + $value &= 0xFFFFFFFF; + + return $value > 0x7FFFFFFF ? $value - 0x100000000 : $value; + } +} diff --git a/tests/Auth/AuthenticateTest.php b/tests/Auth/AuthenticateTest.php index 828594cd..adcb81b0 100644 --- a/tests/Auth/AuthenticateTest.php +++ b/tests/Auth/AuthenticateTest.php @@ -33,8 +33,22 @@ public function regenerate(bool $deleteOldSession = true): void } } + /* + * These stubs stand in for the global helpers while AuthenticateTest runs. + * Once this file is loaded they shadow the helpers for every later test that + * uses Authenticate, so outside AuthenticateTest they defer to the real ones. + */ + function authenticateStubsActive(): bool + { + return !empty($GLOBALS['authenticateStubsActive']); + } + function session(?string $key = null, mixed $default = null): mixed { + if (!authenticateStubsActive()) { + return \session($key, $default); + } + global $authenticateSessionStore; $authenticateSessionStore ??= new TestSessionStore(); @@ -48,6 +62,10 @@ function session(?string $key = null, mixed $default = null): mixed function now(): object { + if (!authenticateStubsActive()) { + return \now(); + } + return new class { public function addMinutes(int $minutes): object { @@ -60,6 +78,10 @@ public function addMinutes(int $minutes): object function cookie(): object { + if (!authenticateStubsActive()) { + return \cookie(); + } + return new class { public function has(string $key): bool { @@ -70,6 +92,10 @@ public function has(string $key): bool function request(): object { + if (!authenticateStubsActive()) { + return \request(); + } + return new class { public function isApiRequest(): bool { @@ -131,10 +157,17 @@ public function user(): ?Authable class AuthenticateTest extends TestCase { + protected function tearDown(): void + { + $GLOBALS['authenticateStubsActive'] = false; + } + protected function setUp(): void { global $authenticateSessionStore; + $GLOBALS['authenticateStubsActive'] = true; + $authenticateSessionStore = new \Phaseolies\Auth\Security\TestSessionStore(); FakeAuthableModel::$resolvedUser = null; } diff --git a/tests/Auth/LogoutOtherDevicesTest.php b/tests/Auth/LogoutOtherDevicesTest.php new file mode 100644 index 00000000..f7526a02 --- /dev/null +++ b/tests/Auth/LogoutOtherDevicesTest.php @@ -0,0 +1,535 @@ + + */ + public static array $sent = []; + + protected function setRememberCookie(string $value): void + { + self::$sent[$this->getRememberCookieName()] = \Phaseolies\Support\Facades\Crypt::encrypt($value); + } +} + +class CookieRecordingActorManager extends ActorManager +{ + protected function resolve(string $name): Authenticate + { + return new CookieRecordingAuthenticate($name, config("auth.actors.{$name}")); + } +} + +/** + * A model whose $creatable list leaves the credential columns out. + */ +class LockedDownUser extends Authable +{ + protected $table = 'users'; + + protected $timeStamps = false; + + protected $creatable = ['email']; +} + +/** + * Signs one user in on several devices, each a separate request with its own + * session cookie, and checks what logoutOtherDevices() leaves signed in. + */ +class LogoutOtherDevicesTest extends TestCase +{ + private const CONNECTION = 'logout_devices_test'; + + private const COOKIE = 'test_session'; + + private const PASSWORD = 'correct horse battery staple'; + + private string $database; + + private SessionStoreInterface $store; + + private array $sessionConfig = [ + 'cookie' => self::COOKIE, + 'lifetime' => 120, + 'stateless' => [], + 'lottery' => [0, 100], + 'secure' => false, + ]; + + protected function setUp(): void + { + $this->database = sys_get_temp_dir() . '/doppar_devices_' . bin2hex(random_bytes(6)) . '.sqlite'; + + $container = new MockContainer(); + Container::setInstance($container); + $this->resetConfig(); + + Config::set('app.key', 'base64:ImGTGoQ6ZhM7yBlMvp41ejp0nt8juImy1aIbf6shQCI='); + Config::set('database.connections.' . self::CONNECTION, ['driver' => 'sqlite', 'database' => $this->database, 'prefix' => '']); + Config::set('database.default', self::CONNECTION); + Config::set('hashing', ['driver' => 'bcrypt', 'bcrypt' => ['rounds' => 4]]); + Config::set('auth', [ + 'default' => 'web', + 'actors' => [ + 'web' => ['model' => DeviceUser::class, 'session_key' => 'user_id'], + 'admin' => ['model' => DeviceUser::class, 'session_key' => 'admin_id'], + ], + ]); + + $container->bind('db', fn () => new Database()); + $container->singleton('hash', PasswordHashing::class); + $container->singleton('crypt', Encryption::class); + $container->singleton('cookie', CookieJar::class); + $container->singleton('auth', CookieRecordingActorManager::class); + CookieRecordingAuthenticate::$sent = []; + + if (method_exists(Facade::class, 'clearResolvedInstances')) { + Facade::clearResolvedInstances(); + } + + $pdo = Database::getPdoInstance(self::CONNECTION); + $pdo->exec('CREATE TABLE users (id INTEGER PRIMARY KEY, email TEXT, password TEXT, remember_token TEXT NULL, two_factor_secret TEXT NULL)'); + $pdo->exec( + 'CREATE TABLE sessions (id VARCHAR(128) PRIMARY KEY, user_id INTEGER NULL, ip_address VARCHAR(45) NULL, ' + . 'user_agent TEXT NULL, payload TEXT NOT NULL, last_activity INTEGER NOT NULL)' + ); + $pdo->prepare('INSERT INTO users (id, email, password) VALUES (?, ?, ?)') + ->execute([1, 'ada@example.com', password_hash(self::PASSWORD, PASSWORD_BCRYPT, ['cost' => 4])]); + + $this->useStore($this->databaseStore()); + + $_COOKIE = []; + } + + protected function tearDown(): void + { + $_COOKIE = []; + + SessionManager::useStore(null); + + foreach (['connections', 'drivers', 'transactions'] as $property) { + (new \ReflectionClass(Database::class))->getProperty($property)->setValue(null, []); + } + + $this->resetConfig(); + (new \ReflectionClass(Container::class))->getProperty('instance')->setValue(null, null); + + @unlink($this->database); + } + + public function test_it_signs_out_every_other_device_and_keeps_this_one(): void + { + $laptop = $this->signIn('Laptop'); + $phone = $this->signIn('Phone'); + $tablet = $this->signIn('Tablet'); + + [$laptop, $result] = $this->onDevice($laptop, fn (Authenticate $auth) => $auth->logoutOtherDevices(self::PASSWORD)); + + $this->assertTrue($result); + $this->assertTrue($this->isSignedIn($laptop)); + $this->assertFalse($this->isSignedIn($phone)); + $this->assertFalse($this->isSignedIn($tablet)); + $this->assertSame(1, $this->sessionRows(), 'Only the current device keeps a stored session.'); + } + + public function test_the_password_still_works_after_it_is_rehashed(): void + { + $laptop = $this->signIn('Laptop'); + $before = $this->storedPassword(); + + $this->onDevice($laptop, fn (Authenticate $auth) => $auth->logoutOtherDevices(self::PASSWORD)); + + $this->assertNotSame($before, $this->storedPassword()); + $this->assertTrue(password_verify(self::PASSWORD, $this->storedPassword())); + } + + public function test_a_wrong_password_changes_nothing(): void + { + $laptop = $this->signIn('Laptop'); + $phone = $this->signIn('Phone'); + $before = $this->storedPassword(); + + [, $result] = $this->onDevice($laptop, fn (Authenticate $auth) => $auth->logoutOtherDevices('wrong password')); + + $this->assertFalse($result); + $this->assertSame($before, $this->storedPassword()); + $this->assertTrue($this->isSignedIn($phone)); + } + + public function test_it_does_nothing_for_a_guest(): void + { + [, $result] = $this->onDevice(null, fn (Authenticate $auth) => $auth->logoutOtherDevices(self::PASSWORD)); + + $this->assertFalse($result); + } + + public function test_with_a_store_that_does_not_track_users_the_fingerprint_signs_other_devices_out(): void + { + $this->useStore(new ArrayStore(7200)); + + $laptop = $this->signIn('Laptop'); + $phone = $this->signIn('Phone'); + + $this->onDevice($laptop, fn (Authenticate $auth) => $auth->logoutOtherDevices(self::PASSWORD)); + + // The phone's session still exists, but was signed in with the old password. + [, $phoneCurrent] = $this->onDevice($phone, fn (Authenticate $auth) => $auth->isSessionCurrent()); + [, $laptopCurrent] = $this->onDevice($laptop, fn (Authenticate $auth) => $auth->isSessionCurrent()); + + $this->assertFalse($phoneCurrent); + $this->assertTrue($laptopCurrent); + $this->assertFalse($this->isSignedIn($phone), 'A stale session stays signed out.'); + $this->assertTrue($this->isSignedIn($laptop)); + } + + public function test_the_middleware_signs_a_stale_session_out_before_the_request_runs(): void + { + $this->useStore(new ArrayStore(7200)); + + $laptop = $this->signIn('Laptop'); + $phone = $this->signIn('Phone'); + $this->onDevice($laptop, fn (Authenticate $auth) => $auth->logoutOtherDevices(self::PASSWORD)); + + [, $seenByRoute] = $this->onDevice($phone, function (Authenticate $auth, Request $request) { + $seen = null; + + (new AuthenticateSession())($request, function () use (&$seen): Response { + $seen = app('auth')->check(); + + return new Response(); + }); + + return $seen; + }); + + $this->assertFalse($seenByRoute); + } + + public function test_changing_the_password_during_a_request_keeps_that_device_signed_in(): void + { + $this->useStore(new ArrayStore(7200)); + + $laptop = $this->signIn('Laptop'); + $phone = $this->signIn('Phone'); + + $this->onDevice($laptop, function (Authenticate $auth, Request $request): void { + (new AuthenticateSession())($request, function (): Response { + $user = app('auth')->user(); + $user->password = password_hash('a new password', PASSWORD_BCRYPT, ['cost' => 4]); + $user->save(); + + return new Response(); + }); + }); + + $this->assertTrue($this->isCurrentAndSignedIn($laptop)); + $this->assertFalse($this->isCurrentAndSignedIn($phone)); + } + + public function test_a_session_from_before_fingerprints_is_accepted_and_fingerprinted(): void + { + $laptop = $this->signIn('Laptop'); + + $this->onDevice($laptop, fn (Authenticate $auth, Request $request) => $request->session()->forget('password_hash_web')); + + [, $current] = $this->onDevice($laptop, fn (Authenticate $auth) => $auth->isSessionCurrent()); + [, $fingerprinted] = $this->onDevice($laptop, fn (Authenticate $auth, Request $request) => $request->session()->has('password_hash_web')); + + $this->assertTrue($current); + $this->assertTrue($fingerprinted); + } + + public function test_the_fingerprint_does_not_reveal_the_password_hash(): void + { + $laptop = $this->signIn('Laptop'); + + [, $fingerprint] = $this->onDevice($laptop, fn (Authenticate $auth, Request $request) => $request->session()->get('password_hash_web')); + + $this->assertMatchesRegularExpression('/^[a-f0-9]{64}$/', $fingerprint); + $this->assertStringNotContainsString($fingerprint, $this->storedPassword()); + } + + public function test_remember_me_cookies_of_other_devices_stop_working(): void + { + $laptop = $this->signIn('Laptop'); + [, $phoneRemember] = $this->signInRemembered('Phone'); + + // The phone comes back without a session, with only its remember cookie. + $phoneReturns = fn (): bool => $this->onDevice(null, fn (Authenticate $auth) => $auth->check(), $phoneRemember)[1]; + + $this->assertNotSame([], $phoneRemember, 'The phone received a remember cookie.'); + $this->assertTrue($phoneReturns(), 'Before: the remember cookie signs the phone back in.'); + + $this->onDevice($laptop, fn (Authenticate $auth) => $auth->logoutOtherDevices(self::PASSWORD)); + + $this->assertFalse($phoneReturns(), 'After: the remember cookie no longer works.'); + } + + public function test_a_remembered_device_stays_remembered_after_signing_others_out(): void + { + [$laptop, $laptopRemember] = $this->signInRemembered('Laptop'); + + $issued = []; + $this->onDevice($laptop, function (Authenticate $auth) use (&$issued): bool { + return $this->capturingCookies($issued, fn () => $auth->logoutOtherDevices(self::PASSWORD)); + }, $laptopRemember); + + $this->assertNotNull($this->storedRememberToken(), 'This device keeps a remember token.'); + $this->assertNotSame([], $issued, 'This device received its new remember cookie.'); + + [, $signedIn] = $this->onDevice(null, fn (Authenticate $auth) => $auth->check(), $issued); + + $this->assertTrue($signedIn); + } + + public function test_credentials_are_saved_even_when_the_model_does_not_list_them_as_creatable(): void + { + Config::set('auth.actors.web.model', LockedDownUser::class); + + $laptop = $this->onDevice(null, fn (Authenticate $auth) => $auth->login(LockedDownUser::find(1)))[0]; + $phone = $this->onDevice(null, fn (Authenticate $auth) => $auth->login(LockedDownUser::find(1)))[0]; + $before = $this->storedPassword(); + + [, $result] = $this->onDevice($laptop, fn (Authenticate $auth) => $auth->logoutOtherDevices(self::PASSWORD)); + + $this->assertTrue($result); + $this->assertNotSame($before, $this->storedPassword(), 'The new hash reached the database.'); + $this->assertFalse($this->isSignedIn($phone)); + $this->assertTrue($this->isSignedIn($laptop)); + } + + public function test_a_users_table_without_a_remember_token_column_is_supported(): void + { + Database::getPdoInstance(self::CONNECTION)->exec('ALTER TABLE users DROP COLUMN remember_token'); + + $laptop = $this->signIn('Laptop'); + + [, $result] = $this->onDevice($laptop, fn (Authenticate $auth) => $auth->logoutOtherDevices(self::PASSWORD)); + + $this->assertTrue($result); + $this->assertTrue($this->isSignedIn($laptop)); + } + + public function test_sessions_lists_the_devices_without_exposing_session_ids(): void + { + $laptop = $this->signIn('Laptop'); + $this->signIn('Phone'); + + [, $sessions] = $this->onDevice($laptop, fn (Authenticate $auth) => $auth->sessions()); + + $this->assertCount(2, $sessions); + $this->assertSame(['ip_address', 'user_agent', 'last_activity', 'is_current'], array_keys($sessions[0])); + $this->assertSame([true], array_values(array_unique(array_filter(array_column($sessions, 'is_current'))))); + $this->assertEqualsCanonicalizing(['Laptop', 'Phone'], array_column($sessions, 'user_agent')); + $this->assertSame('203.0.113.10', $sessions[0]['ip_address']); + } + + public function test_sessions_is_empty_with_a_store_that_does_not_track_users(): void + { + $this->useStore(new ArrayStore(7200)); + $laptop = $this->signIn('Laptop'); + + [, $sessions] = $this->onDevice($laptop, fn (Authenticate $auth) => $auth->sessions()); + + $this->assertSame([], $sessions); + } + + public function test_another_actor_never_deletes_sessions_by_the_default_actors_user_ids(): void + { + $laptop = $this->signIn('Laptop'); + $phone = $this->signIn('Phone'); + + // The admin actor signs in user 1 too, but its ids are not the ones the store tracks. + [, $result] = $this->onDevice($laptop, function (Authenticate $auth): bool { + $admin = app('auth')->actor('admin'); + $admin->login(DeviceUser::find(1)); + + return $admin->logoutOtherDevices(self::PASSWORD); + }); + + $this->assertTrue($result); + $this->assertSame(2, $this->sessionRows(), 'No stored session was deleted.'); + $this->assertTrue($this->isSignedIn($phone), 'The phone is only signed out once its fingerprint is checked.'); + $this->assertFalse($this->isCurrentAndSignedIn($phone)); + } + + /** + * Sign the user in on a new device and return its session cookie. + */ + private function signIn(string $agent): string + { + [$cookie] = $this->onDevice(null, fn (Authenticate $auth) => $auth->login(DeviceUser::find(1)), [], $agent); + + return $cookie; + } + + /** + * Sign the user in with "remember me" and return [session cookie, remember cookies]. + * + * @return array{0: string, 1: array} + */ + private function signInRemembered(string $agent): array + { + $issued = []; + + [$cookie] = $this->onDevice(null, function (Authenticate $auth) use (&$issued): bool { + return $this->capturingCookies($issued, fn () => $auth->login(DeviceUser::find(1), true)); + }, [], $agent); + + return [$cookie, $issued]; + } + + /** + * Run a request on a device and return [the device's session cookie afterwards, the result]. + * + * @param array $cookies Other cookies the device sends, such as remember-me + * @return array{0: string|null, 1: mixed} + */ + private function onDevice(?string $sessionCookie, callable $action, array $cookies = [], string $agent = 'Laptop'): array + { + $request = new class extends Request { + public function __construct() + { + } + }; + + $request->cookies = new InputBag(array_filter([self::COOKIE => $sessionCookie])); + $request->server = new ServerBag(['REMOTE_ADDR' => '203.0.113.10', 'REQUEST_URI' => '/', 'REQUEST_METHOD' => 'GET']); + $request->headers = new HeaderBag(['USER_AGENT' => $agent]); + $request->query = new InputBag(); + $request->request = new InputBag(); + $request->session = new Session($this->store, $this->sessionConfig); + $request->session->attach($request); + + $container = Container::getInstance(); + $container->instance('request', $request); + $container->instance('session', $request->session); + app('auth')->forgetActors(); + + $_COOKIE = $cookies; + + $result = $action(app('auth')->actor('web'), $request); + + $response = new Response(); + $request->session->commit($response); + + foreach ($response->headers->getCookies() as $cookie) { + if ($cookie->getName() === self::COOKIE) { + $sessionCookie = $cookie->getValue() === '' ? null : $cookie->getValue(); + } + } + + return [$sessionCookie, $result]; + } + + private function isSignedIn(?string $sessionCookie): bool + { + return $this->onDevice($sessionCookie, fn (Authenticate $auth) => $auth->check())[1]; + } + + private function isCurrentAndSignedIn(?string $sessionCookie): bool + { + return $this->onDevice($sessionCookie, fn (Authenticate $auth) => $auth->isSessionCurrent() && $auth->check())[1]; + } + + /** + * Record the remember cookies sent to the device while running a callback. + * + * @param array $issued + */ + private function capturingCookies(array &$issued, callable $callback): mixed + { + CookieRecordingAuthenticate::$sent = []; + + $result = $callback(); + + $issued = CookieRecordingAuthenticate::$sent + $issued; + + return $result; + } + + private function databaseStore(): DatabaseStore + { + return new DatabaseStore( + static fn (): PDO => Database::getPdoInstance(self::CONNECTION), + 'sessions', + 7200, + SessionManager::userResolver(), + static fn (): array => ['ip_address' => request()->ip(), 'user_agent' => request()->userAgent()] + ); + } + + private function useStore(SessionStoreInterface $store): void + { + $this->store = $store; + SessionManager::useStore($store); + } + + private function storedPassword(): string + { + return (string) Database::getPdoInstance(self::CONNECTION)->query('SELECT password FROM users WHERE id = 1')->fetchColumn(); + } + + private function storedRememberToken(): ?string + { + $token = Database::getPdoInstance(self::CONNECTION)->query('SELECT remember_token FROM users WHERE id = 1')->fetchColumn(); + + return $token === null || $token === false ? null : (string) $token; + } + + private function sessionRows(): int + { + return (int) Database::getPdoInstance(self::CONNECTION)->query('SELECT COUNT(*) FROM sessions')->fetchColumn(); + } + + private function resetConfig(): void + { + $reflection = new \ReflectionClass(Config::class); + + foreach (['config' => [], 'cacheFile' => null, 'loadedFromCache' => false, 'fileHashes' => []] as $name => $value) { + if ($reflection->hasProperty($name)) { + $reflection->getProperty($name)->setValue(null, $value); + } + } + } +} diff --git a/tests/Console/SessionTableCommandTest.php b/tests/Console/SessionTableCommandTest.php new file mode 100644 index 00000000..233c104f --- /dev/null +++ b/tests/Console/SessionTableCommandTest.php @@ -0,0 +1,114 @@ +dir = sys_get_temp_dir() . '/doppar-session-table-' . bin2hex(random_bytes(5)); + + Container::setInstance(new MockContainer()); + $this->resetConfig(); + } + + protected function tearDown(): void + { + foreach (glob($this->dir . '/*') ?: [] as $file) { + @unlink($file); + } + + @rmdir($this->dir); + + $this->resetConfig(); + (new \ReflectionClass(Container::class))->getProperty('instance')->setValue(null, null); + } + + public function test_it_creates_a_migration_for_the_configured_table(): void + { + Config::set('session.table', 'user_sessions'); + + $tester = $this->run_(); + $files = glob($this->dir . '/*_create_user_sessions_table.php'); + + $this->assertSame(0, $tester->getStatusCode()); + $this->assertCount(1, $files); + $this->assertMatchesRegularExpression('/^\d{4}_\d{2}_\d{2}_\d{6}_create_user_sessions_table\.php$/', basename($files[0])); + + $migration = (string) file_get_contents($files[0]); + + $this->assertStringContainsString("Schema::create('user_sessions'", $migration); + $this->assertStringContainsString("Schema::dropIfExists('user_sessions')", $migration); + $this->assertStringNotContainsString('{{ table }}', $migration); + } + + public function test_it_defaults_to_the_sessions_table(): void + { + $this->run_(); + + $this->assertCount(1, glob($this->dir . '/*_create_sessions_table.php')); + } + + public function test_it_does_not_create_a_second_migration(): void + { + $this->run_(); + + $tester = $this->run_(); + + $this->assertSame(1, $tester->getStatusCode()); + $this->assertStringContainsString('already exists', $tester->getDisplay()); + $this->assertCount(1, glob($this->dir . '/*.php')); + } + + public function test_it_rejects_an_unsafe_table_name(): void + { + Config::set('session.table', 'sessions; DROP TABLE users'); + + $tester = $this->run_(); + + $this->assertSame(1, $tester->getStatusCode()); + $this->assertSame([], glob($this->dir . '/*.php') ?: []); + } + + private function run_(): CommandTester + { + $directory = $this->dir; + + $command = new class ($directory) extends SessionTableCommand { + public function __construct(private string $directory) + { + parent::__construct(); + } + + protected function migrationPath(): string + { + return $this->directory; + } + }; + + $tester = new CommandTester($command); + $tester->execute([], ['interactive' => false, 'decorated' => false]); + + return $tester; + } + + private function resetConfig(): void + { + $reflection = new \ReflectionClass(Config::class); + + foreach (['config' => [], 'cacheFile' => null, 'loadedFromCache' => false, 'fileHashes' => []] as $name => $value) { + if ($reflection->hasProperty($name)) { + $reflection->getProperty($name)->setValue(null, $value); + } + } + } +} diff --git a/tests/Session/DatabaseRotationTest.php b/tests/Session/DatabaseRotationTest.php new file mode 100644 index 00000000..803e1408 --- /dev/null +++ b/tests/Session/DatabaseRotationTest.php @@ -0,0 +1,63 @@ +database = sys_get_temp_dir() . '/doppar_rotation_db_' . bin2hex(random_bytes(6)) . '.sqlite'; + $this->pdo = new PDO('sqlite:' . $this->database, null, null, [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION]); + $this->pdo->exec( + 'CREATE TABLE ' . self::TABLE . ' (id VARCHAR(128) PRIMARY KEY, user_id INTEGER NULL, ip_address VARCHAR(45) NULL, ' + . 'user_agent TEXT NULL, payload TEXT NOT NULL, last_activity INTEGER NOT NULL)' + ); + } + + protected function tearDown(): void + { + unset($this->pdo); + @unlink($this->database); + + parent::tearDown(); + } + + protected function store(): LockableSessionStore + { + $pdo = $this->pdo; + + return new DatabaseStore(static fn (): PDO => $pdo, self::TABLE, 7200); + } + + protected function liveRecords(): array + { + $records = []; + + foreach ($this->pdo->query('SELECT id, payload FROM ' . self::TABLE)->fetchAll(PDO::FETCH_ASSOC) as $row) { + $data = @unserialize((string) base64_decode($row['payload'], true)); + + if (is_array($data) && !isset($data['_rotated_to'])) { + $records[$row['id']] = $data; + } + } + + return $records; + } +} diff --git a/tests/Session/DatabaseStoreMysqlTest.php b/tests/Session/DatabaseStoreMysqlTest.php new file mode 100644 index 00000000..09354c0b --- /dev/null +++ b/tests/Session/DatabaseStoreMysqlTest.php @@ -0,0 +1,40 @@ +markTestSkipped('Configure DOPPAR_TEST_MYSQL_HOST/DOPPAR_TEST_MYSQL_DATABASE to run this test.'); + } + + return [ + 'driver' => 'mysql', + 'host' => $host, + 'port' => getenv('DOPPAR_TEST_MYSQL_PORT') ?: '3306', + 'database' => $database, + 'username' => getenv('DOPPAR_TEST_MYSQL_USERNAME') ?: 'root', + 'password' => getenv('DOPPAR_TEST_MYSQL_PASSWORD') ?: '', + 'charset' => 'utf8mb4', + 'collation' => 'utf8mb4_unicode_ci', + 'prefix' => '', + ]; + } +} diff --git a/tests/Session/DatabaseStorePgsqlTest.php b/tests/Session/DatabaseStorePgsqlTest.php new file mode 100644 index 00000000..8c71b79b --- /dev/null +++ b/tests/Session/DatabaseStorePgsqlTest.php @@ -0,0 +1,39 @@ +markTestSkipped('Configure DOPPAR_TEST_PGSQL_HOST/DOPPAR_TEST_PGSQL_DATABASE to run this test.'); + } + + return [ + 'driver' => 'pgsql', + 'host' => $host, + 'port' => getenv('DOPPAR_TEST_PGSQL_PORT') ?: '5432', + 'database' => $database, + 'username' => getenv('DOPPAR_TEST_PGSQL_USERNAME') ?: 'postgres', + 'password' => getenv('DOPPAR_TEST_PGSQL_PASSWORD') ?: '', + 'charset' => 'utf8', + 'prefix' => '', + ]; + } +} diff --git a/tests/Session/DatabaseStoreTest.php b/tests/Session/DatabaseStoreTest.php new file mode 100644 index 00000000..8b6c502c --- /dev/null +++ b/tests/Session/DatabaseStoreTest.php @@ -0,0 +1,513 @@ +dir = sys_get_temp_dir() . '/doppar-session-db-' . bin2hex(random_bytes(5)); + mkdir($this->dir, 0700, true); + + $container = new MockContainer(); + Container::setInstance($container); + $this->resetConfig(); + + Config::set('database.connections.' . static::CONNECTION, $this->connectionConfig()); + Config::set('database.connections.' . static::SECOND_CONNECTION, $this->connectionConfig()); + Config::set('database.default', static::CONNECTION); + + $container->bind('db', fn () => new Database()); + $container->bind('schema', fn () => new Schema()); + + if (method_exists(Facade::class, 'clearResolvedInstances')) { + Facade::clearResolvedInstances(); + } + + $this->dropTable(); + $this->migrate(); + } + + protected function tearDown(): void + { + $this->dropTable(); + + foreach (['connections', 'drivers', 'transactions'] as $property) { + (new \ReflectionClass(Database::class))->getProperty($property)->setValue(null, []); + } + + $this->resetConfig(); + (new \ReflectionClass(Container::class))->getProperty('instance')->setValue(null, null); + + foreach (glob($this->dir . '/*') ?: [] as $file) { + @unlink($file); + } + + @rmdir($this->dir); + } + + /** + * The scratch connection the store runs against. + * + * @return array + */ + protected function connectionConfig(): array + { + return ['driver' => 'sqlite', 'database' => $this->dir . '/sessions.sqlite', 'prefix' => '']; + } + + public function test_it_round_trips_session_data_including_objects_and_binary_strings(): void + { + $store = $this->store(); + $id = $this->id(); + $data = ['user' => 7, 'nul' => "a\0b", 'object' => new \ArrayObject(['x' => 1]), 'utf8' => 'héllo']; + + $this->assertSame($id, $store->write($id, $data)); + + $record = $this->store()->read($id); + + $this->assertSame($id, $record['id']); + $this->assertSame("a\0b", $record['data']['nul']); + $this->assertSame('héllo', $record['data']['utf8']); + $this->assertEquals(new \ArrayObject(['x' => 1]), $record['data']['object']); + } + + public function test_a_write_replaces_the_stored_session(): void + { + $store = $this->store(); + $id = $this->id(); + + $store->write($id, ['step' => 1]); + $store->write($id, ['step' => 2]); + + $this->assertSame(['step' => 2], $store->read($id)['data']); + $this->assertSame(1, $this->countRows()); + } + + public function test_it_records_the_user_ip_address_and_user_agent(): void + { + $store = $this->store(user: fn (array $data) => $data['user'] ?? null, client: fn () => [ + 'ip_address' => '203.0.113.9', + 'user_agent' => 'Mozilla/5.0 Test', + ]); + + $store->write($id = $this->id(), ['user' => 42]); + + $row = $this->row($id); + + $this->assertSame('42', (string) $row['user_id']); + $this->assertSame('203.0.113.9', $row['ip_address']); + $this->assertSame('Mozilla/5.0 Test', $row['user_agent']); + } + + public function test_a_session_without_a_user_stores_no_user(): void + { + $store = $this->store(user: fn (array $data) => $data['user'] ?? null); + + $store->write($id = $this->id(), ['cart' => [1, 2]]); + + $this->assertNull($this->row($id)['user_id']); + } + + public function test_hostile_user_agents_are_bounded_and_stored_as_printable_ascii(): void + { + $store = $this->store(client: fn () => [ + 'ip_address' => '198.51.100.1', + 'user_agent' => "Bad\xC3\x28Agent\n" . str_repeat('A', 5000), + ]); + + $store->write($id = $this->id(), ['x' => 1]); + + $agent = $this->row($id)['user_agent']; + + $this->assertSame(1024, strlen($agent)); + $this->assertStringStartsWith('Bad?(Agent?', $agent); + $this->assertMatchesRegularExpression('/^[\x20-\x7E]+$/', $agent); + } + + public function test_a_failing_client_resolver_does_not_stop_the_session_from_saving(): void + { + $store = $this->store(client: function (): array { + throw new \RuntimeException('no request'); + }); + + $store->write($id = $this->id(), ['x' => 1]); + + $this->assertSame(['x' => 1], $store->read($id)['data']); + $this->assertNull($this->row($id)['ip_address']); + } + + public function test_it_rejects_ids_that_are_not_generated_ids(): void + { + $store = $this->store(); + + $this->assertNull($store->read('../../etc/passwd')); + $this->assertNull($store->read(str_repeat('a', 40) . "\n")); + $this->assertNull($store->read(str_repeat('A', 40))); + + $this->expectException(SessionException::class); + + $store->write('not-an-id', ['x' => 1]); + } + + public function test_unknown_expired_and_corrupt_sessions_read_as_missing(): void + { + $store = $this->store(lifetime: 60); + + $this->assertNull($store->read($this->id())); + + $store->write($expired = $this->id(), ['x' => 1]); + $this->setActivity($expired, time() - 120); + + $this->assertNull($this->store(lifetime: 60)->read($expired)); + $this->assertNull($this->row($expired), 'An expired session is removed when it is read.'); + + $store->write($corrupt = $this->id(), ['x' => 1]); + $this->pdo()->prepare('UPDATE ' . static::TABLE . ' SET payload = ? WHERE id = ?')->execute(['%%not-base64%%', $corrupt]); + + $this->assertNull($this->store()->read($corrupt)); + } + + public function test_touch_refreshes_a_stale_session_and_skips_a_fresh_one(): void + { + $store = $this->store(lifetime: 7200); + $store->write($id = $this->id(), ['x' => 1]); + + // Fresh in this process: no write at all. + $this->setActivity($id, $stale = time() - 3600); + $store->touch($id); + $this->assertSame($stale, (int) $this->row($id)['last_activity']); + + // A process that has not seen it recently refreshes it. + $this->store(lifetime: 7200)->touch($id); + $this->assertGreaterThanOrEqual(time() - 1, (int) $this->row($id)['last_activity']); + } + + public function test_destroy_removes_the_session(): void + { + $store = $this->store(); + $store->write($id = $this->id(), ['x' => 1]); + + $store->destroy($id); + + $this->assertNull($store->read($id)); + $this->assertSame(0, $this->countRows()); + } + + public function test_gc_removes_only_expired_sessions(): void + { + $store = $this->store(lifetime: 60); + $store->write($live = $this->id(), ['x' => 1]); + $store->write($old = $this->id(), ['x' => 1]); + $this->setActivity($old, time() - 120); + + $this->assertSame(1, $store->gc(60)); + $this->assertNotNull($this->row($live)); + $this->assertNull($this->row($old)); + } + + public function test_sweep_removes_a_bounded_slice_of_expired_sessions(): void + { + $store = $this->store(lifetime: 60); + $this->insertExpired(1005); + + $this->assertSame(1000, $store->sweep(60)); + $this->assertSame(5, $this->countRows()); + $this->assertSame(5, $store->sweep(60)); + $this->assertSame(0, $this->countRows()); + } + + public function test_flush_removes_every_session(): void + { + $store = $this->store(); + $store->write($this->id(), ['x' => 1]); + $store->write($this->id(), ['x' => 2]); + + $this->assertSame(2, $store->flush()); + $this->assertSame(0, $this->countRows()); + } + + public function test_user_sessions_lists_the_live_sessions_of_one_user_most_recent_first(): void + { + $store = $this->store(lifetime: 600, user: fn (array $data) => $data['user'] ?? null); + + $store->write($older = $this->id(), ['user' => 1]); + $store->write($newer = $this->id(), ['user' => 1]); + $store->write($expired = $this->id(), ['user' => 1]); + $store->write($otherUser = $this->id(), ['user' => 2]); + $this->setActivity($older, time() - 100); + $this->setActivity($expired, time() - 1000); + + $sessions = $store->userSessions(1); + + $this->assertSame([$newer, $older], array_column($sessions, 'id')); + $this->assertIsInt($sessions[0]['last_activity']); + $this->assertSame([], $store->userSessions(99)); + } + + public function test_destroy_user_sessions_ends_every_session_of_the_user_except_one(): void + { + $store = $this->store(user: fn (array $data) => $data['user'] ?? null); + + $store->write($current = $this->id(), ['user' => 5]); + $store->write($laptop = $this->id(), ['user' => 5]); + $store->write($phone = $this->id(), ['user' => 5]); + $store->write($someoneElse = $this->id(), ['user' => 6]); + + $this->assertSame(2, $store->destroyUserSessions(5, $current)); + + $this->assertNotNull($store->read($current)); + $this->assertNull($store->read($laptop)); + $this->assertNull($store->read($phone)); + $this->assertNotNull($store->read($someoneElse)); + } + + public function test_a_callback_under_a_lock_can_use_the_store_without_waiting_for_itself(): void + { + $store = $this->store(); + $store->write($id = $this->id(), ['x' => 1]); + + $started = microtime(true); + + $result = $store->synchronized($id, function () use ($store, $id): string { + $store->write($id, ['x' => 2]); + $store->destroy($id); + + return 'done'; + }); + + $this->assertSame('done', $result); + $this->assertLessThan(1.0, microtime(true) - $started); + $this->assertNull($store->read($id)); + } + + public function test_a_second_connection_waits_for_the_lock_then_gives_up_and_reports(): void + { + $holder = $this->store(); + $waiter = $this->store(connection: static::SECOND_CONNECTION, lockTimeout: 0.2); + $holder->write($id = $this->id(), ['x' => 1]); + + $ran = $holder->synchronized($id, fn () => $waiter->synchronized($id, fn () => 'still ran')); + + $this->assertSame('still ran', $ran); + $this->assertCount(1, $waiter->reports); + $this->assertStringContainsString('Session lock was not acquired within 0.2 seconds', $waiter->reports[0]); + } + + public function test_the_lock_is_free_for_another_connection_after_a_callback_throws(): void + { + $holder = $this->store(); + $other = $this->store(connection: static::SECOND_CONNECTION, lockTimeout: 0.2); + $holder->write($id = $this->id(), ['x' => 1]); + + try { + $holder->synchronized($id, function (): void { + throw new \RuntimeException('boom'); + }); + } catch (\RuntimeException) { + } + + $other->synchronized($id, fn () => null); + + $this->assertSame([], $other->reports); + } + + public function test_a_lock_on_one_session_does_not_block_another_session(): void + { + $holder = $this->store(); + $other = $this->store(connection: static::SECOND_CONNECTION, lockTimeout: 0.2); + $holder->write($first = $this->id(), ['x' => 1]); + $other->write($second = $this->id(), ['x' => 1]); + + if ($this->pdo()->getAttribute(PDO::ATTR_DRIVER_NAME) === 'sqlite') { + $this->markTestSkipped('SQLite has a single writer, so its lock covers the whole database.'); + } + + $holder->synchronized($first, fn () => $other->synchronized($second, fn () => null)); + + $this->assertSame([], $other->reports); + } + + public function test_an_invalid_table_name_is_rejected(): void + { + $this->expectException(SessionException::class); + + new DatabaseStore(fn () => $this->pdo(), 'sessions; DROP TABLE users'); + } + + public function test_a_missing_table_explains_how_to_create_it(): void + { + $store = new DatabaseStore(fn () => $this->pdo(), 'missing_sessions_table'); + + $this->expectException(SessionException::class); + $this->expectExceptionMessage('php pool session:table'); + + $store->write($this->id(), ['x' => 1]); + } + + public function test_it_keeps_working_when_the_connection_is_replaced(): void + { + $pdo = $this->pdo(); + $store = new RecordingDatabaseStore(function () use (&$pdo): PDO { + return $pdo; + }, static::TABLE); + + $store->write($id = $this->id(), ['x' => 1]); + + // A fresh connection to the same database, e.g. after a reconnect. + (new \ReflectionClass(Database::class))->getProperty('connections')->setValue(null, []); + $pdo = Database::getPdoInstance(static::CONNECTION); + + $this->assertSame(['x' => 1], $store->read($id)['data']); + } + + public function test_it_is_never_client_side(): void + { + $this->assertFalse($this->store()->isClientSide()); + } + + /** + * @param int $lifetime + * @param \Closure|null $user + * @param \Closure|null $client + * @param string|null $connection + * @param float $lockTimeout + * @return RecordingDatabaseStore + */ + protected function store( + int $lifetime = 7200, + ?\Closure $user = null, + ?\Closure $client = null, + ?string $connection = null, + float $lockTimeout = 2.0 + ): RecordingDatabaseStore { + $connection ??= static::CONNECTION; + + return new RecordingDatabaseStore( + static fn (): PDO => Database::getPdoInstance($connection), + static::TABLE, + $lifetime, + $user, + $client, + $lockTimeout + ); + } + + protected function id(): string + { + return bin2hex(random_bytes(20)); + } + + protected function pdo(): PDO + { + return Database::getPdoInstance(static::CONNECTION); + } + + protected function row(string $id): ?array + { + $statement = $this->pdo()->prepare('SELECT * FROM ' . static::TABLE . ' WHERE id = ?'); + $statement->execute([$id]); + $row = $statement->fetch(PDO::FETCH_ASSOC); + + return $row === false ? null : $row; + } + + protected function countRows(): int + { + return (int) $this->pdo()->query('SELECT COUNT(*) FROM ' . static::TABLE)->fetchColumn(); + } + + protected function setActivity(string $id, int $time): void + { + $this->pdo()->prepare('UPDATE ' . static::TABLE . ' SET last_activity = ? WHERE id = ?')->execute([$time, $id]); + } + + protected function insertExpired(int $count): void + { + $pdo = $this->pdo(); + $statement = $pdo->prepare('INSERT INTO ' . static::TABLE . ' (id, payload, last_activity) VALUES (?, ?, ?)'); + $payload = base64_encode(serialize(['x' => 1])); + + $pdo->beginTransaction(); + + for ($i = 0; $i < $count; $i++) { + $statement->execute([$this->id(), $payload, time() - 3600]); + } + + $pdo->commit(); + } + + /** + * Create the table with the migration "php pool session:table" generates. + * + * @return void + */ + protected function migrate(): void + { + $stub = (string) file_get_contents(dirname(__DIR__, 2) . '/src/Phaseolies/Console/Commands/stubs/session/create_sessions_table.stub'); + $file = $this->dir . '/create_sessions_table.php'; + + file_put_contents($file, str_replace('{{ table }}', static::TABLE, $stub)); + + (require $file)->up(); + } + + protected function dropTable(): void + { + try { + $this->pdo()->exec('DROP TABLE IF EXISTS ' . static::TABLE); + } catch (\Throwable) { + } + } + + protected function resetConfig(): void + { + $reflection = new \ReflectionClass(Config::class); + + foreach (['config' => [], 'cacheFile' => null, 'loadedFromCache' => false, 'fileHashes' => []] as $name => $value) { + if ($reflection->hasProperty($name)) { + $reflection->getProperty($name)->setValue(null, $value); + } + } + } +} + +/** + * Records what the store reports instead of logging it. + */ +class RecordingDatabaseStore extends DatabaseStore +{ + /** + * @var string[] + */ + public array $reports = []; + + protected function report(string $message): void + { + $this->reports[] = $message; + } +} diff --git a/tests/Session/RotationTest.php b/tests/Session/RotationTest.php index dd59470e..db7282a2 100644 --- a/tests/Session/RotationTest.php +++ b/tests/Session/RotationTest.php @@ -7,6 +7,7 @@ use Phaseolies\Http\Response; use Phaseolies\Http\Response\Cookie; use Phaseolies\Http\ServerBag; +use Phaseolies\Session\Contracts\LockableSessionStore; use Phaseolies\Session\Stores\ArrayStore; use Phaseolies\Session\Stores\FileStore; use Phaseolies\Support\Session; @@ -53,7 +54,7 @@ protected function tearDown(): void @rmdir($this->path); } - private function store(): FileStore + protected function store(): LockableSessionStore { return new FileStore($this->path, 7200); } @@ -118,7 +119,7 @@ private function dueSession(array $data = ['user' => 7], $store = null): string * * @return array */ - private function liveRecords(): array + protected function liveRecords(): array { $records = []; diff --git a/tests/Session/SessionManagerTest.php b/tests/Session/SessionManagerTest.php index d335b449..602b7ce9 100644 --- a/tests/Session/SessionManagerTest.php +++ b/tests/Session/SessionManagerTest.php @@ -2,9 +2,13 @@ namespace Tests\Unit\Session; +use Phaseolies\Config\Config; +use Phaseolies\DI\Container; use Phaseolies\Session\SessionManager; +use Phaseolies\Session\Stores\DatabaseStore; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; +use Tests\Support\MockContainer; class SessionManagerTest extends TestCase { @@ -61,4 +65,70 @@ public function test_a_config_without_a_lottery_gets_the_default(): void { $this->assertSame([2, 100], SessionManager::normalize([])['lottery']); } + + public function test_a_config_without_database_options_gets_the_defaults(): void + { + $config = SessionManager::normalize([]); + + $this->assertNull($config['connection']); + $this->assertSame('sessions', $config['table']); + } + + public function test_the_database_driver_builds_a_database_store_without_connecting(): void + { + $this->withAuthConfig(function (): void { + $store = SessionManager::createStore(SessionManager::normalize([ + 'driver' => 'database', + 'connection' => 'not_configured_so_connecting_would_fail', + ])); + + $this->assertInstanceOf(DatabaseStore::class, $store); + $this->assertFalse($store->isClientSide()); + }); + } + + public function test_sessions_are_linked_to_the_user_of_the_default_actor(): void + { + $this->withAuthConfig(function (): void { + $resolve = SessionManager::userResolver(); + + $this->assertSame('user', SessionManager::userSessionKey()); + $this->assertSame(5, $resolve(['user' => 5, 'admin' => 9])); + $this->assertNull($resolve(['cart' => []])); + }); + } + + public function test_without_an_auth_actor_sessions_are_not_linked_to_users(): void + { + $this->withAuthConfig(function (): void { + Config::set('auth.default', 'missing'); + + $this->assertNull(SessionManager::userSessionKey()); + $this->assertNull(SessionManager::userResolver()); + }); + } + + /** + * Run a test with a container and an auth config where "web" keeps the user under "user". + * + * @param callable $test + * @return void + */ + private function withAuthConfig(callable $test): void + { + Container::setInstance(new MockContainer()); + + $previous = (new \ReflectionClass(Config::class))->getProperty('config')->getValue(); + + Config::set('auth.default', 'web'); + Config::set('auth.actors.web.session_key', 'user'); + Config::set('auth.actors.admin.session_key', 'admin'); + + try { + $test(); + } finally { + (new \ReflectionClass(Config::class))->getProperty('config')->setValue(null, $previous); + (new \ReflectionClass(Container::class))->getProperty('instance')->setValue(null, null); + } + } } From 451f6eae0e7410e8990fa7f614206c803058bbe5 Mon Sep 17 00:00:00 2001 From: techmahedy Date: Thu, 5 Nov 2026 20:41:46 +0600 Subject: [PATCH 2/6] fix(session): address review of database sessions and logout from other devices --- src/Phaseolies/Auth/Security/Authenticate.php | 197 +++++++++++---- .../Console/Commands/SessionTableCommand.php | 32 ++- .../stubs/session/create_sessions_table.stub | 4 +- .../Middleware/AuthenticateSession.php | 3 +- .../Session/Contracts/CommittableSession.php | 7 + .../Session/Contracts/UserSessionStore.php | 8 +- src/Phaseolies/Session/RotationPointer.php | 63 +++++ .../Session/Stores/DatabaseStore.php | 85 ++++++- src/Phaseolies/Support/Facades/Schema.php | 1 + src/Phaseolies/Support/Session.php | 37 +-- tests/Auth/LogoutOtherDevicesTest.php | 224 +++++++++++++++++- tests/Console/SessionTableCommandTest.php | 37 +++ tests/Session/DatabaseStoreTest.php | 69 +++++- 13 files changed, 675 insertions(+), 92 deletions(-) create mode 100644 src/Phaseolies/Session/RotationPointer.php diff --git a/src/Phaseolies/Auth/Security/Authenticate.php b/src/Phaseolies/Auth/Security/Authenticate.php index 2f09a0d3..16f7fe75 100644 --- a/src/Phaseolies/Auth/Security/Authenticate.php +++ b/src/Phaseolies/Auth/Security/Authenticate.php @@ -5,6 +5,7 @@ use Phaseolies\Support\Facades\Hash; use Phaseolies\Support\Facades\Crypt; use Phaseolies\Auth\Authable; +use Phaseolies\Session\Contracts\CommittableSession; use Phaseolies\Session\Contracts\SessionStoreInterface; use Phaseolies\Session\Contracts\UserSessionStore; use Phaseolies\Session\SessionManager; @@ -46,6 +47,13 @@ class Authenticate */ private ?Authable $resolvedUser = null; + /** + * Fingerprint of the signed-in user's password when isSessionCurrent() checked it + * + * @var string|null + */ + private ?string $checkedFingerprint = null; + /** * Create a new actor instance. * @@ -295,38 +303,10 @@ public function user(): ?Authable } if (cookie()->has($this->getRememberCookieName())) { - try { - /** - * @var string - */ - $rememberToken = Crypt::decrypt(cookie()->get($this->getRememberCookieName())); - } catch (\Exception $e) { - $this->expireRememberCookie(); - return null; - } - - $segments = explode('|', $rememberToken); - - if (count($segments) !== 3) { - $this->expireRememberCookie(); - return null; - } - - [$id, $token, $hash] = $segments; - - if (!Hash::check($id . $token, $hash)) { - $this->expireRememberCookie(); - return null; - } + $remembered = $this->readRememberCookie(); + $user = $remembered === null ? null : $authModel::find($remembered[0]); - $user = $authModel::find($id); - - if (!$user || !$user->getRememberToken()) { - $this->expireRememberCookie(); - return null; - } - - if (Hash::check($token, $user->getRememberToken())) { + if ($user && $this->rememberTokenMatches($user, $remembered[1])) { if ($this->hasTwoFactorEnabled($user)) { session()->put($this->getTwoFactorUserKey(), $user->getAuthIdentifier()); session()->put($this->getTwoFactorRememberKey(), true); @@ -337,15 +317,71 @@ public function user(): ?Authable return $this->resolvedUser = $user; } - // Token didn't match - possible theft attempt $this->expireRememberCookie(); - $user->setRememberToken(null); - $user->save(); } return null; } + /** + * Read this device's remember cookie. + * + * @return array{0: string, 1: string}|null + */ + protected function readRememberCookie(): ?array + { + $value = cookie()->get($this->getRememberCookieName()); + + if (!is_string($value) || $value === '') { + return null; + } + + try { + $plain = Crypt::decrypt($value); + } catch (\Exception) { + return null; + } + + $segments = is_string($plain) ? explode('|', $plain) : []; + + if (count($segments) !== 3) { + return null; + } + + [$id, $token, $hash] = $segments; + + return Hash::check($id . $token, $hash) ? [$id, $token] : null; + } + + /** + * Whether a remember token is the one the user currently holds. + * + * @param Authable $user + * @param string $token + * @return bool + */ + protected function rememberTokenMatches(Authable $user, string $token): bool + { + $stored = $user->getRememberToken(); + + return is_string($stored) && $stored !== '' && Hash::check($token, $stored); + } + + /** + * Whether this device's remember cookie is valid for the user. + * + * @param Authable $user + * @return bool + */ + protected function hasValidRememberCookie(Authable $user): bool + { + $remembered = $this->readRememberCookie(); + + return $remembered !== null + && $remembered[0] === (string) $user->getAuthIdentifier() + && $this->rememberTokenMatches($user, $remembered[1]); + } + /** * Check if the user is authenticated. * @@ -377,12 +413,14 @@ public function logout(): void /** * Sign this device out, leaving the user's other devices alone. * + * @param bool $expireRememberCookie * @return void */ - protected function endSession(): void + protected function endSession(bool $expireRememberCookie = true): void { $this->resolvedUser = null; $this->statelessUser = null; + $this->checkedFingerprint = null; session()->forget($this->getSessionKey()); session()->forget($this->getViaRememberKey()); @@ -395,7 +433,7 @@ protected function endSession(): void session()->regenerateToken(); } - if (cookie()->has($this->getRememberCookieName())) { + if ($expireRememberCookie && cookie()->has($this->getRememberCookieName())) { $this->expireRememberCookie(); } } @@ -405,6 +443,7 @@ protected function endSession(): void * * @param string $password * @return bool + * @throws \LogicException */ public function logoutOtherDevices(string $password): bool { @@ -414,15 +453,29 @@ public function logoutOtherDevices(string $password): bool return false; } - $changes = [$user->getAuthPasswordName() => Hash::make($password)]; + $passwordName = $user->getAuthPasswordName(); + + if ($user->{$passwordName} !== $user->getAuthPassword()) { + throw new \LogicException(sprintf( + '%s overrides getAuthPassword() but not getAuthPasswordName(), which still names [%s]. Override getAuthPasswordName() to return the password attribute.', + $user::class, + $passwordName + )); + } + + $changes = [$passwordName => Hash::make($password)]; $tokenName = $user->getRememberTokenName(); if (array_key_exists($tokenName, $user->getAttributes())) { - if (cookie()->has($this->getRememberCookieName())) { + if ($this->hasValidRememberCookie($user)) { $this->issueRememberToken($user); } else { $user->setRememberToken(null); + + if (cookie()->has($this->getRememberCookieName())) { + $this->expireRememberCookie(); + } } $changes[$tokenName] = $user->getRememberToken(); @@ -430,14 +483,16 @@ public function logoutOtherDevices(string $password): bool $this->persistCredentials($user, $changes); - $this->refreshSessionFingerprint(); + if (session()->has($this->getSessionKey())) { + session()->put($this->getPasswordFingerprintKey(), $this->passwordFingerprint($user)); + } $store = $this->sessionStore(); $id = $user->getAuthIdentifier(); if ($id !== null && $this->tracksUsersIn($store)) { /** @var UserSessionStore $store */ - $store->destroyUserSessions($id, session()->getId()); + $store->destroyUserSessions($id, $this->currentSessionIds()); } return true; @@ -463,12 +518,24 @@ public function isSessionCurrent(): bool $stored = session($this->getPasswordFingerprintKey()); if (!is_string($stored)) { - $this->refreshSessionFingerprint(); + $this->endSession(false); + + $user = $this->user(); + + if ($user === null) { + return false; + } + + $this->checkedFingerprint = $this->passwordFingerprint($user); return true; } - if (hash_equals($this->passwordFingerprint($user), $stored)) { + $fingerprint = $this->passwordFingerprint($user); + + if (hash_equals($fingerprint, $stored)) { + $this->checkedFingerprint = $fingerprint; + return true; } @@ -478,15 +545,32 @@ public function isSessionCurrent(): bool } /** - * Record the current password of the signed-in user in this device's session. + * Record the user's current password in this device's session, after this request changed it. * + * @param bool $reload * @return void */ - public function refreshSessionFingerprint(): void + public function refreshSessionFingerprint(bool $reload = true): void { - $user = $this->resolvedUser; + if (!session()->has($this->getSessionKey())) { + return; + } + + if ($reload) { + $user = $this->getModel()::find(session($this->getSessionKey())); + } else { + $user = $this->resolvedUser; + + if ($user === null || $this->checkedFingerprint === null || !is_string(session($this->getPasswordFingerprintKey()))) { + return; + } + + if (hash_equals($this->checkedFingerprint, $this->passwordFingerprint($user))) { + return; + } + } - if ($user === null || !session()->has($this->getSessionKey())) { + if (!$user instanceof Authable) { return; } @@ -512,17 +596,34 @@ public function sessions(): array return []; } - $current = session()->getId(); + $current = $this->currentSessionIds(); /** @var UserSessionStore $store */ return array_map(static fn (array $session): array => [ 'ip_address' => $session['ip_address'], 'user_agent' => $session['user_agent'], 'last_activity' => $session['last_activity'], - 'is_current' => hash_equals($session['id'], $current), + 'is_current' => in_array($session['id'], $current, true), ], $store->userSessions($id)); } + /** + * The ids this device's session goes by + * + * @return array + */ + protected function currentSessionIds(): array + { + $session = session(); + $ids = [$session->getId()]; + + if ($session instanceof CommittableSession && ($stored = $session->storedId()) !== null) { + $ids[] = $stored; + } + + return array_values(array_unique($ids)); + } + /** * Write credential columns straight to the user's row. * diff --git a/src/Phaseolies/Console/Commands/SessionTableCommand.php b/src/Phaseolies/Console/Commands/SessionTableCommand.php index 5b5a2b7b..b45b8e28 100644 --- a/src/Phaseolies/Console/Commands/SessionTableCommand.php +++ b/src/Phaseolies/Console/Commands/SessionTableCommand.php @@ -4,6 +4,7 @@ use Phaseolies\Console\Schedule\Command; use Phaseolies\Session\SessionManager; +use Phaseolies\Session\Stores\DatabaseStore; class SessionTableCommand extends Command { @@ -29,16 +30,18 @@ class SessionTableCommand extends Command public function handle(): int { return $this->executeWithTiming(function (): int { - $table = (string) SessionManager::config()['table']; + $config = SessionManager::config(); + $table = (string) $config['table']; + $connection = is_string($config['connection']) && $config['connection'] !== '' ? $config['connection'] : null; - if (preg_match('/^[A-Za-z_][A-Za-z0-9_]*(\.[A-Za-z_][A-Za-z0-9_]*)?\z/', $table) !== 1) { + if (!DatabaseStore::isValidTableName($table)) { $this->displayError("Invalid session table name [{$table}]."); return Command::FAILURE; } $directory = $this->migrationPath(); - $name = 'create_' . str_replace('.', '_', $table) . '_table'; + $name = 'create_' . $table . '_table'; if ($existing = $this->existingMigration($directory, $name)) { $this->displayError('A migration for the session table already exists.'); @@ -56,15 +59,36 @@ public function handle(): int $file = $directory . '/' . date('Y_m_d_His') . '_' . $name . '.php'; $stub = (string) file_get_contents(__DIR__ . '/stubs/session/create_sessions_table.stub'); - file_put_contents($file, str_replace('{{ table }}', $table, $stub)); + file_put_contents($file, str_replace( + ['{{ schema }}', '{{ table }}'], + [$this->schemaAccessor($connection), $table], + $stub + )); $this->displaySuccess('Session table migration created successfully.'); $this->line("📁 File: {$file}"); + if ($connection !== null && $connection !== config('database.default')) { + $this->line("ℹ Run it on the session connection: php pool migrate --connection={$connection}"); + } + return Command::SUCCESS; }); } + /** + * The schema call that targets the connection the sessions are stored on. + * + * @param string|null $connection + * @return string + */ + protected function schemaAccessor(?string $connection): string + { + return $connection === null + ? 'Schema::' + : 'Schema::connection(' . var_export($connection, true) . ')->'; + } + /** * Path of the application's migrations. * diff --git a/src/Phaseolies/Console/Commands/stubs/session/create_sessions_table.stub b/src/Phaseolies/Console/Commands/stubs/session/create_sessions_table.stub index 3f78f945..38fe9056 100644 --- a/src/Phaseolies/Console/Commands/stubs/session/create_sessions_table.stub +++ b/src/Phaseolies/Console/Commands/stubs/session/create_sessions_table.stub @@ -13,7 +13,7 @@ return new class extends Migration */ public function up(): void { - Schema::create('{{ table }}', function (Blueprint $table) { + {{ schema }}create('{{ table }}', function (Blueprint $table) { $table->string('id', 128)->primary(); // For UUID or ULID user keys, use: $table->string('user_id', 36)->nullable()->index(); $table->unsignedBigInteger('user_id')->nullable()->index(); @@ -31,6 +31,6 @@ return new class extends Migration */ public function down(): void { - Schema::dropIfExists('{{ table }}'); + {{ schema }}dropIfExists('{{ table }}'); } }; diff --git a/src/Phaseolies/Middleware/AuthenticateSession.php b/src/Phaseolies/Middleware/AuthenticateSession.php index 022f2f62..a1a9cc1c 100644 --- a/src/Phaseolies/Middleware/AuthenticateSession.php +++ b/src/Phaseolies/Middleware/AuthenticateSession.php @@ -27,8 +27,7 @@ public function __invoke(Request $request, Closure $next, ?string $actor = null) $response = $next($request); - // Keep this device signed in when this request changed the password. - $auth->refreshSessionFingerprint(); + $auth->refreshSessionFingerprint(false); return $response; } diff --git a/src/Phaseolies/Session/Contracts/CommittableSession.php b/src/Phaseolies/Session/Contracts/CommittableSession.php index b2693252..29e23954 100644 --- a/src/Phaseolies/Session/Contracts/CommittableSession.php +++ b/src/Phaseolies/Session/Contracts/CommittableSession.php @@ -52,6 +52,13 @@ public function isStateless(): bool; */ public function isLoaded(): bool; + /** + * The id the session is stored under right now, if it is stored + * + * @return string|null + */ + public function storedId(): ?string; + /** * Run the housekeeping that must not delay the response * diff --git a/src/Phaseolies/Session/Contracts/UserSessionStore.php b/src/Phaseolies/Session/Contracts/UserSessionStore.php index e304d85f..5c58c258 100644 --- a/src/Phaseolies/Session/Contracts/UserSessionStore.php +++ b/src/Phaseolies/Session/Contracts/UserSessionStore.php @@ -13,11 +13,11 @@ interface UserSessionStore extends SessionStoreInterface public function userSessions(int|string $userId): array; /** - * End every session of a user, except the one given. + * End every session of a user, except the ones given and what they were rotated into. * * @param int|string $userId - * @param string|null $exceptId - * @return int Number of ended sessions + * @param array $keep + * @return int */ - public function destroyUserSessions(int|string $userId, ?string $exceptId = null): int; + public function destroyUserSessions(int|string $userId, array $keep = []): int; } diff --git a/src/Phaseolies/Session/RotationPointer.php b/src/Phaseolies/Session/RotationPointer.php new file mode 100644 index 00000000..ae8f325a --- /dev/null +++ b/src/Phaseolies/Session/RotationPointer.php @@ -0,0 +1,63 @@ + $to, self::UNTIL => $until]; + } + + /** + * Whether the session data is a pointer rather than a session. + * + * @param array $data + * @return bool + */ + public static function isPointer(array $data): bool + { + return array_key_exists(self::TO, $data); + } + + /** + * The id a pointer leads to, while it is still allowed to. + * + * @param array $data + * @param bool $honourExpiry + * @return string|null + */ + public static function target(array $data, bool $honourExpiry = true): ?string + { + $target = $data[self::TO] ?? null; + + if (!is_string($target) || !FileStore::isValidId($target)) { + return null; + } + + if ($honourExpiry && (int) ($data[self::UNTIL] ?? 0) < time()) { + return null; + } + + return $target; + } +} diff --git a/src/Phaseolies/Session/Stores/DatabaseStore.php b/src/Phaseolies/Session/Stores/DatabaseStore.php index cf76626f..ae282070 100644 --- a/src/Phaseolies/Session/Stores/DatabaseStore.php +++ b/src/Phaseolies/Session/Stores/DatabaseStore.php @@ -9,6 +9,7 @@ use Phaseolies\Session\Contracts\LockableSessionStore; use Phaseolies\Session\Contracts\UserSessionStore; use Phaseolies\Session\Exceptions\SessionException; +use Phaseolies\Session\RotationPointer; class DatabaseStore implements LockableSessionStore, UserSessionStore { @@ -32,6 +33,11 @@ class DatabaseStore implements LockableSessionStore, UserSessionStore */ protected const PGSQL_LOCK_NAMESPACE = 'doppar_session'; + /** + * Most rotation pointers followed from one session, which only matters for a loop. + */ + protected const MAX_HOPS = 3; + /** * The connection prepared statements belong to. */ @@ -95,11 +101,22 @@ public function __construct( protected ?Closure $metadataResolver = null, protected float $lockTimeout = 2.0 ) { - if (preg_match('/^[A-Za-z_][A-Za-z0-9_]*(\.[A-Za-z_][A-Za-z0-9_]*)?\z/', $table) !== 1) { + if (!self::isValidTableName($table)) { throw new SessionException("Invalid session table name [{$table}]."); } } + /** + * Whether a session table name is a plain identifier the store and the migration quote the same way. + * + * @param string $table + * @return bool + */ + public static function isValidTableName(string $table): bool + { + return preg_match('/^[A-Za-z_][A-Za-z0-9_]*\z/', $table) === 1; + } + /** * Load a session from the value of the session cookie * @@ -331,14 +348,14 @@ public function userSessions(int|string $userId): array } /** - * End every session of a user, except the one given + * End every session of a user, except the ones given and what they were rotated into * * @param int|string $userId - * @param string|null $exceptId + * @param array $keep * @return int * @throws SessionException */ - public function destroyUserSessions(int|string $userId, ?string $exceptId = null): int + public function destroyUserSessions(int|string $userId, array $keep = []): int { $ids = $this->attempt('destroy', function () use ($userId): array { $statement = $this->execute("SELECT id FROM {$this->table()} WHERE user_id = ?", [$userId]); @@ -349,14 +366,15 @@ public function destroyUserSessions(int|string $userId, ?string $exceptId = null return $ids; }); + $keep = $this->lineage($keep); + $ended = 0; - // One by one, each under its own lock: a single bulk DELETE could race - // with a request of that device that is saving its session right now, - // which would write the session back after it was deleted. foreach ($ids as $id) { - if ($id !== $exceptId) { - $this->destroy((string) $id); + $id = (string) $id; + + if (!isset($keep[$id])) { + $this->destroyFollowing($id, $keep); $ended++; } } @@ -364,6 +382,55 @@ public function destroyUserSessions(int|string $userId, ?string $exceptId = null return $ended; } + /** + * The given sessions and every session their rotation pointers lead to. + * + * @param array $ids + * @return array + */ + protected function lineage(array $ids): array + { + $lineage = []; + + foreach ($ids as $id) { + for ($hop = 0; is_string($id) && FileStore::isValidId($id) && !isset($lineage[$id]) && $hop <= self::MAX_HOPS; $hop++) { + $lineage[$id] = true; + + $record = $this->read($id); + + $id = $record === null ? null : RotationPointer::target($record['data'], false); + } + } + + return $lineage; + } + + /** + * End a session and the sessions a rotation made of it, stopping at a kept one. + * + * @param string $id + * @param array $keep + * @return void + */ + protected function destroyFollowing(string $id, array $keep): void + { + for ($hop = 0; $hop <= self::MAX_HOPS; $hop++) { + $next = $this->synchronized($id, function () use ($id): ?string { + $record = $this->read($id); + + $this->destroy($id); + + return $record === null ? null : RotationPointer::target($record['data'], false); + }); + + if (!is_string($next) || isset($keep[$next])) { + return; + } + + $id = $next; + } + } + /** * Run a callback while no other request can save or destroy the same session * diff --git a/src/Phaseolies/Support/Facades/Schema.php b/src/Phaseolies/Support/Facades/Schema.php index 1d9ed2a2..f0c18935 100644 --- a/src/Phaseolies/Support/Facades/Schema.php +++ b/src/Phaseolies/Support/Facades/Schema.php @@ -5,6 +5,7 @@ use Phaseolies\Facade\BaseFacade; /** + * @method static \Phaseolies\Database\Migration\Schema connection(?string $connection) * @method static void create(string $table, callable $callback) * @method static void table(string $table, callable $callback) * @method static void drop(string $table) diff --git a/src/Phaseolies/Support/Session.php b/src/Phaseolies/Support/Session.php index e91dec27..dead6fe2 100644 --- a/src/Phaseolies/Support/Session.php +++ b/src/Phaseolies/Support/Session.php @@ -9,6 +9,7 @@ use Phaseolies\Session\Contracts\LockableSessionStore; use Phaseolies\Session\Contracts\SessionInterface; use Phaseolies\Session\Contracts\SessionStoreInterface; +use Phaseolies\Session\RotationPointer; use Phaseolies\Session\SessionManager; use Phaseolies\Session\Stores\FileStore; use Phaseolies\Utilities\PathMatcher; @@ -26,9 +27,9 @@ class Session implements SessionInterface, CommittableSession * Keys of the pointer an automatically rotated id leaves behind. * */ - protected const ROTATED_TO = '_rotated_to'; + protected const ROTATED_TO = RotationPointer::TO; - protected const ROTATED_UNTIL = '_rotated_until'; + protected const ROTATED_UNTIL = RotationPointer::UNTIL; /** * Seconds the old id of an automatic rotation keeps leading to the new one. @@ -363,7 +364,7 @@ protected function stillCurrent(SessionStoreInterface $store): bool $record = $store->read($this->storedId); - return $record !== null && !array_key_exists(self::ROTATED_TO, $record['data']); + return $record !== null && !RotationPointer::isPointer($record['data']); } /** @@ -441,7 +442,7 @@ protected function saveUnderLock(LockableSessionStore $store): array { $record = $store->read($this->storedId); - if ($record !== null && array_key_exists(self::ROTATED_TO, $record['data'])) { + if ($record !== null && RotationPointer::isPointer($record['data'])) { $target = $this->pointerTarget($record['data']); if ($target !== null) { @@ -500,7 +501,7 @@ protected function persist(LockableSessionStore $store, bool $live): string $this->data['last_regenerated'] = time(); $store->write($new, $this->data); - $store->write($old, [self::ROTATED_TO => $new, self::ROTATED_UNTIL => time() + self::ROTATION_GRACE]); + $store->write($old, RotationPointer::make($new, time() + self::ROTATION_GRACE)); return $this->id = $new; } @@ -529,17 +530,7 @@ protected function follow(string $to): void */ protected function pointerTarget(array $data, bool $honourExpiry = true): ?string { - $target = $data[self::ROTATED_TO] ?? null; - - if (!is_string($target) || !FileStore::isValidId($target)) { - return null; - } - - if ($honourExpiry && (int) ($data[self::ROTATED_UNTIL] ?? 0) < time()) { - return null; - } - - return $target; + return RotationPointer::target($data, $honourExpiry); } /** @@ -554,7 +545,7 @@ protected function readFollowing(SessionStoreInterface $store, string $id): ?arr for ($hop = 0; $hop <= self::MAX_HOPS; $hop++) { $record = $store->read($id); - if ($record === null || !array_key_exists(self::ROTATED_TO, $record['data'])) { + if ($record === null || !RotationPointer::isPointer($record['data'])) { return $record; } @@ -1094,6 +1085,18 @@ public function getId(): string return $this->id ??= self::newId(); } + /** + * The id the session is stored under right now, if it is stored. + * + * @return string|null + */ + public function storedId(): ?string + { + $this->load(); + + return $this->storedId; + } + /** * Set the session ID. * diff --git a/tests/Auth/LogoutOtherDevicesTest.php b/tests/Auth/LogoutOtherDevicesTest.php index f7526a02..c4b321cc 100644 --- a/tests/Auth/LogoutOtherDevicesTest.php +++ b/tests/Auth/LogoutOtherDevicesTest.php @@ -18,6 +18,7 @@ use Phaseolies\Http\ServerBag; use Phaseolies\Middleware\AuthenticateSession; use Phaseolies\Session\Contracts\SessionStoreInterface; +use Phaseolies\Session\RotationPointer; use Phaseolies\Session\SessionManager; use Phaseolies\Session\Stores\ArrayStore; use Phaseolies\Session\Stores\DatabaseStore; @@ -72,6 +73,21 @@ class LockedDownUser extends Authable protected $creatable = ['email']; } +/** + * A model that reads its password from another column, but only overrides getAuthPassword(). + */ +class RenamedPasswordUser extends Authable +{ + protected $table = 'users'; + + protected $timeStamps = false; + + public function getAuthPassword(): ?string + { + return $this->passwd; + } +} + /** * Signs one user in on several devices, each a separate request with its own * session cookie, and checks what logoutOtherDevices() leaves signed in. @@ -266,17 +282,217 @@ public function test_changing_the_password_during_a_request_keeps_that_device_si $this->assertFalse($this->isCurrentAndSignedIn($phone)); } - public function test_a_session_from_before_fingerprints_is_accepted_and_fingerprinted(): void + public function test_a_session_from_before_fingerprints_does_not_survive_signing_others_out(): void { + $this->useStore(new ArrayStore(7200)); + $laptop = $this->signIn('Laptop'); + $phone = $this->signIn('Phone'); + $this->onDevice($phone, fn (Authenticate $auth, Request $request) => $request->session()->forget('password_hash_web')); - $this->onDevice($laptop, fn (Authenticate $auth, Request $request) => $request->session()->forget('password_hash_web')); + $this->onDevice($laptop, fn (Authenticate $auth) => $auth->logoutOtherDevices(self::PASSWORD)); + + [, $current] = $this->onDevice($phone, fn (Authenticate $auth) => $auth->isSessionCurrent()); - [, $current] = $this->onDevice($laptop, fn (Authenticate $auth) => $auth->isSessionCurrent()); - [, $fingerprinted] = $this->onDevice($laptop, fn (Authenticate $auth, Request $request) => $request->session()->has('password_hash_web')); + $this->assertFalse($current); + $this->assertFalse($this->isSignedIn($phone)); + } + + public function test_a_session_from_before_fingerprints_with_a_valid_remember_cookie_signs_back_in(): void + { + $this->useStore(new ArrayStore(7200)); + + [$laptop, $remember] = $this->signInRemembered('Laptop'); + $this->onDevice($laptop, fn (Authenticate $auth, Request $request) => $request->session()->forget('password_hash_web'), $remember); + + [$laptop, $current] = $this->onDevice($laptop, fn (Authenticate $auth) => $auth->isSessionCurrent(), $remember); + [, $fingerprinted] = $this->onDevice($laptop, fn (Authenticate $auth, Request $request) => $request->session()->has('password_hash_web'), $remember); $this->assertTrue($current); $this->assertTrue($fingerprinted); + $this->assertNotNull($this->storedRememberToken(), 'The remember token was left alone.'); + } + + public function test_a_session_from_before_fingerprints_with_a_replaced_remember_cookie_is_signed_out(): void + { + $this->useStore(new ArrayStore(7200)); + + $laptop = $this->signIn('Laptop'); + [$phone, $phoneRemember] = $this->signInRemembered('Phone'); + $this->onDevice($phone, fn (Authenticate $auth, Request $request) => $request->session()->forget('password_hash_web'), $phoneRemember); + + $this->onDevice($laptop, fn (Authenticate $auth) => $auth->logoutOtherDevices(self::PASSWORD)); + + [, $current] = $this->onDevice($phone, fn (Authenticate $auth) => $auth->isSessionCurrent() && $auth->check(), $phoneRemember); + + $this->assertFalse($current); + } + + public function test_a_stale_remember_cookie_does_not_sign_out_the_device_holding_the_current_one(): void + { + // One token per user: the laptop, remembered last, holds the current one. + [, $phoneRemember] = $this->signInRemembered('Phone'); + [$laptop, $laptopRemember] = $this->signInRemembered('Laptop'); + + $issued = []; + $this->onDevice($laptop, function (Authenticate $auth) use (&$issued): bool { + return $this->capturingCookies($issued, fn () => $auth->logoutOtherDevices(self::PASSWORD)); + }, $laptopRemember); + + // The phone comes back with its old remember cookie. + [, $phoneSignedIn] = $this->onDevice(null, fn (Authenticate $auth) => $auth->check(), $phoneRemember); + + $this->assertFalse($phoneSignedIn); + $this->assertNotNull($this->storedRememberToken(), 'The current remember token survives.'); + + [, $laptopSignedIn] = $this->onDevice(null, fn (Authenticate $auth) => $auth->check(), $issued); + + $this->assertTrue($laptopSignedIn, "The laptop's new remember cookie still works."); + } + + public function test_signing_others_out_right_after_regenerating_the_session_keeps_this_device(): void + { + $laptop = $this->signIn('Laptop'); + $phone = $this->signIn('Phone'); + + [$laptop] = $this->onDevice($laptop, function (Authenticate $auth, Request $request): bool { + $request->session()->regenerate(); + + return $auth->logoutOtherDevices(self::PASSWORD); + }); + + $this->assertTrue($this->isSignedIn($laptop)); + $this->assertFalse($this->isSignedIn($phone)); + $this->assertSame(1, $this->sessionRows()); + } + + public function test_signing_others_out_right_after_signing_in_again_keeps_this_device(): void + { + $laptop = $this->signIn('Laptop'); + $phone = $this->signIn('Phone'); + + [$laptop] = $this->onDevice($laptop, function (Authenticate $auth): bool { + $auth->login(DeviceUser::find(1)); + + return $auth->logoutOtherDevices(self::PASSWORD); + }); + + $this->assertTrue($this->isSignedIn($laptop)); + $this->assertFalse($this->isSignedIn($phone)); + } + + public function test_signing_others_out_keeps_this_device_when_a_concurrent_request_rotated_its_id(): void + { + $laptop = $this->signIn('Laptop'); + $phone = $this->signIn('Phone'); + $rotated = bin2hex(random_bytes(20)); + + $this->onDevice($laptop, function (Authenticate $auth, Request $request) use ($laptop, $rotated): bool { + $auth->user(); + + // Another request of the laptop rotates its id meanwhile and hands + // the browser a cookie for the new one. + $this->store->write($rotated, $this->store->read($laptop)['data']); + $this->store->write($laptop, RotationPointer::make($rotated, time() + 60)); + + return $auth->logoutOtherDevices(self::PASSWORD); + }); + + $this->assertTrue($this->isSignedIn($rotated)); + $this->assertFalse($this->isSignedIn($phone)); + } + + public function test_sessions_marks_this_device_as_current_after_regenerating(): void + { + $laptop = $this->signIn('Laptop'); + $this->signIn('Phone'); + + [, $sessions] = $this->onDevice($laptop, function (Authenticate $auth, Request $request): array { + $request->session()->regenerate(); + + return $auth->sessions(); + }); + + $this->assertSame(['Laptop'], array_column(array_filter($sessions, fn (array $s) => $s['is_current']), 'user_agent')); + } + + public function test_a_password_changed_with_a_query_keeps_this_device_after_refreshing_the_fingerprint(): void + { + $this->useStore(new ArrayStore(7200)); + + $laptop = $this->signIn('Laptop'); + $phone = $this->signIn('Phone'); + + $this->onDevice($laptop, function (Authenticate $auth, Request $request): void { + (new AuthenticateSession())($request, function (): Response { + DeviceUser::where('id', 1)->update(['password' => password_hash('a new password', PASSWORD_BCRYPT, ['cost' => 4])]); + app('auth')->refreshSessionFingerprint(); + + return new Response(); + }); + }); + + $this->assertTrue($this->isCurrentAndSignedIn($laptop)); + $this->assertFalse($this->isCurrentAndSignedIn($phone)); + } + + public function test_the_middleware_does_not_adopt_a_password_another_device_changed_meanwhile(): void + { + $this->useStore(new ArrayStore(7200)); + + $laptop = $this->signIn('Laptop'); + $phone = $this->signIn('Phone'); + + // A request of the phone is running while the laptop signs it out. + $this->onDevice($phone, function (Authenticate $auth, Request $request) use ($laptop): void { + (new AuthenticateSession())($request, function () use ($laptop, $request): Response { + $this->onDevice($laptop, fn (Authenticate $auth) => $auth->logoutOtherDevices(self::PASSWORD)); + + // Back to the phone's request. + Container::getInstance()->instance('request', $request); + Container::getInstance()->instance('session', $request->session); + $_COOKIE = []; + + return new Response(); + }); + }); + + $this->assertFalse($this->isCurrentAndSignedIn($phone)); + $this->assertTrue($this->isCurrentAndSignedIn($laptop)); + } + + public function test_a_model_that_only_overrides_get_auth_password_is_refused(): void + { + $pdo = Database::getPdoInstance(self::CONNECTION); + $pdo->exec('ALTER TABLE users ADD COLUMN passwd TEXT'); + $pdo->exec("UPDATE users SET passwd = password, password = 'not the password'"); + Config::set('auth.actors.web.model', RenamedPasswordUser::class); + + $laptop = $this->onDevice(null, fn (Authenticate $auth) => $auth->login(RenamedPasswordUser::find(1)))[0]; + $before = $this->storedPassword(); + + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('getAuthPasswordName()'); + + try { + $this->onDevice($laptop, fn (Authenticate $auth) => $auth->logoutOtherDevices(self::PASSWORD)); + } finally { + $this->assertSame($before, $this->storedPassword(), 'Nothing was written.'); + } + } + + public function test_an_invalid_remember_cookie_does_not_earn_a_new_remember_token(): void + { + $laptop = $this->signIn('Laptop'); + $forged = ['remember_doppar_web_' . sha1('doppar') => \Phaseolies\Support\Facades\Crypt::encrypt('1|nottherealtoken|' . password_hash('1nottherealtoken', PASSWORD_BCRYPT, ['cost' => 4]))]; + + $issued = []; + $this->onDevice($laptop, function (Authenticate $auth) use (&$issued): bool { + return $this->capturingCookies($issued, fn () => $auth->logoutOtherDevices(self::PASSWORD)); + }, $forged); + + $this->assertSame([], $issued, 'No remember cookie was issued.'); + $this->assertNull($this->storedRememberToken()); } public function test_the_fingerprint_does_not_reveal_the_password_hash(): void diff --git a/tests/Console/SessionTableCommandTest.php b/tests/Console/SessionTableCommandTest.php index 233c104f..06ece85f 100644 --- a/tests/Console/SessionTableCommandTest.php +++ b/tests/Console/SessionTableCommandTest.php @@ -69,6 +69,43 @@ public function test_it_does_not_create_a_second_migration(): void $this->assertCount(1, glob($this->dir . '/*.php')); } + public function test_it_creates_the_table_on_the_configured_session_connection(): void + { + Config::set('database.default', 'mysql'); + Config::set('session.connection', 'sessions_db'); + + $tester = $this->run_(); + $migration = (string) file_get_contents(glob($this->dir . '/*_create_sessions_table.php')[0]); + + $this->assertStringContainsString("Schema::connection('sessions_db')->create('sessions'", $migration); + $this->assertStringContainsString("Schema::connection('sessions_db')->dropIfExists('sessions')", $migration); + $this->assertStringNotContainsString('{{ schema }}', $migration); + $this->assertStringContainsString('migrate --connection=sessions_db', $tester->getDisplay()); + + // The pattern the migrator assigns a migration to its connection by. + $this->assertMatchesRegularExpression("/Schema::connection\(\s*['\"]sessions_db['\"]\s*\)/i", $migration); + } + + public function test_it_uses_the_default_connection_when_none_is_configured(): void + { + $this->run_(); + + $migration = (string) file_get_contents(glob($this->dir . '/*_create_sessions_table.php')[0]); + + $this->assertStringContainsString("Schema::create('sessions'", $migration); + $this->assertStringNotContainsString('Schema::connection', $migration); + } + + public function test_it_rejects_a_schema_qualified_table_name(): void + { + Config::set('session.table', 'app.sessions'); + + $tester = $this->run_(); + + $this->assertSame(1, $tester->getStatusCode()); + $this->assertSame([], glob($this->dir . '/*.php') ?: []); + } + public function test_it_rejects_an_unsafe_table_name(): void { Config::set('session.table', 'sessions; DROP TABLE users'); diff --git a/tests/Session/DatabaseStoreTest.php b/tests/Session/DatabaseStoreTest.php index 8b6c502c..23d218cd 100644 --- a/tests/Session/DatabaseStoreTest.php +++ b/tests/Session/DatabaseStoreTest.php @@ -9,6 +9,7 @@ use Phaseolies\Database\Migration\Schema; use Phaseolies\DI\Container; use Phaseolies\Session\Exceptions\SessionException; +use Phaseolies\Session\RotationPointer; use Phaseolies\Session\Stores\DatabaseStore; use Phaseolies\Support\Facades\Facade; use Tests\Support\MockContainer; @@ -278,7 +279,7 @@ public function test_destroy_user_sessions_ends_every_session_of_the_user_except $store->write($phone = $this->id(), ['user' => 5]); $store->write($someoneElse = $this->id(), ['user' => 6]); - $this->assertSame(2, $store->destroyUserSessions(5, $current)); + $this->assertSame(2, $store->destroyUserSessions(5, [$current])); $this->assertNotNull($store->read($current)); $this->assertNull($store->read($laptop)); @@ -286,6 +287,65 @@ public function test_destroy_user_sessions_ends_every_session_of_the_user_except $this->assertNotNull($store->read($someoneElse)); } + public function test_destroy_user_sessions_keeps_what_a_kept_session_was_rotated_into(): void + { + $store = $this->store(user: fn (array $data) => $data['user'] ?? null); + + // This device's id was rotated by a concurrent request: the old id is + // a pointer and the session lives on under the new id. + $store->write($new = $this->id(), ['user' => 5]); + $store->write($old = $this->id(), RotationPointer::make($new, time() + 60)); + $store->write($phone = $this->id(), ['user' => 5]); + + $this->assertSame(1, $store->destroyUserSessions(5, [$old])); + + $this->assertNotNull($store->read($new)); + $this->assertNull($store->read($phone)); + } + + public function test_destroy_user_sessions_follows_a_rotation_that_happens_while_it_runs(): void + { + $phone = $this->id(); + $rotated = $this->id(); + + // Rotates the phone's session after the sessions were listed and + // before they are destroyed. + $store = new class (fn () => $this->pdo(), static::TABLE, $phone, $rotated) extends DatabaseStore { + public function __construct(\Closure $connection, string $table, private string $phone, private string $rotated) + { + parent::__construct($connection, $table, 7200, fn (array $data) => $data['user'] ?? null); + } + + protected function lineage(array $ids): array + { + $this->write($this->rotated, ['user' => 5]); + $this->write($this->phone, RotationPointer::make($this->rotated, time() + 60)); + + return parent::lineage($ids); + } + }; + + $store->write($current = $this->id(), ['user' => 5]); + $store->write($phone, ['user' => 5]); + + $store->destroyUserSessions(5, [$current]); + + $this->assertNull($store->read($phone)); + $this->assertNull($store->read($rotated), 'The session the phone was rotated into is ended too.'); + $this->assertNotNull($store->read($current)); + } + + public function test_a_schema_qualified_table_name_is_rejected(): void + { + // The migration would quote it as one identifier and the store as two. + $this->assertFalse(DatabaseStore::isValidTableName('app.sessions')); + $this->assertTrue(DatabaseStore::isValidTableName('user_sessions')); + + $this->expectException(SessionException::class); + + new DatabaseStore(fn () => $this->pdo(), 'app.sessions'); + } + public function test_a_callback_under_a_lock_can_use_the_store_without_waiting_for_itself(): void { $store = $this->store(); @@ -471,7 +531,12 @@ protected function migrate(): void $stub = (string) file_get_contents(dirname(__DIR__, 2) . '/src/Phaseolies/Console/Commands/stubs/session/create_sessions_table.stub'); $file = $this->dir . '/create_sessions_table.php'; - file_put_contents($file, str_replace('{{ table }}', static::TABLE, $stub)); + // As generated for a configured session.connection. + file_put_contents($file, str_replace( + ['{{ schema }}', '{{ table }}'], + ["Schema::connection('" . static::CONNECTION . "')->", static::TABLE], + $stub + )); (require $file)->up(); } From 9941f047f2329830b880aa52ab9a570696af258c Mon Sep 17 00:00:00 2001 From: techmahedy Date: Thu, 5 Nov 2026 23:19:04 +0600 Subject: [PATCH 3/6] fix(session): keep this device signed in across parallel requests and actors when logging out other devices --- src/Phaseolies/Auth/Security/Authenticate.php | 176 ++++++++++++++++-- .../Security/InteractsWithRememberCookie.php | 34 +++- .../stubs/session/create_sessions_table.stub | 2 +- .../Session/Contracts/CommittableSession.php | 26 +++ .../Session/Stores/DatabaseStore.php | 19 +- src/Phaseolies/Support/Session.php | 104 ++++++++++- tests/Auth/ActorManagerTest.php | 14 ++ tests/Auth/AuthenticateTest.php | 7 + tests/Auth/LogoutOtherDevicesTest.php | 111 ++++++++++- tests/Auth/PasswordHashingTest.php | 15 +- tests/Session/DatabaseStoreTest.php | 25 +++ 11 files changed, 492 insertions(+), 41 deletions(-) diff --git a/src/Phaseolies/Auth/Security/Authenticate.php b/src/Phaseolies/Auth/Security/Authenticate.php index 16f7fe75..efa1e45c 100644 --- a/src/Phaseolies/Auth/Security/Authenticate.php +++ b/src/Phaseolies/Auth/Security/Authenticate.php @@ -463,29 +463,37 @@ public function logoutOtherDevices(string $password): bool )); } - $changes = [$passwordName => Hash::make($password)]; + $hash = Hash::make($password); + $changes = [$passwordName => $hash]; + + $actors = $this->actorsSignedInAs($user); $tokenName = $user->getRememberTokenName(); if (array_key_exists($tokenName, $user->getAttributes())) { - if ($this->hasValidRememberCookie($user)) { - $this->issueRememberToken($user); - } else { + $remembered = array_values(array_filter( + $actors, + static fn (self $actor): bool => $actor->hasValidRememberCookie($user) + )); + + if ($remembered === []) { $user->setRememberToken(null); + } else { + $token = $this->newRememberToken($user); - if (cookie()->has($this->getRememberCookieName())) { - $this->expireRememberCookie(); + foreach ($remembered as $actor) { + $actor->sendRememberCookie($user, $token); } } + if (!in_array($this, $remembered, true) && cookie()->has($this->getRememberCookieName())) { + $this->expireRememberCookie(); + } + $changes[$tokenName] = $user->getRememberToken(); } - $this->persistCredentials($user, $changes); - - if (session()->has($this->getSessionKey())) { - session()->put($this->getPasswordFingerprintKey(), $this->passwordFingerprint($user)); - } + $this->replaceCredentials($user, $changes, $actors, $this->fingerprintOf($hash)); $store = $this->sessionStore(); $id = $user->getAuthIdentifier(); @@ -518,7 +526,7 @@ public function isSessionCurrent(): bool $stored = session($this->getPasswordFingerprintKey()); if (!is_string($stored)) { - $this->endSession(false); + $this->signOutKeepingSession(); $user = $this->user(); @@ -539,11 +547,54 @@ public function isSessionCurrent(): bool return true; } + $current = $this->fingerprintStoredSince(); + + if ($current !== null) { + session()->put($this->getPasswordFingerprintKey(), $current); + $this->checkedFingerprint = $current; + + return true; + } + $this->endSession(); return false; } + /** + * The fingerprint this device's session holds now, when it matches the + * password in the database now. + * + * @return string|null + */ + protected function fingerprintStoredSince(): ?string + { + $session = session(); + + if (!$session instanceof CommittableSession) { + return null; + } + + return $session->exclusively(function () use ($session): ?string { + $stored = $session->storedValue($this->getPasswordFingerprintKey()); + $user = $this->getModel()::find(session($this->getSessionKey())); + + if (!is_string($stored) || !$user instanceof Authable) { + return null; + } + + $fingerprint = $this->passwordFingerprint($user); + + if (!hash_equals($fingerprint, $stored)) { + return null; + } + + $this->resolvedUser = $user; + + return $fingerprint; + }); + } + /** * Record the user's current password in this device's session, after this request changed it. * @@ -624,6 +675,94 @@ protected function currentSessionIds(): array return array_values(array_unique($ids)); } + /** + * Save the new credentials, and the new fingerprint of every actor + * + * @param Authable $user + * @param array $changes + * @param array $actors + * @param string $fingerprint + * @return void + */ + protected function replaceCredentials(Authable $user, array $changes, array $actors, string $fingerprint): void + { + $session = session(); + + $put = static function (string $key, mixed $value) use ($session): void { + $session instanceof CommittableSession ? $session->putThrough($key, $value) : $session->put($key, $value); + }; + + $replace = function () use ($user, $changes, $actors, $fingerprint, $put): void { + $previous = []; + + foreach ($actors as $actor) { + if (session()->has($actor->getSessionKey())) { + $key = $actor->getPasswordFingerprintKey(); + $previous[$key] = session($key); + $put($key, $fingerprint); + } + } + + try { + $this->persistCredentials($user, $changes); + } catch (\Throwable $e) { + foreach ($previous as $key => $value) { + $put($key, $value); + } + + throw $e; + } + }; + + $session instanceof CommittableSession ? $session->exclusively($replace) : $replace(); + } + + /** + * The actors this device is signed in to as the user, this one first. + * + * @param Authable $user + * @return array + */ + protected function actorsSignedInAs(Authable $user): array + { + $actors = [$this]; + $id = (string) $user->getAuthIdentifier(); + + foreach (array_keys((array) config('auth.actors', [])) as $name) { + if ($name === $this->actorName) { + continue; + } + + $actor = app('auth')->actor($name); + + if ( + $actor instanceof self + && (string) session($actor->getSessionKey()) === $id + && $actor->getModel()->getTable() === $user->getTable() + ) { + $actors[] = $actor; + } + } + + return $actors; + } + + /** + * Sign this actor out without ending the session, keeping its other data. + * + * @return void + */ + protected function signOutKeepingSession(): void + { + $this->resolvedUser = null; + $this->checkedFingerprint = null; + + session()->forget($this->getSessionKey()); + session()->forget($this->getViaRememberKey()); + session()->forget($this->getPasswordFingerprintKey()); + session()->regenerate(); + } + /** * Write credential columns straight to the user's row. * @@ -652,7 +791,18 @@ protected function persistCredentials(Authable $user, array $changes): void */ protected function passwordFingerprint(Authable $user): string { - return hash_hmac('sha256', (string) $user->getAuthPassword(), (string) config('app.key', '')); + return $this->fingerprintOf($user->getAuthPassword()); + } + + /** + * A fingerprint of a password hash, safe to keep in the session. + * + * @param string|null $hash + * @return string + */ + protected function fingerprintOf(?string $hash): string + { + return hash_hmac('sha256', (string) $hash, (string) config('app.key', '')); } /** diff --git a/src/Phaseolies/Auth/Security/InteractsWithRememberCookie.php b/src/Phaseolies/Auth/Security/InteractsWithRememberCookie.php index 1df861e2..b66bf60d 100644 --- a/src/Phaseolies/Auth/Security/InteractsWithRememberCookie.php +++ b/src/Phaseolies/Auth/Security/InteractsWithRememberCookie.php @@ -44,19 +44,45 @@ private function setRememberToken(Authable $user): void * Give the user a new remember token and send it to this device, without saving the user. * * @param Authable $user - * @return void + * @return string + */ + private function issueRememberToken(Authable $user): string + { + $token = $this->newRememberToken($user); + + $this->sendRememberCookie($user, $token); + + return $token; + } + + /** + * Give the user a new remember token, without sending it anywhere or saving the user. + * + * @param Authable $user + * @return string */ - private function issueRememberToken(Authable $user): void + private function newRememberToken(Authable $user): string { $token = bin2hex(random_bytes(32)); $user->setRememberToken(Hash::make($token)); + return $token; + } + + /** + * Send this actor's remember cookie for a token the user already holds. + * + * @param Authable $user + * @param string $token + * @return void + */ + private function sendRememberCookie(Authable $user, string $token): void + { $id = $user->getAuthIdentifier(); - $cookieValue = $id . '|' . $token . '|' . Hash::make($id . $token); session()->put($this->getViaRememberKey(), true); - $this->setRememberCookie($cookieValue); + $this->setRememberCookie($id . '|' . $token . '|' . Hash::make($id . $token)); } /** diff --git a/src/Phaseolies/Console/Commands/stubs/session/create_sessions_table.stub b/src/Phaseolies/Console/Commands/stubs/session/create_sessions_table.stub index 38fe9056..bf289855 100644 --- a/src/Phaseolies/Console/Commands/stubs/session/create_sessions_table.stub +++ b/src/Phaseolies/Console/Commands/stubs/session/create_sessions_table.stub @@ -20,7 +20,7 @@ return new class extends Migration $table->string('ip_address', 45)->nullable(); $table->text('user_agent')->nullable(); $table->longText('payload'); - $table->unsignedInteger('last_activity')->index(); + $table->unsignedBigInteger('last_activity')->index(); }); } diff --git a/src/Phaseolies/Session/Contracts/CommittableSession.php b/src/Phaseolies/Session/Contracts/CommittableSession.php index 29e23954..3a541362 100644 --- a/src/Phaseolies/Session/Contracts/CommittableSession.php +++ b/src/Phaseolies/Session/Contracts/CommittableSession.php @@ -59,6 +59,32 @@ public function isLoaded(): bool; */ public function storedId(): ?string; + /** + * Run a callback while no other request can save this session + * + * @param callable $callback + * @return mixed + */ + public function exclusively(callable $callback): mixed; + + /** + * A value as the stored session holds it right now + * + * @param string $key + * @param mixed $default + * @return mixed + */ + public function storedValue(string $key, mixed $default = null): mixed; + + /** + * Put a value, and save it into the stored session right away + * + * @param string $key + * @param mixed $value + * @return void + */ + public function putThrough(string $key, mixed $value): void; + /** * Run the housekeeping that must not delay the response * diff --git a/src/Phaseolies/Session/Stores/DatabaseStore.php b/src/Phaseolies/Session/Stores/DatabaseStore.php index ae282070..b8979e47 100644 --- a/src/Phaseolies/Session/Stores/DatabaseStore.php +++ b/src/Phaseolies/Session/Stores/DatabaseStore.php @@ -226,9 +226,11 @@ public function touch(string $id): void return; } + $metadata = $this->metadata(); + $this->attempt('touch', fn() => $this->execute( - "UPDATE {$this->table()} SET last_activity = ? WHERE id = ? AND last_activity < ?", - [$now, $id, $now - $threshold] + "UPDATE {$this->table()} SET last_activity = ?, ip_address = ?, user_agent = ? WHERE id = ? AND last_activity < ?", + [$now, $metadata['ip_address'], $metadata['user_agent'], $id, $now - $threshold] )); $this->remember($id, $now); @@ -725,16 +727,13 @@ protected function driver(): string */ protected function table(): string { - if ($this->quotedTable !== '') { - return $this->quotedTable; - } + if ($this->quotedTable === '') { + $quote = $this->driver() === 'mysql' ? '`' : '"'; - $quote = $this->driver() === 'mysql' ? '`' : '"'; + $this->quotedTable = $quote . $this->table . $quote; + } - return $this->quotedTable = implode('.', array_map( - static fn(string $part): string => $quote . $part . $quote, - explode('.', $this->table) - )); + return $this->quotedTable; } /** diff --git a/src/Phaseolies/Support/Session.php b/src/Phaseolies/Support/Session.php index dead6fe2..5156c515 100644 --- a/src/Phaseolies/Support/Session.php +++ b/src/Phaseolies/Support/Session.php @@ -23,14 +23,6 @@ class Session implements SessionInterface, CommittableSession */ protected const INTERNAL_KEYS = ['last_regenerated' => true]; - /** - * Keys of the pointer an automatically rotated id leaves behind. - * - */ - protected const ROTATED_TO = RotationPointer::TO; - - protected const ROTATED_UNTIL = RotationPointer::UNTIL; - /** * Seconds the old id of an automatic rotation keeps leading to the new one. */ @@ -1097,6 +1089,102 @@ public function storedId(): ?string return $this->storedId; } + /** + * Run a callback while no other request can save this session. + * + * @param callable $callback + * @return mixed + */ + public function exclusively(callable $callback): mixed + { + $this->load(); + + $store = $this->store(); + + if ($this->storedId === null || $this->isStateless() || !$store instanceof LockableSessionStore) { + return $callback(); + } + + return $store->synchronized($this->storedId, $callback); + } + + /** + * A value as the stored session holds it + * + * @param string $key + * @param mixed $default + * @return mixed + */ + public function storedValue(string $key, mixed $default = null): mixed + { + $this->load(); + + $store = $this->store(); + + if ($this->storedId === null || $this->isStateless() || $store->isClientSide()) { + return $this->data[$key] ?? $default; + } + + $record = $this->readFollowing($store, $this->storedId); + + return $record['data'][$key] ?? $default; + } + + /** + * Put a value, and save it into the stored session right away + * + * @param string $key + * @param mixed $value + * @return void + */ + public function putThrough(string $key, mixed $value): void + { + $this->put($key, $value); + + $store = $this->store(); + + if ($this->storedId === null || $this->isStateless() || $store->isClientSide()) { + return; + } + + $id = $this->storedId; + + for ($hop = 0; is_string($id) && $hop <= self::MAX_HOPS; $hop++) { + $id = $this->writeStoredValue($store, $id, $key, $value); + } + } + + /** + * Write one value into a stored session, or return where a rotation moved it. + * + * @param SessionStoreInterface $store + * @param string $id + * @param string $key + * @param mixed $value + * @return string|null + */ + protected function writeStoredValue(SessionStoreInterface $store, string $id, string $key, mixed $value): ?string + { + $write = function () use ($store, $id, $key, $value): ?string { + $record = $store->read($id); + + if ($record === null) { + return null; + } + + if (RotationPointer::isPointer($record['data'])) { + return $this->pointerTarget($record['data']); + } + + $record['data'][$key] = $value; + $store->write($id, $record['data']); + + return null; + }; + + return $store instanceof LockableSessionStore ? $store->synchronized($id, $write) : $write(); + } + /** * Set the session ID. * diff --git a/tests/Auth/ActorManagerTest.php b/tests/Auth/ActorManagerTest.php index 1f0cdeae..24fe1804 100644 --- a/tests/Auth/ActorManagerTest.php +++ b/tests/Auth/ActorManagerTest.php @@ -1,8 +1,16 @@ 'web', 'auth.actors.web' => [ @@ -42,9 +50,15 @@ class ActorManagerTest extends TestCase protected function setUp(): void { + $GLOBALS['doppar_actor_manager_test'] = true; $this->manager = new ActorManager(); } + protected function tearDown(): void + { + unset($GLOBALS['doppar_actor_manager_test']); + } + public function testActorReturnsAuthenticateInstance() { $actor = $this->manager->actor('web'); diff --git a/tests/Auth/AuthenticateTest.php b/tests/Auth/AuthenticateTest.php index adcb81b0..73889329 100644 --- a/tests/Auth/AuthenticateTest.php +++ b/tests/Auth/AuthenticateTest.php @@ -109,6 +109,8 @@ public function isApiRequest(): bool use Phaseolies\Auth\Authable; use Phaseolies\Auth\Security\Authenticate; + use Phaseolies\DI\Container; + use Tests\Support\MockContainer; use PHPUnit\Framework\TestCase; class FakeAuthableModel extends Authable @@ -160,6 +162,8 @@ class AuthenticateTest extends TestCase protected function tearDown(): void { $GLOBALS['authenticateStubsActive'] = false; + + (new \ReflectionClass(Container::class))->getProperty('instance')->setValue(null, null); } protected function setUp(): void @@ -168,6 +172,9 @@ protected function setUp(): void $GLOBALS['authenticateStubsActive'] = true; + // Logins fingerprint the password with the app key, read through config(). + Container::setInstance(new MockContainer()); + $authenticateSessionStore = new \Phaseolies\Auth\Security\TestSessionStore(); FakeAuthableModel::$resolvedUser = null; } diff --git a/tests/Auth/LogoutOtherDevicesTest.php b/tests/Auth/LogoutOtherDevicesTest.php index c4b321cc..681ff4e1 100644 --- a/tests/Auth/LogoutOtherDevicesTest.php +++ b/tests/Auth/LogoutOtherDevicesTest.php @@ -461,6 +461,105 @@ public function test_the_middleware_does_not_adopt_a_password_another_device_cha $this->assertTrue($this->isCurrentAndSignedIn($laptop)); } + /** + * @return array + */ + public static function serverSideStores(): array + { + return [ + 'database' => [fn (self $test) => $test->databaseStore()], + 'array' => [fn () => new ArrayStore(7200)], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('serverSideStores')] + public function test_a_parallel_request_of_this_device_does_not_sign_it_out(\Closure $store): void + { + $this->useStore($store($this)); + + $laptop = $this->signIn('Laptop'); + $phone = $this->signIn('Phone'); + + $this->onDevice($laptop, function (Authenticate $auth, Request $request) use ($laptop): bool { + $result = $auth->logoutOtherDevices(self::PASSWORD); + + // Another request of the laptop runs before this one saves its session. + [, $signedIn] = $this->onDevice($laptop, function (Authenticate $auth, Request $request): bool { + $signedIn = null; + + (new AuthenticateSession())($request, function () use (&$signedIn): Response { + $signedIn = app('auth')->check(); + + return new Response(); + }); + + return $signedIn; + }); + + $this->backTo($request); + $this->assertTrue($signedIn, 'The parallel request is still signed in.'); + + return $result; + }); + + $this->assertTrue($this->isCurrentAndSignedIn($laptop)); + $this->assertFalse($this->isCurrentAndSignedIn($phone)); + } + + public function test_a_parallel_request_that_read_the_user_before_the_change_does_not_sign_this_device_out(): void + { + $laptop = $this->signIn('Laptop'); + + // A request of the laptop has loaded its user, then this device signs the others out. + $this->onDevice($laptop, function (Authenticate $auth, Request $request) use ($laptop): void { + $auth->user(); + + $this->onDevice($laptop, fn (Authenticate $auth) => $auth->logoutOtherDevices(self::PASSWORD)); + $this->backTo($request); + + $this->assertTrue($auth->isSessionCurrent(), 'It checks again against what is stored now.'); + }); + + $this->assertTrue($this->isCurrentAndSignedIn($laptop)); + } + + public function test_signing_others_out_through_another_actor_keeps_both_actors_signed_in(): void + { + $laptop = $this->signIn('Laptop'); + + [$laptop] = $this->onDevice($laptop, function (Authenticate $auth): bool { + $admin = app('auth')->actor('admin'); + $admin->login(DeviceUser::find(1)); + + return $admin->logoutOtherDevices(self::PASSWORD); + }); + + [, $both] = $this->onDevice($laptop, function (Authenticate $auth): array { + $admin = app('auth')->actor('admin'); + + return [$auth->isSessionCurrent() && $auth->check(), $admin->isSessionCurrent() && $admin->check()]; + }); + + $this->assertSame([true, true], $both); + } + + public function test_signing_out_a_session_from_before_fingerprints_keeps_its_other_data(): void + { + $this->useStore(new ArrayStore(7200)); + + $laptop = $this->signIn('Laptop'); + $this->onDevice($laptop, function (Authenticate $auth, Request $request): void { + $request->session()->forget('password_hash_web'); + $request->session()->put('cart', [42]); + }); + + [$laptop, $current] = $this->onDevice($laptop, fn (Authenticate $auth) => $auth->isSessionCurrent()); + [, $after] = $this->onDevice($laptop, fn (Authenticate $auth, Request $request) => [$auth->check(), $request->session()->get('cart')]); + + $this->assertFalse($current); + $this->assertSame([false, [42]], $after); + } + public function test_a_model_that_only_overrides_get_auth_password_is_refused(): void { $pdo = Database::getPdoInstance(self::CONNECTION); @@ -678,6 +777,16 @@ public function __construct() return [$sessionCookie, $result]; } + /** + * Carry on with a request after another one ran in the middle of it. + */ + private function backTo(Request $request): void + { + Container::getInstance()->instance('request', $request); + Container::getInstance()->instance('session', $request->session); + $_COOKIE = []; + } + private function isSignedIn(?string $sessionCookie): bool { return $this->onDevice($sessionCookie, fn (Authenticate $auth) => $auth->check())[1]; @@ -704,7 +813,7 @@ private function capturingCookies(array &$issued, callable $callback): mixed return $result; } - private function databaseStore(): DatabaseStore + public function databaseStore(): DatabaseStore { return new DatabaseStore( static fn (): PDO => Database::getPdoInstance(self::CONNECTION), diff --git a/tests/Auth/PasswordHashingTest.php b/tests/Auth/PasswordHashingTest.php index 42f42590..91e3afb1 100644 --- a/tests/Auth/PasswordHashingTest.php +++ b/tests/Auth/PasswordHashingTest.php @@ -1,12 +1,19 @@ 'bcrypt', - 'hashing.bcrypt.rounds' => 10, - ]; + if (!isset($GLOBALS['doppar_hashing_test_config'])) { + return \config($key, $default); + } + + $map = $GLOBALS['doppar_hashing_test_config']; + return array_key_exists($key, $map) ? $map[$key] : $default; } } diff --git a/tests/Session/DatabaseStoreTest.php b/tests/Session/DatabaseStoreTest.php index 23d218cd..a2e1a1a5 100644 --- a/tests/Session/DatabaseStoreTest.php +++ b/tests/Session/DatabaseStoreTest.php @@ -193,6 +193,31 @@ public function test_unknown_expired_and_corrupt_sessions_read_as_missing(): voi $this->assertNull($this->store()->read($corrupt)); } + public function test_touch_records_the_client_again(): void + { + $client = ['ip_address' => '203.0.113.1', 'user_agent' => 'Laptop']; + $store = $this->store(client: function () use (&$client): array { + return $client; + }); + + $store->write($id = $this->id(), ['x' => 1]); + $this->setActivity($id, time() - 3600); + + $client = ['ip_address' => '198.51.100.7', 'user_agent' => 'Laptop on another network']; + $this->store(client: fn (): array => $client)->touch($id); + + $this->assertSame('198.51.100.7', $this->row($id)['ip_address']); + $this->assertSame('Laptop on another network', $this->row($id)['user_agent']); + } + + public function test_last_activity_holds_times_after_2038(): void + { + $this->store()->write($id = $this->id(), ['x' => 1]); + $this->setActivity($id, 4102444800); // 2100-01-01 + + $this->assertSame(4102444800, (int) $this->row($id)['last_activity']); + } + public function test_touch_refreshes_a_stale_session_and_skips_a_fresh_one(): void { $store = $this->store(lifetime: 7200); From 279aad5dcd039a82dd4c240a66e39c25415499e0 Mon Sep 17 00:00:00 2001 From: techmahedy Date: Fri, 6 Nov 2026 11:22:07 +0600 Subject: [PATCH 4/6] fix(auth): keep parallel requests signed in when the password is saved through the model --- src/Phaseolies/Auth/Authable.php | 64 +++++++++ src/Phaseolies/Auth/Security/Authenticate.php | 72 +++++++--- tests/Auth/LogoutOtherDevicesTest.php | 132 ++++++++++++++++++ 3 files changed, 249 insertions(+), 19 deletions(-) diff --git a/src/Phaseolies/Auth/Authable.php b/src/Phaseolies/Auth/Authable.php index 1e15830c..8bd2a227 100644 --- a/src/Phaseolies/Auth/Authable.php +++ b/src/Phaseolies/Auth/Authable.php @@ -2,6 +2,7 @@ namespace Phaseolies\Auth; +use Phaseolies\Auth\Security\Authenticate; use Phaseolies\Database\Entity\Model; /** @@ -125,4 +126,67 @@ public function setTwoFactorRecoveryCodes(?string $value): void { $this->two_factor_recovery_codes = $value; } + + /** + * Save the model, keeping this device signed in when it changes the password of the signed-in user. + * + * @return bool + */ + public function save(): bool + { + $name = $this->getAuthPasswordName(); + + $hash = isset($this->attributes[$this->primaryKey]) + && $this->isDirtyAttr($name) + && ($this->creatable === [] || in_array($name, $this->creatable, true)) + ? $this->attributes[$name] + : null; + + return (bool) $this->whileSavingPassword($hash, fn (): bool => parent::save()); + } + + /** + * Update the model, keeping this device signed in when it changes the password of the signed-in user. + * + * @param array $attributes + * @return bool + */ + public function update(array $attributes): bool + { + $name = $this->getAuthPasswordName(); + + $hash = isset($this->attributes[$this->primaryKey]) + && array_key_exists($name, $attributes) + && in_array($name, $this->creatable, true) + && $attributes[$name] !== ($this->getOriginalAttributes()[$name] ?? null) + ? $attributes[$name] + : null; + + return (bool) $this->whileSavingPassword($hash, fn (): bool => parent::update($attributes)); + } + + /** + * Run a write, through the auth layer when it saves a new password hash. + * + * @param mixed $hash + * @param callable(): bool $write + * @return bool + */ + private function whileSavingPassword(mixed $hash, callable $write): bool + { + if (!is_string($hash) || $hash === '') { + return $write(); + } + + try { + $manager = app('auth'); + $actor = $manager instanceof ActorManager ? $manager->actor() : null; + } catch (\Throwable) { + $actor = null; + } + + return $actor instanceof Authenticate + ? (bool) $actor->savePassword($this, $hash, $write) + : $write(); + } } diff --git a/src/Phaseolies/Auth/Security/Authenticate.php b/src/Phaseolies/Auth/Security/Authenticate.php index efa1e45c..04f47236 100644 --- a/src/Phaseolies/Auth/Security/Authenticate.php +++ b/src/Phaseolies/Auth/Security/Authenticate.php @@ -493,7 +493,7 @@ public function logoutOtherDevices(string $password): bool $changes[$tokenName] = $user->getRememberToken(); } - $this->replaceCredentials($user, $changes, $actors, $this->fingerprintOf($hash)); + $this->writeWithFingerprint($user, $actors, $this->fingerprintOf($hash), fn () => $this->persistCredentials($user, $changes)); $store = $this->sessionStore(); $id = $user->getAuthIdentifier(); @@ -676,27 +676,60 @@ protected function currentSessionIds(): array } /** - * Save the new credentials, and the new fingerprint of every actor + * Save a new password of a user, keeping this device signed in as that user, including its other requests + * + * + * @param Authable $user + * @param string $hash + * @param callable(): mixed $write + * @return mixed + */ + public function savePassword(Authable $user, string $hash, callable $write): mixed + { + try { + $session = session(); + } catch (\Throwable) { + $session = null; + } + + if (!$session instanceof CommittableSession) { + return $write(); + } + + $actors = $this->actorsSignedInAs($user, false); + + if ($actors === []) { + return $write(); + } + + return $this->writeWithFingerprint($user, $actors, $this->fingerprintOf($hash), $write); + } + + /** + * Save new credentials together with the new fingerprint of every actor * * @param Authable $user - * @param array $changes * @param array $actors * @param string $fingerprint - * @return void + * @param callable(): mixed $write + * @return mixed */ - protected function replaceCredentials(Authable $user, array $changes, array $actors, string $fingerprint): void + protected function writeWithFingerprint(Authable $user, array $actors, string $fingerprint, callable $write): mixed { $session = session(); + $id = (string) $user->getAuthIdentifier(); $put = static function (string $key, mixed $value) use ($session): void { $session instanceof CommittableSession ? $session->putThrough($key, $value) : $session->put($key, $value); }; - $replace = function () use ($user, $changes, $actors, $fingerprint, $put): void { + $replace = function () use ($actors, $fingerprint, $put, $id, $write): mixed { $previous = []; foreach ($actors as $actor) { - if (session()->has($actor->getSessionKey())) { + // An actor signed in only by its remember cookie has no + // fingerprint here; it gets one when the cookie signs it in. + if ((string) session($actor->getSessionKey()) === $id) { $key = $actor->getPasswordFingerprintKey(); $previous[$key] = session($key); $put($key, $fingerprint); @@ -704,7 +737,7 @@ protected function replaceCredentials(Authable $user, array $changes, array $act } try { - $this->persistCredentials($user, $changes); + return $write(); } catch (\Throwable $e) { foreach ($previous as $key => $value) { $put($key, $value); @@ -714,32 +747,33 @@ protected function replaceCredentials(Authable $user, array $changes, array $act } }; - $session instanceof CommittableSession ? $session->exclusively($replace) : $replace(); + return $session instanceof CommittableSession ? $session->exclusively($replace) : $replace(); } /** - * The actors this device is signed in to as the user, this one first. + * The actors this device is signed in to as the user * * @param Authable $user + * @param bool $includeThis * @return array */ - protected function actorsSignedInAs(Authable $user): array + protected function actorsSignedInAs(Authable $user, bool $includeThis = true): array { - $actors = [$this]; + $actors = $includeThis ? [$this] : []; $id = (string) $user->getAuthIdentifier(); foreach (array_keys((array) config('auth.actors', [])) as $name) { - if ($name === $this->actorName) { + $actor = $name === $this->actorName ? $this : app('auth')->actor($name); + + if (!$actor instanceof self || in_array($actor, $actors, true)) { continue; } - $actor = app('auth')->actor($name); + if ($actor->getModel()->getTable() !== $user->getTable()) { + continue; + } - if ( - $actor instanceof self - && (string) session($actor->getSessionKey()) === $id - && $actor->getModel()->getTable() === $user->getTable() - ) { + if ((string) session($actor->getSessionKey()) === $id || $actor->hasValidRememberCookie($user)) { $actors[] = $actor; } } diff --git a/tests/Auth/LogoutOtherDevicesTest.php b/tests/Auth/LogoutOtherDevicesTest.php index 681ff4e1..73773a86 100644 --- a/tests/Auth/LogoutOtherDevicesTest.php +++ b/tests/Auth/LogoutOtherDevicesTest.php @@ -22,6 +22,7 @@ use Phaseolies\Session\SessionManager; use Phaseolies\Session\Stores\ArrayStore; use Phaseolies\Session\Stores\DatabaseStore; +use Phaseolies\Session\Stores\FileStore; use Phaseolies\Support\CookieJar; use Phaseolies\Support\Encryption; use Phaseolies\Support\Facades\Facade; @@ -104,6 +105,8 @@ class LogoutOtherDevicesTest extends TestCase private SessionStoreInterface $store; + private ?string $sessionPath = null; + private array $sessionConfig = [ 'cookie' => self::COOKIE, 'lifetime' => 120, @@ -171,6 +174,19 @@ protected function tearDown(): void (new \ReflectionClass(Container::class))->getProperty('instance')->setValue(null, null); @unlink($this->database); + + if ($this->sessionPath !== null && is_dir($this->sessionPath)) { + $files = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($this->sessionPath, \FilesystemIterator::SKIP_DOTS), + \RecursiveIteratorIterator::CHILD_FIRST + ); + + foreach ($files as $file) { + $file->isDir() ? @rmdir($file->getPathname()) : @unlink($file->getPathname()); + } + + @rmdir($this->sessionPath); + } } public function test_it_signs_out_every_other_device_and_keeps_this_one(): void @@ -469,9 +485,102 @@ public static function serverSideStores(): array return [ 'database' => [fn (self $test) => $test->databaseStore()], 'array' => [fn () => new ArrayStore(7200)], + 'file' => [fn (self $test) => $test->fileStore()], ]; } + #[\PHPUnit\Framework\Attributes\DataProvider('serverSideStores')] + public function test_a_parallel_request_survives_a_password_change_saved_on_the_signed_in_user(\Closure $store): void + { + $this->useStore($store($this)); + + $laptop = $this->signIn('Laptop'); + $phone = $this->signIn('Phone'); + + $this->onDevice($laptop, function (Authenticate $auth, Request $request) use ($laptop): void { + (new AuthenticateSession())($request, function () use ($laptop, $request): Response { + $user = app('auth')->user(); + $user->password = password_hash('a new password', PASSWORD_BCRYPT, ['cost' => 4]); + $user->save(); + + // Another request of the laptop runs before this one saves its session. + [, $signedIn] = $this->onDevice($laptop, fn (Authenticate $auth, Request $request): bool => $this->throughMiddleware($request)); + $this->backTo($request); + $this->assertTrue($signedIn, 'The parallel request is still signed in.'); + + return new Response(); + }); + }); + + $this->assertTrue($this->isCurrentAndSignedIn($laptop)); + $this->assertFalse($this->isCurrentAndSignedIn($phone)); + } + + public function test_a_parallel_request_survives_a_password_change_through_update(): void + { + $laptop = $this->signIn('Laptop'); + + $this->onDevice($laptop, function (Authenticate $auth, Request $request) use ($laptop): void { + DeviceUser::find(1)->update(['password' => password_hash('a new password', PASSWORD_BCRYPT, ['cost' => 4])]); + + [, $signedIn] = $this->onDevice($laptop, fn (Authenticate $auth, Request $request): bool => $this->throughMiddleware($request)); + $this->backTo($request); + $this->assertTrue($signedIn); + }); + + $this->assertTrue($this->isCurrentAndSignedIn($laptop)); + } + + public function test_changing_another_users_password_leaves_this_session_alone(): void + { + Database::getPdoInstance(self::CONNECTION) + ->prepare('INSERT INTO users (id, email, password) VALUES (?, ?, ?)') + ->execute([2, 'grace@example.com', password_hash('her password', PASSWORD_BCRYPT, ['cost' => 4])]); + + $laptop = $this->signIn('Laptop'); + + [, $fingerprints] = $this->onDevice($laptop, function (Authenticate $auth, Request $request): array { + $before = $request->session()->get('password_hash_web'); + + $other = DeviceUser::find(2); + $other->password = password_hash('reset by an admin', PASSWORD_BCRYPT, ['cost' => 4]); + $other->save(); + + return [$before, $request->session()->get('password_hash_web')]; + }); + + $this->assertSame($fingerprints[0], $fingerprints[1]); + $this->assertTrue($this->isCurrentAndSignedIn($laptop)); + } + + public function test_an_actor_signed_in_only_by_its_remember_cookie_stays_remembered(): void + { + $laptop = $this->signIn('Laptop'); + + // The admin actor is remembered on this device, but has no session sign-in. + $adminCookie = []; + [$laptop] = $this->onDevice($laptop, function (Authenticate $auth, Request $request) use (&$adminCookie): void { + $this->capturingCookies($adminCookie, fn () => app('auth')->actor('admin')->login(DeviceUser::find(1), true)); + + // Its session sign-in has expired since; only the cookie is left. + $request->session()->forget('admin_id'); + $request->session()->forget('password_hash_admin'); + }); + $adminCookie = array_filter($adminCookie, fn (string $name) => str_contains($name, '_admin_'), ARRAY_FILTER_USE_KEY); + $this->assertNotSame([], $adminCookie); + + $issued = []; + $this->onDevice($laptop, function (Authenticate $auth) use (&$issued): bool { + return $this->capturingCookies($issued, fn () => $auth->logoutOtherDevices(self::PASSWORD)); + }, $adminCookie); + + $this->assertNotNull($this->storedRememberToken(), 'The remember token is kept for the admin actor.'); + + [, $adminSignedIn] = $this->onDevice(null, fn () => app('auth')->actor('admin')->check(), $issued); + + $this->assertTrue($adminSignedIn, 'The new remember cookie of the admin actor still works.'); + } + #[\PHPUnit\Framework\Attributes\DataProvider('serverSideStores')] public function test_a_parallel_request_of_this_device_does_not_sign_it_out(\Closure $store): void { @@ -777,6 +886,22 @@ public function __construct() return [$sessionCookie, $result]; } + /** + * Run a request through AuthenticateSession and report whether it was signed in. + */ + private function throughMiddleware(Request $request): bool + { + $signedIn = false; + + (new AuthenticateSession())($request, function () use (&$signedIn): Response { + $signedIn = app('auth')->check(); + + return new Response(); + }); + + return $signedIn; + } + /** * Carry on with a request after another one ran in the middle of it. */ @@ -813,6 +938,13 @@ private function capturingCookies(array &$issued, callable $callback): mixed return $result; } + public function fileStore(): FileStore + { + $this->sessionPath ??= sys_get_temp_dir() . '/doppar_device_sessions_' . bin2hex(random_bytes(6)); + + return new FileStore($this->sessionPath, 7200); + } + public function databaseStore(): DatabaseStore { return new DatabaseStore( From 6e96e3dbe1ab2d529f2c94782134ba71f66e8d1f Mon Sep 17 00:00:00 2001 From: techmahedy Date: Fri, 6 Nov 2026 12:09:05 +0600 Subject: [PATCH 5/6] fix(auth): keep the device signed in when a password change is rolled back --- src/Phaseolies/Auth/Security/Authenticate.php | 120 +++++++++--------- .../Middleware/AuthenticateSession.php | 8 +- tests/Auth/LogoutOtherDevicesTest.php | 72 +++++++++++ 3 files changed, 138 insertions(+), 62 deletions(-) diff --git a/src/Phaseolies/Auth/Security/Authenticate.php b/src/Phaseolies/Auth/Security/Authenticate.php index 04f47236..39aff1ad 100644 --- a/src/Phaseolies/Auth/Security/Authenticate.php +++ b/src/Phaseolies/Auth/Security/Authenticate.php @@ -47,12 +47,6 @@ class Authenticate */ private ?Authable $resolvedUser = null; - /** - * Fingerprint of the signed-in user's password when isSessionCurrent() checked it - * - * @var string|null - */ - private ?string $checkedFingerprint = null; /** * Create a new actor instance. @@ -137,6 +131,16 @@ protected function getPasswordFingerprintKey(): string return 'password_hash_' . $this->actorName; } + /** + * The session key holding the fingerprint of a password this device is saving. + * + * @return string + */ + protected function getPendingFingerprintKey(): string + { + return $this->getPasswordFingerprintKey() . '_next'; + } + /** * The session key used to store a pending 2FA user ID for this actor. * @@ -420,11 +424,11 @@ protected function endSession(bool $expireRememberCookie = true): void { $this->resolvedUser = null; $this->statelessUser = null; - $this->checkedFingerprint = null; session()->forget($this->getSessionKey()); session()->forget($this->getViaRememberKey()); session()->forget($this->getPasswordFingerprintKey()); + session()->forget($this->getPendingFingerprintKey()); // Only fully invalidate the session on the default actor so that other // actors (e.g. "admin") remain active when only the "web" actor logs out. @@ -528,98 +532,98 @@ public function isSessionCurrent(): bool if (!is_string($stored)) { $this->signOutKeepingSession(); - $user = $this->user(); + return $this->user() !== null; + } - if ($user === null) { - return false; - } + $fingerprint = $this->passwordFingerprint($user); - $this->checkedFingerprint = $this->passwordFingerprint($user); + if ($this->acceptFingerprint($fingerprint, $stored, session($this->getPendingFingerprintKey()))) { + return true; + } + if ($this->acceptStoredFingerprint()) { return true; } - $fingerprint = $this->passwordFingerprint($user); + $this->endSession(); - if (hash_equals($fingerprint, $stored)) { - $this->checkedFingerprint = $fingerprint; + return false; + } + + /** + * Accept a password fingerprint that matches the current or the pending one. + * + * @param string $fingerprint + * @param mixed $current + * @param mixed $pending + * @return bool + */ + protected function acceptFingerprint(string $fingerprint, mixed $current, mixed $pending): bool + { + if (is_string($current) && hash_equals($current, $fingerprint)) { + if (session($this->getPasswordFingerprintKey()) !== $fingerprint) { + session()->put($this->getPasswordFingerprintKey(), $fingerprint); + } return true; } - $current = $this->fingerprintStoredSince(); - - if ($current !== null) { - session()->put($this->getPasswordFingerprintKey(), $current); - $this->checkedFingerprint = $current; + if (is_string($pending) && hash_equals($pending, $fingerprint)) { + session()->put($this->getPasswordFingerprintKey(), $fingerprint); + session()->forget($this->getPendingFingerprintKey()); return true; } - $this->endSession(); - return false; } /** - * The fingerprint this device's session holds now, when it matches the - * password in the database now. + * Check the fingerprints again as this device's session store * - * @return string|null + * @return bool */ - protected function fingerprintStoredSince(): ?string + protected function acceptStoredFingerprint(): bool { $session = session(); if (!$session instanceof CommittableSession) { - return null; + return false; } - return $session->exclusively(function () use ($session): ?string { - $stored = $session->storedValue($this->getPasswordFingerprintKey()); + return (bool) $session->exclusively(function () use ($session): bool { $user = $this->getModel()::find(session($this->getSessionKey())); - if (!is_string($stored) || !$user instanceof Authable) { - return null; + if (!$user instanceof Authable) { + return false; } - $fingerprint = $this->passwordFingerprint($user); + $accepted = $this->acceptFingerprint( + $this->passwordFingerprint($user), + $session->storedValue($this->getPasswordFingerprintKey()), + $session->storedValue($this->getPendingFingerprintKey()) + ); - if (!hash_equals($fingerprint, $stored)) { - return null; + if ($accepted) { + $this->resolvedUser = $user; } - $this->resolvedUser = $user; - - return $fingerprint; + return $accepted; }); } /** - * Record the user's current password in this device's session, after this request changed it. + * Record the user's password in this device's session, after changing it with a query. * - * @param bool $reload * @return void */ - public function refreshSessionFingerprint(bool $reload = true): void + public function refreshSessionFingerprint(): void { if (!session()->has($this->getSessionKey())) { return; } - if ($reload) { - $user = $this->getModel()::find(session($this->getSessionKey())); - } else { - $user = $this->resolvedUser; - - if ($user === null || $this->checkedFingerprint === null || !is_string(session($this->getPasswordFingerprintKey()))) { - return; - } - - if (hash_equals($this->checkedFingerprint, $this->passwordFingerprint($user))) { - return; - } - } + $user = $this->getModel()::find(session($this->getSessionKey())); if (!$user instanceof Authable) { return; @@ -630,6 +634,8 @@ public function refreshSessionFingerprint(bool $reload = true): void if (session($this->getPasswordFingerprintKey()) !== $fingerprint) { session()->put($this->getPasswordFingerprintKey(), $fingerprint); } + + session()->forget($this->getPendingFingerprintKey()); } /** @@ -730,7 +736,7 @@ protected function writeWithFingerprint(Authable $user, array $actors, string $f // An actor signed in only by its remember cookie has no // fingerprint here; it gets one when the cookie signs it in. if ((string) session($actor->getSessionKey()) === $id) { - $key = $actor->getPasswordFingerprintKey(); + $key = $actor->getPendingFingerprintKey(); $previous[$key] = session($key); $put($key, $fingerprint); } @@ -769,11 +775,11 @@ protected function actorsSignedInAs(Authable $user, bool $includeThis = true): a continue; } - if ($actor->getModel()->getTable() !== $user->getTable()) { + if ((string) session($actor->getSessionKey()) !== $id && !$actor->hasValidRememberCookie($user)) { continue; } - if ((string) session($actor->getSessionKey()) === $id || $actor->hasValidRememberCookie($user)) { + if ($actor->getModel()->getTable() === $user->getTable()) { $actors[] = $actor; } } @@ -789,11 +795,11 @@ protected function actorsSignedInAs(Authable $user, bool $includeThis = true): a protected function signOutKeepingSession(): void { $this->resolvedUser = null; - $this->checkedFingerprint = null; session()->forget($this->getSessionKey()); session()->forget($this->getViaRememberKey()); session()->forget($this->getPasswordFingerprintKey()); + session()->forget($this->getPendingFingerprintKey()); session()->regenerate(); } diff --git a/src/Phaseolies/Middleware/AuthenticateSession.php b/src/Phaseolies/Middleware/AuthenticateSession.php index a1a9cc1c..d7b9f884 100644 --- a/src/Phaseolies/Middleware/AuthenticateSession.php +++ b/src/Phaseolies/Middleware/AuthenticateSession.php @@ -23,12 +23,10 @@ public function __invoke(Request $request, Closure $next, ?string $actor = null) // A stale session is signed out here; the request goes on as a guest, // so the "auth" middleware of a protected route sends it to sign in. + // A password this request saves through the model or by + // logoutOtherDevices() keeps the device signed in by itself. $auth->isSessionCurrent(); - $response = $next($request); - - $auth->refreshSessionFingerprint(false); - - return $response; + return $next($request); } } diff --git a/tests/Auth/LogoutOtherDevicesTest.php b/tests/Auth/LogoutOtherDevicesTest.php index 73773a86..50a3f87b 100644 --- a/tests/Auth/LogoutOtherDevicesTest.php +++ b/tests/Auth/LogoutOtherDevicesTest.php @@ -531,6 +531,78 @@ public function test_a_parallel_request_survives_a_password_change_through_updat $this->assertTrue($this->isCurrentAndSignedIn($laptop)); } + #[\PHPUnit\Framework\Attributes\DataProvider('serverSideStores')] + public function test_a_rolled_back_password_change_keeps_this_device_signed_in(\Closure $store): void + { + $this->useStore($store($this)); + + $laptop = $this->signIn('Laptop'); + $before = $this->storedPassword(); + + [$laptop] = $this->onDevice($laptop, function (Authenticate $auth, Request $request): void { + (new AuthenticateSession())($request, function (): Response { + $pdo = Database::getPdoInstance(self::CONNECTION); + $pdo->beginTransaction(); + + $user = app('auth')->user(); + $user->password = password_hash('a new password', PASSWORD_BCRYPT, ['cost' => 4]); + $user->save(); + + // A later step of the same transaction fails. + $pdo->rollBack(); + + return new Response(); + }); + }); + + $this->assertSame($before, $this->storedPassword(), 'The password did not change.'); + $this->assertTrue($this->isCurrentAndSignedIn($laptop)); + } + + public function test_a_committed_password_change_is_confirmed_on_the_next_request(): void + { + $laptop = $this->signIn('Laptop'); + + [$laptop] = $this->onDevice($laptop, function (Authenticate $auth): void { + $user = $auth->user(); + $user->password = password_hash('a new password', PASSWORD_BCRYPT, ['cost' => 4]); + $user->save(); + }); + + [$laptop, $keys] = $this->onDevice($laptop, function (Authenticate $auth, Request $request): array { + $auth->isSessionCurrent(); + + return [$request->session()->has('password_hash_web'), $request->session()->has('password_hash_web_next')]; + }); + + $this->assertSame([true, false], $keys, 'The pending fingerprint became the current one.'); + $this->assertTrue($this->isCurrentAndSignedIn($laptop)); + } + + public function test_an_actor_with_a_missing_model_class_does_not_block_password_changes(): void + { + Config::set('auth.actors.legacy', ['model' => 'App\\Models\\RemovedLongAgo', 'session_key' => 'legacy_id']); + + $laptop = $this->signIn('Laptop'); + $phone = $this->signIn('Phone'); + + [$laptop] = $this->onDevice($laptop, function (Authenticate $auth): void { + $user = $auth->user(); + $user->password = password_hash('a new password', PASSWORD_BCRYPT, ['cost' => 4]); + $user->save(); + + DeviceUser::find(1)->update(['password' => password_hash(self::PASSWORD, PASSWORD_BCRYPT, ['cost' => 4])]); + }); + + $this->assertTrue(password_verify(self::PASSWORD, $this->storedPassword())); + + [$laptop, $result] = $this->onDevice($laptop, fn (Authenticate $auth) => $auth->logoutOtherDevices(self::PASSWORD)); + + $this->assertTrue($result); + $this->assertTrue($this->isCurrentAndSignedIn($laptop)); + $this->assertFalse($this->isCurrentAndSignedIn($phone)); + } + public function test_changing_another_users_password_leaves_this_session_alone(): void { Database::getPdoInstance(self::CONNECTION) From 8606114cf378de5a0312a6c542f0e87f1784b813 Mon Sep 17 00:00:00 2001 From: techmahedy Date: Mon, 9 Nov 2026 11:04:41 +0600 Subject: [PATCH 6/6] fix(auth): record the hashed password before the write, outside the session lock --- src/Phaseolies/Auth/Authable.php | 83 +++++++---- src/Phaseolies/Auth/Security/Authenticate.php | 110 ++++++++------ tests/Auth/LogoutOtherDevicesTest.php | 136 +++++++++++++++++- 3 files changed, 261 insertions(+), 68 deletions(-) diff --git a/src/Phaseolies/Auth/Authable.php b/src/Phaseolies/Auth/Authable.php index 8bd2a227..33111d30 100644 --- a/src/Phaseolies/Auth/Authable.php +++ b/src/Phaseolies/Auth/Authable.php @@ -128,54 +128,86 @@ public function setTwoFactorRecoveryCodes(?string $value): void } /** - * Save the model, keeping this device signed in when it changes the password of the signed-in user. + * Save the model. * * @return bool */ public function save(): bool { - $name = $this->getAuthPasswordName(); - - $hash = isset($this->attributes[$this->primaryKey]) - && $this->isDirtyAttr($name) - && ($this->creatable === [] || in_array($name, $this->creatable, true)) - ? $this->attributes[$name] - : null; + if ($this->hooksSuppressed) { + $this->recordPasswordChange($this->changedPasswordHash()); + } - return (bool) $this->whileSavingPassword($hash, fn (): bool => parent::save()); + return parent::save(); } /** - * Update the model, keeping this device signed in when it changes the password of the signed-in user. + * Update the model. * * @param array $attributes * @return bool */ public function update(array $attributes): bool + { + if ($this->hooksSuppressed) { + $name = $this->getAuthPasswordName(); + + $this->recordPasswordChange( + isset($this->attributes[$this->primaryKey]) + && array_key_exists($name, $attributes) + && in_array($name, $this->creatable, true) + && $attributes[$name] !== ($this->getOriginalAttributes()[$name] ?? null) + ? $attributes[$name] + : null + ); + } + + return parent::update($attributes); + } + + /** + * Run the model's before hooks, then record a password about to change. + * + * @param string $event + * @return bool + */ + protected function fireBeforeHooks(string $event): bool + { + $result = parent::fireBeforeHooks($event); + + if ($event === 'updated') { + $this->recordPasswordChange($this->changedPasswordHash()); + } + + return $result; + } + + /** + * The new password hash the next save writes, if it changes the password. + * + * @return mixed + */ + private function changedPasswordHash(): mixed { $name = $this->getAuthPasswordName(); - $hash = isset($this->attributes[$this->primaryKey]) - && array_key_exists($name, $attributes) - && in_array($name, $this->creatable, true) - && $attributes[$name] !== ($this->getOriginalAttributes()[$name] ?? null) - ? $attributes[$name] + return isset($this->attributes[$this->primaryKey]) + && $this->isDirtyAttr($name) + && ($this->creatable === [] || in_array($name, $this->creatable, true)) + ? $this->attributes[$name] : null; - - return (bool) $this->whileSavingPassword($hash, fn (): bool => parent::update($attributes)); } /** - * Run a write, through the auth layer when it saves a new password hash. + * Tell the auth layer that a password is about to be saved. * * @param mixed $hash - * @param callable(): bool $write - * @return bool + * @return void */ - private function whileSavingPassword(mixed $hash, callable $write): bool + private function recordPasswordChange(mixed $hash): void { if (!is_string($hash) || $hash === '') { - return $write(); + return; } try { @@ -185,8 +217,9 @@ private function whileSavingPassword(mixed $hash, callable $write): bool $actor = null; } - return $actor instanceof Authenticate - ? (bool) $actor->savePassword($this, $hash, $write) - : $write(); + if ($actor instanceof Authenticate) { + $actor->recordPendingPassword($this, $hash); + } } + } diff --git a/src/Phaseolies/Auth/Security/Authenticate.php b/src/Phaseolies/Auth/Security/Authenticate.php index 39aff1ad..82ae6a8a 100644 --- a/src/Phaseolies/Auth/Security/Authenticate.php +++ b/src/Phaseolies/Auth/Security/Authenticate.php @@ -141,6 +141,11 @@ protected function getPendingFingerprintKey(): string return $this->getPasswordFingerprintKey() . '_next'; } + /** + * Most password changes of one request whose fingerprints are kept pending. + */ + protected const MAX_PENDING_FINGERPRINTS = 5; + /** * The session key used to store a pending 2FA user ID for this actor. * @@ -497,7 +502,9 @@ public function logoutOtherDevices(string $password): bool $changes[$tokenName] = $user->getRememberToken(); } - $this->writeWithFingerprint($user, $actors, $this->fingerprintOf($hash), fn () => $this->persistCredentials($user, $changes)); + $this->recordPending($user, $actors, $hash); + + $this->persistCredentials($user, $changes); $store = $this->sessionStore(); $id = $user->getAuthIdentifier(); @@ -568,16 +575,33 @@ protected function acceptFingerprint(string $fingerprint, mixed $current, mixed return true; } - if (is_string($pending) && hash_equals($pending, $fingerprint)) { - session()->put($this->getPasswordFingerprintKey(), $fingerprint); - session()->forget($this->getPendingFingerprintKey()); + foreach (self::fingerprintList($pending) as $candidate) { + if (hash_equals($candidate, $fingerprint)) { + session()->put($this->getPasswordFingerprintKey(), $fingerprint); + session()->forget($this->getPendingFingerprintKey()); - return true; + return true; + } } return false; } + /** + * The fingerprints a pending value holds. + * + * @param mixed $value + * @return array + */ + protected static function fingerprintList(mixed $value): array + { + if (is_string($value)) { + return [$value]; + } + + return is_array($value) ? array_values(array_filter($value, 'is_string')) : []; + } + /** * Check the fingerprints again as this device's session store * @@ -682,78 +706,82 @@ protected function currentSessionIds(): array } /** - * Save a new password of a user, keeping this device signed in as that user, including its other requests - * + * Record a password about to be saved for a user * * @param Authable $user * @param string $hash - * @param callable(): mixed $write - * @return mixed + * @return void */ - public function savePassword(Authable $user, string $hash, callable $write): mixed + public function recordPendingPassword(Authable $user, string $hash): void { try { $session = session(); } catch (\Throwable) { - $session = null; + return; } if (!$session instanceof CommittableSession) { - return $write(); + return; } $actors = $this->actorsSignedInAs($user, false); - if ($actors === []) { - return $write(); + if ($actors !== []) { + $this->recordPending($user, $actors, $hash); } - - return $this->writeWithFingerprint($user, $actors, $this->fingerprintOf($hash), $write); } /** - * Save new credentials together with the new fingerprint of every actor + * Record a password hash as pending for every actor * * @param Authable $user * @param array $actors - * @param string $fingerprint - * @param callable(): mixed $write - * @return mixed + * @param string $hash + * @return void */ - protected function writeWithFingerprint(Authable $user, array $actors, string $fingerprint, callable $write): mixed + protected function recordPending(Authable $user, array $actors, string $hash): void { $session = session(); $id = (string) $user->getAuthIdentifier(); + $fingerprint = $this->fingerprintOf($hash); - $put = static function (string $key, mixed $value) use ($session): void { - $session instanceof CommittableSession ? $session->putThrough($key, $value) : $session->put($key, $value); - }; - - $replace = function () use ($actors, $fingerprint, $put, $id, $write): mixed { - $previous = []; - + $record = function () use ($actors, $id, $fingerprint): void { foreach ($actors as $actor) { // An actor signed in only by its remember cookie has no // fingerprint here; it gets one when the cookie signs it in. if ((string) session($actor->getSessionKey()) === $id) { - $key = $actor->getPendingFingerprintKey(); - $previous[$key] = session($key); - $put($key, $fingerprint); + $actor->addPendingFingerprint($fingerprint); } } + }; - try { - return $write(); - } catch (\Throwable $e) { - foreach ($previous as $key => $value) { - $put($key, $value); - } + $session instanceof CommittableSession ? $session->exclusively($record) : $record(); + } - throw $e; - } - }; + /** + * Add a fingerprint to this actor's pending ones, in the stored session too. + * + * @param string $fingerprint + * @return void + */ + protected function addPendingFingerprint(string $fingerprint): void + { + $session = session(); + $key = $this->getPendingFingerprintKey(); + + $pending = self::fingerprintList(session($key)); + + if ($session instanceof CommittableSession) { + $pending = array_merge(self::fingerprintList($session->storedValue($key)), $pending); + } + + if (in_array($fingerprint, $pending, true)) { + return; + } + + $pending = array_slice(array_values(array_unique([...$pending, $fingerprint])), -self::MAX_PENDING_FINGERPRINTS); - return $session instanceof CommittableSession ? $session->exclusively($replace) : $replace(); + $session instanceof CommittableSession ? $session->putThrough($key, $pending) : $session->put($key, $pending); } /** diff --git a/tests/Auth/LogoutOtherDevicesTest.php b/tests/Auth/LogoutOtherDevicesTest.php index 50a3f87b..e9564f02 100644 --- a/tests/Auth/LogoutOtherDevicesTest.php +++ b/tests/Auth/LogoutOtherDevicesTest.php @@ -10,6 +10,7 @@ use Phaseolies\Auth\Security\PasswordHashing; use Phaseolies\Config\Config; use Phaseolies\Database\Database; +use Phaseolies\Database\Entity\Hooks\HookHandler; use Phaseolies\DI\Container; use Phaseolies\Http\HeaderBag; use Phaseolies\Http\InputBag; @@ -74,6 +75,18 @@ class LockedDownUser extends Authable protected $creatable = ['email']; } +/** + * A model whose before_updated hook hashes a plain password, as applications often do. + */ +class HashingUser extends Authable +{ + protected $table = 'users'; + + protected $timeStamps = false; + + protected $creatable = ['email', 'password', 'remember_token']; +} + /** * A model that reads its password from another column, but only overrides getAuthPassword(). */ @@ -175,6 +188,8 @@ protected function tearDown(): void @unlink($this->database); + unset(HookHandler::$hooks[HashingUser::class]); + if ($this->sessionPath !== null && is_dir($this->sessionPath)) { $files = new \RecursiveIteratorIterator( new \RecursiveDirectoryIterator($this->sessionPath, \FilesystemIterator::SKIP_DOTS), @@ -603,6 +618,107 @@ public function test_an_actor_with_a_missing_model_class_does_not_block_password $this->assertFalse($this->isCurrentAndSignedIn($phone)); } + #[\PHPUnit\Framework\Attributes\DataProvider('serverSideStores')] + public function test_a_password_hashed_by_a_model_hook_keeps_this_device_signed_in(\Closure $store): void + { + $this->useStore($store($this)); + $this->useHashingUser(); + + $laptop = $this->signIn('Laptop'); + + [$laptop] = $this->onDevice($laptop, function (Authenticate $auth, Request $request) use ($laptop): void { + $user = $auth->user(); + $user->password = 'a new password'; + $user->save(); + + [, $signedIn] = $this->onDevice($laptop, fn (Authenticate $auth, Request $request): bool => $this->throughMiddleware($request)); + $this->backTo($request); + $this->assertTrue($signedIn, 'A parallel request is still signed in.'); + }); + + $this->assertTrue(password_verify('a new password', $this->storedPassword()), 'The hook hashed it.'); + $this->assertTrue($this->isCurrentAndSignedIn($laptop)); + } + + public function test_a_password_hashed_by_a_hook_on_another_instance_keeps_this_device_signed_in(): void + { + $this->useHashingUser(); + + $laptop = $this->signIn('Laptop'); + + [$laptop] = $this->onDevice($laptop, function (Authenticate $auth): void { + $auth->user(); + + $user = HashingUser::find(1); + $user->password = 'a new password'; + $user->save(); + }); + + $this->assertTrue($this->isCurrentAndSignedIn($laptop)); + } + + public function test_a_committed_change_survives_a_later_rolled_back_one_in_the_same_request(): void + { + $laptop = $this->signIn('Laptop'); + + [$laptop] = $this->onDevice($laptop, function (Authenticate $auth): void { + $user = $auth->user(); + $user->password = password_hash('first', PASSWORD_BCRYPT, ['cost' => 4]); + $user->save(); + + $pdo = Database::getPdoInstance(self::CONNECTION); + $pdo->beginTransaction(); + $user->password = password_hash('second', PASSWORD_BCRYPT, ['cost' => 4]); + $user->save(); + $pdo->rollBack(); + }); + + $this->assertTrue(password_verify('first', $this->storedPassword())); + $this->assertTrue($this->isCurrentAndSignedIn($laptop)); + } + + #[\PHPUnit\Framework\Attributes\DataProvider('serverSideStores')] + public function test_an_after_hook_that_throws_once_the_password_is_saved_keeps_this_device_signed_in(\Closure $store): void + { + $this->useStore($store($this)); + $this->useHashingUser(['after_updated' => function (): void { + throw new \RuntimeException('a listener failed'); + }]); + + $laptop = $this->signIn('Laptop'); + + [$laptop] = $this->onDevice($laptop, function (Authenticate $auth): void { + $user = $auth->user(); + $user->password = 'a new password'; + + try { + $user->save(); + $this->fail('The hook throws.'); + } catch (\RuntimeException) { + } + }); + + $this->assertTrue(password_verify('a new password', $this->storedPassword()), 'The password was saved before the hook threw.'); + $this->assertTrue($this->isCurrentAndSignedIn($laptop)); + } + + public function test_a_password_saved_without_hooks_keeps_a_parallel_request_signed_in(): void + { + $laptop = $this->signIn('Laptop'); + + $this->onDevice($laptop, function (Authenticate $auth, Request $request) use ($laptop): void { + $user = $auth->user(); + $user->password = password_hash('a new password', PASSWORD_BCRYPT, ['cost' => 4]); + $user->withoutHook()->save(); + + [, $signedIn] = $this->onDevice($laptop, fn (Authenticate $auth, Request $request): bool => $this->throughMiddleware($request)); + $this->backTo($request); + $this->assertTrue($signedIn); + }); + + $this->assertTrue($this->isCurrentAndSignedIn($laptop)); + } + public function test_changing_another_users_password_leaves_this_session_alone(): void { Database::getPdoInstance(self::CONNECTION) @@ -894,7 +1010,7 @@ public function test_another_actor_never_deletes_sessions_by_the_default_actors_ */ private function signIn(string $agent): string { - [$cookie] = $this->onDevice(null, fn (Authenticate $auth) => $auth->login(DeviceUser::find(1)), [], $agent); + [$cookie] = $this->onDevice(null, fn (Authenticate $auth) => $auth->login(config('auth.actors.web.model')::find(1)), [], $agent); return $cookie; } @@ -909,7 +1025,7 @@ private function signInRemembered(string $agent): array $issued = []; [$cookie] = $this->onDevice(null, function (Authenticate $auth) use (&$issued): bool { - return $this->capturingCookies($issued, fn () => $auth->login(DeviceUser::find(1), true)); + return $this->capturingCookies($issued, fn () => $auth->login(config('auth.actors.web.model')::find(1), true)); }, [], $agent); return [$cookie, $issued]; @@ -958,6 +1074,22 @@ public function __construct() return [$sessionCookie, $result]; } + /** + * Sign in through HashingUser, whose before_updated hook hashes a plain password. + * + * @param array $extra More hooks for the model + */ + private function useHashingUser(array $extra = []): void + { + Config::set('auth.actors.web.model', HashingUser::class); + + HookHandler::register(HashingUser::class, ['before_updated' => function (HashingUser $user): void { + if ($user->isDirtyAttr('password') && password_get_info((string) $user->password)['algo'] === null) { + $user->password = password_hash((string) $user->password, PASSWORD_BCRYPT, ['cost' => 4]); + } + }] + $extra); + } + /** * Run a request through AuthenticateSession and report whether it was signed in. */