bert.hausmans/crewli
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

TECH-CHANNEL-AUTH-ORG-ADMIN — Extend submission.{id} channel auth to organisation admins #13

Merged
bert.hausmans merged 3 commits from feat/channel-auth-org-admin into main 2026-05-08 12:24:21 +02:00
Conversation 0 Commits 3 Files Changed 3 +136 -59
bert.hausmans commented 2026-05-08 12:23:52 +02:00
Owner
Copy Link

TECH-CHANNEL-AUTH-ORG-ADMIN — Extend submission.{id} channel auth to organisation admins

Closes BACKLOG entry TECH-CHANNEL-AUTH-ORG-ADMIN. Extends the submission.{submissionId} private broadcast channel authorization from submitter-only (introduced in WS-6 v1.3-delta D2, PR #11) to a three-path check: submitter, super_admin (app-wide bypass), or organisation admin of the submission's organisation.

The original deferral reason (Spatie Permission helper convention not yet audited) is now closed: Phase A audit established the codebase's canonical pattern, used in 17+ policy sites.

Refs

  • BACKLOG TECH-CHANNEL-AUTH-ORG-ADMIN — closed by this PR
  • BACKLOG FRONTEND-ECHO-IDENTITY-MATCH-SUBSCRIPTION — sibling, still open (frontend portal Echo subscription)
  • WS-6 v1.3-delta D2 (PR #11 23a5696) — introduced the channel + submitter-only auth + denied-by-default org-admin contract test

What this PR delivers

api/routes/channels.php — three-path auth callback (commit f5cb371)

Direct port of the FormSubmissionActionFailurePolicy::canAccess pattern (codebase canonical, used in 17+ policy sites). Auth resolution:

  1. submitted_by_user_id === user.id — submitter short-circuit (fast path, no DB lookup)
  2. $user->hasRole('super_admin') — Spatie HasRoles app-wide bypass
  3. Pivot-table check: $organisation->users()->where('user_id', $user->id)->wherePivot('role', 'org_admin')->exists() — org-scoped admin authorization

withoutGlobalScopes() preserved on both FormSubmission and Organisation lookups; channel auth is a structural gate, not a tenant-scoped query.

Why three paths instead of two: The super_admin bypass was an audit-surfaced bonus addition. Every analogous policy in the codebase includes it; without it super_admins (impersonating, debugging, troubleshooting in production) would mysteriously fail to subscribe even though they can do everything else in the platform. Pattern consistency wins.

Why pivot-table check instead of Spatie scoped roles: Spatie's teams feature is disabled in this codebase (config/permission.php). Org-scoping lives in the user_organisation pivot's role column, not in Spatie's model_has_roles table. Role::findOrCreate('org_admin', 'web') exists in Spatie too (see RoleSeeder.php) but no code uses $user->hasRole('org_admin') without scope — the pivot-table check is the canonical pattern.

Tests (commit e04b084)

# Test Coverage
New test_super_admin_can_subscribe App-wide bypass works
New test_organisation_admin_of_submission_org_can_subscribe Positive case for the new auth path
New test_organisation_admin_of_different_org_cannot_subscribe Critical multi-tenancy guard — admin of org A cannot subscribe to a submission in org B
New test_regular_organisation_member_cannot_subscribe Non-admin member of submission's org is still denied
Deleted test_org_admin_is_currently_denied_per_backlog_entry The "should flip" test from PR #11; replaced by positive test above

Existing tests preserved (per Phase A compatibility check — they test scenarios orthogonal to the new path):

  • test_submitter_is_authorised
  • test_other_authenticated_user_is_denied (no org membership at all)
  • test_subscription_is_denied_when_submission_does_not_exist

Test fixture refinement: makeSubmission(?User $submitter) now accepts an explicit submitter argument. Positive role-based tests use a separate User as submitter so the submitter short-circuit doesn't accidentally authorise role-based test subjects.

BACKLOG closure (commit 5d53cca)

  • TECH-CHANNEL-AUTH-ORG-ADMIN moved from open Technische schuld to ## Opgeloste items (mei 2026) with closure summary citing the PR + the three-path auth pattern + super_admin bypass addition
  • Stale forward-reference "(submitter-only voor nu — zie ook TECH-CHANNEL-AUTH-ORG-ADMIN)" inside the still-open FRONTEND-ECHO-IDENTITY-MATCH-SUBSCRIPTION entry updated to reflect the post-PR three-path auth (no-compromises hygiene — same pattern as the FORM-05 stub-status touch-up from the WS-6 closure docs-PR)

Test counts

Count
Pre-PR baseline 1621
Added +4
Deleted −1
New total 1624
Failures 0
Larastan errors 0
Channel auth file (focused) 7/7 green

Out-of-scope (NOT in this PR)

  • Frontend Echo subscription for FormSubmissionIdentityMatchResolved — sibling BACKLOG entry FRONTEND-ECHO-IDENTITY-MATCH-SUBSCRIPTION. Backend infrastructure now fully ready (auth callback, broadcast event class, channel registration); frontend follow-up subscribes the portal IdentityMatchBanner to live updates.
  • Refactoring org-admin checks across the codebase into a helper — 17+ policy sites duplicate the pivot-table check. Centralising into a User::isOrgAdminOf($organisation) helper or OrganisationPolicy::admin is a separate concern. Phase A's directive was "discovery, not standardisation."
  • GlitchTip alert rule for apply_status=failed AND form_schema.has_public_token=true — operational task, configured manually in the GlitchTip web UI on monitoring.hausdesign.nl. Runbook procedure documented in dev-docs/runbooks/observability-triage.md §7.

Audit findings (Phase A)

  • Spatie teams disabled. Org-scoping uses pivot-table convention, not Spatie scoped roles. Confirmed via config/permission.php inspection.
  • Curiosity: Role::findOrCreate('org_admin', 'web') is registered in Spatie (RoleSeeder.php) but no code uses $user->hasRole('org_admin') (without scope). Spatie-side org_admin role exists for compatibility/future-use but is not the canonical check. Potential dual-source tech debt — out of scope for this PR.
  • super_admin bypass as audit bonus. Not in the original BACKLOG entry scope but added per Bert's confirmation; matches every analogous policy and prevents a confusing "super_admin denied" edge case.
  • Test isolation refinement in makeSubmission() was necessary because positive role-based tests would otherwise be authorised via the submitter short-circuit instead of the role check. Adding an explicit $submitter parameter ensures we test the path we think we're testing.
  • Compatibility verification: existing test_other_authenticated_user_is_denied creates a non-submitter User with NO organisation membership — falls through all three auth branches and stays denied. Test still holds post-change; no collision with the new "regular member cannot subscribe" test.

Commits

  1. f5cb371 — feat(broadcasting): extend submission.{id} channel auth to organisation admins
  2. e04b084 — test(broadcasting): add org-admin auth + cross-tenant guard tests
  3. 5d53cca — docs(backlog): close TECH-CHANNEL-AUTH-ORG-ADMIN

Review hints

  • The cross-tenant guard test (test_organisation_admin_of_different_org_cannot_subscribe) is the critical assertion. If this passes, an admin of organisation A genuinely cannot subscribe to a submission in organisation B — the multi-tenancy guarantee Crewli depends on for broadcast channels.
  • Three-path order matters. Submitter check first (fast path, common case, no DB lookup), then super_admin (single Spatie call), then the heavier pivot-table query last. Matches FormSubmissionActionFailurePolicy::canAccess exactly.
  • No code change to FormBindingApplicator, TriggerPersonIdentityMatchOnFormSubmit, or any listener. This PR only touches the auth callback, its tests, and BACKLOG hygiene.

🤖 Co-Authored-By: Claude Opus 4.7

## TECH-CHANNEL-AUTH-ORG-ADMIN — Extend submission.{id} channel auth to organisation admins Closes BACKLOG entry `TECH-CHANNEL-AUTH-ORG-ADMIN`. Extends the `submission.{submissionId}` private broadcast channel authorization from submitter-only (introduced in WS-6 v1.3-delta D2, PR #11) to a three-path check: submitter, super_admin (app-wide bypass), or organisation admin of the submission's organisation. The original deferral reason (Spatie Permission helper convention not yet audited) is now closed: Phase A audit established the codebase's canonical pattern, used in 17+ policy sites. ### Refs - BACKLOG `TECH-CHANNEL-AUTH-ORG-ADMIN` — closed by this PR - BACKLOG `FRONTEND-ECHO-IDENTITY-MATCH-SUBSCRIPTION` — sibling, still open (frontend portal Echo subscription) - WS-6 v1.3-delta D2 (PR #11 `23a5696`) — introduced the channel + submitter-only auth + denied-by-default org-admin contract test --- ### What this PR delivers **`api/routes/channels.php` — three-path auth callback** (commit `f5cb371`) Direct port of the `FormSubmissionActionFailurePolicy::canAccess` pattern (codebase canonical, used in 17+ policy sites). Auth resolution: 1. `submitted_by_user_id === user.id` — submitter short-circuit (fast path, no DB lookup) 2. `$user->hasRole('super_admin')` — Spatie HasRoles app-wide bypass 3. Pivot-table check: `$organisation->users()->where('user_id', $user->id)->wherePivot('role', 'org_admin')->exists()` — org-scoped admin authorization `withoutGlobalScopes()` preserved on both `FormSubmission` and `Organisation` lookups; channel auth is a structural gate, not a tenant-scoped query. **Why three paths instead of two:** The super_admin bypass was an audit-surfaced bonus addition. Every analogous policy in the codebase includes it; without it super_admins (impersonating, debugging, troubleshooting in production) would mysteriously fail to subscribe even though they can do everything else in the platform. Pattern consistency wins. **Why pivot-table check instead of Spatie scoped roles:** Spatie's `teams` feature is **disabled** in this codebase (`config/permission.php`). Org-scoping lives in the `user_organisation` pivot's `role` column, not in Spatie's `model_has_roles` table. `Role::findOrCreate('org_admin', 'web')` exists in Spatie too (see `RoleSeeder.php`) but no code uses `$user->hasRole('org_admin')` without scope — the pivot-table check is the canonical pattern. **Tests** (commit `e04b084`) | # | Test | Coverage | |---|---|---| | New | `test_super_admin_can_subscribe` | App-wide bypass works | | New | `test_organisation_admin_of_submission_org_can_subscribe` | Positive case for the new auth path | | New | `test_organisation_admin_of_different_org_cannot_subscribe` | **Critical multi-tenancy guard** — admin of org A cannot subscribe to a submission in org B | | New | `test_regular_organisation_member_cannot_subscribe` | Non-admin member of submission's org is still denied | | Deleted | `test_org_admin_is_currently_denied_per_backlog_entry` | The "should flip" test from PR #11; replaced by positive test above | Existing tests preserved (per Phase A compatibility check — they test scenarios orthogonal to the new path): - `test_submitter_is_authorised` - `test_other_authenticated_user_is_denied` (no org membership at all) - `test_subscription_is_denied_when_submission_does_not_exist` **Test fixture refinement:** `makeSubmission(?User $submitter)` now accepts an explicit submitter argument. Positive role-based tests use a separate User as submitter so the submitter short-circuit doesn't accidentally authorise role-based test subjects. **BACKLOG closure** (commit `5d53cca`) - `TECH-CHANNEL-AUTH-ORG-ADMIN` moved from open Technische schuld to `## Opgeloste items (mei 2026)` with closure summary citing the PR + the three-path auth pattern + super_admin bypass addition - Stale forward-reference *"(submitter-only voor nu — zie ook TECH-CHANNEL-AUTH-ORG-ADMIN)"* inside the still-open `FRONTEND-ECHO-IDENTITY-MATCH-SUBSCRIPTION` entry updated to reflect the post-PR three-path auth (no-compromises hygiene — same pattern as the FORM-05 stub-status touch-up from the WS-6 closure docs-PR) --- ### Test counts | | Count | |---|---| | Pre-PR baseline | 1621 | | Added | +4 | | Deleted | −1 | | **New total** | **1624** | | Failures | 0 | | Larastan errors | 0 | | Channel auth file (focused) | 7/7 green | --- ### Out-of-scope (NOT in this PR) - **Frontend Echo subscription** for `FormSubmissionIdentityMatchResolved` — sibling BACKLOG entry `FRONTEND-ECHO-IDENTITY-MATCH-SUBSCRIPTION`. Backend infrastructure now fully ready (auth callback, broadcast event class, channel registration); frontend follow-up subscribes the portal IdentityMatchBanner to live updates. - **Refactoring org-admin checks across the codebase into a helper** — 17+ policy sites duplicate the pivot-table check. Centralising into a `User::isOrgAdminOf($organisation)` helper or `OrganisationPolicy::admin` is a separate concern. Phase A's directive was "discovery, not standardisation." - **GlitchTip alert rule** for `apply_status=failed AND form_schema.has_public_token=true` — operational task, configured manually in the GlitchTip web UI on `monitoring.hausdesign.nl`. Runbook procedure documented in `dev-docs/runbooks/observability-triage.md` §7. --- ### Audit findings (Phase A) - **Spatie teams disabled.** Org-scoping uses pivot-table convention, not Spatie scoped roles. Confirmed via `config/permission.php` inspection. - **Curiosity:** `Role::findOrCreate('org_admin', 'web')` is registered in Spatie (`RoleSeeder.php`) but no code uses `$user->hasRole('org_admin')` (without scope). Spatie-side org_admin role exists for compatibility/future-use but is not the canonical check. Potential dual-source tech debt — out of scope for this PR. - **super_admin bypass as audit bonus.** Not in the original BACKLOG entry scope but added per Bert's confirmation; matches every analogous policy and prevents a confusing "super_admin denied" edge case. - **Test isolation refinement** in `makeSubmission()` was necessary because positive role-based tests would otherwise be authorised via the submitter short-circuit instead of the role check. Adding an explicit `$submitter` parameter ensures we test the path we think we're testing. - **Compatibility verification:** existing `test_other_authenticated_user_is_denied` creates a non-submitter User with NO organisation membership — falls through all three auth branches and stays denied. Test still holds post-change; no collision with the new "regular member cannot subscribe" test. --- ### Commits 1. `f5cb371` — feat(broadcasting): extend submission.{id} channel auth to organisation admins 2. `e04b084` — test(broadcasting): add org-admin auth + cross-tenant guard tests 3. `5d53cca` — docs(backlog): close TECH-CHANNEL-AUTH-ORG-ADMIN --- ### Review hints - **The cross-tenant guard test (`test_organisation_admin_of_different_org_cannot_subscribe`) is the critical assertion.** If this passes, an admin of organisation A genuinely cannot subscribe to a submission in organisation B — the multi-tenancy guarantee Crewli depends on for broadcast channels. - **Three-path order matters.** Submitter check first (fast path, common case, no DB lookup), then super_admin (single Spatie call), then the heavier pivot-table query last. Matches `FormSubmissionActionFailurePolicy::canAccess` exactly. - **No code change to `FormBindingApplicator`, `TriggerPersonIdentityMatchOnFormSubmit`, or any listener.** This PR only touches the auth callback, its tests, and BACKLOG hygiene. 🤖 Co-Authored-By: Claude Opus 4.7
bert.hausmans added 3 commits 2026-05-08 12:23:52 +02:00
feat(broadcasting): extend submission.{id} channel auth to organisation admins f5cb371023 
Per BACKLOG TECH-CHANNEL-AUTH-ORG-ADMIN.

WS-6 v1.3-delta D2 (PR #11 23a5696) introduced submission.{id} private
channel with submitter-only authorization, deferring org-admin auth
to a follow-up after the Spatie Permission helper convention was
audited. This commit closes that follow-up.

Authorization now permits (cheap-first short-circuit):
1. Submitter (submitted_by_user_id === user.id) — unchanged
2. super_admin (Spatie HasRoles app-wide bypass) — audit-surfaced bonus,
   matches every analogous policy in the codebase
3. Organisation admins of the submission's organisation — new

Pattern: direct port of FormSubmissionActionFailurePolicy::canAccess.
Spatie teams is disabled in config/permission.php, so org-scoping
lives in the user_organisation pivot table's `role` column with
wherePivot('role', 'org_admin') — codebase canonical (used in 17+
policy sites). withoutGlobalScopes() preserved on both FormSubmission
and Organisation lookups so channel auth is a structural gate, not a
tenant-scoped query.

Inline TODO removed; the BACKLOG entry transitions to resolved in a
follow-up commit on this branch.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
test(broadcasting): add org-admin auth + cross-tenant guard tests e04b084be5 
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 <noreply@anthropic.com>
docs(backlog): close TECH-CHANNEL-AUTH-ORG-ADMIN 5d53ccabae 
Mark TECH-CHANNEL-AUTH-ORG-ADMIN as resolved with PR reference,
date, and one-paragraph summary of what was delivered.

Three edits:

1. Open entry block removed from "Technische schuld" section.
2. Closure bullet appended under "Opgeloste items (mei 2026)" — full
   summary of the three-path auth (submitter / super_admin / org_admin),
   pattern source (FormSubmissionActionFailurePolicy::canAccess port),
   the audit-surfaced super_admin bypass bonus, test deltas, and
   sibling FRONTEND-ECHO-IDENTITY-MATCH-SUBSCRIPTION pointer.
3. Stale forward-reference inside FRONTEND-ECHO-IDENTITY-MATCH-SUBSCRIPTION
   updated: "submitter-only voor nu" → "submitter / super_admin /
   org_admin van submission's organisatie — TECH-CHANNEL-AUTH-ORG-ADMIN
   closed mei 2026". Closes the same no-compromises gap as the FORM-05
   stub-status touch-up (PR #12).

Sibling BACKLOG entry FRONTEND-ECHO-IDENTITY-MATCH-SUBSCRIPTION stays
open — that's the frontend portal IdentityMatchBanner work that pairs
with this channel auth extension.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
bert.hausmans merged commit e8bd768212 into main 2026-05-08 12:24:21 +02:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
bert.hausmans
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: bert.hausmans/crewli#13
Delete Branch "feat/channel-auth-org-admin"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?