Skip to content

Commit 14dd1b4

Browse files
aspiersclaude
aspiers
and
claude
committed
feat(auth): sign epds_handle_mode through the callback hop
The chooser and consent screens need to know whether the account's handle was user-chosen or server-generated, so they can decide which identifier to show as primary. That mode was resolved on the way in but lost on the auth-service -> pds-core hop, so the approval step could fall back to showing a generated random handle. Carry epds_handle_mode across the hop, and sign it rather than appending it afterwards: the parameter decides what the approval screen shows, so leaving it unsigned would let the browser flip the presentation of a flow it does not own. Invalid values are dropped at the boundary instead of being forwarded. Adding a field to the signed payload changes the HMAC input, so auth-service and pds-core must be deployed together. In-flight callbacks signed by the old auth-service will fail verification on the new pds-core, and the user retries the sign-in. Also extracts resolveCompleteIdentity() from the /auth/complete handler. Unrelated to handle mode, but the recovery-email lookup was already nested three levels deep in a function this change had to touch anyway. Split out of #148. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 707355b commit 14dd1b4

9 files changed

Lines changed: 444 additions & 38 deletions

File tree

‎.changeset/handle-mode-through-approval.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'ePDS': patch
3+
---
4+
5+
Apps can now control how accounts are labelled on the approval and account-chooser screens, including for pushed authorization requests.
6+
7+
**Affects:** Client app developers
8+
9+
**Client app developers:** `epds_handle_mode` is now resolved from either the `/oauth/authorize` query parameter or your OAuth client metadata, so it applies to pushed authorization requests where the browser URL carries only `request_uri`. An explicit query parameter still takes precedence over client metadata, and metadata lookup failures fall back to the previous default behaviour.
10+
11+
A valid mode stored for an auth flow is preserved across `/oauth/epds-callback`; values that are not one of the recognised modes are ignored rather than forwarded.

‎packages/auth-service/src/__tests__/callback-handle-mode.test.ts‎

