From 55583f8d807c0c4a7a04c5785b0f9b0c3a64cc51 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Wed, 14 Oct 2026 03:05:04 +1100 Subject: [PATCH] fix(auth): require state before GitHub callback exchange --- .../identity/server/routes/auth/github.get.ts | 8 +++++++ test/unit/github-login-intent.nuxt.test.ts | 23 +++++++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/layers/identity/server/routes/auth/github.get.ts b/layers/identity/server/routes/auth/github.get.ts index 77fcb409..064802bb 100644 --- a/layers/identity/server/routes/auth/github.get.ts +++ b/layers/identity/server/routes/auth/github.get.ts @@ -197,6 +197,14 @@ function loginIntentSession(event: H3Event) { export default defineEventHandler(async (event) => { const query = getQuery(event) + const stateCookie = getCookie(event, 'nuxt-auth-state') + + // Require browser state before the provider exchanges a callback code. + if (query.code !== undefined && (typeof query.state !== 'string' || !query.state || !stateCookie || query.state !== stateCookie)) { + deleteCookie(event, 'nuxt-auth-state', { path: '/' }) + return loginFailed(event) + } + if (!query.code && !query.error && !query.state) { const intent = await loginIntentSession(event) const action = ['like-skill', 'watch-skill', 'watch-collection'].includes(String(query.action)) diff --git a/test/unit/github-login-intent.nuxt.test.ts b/test/unit/github-login-intent.nuxt.test.ts index 9e4e6b22..de8b21cf 100644 --- a/test/unit/github-login-intent.nuxt.test.ts +++ b/test/unit/github-login-intent.nuxt.test.ts @@ -22,7 +22,8 @@ vi.mock('../../layers/identity/server/utils/skill-validation-email', () => ({ se // Simulate GitHub's callback: it carries code and state, never our query fields. describe('gitHub browser login round trip', () => { let handler: (event: H3Event) => Promise - let query: Record + let query: Record + let stateCookie: string | undefined let intent: Record let failure: boolean let providerFailure: boolean @@ -35,6 +36,7 @@ describe('gitHub browser login round trip', () => { vi.resetModules() vi.clearAllMocks() query = {} + stateCookie = 'valid-state' intent = {} failure = false providerFailure = false @@ -42,7 +44,7 @@ describe('gitHub browser login round trip', () => { mocks.welcome.mockResolvedValue('accepted') mocks.replay.mockResolvedValue(undefined) vi.stubGlobal('getQuery', () => query) - vi.stubGlobal('getCookie', () => undefined) + vi.stubGlobal('getCookie', () => stateCookie) vi.stubGlobal('deleteCookie', vi.fn()) vi.stubGlobal('defineEventHandler', (fn: unknown) => fn) vi.stubGlobal('setUserSession', mocks.establish) @@ -73,6 +75,23 @@ describe('gitHub browser login round trip', () => { handler = (await import('../../layers/identity/server/routes/auth/github.get')).default as typeof handler }) + it.each([ + ['missing query and cookie', undefined, undefined], + ['missing query state', undefined, 'valid-state'], + ['missing cookie', 'valid-state', undefined], + ['empty query and cookie', '', ''], + ['empty query state', '', 'valid-state'], + ['empty cookie', 'valid-state', ''], + ['duplicate query state', ['valid-state', 'valid-state'], 'valid-state'], + ['mismatched state', 'other-state', 'valid-state'], + ])('rejects %s before establishing identity', async (_label, state, cookie) => { + query = { code: 'github-code', ...(state === undefined ? {} : { state }) } + stateCookie = cookie as string | undefined + expect(await handler(event)).toBe('/login?error=oauth') + expect(mocks.upsert).not.toHaveBeenCalled() + expect(mocks.establish).not.toHaveBeenCalled() + }) + it('sends the new account welcome without subscribing to recurring emails', async () => { mocks.upsert.mockResolvedValue({ _tag: 'created', user: { id: 1, github_id: 1, login: 'octocat', email: 'octocat@example.com', onboarded_at: null, repo_indexing: 0 } }) query = { code: 'github-code', state: 'valid-state' }