diff --git a/api/routes/channels.php b/api/routes/channels.php index b6ec7bc2..c5a62a10 100644 --- a/api/routes/channels.php +++ b/api/routes/channels.php @@ -3,6 +3,7 @@ declare(strict_types=1); use App\Models\FormBuilder\FormSubmission; +use App\Models\Organisation; use App\Models\User; use Illuminate\Support\Facades\Broadcast; @@ -30,11 +31,19 @@ use Illuminate\Support\Facades\Broadcast; * IdentityMatchBanner subscribes via Echo.private('submission.{id}') and * refetches the submission resource on receipt. * - * v1 authz scope: only the submitter who created the submission via an - * authenticated session is allowed to subscribe. Org-admin access is - * deferred — see BACKLOG entry TECH-CHANNEL-AUTH-ORG-ADMIN. Public - * (token-based) submitters are not on this channel; their flow is - * already polling-based and they don't have a User to authenticate with. + * Authorisation paths (in order of cheap-first short-circuit): + * 1. Submitter (submitted_by_user_id === user.id) — common case for the + * portal banner; no DB lookup beyond the submission itself. + * 2. super_admin (Spatie HasRoles, app-wide bypass) — debugging, + * impersonation, platform-level support. + * 3. Organisation admin of the submission's organisation — pivot-table + * check on user_organisation with role='org_admin'. Codebase + * canonical pattern, mirroring FormSubmissionActionFailurePolicy::canAccess + * (Spatie teams is disabled in config/permission.php; org-scoping + * lives in the pivot, not in Spatie). + * + * Public (token-based) submitters are not on this channel; their flow is + * polling-based and they don't have a User to authenticate with. */ Broadcast::channel( 'submission.{submissionId}', @@ -47,11 +56,32 @@ Broadcast::channel( return false; } - // TODO TECH-CHANNEL-AUTH-ORG-ADMIN — extend to organisation admins - // once we audit the Spatie Permission helper for an - // organisation-scoped role check (hasRoleInOrganisation or - // similar). Until that audit lands, only the submitter has - // channel access. - return $submission->submitted_by_user_id === $user->id; + // Submitter has access (authenticated session at submit time). + if ($submission->submitted_by_user_id === $user->id) { + return true; + } + + // super_admin app-wide bypass (Spatie HasRoles, global role). + if ($user->hasRole('super_admin')) { + return true; + } + + // Org admins of the submission's organisation. Pivot-table check + // matching the codebase's canonical pattern (see e.g. + // FormSubmissionActionFailurePolicy::canAccess). Spatie teams is + // disabled in config/permission.php, so org-scoping lives in the + // user_organisation pivot's `role` column, not Spatie. + $organisation = Organisation::query() + ->withoutGlobalScopes() + ->find($submission->organisation_id); + + if (! $organisation instanceof Organisation) { + return false; + } + + return $organisation->users() + ->where('user_id', $user->id) + ->wherePivot('role', 'org_admin') + ->exists(); }, ); 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.', - ); - } } diff --git a/dev-docs/BACKLOG.md b/dev-docs/BACKLOG.md index 84b4c3a5..2b6c04e8 100644 --- a/dev-docs/BACKLOG.md +++ b/dev-docs/BACKLOG.md @@ -560,7 +560,8 @@ maar niet de ontworpen UX. resource zodat de banner copy automatisch update naar de finale staat ('matched' / 'pending' / 'none') - Authorization wordt al afgedwongen door `routes/channels.php` callback - (submitter-only voor nu — zie ook TECH-CHANNEL-AUTH-ORG-ADMIN) + (submitter / super_admin / org_admin van submission's organisatie — + TECH-CHANNEL-AUTH-ORG-ADMIN closed mei 2026) - E2E test: simuleer een form submission, verify dat de banner copy in real-time van "we're checking matches…" naar de finale status flipt zonder reload @@ -1111,22 +1112,6 @@ ARCH-discussie en RFC. --- -### TECH-CHANNEL-AUTH-ORG-ADMIN — Extend `submission.{id}` private channel auth to organisation admins - -**Aanleiding:** WS-6 v1.3-delta D2 wires the broadcast event `FormSubmissionIdentityMatchResolved` (RFC-WS-6 §Q1 v1.3 addition 2) on the `submission.{id}` private channel. The auth callback in `routes/channels.php` currently authorises only the submitter (`submitted_by_user_id === user.id`). Org-admin access was deferred because the codebase does not yet have a vetted Spatie Permission helper for organisation-scoped role checks (e.g. `hasRoleInOrganisation('organizer_admin', $orgId)`); guessing the API would risk authorising too broadly or too narrowly without test coverage. Phase A audit confirmed no precedent for this check pattern in `app/`. - -**Wat:** -- Audit Spatie Permission usage across the codebase to identify (or design) the canonical "is X a role-holder in organisation Y" helper. Likely candidates: extension method on `User`, query scope on `Organisation::users()` pivot, or a dedicated Policy method. -- Extend `routes/channels.php`'s `submission.{submissionId}` callback to additionally authorise organisation admins of the submission's organisation. Replace the inline TODO with the resolved helper. -- Add channel authorization tests covering the four cases: submitter (allow), org admin (allow), other org user (deny), anonymous (deny). -- Update RFC-WS-6 §Q1 v1.3 addition 2 reference if the auth contract changes shape; otherwise leave RFC alone. - -**Prioriteit:** Medium — frontend Echo subscription is also out of WS-6 scope, so the channel has no live subscribers yet. When the frontend follow-up ships, org admins are expected to see live updates on submissions in their orgs — that is when this work blocks the user-facing feature. - -**Refs:** `api/routes/channels.php` (TODO marker in submission channel callback), RFC-WS-6 §Q1 v1.3 addition 2, ARCH-BINDINGS §11 (admin failures UI — analogous tenant scope via FK chain). - ---- - ### HARD-DEADLINE-QUERY-TIMEOUT **Aanleiding:** WS-6 v1.3-delta D2 (PR #11 `23a5696`, 2026-05-08) introduceerde @@ -1193,6 +1178,7 @@ deadline implementation). - ~~**TECH-DOCS-APPS-PORTAL-PURGE**: per-file DELETE/REWRITE/KEEP_AND_PURGE matrix uitgevoerd op alle 9 docs uit de oorspronkelijke entry, plus de `post-edit-eslint.sh` hook (out-of-scope vondst uit Phase A). Vijf obsolete docs verwijderd (`.cursor/instructions.md`, `.cursor/ARCHITECTURE.md`, `dev-docs/MASTER_PROMPT_CC.md`, `dev-docs/MASTER_PROMPT_CURSOR.md`, `dev-docs/dev-guide.md` — totaal ~80 KB). Drie herschreven (`SETUP.md`, `101_vue.mdc`, hook-script). Twee chirurgisch gepurgeerd (`102_multi_tenancy.mdc`, `CLAUDE_CODE_TOOLING.md`). Externe verwijzingen in README.md, CLAUDE_DESKTOP_SETUP.md, ARCH-CONSOLIDATION-2026-04.md en VIBE_CODING_CHECKLIST.md mee bijgewerkt. WS-3 PR-C op 2026-05-06. Single SPA, single cookie, single deploy host. WS-3 compleet.~~ ✅ - ~~**WS-7 Observability — closure (mei 2026)**: 4 PRs gemerged op `feat/ws-7-observability` (infra `5f6fc07`, backend SDK `bdb89a2..0379016`, frontend SDK `bc47783..5c42f27`, docs `754222f..e9da01f`). 1551 backend + 252 frontend tests groen. Acceptance criteria 1-14 voldaan; observability volledig operationeel op `monitoring.hausdesign.nl`. Implementation criteria 3, 4, 5, 6, 8, 11, 12, 13, 14 via PRs; operationele criteria 1, 2, 7, 9, 10 via deploy-checklist (DNS, TLS, superuser+2FA, prod DSNs, email-alerting, retention 90d, cron backup). Architecturale patronen vastgelegd in `dev-docs/ARCH-OBSERVABILITY.md` (730 regels) + 2 runbooks (`observability-triage.md`, `observability-erasure.md`). Twee GlitchTip projecten (`crewli-api` + `crewli-app`), één DSN per project, runtime context-split via `actor_scope` tag. Patronen: explicit > implicit listener registration, default-in-listener / override-in-middleware voor binary tags, tenant resolution chain (route-param → portal-token → super_admin platform → user fallback). Volgsporen: OBS-1, OBS-4, OBS-6, OBS-7, OBS-9 (zie "Observability follow-ups" sectie hieronder).~~ ✅ - ~~**WS-6 v1.3-delta — closure (mei 2026)**: Architecturele review-sessie 2026-05-07 identificeerde vijf verfijningen op RFC-WS-6 v1.2 (Q1 listener queueing, Q2 invariant cleanup, Q3 failure-UX additions, plus §19 BACKLOG-pointer). v1.3 amendement gecommit (`845b6e6`, 2026-05-07); v1.3.1 drift closure (`b255879`, 2026-05-08) sloot code-vs-docs gaten pre-implementation. Implementatie geland als D1 + D2: **D1** (PR #10 `c6f4d1b`) leverde de data-laag — `failure_response_code` kolom op `form_submissions`, abstract `FormBindingApplicatorException` hiërarchie + 4 reason-coded subclasses (`FormBindingSchemaConfigException`, `FormBindingInfraException`, `FormBindingApplicatorTimeoutException`, `FormBindingDataIntegrityException`), `IdentityMatchInvariantViolation` sibling, `FormBindingExceptionClassifier` helper, `FormSubmissionIdentityMatchResolved` broadcast event class, `FormFieldBindingMergeStrategy::validForTargetType` matrix method, cast + factory state. **D2** (PR #11 `23a5696`) wired alle building blocks in de listener-chain — `ApplyBindings` initial `pending` write + deadline wrapper + classifier in catch; `TriggerPersonIdentityMatch` queued + gating-invariant + invariant throw + broadcast dispatch; `routes/channels.php` + bootstrap routing (NIEUWE broadcast wiring, submitter-only auth); gating-invariant op `SyncTagPicker`; `AppServiceProvider::boot` v1.3 layout; `FormFailureRetryService::recordFailure` classifier + apply_completed_at symmetrie-fix; `apply_deadline_seconds` config key (default 5). Tests: pre-WS-6 baseline 1208 → pre-D1 1551 → post-D2 1621. 0 Larastan errors. Phase F (`ConditionalRequirement(public_token)` wrapper drop) was no-op — change had silently landed pre-D2. **Open follow-ups:** `TECH-CHANNEL-AUTH-ORG-ADMIN` (extend `submission.{id}` channel auth to org admins na Spatie Permission helper-audit); GlitchTip alert rule op `apply_status=failed AND form_schema.has_public_token=true` (operationele taak in GlitchTip web-UI op `monitoring.hausdesign.nl`; runbook procedure in `dev-docs/runbooks/observability-triage.md` §7); frontend Echo subscription voor `FormSubmissionIdentityMatchResolved` (separate frontend follow-up, out of WS-6 scope, backend-infra ready). `PARTIAL-BINDING-SUCCESS` en `FORM-SCHEMA-DRIFT-DETECTION` blijven open conform v1.3 amendement (trigger-condities nog niet gevuurd). Closure docs-PR: RFC-WS-6.md v1.3.1 implementation-status marker + §10 closure entry, ARCH-BINDINGS.md v1.2 onveranderd, runbook §7 toegevoegd.~~ ✅ +- ~~**TECH-CHANNEL-AUTH-ORG-ADMIN — closure (mei 2026)**: `submission.{id}` private channel auth uitgebreid van submitter-only naar drie-paths: submitter (`submitted_by_user_id === user.id`) → super_admin Spatie HasRoles app-wide bypass → org_admin van submission's organisatie via pivot-table check op `user_organisation` (`->wherePivot('role', 'org_admin')`). Pattern: directe port van `FormSubmissionActionFailurePolicy::canAccess`, codebase canonical (gebruikt in 17+ policy sites). Spatie teams is disabled in `config/permission.php`, dus org-scoping leeft in de pivot, niet in Spatie. **super_admin bypass is een audit-surfaced bonus** (origineel BACKLOG entry vroeg alleen om org-admin extension; tijdens Phase A audit bleek dat elke analoge policy super_admin bypass heeft, dus toegevoegd voor consistency — zonder die bypass zouden super_admins op de admin-panel banner mysterieus geen live updates krijgen). Tests: 4 nieuw (`test_super_admin_can_subscribe`, `test_organisation_admin_of_submission_org_can_subscribe`, `test_organisation_admin_of_different_org_cannot_subscribe` (kritische cross-tenant guard), `test_regular_organisation_member_cannot_subscribe`); 1 verwijderd (de "should flip" denied-by-default test uit PR #11). Test count: 1621 → 1624 (+3 net). 0 Larastan errors. Inline TODO uit `routes/channels.php` verwijderd. Sibling `FRONTEND-ECHO-IDENTITY-MATCH-SUBSCRIPTION` blijft open (frontend portal IdentityMatchBanner subscription is de pair met deze backend-auth uitbreiding).~~ ✅ ---