fix(form-builder): explicit OrganisationScope bypass on every public-form query

Five models that the public form endpoints touch carry a global OrganisationScope: FormSchema, Event, TimeSlot, FestivalSection, PersonTag. The initial S2c implementation relied on the scope no-opping because /public/forms/* has no `{organisation}` route parameter and OrganisationScope::resolveOrganisationId returns null in that case. That's accidentally-correct. Any middleware that sets an implicit org context later (route model binding for platform admin, impersonation, default-org fallback on an authed Sanctum session) would start filtering public schema resolution by the wrong org. - PublicFormTokenResolver: both FormSchema::query() calls now pass withoutGlobalScope(OrganisationScope::class). public_token is globally unique so this is safe. - PublicFormController::timeSlots() / sections() / festivalEventIds(): Event, TimeSlot, FestivalSection queries all explicit now, including the eager-loaded event relation on time-slots. - PublicFormController::ownerEvent(): narrowed from Event::withoutGlobalScopes() to withoutGlobalScope(OrganisationScope) so future scopes (soft-delete, archived) aren't accidentally stripped. - PublicFormSchemaResource::availableTagsByCategory: same narrowing on the PersonTag query. PublicFormCrossOrgScopeTest pins the expectation — 4 cases hit every public endpoint under a stashed foreign-org route parameter and assert the owner-org data still surfaces. Verified the tests fail when the fix is reverted (all 4 return `SCHEMA_NOT_FOUND` with the bypass absent). Full suite 893 → 897 green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-17 23:16:22 +02:00
parent 68d2c830a0
commit 6ba921442c
4 changed files with 194 additions and 3 deletions
--- a/api/app/Http/Controllers/Api/V1/FormBuilder/PublicFormController.php
+++ b/api/app/Http/Controllers/Api/V1/FormBuilder/PublicFormController.php
@@ -9,6 +9,7 @@ use App\Http\Resources\FormBuilder\PublicFormSchemaResource;
 use App\Models\Event;
 use App\Models\FestivalSection;
 use App\Models\FormBuilder\FormSchema;
+use App\Models\Scopes\OrganisationScope;
 use App\Models\TimeSlot;
 use App\Services\FormBuilder\PublicFormTokenResolver;
 use Illuminate\Http\JsonResponse;
@@ -45,10 +46,14 @@ final class PublicFormController extends Controller

        $eventIds = $this->festivalEventIds($event);

+        // OrganisationScope explicitly bypassed — public endpoint has no
+        // {organisation} route param so the scope would no-op today, but
+        // relying on that is accidentally-correct.
        $slots = TimeSlot::query()
+            ->withoutGlobalScope(OrganisationScope::class)
            ->whereIn('event_id', $eventIds)
            ->where('person_type', 'VOLUNTEER')
-            ->with(['event:id,name'])
+            ->with(['event' => fn ($q) => $q->withoutGlobalScope(OrganisationScope::class)->select('id', 'name')])
            ->orderBy('date')
            ->orderBy('start_time')
            ->get();
@@ -77,7 +82,9 @@ final class PublicFormController extends Controller

        $eventIds = $this->festivalEventIds($event);

+        // OrganisationScope explicitly bypassed — see timeSlots().
        $sections = FestivalSection::query()
+            ->withoutGlobalScope(OrganisationScope::class)
            ->whereIn('event_id', $eventIds)
            ->where('show_in_registration', true)
            ->where('type', 'standard')
@@ -109,7 +116,9 @@ final class PublicFormController extends Controller
            return null;
        }

-        return Event::withoutGlobalScopes()->find($schema->owner_id);
+        return Event::query()
+            ->withoutGlobalScope(OrganisationScope::class)
+            ->find($schema->owner_id);
    }

    /**
@@ -122,7 +131,9 @@ final class PublicFormController extends Controller
     */
    private function festivalEventIds(Event $event): array
    {
+        // OrganisationScope explicitly bypassed — see timeSlots().
        $childIds = Event::query()
+            ->withoutGlobalScope(OrganisationScope::class)
            ->where('parent_event_id', $event->id)
            ->pluck('id')
            ->map(fn ($id) => (string) $id)
--- a/api/app/Http/Resources/FormBuilder/PublicFormSchemaResource.php
+++ b/api/app/Http/Resources/FormBuilder/PublicFormSchemaResource.php
@@ -8,6 +8,7 @@ use App\Enums\FormBuilder\FormFieldType;
 use App\Models\FormBuilder\FormField;
 use App\Models\FormBuilder\FormSchema;
 use App\Models\PersonTag;
+use App\Models\Scopes\OrganisationScope;
 use Illuminate\Http\Request;
 use Illuminate\Http\Resources\Json\JsonResource;

@@ -100,7 +101,10 @@ final class PublicFormSchemaResource extends JsonResource
            return [];
        }

