diff --git a/app/Http/Middleware/RequireOperatorTwoFactor.php b/app/Http/Middleware/RequireOperatorTwoFactor.php index 8ece44d..bec7898 100644 --- a/app/Http/Middleware/RequireOperatorTwoFactor.php +++ b/app/Http/Middleware/RequireOperatorTwoFactor.php @@ -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); } diff --git a/tests/Feature/Admin/ConsoleHostSeparationTest.php b/tests/Feature/Admin/ConsoleHostSeparationTest.php index f26dd39..9518ec3 100644 --- a/tests/Feature/Admin/ConsoleHostSeparationTest.php +++ b/tests/Feature/Admin/ConsoleHostSeparationTest.php @@ -1,9 +1,15 @@ 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'); +});