mirror of
https://github.com/nestriness/nestri.git
synced 2026-11-03 22:22:39 +02:00
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<boolean>;
|
||||
/**
|
||||
* 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) {
|
||||
|
||||
@@ -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.'
|
||||
};
|
||||
},
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user