-        $rows = PersonTag::withoutGlobalScopes()
+        // Named-scope bypass only — don't unintentionally strip future
+        // soft-delete or is_active scopes if any land later.
+        $rows = PersonTag::query()
+            ->withoutGlobalScope(OrganisationScope::class)
            ->where('organisation_id', $organisationId)
            ->where('is_active', true)
            ->orderBy('sort_order')
--- a/api/app/Services/FormBuilder/PublicFormTokenResolver.php
+++ b/api/app/Services/FormBuilder/PublicFormTokenResolver.php
@@ -7,6 +7,7 @@ namespace App\Services\FormBuilder;
 use App\Exceptions\FormBuilder\SchemaNotFoundException;
 use App\Exceptions\FormBuilder\TokenExpiredException;
 use App\Models\FormBuilder\FormSchema;
+use App\Models\Scopes\OrganisationScope;

 /**
 * Token-to-schema resolution for every /public/forms/* endpoint.
@@ -15,6 +16,13 @@ use App\Models\FormBuilder\FormSchema;
 *
 * Throws the standardised public-form exceptions directly so callers
 * don't need to branch on grace state themselves.
+ *
+ * Org scope is explicitly bypassed here: public_token is globally unique
+ * across the platform, and the public routes have no `{organisation}`
+ * route parameter to drive OrganisationScope. Relying on the scope to
+ * no-op silently works today but is accidentally-correct — any future
+ * middleware that sets an org context (impersonation, platform admin
+ * visibility toggle, etc.) would start filtering public resolutions.
 */
 final class PublicFormTokenResolver
 {
@@ -23,6 +31,7 @@ final class PublicFormTokenResolver
    public function resolve(string $token): FormSchema
    {
        $current = FormSchema::query()
+            ->withoutGlobalScope(OrganisationScope::class)
            ->where('public_token', $token)
            ->first();
        if ($current !== null) {
@@ -30,6 +39,7 @@ final class PublicFormTokenResolver
        }

        $previous = FormSchema::query()
+            ->withoutGlobalScope(OrganisationScope::class)
            ->where('public_token_previous', $token)
            ->first();

--- a/api/tests/Feature/Api/V1/Public/FormBuilder/PublicFormCrossOrgScopeTest.php
+++ b/api/tests/Feature/Api/V1/Public/FormBuilder/PublicFormCrossOrgScopeTest.php
@@ -0,0 +1,166 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Feature\Api\V1\Public\FormBuilder;
+
+use App\Enums\FormBuilder\FormFieldType;
+use App\Enums\FormBuilder\FormPurpose;
+use App\Models\Event;
+use App\Models\FestivalSection;
+use App\Models\FormBuilder\FormField;
+use App\Models\FormBuilder\FormSchema;
+use App\Models\Organisation;
+use App\Models\TimeSlot;
+use App\Models\User;
+use Database\Seeders\RoleSeeder;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Routing\Route;
+use Illuminate\Support\Str;
+use Tests\TestCase;
+
+/**
+ * Pins the expectation that /public/forms/* resolves schemas + their
+ * dependency data cross-org regardless of whether a foreign
+ * OrganisationScope context has somehow leaked into request state.
+ *
+ * These tests exist because the first S2c implementation accidentally
+ * relied on OrganisationScope no-opping (no {organisation} route
+ * parameter means resolveOrganisationId returns null). If anyone
+ * later introduces middleware that sets an implicit org context the
+ * public endpoints must not start filtering.
+ */
+final class PublicFormCrossOrgScopeTest extends TestCase
+{
+    use RefreshDatabase;
+
+    private Organisation $ownerOrg;
+
+    private Organisation $otherOrg;
+
+    private Event $ownerEvent;
+
+    private FormSchema $schema;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        $this->seed(RoleSeeder::class);
+
+        $this->ownerOrg = Organisation::factory()->create(['name' => 'Owner Org']);
+        $this->otherOrg = Organisation::factory()->create(['name' => 'Foreign Org']);
+
+        $this->ownerEvent = Event::factory()->create(['organisation_id' => $this->ownerOrg->id]);
+
+        $this->schema = FormSchema::factory()->create([
+            'organisation_id' => $this->ownerOrg->id,
+            'owner_type' => 'event',
+            'owner_id' => $this->ownerEvent->id,
+            'purpose' => FormPurpose::EVENT_REGISTRATION,
+            'is_published' => true,
+            'public_token' => (string) Str::ulid(),
+        ]);
+        FormField::factory()->create([
+            'form_schema_id' => $this->schema->id,
+            'field_type' => FormFieldType::TAG_PICKER->value,
+            'slug' => 'vaardigheden',
+            'is_portal_visible' => true,
+        ]);
+
+        $this->ownerOrg->personTags()->create([
+            'name' => 'Tapper',
+            'category' => 'Horeca',
+            'is_active' => true,
+            'sort_order' => 1,
+        ]);
+
+        TimeSlot::create([
+            'event_id' => $this->ownerEvent->id,
+            'name' => 'Ochtend',
+            'person_type' => 'VOLUNTEER',
+            'date' => '2026-07-10',
+            'start_time' => '09:00',
+            'end_time' => '13:00',
+            'duration_hours' => 4,
+        ]);
+        FestivalSection::create([
+            'event_id' => $this->ownerEvent->id,
+            'name' => 'Bar',
+            'type' => 'standard',
+            'sort_order' => 1,
+            'show_in_registration' => true,
+        ]);
+    }
+
+    public function test_public_schema_get_resolves_when_foreign_org_is_in_route_context(): void
+    {
+        $this->setOrganisationRouteContext($this->otherOrg);
+
+        $response = $this->getJson("/api/v1/public/forms/{$this->schema->public_token}");
+
+        $response->assertOk();
+        $this->assertSame($this->schema->id, $response->json('data.id'));
+    }
+
+    public function test_available_tags_populated_when_foreign_org_is_in_route_context(): void
+    {
+        $this->setOrganisationRouteContext($this->otherOrg);
+
+        $response = $this->getJson("/api/v1/public/forms/{$this->schema->public_token}");
+
+        $field = collect($response->json('data.fields'))->firstWhere('slug', 'vaardigheden');
+        $this->assertNotNull($field);
+        $this->assertNotEmpty($field['available_tags'], 'TAG_PICKER tags must resolve cross-org regardless of route parameter');
+        $this->assertSame('Tapper', $field['available_tags'][0]['name']);
+    }
+
+    public function test_time_slots_endpoint_returns_owner_event_slots_under_foreign_org_context(): void
+    {
+        $this->setOrganisationRouteContext($this->otherOrg);
+
+        $response = $this->getJson("/api/v1/public/forms/{$this->schema->public_token}/time-slots");
+
+        $response->assertOk();
+        $this->assertCount(1, $response->json('data'));
+        $this->assertSame('Ochtend', $response->json('data.0.name'));
+    }
+
+    public function test_sections_endpoint_returns_owner_event_sections_under_foreign_org_context(): void
+    {
+        $this->setOrganisationRouteContext($this->otherOrg);
+
+        $response = $this->getJson("/api/v1/public/forms/{$this->schema->public_token}/sections");
+
+        $response->assertOk();
+        $this->assertCount(1, $response->json('data'));
+        $this->assertSame('Bar', $response->json('data.0.name'));
+    }
+
+    /**
+     * Hypothetical: a future middleware binds `{organisation}` even on
+     * public routes. We simulate by stuffing it into the route resolver
+     * so OrganisationScope::resolveOrganisationId() would match. The
+     * endpoint must still return the owner-org schema.
+     */
+    private function setOrganisationRouteContext(Organisation $org): void
+    {
+        // Build a fake route and pre-register the parameter so a
+        // follow-up request() (inside the scope resolver) could match.
+        // Sanctum/HTTP test harness makes a fresh request on getJson, so
+        // we attach a before-filter on the fresh request's route via a
+        // service container binding for the OrganisationScope resolver
+        // input. The simplest repro: set an organisation_id directly on
+        // the scope constructor's fallback path — by binding a resolver
+        // into app() so request()->route('organisation') returns this
+        // org. Here we short-circuit with a route-param stash:
+        $user = User::factory()->create();
+        $org->users()->attach($user, ['role' => 'org_member']);
+
+        // Register a before-request hook that sets the parameter on the
+        // resolved route. The public test harness re-creates the route
+        // per request, so this binds on the Router once:
+        $this->app['router']->matched(function ($event) use ($org): void {
+            $event->route->setParameter('organisation', $org);
+        });
+    }
+}