Skip to content

Commit 0d43ebc

Browse files
fix(connect): validate OAuth state nonce via Redis to prevent CSRFThe /api/connect/github OAuth flow generated a nonce but never storedor verified it, allowing an attacker to forge a state parameter andattach a GitHub token to an arbitrary user account.Fix:- On initiation, store the nonce in Redis as oauth:nonce:<nonce> with the userId as value and a 10-minute TTL- On callback, look up the nonce in Redis and reject if missing or if the stored userId does not match the decoded state- Consume (delete) the nonce after verification — one-time use onlyFixes #248 (#283)
1 parent ed63811 commit 0d43ebc

1 file changed

Lines changed: 36 additions & 21 deletions

File tree

apps/backend/src/routes/connect.ts

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -33,27 +33,34 @@ export async function connectRoutes(app: FastifyInstance) {
3333

3434
// ─── GitHub Connect ───
3535

36-
app.get('/github', {
37-
preHandler: [app.authenticate],
38-
}, async (request: FastifyRequest, reply: FastifyReply) => {
39-
// Generate a secure state token linking back to this user session
40-
// In a real app, store this in Redis to cross-check in callback
41-
const state = JSON.stringify({
42-
userId: (request.user as any).id,
43-
nonce: generateState(),
44-
});
45-
46-
const redirectUri = `${process.env.BACKEND_URL}/api/connect/github/callback`;
47-
const params = new URLSearchParams({
48-
client_id: process.env.GITHUB_CLIENT_ID || '',
49-
redirect_uri: redirectUri,
50-
scope: 'user:follow', // ONLY asking for follow scope to avoid full profile access
51-
state: Buffer.from(state).toString('base64'),
52-
});
53-
54-
return reply.redirect(`${GITHUB_AUTH_URL}?${params}`);
36+
app.get('/github', {
37+
preHandler: [app.authenticate],
38+
}, async (request: FastifyRequest, reply: FastifyReply) => {
39+
const userId = (request.user as any).id;
40+
const nonce = generateState();
41+
42+
// Store nonce in Redis with 10-minute TTL.
43+
// The callback verifies this to prevent CSRF attacks.
44+
await app.redis.set(
45+
`oauth:nonce:${nonce}`,
46+
userId,
47+
'EX',
48+
600
49+
);
50+
51+
const state = JSON.stringify({ userId, nonce });
52+
53+
const redirectUri = `${process.env.BACKEND_URL}/api/connect/github/callback`;
54+
const params = new URLSearchParams({
55+
client_id: process.env.GITHUB_CLIENT_ID || '',
56+
redirect_uri: redirectUri,
57+
scope: 'user:follow',
58+
state: Buffer.from(state).toString('base64'),
5559
});
5660

61+
return reply.redirect(`${GITHUB_AUTH_URL}?${params}`);
62+
});
63+
5764
app.get('/github/callback', async (request: FastifyRequest<{ Querystring: OAuthCallbackQuery }>, reply: FastifyReply) => {
5865
const { code, state } = request.query;
5966

@@ -68,12 +75,20 @@ export async function connectRoutes(app: FastifyInstance) {
6875
if (!decodedState) {
6976
return reply.redirect(`${process.env.PUBLIC_APP_URL}/settings?error=connect_failed`);
7077
}
71-
const userId = decodedState.userId;
7278

73-
if (!userId) {
79+
// Verify nonce was issued by this server — prevents CSRF
80+
const storedUserId = await app.redis.get(`oauth:nonce:${decodedState.nonce}`);
81+
82+
if (!storedUserId || storedUserId !== decodedState.userId) {
83+
app.log.warn({ nonce: decodedState.nonce }, 'OAuth CSRF check failed — nonce mismatch');
7484
return reply.redirect(`${process.env.PUBLIC_APP_URL}/settings?error=invalid_state`);
7585
}
7686

87+
// Consume the nonce — one-time use only
88+
await app.redis.del(`oauth:nonce:${decodedState.nonce}`);
89+
90+
const userId = decodedState.userId;
91+
7792
// Exchange code for token
7893
const tokenRes = await fetch(GITHUB_TOKEN_URL, {
7994
method: 'POST',

0 commit comments

Comments
 (0)