Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/Http/Controllers/Api/DeviceController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
8 changes: 4 additions & 4 deletions app/Http/Requests/CreateApiDeviceRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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<string, list<mixed>>
*/
Expand All @@ -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'],
];
}

Expand Down
23 changes: 15 additions & 8 deletions tests/Feature/Controllers/Api/DeviceControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});

Expand Down
Loading