Lines changed: 239 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,239 @@
1+
import { randomBytes } from 'node:crypto'
2+
import type { AddressInfo } from 'node:net'
3+
import express from 'express'
4+
import cookieParser from 'cookie-parser'
5+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
6+
import {
7+
verifyCallback,
8+
type CallbackParams,
9+
type HandleMode,
10+
} from '@certified-app/shared'
11+
import type { AuthServiceContext } from '../context.js'
12+
import { createChooseHandleRouter } from '../routes/choose-handle.js'
13+
import { createCompleteRouter } from '../routes/complete.js'
14+
15+
const mocks = vi.hoisted(() => ({
16+
getDidByEmail: vi.fn(),
17+
pingParRequest: vi.fn(),
18+
resolveRecoveryEmail: vi.fn(),
19+
resolveClientBranding: vi.fn(),
20+
}))
21+
22+
vi.mock('../lib/get-did-by-email.js', () => ({
23+
getDidByEmail: mocks.getDidByEmail,
24+
}))
25+
26+
vi.mock('../lib/ping-par-request.js', () => ({
27+
pingParRequest: mocks.pingParRequest,
28+
}))
29+
30+
vi.mock('../lib/resolve-recovery-email.js', () => ({
31+
resolveRecoveryEmail: mocks.resolveRecoveryEmail,
32+
}))
33+
34+
vi.mock('../lib/client-metadata.js', () => ({
35+
resolveClientBranding: mocks.resolveClientBranding,
36+
}))
37+
38+
const AUTH_FLOW_COOKIE = 'epds_auth_flow'
39+
const realFetch = globalThis.fetch.bind(globalThis)
40+
41+
function makeCtx(handleMode: HandleMode | null): AuthServiceContext {
42+
return {
43+
config: {
44+
pdsPublicUrl: 'https://pds.example',
45+
pdsHostname: 'pds.example',
46+
epdsCallbackSecret: 'test-callback-secret',
47+
trustedClients: [],
48+
},
49+
db: {
50+
getAuthFlow: vi.fn(() => ({
51+
flowId: 'flow-1',
52+
requestUri: 'urn:ietf:params:oauth:request_uri:req-123',
53+
clientId: 'https://app.example/client.json',
54+
handleMode,
55+
})),
56+
deleteAuthFlow: vi.fn(),
57+
},
58+
} as unknown as AuthServiceContext
59+
}
60+
61+
function makeAuth() {
62+
return {
63+
api: {
64+
getSession: vi.fn(() =>
65+
Promise.resolve({ user: { email: 'Alice@example.com' } }),
66+
),
67+
},
68+
}
69+
}
70+
71+
async function startApp(
72+
ctx: AuthServiceContext,
73+
auth: ReturnType<typeof makeAuth>,
74+
): Promise<{ baseUrl: string; close: () => Promise<void> }> {
75+
const app = express()
76+
app.disable('x-powered-by')
77+
app.use(cookieParser())
78+
app.use(express.urlencoded({ extended: false }))
79+
app.use(createCompleteRouter(ctx, auth))
80+
app.use(createChooseHandleRouter(ctx, auth))
81+
82+
const server = app.listen(0)
83+
await new Promise<void>((resolve, reject) => {
84+
server.once('error', reject)
85+
server.once('listening', () => {
86+
resolve()
87+
})
88+
})
89+
server.unref()
90+
const port = (server.address() as AddressInfo).port
91+
return {
92+
baseUrl: `http://127.0.0.1:${port}`,
93+
close: () =>
94+
new Promise<void>((resolve) => {
95+
server.close(() => {
96+
resolve()
97+
})
98+
}),
99+
}
100+
}
101+
102+
function parseRedirect(res: globalThis.Response): URL {
103+
const location = res.headers.get('location')
104+
if (!location) throw new Error('Missing redirect location')
105+
return new URL(location)
106+
}
107+
108+
function normalizeFetchUrl(input: Parameters<typeof fetch>[0]): URL {
109+
if (input instanceof URL) return input
110+
if (typeof input === 'string') return new URL(input)
111+
return new URL(input.url)
112+
}
113+
114+
function verifySignedCallbackUrl(url: URL): boolean {
115+
const callbackParams: CallbackParams = {
116+
request_uri: url.searchParams.get('request_uri') ?? '',
117+
email: url.searchParams.get('email') ?? '',
118+
approved: url.searchParams.get('approved') ?? '',
119+
new_account: url.searchParams.get('new_account') ?? '',
120+
...(url.searchParams.has('handle')
121+
? { handle: url.searchParams.get('handle') ?? '' }
122+
: {}),
123+
// /auth/complete signs client_id too (so a dead-PAR clean exit can still
124+
// reach the right client); omitting it here would fail verification.
125+
...(url.searchParams.has('client_id')
126+
? { client_id: url.searchParams.get('client_id') ?? '' }
127+
: {}),
128+
...(url.searchParams.has('epds_handle_mode')
129+
? { epds_handle_mode: url.searchParams.get('epds_handle_mode') ?? '' }
130+
: {}),
131+
}
132+
133+
return verifyCallback(
134+
callbackParams,
135+
url.searchParams.get('ts') ?? '',
136+
url.searchParams.get('sig') ?? '',
137+
'test-callback-secret',
138+
)
139+
}
140+
141+
async function fetchCompleteRedirect(handleMode: HandleMode | null) {
142+
const app = await startApp(makeCtx(handleMode), makeAuth())
143+
try {
144+
const res = await fetch(`${app.baseUrl}/auth/complete`, {
145+
redirect: 'manual',
146+
headers: { cookie: `${AUTH_FLOW_COOKIE}=flow-1` },
147+
})
148+
149+
expect(res.status).toBe(303)
150+
const url = parseRedirect(res)
151+
expect(url.pathname).toBe('/oauth/epds-callback')
152+
return url
153+
} finally {
154+
await app.close()
155+
}
156+
}
157+
158+
describe('auth-service epds-callback handle mode threading', () => {
159+
let priorEnv: { pdsInternalUrl?: string; internalSecret?: string }
160+
161+
beforeEach(() => {
162+
priorEnv = {
163+
pdsInternalUrl: process.env.PDS_INTERNAL_URL,
164+
internalSecret: process.env.EPDS_INTERNAL_SECRET,
165+
}
166+
process.env.PDS_INTERNAL_URL = 'http://pds.internal' // NOSONAR test-only internal mocked URL
167+
process.env.EPDS_INTERNAL_SECRET = 'test-internal-secret'
168+
mocks.getDidByEmail.mockReset()
169+
mocks.pingParRequest.mockReset()
170+
mocks.resolveRecoveryEmail.mockReset()
171+
mocks.resolveClientBranding.mockReset()
172+
mocks.pingParRequest.mockResolvedValue({ ok: true })
173+
mocks.resolveRecoveryEmail.mockResolvedValue(null)
174+
mocks.resolveClientBranding.mockResolvedValue({
175+
customCss: null,
176+
customFaviconUrl: null,
177+
customFaviconUrlDark: null,
178+
})
179+
vi.stubGlobal(
180+
'fetch',
181+
vi.fn((input: Parameters<typeof fetch>[0], init?: RequestInit) => {
182+
const url = normalizeFetchUrl(input)
183+
if (url.hostname === '127.0.0.1') return realFetch(input, init)
184+
return Promise.resolve({
185+
ok: true,
186+
json: () => Promise.resolve({ exists: false }),
187+
})
188+
}),
189+
)
190+
})
191+
192+
afterEach(() => {
193+
if (priorEnv.pdsInternalUrl === undefined)
194+
delete process.env.PDS_INTERNAL_URL
195+
else process.env.PDS_INTERNAL_URL = priorEnv.pdsInternalUrl
196+
if (priorEnv.internalSecret === undefined)
197+
delete process.env.EPDS_INTERNAL_SECRET
198+
else process.env.EPDS_INTERNAL_SECRET = priorEnv.internalSecret
199+
vi.unstubAllGlobals()
200+
})
201+
202+
it('includes the stored canonical handle mode for random-mode new users', async () => {
203+
mocks.getDidByEmail.mockResolvedValue(null)
204+
const url = await fetchCompleteRedirect('random')
205+
expect(url.searchParams.get('epds_handle_mode')).toBe('random')
206+
expect(url.searchParams.has('handle')).toBe(false)
207+
expect(verifySignedCallbackUrl(url)).toBe(true)
208+
})
209+
210+
it('includes the stored canonical handle mode for existing users', async () => {
211+
mocks.getDidByEmail.mockResolvedValue(randomBytes(16).toString('hex'))
212+
const url = await fetchCompleteRedirect('picker-with-random')
213+
expect(url.searchParams.get('epds_handle_mode')).toBe('picker-with-random')
214+
})
215+
216+
it('includes the stored canonical handle mode for chosen-handle callbacks', async () => {
217+
mocks.getDidByEmail.mockResolvedValue(null)
218+
const app = await startApp(makeCtx('picker'), makeAuth())
219+
try {
220+
const res = await fetch(`${app.baseUrl}/auth/choose-handle`, {
221+
method: 'POST',
222+
redirect: 'manual',
223+
headers: {
224+
cookie: `${AUTH_FLOW_COOKIE}=flow-1`,
225+
'content-type': 'application/x-www-form-urlencoded',
226+
},
227+
body: new URLSearchParams({ handle: 'Alice1' }),
228+
})
229+
230+
expect(res.status).toBe(303)
231+
const url = parseRedirect(res)
232+
expect(url.pathname).toBe('/oauth/epds-callback')
233+
expect(url.searchParams.get('epds_handle_mode')).toBe('picker')
234+
expect(url.searchParams.get('handle')).toBe('alice1')
235+
} finally {
236+
await app.close()
237+
}
238+
})
239+
})

