From 8cfec951c6c33ae5ec32fc67ecc9064aa61f3bd5 Mon Sep 17 00:00:00 2001 From: Wanjohi Date: Fri, 16 Oct 2026 00:55:24 +0300 Subject: [PATCH] feat(auth): approving the installer says it adds a host The device confirmation page asks "Is this you?" whatever is asking, which is right for signing an app in and undersells approving a computer as a host on your account. The issuer gains `deviceConfirm`, wording looked up by client id. It is chosen by the issuer and never built from the request, so a grant still cannot introduce itself in words of its own. The installer's page reads "Add a computer as a Nestri host?" over the code to compare with the terminal; every other client keeps the generic page. The preview gains the new state. --- apps/auth/WIP-installer.md | 1 + apps/auth/script/preview.ts | 12 ++++++++++++ apps/auth/src/index.ts | 22 ++++++++++++++++++++++ apps/auth/test/device.test.ts | 19 ++++++++++++++++++- packages/auth/src/issuer.ts | 16 ++++++++++++++-- packages/auth/src/ui/issuer.ts | 18 ++++++++++++++++-- packages/auth/test/device.test.ts | 14 ++++++++++++++ 7 files changed, 97 insertions(+), 5 deletions(-) diff --git a/apps/auth/WIP-installer.md b/apps/auth/WIP-installer.md index f26e5187..1893bb03 100644 --- a/apps/auth/WIP-installer.md +++ b/apps/auth/WIP-installer.md @@ -5,6 +5,7 @@ Branch: feat/installer-sign-in ## Done - 1. installer device client, 15 min, no refresh (engine `clientTTL`, apps/auth `clientTTL`) - 2. per-address limit on POST /device/authorize (engine `deviceVerification.requestLimit`), 429 slow_down + Retry-After +- 3. confirm wording per client (engine `deviceConfirm`, apps/auth `deviceConfirm`), preview entry ## Left 1. issuer: `installer` device client, 15 min access, no refresh token diff --git a/apps/auth/script/preview.ts b/apps/auth/script/preview.ts index 491e9f63..fb6ffad7 100644 --- a/apps/auth/script/preview.ts +++ b/apps/auth/script/preview.ts @@ -23,6 +23,7 @@ import { MARK_CODE, MARK_GITHUB, MARK_GOOGLE } from '@nestri/auth/ui/mark'; import { HtmlRenderer } from '@nestri/auth/ui/render'; import type { Screen } from '@nestri/auth/ui/screen'; +import { deviceConfirm } from '../src/index.js'; import { THEME_NESTRI } from '../src/theme.js'; const PORT = Number(process.env.PORT ?? 1338); @@ -120,6 +121,17 @@ const STATES: State[] = [ csrf: 'x' }) }, + { + id: 'device-confirm-installer', + group: 'Device', + label: 'Add a host?', + screen: () => + IssuerScreens.deviceConfirm({ + userCode: 'BCDF3467', + csrf: 'x', + wording: deviceConfirm('installer') + }) + }, { id: 'device-approved', group: 'Device', diff --git a/apps/auth/src/index.ts b/apps/auth/src/index.ts index ee1b722b..837f530a 100644 --- a/apps/auth/src/index.ts +++ b/apps/auth/src/index.ts @@ -49,6 +49,27 @@ const DEVICE_CLIENTS = new Set(['desktop', 'installer']); export const allowDeviceClient = async (clientID: string) => DEVICE_CLIENTS.has(clientID); +/** + * What the person approving a device client is asked, where it is not the + * generic "is this you?". + * + * Approving the installer adds a computer to the person's account as a host, + * which is a bigger thing to agree to than signing an app in, so the page says + * that in as many words. Fixed per client id: nothing the request carries is + * shown. + */ +export const deviceConfirm = (clientID: string) => + clientID === 'installer' + ? { + heading: 'Add a computer as a Nestri host?', + body: [ + 'A computer is asking to become a host on your account. The code above should match the one in its terminal.' + ], + footer: + 'If it does not, or you did not run the installer yourself, choose Deny. Approving lets that computer add itself as one host and nothing else.' + } + : undefined; + /** * How long a session for each device client lasts, where it is not the * default. @@ -210,6 +231,7 @@ export default { deviceStore: PostgresDeviceStore(), allowDeviceClient, clientTTL, + deviceConfirm, // The default rule allows a redirect back to whatever hostname the // request arrived on, which is right for a site served beside this // one and refuses the proxy in front of user-owned hosts — it diff --git a/apps/auth/test/device.test.ts b/apps/auth/test/device.test.ts index 51985f0d..e3122bb3 100644 --- a/apps/auth/test/device.test.ts +++ b/apps/auth/test/device.test.ts @@ -4,7 +4,7 @@ import { issuer } from '@nestri/auth/index'; import { MemoryStorage } from '@nestri/auth/storage/memory'; import { subjects } from '@nestri/core/auth/subjects'; -import { allowDeviceClient, clientTTL } from '../src/index.js'; +import { allowDeviceClient, clientTTL, deviceConfirm } from '../src/index.js'; /** * The issuer the worker builds, with the database taken out and the real rules @@ -20,6 +20,7 @@ const auth = issuer({ allow: async () => true, allowDeviceClient, clientTTL, + deviceConfirm, providers: { stub: { type: 'stub', @@ -122,6 +123,22 @@ describe('device clients', () => { expect(tokens).not.toHaveProperty('refresh_token'); }); + test('approving the installer is asked as adding a host, with the code to compare', async () => { + const { started, html } = await reachConfirmation('installer'); + expect(html).toContain('Add a computer as a Nestri host?'); + expect(html).toContain('the one in its terminal'); + expect(html).toContain(started.user_code.slice(0, 4)); + expect(html).toContain(started.user_code.slice(4)); + expect(html).toContain('Approve'); + expect(html).toContain('Deny'); + }); + + test('the desktop app is asked the generic question', async () => { + const { html } = await reachConfirmation('desktop'); + expect(html).toContain('Is this you?'); + expect(html).not.toContain('Nestri host'); + }); + test('the desktop app keeps a session it can refresh', async () => { const tokens = await approved('desktop'); expect(tokens.expires_in).toBe(60 * 60 * 24 * 30); diff --git a/packages/auth/src/issuer.ts b/packages/auth/src/issuer.ts index 90821c68..553031d1 100644 --- a/packages/auth/src/issuer.ts +++ b/packages/auth/src/issuer.ts @@ -231,7 +231,7 @@ import { DynamoStorage } from './storage/dynamo.js'; import { MemoryStorage } from './storage/memory.js'; import { Storage, StorageAdapter } from './storage/storage.js'; import { HtmlRenderer, type Renderer } from './ui/render.js'; -import { IssuerScreens } from './ui/issuer.js'; +import { type DeviceConfirmWording, IssuerScreens } from './ui/issuer.js'; import type { ChooseOption, Screen } from './ui/screen.js'; import type { Theme } from './ui/theme.js'; import { getRelativeUrl, isDomainMatch, lazy } from './util.js'; @@ -492,6 +492,15 @@ export interface IssuerInput< * issuer that has not thought about it, and is worth thinking about. */ allowDeviceClient?(clientID: string, req: Request): Promise; + /** + * What the confirmation page says to a person approving `clientID`, where + * the generic wording would undersell what they are agreeing to. + * + * Looked up by the client id and never built from it, so a request cannot + * put words of its own in front of the person deciding on it. Undefined + * keeps the generic page. + */ + deviceConfirm?(clientID: string): DeviceConfirmWording | undefined; /** * Which providers appear on the screen offering a choice of them, and in * what order. @@ -965,7 +974,10 @@ export function issuer< } function deviceConfirmScreen(confirmation: DeviceConfirmation): Screen { - return IssuerScreens.deviceConfirm(confirmation); + return IssuerScreens.deviceConfirm({ + ...confirmation, + wording: input.deviceConfirm?.(confirmation.clientID) + }); } async function getAuthorization(ctx: Context) { diff --git a/packages/auth/src/ui/issuer.ts b/packages/auth/src/ui/issuer.ts index 5cbe7e09..3d384df3 100644 --- a/packages/auth/src/ui/issuer.ts +++ b/packages/auth/src/ui/issuer.ts @@ -14,10 +14,23 @@ import type { Screen } from './screen.js'; +/** + * What the confirmation says, for a client that warrants its own words. + * + * Chosen by the issuer from the client id, never taken from the request: the + * id is a claim, and the words a person decides on must not be the claimant's. + */ +export interface DeviceConfirmWording { + heading: string; + body: string[]; + footer?: string; +} + /** The confirmation a device grant asks for, before it is approved. */ export interface DeviceConfirmCopy { userCode: string; csrf: string; + wording?: DeviceConfirmWording; } export const IssuerScreens = { @@ -93,12 +106,12 @@ export const IssuerScreens = { deviceConfirm(confirmation: DeviceConfirmCopy): Screen { return { kind: 'confirm', - heading: 'Is this you?', + heading: confirmation.wording?.heading ?? 'Is this you?', verify: { code: confirmation.userCode, group: 4 }, // Not the client id. It is chosen by whoever started the grant, so // naming it would let a stranger's request introduce itself as // anything it likes — and to the person reading, it is a device. - body: [ + body: confirmation.wording?.body ?? [ 'A device is asking to sign in to your account. The code above should match the one it is showing you.' ], action: '/device/confirm', @@ -106,6 +119,7 @@ export const IssuerScreens = { approve: { label: 'Approve', name: 'action', value: 'approve' }, deny: { label: 'Deny', name: 'action', value: 'deny' }, footer: + confirmation.wording?.footer ?? 'If it does not, or you did not start this on a device of your own, choose Deny. Nobody can sign in as you unless you approve here.' }; }, diff --git a/packages/auth/test/device.test.ts b/packages/auth/test/device.test.ts index f84bd402..3f2598b7 100644 --- a/packages/auth/test/device.test.ts +++ b/packages/auth/test/device.test.ts @@ -23,6 +23,10 @@ const auth = issuer({ allow: async () => true, allowDeviceClient: async (clientID) => clientID !== 'banned', clientTTL: (clientID) => (clientID === 'one-step' ? { access: 15 * 60, refresh: 0 } : undefined), + deviceConfirm: (clientID) => + clientID === 'one-step' + ? { heading: 'Let this one step through?', body: ['Words chosen for this client.'] } + : undefined, // Every test here asks for codes from one address at one frozen instant, // so the request limit is widened out of the way; it has its own issuer // below, on the defaults. @@ -375,6 +379,16 @@ describe('a code somebody else started', () => { expect(reached.html).toContain('A device is asking'); }); + test('a client the issuer has words for is asked about in those words', async () => { + const grant = await started('one-step'); + const reached = await signInAndReachConfirmation(grant.user_code); + + expect(reached.html).toContain('Let this one step through?'); + expect(reached.html).toContain('Words chosen for this client.'); + expect(reached.html).not.toContain('Is this you?'); + expect(reached.html).toContain(grant.user_code.slice(0, 4)); + }); + test('a confirmation posted without the value from the cookie is refused', async () => { const grant = await started(); const { cookies } = await signInAndReachConfirmation(grant.user_code);