From f519393cc04e8f4747ddb66ac774a94e9e09f781 Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Thu, 1 Oct 2026 20:04:13 +0200 Subject: [PATCH 1/3] feat(server-api): SCIM 2.0 user and group provisioning with immediate deactivation Refs #1682 --- .../geolibre_server_api/auth.py | 24 +- .../geolibre_server_api/enterprise_admin.py | 89 +- .../geolibre_server_api/enterprise_models.py | 57 + .../geolibre_server_api/main.py | 18 + .../geolibre_server_api/oidc.py | 49 +- .../geolibre_server_api/policy.py | 45 + .../geolibre_server_api/scim.py | 1083 +++++++++++++++++ .../geolibre_server_api/tests/test_scim.py | 583 +++++++++ .../tests/test_scim_concurrency.py | 71 ++ docs/server-api.md | 109 ++ 10 files changed, 2122 insertions(+), 6 deletions(-) create mode 100644 backend/geolibre_server_api/geolibre_server_api/scim.py create mode 100644 backend/geolibre_server_api/tests/test_scim.py create mode 100644 backend/geolibre_server_api/tests/test_scim_concurrency.py diff --git a/backend/geolibre_server_api/geolibre_server_api/auth.py b/backend/geolibre_server_api/geolibre_server_api/auth.py index 673ca1962f..5b59d4fe79 100644 --- a/backend/geolibre_server_api/geolibre_server_api/auth.py +++ b/backend/geolibre_server_api/geolibre_server_api/auth.py @@ -69,6 +69,7 @@ credential_expired, effective_policy, ensure_account_security, + is_deactivated, password_login_allowed, password_policy_error, record_failed_login, @@ -506,6 +507,9 @@ def __init__(self, code: str): "Your organization requires single sign-on. Use “Sign in with your organization”." ), } +# Single sign-on and proxy sign-in of a SCIM-deactivated account. Password +# sign-in reports "invalid" instead, so it reveals nothing to a password guesser. +DEACTIVATED_MESSAGE = "This account has been deactivated." def verify_password_login( @@ -533,6 +537,8 @@ def verify_password_login( if not password_matches(password, account.password_hash): record_failed_login(session, account.id, now_ts, policy) raise PasswordLoginError("invalid") + if is_deactivated(session, account.id): + raise PasswordLoginError("invalid") if not password_login_allowed(session, account.id): raise PasswordLoginError("sso_required") if ( @@ -579,7 +585,7 @@ def optional_principal( 401, "invalid or expired token", headers=bearer_challenge("invalid_token") ) account = session.get(Account, oauth_session.account_id) - if account is None: + if account is None or is_deactivated(session, account.id): raise HTTPException( 401, "invalid or expired token", headers=bearer_challenge("invalid_token") ) @@ -627,7 +633,7 @@ def optional_principal( 401, "invalid or expired token", headers=bearer_challenge("invalid_token") ) account = session.get(Account, token_row.account_id) - if account is None: + if account is None or is_deactivated(session, account.id): raise HTTPException( 401, "invalid or expired token", headers=bearer_challenge("invalid_token") ) @@ -1852,6 +1858,16 @@ def _complete_authorization( session.rollback() oidc.logger.warning("proxy sign-in rejected: %s", exc) return oauth_error_page(400, "invalid_request", "proxy sign-in failed") + if is_deactivated(session, account.id): + return _consent_error( + session, + interaction, + client, + csrf, + label, + DEACTIVATED_MESSAGE, + proxy_user=proxy_user, + ) return _approve_interaction( session, interaction, account.id, label, now_ts, authenticated_at=now_ts ) @@ -2114,6 +2130,8 @@ def _finish_single_sign_on(request: Request, session: Session): session.rollback() oidc.logger.warning("oidc sign-in rejected: %s", exc) return _sso_rejected() + if is_deactivated(session, account.id): + return oauth_error_page(403, "access_denied", DEACTIVATED_MESSAGE) auth_time = claims.get("auth_time") authenticated_at = ( min(auth_time, now_ts) @@ -2292,7 +2310,7 @@ def refresh_tokens( ): return oauth_token_error(400, "invalid_grant") policy = effective_policy(session, oauth_session.account_id) - if credential_expired( + if is_deactivated(session, oauth_session.account_id) or credential_expired( policy, authenticated_at=oauth_session.authenticated_at or oauth_session.created_at, last_activity_at=oauth_session.last_used_at or oauth_session.created_at, diff --git a/backend/geolibre_server_api/geolibre_server_api/enterprise_admin.py b/backend/geolibre_server_api/geolibre_server_api/enterprise_admin.py index f1cffc1db8..7ffb3d4c89 100644 --- a/backend/geolibre_server_api/geolibre_server_api/enterprise_admin.py +++ b/backend/geolibre_server_api/geolibre_server_api/enterprise_admin.py @@ -5,6 +5,7 @@ import ipaddress import json import re +import secrets import uuid from typing import Literal from urllib.parse import urlparse @@ -12,7 +13,7 @@ import httpx from fastapi import APIRouter, Depends, HTTPException, Request, Response from pydantic import BaseModel, Field -from sqlalchemy import delete, select +from sqlalchemy import delete, select, update from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session @@ -22,11 +23,13 @@ get_session, iso_ts, require_scope, + token_digest, ) from geolibre_server_api.auth_models import Account from geolibre_server_api.enterprise_models import ( OrganizationIdentityProvider, OrganizationSecurityPolicy, + ScimToken, ) from geolibre_server_api.oidc import OidcError, fetch_json from geolibre_server_api.org_models import Group, OrganizationRole @@ -206,6 +209,24 @@ def _provider_for(session: Session, organization_id: str) -> OrganizationIdentit ) +class ScimTokenBody(BaseModel): + """A new SCIM token for an organization's identity provider.""" + + label: str = Field(min_length=1, max_length=100) + + model_config = {"extra": "forbid", "populate_by_name": True} + + +def scim_token_json(row: ScimToken) -> dict: + return { + "id": row.id, + "label": row.label, + "createdAt": iso_ts(row.created_at), + "lastUsedAt": iso_ts(row.last_used_at) if row.last_used_at is not None else None, + "revokedAt": iso_ts(row.revoked_at) if row.revoked_at is not None else None, + } + + def build_enterprise_admin_router() -> APIRouter: """Build the per-organization enterprise sign-in administration routes.""" router = APIRouter() @@ -410,4 +431,70 @@ def delete_identity_provider( session.commit() return Response(status_code=204, headers={"Cache-Control": "private, no-store"}) + @router.post("/api/organizations/{organization_id}/scim-tokens", status_code=201) + def create_scim_token( + organization_id: str, + body: ScimTokenBody, + request: Request, + response: Response, + principal: AuthPrincipal = Depends(require_scope("write:projects")), + session: Session = Depends(get_session), + ): + require_organization_admin(session, organization_id, principal, request, mutation=True) + response.headers["Cache-Control"] = "private, no-store" + token = secrets.token_urlsafe(32) + row = ScimToken( + id=str(uuid.uuid4()), + organization_id=organization_id, + created_by_id=principal.account.id, + digest=token_digest(token), + label=body.label, + created_at=get_clock(request)(), + ) + session.add(row) + session.commit() + return { + "token": token, + "scimToken": scim_token_json(row), + "baseUrl": f"{request.app.state.base_url}/scim/v2/{organization_id}", + } + + @router.get("/api/organizations/{organization_id}/scim-tokens") + def list_scim_tokens( + organization_id: str, + request: Request, + response: Response, + principal: AuthPrincipal = Depends(require_scope("read:projects")), + session: Session = Depends(get_session), + ): + require_organization_admin(session, organization_id, principal, request, mutation=False) + response.headers["Cache-Control"] = "private, no-store" + rows = session.scalars( + select(ScimToken) + .where(ScimToken.organization_id == organization_id) + .order_by(ScimToken.created_at.desc(), ScimToken.id) + ) + return {"scimTokens": [scim_token_json(row) for row in rows]} + + @router.delete("/api/organizations/{organization_id}/scim-tokens/{token_id}", status_code=204) + def revoke_scim_token( + organization_id: str, + token_id: str, + request: Request, + principal: AuthPrincipal = Depends(require_scope("write:projects")), + session: Session = Depends(get_session), + ): + require_organization_admin(session, organization_id, principal, request, mutation=True) + row = session.get(ScimToken, token_id) + if row is None or row.organization_id != organization_id: + raise HTTPException(404, "SCIM token not found") + # Revoking again keeps the first revocation time. + session.execute( + update(ScimToken) + .where(ScimToken.id == token_id, ScimToken.revoked_at.is_(None)) + .values(revoked_at=get_clock(request)()) + ) + session.commit() + return Response(status_code=204, headers={"Cache-Control": "private, no-store"}) + return router diff --git a/backend/geolibre_server_api/geolibre_server_api/enterprise_models.py b/backend/geolibre_server_api/geolibre_server_api/enterprise_models.py index 0fa5a1056c..41735f3360 100644 --- a/backend/geolibre_server_api/geolibre_server_api/enterprise_models.py +++ b/backend/geolibre_server_api/geolibre_server_api/enterprise_models.py @@ -127,3 +127,60 @@ class OidcLoginState(Base): max_age: Mapped[int | None] = mapped_column(Integer, nullable=True) expires_at: Mapped[int] = mapped_column(Integer, index=True) consumed_at: Mapped[int | None] = mapped_column(Integer, nullable=True) + + +class ScimToken(Base): + """A bearer token an identity provider uses to provision one organization over SCIM.""" + + __tablename__ = "scim_tokens" + + id: Mapped[str] = mapped_column(String(36), primary_key=True) + organization_id: Mapped[str] = mapped_column( + String(36), ForeignKey("organizations.id", ondelete="CASCADE"), index=True + ) + created_by_id: Mapped[str] = mapped_column( + String(36), ForeignKey("accounts.id", ondelete="CASCADE") + ) + digest: Mapped[str] = mapped_column(String(64), unique=True) + label: Mapped[str] = mapped_column(String(100)) + created_at: Mapped[int] = mapped_column(Integer) + last_used_at: Mapped[int | None] = mapped_column(Integer, nullable=True) + revoked_at: Mapped[int | None] = mapped_column(Integer, nullable=True) + + +class ScimUser(Base): + """An account provisioned into (or linked to) an organization over SCIM.""" + + __tablename__ = "scim_users" + __table_args__ = (UniqueConstraint("organization_id", "user_name", name="uq_scim_user_name"),) + + organization_id: Mapped[str] = mapped_column( + String(36), ForeignKey("organizations.id", ondelete="CASCADE"), primary_key=True + ) + account_id: Mapped[str] = mapped_column( + String(36), ForeignKey("accounts.id", ondelete="CASCADE"), primary_key=True + ) + # Stored lowercased; SCIM userName comparisons are case-insensitive. + user_name: Mapped[str] = mapped_column(String(255)) + external_id: Mapped[str | None] = mapped_column(String(255), nullable=True) + # The SCIM representation only; never copied to Account.email. + email: Mapped[str | None] = mapped_column(String(320), nullable=True) + display_name: Mapped[str | None] = mapped_column(String(255), nullable=True) + created_at: Mapped[int] = mapped_column(Integer) + updated_at: Mapped[int] = mapped_column(Integer) + + +class ScimGroup(Base): + """Marks a group as provisioned over SCIM by its organization's identity provider.""" + + __tablename__ = "scim_groups" + + group_id: Mapped[str] = mapped_column( + String(36), ForeignKey("groups.id", ondelete="CASCADE"), primary_key=True + ) + organization_id: Mapped[str] = mapped_column( + String(36), ForeignKey("organizations.id", ondelete="CASCADE"), index=True + ) + external_id: Mapped[str | None] = mapped_column(String(255), nullable=True) + created_at: Mapped[int] = mapped_column(Integer) + updated_at: Mapped[int] = mapped_column(Integer) diff --git a/backend/geolibre_server_api/geolibre_server_api/main.py b/backend/geolibre_server_api/geolibre_server_api/main.py index 71f4e98869..d5271462cf 100644 --- a/backend/geolibre_server_api/geolibre_server_api/main.py +++ b/backend/geolibre_server_api/geolibre_server_api/main.py @@ -79,6 +79,7 @@ ) from geolibre_server_api.projects import demote_disallowed_public_projects, log_project_activity from geolibre_server_api.proxy_identity import load_trusted_proxy_config +from geolibre_server_api.scim import SCIM_MEDIA_TYPE, ScimError, build_scim_router Visibility = Literal["public", "unlisted", "private", "organization"] PublicSharingPolicy = Literal["yes", "publishers", "no"] @@ -727,6 +728,7 @@ async def sweep() -> None: app.state.session_factory = sessions app.state.clock = clock_fn app.state.oauth_config = oauth_config + app.state.base_url = base_url app.state.trusted_proxy = load_trusted_proxy_config() # Outbound identity-provider HTTP; lives for the process. idp_transport = oidc_transport or build_transport() @@ -821,6 +823,21 @@ async def insufficient_scope_handler(_request: Request, exc: InsufficientScopeEr async def validation_error(_request: Request, exc: RequestValidationError): return JSONResponse({"error": str(exc.errors()[0]["msg"])}, status_code=422) + @app.exception_handler(ScimError) + async def scim_error(_request: Request, exc: ScimError): + """Serialize a SCIM failure as an RFC 7644 error message.""" + return JSONResponse( + { + "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"], + "status": str(exc.status), + "detail": exc.detail, + **({"scimType": exc.scim_type} if exc.scim_type else {}), + }, + status_code=exc.status, + media_type=SCIM_MEDIA_TYPE, + headers=exc.headers, + ) + @app.exception_handler(Exception) async def unexpected_error(_request: Request, exc: Exception): # Only HTTPException and RequestValidationError were handled, so anything @@ -833,6 +850,7 @@ async def unexpected_error(_request: Request, exc: Exception): # Identity routes must precede the username/slug catch-alls below. app.include_router(build_identity_router()) app.include_router(build_enterprise_admin_router()) + app.include_router(build_scim_router()) if oauth_config is not None: app.include_router(build_oauth_router(oauth_config)) diff --git a/backend/geolibre_server_api/geolibre_server_api/oidc.py b/backend/geolibre_server_api/geolibre_server_api/oidc.py index 0348282bfa..cf08c6e716 100644 --- a/backend/geolibre_server_api/geolibre_server_api/oidc.py +++ b/backend/geolibre_server_api/geolibre_server_api/oidc.py @@ -38,6 +38,7 @@ AccountSecurity, FederatedIdentity, OrganizationIdentityProvider, + ScimUser, ) from geolibre_server_api.org_models import ( ROLE_RANK, @@ -485,6 +486,47 @@ def _first_string(*values: object) -> str: return "" +def _link_scim_user( + session: Session, provider: OrganizationIdentityProvider, claims: dict, now_ts: int +) -> None: + """Link a subject's first sign-in to the account SCIM provisioned for that user name. + + The username claim is tried first, then the email claim; only string claims count. + """ + if _find_identity(session, provider.id, claims["sub"]) is not None: + return + for claim_name in (provider.username_claim, provider.email_claim): + value = claims.get(claim_name) + if not isinstance(value, str) or not value: + continue + account_id = session.scalar( + select(ScimUser.account_id).where( + ScimUser.organization_id == provider.organization_id, + ScimUser.user_name == value.lower(), + ) + ) + if account_id is None: + continue + try: + with session.begin_nested(): + session.add( + FederatedIdentity( + id=str(uuid.uuid4()), + account_id=account_id, + provider_key=provider.id, + provider_id=provider.id, + subject=claims["sub"], + created_at=now_ts, + last_login_at=now_ts, + ) + ) + session.flush() + except IntegrityError: + # A concurrent first sign-in linked the subject; the caller re-reads it. + pass + return + + def resolve_oidc_account( session: Session, provider: OrganizationIdentityProvider, @@ -493,9 +535,12 @@ def resolve_oidc_account( ) -> Account: """Find or JIT-create the account for validated claims and apply the org mapping. - Never links to an existing account by email: that would let any IdP that - asserts an address take over a local account. + A first sign-in links to the account the provider's own organization + provisioned over SCIM under the same user name. It never links to any other + existing account by email: that would let any IdP that asserts an address + take over a local account. """ + _link_scim_user(session, provider, claims, now_ts) account = _resolve_identity( session, provider_key=provider.id, diff --git a/backend/geolibre_server_api/geolibre_server_api/policy.py b/backend/geolibre_server_api/geolibre_server_api/policy.py index bd8eeeb6fc..44f898e187 100644 --- a/backend/geolibre_server_api/geolibre_server_api/policy.py +++ b/backend/geolibre_server_api/geolibre_server_api/policy.py @@ -12,6 +12,7 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session +from geolibre_server_api.auth_models import OAuthSession, PersonalTokenPolicy, Token from geolibre_server_api.enterprise_models import ( AccountSecurity, OrganizationIdentityProvider, @@ -216,3 +217,47 @@ def require_not_break_glass(session: Session, organization_id: str, account_id: ) if break_glass is not None: raise HTTPException(422, "account is the organization's break-glass administrator") + + +def is_deactivated(session: Session, account_id: str) -> bool: + """True when SCIM deactivated the account.""" + status = session.scalar( + select(AccountSecurity.status).where(AccountSecurity.account_id == account_id) + ) + return status == "deactivated" + + +def deactivate_account(session: Session, account_id: str, now_ts: int) -> None: + """Deactivate the account and revoke every OAuth family and personal token. + + The caller commits, so the status flip and the revocations land together. + """ + from geolibre_server_api.auth import backfill_account_policies + + security = ensure_account_security(session, account_id) + security.status = "deactivated" + security.deactivated_at = now_ts + session.execute( + update(OAuthSession) + .where(OAuthSession.account_id == account_id, OAuthSession.revoked_at.is_(None)) + .values(revoked_at=now_ts) + ) + # Legacy tokens get a policy row first so the revocation below covers them. + backfill_account_policies(session, account_id) + session.execute( + update(PersonalTokenPolicy) + .where( + PersonalTokenPolicy.token_digest.in_( + select(Token.digest).where(Token.account_id == account_id) + ), + PersonalTokenPolicy.revoked_at.is_(None), + ) + .values(revoked_at=now_ts) + ) + + +def reactivate_account(session: Session, account_id: str) -> None: + """Allow sign-in again; credentials revoked by the deactivation stay revoked.""" + security = ensure_account_security(session, account_id) + security.status = "active" + security.deactivated_at = None diff --git a/backend/geolibre_server_api/geolibre_server_api/scim.py b/backend/geolibre_server_api/geolibre_server_api/scim.py new file mode 100644 index 0000000000..5658cefd46 --- /dev/null +++ b/backend/geolibre_server_api/geolibre_server_api/scim.py @@ -0,0 +1,1083 @@ +"""SCIM 2.0 (RFC 7643/7644) provisioning of one organization's users and groups. + +An identity provider authenticates with a SCIM token an organization +administrator minted (``enterprise_admin``). Bodies are parsed by hand so every +failure is an RFC 7644 error message, and every response is +``application/scim+json``. + +Deactivation is the load-bearing rule. An account the organization manages +(provisioned by its SCIM or created by its single sign-on) is deactivated +outright, which revokes every credential in the same transaction. Any other +account only loses its membership in this organization and its groups, so one +organization can never lock an account out of another. +""" + +from __future__ import annotations + +import json +import re +import uuid + +from fastapi import APIRouter, Depends, Request, Response +from fastapi.responses import JSONResponse +from sqlalchemy import and_, delete, func, select, update +from sqlalchemy.exc import IntegrityError +from sqlalchemy.orm import Session + +from geolibre_server_api import auth +from geolibre_server_api.auth import get_clock, get_session, iso_ts, token_digest +from geolibre_server_api.auth_models import Account +from geolibre_server_api.enterprise_models import ( + AccountSecurity, + OrganizationIdentityProvider, + ScimGroup, + ScimToken, + ScimUser, +) +from geolibre_server_api.oidc import SSO_PASSWORD_HASH, derive_username +from geolibre_server_api.org_models import Group, GroupMember, Organization, OrganizationMember +from geolibre_server_api.policy import deactivate_account, reactivate_account +from geolibre_server_api.projects import demote_disallowed_public_projects + +SCIM_MEDIA_TYPE = "application/scim+json" +USER_SCHEMA = "urn:ietf:params:scim:schemas:core:2.0:User" +GROUP_SCHEMA = "urn:ietf:params:scim:schemas:core:2.0:Group" +LIST_SCHEMA = "urn:ietf:params:scim:api:messages:2.0:ListResponse" +PATCH_SCHEMA = "urn:ietf:params:scim:api:messages:2.0:PatchOp" +SERVICE_PROVIDER_SCHEMA = "urn:ietf:params:scim:schemas:core:2.0:ServiceProviderConfig" +RESOURCE_TYPE_SCHEMA = "urn:ietf:params:scim:schemas:core:2.0:ResourceType" +SCHEMA_SCHEMA = "urn:ietf:params:scim:schemas:core:2.0:Schema" +MAX_RESULTS = 100 +# Token last-use is recorded at most once a minute, like OAuth sessions. +TOUCH_SECONDS = 60 + +_QUOTED = r'"((?:[^"\\]|\\.)*)"' +USER_FILTER_RE = re.compile(rf"^(userName|externalId) eq {_QUOTED}$") +GROUP_FILTER_RE = re.compile(rf"^(displayName|externalId) eq {_QUOTED}$") +MEMBER_PATH_RE = re.compile(rf"^members\[value eq {_QUOTED}\]$", re.IGNORECASE) +# Attribute names are case-insensitive (RFC 7643 section 2.1). Entra ID +# addresses the work email with a value-filter path. +USER_ATTRIBUTES = { + "username": "userName", + "externalid": "externalId", + "displayname": "displayName", + "emails": "emails", + 'emails[type eq "work"].value': "workEmail", + "active": "active", +} +GROUP_ATTRIBUTES = {"displayname": "displayName", "externalid": "externalId", "members": "members"} + + +class ScimError(Exception): + """A SCIM failure, serialized by ``create_app`` as an RFC 7644 error message.""" + + def __init__( + self, + status: int, + detail: str, + scim_type: str | None = None, + headers: dict[str, str] | None = None, + ): + super().__init__(detail) + self.status = status + self.detail = detail + self.scim_type = scim_type + self.headers = headers + + +def _unsupported_patch() -> ScimError: + return ScimError(400, "unsupported patch operation", "invalidSyntax") + + +def scim_response( + content: dict, status_code: int = 200, headers: dict[str, str] | None = None +) -> JSONResponse: + return JSONResponse( + content, status_code=status_code, media_type=SCIM_MEDIA_TYPE, headers=headers + ) + + +def scim_organization( + organization_id: str, + request: Request, + session: Session = Depends(get_session), +) -> ScimToken: + """Authenticate the identity provider's SCIM token for this organization.""" + header = request.headers.get("authorization", "") + token = header[7:] if header.startswith("Bearer ") else "" + row = None + if token: + row = session.scalar( + select(ScimToken).where( + ScimToken.digest == token_digest(token), + ScimToken.organization_id == organization_id, + ScimToken.revoked_at.is_(None), + ) + ) + if row is None: + raise ScimError(401, "invalid SCIM token", None, {"WWW-Authenticate": "Bearer"}) + now_ts = get_clock(request)() + if row.last_used_at is None or row.last_used_at < now_ts - TOUCH_SECONDS: + session.execute(update(ScimToken).where(ScimToken.id == row.id).values(last_used_at=now_ts)) + session.commit() + return row + + +async def scim_body(request: Request) -> dict: + """The request's JSON object (``application/scim+json`` or ``application/json``).""" + media_type = request.headers.get("content-type", "").split(";", 1)[0].strip().lower() + if media_type not in (SCIM_MEDIA_TYPE, "application/json"): + raise ScimError(415, "content type must be application/scim+json") + try: + body = json.loads(await request.body()) + except (ValueError, UnicodeDecodeError): + raise ScimError(400, "invalid JSON body", "invalidSyntax") from None + if not isinstance(body, dict): + raise ScimError(400, "invalid JSON body", "invalidSyntax") + return body + + +def _unescape(value: str) -> str: + return re.sub(r"\\(.)", r"\1", value) + + +def _pagination(request: Request) -> tuple[int, int]: + """``startIndex`` and ``count``, clamped to 1.. and 0..100 (RFC 7644 section 3.4.2.4).""" + + def read(name: str, default: int) -> int: + raw = request.query_params.get(name) + if raw is None: + return default + try: + return int(raw) + except ValueError: + raise ScimError(400, f"{name} must be an integer", "invalidValue") from None + + start = max(1, read("startIndex", 1)) + count = min(max(0, read("count", MAX_RESULTS)), MAX_RESULTS) + return start, count + + +def _list_response(resources: list[dict], total: int, start: int) -> dict: + return { + "schemas": [LIST_SCHEMA], + "totalResults": total, + "startIndex": start, + "itemsPerPage": len(resources), + "Resources": resources, + } + + +def _location(request: Request, organization_id: str, path: str) -> str: + return f"{request.app.state.base_url}/scim/v2/{organization_id}/{path}" + + +# --------------------------------------------------------------------------- +# Attribute parsing +# --------------------------------------------------------------------------- + + +def _optional_string(name: str, value: object, max_length: int) -> str | None: + if value is None: + return None + if not isinstance(value, str) or len(value) > max_length: + raise ScimError( + 400, f"{name} must be a string of at most {max_length} characters", "invalidValue" + ) + return value or None + + +def _user_name(value: object) -> str: + if not isinstance(value, str) or not value.strip() or len(value) > 255: + raise ScimError(400, "userName must be a string of 1 to 255 characters", "invalidValue") + return value.lower() + + +def _active(value: object) -> bool: + # Entra ID sends "True"/"False" strings in PATCH operations. + if isinstance(value, bool): + return value + if isinstance(value, str) and value.lower() in ("true", "false"): + return value.lower() == "true" + raise ScimError(400, "active must be a boolean", "invalidValue") + + +def _primary_email(value: object) -> str | None: + if value is None: + return None + if not isinstance(value, list) or not all(isinstance(item, dict) for item in value): + raise ScimError(400, "emails must be a list of objects", "invalidValue") + if not value: + return None + primary = next( + (item for item in value if item.get("primary") is True or item.get("primary") == "true"), + value[0], + ) + return _optional_string("emails.value", primary.get("value"), 320) + + +def _user_change(attribute: str, value: object) -> dict: + """One user attribute as a column update.""" + if attribute == "userName": + return {"user_name": _user_name(value)} + if attribute == "externalId": + return {"external_id": _optional_string("externalId", value, 255)} + if attribute == "displayName": + return {"display_name": _optional_string("displayName", value, 255)} + if attribute == "emails": + return {"email": _primary_email(value)} + if attribute == "workEmail": + return {"email": _optional_string("emails.value", value, 320)} + return {"active": _active(value)} + + +def _user_changes(values: dict) -> dict: + """Column updates for the supported attributes of a user object; others are ignored.""" + changes: dict = {} + for key, value in values.items(): + attribute = USER_ATTRIBUTES.get(key.lower()) if isinstance(key, str) else None + if attribute is not None: + changes.update(_user_change(attribute, value)) + return changes + + +def _patch_operations(body: dict) -> list[tuple[str, str | None, object]]: + """``(op, path, value)`` triples of a PatchOp request; ``op`` lowercased.""" + schemas = body.get("schemas") + operations = body.get("Operations") + if not isinstance(schemas, list) or PATCH_SCHEMA not in schemas: + raise ScimError(400, "PatchOp schema required", "invalidSyntax") + if not isinstance(operations, list): + raise ScimError(400, "Operations must be a list", "invalidSyntax") + parsed = [] + for operation in operations: + if not isinstance(operation, dict) or not isinstance(operation.get("op"), str): + raise _unsupported_patch() + path = operation.get("path") + if path is not None and not isinstance(path, str): + raise _unsupported_patch() + parsed.append( + (operation["op"].lower(), path.strip() if path else None, operation.get("value")) + ) + return parsed + + +def _member_ids(value: object) -> list[str]: + if not isinstance(value, list) or not all( + isinstance(item, dict) and isinstance(item.get("value"), str) for item in value + ): + raise ScimError(400, "members must be a list of objects with a value", "invalidValue") + return list(dict.fromkeys(item["value"] for item in value)) + + +def _group_changes(values: dict) -> dict: + changes: dict = {} + for key, value in values.items(): + attribute = GROUP_ATTRIBUTES.get(key.lower()) if isinstance(key, str) else None + if attribute == "displayName": + if not isinstance(value, str) or not value.strip(): + raise ScimError(400, "displayName is required", "invalidValue") + changes["name"] = value.strip()[:100] + elif attribute == "externalId": + changes["external_id"] = _optional_string("externalId", value, 255) + elif attribute == "members": + changes["members"] = _member_ids(value) + return changes + + +# --------------------------------------------------------------------------- +# Membership and deactivation +# --------------------------------------------------------------------------- + + +def _default_role(session: Session, organization_id: str) -> str: + role = session.scalar( + select(OrganizationIdentityProvider.default_role).where( + OrganizationIdentityProvider.organization_id == organization_id + ) + ) + return role or "member" + + +def _is_last_admin(session: Session, organization_id: str, account_id: str) -> bool: + member = session.get(OrganizationMember, (organization_id, account_id)) + if member is None or member.role != "administrator": + return False + admin_count = session.scalar( + select(func.count()) + .select_from(OrganizationMember) + .where( + OrganizationMember.organization_id == organization_id, + OrganizationMember.role == "administrator", + ) + ) + return admin_count == 1 + + +def _guard_removal(session: Session, organization_id: str, account_id: str) -> None: + if _is_last_admin(session, organization_id, account_id): + raise ScimError(409, "cannot remove the last organization administrator", "mutability") + break_glass = session.scalar( + select(OrganizationIdentityProvider.id).where( + OrganizationIdentityProvider.organization_id == organization_id, + OrganizationIdentityProvider.break_glass_account_id == account_id, + ) + ) + if break_glass is not None: + raise ScimError( + 409, "cannot remove the organization's break-glass administrator", "mutability" + ) + + +def _remove_memberships(session: Session, organization_id: str, account_id: str) -> None: + """Drop the account's membership in the organization and its groups (not ownership).""" + _guard_removal(session, organization_id, account_id) + session.execute( + delete(OrganizationMember).where( + OrganizationMember.organization_id == organization_id, + OrganizationMember.account_id == account_id, + ) + ) + # A group's owner row is kept: removing it would leave the group ownerless. + session.execute( + delete(GroupMember).where( + GroupMember.account_id == account_id, + GroupMember.role != "owner", + GroupMember.group_id.in_( + select(Group.id).where(Group.organization_id == organization_id) + ), + ) + ) + # Same as leaving through the members API: a non-member may not keep projects public. + organization = session.get(Organization, organization_id) + if organization is not None: + demote_disallowed_public_projects(session, organization, account_id) + + +def _is_managed(session: Session, organization_id: str, account_id: str) -> bool: + managed_by = session.scalar( + select(AccountSecurity.managed_by_organization_id).where( + AccountSecurity.account_id == account_id + ) + ) + return managed_by == organization_id + + +def _deactivate(session: Session, organization_id: str, account_id: str, now_ts: int) -> None: + if _is_managed(session, organization_id, account_id): + deactivate_account(session, account_id, now_ts) + else: + # Authorization reads memberships live, so access ends on the next request. + _remove_memberships(session, organization_id, account_id) + + +def _activate(session: Session, organization_id: str, account_id: str) -> None: + if _is_managed(session, organization_id, account_id): + reactivate_account(session, account_id) + if session.get(OrganizationMember, (organization_id, account_id)) is None: + session.add( + OrganizationMember( + organization_id=organization_id, + account_id=account_id, + role=_default_role(session, organization_id), + created_at=auth.now(), + ) + ) + + +# --------------------------------------------------------------------------- +# Resources +# --------------------------------------------------------------------------- + + +def _user_query(organization_id: str): + """ScimUser rows with the account's status and whether it is still a member.""" + return ( + select(ScimUser, AccountSecurity.status, OrganizationMember.account_id) + .outerjoin(AccountSecurity, AccountSecurity.account_id == ScimUser.account_id) + .outerjoin( + OrganizationMember, + and_( + OrganizationMember.organization_id == ScimUser.organization_id, + OrganizationMember.account_id == ScimUser.account_id, + ), + ) + .where(ScimUser.organization_id == organization_id) + ) + + +def _user_json(request: Request, row) -> dict: + scim_user, status, member_id = row + location = _location(request, scim_user.organization_id, f"Users/{scim_user.account_id}") + resource: dict = { + "schemas": [USER_SCHEMA], + "id": scim_user.account_id, + "userName": scim_user.user_name, + # A non-managed account is deactivated here by losing its membership. + "active": status != "deactivated" and member_id is not None, + "meta": { + "resourceType": "User", + "created": iso_ts(scim_user.created_at), + "lastModified": iso_ts(scim_user.updated_at), + "location": location, + }, + } + if scim_user.external_id is not None: + resource["externalId"] = scim_user.external_id + if scim_user.display_name is not None: + resource["displayName"] = scim_user.display_name + if scim_user.email is not None: + resource["emails"] = [{"value": scim_user.email, "primary": True}] + return resource + + +def _load_user(session: Session, organization_id: str, account_id: str): + row = session.execute( + _user_query(organization_id).where(ScimUser.account_id == account_id) + ).one_or_none() + if row is None: + raise ScimError(404, "resource not found") + return row + + +def _user_name_taken( + session: Session, organization_id: str, user_name: str, account_id: str | None = None +) -> bool: + query = select(ScimUser.account_id).where( + ScimUser.organization_id == organization_id, ScimUser.user_name == user_name + ) + if account_id is not None: + query = query.where(ScimUser.account_id != account_id) + return session.scalar(query) is not None + + +def _duplicate_user_name() -> ScimError: + return ScimError(409, "userName already exists", "uniqueness") + + +def _apply_user_changes(session: Session, scim_user: ScimUser, changes: dict, now_ts: int) -> None: + organization_id = scim_user.organization_id + user_name = changes.get("user_name") + if user_name is not None and user_name != scim_user.user_name: + if _user_name_taken(session, organization_id, user_name, scim_user.account_id): + raise _duplicate_user_name() + scim_user.user_name = user_name + for column in ("external_id", "display_name", "email"): + if column in changes: + setattr(scim_user, column, changes[column]) + if "active" in changes: + if changes["active"]: + _activate(session, organization_id, scim_user.account_id) + else: + _deactivate(session, organization_id, scim_user.account_id, now_ts) + scim_user.updated_at = now_ts + + +def _commit_user(session: Session) -> None: + try: + session.commit() + except IntegrityError: + session.rollback() + raise _duplicate_user_name() from None + + +def _group_member_rows(session: Session, group_id: str) -> list[tuple[str, str | None]]: + return list( + session.execute( + select(GroupMember.account_id, Account.username) + .join(Account, Account.id == GroupMember.account_id) + .where( + GroupMember.group_id == group_id, + GroupMember.role == "member", + GroupMember.status == "accepted", + ) + .order_by(GroupMember.created_at, GroupMember.account_id) + ).all() + ) + + +def _group_json(request: Request, session: Session, group: Group, scim_group: ScimGroup) -> dict: + members = [] + for account_id, username in _group_member_rows(session, group.id): + member = {"value": account_id} + if username is not None: + member["display"] = username + members.append(member) + resource: dict = { + "schemas": [GROUP_SCHEMA], + "id": group.id, + "displayName": group.name, + "members": members, + "meta": { + "resourceType": "Group", + "created": iso_ts(scim_group.created_at), + "lastModified": iso_ts(scim_group.updated_at), + "location": _location(request, scim_group.organization_id, f"Groups/{group.id}"), + }, + } + if scim_group.external_id is not None: + resource["externalId"] = scim_group.external_id + return resource + + +def _load_group(session: Session, organization_id: str, group_id: str) -> tuple[Group, ScimGroup]: + row = session.execute( + select(Group, ScimGroup) + .join(ScimGroup, ScimGroup.group_id == Group.id) + .where( + Group.id == group_id, + Group.organization_id == organization_id, + ScimGroup.organization_id == organization_id, + ) + ).one_or_none() + if row is None: + raise ScimError(404, "resource not found") + return row[0], row[1] + + +def _require_provisioned(session: Session, organization_id: str, account_ids: list[str]) -> None: + if not account_ids: + return + found = set( + session.scalars( + select(ScimUser.account_id).where( + ScimUser.organization_id == organization_id, + ScimUser.account_id.in_(account_ids), + ) + ) + ) + if found != set(account_ids): + raise ScimError(400, "member is not provisioned in this organization", "invalidValue") + + +def _add_members(session: Session, group_id: str, account_ids: list[str]) -> None: + """Add plain members; owner and manager rows are never modified.""" + for account_id in account_ids: + row = session.get(GroupMember, (group_id, account_id)) + if row is None: + session.add( + GroupMember( + group_id=group_id, + account_id=account_id, + role="member", + status="accepted", + created_at=auth.now(), + ) + ) + elif row.role == "member" and row.status != "accepted": + row.status = "accepted" + session.flush() + + +def _remove_members(session: Session, group_id: str, account_ids: list[str] | None) -> None: + """Remove plain members (all of them when ``account_ids`` is None).""" + statement = delete(GroupMember).where( + GroupMember.group_id == group_id, GroupMember.role == "member" + ) + if account_ids is not None: + statement = statement.where(GroupMember.account_id.in_(account_ids)) + session.execute(statement) + + +def _replace_members(session: Session, group_id: str, account_ids: list[str]) -> None: + session.execute( + delete(GroupMember).where( + GroupMember.group_id == group_id, + GroupMember.role == "member", + GroupMember.account_id.not_in(account_ids), + ) + ) + _add_members(session, group_id, account_ids) + + +# --------------------------------------------------------------------------- +# Discovery documents +# --------------------------------------------------------------------------- + + +def _attribute( + name: str, + type_: str = "string", + *, + multi_valued: bool = False, + required: bool = False, + mutability: str = "readWrite", + uniqueness: str = "none", + sub_attributes: list[dict] | None = None, +) -> dict: + attribute = { + "name": name, + "type": type_, + "multiValued": multi_valued, + "required": required, + "caseExact": False, + "mutability": mutability, + "returned": "default", + "uniqueness": uniqueness, + } + if sub_attributes is not None: + attribute["subAttributes"] = sub_attributes + return attribute + + +SCHEMA_DEFINITIONS = [ + { + "id": USER_SCHEMA, + "name": "User", + "description": "User account", + "attributes": [ + _attribute("userName", required=True, uniqueness="server"), + _attribute("displayName"), + _attribute("active", "boolean"), + _attribute( + "emails", + "complex", + multi_valued=True, + sub_attributes=[ + _attribute("value"), + _attribute("type"), + _attribute("primary", "boolean"), + ], + ), + ], + }, + { + "id": GROUP_SCHEMA, + "name": "Group", + "description": "Group", + "attributes": [ + _attribute("displayName", required=True), + _attribute( + "members", + "complex", + multi_valued=True, + sub_attributes=[ + _attribute("value", mutability="immutable"), + _attribute("display", mutability="readOnly"), + ], + ), + ], + }, +] + + +# --------------------------------------------------------------------------- +# Router +# --------------------------------------------------------------------------- + + +def build_scim_router() -> APIRouter: + """Build the SCIM 2.0 service provider for ``/scim/v2/{organization_id}``.""" + router = APIRouter( + prefix="/scim/v2/{organization_id}", dependencies=[Depends(scim_organization)] + ) + + # -- discovery -- + + @router.get("/ServiceProviderConfig") + def service_provider_config(organization_id: str, request: Request): + return scim_response( + { + "schemas": [SERVICE_PROVIDER_SCHEMA], + "patch": {"supported": True}, + "bulk": {"supported": False, "maxOperations": 0, "maxPayloadSize": 0}, + "filter": {"supported": True, "maxResults": MAX_RESULTS}, + "changePassword": {"supported": False}, + "sort": {"supported": False}, + "etag": {"supported": False}, + "authenticationSchemes": [ + { + "type": "oauthbearertoken", + "name": "OAuth Bearer Token", + "description": "SCIM token issued by an organization administrator", + } + ], + "meta": { + "resourceType": "ServiceProviderConfig", + "location": _location(request, organization_id, "ServiceProviderConfig"), + }, + } + ) + + @router.get("/ResourceTypes") + def resource_types(organization_id: str, request: Request): + resources = [ + { + "schemas": [RESOURCE_TYPE_SCHEMA], + "id": name, + "name": name, + "endpoint": f"/{name}s", + "schema": schema, + "meta": { + "resourceType": "ResourceType", + "location": _location(request, organization_id, f"ResourceTypes/{name}"), + }, + } + for name, schema in (("User", USER_SCHEMA), ("Group", GROUP_SCHEMA)) + ] + return scim_response(_list_response(resources, len(resources), 1)) + + @router.get("/Schemas") + def schemas(organization_id: str, request: Request): + resources = [ + { + "schemas": [SCHEMA_SCHEMA], + **definition, + "meta": { + "resourceType": "Schema", + "location": _location(request, organization_id, f"Schemas/{definition['id']}"), + }, + } + for definition in SCHEMA_DEFINITIONS + ] + return scim_response(_list_response(resources, len(resources), 1)) + + # -- users -- + + @router.get("/Users") + def list_users(organization_id: str, request: Request, session: Session = Depends(get_session)): + start, count = _pagination(request) + query = _user_query(organization_id) + raw_filter = request.query_params.get("filter") + if raw_filter is not None: + match = USER_FILTER_RE.fullmatch(raw_filter.strip()) + if match is None: + raise ScimError(400, "unsupported filter", "invalidFilter") + attribute, value = match.group(1), _unescape(match.group(2)) + if attribute == "userName": + query = query.where(ScimUser.user_name == value.lower()) + else: + query = query.where(ScimUser.external_id == value) + total = session.scalar(select(func.count()).select_from(query.subquery())) + rows = session.execute( + query.order_by(ScimUser.created_at, ScimUser.account_id).offset(start - 1).limit(count) + ).all() + return scim_response( + _list_response([_user_json(request, row) for row in rows], total or 0, start) + ) + + @router.post("/Users") + def create_user( + organization_id: str, + request: Request, + body: dict = Depends(scim_body), + session: Session = Depends(get_session), + ): + changes = { + "external_id": None, + "display_name": None, + "email": None, + "active": True, + **_user_changes(body), + } + if "user_name" not in changes: + raise ScimError(400, "userName is required", "invalidValue") + user_name = changes["user_name"] + now_ts = get_clock(request)() + # A concurrent request may win the userName (409) or the derived + # account username (retried with a fresh one). + for _ in range(3): + if _user_name_taken(session, organization_id, user_name): + raise _duplicate_user_name() + account = Account( + id=str(uuid.uuid4()), + username=derive_username(session, user_name), + password_hash=SSO_PASSWORD_HASH, + created_at=auth.now(), + ) + active = changes["active"] + try: + session.add(account) + session.flush() + session.add( + AccountSecurity( + account_id=account.id, + status="active" if active else "deactivated", + deactivated_at=None if active else now_ts, + failed_login_count=0, + managed_by_organization_id=organization_id, + ) + ) + session.add( + ScimUser( + organization_id=organization_id, + account_id=account.id, + user_name=user_name, + external_id=changes["external_id"], + email=changes["email"], + display_name=changes["display_name"], + created_at=now_ts, + updated_at=now_ts, + ) + ) + session.add( + OrganizationMember( + organization_id=organization_id, + account_id=account.id, + role=_default_role(session, organization_id), + created_at=auth.now(), + ) + ) + session.commit() + except IntegrityError: + session.rollback() + continue + resource = _user_json(request, _load_user(session, organization_id, account.id)) + return scim_response(resource, 201, headers={"Location": resource["meta"]["location"]}) + if _user_name_taken(session, organization_id, user_name): + raise _duplicate_user_name() + raise ScimError(409, "could not allocate an account for this user", "uniqueness") + + @router.get("/Users/{user_id}") + def get_user( + organization_id: str, + user_id: str, + request: Request, + session: Session = Depends(get_session), + ): + return scim_response(_user_json(request, _load_user(session, organization_id, user_id))) + + @router.put("/Users/{user_id}") + def replace_user( + organization_id: str, + user_id: str, + request: Request, + body: dict = Depends(scim_body), + session: Session = Depends(get_session), + ): + scim_user = _load_user(session, organization_id, user_id)[0] + changes = {"external_id": None, "display_name": None, "email": None, **_user_changes(body)} + if "user_name" not in changes: + raise ScimError(400, "userName is required", "invalidValue") + _apply_user_changes(session, scim_user, changes, get_clock(request)()) + _commit_user(session) + return scim_response(_user_json(request, _load_user(session, organization_id, user_id))) + + @router.patch("/Users/{user_id}") + def patch_user( + organization_id: str, + user_id: str, + request: Request, + body: dict = Depends(scim_body), + session: Session = Depends(get_session), + ): + scim_user = _load_user(session, organization_id, user_id)[0] + changes: dict = {} + for op, path, value in _patch_operations(body): + if op in ("add", "replace"): + if path is None: + if not isinstance(value, dict): + raise ScimError(400, "patch value must be an object", "invalidValue") + changes.update(_user_changes(value)) + continue + attribute = USER_ATTRIBUTES.get(path.lower()) + # Entra ID also sends attributes this server does not keep + # (name.givenName, enterprise extension, ...): ignored. + if attribute is not None: + changes.update(_user_change(attribute, value)) + elif op == "remove" and path is not None: + attribute = USER_ATTRIBUTES.get(path.lower()) + if attribute == "externalId": + changes["external_id"] = None + elif attribute == "displayName": + changes["display_name"] = None + else: + raise _unsupported_patch() + else: + raise _unsupported_patch() + _apply_user_changes(session, scim_user, changes, get_clock(request)()) + _commit_user(session) + return scim_response(_user_json(request, _load_user(session, organization_id, user_id))) + + @router.delete("/Users/{user_id}", status_code=204) + def delete_user( + organization_id: str, + user_id: str, + request: Request, + session: Session = Depends(get_session), + ): + scim_user = _load_user(session, organization_id, user_id)[0] + _guard_removal(session, organization_id, user_id) + _deactivate(session, organization_id, user_id, get_clock(request)()) + _remove_memberships(session, organization_id, user_id) + session.delete(scim_user) + session.commit() + return Response(status_code=204) + + # -- groups -- + + @router.get("/Groups") + def list_groups( + organization_id: str, request: Request, session: Session = Depends(get_session) + ): + start, count = _pagination(request) + query = ( + select(Group, ScimGroup) + .join(ScimGroup, ScimGroup.group_id == Group.id) + .where( + Group.organization_id == organization_id, + ScimGroup.organization_id == organization_id, + ) + ) + raw_filter = request.query_params.get("filter") + if raw_filter is not None: + match = GROUP_FILTER_RE.fullmatch(raw_filter.strip()) + if match is None: + raise ScimError(400, "unsupported filter", "invalidFilter") + attribute, value = match.group(1), _unescape(match.group(2)) + if attribute == "displayName": + query = query.where(func.lower(Group.name) == value.lower()) + else: + query = query.where(ScimGroup.external_id == value) + total = session.scalar(select(func.count()).select_from(query.subquery())) + rows = session.execute( + query.order_by(ScimGroup.created_at, Group.id).offset(start - 1).limit(count) + ).all() + resources = [_group_json(request, session, group, scim_group) for group, scim_group in rows] + return scim_response(_list_response(resources, total or 0, start)) + + @router.post("/Groups") + def create_group( + organization_id: str, + request: Request, + body: dict = Depends(scim_body), + scim_token: ScimToken = Depends(scim_organization), + session: Session = Depends(get_session), + ): + changes = _group_changes(body) + if "name" not in changes: + raise ScimError(400, "displayName is required", "invalidValue") + member_ids = changes.get("members", []) + _require_provisioned(session, organization_id, member_ids) + now_ts = get_clock(request)() + # The shape of POST /api/groups, owned by the token's creator. + group = Group( + id=str(uuid.uuid4()), + organization_id=organization_id, + owner_id=scim_token.created_by_id, + name=changes["name"], + join_policy="invite", + created_at=auth.now(), + ) + session.add(group) + session.flush() + session.add( + GroupMember( + group_id=group.id, + account_id=scim_token.created_by_id, + role="owner", + status="accepted", + created_at=auth.now(), + ) + ) + scim_group = ScimGroup( + group_id=group.id, + organization_id=organization_id, + external_id=changes.get("external_id"), + created_at=now_ts, + updated_at=now_ts, + ) + session.add(scim_group) + session.flush() + _add_members(session, group.id, member_ids) + session.commit() + resource = _group_json(request, session, group, scim_group) + return scim_response(resource, 201, headers={"Location": resource["meta"]["location"]}) + + @router.get("/Groups/{group_id}") + def get_group( + organization_id: str, + group_id: str, + request: Request, + session: Session = Depends(get_session), + ): + group, scim_group = _load_group(session, organization_id, group_id) + return scim_response(_group_json(request, session, group, scim_group)) + + @router.put("/Groups/{group_id}") + def replace_group( + organization_id: str, + group_id: str, + request: Request, + body: dict = Depends(scim_body), + session: Session = Depends(get_session), + ): + group, scim_group = _load_group(session, organization_id, group_id) + changes = {"external_id": None, "members": [], **_group_changes(body)} + if "name" not in changes: + raise ScimError(400, "displayName is required", "invalidValue") + _require_provisioned(session, organization_id, changes["members"]) + group.name = changes["name"] + scim_group.external_id = changes["external_id"] + _replace_members(session, group.id, changes["members"]) + scim_group.updated_at = get_clock(request)() + session.commit() + return scim_response(_group_json(request, session, group, scim_group)) + + @router.patch("/Groups/{group_id}") + def patch_group( + organization_id: str, + group_id: str, + request: Request, + body: dict = Depends(scim_body), + session: Session = Depends(get_session), + ): + group, scim_group = _load_group(session, organization_id, group_id) + + def apply(changes: dict, *, replace_members: bool) -> None: + if "name" in changes: + group.name = changes["name"] + if "external_id" in changes: + scim_group.external_id = changes["external_id"] + if "members" in changes: + _require_provisioned(session, organization_id, changes["members"]) + if replace_members: + _replace_members(session, group.id, changes["members"]) + else: + _add_members(session, group.id, changes["members"]) + + for op, path, value in _patch_operations(body): + if op not in ("add", "replace", "remove"): + raise _unsupported_patch() + if op == "remove": + if path is None: + raise _unsupported_patch() + match = MEMBER_PATH_RE.fullmatch(path) + if match is not None: + _remove_members(session, group.id, [_unescape(match.group(1))]) + elif path.lower() == "members": + _remove_members( + session, group.id, None if value is None else _member_ids(value) + ) + elif path.lower() == "externalid": + scim_group.external_id = None + else: + raise _unsupported_patch() + continue + if path is None: + if not isinstance(value, dict): + raise ScimError(400, "patch value must be an object", "invalidValue") + changes = _group_changes(value) + else: + attribute = GROUP_ATTRIBUTES.get(path.lower()) + if attribute is None: + raise _unsupported_patch() + changes = _group_changes({attribute: value}) + apply(changes, replace_members=op == "replace") + scim_group.updated_at = get_clock(request)() + session.commit() + return scim_response(_group_json(request, session, group, scim_group)) + + @router.delete("/Groups/{group_id}", status_code=204) + def delete_group( + organization_id: str, + group_id: str, + session: Session = Depends(get_session), + ): + _load_group(session, organization_id, group_id) + # The foreign keys cascade members, invitations, project shares, and the SCIM row. + session.execute(delete(Group).where(Group.id == group_id)) + session.commit() + return Response(status_code=204) + + return router diff --git a/backend/geolibre_server_api/tests/test_scim.py b/backend/geolibre_server_api/tests/test_scim.py new file mode 100644 index 0000000000..ec875e6408 --- /dev/null +++ b/backend/geolibre_server_api/tests/test_scim.py @@ -0,0 +1,583 @@ +"""SCIM 2.0 provisioning: tokens, users, groups, and deactivation everywhere.""" + +from __future__ import annotations + +import json + +import pytest +from conftest import OAUTH_CLIENTS, PUBLIC_URL, WEB_REDIRECT, _make_app +from fastapi.testclient import TestClient +from geolibre_server_api import auth as server_auth +from geolibre_server_api.auth_models import Account +from geolibre_server_api.enterprise_models import ScimUser +from geolibre_server_api.policy import deactivate_account +from helpers import ( + add_member, + admin_token, + approve, + auth, + configure_idp, + create_org, + ensure_account, + exchange_code, + pat, + redirect_params, + refresh, + sso_sign_in, + start_authorize, +) +from sqlalchemy import select + +ERROR_SCHEMA = "urn:ietf:params:scim:api:messages:2.0:Error" +PATCH_SCHEMA = "urn:ietf:params:scim:api:messages:2.0:PatchOp" +DEACTIVATED = "This account has been deactivated." + + +def _scim_org(client, slug="acme") -> tuple[str, str, str]: + """ada administers *slug* with the fake provider; returns (admin token, org id, SCIM token).""" + token = admin_token(client) + org_id = create_org(client, token, slug=slug, name=slug.title()) + assert configure_idp(client, token, org_id).status_code == 200 + minted = client.post( + f"/api/organizations/{org_id}/scim-tokens", json={"label": "Entra"}, headers=auth(token) + ) + assert minted.status_code == 201, minted.text + assert minted.json()["baseUrl"] == f"{PUBLIC_URL}/scim/v2/{org_id}" + return token, org_id, minted.json()["token"] + + +def _scim(client, scim_token, method, org_id, path, body=None, **kwargs): + headers = {"Authorization": f"Bearer {scim_token}"} + if body is not None: + headers["Content-Type"] = "application/scim+json" + kwargs["content"] = json.dumps(body) + return client.request(method, f"/scim/v2/{org_id}{path}", headers=headers, **kwargs) + + +def _patch(*operations) -> dict: + return {"schemas": [PATCH_SCHEMA], "Operations": list(operations)} + + +def _create_user(client, scim_token, org_id, user_name, **extra) -> dict: + created = _scim(client, scim_token, "POST", org_id, "/Users", {"userName": user_name, **extra}) + assert created.status_code == 201, created.text + return created.json() + + +def _sso_tokens(client, callback, verifier) -> dict: + assert callback.status_code == 303, (callback.status_code, callback.text[:300]) + assert callback.headers["location"].startswith(f"{WEB_REDIRECT}?") + exchanged = exchange_code(client, redirect_params(callback)["code"], verifier=verifier) + assert exchanged.status_code == 200, exchanged.text + return exchanged.json() + + +def _status(client, token) -> int: + return client.get("/api/users/me", headers=auth(token)).status_code + + +def test_scim_token_admin_routes(oauth_client): + token, org_id, _ = _scim_org(oauth_client) + listed = oauth_client.get(f"/api/organizations/{org_id}/scim-tokens", headers=auth(token)) + assert listed.status_code == 200 + assert listed.headers["cache-control"] == "private, no-store" + [entry] = listed.json()["scimTokens"] + assert entry["label"] == "Entra" + assert entry["revokedAt"] is None + assert "token" not in entry + + other_org = create_org(oauth_client, token, slug="other", name="Other") + foreign = oauth_client.delete( + f"/api/organizations/{other_org}/scim-tokens/{entry['id']}", headers=auth(token) + ) + assert foreign.status_code == 404 + assert foreign.json() == {"error": "SCIM token not found"} + revoked = oauth_client.delete( + f"/api/organizations/{org_id}/scim-tokens/{entry['id']}", headers=auth(token) + ) + assert revoked.status_code == 204 + listed = oauth_client.get(f"/api/organizations/{org_id}/scim-tokens", headers=auth(token)) + assert listed.json()["scimTokens"][0]["revokedAt"] is not None + + add_member(oauth_client, token, org_id, "bob") + member = admin_token(oauth_client, username="bob") + denied = oauth_client.post( + f"/api/organizations/{org_id}/scim-tokens", json={"label": "x"}, headers=auth(member) + ) + assert denied.status_code == 403 + + +def test_provisioned_user_links_to_sso_and_deactivation_revokes_everything(oauth_client, fake_idp): + _, org_id, scim_token = _scim_org(oauth_client) + created = _scim( + oauth_client, + scim_token, + "POST", + org_id, + "/Users", + {"userName": "Grace@Example.org", "active": True}, + ) + assert created.status_code == 201, created.text + assert created.headers["content-type"] == "application/scim+json" + user = created.json() + assert user["userName"] == "grace@example.org" + assert user["active"] is True + assert created.headers["location"] == f"{PUBLIC_URL}/scim/v2/{org_id}/Users/{user['id']}" + + claims = fake_idp.base_claims("entra-oid-1", preferred_username="grace@example.org") + tokens = _sso_tokens(oauth_client, *sso_sign_in(oauth_client, fake_idp, "acme", claims)) + me = oauth_client.get("/api/users/me", headers=auth(tokens["access_token"])) + assert me.status_code == 200, me.text + assert me.json()["user"]["id"] == user["id"] + projects = oauth_client.get("/api/projects", headers=auth(tokens["access_token"])) + assert projects.status_code == 200, projects.text + + app = oauth_client.app + with app.state.session_factory() as session: + account = session.get(Account, user["id"]) + personal_token, _ = server_auth.issue_token( + session, account, name="CI", scopes=["read:projects"], clock=app.state.clock + ) + assert _status(oauth_client, personal_token) == 200 + + deactivated = _scim( + oauth_client, + scim_token, + "PATCH", + org_id, + f"/Users/{user['id']}", + _patch({"op": "Replace", "value": {"active": "False"}}), + ) + assert deactivated.status_code == 200, deactivated.text + assert deactivated.json()["active"] is False + # No clock advance: every credential is dead at once. + assert _status(oauth_client, tokens["access_token"]) == 401 + refreshed = refresh(oauth_client, tokens["refresh_token"]) + assert refreshed.status_code == 400 + assert refreshed.json()["error"] == "invalid_grant" + assert _status(oauth_client, personal_token) == 401 + blocked, _ = sso_sign_in(oauth_client, fake_idp, "acme", claims) + assert blocked.status_code == 403 + assert DEACTIVATED in blocked.text + + reactivated = _scim( + oauth_client, + scim_token, + "PATCH", + org_id, + f"/Users/{user['id']}", + _patch({"op": "replace", "path": "active", "value": "True"}), + ) + assert reactivated.status_code == 200, reactivated.text + assert reactivated.json()["active"] is True + fresh = _sso_tokens(oauth_client, *sso_sign_in(oauth_client, fake_idp, "acme", claims)) + assert _status(oauth_client, fresh["access_token"]) == 200 + assert _status(oauth_client, tokens["access_token"]) == 401 + assert refresh(oauth_client, tokens["refresh_token"]).status_code == 400 + assert _status(oauth_client, personal_token) == 401 + + +def test_deactivating_an_unmanaged_account_only_removes_its_memberships(oauth_client): + token, org_id, scim_token = _scim_org(oauth_client) + ensure_account(oauth_client, "bob") + add_member(oauth_client, token, org_id, "bob") + group = oauth_client.post( + "/api/groups", json={"name": "Field team", "organizationId": org_id}, headers=auth(token) + ) + assert group.status_code == 201, group.text + group_id = group.json()["group"]["id"] + added = oauth_client.put( + f"/api/groups/{group_id}/members", + json={"username": "bob", "role": "member"}, + headers=auth(token), + ) + assert added.status_code in (200, 201), added.text + bob_pat = pat(oauth_client, "bob") + app = oauth_client.app + with app.state.session_factory() as session: + bob_id = session.scalar(select(Account.id).where(Account.username == "bob")) + now = app.state.clock() + session.add( + ScimUser( + organization_id=org_id, + account_id=bob_id, + user_name="bob", + created_at=now, + updated_at=now, + ) + ) + session.commit() + + patched = _scim( + oauth_client, + scim_token, + "PATCH", + org_id, + f"/Users/{bob_id}", + _patch({"op": "replace", "value": {"active": False}}), + ) + assert patched.status_code == 200, patched.text + assert patched.json()["active"] is False + assert _status(oauth_client, bob_pat) == 200 + orgs = oauth_client.get("/api/organizations/mine", headers=auth(bob_pat)) + assert orgs.status_code == 200 + assert org_id not in {org["id"] for org in orgs.json()["organizations"]} + groups = oauth_client.get("/api/groups/mine", headers=auth(bob_pat)) + assert group_id not in {item["id"] for item in groups.json()["groups"]} + + +def test_user_filters_and_token_scoping(oauth_client, clock): + _, org_id, scim_token = _scim_org(oauth_client) + _create_user(oauth_client, scim_token, org_id, "grace@example.org", externalId="ext-1") + clock.advance(1) # Lists are ordered by creation time. + _create_user(oauth_client, scim_token, org_id, "linus@example.org") + + by_name = _scim( + oauth_client, + scim_token, + "GET", + org_id, + "/Users", + params={"filter": 'userName eq "GRACE@example.org"'}, + ) + assert by_name.status_code == 200, by_name.text + assert by_name.json()["totalResults"] == 1 + assert by_name.json()["Resources"][0]["externalId"] == "ext-1" + page = _scim( + oauth_client, scim_token, "GET", org_id, "/Users", params={"startIndex": 2, "count": 1} + ) + assert page.json()["totalResults"] == 2 + assert [item["userName"] for item in page.json()["Resources"]] == ["linus@example.org"] + unsupported = _scim( + oauth_client, scim_token, "GET", org_id, "/Users", params={"filter": "title pr"} + ) + assert unsupported.status_code == 400 + assert unsupported.json()["scimType"] == "invalidFilter" + assert unsupported.json()["schemas"] == [ERROR_SCHEMA] + assert unsupported.headers["content-type"] == "application/scim+json" + + duplicate = _scim( + oauth_client, scim_token, "POST", org_id, "/Users", {"userName": "Grace@example.org"} + ) + assert duplicate.status_code == 409 + assert duplicate.json()["scimType"] == "uniqueness" + + admin = admin_token(oauth_client) + other_org = create_org(oauth_client, admin, slug="other", name="Other") + other = oauth_client.post( + f"/api/organizations/{other_org}/scim-tokens", json={"label": "x"}, headers=auth(admin) + ).json() + foreign = _scim(oauth_client, other["token"], "GET", org_id, "/Users") + assert foreign.status_code == 401 + assert foreign.headers["www-authenticate"] == "Bearer" + assert foreign.json()["detail"] == "invalid SCIM token" + + tokens = oauth_client.get(f"/api/organizations/{org_id}/scim-tokens", headers=auth(admin)) + token_id = tokens.json()["scimTokens"][0]["id"] + oauth_client.delete(f"/api/organizations/{org_id}/scim-tokens/{token_id}", headers=auth(admin)) + assert _scim(oauth_client, scim_token, "GET", org_id, "/Users").status_code == 401 + + +def test_user_patch_quirks_and_put(oauth_client): + _, org_id, scim_token = _scim_org(oauth_client) + user = _create_user(oauth_client, scim_token, org_id, "grace@example.org") + patched = _scim( + oauth_client, + scim_token, + "PATCH", + org_id, + f"/Users/{user['id']}", + _patch( + {"op": "Add", "path": 'emails[type eq "work"].value', "value": "g@example.org"}, + {"op": "Replace", "path": "displayName", "value": "Grace Hopper"}, + {"op": "Add", "path": "name.givenName", "value": "Grace"}, + {"op": "Add", "path": "externalId", "value": "oid-1"}, + ), + ) + assert patched.status_code == 200, patched.text + body = patched.json() + assert body["emails"] == [{"value": "g@example.org", "primary": True}] + assert body["displayName"] == "Grace Hopper" + assert body["externalId"] == "oid-1" + removed = _scim( + oauth_client, + scim_token, + "PATCH", + org_id, + f"/Users/{user['id']}", + _patch({"op": "Remove", "path": "externalId"}), + ) + assert "externalId" not in removed.json() + unsupported = _scim( + oauth_client, + scim_token, + "PATCH", + org_id, + f"/Users/{user['id']}", + _patch({"op": "remove", "path": "userName"}), + ) + assert unsupported.status_code == 400 + assert unsupported.json()["scimType"] == "invalidSyntax" + not_patch = _scim( + oauth_client, scim_token, "PATCH", org_id, f"/Users/{user['id']}", {"Operations": []} + ) + assert not_patch.status_code == 400 + assert not_patch.json()["scimType"] == "invalidSyntax" + + replaced = _scim( + oauth_client, + scim_token, + "PUT", + org_id, + f"/Users/{user['id']}", + {"userName": "grace.hopper@example.org", "emails": [{"value": "gh@example.org"}]}, + ) + assert replaced.status_code == 200, replaced.text + assert replaced.json()["userName"] == "grace.hopper@example.org" + assert "displayName" not in replaced.json() + assert replaced.json()["emails"][0]["value"] == "gh@example.org" + # The SCIM email is the representation only, never the account's address. + with oauth_client.app.state.session_factory() as session: + assert session.get(Account, user["id"]).email is None + + missing = _scim(oauth_client, scim_token, "GET", org_id, "/Users/nope") + assert missing.status_code == 404 + invalid = oauth_client.post( + f"/scim/v2/{org_id}/Users", + content=b"[1]", + headers={"Authorization": f"Bearer {scim_token}", "Content-Type": "application/json"}, + ) + assert invalid.status_code == 400 + assert invalid.json()["scimType"] == "invalidSyntax" + + +def test_group_membership_provisioning(oauth_client): + token, org_id, scim_token = _scim_org(oauth_client) + grace = _create_user(oauth_client, scim_token, org_id, "grace@example.org") + linus = _create_user(oauth_client, scim_token, org_id, "linus@example.org") + + stranger = _scim( + oauth_client, + scim_token, + "POST", + org_id, + "/Groups", + {"displayName": "GIS", "members": [{"value": "not-provisioned"}]}, + ) + assert stranger.status_code == 400 + assert stranger.json()["detail"] == "member is not provisioned in this organization" + + created = _scim( + oauth_client, + scim_token, + "POST", + org_id, + "/Groups", + {"displayName": "GIS", "externalId": "g-1", "members": [{"value": grace["id"]}]}, + ) + assert created.status_code == 201, created.text + group = created.json() + assert [member["value"] for member in group["members"]] == [grace["id"]] + listed = _scim( + oauth_client, + scim_token, + "GET", + org_id, + "/Groups", + params={"filter": 'displayName eq "gis"'}, + ) + assert listed.json()["totalResults"] == 1 + + added = _scim( + oauth_client, + scim_token, + "PATCH", + org_id, + f"/Groups/{group['id']}", + _patch({"op": "Add", "path": "members", "value": [{"value": linus["id"]}]}), + ) + assert {member["value"] for member in added.json()["members"]} == {grace["id"], linus["id"]} + removed = _scim( + oauth_client, + scim_token, + "PATCH", + org_id, + f"/Groups/{group['id']}", + _patch({"op": "Remove", "path": f'members[value eq "{grace["id"]}"]'}), + ) + assert removed.status_code == 200, removed.text + assert [member["value"] for member in removed.json()["members"]] == [linus["id"]] + + members = oauth_client.get(f"/api/groups/{group['id']}/members", headers=auth(token)) + assert members.status_code == 200, members.text + usernames = {member["username"]: member["role"] for member in members.json()["members"]} + assert "grace" not in usernames + assert usernames["ada"] == "owner" + assert usernames["linus"] == "member" + + renamed = _scim( + oauth_client, + scim_token, + "PUT", + org_id, + f"/Groups/{group['id']}", + {"displayName": "GIS team", "members": []}, + ) + assert renamed.json()["displayName"] == "GIS team" + assert renamed.json()["members"] == [] + deleted = _scim(oauth_client, scim_token, "DELETE", org_id, f"/Groups/{group['id']}") + assert deleted.status_code == 204 + gone = oauth_client.get(f"/api/groups/{group['id']}/members", headers=auth(token)) + assert gone.status_code == 404 + + +def test_last_administrator_cannot_be_removed(oauth_client): + token, org_id, scim_token = _scim_org(oauth_client) + admin = _create_user(oauth_client, scim_token, org_id, "root@example.org") + with oauth_client.app.state.session_factory() as session: + username = session.get(Account, admin["id"]).username + add_member(oauth_client, token, org_id, username, role="administrator") + # ada steps down, leaving the provisioned account as the only administrator. + stepped_down = oauth_client.put( + f"/api/organizations/{org_id}/members", + json={"username": "ada", "role": "member"}, + headers=auth(token), + ) + assert stepped_down.status_code == 200, stepped_down.text + + refused = _scim(oauth_client, scim_token, "DELETE", org_id, f"/Users/{admin['id']}") + assert refused.status_code == 409 + assert refused.json()["scimType"] == "mutability" + assert refused.json()["detail"] == "cannot remove the last organization administrator" + assert ( + _scim(oauth_client, scim_token, "GET", org_id, f"/Users/{admin['id']}").status_code == 200 + ) + + member = _create_user(oauth_client, scim_token, org_id, "temp@example.org") + assert ( + _scim(oauth_client, scim_token, "DELETE", org_id, f"/Users/{member['id']}").status_code + == 204 + ) + assert ( + _scim(oauth_client, scim_token, "GET", org_id, f"/Users/{member['id']}").status_code == 404 + ) + + +def test_break_glass_administrator_cannot_be_removed(oauth_client): + token, org_id, scim_token = _scim_org(oauth_client) + admin = _create_user(oauth_client, scim_token, org_id, "root@example.org") + with oauth_client.app.state.session_factory() as session: + username = session.get(Account, admin["id"]).username + add_member(oauth_client, token, org_id, username, role="administrator") + configured = configure_idp(oauth_client, token, org_id, breakGlassUsername=username) + assert configured.status_code == 200, configured.text + + refused = _scim(oauth_client, scim_token, "DELETE", org_id, f"/Users/{admin['id']}") + assert refused.status_code == 409 + assert refused.json()["scimType"] == "mutability" + assert refused.json()["detail"] == "cannot remove the organization's break-glass administrator" + assert ( + _scim(oauth_client, scim_token, "GET", org_id, f"/Users/{admin['id']}").status_code == 200 + ) + + +def test_removing_a_publisher_unpublishes_their_public_projects(oauth_client): + token, org_id, scim_token = _scim_org(oauth_client) + policy = oauth_client.patch( + f"/api/organizations/{org_id}", + json={"publicSharingPolicy": "publishers"}, + headers=auth(token), + ) + assert policy.status_code == 200, policy.text + ensure_account(oauth_client, "bob") + add_member(oauth_client, token, org_id, "bob", role="publisher") + bob_pat = pat(oauth_client, "bob") + created = oauth_client.post( + "/api/projects", + headers=auth(bob_pat), + json={ + "filename": "wetlands.geolibre.json", + "content": json.dumps({"version": "1.0", "title": "Wetlands", "layers": []}), + "visibility": "public", + "organizationId": org_id, + }, + ) + assert created.status_code == 201, created.text + project_id = created.json()["project"]["id"] + app = oauth_client.app + with app.state.session_factory() as session: + bob_id = session.scalar(select(Account.id).where(Account.username == "bob")) + now = app.state.clock() + session.add( + ScimUser( + organization_id=org_id, + account_id=bob_id, + user_name="bob", + created_at=now, + updated_at=now, + ) + ) + session.commit() + + removed = _scim(oauth_client, scim_token, "DELETE", org_id, f"/Users/{bob_id}") + assert removed.status_code == 204, removed.text + project = oauth_client.get(f"/api/projects/{project_id}", headers=auth(token)) + assert project.status_code == 200, project.text + assert project.json()["project"]["visibility"] == "organization" + + +def test_discovery_documents(oauth_client): + _, org_id, scim_token = _scim_org(oauth_client) + config = _scim(oauth_client, scim_token, "GET", org_id, "/ServiceProviderConfig") + assert config.status_code == 200 + assert config.json()["patch"] == {"supported": True} + assert config.json()["filter"] == {"supported": True, "maxResults": 100} + types = _scim(oauth_client, scim_token, "GET", org_id, "/ResourceTypes").json() + assert {item["endpoint"] for item in types["Resources"]} == {"/Users", "/Groups"} + schemas = _scim(oauth_client, scim_token, "GET", org_id, "/Schemas").json() + assert schemas["totalResults"] == 2 + assert _scim(oauth_client, "wrong", "GET", org_id, "/Schemas").status_code == 401 + + +@pytest.fixture +def proxied_app(tmp_path, monkeypatch, clock, fake_idp): + monkeypatch.setenv("GEOLIBRE_OAUTH_CLIENTS", json.dumps(OAUTH_CLIENTS)) + monkeypatch.setenv("GEOLIBRE_TRUSTED_PROXIES", "10.0.0.0/8") + monkeypatch.setenv("GEOLIBRE_PROXY_AUTH", "true") + return _make_app(tmp_path, PUBLIC_URL, clock=clock.now, oidc_transport=fake_idp.transport) + + +def _deactivate(app, username): + with app.state.session_factory() as session: + account_id = session.scalar(select(Account.id).where(Account.username == username)) + deactivate_account(session, account_id, app.state.clock()) + session.commit() + + +def test_deactivated_account_cannot_sign_in_with_password_or_proxy(proxied_app): + with TestClient(proxied_app, base_url=PUBLIC_URL) as direct: + ensure_account(direct, "bob") + _deactivate(proxied_app, "bob") + login = direct.post( + "/api/auth/token", json={"username": "bob", "password": "correct horse"} + ) + assert login.status_code == 401 + assert login.json() == {"error": "invalid username or password"} + _, _, interaction, csrf = start_authorize(direct) + page = approve(direct, interaction, csrf, username="bob") + assert "Invalid username or password" in page.text + + with TestClient( + proxied_app, + base_url=PUBLIC_URL, + client=("10.0.0.5", 5000), + headers={"Remote-User": "grace"}, + ) as proxy: + _, _, interaction, csrf = start_authorize(proxy) + first = approve(proxy, interaction, csrf, username="", password="") + assert first.status_code == 303, first.text + _deactivate(proxied_app, "grace") + _, _, interaction, csrf = start_authorize(proxy) + blocked = approve(proxy, interaction, csrf, username="", password="") + assert blocked.status_code == 200 + assert DEACTIVATED in blocked.text diff --git a/backend/geolibre_server_api/tests/test_scim_concurrency.py b/backend/geolibre_server_api/tests/test_scim_concurrency.py new file mode 100644 index 0000000000..79282dfeae --- /dev/null +++ b/backend/geolibre_server_api/tests/test_scim_concurrency.py @@ -0,0 +1,71 @@ +"""PostgreSQL concurrency gate for SCIM user provisioning. + +Runs only under ``python -m pytest -m postgres`` with ``GEOLIBRE_TEST_POSTGRES_URL`` +set (see conftest.py): two identity-provider requests creating the same +``userName`` must provision exactly one account. +""" + +from __future__ import annotations + +import json +import threading + +import pytest +from fastapi.testclient import TestClient +from helpers import admin_token, auth, create_org + +pytestmark = pytest.mark.postgres + +BASE_URL = "https://share.example" + + +def _run_concurrently(fns): + """Run *fns* on threads, returning (results, errors).""" + results: list = [None] * len(fns) + errors: list = [] + barrier = threading.Barrier(len(fns)) + + def runner(index, fn): + try: + barrier.wait(timeout=10) + results[index] = fn() + except Exception as exc: # noqa: BLE001 - record for assertion + errors.append(exc) + + threads = [threading.Thread(target=runner, args=(i, fn)) for i, fn in enumerate(fns)] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=30) + return results, errors + + +def test_concurrent_creates_of_one_user_name_provision_once(postgres_app): + with TestClient(postgres_app, base_url=BASE_URL) as admin: + token = admin_token(admin) + org_id = create_org(admin, token) + minted = admin.post( + f"/api/organizations/{org_id}/scim-tokens", json={"label": "Entra"}, headers=auth(token) + ) + assert minted.status_code == 201, minted.text + scim_token = minted.json()["token"] + + headers = {"Authorization": f"Bearer {scim_token}", "Content-Type": "application/scim+json"} + body = json.dumps({"userName": "grace@example.org"}) + clients = [TestClient(postgres_app, base_url=BASE_URL) for _ in range(2)] + + def create(client): + return lambda: client.post(f"/scim/v2/{org_id}/Users", content=body, headers=headers) + + try: + results, errors = _run_concurrently([create(client) for client in clients]) + listed = clients[0].get(f"/scim/v2/{org_id}/Users", headers=headers) + finally: + for client in clients: + client.close() + + assert errors == [] + assert sorted(result.status_code for result in results) == [201, 409] + loser = next(result for result in results if result.status_code == 409) + assert loser.json()["scimType"] == "uniqueness" + assert listed.json()["totalResults"] == 1 diff --git a/docs/server-api.md b/docs/server-api.md index f1f4823b04..578ac6935e 100644 --- a/docs/server-api.md +++ b/docs/server-api.md @@ -692,6 +692,115 @@ sign-in failed`. Identity headers from any other peer are never read, so the proxy must strip client-sent identity headers and be the only network path to the API. +### SCIM 2.0 + +An organization's identity provider can provision its users and groups with +SCIM 2.0 (RFC 7643/7644). An organization administrator mints a token for it: + +- `POST /api/organizations/{id}/scim-tokens` (`write:projects`), body + `{"label": "Entra ID"}` (1–100 characters), returns `201` + `{"token": "...", "scimToken": {...}, "baseUrl": "/scim/v2/"}`. + The raw token is shown only here; the server stores its digest. +- `GET /api/organizations/{id}/scim-tokens` (`read:projects`) returns + `{"scimTokens": [...]}`, newest first. +- `DELETE /api/organizations/{id}/scim-tokens/{tokenId}` (`write:projects`) + revokes the token. Response: `204`; an unknown token, or one of another + organization, is `404 SCIM token not found`. + +A token object is `{"id", "label", "createdAt", "lastUsedAt", "revokedAt"}` +(`lastUsedAt` is updated at most once a minute). The routes require an +organization administrator (with the organization's IP allowlist, and +re-authentication for `POST` and `DELETE`) and respond with +`Cache-Control: private, no-store`. + +Configure the identity provider with `baseUrl` as the tenant/SCIM URL and the +token as a Bearer secret. Every SCIM request needs +`Authorization: Bearer ` for that organization; a missing, revoked, or +other organization's token is `401` with `WWW-Authenticate: Bearer`. SCIM +requests are machine-to-machine, so the organization's administrator IP +allowlist and re-authentication window do not apply to them; revoke a token to +cut off a provider. + +Request bodies are `application/scim+json` (or `application/json`); responses +are `application/scim+json`. Errors use the SCIM error message: +`{"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"], "status": "409", "detail": "userName already exists", "scimType": "uniqueness"}`. + +| Endpoint | Notes | +| --- | --- | +| `GET /ServiceProviderConfig`, `/ResourceTypes`, `/Schemas` | Discovery. PATCH and filtering are supported (up to 100 results); bulk, sort, ETags, and password changes are not. | +| `GET /Users` | `startIndex` (default 1) and `count` (0–100, default 100), ordered by creation. `filter` only `userName eq "…"` (case-insensitive) or `externalId eq "…"`; anything else is `400 invalidFilter`. | +| `POST /Users` | Creates an account. `userName` (1–255 characters, stored lowercased) is required and unique per organization (`409 uniqueness`). | +| `GET`/`PUT`/`PATCH`/`DELETE /Users/{id}` | `id` is the account id. `PUT` replaces `userName`, `externalId`, `displayName`, `emails`, and (when present) `active`. | +| `GET /Groups` | `filter` only `displayName eq "…"` (case-insensitive) or `externalId eq "…"`. | +| `POST /Groups` | Creates an organization group owned by the token's creator, with `join_policy` `invite`. Every member must be a user provisioned in this organization (`400 invalidValue`). | +| `GET`/`PUT`/`PATCH`/`DELETE /Groups/{id}` | `PUT` replaces `displayName`, `externalId`, and the member set. `DELETE` deletes the group. | + +A user resource carries `id`, `userName`, `externalId`, `displayName`, +`active`, `emails` (the primary address only), and `meta`. Other attributes are +accepted and ignored. A provisioned account has no password and no email +address of its own: `emails` is kept for the SCIM representation only. Its +username is derived from `userName` the way single sign-on derives it, and it +joins the organization with the identity provider's `defaultRole` (`member` +without a provider). A group resource's `members` lists only plain accepted +members; the owner and managers are never listed or changed by SCIM. Only users +and groups created through SCIM are visible to it (`404 resource not found` +otherwise). + +**PATCH** bodies need the `urn:ietf:params:scim:api:messages:2.0:PatchOp` +schema and an `Operations` list (else `400 invalidSyntax`). `op` is +case-insensitive. For users, `add` and `replace` accept the paths `active`, +`userName`, `externalId`, `displayName`, `emails`, and +`emails[type eq "work"].value`, or no path with an object of attributes; +unknown paths are ignored. `remove` accepts only `externalId` and +`displayName`. Other operations are `400 unsupported patch operation` +(`invalidSyntax`). For groups, `add` takes `members` (a list of +`{"value": ""}`), `displayName`, or `externalId`; `remove` takes +`members[value eq ""]`, `members` with a value list (or no value to +remove every member), or `externalId`; `replace` takes `displayName`, +`externalId`, `members`, or no path with an object of those. A successful +PATCH returns `200` with the resource. + +**Entra ID:** `active` may be the strings `"True"`/`"False"`, operation names +may be capitalized (`Replace`), and Entra's extra attribute paths are ignored, +so its default attribute mappings work unchanged. Entra soft-deletes by +setting `active` to `false`, and later sends `DELETE`. + +**Deactivation.** Setting `active` to `false` (`PUT` or `PATCH`), or +`DELETE`, depends on who manages the account: + +- An account this organization manages (created by its SCIM or its single + sign-on) is deactivated: every OAuth session and personal token is revoked in + the same transaction, Bearer use returns `401 invalid or expired token`, + refresh returns `400 invalid_grant`, password sign-in fails as an invalid + password, single sign-on returns a `403` page with + `This account has been deactivated.`, and trusted-proxy sign-in shows the + same message on the consent page. +- Any other account only loses its membership in this organization and its + membership (except group ownership) in the organization's groups; the account + itself and its other organizations are untouched. Access ends on the next + request because authorization reads memberships live. Its SCIM `active` reads + `false` while it is not a member. +- Setting `active` back to `true` reactivates a managed account and re-adds the + organization membership if missing. Credentials revoked by the deactivation + stay revoked; the user signs in again. +- `DELETE /Users/{id}` deactivates as above, removes the organization and group + memberships, and forgets the SCIM user. Removing the organization's only + administrator, by deactivation of an account it does not manage or by + `DELETE`, is `409 cannot remove the last organization administrator` + (`mutability`); removing the identity provider's break-glass account is + `409 cannot remove the organization's break-glass administrator` + (`mutability`). Clear `breakGlassUsername` on the provider first. +- Removing a membership applies the organization's public sharing policy the + same way leaving through the members API does: the removed account's public + organization projects become organization-only unless the policy is `yes`. + +**Single sign-on link:** the first single sign-on of a subject in an +organization with SCIM users links to the provisioned account whose `userName` +equals the ID token's `usernameClaim` (lowercased), or failing that its +`emailClaim`, instead of creating a new account. Provision `userName` as the +value the provider puts in that claim (for Entra ID, the UPN in +`preferred_username`). + ## Projects ### Project representation From 212a6009a8492d1b38d969c7bc271d957be4b51f Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Sat, 3 Oct 2026 00:53:12 +0200 Subject: [PATCH 2/3] fix(server-api): close SCIM deprovisioning gaps found in review --- .../geolibre_server_api/auth.py | 2 + .../geolibre_server_api/enterprise_models.py | 30 ++- .../geolibre_server_api/main.py | 35 +-- .../geolibre_server_api/oidc.py | 245 +++++++++++++++--- .../geolibre_server_api/policy.py | 67 ++++- .../geolibre_server_api/scim.py | 169 +++++++++--- .../tests/test_enterprise_policy.py | 22 +- .../geolibre_server_api/tests/test_scim.py | 191 +++++++++++++- docs/server-api.md | 27 +- 9 files changed, 669 insertions(+), 119 deletions(-) diff --git a/backend/geolibre_server_api/geolibre_server_api/auth.py b/backend/geolibre_server_api/geolibre_server_api/auth.py index 5b59d4fe79..3cd7fe891a 100644 --- a/backend/geolibre_server_api/geolibre_server_api/auth.py +++ b/backend/geolibre_server_api/geolibre_server_api/auth.py @@ -2203,6 +2203,8 @@ def exchange_code( or code_row.account_id is None or code_row.code_expires_at is None or code_row.code_expires_at <= now_ts + # Deactivated between approval and exchange. + or is_deactivated(session, code_row.account_id) ): return oauth_token_error(400, "invalid_grant") diff --git a/backend/geolibre_server_api/geolibre_server_api/enterprise_models.py b/backend/geolibre_server_api/geolibre_server_api/enterprise_models.py index 41735f3360..9cb090dfef 100644 --- a/backend/geolibre_server_api/geolibre_server_api/enterprise_models.py +++ b/backend/geolibre_server_api/geolibre_server_api/enterprise_models.py @@ -5,7 +5,7 @@ from __future__ import annotations -from sqlalchemy import Boolean, ForeignKey, Integer, String, Text, UniqueConstraint +from sqlalchemy import Boolean, ForeignKey, Index, Integer, String, Text, UniqueConstraint from sqlalchemy.orm import Mapped, mapped_column from geolibre_server_api.auth_models import Base @@ -106,6 +106,34 @@ class FederatedIdentity(Base): subject: Mapped[str] = mapped_column(String(255)) created_at: Mapped[int] = mapped_column(Integer) last_login_at: Mapped[int] = mapped_column(Integer) + # Refreshed at every OIDC sign-in so SCIM can adopt the account (never set + # for proxy identities): the lowercased username claim, and the lowercased + # email claim only while the provider asserts ``email_verified``. + claimed_username: Mapped[str | None] = mapped_column(String(255), nullable=True) + claimed_email: Mapped[str | None] = mapped_column(String(320), nullable=True) + + +# Separate metadata objects so startup can add them to an existing table too. +FEDERATED_IDENTITY_INDEXES = ( + # One link per account and provider: SSO never links a second subject to an + # account (a concurrent second link fails here). + Index( + "uq_federated_identity_account_provider", + FederatedIdentity.__table__.c.account_id, + FederatedIdentity.__table__.c.provider_key, + unique=True, + ), + Index( + "ix_federated_identities_claimed_username", + FederatedIdentity.__table__.c.provider_id, + FederatedIdentity.__table__.c.claimed_username, + ), + Index( + "ix_federated_identities_claimed_email", + FederatedIdentity.__table__.c.provider_id, + FederatedIdentity.__table__.c.claimed_email, + ), +) class OidcLoginState(Base): diff --git a/backend/geolibre_server_api/geolibre_server_api/main.py b/backend/geolibre_server_api/geolibre_server_api/main.py index d5271462cf..7058f4a679 100644 --- a/backend/geolibre_server_api/geolibre_server_api/main.py +++ b/backend/geolibre_server_api/geolibre_server_api/main.py @@ -34,7 +34,6 @@ from sqlalchemy.exc import IntegrityError, OperationalError from sqlalchemy.orm import Session, selectinload, sessionmaker -from geolibre_server_api import enterprise_models # noqa: F401 from geolibre_server_api.auth import ( AuthPrincipal, InsufficientScopeError, @@ -52,6 +51,7 @@ ) from geolibre_server_api.auth_models import OAUTH_INDEXES, Account, Base from geolibre_server_api.enterprise_admin import build_enterprise_admin_router +from geolibre_server_api.enterprise_models import FEDERATED_IDENTITY_INDEXES from geolibre_server_api.oidc import build_http_client, build_transport from geolibre_server_api.org_models import ( Group, @@ -64,6 +64,7 @@ OrganizationRole, ) from geolibre_server_api.policy import ( + is_last_active_admin, organization_role, require_not_break_glass, require_organization_admin, @@ -426,6 +427,8 @@ def postgresql_upgrade_statements() -> list[str]: """, "ALTER TABLE oauth_authorization_codes ADD COLUMN IF NOT EXISTS authenticated_at INTEGER", "ALTER TABLE oauth_sessions ADD COLUMN IF NOT EXISTS authenticated_at INTEGER", + "ALTER TABLE federated_identities ADD COLUMN IF NOT EXISTS claimed_username VARCHAR(255)", + "ALTER TABLE federated_identities ADD COLUMN IF NOT EXISTS claimed_email VARCHAR(320)", ] @@ -446,6 +449,10 @@ def upgrade_sqlite_schema(engine) -> None: tables = set(inspector.get_table_names()) additions = { "accounts": [("email", "VARCHAR(320)")], + "federated_identities": [ + ("claimed_username", "VARCHAR(255)"), + ("claimed_email", "VARCHAR(320)"), + ], "oauth_authorization_codes": [("authenticated_at", "INTEGER")], "oauth_sessions": [("authenticated_at", "INTEGER")], "projects": [ @@ -678,6 +685,9 @@ def _enable_foreign_keys(dbapi_connection, _record): # pragma: no cover - drive index.create(engine, checkfirst=True) # create_all does not add an index to an already-existing transfer table. PENDING_TRANSFER_INDEX.create(engine, checkfirst=True) + # Nor the federated identity indexes to a table from an earlier release. + for index in FEDERATED_IDENTITY_INDEXES: + index.create(engine, checkfirst=True) sessions = sessionmaker(engine, expire_on_commit=False) oauth_config = make_oauth_config(public_url) clock_fn = clock or (lambda: int(datetime.now(UTC).timestamp())) @@ -1779,15 +1789,7 @@ def put_organization_member( session.add(member) else: if member.role == "administrator" and body.role != "administrator": - admin_count = session.scalar( - select(func.count()) - .select_from(OrganizationMember) - .where( - OrganizationMember.organization_id == organization_id, - OrganizationMember.role == "administrator", - ) - ) - if admin_count == 1: + if is_last_active_admin(session, organization_id, target.id): raise HTTPException(409, "organization must have an administrator") require_not_break_glass(session, organization_id, target.id) member.role = body.role @@ -1833,17 +1835,8 @@ def delete_organization_member( member = session.get(OrganizationMember, (organization_id, target.id)) if target else None if member is None: raise HTTPException(404, "organization member not found") - if member.role == "administrator": - admin_count = session.scalar( - select(func.count()) - .select_from(OrganizationMember) - .where( - OrganizationMember.organization_id == organization_id, - OrganizationMember.role == "administrator", - ) - ) - if admin_count == 1: - raise HTTPException(409, "organization must have an administrator") + if is_last_active_admin(session, organization_id, target.id): + raise HTTPException(409, "organization must have an administrator") require_not_break_glass(session, organization_id, target.id) session.delete(member) detach_from_organization_groups(session, organization_id, target, account) diff --git a/backend/geolibre_server_api/geolibre_server_api/oidc.py b/backend/geolibre_server_api/geolibre_server_api/oidc.py index cf08c6e716..b4920ac134 100644 --- a/backend/geolibre_server_api/geolibre_server_api/oidc.py +++ b/backend/geolibre_server_api/geolibre_server_api/oidc.py @@ -27,7 +27,7 @@ from joserfc import jwt from joserfc.errors import InvalidKeyIdError, JoseError from joserfc.jwk import KeySet -from sqlalchemy import func, select +from sqlalchemy import delete, select, update from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session @@ -38,6 +38,7 @@ AccountSecurity, FederatedIdentity, OrganizationIdentityProvider, + ScimGroup, ScimUser, ) from geolibre_server_api.org_models import ( @@ -47,6 +48,7 @@ Organization, OrganizationMember, ) +from geolibre_server_api.policy import deactivate_account, is_deactivated, is_last_active_admin from geolibre_server_api.projects import demote_disallowed_public_projects from geolibre_server_api.proxy_identity import ProxyIdentity, parse_networks @@ -361,15 +363,7 @@ def apply_org_mapping( # The break-glass account must stay an administrator to keep its # password sign-in; clearing it on the provider is the way out. provider.break_glass_account_id == account_id - or session.scalar( - select(func.count()) - .select_from(OrganizationMember) - .where( - OrganizationMember.organization_id == organization_id, - OrganizationMember.role == "administrator", - ) - ) - == 1 + or is_last_active_admin(session, organization_id, account_id) ) if not keeps_admin: lowered = ROLE_RANK[mapped_role] < ROLE_RANK[member.role] @@ -429,6 +423,8 @@ def _resolve_identity( new_email: Callable[[], str | None], managed_by: str | None, now_ts: int, + claimed_username: str | None = None, + claimed_email: str | None = None, ) -> Account: """The account linked to (provider_key, subject), created just in time when missing.""" identity = _find_identity(session, provider_key, subject) @@ -452,6 +448,8 @@ def _resolve_identity( subject=subject, created_at=now_ts, last_login_at=now_ts, + claimed_username=claimed_username, + claimed_email=claimed_email, ) try: with session.begin_nested(): @@ -473,6 +471,8 @@ def _resolve_identity( if identity is None: raise OidcError("account creation conflict") identity.last_login_at = now_ts + identity.claimed_username = claimed_username + identity.claimed_email = claimed_email account = session.get(Account, identity.account_id) if account is None: # pragma: no cover - the FK cascades account deletion raise RuntimeError("federated identity without an account") @@ -486,45 +486,220 @@ def _first_string(*values: object) -> str: return "" -def _link_scim_user( - session: Session, provider: OrganizationIdentityProvider, claims: dict, now_ts: int +def _claimed_values( + claims: dict, provider: OrganizationIdentityProvider +) -> tuple[str | None, str | None]: + """The lowercased username claim, and the lowercased email claim when verified.""" + username = claims.get(provider.username_claim) + email = claims.get(provider.email_claim) + claimed_username = ( + username.lower() if isinstance(username, str) and 0 < len(username) <= 255 else None + ) + claimed_email = ( + email.lower() + if claims.get("email_verified") is True and isinstance(email, str) and 0 < len(email) <= 320 + else None + ) + return claimed_username, claimed_email + + +def _managed_here(session: Session, organization_id: str, account_id: str) -> bool: + managed_by = session.scalar( + select(AccountSecurity.managed_by_organization_id).where( + AccountSecurity.account_id == account_id + ) + ) + return managed_by == organization_id + + +def _has_identity(session: Session, account_id: str, provider_key: str | None = None) -> bool: + query = select(FederatedIdentity.id).where(FederatedIdentity.account_id == account_id) + if provider_key is not None: + query = query.where(FederatedIdentity.provider_key == provider_key) + return session.scalar(query.limit(1)) is not None + + +def _has_scim_user(session: Session, organization_id: str, account_id: str) -> bool: + return session.get(ScimUser, (organization_id, account_id), populate_existing=True) is not None + + +def _adopt_orphan( + session: Session, + provider: OrganizationIdentityProvider, + account_id: str, + orphan: ScimUser, + now_ts: int, ) -> None: - """Link a subject's first sign-in to the account SCIM provisioned for that user name. + """Hand the SCIM user of an unused account SCIM created to the subject's real account. - The username claim is tried first, then the email claim; only string claims count. + The orphan was provisioned while the user already had a single sign-on + account, so it never had a way to sign in. The real account takes over the + SCIM user, its deactivation, and its plain memberships in the organization's + SCIM groups; the orphan is deactivated and leaves the organization. """ - if _find_identity(session, provider.id, claims["sub"]) is not None: + organization_id = provider.organization_id + orphan_id = orphan.account_id + orphan_deactivated = is_deactivated(session, orphan_id) + try: + with session.begin_nested(): + # Conditional on the orphan still holding it: of two concurrent + # sign-ins, only one moves the SCIM user. + moved = session.execute( + update(ScimUser) + .where( + ScimUser.organization_id == organization_id, + ScimUser.account_id == orphan_id, + ) + .values(account_id=account_id) + .execution_options(synchronize_session=False) + ).rowcount + except IntegrityError: + # The real account was provisioned concurrently. + return + session.expunge(orphan) + if not moved: return - for claim_name in (provider.username_claim, provider.email_claim): - value = claims.get(claim_name) - if not isinstance(value, str) or not value: + + scim_group_ids = select(ScimGroup.group_id).where(ScimGroup.organization_id == organization_id) + orphan_rows = session.scalars( + select(GroupMember).where( + GroupMember.account_id == orphan_id, + GroupMember.role == "member", + GroupMember.group_id.in_(scim_group_ids), + ) + ).all() + for row in orphan_rows: + if session.get(GroupMember, (row.group_id, account_id)) is not None: continue - account_id = session.scalar( - select(ScimUser.account_id).where( - ScimUser.organization_id == provider.organization_id, - ScimUser.user_name == value.lower(), - ) + try: + with session.begin_nested(): + session.add( + GroupMember( + group_id=row.group_id, + account_id=account_id, + role="member", + status=row.status, + created_at=row.created_at, + ) + ) + session.flush() + except IntegrityError: + pass + + if orphan_deactivated: + if provider.break_glass_account_id == account_id or is_last_active_admin( + session, organization_id, account_id + ): + # SCIM itself refuses this deactivation (409); the provider sees + # the account active and its next update is refused the same way. + logger.warning("scim deactivation not carried over to the last administrator") + else: + deactivate_account(session, account_id, now_ts) + + deactivate_account(session, orphan_id, now_ts) + session.execute( + delete(OrganizationMember).where( + OrganizationMember.organization_id == organization_id, + OrganizationMember.account_id == orphan_id, ) - if account_id is None: + ) + session.execute( + delete(GroupMember).where( + GroupMember.account_id == orphan_id, + GroupMember.role == "member", + GroupMember.group_id.in_( + select(Group.id).where(Group.organization_id == organization_id) + ), + ) + ) + + +def _link_scim_user( + session: Session, + provider: OrganizationIdentityProvider, + subject: str, + claimed: tuple[str | None, str | None], + now_ts: int, +) -> None: + """Connect the subject with the account SCIM provisioned for its claimed user name. + + The SCIM user is found by the username claim, else the verified email claim. + + - A first sign-in links to that account, unless another subject of this + provider already holds it (then the sign-in creates its own account). + - A subject whose account SCIM deleted (deactivated, no SCIM user) moves to + the account SCIM provisioned again, when no subject holds that one yet. + - A subject whose account predates SCIM (managed here, no SCIM user) takes + over the SCIM user of an account SCIM created for it that never signed in. + """ + organization_id = provider.organization_id + scim_user = None + for value in claimed: + if value is None: continue + scim_user = session.scalar( + select(ScimUser).where( + ScimUser.organization_id == organization_id, ScimUser.user_name == value + ) + ) + if scim_user is not None: + break + if scim_user is None: + return + target_id = scim_user.account_id + identity = _find_identity(session, provider.id, subject) + if identity is None: + if _has_identity(session, target_id, provider.id): + return try: with session.begin_nested(): session.add( FederatedIdentity( id=str(uuid.uuid4()), - account_id=account_id, + account_id=target_id, provider_key=provider.id, provider_id=provider.id, - subject=claims["sub"], + subject=subject, created_at=now_ts, last_login_at=now_ts, ) ) session.flush() except IntegrityError: - # A concurrent first sign-in linked the subject; the caller re-reads it. + # A concurrent sign-in linked the subject or the account; the caller + # re-reads the subject's link or creates its own account. + pass + return + + current_id = identity.account_id + if ( + current_id == target_id + or _has_scim_user(session, organization_id, current_id) + or not _managed_here(session, organization_id, current_id) + or not _managed_here(session, organization_id, target_id) + ): + return + if is_deactivated(session, current_id): + if _has_identity(session, target_id, provider.id): + return + try: + with session.begin_nested(): + session.execute( + update(FederatedIdentity) + .where( + FederatedIdentity.id == identity.id, + FederatedIdentity.account_id == current_id, + ) + .values(account_id=target_id) + .execution_options(synchronize_session=False) + ) + except IntegrityError: + # A concurrent sign-in linked another subject to the account. pass + session.expire(identity) return + if not _has_identity(session, target_id): + _adopt_orphan(session, provider, current_id, scim_user, now_ts) def resolve_oidc_account( @@ -535,12 +710,13 @@ def resolve_oidc_account( ) -> Account: """Find or JIT-create the account for validated claims and apply the org mapping. - A first sign-in links to the account the provider's own organization - provisioned over SCIM under the same user name. It never links to any other - existing account by email: that would let any IdP that asserts an address - take over a local account. + Sign-in reconciles with the provider's own organization's SCIM users + (``_link_scim_user``). It never links to any other existing account by + email: that would let any IdP that asserts an address take over a local + account. A deactivated account gets no mapping, so its memberships stay gone. """ - _link_scim_user(session, provider, claims, now_ts) + claimed_username, claimed_email = _claimed_values(claims, provider) + _link_scim_user(session, provider, claims["sub"], (claimed_username, claimed_email), now_ts) account = _resolve_identity( session, provider_key=provider.id, @@ -552,8 +728,11 @@ def resolve_oidc_account( new_email=lambda: _verified_email(session, claims, provider.email_claim), managed_by=provider.organization_id, now_ts=now_ts, + claimed_username=claimed_username, + claimed_email=claimed_email, ) - apply_org_mapping(session, provider, account.id, _claim_groups(claims, provider)) + if not is_deactivated(session, account.id): + apply_org_mapping(session, provider, account.id, _claim_groups(claims, provider)) session.commit() return account diff --git a/backend/geolibre_server_api/geolibre_server_api/policy.py b/backend/geolibre_server_api/geolibre_server_api/policy.py index 44f898e187..61bd72d3ed 100644 --- a/backend/geolibre_server_api/geolibre_server_api/policy.py +++ b/backend/geolibre_server_api/geolibre_server_api/policy.py @@ -8,7 +8,7 @@ from typing import TYPE_CHECKING from fastapi import HTTPException, Request -from sqlalchemy import func, select, update +from sqlalchemy import func, or_, select, update from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session @@ -227,16 +227,41 @@ def is_deactivated(session: Session, account_id: str) -> bool: return status == "deactivated" -def deactivate_account(session: Session, account_id: str, now_ts: int) -> None: - """Deactivate the account and revoke every OAuth family and personal token. +def active_admin_count( + session: Session, organization_id: str, *, excluding: str | None = None +) -> int: + """The organization's administrators whose accounts are not deactivated.""" + query = ( + select(func.count()) + .select_from(OrganizationMember) + .outerjoin(AccountSecurity, AccountSecurity.account_id == OrganizationMember.account_id) + .where( + OrganizationMember.organization_id == organization_id, + OrganizationMember.role == "administrator", + or_(AccountSecurity.status.is_(None), AccountSecurity.status != "deactivated"), + ) + ) + if excluding is not None: + query = query.where(OrganizationMember.account_id != excluding) + return session.scalar(query) or 0 - The caller commits, so the status flip and the revocations land together. + +def is_last_active_admin(session: Session, organization_id: str, account_id: str) -> bool: + """True when the account is an administrator and no other active administrator remains. + + Demoting, removing, or deactivating it would leave the organization without + an administrator who can sign in. """ + return ( + organization_role(session, organization_id, account_id) == "administrator" + and active_admin_count(session, organization_id, excluding=account_id) == 0 + ) + + +def _revoke_credentials(session: Session, account_id: str, now_ts: int) -> None: + """Revoke every live OAuth family and personal token of the account.""" from geolibre_server_api.auth import backfill_account_policies - security = ensure_account_security(session, account_id) - security.status = "deactivated" - security.deactivated_at = now_ts session.execute( update(OAuthSession) .where(OAuthSession.account_id == account_id, OAuthSession.revoked_at.is_(None)) @@ -256,8 +281,28 @@ def deactivate_account(session: Session, account_id: str, now_ts: int) -> None: ) -def reactivate_account(session: Session, account_id: str) -> None: - """Allow sign-in again; credentials revoked by the deactivation stay revoked.""" +def deactivate_account(session: Session, account_id: str, now_ts: int) -> None: + """Deactivate the account and revoke every OAuth family and personal token. + + The caller commits, so the status flip and the revocations land together. + """ security = ensure_account_security(session, account_id) - security.status = "active" - security.deactivated_at = None + security.status = "deactivated" + security.deactivated_at = now_ts + _revoke_credentials(session, account_id, now_ts) + + +def reactivate_account(session: Session, account_id: str, now_ts: int) -> None: + """Allow sign-in again; nothing issued before or during the deactivation survives. + + Only a deactivated account changes: credentials minted while it was + deactivated (a code exchange racing the deactivation) are revoked too, and + an already active account keeps its sessions. + """ + reactivated = session.execute( + update(AccountSecurity) + .where(AccountSecurity.account_id == account_id, AccountSecurity.status == "deactivated") + .values(status="active", deactivated_at=None) + ).rowcount + if reactivated: + _revoke_credentials(session, account_id, now_ts) diff --git a/backend/geolibre_server_api/geolibre_server_api/scim.py b/backend/geolibre_server_api/geolibre_server_api/scim.py index 5658cefd46..1b70438a81 100644 --- a/backend/geolibre_server_api/geolibre_server_api/scim.py +++ b/backend/geolibre_server_api/geolibre_server_api/scim.py @@ -20,7 +20,7 @@ from fastapi import APIRouter, Depends, Request, Response from fastapi.responses import JSONResponse -from sqlalchemy import and_, delete, func, select, update +from sqlalchemy import and_, delete, func, or_, select, update from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session @@ -29,6 +29,7 @@ from geolibre_server_api.auth_models import Account from geolibre_server_api.enterprise_models import ( AccountSecurity, + FederatedIdentity, OrganizationIdentityProvider, ScimGroup, ScimToken, @@ -36,7 +37,13 @@ ) from geolibre_server_api.oidc import SSO_PASSWORD_HASH, derive_username from geolibre_server_api.org_models import Group, GroupMember, Organization, OrganizationMember -from geolibre_server_api.policy import deactivate_account, reactivate_account +from geolibre_server_api.policy import ( + deactivate_account, + is_deactivated, + is_last_active_admin, + organization_role, + reactivate_account, +) from geolibre_server_api.projects import demote_disallowed_public_projects SCIM_MEDIA_TYPE = "application/scim+json" @@ -114,6 +121,13 @@ def scim_organization( ScimToken.revoked_at.is_(None), ) ) + # The token acts for its creator (who owns SCIM groups): it stops working + # while the creator is not an active administrator of the organization. + if row is not None and ( + organization_role(session, organization_id, row.created_by_id) != "administrator" + or is_deactivated(session, row.created_by_id) + ): + row = None if row is None: raise ScimError(401, "invalid SCIM token", None, {"WWW-Authenticate": "Bearer"}) now_ts = get_clock(request)() @@ -299,23 +313,9 @@ def _default_role(session: Session, organization_id: str) -> str: return role or "member" -def _is_last_admin(session: Session, organization_id: str, account_id: str) -> bool: - member = session.get(OrganizationMember, (organization_id, account_id)) - if member is None or member.role != "administrator": - return False - admin_count = session.scalar( - select(func.count()) - .select_from(OrganizationMember) - .where( - OrganizationMember.organization_id == organization_id, - OrganizationMember.role == "administrator", - ) - ) - return admin_count == 1 - - def _guard_removal(session: Session, organization_id: str, account_id: str) -> None: - if _is_last_admin(session, organization_id, account_id): + """Refuse to remove or deactivate the last active administrator or the break-glass account.""" + if is_last_active_admin(session, organization_id, account_id): raise ScimError(409, "cannot remove the last organization administrator", "mutability") break_glass = session.scalar( select(OrganizationIdentityProvider.id).where( @@ -330,8 +330,10 @@ def _guard_removal(session: Session, organization_id: str, account_id: str) -> N def _remove_memberships(session: Session, organization_id: str, account_id: str) -> None: - """Drop the account's membership in the organization and its groups (not ownership).""" - _guard_removal(session, organization_id, account_id) + """Drop the account's membership in the organization and its groups (not ownership). + + The caller runs ``_guard_removal`` first. + """ session.execute( delete(OrganizationMember).where( OrganizationMember.organization_id == organization_id, @@ -364,6 +366,7 @@ def _is_managed(session: Session, organization_id: str, account_id: str) -> bool def _deactivate(session: Session, organization_id: str, account_id: str, now_ts: int) -> None: + _guard_removal(session, organization_id, account_id) if _is_managed(session, organization_id, account_id): deactivate_account(session, account_id, now_ts) else: @@ -371,9 +374,7 @@ def _deactivate(session: Session, organization_id: str, account_id: str, now_ts: _remove_memberships(session, organization_id, account_id) -def _activate(session: Session, organization_id: str, account_id: str) -> None: - if _is_managed(session, organization_id, account_id): - reactivate_account(session, account_id) +def _ensure_member(session: Session, organization_id: str, account_id: str) -> None: if session.get(OrganizationMember, (organization_id, account_id)) is None: session.add( OrganizationMember( @@ -385,6 +386,12 @@ def _activate(session: Session, organization_id: str, account_id: str) -> None: ) +def _activate(session: Session, organization_id: str, account_id: str, now_ts: int) -> None: + if _is_managed(session, organization_id, account_id): + reactivate_account(session, account_id, now_ts) + _ensure_member(session, organization_id, account_id) + + # --------------------------------------------------------------------------- # Resources # --------------------------------------------------------------------------- @@ -455,6 +462,69 @@ def _duplicate_user_name() -> ScimError: return ScimError(409, "userName already exists", "uniqueness") +def _adoptable_account(session: Session, organization_id: str, user_name: str) -> str | None: + """The one account this organization's single sign-on created for ``user_name``. + + Users who signed in before SCIM was set up already have an account: SCIM + takes it over instead of creating a second one. Only accounts this + organization manages, linked to its provider, and not yet provisioned here + qualify, so another organization's, password, or proxy accounts never do. + The username claim decides first, then the verified email claim; an + ambiguous match adopts nothing. + """ + provider_id = session.scalar( + select(OrganizationIdentityProvider.id).where( + OrganizationIdentityProvider.organization_id == organization_id + ) + ) + if provider_id is None: + return None + candidates = ( + select(FederatedIdentity.account_id) + .distinct() + .join(AccountSecurity, AccountSecurity.account_id == FederatedIdentity.account_id) + .where( + FederatedIdentity.provider_id == provider_id, + AccountSecurity.managed_by_organization_id == organization_id, + FederatedIdentity.account_id.not_in( + select(ScimUser.account_id).where(ScimUser.organization_id == organization_id) + ), + ) + .limit(2) + ) + for claimed in (FederatedIdentity.claimed_username, FederatedIdentity.claimed_email): + matches = session.scalars(candidates.where(claimed == user_name)).all() + if matches: + return matches[0] if len(matches) == 1 else None + return None + + +def _new_scim_user(organization_id: str, account_id: str, changes: dict, now_ts: int) -> ScimUser: + return ScimUser( + organization_id=organization_id, + account_id=account_id, + user_name=changes["user_name"], + external_id=changes["external_id"], + email=changes["email"], + display_name=changes["display_name"], + created_at=now_ts, + updated_at=now_ts, + ) + + +def _adopt_account( + session: Session, organization_id: str, account_id: str, changes: dict, now_ts: int +) -> None: + """Provision an existing single sign-on account instead of creating one.""" + session.add(_new_scim_user(organization_id, account_id, changes, now_ts)) + session.flush() + if changes["active"]: + _activate(session, organization_id, account_id, now_ts) + else: + _ensure_member(session, organization_id, account_id) + _deactivate(session, organization_id, account_id, now_ts) + + def _apply_user_changes(session: Session, scim_user: ScimUser, changes: dict, now_ts: int) -> None: organization_id = scim_user.organization_id user_name = changes.get("user_name") @@ -467,7 +537,7 @@ def _apply_user_changes(session: Session, scim_user: ScimUser, changes: dict, no setattr(scim_user, column, changes[column]) if "active" in changes: if changes["active"]: - _activate(session, organization_id, scim_user.account_id) + _activate(session, organization_id, scim_user.account_id, now_ts) else: _deactivate(session, organization_id, scim_user.account_id, now_ts) scim_user.updated_at = now_ts @@ -536,13 +606,24 @@ def _load_group(session: Session, organization_id: str, group_id: str) -> tuple[ def _require_provisioned(session: Session, organization_id: str, account_ids: list[str]) -> None: + """Group members must be provisioned users who are active members of the organization.""" if not account_ids: return found = set( session.scalars( - select(ScimUser.account_id).where( + select(ScimUser.account_id) + .join( + OrganizationMember, + and_( + OrganizationMember.organization_id == ScimUser.organization_id, + OrganizationMember.account_id == ScimUser.account_id, + ), + ) + .outerjoin(AccountSecurity, AccountSecurity.account_id == ScimUser.account_id) + .where( ScimUser.organization_id == organization_id, ScimUser.account_id.in_(account_ids), + or_(AccountSecurity.status.is_(None), AccountSecurity.status != "deactivated"), ) ) ) @@ -774,11 +855,25 @@ def create_user( raise ScimError(400, "userName is required", "invalidValue") user_name = changes["user_name"] now_ts = get_clock(request)() - # A concurrent request may win the userName (409) or the derived - # account username (retried with a fresh one). + + def created(account_id: str): + resource = _user_json(request, _load_user(session, organization_id, account_id)) + return scim_response(resource, 201, headers={"Location": resource["meta"]["location"]}) + + # A concurrent request may win the userName (409), the adopted account + # (re-checked), or the derived account username (retried with a fresh one). for _ in range(3): if _user_name_taken(session, organization_id, user_name): raise _duplicate_user_name() + adopted_id = _adoptable_account(session, organization_id, user_name) + if adopted_id is not None: + try: + _adopt_account(session, organization_id, adopted_id, changes, now_ts) + session.commit() + except IntegrityError: + session.rollback() + continue + return created(adopted_id) account = Account( id=str(uuid.uuid4()), username=derive_username(session, user_name), @@ -798,18 +893,7 @@ def create_user( managed_by_organization_id=organization_id, ) ) - session.add( - ScimUser( - organization_id=organization_id, - account_id=account.id, - user_name=user_name, - external_id=changes["external_id"], - email=changes["email"], - display_name=changes["display_name"], - created_at=now_ts, - updated_at=now_ts, - ) - ) + session.add(_new_scim_user(organization_id, account.id, changes, now_ts)) session.add( OrganizationMember( organization_id=organization_id, @@ -822,8 +906,7 @@ def create_user( except IntegrityError: session.rollback() continue - resource = _user_json(request, _load_user(session, organization_id, account.id)) - return scim_response(resource, 201, headers={"Location": resource["meta"]["location"]}) + return created(account.id) if _user_name_taken(session, organization_id, user_name): raise _duplicate_user_name() raise ScimError(409, "could not allocate an account for this user", "uniqueness") @@ -897,7 +980,9 @@ def delete_user( session: Session = Depends(get_session), ): scim_user = _load_user(session, organization_id, user_id)[0] - _guard_removal(session, organization_id, user_id) + # The account keeps its single sign-on link, so a deprovisioned user who + # can still authenticate at the provider reaches this deactivated account + # (403) instead of a new one. _deactivate(session, organization_id, user_id, get_clock(request)()) _remove_memberships(session, organization_id, user_id) session.delete(scim_user) diff --git a/backend/geolibre_server_api/tests/test_enterprise_policy.py b/backend/geolibre_server_api/tests/test_enterprise_policy.py index 58c54a9242..1be5eb3055 100644 --- a/backend/geolibre_server_api/tests/test_enterprise_policy.py +++ b/backend/geolibre_server_api/tests/test_enterprise_policy.py @@ -180,18 +180,30 @@ def test_strictest_policy_wins_across_organizations(oauth_client): assert "at least 14 characters" in response.json()["error"] -def test_sqlite_schema_upgrade_adds_authenticated_at(oauth_client, tmp_path, clock): +def test_sqlite_schema_upgrade_adds_legacy_columns(oauth_client, tmp_path, clock): ensure_account(oauth_client) engine = oauth_client.app.state.engine with engine.begin() as connection: - for table in ("oauth_authorization_codes", "oauth_sessions"): - connection.execute(text(f"ALTER TABLE {table} DROP COLUMN authenticated_at")) + connection.execute(text("DROP INDEX ix_federated_identities_claimed_username")) + connection.execute(text("DROP INDEX ix_federated_identities_claimed_email")) + for table, column in ( + ("oauth_authorization_codes", "authenticated_at"), + ("oauth_sessions", "authenticated_at"), + ("federated_identities", "claimed_username"), + ("federated_identities", "claimed_email"), + ): + connection.execute(text(f"ALTER TABLE {table} DROP COLUMN {column}")) upgraded = _make_app(tmp_path, PUBLIC_URL, clock=clock.now) try: - for table in ("oauth_authorization_codes", "oauth_sessions"): + for table, expected in ( + ("oauth_authorization_codes", "authenticated_at"), + ("oauth_sessions", "authenticated_at"), + ("federated_identities", "claimed_username"), + ("federated_identities", "claimed_email"), + ): columns = {c["name"] for c in inspect(upgraded.state.engine).get_columns(table)} - assert "authenticated_at" in columns + assert expected in columns with TestClient(upgraded, base_url=PUBLIC_URL) as client: assert sign_in(client)["access_token"] finally: diff --git a/backend/geolibre_server_api/tests/test_scim.py b/backend/geolibre_server_api/tests/test_scim.py index ec875e6408..36a3c99fd1 100644 --- a/backend/geolibre_server_api/tests/test_scim.py +++ b/backend/geolibre_server_api/tests/test_scim.py @@ -10,6 +10,7 @@ from geolibre_server_api import auth as server_auth from geolibre_server_api.auth_models import Account from geolibre_server_api.enterprise_models import ScimUser +from geolibre_server_api.org_models import OrganizationMember from geolibre_server_api.policy import deactivate_account from helpers import ( add_member, @@ -107,6 +108,21 @@ def test_scim_token_admin_routes(oauth_client): assert denied.status_code == 403 +def test_scim_token_stops_working_when_creator_loses_admin(oauth_client): + token, org_id, scim_token = _scim_org(oauth_client) + ensure_account(oauth_client, "otheradmin") + add_member(oauth_client, token, org_id, "otheradmin", role="administrator") + removed = oauth_client.put( + f"/api/organizations/{org_id}/members", + json={"username": "ada", "role": "member"}, + headers=auth(token), + ) + assert removed.status_code == 200 + response = _scim(oauth_client, scim_token, "GET", org_id, "/Users") + assert response.status_code == 401 + assert response.json()["detail"] == "invalid SCIM token" + + def test_provisioned_user_links_to_sso_and_deactivation_revokes_everything(oauth_client, fake_idp): _, org_id, scim_token = _scim_org(oauth_client) created = _scim( @@ -177,6 +193,140 @@ def test_provisioned_user_links_to_sso_and_deactivation_revokes_everything(oauth assert _status(oauth_client, personal_token) == 401 +def test_scim_post_adopts_existing_managed_sso_account(oauth_client, fake_idp): + _, org_id, scim_token = _scim_org(oauth_client) + claims = fake_idp.base_claims( + "existing-sso-sub", + preferred_username="existing@example.org", + email="existing@example.org", + email_verified=True, + ) + tokens = _sso_tokens(oauth_client, *sso_sign_in(oauth_client, fake_idp, "acme", claims)) + account_id = oauth_client.get("/api/users/me", headers=auth(tokens["access_token"])).json()[ + "user" + ]["id"] + provisioned = _scim( + oauth_client, + scim_token, + "POST", + org_id, + "/Users", + {"userName": "existing@example.org"}, + ) + assert provisioned.status_code == 201, provisioned.text + assert provisioned.json()["id"] == account_id + deactivated = _scim( + oauth_client, + scim_token, + "PATCH", + org_id, + f"/Users/{account_id}", + _patch({"op": "replace", "path": "active", "value": False}), + ) + assert deactivated.status_code == 200 + assert _status(oauth_client, tokens["access_token"]) == 401 + + +def test_scim_email_link_requires_verified_email_and_never_double_links(oauth_client, fake_idp): + _, org_id, scim_token = _scim_org(oauth_client) + provisioned = _create_user(oauth_client, scim_token, org_id, "mail@example.org") + unverified = fake_idp.base_claims( + "unverified-email-sub", + preferred_username="different@example.org", + email="mail@example.org", + email_verified=False, + ) + first = _sso_tokens(oauth_client, *sso_sign_in(oauth_client, fake_idp, "acme", unverified)) + first_id = oauth_client.get("/api/users/me", headers=auth(first["access_token"])).json()[ + "user" + ]["id"] + assert first_id != provisioned["id"] + + verified = fake_idp.base_claims( + "first-link-sub", + preferred_username="mail@example.org", + email="mail@example.org", + email_verified=True, + ) + linked = _sso_tokens(oauth_client, *sso_sign_in(oauth_client, fake_idp, "acme", verified)) + linked_id = oauth_client.get("/api/users/me", headers=auth(linked["access_token"])).json()[ + "user" + ]["id"] + assert linked_id == provisioned["id"] + second_subject = {**verified, "sub": "second-link-sub"} + separate = _sso_tokens( + oauth_client, *sso_sign_in(oauth_client, fake_idp, "acme", second_subject) + ) + separate_id = oauth_client.get("/api/users/me", headers=auth(separate["access_token"])).json()[ + "user" + ]["id"] + assert separate_id not in {provisioned["id"], first_id} + + +def test_deleted_sso_user_can_be_reprovisioned_without_resurrecting_credentials( + oauth_client, fake_idp +): + _, org_id, scim_token = _scim_org(oauth_client) + claims = fake_idp.base_claims("reprovision-sub", preferred_username="reprovision@example.org") + old_tokens = _sso_tokens(oauth_client, *sso_sign_in(oauth_client, fake_idp, "acme", claims)) + old_id = oauth_client.get("/api/users/me", headers=auth(old_tokens["access_token"])).json()[ + "user" + ]["id"] + created = _scim( + oauth_client, + scim_token, + "POST", + org_id, + "/Users", + {"userName": "reprovision@example.org"}, + ) + assert created.status_code == 201 + assert created.json()["id"] == old_id + deleted = _scim(oauth_client, scim_token, "DELETE", org_id, f"/Users/{old_id}") + assert deleted.status_code == 204 + assert _status(oauth_client, old_tokens["access_token"]) == 401 + blocked, _ = sso_sign_in(oauth_client, fake_idp, "acme", claims) + assert blocked.status_code == 403 + with oauth_client.app.state.session_factory() as session: + assert session.get(OrganizationMember, (org_id, old_id)) is None + reprovisioned = _scim( + oauth_client, + scim_token, + "POST", + org_id, + "/Users", + {"userName": "reprovision@example.org"}, + ) + assert reprovisioned.status_code == 201, reprovisioned.text + assert reprovisioned.json()["id"] == old_id + fresh_callback, verifier = sso_sign_in(oauth_client, fake_idp, "acme", claims) + fresh = _sso_tokens(oauth_client, fresh_callback, verifier) + assert _status(oauth_client, fresh["access_token"]) == 200 + + +def test_group_add_rejects_scim_user_removed_from_organization(oauth_client): + token, org_id, scim_token = _scim_org(oauth_client) + user = _create_user(oauth_client, scim_token, org_id, "former@example.org") + group = _scim(oauth_client, scim_token, "POST", org_id, "/Groups", {"displayName": "Team"}) + assert group.status_code == 201 + with oauth_client.app.state.session_factory() as session: + account_name = session.get(Account, user["id"]).username + removed = oauth_client.delete( + f"/api/organizations/{org_id}/members/{account_name}", headers=auth(token) + ) + assert removed.status_code == 204 + rejected = _scim( + oauth_client, + scim_token, + "PATCH", + org_id, + f"/Groups/{group.json()['id']}", + _patch({"op": "add", "path": "members", "value": [{"value": user["id"]}]}), + ) + assert rejected.status_code == 400 + assert rejected.json()["detail"] == "member is not provisioned in this organization" + + def test_deactivating_an_unmanaged_account_only_removes_its_memberships(oauth_client): token, org_id, scim_token = _scim_org(oauth_client) ensure_account(oauth_client, "bob") @@ -431,12 +581,24 @@ def test_group_membership_provisioning(oauth_client): assert gone.status_code == 404 -def test_last_administrator_cannot_be_removed(oauth_client): +def test_last_administrator_cannot_be_removed(oauth_client, fake_idp): token, org_id, scim_token = _scim_org(oauth_client) admin = _create_user(oauth_client, scim_token, org_id, "root@example.org") with oauth_client.app.state.session_factory() as session: username = session.get(Account, admin["id"]).username add_member(oauth_client, token, org_id, username, role="administrator") + claims = fake_idp.base_claims("root-sub", preferred_username="root@example.org") + owner_token = _sso_tokens( + oauth_client, + *sso_sign_in(oauth_client, fake_idp, "acme", claims, scope="read:projects write:projects"), + )["access_token"] + minted = oauth_client.post( + f"/api/organizations/{org_id}/scim-tokens", + json={"label": "Root"}, + headers=auth(owner_token), + ) + assert minted.status_code == 201 + scim_token = minted.json()["token"] # ada steps down, leaving the provisioned account as the only administrator. stepped_down = oauth_client.put( f"/api/organizations/{org_id}/members", @@ -445,6 +607,19 @@ def test_last_administrator_cannot_be_removed(oauth_client): ) assert stepped_down.status_code == 200, stepped_down.text + refused_deactivation = _scim( + oauth_client, + scim_token, + "PATCH", + org_id, + f"/Users/{admin['id']}", + _patch({"op": "replace", "path": "active", "value": False}), + ) + assert refused_deactivation.status_code == 409 + assert refused_deactivation.json()["scimType"] == "mutability" + assert ( + refused_deactivation.json()["detail"] == "cannot remove the last organization administrator" + ) refused = _scim(oauth_client, scim_token, "DELETE", org_id, f"/Users/{admin['id']}") assert refused.status_code == 409 assert refused.json()["scimType"] == "mutability" @@ -472,6 +647,20 @@ def test_break_glass_administrator_cannot_be_removed(oauth_client): configured = configure_idp(oauth_client, token, org_id, breakGlassUsername=username) assert configured.status_code == 200, configured.text + refused_deactivation = _scim( + oauth_client, + scim_token, + "PATCH", + org_id, + f"/Users/{admin['id']}", + _patch({"op": "replace", "path": "active", "value": False}), + ) + assert refused_deactivation.status_code == 409 + assert ( + refused_deactivation.json()["detail"] + == "cannot remove the organization's break-glass administrator" + ) + refused = _scim(oauth_client, scim_token, "DELETE", org_id, f"/Users/{admin['id']}") assert refused.status_code == 409 assert refused.json()["scimType"] == "mutability" diff --git a/docs/server-api.md b/docs/server-api.md index 578ac6935e..4b7746cd25 100644 --- a/docs/server-api.md +++ b/docs/server-api.md @@ -716,10 +716,11 @@ re-authentication for `POST` and `DELETE`) and respond with Configure the identity provider with `baseUrl` as the tenant/SCIM URL and the token as a Bearer secret. Every SCIM request needs `Authorization: Bearer ` for that organization; a missing, revoked, or -other organization's token is `401` with `WWW-Authenticate: Bearer`. SCIM -requests are machine-to-machine, so the organization's administrator IP -allowlist and re-authentication window do not apply to them; revoke a token to -cut off a provider. +other organization's token is `401` with `WWW-Authenticate: Bearer`. Tokens +also stop working when their creator is deactivated or ceases to be an active +organization administrator. SCIM requests are machine-to-machine, so the +organization's administrator IP allowlist and re-authentication window do not +apply to them; revoke a token to cut off a provider. Request bodies are `application/scim+json` (or `application/json`); responses are `application/scim+json`. Errors use the SCIM error message: @@ -729,7 +730,7 @@ are `application/scim+json`. Errors use the SCIM error message: | --- | --- | | `GET /ServiceProviderConfig`, `/ResourceTypes`, `/Schemas` | Discovery. PATCH and filtering are supported (up to 100 results); bulk, sort, ETags, and password changes are not. | | `GET /Users` | `startIndex` (default 1) and `count` (0–100, default 100), ordered by creation. `filter` only `userName eq "…"` (case-insensitive) or `externalId eq "…"`; anything else is `400 invalidFilter`. | -| `POST /Users` | Creates an account. `userName` (1–255 characters, stored lowercased) is required and unique per organization (`409 uniqueness`). | +| `POST /Users` | Creates an account, or adopts the existing SSO account when this organization's enabled identity provider has a unique matching lowercased username claim or verified email claim. Password accounts, proxy identities, accounts managed by another organization, and ambiguous matches are not adopted. `userName` (1–255 characters, stored lowercased) is required and unique per organization (`409 uniqueness`). | | `GET`/`PUT`/`PATCH`/`DELETE /Users/{id}` | `id` is the account id. `PUT` replaces `userName`, `externalId`, `displayName`, `emails`, and (when present) `active`. | | `GET /Groups` | `filter` only `displayName eq "…"` (case-insensitive) or `externalId eq "…"`. | | `POST /Groups` | Creates an organization group owned by the token's creator, with `join_policy` `invite`. Every member must be a user provisioned in this organization (`400 invalidValue`). | @@ -746,6 +747,22 @@ members; the owner and managers are never listed or changed by SCIM. Only users and groups created through SCIM are visible to it (`404 resource not found` otherwise). +SCIM identity claims are refreshed on every OIDC sign-in. An email claim is +eligible for matching only when the provider asserts `email_verified: true`. +When a previously signed-in account has no SCIM resource and a matching SCIM +resource belongs to an unused SCIM-created account, the next sign-in reconciles +the SCIM resource onto the real account, transfers only member-role entries in +this organization's SCIM groups (without duplicates), then deactivates the +orphan and removes its organization membership and group-member rows. The +orphan account record is retained. A deactivated SCIM resource remains +deactivated after reconciliation. + +Deleting a managed SCIM user deactivates the account and revokes its OAuth and +personal-token credentials, but retains its federated identity. Reprovisioning +the same username adopts that deactivated account and reactivates it without +restoring old credentials; an authorization code approved before deactivation +cannot be exchanged afterward. + **PATCH** bodies need the `urn:ietf:params:scim:api:messages:2.0:PatchOp` schema and an `Operations` list (else `400 invalidSyntax`). `op` is case-insensitive. For users, `add` and `replace` accept the paths `active`, From a7b33c2c92b1cb3409927852002b24843712a11b Mon Sep 17 00:00:00 2001 From: giswqs Date: Fri, 2 Oct 2026 20:48:45 -0400 Subject: [PATCH 3/3] Address review feedback - Make the legacy-token policy backfill non-committing during deactivation so the status flip and revocations stay in the caller's transaction (CodeRabbit). - Keep the orphan account's membership when it is the last administrator or the break-glass account during SSO reconciliation (Claude review). - Add a test for reconciling an orphan SCIM user and its group memberships onto an existing SSO account (Claude review). --- .../geolibre_server_api/auth.py | 27 ++++++++++++--- .../geolibre_server_api/oidc.py | 7 ++++ .../geolibre_server_api/policy.py | 3 +- .../geolibre_server_api/tests/test_scim.py | 33 ++++++++++++++++++- 4 files changed, 63 insertions(+), 7 deletions(-) diff --git a/backend/geolibre_server_api/geolibre_server_api/auth.py b/backend/geolibre_server_api/geolibre_server_api/auth.py index 3cd7fe891a..bed1dfd55b 100644 --- a/backend/geolibre_server_api/geolibre_server_api/auth.py +++ b/backend/geolibre_server_api/geolibre_server_api/auth.py @@ -457,8 +457,17 @@ def touch_policy(session: Session, token_digest: str, now_ts: int) -> None: session.commit() -def backfill_policy(session: Session, digest: str) -> PersonalTokenPolicy: - """Create legacy PAT metadata without racing another request doing the same.""" +def backfill_policy(session: Session, digest: str, *, commit: bool = True) -> PersonalTokenPolicy: + """Create legacy PAT metadata without racing another request doing the same. + + Args: + session: The request session. + digest: Digest of the legacy personal token. + commit: Commit afterwards; pass False when the caller owns the transaction. + + Returns: + The new or already-existing policy row. + """ policy = PersonalTokenPolicy( id=str(uuid.uuid4()), token_digest=digest, @@ -479,7 +488,8 @@ def backfill_policy(session: Session, digest: str) -> PersonalTokenPolicy: if existing is None: raise return existing - session.commit() + if commit: + session.commit() return policy @@ -1190,14 +1200,21 @@ def owned_project_session( return project -def backfill_account_policies(session: Session, account_id: str) -> None: +def backfill_account_policies(session: Session, account_id: str, *, commit: bool = True) -> None: + """Give every legacy token of the account a policy row. + + Args: + session: The request session. + account_id: Account whose legacy tokens are backfilled. + commit: Commit per row; pass False when the caller owns the transaction. + """ missing = session.scalars( select(Token.digest) .outerjoin(PersonalTokenPolicy, PersonalTokenPolicy.token_digest == Token.digest) .where(Token.account_id == account_id, PersonalTokenPolicy.id.is_(None)) ).all() for digest in missing: - backfill_policy(session, digest) + backfill_policy(session, digest, commit=commit) def _validate_pat_lifetime(days: int | None) -> None: diff --git a/backend/geolibre_server_api/geolibre_server_api/oidc.py b/backend/geolibre_server_api/geolibre_server_api/oidc.py index b4920ac134..72ceafdfdd 100644 --- a/backend/geolibre_server_api/geolibre_server_api/oidc.py +++ b/backend/geolibre_server_api/geolibre_server_api/oidc.py @@ -596,6 +596,13 @@ def _adopt_orphan( else: deactivate_account(session, account_id, now_ts) + if provider.break_glass_account_id == orphan_id or is_last_active_admin( + session, organization_id, orphan_id + ): + # Removing it would leave the organization without an administrator, the + # removal SCIM refuses with a 409; the orphan keeps its membership. + logger.warning("scim orphan kept: it is the last administrator") + return deactivate_account(session, orphan_id, now_ts) session.execute( delete(OrganizationMember).where( diff --git a/backend/geolibre_server_api/geolibre_server_api/policy.py b/backend/geolibre_server_api/geolibre_server_api/policy.py index 61bd72d3ed..8f934fe803 100644 --- a/backend/geolibre_server_api/geolibre_server_api/policy.py +++ b/backend/geolibre_server_api/geolibre_server_api/policy.py @@ -268,7 +268,8 @@ def _revoke_credentials(session: Session, account_id: str, now_ts: int) -> None: .values(revoked_at=now_ts) ) # Legacy tokens get a policy row first so the revocation below covers them. - backfill_account_policies(session, account_id) + # The caller owns the transaction, so the backfill must not commit. + backfill_account_policies(session, account_id, commit=False) session.execute( update(PersonalTokenPolicy) .where( diff --git a/backend/geolibre_server_api/tests/test_scim.py b/backend/geolibre_server_api/tests/test_scim.py index 36a3c99fd1..e2a4c4d034 100644 --- a/backend/geolibre_server_api/tests/test_scim.py +++ b/backend/geolibre_server_api/tests/test_scim.py @@ -10,7 +10,7 @@ from geolibre_server_api import auth as server_auth from geolibre_server_api.auth_models import Account from geolibre_server_api.enterprise_models import ScimUser -from geolibre_server_api.org_models import OrganizationMember +from geolibre_server_api.org_models import GroupMember, OrganizationMember from geolibre_server_api.policy import deactivate_account from helpers import ( add_member, @@ -263,6 +263,37 @@ def test_scim_email_link_requires_verified_email_and_never_double_links(oauth_cl assert separate_id not in {provisioned["id"], first_id} +def test_sign_in_adopts_orphan_scim_user_and_group_memberships(oauth_client, fake_idp): + _, org_id, scim_token = _scim_org(oauth_client) + claims = fake_idp.base_claims("adopter-sub", preferred_username="adopter@example.org") + tokens = _sso_tokens(oauth_client, *sso_sign_in(oauth_client, fake_idp, "acme", claims)) + real_id = oauth_client.get("/api/users/me", headers=auth(tokens["access_token"])).json()[ + "user" + ]["id"] + orphan = _create_user(oauth_client, scim_token, org_id, "orphan@example.org") + group = _scim( + oauth_client, + scim_token, + "POST", + org_id, + "/Groups", + {"displayName": "GIS", "members": [{"value": orphan["id"]}]}, + ) + assert group.status_code == 201, group.text + assert orphan["id"] != real_id + + moved = {**claims, "preferred_username": "orphan@example.org"} + _sso_tokens(oauth_client, *sso_sign_in(oauth_client, fake_idp, "acme", moved)) + with oauth_client.app.state.session_factory() as session: + scim_user = session.scalar( + select(ScimUser).where(ScimUser.user_name == "orphan@example.org") + ) + assert scim_user.account_id == real_id + assert session.get(GroupMember, (group.json()["id"], real_id)) is not None + assert session.get(GroupMember, (group.json()["id"], orphan["id"])) is None + assert session.get(OrganizationMember, (org_id, orphan["id"])) is None + + def test_deleted_sso_user_can_be_reprovisioned_without_resurrecting_credentials( oauth_client, fake_idp ):