From 6ba921442c4a426def50fb0e36a717a0d6feaa7b Mon Sep 17 00:00:00 2001 From: "bert.hausmans" Date: Fri, 17 Apr 2026 23:16:22 +0200 Subject: [PATCH] fix(form-builder): explicit OrganisationScope bypass on every public-form query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../V1/FormBuilder/PublicFormController.php | 15 +- .../FormBuilder/PublicFormSchemaResource.php | 6 +- .../FormBuilder/PublicFormTokenResolver.php | 10 ++ .../PublicFormCrossOrgScopeTest.php | 166 ++++++++++++++++++ 4 files changed, 194 insertions(+), 3 deletions(-) create mode 100644 api/tests/Feature/Api/V1/Public/FormBuilder/PublicFormCrossOrgScopeTest.php diff --git a/api/app/Http/Controllers/Api/V1/FormBuilder/PublicFormController.php b/api/app/Http/Controllers/Api/V1/FormBuilder/PublicFormController.php index 08f464a6..6eccdf10 100644 --- 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) diff --git a/api/app/Http/Resources/FormBuilder/PublicFormSchemaResource.php b/api/app/Http/Resources/FormBuilder/PublicFormSchemaResource.php index c4bd629c..dabe7115 100644 --- 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') diff --git a/api/app/Services/FormBuilder/PublicFormTokenResolver.php b/api/app/Services/FormBuilder/PublicFormTokenResolver.php index ba725691..b23aaa2f 100644 --- 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(); diff --git a/api/tests/Feature/Api/V1/Public/FormBuilder/PublicFormCrossOrgScopeTest.php b/api/tests/Feature/Api/V1/Public/FormBuilder/PublicFormCrossOrgScopeTest.php new file mode 100644 index 00000000..9a7339ce --- /dev/null +++ b/api/tests/Feature/Api/V1/Public/FormBuilder/PublicFormCrossOrgScopeTest.php @@ -0,0 +1,166 @@ +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); + }); + } +}