From 94155614f5ee55a2cf76cc0993c87d752e33e52b Mon Sep 17 00:00:00 2001 From: hzhang Date: Sun, 17 May 2026 20:22:04 +0100 Subject: [PATCH 1/6] feat(auth): OIDC login + identity binding + HARBORFORGE_OIDC_ONLY - Generic OIDC (Authlib discovery) Authorization Code flow; backend issues the existing HS256 JWT on success. Unbound identities are rejected (no auto-provisioning). - User.oidc_issuer/oidc_subject (unique together) + startup migration. - PUT/DELETE /users/{id}/oidc-binding (admin or account-manager; JWT or API key; 409 on conflict). Self-link /auth/oidc/link (non-OIDC_ONLY only). Public GET /auth/config. - HARBORFORGE_OIDC_ONLY: /auth/token rejected, create/update ignore password (passwordless users; API keys + OIDC still work). - Dockerfile ARG/ENV HARBORFORGE_OIDC_ONLY; authlib+itsdangerous deps; SessionMiddleware for OIDC state. Fixed _user_response to expose the new binding fields. Co-Authored-By: Claude Opus 4.7 (1M context) --- Dockerfile | 6 ++ app/api/routers/auth.py | 2 + app/api/routers/oidc.py | 145 +++++++++++++++++++++++++++++++++++++++ app/api/routers/users.py | 79 ++++++++++++++++++++- app/core/config.py | 14 ++++ app/main.py | 24 +++++++ app/models/models.py | 9 ++- app/schemas/schemas.py | 4 +- requirements.txt | 2 + 9 files changed, 280 insertions(+), 5 deletions(-) create mode 100644 app/api/routers/oidc.py diff --git a/Dockerfile b/Dockerfile index 3b72f97..ddb411f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -42,5 +42,11 @@ COPY requirements.txt ./ COPY entrypoint.sh . RUN chmod +x entrypoint.sh +# OIDC-only mode: when "true", password login is rejected, user creation +# ignores passwords (passwordless users that sign in via a bound OIDC +# identity / API keys). Overridable at runtime via the same env var. +ARG HARBORFORGE_OIDC_ONLY=false +ENV HARBORFORGE_OIDC_ONLY=${HARBORFORGE_OIDC_ONLY} + EXPOSE 8000 ENTRYPOINT ["./entrypoint.sh"] diff --git a/app/api/routers/auth.py b/app/api/routers/auth.py index 825675c..24b9097 100644 --- a/app/api/routers/auth.py +++ b/app/api/routers/auth.py @@ -18,6 +18,8 @@ router = APIRouter(prefix="/auth", tags=["Auth"]) @router.post("/token", response_model=Token) async def login(form_data: OAuth2PasswordRequestForm = Depends(), db: Session = Depends(get_db)): + if settings.HARBORFORGE_OIDC_ONLY: + raise HTTPException(status_code=403, detail="Password login is disabled (OIDC only)") user = db.query(models.User).filter(models.User.username == form_data.username).first() if not user or not verify_password(form_data.password, user.hashed_password or ""): raise HTTPException(status_code=401, detail="Incorrect username or password", diff --git a/app/api/routers/oidc.py b/app/api/routers/oidc.py new file mode 100644 index 0000000..00cc048 --- /dev/null +++ b/app/api/routers/oidc.py @@ -0,0 +1,145 @@ +"""OIDC (OpenID Connect) login + public auth-config. + +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. + +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. +""" +from datetime import timedelta +from urllib.parse import urlencode + +from fastapi import APIRouter, Depends, HTTPException, Request +from fastapi.responses import RedirectResponse +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 + +router = APIRouter(prefix="/auth", tags=["Auth"]) + +# Authlib registry — only configured when OIDC is enabled. +_oauth = 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): + raise HTTPException(status_code=503, detail="OIDC is not configured") + if _oauth is None: + 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}, + ) + _oauth = oauth + 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 "/" + url = base + if suffix_qs: + url += ("&" if "?" in base else "?") + urlencode(suffix_qs) + if fragment: + url += "#" + fragment + return url + + +@router.get("/config") +def auth_config(): + """Public: lets the frontend decide which login UI to render.""" + return { + "oidc_enabled": bool(settings.OIDC_ENABLED and settings.OIDC_ISSUER and settings.OIDC_CLIENT_ID), + "oidc_only": bool(settings.HARBORFORGE_OIDC_ONLY), + "password_login": not bool(settings.HARBORFORGE_OIDC_ONLY), + "oidc_login_url": "/auth/oidc/login", + } + + +@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) + request.session.pop("hf_oidc_uid", None) + request.session["hf_oidc_mode"] = "login" + return await oidc.authorize_redirect(request, settings.OIDC_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).""" + if settings.HARBORFORGE_OIDC_ONLY: + raise HTTPException(status_code=403, detail="Self-service linking is disabled in OIDC-only mode") + oidc = _get_oidc() + request.session["hf_oidc_mode"] = "link" + request.session["hf_oidc_uid"] = current_user.id + return await oidc.authorize_redirect(request, settings.OIDC_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() + 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"})) + + claims = token.get("userinfo") or {} + if not claims: + try: + claims = await oidc.userinfo(token=token) + except Exception: + claims = {} + subject = claims.get("sub") + issuer = claims.get("iss") or settings.OIDC_ISSUER + if not subject: + return RedirectResponse(_frontend({"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"})) + user = db.query(models.User).filter(models.User.id == link_uid).first() + if not user: + return RedirectResponse(_frontend({"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"})) + user.oidc_issuer = issuer + user.oidc_subject = subject + db.commit() + return RedirectResponse(_frontend({"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"})) + + 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}))) diff --git a/app/api/routers/users.py b/app/api/routers/users.py index 22df8f1..b0a84db 100644 --- a/app/api/routers/users.py +++ b/app/api/routers/users.py @@ -8,7 +8,7 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session from app.api.deps import get_current_user, get_current_user_or_apikey, get_password_hash -from app.core.config import get_db +from app.core.config import get_db, settings from app.init_wizard import DELETED_USER_USERNAME from app.models import models from app.models.agent import Agent @@ -32,6 +32,8 @@ def _user_response(user: models.User) -> dict: "role_name": user.role_name, "agent_id": user.agent.agent_id if user.agent else None, "discord_user_id": user.discord_user_id, + "oidc_issuer": user.oidc_issuer, + "oidc_subject": user.oidc_subject, "created_at": user.created_at, } return data @@ -111,7 +113,13 @@ def create_user( raise HTTPException(status_code=400, detail="agent_id already in use") assigned_role = _resolve_user_role(db, user.role_id) - hashed_password = get_password_hash(user.password) if user.password else None + # In OIDC-only mode, ignore any supplied password: the user is created + # passwordless (cannot password-login) and is expected to sign in via a + # bound OIDC identity. API keys still work for such users. + if settings.HARBORFORGE_OIDC_ONLY: + hashed_password = None + else: + hashed_password = get_password_hash(user.password) if user.password else None db_user = models.User( username=user.username, email=user.email, @@ -191,7 +199,7 @@ def update_user( if payload.full_name is not None: user.full_name = payload.full_name - if payload.password is not None and payload.password.strip(): + if payload.password is not None and payload.password.strip() and not settings.HARBORFORGE_OIDC_ONLY: user.hashed_password = get_password_hash(payload.password) if payload.role_id is not None: @@ -414,3 +422,68 @@ def list_user_worklogs( if current_user.id != user.id and not current_user.is_admin: raise HTTPException(status_code=403, detail="Forbidden") return db.query(WorkLog).filter(WorkLog.user_id == user.id).order_by(WorkLog.logged_date.desc()).limit(limit).all() + + +# ---- OIDC identity binding ------------------------------------------------ + +class OidcBindingRequest(BaseModel): + issuer: str + subject: str + + +class OidcBindingResponse(BaseModel): + user_id: int + username: str + oidc_issuer: str | None = None + oidc_subject: str | None = None + + +@router.put("/{identifier}/oidc-binding", response_model=OidcBindingResponse) +def bind_user_oidc( + identifier: str, + payload: OidcBindingRequest, + db: Session = Depends(get_db), + _: models.User = Depends(require_account_creator), +): + """Bind an hf user to an external OIDC identity (issuer + subject). + + Admin or account-manager only (JWT or API key). One OIDC identity maps + to at most one user.""" + issuer = (payload.issuer or "").strip() + subject = (payload.subject or "").strip() + if not issuer or not subject: + raise HTTPException(status_code=400, detail="issuer and subject are required") + user = _find_user_by_id_or_username(db, identifier) + if not user: + raise HTTPException(status_code=404, detail="User not found") + clash = db.query(models.User).filter( + models.User.oidc_issuer == issuer, + models.User.oidc_subject == subject, + models.User.id != user.id, + ).first() + if clash: + raise HTTPException(status_code=409, detail=f"OIDC identity already bound to user '{clash.username}'") + user.oidc_issuer = issuer + user.oidc_subject = subject + db.commit() + db.refresh(user) + return OidcBindingResponse(user_id=user.id, username=user.username, + oidc_issuer=user.oidc_issuer, oidc_subject=user.oidc_subject) + + +@router.delete("/{identifier}/oidc-binding", response_model=OidcBindingResponse) +def unbind_user_oidc( + identifier: str, + db: Session = Depends(get_db), + _: models.User = Depends(require_account_creator), +): + """Remove a user's OIDC binding. Admin or account-manager only.""" + user = _find_user_by_id_or_username(db, identifier) + if not user: + raise HTTPException(status_code=404, detail="User not found") + user.oidc_issuer = None + user.oidc_subject = None + db.commit() + db.refresh(user) + return OidcBindingResponse(user_id=user.id, username=user.username, + oidc_issuer=None, oidc_subject=None) diff --git a/app/core/config.py b/app/core/config.py index e011199..27d1c1d 100644 --- a/app/core/config.py +++ b/app/core/config.py @@ -38,6 +38,20 @@ class Settings(BaseSettings): ALGORITHM: str = "HS256" ACCESS_TOKEN_EXPIRE_MINUTES: int = 30 + # --- OIDC (generic, OpenID Connect discovery) --- + OIDC_ENABLED: bool = False + OIDC_ISSUER: str = "" # e.g. https://idp.example.com (we use {issuer}/.well-known/openid-configuration) + OIDC_CLIENT_ID: str = "" + OIDC_CLIENT_SECRET: str = "" + OIDC_REDIRECT_URI: str = "" # backend callback, e.g. https://hf-api.example.com/auth/oidc/callback + OIDC_SCOPES: str = "openid email profile" + OIDC_POST_LOGIN_REDIRECT: str = "" # frontend URL to return to (token in fragment). Falls back to "/" + + # When true: no password login at all. Password login endpoint rejects, + # user creation ignores any password (passwordless user that can only use + # API keys / OIDC), and the frontend hides all password UI. + HARBORFORGE_OIDC_ONLY: bool = False + class Config: env_file = ".env" diff --git a/app/main.py b/app/main.py index 0baa3c0..de2eacd 100644 --- a/app/main.py +++ b/app/main.py @@ -1,6 +1,9 @@ """HarborForge API — Agent/人类协同任务管理平台""" from fastapi import FastAPI from fastapi.middleware.cors import CORSMiddleware +from starlette.middleware.sessions import SessionMiddleware + +from app.core.config import settings app = FastAPI( title="HarborForge API", @@ -17,6 +20,17 @@ app.add_middleware( allow_headers=["*"], ) +# Short-lived signed session cookie — only used to carry the OIDC +# state/nonce between /auth/oidc/login and the callback. +app.add_middleware( + SessionMiddleware, + secret_key=settings.SECRET_KEY, + session_cookie="hf_oidc", + same_site="lax", + https_only=False, + max_age=600, +) + # Health & version (kept at top level) @app.get("/health", tags=["System"]) def health_check(): @@ -65,8 +79,10 @@ from app.api.routers.meetings import router as meetings_router from app.api.routers.essentials import router as essentials_router from app.api.routers.schedule_type import router as schedule_type_router from app.api.routers.calendar import router as calendar_router +from app.api.routers.oidc import router as oidc_router app.include_router(auth_router) +app.include_router(oidc_router) app.include_router(tasks_router) app.include_router(projects_router) app.include_router(users_router) @@ -277,6 +293,14 @@ def _migrate_schema(): if _has_table(db, "users") and not _has_column(db, "users", "discord_user_id"): db.execute(text("ALTER TABLE users ADD COLUMN discord_user_id VARCHAR(32) NULL")) + # --- users OIDC binding (issuer + subject), unique together --- + if _has_table(db, "users") and not _has_column(db, "users", "oidc_issuer"): + db.execute(text("ALTER TABLE users ADD COLUMN oidc_issuer VARCHAR(255) NULL")) + if _has_table(db, "users") and not _has_column(db, "users", "oidc_subject"): + db.execute(text("ALTER TABLE users ADD COLUMN oidc_subject VARCHAR(255) NULL")) + if _has_table(db, "users") and _has_column(db, "users", "oidc_subject"): + _ensure_unique_index(db, "users", "uq_users_oidc_identity", "oidc_issuer, oidc_subject") + # --- monitored_servers.api_key for heartbeat v2 --- if _has_table(db, "monitored_servers") and not _has_column(db, "monitored_servers", "api_key"): db.execute(text("ALTER TABLE monitored_servers ADD COLUMN api_key VARCHAR(64) NULL")) diff --git a/app/models/models.py b/app/models/models.py index 8e05ca1..db1cd00 100644 --- a/app/models/models.py +++ b/app/models/models.py @@ -1,4 +1,4 @@ -from sqlalchemy import Column, Integer, String, Text, DateTime, ForeignKey, Enum, Boolean, JSON, Time +from sqlalchemy import Column, Integer, String, Text, DateTime, ForeignKey, Enum, Boolean, JSON, Time, UniqueConstraint from sqlalchemy.orm import relationship from sqlalchemy.sql import func from app.core.config import Base @@ -66,6 +66,9 @@ class Project(Base): class User(Base): __tablename__ = "users" + __table_args__ = ( + UniqueConstraint("oidc_issuer", "oidc_subject", name="uq_users_oidc_identity"), + ) id = Column(Integer, primary_key=True, index=True) username = Column(String(50), unique=True, nullable=False) @@ -73,6 +76,10 @@ class User(Base): hashed_password = Column(String(255), nullable=True) full_name = Column(String(100), nullable=True) discord_user_id = Column(String(32), nullable=True) + # OIDC binding: an hf user is linked to at most one external OIDC identity + # (issuer + subject). Unique together so one IdP identity maps to one user. + oidc_issuer = Column(String(255), nullable=True) + oidc_subject = Column(String(255), nullable=True) is_active = Column(Boolean, default=True) is_admin = Column(Boolean, default=False) role_id = Column(Integer, ForeignKey("roles.id"), nullable=True) diff --git a/app/schemas/schemas.py b/app/schemas/schemas.py index cec1e59..5f3ebe6 100644 --- a/app/schemas/schemas.py +++ b/app/schemas/schemas.py @@ -194,8 +194,10 @@ class UserResponse(UserBase): role_name: Optional[str] = None agent_id: Optional[str] = None discord_user_id: Optional[str] = None + oidc_issuer: Optional[str] = None + oidc_subject: Optional[str] = None created_at: datetime - + class Config: from_attributes = True diff --git a/requirements.txt b/requirements.txt index 85208cb..5d79515 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,3 +12,5 @@ alembic==1.13.1 python-dotenv==1.0.0 httpx==0.27.0 requests==2.31.0 +authlib==1.3.2 +itsdangerous==2.2.0 From a115e380cbd33323efa0e12b9e2679c2c6b8ec67 Mon Sep 17 00:00:00 2001 From: hzhang Date: Sun, 17 May 2026 20:29:15 +0100 Subject: [PATCH 2/6] 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()) From ffce4298c83d96b11ea3c79017b41ba4127564a2 Mon Sep 17 00:00:00 2001 From: hzhang Date: Sun, 17 May 2026 20:40:26 +0100 Subject: [PATCH 3/6] docs: OIDC feature test plan / test points Test points for OIDC login, user binding, HARBORFORGE_OIDC_ONLY mode, and the admin OIDC settings page, with local verification status. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/oidc-test-plan.md | 99 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 docs/oidc-test-plan.md diff --git a/docs/oidc-test-plan.md b/docs/oidc-test-plan.md new file mode 100644 index 0000000..5054154 --- /dev/null +++ b/docs/oidc-test-plan.md @@ -0,0 +1,99 @@ +# OIDC 接入 — 测试点描述 + +覆盖范围:OIDC 登录、hf 用户与 OIDC 身份绑定、`HARBORFORGE_OIDC_ONLY` +模式、管理员 OIDC 配置页面。涉及仓库分支 `feature/oidc-login` +(`HarborForge.Backend` + `HarborForge.Frontend`)。 + +“本地状态” 列:✅ 已用真实 Keycloak 在本地栈端到端验证;🟡 已用接口/构建 +验证但未走真实 IdP UI;⬜ 待测。 + +## 0. 测试环境 + +- 后端 `http://127.0.0.1:18000`、前端 `http://127.0.0.1:13000`(本地验证栈) +- IdP:Keycloak 容器,realm `hf`,confidential client `hf-client` +- IdP 测试用户:`tester` / `Test123!`(emailVerified=true) +- 关键约束:**issuer URL 必须浏览器与后端容器都能用同一地址访问** + (否则 token `iss` 校验失败)。本地用宿主机 IP 统一两端。 +- 配置项(运行时 env 或 DB,DB 覆盖 env): + `OIDC_ENABLED / OIDC_ISSUER / OIDC_CLIENT_ID / OIDC_CLIENT_SECRET / + OIDC_REDIRECT_URI / OIDC_SCOPES / OIDC_POST_LOGIN_REDIRECT`; + 部署级 `HARBORFORGE_OIDC_ONLY`(Docker ARG/ENV,前后端均有)。 + +## 1. 管理员 OIDC 配置(页面 + API) + +| # | 测试点 | 步骤 | 预期 | 本地 | +|---|--------|------|------|------| +|1.1|读取初始配置|`GET /auth/oidc/settings`(admin)|返回 `source=env`,`has_client_secret=false`,`effective_enabled` 反映 env|✅| +|1.2|保存配置|`PUT /auth/oidc/settings` 填 issuer/client/secret/redirect/scopes/post_login|200,`source=db`,`effective_enabled=true`|✅| +|1.3|client_secret 只写不回显|保存后再 `GET`|响应无明文 secret,仅 `has_client_secret=true`|✅| +|1.4|secret 留空保留原值|`PUT` 不带 `client_secret`|原 secret 不变,登录仍可用|🟡| +|1.5|配置即时生效|保存后 `GET /auth/config`|`oidc_enabled` 立即变化,无需重启|✅| +|1.6|页面仅 admin 可见|非 admin 访问 `/settings/oidc`|被重定向到 `/`;侧栏无入口;API 返回 401/403|🟡| +|1.7|页面展示 Callback URL|打开 OIDC 设置页|醒目显示需在 IdP 注册的 redirect/callback URL、当前状态与配置来源|🟡| + +## 2. OIDC 登录流程(授权码) + +| # | 测试点 | 步骤 | 预期 | 本地 | +|---|--------|------|------|------| +|2.1|发起登录|`GET /auth/oidc/login`|302 跳转到 IdP authorize 端点(state/nonce 入会话 cookie)|✅| +|2.2|IdP 登录成功回跳|在 IdP 输入 `tester/Test123!`|302 回 `…/auth/oidc/callback?code=…&state=…`|✅| +|2.3|回调换码并签发|后端处理 callback|校验 ID token(JWKS)→定位绑定用户→签发 hf JWT→302 到前端 `post_login#token=…`|✅| +|2.4|登录态可用|用返回 token 调 `GET /auth/me`|返回被绑定的 hf 用户|✅| +|2.5|前端按钮|登录页点 “Sign in with SSO”|全页跳转到后端 `/auth/oidc/login`|🟡| +|2.6|未配置 OIDC|`OIDC` 关闭时访问 `/auth/oidc/login`|503 “OIDC is not configured”|🟡| +|2.7|token 交换失败|code 失效/被篡改|回前端 `?oidc_error=exchange_failed`,登录页提示|⬜| + +## 3. hf 用户 ↔ OIDC 身份绑定 + +| # | 测试点 | 步骤 | 预期 | 本地 | +|---|--------|------|------|------| +|3.1|管理员绑定|`PUT /users/{id}/oidc-binding` {issuer,subject}(admin 或 acc-manager)|200,用户响应含 `oidc_issuer/oidc_subject`|✅| +|3.2|未绑定身份拒绝登录|解绑后用该 IdP 账号登录|回 `?oidc_error=not_linked`,**不签发 token**(不自动开号)|✅| +|3.3|身份唯一性|把同一 (issuer,subject) 绑到另一用户|409 冲突|✅| +|3.4|解绑|`DELETE /users/{id}/oidc-binding`|200,绑定清空|✅| +|3.5|绑定鉴权|无凭据 / 普通用户调用绑定 API|401 / 403|✅| +|3.6|API key 通道|admin 的 API key 调用绑定/配置 API|200(支持 JWT 或 API key)|✅| +|3.7|自助绑定(非 ONLY)|登录用户点侧栏 “Link OIDC account” 走一次 OIDC|回 `?oidc_linked=1`,当前账号绑定成功|🟡| +|3.8|自助绑定冲突|自助绑定到已被占用的身份|`?oidc_error=already_bound`|⬜| +|3.9|OIDC_ONLY 下禁自助|ONLY 模式访问 `/auth/oidc/link`|403(仅管理员 API 可绑)|🟡| + +## 4. HARBORFORGE_OIDC_ONLY 模式 + +| # | 测试点 | 步骤 | 预期 | 本地 | +|---|--------|------|------|------| +|4.1|配置反映|ONLY=true 时 `GET /auth/config`|`oidc_only=true`,`password_login=false`|✅| +|4.2|禁用密码登录|`POST /auth/token`|403 “Password login is disabled (OIDC only)”|✅| +|4.3|建用户忽略密码|`POST /users` 带 password|201,但 DB `hashed_password=NULL`(无密码用户)|✅| +|4.4|改用户忽略密码|`PATCH /users/{id}` 带 password|密码不被设置|🟡| +|4.5|无密码用户仍可用|对该用户绑定 OIDC、生成 API key|绑定 200、`reset-apikey` 200;可经 OIDC 登录|✅| +|4.6|前端隐藏密码 UI|ONLY 模式打开登录页 / 用户管理页|无用户名密码框;用户管理无密码/重置密码组件|🟡| +|4.7|防锁死恢复|ONLY 模式且 OIDC 配错|管理员 API key 仍可调 `GET/PUT /auth/oidc/settings` 修复|✅| + +## 5. 回归(不破坏既有功能) + +| # | 测试点 | 步骤 | 预期 | 本地 | +|---|--------|------|------|------| +|5.1|默认模式密码登录|未开 ONLY 时 `POST /auth/token`|正常 200 签发 token|✅| +|5.2|用户响应新增字段|`GET /users` `/users/{id}` `/auth/me`|含 `oidc_issuer/oidc_subject`(未绑定为 null)|✅| +|5.3|启动迁移幂等|新旧库重复启动后端|`users.oidc_*` 列与 `oidc_settings` 表存在,无报错|✅| +|5.4|前端构建|`npm run build`(Docker 镜像)|TS 编译通过|✅| +|5.5|后端导入|镜像内 `import app.main`|无导入错误,OIDC 路由注册|✅| +|5.6|镜像参数|前后端 Dockerfile|含 `ARG/ENV HARBORFORGE_OIDC_ONLY`|✅| + +## 6. 安全检查点 + +- ID token 经 IdP JWKS 校验(Authlib discovery + nonce);签发的是 hf 原有 + HS256 JWT,依赖强 `SECRET_KEY`(弱 key 后端拒绝启动)。 +- `client_secret` 持久化在 DB,接口永不回显。 +- 绑定/配置接口强制 admin(或 account-manager 绑定),支持 API key 作为 + OIDC_ONLY 下的恢复通道。 +- 未绑定 OIDC 身份一律拒绝,不自动开号(防越权开户)。 +- `redirect_uri` 必须在 IdP 精确注册;后端回跳前端地址来自服务端配置 + (`post_login_redirect`),不接受客户端传入,避免开放重定向。 + +## 7. 已知限制 / 待补 + +- 真实浏览器交互(点按钮、IdP 同意页、SameSite cookie 行为)需人工过一遍 + (本地已用 curl 无头跑通授权码全流程,逻辑等价)。 +- 多 IdP / 多 issuer、token 刷新、登出联动(SLO)未实现,超出本次范围。 +- 自助绑定相关 UI(3.7/3.8)建议人工在浏览器复核。 From 0229fbb54c1b6cea7bcf42217896b6e79093ae36 Mon Sep 17 00:00:00 2001 From: hzhang Date: Sun, 17 May 2026 20:50:59 +0100 Subject: [PATCH 4/6] feat(init): bootstrap OIDC from wizard config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit init_wizard applies config['oidc'] on first init: creates the oidc_settings row and, when admin_subject is given, binds the bootstrap admin so OIDC-only deployments are reachable. Idempotent — an existing row / admin binding is preserved (later admin edits via the API survive restarts). Co-Authored-By: Claude Opus 4.7 (1M context) --- app/init_wizard.py | 47 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/app/init_wizard.py b/app/init_wizard.py index d49594d..c8e5d38 100644 --- a/app/init_wizard.py +++ b/app/init_wizard.py @@ -11,6 +11,7 @@ from sqlalchemy.orm import Session from app.models import models from app.models.role_permission import Role, Permission, RolePermission +from app.models.oidc_settings import OidcSettings from app.api.deps import get_password_hash logger = logging.getLogger("harborforge.init") @@ -328,6 +329,49 @@ def init_deleted_user(db: Session) -> models.User | None: return user +def init_oidc_settings(db: Session, oidc_cfg: dict, admin_user: models.User | None) -> None: + """Bootstrap OIDC from the wizard config (first init only). + + Creates the single oidc_settings row if absent so the deployment comes + up with OIDC configured. If admin_subject is given, binds the bootstrap + admin so it can sign in (critical in OIDC-only mode). Idempotent: an + existing row / existing admin binding is left untouched so later admin + edits via the API are not clobbered on restart.""" + if not oidc_cfg: + return + + existing = db.query(OidcSettings).filter(OidcSettings.id == 1).first() + if existing is None: + db.add(OidcSettings( + id=1, + enabled=bool(oidc_cfg.get("enabled", True)), + issuer=(oidc_cfg.get("issuer") or "").strip() or None, + client_id=(oidc_cfg.get("client_id") or "").strip() or None, + client_secret=oidc_cfg.get("client_secret") or None, + redirect_uri=(oidc_cfg.get("redirect_uri") or "").strip() or None, + scopes=(oidc_cfg.get("scopes") or "").strip() or None, + post_login_redirect=(oidc_cfg.get("post_login_redirect") or "").strip() or None, + )) + db.commit() + logger.info("OIDC settings bootstrapped from wizard config") + + admin_subject = (oidc_cfg.get("admin_subject") or "").strip() + issuer = (oidc_cfg.get("issuer") or "").strip() + if admin_user and admin_subject and issuer and not admin_user.oidc_subject: + clash = db.query(models.User).filter( + models.User.oidc_issuer == issuer, + models.User.oidc_subject == admin_subject, + models.User.id != admin_user.id, + ).first() + if clash: + logger.warning("Admin OIDC subject already bound to '%s'; skipping admin bind", clash.username) + else: + admin_user.oidc_issuer = issuer + admin_user.oidc_subject = admin_subject + db.commit() + logger.info("Bootstrap admin '%s' bound to OIDC subject", admin_user.username) + + def run_init(db: Session) -> None: """Main initialization entry point. Reads config from shared volume.""" config = load_config() @@ -360,4 +404,7 @@ def run_init(db: Session) -> None: if project_cfg and admin_user: init_default_project(db, project_cfg, admin_user.id, admin_user.username) + # OIDC bootstrap (provider config + optional bootstrap-admin binding) + init_oidc_settings(db, config.get("oidc") or {}, admin_user) + logger.info("Initialization complete") From 9429e375428b6eebc45c07f2db9e235d6a0819db Mon Sep 17 00:00:00 2001 From: hzhang Date: Sun, 17 May 2026 21:05:39 +0100 Subject: [PATCH 5/6] feat(auth): OIDC-only admin-role bootstrap auto-connect In OIDC-only mode, before any admin is linked, an IdP user whose token carries the configured admin role (default "admin"; OIDC_ADMIN_ROLE / oidc_settings.admin_role) auto-connects to the unbound hf admin on first OIDC sign-in, then the window self-closes once any admin is bound. Roles are scanned across userinfo + the (unverified) access token: realm_access.roles, resource_access.*.roles, roles/role/groups. Adds admin_role to settings model/env/effective/API and to the wizard bootstrap config. Replaces the manual admin-subject approach. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/api/routers/oidc.py | 72 ++++++++++++++++++++++++++++++++++++- app/core/config.py | 1 + app/init_wizard.py | 1 + app/main.py | 4 +++ app/models/oidc_settings.py | 3 ++ 5 files changed, 80 insertions(+), 1 deletion(-) diff --git a/app/api/routers/oidc.py b/app/api/routers/oidc.py index adf50e7..52a7706 100644 --- a/app/api/routers/oidc.py +++ b/app/api/routers/oidc.py @@ -7,6 +7,7 @@ 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. """ +import logging from datetime import timedelta from urllib.parse import urlencode @@ -21,13 +22,14 @@ 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"]) +logger = logging.getLogger("harborforge.oidc") # ---- effective config (DB row overrides env) ------------------------------ class EffectiveOidc: def __init__(self, enabled, issuer, client_id, client_secret, - redirect_uri, scopes, post_login_redirect): + redirect_uri, scopes, post_login_redirect, admin_role): self.enabled = enabled self.issuer = issuer self.client_id = client_id @@ -35,6 +37,7 @@ class EffectiveOidc: self.redirect_uri = redirect_uri self.scopes = scopes or "openid email profile" self.post_login_redirect = post_login_redirect + self.admin_role = (admin_role or "admin").strip() or "admin" @property def configured(self) -> bool: @@ -58,6 +61,7 @@ def get_effective_oidc(db: Session) -> 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, + settings.OIDC_ADMIN_ROLE, ) return EffectiveOidc( bool(row.enabled), @@ -67,6 +71,7 @@ def get_effective_oidc(db: Session) -> EffectiveOidc: pick(row.redirect_uri, settings.OIDC_REDIRECT_URI), pick(row.scopes, settings.OIDC_SCOPES), pick(row.post_login_redirect, settings.OIDC_POST_LOGIN_REDIRECT), + pick(getattr(row, "admin_role", None), settings.OIDC_ADMIN_ROLE), ) @@ -100,6 +105,39 @@ def _invalidate_client(): _oauth_fp = None +def _collect_roles(claims: dict, token: dict) -> set[str]: + """Roles from common OIDC claim shapes, across the ID-token/userinfo + claims and the (unverified) access token — Keycloak puts realm/client + roles in the access token by default.""" + pools = [claims if isinstance(claims, dict) else {}] + at = token.get("access_token") + if at: + try: + from jose import jwt as _jwt + pools.append(_jwt.get_unverified_claims(at)) + except Exception: + pass + roles: set[str] = set() + for p in pools: + if not isinstance(p, dict): + continue + ra = p.get("realm_access") + if isinstance(ra, dict): + roles.update(ra.get("roles") or []) + res = p.get("resource_access") + if isinstance(res, dict): + for v in res.values(): + if isinstance(v, dict): + roles.update(v.get("roles") or []) + for key in ("roles", "role", "groups"): + val = p.get(key) + if isinstance(val, str): + roles.add(val) + elif isinstance(val, (list, tuple)): + roles.update(str(x) for x in val) + return {str(r).strip().lstrip("/").lower() for r in roles if r} + + def _frontend(cfg: EffectiveOidc, qs: dict | None = None, fragment: str | None = None) -> str: base = cfg.post_login_redirect or "/" url = base @@ -190,6 +228,33 @@ async def oidc_callback(request: Request, db: Session = Depends(get_db)): models.User.oidc_issuer == issuer, models.User.oidc_subject == subject, ).first() + + # OIDC-only bootstrap: before any admin is linked, an IdP user whose + # token carries the configured admin role auto-connects to the unbound + # hf admin. Self-closes once any admin is bound. + if user is None and settings.HARBORFORGE_OIDC_ONLY: + any_admin_bound = db.query(models.User).filter( + models.User.is_admin == True, # noqa: E712 + models.User.oidc_subject.isnot(None), + ).first() + if not any_admin_bound and cfg.admin_role.lower() in _collect_roles(claims, token): + taken = db.query(models.User).filter( + models.User.oidc_issuer == issuer, + models.User.oidc_subject == subject, + ).first() + if taken is None: + boot = db.query(models.User).filter( + models.User.is_admin == True, # noqa: E712 + models.User.is_active == True, # noqa: E712 + models.User.oidc_subject.is_(None), + ).order_by(models.User.id).first() + if boot is not None: + boot.oidc_issuer = issuer + boot.oidc_subject = subject + db.commit() + logger.info("OIDC bootstrap: auto-connected admin '%s' via admin role", boot.username) + user = boot + if not user or not user.is_active or user.username in ("acc-mgr", "deleted-user"): return RedirectResponse(_frontend(cfg, {"oidc_error": "not_linked"})) @@ -218,6 +283,7 @@ class OidcSettingsIn(BaseModel): redirect_uri: str | None = None scopes: str | None = None post_login_redirect: str | None = None + admin_role: str | None = None class OidcSettingsOut(BaseModel): @@ -228,6 +294,7 @@ class OidcSettingsOut(BaseModel): redirect_uri: str | None scopes: str | None post_login_redirect: str | None + admin_role: str oidc_only: bool # read-only (deploy env) effective_enabled: bool # provider actually usable source: str # "db" or "env" @@ -245,6 +312,7 @@ def get_oidc_settings(db: Session = Depends(get_db), _: models.User = Depends(_r 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, + admin_role=cfg.admin_role, oidc_only=bool(settings.HARBORFORGE_OIDC_ONLY), effective_enabled=cfg.configured, source="db" if row else "env", @@ -274,6 +342,8 @@ def update_oidc_settings(payload: OidcSettingsIn, db: Session = Depends(get_db), 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 + if payload.admin_role is not None: + row.admin_role = payload.admin_role.strip() or None db.commit() _invalidate_client() diff --git a/app/core/config.py b/app/core/config.py index 27d1c1d..18e0271 100644 --- a/app/core/config.py +++ b/app/core/config.py @@ -46,6 +46,7 @@ class Settings(BaseSettings): OIDC_REDIRECT_URI: str = "" # backend callback, e.g. https://hf-api.example.com/auth/oidc/callback OIDC_SCOPES: str = "openid email profile" OIDC_POST_LOGIN_REDIRECT: str = "" # frontend URL to return to (token in fragment). Falls back to "/" + OIDC_ADMIN_ROLE: str = "admin" # OIDC role name that bootstraps the unbound hf admin (OIDC-only) # When true: no password login at all. Password login endpoint rejects, # user creation ignores any password (passwordless user that can only use diff --git a/app/init_wizard.py b/app/init_wizard.py index c8e5d38..4ac7ad7 100644 --- a/app/init_wizard.py +++ b/app/init_wizard.py @@ -351,6 +351,7 @@ def init_oidc_settings(db: Session, oidc_cfg: dict, admin_user: models.User | No redirect_uri=(oidc_cfg.get("redirect_uri") or "").strip() or None, scopes=(oidc_cfg.get("scopes") or "").strip() or None, post_login_redirect=(oidc_cfg.get("post_login_redirect") or "").strip() or None, + admin_role=(oidc_cfg.get("admin_role") or "").strip() or None, )) db.commit() logger.info("OIDC settings bootstrapped from wizard config") diff --git a/app/main.py b/app/main.py index d44e375..45d9a7a 100644 --- a/app/main.py +++ b/app/main.py @@ -301,6 +301,10 @@ def _migrate_schema(): if _has_table(db, "users") and _has_column(db, "users", "oidc_subject"): _ensure_unique_index(db, "users", "uq_users_oidc_identity", "oidc_issuer, oidc_subject") + # --- oidc_settings.admin_role (added after the table shipped) --- + if _has_table(db, "oidc_settings") and not _has_column(db, "oidc_settings", "admin_role"): + db.execute(text("ALTER TABLE oidc_settings ADD COLUMN admin_role VARCHAR(128) NULL")) + # --- monitored_servers.api_key for heartbeat v2 --- if _has_table(db, "monitored_servers") and not _has_column(db, "monitored_servers", "api_key"): db.execute(text("ALTER TABLE monitored_servers ADD COLUMN api_key VARCHAR(64) NULL")) diff --git a/app/models/oidc_settings.py b/app/models/oidc_settings.py index 4c8694e..6edc60d 100644 --- a/app/models/oidc_settings.py +++ b/app/models/oidc_settings.py @@ -19,4 +19,7 @@ class OidcSettings(Base): redirect_uri = Column(String(512), nullable=True) scopes = Column(String(255), nullable=True) post_login_redirect = Column(String(512), nullable=True) + # OIDC role name that, in OIDC-only mode, auto-connects an unbound + # hf admin on first login (bootstrap). Default "admin". + admin_role = Column(String(128), nullable=True) updated_at = Column(DateTime(timezone=True), server_default=func.now(), onupdate=func.now()) From a9d075bc191fd2c66106234c27191400c772097a Mon Sep 17 00:00:00 2001 From: hzhang Date: Sun, 17 May 2026 22:07:43 +0100 Subject: [PATCH 6/6] fix(security): block OIDC-binding privilege escalation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The oidc-binding PUT/DELETE endpoints allowed any account.create holder (non-admin role 'account-manager') to bind an attacker-controlled OIDC identity to the admin account (or unbind admin, reopening the OIDC-only bootstrap window) — full admin takeover. Non-admin callers may now only manage bindings of non-privileged accounts: requests targeting an is_admin user, the built-in acc-mgr/deleted-user, or any holder of account.create / user.reset-apikey are rejected with 403. Global admins remain unrestricted, so the intended "account-manager binds normal users" capability is preserved. Found by post-feature security audit. Verified locally. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/api/routers/users.py | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/app/api/routers/users.py b/app/api/routers/users.py index b0a84db..fee741c 100644 --- a/app/api/routers/users.py +++ b/app/api/routers/users.py @@ -438,17 +438,38 @@ class OidcBindingResponse(BaseModel): oidc_subject: str | None = None +def _assert_can_manage_oidc_binding(db: Session, caller: models.User, target: models.User) -> None: + """Global admins may (un)bind anyone. Non-admin account managers may + only operate on non-privileged accounts — never on an admin or another + privileged account — otherwise binding an attacker-controlled OIDC + identity to an admin would be a privilege-escalation primitive.""" + if getattr(caller, "is_admin", False): + return + privileged = ( + getattr(target, "is_admin", False) + or target.username in ("acc-mgr", "deleted-user") + or _has_global_permission(db, target, "account.create") + or _has_global_permission(db, target, "user.reset-apikey") + ) + if privileged: + raise HTTPException( + status_code=403, + detail="Only a global admin may manage the OIDC binding of a privileged account", + ) + + @router.put("/{identifier}/oidc-binding", response_model=OidcBindingResponse) def bind_user_oidc( identifier: str, payload: OidcBindingRequest, db: Session = Depends(get_db), - _: models.User = Depends(require_account_creator), + caller: models.User = Depends(require_account_creator), ): """Bind an hf user to an external OIDC identity (issuer + subject). - Admin or account-manager only (JWT or API key). One OIDC identity maps - to at most one user.""" + Admin or account-manager (JWT or API key). Account managers may not + target privileged/admin accounts. One OIDC identity maps to at most + one user.""" issuer = (payload.issuer or "").strip() subject = (payload.subject or "").strip() if not issuer or not subject: @@ -456,6 +477,7 @@ def bind_user_oidc( user = _find_user_by_id_or_username(db, identifier) if not user: raise HTTPException(status_code=404, detail="User not found") + _assert_can_manage_oidc_binding(db, caller, user) clash = db.query(models.User).filter( models.User.oidc_issuer == issuer, models.User.oidc_subject == subject, @@ -475,12 +497,14 @@ def bind_user_oidc( def unbind_user_oidc( identifier: str, db: Session = Depends(get_db), - _: models.User = Depends(require_account_creator), + caller: models.User = Depends(require_account_creator), ): - """Remove a user's OIDC binding. Admin or account-manager only.""" + """Remove a user's OIDC binding. Admin or account-manager; account + managers may not target privileged/admin accounts.""" user = _find_user_by_id_or_username(db, identifier) if not user: raise HTTPException(status_code=404, detail="User not found") + _assert_can_manage_oidc_binding(db, caller, user) user.oidc_issuer = None user.oidc_subject = None db.commit()