From 8abdaaf8cc993013de054ffedbb458ef6c76af29 Mon Sep 17 00:00:00 2001 From: insuffer Date: Wed, 23 Sep 2026 00:05:26 +0800 Subject: [PATCH] fix(auth): preserve state parameter on error redirects per RFC 6749 4.1.2.1 --- src/server/auth/handlers/authorize.ts | 10 +++++- test/server/auth/handlers/authorize.test.ts | 34 +++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/server/auth/handlers/authorize.ts b/src/server/auth/handlers/authorize.ts index 4b9f3b327f..3e0fdaaf21 100644 --- a/src/server/auth/handlers/authorize.ts +++ b/src/server/auth/handlers/authorize.ts @@ -142,8 +142,16 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A let state; try { // Parse and validate authorization parameters - const parseResult = RequestAuthorizationParamsSchema.safeParse(req.method === 'POST' ? req.body : req.query); + const params = req.method === 'POST' ? req.body : req.query; + const parseResult = RequestAuthorizationParamsSchema.safeParse(params); if (!parseResult.success) { + // RFC 6749 §4.1.2.1: if the request contained a state, error + // redirects MUST echo it so the client can correlate the + // response. Recover it from the raw params before failing. + const rawState = (params as { state?: unknown }).state; + if (typeof rawState === 'string') { + state = rawState; + } throw new InvalidRequestError(parseResult.error.message); } diff --git a/test/server/auth/handlers/authorize.test.ts b/test/server/auth/handlers/authorize.test.ts index f4d68d4df8..67e5fce085 100644 --- a/test/server/auth/handlers/authorize.test.ts +++ b/test/server/auth/handlers/authorize.test.ts @@ -324,6 +324,40 @@ describe('Authorization Handler', () => { }); }); + describe('State preservation on error redirects (RFC 6749 §4.1.2.1)', () => { + it('includes state in the error redirect when request parameters fail validation', async () => { + const response = await supertest(app).get('/authorize').query({ + client_id: 'valid-client', + redirect_uri: 'https://example.com/callback', + response_type: 'code', + code_challenge: 'challenge123', + code_challenge_method: 'plain', // invalid - only S256 is supported + state: 'csrf-state-42' + }); + + expect(response.status).toBe(302); + const location = new URL(response.header.location); + expect(location.searchParams.get('error')).toBe('invalid_request'); + expect(location.searchParams.get('state')).toBe('csrf-state-42'); + }); + + it('includes state in the error redirect for POST requests', async () => { + const response = await supertest(app).post('/authorize').type('form').send({ + client_id: 'valid-client', + redirect_uri: 'https://example.com/callback', + response_type: 'code', + code_challenge_method: 'S256', + state: 'post-csrf-state-7' + // Missing code_challenge + }); + + expect(response.status).toBe(302); + const location = new URL(response.header.location); + expect(location.searchParams.get('error')).toBe('invalid_request'); + expect(location.searchParams.get('state')).toBe('post-csrf-state-7'); + }); + }); + describe('Resource parameter validation', () => { it('propagates resource parameter', async () => { const mockProviderWithResource = vi.spyOn(mockProvider, 'authorize');