feat(scope): declarative FK-chain strategy for OrganisationScope, register on 14 models per addendum Q2 + D-03/D-04

Refactors OrganisationScope to support a declarative, recursive FK-chain resolver and registers the scope on 14 models that previously relied on caller-discipline for tenant isolation. Scope resolver (app/Models/Scopes/OrganisationScope.php): Models now declare their strategy via: public static function tenantScopeStrategy(): array { return ['column' => 'organisation_id']; // terminal // OR return ['via' => FormSchema::class, 'fk' => 'form_schema_id']; } The apply() path walks the chain recursively, building whereIn subqueries against parent models until it hits a column-based strategy. Max 3 hops; deeper chains raise App\Exceptions\TenantScopeResolutionException. The walker accepts BOTH the new tenantScopeStrategy() and the legacy $organisationScopeColumn property at every hop — so PersonIdentityMatch can chain via Person, which still uses the legacy event_id bridge, without requiring Person/Event/Shift/FestivalSection/TimeSlot to migrate to the new convention in this work package. That migration is a separate backlog ticket — explicitly scope-controlled per the addendum. Fourteen newly-scoped models: Form-builder child models (D-03): FormSchemaSection via FormSchema (1 hop) FormField via FormSchema (1 hop) FormSubmission column organisation_id (Commit 2) FormValue via FormSubmission (1 hop) FormValueOption via FormValue -> FormSubmission (2 hops) FormSubmissionSectionStatus via FormSubmission (1 hop) FormSubmissionDelegation via FormSubmission (1 hop) FormSchemaWebhook via FormSchema (1 hop) FormWebhookDelivery via FormSubmission (1 hop) Event-data models (D-04 event-data subset): ShiftAssignment via Shift (legacy festival_section_id) ShiftWaitlist via Shift VolunteerAvailability via TimeSlot (legacy event_id) PersonSectionPreference via FestivalSection (legacy event_id) PersonIdentityMatch via Person (legacy event_id) Note — task directive specified VolunteerAvailability "via: Event, fk: event_id", but the table has no event_id column (only person_id + time_slot_id). Rerouted via TimeSlot, which carries the legacy event_id bridge; same end result, correct FK. Security-relevant callers made explicit: PublicFormSchemaResource::toArray() now eagerly loads fields + sections with withoutGlobalScope(OrganisationScope::class). Prior to this commit the public form endpoint silently relied on those relations being unscoped. The PublicFormCrossOrgScopeTest pre-existing assertions still pass — behaviour unchanged, intent now explicit. Test fix: FormSchemaApiTest::test_publish_sets_is_published_true was flaky (factory randomly picked EVENT_REGISTRATION which requires bindings). Pinned to USER_PROFILE for determinism; PurposeSchemaLifecycleTest covers the binding-enforcement path. Test flip: MultiTenancyTest::test_form_schema_webhook_is_not_globally_scoped renamed to is_scoped_via_fk_chain and asserts the new behaviour: scope filters by route org, withoutGlobalScope() still exposes cross-org rows. The test's original purpose ("pin current behaviour so a future refactor is intentional") is now satisfied by Commit 3 being that intentional refactor. Docs: SCHEMA.md §3.5.11 Rule 5 — tenantScopeStrategy() convention documented; the 14 newly-scoped models enumerated; link to addendum Q2. ARCH-FORM-BUILDER.md §4.14 — new section "Multi-tenancy scope chain" with the hop-count table for all 14 chains and the withoutGlobalScope pattern for cross-org callers. Tests: tests/Feature/MultiTenancy/ScopeLeakageTest.php — two orgs with fully-populated record chains down to each of the 14 leaf models; asserts scoped queries never cross, withoutGlobalScope still does. Plus: three- hop chain (FormValueOption) explicitly exercised, legacy-column bridge verified, over-deep chain raises TenantScopeResolutionException. 16 tests / 31 new assertions. Full suite: 1000 passed (2706 assertions). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-24 17:08:33 +02:00
parent ae8e2fdb4e
commit b688ec26f0
22 changed files with 942 additions and 35 deletions
--- a/api/app/Models/Scopes/OrganisationScope.php
+++ b/api/app/Models/Scopes/OrganisationScope.php
@@ -4,6 +4,7 @@ declare(strict_types=1);

 namespace App\Models\Scopes;

+use App\Exceptions\TenantScopeResolutionException;
 use App\Models\Event;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Model;
@@ -13,17 +14,37 @@ use Illuminate\Database\Eloquent\Scope;
 * Global scope that filters models by organisation_id.
 *
 * Resolution order:
- * 1. Explicitly provided organisation ID (constructor)
- * 2. Route parameter 'organisation' (object or string)
- * 3. Skip scope if no context (CLI, queue jobs, unauthenticated)
+ *   1. Explicitly provided organisation ID (constructor)
+ *   2. Route parameter 'organisation' (object or string)
+ *   3. Skip scope if no context (CLI, queue jobs, unauthenticated)
 *
- * Models declare their scoping strategy via the $organisationScopeColumn property:
- * - 'organisation_id' (default) — model has a direct organisation_id column
- * - 'event_id' — model is scoped through events.organisation_id
- * - 'festival_section_id' — model is scoped through festival_sections.event_id → events.organisation_id
+ * Two strategies are supported, discovered per-model:
+ *
+ *   (a) Declarative FK-chain (addendum Q2):
+ *       The model declares a `tenantScopeStrategy()` static method returning
+ *       either:
+ *         ['column' => 'organisation_id']  — direct scope column, OR
+ *         ['via' => ParentModel::class, 'fk' => 'parent_id']  — walk to parent
+ *       The resolver recurses through parents until it hits a column strategy
+ *       or the legacy $organisationScopeColumn bridge. Cap at 3 hops; deeper
+ *       chains raise TenantScopeResolutionException.
+ *
+ *   (b) Legacy column property (pre-addendum — still supported so Person,
+ *       Event, FestivalSection, etc. need not migrate in this work package):
+ *       `public string $organisationScopeColumn = 'organisation_id'
+ *                                                 | 'event_id'
+ *                                                 | 'festival_section_id';`
+ *       The resolver treats this as a terminal column-based strategy.
+ *
+ * The recursive walker accepts EITHER shape at every hop, so a model using
+ * the new tenantScopeStrategy() can chain through a parent using the legacy
+ * $organisationScopeColumn (e.g. PersonIdentityMatch → Person → event_id →
+ * organisation_id), and vice versa.
 */
 final class OrganisationScope implements Scope
 {
+    private const MAX_CHAIN_HOPS = 3;
+
    public function __construct(
        private readonly ?string $organisationId = null,
    ) {}
@@ -36,25 +57,100 @@ final class OrganisationScope implements Scope
            return;
        }

+        $this->applyStrategy($builder, $model, $id, 0);
+    }
+
+    /**
+     * Apply a tenant filter by walking the scope strategy declared on
+     * $model down to the first column-based strategy.
+     */
+    private function applyStrategy(Builder $builder, Model $model, string $orgId, int $hops): void
+    {
+        if ($hops > self::MAX_CHAIN_HOPS) {
+            throw TenantScopeResolutionException::tooDeep($model::class, self::MAX_CHAIN_HOPS);
+        }
+
+        $strategy = $this->strategyFor($model);
+
+        if (isset($strategy['column'])) {
+            $this->applyColumnFilter($builder, $model, (string) $strategy['column'], $orgId);
+
+            return;
+        }
+
+        if (isset($strategy['via'], $strategy['fk'])) {
+            /** @var class-string<Model> $parentClass */
+            $parentClass = $strategy['via'];
+            $fk = (string) $strategy['fk'];
+
+            $parentInstance = new $parentClass();
+            $parentTable = $parentInstance->getTable();
+            $parentKey = $parentInstance->getKeyName();
+
+            // Build the subquery constraining the parent by the SAME
+            // recursive strategy walk — parent may itself be chained.
+            $parentQuery = $parentClass::query()
+                ->withoutGlobalScope(self::class)
+                ->select("$parentTable.$parentKey");
+
+            $this->applyStrategy($parentQuery, $parentInstance, $orgId, $hops + 1);
+
+            $builder->whereIn($model->getTable().'.'.$fk, $parentQuery);
+
+            return;
+        }
+
+        throw TenantScopeResolutionException::invalidStrategy($model::class);
+    }
+
+    /**
+     * Discover the scope strategy for a model. Accepts:
+     *   - tenantScopeStrategy() static method returning ['column'=>…] or ['via'=>…,'fk'=>…]
+     *   - Legacy $organisationScopeColumn instance property
+     * @return array{column?: string, via?: class-string, fk?: string}
+     */
+    private function strategyFor(Model $model): array
+    {
+        if (method_exists($model, 'tenantScopeStrategy')) {
+            /** @var array<string, mixed> $declared */
+            $declared = $model::tenantScopeStrategy();
+
+            return $declared;
+        }
+
+        // Legacy bridge — existing column-scoped models continue to work
+        // without migrating to tenantScopeStrategy() in this work package.
        $column = $model->organisationScopeColumn ?? 'organisation_id';

+        return ['column' => $column];
+    }
+
+    /**
+     * Terminal column filter. `organisation_id` is a direct where; the
+     * legacy `event_id` / `festival_section_id` strategies expand to the
+     * same subquery pattern the old implementation used.
+     */
+    private function applyColumnFilter(Builder $builder, Model $model, string $column, string $orgId): void
+    {
+        $table = $model->getTable();
+
        match ($column) {
-            'organisation_id' => $builder->where($model->getTable() . '.organisation_id', $id),
+            'organisation_id' => $builder->where("$table.organisation_id", $orgId),
            'event_id' => $builder->whereIn(
-                $model->getTable() . '.event_id',
+                "$table.event_id",
                Event::withoutGlobalScope(self::class)
-                    ->where('organisation_id', $id)
+                    ->where('organisation_id', $orgId)
                    ->select('id')
            ),
            'festival_section_id' => $builder->whereIn(
-                $model->getTable() . '.festival_section_id',
+                "$table.festival_section_id",
                \App\Models\FestivalSection::withoutGlobalScope(self::class)
                    ->whereIn('event_id', Event::withoutGlobalScope(self::class)
-                        ->where('organisation_id', $id)
+                        ->where('organisation_id', $orgId)
                        ->select('id'))
                    ->select('id')
            ),
-            default => null,
+            default => throw TenantScopeResolutionException::invalidStrategy($model::class),
        };
    }