Skip to content

Commit 1778d20

Browse files
committed
add pinger to fix session expired issue
1 parent 57e48b7 commit 1778d20

7 files changed

Lines changed: 79 additions & 45 deletions

File tree

packages/auth-service/src/__tests__/consent.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ describe('Consent route logic', () => {
212212
}
213213
const { sig, ts } = signCallback(params, 'shared-secret')
214214
const result = verifyCallback(params, ts, sig, 'shared-secret')
215-
expect(result.valid).toBe(true)
215+
expect(result).toBe(true)
216216
})
217217
})
218218

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,25 @@ export function createChooseHandleRouter(
149149
return
150150
}
151151

152+
// Reset the PAR request inactivity timer so it doesn't expire while the
153+
// user is on this page. atproto's AUTHORIZATION_INACTIVITY_TIMEOUT is 5 min
154+
// — without this ping, users who take >5 min to pick a handle would hit
155+
// "This request has expired" inside epds-callback after account creation.
156+
try {
157+
await fetch(
158+
`${pdsUrl}/_internal/ping-request?request_uri=${encodeURIComponent(result.flow.requestUri)}`,
159+
{
160+
headers: { 'x-internal-secret': internalSecret },
161+
signal: AbortSignal.timeout(3000),
162+
},
163+
)
164+
} catch (err) {
165+
logger.debug(
166+
{ err },
167+
'Failed to ping request_uri on choose-handle — ignoring',
168+
)
169+
}
170+
152171
const error = req.query.error as string | undefined
153172
res
154173
.type('html')
@@ -202,7 +221,7 @@ export function createChooseHandleRouter(
202221

203222
// Step 4: Construct full handle and check availability via PDS internal API
204223
const fullHandle = `${rawHandle}.${handleDomain}`
205-
let handleAvailable = false
224+
let handleAvailable: boolean
206225
try {
207226
const checkRes = await fetch(
208227
`${pdsUrl}/_internal/check-handle?handle=${encodeURIComponent(fullHandle)}`,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ export function createCompleteRouter(
134134
request_uri: flow.requestUri,
135135
email,
136136
approved: '1',
137-
new_account: isNewAccount ? '1' : '0',
137+
new_account: '0',
138138
}
139139
const { sig, ts } = signCallback(
140140
callbackParams,

packages/pds-core/src/index.ts

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ async function main() {
8585
const approvedStr = req.query.approved as string
8686
const newAccountStr = req.query.new_account as string
8787
const handleParam = req.query.handle as string | undefined
88-
const callbackVerification = verifyCallback(
88+
const isValidCallback = verifyCallback(
8989
{
9090
request_uri: requestUri,
9191
email,
@@ -98,7 +98,7 @@ async function main() {
9898
epdsCallbackSecret,
9999
)
100100

101-
if (!callbackVerification.valid) {
101+
if (!isValidCallback) {
102102
// Distinguish expired from invalid to help with clock-skew debugging
103103
const tsNum = parseInt(ts, 10)
104104
const age = Math.floor(Date.now() / 1000) - tsNum
@@ -113,7 +113,7 @@ async function main() {
113113
// Extract handle local part from verified callback params (tamper-proof — covered by HMAC).
114114
// The callback now carries only the local part (e.g. 'alice'); we append our own
115115
// trusted handleDomain here so there is no possibility of domain mismatch.
116-
const chosenHandleLocal = callbackVerification.handle
116+
const chosenHandleLocal = handleParam
117117
const chosenHandle = chosenHandleLocal
118118
? `${chosenHandleLocal}.${handleDomain}`
119119
: undefined
@@ -471,6 +471,40 @@ async function main() {
471471
}
472472
})
473473

474+
// Protected internal endpoint for auth service to reset the inactivity timer
475+
// on a pending PAR request_uri. Called when the user loads the handle selection
476+
// page so the request doesn't expire while they are choosing a handle.
477+
// atproto's AUTHORIZATION_INACTIVITY_TIMEOUT is 5 minutes — without this ping,
478+
// users who take >5 min on the handle page would get "This request has expired"
479+
// inside epds-callback after account creation, leaving the auth flow broken.
480+
pds.app.get('/_internal/ping-request', async (req, res) => {
481+
if (!verifyInternalSecret(req.headers['x-internal-secret'])) {
482+
res.status(401).json({ error: 'Unauthorized' })
483+
return
484+
}
485+
const requestUri = ((req.query.request_uri as string) || '').trim()
486+
if (!requestUri) {
487+
res.status(400).json({ error: 'Missing request_uri' })
488+
return
489+
}
490+
if (!provider) {
491+
res.status(503).json({ error: 'OAuth provider not available' })
492+
return
493+
}
494+
try {
495+
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- @atproto/oauth-provider requestManager not exported
496+
await (provider.requestManager as any).get(requestUri)
497+
res.json({ ok: true })
498+
} catch (err) {
499+
// Request expired or not found — not a server error, just report it
500+
logger.debug(
501+
{ err, requestUri },
502+
'ping-request: request_uri expired or not found',
503+
)
504+
res.status(404).json({ error: 'request_expired' })
505+
}
506+
})
507+
474508
// Protected internal endpoint for auth service to retrieve the login_hint
475509
// stored in a PAR request. Third-party apps put the handle/DID in the PAR body
476510
// but don't duplicate it on the authorization redirect URL. The auth service

packages/shared/src/__tests__/crypto.test.ts

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import {
88
signCallback,
99
verifyCallback,
1010
type CallbackParams,
11-
type VerifyCallbackResult,
1211
} from '../crypto.js'
1312

1413
describe('generateVerificationToken', () => {
@@ -100,20 +99,20 @@ describe('signCallback / verifyCallback', () => {
10099
expect(ts).toMatch(/^\d+$/)
101100
})
102101

103-
it('round-trips: sign then verify returns valid=true', () => {
102+
it('round-trips: sign then verify returns true', () => {
104103
const { sig, ts } = signCallback(params, secret)
105-
expect(verifyCallback(params, ts, sig, secret).valid).toBe(true)
104+
expect(verifyCallback(params, ts, sig, secret)).toBe(true)
106105
})
107106

108107
it('rejects wrong secret', () => {
109108
const { sig, ts } = signCallback(params, secret)
110-
expect(verifyCallback(params, ts, sig, 'wrong-secret').valid).toBe(false)
109+
expect(verifyCallback(params, ts, sig, 'wrong-secret')).toBe(false)
111110
})
112111

113112
it('rejects tampered email', () => {
114113
const { sig, ts } = signCallback(params, secret)
115114
const tampered = { ...params, email: 'attacker@evil.com' }
116-
expect(verifyCallback(tampered, ts, sig, secret).valid).toBe(false)
115+
expect(verifyCallback(tampered, ts, sig, secret)).toBe(false)
117116
})
118117

119118
it('rejects tampered request_uri', () => {
@@ -122,7 +121,7 @@ describe('signCallback / verifyCallback', () => {
122121
...params,
123122
request_uri: 'urn:ietf:params:oauth:request_uri:evil',
124123
}
125-
expect(verifyCallback(tampered, ts, sig, secret).valid).toBe(false)
124+
expect(verifyCallback(tampered, ts, sig, secret)).toBe(false)
126125
})
127126

128127
it('rejects expired timestamp (>5 min old)', async () => {
@@ -139,7 +138,7 @@ describe('signCallback / verifyCallback', () => {
139138
].join('\n')
140139
const { createHmac } = await import('node:crypto')
141140
const staleSig = createHmac('sha256', secret).update(payload).digest('hex')
142-
expect(verifyCallback(params, staleTs, staleSig, secret).valid).toBe(false)
141+
expect(verifyCallback(params, staleTs, staleSig, secret)).toBe(false)
143142
})
144143

145144
it('rejects future timestamp', async () => {
@@ -154,21 +153,17 @@ describe('signCallback / verifyCallback', () => {
154153
].join('\n')
155154
const { createHmac } = await import('node:crypto')
156155
const futureSig = createHmac('sha256', secret).update(payload).digest('hex')
157-
expect(verifyCallback(params, futureTs, futureSig, secret).valid).toBe(
158-
false,
159-
)
156+
expect(verifyCallback(params, futureTs, futureSig, secret)).toBe(false)
160157
})
161158

162159
it('rejects non-numeric timestamp', () => {
163160
const { sig } = signCallback(params, secret)
164-
expect(verifyCallback(params, 'not-a-number', sig, secret).valid).toBe(
165-
false,
166-
)
161+
expect(verifyCallback(params, 'not-a-number', sig, secret)).toBe(false)
167162
})
168163

169164
it('rejects wrong-length sig', () => {
170165
const { ts } = signCallback(params, secret)
171-
expect(verifyCallback(params, ts, 'tooshort', secret).valid).toBe(false)
166+
expect(verifyCallback(params, ts, 'tooshort', secret)).toBe(false)
172167
})
173168
})
174169

@@ -183,12 +178,10 @@ describe('signCallback / verifyCallback with handle', () => {
183178
handle: 'alice.pds.example.com',
184179
}
185180
const { sig, ts } = signCallback(params, secret)
186-
const result = verifyCallback(params, ts, sig, secret)
187-
expect(result.valid).toBe(true)
188-
expect(result.handle).toBe('alice.pds.example.com')
181+
expect(verifyCallback(params, ts, sig, secret)).toBe(true)
189182
})
190183

191-
it('signs and verifies callback WITHOUT handle (backward compat)', () => {
184+
it('signs and verifies callback WITHOUT handle', () => {
192185
const secret = 'test-secret'
193186
const params: CallbackParams = {
194187
request_uri: 'urn:ietf:params:oauth:request_uri:test',
@@ -197,9 +190,7 @@ describe('signCallback / verifyCallback with handle', () => {
197190
new_account: '1',
198191
}
199192
const { sig, ts } = signCallback(params, secret)
200-
const result = verifyCallback(params, ts, sig, secret)
201-
expect(result.valid).toBe(true)
202-
expect(result.handle).toBeUndefined()
193+
expect(verifyCallback(params, ts, sig, secret)).toBe(true)
203194
})
204195

205196
it('produces different signatures with vs without handle', () => {
@@ -233,7 +224,6 @@ describe('signCallback / verifyCallback with handle', () => {
233224
...params,
234225
handle: 'evil.pds.example.com',
235226
}
236-
const result = verifyCallback(tamperedParams, ts, sig, secret)
237-
expect(result.valid).toBe(false)
227+
expect(verifyCallback(tamperedParams, ts, sig, secret)).toBe(false)
238228
})
239229
})

packages/shared/src/crypto.ts

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -82,30 +82,23 @@ export function signCallback(
8282

8383
const CALLBACK_MAX_AGE_SECONDS = 5 * 60 // 5 minutes
8484

85-
export interface VerifyCallbackResult {
86-
valid: boolean
87-
handle?: string // present only when the callback was signed with a handle
88-
}
89-
9085
/**
9186
* Verify a signed epds-callback redirect URL.
92-
* Returns { valid, handle? } where valid is true only when the signature is
93-
* valid and the timestamp is fresh. handle is included when the callback was
94-
* signed with a chosen handle (new account creation flow).
87+
* Returns true only when the signature is valid and the timestamp is fresh.
9588
* Uses timingSafeEqual to avoid timing side-channels.
9689
*/
9790
export function verifyCallback(
9891
params: CallbackParams,
9992
ts: string,
10093
sig: string,
10194
secret: string,
102-
): VerifyCallbackResult {
95+
): boolean {
10396
const tsNum = parseInt(ts, 10)
104-
if (isNaN(tsNum)) return { valid: false }
97+
if (isNaN(tsNum)) return false
10598

10699
const now = Math.floor(Date.now() / 1000)
107100
const age = now - tsNum
108-
if (age < 0 || age > CALLBACK_MAX_AGE_SECONDS) return { valid: false }
101+
if (age < 0 || age > CALLBACK_MAX_AGE_SECONDS) return false
109102

110103
const payload = [
111104
params.request_uri,
@@ -122,13 +115,11 @@ export function verifyCallback(
122115

123116
// Both are hex-encoded HMAC-SHA256 (always 64 chars / 32 bytes).
124117
// Guard against wrong-length input to keep timingSafeEqual happy.
125-
if (sig.length !== expected.length) return { valid: false }
126-
const isValid = crypto.timingSafeEqual(
118+
if (sig.length !== expected.length) return false
119+
return crypto.timingSafeEqual(
127120
Buffer.from(expected, 'hex'),
128121
Buffer.from(sig, 'hex'),
129122
)
130-
if (!isValid) return { valid: false }
131-
return { valid: true, ...(params.handle ? { handle: params.handle } : {}) }
132123
}
133124

134125
/**

packages/shared/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export {
1515
signCallback,
1616
verifyCallback,
1717
} from './crypto.js'
18-
export type { CallbackParams, VerifyCallbackResult } from './crypto.js'
18+
export type { CallbackParams } from './crypto.js'
1919
export type {
2020
EpdsLinkConfig,
2121
EmailConfig,

0 commit comments

Comments
 (0)