diff --git a/backend/__tests__/service/accountDeletion.test.js b/backend/__tests__/service/accountDeletion.test.js index a45cecb2eb..d558a5e8fa 100644 --- a/backend/__tests__/service/accountDeletion.test.js +++ b/backend/__tests__/service/accountDeletion.test.js @@ -211,12 +211,16 @@ describe('Self-serve account deletion', () => { } await deletion(app, issueSessionToken(user, now - 1, 0), { confirmation: 'DELETE' }).expect(401); // Real one-time OAuth exchange is the positive witness, not a fabricated claim. - await OAuthLoginState.create({ provider: 'github', state: 'deletion-state', userId: user._id, + const browserNonce = 'b'.repeat(64); + await OAuthLoginState.create({ provider: 'github', state: 'a'.repeat(48), userId: user._id, + browserNonceHash: createHash('sha256').update(browserNonce).digest('hex'), exchangeCode: 'deletion-exchange', exchangeExpiresAt: new Date(Date.now() + 60000), expiresAt: new Date(Date.now() + 60000) }); let session; - await oauthController.exchangeOAuthCode({ body: { code: 'deletion-exchange' } }, { - json: (body) => { session = body; }, status: () => { throw new Error('Exchange failed'); } }); + await oauthController.exchangeOAuthCode({ body: { code: 'deletion-exchange' }, + headers: { cookie: `cm-oauth-${'a'.repeat(48)}=${browserNonce}` } }, { + clearCookie: () => {}, json: (body) => { session = body; }, + status: () => { throw new Error('Exchange failed'); } }); expect(jwt.verify(session.token, process.env.JWT_SECRET).oauthAuthenticatedAt).toBeGreaterThanOrEqual(now); await deletion(app, session.token, { confirmation: 'DELETE' }).expect(204); }); diff --git a/backend/__tests__/service/oauthBrowserBinding.test.js b/backend/__tests__/service/oauthBrowserBinding.test.js new file mode 100644 index 0000000000..3b69e9a9a2 --- /dev/null +++ b/backend/__tests__/service/oauthBrowserBinding.test.js @@ -0,0 +1,170 @@ +const crypto = require('crypto'); +const express = require('express'); +const cors = require('cors'); +const request = require('supertest'); +const jwt = require('jsonwebtoken'); +const axios = require('axios'); +const User = require('../../models/User'); +const OAuthLoginState = require('../../models/OAuthLoginState'); +const { corsOptions } = require('../../middleware/corsPolicy'); +const { setupMongoDb, closeMongoDb, clearMongoDb } = require('../utils/testUtils'); + +jest.mock('axios', () => ({ post: jest.fn(), get: jest.fn() })); +jest.mock('../../services/emailService', () => ({ sendEmail: jest.fn().mockResolvedValue(undefined) })); +jest.mock('../../services/communityPodService', () => ({ ensureUserInCommunityPod: jest.fn().mockResolvedValue(undefined) })); + +const app = express(); +app.set('trust proxy', 1); +app.use(cors(corsOptions(['https://app.test.local']))); +app.use(express.json()); +app.use('/api/auth', require('../../routes/auth')); +const cookiePairs = (res) => (res.headers['set-cookie'] || []).map((value) => value.split(';')[0]); +const withBrowser = (call, cookies = []) => call.set('Host', 'api.test.local').set('X-Forwarded-Proto', 'https').set('Cookie', cookies); +const challengeFor = (verifier) => crypto.createHash('sha256').update(verifier).digest('base64url'); +const callback = (flow, state = flow.state) => `/api/auth/oauth/${flow.provider}/callback?state=${state}&code=provider-code`; +const start = async (provider) => { + const res = await withBrowser(request(app).get(`/api/auth/oauth/${provider}/start?next=/v2/pods/home`)).expect(302); + const url = new URL(res.headers.location); + return { provider, url, state: url.searchParams.get('state'), cookies: cookiePairs(res), res }; +}; +const stubProvider = (flow) => { + axios.post.mockImplementation(async (url, body) => { + const data = typeof body === 'string' ? Object.fromEntries(new URLSearchParams(body)) : body; + if (!data.code_verifier || challengeFor(data.code_verifier) !== flow.url.searchParams.get('code_challenge')) { + throw new Error('invalid_grant'); + } + expect(data.client_secret).toBe(`${flow.provider}-secret`); + expect(data.redirect_uri).toBe(`https://api.test.local/api/auth/oauth/${flow.provider}/callback`); + return { data: { access_token: 'synthetic-provider-token' } }; + }); + axios.get.mockImplementation(async (url) => ({ data: url.endsWith('/emails') + ? [{ email: 'person@example.com', primary: true, verified: true }] + : flow.provider === 'github' ? { id: 1234, login: 'person' } + : { sub: 'google-person', email: 'person@example.com', email_verified: true, name: 'person' } })); +}; + +describe('OAuth browser round trip over HTTP', () => { + beforeAll(setupMongoDb); + afterAll(closeMongoDb); + beforeEach(() => { + for (const provider of ['GITHUB', 'GOOGLE']) { + process.env[`${provider}_OAUTH_CLIENT_ID`] = `${provider.toLowerCase()}-client`; + process.env[`${provider}_OAUTH_CLIENT_SECRET`] = `${provider.toLowerCase()}-secret`; + } + process.env.FRONTEND_URL = 'https://app.test.local'; + delete process.env.OAUTH_CALLBACK_BASE_URL; + delete process.env.REGISTRATION_INVITE_ONLY; + delete process.env.AUTH_ALLOWED_EMAILS; + delete process.env.AUTH_ALLOWED_DOMAINS; + }); + afterEach(async () => { + await clearMongoDb(); + jest.clearAllMocks(); + for (const provider of ['GITHUB', 'GOOGLE']) { + delete process.env[`${provider}_OAUTH_CLIENT_ID`]; + delete process.env[`${provider}_OAUTH_CLIENT_SECRET`]; + } + delete process.env.FRONTEND_URL; + }); + + it.each(['github', 'google'])('completes %s across frontend/API origins with S256 and a single-use exchange', async (provider) => { + const flow = await start(provider); + const row = await OAuthLoginState.findOne({ state: flow.state }).select('+pkceVerifier'); + expect(flow.url.searchParams.get('code_challenge_method')).toBe('S256'); + expect(flow.url.searchParams.get('code_challenge')).toBe(challengeFor(row.pkceVerifier)); + expect(row.pkceVerifier).toMatch(/^[A-Za-z0-9_-]{43}$/); + expect(flow.url.href).not.toContain(row.pkceVerifier); + const cookie = flow.res.headers['set-cookie'][0]; + expect(cookie).toMatch(/^__Host-cm-oauth-/); + expect(cookie).toContain('HttpOnly'); + expect(cookie).toContain('Secure'); + expect(cookie).toContain('SameSite=Lax'); + expect(cookie).toContain('Path=/'); + expect(cookie).toContain('Max-Age=600'); + expect(cookie).not.toContain('Domain='); + const nonce = flow.cookies[0].split('=')[1]; + expect(row.browserNonceHash).toBe(crypto.createHash('sha256').update(nonce).digest('hex')); + expect(row.browserNonceHash).not.toBe(nonce); + stubProvider(flow); + const completed = await withBrowser(request(app).get(callback(flow)), flow.cookies).expect(302); + expect(completed.headers.location).toContain('https://app.test.local/v2/oauth/complete?'); + expect(completed.headers.location).not.toMatch(/synthetic-provider-token|code_verifier|cm-oauth/); + const code = new URL(completed.headers.location).searchParams.get('code'); + const denied = await withBrowser(request(app).post('/api/auth/oauth/exchange')).send({ code }).expect(400); + expect(denied.body.token).toBeUndefined(); + const wrongCookies = [flow.cookies[0].replace(/=.*/, `=${'0'.repeat(64)}`)]; + await withBrowser(request(app).post('/api/auth/oauth/exchange'), wrongCookies).send({ code }).expect(400); + expect((await OAuthLoginState.findOne({ state: flow.state })).exchangedAt).toBeNull(); + const exchanged = await withBrowser(request(app).post('/api/auth/oauth/exchange'), flow.cookies) + .set('Origin', 'https://app.test.local').send({ code }).expect(200); + expect(exchanged.headers['access-control-allow-origin']).toBe('https://app.test.local'); + expect(exchanged.headers['access-control-allow-credentials']).toBe('true'); + expect(jwt.verify(exchanged.body.token, process.env.JWT_SECRET).id).toBe(String((await User.findOne({ email: 'person@example.com' }))._id)); + expect(exchanged.headers['set-cookie'][0]).toContain(`${flow.cookies[0].split('=')[0]}=;`); + expect(exchanged.headers['set-cookie'][0]).toContain('Expires=Thu, 01 Jan 1970'); + expect((await OAuthLoginState.findOne({ state: flow.state }).select('+pkceVerifier')).pkceVerifier).toBeUndefined(); + await withBrowser(request(app).post('/api/auth/oauth/exchange'), flow.cookies).send({ code }).expect(400); + const replay = await withBrowser(request(app).get(callback(flow)), flow.cookies).expect(302); + expect(replay.headers.location).toContain('oauthError=state_invalid'); + expect(axios.post).toHaveBeenCalledTimes(1); + }); + + it.each(['missing cookie', 'wrong cookie', 'duplicate cookie', 'missing state', 'wrong state'])('refuses a callback with %s before provider calls or state consumption', async (invalid) => { + const flow = await start('google'); + let cookies = flow.cookies; + if (invalid === 'missing cookie') cookies = []; + if (invalid === 'wrong cookie') cookies = [flow.cookies[0].replace(/=.*/, `=${'0'.repeat(64)}`)]; + if (invalid === 'duplicate cookie') cookies = [...flow.cookies, ...flow.cookies]; + const url = invalid === 'missing state' ? '/api/auth/oauth/google/callback?code=x' + : callback(flow, invalid === 'wrong state' ? 'f'.repeat(48) : flow.state); + const res = await withBrowser(request(app).get(url), cookies).expect(302); + expect(res.headers.location).toContain('oauthError=state_invalid'); + expect(axios.post).not.toHaveBeenCalled(); + expect(axios.get).not.toHaveBeenCalled(); + expect(await User.countDocuments({ isBot: { $ne: true } })).toBe(0); + const row = await OAuthLoginState.findOne({ state: flow.state }); + expect(row.usedAt).toBeNull(); + expect(row.exchangeCode).toBeUndefined(); + }); + + it.each(['github', 'google'].flatMap((provider) => [[provider, 'mismatched'], [provider, 'missing']]))('refuses a %s attempt with a %s verifier', async (provider, kind) => { + const flow = await start(provider); + stubProvider(flow); + const missing = kind === 'missing'; + await OAuthLoginState.updateOne({ state: flow.state }, missing + ? { $unset: { pkceVerifier: 1 } } : { $set: { pkceVerifier: 'x'.repeat(43) } }); + const res = await withBrowser(request(app).get(callback(flow)), flow.cookies).expect(302); + expect(res.headers.location).toContain(`oauthError=${missing ? 'state_invalid' : 'provider_error'}`); + expect(await User.countDocuments({ isBot: { $ne: true } })).toBe(0); + expect((await OAuthLoginState.findOne({ state: flow.state })).exchangeCode).toBeUndefined(); + expect(axios.post).toHaveBeenCalledTimes(missing ? 0 : 1); + }); + + it('refuses an expired bound attempt without consuming it', async () => { + const flow = await start('google'); + // Keep the row present: exercise the time predicate rather than the TTL reaper. + await OAuthLoginState.updateOne({ state: flow.state }, { $set: { expiresAt: new Date(Date.now() - 1000) } }); + const res = await withBrowser(request(app).get(callback(flow)), flow.cookies).expect(302); + expect(res.headers.location).toContain('oauthError=state_invalid'); + expect(axios.post).not.toHaveBeenCalled(); + expect(await User.countDocuments({ isBot: { $ne: true } })).toBe(0); + expect((await OAuthLoginState.findOne({ state: flow.state })).usedAt).toBeNull(); + }); + + it('keeps parallel starts independent and refuses an old unbound attempt', async () => { + const first = await start('github'); + const second = await start('github'); + expect(first.cookies[0].split('=')[0]).not.toBe(second.cookies[0].split('=')[0]); + stubProvider(first); + await withBrowser(request(app).get(callback(first)), [...first.cookies, ...second.cookies]).expect(302); + expect((await OAuthLoginState.findOne({ state: second.state })).usedAt).toBeNull(); + stubProvider(second); + await withBrowser(request(app).get(callback(second)), [...first.cookies, ...second.cookies]).expect(302); + expect(await User.countDocuments({ email: 'person@example.com' })).toBe(1); + const legacy = await start('google'); + await OAuthLoginState.updateOne({ state: legacy.state }, { $unset: { browserNonceHash: 1, pkceVerifier: 1 } }); + const res = await withBrowser(request(app).get(callback(legacy)), legacy.cookies).expect(302); + expect(res.headers.location).toContain('oauthError=state_invalid'); + expect(axios.post).toHaveBeenCalledTimes(2); + }); +}); diff --git a/backend/__tests__/service/oauthNewAccount.test.js b/backend/__tests__/service/oauthNewAccount.test.js index b4d72cd7ec..9e82c2735f 100644 --- a/backend/__tests__/service/oauthNewAccount.test.js +++ b/backend/__tests__/service/oauthNewAccount.test.js @@ -70,12 +70,14 @@ tier1('new-account OAuth — Google and GitHub', () => { }); const start = await request(app).get(`/api/auth/oauth/${provider}/start?next=/v2`).expect(302); + // The browser cookie set at /start binds the callback and the exchange to this attempt. + const browserCookie = start.headers['set-cookie'].map((cookie) => cookie.split(';')[0]).join('; '); const upstream = new URL(start.headers.location); const state = upstream.searchParams.get('state'); expect(state).toBeTruthy(); expect(await OAuthLoginState.findOne({ provider, state })).toBeTruthy(); const callbackPath = `/api/auth/oauth/${provider}/callback?state=${encodeURIComponent(state)}&code=provider-code`; - const callback = await request(app).get(callbackPath).expect(302); + const callback = await request(app).get(callbackPath).set('Cookie', browserCookie).expect(302); const landing = new URL(callback.headers.location); expect({ origin: landing.origin, path: landing.pathname, next: landing.searchParams.get('next') }) .toEqual({ origin: 'http://localhost:3000', path: '/v2/oauth/complete', next: '/v2' }); @@ -95,7 +97,7 @@ tier1('new-account OAuth — Google and GitHub', () => { // This is the exact exchange and bearer token used by V2OAuthComplete // and AuthContext, not a JWT minted by the test fixture. - const session = await request(app).post('/api/auth/oauth/exchange').send({ code }).expect(200); + const session = await request(app).post('/api/auth/oauth/exchange').set('Cookie', browserCookie).send({ code }).expect(200); expect(session.body.token).toEqual(expect.any(String)); expect(jwt.verify(session.body.token, process.env.JWT_SECRET).id).toBe(String(created._id)); const protectedState = (row) => ({ @@ -117,11 +119,11 @@ tier1('new-account OAuth — Google and GitHub', () => { return membership.rows.length === 1 && welcome.rows.length > 0 && await Task.countDocuments({ podId: workspace._id, source: 'onboarding' }) === 3; }, { timeout: 15000 }); - const replay = await request(app).get(callbackPath).expect(302); + const replay = await request(app).get(callbackPath).set('Cookie', browserCookie).expect(302); expect(replay.headers.location).toContain('state_invalid'); expect(protectedState(await OAuthLoginState.findOne({ state }))).toEqual(stateSnapshot); expect(tokenPost).toHaveBeenCalledTimes(1); - await request(app).post('/api/auth/oauth/exchange').send({ code }).expect(400); + await request(app).post('/api/auth/oauth/exchange').set('Cookie', browserCookie).send({ code }).expect(400); expect(protectedState(await OAuthLoginState.findOne({ state }))).toEqual(stateSnapshot); expect(await User.countDocuments({ email })).toBe(1); expect(await Pod.countDocuments({ createdBy: created._id, name: 'My Workspace' })).toBe(1); diff --git a/backend/__tests__/service/signupSourceHttp.test.js b/backend/__tests__/service/signupSourceHttp.test.js index 7d8e4eae22..b3d5d71d2b 100644 --- a/backend/__tests__/service/signupSourceHttp.test.js +++ b/backend/__tests__/service/signupSourceHttp.test.js @@ -56,7 +56,7 @@ describe('signup source over the public HTTP auth surface', () => { ? [{ email: 'source-http@example.com', verified: true, primary: true }] : { id: 4245, login: 'source-http' } })); const callback = `/api/auth/oauth/github/callback?state=${state}&code=source-code`; - await request(app).get(`${callback}&signupSource=forged`).expect(302); + await request(app).get(`${callback}&signupSource=forged`).set('Cookie', response.headers['set-cookie'].map((cookie) => cookie.split(';')[0])).expect(302); const user = await User.findOne({ email: 'source-http@example.com' }).select('+signupSource'); expect(user.signupSource.authMethod).toBe('github'); expect(user.signupSource.ref).toBe('hn'); diff --git a/backend/__tests__/unit/controllers/oauthController.test.js b/backend/__tests__/unit/controllers/oauthController.test.js index da3bcf3541..38738991b9 100644 --- a/backend/__tests__/unit/controllers/oauthController.test.js +++ b/backend/__tests__/unit/controllers/oauthController.test.js @@ -1,3 +1,4 @@ +const crypto = require('crypto'); const axios = require('axios'); const jwt = require('jsonwebtoken'); const User = require('../../../models/User'); @@ -28,11 +29,17 @@ jest.mock('jsonwebtoken', () => ({ })); const mockRes = () => ({ + cookie: jest.fn(), + clearCookie: jest.fn(), status: jest.fn().mockReturnThis(), json: jest.fn(), redirect: jest.fn(), }); +const browserNonce = 'a'.repeat(64); +const browserNonceHash = crypto.createHash('sha256').update(browserNonce).digest('hex'); +const browserHeaders = (state) => ({ cookie: `__Host-cm-oauth-${state}=${browserNonce}` }); + const startReq = (provider, query = {}) => ({ params: { provider }, query, @@ -119,6 +126,7 @@ describe('OAuth Controller', () => { } const callbackRes = mockRes(); await oauthController.oauthCallback({ ...startReq(method), + headers: { cookie: startRes.cookie.mock.calls.map(([name, value]) => `${name}=${value}`).join('; ') }, query: { code: 'provider-code', state, signupSource: JSON.stringify({ ref: 'forged' }), ref: 'forged' } }, callbackRes); expect(callbackRes.redirect).toHaveBeenCalledWith(expect.stringContaining('/v2/oauth/complete?')); expect(callbackRes.redirect.mock.calls[0][0]).not.toMatch(/signupSource|firstSeenAt|search\.example|private/); @@ -270,14 +278,18 @@ describe('OAuth Controller', () => { describe('oauthCallback', () => { const seedState = (overrides = {}) => OAuthLoginState.create({ provider: 'github', - state: 'state-1', + browserNonceHash, + pkceVerifier: 'v'.repeat(43), + state: '111111111111111111111111111111111111111111111111', expiresAt: new Date(Date.now() + 60000), ...overrides, }); const callbackReq = (query = {}) => ({ params: { provider: 'github' }, - query: { code: 'gh-code', state: 'state-1', ...query }, + protocol: 'https', + headers: browserHeaders('111111111111111111111111111111111111111111111111'), + query: { code: 'gh-code', state: '111111111111111111111111111111111111111111111111', ...query }, protocol: 'https', get: () => 'api.test.local', }); @@ -334,7 +346,7 @@ describe('OAuth Controller', () => { await oauthController.oauthCallback(callbackReq(), res); expect(res.redirect).toHaveBeenCalledWith('https://app.test.local/v2/login?oauthError=team_only'); expect(await User.countDocuments({ email: 'octo@example.com' })).toBe(0); - expect((await OAuthLoginState.findOne({ state: 'state-1' })).exchangeCode).toBeUndefined(); + expect((await OAuthLoginState.findOne({ state: '111111111111111111111111111111111111111111111111' })).exchangeCode).toBeUndefined(); }); it('refuses an existing linked account whose current address is outside the allowlist', async () => { @@ -355,7 +367,7 @@ describe('OAuth Controller', () => { const res = mockRes(); await oauthController.oauthCallback(callbackReq(), res); expect(res.redirect).toHaveBeenCalledWith('https://app.test.local/v2/login?oauthError=email_unverified'); - expect((await OAuthLoginState.findOne({ state: 'state-1' })).exchangeCode).toBeUndefined(); + expect((await OAuthLoginState.findOne({ state: '111111111111111111111111111111111111111111111111' })).exchangeCode).toBeUndefined(); }); it.each([ @@ -419,7 +431,7 @@ describe('OAuth Controller', () => { const res = mockRes(); await oauthController.oauthCallback({ ...callbackReq(), params: { provider } }, res); expect(res.redirect).toHaveBeenCalledWith(expect.stringContaining('/v2/oauth/complete?')); - const state = await OAuthLoginState.findOne({ state: 'state-1' }); + const state = await OAuthLoginState.findOne({ state: '111111111111111111111111111111111111111111111111' }); expect(String(state.userId)).toBe(String(intended._id)); expect(state.exchangeCode).toEqual(expect.any(String)); expect(await User.countDocuments({ isBot: { $ne: true } })).toBe(2); @@ -452,7 +464,7 @@ describe('OAuth Controller', () => { expect(redirectUrl.searchParams.get('code')).toBeTruthy(); expect(redirectUrl.searchParams.get('inviteSignup')).toBeNull(); - const state = await OAuthLoginState.findOne({ state: 'state-1' }); + const state = await OAuthLoginState.findOne({ state: '111111111111111111111111111111111111111111111111' }); expect(String(state.userId)).toBe(String(user._id)); expect(state.usedAt).toBeTruthy(); }); @@ -679,7 +691,8 @@ describe('OAuth Controller', () => { }); await OAuthLoginState.create({ provider: 'github', - state: 'state-2', + state: '222222222222222222222222222222222222222222222222', + browserNonceHash, userId: user._id, exchangeCode: 'one-time-code', exchangeExpiresAt: new Date(Date.now() + 60000), @@ -688,14 +701,14 @@ describe('OAuth Controller', () => { }); const res1 = mockRes(); - await oauthController.exchangeOAuthCode({ body: { code: 'one-time-code' } }, res1); + await oauthController.exchangeOAuthCode({ protocol: 'https', headers: browserHeaders('222222222222222222222222222222222222222222222222'), body: { code: 'one-time-code' } }, res1); expect(res1.json).toHaveBeenCalledWith(expect.objectContaining({ token: 'test-jwt-token', user: expect.objectContaining({ username: 'octo-dev' }), })); const res2 = mockRes(); - await oauthController.exchangeOAuthCode({ body: { code: 'one-time-code' } }, res2); + await oauthController.exchangeOAuthCode({ protocol: 'https', headers: browserHeaders('222222222222222222222222222222222222222222222222'), body: { code: 'one-time-code' } }, res2); expect(res2.status).toHaveBeenCalledWith(400); }); @@ -712,7 +725,8 @@ describe('OAuth Controller', () => { }); await OAuthLoginState.create({ provider: 'github', - state: 'state-bot', + state: 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb', + browserNonceHash, userId: bot._id, exchangeCode: 'bot-code', exchangeExpiresAt: new Date(Date.now() + 60000), @@ -720,7 +734,7 @@ describe('OAuth Controller', () => { }); const res = mockRes(); - await oauthController.exchangeOAuthCode({ body: { code: 'bot-code' } }, res); + await oauthController.exchangeOAuthCode({ protocol: 'https', headers: browserHeaders('bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'), body: { code: 'bot-code' } }, res); expect(res.status).toHaveBeenCalledWith(403); expect(res.json).toHaveBeenCalledWith({ @@ -739,7 +753,8 @@ describe('OAuth Controller', () => { }); await OAuthLoginState.create({ provider: 'github', - state: 'state-3', + state: '333333333333333333333333333333333333333333333333', + browserNonceHash, userId: user._id, exchangeCode: 'stale-code', exchangeExpiresAt: new Date(Date.now() - 1000), @@ -747,7 +762,7 @@ describe('OAuth Controller', () => { }); const res = mockRes(); - await oauthController.exchangeOAuthCode({ body: { code: 'stale-code' } }, res); + await oauthController.exchangeOAuthCode({ protocol: 'https', headers: browserHeaders('333333333333333333333333333333333333333333333333'), body: { code: 'stale-code' } }, res); expect(res.status).toHaveBeenCalledWith(400); }); }); diff --git a/backend/controllers/oauthController.ts b/backend/controllers/oauthController.ts index ab53fc678c..14d830cc42 100644 --- a/backend/controllers/oauthController.ts +++ b/backend/controllers/oauthController.ts @@ -28,7 +28,8 @@ const joinCommunityPodBestEffort = (userId: any) => { // backend/integrations is a separate concern (agent integrations) and is // intentionally untouched. // -// Flow (frontend on a different origin than the API, so no cookies): +// Flow: the API holds a short-lived browser cookie; the frontend exchanges +// the completion code with credentials, including across same-site origins. // GET /api/auth/oauth/providers → which providers are configured // GET /api/auth/oauth/:provider/start → 302 to provider (state row minted) // GET /api/auth/oauth/:provider/callback → link-or-create user, then 302 to @@ -40,6 +41,24 @@ const joinCommunityPodBestEffort = (userId: any) => { const STATE_TTL_MS = 10 * 60 * 1000; // full provider round-trip budget const EXCHANGE_TTL_MS = 2 * 60 * 1000; // frontend picks this up immediately +// Per-attempt cookies keep independent login tabs from overwriting each other. +// __Host- disallows Domain cookies and requires Path=/ on HTTPS deployments. +const browserCookieOptions = (req: any) => ({ + httpOnly: true, secure: req.protocol === 'https', sameSite: 'lax' as const, path: '/', +}); +const browserCookieName = (state: string, req: any) => + `${req.protocol === 'https' ? '__Host-' : ''}cm-oauth-${state}`; +const nonceHash = (nonce: string) => crypto.createHash('sha256').update(nonce).digest('hex'); +const readBrowserHash = (req: any, state: string): string | null => { + if (!/^[a-f0-9]{48}$/.test(state)) return null; + const name = browserCookieName(state, req); + const matches = String(req.headers?.cookie || '').split(';').map((part) => part.trim()) + .filter((part) => part.startsWith(`${name}=`)); + if (matches.length !== 1) return null; + const nonce = matches[0].slice(name.length + 1); + return /^[a-f0-9]{64}$/.test(nonce) ? nonceHash(nonce) : null; +}; + interface ProviderProfile { providerId: string; email: string; @@ -53,7 +72,7 @@ const PROVIDERS: Record string | undefined; authorizeUrl: string; scope: string; - fetchProfile: (code: string, redirectUri: string) => Promise; + fetchProfile: (code: string, redirectUri: string, codeVerifier: string) => Promise; }> = { github: { label: 'GitHub', @@ -61,13 +80,14 @@ const PROVIDERS: Record process.env.GITHUB_OAUTH_CLIENT_SECRET, authorizeUrl: 'https://github.com/login/oauth/authorize', scope: 'read:user user:email', - async fetchProfile(code: string, redirectUri: string): Promise { + async fetchProfile(code: string, redirectUri: string, codeVerifier: string): Promise { const tokenRes = await axios.post( 'https://github.com/login/oauth/access_token', { client_id: process.env.GITHUB_OAUTH_CLIENT_ID, client_secret: process.env.GITHUB_OAUTH_CLIENT_SECRET, code, + code_verifier: codeVerifier, redirect_uri: redirectUri, }, { headers: { Accept: 'application/json' }, timeout: 15000 }, @@ -109,13 +129,14 @@ const PROVIDERS: Record process.env.GOOGLE_OAUTH_CLIENT_SECRET, authorizeUrl: 'https://accounts.google.com/o/oauth2/v2/auth', scope: 'openid email profile', - async fetchProfile(code: string, redirectUri: string): Promise { + async fetchProfile(code: string, redirectUri: string, codeVerifier: string): Promise { const tokenRes = await axios.post( 'https://oauth2.googleapis.com/token', new URLSearchParams({ client_id: process.env.GOOGLE_OAUTH_CLIENT_ID || '', client_secret: process.env.GOOGLE_OAUTH_CLIENT_SECRET || '', code, + code_verifier: codeVerifier, grant_type: 'authorization_code', redirect_uri: redirectUri, }).toString(), @@ -232,9 +253,13 @@ exports.startOAuth = async (req: any, res: any) => { } const state = crypto.randomBytes(24).toString('hex'); + const browserNonce = crypto.randomBytes(32).toString('hex'); + const pkceVerifier = crypto.randomBytes(32).toString('base64url'); await OAuthLoginState.create({ provider, state, + browserNonceHash: nonceHash(browserNonce), + pkceVerifier, invitationCode: String(input.invite || '').trim(), next: safeNextPath(input.next), signupRef: cleanSignupRef(input.ref) || '', @@ -250,6 +275,11 @@ exports.startOAuth = async (req: any, res: any) => { redirect_uri: callbackUrl(provider, req), scope: p.scope, state, + code_challenge_method: 'S256', + code_challenge: crypto.createHash('sha256').update(pkceVerifier).digest('base64url'), + }); + res.cookie(browserCookieName(state, req), browserNonce, { + ...browserCookieOptions(req), maxAge: STATE_TTL_MS, }); if (provider === 'google') params.set('response_type', 'code'); return res.redirect(`${p.authorizeUrl}?${params.toString()}`); @@ -266,24 +296,32 @@ exports.oauthCallback = async (req: any, res: any) => { if (!isProviderConfigured(provider)) return loginErrorRedirect(res, 'provider_error'); if (req.query.error) return loginErrorRedirect(res, 'provider_denied'); + const state = typeof req.query.state === 'string' ? req.query.state : ''; + const browserNonceHash = readBrowserHash(req, state); + if (!browserNonceHash) return loginErrorRedirect(res, 'state_invalid'); + // One-time state: atomically claim it so a replayed callback URL can't // mint a second session. const stateRow = await OAuthLoginState.findOneAndUpdate( { provider, - state: String(req.query.state || ''), + state, + browserNonceHash, + pkceVerifier: { $regex: /^[A-Za-z0-9_-]{43}$/ }, usedAt: null, expiresAt: { $gt: new Date() }, }, { $set: { usedAt: new Date() } }, { new: true }, - ); + ).select('+pkceVerifier'); if (!stateRow) return loginErrorRedirect(res, 'state_invalid'); const profile = await PROVIDERS[provider].fetchProfile( String(req.query.code || ''), callbackUrl(provider, req), + stateRow.pkceVerifier, ); + stateRow.pkceVerifier = undefined; if (!profile.providerId) return loginErrorRedirect(res, 'provider_error'); // Linking by email is only safe when the provider vouches for it. if (!profile.email || !profile.emailVerified) return loginErrorRedirect(res, 'email_unverified'); @@ -385,10 +423,17 @@ exports.exchangeOAuthCode = async (req: any, res: any) => { const code = String(req.body?.code || ''); if (!code) return res.status(400).json({ error: 'Missing exchange code.' }); + const pending = await OAuthLoginState.findOne({ exchangeCode: code }); + const browserNonceHash = pending && readBrowserHash(req, pending.state); + if (!browserNonceHash) { + return res.status(400).json({ error: 'Invalid or expired exchange code.', code: 'EXCHANGE_INVALID' }); + } + // Atomic claim — the code is single-use. const stateRow = await OAuthLoginState.findOneAndUpdate( { exchangeCode: code, + browserNonceHash, exchangedAt: null, exchangeExpiresAt: { $gt: new Date() }, }, @@ -411,6 +456,7 @@ exports.exchangeOAuthCode = async (req: any, res: any) => { }); } + res.clearCookie(browserCookieName(stateRow.state, req), browserCookieOptions(req)); return res.json(issueSession(user)); } catch (err: any) { console.error('OAuth exchange failed:', err.message); diff --git a/backend/models/OAuthLoginState.ts b/backend/models/OAuthLoginState.ts index 7c6ae4f610..3be7ba7ae7 100644 --- a/backend/models/OAuthLoginState.ts +++ b/backend/models/OAuthLoginState.ts @@ -16,6 +16,8 @@ export type OAuthLoginProvider = 'github' | 'google'; export interface IOAuthLoginState extends Document { provider: OAuthLoginProvider; state: string; + browserNonceHash?: string; + pkceVerifier?: string; // Optional invite code captured at /start so an OAuth signup can satisfy // the invite-only gate without a form round-trip. invitationCode?: string; @@ -38,6 +40,8 @@ const OAuthLoginStateSchema = new Schema( { provider: { type: String, required: true, enum: ['github', 'google'], index: true }, state: { type: String, required: true, unique: true, index: true }, + browserNonceHash: { type: String }, + pkceVerifier: { type: String, select: false }, invitationCode: { type: String, default: '' }, next: { type: String, default: '/v2' }, signupRef: { type: String, default: '' }, diff --git a/frontend/src/v2/__tests__/V2OAuth.test.tsx b/frontend/src/v2/__tests__/V2OAuth.test.tsx index e5b12a9035..982afd9b3e 100644 --- a/frontend/src/v2/__tests__/V2OAuth.test.tsx +++ b/frontend/src/v2/__tests__/V2OAuth.test.tsx @@ -218,7 +218,7 @@ describe('V2OAuthComplete', () => { }); expect(await screen.findByText('pod page REPLACE')).toBeInTheDocument(); - expect(axios.post).toHaveBeenCalledWith('/api/auth/oauth/exchange', { code: 'one-time' }); + expect(axios.post).toHaveBeenCalledWith('/api/auth/oauth/exchange', { code: 'one-time' }, { withCredentials: true }); expect(signInWithToken).toHaveBeenCalledWith('session-jwt'); expect(locationSpies.replace).not.toHaveBeenCalled(); expect(locationSpies.assign).not.toHaveBeenCalled(); diff --git a/frontend/src/v2/components/V2OAuthComplete.tsx b/frontend/src/v2/components/V2OAuthComplete.tsx index 9e2a53fcb0..d5c19fe700 100644 --- a/frontend/src/v2/components/V2OAuthComplete.tsx +++ b/frontend/src/v2/components/V2OAuthComplete.tsx @@ -42,7 +42,7 @@ const V2OAuthComplete: React.FC = () => { return; } - axios.post('/api/auth/oauth/exchange', { code }) + axios.post('/api/auth/oauth/exchange', { code }, { withCredentials: true }) .then((res) => { const token = (res.data as { token?: string })?.token; if (!token) throw new Error('no token');