refactor(form-builder): restore type-hinted route model binding for failures controller (WS-6)

Replace the manual `$request->route('formSubmissionActionFailure')` workaround
with type-hinted parameters. Implicit route model binding now resolves
FormSubmissionActionFailure correctly on both the platform admin route
(/admin/form-failures/{id}) and the org-scoped route
(/organisations/{organisation}/form-failures/{id}).

Root cause:
On the nested org-scoped route, Laravel's implicit binding triggers its
scoped-binding code path: for the second URL segment, it tries to resolve
the failure as a relation of the route's parent ({organisation}) by calling
`$organisation->formSubmissionActionFailures()`. Organisation has no such
relation (failures live under FormSubmission, not Organisation directly),
so the lookup silently fell through and the controller received a raw
string. PHP then raised a TypeError on the type-hinted parameter.

A second issue compounded it: with the controller method declaring
`(FormSubmissionActionFailure $formSubmissionActionFailure, ?Organisation $organisation)`
the parameter order did NOT match the URL parameter order
(/{organisation}/.../{formSubmissionActionFailure}), so Laravel's
resolveMethodDependencies — which falls back to positional binding when
parameter counts diverge — bound them to the wrong slots.

Fix:
- Register an explicit `Route::bind('formSubmissionActionFailure', ...)`
  in AppServiceProvider that loads the model `withoutGlobalScopes()` and
  throws ModelNotFoundException on miss. This sidesteps the scoped-binding
  parent-relation lookup entirely.
- Add `->withoutScopedBindings()` to all four org-scoped routes (show,
  retry, resolve, dismiss) as a belt-and-braces guarantee that Laravel
  never enters the scoped-binding path for these nested routes.
- Reorder controller method signatures to put `?Organisation $organisation`
  FIRST, matching URL parameter order so positional binding lands the
  ULID strings on the correct method parameters.
- Drop the now-unused private `resolveFailure()` helper.
- Tenant scoping continues to be enforced by FormSubmissionActionFailurePolicy
  via the failure.submission.organisation_id FK chain (RFC V3); cross-
  tenant access still translates denied → 404, never 403.

Tests: all 9 controller tests pass (cross-tenant 404 contract verified for
view, dismiss, and resolve).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-04-28 08:57:06 +02:00
parent 6dace312b4
commit 6399bacdb6
4 changed files with 105 additions and 80 deletions

View File

