Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
46 changes: 46 additions & 0 deletions api-nei/alembic/versions/b9e4c7f2a1d8_add_oauth_state_table.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
"""Add oauth_state table for server-side PKCE state storage

Revision ID: b9e4c7f2a1d8
Revises: 3f1a2b4c5d6e
Create Date: 2026-06-17 00:00:00.000000

"""

from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision = "b9e4c7f2a1d8"
down_revision = "3f1a2b4c5d6e"
branch_labels = None
depends_on = None


def upgrade() -> None:
op.create_table(
"oauth_state",
sa.Column("state", sa.String(), nullable=False),
sa.Column("verifier", sa.String(), nullable=True),
sa.Column("nonce", sa.String(), nullable=True),
sa.Column("redirect", sa.String(), nullable=True),
sa.Column("expires_at", sa.DateTime(), nullable=False),
sa.PrimaryKeyConstraint("state", name=op.f("pk_oauth_state")),
schema="nei",
)
op.create_index(
op.f("ix_oauth_state_expires_at"),
"oauth_state",
["expires_at"],
unique=False,
schema="nei",
)


def downgrade() -> None:
op.drop_index(
op.f("ix_oauth_state_expires_at"),
table_name="oauth_state",
schema="nei",
)
op.drop_table("oauth_state", schema="nei")
114 changes: 90 additions & 24 deletions api-nei/app/api/api_v1/auth/oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

import json
import secrets
from datetime import datetime, timedelta
from typing import Annotated, Any, Optional
from urllib.parse import quote, unquote

Expand All @@ -47,6 +48,7 @@
from app import crud
from app.api import deps
from app.core.config import settings
from app.models.oauth_state import OAuthState
from app.models.user import User
from app.models.user.user_email import UserEmail
from app.schemas.user import ScopeEnum, UserCreate
Expand Down Expand Up @@ -88,7 +90,7 @@
# ---------------------------------------------------------------------------

_STATE_COOKIE = "oauth_state"
_STATE_MAX_AGE = 600 # 10 minutes
_STATE_MAX_AGE = 1800 # 30 minutes — enough time for slow email delivery


def _signer() -> URLSafeTimedSerializer:
Expand All @@ -103,6 +105,7 @@
redirect_to: Optional[str] = None,
user_id: Optional[int] = None,
code_verifier: Optional[str] = None,
db: Optional[Session] = None,
) -> None:
payload: dict = {"s": state}
if oidc_nonce:
Expand All @@ -121,29 +124,82 @@
samesite="lax",
max_age=_STATE_MAX_AGE,
)


def _verify_and_pop_state_cookie(request: Request, response: Response, state: str) -> dict:
if db is not None:
try:
db.query(OAuthState).filter(OAuthState.state == state).delete()
db.add(OAuthState(
state=state,
verifier=code_verifier,
nonce=oidc_nonce,
redirect=redirect_to,
expires_at=datetime.utcnow() + timedelta(seconds=_STATE_MAX_AGE),

Check failure on line 135 in api-nei/app/api/api_v1/auth/oidc.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Don't use `datetime.datetime.utcnow` to create this datetime object.

See more on https://sonarcloud.io/project/issues?id=NEI-AAUAV_Platform&issues=AZ7lOWYG8N9AxeIuvldJ&open=AZ7lOWYG8N9AxeIuvldJ&pullRequest=205
))
db.commit()
except Exception:
db.rollback()
logger.warning("Failed to persist OAuth state to DB — cookie-only fallback applies")


def _verify_and_pop_state_cookie(

Check failure on line 143 in api-nei/app/api/api_v1/auth/oidc.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 30 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=NEI-AAUAV_Platform&issues=AZ7lOWYG8N9AxeIuvldK&open=AZ7lOWYG8N9AxeIuvldK&pullRequest=205
request: Request,
response: Response,
state: str,
*,
db: Optional[Session] = None,
) -> dict:
cookie_val = request.cookies.get(_STATE_COOKIE)
response.delete_cookie(_STATE_COOKIE, httponly=True, samesite="lax")

if not cookie_val:
raise HTTPException(status.HTTP_401_UNAUTHORIZED, "Missing OAuth state cookie")

