diff --git a/backend/app/api/auth.py b/backend/app/api/auth.py index 03c045443..6f7a0e161 100644 --- a/backend/app/api/auth.py +++ b/backend/app/api/auth.py @@ -1,11 +1,12 @@ """Authentication API routes.""" +import secrets import uuid from datetime import datetime, timezone from time import perf_counter from typing import Any -from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Query, Request, status +from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Query, Request, Response, status from loguru import logger from app.dao import query_dao from app.config import get_settings @@ -909,6 +910,7 @@ async def list_providers( # Redis keys for OAuth two-step tenant selection _OAUTH_PENDING_PREFIX = "oauth_pending:" _OAUTH_PENDING_TTL = 600 # 10 minutes +_OAUTH_STATE_COOKIE = "oauth_state" async def _cache_oauth_pending( @@ -948,9 +950,9 @@ async def _get_oauth_pending(pending_token: str) -> dict | None: @router.get("/{provider}/authorize", response_model=OAuthAuthorizeResponse) async def authorize( + response: Response, provider: str, redirect_uri: str = Query(..., description="OAuth callback URI"), - state: str = Query("", description="CSRF state parameter"), ): """Start OAuth authorization flow for a provider.""" from app.services.auth_registry import auth_provider_registry @@ -960,6 +962,19 @@ async def authorize( if not auth_provider: raise HTTPException(status_code=404, detail=f"Provider '{provider}' not supported") + # Bind the provider callback to the browser that initiated this authorization. + # The state is intentionally generated server-side; caller supplied state values + # are not an adequate CSRF defense. + state = secrets.token_urlsafe(32) + response.set_cookie( + key=_OAUTH_STATE_COOKIE, + value=state, + httponly=True, + secure=not settings.DEBUG, + samesite="lax", + max_age=_OAUTH_PENDING_TTL, + ) + # Generate authorization URL try: auth_url = await auth_provider.get_authorization_url(redirect_uri, state) @@ -976,6 +991,7 @@ async def authorize( async def oauth_callback( provider: str, data: OAuthCallbackRequest, + request: Request, ): """Handle OAuth callback — supports a two-step flow for multi-tenant selection. @@ -988,6 +1004,10 @@ async def oauth_callback( import uuid as _uuid from app.services.auth_registry import auth_provider_registry + expected_state = request.cookies.get(_OAUTH_STATE_COOKIE) + if not expected_state or not secrets.compare_digest(data.state, expected_state): + raise HTTPException(status_code=400, detail="OAuth state is invalid or does not match this browser") + # ── Step 2: User has selected a tenant ─────────────────────────────────── if data.pending_token and data.tenant_id: pending = await _get_oauth_pending(data.pending_token) diff --git a/backend/app/api/sso.py b/backend/app/api/sso.py index 8dc045336..301a4f97c 100644 --- a/backend/app/api/sso.py +++ b/backend/app/api/sso.py @@ -2,21 +2,29 @@ from datetime import datetime, timedelta, timezone from urllib.parse import quote -from fastapi import APIRouter, Depends, HTTPException, Request +from fastapi import APIRouter, Depends, HTTPException, Request, Response from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession from app.dao import query_dao +from app.config import get_settings from app.database import get_db from app.models.identity import SSOScanSession, IdentityProvider from app.schemas.schemas import UserOut +from app.services.sso_session_security import ( + is_valid_sso_browser_binding, + sign_sso_browser_binding, + sso_browser_cookie_name, +) router = APIRouter(tags=["sso"]) +settings = get_settings() @router.post("/sso/session") async def create_sso_session( + response: Response, tenant_id: uuid.UUID | None = None, - db: AsyncSession = Depends(get_db) + db: AsyncSession = Depends(get_db), ): """Create a new SSO scan session for QR code login.""" session = SSOScanSession( @@ -27,11 +35,26 @@ async def create_sso_session( ) query_dao.add(db, session) await query_dao.commit(db) + response.set_cookie( + key=sso_browser_cookie_name(session.id), + value=sign_sso_browser_binding(session.id), + httponly=True, + secure=not settings.DEBUG, + samesite="lax", + max_age=5 * 60, + ) return {"session_id": str(session.id), "expires_at": session.expires_at} @router.get("/sso/session/{sid}/status") -async def get_sso_session_status(sid: uuid.UUID, db: AsyncSession = Depends(get_db)): +async def get_sso_session_status( + sid: uuid.UUID, + request: Request, + db: AsyncSession = Depends(get_db), +): """Check the status of an SSO scan session.""" + if not is_valid_sso_browser_binding(sid, request.cookies.get(sso_browser_cookie_name(sid))): + raise HTTPException(status_code=403, detail="SSO session is not bound to this browser") + result = await query_dao.execute(db, select(SSOScanSession).where(SSOScanSession.id == sid)) session = result.scalar_one_or_none() if not session: @@ -91,8 +114,8 @@ async def get_sso_config(sid: uuid.UUID, request: Request, db: AsyncSession = De # 2. Query IdentityProviders for this tenant (only those that are active AND SSO-enabled) query = select(IdentityProvider).where( - IdentityProvider.is_active == True, - IdentityProvider.sso_login_enabled == True, + IdentityProvider.is_active, + IdentityProvider.sso_login_enabled, ) if session.tenant_id: query = query.where(IdentityProvider.tenant_id == session.tenant_id) diff --git a/backend/app/services/sso_session_security.py b/backend/app/services/sso_session_security.py new file mode 100644 index 000000000..178b31045 --- /dev/null +++ b/backend/app/services/sso_session_security.py @@ -0,0 +1,28 @@ +"""Browser-binding helpers for temporary SSO scan sessions.""" + +import hashlib +import hmac +import uuid + +from app.config import get_settings + + +_COOKIE_PREFIX = "sso_browser_" + + +def sso_browser_cookie_name(session_id: uuid.UUID) -> str: + """Return the per-session cookie name used to bind a scan session to a browser.""" + return f"{_COOKIE_PREFIX}{session_id.hex}" + + +def sign_sso_browser_binding(session_id: uuid.UUID) -> str: + """Create an HttpOnly-cookie value that cannot be forged for another session.""" + secret_key = get_settings().SECRET_KEY.encode() + return hmac.new(secret_key, str(session_id).encode(), hashlib.sha256).hexdigest() + + +def is_valid_sso_browser_binding(session_id: uuid.UUID, cookie_value: str | None) -> bool: + """Verify that a browser cookie was minted for this exact scan session.""" + if not cookie_value: + return False + return hmac.compare_digest(cookie_value, sign_sso_browser_binding(session_id)) diff --git a/backend/tests/test_auth.py b/backend/tests/test_auth.py index b805801f3..3d5772d39 100644 --- a/backend/tests/test_auth.py +++ b/backend/tests/test_auth.py @@ -1,15 +1,19 @@ """Unit tests for the authentication API (app/api/auth.py).""" import uuid +from datetime import datetime, timedelta, timezone from types import SimpleNamespace from unittest.mock import AsyncMock, patch import pytest from fastapi import HTTPException +from starlette.requests import Request from app.api import auth as auth_api +from app.api import sso as sso_api from app.core.security import hash_password from app.database import _session_ctx +from app.services.sso_session_security import sso_browser_cookie_name async def run_with_db(db, func, *args, **kwargs): @@ -231,7 +235,62 @@ def __init__(self, access_token, **kwargs): with patch("app.api.auth.UserOut") as MockUserOut: MockUserOut.model_validate.return_value = {"id": str(user.id)} with patch.object(auth_api, "create_access_token", return_value="jwt-token"): - result = await run_with_db(RecordingDB(), auth_api.oauth_callback, "google", data) + request = Request( + {"type": "http", "headers": [(b"cookie", b"oauth_state=oauth-state")]} + ) + result = await run_with_db(RecordingDB(), auth_api.oauth_callback, "google", data, request) provider.exchange_code_for_token.assert_awaited_once_with("oauth-code", "https://example.com/oauth/callback/google") assert result.access_token == "jwt-token" + + +@pytest.mark.asyncio +async def test_oauth_callback_rejects_state_from_another_browser(): + data = SimpleNamespace( + code="oauth-code", + state="attacker-state", + redirect_uri="https://example.com/oauth/callback/google", + pending_token=None, + tenant_id=None, + ) + request = Request({"type": "http", "headers": [(b"cookie", b"oauth_state=expected-state")]}) + + with pytest.raises(HTTPException, match="does not match this browser") as exc_info: + await run_with_db(RecordingDB(), auth_api.oauth_callback, "google", data, request) + + assert exc_info.value.status_code == 400 + + +@pytest.mark.asyncio +async def test_sso_session_status_rejects_a_browser_without_its_binding_cookie(): + session_id = uuid.uuid4() + request = Request({"type": "http", "headers": []}) + + with pytest.raises(HTTPException, match="not bound to this browser") as exc_info: + await sso_api.get_sso_session_status(session_id, request, RecordingDB()) + + assert exc_info.value.status_code == 403 + + +@pytest.mark.asyncio +async def test_sso_session_status_accepts_the_initiating_browser_cookie(): + session_id = uuid.uuid4() + cookie_name = sso_browser_cookie_name(session_id) + cookie_value = sso_api.sign_sso_browser_binding(session_id) + request = Request( + {"type": "http", "headers": [(b"cookie", f"{cookie_name}={cookie_value}".encode())]} + ) + session = SimpleNamespace( + expires_at=datetime.now(timezone.utc) + timedelta(minutes=1), + status="pending", + provider_type=None, + error_msg=None, + ) + + result = await sso_api.get_sso_session_status( + session_id, + request, + RecordingDB([DummyResult(scalar_value=session)]), + ) + + assert result == {"status": "pending", "provider_type": None, "error_msg": None}