fix(auth): require state before GitHub callback exchange

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))

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<unknown>
-  let query: Record<string, string>
+  let query: Record<string, string | string[]>
+  let stateCookie: string | undefined
   let intent: Record<string, unknown>
   let failure: boolean
   let providerFailure: boolean
@@ -35,14 +36,15 @@ describe('gitHub browser login round trip', () => {
     vi.resetModules()
     vi.clearAllMocks()
     query = {}
+    stateCookie = 'valid-state'
     intent = {}
     failure = false
     providerFailure = false
     mocks.upsert.mockResolvedValue({ _tag: 'existing', user: { id: 1, github_id: 1, login: 'octocat', onboarded_at: 1 } })
     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' }