try:
payload = _signer().loads(cookie_val, max_age=_STATE_MAX_AGE)
except SignatureExpired:
raise HTTPException(status.HTTP_401_UNAUTHORIZED, "OAuth state expired — please try again")
except BadSignature:
raise HTTPException(status.HTTP_401_UNAUTHORIZED, "Invalid OAuth state")

# Constant-time comparison; also handle the pre-migration "n" key for
# cookies issued by the previous version of this code.
stored = payload.get("s") or payload.get("n")
if not stored or not secrets.compare_digest(stored, state):
raise HTTPException(status.HTTP_401_UNAUTHORIZED, "OAuth state mismatch")

return payload
if cookie_val:
try:
payload = _signer().loads(cookie_val, max_age=_STATE_MAX_AGE)
except SignatureExpired:
raise HTTPException(status.HTTP_401_UNAUTHORIZED, "OAuth state expired — please try again")
except BadSignature:
raise HTTPException(status.HTTP_401_UNAUTHORIZED, "Invalid OAuth state")

# Constant-time comparison; also handle the pre-migration "n" key for
# cookies issued by the previous version of this code.
stored = payload.get("s") or payload.get("n")
if not stored or not secrets.compare_digest(stored, state):
raise HTTPException(status.HTTP_401_UNAUTHORIZED, "OAuth state mismatch")

if db is not None:
try:
db.query(OAuthState).filter(OAuthState.state == state).delete()
db.commit()
except Exception:
db.rollback()

return payload

# Cookie missing (e.g. callback arrived in a different browser after email
# verification). Try the DB-backed state as a fallback.
if db is not None:
row = db.query(OAuthState).filter(OAuthState.state == state).first()
if row and row.expires_at > datetime.utcnow():

Check failure on line 180 in api-nei/app/api/api_v1/auth/oidc.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Don't use `datetime.datetime.utcnow` to create this datetime object.

See more on https://sonarcloud.io/project/issues?id=NEI-AAUAV_Platform&issues=AZ7lOWYG8N9AxeIuvldL&open=AZ7lOWYG8N9AxeIuvldL&pullRequest=205
payload = {"s": row.state}
if row.nonce:
payload["n"] = row.nonce
if row.redirect:
payload["r"] = row.redirect
if row.verifier:
payload["v"] = row.verifier
try:
db.delete(row)
db.commit()
except Exception:
db.rollback()
return payload

raise HTTPException(status.HTTP_401_UNAUTHORIZED, "missing_state_cookie")


def cleanup_expired_oauth_states(db: Session) -> int:
"""Delete expired OAuthState rows. Returns the number of rows deleted."""
deleted = db.query(OAuthState).filter(OAuthState.expires_at < datetime.utcnow()).delete()

Check failure on line 200 in api-nei/app/api/api_v1/auth/oidc.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Don't use `datetime.datetime.utcnow` to create this datetime object.

See more on https://sonarcloud.io/project/issues?id=NEI-AAUAV_Platform&issues=AZ7lOWYG8N9AxeIuvldM&open=AZ7lOWYG8N9AxeIuvldM&pullRequest=205
db.commit()
return deleted


# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -508,6 +564,7 @@
)
async def oidc_login(
response: Response,
db: DbSession,
redirect_to: Optional[str] = None,
):
_require_oidc()
Expand Down Expand Up @@ -542,6 +599,7 @@
oidc_nonce=oidc_nonce,
redirect_to=safe_redirect,
code_verifier=auth_url_data.get("code_verifier"),
db=db,
)
return redirect_response

Expand All @@ -553,7 +611,7 @@
503: {"description": "OIDC authentication is disabled"},
},
)
async def oidc_callback(

Check failure on line 614 in api-nei/app/api/api_v1/auth/oidc.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 22 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=NEI-AAUAV_Platform&issues=AZ7lOWYG8N9AxeIuvldN&open=AZ7lOWYG8N9AxeIuvldN&pullRequest=205
request: Request,
response: Response,
code: str,
Expand All @@ -562,7 +620,7 @@
):
_require_oidc()

