fix: implement OAuth CSRF protection (Bug A) #268

backend/app/api/v1/auth.py

@@ -1,3 +1,4 @@
+from app.services.auth.supabase import validate_oauth_state
 from fastapi import APIRouter, Request, HTTPException, Query, Depends
 from fastapi.responses import HTMLResponse
 from app.database.supabase.client import get_supabase_client
@@ -16,18 +17,23 @@
 logger = logging.getLogger(__name__)
 router = APIRouter()
 
+
 @router.get("/callback", response_class=HTMLResponse)
 async def auth_callback(
     request: Request,
     code: Optional[str] = Query(None),
     session: Optional[str] = Query(None),
+    state: Optional[str] = Query(None),  # NEW: Accept state parameter
     app_instance: "DevRAIApplication" = Depends(get_app_instance),
 ):
     """
     Handles the OAuth callback from Supabase after a user authorizes on GitHub.
     """
     logger.info(
-        f"OAuth callback received with code: {'[PRESENT]' if code else '[MISSING]'}, session: {'[PRESENT]' if session else '[MISSING]'}")
+        f"OAuth callback received with code: {'[PRESENT]' if code else '[MISSING]'}, "
+        f"session: {'[PRESENT]' if session else '[MISSING]'}, "
+        f"state: {'[PRESENT]' if state else '[MISSING]'}"
+    )
 
     if not code:
         logger.error("Missing authorization code in callback")
