From a939820122b4065ef9af229d8fb3b6eaa96298a3 Mon Sep 17 00:00:00 2001 From: "bert.hausmans" Date: Thu, 7 May 2026 17:30:27 +0200 Subject: [PATCH] fix: impersonation.active default tag for non-impersonation authenticated events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC §3.6 vereist impersonation.active als always-present binary signal op authenticated events. Originele PR-2 architectural-fixes verplaatste impersonation-tagging naar HandleImpersonation middleware, die alleen draait bij actieve impersonation. Resultaat: non-impersonation events hadden GEEN tag, niet 'false' tag — wat filtering op "alle impersonation events" in GlitchTip stilletjes onmogelijk maakte. Fix: AuthScopeContextListener::bindForUser() zet baseline 'false'; HandleImpersonation overschrijft naar 'true' + impersonator_user_id wanneer actief. Default-in-listener, override-in-middleware pattern. HandleImpersonation deed de override-set al correct sinds commit 9414d09; alleen de baseline ontbrak. Bert's live verification toonde de gap: super_admin event zonder impersonation actief, GlitchTip event zonder impersonation.active tag. Tests: - test_impersonation_active_default_false_for_non_impersonation_authenticated_event (was test_authenticated_event_does_not_set_impersonation_tags; hernoemd + assertion gewijzigd) - test_impersonation_active_default_false_across_every_actor_scope_branch walks elke actor_scope branch (user/organisation/platform) en bewijst baseline geldt uniform — vangt toekomstige refactors die per branch vroegtijdig returnen. Test count 1544 to 1545. Larastan + Pint clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../AuthScopeContextListener.php | 5 ++ .../AuthScopeContextListenerTest.php | 54 ++++++++++++++++++- 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/api/app/Listeners/Observability/AuthScopeContextListener.php b/api/app/Listeners/Observability/AuthScopeContextListener.php index e857b775..68fcff10 100644 --- a/api/app/Listeners/Observability/AuthScopeContextListener.php +++ b/api/app/Listeners/Observability/AuthScopeContextListener.php @@ -77,6 +77,11 @@ final class AuthScopeContextListener 'id' => $user->id, 'username' => $user->id, // RFC §3.8: ULID, never email. ]); + // Default baseline; HandleImpersonation middleware overschrijft + // naar 'true' + zet impersonation.impersonator_user_id wanneer + // impersonation actief is. RFC §3.6 vereist always-present + // binary signal voor betrouwbare filtering. + $scope->setTag('impersonation.active', 'false'); $scope->setTag('user_id', $user->id); $scope->setTag('actor_type', $actorType->value); $scope->setTag('actor_scope', $actorScope); diff --git a/api/tests/Feature/Observability/AuthScopeContextListenerTest.php b/api/tests/Feature/Observability/AuthScopeContextListenerTest.php index cdbf2925..548a5a9d 100644 --- a/api/tests/Feature/Observability/AuthScopeContextListenerTest.php +++ b/api/tests/Feature/Observability/AuthScopeContextListenerTest.php @@ -251,18 +251,68 @@ final class AuthScopeContextListenerTest extends TestCase $this->assertTrue(\Symfony\Component\Uid\Ulid::isValid($tags['organisation_id'])); } - public function test_authenticated_event_does_not_set_impersonation_tags(): void + public function test_impersonation_active_default_false_for_non_impersonation_authenticated_event(): void { + // RFC §3.6 binary signal invariant: impersonation.active MUST be + // present on every authenticated event. Listener seeds 'false'; + // HandleImpersonation overrides to 'true' when active. $user = User::factory()->create(); $user->assignRole('org_admin'); event(new Authenticated('web', $user)); $tags = $this->captureScopeTags(); - $this->assertArrayNotHasKey('impersonation.active', $tags); + $this->assertSame('false', $tags['impersonation.active'] ?? null); $this->assertArrayNotHasKey('impersonation.impersonator_user_id', $tags); } + public function test_impersonation_active_default_false_across_every_actor_scope_branch(): void + { + // Hard invariant guard: every actor_scope branch in the listener + // must seed impersonation.active='false'. Future refactors that + // shortcut branches (e.g. early-return on platform) cannot silently + // drop the binary signal. + $org = Organisation::factory()->create(); + $superAdmin = User::factory()->create(); + $superAdmin->assignRole('super_admin'); + $orgAdmin = User::factory()->create(); + $orgAdmin->assignRole('org_admin'); + $orgMember = User::factory()->create(); + $orgMember->assignRole('org_member'); + + $cases = [ + 'user' => fn () => event(new Authenticated('web', $orgMember)), + 'organisation' => function () use ($org, $orgAdmin): void { + $request = Request::create('http://localhost/api/v1/organisations/'.$org->id, 'GET'); + $route = new \Illuminate\Routing\Route(['GET'], 'organisations/{organisation}', static fn () => null); + $route->bind($request); + $route->setParameter('organisation', $org); + $route->name('organisations.show'); + $request->setRouteResolver(static fn () => $route); + $this->app->instance('request', $request); + event(new Authenticated('web', $orgAdmin)); + }, + 'platform' => function () use ($superAdmin): void { + $request = Request::create('http://localhost/api/v1/admin/users', 'GET'); + $route = new \Illuminate\Routing\Route(['GET'], 'admin/users', static fn () => null); + $route->bind($request); + $route->name('admin.users.index'); + $request->setRouteResolver(static fn () => $route); + $this->app->instance('request', $request); + event(new Authenticated('web', $superAdmin)); + }, + ]; + + foreach ($cases as $label => $trigger) { + SentrySdk::getCurrentHub()->pushScope(); + $trigger(); + $tags = $this->captureScopeTags(); + $this->assertSame($label, $tags['actor_scope'] ?? null, "actor_scope mismatch for {$label} branch"); + $this->assertSame('false', $tags['impersonation.active'] ?? null, + "impersonation.active baseline missing for {$label} branch"); + } + } + public function test_handle_impersonation_rebinds_user_id_and_tags_impersonation_after_swap(): void { Organisation::factory()->create(); // tenancy fixture