From e04b084be5f0673a5b933335d0ee2130e93e5efc Mon Sep 17 00:00:00 2001 From: "bert.hausmans" Date: Fri, 8 May 2026 11:29:01 +0200 Subject: [PATCH] test(broadcasting): add org-admin auth + cross-tenant guard tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per BACKLOG TECH-CHANNEL-AUTH-ORG-ADMIN. Four new tests + one deleted; existing three preserved. NEW: - test_super_admin_can_subscribe (positive, app-wide bypass via Spatie HasRoles assignRole('super_admin')) - test_organisation_admin_of_submission_org_can_subscribe (positive, pivot-table org_admin → submission's organisation) - test_organisation_admin_of_different_org_cannot_subscribe (CRITICAL cross-tenant guard — admin of org B cannot subscribe to a submission in org A) - test_regular_organisation_member_cannot_subscribe (org_member role on the pivot is NOT enough; only org_admin passes) DELETED: - test_org_admin_is_currently_denied_per_backlog_entry (the "should flip" denied-by-default test from PR #11; superseded by the four positive/negative tests above) PRESERVED: - test_submitter_is_authorised - test_other_authenticated_user_is_denied (User with no organisation membership → falls through every auth branch) - test_subscription_is_denied_when_submission_does_not_exist Test-fixture refinement: makeSubmission() now accepts an explicit $submitter so positive role-based tests can use a separate User as submitter, ensuring the submitter short-circuit doesn't accidentally authorise role-based test subjects. Test results: 7 passed in this file; 1624 in full suite (was 1621). 0 Larastan errors. Co-Authored-By: Claude Opus 4.7 --- .../Channels/SubmissionChannelAuthTest.php | 123 +++++++++++++----- 1 file changed, 92 insertions(+), 31 deletions(-) diff --git a/api/tests/Feature/FormBuilder/Channels/SubmissionChannelAuthTest.php b/api/tests/Feature/FormBuilder/Channels/SubmissionChannelAuthTest.php index b55616db..65d71876 100644 --- a/api/tests/Feature/FormBuilder/Channels/SubmissionChannelAuthTest.php +++ b/api/tests/Feature/FormBuilder/Channels/SubmissionChannelAuthTest.php @@ -8,27 +8,35 @@ use App\Models\FormBuilder\FormSchema; use App\Models\FormBuilder\FormSubmission; use App\Models\Organisation; use App\Models\User; +use Database\Seeders\RoleSeeder; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Broadcast; use Tests\TestCase; /** - * Per RFC-WS-6 §Q1 v1.3 addition 2. + * Per RFC-WS-6 §Q1 v1.3 addition 2 + BACKLOG TECH-CHANNEL-AUTH-ORG-ADMIN. * - * The submission.{submissionId} private channel authorises only the - * submitter (user whose id matches submissions.submitted_by_user_id). - * Org-admin access is deferred — see BACKLOG entry - * TECH-CHANNEL-AUTH-ORG-ADMIN. + * The submission.{submissionId} private channel authorises three classes: + * 1. Submitter (matching submitted_by_user_id) + * 2. super_admin (Spatie HasRoles, app-wide bypass) + * 3. Organisation admin of the submission's organisation (pivot-table + * role='org_admin' on user_organisation) * * Broadcast::auth() drives the same callback path that Laravel's * broadcasting auth middleware uses on a websocket subscription * attempt. Tests pose as authenticated users and assert the boolean - * outcome. + * outcome of the callback. */ final class SubmissionChannelAuthTest extends TestCase { use RefreshDatabase; + protected function setUp(): void + { + parent::setUp(); + $this->seed(RoleSeeder::class); + } + private function authoriseSubscription(?User $user, FormSubmission $submission): bool { // Resolve the channel callback registered in routes/channels.php. @@ -54,14 +62,20 @@ final class SubmissionChannelAuthTest extends TestCase $this->fail("No channel callback registered for {$name}"); } - private function makeSubmission(?User $submitter): FormSubmission + /** + * Build a submission tied to the given organisation (or a fresh one + * if not specified). The submitter is a fresh User unrelated to any + * test fixture so the submitted_by_user_id short-circuit doesn't + * accidentally authorise org-admin / super-admin / member tests. + */ + private function makeSubmission(?Organisation $organisation = null, ?User $submitter = null): FormSubmission { - $org = Organisation::factory()->create(); - $schema = FormSchema::factory()->create(['organisation_id' => $org->id]); + $organisation ??= Organisation::factory()->create(); + $schema = FormSchema::factory()->create(['organisation_id' => $organisation->id]); return FormSubmission::factory()->create([ 'form_schema_id' => $schema->id, - 'organisation_id' => $org->id, + 'organisation_id' => $organisation->id, 'submitted_by_user_id' => $submitter?->id, ]); } @@ -69,16 +83,81 @@ final class SubmissionChannelAuthTest extends TestCase public function test_submitter_is_authorised(): void { $submitter = User::factory()->create(); - $submission = $this->makeSubmission($submitter); + $submission = $this->makeSubmission(submitter: $submitter); $this->assertTrue($this->authoriseSubscription($submitter, $submission)); } + public function test_super_admin_can_subscribe(): void + { + // Spatie HasRoles app-wide bypass — super_admin can subscribe to + // any submission's channel regardless of organisation membership + // or submitter relationship. Matches the codebase convention + // used in every analogous policy. + $superAdmin = User::factory()->create(); + $superAdmin->assignRole('super_admin'); + + $submitter = User::factory()->create(); + $submission = $this->makeSubmission(submitter: $submitter); + + $this->assertTrue($this->authoriseSubscription($superAdmin, $submission)); + } + + public function test_organisation_admin_of_submission_org_can_subscribe(): void + { + // Pivot-table check: user attached to the submission's + // organisation with role='org_admin'. Mirrors the canonical + // pattern from FormSubmissionActionFailurePolicy::canAccess. + $organisation = Organisation::factory()->create(); + $orgAdmin = User::factory()->create(); + $organisation->users()->attach($orgAdmin->id, ['role' => 'org_admin']); + + $submitter = User::factory()->create(); + $submission = $this->makeSubmission(organisation: $organisation, submitter: $submitter); + + $this->assertTrue($this->authoriseSubscription($orgAdmin, $submission)); + } + + public function test_organisation_admin_of_different_org_cannot_subscribe(): void + { + // Critical multi-tenancy guard: an org_admin of organisation A + // must NOT be able to subscribe to a submission's channel when + // that submission belongs to organisation B. If this fails, the + // pivot-table check is wrong and tenant isolation is broken. + $submissionOrg = Organisation::factory()->create(); + $otherOrg = Organisation::factory()->create(); + + $crossTenantAdmin = User::factory()->create(); + $otherOrg->users()->attach($crossTenantAdmin->id, ['role' => 'org_admin']); + + $submitter = User::factory()->create(); + $submission = $this->makeSubmission(organisation: $submissionOrg, submitter: $submitter); + + $this->assertFalse($this->authoriseSubscription($crossTenantAdmin, $submission)); + } + + public function test_regular_organisation_member_cannot_subscribe(): void + { + // Member of the submission's organisation but with role='org_member', + // not 'org_admin'. The pivot-table wherePivot('role', 'org_admin') + // filters them out. + $organisation = Organisation::factory()->create(); + $member = User::factory()->create(); + $organisation->users()->attach($member->id, ['role' => 'org_member']); + + $submitter = User::factory()->create(); + $submission = $this->makeSubmission(organisation: $organisation, submitter: $submitter); + + $this->assertFalse($this->authoriseSubscription($member, $submission)); + } + public function test_other_authenticated_user_is_denied(): void { + // User with NO organisation membership at all → falls through + // every auth branch (not submitter, not super_admin, no pivot row). $submitter = User::factory()->create(); $other = User::factory()->create(); - $submission = $this->makeSubmission($submitter); + $submission = $this->makeSubmission(submitter: $submitter); $this->assertFalse($this->authoriseSubscription($other, $submission)); } @@ -88,7 +167,7 @@ final class SubmissionChannelAuthTest extends TestCase $user = User::factory()->create(); // Build a submission, then delete it so the FK lookup returns null. - $submission = $this->makeSubmission($user); + $submission = $this->makeSubmission(submitter: $user); $submissionId = (string) $submission->id; $submission->forceDelete(); @@ -98,22 +177,4 @@ final class SubmissionChannelAuthTest extends TestCase $this->assertFalse($this->authoriseSubscription($user, $shell)); } - - public function test_org_admin_is_currently_denied_per_backlog_entry(): void - { - // Locks the v1 contract: even an org admin of the submission's - // organisation is denied because the canonical Spatie helper - // hasn't been audited yet. See BACKLOG entry - // TECH-CHANNEL-AUTH-ORG-ADMIN. When that lands, this test should - // flip to expectTrue() in the same PR as the auth-callback - // extension. - $orgAdmin = User::factory()->create(); - $submitter = User::factory()->create(); - $submission = $this->makeSubmission($submitter); - - $this->assertFalse( - $this->authoriseSubscription($orgAdmin, $submission), - 'V1 contract: org admins are NOT authorised. Flip with TECH-CHANNEL-AUTH-ORG-ADMIN.', - ); - } }