From ad8764e205fbe5331ce4505651dd284879a4f09b Mon Sep 17 00:00:00 2001 From: mertcano <35747700+mertcano@users.noreply.github.com> Date: Sun, 16 Aug 2026 12:50:33 +0300 Subject: [PATCH] Security: Prevent OAuth CSRF, open redirect, and account takeover This PR addresses a HIGH-severity vulnerability in the Spotify OAuth connector where the state parameter was unsigned and reusable, and the callback trusted client-supplied user IDs and redirect paths. The flow has been secured with session-bound nonces and strict path validation to prevent account-linking confusion and arbitrary external redirects. CSRF Protection: Replaced the static, unsigned state payload with a short-lived, single-use nonce stored in a secure, HTTP-only cookie (spotify-oauth-nonce). The callback now strictly verifies this nonce to bind the OAuth flow to the active browser session. Account Takeover Prevention: Removed reliance on the userId decoded from the OAuth state parameter. The callback now explicitly re-validates the current user's session (req.cookies.get('realism-session')) to ensure connector credentials are only bound to the genuinely authenticated user. Open Redirect Prevention: Enforced strict relative-path validation for the returnTo parameter in both the initialization and callback routes. Absolute URLs and protocol-relative paths (e.g., //evil.com) are securely downgraded to the default /dashboard route. --- .../app/api/connectors/spotify/auth/route.ts | 112 +++++---- .../api/connectors/spotify/callback/route.ts | 224 ++++++++++-------- 2 files changed, 196 insertions(+), 140 deletions(-) diff --git a/Realism/app/api/connectors/spotify/auth/route.ts b/Realism/app/api/connectors/spotify/auth/route.ts index 3ec4c70..833d68e 100644 --- a/Realism/app/api/connectors/spotify/auth/route.ts +++ b/Realism/app/api/connectors/spotify/auth/route.ts @@ -1,43 +1,69 @@ -import { NextRequest, NextResponse } from 'next/server' -import { validateSession } from '@/lib/auth' - -export async function GET(req: NextRequest) { - const token = req.cookies.get('realism-session')?.value - if (!token) { - return NextResponse.redirect(new URL('/?auth=required', req.url)) - } - - const userId = await validateSession(token) - if (!userId) { - return NextResponse.redirect(new URL('/?auth=required', req.url)) - } - - const clientId = process.env.SPOTIFY_CLIENT_ID - if (!clientId) { - return NextResponse.json( - { error: 'Spotify is not configured on this server' }, - { status: 503 } - ) - } - - const returnTo = req.nextUrl.searchParams.get('returnTo') ?? '/dashboard' - const state = Buffer.from(`${userId}:${returnTo}`).toString('base64url') - - const scopes = [ - 'user-top-read', - 'user-read-recently-played', - 'user-read-private', - ].join(' ') - - const params = new URLSearchParams({ - client_id: clientId, - response_type: 'code', - redirect_uri: `${process.env.NEXT_PUBLIC_APP_URL}/api/connectors/spotify/callback`, - scope: scopes, - state, - }) - - return NextResponse.redirect( - `https://accounts.spotify.com/authorize?${params}` - ) -} +import { NextRequest, NextResponse } from 'next/server' +import { validateSession } from '@/lib/auth' + +export async function GET(req: NextRequest) { + const token = req.cookies.get('realism-session')?.value + if (!token) { + return NextResponse.redirect(new URL('/?auth=required', req.url)) + } + + const userId = await validateSession(token) + if (!userId) { + return NextResponse.redirect(new URL('/?auth=required', req.url)) + } + + const clientId = process.env.SPOTIFY_CLIENT_ID + if (!clientId) { + return NextResponse.json( + { error: 'Spotify is not configured on this server' }, + { status: 503 } + ) + } + + let returnTo = req.nextUrl.searchParams.get('returnTo') ?? '/dashboard' + + // SECURITY FIX: Open Redirect Prevention. + // Ensure the returnTo path is strictly a relative local path. + // Reject absolute URLs or protocol-relative URLs (//evil.com). + if (!returnTo.startsWith('/') || returnTo.startsWith('//')) { + returnTo = '/dashboard' + } + + // SECURITY FIX: CSRF Prevention. + // Generate a random nonce to securely bind the OAuth flow to the current browser session. + // We no longer expose the userId in the unsigned state parameter to prevent spoofing. + const nonce = crypto.randomUUID() + const statePayload = JSON.stringify({ returnTo, nonce }) + const state = Buffer.from(statePayload).toString('base64url') + + const scopes = [ + 'user-top-read', + 'user-read-recently-played', + 'user-read-private', + ].join(' ') + + const params = new URLSearchParams({ + client_id: clientId, + response_type: 'code', + redirect_uri: `${process.env.NEXT_PUBLIC_APP_URL}/api/connectors/spotify/callback`, + scope: scopes, + state, + }) + + const response = NextResponse.redirect( + `https://accounts.spotify.com/authorize?${params}` + ) + + // SECURITY FIX: Set the nonce in a short-lived, HTTP-only, secure cookie. + response.cookies.set({ + name: 'spotify-oauth-nonce', + value: nonce, + httpOnly: true, + secure: process.env.NODE_ENV === 'production', + sameSite: 'lax', + path: '/', + maxAge: 600, // Valid for 10 minutes + }) + + return response +} \ No newline at end of file diff --git a/Realism/app/api/connectors/spotify/callback/route.ts b/Realism/app/api/connectors/spotify/callback/route.ts index 549b798..ba5d652 100644 --- a/Realism/app/api/connectors/spotify/callback/route.ts +++ b/Realism/app/api/connectors/spotify/callback/route.ts @@ -1,97 +1,127 @@ -import { NextRequest, NextResponse } from 'next/server' -import { setConnectorCredentials } from '@/lib/live-apps' - -export async function GET(req: NextRequest) { - const { searchParams } = req.nextUrl - const code = searchParams.get('code') - const state = searchParams.get('state') - const error = searchParams.get('error') - - if (error) { - return NextResponse.redirect( - new URL('/dashboard?connector_error=spotify_denied', req.url) - ) - } - - if (!code || !state) { - return NextResponse.redirect( - new URL('/dashboard?connector_error=spotify_invalid', req.url) - ) - } - - let userId: string - let returnTo: string - try { - const decoded = Buffer.from(state, 'base64url').toString() - const colonIndex = decoded.indexOf(':') - userId = decoded.slice(0, colonIndex) - returnTo = decoded.slice(colonIndex + 1) || '/dashboard' - } catch { - return NextResponse.redirect( - new URL('/dashboard?connector_error=spotify_state', req.url) - ) - } - - if (!userId) { - return NextResponse.redirect( - new URL('/dashboard?connector_error=spotify_state', req.url) - ) - } - - const clientId = process.env.SPOTIFY_CLIENT_ID - const clientSecret = process.env.SPOTIFY_CLIENT_SECRET - - if (!clientId || !clientSecret) { - return NextResponse.redirect( - new URL('/dashboard?connector_error=spotify_config', req.url) - ) - } - - let tokenData: { - access_token: string - refresh_token: string - expires_in: number - token_type: string - } - - try { - const tokenRes = await fetch('https://accounts.spotify.com/api/token', { - method: 'POST', - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - 'Authorization': `Basic ${Buffer.from(`${clientId}:${clientSecret}`).toString('base64')}`, - }, - body: new URLSearchParams({ - grant_type: 'authorization_code', - code, - redirect_uri: `${process.env.NEXT_PUBLIC_APP_URL}/api/connectors/spotify/callback`, - }), - }) - - if (!tokenRes.ok) { - const errText = await tokenRes.text() - console.error('[spotify/callback] Token exchange failed:', errText) - return NextResponse.redirect( - new URL('/dashboard?connector_error=spotify_token', req.url) - ) - } - - tokenData = await tokenRes.json() - } catch (err) { - console.error('[spotify/callback] Token exchange error:', err) - return NextResponse.redirect( - new URL('/dashboard?connector_error=spotify_token', req.url) - ) - } - - await setConnectorCredentials(userId, 'spotify', { - accessToken: tokenData.access_token, - refreshToken: tokenData.refresh_token, - expiresAt: new Date(Date.now() + tokenData.expires_in * 1000).toISOString(), - }) - - const redirectUrl = new URL(returnTo, req.url) - redirectUrl.searchParams.set('connector_connected', 'spotify') - - return NextResponse.redirect(redirectUrl) -} +import { NextRequest, NextResponse } from 'next/server' +import { setConnectorCredentials } from '@/lib/live-apps' +import { validateSession } from '@/lib/auth' + +export async function GET(req: NextRequest) { + const { searchParams } = req.nextUrl + const code = searchParams.get('code') + const state = searchParams.get('state') + const error = searchParams.get('error') + + if (error) { + return NextResponse.redirect( + new URL('/dashboard?connector_error=spotify_denied', req.url) + ) + } + + if (!code || !state) { + return NextResponse.redirect( + new URL('/dashboard?connector_error=spotify_invalid', req.url) + ) + } + + // SECURITY FIX: Account Takeover & Identity Confusion Prevention. + // We do NOT trust the userId from the OAuth state parameter. + // Instead, we strictly re-validate the current user's session to ensure + // the credentials are bound to the genuinely authenticated user. + const sessionToken = req.cookies.get('realism-session')?.value + if (!sessionToken) { + return NextResponse.redirect(new URL('/?auth=required', req.url)) + } + + const userId = await validateSession(sessionToken) + if (!userId) { + return NextResponse.redirect(new URL('/?auth=required', req.url)) + } + + let returnTo = '/dashboard' + let nonce = '' + + try { + const decoded = Buffer.from(state, 'base64url').toString() + const payload = JSON.parse(decoded) + returnTo = payload.returnTo || '/dashboard' + nonce = payload.nonce || '' + } catch { + return NextResponse.redirect( + new URL('/dashboard?connector_error=spotify_state', req.url) + ) + } + + // SECURITY FIX: CSRF Prevention. + // Verify that the nonce in the state payload strictly matches the nonce cookie. + const cookieNonce = req.cookies.get('spotify-oauth-nonce')?.value + if (!cookieNonce || nonce !== cookieNonce) { + return NextResponse.redirect( + new URL('/dashboard?connector_error=spotify_state_mismatch', req.url) + ) + } + + // SECURITY FIX: Open Redirect Prevention. + // Ensure the decoded returnTo path is strictly a relative local path. + if (!returnTo.startsWith('/') || returnTo.startsWith('//')) { + returnTo = '/dashboard' + } + + const clientId = process.env.SPOTIFY_CLIENT_ID + const clientSecret = process.env.SPOTIFY_CLIENT_SECRET + + if (!clientId || !clientSecret) { + return NextResponse.redirect( + new URL('/dashboard?connector_error=spotify_config', req.url) + ) + } + + let tokenData: { + access_token: string + refresh_token: string + expires_in: number + token_type: string + } + + try { + const tokenRes = await fetch('https://accounts.spotify.com/api/token', { + method: 'POST', + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + 'Authorization': `Basic ${Buffer.from(`${clientId}:${clientSecret}`).toString('base64')}`, + }, + body: new URLSearchParams({ + grant_type: 'authorization_code', + code, + redirect_uri: `${process.env.NEXT_PUBLIC_APP_URL}/api/connectors/spotify/callback`, + }), + }) + + if (!tokenRes.ok) { + const errText = await tokenRes.text() + console.error('[spotify/callback] Token exchange failed:', errText) + return NextResponse.redirect( + new URL('/dashboard?connector_error=spotify_token', req.url) + ) + } + + tokenData = await tokenRes.json() + } catch (err) { + console.error('[spotify/callback] Token exchange error:', err) + return NextResponse.redirect( + new URL('/dashboard?connector_error=spotify_token', req.url) + ) + } + + await setConnectorCredentials(userId, 'spotify', { + accessToken: tokenData.access_token, + refreshToken: tokenData.refresh_token, + expiresAt: new Date(Date.now() + tokenData.expires_in * 1000).toISOString(), + }) + + const redirectUrl = new URL(returnTo, req.url) + redirectUrl.searchParams.set('connector_connected', 'spotify') + + const response = NextResponse.redirect(redirectUrl) + + // SECURITY FIX: Clean up the nonce cookie after successful authentication to prevent reuse. + response.cookies.delete('spotify-oauth-nonce') + + return response +} \ No newline at end of file