@@ -55,17 +55,23 @@ final class FormSubmissionActionFailureController extends Controller
return FormSubmissionActionFailureResource::collection($failures);
}
public function show(\Illuminate\Http\Request $request): FormSubmissionActionFailureResource
public function show(?Organisation $organisation, FormSubmissionActionFailure $formSubmissionActionFailure): FormSubmissionActionFailureResource
{
$failure = $this->resolveFailure((string) $request->route('formSubmissionActionFailure'));
$this->authorizeOrNotFound('view', $failure);
// $organisation is bound only on the org-scoped route; null on the
// platform admin route. We don't use it directly — the policy
// enforces tenant scope via failure.submission.organisation_id (RFC V3).
// Declaration order matches URL parameter order so Laravel's
// resolveMethodDependencies binds positionally as expected.
unset($organisation);
$this->authorizeOrNotFound('view', $formSubmissionActionFailure);
return new FormSubmissionActionFailureResource($failure);
return new FormSubmissionActionFailureResource($formSubmissionActionFailure);
}
public function retry(\Illuminate\Http\Request $request, FormBindingApplicator $applicator): FormSubmissionActionFailureResource|JsonResponse
public function retry(?Organisation $organisation, FormSubmissionActionFailure $formSubmissionActionFailure, FormBindingApplicator $applicator): FormSubmissionActionFailureResource|JsonResponse
{
$failure = $this->resolveFailure((string) $request->route('formSubmissionActionFailure'));
unset($organisation);
$failure = $formSubmissionActionFailure;
$this->authorizeOrNotFound('retry', $failure);
if (! $failure->canBeRetried()) {
@@ -119,9 +125,12 @@ final class FormSubmissionActionFailureController extends Controller
}
public function resolve(
?Organisation $organisation,
FormSubmissionActionFailure $formSubmissionActionFailure,
ResolveFailureRequest $request,
): FormSubmissionActionFailureResource|JsonResponse {
$failure = $this->resolveFailure((string) $request->route('formSubmissionActionFailure'));
unset($organisation);
$failure = $formSubmissionActionFailure;
$this->authorizeOrNotFound('resolve', $failure);
if ($failure->resolved_at !== null) {
@@ -145,9 +154,12 @@ final class FormSubmissionActionFailureController extends Controller
}
public function dismiss(
?Organisation $organisation,
FormSubmissionActionFailure $formSubmissionActionFailure,
DismissFailureRequest $request,
): FormSubmissionActionFailureResource|JsonResponse {
$failure = $this->resolveFailure((string) $request->route('formSubmissionActionFailure'));
unset($organisation);
$failure = $formSubmissionActionFailure;
$this->authorizeOrNotFound('dismiss', $failure);
if ($failure->resolved_at !== null) {
@@ -171,24 +183,6 @@ final class FormSubmissionActionFailureController extends Controller
return new FormSubmissionActionFailureResource($failure->refresh());
}
/**
* Manual model resolution. Implicit binding doesn't pick up nested-
* namespace ULID models reliably across nested route groups, so we
* load explicitly without global scopes (cross-tenant access reaches
* the policy, which then translates denied 404 per RFC V3).
*/
private function resolveFailure(string $id): FormSubmissionActionFailure
{
$failure = FormSubmissionActionFailure::query()
->withoutGlobalScopes()
->find($id);
if ($failure === null) {
throw new ModelNotFoundException();
}
return $failure;
}
/**
* Translate a denied policy to 404 (RFC V3) so cross-tenant access
* does NOT confirm resource existence.
@@ -196,7 +190,7 @@ final class FormSubmissionActionFailureController extends Controller
private function authorizeOrNotFound(string $ability, FormSubmissionActionFailure $failure): void
{
if (Gate::denies($ability, $failure)) {
throw new ModelNotFoundException();
throw new ModelNotFoundException;
}
}
}

View File

@@ -4,12 +4,18 @@ declare(strict_types=1);
namespace App\Providers;
use App\Events\FormBuilder\FormSubmissionSectionSubmitted;
use App\Events\FormBuilder\FormSubmissionSubmitted;
use App\FormBuilder\Bindings\BindingActivityLogger;
use App\FormBuilder\Bindings\BindingConflictResolver;
use App\FormBuilder\Bindings\BindingTypeRegistry;
use App\FormBuilder\Bindings\FormBindingApplicator;
use App\FormBuilder\Bindings\PersonProvisioner;
use App\FormBuilder\Purposes\PurposeRegistry;
use App\Listeners\FormBuilder\ApplyBindingsOnFormSectionSubmitted;
use App\Listeners\FormBuilder\ApplyBindingsOnFormSubmit;
use App\Listeners\FormBuilder\SyncTagPickerSelectionsOnSubmit;
use App\Listeners\FormBuilder\TriggerPersonIdentityMatchOnFormSubmit;
use App\Models\Company;
use App\Models\CrowdList;
use App\Models\CrowdType;
@@ -50,12 +56,6 @@ use App\Models\UserInvitation;
use App\Models\UserOrganisationTag;
use App\Models\UserProfile;
use App\Models\VolunteerAvailability;
use App\Events\FormBuilder\FormSubmissionSectionSubmitted;
use App\Events\FormBuilder\FormSubmissionSubmitted;
use App\Listeners\FormBuilder\ApplyBindingsOnFormSectionSubmitted;
use App\Listeners\FormBuilder\ApplyBindingsOnFormSubmit;
use App\Listeners\FormBuilder\SyncTagPickerSelectionsOnSubmit;
use App\Listeners\FormBuilder\TriggerPersonIdentityMatchOnFormSubmit;
use App\Observers\FormBuilder\FormFieldChildTablesCascadeObserver;
use App\Observers\FormBuilder\FormSubmissionObserver;
use App\Observers\FormBuilder\FormValueObserver;
@@ -63,8 +63,8 @@ use App\Observers\PersonObserver;
use App\Observers\UserObserver;
use Illuminate\Auth\Notifications\ResetPassword;
use Illuminate\Database\Eloquent\Relations\Relation;
use Illuminate\Support\ServiceProvider;
use Illuminate\Support\Facades\Gate;
use Illuminate\Support\ServiceProvider;
use Spatie\Activitylog\Models\Activity;
class AppServiceProvider extends ServiceProvider
@@ -130,6 +130,26 @@ class AppServiceProvider extends ServiceProvider
\App\Policies\FormBuilder\FormSubmissionActionFailurePolicy::class,
);
// RFC-WS-6 v1.1 §3 Q5 — explicit route model binding for the
// form-failures admin endpoints. Implicit binding on nested route
// groups triggers Laravel's scoped-binding logic that tries to
// resolve the failure as a relation of the route's parent
// ({organisation}) — Organisation has no formSubmissionActionFailures()
// relation, so the implicit lookup silently fails and the
// controller receives a string. Route::bind sidesteps the
// scoped-binding path entirely. Bypass global scopes so cross-
// tenant access reaches the policy (which translates denied → 404
// per RFC V3 in the controller).
\Illuminate\Support\Facades\Route::bind('formSubmissionActionFailure', static function (string $value): \App\Models\FormBuilder\FormSubmissionActionFailure {
$model = \App\Models\FormBuilder\FormSubmissionActionFailure::query()
->withoutGlobalScopes()
->find($value);
if ($model === null) {
throw new \Illuminate\Database\Eloquent\ModelNotFoundException;
}
return $model;
});
Person::observe(PersonObserver::class);
User::observe(UserObserver::class);
@@ -177,7 +197,7 @@ class AppServiceProvider extends ServiceProvider
);
ResetPassword::createUrlUsing(function ($user, string $token) {
return config('crewli.portal_url') . '/wachtwoord-resetten?token=' . $token . '&email=' . urlencode($user->email);
return config('crewli.portal_url').'/wachtwoord-resetten?token='.$token.'&email='.urlencode($user->email);
});
// Tag activity log entries with impersonation context

View File

@@ -2,46 +2,24 @@
declare(strict_types=1);
use App\Http\Controllers\Api\V1\CheckEmailController;
use App\Http\Controllers\Api\V1\CompanyController;
use App\Http\Controllers\Api\V1\EmailLogController;
use App\Http\Controllers\Api\V1\CrowdListController;
use App\Http\Controllers\Api\V1\CrowdTypeController;
use App\Http\Controllers\Api\V1\EventController;
use App\Http\Controllers\Api\V1\FestivalSectionController;
use App\Http\Controllers\Api\V1\InvitationController;
use App\Http\Controllers\Api\V1\LocationController;
use App\Http\Controllers\Api\V1\LoginController;
use App\Http\Controllers\Api\V1\LogoutController;
use App\Http\Controllers\Api\V1\MeController;
use App\Http\Controllers\Api\V1\MemberController;
use App\Http\Controllers\Api\V1\OrganisationController;
use App\Http\Controllers\Api\V1\OrganisationEmailSettingsController;
use App\Http\Controllers\Api\V1\OrganisationEmailTemplateController;
use App\Http\Controllers\Api\V1\PersonController;
use App\Http\Controllers\Api\V1\PersonIdentityMatchController;
use App\Http\Controllers\Api\V1\PersonTagController;
use App\Http\Controllers\Api\V1\ShiftAssignmentController;
use App\Http\Controllers\Api\V1\ShiftController;
use App\Http\Controllers\Api\V1\TimeSlotController;
use App\Http\Controllers\Api\V1\VolunteerAvailabilityController;
use App\Http\Controllers\Api\V1\PortalTokenController;
use App\Http\Controllers\Api\V1\AccountController;
use App\Http\Controllers\Api\V1\AuthRefreshController;
use App\Http\Controllers\Api\V1\EmailChangeController;
use App\Http\Controllers\Api\V1\PasswordResetController;
use App\Http\Controllers\Api\V1\PortalMeController;
use App\Http\Controllers\Api\V1\Portal\PortalShiftController;
use App\Http\Controllers\Api\V1\UploadController;
use App\Http\Controllers\Api\V1\UserOrganisationTagController;
use App\Http\Controllers\Api\V1\Admin\AdminOrganisationController;
use App\Http\Controllers\Api\V1\Admin\AdminUserController;
use App\Http\Controllers\Api\V1\Admin\AdminStatsController;
use App\Http\Controllers\Api\V1\Admin\AdminActivityLogController;
use App\Http\Controllers\Api\V1\Admin\AdminImpersonationController;
use App\Http\Controllers\Api\V1\Admin\AdminOrganisationController;
use App\Http\Controllers\Api\V1\Admin\AdminStatsController;
use App\Http\Controllers\Api\V1\Admin\AdminUserController;
use App\Http\Controllers\Api\V1\Auth\MfaSetupController;
use App\Http\Controllers\Api\V1\Auth\MfaVerifyController;
use App\Http\Controllers\Api\V1\Auth\TrustedDeviceController;
use App\Http\Controllers\Api\V1\AuthRefreshController;
use App\Http\Controllers\Api\V1\CheckEmailController;
use App\Http\Controllers\Api\V1\CompanyController;
use App\Http\Controllers\Api\V1\CrowdListController;
use App\Http\Controllers\Api\V1\CrowdTypeController;
use App\Http\Controllers\Api\V1\EmailChangeController;
use App\Http\Controllers\Api\V1\EmailLogController;
use App\Http\Controllers\Api\V1\EventController;
use App\Http\Controllers\Api\V1\FestivalSectionController;
use App\Http\Controllers\Api\V1\FormBuilder\FilterRegistryController;
use App\Http\Controllers\Api\V1\FormBuilder\FormFieldController;
use App\Http\Controllers\Api\V1\FormBuilder\FormFieldLibraryController;
@@ -53,6 +31,28 @@ use App\Http\Controllers\Api\V1\FormBuilder\FormSubmissionReviewController;
use App\Http\Controllers\Api\V1\FormBuilder\FormTemplateController;
use App\Http\Controllers\Api\V1\FormBuilder\FormValueController;
use App\Http\Controllers\Api\V1\FormBuilder\PublicFormController;
use App\Http\Controllers\Api\V1\InvitationController;
use App\Http\Controllers\Api\V1\LocationController;
use App\Http\Controllers\Api\V1\LoginController;
use App\Http\Controllers\Api\V1\LogoutController;
use App\Http\Controllers\Api\V1\MeController;
use App\Http\Controllers\Api\V1\MemberController;
use App\Http\Controllers\Api\V1\OrganisationController;
use App\Http\Controllers\Api\V1\OrganisationEmailSettingsController;
use App\Http\Controllers\Api\V1\OrganisationEmailTemplateController;
use App\Http\Controllers\Api\V1\PasswordResetController;
use App\Http\Controllers\Api\V1\PersonController;
use App\Http\Controllers\Api\V1\PersonIdentityMatchController;
use App\Http\Controllers\Api\V1\PersonTagController;
use App\Http\Controllers\Api\V1\Portal\PortalShiftController;
use App\Http\Controllers\Api\V1\PortalMeController;
use App\Http\Controllers\Api\V1\PortalTokenController;
use App\Http\Controllers\Api\V1\ShiftAssignmentController;
use App\Http\Controllers\Api\V1\ShiftController;
use App\Http\Controllers\Api\V1\TimeSlotController;
use App\Http\Controllers\Api\V1\UploadController;
use App\Http\Controllers\Api\V1\UserOrganisationTagController;
use App\Http\Controllers\Api\V1\VolunteerAvailabilityController;
use App\Models\FestivalSection;
use App\Models\Organisation;
use Illuminate\Support\Facades\Gate;
@@ -243,11 +243,15 @@ Route::middleware(['auth:sanctum', 'impersonation'])->group(function () {
Route::get('email-logs', [EmailLogController::class, 'index']);
// RFC-WS-6 §3 (Q5) — org-scoped form-failure admin endpoints.
// withoutScopedBindings(): tenant scope is enforced by the policy
// (RFC V3), not by Laravel's parent-relation scoped binding.
// Organisation has no formSubmissionActionFailures() relation;
// the policy's FK-chain check is the tenant gate.
Route::get('form-failures', [\App\Http\Controllers\Api\V1\FormBuilder\FormSubmissionActionFailureController::class, 'orgIndex']);
Route::get('form-failures/{formSubmissionActionFailure}', [\App\Http\Controllers\Api\V1\FormBuilder\FormSubmissionActionFailureController::class, 'show']);
Route::post('form-failures/{formSubmissionActionFailure}/retry', [\App\Http\Controllers\Api\V1\FormBuilder\FormSubmissionActionFailureController::class, 'retry']);
Route::post('form-failures/{formSubmissionActionFailure}/resolve', [\App\Http\Controllers\Api\V1\FormBuilder\FormSubmissionActionFailureController::class, 'resolve']);
Route::post('form-failures/{formSubmissionActionFailure}/dismiss', [\App\Http\Controllers\Api\V1\FormBuilder\FormSubmissionActionFailureController::class, 'dismiss']);
Route::get('form-failures/{formSubmissionActionFailure}', [\App\Http\Controllers\Api\V1\FormBuilder\FormSubmissionActionFailureController::class, 'show'])->withoutScopedBindings();
Route::post('form-failures/{formSubmissionActionFailure}/retry', [\App\Http\Controllers\Api\V1\FormBuilder\FormSubmissionActionFailureController::class, 'retry'])->withoutScopedBindings();
Route::post('form-failures/{formSubmissionActionFailure}/resolve', [\App\Http\Controllers\Api\V1\FormBuilder\FormSubmissionActionFailureController::class, 'resolve'])->withoutScopedBindings();
Route::post('form-failures/{formSubmissionActionFailure}/dismiss', [\App\Http\Controllers\Api\V1\FormBuilder\FormSubmissionActionFailureController::class, 'dismiss'])->withoutScopedBindings();
// Person tags (organisation settings)
Route::apiResource('person-tags', PersonTagController::class)

View File

@@ -72,7 +72,8 @@ final class FormSubmissionActionFailureControllerTest extends TestCase
public function test_super_admin_can_view_failure(): void
{
$this->withoutExceptionHandling();
Sanctum::actingAs($this->superAdmin); $this
Sanctum::actingAs($this->superAdmin);
$this
->getJson("/api/v1/admin/form-failures/{$this->failureInOrgA->id}")
->assertOk()
->assertJsonPath('data.id', (string) $this->failureInOrgA->id);
@@ -91,14 +92,16 @@ final class FormSubmissionActionFailureControllerTest extends TestCase
*/
public function test_cross_tenant_access_returns_404_not_403(): void
{
Sanctum::actingAs($this->orgAdminB); $this
Sanctum::actingAs($this->orgAdminB);
$this
->getJson("/api/v1/organisations/{$this->orgA->id}/form-failures/{$this->failureInOrgA->id}")
->assertStatus(404);
}
public function test_resolve_endpoint_with_note(): void
{
Sanctum::actingAs($this->superAdmin); $this
Sanctum::actingAs($this->superAdmin);
$this
->postJson("/api/v1/admin/form-failures/{$this->failureInOrgA->id}/resolve", [
'note' => 'fixed via direct edit',
])
@@ -109,7 +112,8 @@ final class FormSubmissionActionFailureControllerTest extends TestCase
public function test_dismiss_endpoint_with_enum_reason(): void
{
Sanctum::actingAs($this->superAdmin); $this
Sanctum::actingAs($this->superAdmin);
$this
->postJson("/api/v1/admin/form-failures/{$this->failureInOrgA->id}/dismiss", [
'reason_type' => DismissalReasonType::SCHEMA_DELETED->value,
])
@@ -120,7 +124,8 @@ final class FormSubmissionActionFailureControllerTest extends TestCase
public function test_dismiss_other_without_note_fails_422(): void
{
Sanctum::actingAs($this->superAdmin); $this
Sanctum::actingAs($this->superAdmin);
$this
->postJson("/api/v1/admin/form-failures/{$this->failureInOrgA->id}/dismiss", [
'reason_type' => DismissalReasonType::OTHER->value,
])
@@ -130,7 +135,8 @@ final class FormSubmissionActionFailureControllerTest extends TestCase
public function test_cross_tenant_dismiss_returns_404(): void
{
Sanctum::actingAs($this->orgAdminB); $this
Sanctum::actingAs($this->orgAdminB);
$this
->postJson("/api/v1/organisations/{$this->orgA->id}/form-failures/{$this->failureInOrgA->id}/dismiss", [
'reason_type' => DismissalReasonType::OTHER->value,
'note' => 'evil',
@@ -140,7 +146,8 @@ final class FormSubmissionActionFailureControllerTest extends TestCase
public function test_cross_tenant_resolve_returns_404(): void
{
Sanctum::actingAs($this->orgAdminB); $this
Sanctum::actingAs($this->orgAdminB);
$this
->postJson("/api/v1/organisations/{$this->orgA->id}/form-failures/{$this->failureInOrgA->id}/resolve", [])
->assertStatus(404);
}