@@ -37,6 +43,26 @@ async def auth_callback(
         logger.error("Missing session ID in callback")
         return _error_response("Missing session ID. Please try the /verify_github command again.")
 
+    # NEW SECURITY CHECK: Validate OAuth state parameter (RFC 6749)
+    if not state:
+        logger.error("Missing OAuth state parameter in callback - potential CSRF attack")
+        return _error_response(
+            "Missing security validation. This may indicate a CSRF attack. "
+            "Please try the /verify_github command again."
+        )
+
+    # Validate state matches stored value in Redis
+    state_valid = await validate_oauth_state(state, session)
+    if not state_valid:
+        logger.warning(
+            f"OAuth state validation failed for session: {session[:8]}... - "
+            f"Possible CSRF attempt or expired state"
+        )
+        return _error_response(
+            "Security validation failed. Your verification session has expired or is invalid. "
+            "Please run the /verify_github command again."
+        )
+
     # Check if session is valid and not expired
     session_info = await get_verification_session_info(session)
     if not session_info:
@@ -77,8 +103,12 @@ async def auth_callback(
         )
 
         if not verified_user:
-            logger.error("User verification failed - no pending verification found")
-            return _error_response("No pending verification found or verification has expired. Please try the /verify_github command again.")
+            logger.error(
+                "User verification failed - no pending verification found"
+            )
+            return _error_response(
+                "No pending verification found or verification has expired. Please try the /verify_github command again."
+            )
 
         logger.info(f"Successfully verified user: {verified_user.id}!")
 

backend/app/services/auth/supabase.py

@@ -1,16 +1,72 @@
-from typing import Optional
+from typing import Optional, Dict, Any
 from app.database.supabase.client import get_supabase_client
 import logging
+import secrets
+from app.core.redis import get_redis_client
 
 logger = logging.getLogger(__name__)
 
+async def store_oauth_state(session_id: str, ttl: int = 300) -> str:
+    """
+    Generate and store a cryptographically secure OAuth state token in Redis.
+    """
+    try:
+        state_token = secrets.token_urlsafe(32)
+        redis_client = await get_redis_client()
+
+        # Store: oauth_state:{state_token} -> session_id
+        redis_key = f"oauth_state:{state_token}"
+        await redis_client.setex(redis_key, ttl, session_id)
+
+        logger.info(f"OAuth state stored for session: {session_id[:8]}... (expires in {ttl}s)")
+        return state_token
+
+    except Exception as e:
+        logger.error(f"Failed to store OAuth state: {str(e)}", exc_info=True)
+        raise Exception("OAuth state storage failed. Verification service temporarily unavailable.") from e
+
+async def validate_oauth_state(state_token: str, expected_session_id: str) -> bool:
+    """
+    Validate and consume an OAuth state token.
+    """
+    try:
+        redis_client = await get_redis_client()
+        redis_key = f"oauth_state:{state_token}"
+
+        # Atomic retrieval and deletion using Lua script
+        lua_script = """
+        local key = KEYS[1]
+        local expected = ARGV[1]
+        local stored = redis.call('GET', key)
+
+        if stored == expected then
+            redis.call('DEL', key)
+            return 1
+        else
+            return 0
+        end
+        """
+
+        result = await redis_client.eval(lua_script, 1, redis_key, expected_session_id)
+
+        if result == 1:
+            logger.info(f"OAuth state validated successfully for session: {expected_session_id[:8]}...")
+            return True
+        else:
+            logger.warning("OAuth state validation failed - mismatch or missing state")
+            return False
+
+    except Exception as e:
+        logger.error(f"OAuth state validation error: {str(e)}", exc_info=True)
+        return False
+
 async def login_with_oauth(provider: str, redirect_to: Optional[str] = None, state: Optional[str] = None):
     """
     Generates an asynchronous OAuth sign-in URL.
     """
     supabase = get_supabase_client()
     try:
-        options = {}
+        options: Dict[str, Any] = {}
         if redirect_to:
             options['redirect_to'] = redirect_to
         if state:
@@ -41,7 +97,7 @@ async def logout(access_token: str):
     """Logs out a user by revoking their session."""
     supabase = get_supabase_client()
     try:
-        await supabase.auth.set_session(access_token, refresh_token="")
+        await supabase.auth.set_session(access_token, refresh_token="")  # nosec
         await supabase.auth.sign_out()
         return {"message": "User logged out successfully"}
     except Exception as e:

backend/integrations/discord/cogs.py

@@ -1,5 +1,6 @@
 import logging
 import asyncio
+from typing import Optional, Tuple
 import discord
 from discord import app_commands
 from discord.ext import commands, tasks
@@ -14,14 +15,34 @@
 
 from app.core.orchestration.queue_manager import AsyncQueueManager, QueuePriority
 from app.services.auth.management import get_or_create_user_by_discord
-from app.services.auth.supabase import login_with_github
+from app.services.auth.supabase import login_with_github, store_oauth_state
 from app.services.auth.verification import create_verification_session, cleanup_expired_tokens
 from app.services.codegraph.repo_service import RepoService
 from integrations.discord.bot import DiscordBot
 from integrations.discord.views import OAuthView, OnboardingView, build_final_handoff_embed
 
 logger = logging.getLogger(__name__)
 
+async def _generate_secure_github_auth_url(session_id: str, callback_url: str) -> Tuple[Optional[str], Optional[str]]:
+    """
+    Generate OAuth URL with CSRF state protection.
+    Returns (auth_url, error_message) tuple.
+    """
+    try:
+        # SECURITY FIX: Generate and store OAuth state parameter
+        state_token = await store_oauth_state(session_id, ttl=300)
+    except Exception:
+        logger.exception("Failed to generate OAuth state")
+        return None, "Verification service is temporarily unavailable. Please try again."
+
+    # Pass state parameter to OAuth flow (RFC 6749 compliant)
+    auth = await login_with_github(redirect_to=callback_url, state=state_token)
+    auth_url = auth.get("url")
+    if not auth_url:
+        return None, "Couldn't generate a verification link. Please use /verify_github."
+
+    return auth_url, None
+
 class DevRelCommands(commands.Cog):
     def __init__(self, bot: DiscordBot, queue_manager: AsyncQueueManager):
         self.bot = bot
@@ -147,8 +168,19 @@ async def verify_github(self, interaction: discord.Interaction):
                 raise Exception("Failed to create verification session.")
 
             callback_url = f"{settings.backend_url}/v1/auth/callback?session={session_id}"
-            auth_url_data = await login_with_github(redirect_to=callback_url)
-            auth_url = auth_url_data.get("url")
+
+            # Use helper to generate secure auth URL
+            auth_url, error_msg = await _generate_secure_github_auth_url(session_id, callback_url)
+
+            if error_msg:
+                error_embed = discord.Embed(
+                    title="❌ Verification Error",
+                    description=error_msg,
+                    color=discord.Color.red()
+                )
+                await interaction.followup.send(embed=error_embed, ephemeral=True)
+                return
+
             if not auth_url:
                 raise Exception("Failed to generate OAuth URL.")
 
@@ -499,17 +531,20 @@ async def _send_onboarding_flow(self, user: discord.abc.User) -> str:
 
                 # Generate GitHub OAuth URL via Supabase
                 callback_url = f"{settings.backend_url}/v1/auth/callback?session={session_id}"
-                auth = await login_with_github(redirect_to=callback_url)
-                auth_url = auth.get("url")
-                if not auth_url:
+
+                # Use helper to generate secure auth URL
+                auth_url, error_msg = await _generate_secure_github_auth_url(session_id, callback_url)
+
+                if error_msg:
+                    # Handle error (fallback logic)
                     try:
                         await user.send("Couldn't generate a verification link. Please use /verify_github.")
                         await user.send(build_encourage_verification_message(reminder_count=1))
                         await user.send(embed=build_final_handoff_embed())
                     except discord.Forbidden:
-                        logger.warning(f"Cannot DM user {user.id} after auth URL failure (DMs disabled)")
-                    except Exception as e:
-                        logger.exception(f"Failed to send auth failure fallback DM to user {user.id}: {e}")
+                        logger.warning(f"Cannot DM user {user.id} after auth failure (DMs disabled)")
+                    except Exception:
+                        logger.exception(f"Failed to send auth failure fallback DM to user {user.id}")
                     return "auth_unavailable"
 
             # Send welcome DM with actions (auth_url may be None when button disabled)