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. diff --git a/app/Actions/CreateDeviceActivation.php b/app/Actions/CreateDeviceActivation.php new file mode 100644 index 0000000000..70268e5519 --- /dev/null +++ b/app/Actions/CreateDeviceActivation.php @@ -0,0 +1,54 @@ +key}:decided"; + + if (! Cache::add($claim, true, $authorization->expiresAt)) { + return false; + } + + if ($status === DeviceAuthorizationStatus::Approved) { + 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); + Cache::forget(DeviceAuthorization::userCodeKeyFor($authorization->userCode)); + + return true; + } +} 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..4b3e8cd86f --- /dev/null +++ b/app/Dto/DeviceAuthorization.php @@ -0,0 +1,103 @@ + $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/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/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 @@ + $interval]), + data: ['interval' => $interval], + ); + } + + 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/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/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/Controllers/DeviceActivationController.php b/app/Http/Controllers/DeviceActivationController.php new file mode 100644 index 0000000000..1dccd90976 --- /dev/null +++ b/app/Http/Controllers/DeviceActivationController.php @@ -0,0 +1,56 @@ +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. 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(); + + throw_unless($action->handle($user, $authorization, $decision), fn (): HttpResponseException => ShowDeviceActivationRequest::invalid($request, ShowDeviceActivationRequest::unknownCode())); + + return Inertia::render('device-activation/show', [ + 'done' => [ + 'status' => $decision->value, + 'device_name' => $authorization->deviceName, + ], + ]); + } +} diff --git a/app/Http/Requests/CreateApiDeviceAuthorizationRequest.php b/app/Http/Requests/CreateApiDeviceAuthorizationRequest.php new file mode 100644 index 0000000000..70008f4942 --- /dev/null +++ b/app/Http/Requests/CreateApiDeviceAuthorizationRequest.php @@ -0,0 +1,61 @@ +fingerprint = $signedRequests->verifySignature($this)->fingerprint(); + } catch (InvalidArgumentException $invalidArgumentException) { + throw new ApiException(ApiErrorCode::InvalidSignature, $invalidArgumentException->getMessage()); + } + + $key = "device-authorizations:{$this->fingerprint}"; + + throw_if(RateLimiter::hit($key) > self::SignInsPerKeyPerMinute, fn (): ApiException => new ApiException(ApiErrorCode::RateLimited, __('Too Many Attempts.'), [ + 'Retry-After' => RateLimiter::availableIn($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/app/Http/Requests/CreateApiDeviceRequest.php b/app/Http/Requests/CreateApiDeviceRequest.php index f4480773a6..6b3bad563c 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(DeviceAuthorization::INTERVAL_SECONDS + DeviceAuthorization::SLOW_DOWN_SECONDS); + } + + 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/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..3803407469 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -199,12 +199,22 @@ 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)); + + // 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))); + // 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..ceb1f4f3b4 --- /dev/null +++ b/app/Services/DeviceAuthorizations.php @@ -0,0 +1,42 @@ +fromKey($key) : null; + + 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); + + 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/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. + +
  3. +
    + +
    +
    +

    + {command} +

    + {children} +
    +
  4. +
+
+ + ); +} 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/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. - -
  3. -
    - -
    -
    -

    - {command} -

    - {children} -
    -
  4. -
-
- + ); } diff --git a/routes/api.php b/routes/api.php index ccdfa403d7..791ad478f6 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:device-authorizations') + ->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/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/Api/DeviceAuthorizationControllerTest.php b/tests/Feature/Controllers/Api/DeviceAuthorizationControllerTest.php new file mode 100644 index 0000000000..05066e53ff --- /dev/null +++ b/tests/Feature/Controllers/Api/DeviceAuthorizationControllerTest.php @@ -0,0 +1,129 @@ +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'); +}); + +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/Feature/Controllers/Api/DeviceControllerTest.php b/tests/Feature/Controllers/Api/DeviceControllerTest.php index acaa971980..f31b58b0b6 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,163 @@ '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, 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 10 seconds between tries from now on.', + 'code' => 'slow_down', + 'interval' => 10, + ]); + + $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/Feature/Controllers/DeviceActivationControllerTest.php b/tests/Feature/Controllers/DeviceActivationControllerTest.php new file mode 100644 index 0000000000..ac218d8146 --- /dev/null +++ b/tests/Feature/Controllers/DeviceActivationControllerTest.php @@ -0,0 +1,272 @@ +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 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'); + + 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..938b0cf295 --- /dev/null +++ b/tests/Unit/Actions/CreateDeviceActivationTest.php @@ -0,0 +1,106 @@ +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']; + + 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('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']; + + 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'); +}); 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..0811833c7c 100644 --- a/tests/Unit/Providers/AppServiceProviderTest.php +++ b/tests/Unit/Providers/AppServiceProviderTest.php @@ -100,6 +100,54 @@ ->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 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(); + + $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..84da52e956 --- /dev/null +++ b/tests/Unit/Services/DeviceAuthorizationsTest.php @@ -0,0 +1,55 @@ +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 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']; + + 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'); +});