From 64ca0db4bd66927e6a7255ab8dec32231afaf119 Mon Sep 17 00:00:00 2001 From: Simon Hamp Date: Mon, 2 Nov 2026 13:22:30 +0000 Subject: [PATCH] fix: take a yes for the terms at app sign-in instead of a version The app had to send the exact version in force or sign-in failed with a 422. A version in the app is fixed when it's built, so raising the version would lock out every installed build. The value also did nothing: acceptance is recorded as a timestamp and compared with the version. The app now says terms_accepted, and the time we record stands for the version in force. Co-Authored-By: Claude Opus 5.5 --- app/Http/Controllers/Api/DeviceController.php | 2 +- app/Http/Requests/CreateApiDeviceRequest.php | 8 +++---- .../Controllers/Api/DeviceControllerTest.php | 23 ++++++++++++------- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/app/Http/Controllers/Api/DeviceController.php b/app/Http/Controllers/Api/DeviceController.php index 8a105c551..7cb5dfc58 100644 --- a/app/Http/Controllers/Api/DeviceController.php +++ b/app/Http/Controllers/Api/DeviceController.php @@ -43,7 +43,7 @@ public function store(CreateApiDeviceRequest $request, CreateDevice $action, Upd return ApiException::invalidGrant()->render(); } - if ($request->has('terms_version')) { + if ($request->boolean('terms_accepted')) { $acceptTerms->handle($device->user); } diff --git a/app/Http/Requests/CreateApiDeviceRequest.php b/app/Http/Requests/CreateApiDeviceRequest.php index f685cc153..f4480773a 100644 --- a/app/Http/Requests/CreateApiDeviceRequest.php +++ b/app/Http/Requests/CreateApiDeviceRequest.php @@ -12,7 +12,6 @@ use Illuminate\Foundation\Http\FormRequest; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\RateLimiter; -use Illuminate\Validation\Rule; use InvalidArgumentException; /** @@ -52,8 +51,9 @@ public function authorize(SignedRequests $signedRequests): bool } /** - * The verifier is the one the app made its PKCE challenge from (RFC 7636). An app that showed the terms and privacy - * policy before sending the person to sign in says which version, and it is recorded as accepted. + * The verifier is the one the app made its PKCE challenge from (RFC 7636). An app that asked the person to accept the + * terms and privacy policy before sending them to sign in says they did. It links to the pages themselves rather + * than carrying a copy, so what they accepted is the version in force, which the time it's recorded stands for. * * @return array> */ @@ -62,7 +62,7 @@ public function rules(): array return [ 'code' => ['required', 'string', 'max:255'], 'code_verifier' => ['required', 'string', 'regex:/^[A-Za-z0-9._~-]{43,128}$/'], - 'terms_version' => ['sometimes', 'string', Rule::in([config('legal.terms_version')])], + 'terms_accepted' => ['sometimes', 'boolean'], ]; } diff --git a/tests/Feature/Controllers/Api/DeviceControllerTest.php b/tests/Feature/Controllers/Api/DeviceControllerTest.php index 95092a061..acaa97198 100644 --- a/tests/Feature/Controllers/Api/DeviceControllerTest.php +++ b/tests/Feature/Controllers/Api/DeviceControllerTest.php @@ -26,23 +26,30 @@ )); }); -it('records the terms as accepted when the app says which version it showed before sign-in', function (): void { - signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['code' => $this->code, 'code_verifier' => $this->verifier, 'terms_version' => config('legal.terms_version')]) +it('records the terms in force as accepted when the app says the person accepted them before sign-in', function (): void { + signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['code' => $this->code, 'code_verifier' => $this->verifier, 'terms_accepted' => true]) ->assertOk() ->assertJsonPath('data.terms_accepted', true); expect($this->user->refresh()->hasAcceptedCurrentTerms())->toBeTrue(); }); -it('leaves the terms alone when the app does not say, and refuses a version that is not in force', function (): void { - signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['code' => $this->code, 'code_verifier' => $this->verifier, 'terms_version' => '1999-01-01']) - ->assertUnprocessable() - ->assertJsonValidationErrors('terms_version'); - - signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['code' => $this->code, 'code_verifier' => $this->verifier]) +it('leaves the terms alone when the app does not say they were accepted', function (array $terms): void { + signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['code' => $this->code, 'code_verifier' => $this->verifier, ...$terms]) ->assertOk() ->assertJsonPath('data.terms_accepted', false); + expect($this->user->refresh()->terms_accepted_at)->toBeNull(); +})->with([ + 'not mentioned' => [[]], + 'refused' => [['terms_accepted' => false]], +]); + +it('refuses a terms answer that is not a yes or no', function (): void { + signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['code' => $this->code, 'code_verifier' => $this->verifier, 'terms_accepted' => 'maybe']) + ->assertUnprocessable() + ->assertJsonValidationErrors('terms_accepted'); + expect($this->user->refresh()->terms_accepted_at)->toBeNull(); });