Match the logout exemption through AdminArea, not a bare route check
The setup-route exemption already went through AdminArea::routeIs(),
which matches both admin.<name> and admin.via*.<name>. The logout
exemption next to it used a bare $request->routeIs('admin.logout'),
which matches only the canonical name — on a recovery hostname the
logout route is admin.via0.logout, so the check failed there and an
unenrolled operator's logout POST bounced back to enrolment instead
of being let through, on exactly the host that exists because the
canonical one is not working.
feat/operator-identity
parent
27292237c3
commit
278c4b9953
|
|
@ -40,8 +40,13 @@ class RequireOperatorTwoFactor
|
|||
}
|
||||
|
||||
// The two pages that must stay reachable: where two-factor is
|
||||
// enrolled, and signing out.
|
||||
if (AdminArea::routeIs('admin.two-factor-setup') || $request->routeIs('admin.logout')) {
|
||||
// enrolled, and signing out. Both through AdminArea::routeIs(), not
|
||||
// a bare $request->routeIs() — a recovery hostname's routes are
|
||||
// named admin.viaN.*, and a bare check matches only the canonical
|
||||
// name, which would leave an unenrolled operator unable to sign out
|
||||
// on exactly the host that exists because the canonical one is not
|
||||
// working.
|
||||
if (AdminArea::routeIs('admin.two-factor-setup') || AdminArea::routeIs('admin.logout')) {
|
||||
return $next($request);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,9 +1,15 @@
|
|||
<?php
|
||||
|
||||
use App\Http\Middleware\RequireOperatorTwoFactor;
|
||||
use App\Http\Middleware\RestrictAdminHost;
|
||||
use App\Models\Operator;
|
||||
use App\Support\AdminArea;
|
||||
use App\Support\Settings;
|
||||
use Illuminate\Http\Request;
|
||||
use Illuminate\Routing\Route;
|
||||
use Illuminate\Routing\Route as RoutingRoute;
|
||||
use Illuminate\Routing\Router;
|
||||
use Illuminate\Support\Facades\Auth;
|
||||
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
|
||||
|
||||
/**
|
||||
|
|
@ -150,7 +156,7 @@ it('can cache its routes with several console hostnames configured', function ()
|
|||
config()->set('admin_access.hosts', ['admin.example.test', '127.0.0.1', 'localhost']);
|
||||
config()->set('admin_access.exclusive', true);
|
||||
|
||||
$router = new Illuminate\Routing\Router(app('events'), app());
|
||||
$router = new Router(app('events'), app());
|
||||
Illuminate\Support\Facades\Route::swap($router);
|
||||
require base_path('routes/web.php');
|
||||
$router->getRoutes()->refreshNameLookups();
|
||||
|
|
@ -174,7 +180,7 @@ it('keeps the navigation marked on a recovery hostname', function () {
|
|||
// serialise them — which silently breaks every exact name check, and the
|
||||
// navigation loses its active state exactly when something is already wrong.
|
||||
$named = fn (string $name) => tap(request(), fn ($r) => $r->setRouteResolver(
|
||||
fn () => (new Illuminate\Routing\Route(['GET'], '/', fn () => null))->name($name),
|
||||
fn () => (new Route(['GET'], '/', fn () => null))->name($name),
|
||||
));
|
||||
|
||||
$named('admin.overview');
|
||||
|
|
@ -184,3 +190,48 @@ it('keeps the navigation marked on a recovery hostname', function () {
|
|||
expect(AdminArea::routeIs('admin.overview'))->toBeTrue()
|
||||
->and(AdminArea::routeIs('admin.settings'))->toBeFalse();
|
||||
});
|
||||
|
||||
it('lets an unenrolled operator sign out on a recovery hostname, not just the canonical one', function () {
|
||||
// Whole-branch review, P2: RequireOperatorTwoFactor's exemption used
|
||||
// AdminArea::routeIs() for the setup route but bare
|
||||
// $request->routeIs('admin.logout') for logout — which matches only the
|
||||
// canonical name. On a recovery host the logout route is named
|
||||
// admin.via0.logout (same reason the navigation test above exists), so
|
||||
// the bare check failed there and the middleware bounced an unenrolled
|
||||
// operator's logout POST back to enrolment — trapping them on exactly
|
||||
// the host that exists because the canonical one is not working. A
|
||||
// canonical-host test would pass whichever form the check takes, so this
|
||||
// one is deliberately on the alternate.
|
||||
config()->set('admin_access.hosts', ['admin.example.test', 'admin-recovery.example.test']);
|
||||
config()->set('admin_access.exclusive', true);
|
||||
Settings::set('console.require_2fa', true);
|
||||
|
||||
// Register routes for real from the config just set, rather than
|
||||
// asserting the via0 name by fiat — same technique as "it can cache its
|
||||
// routes…" above. Proves the whole path: this host, this URL, genuinely
|
||||
// resolves to admin.via0.logout before the middleware is ever involved.
|
||||
$router = new Router(app('events'), app());
|
||||
Illuminate\Support\Facades\Route::swap($router);
|
||||
require base_path('routes/web.php');
|
||||
$router->getRoutes()->refreshNameLookups();
|
||||
|
||||
$matched = $router->getRoutes()->match(
|
||||
Request::create('http://admin-recovery.example.test/logout', 'POST'),
|
||||
);
|
||||
expect($matched->getName())->toBe('admin.via0.logout');
|
||||
|
||||
$operator = Operator::factory()->role('Owner')->create();
|
||||
Auth::guard('operator')->login($operator);
|
||||
|
||||
// Drive the middleware directly, on a request resolved to that genuinely
|
||||
// registered route — RequireOperatorTwoFactor reads the route's name, not
|
||||
// which router produced it, so this is the real check without needing a
|
||||
// full dispatch (session/CSRF middleware) around it.
|
||||
$request = Request::create('http://admin-recovery.example.test/logout', 'POST');
|
||||
$request->setRouteResolver(fn () => $matched);
|
||||
app()->instance('request', $request);
|
||||
|
||||
$response = (new RequireOperatorTwoFactor)->handle($request, fn ($req) => response('through'));
|
||||
|
||||
expect($response->getContent())->toBe('through');
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue