From 6b22c8d76341112f3adaa84ce5130ffeb9c035e7 Mon Sep 17 00:00:00 2001
From: "bert.hausmans" <bert@hausmans.nl>
Date: Tue, 28 Apr 2026 16:13:40 +0200
Subject: [PATCH] test(form-builder): IDOR-class route-level security for
 form-failures admin (WS-6)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

RFC §4 V3 compliance — cross-tenant access to FormSubmissionActionFailure
endpoints returns 404, not 403, to prevent resource-existence
enumeration. The FormSubmissionActionFailurePolicy is the single tenant
gate; these tests assert the route-level integration end-to-end.

Production-code finding (in scope per "security gaps zijn altijd urgent"):
the orgIndex endpoint had a real IDOR gap. Original implementation called
`Gate::authorize('viewAny', ...)` which permits any org_admin in any org,
then filtered the result set by the URL's `{organisation}` param. orgB's
admin hitting `/organisations/{orgA}/form-failures` would get back orgA's
failures — leakage.

Fix:
- New policy method `viewAnyInOrganisation(User, Organisation)` that
  requires super_admin OR org_admin on THIS specific organisation.
- Controller `orgIndex` calls `authorizeViewAnyInOrgOrNotFound()` which
  translates a denied policy → 404 (matches the show/retry/resolve/dismiss
  pattern).
- viewAny on the class level stays as the platformIndex gate (super_admin
  + any-org_admin enumeration is acceptable on the platform endpoint
  because the role middleware already restricts to super_admin).

Test coverage (24 tests, all passing):
- 5 org-scoped endpoints × cross-tenant scenarios (all return 404)
- 5 platform endpoints × role-class scenarios (org_admin gets 403, never 404)
- Edge cases: soft-deleted parent submission, invalid ULID format,
  non-existent ID, unauthenticated, authenticated-without-role on org

The 403 vs 404 distinction matters: role-gated endpoints return 403
(auth-class — "not allowed in this room"); ownership-gated endpoints
return 404 (IDOR-class — "this room doesn't exist for you").

Refs: RFC-WS-6.md §4 V3, ARCH-BINDINGS.md §8.2 (Task 3 of this session)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 .../FormSubmissionActionFailureController.php |  19 +-
 .../FormSubmissionActionFailurePolicy.php     |  19 +
 ...bmissionActionFailureRouteSecurityTest.php | 383 ++++++++++++++++++
 3 files changed, 420 insertions(+), 1 deletion(-)
 create mode 100644 api/tests/Feature/FormBuilder/Api/Security/FormSubmissionActionFailureRouteSecurityTest.php

diff --git a/api/app/Http/Controllers/Api/V1/FormBuilder/FormSubmissionActionFailureController.php b/api/app/Http/Controllers/Api/V1/FormBuilder/FormSubmissionActionFailureController.php
index 4b73c2a4..88a2675a 100644
--- a/api/app/Http/Controllers/Api/V1/FormBuilder/FormSubmissionActionFailureController.php
+++ b/api/app/Http/Controllers/Api/V1/FormBuilder/FormSubmissionActionFailureController.php
@@ -31,7 +31,13 @@ final class FormSubmissionActionFailureController extends Controller
 {
     public function orgIndex(Organisation $organisation): AnonymousResourceCollection
     {
-        Gate::authorize('viewAny', FormSubmissionActionFailure::class);
+        // RFC V3 IDOR-class — the user must be super_admin OR an
+        // org_admin on THIS specific organisation. Viewing any org's
+        // index without role on it would let orgB's admin enumerate
+        // orgA's failure rows. Treat denied → 404 to keep the "this
+        // doesn't exist for you" contract; same pattern as show/retry/
+        // resolve/dismiss.
+        $this->authorizeViewAnyInOrgOrNotFound($organisation);
 
         $failures = FormSubmissionActionFailure::query()
             ->whereHas('submission', function ($q) use ($organisation): void {
@@ -193,4 +199,15 @@ final class FormSubmissionActionFailureController extends Controller
             throw new ModelNotFoundException;
         }
     }
+
+    /**
+     * Tenant gate for orgIndex: caller must be super_admin OR an
+     * org_admin on THIS specific organisation. Denied → 404 (RFC V3).
+     */
+    private function authorizeViewAnyInOrgOrNotFound(Organisation $organisation): void
+    {
+        if (Gate::denies('viewAnyInOrganisation', [FormSubmissionActionFailure::class, $organisation])) {
+            throw new ModelNotFoundException;
+        }
+    }
 }
diff --git a/api/app/Policies/FormBuilder/FormSubmissionActionFailurePolicy.php b/api/app/Policies/FormBuilder/FormSubmissionActionFailurePolicy.php
index 40f0085f..91c6080f 100644
--- a/api/app/Policies/FormBuilder/FormSubmissionActionFailurePolicy.php
+++ b/api/app/Policies/FormBuilder/FormSubmissionActionFailurePolicy.php
@@ -32,6 +32,25 @@ final class FormSubmissionActionFailurePolicy
             ->exists();
     }
 
