From adab3be78170d00a9b80e328342779de9e79b778 Mon Sep 17 00:00:00 2001 From: "bert.hausmans" Date: Wed, 6 May 2026 13:58:42 +0200 Subject: [PATCH] fix: register AuthScopeContextListener for Sanctum bearer-token flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live HTTP smoke test on the post-architectural-fixes branch surfaced that captured Sentry events carried only route-scope tags (app, route_name, http.method) — auth-scope tags (user_id, actor_type, actor_scope) were absent on every request. Root cause: Sanctum's Guard fires Laravel\Sanctum\Events\TokenAuthenticated (vendor/laravel/sanctum/src/Guard.php:77) on bearer-token resolution, NOT Illuminate\Auth\Events\Authenticated. The Authenticated event only fires from SessionGuard (vendor/laravel/framework/src/Illuminate/Auth/SessionGuard.php:833), which Crewli does not use — CookieBearerToken middleware injects the httpOnly cookie as Authorization: Bearer, then auth:sanctum invokes Sanctum's Guard. So the listener never ran on Crewli's HTTP path. Offline tests in AuthScopeContextListenerTest passed because they dispatch event(new Authenticated(...)) directly, bypassing the Guard layer. Sanctum::actingAs() in tests has the same blind spot — it short-circuits the Guard via guard('sanctum')->setUser() and fires neither event. Fix: - New handleTokenAuthenticated(TokenAuthenticated $event) method on AuthScopeContextListener extracts the user via $event->token->tokenable and delegates to a private bindForUser() shared with handle(). - AppServiceProvider registers the listener for both Authenticated (covers SessionGuard / login flow / future authenticators) and TokenAuthenticated (covers Crewli's bearer-token Sanctum flow). Regression coverage: AuthScopeBindingHttpFlowTest exercises the real Sanctum Guard via $user->createToken() + Authorization: Bearer header. Three cases: - super_admin on a user-scope route: actor_scope=user, all auth tags present. - super_admin on an admin.* route: actor_scope=platform, no organisation_id (correct platform-mode behaviour). - org_admin on a route with {organisation} param: actor_scope= organisation, organisation_id valid ULID. Test count 1541 to 1544. Larastan clean. Pint clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../AuthScopeContextListener.php | 47 ++++-- api/app/Providers/AppServiceProvider.php | 16 +- .../AuthScopeBindingHttpFlowTest.php | 145 ++++++++++++++++++ 3 files changed, 193 insertions(+), 15 deletions(-) create mode 100644 api/tests/Feature/Observability/AuthScopeBindingHttpFlowTest.php diff --git a/api/app/Listeners/Observability/AuthScopeContextListener.php b/api/app/Listeners/Observability/AuthScopeContextListener.php index 131f7837..e857b775 100644 --- a/api/app/Listeners/Observability/AuthScopeContextListener.php +++ b/api/app/Listeners/Observability/AuthScopeContextListener.php @@ -11,25 +11,34 @@ use App\Models\User; use Illuminate\Auth\Events\Authenticated; use Illuminate\Http\Request; use Illuminate\Support\Facades\Log; +use Laravel\Sanctum\Events\TokenAuthenticated; use Sentry\State\Scope; use function Sentry\configureScope; /** * Binds auth-scope context to Sentry and Laravel log context on every - * successful authentication (Sanctum, portal-token, future authenticators). + * successful authentication. * - * Auth-scope tags (user_id, actor_type, organisation_id, actor_scope) are - * decoupled from route-scope tags (which live in BindSentryRouteContext) - * so that authentication-mechanism additions don't require touching every - * route-group's middleware stack. + * Listens to TWO events: + * - {@see Authenticated} fires from SessionGuard (login flow / future + * cookie-session authenticators). + * - {@see TokenAuthenticated} fires from {@see \Laravel\Sanctum\Guard} + * on every bearer-token resolution. Crewli's HTTP flow is + * bearer-token (CookieBearerToken middleware reads the httpOnly + * cookie and injects Authorization: Bearer); without listening to + * TokenAuthenticated, no auth-scope tags would ever bind on live + * requests — a regression the offline tests miss because they + * dispatch Authenticated directly. * - * Lifecycle nuance for impersonation: Sanctum's auth resolves to the - * admin user and fires Authenticated → this listener tags the admin. - * Then HandleImpersonation middleware swaps the auth user to the target - * and re-binds Sentry scope itself (impersonation.active=true plus the - * target's user_id/actor_type). That keeps impersonation-specific logic - * co-located with the middleware that performs the swap. + * Auth-scope tags (user_id, actor_type, organisation_id, actor_scope) + * are decoupled from route-scope tags (BindSentryRouteContext middleware) + * so that authentication-mechanism additions don't require touching + * every route-group's middleware stack. + * + * Impersonation re-binding (target user_id + impersonation.* tags) is + * co-located in HandleImpersonation middleware and runs after the user + * swap. * * RFC-WS-7 §3.6, §3.13 (Log::withContext OBS-2 fix). */ @@ -43,6 +52,22 @@ final class AuthScopeContextListener return; } + $this->bindForUser($user); + } + + public function handleTokenAuthenticated(TokenAuthenticated $event): void + { + $tokenable = $event->token->tokenable ?? null; + + if (! $tokenable instanceof User) { + return; + } + + $this->bindForUser($tokenable); + } + + private function bindForUser(User $user): void + { $request = request(); $actorType = ActorType::resolve($user, $request); [$organisationId, $actorScope] = $this->resolveTenantContext($user, $request); diff --git a/api/app/Providers/AppServiceProvider.php b/api/app/Providers/AppServiceProvider.php index 842eedbf..699accae 100644 --- a/api/app/Providers/AppServiceProvider.php +++ b/api/app/Providers/AppServiceProvider.php @@ -205,12 +205,20 @@ class AppServiceProvider extends ServiceProvider ); // RFC-WS-7 §3.6 — auth-scope Sentry tags + Log::withContext on - // every successful authentication. Decoupled from middleware so - // future authenticators (e.g. portal-token) only need to fire - // the Authenticated event for tagging to work. + // every successful authentication. Listens to two events: + // - Authenticated covers SessionGuard flows (login etc.). + // - TokenAuthenticated covers Sanctum bearer-token flows; this + // is Crewli's actual SPA auth path because CookieBearerToken + // middleware injects the cookie as an Authorization header. + // Without this, live HTTP events would carry no auth-scope + // tags even though the offline (event-dispatch) tests pass. \Illuminate\Support\Facades\Event::listen( \Illuminate\Auth\Events\Authenticated::class, - \App\Listeners\Observability\AuthScopeContextListener::class, + [\App\Listeners\Observability\AuthScopeContextListener::class, 'handle'], + ); + \Illuminate\Support\Facades\Event::listen( + \Laravel\Sanctum\Events\TokenAuthenticated::class, + [\App\Listeners\Observability\AuthScopeContextListener::class, 'handleTokenAuthenticated'], ); ResetPassword::createUrlUsing(function ($user, string $token) { diff --git a/api/tests/Feature/Observability/AuthScopeBindingHttpFlowTest.php b/api/tests/Feature/Observability/AuthScopeBindingHttpFlowTest.php new file mode 100644 index 00000000..f501b4b2 --- /dev/null +++ b/api/tests/Feature/Observability/AuthScopeBindingHttpFlowTest.php @@ -0,0 +1,145 @@ +createToken() and passes Authorization: Bearer in the + * request — Sanctum::actingAs() short-circuits the Guard layer and would + * NOT detect the regression. + */ +final class AuthScopeBindingHttpFlowTest extends TestCase +{ + use RefreshDatabase; + + /** + * Captured events from the recording before_send hook. + * + * @var list + */ + private static array $captured = []; + + protected function setUp(): void + { + parent::setUp(); + $this->seed(RoleSeeder::class); + self::$captured = []; + + $clientBuilder = ClientBuilder::create([ + 'dsn' => 'https://test@localhost/1', + 'environment' => 'testing', + 'release' => 'crewli-api@test', + 'send_default_pii' => false, + 'traces_sample_rate' => 0.0, + 'profiles_sample_rate' => 0.0, + 'before_send' => static function (SentryEvent $event, ?EventHint $hint = null): ?SentryEvent { + self::$captured[] = ['event' => $event, 'hint' => $hint]; + + return null; + }, + ]); + SentrySdk::setCurrentHub(new Hub($clientBuilder->getClient())); + } + + /** + * @return array + */ + private function authHeader(User $user): array + { + $token = $user->createToken('regression-test')->plainTextToken; + + return ['Authorization' => 'Bearer '.$token]; + } + + public function test_authenticated_http_request_captures_auth_scope_tags_on_thrown_exception(): void + { + $user = User::factory()->create(); + $user->assignRole('super_admin'); + + Route::middleware(['auth:sanctum', \App\Http\Middleware\BindSentryRouteContext::class])->group(function (): void { + Route::get('_obs_authflow_throw', static fn () => throw new RuntimeException('regression-test')) + ->name('test.obs.authflow_throw'); + }); + + $response = $this->withHeaders($this->authHeader($user))->getJson('/_obs_authflow_throw'); + $response->assertStatus(500); + + $this->assertCount(1, self::$captured, 'Sentry event must be captured for thrown RuntimeException'); + $tags = self::$captured[0]['event']->getTags(); + + $this->assertSame($user->id, $tags['user_id'] ?? null, 'user_id tag missing on live HTTP flow'); + $this->assertSame('super_admin', $tags['actor_type'] ?? null, 'actor_type tag missing on live HTTP flow'); + $this->assertArrayHasKey('actor_scope', $tags, 'actor_scope tag missing on live HTTP flow'); + } + + public function test_authenticated_http_request_to_admin_route_tags_actor_scope_platform(): void + { + $user = User::factory()->create(); + $user->assignRole('super_admin'); + + Route::middleware(['auth:sanctum', \App\Http\Middleware\BindSentryRouteContext::class]) + ->name('admin.') + ->group(function (): void { + Route::get('_obs_admin_throw', static fn () => throw new RuntimeException('regression-test')) + ->name('platform_throw'); + }); + + $this->withHeaders($this->authHeader($user))->getJson('/_obs_admin_throw'); + + $tags = self::$captured[0]['event']->getTags(); + + $this->assertSame('platform', $tags['actor_scope'] ?? null, + 'super_admin on admin.* route must tag actor_scope=platform'); + $this->assertArrayNotHasKey('organisation_id', $tags, + 'organisation_id MUST be absent on platform-scoped events'); + } + + public function test_authenticated_http_request_to_organisation_route_tags_organisation_scope(): void + { + $org = Organisation::factory()->create(); + $user = User::factory()->create(); + $org->users()->attach($user, ['role' => 'org_admin']); + $user->assignRole('org_admin'); + + Route::middleware(['auth:sanctum', \App\Http\Middleware\BindSentryRouteContext::class])->group(function (): void { + Route::get('_obs_org_throw/{organisation}', static fn () => throw new RuntimeException('regression-test')) + ->name('test.obs.org_throw'); + }); + + $this->withHeaders($this->authHeader($user))->getJson('/_obs_org_throw/'.$org->id); + + $tags = self::$captured[0]['event']->getTags(); + + $this->assertSame('organisation', $tags['actor_scope'] ?? null); + $this->assertSame($org->id, $tags['organisation_id'] ?? null); + $this->assertTrue(Ulid::isValid($tags['organisation_id'])); + } +}