‎packages/auth-service/src/routes/choose-handle.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,7 @@ export function createChooseHandleRouter(
404404
approved: '1',
405405
new_account: '1',
406406
handle: normalizedLocal,
407+
epds_handle_mode: flow.handleMode ?? '',
407408
}
408409
if (flow.clientId) callbackParams.client_id = flow.clientId
409410
const { sig, ts } = signCallback(

‎packages/auth-service/src/routes/complete.ts‎

Lines changed: 50 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ const AUTH_FLOW_COOKIE = 'epds_auth_flow'
5757
* the same `params.handle ?? ''` shape; the sentinel is pinned by
5858
* tests in packages/shared/src/__tests__/crypto.test.ts.
5959
*
60+
* `epds_handle_mode` is signed too rather than appended afterwards, so
61+
* the browser cannot flip the chooser/consent presentation mode on the
62+
* hop to pds-core without invalidating the signature.
63+
*
6064
* Exported so it can be unit-tested without standing up the full
6165
* /auth/complete route.
6266
*/
@@ -65,6 +69,7 @@ export function buildEpdsCallbackUrl(args: {
6569
flowClientId: string | null
6670
email: string
6771
isNewAccount: boolean
72+
flowHandleMode?: string | null
6873
pdsPublicUrl: string
6974
epdsCallbackSecret: string
7075
}): string {
@@ -75,11 +80,41 @@ export function buildEpdsCallbackUrl(args: {
7580
new_account: args.isNewAccount ? '1' : '0',
7681
}
7782
if (args.flowClientId) callbackParams.client_id = args.flowClientId
83+
if (args.flowHandleMode) callbackParams.epds_handle_mode = args.flowHandleMode
7884
const { sig, ts } = signCallback(callbackParams, args.epdsCallbackSecret)
7985
const params = new URLSearchParams({ ...callbackParams, ts, sig })
8086
return `${args.pdsPublicUrl}/oauth/epds-callback?${params.toString()}`
8187
}
8288

89+
async function resolveCompleteIdentity(
90+
email: string,
91+
flowId: string,
92+
ctx: AuthServiceContext,
93+
pdsUrl: string,
94+
internalSecret: string,
95+
): Promise<{ email: string; did: string | null }> {
96+
const did = await getDidByEmail(email, pdsUrl, internalSecret)
97+
if (did) return { email, did }
98+
99+
// Recovery path: session email is a backup email, not a primary. Resolve
100+
// the backup-email -> DID mapping (auth-service-owned) and then DID ->
101+
// primary email via pds-core's internal API, so the downstream callback
102+
// signs the user's real account email, not the recovery address.
103+
const recovered = await resolveRecoveryEmail(
104+
email,
105+
ctx,
106+
pdsUrl,
107+
internalSecret,
108+
)
109+
if (!recovered) return { email, did: null }
110+
111+
logger.info(
112+
{ flowId, did: recovered.did },
113+
'Recovery: translated backup email to primary email via DID',
114+
)
115+
return { email: recovered.email, did: recovered.did }
116+
}
117+
83118
export function createCompleteRouter(
84119
ctx: AuthServiceContext,
85120
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- better-auth instance has no exported type
@@ -127,7 +162,11 @@ export function createCompleteRouter(
127162
*/
128163
async function redirectNewUserRandomMode(
129164
res: Response,
130-
flow: { requestUri: string; clientId: string | null },
165+
flow: {
166+
requestUri: string
167+
clientId: string | null
168+
handleMode: string | null
169+
},
131170
email: string,
132171
flowId: string,
133172
): Promise<void> {
@@ -143,6 +182,7 @@ export function createCompleteRouter(
143182
flowClientId: flow.clientId,
144183
email,
145184
isNewAccount: true,
185+
flowHandleMode: flow.handleMode,
146186
pdsPublicUrl: ctx.config.pdsPublicUrl,
147187
epdsCallbackSecret: ctx.config.epdsCallbackSecret,
148188
})
@@ -204,31 +244,16 @@ export function createCompleteRouter(
204244
return
205245
}
206246

207-
let email = session.user.email.toLowerCase()
247+
const sessionEmail = session.user.email.toLowerCase()
208248

209249
// Step 4: Check whether this is a new user (no PDS account for email).
210-
let did = await getDidByEmail(email, pdsUrl, internalSecret)
211-
212-
// Recovery path: session email is a backup email, not a primary. Resolve
213-
// the backup-email → DID mapping (auth-service-owned) and then DID →
214-
// primary email via pds-core's internal API, so the downstream callback
215-
// signs the user's real account email, not the recovery address.
216-
if (!did) {
217-
const recovered = await resolveRecoveryEmail(
218-
email,
219-
ctx,
220-
pdsUrl,
221-
internalSecret,
222-
)
223-
if (recovered) {
224-
logger.info(
225-
{ flowId, did: recovered.did },
226-
'Recovery: translated backup email to primary email via DID',
227-
)
228-
email = recovered.email
229-
did = recovered.did
230-
}
231-
}
250+
const { email, did } = await resolveCompleteIdentity(
251+
sessionEmail,
252+
flowId,
253+
ctx,
254+
pdsUrl,
255+
internalSecret,
256+
)
232257

233258
const isNewAccount = !did
234259

@@ -264,6 +289,7 @@ export function createCompleteRouter(
264289
flowClientId: flow.clientId,
265290
email,
266291
isNewAccount: false,
292+
flowHandleMode: flow.handleMode,
267293
pdsPublicUrl: ctx.config.pdsPublicUrl,
268294
epdsCallbackSecret: ctx.config.epdsCallbackSecret,
269295
})

0 commit comments

Comments
 (0)