[auto] Bind OAuth login state to browser nonce

server/lib/session.js

@@ -1,6 +1,9 @@
+const crypto = require('crypto')
 const jwt = require('jsonwebtoken')
 
 const JWT_EXPIRY = '7d'
+const OAUTH_NONCE_COOKIE = 'oauth_nonce'
+const OAUTH_NONCE_MAX_AGE_MS = 10 * 60 * 1000
 
 function signToken(payload) {
   return jwt.sign(payload, process.env.JWT_SECRET, { expiresIn: JWT_EXPIRY })
@@ -25,10 +28,34 @@ function clearTokenCookie(res) {
   res.cookie('token', '', tokenCookieOptions(0))
 }
 
+function createOAuthNonce() {
+  return crypto.randomBytes(32).toString('hex')
+}
+
+function hashOAuthNonce(nonce) {
+  return crypto.createHash('sha256').update(nonce).digest('hex')
+}
+
+function setOAuthNonceCookie(res, nonce) {
+  res.cookie(OAUTH_NONCE_COOKIE, nonce, tokenCookieOptions(OAUTH_NONCE_MAX_AGE_MS))
+}
+
+function clearOAuthNonceCookie(res) {
+  res.cookie(OAUTH_NONCE_COOKIE, '', tokenCookieOptions(0))
+}
+
 function signOAuthState(payload, expiresIn = '10m') {
   return jwt.sign(payload, process.env.JWT_SECRET, { expiresIn })
 }
 
+function signOAuthLoginState(provider, nonce) {
+  return signOAuthState({
+    provider,
+    mode: 'login',
+    nonce_hash: hashOAuthNonce(nonce),
+  })
+}
+
 function verifyOAuthState(token) {
   try {
     return jwt.verify(token, process.env.JWT_SECRET)
@@ -37,6 +64,12 @@ function verifyOAuthState(token) {
   }
 }
 
+function hasValidOAuthNonce(req, statePayload) {
+  const nonce = req.cookies?.[OAUTH_NONCE_COOKIE]
+  if (!nonce || !statePayload?.nonce_hash) return false
+  return hashOAuthNonce(nonce) === statePayload.nonce_hash
+}
+
 function signCompletionToken(payload) {
   return jwt.sign(payload, process.env.JWT_SECRET, { expiresIn: '30m' })
 }
@@ -53,9 +86,15 @@ module.exports = {
   signToken,
   setTokenCookie,
   clearTokenCookie,
+  createOAuthNonce,
+  setOAuthNonceCookie,
+  clearOAuthNonceCookie,
   signOAuthState,
+  signOAuthLoginState,
   verifyOAuthState,
+  hasValidOAuthNonce,
   signCompletionToken,
   verifyCompletionToken,
   JWT_EXPIRY,
+  OAUTH_NONCE_COOKIE,
 }

server/routes/github.js

@@ -3,8 +3,13 @@ const jwt = require('jsonwebtoken')
 const {
   signToken,
   setTokenCookie,
-  signOAuthState,
+  createOAuthNonce,
+  setOAuthNonceCookie,
+  clearOAuthNonceCookie,
   verifyOAuthState,
+  signOAuthState,
+  signOAuthLoginState,
+  hasValidOAuthNonce,
 } = require('../lib/session')
 const { formatUserResponse, linkOAuthProvider } = require('../lib/oauthUsers')
 const { findOrCreateOAuthUser } = require('./google')
@@ -39,7 +44,9 @@ router.get('/url', (req, res) => {
   if (!process.env.GITHUB_CLIENT_ID) {
     return res.status(503).json({ error: 'GitHub sign-in is not configured' })
   }
-  const state = signOAuthState({ provider: 'github', mode: 'login' })
+  const nonce = createOAuthNonce()
+  const state = signOAuthLoginState('github', nonce)
+  setOAuthNonceCookie(res, nonce)
   res.json({ url: buildGithubAuthUrl(state) })
 })
 
@@ -131,6 +138,11 @@ router.post('/callback', async (req, res) => {
       return res.status(400).json({ error: 'Invalid or expired state' })
     }
 
+    if (statePayload.mode === 'login' && !hasValidOAuthNonce(req, statePayload)) {
+      clearOAuthNonceCookie(res)
+      return res.status(400).json({ error: 'Invalid or expired state' })
+    }
+
     const githubData = await fetchGithubProfile(code)
     if (!githubData) {
       return res.status(502).json({ error: 'Failed to obtain GitHub access token' })
@@ -165,6 +177,7 @@ router.post('/callback', async (req, res) => {
 
     const sessionToken = signToken({ userId: user.id, username: user.username })
     setTokenCookie(res, sessionToken)
+    clearOAuthNonceCookie(res)
     res.json(formatUserResponse(user))
   } catch (err) {
     if (err.statusCode) {

server/routes/google.js

@@ -5,8 +5,13 @@ const AppError = require('../lib/AppError')
 const {
   signToken,
   setTokenCookie,
-  signOAuthState,
+  createOAuthNonce,
+  setOAuthNonceCookie,
+  clearOAuthNonceCookie,
   verifyOAuthState,
+  signOAuthState,
+  signOAuthLoginState,
+  hasValidOAuthNonce,
 } = require('../lib/session')
 const { findAvailableUsername, formatUserResponse, linkOAuthProvider } = require('../lib/oauthUsers')
 const authenticateToken = require('../middleware/authenticateToken')
@@ -44,7 +49,9 @@ router.get('/url', (req, res) => {
   if (!process.env.GOOGLE_CLIENT_ID) {
     return res.status(503).json({ error: 'Google sign-in is not configured' })
   }
-  const state = signOAuthState({ provider: 'google', mode: 'login' })
+  const nonce = createOAuthNonce()
+  const state = signOAuthLoginState('google', nonce)
+  setOAuthNonceCookie(res, nonce)
   res.json({ url: buildGoogleAuthUrl(state) })
 })
 
@@ -93,6 +100,11 @@ router.post('/callback', async (req, res) => {
       return res.status(400).json({ error: 'Invalid or expired state' })
     }
 
+    if (statePayload.mode === 'login' && !hasValidOAuthNonce(req, statePayload)) {
+      clearOAuthNonceCookie(res)
+      return res.status(400).json({ error: 'Invalid or expired state' })
+    }
+
     const profile = await exchangeGoogleCode(code)
     if (!profile?.id) {
       return res.status(502).json({ error: 'Failed to fetch Google profile' })
@@ -127,6 +139,7 @@ router.post('/callback', async (req, res) => {
 
     const sessionToken = signToken({ userId: user.id, username: user.username })
     setTokenCookie(res, sessionToken)
+    clearOAuthNonceCookie(res)
     res.json(formatUserResponse(user))
   } catch (err) {
     if (err.statusCode) {

server/routes/orcid.js

@@ -6,8 +6,13 @@ const authenticateToken = require('../middleware/authenticateToken')
 const {
   signToken,
   setTokenCookie,
+  createOAuthNonce,
+  setOAuthNonceCookie,
+  clearOAuthNonceCookie,
   signOAuthState,
   verifyOAuthState,
+  signOAuthLoginState,
+  hasValidOAuthNonce,
   signCompletionToken,
 } = require('../lib/session')
 const { formatUserResponse } = require('../lib/oauthUsers')
@@ -84,7 +89,9 @@ router.get('/login/url', (req, res) => {
   if (!process.env.ORCID_CLIENT_ID) {
     return res.status(503).json({ error: 'ORCID sign-in is not configured' })
   }
-  const state = signOAuthState({ mode: 'login', provider: 'orcid' })
+  const nonce = createOAuthNonce()
+  const state = signOAuthLoginState('orcid', nonce)
+  setOAuthNonceCookie(res, nonce)
   return res.json({ url: buildOrcidAuthUrl(state) })
 })
 
@@ -143,6 +150,15 @@ router.post('/callback', async (req, res) => {
       return res.status(400).json({ error: 'Invalid or expired state' })
     }
 
+    if (statePayload.mode === 'login') {
+      if (statePayload.provider !== 'orcid' || !hasValidOAuthNonce(req, statePayload)) {
+        clearOAuthNonceCookie(res)
+        return res.status(400).json({ error: 'Invalid or expired state' })
+      }
+    } else if (statePayload.provider && statePayload.provider !== 'orcid') {
+      return res.status(400).json({ error: 'Invalid or expired state' })
+    }
+
     const tokenData = await exchangeOrcidCode(code)
     if (!tokenData) {
       return res.status(502).json({ error: 'Failed to exchange ORCID code' })
@@ -188,6 +204,7 @@ async function handleOrcidLogin(req, res, { orcidId, displayName }) {
     const user = existing.rows[0]
     const sessionToken = signToken({ userId: user.id, username: user.username })
     setTokenCookie(res, sessionToken)
+    clearOAuthNonceCookie(res)
     return res.json({ ...formatUserResponse(user), mode: 'login' })
   }
 
@@ -198,6 +215,7 @@ async function handleOrcidLogin(req, res, { orcidId, displayName }) {
     email: null,
   })
 
+  clearOAuthNonceCookie(res)
   return res.json({
     needs_completion: true,
     completion_token: completionToken,

server/tests/oauth-state.test.js

@@ -0,0 +1,113 @@
+const request = require('supertest')
+const app = require('../index')
+const {
+  OAUTH_NONCE_COOKIE,
+  verifyOAuthState,
+} = require('../lib/session')
+
+const originalEnv = {
+  JWT_SECRET: process.env.JWT_SECRET,
+  CLIENT_URL: process.env.CLIENT_URL,
+  GOOGLE_CLIENT_ID: process.env.GOOGLE_CLIENT_ID,
+  GITHUB_CLIENT_ID: process.env.GITHUB_CLIENT_ID,
+  ORCID_CLIENT_ID: process.env.ORCID_CLIENT_ID,
+}
+
+const providers = [
+  {
+    provider: 'google',
+    urlPath: '/auth/google/url',
+    callbackPath: '/auth/google/callback',
+    clientIdEnv: 'GOOGLE_CLIENT_ID',
+  },
+  {
+    provider: 'github',
+    urlPath: '/auth/github/url',
+    callbackPath: '/auth/github/callback',
+    clientIdEnv: 'GITHUB_CLIENT_ID',
+  },
+  {
+    provider: 'orcid',
+    urlPath: '/auth/orcid/login/url',
+    callbackPath: '/auth/orcid/callback',
+    clientIdEnv: 'ORCID_CLIENT_ID',
+  },
+]
+
+function restoreEnv() {
+  Object.entries(originalEnv).forEach(([key, value]) => {
+    if (value === undefined) {
+      delete process.env[key]
+    } else {
+      process.env[key] = value
+    }
+  })
+}
+
+function configureOAuth(provider) {
+  process.env.JWT_SECRET = 'test-secret'
+  process.env.CLIENT_URL = 'https://client.example.test'
+  process.env[provider.clientIdEnv] = `${provider.provider}-client-id`
+}
+
+function stateFromUrl(url) {
+  return new URL(url).searchParams.get('state')
+}
+
+function nonceCookieFrom(res) {
+  return res.headers['set-cookie']?.find(cookie => (
+    cookie.startsWith(`${OAUTH_NONCE_COOKIE}=`)
+  ))
+}
+
+afterEach(() => {
+  jest.restoreAllMocks()
+  restoreEnv()
+})
+
+describe('OAuth login state nonce', () => {
+  it.each(providers)('binds $provider login state to an httpOnly nonce cookie', async (provider) => {
+    configureOAuth(provider)
+
+    const res = await request(app).get(provider.urlPath)
+
+    expect(res.status).toBe(200)
+    expect(nonceCookieFrom(res)).toContain('HttpOnly')
+
+    const statePayload = verifyOAuthState(stateFromUrl(res.body.url))
+    expect(statePayload).toMatchObject({
+      provider: provider.provider,
+      mode: 'login',
+    })
+    expect(statePayload.nonce_hash).toMatch(/^[a-f0-9]{64}$/)
+  })
+
+  it.each(providers)('rejects $provider login callbacks without the nonce cookie', async (provider) => {
+    configureOAuth(provider)
+    const fetchSpy = jest.spyOn(global, 'fetch')
+
+    const urlRes = await request(app).get(provider.urlPath)
+    const callbackRes = await request(app)
+      .post(provider.callbackPath)
+      .send({ code: 'attacker-code', state: stateFromUrl(urlRes.body.url) })
+
+    expect(callbackRes.status).toBe(400)
+    expect(callbackRes.body.error).toBe('Invalid or expired state')
+    expect(fetchSpy).not.toHaveBeenCalled()
+  })
+
+  it.each(providers)('rejects $provider login callbacks with another browser nonce', async (provider) => {
+    configureOAuth(provider)
+    const fetchSpy = jest.spyOn(global, 'fetch')
+
+    const urlRes = await request(app).get(provider.urlPath)
+    const callbackRes = await request(app)
+      .post(provider.callbackPath)
+      .set('Cookie', `${OAUTH_NONCE_COOKIE}=wrong-browser-nonce`)
+      .send({ code: 'attacker-code', state: stateFromUrl(urlRes.body.url) })
+
+    expect(callbackRes.status).toBe(400)
+    expect(callbackRes.body.error).toBe('Invalid or expired state')
+    expect(fetchSpy).not.toHaveBeenCalled()
+  })
+})