fix(auth): say "a device" on the confirm screen, and move the warning under it

The confirm screen named the client asking to sign in. That name is
chosen by whoever started the request, so it let a stranger's request
introduce itself as anything at all; and to the person reading, it is
just a device. It now says "A device is asking to sign in", and a test
checks that a client's self-chosen name never appears.

The warning about choosing Deny moves under the two buttons, as small
print in the same style as the terms line on the email screen, so the
question and its two answers read first.
This commit is contained in:
Wanjohi
2026-10-02 05:50:55 +03:00
parent 5353b9a1db
commit 11f62c69aa
5 changed files with 18 additions and 8 deletions
-1
View File
@@ -116,7 +116,6 @@ const STATES: State[] = [
label: 'Is this you?',
screen: () =>
IssuerScreens.deviceConfirm({
clientID: 'nestri-cli',
userCode: 'BCDF3467',
csrf: 'x'
})
+7 -6
View File
@@ -16,7 +16,6 @@ import type { Screen } from './screen.js';
/** The confirmation a device grant asks for, before it is approved. */
export interface DeviceConfirmCopy {
clientID: string;
userCode: string;
csrf: string;
}
@@ -95,16 +94,18 @@ export const IssuerScreens = {
kind: 'confirm',
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: [
`${confirmation.clientID} is asking to sign in to your account. The code above should match the one it is showing you.`,
'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.'
'A device is asking to sign in to your account. The code above should match the one it is showing you.'
],
action: '/device/confirm',
// The client id is escaped by the renderer like any other text. It
// is chosen by whoever started the grant, so it is never markup.
fields: [{ kind: 'hidden', name: 'csrf', value: confirmation.csrf }],
approve: { label: 'Approve', name: 'action', value: 'approve' },
deny: { label: 'Deny', name: 'action', value: 'deny' }
deny: { label: 'Deny', name: 'action', value: 'deny' },
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.'
};
},
+1
View File
@@ -179,6 +179,7 @@ function Confirm(props: { theme?: Theme; screen: ConfirmScreen }) {
{screen.deny.label}
</button>
</form>
{screen.footer && <Footer copy={screen.footer} />}
</Layout>
);
}
+2
View File
@@ -167,6 +167,8 @@ export interface ConfirmScreen {
fields?: Field[];
approve: Action;
deny: Action;
/** The small print under the two answers. */
footer?: Copy;
status?: number;
}
+8 -1
View File
@@ -313,7 +313,14 @@ describe('a code somebody else started', () => {
expect(reached.html).toContain(grant.user_code.slice(0, 4));
expect(reached.html).toContain(grant.user_code.slice(4));
expect(reached.html).toContain('desktop');
});
test('the page does not name the client, which named itself', async () => {
const grant = await started('Your bank');
const reached = await signInAndReachConfirmation(grant.user_code);
expect(reached.html).not.toContain('Your bank');
expect(reached.html).toContain('A device is asking');
});
test('a confirmation posted without the value from the cookie is refused', async () => {