Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion backend/app/api/v1/auth.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
from fastapi import APIRouter, Request, HTTPException, Query, Depends
from fastapi.responses import HTMLResponse
from app.database.supabase.client import get_supabase_client
from app.services.auth.verification import find_user_by_session_and_verify, get_verification_session_info
from app.services.auth.verification import (
find_user_by_session_and_verify,
get_verification_session_info,
validate_oauth_state,
)
from app.services.github.user.profiling import profile_user_from_github
from typing import Optional
import logging
Expand All @@ -21,6 +25,7 @@ async def auth_callback(
request: Request,
code: Optional[str] = Query(None),
session: Optional[str] = Query(None),
state: Optional[str] = Query(None),
app_instance: "DevRAIApplication" = Depends(get_app_instance),
):
"""
Expand All @@ -37,6 +42,12 @@ async def auth_callback(
logger.error("Missing session ID in callback")
return _error_response("Missing session ID. Please try the /verify_github command again.")

# Validate the OAuth state parameter to protect against login CSRF (RFC 6749).
# The state is bound to the verification session when the OAuth URL is created.
if not validate_oauth_state(session, state):
logger.error(f"OAuth state validation failed for session ID: {session}")
return _error_response("Invalid or missing security token. 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:
Expand Down
54 changes: 44 additions & 10 deletions backend/app/services/auth/verification.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import uuid
import secrets
from datetime import datetime, timedelta
from typing import Optional, Dict, Tuple
from app.database.supabase.client import get_supabase_client
Expand All @@ -7,8 +8,8 @@

logger = logging.getLogger(__name__)

# session_id -> (discord_id, expiry_time)
_verification_sessions: Dict[str, Tuple[str, datetime]] = {}
# session_id -> (discord_id, expiry_time, oauth_state)
_verification_sessions: Dict[str, Tuple[str, datetime, str]] = {}

SESSION_EXPIRY_MINUTES = 5

Expand All @@ -18,28 +19,33 @@ def _cleanup_expired_sessions():
"""
current_time = datetime.now()
expired_sessions = [
session_id for session_id, (discord_id, expiry_time) in _verification_sessions.items()
session_id for session_id, (discord_id, expiry_time, _state) in _verification_sessions.items()
if current_time > expiry_time
]

for session_id in expired_sessions:
discord_id, _ = _verification_sessions[session_id]
discord_id, _, _ = _verification_sessions[session_id]
del _verification_sessions[session_id]
logger.info(f"Cleaned up expired verification session {session_id} for Discord user {discord_id}")

if expired_sessions:
logger.info(f"Cleaned up {len(expired_sessions)} expired verification sessions")

async def create_verification_session(discord_id: str) -> Optional[str]:
async def create_verification_session(discord_id: str) -> Optional[Tuple[str, str]]:
"""
Create a verification session with expiry and return session ID.
Create a verification session with expiry and return (session_id, oauth_state).

The OAuth ``state`` is a cryptographically-random token bound to the session.
It must be threaded through the OAuth authorize URL and validated in the
callback to protect against login CSRF (RFC 6749, Section 10.12).
"""
supabase = get_supabase_client()

_cleanup_expired_sessions()

token = str(uuid.uuid4())
session_id = str(uuid.uuid4())
oauth_state = secrets.token_urlsafe(32)
expiry_time = datetime.now() + timedelta(minutes=SESSION_EXPIRY_MINUTES)

try:
Expand All @@ -50,16 +56,44 @@ async def create_verification_session(discord_id: str) -> Optional[str]:
}).eq("discord_id", discord_id).execute()

if update_res.data:
_verification_sessions[session_id] = (discord_id, expiry_time)
_verification_sessions[session_id] = (discord_id, expiry_time, oauth_state)
logger.info(
f"Created verification session {session_id} for Discord user {discord_id}, expires at {expiry_time}")
return session_id
return session_id, oauth_state
logger.error(f"Failed to set verification token for Discord ID: {discord_id}. User not found.")
return None
except Exception as e:
logger.error(f"Error creating verification session for Discord ID {discord_id}: {str(e)}")
return None

def validate_oauth_state(session_id: str, state: Optional[str]) -> bool:
"""
Validate the OAuth ``state`` returned in the callback against the value
bound to the verification session.

Uses a constant-time comparison and rejects missing/expired sessions or any
mismatch. Does not consume the session (the session itself is consumed by
:func:`find_user_by_session_and_verify`).
"""
_cleanup_expired_sessions()

if not state:
logger.warning(f"OAuth state missing in callback for session ID: {session_id}")
return False

session_data = _verification_sessions.get(session_id)
if not session_data:
logger.warning(f"No verification session found while validating state for session ID: {session_id}")
return False

_discord_id, _expiry_time, expected_state = session_data

if not secrets.compare_digest(state, expected_state):
logger.warning(f"OAuth state mismatch for session ID: {session_id}")
return False

return True

async def find_user_by_session_and_verify(
session_id: str, github_id: str, github_username: str, email: Optional[str]
) -> Optional[User]:
Expand All @@ -77,7 +111,7 @@ async def find_user_by_session_and_verify(
logger.warning(f"No verification session found for session ID: {session_id}")
return None

discord_id, expiry_time = session_data
discord_id, expiry_time, _oauth_state = session_data

current_time = datetime.now().isoformat()
user_res = await supabase.table("users").select("*").eq(
Expand Down Expand Up @@ -163,7 +197,7 @@ async def get_verification_session_info(session_id: str) -> Optional[Dict[str, s
if not session_data:
return None

discord_id, expiry_time = session_data
discord_id, expiry_time, _oauth_state = session_data

if datetime.now() > expiry_time:
del _verification_sessions[session_id]
Expand Down
14 changes: 8 additions & 6 deletions backend/integrations/discord/cogs.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,13 @@ async def verify_github(self, interaction: discord.Interaction):
await interaction.followup.send(embed=embed, ephemeral=True)
return

session_id = await create_verification_session(str(interaction.user.id))
if not session_id:
session = await create_verification_session(str(interaction.user.id))
if not session:
raise Exception("Failed to create verification session.")
session_id, oauth_state = 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_data = await login_with_github(redirect_to=callback_url, state=oauth_state)
auth_url = auth_url_data.get("url")
if not auth_url:
raise Exception("Failed to generate OAuth URL.")
Expand Down Expand Up @@ -485,8 +486,8 @@ async def _send_onboarding_flow(self, user: discord.abc.User) -> str:

if show_oauth_button:
# Create verification session
session_id = await create_verification_session(str(user.id))
if not session_id:
session = await create_verification_session(str(user.id))
if not session:
try:
await user.send("I couldn't start verification right now. You can use /verify_github anytime.")
await user.send(build_encourage_verification_message(reminder_count=1))
Expand All @@ -496,10 +497,11 @@ async def _send_onboarding_flow(self, user: discord.abc.User) -> str:
except Exception as e:
logger.exception(f"Failed to send session failure fallback DM to user {user.id}: {e}")
return "session_unavailable"
session_id, oauth_state = session

# 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 = await login_with_github(redirect_to=callback_url, state=oauth_state)
auth_url = auth.get("url")
if not auth_url:
try:
Expand Down