From 485d28cacdd18e3f33b1f885bf043de58fd18a7b Mon Sep 17 00:00:00 2001 From: Julien BIANCHI Date: Mon, 19 Oct 2026 12:15:01 +0200 Subject: [PATCH 01/10] refactor: extract the sign-in page layout The mobile approval page draws itself as two commits on a lane: the logo, then the step the user is asked for. Signing in from a terminal needs the same page for another step, so the layout moves to a component of its own. A pure move: nothing changes for the mobile page, so there is nothing to test-drive. --- .../js/components/authorization-page.tsx | 60 +++++++++++++++++ .../js/pages/mobile-authorization/show.tsx | 65 ++----------------- 2 files changed, 65 insertions(+), 60 deletions(-) create mode 100644 resources/js/components/authorization-page.tsx diff --git a/resources/js/components/authorization-page.tsx b/resources/js/components/authorization-page.tsx new file mode 100644 index 0000000000..fbf162061e --- /dev/null +++ b/resources/js/components/authorization-page.tsx @@ -0,0 +1,60 @@ +import { Head } from '@inertiajs/react'; +import type { CSSProperties, ReactNode } from 'react'; +import AppLogo from '@/components/app-logo'; + +/** + * The standalone page a sign-in approval is asked on: the logo, then the command line the step reads as, then the + * step itself, laid out as two commits on a lane. + */ +export function AuthorizationPage({ + title, + lane, + command, + children, +}: { + title: string; + lane: string; + command: string; + children: ReactNode; +}) { + return ( + <> + + +
+
    +
  1. +
    + + +
    +
    + +
    +
    2. + +
  2. +
    + +
    +
    +

    + {command} +

    + {children} +
    +
    3. +
+
+ + ); +} diff --git a/resources/js/pages/mobile-authorization/show.tsx b/resources/js/pages/mobile-authorization/show.tsx index ef979f0948..de8e7c178e 100644 --- a/resources/js/pages/mobile-authorization/show.tsx +++ b/resources/js/pages/mobile-authorization/show.tsx @@ -1,8 +1,6 @@ -import { Head } from '@inertiajs/react'; -import type { CSSProperties, ReactNode } from 'react'; import { useState } from 'react'; import MobileAuthorizationController from '@/actions/App/Http/Controllers/MobileAuthorizationController'; -import AppLogo from '@/components/app-logo'; +import { AuthorizationPage } from '@/components/authorization-page'; import { Button } from '@/components/ui/button'; import { UserAvatar } from '@/components/user-avatar'; import { WordmarkText } from '@/components/wordmark-text'; @@ -49,7 +47,7 @@ function Approve({ : `${authorization.app_name} on ${authorization.device_name}`; return ( - Cancel - + ); } function Invalid({ error }: { error: string }) { return ( - /dev/null app.

- - ); -} - -function Page({ - title, - lane, - command, - children, -}: { - title: string; - lane: string; - command: string; - children: ReactNode; -}) { - return ( - <> - - -
-
    -
  1. -
    - - -
    -
    - -
    -
    2. - -
  2. -
    - -
    -
    -

    - {command} -

    - {children} -
    -
    3. -
-
- + ); } From ee2dc7585a66ce2f04d0ee4e06adf64db4eed0cf Mon Sep 17 00:00:00 2001 From: Julien BIANCHI Date: Mon, 19 Oct 2026 12:27:53 +0200 Subject: [PATCH 02/10] feat: approve devices on the web with a user code A terminal or an agent that signs in to the API has no browser to be sent back to, so it can't go through /mobile/authorize. The device authorization grant of RFC 8628 gives it another way in: the sign-in lives in the cache under the hash of a long device code, with a short user code pointing at it, and the user types that code at /activate, sees which key and device name ask to sign in as them, and approves or denies it. Approving makes the key a device right away, as redeeming a code does for the mobile app: the key proved itself when it started the sign-in, and the user approved that very fingerprint. The decision is kept until the sign-in would have expired, for the terminal to find, and the user code is taken back, so a code is decided once. The user code is eight consonants from the alphabet RFC 8628 suggests, read however it is typed. Each user gets 20 lookups and decisions a minute, which is where a code could be guessed, and the sign-in expires after fifteen minutes. Nothing goes to the database but the device the user approved. --- app/Actions/CreateDeviceActivation.php | 35 +++ app/Actions/CreateDeviceAuthorization.php | 43 +++ app/Dto/DeviceAuthorization.php | 100 +++++++ app/Enums/DeviceAuthorizationStatus.php | 15 + .../DeviceActivationController.php | 54 ++++ .../CreateDeviceActivationRequest.php | 82 ++++++ .../Requests/ShowDeviceActivationRequest.php | 126 ++++++++ app/Providers/AppServiceProvider.php | 3 + app/Services/DeviceAuthorizations.php | 34 +++ app/Services/UserCodes.php | 41 +++ resources/js/lib/user-code.ts | 18 ++ resources/js/pages/device-activation/show.tsx | 271 ++++++++++++++++++ routes/web.php | 7 + .../DeviceActivationControllerTest.php | 251 ++++++++++++++++ tests/Pest.php | 13 + .../Actions/CreateDeviceActivationTest.php | 68 +++++ .../Actions/CreateDeviceAuthorizationTest.php | 65 +++++ tests/Unit/Dto/DeviceAuthorizationTest.php | 49 ++++ .../Unit/Providers/AppServiceProviderTest.php | 18 ++ .../Services/DeviceAuthorizationsTest.php | 37 +++ tests/Unit/Services/UserCodesTest.php | 36 +++ tests/Unit/UserCode.test.ts | 25 ++ 22 files changed, 1391 insertions(+) create mode 100644 app/Actions/CreateDeviceActivation.php create mode 100644 app/Actions/CreateDeviceAuthorization.php create mode 100644 app/Dto/DeviceAuthorization.php create mode 100644 app/Enums/DeviceAuthorizationStatus.php create mode 100644 app/Http/Controllers/DeviceActivationController.php create mode 100644 app/Http/Requests/CreateDeviceActivationRequest.php create mode 100644 app/Http/Requests/ShowDeviceActivationRequest.php create mode 100644 app/Services/DeviceAuthorizations.php create mode 100644 app/Services/UserCodes.php create mode 100644 resources/js/lib/user-code.ts create mode 100644 resources/js/pages/device-activation/show.tsx create mode 100644 tests/Feature/Controllers/DeviceActivationControllerTest.php create mode 100644 tests/Unit/Actions/CreateDeviceActivationTest.php create mode 100644 tests/Unit/Actions/CreateDeviceAuthorizationTest.php create mode 100644 tests/Unit/Dto/DeviceAuthorizationTest.php create mode 100644 tests/Unit/Services/DeviceAuthorizationsTest.php create mode 100644 tests/Unit/Services/UserCodesTest.php create mode 100644 tests/Unit/UserCode.test.ts diff --git a/app/Actions/CreateDeviceActivation.php b/app/Actions/CreateDeviceActivation.php new file mode 100644 index 0000000000..22a0743ab2 --- /dev/null +++ b/app/Actions/CreateDeviceActivation.php @@ -0,0 +1,35 @@ +updateOrCreate(['fingerprint' => $authorization->fingerprint], [ + 'user_id' => $user->id, + 'name' => $authorization->deviceName, + 'approved_at' => now(), + 'last_used_at' => null, + ]); + } + + Cache::put($authorization->key, $authorization->decided($status, $user)->toArray(), $authorization->expiresAt); + Cache::forget(DeviceAuthorization::userCodeKeyFor($authorization->userCode)); + } +} diff --git a/app/Actions/CreateDeviceAuthorization.php b/app/Actions/CreateDeviceAuthorization.php new file mode 100644 index 0000000000..120626a84e --- /dev/null +++ b/app/Actions/CreateDeviceAuthorization.php @@ -0,0 +1,43 @@ +addMinutes(DeviceAuthorization::EXPIRES_AFTER_MINUTES)->startOfSecond(); + + do { + $userCode = $this->userCodes->generate(); + } while (! Cache::add(DeviceAuthorization::userCodeKeyFor($userCode), $key, $expiresAt)); + + $authorization = new DeviceAuthorization($key, $fingerprint, $deviceName, $userCode, $expiresAt); + + Cache::put($key, $authorization->toArray(), $expiresAt); + + return ['device_code' => $deviceCode, 'authorization' => $authorization]; + } +} diff --git a/app/Dto/DeviceAuthorization.php b/app/Dto/DeviceAuthorization.php new file mode 100644 index 0000000000..9f53f88f68 --- /dev/null +++ b/app/Dto/DeviceAuthorization.php @@ -0,0 +1,100 @@ + $attributes + */ + public static function fromArray(string $key, array $attributes): self + { + $attributes = fluent($attributes); + $userId = $attributes->get('user_id'); + + return new self( + $key, + $attributes->string('fingerprint')->value(), + $attributes->string('device_name')->value(), + $attributes->string('user_code')->value(), + CarbonImmutable::createFromTimestamp($attributes->integer('expires_at')), + DeviceAuthorizationStatus::from($attributes->string('status')->value()), + is_string($userId) ? $userId : null, + ); + } + + /** + * The same sign-in once the user decided on it: approved for them, or denied, in which case it names nobody. + */ + public function decided(DeviceAuthorizationStatus $status, User $user): self + { + return new self( + $this->key, + $this->fingerprint, + $this->deviceName, + $this->userCode, + $this->expiresAt, + $status, + $status === DeviceAuthorizationStatus::Approved ? $user->id : null, + ); + } + + /** + * @return array{fingerprint: string, device_name: string, user_code: string, expires_at: int, status: string, user_id: string|null} + */ + public function toArray(): array + { + return [ + 'fingerprint' => $this->fingerprint, + 'device_name' => $this->deviceName, + 'user_code' => $this->userCode, + 'expires_at' => $this->expiresAt->getTimestamp(), + 'status' => $this->status->value, + 'user_id' => $this->userId, + ]; + } +} diff --git a/app/Enums/DeviceAuthorizationStatus.php b/app/Enums/DeviceAuthorizationStatus.php new file mode 100644 index 0000000000..a18c420e6d --- /dev/null +++ b/app/Enums/DeviceAuthorizationStatus.php @@ -0,0 +1,15 @@ +authorization(); + + return Inertia::render('device-activation/show', $authorization instanceof DeviceAuthorization ? [ + 'authorization' => [ + 'user_code' => $authorization->userCode, + 'device_name' => $authorization->deviceName, + 'fingerprint' => $authorization->fingerprint, + ], + 'csrfToken' => csrf_token(), + ] : []); + } + + /** + * Approves the key as one of the user's devices, or denies it, and tells the user what their terminal will find. + */ + public function store(CreateDeviceActivationRequest $request, #[CurrentUser] User $user, CreateDeviceActivation $action): Response + { + $authorization = $request->authorization(); + $decision = $request->decision(); + + $action->handle($user, $authorization, $decision); + + return Inertia::render('device-activation/show', [ + 'done' => [ + 'status' => $decision->value, + 'device_name' => $authorization->deviceName, + ], + ]); + } +} diff --git a/app/Http/Requests/CreateDeviceActivationRequest.php b/app/Http/Requests/CreateDeviceActivationRequest.php new file mode 100644 index 0000000000..53cd6270c9 --- /dev/null +++ b/app/Http/Requests/CreateDeviceActivationRequest.php @@ -0,0 +1,82 @@ +> + */ + public function rules(): array + { + return [ + 'user_code' => ['required', ...ShowDeviceActivationRequest::userCodeRules()], + 'decision' => ['required', 'string', 'in:approved,denied'], + ]; + } + + /** + * @return array + */ + public function messages(): array + { + return ShowDeviceActivationRequest::userCodeMessages(); + } + + /** + * Only a sign-in still waiting for a decision can be decided: one decided or expired since the page was shown + * gets the same answer as a code that never was. + */ + public function passedValidation(): void + { + $authorization = resolve(DeviceAuthorizations::class)->pendingFromUserCode($this->string('user_code')->value()); + + throw_unless($authorization instanceof DeviceAuthorization, ShowDeviceActivationRequest::invalid($this, ShowDeviceActivationRequest::unknownCode())); + + $this->authorization = $authorization; + } + + public function authorization(): DeviceAuthorization + { + assert($this->authorization instanceof DeviceAuthorization); + + return $this->authorization; + } + + public function decision(): DeviceAuthorizationStatus + { + $decision = $this->enum('decision', DeviceAuthorizationStatus::class); + + assert($decision instanceof DeviceAuthorizationStatus); + + return $decision; + } + + public function prepareForValidation(): void + { + ShowDeviceActivationRequest::normalizeUserCode($this); + } + + public function failedValidation(Validator $validator): never + { + throw ShowDeviceActivationRequest::invalid($this, $validator->errors()->first()); + } +} diff --git a/app/Http/Requests/ShowDeviceActivationRequest.php b/app/Http/Requests/ShowDeviceActivationRequest.php new file mode 100644 index 0000000000..195addd050 --- /dev/null +++ b/app/Http/Requests/ShowDeviceActivationRequest.php @@ -0,0 +1,126 @@ + + */ + public static function userCodeRules(): array + { + return ['string', 'regex:/^['.UserCodes::Alphabet.']{4}-['.UserCodes::Alphabet.']{4}$/']; + } + + /** + * @return array + */ + public static function userCodeMessages(): array + { + return ['user_code.regex' => __('A code is eight letters, like WDJB-MJHT.')]; + } + + public static function unknownCode(): string + { + return __('This code is unknown or has expired. Check your terminal, and start the sign-in again if you need to.'); + } + + /** + * Tidies up the code the user typed, so it is read however they typed it: in lowercase, without the dash, with + * spaces around. + */ + public static function normalizeUserCode(FormRequest $request): void + { + $typed = $request->input('user_code'); + + if (is_string($typed) && $typed !== '') { + $request->merge(['user_code' => resolve(UserCodes::class)->normalize($typed)]); + } + } + + /** + * A code that can't be found, or a request that isn't right, gets the page again with what is wrong and the code + * as it was read, for the user to fix it. Nothing is decided, and nothing is redirected. + */ + public static function invalid(FormRequest $request, string $error): HttpResponseException + { + $typed = $request->input('user_code'); + + return new HttpResponseException(Inertia::render('device-activation/show', [ + 'error' => $error, + 'userCode' => is_string($typed) ? $typed : '', + ])->toResponse($request)->setStatusCode(400)); + } + + public function authorize(): bool + { + return true; + } + + /** + * @return array> + */ + public function rules(): array + { + return [ + 'user_code' => ['nullable', ...self::userCodeRules()], + ]; + } + + /** + * @return array + */ + public function messages(): array + { + return self::userCodeMessages(); + } + + /** + * The sign-in behind the code, when the user typed one: it has to be one that is still waiting for a decision. + */ + public function passedValidation(): void + { + if (! $this->filled('user_code')) { + return; + } + + $authorization = resolve(DeviceAuthorizations::class)->pendingFromUserCode($this->string('user_code')->value()); + + throw_unless($authorization instanceof DeviceAuthorization, self::invalid($this, self::unknownCode())); + + $this->authorization = $authorization; + } + + public function authorization(): ?DeviceAuthorization + { + return $this->authorization; + } + + public function prepareForValidation(): void + { + self::normalizeUserCode($this); + } + + public function failedValidation(Validator $validator): never + { + throw self::invalid($this, $validator->errors()->first()); + } +} diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 9c99c61e37..57921db1b0 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -205,6 +205,9 @@ private function bootRateLimitingDefaults(): void // Approving the devnull app on the web hands out a code each time... RateLimiter::for('mobile-authorizations', fn (Request $request): Limit => Limit::perMinute(10)->by($this->userId($request))); + // Looking up the code a terminal shows is where one could be guessed, so each user gets a few tries... + RateLimiter::for('device-activations', fn (Request $request): Limit => Limit::perMinute(20)->by($this->userId($request))); + // GitHub fetches every README image through a few proxy addresses, so each graph gets its own budget... RateLimiter::for('activity-graph', fn (Request $request): Limit => Limit::perMinute(60)->by("{$request->ip()}|{$request->path()}")); diff --git a/app/Services/DeviceAuthorizations.php b/app/Services/DeviceAuthorizations.php new file mode 100644 index 0000000000..ed617ba3b7 --- /dev/null +++ b/app/Services/DeviceAuthorizations.php @@ -0,0 +1,34 @@ +fromKey($key) : null; + + return $authorization?->status === DeviceAuthorizationStatus::Pending ? $authorization : null; + } + + private function fromKey(string $key): ?DeviceAuthorization + { + $stored = Cache::get($key); + + return is_array($stored) ? DeviceAuthorization::fromArray($key, $stored) : null; + } +} diff --git a/app/Services/UserCodes.php b/app/Services/UserCodes.php new file mode 100644 index 0000000000..f353d8dae7 --- /dev/null +++ b/app/Services/UserCodes.php @@ -0,0 +1,41 @@ +normalize($letters); + } + + /** + * The code as the terminal shows it, out of what the user typed: uppercase, without spaces or dashes, and with + * its dash back once it has the eight letters. Anything else is left as it is, for validation to refuse. + */ + public function normalize(string $typed): string + { + $letters = mb_strtoupper((string) preg_replace('/[\s-]+/u', '', $typed)); + + return mb_strlen($letters) === self::Length + ? mb_substr($letters, 0, 4).'-'.mb_substr($letters, 4) + : $letters; + } +} diff --git a/resources/js/lib/user-code.ts b/resources/js/lib/user-code.ts new file mode 100644 index 0000000000..22102ccc65 --- /dev/null +++ b/resources/js/lib/user-code.ts @@ -0,0 +1,18 @@ +const NOT_A_LETTER = /[^BCDFGHJKLMNPQRSTVWXZ]/g; + +const LENGTH = 8; + +/** + * The code as the terminal shows it, out of what the person typed: its letters in uppercase, anything else left out, + * at most eight of them, with the dash after the fourth as soon as there is a fifth. + */ +export function formatUserCode(input: string): string { + const letters = input + .toUpperCase() + .replace(NOT_A_LETTER, '') + .slice(0, LENGTH); + + return letters.length > 4 + ? `${letters.slice(0, 4)}-${letters.slice(4)}` + : letters; +} diff --git a/resources/js/pages/device-activation/show.tsx b/resources/js/pages/device-activation/show.tsx new file mode 100644 index 0000000000..33b309b936 --- /dev/null +++ b/resources/js/pages/device-activation/show.tsx @@ -0,0 +1,271 @@ +import type { ReactNode } from 'react'; +import { useState } from 'react'; +import DeviceActivationController from '@/actions/App/Http/Controllers/DeviceActivationController'; +import { AuthorizationPage } from '@/components/authorization-page'; +import InputError from '@/components/input-error'; +import { Button } from '@/components/ui/button'; +import { UserAvatar } from '@/components/user-avatar'; +import { WordmarkText } from '@/components/wordmark-text'; +import { useUser } from '@/hooks/use-user'; +import { formatUserCode } from '@/lib/user-code'; +import { laneColor } from '@/lib/utils'; + +type Authorization = { + user_code: string; + device_name: string; + fingerprint: string; +}; + +type Done = { + status: 'approved' | 'denied'; + device_name: string; +}; + +type Props = + | { authorization: Authorization; csrfToken: string } + | { done: Done } + | { error?: string; userCode?: string }; + +export default function Show(props: Props) { + if ('authorization' in props) { + return ; + } + + if ('done' in props) { + return ; + } + + return ; +} + +function Enter({ error, userCode }: { error?: string; userCode?: string }) { + const { user } = useUser(); + const [code, setCode] = useState(formatUserCode(userCode ?? '')); + + return ( + +

+ Enter the code your terminal shows. +

+

+ A terminal or an agent signing in to{' '} + /dev/null as you shows a code like + WDJB-MJHT. Type it here, then check which device it is before + approving it. +

+ + {/* A plain form, like the mobile page's: a code that can't be found comes back as this page... */} +
+
+ + + setCode(formatUserCode(event.target.value)) + } + required + autoFocus + autoComplete="one-time-code" + autoCapitalize="characters" + spellCheck={false} + maxLength={9} + placeholder="WDJB-MJHT" + aria-invalid={error ? true : undefined} + aria-describedby={error ? 'user-code-error' : undefined} + className="h-11 w-full rounded-lg border bg-background px-3 font-mono text-lg tracking-[0.2em] uppercase outline-none placeholder:text-muted-foreground focus:border-ring" + /> + +
+ +
+ + ); +} + +function Approve({ + authorization, + csrfToken, +}: { + authorization: Authorization; + csrfToken: string; +}) { + const { user } = useUser(); + const [isDeciding, setIsDeciding] = useState(false); + + return ( + +

+ Approve {authorization.device_name}? +

+

+ A device calling itself {authorization.device_name} asks to sign + in as you, with the code below. Only approve it if you started + this yourself a moment ago. +

+ +
+
+
Code
+
+ {authorization.user_code} +
+
+
+
+ Signs in as +
+
+ + + + {user.name} + + + @{user.handle} + + +
+
+
+
With key
+
+ {authorization.fingerprint} +
+
+
+ +

+ The device will be able to use{' '} + /dev/null as you. You can revoke it + any time under Settings → Devices. +

+ + {/* Plain forms, as on the mobile page: the answer is this page again, never a redirect. One form per + decision, since a submit button disabled while the page is sent no longer carries its own value... */} +
+ setIsDeciding(true)} + > + + + setIsDeciding(true)} + > + + +
+ + ); +} + +function DecisionForm({ + decision, + userCode, + csrfToken, + disabled, + onSubmit, + children, +}: { + decision: Done['status']; + userCode: string; + csrfToken: string; + disabled: boolean; + onSubmit: () => void; + children: ReactNode; +}) { + return ( +
{ + if (disabled) { + event.preventDefault(); + + return; + } + + onSubmit(); + }} + > + + + + {children} +
+ ); +} + +function Finished({ status, device_name }: Done) { + const { user } = useUser(); + const approved = status === 'approved'; + + return ( + +

+ {approved + ? `${device_name} is signed in.` + : `${device_name} was denied.`} +

+

+ {approved + ? 'Your terminal signs in on its own in a few seconds. You can close this tab, and revoke the device any time under Settings → Devices.' + : 'Nothing was approved. Your terminal is told so on its next try. You can close this tab.'} +

+ + ); +} diff --git a/routes/web.php b/routes/web.php index 42cd4ddaec..5cf4fc1e64 100644 --- a/routes/web.php +++ b/routes/web.php @@ -47,6 +47,7 @@ use App\Http\Controllers\ConversationPingController; use App\Http\Controllers\ConversationReadController; use App\Http\Controllers\ConversationTypingController; +use App\Http\Controllers\DeviceActivationController; use App\Http\Controllers\DeviceController; use App\Http\Controllers\EmailController; use App\Http\Controllers\EmailDraftController; @@ -665,6 +666,12 @@ ->middleware('throttle:mobile-authorizations') ->name('mobile-authorization.store'); + // Device Activation... + Route::middleware('throttle:device-activations')->group(function (): void { + Route::get('activate', [DeviceActivationController::class, 'show'])->name('device-activation.show'); + Route::post('activate', [DeviceActivationController::class, 'store'])->name('device-activation.store'); + }); + // User... Route::delete('user', [UserController::class, 'destroy'])->name('user.destroy'); diff --git a/tests/Feature/Controllers/DeviceActivationControllerTest.php b/tests/Feature/Controllers/DeviceActivationControllerTest.php new file mode 100644 index 0000000000..25690316c5 --- /dev/null +++ b/tests/Feature/Controllers/DeviceActivationControllerTest.php @@ -0,0 +1,251 @@ +user = User::factory()->create(); + $this->fingerprint = 'SHA256:'.str_repeat('a', 43); + $this->authorization = pendingDeviceAuthorization($this->fingerprint, 'devnull on a laptop')['authorization']; +}); + +it('asks the signed-in user for the code their terminal shows', function (): void { + $response = $this->actingAs($this->user)->get(route('device-activation.show')); + + $response->assertOk() + ->assertInertia(fn (Assert $page): Assert => $page + ->component('device-activation/show') + ->missing('authorization') + ->missing('error') + ->missing('done') + ->where('auth.user.id', $this->user->id)); +}); + +it('asks the user to approve the device behind the code', function (): void { + $response = $this->actingAs($this->user)->get(route('device-activation.show', ['user_code' => $this->authorization->userCode])); + + $response->assertOk() + ->assertInertia(fn (Assert $page): Assert => $page + ->component('device-activation/show') + ->where('authorization', [ + 'user_code' => $this->authorization->userCode, + 'device_name' => 'devnull on a laptop', + 'fingerprint' => $this->fingerprint, + ]) + ->where('csrfToken', session()->token()) + ->missing('error') + ->where('auth.user.id', $this->user->id)); +}); + +it('reads the code however it was typed', function (Closure $typed): void { + $response = $this->actingAs($this->user)->get(route('device-activation.show', ['user_code' => $typed($this->authorization->userCode)])); + + $response->assertOk() + ->assertInertia(fn (Assert $page): Assert => $page->where('authorization.user_code', $this->authorization->userCode)); +})->with([ + 'in lowercase' => mb_strtolower(...), + 'without the dash' => fn (string $code): string => str_replace('-', '', $code), + 'with spaces' => fn (string $code): string => ' '.str_replace('-', ' ', $code).' ', +]); + +it('sends a guest through the github sign-in, then back to the code', function (): void { + Queue::fake(); + $this->mock(GitHubProvider::class)->shouldReceive('hasSponsorsListing')->andReturnFalse(); + + $this->get(route('device-activation.show', ['user_code' => $this->authorization->userCode]))->assertRedirectToRoute('home'); + + $user = User::factory()->create(['github_id' => '583231']); + + Socialite::fake('github', GitHubUser::fake([ + 'id' => '583231', + 'nickname' => $user->handle, + ])); + + $location = (string) $this->get(route('login.store'))->assertRedirect()->headers->get('Location'); + + expect($location)->toBe(route('device-activation.show', ['user_code' => $this->authorization->userCode])); + + $this->assertAuthenticatedAs($user); +}); + +it('refuses a code it cannot find, and asks for it again', function (Closure $request, string $error, ?string $userCode): void { + $response = $request($this); + + $response->assertBadRequest() + ->assertInertia(fn (Assert $page): Assert => $page + ->component('device-activation/show') + ->where('error', $error) + ->where('userCode', $userCode ?? $this->authorization->userCode) + ->missing('authorization') + ->missing('done')); +})->with([ + 'an unknown code' => [ + fn ($test): TestResponse => $test->actingAs($test->user)->get(route('device-activation.show', ['user_code' => 'bcdf-ghjk'])), + 'This code is unknown or has expired. Check your terminal, and start the sign-in again if you need to.', + 'BCDF-GHJK', + ], + 'a malformed code' => [ + fn ($test): TestResponse => $test->actingAs($test->user)->get(route('device-activation.show', ['user_code' => 'bcd'])), + 'A code is eight letters, like WDJB-MJHT.', + 'BCD', + ], + 'a code that is not text' => [ + fn ($test): TestResponse => $test->actingAs($test->user)->get(route('device-activation.show', ['user_code' => ['BCDF-GHJK']])), + 'The user code field must be a string.', + '', + ], + 'an expired code' => [ + function ($test): TestResponse { + $test->travel(DeviceAuthorization::EXPIRES_AFTER_MINUTES)->minutes(); + + return $test->actingAs($test->user)->get(route('device-activation.show', ['user_code' => $test->authorization->userCode])); + }, + 'This code is unknown or has expired. Check your terminal, and start the sign-in again if you need to.', + null, + ], + 'a code already approved' => [ + function ($test): TestResponse { + resolve(CreateDeviceActivation::class)->handle($test->user, $test->authorization, DeviceAuthorizationStatus::Approved); + + return $test->actingAs($test->user)->get(route('device-activation.show', ['user_code' => $test->authorization->userCode])); + }, + 'This code is unknown or has expired. Check your terminal, and start the sign-in again if you need to.', + null, + ], + 'a code already denied' => [ + function ($test): TestResponse { + resolve(CreateDeviceActivation::class)->handle($test->user, $test->authorization, DeviceAuthorizationStatus::Denied); + + return $test->actingAs($test->user)->get(route('device-activation.show', ['user_code' => $test->authorization->userCode])); + }, + 'This code is unknown or has expired. Check your terminal, and start the sign-in again if you need to.', + null, + ], +]); + +it('approves the device once the user says so, and tells them their terminal is signing in', function (): void { + $response = $this->actingAs($this->user)->post(route('device-activation.store'), ['user_code' => $this->authorization->userCode, 'decision' => 'approved']); + + $response->assertOk() + ->assertInertia(fn (Assert $page): Assert => $page + ->component('device-activation/show') + ->where('done', ['status' => 'approved', 'device_name' => 'devnull on a laptop']) + ->missing('authorization') + ->missing('error')); + + $device = Device::query()->sole(); + + expect($device->user_id)->toBe($this->user->id) + ->and($device->fingerprint)->toBe($this->fingerprint) + ->and($device->name)->toBe('devnull on a laptop') + ->and($device->approved_at->toDateTimeString())->toBe(now()->toDateTimeString()) + ->and(Cache::get($this->authorization->key))->toBe([...$this->authorization->toArray(), 'status' => 'approved', 'user_id' => $this->user->id]) + ->and(Cache::has(DeviceAuthorization::userCodeKeyFor($this->authorization->userCode)))->toBeFalse(); +}); + +it('denies the device when the user says so', function (): void { + $response = $this->actingAs($this->user)->post(route('device-activation.store'), ['user_code' => mb_strtolower($this->authorization->userCode), 'decision' => 'denied']); + + $response->assertOk() + ->assertInertia(fn (Assert $page): Assert => $page + ->component('device-activation/show') + ->where('done', ['status' => 'denied', 'device_name' => 'devnull on a laptop']) + ->missing('authorization')); + + expect(Device::query()->exists())->toBeFalse() + ->and(Cache::get($this->authorization->key))->toBe([...$this->authorization->toArray(), 'status' => 'denied', 'user_id' => null]) + ->and(Cache::has(DeviceAuthorization::userCodeKeyFor($this->authorization->userCode)))->toBeFalse(); +}); + +it('decides nothing for a code it cannot find, or without a decision', function (Closure $request, string $error): void { + $response = $request($this); + + $response->assertBadRequest() + ->assertInertia(fn (Assert $page): Assert => $page + ->component('device-activation/show') + ->where('error', $error) + ->missing('authorization') + ->missing('done')); + + expect(Device::query()->exists())->toBeFalse() + ->and(Cache::get($this->authorization->key))->toMatchArray(['status' => 'pending', 'user_id' => null]); +})->with([ + 'an unknown code' => [ + fn ($test): TestResponse => $test->actingAs($test->user)->post(route('device-activation.store'), ['user_code' => 'BCDF-GHJK', 'decision' => 'approved']), + 'This code is unknown or has expired. Check your terminal, and start the sign-in again if you need to.', + ], + 'a malformed code' => [ + fn ($test): TestResponse => $test->actingAs($test->user)->post(route('device-activation.store'), ['user_code' => 'bcd', 'decision' => 'approved']), + 'A code is eight letters, like WDJB-MJHT.', + ], + 'no code' => [ + fn ($test): TestResponse => $test->actingAs($test->user)->post(route('device-activation.store'), ['decision' => 'approved']), + 'The user code field is required.', + ], + 'a code that is not text' => [ + fn ($test): TestResponse => $test->actingAs($test->user)->post(route('device-activation.store'), ['user_code' => ['BCDF-GHJK'], 'decision' => 'approved']), + 'The user code field must be a string.', + ], + 'no decision' => [ + fn ($test): TestResponse => $test->actingAs($test->user)->post(route('device-activation.store'), ['user_code' => $test->authorization->userCode]), + 'The decision field is required.', + ], + 'a decision that is not one' => [ + fn ($test): TestResponse => $test->actingAs($test->user)->post(route('device-activation.store'), ['user_code' => $test->authorization->userCode, 'decision' => 'pending']), + 'The selected decision is invalid.', + ], +]); + +it('decides nothing for a code that expired while the page was open', function (): void { + $this->travel(DeviceAuthorization::EXPIRES_AFTER_MINUTES)->minutes(); + + $this->actingAs($this->user)->post(route('device-activation.store'), ['user_code' => $this->authorization->userCode, 'decision' => 'approved']) + ->assertBadRequest() + ->assertInertia(fn (Assert $page): Assert => $page->where('error', 'This code is unknown or has expired. Check your terminal, and start the sign-in again if you need to.')); + + expect(Device::query()->exists())->toBeFalse(); +}); + +it('decides a code only once', function (): void { + $this->actingAs($this->user)->post(route('device-activation.store'), ['user_code' => $this->authorization->userCode, 'decision' => 'denied'])->assertOk(); + + $this->actingAs($this->user)->post(route('device-activation.store'), ['user_code' => $this->authorization->userCode, 'decision' => 'approved']) + ->assertBadRequest() + ->assertInertia(fn (Assert $page): Assert => $page->where('error', 'This code is unknown or has expired. Check your terminal, and start the sign-in again if you need to.')); + + expect(Device::query()->exists())->toBeFalse() + ->and(Cache::get($this->authorization->key))->toMatchArray(['status' => 'denied']); +}); + +it('decides only for a signed-in user', function (): void { + $this->get(route('device-activation.show', ['user_code' => $this->authorization->userCode]))->assertRedirectToRoute('home'); + $this->post(route('device-activation.store'), ['user_code' => $this->authorization->userCode, 'decision' => 'approved'])->assertRedirectToRoute('home'); + + expect(Device::query()->exists())->toBeFalse() + ->and(Cache::get($this->authorization->key))->toMatchArray(['status' => 'pending']); +}); + +it('lets each user look a code up or decide 20 times a minute', function (): void { + foreach (range(1, 20) as $attempt) { + $this->actingAs($this->user)->get(route('device-activation.show', ['user_code' => 'BCDF-GHJK']))->assertBadRequest(); + } + + $this->actingAs($this->user)->get(route('device-activation.show', ['user_code' => $this->authorization->userCode]))->assertTooManyRequests(); + $this->actingAs($this->user)->post(route('device-activation.store'), ['user_code' => $this->authorization->userCode, 'decision' => 'approved'])->assertTooManyRequests(); + + $this->actingAs(User::factory()->create())->get(route('device-activation.show', ['user_code' => $this->authorization->userCode]))->assertOk(); + + expect(Device::query()->exists())->toBeFalse(); +}); diff --git a/tests/Pest.php b/tests/Pest.php index 8b24a19ddc..fde7a0450b 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -2,7 +2,9 @@ declare(strict_types=1); +use App\Actions\CreateDeviceAuthorization; use App\Contracts\Services\EmbeddingProvider; +use App\Dto\DeviceAuthorization; use App\Jobs\UnfurlLink; use App\Models\Device; use App\Models\Message; @@ -384,3 +386,14 @@ function pgvectorLacksIterativeScan(): bool { return version_compare((string) DB::scalar("select extversion from pg_extension where extname = 'vector'"), '0.8.0', '<'); } + +/** + * Starts a sign-in from a terminal for a key, as POST /api/v1/device-authorizations does, and returns what it hands + * out: the device code the terminal polls with, and the authorization behind it, with the code the user types. + * + * @return array{device_code: string, authorization: DeviceAuthorization} + */ +function pendingDeviceAuthorization(string $fingerprint = 'SHA256:key', string $deviceName = 'devnull on a laptop'): array +{ + return resolve(CreateDeviceAuthorization::class)->handle($fingerprint, $deviceName); +} diff --git a/tests/Unit/Actions/CreateDeviceActivationTest.php b/tests/Unit/Actions/CreateDeviceActivationTest.php new file mode 100644 index 0000000000..75fdc6402c --- /dev/null +++ b/tests/Unit/Actions/CreateDeviceActivationTest.php @@ -0,0 +1,68 @@ +create(); + $authorization = pendingDeviceAuthorization('SHA256:key', 'devnull on a laptop')['authorization']; + + resolve(CreateDeviceActivation::class)->handle($user, $authorization, DeviceAuthorizationStatus::Approved); + + $device = Device::query()->sole(); + + expect($device->user_id)->toBe($user->id) + ->and($device->fingerprint)->toBe('SHA256:key') + ->and($device->name)->toBe('devnull on a laptop') + ->and($device->approved_at->toDateTimeString())->toBe(now()->toDateTimeString()) + ->and($device->last_used_at)->toBeNull() + ->and(Cache::get($authorization->key))->toBe([...$authorization->toArray(), 'status' => 'approved', 'user_id' => $user->id]) + ->and(Cache::has(DeviceAuthorization::userCodeKeyFor($authorization->userCode)))->toBeFalse(); +}); + +it('keeps the decision only until the code would have expired', function (): void { + $user = User::factory()->create(); + $authorization = pendingDeviceAuthorization()['authorization']; + + $this->travel(14)->minutes(); + + resolve(CreateDeviceActivation::class)->handle($user, $authorization, DeviceAuthorizationStatus::Approved); + + $this->travel(59)->seconds(); + + expect(Cache::has($authorization->key))->toBeTrue(); + + $this->travel(1)->seconds(); + + expect(Cache::has($authorization->key))->toBeFalse(); +}); + +it('moves a key approved by someone else to the user who approved it now', function (): void { + $user = User::factory()->create(); + $previous = Device::factory()->create(['fingerprint' => 'SHA256:key', 'last_used_at' => now()->subDay(), 'approved_at' => now()->subWeek()]); + + resolve(CreateDeviceActivation::class)->handle($user, pendingDeviceAuthorization('SHA256:key', 'devnull on a desktop')['authorization'], DeviceAuthorizationStatus::Approved); + + expect($previous->refresh()->user_id)->toBe($user->id) + ->and($previous->name)->toBe('devnull on a desktop') + ->and($previous->approved_at->toDateTimeString())->toBe(now()->toDateTimeString()) + ->and($previous->last_used_at)->toBeNull() + ->and(Device::query()->count())->toBe(1); +}); + +it('denies without approving anything, and takes the code back', function (): void { + $user = User::factory()->create(); + $authorization = pendingDeviceAuthorization()['authorization']; + + resolve(CreateDeviceActivation::class)->handle($user, $authorization, DeviceAuthorizationStatus::Denied); + + expect(Device::query()->exists())->toBeFalse() + ->and(Cache::get($authorization->key))->toBe([...$authorization->toArray(), 'status' => 'denied', 'user_id' => null]) + ->and(Cache::has(DeviceAuthorization::userCodeKeyFor($authorization->userCode)))->toBeFalse(); +}); diff --git a/tests/Unit/Actions/CreateDeviceAuthorizationTest.php b/tests/Unit/Actions/CreateDeviceAuthorizationTest.php new file mode 100644 index 0000000000..67bfaad8f0 --- /dev/null +++ b/tests/Unit/Actions/CreateDeviceAuthorizationTest.php @@ -0,0 +1,65 @@ + $deviceCode, 'authorization' => $authorization] = resolve(CreateDeviceAuthorization::class)->handle('SHA256:key', 'devnull on a laptop'); + + expect($deviceCode)->toMatch('/^[0-9a-f]{64}$/') + ->and($authorization->key)->toBe(DeviceAuthorization::keyFor($deviceCode)) + ->and($authorization->fingerprint)->toBe('SHA256:key') + ->and($authorization->deviceName)->toBe('devnull on a laptop') + ->and($authorization->userCode)->toMatch('/^[BCDFGHJKLMNPQRSTVWXZ]{4}-[BCDFGHJKLMNPQRSTVWXZ]{4}$/') + ->and($authorization->status)->toBe(DeviceAuthorizationStatus::Pending) + ->and($authorization->userId)->toBeNull() + ->and($authorization->expiresAt->getTimestamp())->toBe(now()->addMinutes(15)->getTimestamp()) + ->and(Cache::get($authorization->key))->toBe([ + 'fingerprint' => 'SHA256:key', + 'device_name' => 'devnull on a laptop', + 'user_code' => $authorization->userCode, + 'expires_at' => now()->addMinutes(15)->getTimestamp(), + 'status' => 'pending', + 'user_id' => null, + ]) + ->and(Cache::get(DeviceAuthorization::userCodeKeyFor($authorization->userCode)))->toBe($authorization->key); + + $this->travel(DeviceAuthorization::EXPIRES_AFTER_MINUTES)->minutes(); + + expect(Cache::has($authorization->key))->toBeFalse() + ->and(Cache::has(DeviceAuthorization::userCodeKeyFor($authorization->userCode)))->toBeFalse(); +}); + +it('hands out different codes every time', function (): void { + $first = resolve(CreateDeviceAuthorization::class)->handle('SHA256:key', 'devnull on a laptop'); + $second = resolve(CreateDeviceAuthorization::class)->handle('SHA256:key', 'devnull on a laptop'); + + expect($first['device_code'])->not->toBe($second['device_code']) + ->and($first['authorization']->userCode)->not->toBe($second['authorization']->userCode) + ->and(Cache::has($first['authorization']->key))->toBeTrue() + ->and(Cache::has($second['authorization']->key))->toBeTrue(); +}); + +it('picks another user code when the first one is already out', function (): void { + $indexKeys = []; + + Cache::shouldReceive('add') + ->twice() + ->withArgs(function (string $key) use (&$indexKeys): bool { + $indexKeys[] = $key; + + return true; + }) + ->andReturn(false, true); + Cache::shouldReceive('put')->once()->andReturnTrue(); + + ['authorization' => $authorization] = resolve(CreateDeviceAuthorization::class)->handle('SHA256:key', 'devnull on a laptop'); + + expect($indexKeys)->toHaveCount(2) + ->and($indexKeys[0])->not->toBe($indexKeys[1]) + ->and($indexKeys[1])->toBe(DeviceAuthorization::userCodeKeyFor($authorization->userCode)); +}); diff --git a/tests/Unit/Dto/DeviceAuthorizationTest.php b/tests/Unit/Dto/DeviceAuthorizationTest.php new file mode 100644 index 0000000000..2524de0775 --- /dev/null +++ b/tests/Unit/Dto/DeviceAuthorizationTest.php @@ -0,0 +1,49 @@ +toBe('device-authorization:'.hash('sha256', 'the-device-code')) + ->not->toContain('the-device-code'); +}); + +it('keys a user code by its hash, never by the code itself', function (): void { + expect(DeviceAuthorization::userCodeKeyFor('BCDF-GHJK'))->toBe('device-authorization-user-code:'.hash('sha256', 'BCDF-GHJK')) + ->not->toContain('BCDF-GHJK'); +}); + +it('reads back what it stored, whether pending or decided', function (DeviceAuthorization $authorization): void { + expect(DeviceAuthorization::fromArray($authorization->key, $authorization->toArray()))->toEqual($authorization); +})->with([ + 'pending' => fn (): DeviceAuthorization => new DeviceAuthorization('device-authorization:key', 'SHA256:key', 'devnull on a laptop', 'BCDF-GHJK', CarbonImmutable::parse('2026-10-19 10:15:00')), + 'approved' => fn (): DeviceAuthorization => new DeviceAuthorization('device-authorization:key', 'SHA256:key', 'devnull on a laptop', 'BCDF-GHJK', CarbonImmutable::parse('2026-10-19 10:15:00'), DeviceAuthorizationStatus::Approved, 'user-id'), + 'denied' => fn (): DeviceAuthorization => new DeviceAuthorization('device-authorization:key', 'SHA256:key', 'devnull on a laptop', 'BCDF-GHJK', CarbonImmutable::parse('2026-10-19 10:15:00'), DeviceAuthorizationStatus::Denied), +]); + +it('is approved for the user who approved it', function (): void { + $user = User::factory()->create(); + $pending = new DeviceAuthorization('device-authorization:key', 'SHA256:key', 'devnull on a laptop', 'BCDF-GHJK', CarbonImmutable::parse('2026-10-19 10:15:00')); + + $approved = $pending->decided(DeviceAuthorizationStatus::Approved, $user); + + expect($approved->status)->toBe(DeviceAuthorizationStatus::Approved) + ->and($approved->userId)->toBe($user->id) + ->and($approved->key)->toBe($pending->key) + ->and($approved->expiresAt)->toEqual($pending->expiresAt) + ->and($pending->status)->toBe(DeviceAuthorizationStatus::Pending); +}); + +it('names nobody when it is denied', function (): void { + $user = User::factory()->create(); + $pending = new DeviceAuthorization('device-authorization:key', 'SHA256:key', 'devnull on a laptop', 'BCDF-GHJK', CarbonImmutable::parse('2026-10-19 10:15:00')); + + $denied = $pending->decided(DeviceAuthorizationStatus::Denied, $user); + + expect($denied->status)->toBe(DeviceAuthorizationStatus::Denied) + ->and($denied->userId)->toBeNull(); +}); diff --git a/tests/Unit/Providers/AppServiceProviderTest.php b/tests/Unit/Providers/AppServiceProviderTest.php index 5ec13c3909..663b448665 100644 --- a/tests/Unit/Providers/AppServiceProviderTest.php +++ b/tests/Unit/Providers/AppServiceProviderTest.php @@ -100,6 +100,24 @@ ->and($limit->decaySeconds)->toBe(60); }); +it('limits each user to 20 device code lookups and decisions a minute', function (): void { + $user = User::factory()->create(); + + $request = Request::create('/activate', 'POST'); + $request->setUserResolver(fn (): User => $user); + + $limiter = RateLimiter::limiter('device-activations'); + + assert($limiter instanceof Closure); + + $limit = $limiter($request); + + expect($limit)->toBeInstanceOf(Limit::class) + ->and($limit->key)->toBe($user->id) + ->and($limit->maxAttempts)->toBe(20) + ->and($limit->decaySeconds)->toBe(60); +}); + it('limits each user to 10 approvals of the devnull app a minute', function (): void { $user = User::factory()->create(); diff --git a/tests/Unit/Services/DeviceAuthorizationsTest.php b/tests/Unit/Services/DeviceAuthorizationsTest.php new file mode 100644 index 0000000000..9150c8a160 --- /dev/null +++ b/tests/Unit/Services/DeviceAuthorizationsTest.php @@ -0,0 +1,37 @@ +pendingFromUserCode($authorization->userCode))->toEqual($authorization); +}); + +it('finds nothing behind a code that was never handed out', function (): void { + expect(new DeviceAuthorizations()->pendingFromUserCode('BCDF-GHJK'))->toBeNull(); +}); + +it('finds nothing behind a code whose sign-in is gone', function (): void { + Cache::put(DeviceAuthorization::userCodeKeyFor('BCDF-GHJK'), DeviceAuthorization::keyFor('gone'), now()->addMinute()); + + expect(new DeviceAuthorizations()->pendingFromUserCode('BCDF-GHJK'))->toBeNull(); +}); + +it('finds nothing behind a code once it is decided', function (DeviceAuthorizationStatus $decision): void { + $authorization = pendingDeviceAuthorization()['authorization']; + + resolve(CreateDeviceActivation::class)->handle(User::factory()->create(), $authorization, $decision); + + expect(new DeviceAuthorizations()->pendingFromUserCode($authorization->userCode))->toBeNull(); +})->with([ + 'approved' => DeviceAuthorizationStatus::Approved, + 'denied' => DeviceAuthorizationStatus::Denied, +]); diff --git a/tests/Unit/Services/UserCodesTest.php b/tests/Unit/Services/UserCodesTest.php new file mode 100644 index 0000000000..d4140375ba --- /dev/null +++ b/tests/Unit/Services/UserCodesTest.php @@ -0,0 +1,36 @@ +generate())->toMatch('/^[BCDFGHJKLMNPQRSTVWXZ]{4}-[BCDFGHJKLMNPQRSTVWXZ]{4}$/'); +}); + +it('makes a different code every time', function (): void { + $codes = collect(range(1, 20))->map(fn (): string => new UserCodes()->generate()); + + expect($codes->unique())->toHaveCount(20); +}); + +it('reads a code however it was typed', function (string $typed): void { + expect(new UserCodes()->normalize($typed))->toBe('BCDF-GHJK'); +})->with([ + 'as shown' => 'BCDF-GHJK', + 'in lowercase' => 'bcdf-ghjk', + 'without the dash' => 'bcdfghjk', + 'with a space instead' => 'bcdf ghjk', + 'with spaces around' => ' BCDF-GHJK ', + 'with a tab' => "bcdf\tghjk", + 'with two dashes' => 'BCDF--GHJK', +]); + +it('leaves what is not a code for validation to refuse', function (string $typed, string $normalized): void { + expect(new UserCodes()->normalize($typed))->toBe($normalized); +})->with([ + 'too short' => ['bcd', 'BCD'], + 'too long' => ['bcdf-ghjk-lm', 'BCDFGHJKLM'], + 'with letters outside the alphabet' => ['aeio-uyaa', 'AEIO-UYAA'], + 'with digits' => ['1234-5678', '1234-5678'], +]); diff --git a/tests/Unit/UserCode.test.ts b/tests/Unit/UserCode.test.ts new file mode 100644 index 0000000000..78e2332482 --- /dev/null +++ b/tests/Unit/UserCode.test.ts @@ -0,0 +1,25 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { formatUserCode } from '../../resources/js/lib/user-code.ts'; + +void test('a code is shown in uppercase, with its dash after the fourth letter', () => { + assert.equal(formatUserCode('bcdfghjk'), 'BCDF-GHJK'); + assert.equal(formatUserCode('BCDF-GHJK'), 'BCDF-GHJK'); + assert.equal(formatUserCode('bcdf ghjk'), 'BCDF-GHJK'); +}); + +void test('what is not one of its letters is left out', () => { + assert.equal(formatUserCode('a1 bcdf-ghjk!'), 'BCDF-GHJK'); + assert.equal(formatUserCode('aeiouy'), ''); +}); + +void test('a code stops at eight letters', () => { + assert.equal(formatUserCode('bcdfghjklmnp'), 'BCDF-GHJK'); +}); + +void test('a code being typed keeps its shape', () => { + assert.equal(formatUserCode(''), ''); + assert.equal(formatUserCode('bcd'), 'BCD'); + assert.equal(formatUserCode('bcdf'), 'BCDF'); + assert.equal(formatUserCode('bcdfg'), 'BCDF-G'); +}); From 3bf7f9542715705c01c9e00ab2098365dae301df Mon Sep 17 00:00:00 2001 From: Julien BIANCHI Date: Mon, 19 Oct 2026 12:31:32 +0200 Subject: [PATCH 03/10] feat: hand out device authorizations to terminals POST /api/v1/device-authorizations starts the sign-in of RFC 8628 for the key that signs it, which isn't approved yet, with the name the terminal gives itself. It answers with the device code the terminal polls with, the user code to type at /activate, that URL with and without the code filled in, how long the sign-in lasts, and how often to poll. Each key may start five sign-ins a minute once its signature checks out, so no key fills the cache with codes; before that, the route shares the per-address ceiling of POST /api/v1/devices. --- .../Api/DeviceAuthorizationController.php | 32 +++++ .../CreateApiDeviceAuthorizationRequest.php | 62 +++++++++ routes/api.php | 7 ++ .../Api/DeviceAuthorizationControllerTest.php | 119 ++++++++++++++++++ 4 files changed, 220 insertions(+) create mode 100644 app/Http/Controllers/Api/DeviceAuthorizationController.php create mode 100644 app/Http/Requests/CreateApiDeviceAuthorizationRequest.php create mode 100644 tests/Feature/Controllers/Api/DeviceAuthorizationControllerTest.php diff --git a/app/Http/Controllers/Api/DeviceAuthorizationController.php b/app/Http/Controllers/Api/DeviceAuthorizationController.php new file mode 100644 index 0000000000..f6b6ab90c7 --- /dev/null +++ b/app/Http/Controllers/Api/DeviceAuthorizationController.php @@ -0,0 +1,32 @@ + $deviceCode, 'authorization' => $authorization] = $action->handle($request->fingerprint(), $request->string('device_name')->value()); + + return response()->json([ + 'device_code' => $deviceCode, + 'user_code' => $authorization->userCode, + 'verification_uri' => route('device-activation.show'), + 'verification_uri_complete' => route('device-activation.show', ['user_code' => $authorization->userCode]), + 'expires_in' => DeviceAuthorization::EXPIRES_AFTER_MINUTES * 60, + 'interval' => DeviceAuthorization::INTERVAL_SECONDS, + ], 201); + } +} diff --git a/app/Http/Requests/CreateApiDeviceAuthorizationRequest.php b/app/Http/Requests/CreateApiDeviceAuthorizationRequest.php new file mode 100644 index 0000000000..dfbb455bbd --- /dev/null +++ b/app/Http/Requests/CreateApiDeviceAuthorizationRequest.php @@ -0,0 +1,62 @@ +fingerprint = $signedRequests->verifySignature($this)->fingerprint(); + } catch (InvalidArgumentException $invalidArgumentException) { + throw new ApiException(ApiErrorCode::InvalidSignature, $invalidArgumentException->getMessage()); + } + + $key = "device-authorizations:{$this->fingerprint}"; + + throw_if(RateLimiter::tooManyAttempts($key, self::SignInsPerKeyPerMinute), fn (): ApiException => new ApiException(ApiErrorCode::RateLimited, __('Too Many Attempts.'), [ + 'Retry-After' => RateLimiter::availableIn($key), + ])); + + RateLimiter::hit($key); + + return true; + } + + /** + * @return array> + */ + public function rules(): array + { + return [ + 'device_name' => ['required', 'string', 'max:100'], + ]; + } + + public function fingerprint(): string + { + return $this->fingerprint; + } +} diff --git a/routes/api.php b/routes/api.php index ccdfa403d7..0bba71b991 100644 --- a/routes/api.php +++ b/routes/api.php @@ -28,6 +28,7 @@ use App\Http\Controllers\Api\ConversationReadController; use App\Http\Controllers\Api\ConversationTypingController; use App\Http\Controllers\Api\CurrentUserController; +use App\Http\Controllers\Api\DeviceAuthorizationController; use App\Http\Controllers\Api\DeviceController; use App\Http\Controllers\Api\IssueSuggestionController; use App\Http\Controllers\Api\LiveFollowingController; @@ -144,6 +145,12 @@ ->middleware('signed:relative') ->name('api-message-voice.show'); + // Signing in from a terminal: the key that isn't approved yet asks for a code the user types on the web... + Route::post('device-authorizations', [DeviceAuthorizationController::class, 'store']) + ->withoutMiddleware('throttle:api') + ->middleware('throttle:devices') + ->name('api-device-authorization.store'); + // Signed by the mobile app with an SSH key that the user approved as a device... Route::middleware(AuthenticateSignedRequest::class)->group(function (): void { Route::get('me', [CurrentUserController::class, 'show'])->name('api-current-user.show'); diff --git a/tests/Feature/Controllers/Api/DeviceAuthorizationControllerTest.php b/tests/Feature/Controllers/Api/DeviceAuthorizationControllerTest.php new file mode 100644 index 0000000000..3fa3eb30b2 --- /dev/null +++ b/tests/Feature/Controllers/Api/DeviceAuthorizationControllerTest.php @@ -0,0 +1,119 @@ +keyPair = SshKeyPair::generate(); + + $this->signInsStarted = 0; + + Event::listen(function (KeyWritten $event): void { + if (str_starts_with($event->key, 'device-authorization:')) { + $this->signInsStarted++; + } + }); +}); + +it('starts a sign-in for the key: a device code to poll with, and a user code to type on the web', function (): void { + $response = signedApiRequest($this->keyPair, 'POST', route('api-device-authorization.store'), ['device_name' => 'devnull on a laptop']); + + $response->assertCreated(); + + $deviceCode = $response->json('device_code'); + $userCode = $response->json('user_code'); + + expect($deviceCode)->toMatch('/^[0-9a-f]{64}$/') + ->and($userCode)->toMatch('/^[BCDFGHJKLMNPQRSTVWXZ]{4}-[BCDFGHJKLMNPQRSTVWXZ]{4}$/') + ->and($response->json())->toBe([ + 'device_code' => $deviceCode, + 'user_code' => $userCode, + 'verification_uri' => route('device-activation.show'), + 'verification_uri_complete' => route('device-activation.show', ['user_code' => $userCode]), + 'expires_in' => 900, + 'interval' => 5, + ]) + ->and(Cache::get(DeviceAuthorization::keyFor($deviceCode)))->toBe([ + 'fingerprint' => $this->keyPair->publicKey->fingerprint(), + 'device_name' => 'devnull on a laptop', + 'user_code' => $userCode, + 'expires_at' => now()->addMinutes(15)->getTimestamp(), + 'status' => 'pending', + 'user_id' => null, + ]) + ->and(Cache::get(DeviceAuthorization::userCodeKeyFor($userCode)))->toBe(DeviceAuthorization::keyFor($deviceCode)) + ->and($this->signInsStarted)->toBe(1); + + Http::assertNothingSent(); +}); + +it('never needs the key to be on github', function (): void { + expect(SshKey::query()->exists())->toBeFalse(); + + signedApiRequest($this->keyPair, 'POST', route('api-device-authorization.store'), ['device_name' => 'devnull on a laptop']) + ->assertCreated(); +}); + +it('validates the device name', function (array $data): void { + signedApiRequest($this->keyPair, 'POST', route('api-device-authorization.store'), $data) + ->assertUnprocessable() + ->assertJsonValidationErrors('device_name'); + + expect($this->signInsStarted)->toBe(0); +})->with([ + 'no name' => [['device_name' => '']], + 'a long name' => [['device_name' => str_repeat('a', 101)]], + 'a name that is not text' => [['device_name' => ['devnull on a laptop']]], +]); + +it('rejects a request that is not signed right', function (Closure $server, string $message): void { + $body = json_encode(['device_name' => 'devnull on a laptop'], JSON_THROW_ON_ERROR); + + $response = $this->call('POST', route('api-device-authorization.store'), server: $server($this->keyPair, $body), content: $body); + + $response->assertUnauthorized() + ->assertExactJson(['message' => $message, 'code' => 'invalid_signature']); + + expect($this->signInsStarted)->toBe(0); +})->with([ + 'no signature' => [fn (): array => ['CONTENT_TYPE' => 'application/json', 'HTTP_ACCEPT' => 'application/json'], 'Malformed SSH signature.'], + 'a signature over another body' => [fn (SshKeyPair $keyPair): array => signApiRequest($keyPair, 'POST', route('api-device-authorization.store'), '{}'), 'Invalid signature.'], + 'an expired signature' => [fn (SshKeyPair $keyPair, string $body): array => signApiRequest($keyPair, 'POST', route('api-device-authorization.store'), $body, timestamp: now()->getTimestamp() - 301), 'Signature expired. Check that your clock is right.'], +]); + +it('lets each key start 5 sign-ins a minute, wherever they come from', function (): void { + $body = '{"device_name":"devnull on a laptop"}'; + + foreach (range(1, 5) as $attempt) { + $server = [...signApiRequest($this->keyPair, 'POST', route('api-device-authorization.store'), $body), 'REMOTE_ADDR' => "203.0.113.{$attempt}"]; + + $this->call('POST', route('api-device-authorization.store'), server: $server, content: $body)->assertCreated(); + } + + signedApiRequest($this->keyPair, 'POST', route('api-device-authorization.store'), ['device_name' => 'devnull on a laptop']) + ->assertTooManyRequests() + ->assertHeader('Retry-After', 60) + ->assertExactJson(['message' => 'Too Many Attempts.', 'code' => 'rate_limited']); + + signedApiRequest(SshKeyPair::generate(), 'POST', route('api-device-authorization.store'), ['device_name' => 'devnull on a laptop']) + ->assertCreated(); + + expect($this->signInsStarted)->toBe(6); +}); + +it('limits each address to 60 tries a minute, before checking signatures', function (): void { + foreach (range(1, 60) as $attempt) { + $this->postJson(route('api-device-authorization.store'))->assertUnauthorized(); + } + + $this->postJson(route('api-device-authorization.store')) + ->assertTooManyRequests() + ->assertJsonPath('code', 'rate_limited'); +}); From fbc19d3b0d22bed798e45b0e1da17e21faf4a47c Mon Sep 17 00:00:00 2001 From: Julien BIANCHI Date: Mon, 19 Oct 2026 12:35:20 +0200 Subject: [PATCH 04/10] feat: redeem device authorizations for approved devices POST /api/v1/devices now also takes the device code a terminal was handed, in place of the code and verifier the mobile app sends, so the one endpoint turns either grant into a signed-in device, as the token endpoint of RFC 8628 does. Until the user decides on the web the terminal is told authorization_pending, or slow_down when it asks again sooner than the interval; a denial is access_denied; and once approved it gets the user, as the mobile app does, for as long as the sign-in lasts. A sign-in that is unknown, expired, started by another key, or whose device or user is gone since gets invalid_grant, like any bad code. Polls are paced by their sign-in rather than counted against the key's twelve tries a minute at redeeming a code, which a five-second interval would exhaust, and terminals behind one address get a ceiling of their own. --- app/Enums/ApiErrorCode.php | 5 +- app/Exceptions/ApiException.php | 18 ++ app/Http/Controllers/Api/DeviceController.php | 7 +- app/Http/Requests/CreateApiDeviceRequest.php | 82 +++++++-- app/Providers/AppServiceProvider.php | 7 +- app/Services/DeviceAuthorizations.php | 8 + .../Controllers/Api/DeviceControllerTest.php | 163 ++++++++++++++++++ .../Unit/Providers/AppServiceProviderTest.php | 15 ++ .../Services/DeviceAuthorizationsTest.php | 18 ++ 9 files changed, 304 insertions(+), 19 deletions(-) diff --git a/app/Enums/ApiErrorCode.php b/app/Enums/ApiErrorCode.php index dd604800e6..fc7e064396 100644 --- a/app/Enums/ApiErrorCode.php +++ b/app/Enums/ApiErrorCode.php @@ -13,6 +13,9 @@ enum ApiErrorCode: string case InvalidSignature = 'invalid_signature'; case DeviceNotApproved = 'device_not_approved'; case InvalidGrant = 'invalid_grant'; + case AuthorizationPending = 'authorization_pending'; + case SlowDown = 'slow_down'; + case AccessDenied = 'access_denied'; case UserBlocked = 'user_blocked'; case UserUnavailable = 'user_unavailable'; case OwnCommit = 'own_commit'; @@ -27,7 +30,7 @@ enum ApiErrorCode: string public function status(): int { return match ($this) { - self::InvalidGrant => 400, + self::InvalidGrant, self::AuthorizationPending, self::SlowDown, self::AccessDenied => 400, self::InvalidSignature, self::DeviceNotApproved => 401, self::UserBlocked, self::UserUnavailable, self::OwnCommit, self::TimedOut, self::Banned, self::CommitRulesRequired => 403, self::CommitUnavailable => 404, diff --git a/app/Exceptions/ApiException.php b/app/Exceptions/ApiException.php index 07b46997b0..1ee06591c8 100644 --- a/app/Exceptions/ApiException.php +++ b/app/Exceptions/ApiException.php @@ -79,6 +79,24 @@ public static function banned(): self return new self(ApiErrorCode::Banned, __('Your account is banned from /dev/null. All you can do is delete it.')); } + /** + * The answers a terminal polling for its sign-in gets until the user decides, as RFC 8628 names them. + */ + public static function authorizationPending(): self + { + return new self(ApiErrorCode::AuthorizationPending, __('Waiting for the device to be approved on the web.')); + } + + public static function slowDown(): self + { + return new self(ApiErrorCode::SlowDown, __('Asking too often. Wait a few seconds more between tries.')); + } + + public static function accessDenied(): self + { + return new self(ApiErrorCode::AccessDenied, __('The device was denied on the web. Start the sign-in again if that was a mistake.')); + } + public function render(): JsonResponse { return response()->json([ diff --git a/app/Http/Controllers/Api/DeviceController.php b/app/Http/Controllers/Api/DeviceController.php index 7cb5dfc588..9eb0c5a9d2 100644 --- a/app/Http/Controllers/Api/DeviceController.php +++ b/app/Http/Controllers/Api/DeviceController.php @@ -32,12 +32,13 @@ public function index(#[CurrentUser] User $user): AnonymousResourceCollection } /** - * Approves the key that signed the request as a device, for the user who approved the app on the web, and answers - * as GET /api/v1/me does. A code redeemed by another request in the meantime gets the same answer as a wrong one. + * Approves the key that signed the request as a device, for the user who approved the app on the web, or finds + * the device the user approved on the web for a terminal's sign-in, and answers as GET /api/v1/me does. A code + * redeemed by another request in the meantime gets the same answer as a wrong one. */ public function store(CreateApiDeviceRequest $request, CreateDevice $action, UpdateUserTermsAcceptance $acceptTerms, NotificationGroupsQuery $notifications): CurrentUserResource|JsonResponse { - $device = $action->handle($request->authorizationCode()); + $device = $request->device() ?? $action->handle($request->authorizationCode()); if (! $device instanceof Device) { return ApiException::invalidGrant()->render(); diff --git a/app/Http/Requests/CreateApiDeviceRequest.php b/app/Http/Requests/CreateApiDeviceRequest.php index f4480773a6..a2537ea52a 100644 --- a/app/Http/Requests/CreateApiDeviceRequest.php +++ b/app/Http/Requests/CreateApiDeviceRequest.php @@ -5,9 +5,13 @@ namespace App\Http\Requests; use App\Dto\AuthorizationCode; +use App\Dto\DeviceAuthorization; use App\Enums\ApiErrorCode; +use App\Enums\DeviceAuthorizationStatus; use App\Exceptions\ApiException; +use App\Models\Device; use App\Models\User; +use App\Services\DeviceAuthorizations; use App\Services\SignedRequests; use Illuminate\Foundation\Http\FormRequest; use Illuminate\Support\Facades\Cache; @@ -15,8 +19,9 @@ use InvalidArgumentException; /** - * The devnull app redeems the code the web handed it for a device, signed with the key it asked to approve. The key - * isn't approved yet, so only the signature itself is checked here, and the code has to be one that was handed out for + * The devnull app redeems the code the web handed it for a device, signed with the key it asked to approve; or a + * terminal polls, with the device code it was handed, for the sign-in the user decides on the web. The key isn't + * approved yet, so only the signature itself is checked here, and the code has to be one that was handed out for * that key. */ final class CreateApiDeviceRequest extends FormRequest @@ -27,9 +32,12 @@ final class CreateApiDeviceRequest extends FormRequest private ?AuthorizationCode $authorizationCode = null; + private ?Device $device = null; + /** * Once the signature checks out, each key may try a few times a minute. Phones behind one address would throttle - * each other if it went by address, which the route limits before any signature is checked. + * each other if it went by address, which the route limits before any signature is checked. A terminal polling + * for its sign-in is paced by that sign-in instead, since it asks every few seconds by design. */ public function authorize(SignedRequests $signedRequests): bool { @@ -39,29 +47,33 @@ public function authorize(SignedRequests $signedRequests): bool throw new ApiException(ApiErrorCode::InvalidSignature, $invalidArgumentException->getMessage()); } - $key = "devices:{$this->fingerprint}"; + if (! $this->filled('device_code')) { + $key = "devices:{$this->fingerprint}"; - throw_if(RateLimiter::tooManyAttempts($key, self::RedemptionsPerKeyPerMinute), fn (): ApiException => new ApiException(ApiErrorCode::RateLimited, __('Too Many Attempts.'), [ - 'Retry-After' => RateLimiter::availableIn($key), - ])); + throw_if(RateLimiter::tooManyAttempts($key, self::RedemptionsPerKeyPerMinute), fn (): ApiException => new ApiException(ApiErrorCode::RateLimited, __('Too Many Attempts.'), [ + 'Retry-After' => RateLimiter::availableIn($key), + ])); - RateLimiter::hit($key); + RateLimiter::hit($key); + } return true; } /** - * 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. + * Either the code from the web with the verifier the app made its PKCE challenge from (RFC 7636), or the device + * code a terminal was handed (RFC 8628), never both. 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> */ public function rules(): array { return [ - 'code' => ['required', 'string', 'max:255'], - 'code_verifier' => ['required', 'string', 'regex:/^[A-Za-z0-9._~-]{43,128}$/'], + 'code' => ['required_without:device_code', 'prohibits:device_code', 'string', 'max:255'], + 'code_verifier' => ['required_with:code', 'string', 'regex:/^[A-Za-z0-9._~-]{43,128}$/'], + 'device_code' => ['nullable', 'required_without:code', 'string', 'max:255'], 'terms_accepted' => ['sometimes', 'boolean'], ]; } @@ -73,12 +85,26 @@ public function authorizationCode(): AuthorizationCode return $this->authorizationCode; } + /** + * The device a terminal's sign-in approved, when that is what was asked for. + */ + public function device(): ?Device + { + return $this->device; + } + /** * Whatever is wrong with the code, it gets the same answer: one that is unknown, expired or used, handed out for * another key, whose challenge the verifier doesn't match, or whose user was deleted since. */ public function passedValidation(): void { + if ($this->filled('device_code')) { + $this->device = $this->approvedDevice($this->string('device_code')->value()); + + return; + } + $key = AuthorizationCode::keyFor($this->string('code')->value()); $stored = Cache::get($key); $code = is_array($stored) ? AuthorizationCode::fromArray($key, $stored) : null; @@ -93,4 +119,34 @@ public function passedValidation(): void $this->authorizationCode = $code; } + + /** + * The device the user approved on the web for a terminal's sign-in. Until they decide, the terminal is told to + * wait, and to slow down when it asks again sooner than the interval it was given. A sign-in that is unknown, + * expired or started by another key, or whose device or user is gone since, gets the one answer a bad code gets. + */ + private function approvedDevice(string $deviceCode): Device + { + $authorization = resolve(DeviceAuthorizations::class)->fromDeviceCode($deviceCode); + + throw_unless($authorization instanceof DeviceAuthorization && hash_equals($authorization->fingerprint, $this->fingerprint), ApiException::invalidGrant()); + + if ($authorization->status === DeviceAuthorizationStatus::Pending) { + throw Cache::add("{$authorization->key}:polled", true, DeviceAuthorization::INTERVAL_SECONDS - 1) + ? ApiException::authorizationPending() + : ApiException::slowDown(); + } + + throw_if($authorization->status === DeviceAuthorizationStatus::Denied, ApiException::accessDenied()); + + $device = Device::query() + ->with('user') + ->where('fingerprint', $authorization->fingerprint) + ->where('user_id', $authorization->userId) + ->first(); + + throw_unless($device instanceof Device, ApiException::invalidGrant()); + + return $device; + } } diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 57921db1b0..62a343fd6f 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -199,8 +199,11 @@ private function bootRateLimitingDefaults(): void }); // Redeeming a code for a device is limited per key once the signature checks out. Before that, each address gets - // a ceiling, so unsigned or badly signed requests can't force signature checks... - RateLimiter::for('devices', fn (Request $request): Limit => Limit::perMinute(60)->by((string) $request->ip())->response($rateLimited)); + // a ceiling, so unsigned or badly signed requests can't force signature checks. Terminals polling for their + // sign-in every few seconds share an address more often than phones do, so they get a ceiling of their own... + RateLimiter::for('devices', fn (Request $request): Limit => $request->filled('device_code') + ? Limit::perMinute(300)->by('device-polls:'.$request->ip())->response($rateLimited) + : Limit::perMinute(60)->by((string) $request->ip())->response($rateLimited)); // Approving the devnull app on the web hands out a code each time... RateLimiter::for('mobile-authorizations', fn (Request $request): Limit => Limit::perMinute(10)->by($this->userId($request))); diff --git a/app/Services/DeviceAuthorizations.php b/app/Services/DeviceAuthorizations.php index ed617ba3b7..ceb1f4f3b4 100644 --- a/app/Services/DeviceAuthorizations.php +++ b/app/Services/DeviceAuthorizations.php @@ -25,6 +25,14 @@ public function pendingFromUserCode(string $userCode): ?DeviceAuthorization return $authorization?->status === DeviceAuthorizationStatus::Pending ? $authorization : null; } + /** + * The sign-in a terminal polls for, by the device code it was handed, whatever the user decided so far. + */ + public function fromDeviceCode(string $deviceCode): ?DeviceAuthorization + { + return $this->fromKey(DeviceAuthorization::keyFor($deviceCode)); + } + private function fromKey(string $key): ?DeviceAuthorization { $stored = Cache::get($key); diff --git a/tests/Feature/Controllers/Api/DeviceControllerTest.php b/tests/Feature/Controllers/Api/DeviceControllerTest.php index acaa971980..02c175e2f6 100644 --- a/tests/Feature/Controllers/Api/DeviceControllerTest.php +++ b/tests/Feature/Controllers/Api/DeviceControllerTest.php @@ -4,8 +4,11 @@ use App\Actions\CreateAuthorizationCode; use App\Actions\CreateDevice; +use App\Actions\CreateDeviceActivation; use App\Dto\AuthorizationCode; +use App\Dto\DeviceAuthorization; use App\Dto\MobileAuthorization; +use App\Enums\DeviceAuthorizationStatus; use App\Models\Device; use App\Models\SshKey; use App\Models\User; @@ -13,6 +16,7 @@ use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Http; +use Illuminate\Support\Facades\RateLimiter; use Illuminate\Testing\TestResponse; use Tests\Support\SshKeyPair; @@ -285,3 +289,162 @@ 'the list' => ['GET', fn (Device $device): string => route('api-device.index')], 'revoking' => ['DELETE', fn (Device $device): string => route('api-device.destroy', $device)], ]); + +describe('polling a sign-in started from a terminal', function (): void { + beforeEach(function (): void { + ['device_code' => $this->deviceCode, 'authorization' => $this->authorization] = pendingDeviceAuthorization($this->keyPair->publicKey->fingerprint(), 'devnull on a laptop'); + }); + + it('says the user has not decided yet', function (): void { + signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['device_code' => $this->deviceCode]) + ->assertBadRequest() + ->assertExactJson([ + 'message' => 'Waiting for the device to be approved on the web.', + 'code' => 'authorization_pending', + ]); + + expect(Device::query()->exists())->toBeFalse(); + }); + + it('tells a terminal asking again sooner than the interval to slow down', function (): void { + signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['device_code' => $this->deviceCode]) + ->assertJsonPath('code', 'authorization_pending'); + + signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['device_code' => $this->deviceCode]) + ->assertBadRequest() + ->assertExactJson([ + 'message' => 'Asking too often. Wait a few seconds more between tries.', + 'code' => 'slow_down', + ]); + + $this->travel(3)->seconds(); + + signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['device_code' => $this->deviceCode]) + ->assertJsonPath('code', 'slow_down'); + + $this->travel(1)->seconds(); + + signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['device_code' => $this->deviceCode]) + ->assertJsonPath('code', 'authorization_pending'); + }); + + it('finds the device the user approved on the web, and answers like me', function (): void { + resolve(CreateDeviceActivation::class)->handle($this->user, $this->authorization, DeviceAuthorizationStatus::Approved); + + $response = signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['device_code' => $this->deviceCode]); + + $response->assertOk() + ->assertJsonPath('data.id', $this->user->id) + ->assertJsonPath('data.handle', 'octocat') + ->assertJsonPath('data.commits_count', 0) + ->assertJsonPath('unread_notifications_count', 0); + + $device = Device::query()->sole(); + + expect($device->user_id)->toBe($this->user->id) + ->and($device->fingerprint)->toBe($this->keyPair->publicKey->fingerprint()) + ->and($device->name)->toBe('devnull on a laptop'); + + signedApiRequest($this->keyPair, 'GET', route('api-current-user.show')) + ->assertOk() + ->assertJsonPath('data.id', $this->user->id); + + Http::assertNothingSent(); + }); + + it('answers the same again while the sign-in lasts', function (): void { + resolve(CreateDeviceActivation::class)->handle($this->user, $this->authorization, DeviceAuthorizationStatus::Approved); + + signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['device_code' => $this->deviceCode])->assertOk(); + + $this->travel(DeviceAuthorization::INTERVAL_SECONDS)->seconds(); + + signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['device_code' => $this->deviceCode]) + ->assertOk() + ->assertJsonPath('data.id', $this->user->id); + + expect(Device::query()->count())->toBe(1); + }); + + it('says the user denied the device', function (): void { + resolve(CreateDeviceActivation::class)->handle($this->user, $this->authorization, DeviceAuthorizationStatus::Denied); + + signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['device_code' => $this->deviceCode]) + ->assertBadRequest() + ->assertExactJson([ + 'message' => 'The device was denied on the web. Start the sign-in again if that was a mistake.', + 'code' => 'access_denied', + ]); + + expect(Device::query()->exists())->toBeFalse(); + }); + + it('refuses a device code it cannot redeem, the same way whatever is wrong with it', function (Closure $request): void { + $response = $request($this); + + $response->assertBadRequest() + ->assertExactJson([ + 'message' => 'This sign-in has expired or was already used. Sign in again in the app.', + 'code' => 'invalid_grant', + ]); + })->with([ + 'an unknown device code' => fn ($test): TestResponse => signedApiRequest($test->keyPair, 'POST', route('api-device.store'), ['device_code' => 'another-code']), + 'another key' => fn ($test): TestResponse => signedApiRequest(SshKeyPair::generate(), 'POST', route('api-device.store'), ['device_code' => $test->deviceCode]), + 'an expired sign-in' => function ($test): TestResponse { + $test->travel(DeviceAuthorization::EXPIRES_AFTER_MINUTES)->minutes(); + + return signedApiRequest($test->keyPair, 'POST', route('api-device.store'), ['device_code' => $test->deviceCode]); + }, + 'a device revoked since the user approved it' => function ($test): TestResponse { + resolve(CreateDeviceActivation::class)->handle($test->user, $test->authorization, DeviceAuthorizationStatus::Approved); + Device::query()->delete(); + + return signedApiRequest($test->keyPair, 'POST', route('api-device.store'), ['device_code' => $test->deviceCode]); + }, + 'a user deleted since they approved it' => function ($test): TestResponse { + resolve(CreateDeviceActivation::class)->handle($test->user, $test->authorization, DeviceAuthorizationStatus::Approved); + $test->user->delete(); + + return signedApiRequest($test->keyPair, 'POST', route('api-device.store'), ['device_code' => $test->deviceCode]); + }, + ]); + + it('refuses a sign-in started by another key, and leaves it for that key', function (): void { + signedApiRequest(SshKeyPair::generate(), 'POST', route('api-device.store'), ['device_code' => $this->deviceCode]) + ->assertJsonPath('code', 'invalid_grant'); + + resolve(CreateDeviceActivation::class)->handle($this->user, $this->authorization, DeviceAuthorizationStatus::Approved); + + signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['device_code' => $this->deviceCode]) + ->assertOk(); + }); + + it('validates the device code next to the code and its verifier', function (array $data, array $fields): void { + signedApiRequest($this->keyPair, 'POST', route('api-device.store'), $data) + ->assertUnprocessable() + ->assertJsonValidationErrors($fields); + })->with([ + 'neither a code nor a device code' => [['code_verifier' => str_repeat('a', 43)], ['code', 'device_code']], + 'a code and a device code' => [['code' => 'the-code', 'code_verifier' => str_repeat('a', 43), 'device_code' => 'the-device-code'], ['code']], + 'a device code that is not text' => [['device_code' => ['the-device-code']], ['device_code']], + ]); + + it('redeems a code the usual way when the device code is empty', function (): void { + signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['code' => $this->code, 'code_verifier' => $this->verifier, 'device_code' => '']) + ->assertOk() + ->assertJsonPath('data.id', $this->user->id); + + expect(RateLimiter::attempts("devices:{$this->keyPair->publicKey->fingerprint()}"))->toBe(1); + }); + + it("never counts polls against the key's tries at redeeming a code", function (): void { + foreach (range(1, 13) as $attempt) { + signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['device_code' => $this->deviceCode])->assertBadRequest(); + } + + signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['code' => $this->code, 'code_verifier' => $this->verifier]) + ->assertOk(); + + expect(RateLimiter::attempts("devices:{$this->keyPair->publicKey->fingerprint()}"))->toBe(1); + }); +}); diff --git a/tests/Unit/Providers/AppServiceProviderTest.php b/tests/Unit/Providers/AppServiceProviderTest.php index 663b448665..726ae33d11 100644 --- a/tests/Unit/Providers/AppServiceProviderTest.php +++ b/tests/Unit/Providers/AppServiceProviderTest.php @@ -100,6 +100,21 @@ ->and($limit->decaySeconds)->toBe(60); }); +it('limits each address to 300 polls of a terminal sign-in a minute, before their signatures are checked', function (): void { + $request = Request::create('/api/v1/devices', 'POST', server: ['REMOTE_ADDR' => '203.0.113.7', 'CONTENT_TYPE' => 'application/json'], content: '{"device_code":"the-device-code"}'); + + $limiter = RateLimiter::limiter('devices'); + + assert($limiter instanceof Closure); + + $limit = $limiter($request); + + expect($limit)->toBeInstanceOf(Limit::class) + ->and($limit->key)->toBe('device-polls:203.0.113.7') + ->and($limit->maxAttempts)->toBe(300) + ->and($limit->decaySeconds)->toBe(60); +}); + it('limits each user to 20 device code lookups and decisions a minute', function (): void { $user = User::factory()->create(); diff --git a/tests/Unit/Services/DeviceAuthorizationsTest.php b/tests/Unit/Services/DeviceAuthorizationsTest.php index 9150c8a160..84da52e956 100644 --- a/tests/Unit/Services/DeviceAuthorizationsTest.php +++ b/tests/Unit/Services/DeviceAuthorizationsTest.php @@ -25,6 +25,24 @@ expect(new DeviceAuthorizations()->pendingFromUserCode('BCDF-GHJK'))->toBeNull(); }); +it('finds the sign-in behind a device code, whatever was decided', function (?DeviceAuthorizationStatus $decision): void { + ['device_code' => $deviceCode, 'authorization' => $authorization] = pendingDeviceAuthorization(); + + if ($decision instanceof DeviceAuthorizationStatus) { + resolve(CreateDeviceActivation::class)->handle(User::factory()->create(), $authorization, $decision); + } + + expect(new DeviceAuthorizations()->fromDeviceCode($deviceCode)?->status)->toBe($decision ?? DeviceAuthorizationStatus::Pending); +})->with([ + 'pending' => null, + 'approved' => DeviceAuthorizationStatus::Approved, + 'denied' => DeviceAuthorizationStatus::Denied, +]); + +it('finds nothing behind a device code that was never handed out', function (): void { + expect(new DeviceAuthorizations()->fromDeviceCode('another-code'))->toBeNull(); +}); + it('finds nothing behind a code once it is decided', function (DeviceAuthorizationStatus $decision): void { $authorization = pendingDeviceAuthorization()['authorization']; From ab3c8001d8d30f634a0572630880f90224ac61ab Mon Sep 17 00:00:00 2001 From: Julien BIANCHI Date: Mon, 19 Oct 2026 12:37:32 +0200 Subject: [PATCH 05/10] docs: describe signing in from a terminal The knowledge base gets the terminal's side of the signed API: what the user does at /activate, how the terminal waits for the decision, and how long a code lasts, next to the mobile app's sign-in, so the two read as one feature with two doors and no token. The product guide counts terminals among the surfaces of the signed API. --- KNOWLEDGE_BASE.md | 4 +++- PRODUCT.md | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/KNOWLEDGE_BASE.md b/KNOWLEDGE_BASE.md index c79f78a31c..f4ef1aa003 100644 --- a/KNOWLEDGE_BASE.md +++ b/KNOWLEDGE_BASE.md @@ -561,6 +561,8 @@ Sources: [API routes](routes/api.php), [API controllers](app/Http/Controllers/Ap An app can use the same `/api/v1` for you with signed requests. It keeps an SSH key on the device and signs every request with it; there are no passwords or API tokens. To sign in, the app opens `/mobile/authorize` on the web. It sends the device's name with the key, and can also send its own name as `app_name`, like "/dev/null for Mac", so you can tell apps apart: the approval page and your devices show both. An app can also send a stable `app_id`, its platform and bundle identifier like `macos:sh.dev-null.mac`, which stays the same when its name changes. It isn't shown, but it is in your data export. A guest goes straight to GitHub and returns to that page; someone already signed in checks the key's fingerprint and approves it. The key then becomes one of your devices, and it does not need to be on your GitHub account. A key that is only on GitHub, such as a GitHub Actions key, cannot use the signed API. +A terminal or an agent, which has no browser to be sent back to, signs in with a short code instead: it asks the API for one and shows it, you open `/activate` on the web, sign in with GitHub if needed, type the code, check the device's name and the key's fingerprint, and approve or deny it. The terminal asks the API every few seconds whether you decided, and signs in on its own once you approve. A code lasts **15 minutes**, and you can look up or decide **20 codes a minute**. + A signed request sees what the web shows you, such as your own pending commits and your organizations'. It can also, with the web's validation, moderation, and rate limits: - read your account, logs, replies, notifications (including grouped activity), and starred commits; @@ -586,7 +588,7 @@ A signed request sees what the web shows you, such as your own pending commits a Signed request bodies are JSON, or multipart when they carry images or a voice note, so app commits can have images with descriptions or a poll but no link-card choice. Before your first app submission, accept the [commit rules](#moderation) through the web composer. Signed requests are limited to **120 requests per minute per user**. Settings → Devices lists your approved devices, with the app's name when it gave one and when each was last used, and revokes them. -Sources: [signed requests](app/Services/SignedRequests.php), [sign-in page](app/Http/Controllers/MobileAuthorizationController.php), [devices settings](resources/js/pages/device/index.tsx), [API routes](routes/api.php). +Sources: [signed requests](app/Services/SignedRequests.php), [sign-in page](app/Http/Controllers/MobileAuthorizationController.php), [terminal sign-in page](app/Http/Controllers/DeviceActivationController.php), [terminal sign-ins](app/Http/Controllers/Api/DeviceAuthorizationController.php), [devices settings](resources/js/pages/device/index.tsx), [API routes](routes/api.php). ### Mobile apps diff --git a/PRODUCT.md b/PRODUCT.md index 13dbe7437e..0603e6fa74 100644 --- a/PRODUCT.md +++ b/PRODUCT.md @@ -22,7 +22,7 @@ Developers who left X or Bluesky, or are thinking about it, and want a feed with ## Operating Context -- Surfaces: the web app (also installable as a PWA), the mobile app and the Mac app (the whole product as a native workspace, downloadable from /apps) through a signed JSON API, the terminal script and GitHub Action, a public read-only API, Atom feeds, and an activity graph SVG for GitHub READMEs. +- Surfaces: the web app (also installable as a PWA), the mobile app, the Mac app (the whole product as a native workspace, downloadable from /apps) and terminals through a signed JSON API, the terminal script and GitHub Action, a public read-only API, Atom feeds, and an activity graph SVG for GitHub READMEs. - Guests can read the Latest and Trending logs, profiles, approved commits and their threads, and search developers. Who follows whom and commit search (which pays for AI) need sign-in, so bots can't walk the graph or run up the bill, like everything that writes or is personal (Following log, notifications, Saved, Remotes, Settings). - Site admins (`ADMINS`) moderate (`/moderation`) and run the site (`/operations`). - Exact behavior, limits, and edge cases for every feature live in [KNOWLEDGE_BASE.md](KNOWLEDGE_BASE.md) and in the code; this file only records what must stay true. From b84365ee21967fd0e40db86ec2df3211e0ccdad5 Mon Sep 17 00:00:00 2001 From: Julien BIANCHI Date: Thu, 22 Oct 2026 12:42:52 +0200 Subject: [PATCH 06/10] feat: tell a terminal told to slow down which interval to use A terminal polling for its sign-in sooner than the interval only got slow_down, and had to work out on its own how long to wait from then on: an agent driving it could burn the rate limit guessing. The answer now carries the interval to use, the first one plus the five seconds RFC 8628 adds after a slow_down, and its message says it. --- app/Dto/DeviceAuthorization.php | 3 +++ app/Exceptions/ApiException.php | 11 +++++++++-- app/Http/Requests/CreateApiDeviceRequest.php | 2 +- .../Feature/Controllers/Api/DeviceControllerTest.php | 5 +++-- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/app/Dto/DeviceAuthorization.php b/app/Dto/DeviceAuthorization.php index 9f53f88f68..4b3e8cd86f 100644 --- a/app/Dto/DeviceAuthorization.php +++ b/app/Dto/DeviceAuthorization.php @@ -20,6 +20,9 @@ public const int INTERVAL_SECONDS = 5; + /** What RFC 8628 adds to the interval of a terminal told to slow down. */ + public const int SLOW_DOWN_SECONDS = 5; + public function __construct( public string $key, public string $fingerprint, diff --git a/app/Exceptions/ApiException.php b/app/Exceptions/ApiException.php index 1ee06591c8..5220c0c584 100644 --- a/app/Exceptions/ApiException.php +++ b/app/Exceptions/ApiException.php @@ -87,9 +87,16 @@ public static function authorizationPending(): self return new self(ApiErrorCode::AuthorizationPending, __('Waiting for the device to be approved on the web.')); } - public static function slowDown(): self + /** + * Says the interval to wait between tries from now on, so a terminal doesn't have to guess it. + */ + public static function slowDown(int $interval): self { - return new self(ApiErrorCode::SlowDown, __('Asking too often. Wait a few seconds more between tries.')); + return new self( + ApiErrorCode::SlowDown, + __('Asking too often. Wait :seconds seconds between tries from now on.', ['seconds' => $interval]), + data: ['interval' => $interval], + ); } public static function accessDenied(): self diff --git a/app/Http/Requests/CreateApiDeviceRequest.php b/app/Http/Requests/CreateApiDeviceRequest.php index a2537ea52a..6b3bad563c 100644 --- a/app/Http/Requests/CreateApiDeviceRequest.php +++ b/app/Http/Requests/CreateApiDeviceRequest.php @@ -134,7 +134,7 @@ private function approvedDevice(string $deviceCode): Device if ($authorization->status === DeviceAuthorizationStatus::Pending) { throw Cache::add("{$authorization->key}:polled", true, DeviceAuthorization::INTERVAL_SECONDS - 1) ? ApiException::authorizationPending() - : ApiException::slowDown(); + : ApiException::slowDown(DeviceAuthorization::INTERVAL_SECONDS + DeviceAuthorization::SLOW_DOWN_SECONDS); } throw_if($authorization->status === DeviceAuthorizationStatus::Denied, ApiException::accessDenied()); diff --git a/tests/Feature/Controllers/Api/DeviceControllerTest.php b/tests/Feature/Controllers/Api/DeviceControllerTest.php index 02c175e2f6..f31b58b0b6 100644 --- a/tests/Feature/Controllers/Api/DeviceControllerTest.php +++ b/tests/Feature/Controllers/Api/DeviceControllerTest.php @@ -306,15 +306,16 @@ expect(Device::query()->exists())->toBeFalse(); }); - it('tells a terminal asking again sooner than the interval to slow down', function (): void { + it('tells a terminal asking again sooner than the interval to slow down, and the interval to use from then on', function (): void { signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['device_code' => $this->deviceCode]) ->assertJsonPath('code', 'authorization_pending'); signedApiRequest($this->keyPair, 'POST', route('api-device.store'), ['device_code' => $this->deviceCode]) ->assertBadRequest() ->assertExactJson([ - 'message' => 'Asking too often. Wait a few seconds more between tries.', + 'message' => 'Asking too often. Wait 10 seconds between tries from now on.', 'code' => 'slow_down', + 'interval' => 10, ]); $this->travel(3)->seconds(); From 9260af2a6a91e0678dfaa7124909a85a57d73151 Mon Sep 17 00:00:00 2001 From: Julien BIANCHI Date: Wed, 4 Nov 2026 00:17:25 +0100 Subject: [PATCH 07/10] fix: decide a terminal's sign-in only once, even from two requests at once The page checked that the code was still waiting before the decision was written, with nothing in between, so an approval and a denial sent together could both pass: the key could end up a device while the terminal was told access_denied. The first decision now claims the sign-in, as redeeming a mobile code already does, and the other request gets the answer an unknown code gets. --- app/Actions/CreateDeviceActivation.php | 11 ++++++++- .../DeviceActivationController.php | 6 +++-- .../DeviceActivationControllerTest.php | 21 ++++++++++++++++ .../Actions/CreateDeviceActivationTest.php | 24 ++++++++++++++++++- 4 files changed, 58 insertions(+), 4 deletions(-) diff --git a/app/Actions/CreateDeviceActivation.php b/app/Actions/CreateDeviceActivation.php index 22a0743ab2..1d72fb771a 100644 --- a/app/Actions/CreateDeviceActivation.php +++ b/app/Actions/CreateDeviceActivation.php @@ -17,9 +17,16 @@ * a code does for the mobile app, since the key proved itself when it started the sign-in and the user approved * that very fingerprint. Either way, the decision is kept for the terminal to find until the sign-in would have * expired, and the user code is taken back, so a code is decided once. + * + * Of two requests deciding the same sign-in at once, only the first decides it: the other changes nothing and gets + * false, so a denied key is never approved behind the terminal's back. */ - public function handle(User $user, DeviceAuthorization $authorization, DeviceAuthorizationStatus $status): void + public function handle(User $user, DeviceAuthorization $authorization, DeviceAuthorizationStatus $status): bool { + if (! Cache::add("{$authorization->key}:decided", true, $authorization->expiresAt)) { + return false; + } + if ($status === DeviceAuthorizationStatus::Approved) { Device::query()->updateOrCreate(['fingerprint' => $authorization->fingerprint], [ 'user_id' => $user->id, @@ -31,5 +38,7 @@ public function handle(User $user, DeviceAuthorization $authorization, DeviceAut Cache::put($authorization->key, $authorization->decided($status, $user)->toArray(), $authorization->expiresAt); Cache::forget(DeviceAuthorization::userCodeKeyFor($authorization->userCode)); + + return true; } } diff --git a/app/Http/Controllers/DeviceActivationController.php b/app/Http/Controllers/DeviceActivationController.php index 7dbd8fc903..1dccd90976 100644 --- a/app/Http/Controllers/DeviceActivationController.php +++ b/app/Http/Controllers/DeviceActivationController.php @@ -10,6 +10,7 @@ use App\Http\Requests\ShowDeviceActivationRequest; use App\Models\User; use Illuminate\Container\Attributes\CurrentUser; +use Illuminate\Http\Exceptions\HttpResponseException; use Inertia\Inertia; use Inertia\Response; @@ -35,14 +36,15 @@ public function show(ShowDeviceActivationRequest $request): Response } /** - * Approves the key as one of the user's devices, or denies it, and tells the user what their terminal will find. + * Approves the key as one of the user's devices, or denies it, and tells the user what their terminal will find. A + * code decided by another request in the meantime gets the same answer as an unknown one. */ public function store(CreateDeviceActivationRequest $request, #[CurrentUser] User $user, CreateDeviceActivation $action): Response { $authorization = $request->authorization(); $decision = $request->decision(); - $action->handle($user, $authorization, $decision); + throw_unless($action->handle($user, $authorization, $decision), fn (): HttpResponseException => ShowDeviceActivationRequest::invalid($request, ShowDeviceActivationRequest::unknownCode())); return Inertia::render('device-activation/show', [ 'done' => [ diff --git a/tests/Feature/Controllers/DeviceActivationControllerTest.php b/tests/Feature/Controllers/DeviceActivationControllerTest.php index 25690316c5..ac218d8146 100644 --- a/tests/Feature/Controllers/DeviceActivationControllerTest.php +++ b/tests/Feature/Controllers/DeviceActivationControllerTest.php @@ -8,7 +8,9 @@ use App\Enums\DeviceAuthorizationStatus; use App\Models\Device; use App\Models\User; +use Illuminate\Cache\Events\CacheHit; use Illuminate\Support\Facades\Cache; +use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Queue; use Illuminate\Testing\TestResponse; use Inertia\Testing\AssertableInertia as Assert; @@ -229,6 +231,25 @@ function ($test): TestResponse { ->and(Cache::get($this->authorization->key))->toMatchArray(['status' => 'denied']); }); +it('decides nothing for a code that another request decided in the meantime', function (): void { + // The code is checked, then denied by another request before this one approves its device... + $decided = false; + + Event::listen(function (CacheHit $event) use (&$decided): void { + if (! $decided && $event->key === $this->authorization->key) { + $decided = true; + resolve(CreateDeviceActivation::class)->handle($this->user, $this->authorization, DeviceAuthorizationStatus::Denied); + } + }); + + $this->actingAs($this->user)->post(route('device-activation.store'), ['user_code' => $this->authorization->userCode, 'decision' => 'approved']) + ->assertBadRequest() + ->assertInertia(fn (Assert $page): Assert => $page->where('error', 'This code is unknown or has expired. Check your terminal, and start the sign-in again if you need to.')); + + expect(Device::query()->exists())->toBeFalse() + ->and(Cache::get($this->authorization->key))->toMatchArray(['status' => 'denied']); +}); + it('decides only for a signed-in user', function (): void { $this->get(route('device-activation.show', ['user_code' => $this->authorization->userCode]))->assertRedirectToRoute('home'); $this->post(route('device-activation.store'), ['user_code' => $this->authorization->userCode, 'decision' => 'approved'])->assertRedirectToRoute('home'); diff --git a/tests/Unit/Actions/CreateDeviceActivationTest.php b/tests/Unit/Actions/CreateDeviceActivationTest.php index 75fdc6402c..2ff2621bff 100644 --- a/tests/Unit/Actions/CreateDeviceActivationTest.php +++ b/tests/Unit/Actions/CreateDeviceActivationTest.php @@ -60,9 +60,31 @@ $user = User::factory()->create(); $authorization = pendingDeviceAuthorization()['authorization']; - resolve(CreateDeviceActivation::class)->handle($user, $authorization, DeviceAuthorizationStatus::Denied); + expect(resolve(CreateDeviceActivation::class)->handle($user, $authorization, DeviceAuthorizationStatus::Denied))->toBeTrue(); expect(Device::query()->exists())->toBeFalse() ->and(Cache::get($authorization->key))->toBe([...$authorization->toArray(), 'status' => 'denied', 'user_id' => null]) ->and(Cache::has(DeviceAuthorization::userCodeKeyFor($authorization->userCode)))->toBeFalse(); }); + +it('approves nothing for a sign-in that was denied since it was checked', function (): void { + $user = User::factory()->create(); + $authorization = pendingDeviceAuthorization()['authorization']; + + resolve(CreateDeviceActivation::class)->handle($user, $authorization, DeviceAuthorizationStatus::Denied); + + expect(resolve(CreateDeviceActivation::class)->handle($user, $authorization, DeviceAuthorizationStatus::Approved))->toBeFalse() + ->and(Device::query()->exists())->toBeFalse() + ->and(Cache::get($authorization->key)['status'])->toBe('denied'); +}); + +it('keeps the approval of a sign-in that is denied since it was checked', function (): void { + $user = User::factory()->create(); + $authorization = pendingDeviceAuthorization()['authorization']; + + resolve(CreateDeviceActivation::class)->handle($user, $authorization, DeviceAuthorizationStatus::Approved); + + expect(resolve(CreateDeviceActivation::class)->handle($user, $authorization, DeviceAuthorizationStatus::Denied))->toBeFalse() + ->and(Device::query()->count())->toBe(1) + ->and(Cache::get($authorization->key)['status'])->toBe('approved'); +}); From d5a01d182deeccdd317f25b1c1ec55e2af43bdfc Mon Sep 17 00:00:00 2001 From: Julien BIANCHI Date: Wed, 4 Nov 2026 00:18:59 +0100 Subject: [PATCH 08/10] fix: count a terminal's sign-in attempt and check its limit in one step The limit on sign-ins per key was checked, then counted in a second step, so requests from one key sent at once could all pass before any was counted, and start more than 5 sign-ins a minute. The count the rate limiter returns when it records the attempt is now what gets checked. --- app/Http/Requests/CreateApiDeviceAuthorizationRequest.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/Http/Requests/CreateApiDeviceAuthorizationRequest.php b/app/Http/Requests/CreateApiDeviceAuthorizationRequest.php index dfbb455bbd..70008f4942 100644 --- a/app/Http/Requests/CreateApiDeviceAuthorizationRequest.php +++ b/app/Http/Requests/CreateApiDeviceAuthorizationRequest.php @@ -24,7 +24,8 @@ final class CreateApiDeviceAuthorizationRequest extends FormRequest /** * Once the signature checks out, each key may start a few sign-ins a minute, so no key fills the cache with codes. * Terminals behind one address would throttle each other if it went by address, which the route limits before - * any signature is checked. + * any signature is checked. The attempt is counted and checked in one step, so requests sent at once can't all + * slip under the limit. */ public function authorize(SignedRequests $signedRequests): bool { @@ -36,12 +37,10 @@ public function authorize(SignedRequests $signedRequests): bool $key = "device-authorizations:{$this->fingerprint}"; - throw_if(RateLimiter::tooManyAttempts($key, self::SignInsPerKeyPerMinute), fn (): ApiException => new ApiException(ApiErrorCode::RateLimited, __('Too Many Attempts.'), [ + throw_if(RateLimiter::hit($key) > self::SignInsPerKeyPerMinute, fn (): ApiException => new ApiException(ApiErrorCode::RateLimited, __('Too Many Attempts.'), [ 'Retry-After' => RateLimiter::availableIn($key), ])); - RateLimiter::hit($key); - return true; } From a98edd2a35c251a78c7a6850d89523d6e4e78208 Mon Sep 17 00:00:00 2001 From: Julien BIANCHI Date: Wed, 4 Nov 2026 00:34:50 +0100 Subject: [PATCH 09/10] fix: give a terminal's sign-in back when its device could not be saved Deciding a sign-in claims it first, so two requests can't both decide it. If saving the approved device then failed, the claim stayed until the sign-in expired: the user got an error and could not approve it again, and the terminal could never finish signing in. The claim is now released when the device isn't saved, so the user can try again. --- app/Actions/CreateDeviceActivation.php | 26 +++++++++++++------ .../Actions/CreateDeviceActivationTest.php | 16 ++++++++++++ 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/app/Actions/CreateDeviceActivation.php b/app/Actions/CreateDeviceActivation.php index 1d72fb771a..70268e5519 100644 --- a/app/Actions/CreateDeviceActivation.php +++ b/app/Actions/CreateDeviceActivation.php @@ -9,6 +9,7 @@ use App\Models\Device; use App\Models\User; use Illuminate\Support\Facades\Cache; +use Throwable; final readonly class CreateDeviceActivation { @@ -19,21 +20,30 @@ * expired, and the user code is taken back, so a code is decided once. * * Of two requests deciding the same sign-in at once, only the first decides it: the other changes nothing and gets - * false, so a denied key is never approved behind the terminal's back. + * false, so a denied key is never approved behind the terminal's back. An approval whose device couldn't be saved + * gives the sign-in back, so the user can try again. */ public function handle(User $user, DeviceAuthorization $authorization, DeviceAuthorizationStatus $status): bool { - if (! Cache::add("{$authorization->key}:decided", true, $authorization->expiresAt)) { + $claim = "{$authorization->key}:decided"; + + if (! Cache::add($claim, true, $authorization->expiresAt)) { return false; } if ($status === DeviceAuthorizationStatus::Approved) { - Device::query()->updateOrCreate(['fingerprint' => $authorization->fingerprint], [ - 'user_id' => $user->id, - 'name' => $authorization->deviceName, - 'approved_at' => now(), - 'last_used_at' => null, - ]); + try { + Device::query()->updateOrCreate(['fingerprint' => $authorization->fingerprint], [ + 'user_id' => $user->id, + 'name' => $authorization->deviceName, + 'approved_at' => now(), + 'last_used_at' => null, + ]); + } catch (Throwable $throwable) { + Cache::forget($claim); + + throw $throwable; + } } Cache::put($authorization->key, $authorization->decided($status, $user)->toArray(), $authorization->expiresAt); diff --git a/tests/Unit/Actions/CreateDeviceActivationTest.php b/tests/Unit/Actions/CreateDeviceActivationTest.php index 2ff2621bff..938b0cf295 100644 --- a/tests/Unit/Actions/CreateDeviceActivationTest.php +++ b/tests/Unit/Actions/CreateDeviceActivationTest.php @@ -78,6 +78,22 @@ ->and(Cache::get($authorization->key)['status'])->toBe('denied'); }); +it('lets the sign-in be decided again when deciding it failed', function (): void { + $user = User::factory()->create(); + $authorization = pendingDeviceAuthorization()['authorization']; + + Device::creating(fn (): never => throw new RuntimeException('The database went away.')); + + expect(fn (): bool => resolve(CreateDeviceActivation::class)->handle($user, $authorization, DeviceAuthorizationStatus::Approved)) + ->toThrow(RuntimeException::class, 'The database went away.'); + + Device::flushEventListeners(); + + expect(resolve(CreateDeviceActivation::class)->handle($user, $authorization, DeviceAuthorizationStatus::Approved))->toBeTrue() + ->and(Device::query()->count())->toBe(1) + ->and(Cache::get($authorization->key)['status'])->toBe('approved'); +}); + it('keeps the approval of a sign-in that is denied since it was checked', function (): void { $user = User::factory()->create(); $authorization = pendingDeviceAuthorization()['authorization']; From c6950f1fc0dcfee7dc7c842e0cff760c46c61da2 Mon Sep 17 00:00:00 2001 From: Julien BIANCHI Date: Thu, 5 Nov 2026 16:23:08 +0100 Subject: [PATCH 10/10] fix: give starting a terminal sign-in a rate limit bucket of its own Starting a sign-in shared the devices limiter, which picks polling's 300 a minute per address whenever the body has a device code, before the request is checked. The sign-in request doesn't reject that extra field, so adding one moved it to the larger bucket, and rotating keys got around the 5 per key: one address could start up to 300 sign-ins a minute instead of 60. It now has its own limiter, with the same 60 a minute per address whatever the body says. --- app/Providers/AppServiceProvider.php | 4 ++++ routes/api.php | 2 +- .../Api/DeviceAuthorizationControllerTest.php | 10 ++++++++++ tests/Unit/Providers/AppServiceProviderTest.php | 15 +++++++++++++++ 4 files changed, 30 insertions(+), 1 deletion(-) diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 62a343fd6f..3803407469 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -205,6 +205,10 @@ private function bootRateLimitingDefaults(): void ? Limit::perMinute(300)->by('device-polls:'.$request->ip())->response($rateLimited) : Limit::perMinute(60)->by((string) $request->ip())->response($rateLimited)); + // Starting a sign-in from a terminal gets the same ceiling per address, in a bucket of its own: polling's larger + // one is chosen from the body before anything is checked, so it must not be reachable from here... + RateLimiter::for('device-authorizations', fn (Request $request): Limit => Limit::perMinute(60)->by((string) $request->ip())->response($rateLimited)); + // Approving the devnull app on the web hands out a code each time... RateLimiter::for('mobile-authorizations', fn (Request $request): Limit => Limit::perMinute(10)->by($this->userId($request))); diff --git a/routes/api.php b/routes/api.php index 0bba71b991..791ad478f6 100644 --- a/routes/api.php +++ b/routes/api.php @@ -148,7 +148,7 @@ // Signing in from a terminal: the key that isn't approved yet asks for a code the user types on the web... Route::post('device-authorizations', [DeviceAuthorizationController::class, 'store']) ->withoutMiddleware('throttle:api') - ->middleware('throttle:devices') + ->middleware('throttle:device-authorizations') ->name('api-device-authorization.store'); // Signed by the mobile app with an SSH key that the user approved as a device... diff --git a/tests/Feature/Controllers/Api/DeviceAuthorizationControllerTest.php b/tests/Feature/Controllers/Api/DeviceAuthorizationControllerTest.php index 3fa3eb30b2..05066e53ff 100644 --- a/tests/Feature/Controllers/Api/DeviceAuthorizationControllerTest.php +++ b/tests/Feature/Controllers/Api/DeviceAuthorizationControllerTest.php @@ -117,3 +117,13 @@ ->assertTooManyRequests() ->assertJsonPath('code', 'rate_limited'); }); + +it('keeps each address to 60 tries a minute when a device code is slipped in, which only polling gets more for', function (): void { + foreach (range(1, 60) as $attempt) { + $this->postJson(route('api-device-authorization.store'), ['device_code' => 'any-device-code'])->assertUnauthorized(); + } + + $this->postJson(route('api-device-authorization.store'), ['device_code' => 'any-device-code']) + ->assertTooManyRequests() + ->assertJsonPath('code', 'rate_limited'); +}); diff --git a/tests/Unit/Providers/AppServiceProviderTest.php b/tests/Unit/Providers/AppServiceProviderTest.php index 726ae33d11..0811833c7c 100644 --- a/tests/Unit/Providers/AppServiceProviderTest.php +++ b/tests/Unit/Providers/AppServiceProviderTest.php @@ -115,6 +115,21 @@ ->and($limit->decaySeconds)->toBe(60); }); +it('limits each address to 60 terminal sign-ins started a minute, before their signatures are checked, whatever the body says', function (): void { + $request = Request::create('/api/v1/device-authorizations', 'POST', server: ['REMOTE_ADDR' => '203.0.113.7', 'CONTENT_TYPE' => 'application/json'], content: '{"device_name":"devnull on a laptop","device_code":"the-device-code"}'); + + $limiter = RateLimiter::limiter('device-authorizations'); + + assert($limiter instanceof Closure); + + $limit = $limiter($request); + + expect($limit)->toBeInstanceOf(Limit::class) + ->and($limit->key)->toBe('203.0.113.7') + ->and($limit->maxAttempts)->toBe(60) + ->and($limit->decaySeconds)->toBe(60); +}); + it('limits each user to 20 device code lookups and decisions a minute', function (): void { $user = User::factory()->create();