From f8126d0cbcca6cb4d0ccab55df4ffb14e97874f5 Mon Sep 17 00:00:00 2001 From: hzhang Date: Sun, 17 May 2026 20:29:15 +0100 Subject: [PATCH] feat(auth): admin-configurable OIDC provider (oidc_settings) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Persist OIDC config in a single-row oidc_settings table; non-empty DB fields override the OIDC_* env vars (env = bootstrap default). The Authlib client is rebuilt when config changes. - GET/PUT /auth/oidc/settings — admin only, via JWT OR API key. The API-key path is the recovery channel when OIDC-only mode is on and OIDC is misconfigured (avoids total lockout). - client_secret is write-only: never returned (has_client_secret bool), preserved when the field is left blank on update. - /auth/config, login/link/callback now use the effective (DB|env) config so enabling OIDC needs no redeploy. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/api/routers/oidc.py | 235 ++++++++++++++++++++++++++++-------- app/main.py | 2 +- app/models/oidc_settings.py | 22 ++++ 3 files changed, 208 insertions(+), 51 deletions(-) create mode 100644 app/models/oidc_settings.py diff --git a/app/api/routers/oidc.py b/app/api/routers/oidc.py index 00cc048..adf50e7 100644 --- a/app/api/routers/oidc.py +++ b/app/api/routers/oidc.py @@ -1,105 +1,161 @@ -"""OIDC (OpenID Connect) login + public auth-config. +"""OIDC (OpenID Connect) login + admin-configurable provider settings. -Generic OIDC via discovery. The backend performs the Authorization Code -flow, then issues its own existing HS256 JWT (same as password login) so -the rest of the app is unchanged. +The OIDC provider can be configured at runtime from the admin UI +(persisted in the oidc_settings table). A stored row's non-empty fields +override the OIDC_* env vars; env values act as bootstrap defaults. Sign-in policy: an OIDC identity must already be bound to an hf user -(see PUT /users/{id}/oidc-binding). Unbound identities are rejected — no -auto-provisioning. +(see PUT /users/{id}/oidc-binding). Unbound identities are rejected. """ from datetime import timedelta from urllib.parse import urlencode from fastapi import APIRouter, Depends, HTTPException, Request from fastapi.responses import RedirectResponse +from pydantic import BaseModel from sqlalchemy.orm import Session from app.core.config import get_db, settings from app.models import models -from app.api.deps import create_access_token, get_current_user +from app.models.oidc_settings import OidcSettings +from app.api.deps import create_access_token, get_current_user, get_current_user_or_apikey router = APIRouter(prefix="/auth", tags=["Auth"]) -# Authlib registry — only configured when OIDC is enabled. + +# ---- effective config (DB row overrides env) ------------------------------ + +class EffectiveOidc: + def __init__(self, enabled, issuer, client_id, client_secret, + redirect_uri, scopes, post_login_redirect): + self.enabled = enabled + self.issuer = issuer + self.client_id = client_id + self.client_secret = client_secret + self.redirect_uri = redirect_uri + self.scopes = scopes or "openid email profile" + self.post_login_redirect = post_login_redirect + + @property + def configured(self) -> bool: + return bool(self.enabled and self.issuer and self.client_id) + + def fingerprint(self) -> str: + return "|".join([ + str(self.enabled), self.issuer or "", self.client_id or "", + self.client_secret or "", self.redirect_uri or "", self.scopes or "", + ]) + + +def get_effective_oidc(db: Session) -> EffectiveOidc: + row = db.query(OidcSettings).filter(OidcSettings.id == 1).first() + + def pick(db_val, env_val): + return db_val if (db_val is not None and db_val != "") else env_val + + if row is None: + return EffectiveOidc( + settings.OIDC_ENABLED, settings.OIDC_ISSUER, settings.OIDC_CLIENT_ID, + settings.OIDC_CLIENT_SECRET, settings.OIDC_REDIRECT_URI, + settings.OIDC_SCOPES, settings.OIDC_POST_LOGIN_REDIRECT, + ) + return EffectiveOidc( + bool(row.enabled), + pick(row.issuer, settings.OIDC_ISSUER), + pick(row.client_id, settings.OIDC_CLIENT_ID), + pick(row.client_secret, settings.OIDC_CLIENT_SECRET), + pick(row.redirect_uri, settings.OIDC_REDIRECT_URI), + pick(row.scopes, settings.OIDC_SCOPES), + pick(row.post_login_redirect, settings.OIDC_POST_LOGIN_REDIRECT), + ) + + +# Authlib client cache, rebuilt when the effective config changes. _oauth = None +_oauth_fp = None -def _get_oidc(): - """Lazily build the Authlib OIDC client; 503 if not configured.""" - global _oauth - if not (settings.OIDC_ENABLED and settings.OIDC_ISSUER and settings.OIDC_CLIENT_ID): +def _client(cfg: EffectiveOidc): + global _oauth, _oauth_fp + if not cfg.configured: raise HTTPException(status_code=503, detail="OIDC is not configured") - if _oauth is None: + fp = cfg.fingerprint() + if _oauth is None or _oauth_fp != fp: from authlib.integrations.starlette_client import OAuth oauth = OAuth() oauth.register( name="oidc", - server_metadata_url=settings.OIDC_ISSUER.rstrip("/") + "/.well-known/openid-configuration", - client_id=settings.OIDC_CLIENT_ID, - client_secret=settings.OIDC_CLIENT_SECRET, - client_kwargs={"scope": settings.OIDC_SCOPES}, + server_metadata_url=cfg.issuer.rstrip("/") + "/.well-known/openid-configuration", + client_id=cfg.client_id, + client_secret=cfg.client_secret, + client_kwargs={"scope": cfg.scopes}, ) - _oauth = oauth + _oauth, _oauth_fp = oauth, fp return _oauth.oidc -def _frontend(suffix_qs: dict | None = None, fragment: str | None = None) -> str: - """Build the post-login frontend redirect (never client-controlled).""" - base = settings.OIDC_POST_LOGIN_REDIRECT or "/" +def _invalidate_client(): + global _oauth, _oauth_fp + _oauth = None + _oauth_fp = None + + +def _frontend(cfg: EffectiveOidc, qs: dict | None = None, fragment: str | None = None) -> str: + base = cfg.post_login_redirect or "/" url = base - if suffix_qs: - url += ("&" if "?" in base else "?") + urlencode(suffix_qs) + if qs: + url += ("&" if "?" in base else "?") + urlencode(qs) if fragment: url += "#" + fragment return url +# ---- public auth config --------------------------------------------------- + @router.get("/config") -def auth_config(): - """Public: lets the frontend decide which login UI to render.""" +def auth_config(db: Session = Depends(get_db)): + cfg = get_effective_oidc(db) return { - "oidc_enabled": bool(settings.OIDC_ENABLED and settings.OIDC_ISSUER and settings.OIDC_CLIENT_ID), + "oidc_enabled": cfg.configured, "oidc_only": bool(settings.HARBORFORGE_OIDC_ONLY), "password_login": not bool(settings.HARBORFORGE_OIDC_ONLY), "oidc_login_url": "/auth/oidc/login", } +# ---- sign-in / link flows ------------------------------------------------- + @router.get("/oidc/login") -async def oidc_login(request: Request): - """Start the OIDC Authorization Code flow for sign-in.""" - oidc = _get_oidc() - request.session.pop("hf_oidc_mode", None) +async def oidc_login(request: Request, db: Session = Depends(get_db)): + cfg = get_effective_oidc(db) + oidc = _client(cfg) request.session.pop("hf_oidc_uid", None) request.session["hf_oidc_mode"] = "login" - return await oidc.authorize_redirect(request, settings.OIDC_REDIRECT_URI) + return await oidc.authorize_redirect(request, cfg.redirect_uri) @router.get("/oidc/link") -async def oidc_link(request: Request, current_user: models.User = Depends(get_current_user)): - """Self-service: bind the caller's own account to an OIDC identity. - - Only available when NOT in OIDC-only mode (admins use the binding API - in that mode).""" +async def oidc_link(request: Request, db: Session = Depends(get_db), + current_user: models.User = Depends(get_current_user)): if settings.HARBORFORGE_OIDC_ONLY: raise HTTPException(status_code=403, detail="Self-service linking is disabled in OIDC-only mode") - oidc = _get_oidc() + cfg = get_effective_oidc(db) + oidc = _client(cfg) request.session["hf_oidc_mode"] = "link" request.session["hf_oidc_uid"] = current_user.id - return await oidc.authorize_redirect(request, settings.OIDC_REDIRECT_URI) + return await oidc.authorize_redirect(request, cfg.redirect_uri) @router.get("/oidc/callback") async def oidc_callback(request: Request, db: Session = Depends(get_db)): - """OIDC redirect target. Resolves identity → hf user (must be bound).""" - oidc = _get_oidc() + cfg = get_effective_oidc(db) + oidc = _client(cfg) mode = request.session.pop("hf_oidc_mode", "login") link_uid = request.session.pop("hf_oidc_uid", None) try: token = await oidc.authorize_access_token(request) except Exception: - return RedirectResponse(_frontend({"oidc_error": "exchange_failed"})) + return RedirectResponse(_frontend(cfg, {"oidc_error": "exchange_failed"})) claims = token.get("userinfo") or {} if not claims: @@ -108,38 +164,117 @@ async def oidc_callback(request: Request, db: Session = Depends(get_db)): except Exception: claims = {} subject = claims.get("sub") - issuer = claims.get("iss") or settings.OIDC_ISSUER + issuer = claims.get("iss") or cfg.issuer if not subject: - return RedirectResponse(_frontend({"oidc_error": "no_subject"})) + return RedirectResponse(_frontend(cfg, {"oidc_error": "no_subject"})) if mode == "link": if settings.HARBORFORGE_OIDC_ONLY or link_uid is None: - return RedirectResponse(_frontend({"oidc_error": "link_not_allowed"})) + return RedirectResponse(_frontend(cfg, {"oidc_error": "link_not_allowed"})) user = db.query(models.User).filter(models.User.id == link_uid).first() if not user: - return RedirectResponse(_frontend({"oidc_error": "user_gone"})) + return RedirectResponse(_frontend(cfg, {"oidc_error": "user_gone"})) clash = db.query(models.User).filter( models.User.oidc_issuer == issuer, models.User.oidc_subject == subject, models.User.id != user.id, ).first() if clash: - return RedirectResponse(_frontend({"oidc_error": "already_bound"})) + return RedirectResponse(_frontend(cfg, {"oidc_error": "already_bound"})) user.oidc_issuer = issuer user.oidc_subject = subject db.commit() - return RedirectResponse(_frontend({"oidc_linked": "1"})) + return RedirectResponse(_frontend(cfg, {"oidc_linked": "1"})) - # sign-in: identity must already be bound to an active hf user user = db.query(models.User).filter( models.User.oidc_issuer == issuer, models.User.oidc_subject == subject, ).first() if not user or not user.is_active or user.username in ("acc-mgr", "deleted-user"): - return RedirectResponse(_frontend({"oidc_error": "not_linked"})) + return RedirectResponse(_frontend(cfg, {"oidc_error": "not_linked"})) access_token = create_access_token( data={"sub": str(user.id)}, expires_delta=timedelta(minutes=settings.ACCESS_TOKEN_EXPIRE_MINUTES), ) - return RedirectResponse(_frontend(fragment=urlencode({"token": access_token}))) + return RedirectResponse(_frontend(cfg, fragment=urlencode({"token": access_token}))) + + +# ---- admin: OIDC provider settings ---------------------------------------- + +def _require_admin_any(current_user: models.User = Depends(get_current_user_or_apikey)) -> models.User: + """Admin via JWT OR API key. The API-key path is the recovery channel + when OIDC-only mode is on and OIDC is not yet/incorrectly configured.""" + if not getattr(current_user, "is_admin", False): + raise HTTPException(status_code=403, detail="Admin privileges required") + return current_user + + +class OidcSettingsIn(BaseModel): + enabled: bool | None = None + issuer: str | None = None + client_id: str | None = None + client_secret: str | None = None # blank/omitted = keep existing + redirect_uri: str | None = None + scopes: str | None = None + post_login_redirect: str | None = None + + +class OidcSettingsOut(BaseModel): + enabled: bool + issuer: str | None + client_id: str | None + has_client_secret: bool + redirect_uri: str | None + scopes: str | None + post_login_redirect: str | None + oidc_only: bool # read-only (deploy env) + effective_enabled: bool # provider actually usable + source: str # "db" or "env" + + +@router.get("/oidc/settings", response_model=OidcSettingsOut) +def get_oidc_settings(db: Session = Depends(get_db), _: models.User = Depends(_require_admin_any)): + row = db.query(OidcSettings).filter(OidcSettings.id == 1).first() + cfg = get_effective_oidc(db) + return OidcSettingsOut( + enabled=bool(row.enabled) if row else bool(settings.OIDC_ENABLED), + issuer=(row.issuer if row else None) or settings.OIDC_ISSUER or None, + client_id=(row.client_id if row else None) or settings.OIDC_CLIENT_ID or None, + has_client_secret=bool((row.client_secret if row else None) or settings.OIDC_CLIENT_SECRET), + redirect_uri=(row.redirect_uri if row else None) or settings.OIDC_REDIRECT_URI or None, + scopes=(row.scopes if row else None) or settings.OIDC_SCOPES or None, + post_login_redirect=(row.post_login_redirect if row else None) or settings.OIDC_POST_LOGIN_REDIRECT or None, + oidc_only=bool(settings.HARBORFORGE_OIDC_ONLY), + effective_enabled=cfg.configured, + source="db" if row else "env", + ) + + +@router.put("/oidc/settings", response_model=OidcSettingsOut) +def update_oidc_settings(payload: OidcSettingsIn, db: Session = Depends(get_db), + _: models.User = Depends(_require_admin_any)): + row = db.query(OidcSettings).filter(OidcSettings.id == 1).first() + if row is None: + row = OidcSettings(id=1, enabled=False) + db.add(row) + + if payload.enabled is not None: + row.enabled = payload.enabled + if payload.issuer is not None: + row.issuer = payload.issuer.strip() or None + if payload.client_id is not None: + row.client_id = payload.client_id.strip() or None + # client_secret: only overwrite when a non-empty value is supplied + if payload.client_secret: + row.client_secret = payload.client_secret + if payload.redirect_uri is not None: + row.redirect_uri = payload.redirect_uri.strip() or None + if payload.scopes is not None: + row.scopes = payload.scopes.strip() or None + if payload.post_login_redirect is not None: + row.post_login_redirect = payload.post_login_redirect.strip() or None + + db.commit() + _invalidate_client() + return get_oidc_settings(db=db, _=_) diff --git a/app/main.py b/app/main.py index de2eacd..d44e375 100644 --- a/app/main.py +++ b/app/main.py @@ -427,7 +427,7 @@ def _sync_default_user_roles(db): @app.on_event("startup") def startup(): from app.core.config import Base, engine, SessionLocal - from app.models import models, webhook, apikey, activity, milestone, notification, worklog, monitor, role_permission, task, support, meeting, proposal, propose, essential, agent, calendar, minimum_workload, schedule_type + from app.models import models, webhook, apikey, activity, milestone, notification, worklog, monitor, role_permission, task, support, meeting, proposal, propose, essential, agent, calendar, minimum_workload, schedule_type, oidc_settings Base.metadata.create_all(bind=engine) _migrate_schema() diff --git a/app/models/oidc_settings.py b/app/models/oidc_settings.py new file mode 100644 index 0000000..4c8694e --- /dev/null +++ b/app/models/oidc_settings.py @@ -0,0 +1,22 @@ +from sqlalchemy import Column, Integer, String, Boolean, DateTime +from sqlalchemy.sql import func +from app.core.config import Base + + +class OidcSettings(Base): + """Single-row (id=1) runtime OIDC configuration. + + When a row exists its non-empty fields override the OIDC_* env vars, + so the provider can be configured from the admin UI without a redeploy. + """ + __tablename__ = "oidc_settings" + + id = Column(Integer, primary_key=True, default=1) + enabled = Column(Boolean, default=False, nullable=False) + issuer = Column(String(255), nullable=True) + client_id = Column(String(255), nullable=True) + client_secret = Column(String(512), nullable=True) + redirect_uri = Column(String(512), nullable=True) + scopes = Column(String(255), nullable=True) + post_login_redirect = Column(String(512), nullable=True) + updated_at = Column(DateTime(timezone=True), server_default=func.now(), onupdate=func.now())