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()