+    /**
+     * RFC V3 — tenant gate for the org-scoped index endpoint.
+     * The caller must be super_admin OR an org_admin on THIS specific
+     * organisation; without this check the broader `viewAny` would let
+     * orgB's admin enumerate orgA's failure rows via orgA's URL.
+     * Denied → controller translates to 404 to keep the IDOR contract.
+     */
+    public function viewAnyInOrganisation(User $user, Organisation $organisation): bool
+    {
+        if ($user->hasRole('super_admin')) {
+            return true;
+        }
+
+        return $organisation->users()
+            ->where('user_id', $user->id)
+            ->wherePivot('role', 'org_admin')
+            ->exists();
+    }
+
     public function view(User $user, FormSubmissionActionFailure $failure): bool
     {
         return $this->canAccess($user, $failure);
diff --git a/api/tests/Feature/FormBuilder/Api/Security/FormSubmissionActionFailureRouteSecurityTest.php b/api/tests/Feature/FormBuilder/Api/Security/FormSubmissionActionFailureRouteSecurityTest.php
new file mode 100644
index 00000000..f06783f7
--- /dev/null
+++ b/api/tests/Feature/FormBuilder/Api/Security/FormSubmissionActionFailureRouteSecurityTest.php
@@ -0,0 +1,383 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Feature\FormBuilder\Api\Security;
+
+use App\Enums\FormBuilder\DismissalReasonType;
+use App\Models\FormBuilder\FormSchema;
+use App\Models\FormBuilder\FormSubmission;
+use App\Models\FormBuilder\FormSubmissionActionFailure;
+use App\Models\Organisation;
+use App\Models\User;
+use Database\Seeders\RoleSeeder;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Laravel\Sanctum\Sanctum;
+use Tests\TestCase;
+
+/**
+ * IDOR-class route-level security tests for the
+ * FormSubmissionActionFailure admin endpoints.
+ *
+ * RFC §4 V3: cross-tenant access returns 404, NOT 403.
+ *   - 403 confirms the resource exists (only auth lacks).
+ *   - 404 makes existence indistinguishable from absence —
+ *     attackers can't enumerate which IDs exist on other tenants.
+ *
+ * The Policy (FormSubmissionActionFailurePolicy) is the single tenant gate.
+ * These tests assert the route-level integration: every endpoint returns
+ * 404 when the request comes from a different tenant context.
+ *
+ * Two-axis policy enforcement:
+ *   - Role-class (super_admin platform endpoints): 403 for unauthorised roles
+ *   - Ownership-class (org-scoped endpoints): 404 for cross-tenant access
+ *
+ * Ref: ARCH-BINDINGS.md § 8.2 (IDOR class tests).
+ */
+final class FormSubmissionActionFailureRouteSecurityTest extends TestCase
+{
+    use RefreshDatabase;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        $this->seed(RoleSeeder::class);
+    }
+
+    // ============================================================
+    // ORG-SCOPED ENDPOINTS — IDOR class (404, never 403)
+    // ============================================================
+
+    public function test_org_index_unauthenticated_returns_401(): void
+    {
+        $orgA = Organisation::factory()->create();
+
+        $this->getJson("/api/v1/organisations/{$orgA->id}/form-failures")
+            ->assertStatus(401);
+    }
+
+    public function test_org_index_for_other_orgs_admin_returns_404(): void
+    {
+        // RFC V3 IDOR-class — orgB's admin hitting orgA's index URL must
+        // not even confirm the endpoint exists for that org. Without
+        // this check, viewAny (any org_admin in any org) would pass,
+        // and the controller's whereHas filter would emit orgA's
+        // failures to orgB's admin. Tenant gate is the policy's
+        // viewAnyInOrganisation; denied → 404.
+        [$orgA, $adminA, $orgB, $adminB] = $this->makeTwoOrgsWithAdmins();
+        $this->makeFailureInOrg($orgA);
+
+        Sanctum::actingAs($adminB);
+        $this->getJson("/api/v1/organisations/{$orgA->id}/form-failures")
+            ->assertStatus(404);
+    }
+
+    public function test_org_index_for_correct_admin_returns_200_with_only_own_failures(): void
+    {
+        [$orgA, $adminA, $orgB, $adminB] = $this->makeTwoOrgsWithAdmins();
+        $failureInOrgA = $this->makeFailureInOrg($orgA);
+        $this->makeFailureInOrg($orgB);
+
+        // orgA admin hitting orgA's URL → sees only orgA's failures.
+        Sanctum::actingAs($adminA);
+        $response = $this->getJson("/api/v1/organisations/{$orgA->id}/form-failures");
+        $response->assertOk();
+        $this->assertCount(1, $response->json('data'));
+        $this->assertSame((string) $failureInOrgA->id, $response->json('data.0.id'));
+    }
+
+    public function test_org_index_for_authenticated_without_role_returns_404(): void
+    {
+        [$orgA, $adminA] = $this->makeOrgWithAdmin();
+        $this->makeFailureInOrg($orgA);
+        $stranger = User::factory()->create();
+
+        Sanctum::actingAs($stranger);
+        $this->getJson("/api/v1/organisations/{$orgA->id}/form-failures")
+            ->assertStatus(404);
+    }
+
+    public function test_org_show_cross_tenant_returns_404_not_403(): void
+    {
+        [$orgA, $adminA, $orgB, $adminB] = $this->makeTwoOrgsWithAdmins();
+        $failureInOrgA = $this->makeFailureInOrg($orgA);
+
+        // Attempt 1: orgB admin uses orgB's URL prefix to fetch orgA's failure.
+        Sanctum::actingAs($adminB);
+        $this->getJson("/api/v1/organisations/{$orgB->id}/form-failures/{$failureInOrgA->id}")
+            ->assertStatus(404);
+
+        // Attempt 2: orgB admin uses orgA's URL prefix (the "real" path)
+        // — policy denies because orgB admin has no role in orgA → 404.
+        Sanctum::actingAs($adminB);
+        $this->getJson("/api/v1/organisations/{$orgA->id}/form-failures/{$failureInOrgA->id}")
+            ->assertStatus(404);
+    }
+
+    public function test_org_retry_cross_tenant_returns_404(): void
+    {
+        [$orgA, $adminA, $orgB, $adminB] = $this->makeTwoOrgsWithAdmins();
+        $failureInOrgA = $this->makeFailureInOrg($orgA);
+
+        Sanctum::actingAs($adminB);
+        $this->postJson("/api/v1/organisations/{$orgA->id}/form-failures/{$failureInOrgA->id}/retry")
+            ->assertStatus(404);
+    }
+
+    public function test_org_resolve_cross_tenant_returns_404(): void
+    {
+        [$orgA, $adminA, $orgB, $adminB] = $this->makeTwoOrgsWithAdmins();
+        $failureInOrgA = $this->makeFailureInOrg($orgA);
+
+        Sanctum::actingAs($adminB);
+        $this->postJson(
+            "/api/v1/organisations/{$orgA->id}/form-failures/{$failureInOrgA->id}/resolve",
+            ['note' => 'attacker note'],
+        )->assertStatus(404);
+    }
+
+    public function test_org_dismiss_cross_tenant_returns_404(): void
+    {
+        [$orgA, $adminA, $orgB, $adminB] = $this->makeTwoOrgsWithAdmins();
+        $failureInOrgA = $this->makeFailureInOrg($orgA);
+
+        Sanctum::actingAs($adminB);
+        $this->postJson(
+            "/api/v1/organisations/{$orgA->id}/form-failures/{$failureInOrgA->id}/dismiss",
+            ['reason_type' => DismissalReasonType::OTHER->value, 'note' => 'attacker'],
+        )->assertStatus(404);
+    }
+
+    public function test_org_show_with_correct_admin_returns_200(): void
+    {
+        [$orgA, $adminA] = $this->makeOrgWithAdmin();
+        $failureInOrgA = $this->makeFailureInOrg($orgA);
+
+        Sanctum::actingAs($adminA);
+        $this->getJson("/api/v1/organisations/{$orgA->id}/form-failures/{$failureInOrgA->id}")
+            ->assertOk()
+            ->assertJsonPath('data.id', (string) $failureInOrgA->id);
+    }
+
+    // ============================================================
+    // PLATFORM ENDPOINTS — Role class (401/403, never 404 for role miss)
+    // ============================================================
+
+    public function test_platform_index_unauthenticated_returns_401(): void
+    {
+        $this->getJson('/api/v1/admin/form-failures')->assertStatus(401);
+    }
+
+    public function test_platform_show_unauthenticated_returns_401(): void
+    {
+        [$orgA] = $this->makeOrgWithAdmin();
+        $failureInOrgA = $this->makeFailureInOrg($orgA);
+
+        $this->getJson("/api/v1/admin/form-failures/{$failureInOrgA->id}")
+            ->assertStatus(401);
+    }
+
+    public function test_platform_index_org_admin_returns_403(): void
+    {
+        // Role-class denial: org_admin lacks super_admin role → 403,
+        // not 404. The endpoint exists; the user is forbidden.
+        [$orgA, $adminA] = $this->makeOrgWithAdmin();
+
+        Sanctum::actingAs($adminA);
+        $this->getJson('/api/v1/admin/form-failures')->assertStatus(403);
+    }
+
+    public function test_platform_show_org_admin_returns_403(): void
+    {
+        [$orgA, $adminA] = $this->makeOrgWithAdmin();
+        $failureInOrgA = $this->makeFailureInOrg($orgA);
+
+        Sanctum::actingAs($adminA);
+        $this->getJson("/api/v1/admin/form-failures/{$failureInOrgA->id}")
+            ->assertStatus(403);
+    }
+
+    public function test_platform_retry_org_admin_returns_403(): void
+    {
+        [$orgA, $adminA] = $this->makeOrgWithAdmin();
+        $failureInOrgA = $this->makeFailureInOrg($orgA);
+
+        Sanctum::actingAs($adminA);
+        $this->postJson("/api/v1/admin/form-failures/{$failureInOrgA->id}/retry")
+            ->assertStatus(403);
+    }
+
+    public function test_platform_resolve_org_admin_returns_403(): void
+    {
+        [$orgA, $adminA] = $this->makeOrgWithAdmin();
+        $failureInOrgA = $this->makeFailureInOrg($orgA);
+
+        Sanctum::actingAs($adminA);
+        $this->postJson(
+            "/api/v1/admin/form-failures/{$failureInOrgA->id}/resolve",
+            ['note' => 'attempt'],
+        )->assertStatus(403);
+    }
+
+    public function test_platform_dismiss_org_admin_returns_403(): void
+    {
+        [$orgA, $adminA] = $this->makeOrgWithAdmin();
+        $failureInOrgA = $this->makeFailureInOrg($orgA);
+
+        Sanctum::actingAs($adminA);
+        $this->postJson(
+            "/api/v1/admin/form-failures/{$failureInOrgA->id}/dismiss",
+            ['reason_type' => DismissalReasonType::OTHER->value, 'note' => 'attempt'],
+        )->assertStatus(403);
+    }
+
+    public function test_platform_show_super_admin_resolves_any_org(): void
+    {
+        $superAdmin = User::factory()->create();
+        $superAdmin->assignRole('super_admin');
+
+        $orgA = Organisation::factory()->create();
+        $failureInOrgA = $this->makeFailureInOrg($orgA);
+
+        Sanctum::actingAs($superAdmin);
+        $this->getJson("/api/v1/admin/form-failures/{$failureInOrgA->id}")
+            ->assertOk()
+            ->assertJsonPath('data.id', (string) $failureInOrgA->id);
+    }
+
+    // ============================================================
+    // EDGE CASES
+    // ============================================================
+
+    public function test_org_show_soft_deleted_parent_submission_returns_404(): void
+    {
+        // Failure exists, but its parent submission is soft-deleted.
+        // Policy treats parent-gone as resource-gone → 404.
+        [$orgA, $adminA] = $this->makeOrgWithAdmin();
+        $failureInOrgA = $this->makeFailureInOrg($orgA);
+
+        // Soft-delete the parent submission directly via the model
+        // (the FK is cascadeOnDelete, but soft-delete only sets deleted_at).
+        $failureInOrgA->submission?->delete();
+
+        Sanctum::actingAs($adminA);
+        $this->getJson("/api/v1/organisations/{$orgA->id}/form-failures/{$failureInOrgA->id}")
+            ->assertStatus(404);
+    }
+
+    public function test_org_show_invalid_ulid_format_returns_404(): void
+    {
+        [$orgA, $adminA] = $this->makeOrgWithAdmin();
+
+        Sanctum::actingAs($adminA);
+        $this->getJson("/api/v1/organisations/{$orgA->id}/form-failures/notavalidulid")
+            ->assertStatus(404);
+    }
+
+    public function test_org_show_non_existent_id_returns_404(): void
+    {
+        [$orgA, $adminA] = $this->makeOrgWithAdmin();
+        $nonExistent = '01HZZZZZZZZZZZZZZZZZZZZZZZ';
+
+        Sanctum::actingAs($adminA);
+        $this->getJson("/api/v1/organisations/{$orgA->id}/form-failures/{$nonExistent}")
+            ->assertStatus(404);
+    }
+
+    public function test_platform_show_non_existent_id_super_admin_returns_404(): void
+    {
+        $superAdmin = User::factory()->create();
+        $superAdmin->assignRole('super_admin');
+        $nonExistent = '01HZZZZZZZZZZZZZZZZZZZZZZZ';
+
+        Sanctum::actingAs($superAdmin);
+        $this->getJson("/api/v1/admin/form-failures/{$nonExistent}")
+            ->assertStatus(404);
+    }
+
+    public function test_org_show_authenticated_without_role_returns_404(): void
+    {
+        // User is authenticated but has NO role on any org.
+        // Policy denies (not org_admin anywhere) → 404 to keep
+        // the IDOR contract: cross-tenant indistinguishable from
+        // does-not-exist.
+        [$orgA, $adminA] = $this->makeOrgWithAdmin();
+        $failureInOrgA = $this->makeFailureInOrg($orgA);
+
+        $strangerUser = User::factory()->create();
+
+        Sanctum::actingAs($strangerUser);
+        $this->getJson("/api/v1/organisations/{$orgA->id}/form-failures/{$failureInOrgA->id}")
+            ->assertStatus(404);
+    }
+
+    public function test_org_resolve_authenticated_without_role_returns_404(): void
+    {
+        [$orgA, $adminA] = $this->makeOrgWithAdmin();
+        $failureInOrgA = $this->makeFailureInOrg($orgA);
+        $strangerUser = User::factory()->create();
+
+        Sanctum::actingAs($strangerUser);
+        $this->postJson(
+            "/api/v1/organisations/{$orgA->id}/form-failures/{$failureInOrgA->id}/resolve",
+            ['note' => 'stranger'],
+        )->assertStatus(404);
+    }
+
+    public function test_org_dismiss_authenticated_without_role_returns_404(): void
+    {
+        [$orgA, $adminA] = $this->makeOrgWithAdmin();
+        $failureInOrgA = $this->makeFailureInOrg($orgA);
+        $strangerUser = User::factory()->create();
+
+        Sanctum::actingAs($strangerUser);
+        $this->postJson(
+            "/api/v1/organisations/{$orgA->id}/form-failures/{$failureInOrgA->id}/dismiss",
+            ['reason_type' => DismissalReasonType::OTHER->value, 'note' => 'stranger'],
+        )->assertStatus(404);
+    }
+
+    // ============================================================
+    // HELPERS
+    // ============================================================
+
+    /**
+     * @return array{0: Organisation, 1: User, 2: Organisation, 3: User}
+     */
+    private function makeTwoOrgsWithAdmins(): array
+    {
+        [$orgA, $adminA] = $this->makeOrgWithAdmin();
+        [$orgB, $adminB] = $this->makeOrgWithAdmin();
+
+        return [$orgA, $adminA, $orgB, $adminB];
+    }
+
+    /**
+     * Attach a fresh org_admin to a new organisation via the
+     * organisation_users pivot (Crewli's user-org pivot pattern;
+     * NOT Spatie's role table).
+     *
+     * @return array{0: Organisation, 1: User}
+     */
+    private function makeOrgWithAdmin(): array
+    {
+        $org = Organisation::factory()->create();
+        $admin = User::factory()->create();
+        $org->users()->attach($admin, ['role' => 'org_admin']);
+
+        return [$org, $admin];
+    }
+
+    private function makeFailureInOrg(Organisation $org): FormSubmissionActionFailure
+    {
+        $schema = FormSchema::factory()->create(['organisation_id' => $org->id]);
+        $submission = FormSubmission::factory()->create([
+            'form_schema_id' => $schema->id,
+            'organisation_id' => $org->id,
+        ]);
+
+        return FormSubmissionActionFailure::factory()
+            ->for($submission, 'submission')
+            ->create();
+    }
+}