state_payload = _verify_and_pop_state_cookie(request, response, state)
state_payload = _verify_and_pop_state_cookie(request, response, state, db=db)
raw_redirect = state_payload.get("r")
redirect_to = raw_redirect if _is_safe_redirect(raw_redirect) else None
code_verifier = state_payload.get("v")
Expand Down Expand Up @@ -626,9 +684,17 @@
raise
error_code = {
status.HTTP_403_FORBIDDEN: "unverified",
status.HTTP_401_UNAUTHORIZED: "session",
status.HTTP_400_BAD_REQUEST: "invalid_response",
}.get(e.status_code, "unknown")
}.get(e.status_code)
if error_code is None:
if e.status_code == status.HTTP_401_UNAUTHORIZED:
error_code = (
"email_verified_relogin"
if getattr(e, "detail", "") == "missing_state_cookie"
else "session"
)
else:
error_code = "unknown"
logger.info(f"OIDC callback failed ({e.status_code}): {e.detail}")
return _login_error_redirect(error_code)
except httpx.HTTPError as e:
Expand Down
15 changes: 15 additions & 0 deletions api-nei/app/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@
from fastapi.middleware.cors import CORSMiddleware
from fastapi.responses import ORJSONResponse

from loguru import logger

from app.api.api_v1 import router as api_v1_router
from app.db.init_db import init_db
from app.db.session import SessionLocal
from app.core.logging import init_logging
from app.core.config import settings
from app.core.extension_scopes import load_scopes_from_manifests
Expand All @@ -21,6 +24,18 @@ async def lifespan(_: FastAPI):
load_scopes_from_manifests()
# Update OAuth2 scheme with extension scopes
dynamic_oauth2_scheme.update_scopes()
# Remove OAuth state rows left over from previous sessions
try:
from app.api.api_v1.auth.oidc import cleanup_expired_oauth_states
db = SessionLocal()
try:
n = cleanup_expired_oauth_states(db)
if n:
logger.info(f"Cleaned up {n} expired OAuth state row(s) on startup")
finally:
db.close()
except Exception:
logger.warning("OAuth state cleanup on startup failed — continuing")
yield


Expand Down
1 change: 1 addition & 0 deletions api-nei/app/models/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from .oauth_state import OAuthState
from .user import User
from .event import Event
from .faina import FainaMember, FainaRole, Faina
Expand Down
13 changes: 13 additions & 0 deletions api-nei/app/models/oauth_state.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
from datetime import datetime
from sqlalchemy import Column, DateTime, String
from app.db.base_class import Base


class OAuthState(Base):
__tablename__ = "oauth_state"

state = Column(String, primary_key=True)
verifier = Column(String, nullable=True)
nonce = Column(String, nullable=True)
redirect = Column(String, nullable=True)
expires_at = Column(DateTime, nullable=False)
14 changes: 10 additions & 4 deletions web-nei/src/pages/auth/Login/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ import config from "config";
// Error codes surfaced by the backend's oidc_callback. Keep in sync with
// _login_error_redirect in api-nei/app/api/api_v1/auth/oidc.py.
const ERROR_COPY = {
// success:true renders a green notice instead of the default error card
email_verified_relogin: {
title: "Your account is verified!",
body: "Your email was confirmed successfully. Sign in to continue.",
success: true,
},
unverified: {
title: "Verify your email to continue",
body: "We sent you a verification link. Check your inbox (and spam folder), click the link, and come back to sign in.",
Expand Down Expand Up @@ -61,13 +67,13 @@ export function Component() {
: "/auth/login";
return (
<div className="flex h-screen items-center justify-center p-6">
<div className="card w-full max-w-md bg-base-100 shadow-xl">
<div className={`card w-full max-w-md shadow-xl ${copy.success ? "bg-success/10 border border-success" : "bg-base-100"}`}>
<div className="card-body items-center text-center">
<h2 className="card-title text-2xl">{copy.title}</h2>
<h2 className={`card-title text-2xl ${copy.success ? "text-success" : ""}`}>{copy.title}</h2>
<p className="py-2 text-base-content/80">{copy.body}</p>
<div className="card-actions mt-4">
<Link to={retryHref} className="btn btn-primary" replace>
Try again
<Link to={retryHref} className={`btn ${copy.success ? "btn-success" : "btn-primary"}`} replace>
{copy.success ? "Sign in" : "Try again"}
</Link>
</div>
</div>
Expand Down
Loading