Merge pull request 'TECH-CHANNEL-AUTH-ORG-ADMIN — Extend submission.{id} channel auth to organisation admins' (#13) from feat/channel-auth-org-admin into main
Reviewed-on: #13
This commit was merged in pull request #13.
This commit is contained in:
@@ -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();
|
||||
},
|
||||
);
|
||||
|
||||
@@ -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.',
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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).~~ ✅
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in New Issue
Block a user