Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,21 @@ All notable changes to `dskripchenko/laravel-admin` will be documented in this f
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## Unreleased

### Fixed

- **A sign-out could be undone by a request still running.** The sign-out
invalidates the session, but a request already in flight with it — the
shell's manifest, a status poll — saved it back under the old id when it
finished and returned the old cookie, so the next page load was signed in
again. Unnoticed while the SPA stayed on its login page after a sign-out;
visible since 1.51.0, when a sign-out at an identity provider reloads the
page. A sign-out now remembers the ended session id for the session's
lifetime (`Auth\EndedSessions`, in the cache), and a request carrying it is
signed out and gets a fresh session — in the API, the bootstrap and the
shell alike. A cache that is down only skips the check.

## 1.51.0

### Added
Expand Down
2 changes: 2 additions & 0 deletions src/Auth/Controllers/AuthController.php
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,8 @@ public function logout(Request $request): JsonResponse
Auth::guard($guard)->logout();

if ($request->hasSession()) {
// A request still running with this session must not save it back to life.
\Dskripchenko\LaravelAdmin\Auth\EndedSessions::end($request->session()->getId());
$request->session()->invalidate();
$request->session()->regenerateToken();
}
Expand Down
71 changes: 71 additions & 0 deletions src/Auth/EndedSessions.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<?php

declare(strict_types=1);

namespace Dskripchenko\LaravelAdmin\Auth;

use Illuminate\Http\Request;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\Cache;

/**
* Sessions ended by a sign-out stay ended.
*
* A sign-out invalidates the session, but a request that was already running
* with it — the shell's manifest, a status poll — saves the session back when
* it finishes, under the old id, and sends the old cookie with it. The
* session comes back to life and the person is signed in again on the next
* page load. The sign-out remembers the ended id for the session's lifetime;
* a request carrying it is signed out and gets a fresh session.
*
* The cache being down must not take the panel with it: a failed lookup is
* reported and the request goes on as before.
*/
final class EndedSessions
{
private const PREFIX = 'admin:ended-session:';

public static function end(string $sessionId): void
{
if ($sessionId === '') {
return;
}
try {
Cache::put(self::key($sessionId), true, now()->addMinutes(max(1, (int) config('session.lifetime', 120))));
} catch (\Throwable $e) {
report($e);
}
}

/**
* Signs the request out of `$guard` when its session was ended.
*
* @return bool whether it was
*/
public static function clearIfEnded(Request $request, string $guard): bool
{
if (! $request->hasSession()) {
return false;
}
try {
if (! Cache::has(self::key($request->session()->getId()))) {
return false;
}
} catch (\Throwable $e) {
report($e);

return false;
}

Auth::guard($guard)->logout();
$request->session()->invalidate();
$request->session()->regenerateToken();

return true;
}

private static function key(string $sessionId): string
{
return self::PREFIX.hash('sha256', $sessionId);
}
}
7 changes: 5 additions & 2 deletions src/Http/Middleware/AdminAuth.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,18 @@ final class AdminAuth
{
public function handle(Request $request, Closure $next): SymfonyResponse
{
// Before the public actions too: the bootstrap of a signed-out
// session must not see the user again.
$guard = \Dskripchenko\LaravelAdmin\Panel\Panels::currentGuard();
\Dskripchenko\LaravelAdmin\Auth\EndedSessions::clearIfEnded($request, $guard);

if ($this->isExcludedForCurrentAction()) {
/** @var SymfonyResponse $response */
$response = $next($request);

return $response;
}

$guard = \Dskripchenko\LaravelAdmin\Panel\Panels::currentGuard();

if (! Auth::guard($guard)->check()) {
return response()->json([
'success' => false,
Expand Down
2 changes: 2 additions & 0 deletions src/Support/BootstrapBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ public function build(?Request $request = null): array
$request ??= request();

$panel = \Dskripchenko\LaravelAdmin\Panel\Panels::current();
// The shell's inline bootstrap does not pass AdminAuth.
\Dskripchenko\LaravelAdmin\Auth\EndedSessions::clearIfEnded($request, $panel->guard);
$locale = $this->locales->resolve($request);
$user = $this->serializeUser();

Expand Down
78 changes: 78 additions & 0 deletions tests/Feature/EndedSessionTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
<?php

declare(strict_types=1);

use Dskripchenko\LaravelAdmin\Auth\EndedSessions;
use Dskripchenko\LaravelAdmin\Models\AdminUser;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Str;

/*
* A signed-out session stays signed out: a request that was still running
* with it saves it back under the old id, the old cookie returns with it —
* and the next request with that id is a guest again.
*/

/** A signed-in session as the session store holds it, and its id. */
function storedSession(AdminUser $admin): string
{
$id = Str::random(40);
app('session')->driver()->getHandler()->write($id, serialize([
'_token' => Str::random(40),
Auth::guard('admin')->getName() => $admin->getKey(),
]));

return $id;
}

function sessionAdmin(): AdminUser
{
return AdminUser::create(['name' => 'Out', 'email' => 'out-'.uniqid().'@example.com', 'password' => bin2hex(random_bytes(8))]);
}

it('treats a session as signed in until it is ended', function (): void {
$admin = sessionAdmin();
$id = storedSession($admin);

$this->withCredentials()->withCookie(config('session.cookie'), $id)
->getJson('/api/admin/system/bootstrap')->assertOk()->assertJsonPath('payload.user.id', $admin->getKey());
});

it('gives an ended session that came back a guest bootstrap', function (): void {
$id = storedSession(sessionAdmin());
EndedSessions::end($id);

$this->withCredentials()->withCookie(config('session.cookie'), $id)
->getJson('/api/admin/system/bootstrap')->assertOk()->assertJsonPath('payload.user', null);
});

it('turns an ended session that came back away from the API', function (): void {
$id = storedSession(sessionAdmin());
EndedSessions::end($id);

$this->withCredentials()->withCookie(config('session.cookie'), $id)
->getJson('/api/admin/system/manifest')->assertUnauthorized();
// And the stored session is gone, not just skipped.
expect(app('session')->driver()->getHandler()->read($id))->toBe('');
});

it('remembers the session a sign-out ended', function (): void {
$this->actingAs(sessionAdmin(), 'admin');
$this->startSession();
$id = session()->getId();
$this->withCredentials()->withCookie(config('session.cookie'), $id)->postJson('/api/admin/auth/logout')->assertOk();

expect(Cache::has('admin:ended-session:'.hash('sha256', $id)))->toBeTrue();
});

it('goes on as before when the cache is down', function (): void {
$admin = sessionAdmin();
$id = storedSession($admin);
Cache::shouldReceive('has')->andThrow(new RuntimeException('cache is down'));
Cache::shouldReceive('put')->andThrow(new RuntimeException('cache is down'));

EndedSessions::end($id);
$this->withCredentials()->withCookie(config('session.cookie'), $id)
->getJson('/api/admin/system/bootstrap')->assertOk()->assertJsonPath('payload.user.id', $admin->getKey());
});
Loading