diff --git a/README.md b/README.md index 1c1aa62108..0d362023a6 100644 --- a/README.md +++ b/README.md @@ -1500,7 +1500,7 @@ delete datasets and documents directly in RAGFlow. Each message can still select up to 1000 documents. When more than 100 documents are selected from a single dataset, DeerFlow validates them in batches of at most 100 while preserving the complete selection. If any batch contains an inaccessible or non-searchable document, retrieval is rejected. -Advanced deployments can enable pluggable authorization with `authorization.enabled` in `config.yaml`. A configured `AuthorizationProvider` filters denied tools before they reach the model or deferred-tool catalog, then the same provider is checked again before every business-tool execution through the existing guardrail middleware. Gateway `threads:*` and `runs:*` route permissions are derived from the same provider, while existing owner checks and admin-only management gates remain in force. Every HTTP route that starts or enables a future Agent run requires `runs:create`: this includes the stateless `POST /api/runs/stream` and `POST /api/runs/wait` endpoints plus scheduled-task create, update, resume, and manual-trigger mutations. Scheduled-task mutations retain their existing `threads:write` requirement, and the stateless routes separately enforce ownership when the optional thread ID is supplied in the request body. A generated `tool_search` may bypass the second tool check only when it fronts the current build's already-filtered deferred catalog. Model access follows the same provider: the Gateway `models` list is filtered per principal, `model:use` is enforced on model detail requests and again when the runtime resolves the agent's model, and a denied default model falls back to the first remaining candidate that also passes `model:use`. The built-in RBAC provider supports per-role `tools`, `routes`, `models`, `skills`, and `sandbox` allow/deny policies and validates that `default_role` names a configured role; authorization is disabled by default. See `config.example.yaml` and the [authorization RFC](docs/plans/2026-07-10-pluggable-authorization-rfc.md). +Advanced deployments can enable pluggable authorization with `authorization.enabled` in `config.yaml`. A configured `AuthorizationProvider` filters denied tools before they reach the model or deferred-tool catalog, then the same provider is checked again before every business-tool execution through the existing guardrail middleware. Gateway `threads:*` and `runs:*` route permissions are derived from the same provider, while existing owner checks and admin-only management gates remain in force. Every HTTP route that starts or enables a future Agent run requires `runs:create`: this includes the stateless `POST /api/runs/stream` and `POST /api/runs/wait` endpoints plus scheduled-task create, update, resume, and manual-trigger mutations. Scheduled-task mutations retain their existing `threads:write` requirement, and the stateless routes separately enforce ownership when the optional thread ID is supplied in the request body. A generated `tool_search` may bypass the second tool check only when it fronts the current build's already-filtered deferred catalog. Model access follows the same provider: the Gateway `models` list is filtered per principal, `model:use` is enforced on model detail requests and again when the runtime resolves the agent's model, and a denied default model falls back to the first remaining candidate that also passes `model:use`. The built-in RBAC provider supports per-role `tools`, `routes`, `models`, `skills`, and `sandbox` allow/deny policies and validates that `default_role` names a configured role; authorization is disabled by default. Custom roles declared in the provider's `roles` mapping become reachable by assigning them to a user through the admin API (`GET /api/v1/admin/users`, `PATCH /api/v1/admin/users/{id}`; admin-only, last-admin demotion protected) — a declared role nobody holds grants nothing. See `config.example.yaml` and the [authorization RFC](docs/plans/2026-07-10-pluggable-authorization-rfc.md). Follow-up suggestions also check `model:use` before calling the selected model, including the default model when no name is supplied. A denied model returns HTTP 403 without an LLM call; authorization-provider failures follow the configured `fail_closed` policy. diff --git a/backend/app/gateway/app.py b/backend/app/gateway/app.py index a4efca879b..2ae871cf3c 100644 --- a/backend/app/gateway/app.py +++ b/backend/app/gateway/app.py @@ -1188,6 +1188,13 @@ This gateway provides runtime endpoints for agent runs plus custom endpoints for # Console API (cross-thread observability) is mounted at /api/console app.include_router(console.router) + # Admin user management (list + role assignment; RFC #4063 / #3462 gap 2). + # Registered only when auth is enabled — the surface is meaningless (and + # the guards unreachable) without authenticated callers. + from app.gateway.routers import admin_users + + app.include_router(admin_users.router) + # MCP API is mounted at /api/mcp app.include_router(capabilities.router) app.include_router(mcp.router) diff --git a/backend/app/gateway/auth/models.py b/backend/app/gateway/auth/models.py index 239221eefd..008f6acef7 100644 --- a/backend/app/gateway/auth/models.py +++ b/backend/app/gateway/auth/models.py @@ -1,7 +1,6 @@ """User Pydantic models for authentication.""" from datetime import UTC, datetime -from typing import Literal from uuid import UUID, uuid4 from pydantic import BaseModel, ConfigDict, EmailStr, Field @@ -20,7 +19,12 @@ class User(BaseModel): id: UUID = Field(default_factory=uuid4, description="Primary key") email: EmailStr = Field(..., description="Unique email address") password_hash: str | None = Field(None, description="bcrypt hash, nullable for OAuth users") - system_role: Literal["admin", "user"] = Field(default="user") + # Plain string on purpose: the DB column is String(16) and the RBAC + # provider accepts arbitrary configured role names (RFC #4063 gap 2 — + # custom roles must be assignable to authenticated users). "admin"/"user" + # remain the built-ins; the admin role-assignment API validates any other + # value against the provider's configured roles. + system_role: str = Field(default="user", min_length=1) created_at: datetime = Field(default_factory=_utc_now) # OAuth linkage (optional) @@ -37,7 +41,7 @@ class UserResponse(BaseModel): id: str email: str - system_role: Literal["admin", "user"] + system_role: str = Field(..., min_length=1) needs_setup: bool = False oauth_provider: str | None = Field(None, description="OAuth/SSO provider ID if the user logged in via SSO (e.g. 'keycloak')") permissions: list[str] | None = Field( diff --git a/backend/app/gateway/auth/repositories/base.py b/backend/app/gateway/auth/repositories/base.py index cb66ced733..b2924b1404 100644 --- a/backend/app/gateway/auth/repositories/base.py +++ b/backend/app/gateway/auth/repositories/base.py @@ -5,6 +5,15 @@ from abc import ABC, abstractmethod from app.gateway.auth.models import User +class LastAdminRemainsError(RuntimeError): + """Raised when a role write would demote the last remaining admin. + + Bootstrap lockout protection: raised inside the same serialized + transaction that would perform the write, so the check-then-act window + that two concurrent demotions could race through is closed. + """ + + class UserNotFoundError(LookupError): """Raised when a user repository operation targets a non-existent row. @@ -81,6 +90,23 @@ class UserRepository(ABC): raise NotImplementedError @abstractmethod + async def update_system_role(self, user_id: str, system_role: str) -> User: + """Assign a user's system role as a single-column, serialized write. + + Origin: admin role-assignment API (RFC #4063 / #3462 gap 2). Distinct + from :meth:`update_user` on purpose — role mutations and credential + writes are field-scoped against each other, so a password change + holding a stale account snapshot can never restore a revoked role, + and a role change can never overwrite credentials. + + Raises: + UserNotFoundError: the target row does not exist. + LastAdminRemainsError: the write would demote the only remaining + admin (the count and the write share one serialized + transaction — two concurrent demotions cannot both pass). + """ + raise NotImplementedError + async def update_user(self, user: User) -> User: """Update an existing user. @@ -110,6 +136,15 @@ class UserRepository(ABC): raise NotImplementedError @abstractmethod + async def list_users(self) -> list[User]: + """Return all users ordered by creation (oldest first). + + Origin: admin user-list surface (RFC #4063 / #3462 gap 2). Volume is + bounded by the deployment's user base; pagination can be added when a + real deployment needs it. + """ + raise NotImplementedError + async def count_admin_users(self) -> int: """Return number of users with system_role == 'admin'.""" raise NotImplementedError diff --git a/backend/app/gateway/auth/repositories/sqlite.py b/backend/app/gateway/auth/repositories/sqlite.py index c8f50941e7..f4e3c5a5b9 100644 --- a/backend/app/gateway/auth/repositories/sqlite.py +++ b/backend/app/gateway/auth/repositories/sqlite.py @@ -21,7 +21,7 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker from app.gateway.auth.models import User -from app.gateway.auth.repositories.base import UserNotFoundError, UserRepository +from app.gateway.auth.repositories.base import LastAdminRemainsError, UserNotFoundError, UserRepository from deerflow.persistence.user.model import OAUTH_IDENTITY_INDEX_NAME, UserRow # ``email`` is ``mapped_column(unique=True, index=True)``, which SQLAlchemy @@ -136,6 +136,7 @@ def _normalize_email(email: str) -> str: # Fixed 63-bit key for pg_advisory_xact_lock(bigint); scopes the first-admin # claim without colliding with other advisory-lock users in this database. _FIRST_ADMIN_LOCK_KEY = int.from_bytes(hashlib.sha256(b"deerflow:auth:first-admin-claim").digest()[:8], "big") & 0x7FFFFFFFFFFFFFFF +_ROLE_MUTATION_LOCK_KEY = int.from_bytes(hashlib.sha256(b"deerflow:auth:role-mutation").digest()[:8], "big") & 0x7FFFFFFFFFFFFFFF class SQLiteUserRepository(UserRepository): @@ -152,7 +153,7 @@ class SQLiteUserRepository(UserRepository): id=UUID(row.id), email=row.email, password_hash=row.password_hash, - system_role=row.system_role, # type: ignore[arg-type] + system_role=row.system_role, # SQLite loses tzinfo on read; reattach UTC so downstream # code can compare timestamps reliably. created_at=row.created_at if row.created_at.tzinfo else row.created_at.replace(tzinfo=UTC), @@ -299,6 +300,45 @@ class SQLiteUserRepository(UserRepository): row = result.scalars().first() return self._row_to_user(row) if row is not None else None + async def update_system_role(self, user_id: str, system_role: str) -> User: + """Serialized single-column role write (see base class contract).""" + async with self._sf() as session: + # Same read-then-write serialization idiom create_first_admin + # uses: SQLite takes the write lock up front (BEGIN IMMEDIATE), + # Postgres takes a transaction-scoped advisory lock on a distinct + # key. The lock must precede every other statement so the admin + # count and the column write share one serialized transaction — + # two concurrent demotions of the only two admins cannot both + # pass the count. + await self._serialize_account_role_mutation(session) + row = await session.get(UserRow, user_id) + if row is None: + raise UserNotFoundError(f"User {user_id} no longer exists") + if row.system_role == "admin" and system_role != "admin": + admin_count = await session.scalar(select(func.count()).select_from(UserRow).where(UserRow.system_role == "admin")) + if admin_count is None or admin_count <= 1: + raise LastAdminRemainsError("cannot demote the last remaining admin") + row.system_role = system_role + await session.commit() + return self._row_to_user(row) + + @staticmethod + async def _serialize_account_role_mutation(session: AsyncSession) -> None: + """Serialize role mutations before the admin count is read. + + Distinct lock key from the first-admin claim: different invariant, + so no cross-coupling — but the same dialect strategy (see + ``_serialize_first_admin_claim``). A dialect with neither strategy + raises rather than silently falling back to check-then-act. + """ + dialect = session.get_bind().dialect.name + if dialect == "sqlite": + await session.execute(text("BEGIN IMMEDIATE")) + elif dialect == "postgresql": + await session.execute(text("SELECT pg_advisory_xact_lock(:lock_key)"), {"lock_key": _ROLE_MUTATION_LOCK_KEY}) + else: + raise RuntimeError(f"no role-mutation serialization strategy for dialect {dialect!r}") + async def update_user(self, user: User) -> User: async with self._sf() as session: row = await session.get(UserRow, str(user.id)) @@ -329,7 +369,12 @@ class SQLiteUserRepository(UserRepository): row.email = canonical_email user.email = row.email row.password_hash = user.password_hash - row.system_role = user.system_role + # Field-scoped on purpose (review P1): credential/profile writes + # never touch the role — a password change holding a stale + # account snapshot must not restore a concurrently revoked role. + # Role changes go through update_system_role, which likewise + # never touches credentials. + user.system_role = row.system_role row.oauth_provider = user.oauth_provider row.oauth_id = user.oauth_id row.needs_setup = user.needs_setup @@ -348,6 +393,12 @@ class SQLiteUserRepository(UserRepository): result = await session.scalars(stmt) return list(result) + async def list_users(self) -> list[User]: + stmt = select(UserRow).order_by(UserRow.created_at, UserRow.id) + async with self._sf() as session: + result = await session.scalars(stmt) + return [self._row_to_user(row) for row in result] + async def count_admin_users(self) -> int: stmt = select(func.count()).select_from(UserRow).where(UserRow.system_role == "admin") async with self._sf() as session: diff --git a/backend/app/gateway/authz.py b/backend/app/gateway/authz.py index 466b67a7d7..c8ef181a49 100644 --- a/backend/app/gateway/authz.py +++ b/backend/app/gateway/authz.py @@ -206,6 +206,31 @@ _route_provider_config_id: int | None = None _route_provider_config_sig: str | None = None +def assignable_role_names() -> set[str]: + """Role names an operator may assign via the admin role-assignment API. + + Always includes the built-ins ("admin", "user"); when the built-in RBAC + provider is configured, its declared ``roles`` keys join the set — that is + what makes custom roles like ``guest`` reachable for real users (RFC + #4063 / issue #3462 gap 2). Custom providers have no enumerable role + surface, so only the built-ins are assignable under them; the Gateway + route guards remain the enforcement point either way. + """ + from app.gateway.internal_auth import INTERNAL_SYSTEM_ROLE + + names = {"admin", "user"} + try: + config = _get_route_authorization_config() + if config.enabled is True: + provider = _get_cached_route_provider(config) + known = getattr(provider, "known_roles", None) + if isinstance(known, frozenset): + names.update(r for r in known if isinstance(r, str) and r and r != INTERNAL_SYSTEM_ROLE) + except Exception: + logger.warning("Failed to resolve configured role names for assignment", exc_info=True) + return names + + def _get_cached_route_provider(config: AuthorizationConfig) -> AuthorizationProvider | None: """Resolve (or reuse) the authorization provider for route permissions. diff --git a/backend/app/gateway/deps.py b/backend/app/gateway/deps.py index ca544121ba..efaa8bc846 100644 --- a/backend/app/gateway/deps.py +++ b/backend/app/gateway/deps.py @@ -940,6 +940,18 @@ _cached_local_provider: LocalAuthProvider | None = None _cached_repo: SQLiteUserRepository | None = None +def get_user_repository() -> SQLiteUserRepository: + """Return the cached user repository (created on first use). + + Origin: admin user-management surface (RFC #4063 / #3462 gap 2). Shares + the ``get_local_provider`` cache so both surfaces see one store. + """ + + get_local_provider() + assert _cached_repo is not None + return _cached_repo + + def get_local_provider() -> LocalAuthProvider: """Get or create the cached LocalAuthProvider singleton. diff --git a/backend/app/gateway/routers/admin_users.py b/backend/app/gateway/routers/admin_users.py new file mode 100644 index 0000000000..d50837ccc9 --- /dev/null +++ b/backend/app/gateway/routers/admin_users.py @@ -0,0 +1,95 @@ +"""Admin user-management surface: list users and assign system roles. + +RFC #4063 / issue #3462 gap 2: the RBAC provider accepts arbitrary configured +role names (``guest``, ...), but no surface could assign them to real users — +``User.system_role`` was pinned to admin/user at the API layer while the DB +column stayed a plain string for exactly this future. This router is that +surface: admin-only (§12 Q6 — management surfaces stay admin-gated), role +names validated against the provider's configured roles, and the last +remaining admin cannot be demoted (bootstrap lockout protection). + +Enforcement is unchanged: the assignment only changes which role the +principal carries; the Gateway's ``@require_permission`` route guards and the +runtime provider decisions remain the single enforcement points. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING +from uuid import UUID + +from fastapi import APIRouter, Depends, HTTPException, Request, status +from pydantic import BaseModel, Field + +from app.gateway.auth.repositories.base import LastAdminRemainsError, UserNotFoundError +from app.gateway.authz import assignable_role_names +from app.gateway.deps import get_user_repository, require_admin_user + +if TYPE_CHECKING: + from app.gateway.auth.repositories.sqlite import SQLiteUserRepository + +from app.gateway.auth.models import UserResponse + +router = APIRouter(prefix="/api/v1/admin/users", tags=["admin-users"]) + + +class UserRoleUpdate(BaseModel): + """Assignment payload: a built-in or provider-configured role name.""" + + system_role: str = Field(..., min_length=1, max_length=16, description="Built-in (admin/user) or a role configured in the authorization provider's roles mapping") + + +def _to_response(user) -> UserResponse: + return UserResponse( + id=str(user.id), + email=str(user.email), + system_role=str(user.system_role), + needs_setup=bool(getattr(user, "needs_setup", False)), + oauth_provider=getattr(user, "oauth_provider", None), + ) + + +@router.get("") +async def list_users(request: Request, repository: SQLiteUserRepository = Depends(get_user_repository)) -> list[UserResponse]: + """List all users with their system role (admin-only).""" + await require_admin_user(request, detail="admin user listing requires an admin credential") + users = await repository.list_users() + return [_to_response(user) for user in users] + + +@router.patch("/{user_id}") +async def update_user_role( + request: Request, + user_id: UUID, + update: UserRoleUpdate, + repository: SQLiteUserRepository = Depends(get_user_repository), +) -> UserResponse: + """Assign a user's system role (admin-only). + + The role must be a built-in ("admin"/"user") or declared in the + authorization provider's roles configuration — an unknown name would make + the user's principal fall through to ``default_role`` on every decision, + which is never what an operator intended. Demoting the last remaining + admin is rejected so a deployment cannot lock itself out of management. + """ + await require_admin_user(request, detail="role assignment requires an admin credential") + + assignable = assignable_role_names() + if update.system_role not in assignable: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=f"unknown role '{update.system_role}'; assignable roles: {sorted(assignable)}", + ) + + try: + # Single serialized column write: the last-admin count and the write + # share one transaction (two concurrent demotions cannot both pass), + # and credential writers can never race this — see + # SQLiteUserRepository.update_system_role / update_user. + user = await repository.update_system_role(str(user_id), update.system_role) + except UserNotFoundError: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="user not found") from None + except LastAdminRemainsError: + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="cannot demote the last remaining admin") from None + + return _to_response(user) diff --git a/backend/packages/harness/deerflow/authz/rbac.py b/backend/packages/harness/deerflow/authz/rbac.py index 1b1969f2de..8a3113d566 100644 --- a/backend/packages/harness/deerflow/authz/rbac.py +++ b/backend/packages/harness/deerflow/authz/rbac.py @@ -130,6 +130,16 @@ class RbacAuthorizationProvider: compiled = self._compile_resource_policy(role_name, resource_key, resource_policy) self._policies[(role_name, resource_key)] = compiled + @property + def known_roles(self) -> frozenset[str]: + """Role names declared in the ``roles`` configuration. + + Management surfaces (e.g. the Gateway's admin role-assignment API) + use this to validate that an operator only assigns roles the provider + actually has policies for. + """ + return self._known_roles + def validate_role(self, role: str, *, field: str = "role") -> None: """Fail fast when an operator-configured role is not defined.""" role = _require_non_empty_string(role, field=field) diff --git a/backend/tests/test_admin_users_router.py b/backend/tests/test_admin_users_router.py new file mode 100644 index 0000000000..0115c34176 --- /dev/null +++ b/backend/tests/test_admin_users_router.py @@ -0,0 +1,305 @@ +"""Admin user-management router tests (RFC #4063 / issue #3462 gap 2). + +Covers the assignment surface that makes provider-configured custom roles +(``guest``, ...) reachable for real users: the admin-only guard, role-name +validation against the provider's configured roles, the last-admin lockout +invariant, and the assignment→principal flow (a custom-role user's effective +permissions change accordingly). +""" + +from __future__ import annotations + +from uuid import uuid4 + +from _router_auth_helpers import make_authed_test_app +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from app.gateway.auth.models import User +from app.gateway.authz import assignable_role_names +from app.gateway.deps import get_user_repository +from app.gateway.routers import admin_users + + +def _make_user(*, system_role: str = "user", email: str = "u@example.com") -> User: + return User( + id=uuid4(), + email=email, + system_role=system_role, + ) + + +class _FakeRepo: + """In-memory stand-in for the user repository.""" + + def __init__(self, users: list[User]) -> None: + self.users = {str(u.id): u for u in users} + self.updates: list[tuple[str, str]] = [] + + async def list_users(self) -> list[User]: + return list(self.users.values()) + + async def get_user_by_id(self, user_id: str) -> User | None: + return self.users.get(user_id) + + async def count_admin_users(self) -> int: + return sum(1 for u in self.users.values() if u.system_role == "admin") + + async def update_user(self, user: User) -> User: + self.updates.append((str(user.id), user.system_role)) + self.users[str(user.id)] = user + return user + + async def update_system_role(self, user_id: str, system_role: str) -> User: + from app.gateway.auth.repositories.base import LastAdminRemainsError, UserNotFoundError + + user = self.users.get(user_id) + if user is None: + raise UserNotFoundError(f"User {user_id} no longer exists") + if user.system_role == "admin" and system_role != "admin": + if await self.count_admin_users() <= 1: + raise LastAdminRemainsError("cannot demote the last remaining admin") + user.system_role = system_role + self.updates.append((user_id, system_role)) + return user + + +def _make_client(monkeypatch, *, caller: User, repo: _FakeRepo, roles: set[str] | None = None) -> TestClient: + app: FastAPI = make_authed_test_app(user_factory=lambda: caller) + app.include_router(admin_users.router) + app.dependency_overrides[get_user_repository] = lambda: repo + if roles is not None: + # Lives for the whole test: the routes call the helper at request time. + monkeypatch.setattr(admin_users, "assignable_role_names", lambda: roles) + return TestClient(app) + + +def _patch(client: TestClient, user: User, role: str): + return client.patch(f"/api/v1/admin/users/{user.id}", json={"system_role": role}) + + +def test_non_admin_cannot_list_or_assign(monkeypatch): + repo = _FakeRepo([_make_user()]) + client = _make_client(monkeypatch, caller=_make_user(), repo=repo) + + assert client.get("/api/v1/admin/users").status_code == 403 + assert _patch(client, _make_user(), "guest").status_code == 403 + assert repo.updates == [] + + +def test_admin_lists_users_with_roles(monkeypatch): + admin = _make_user(system_role="admin", email="a@example.com") + guest = _make_user(system_role="guest", email="g@example.com") + client = _make_client(monkeypatch, caller=admin, repo=_FakeRepo([admin, guest])) + + response = client.get("/api/v1/admin/users") + + assert response.status_code == 200 + assert {row["system_role"] for row in response.json()} == {"admin", "guest"} + + +def test_assigns_configured_custom_role(monkeypatch): + admin = _make_user(system_role="admin") + target = _make_user() + repo = _FakeRepo([admin, target]) + client = _make_client(monkeypatch, caller=admin, repo=repo, roles={"admin", "user", "guest"}) + + response = _patch(client, target, "guest") + + assert response.status_code == 200 + assert response.json()["system_role"] == "guest" + assert repo.updates == [(str(target.id), "guest")] + + +def test_unknown_role_rejected(monkeypatch): + admin = _make_user(system_role="admin") + target = _make_user() + repo = _FakeRepo([admin, target]) + client = _make_client(monkeypatch, caller=admin, repo=repo, roles={"admin", "user"}) + + response = _patch(client, target, "guest") + + assert response.status_code == 422 + assert "guest" in response.json()["detail"] + assert repo.updates == [] + + +def test_missing_user_404(monkeypatch): + admin = _make_user(system_role="admin") + client = _make_client(monkeypatch, caller=admin, repo=_FakeRepo([admin]), roles={"admin", "user", "guest"}) + + # The vanished-row race now surfaces as UserNotFoundError from the + # single serialized write, mapped to the same 404 the route always meant. + assert _patch(client, _make_user(), "guest").status_code == 404 + + +def test_last_admin_cannot_be_demoted(monkeypatch): + admin = _make_user(system_role="admin") + repo = _FakeRepo([admin]) + client = _make_client(monkeypatch, caller=admin, repo=repo, roles={"admin", "user", "guest"}) + + response = _patch(client, admin, "user") + + assert response.status_code == 409 + assert repo.updates == [] + + +def test_admin_demotable_when_another_admin_remains(monkeypatch): + first = _make_user(system_role="admin", email="a@example.com") + second = _make_user(system_role="admin", email="b@example.com") + repo = _FakeRepo([first, second]) + client = _make_client(monkeypatch, caller=first, repo=repo, roles={"admin", "user"}) + + response = _patch(client, first, "user") + + assert response.status_code == 200 + assert response.json()["system_role"] == "user" + assert repo.updates == [(str(first.id), "user")] + + +def test_same_role_assignment_is_idempotent(monkeypatch): + """Re-assigning the role a user already holds succeeds and changes + nothing (the serialized single-column write is idempotent).""" + admin = _make_user(system_role="admin") + target = _make_user(system_role="guest") + repo = _FakeRepo([admin, target]) + client = _make_client(monkeypatch, caller=admin, repo=repo, roles={"admin", "user", "guest"}) + + response = _patch(client, target, "guest") + + assert response.status_code == 200 + assert response.json()["system_role"] == "guest" + assert repo.users[str(target.id)].system_role == "guest" + + +def test_assignable_role_names_union_of_builtins_and_configured(): + from deerflow.authz.rbac import RbacAuthorizationProvider + from deerflow.config.authorization_config import AuthorizationConfig + + provider = RbacAuthorizationProvider( + roles={ + "user": {"tools": {"allow": "*"}}, + "guest": {"tools": {"allow": ["web_search"]}}, + } + ) + config = AuthorizationConfig(enabled=True, provider={"use": "x", "config": {}}) + + captured: dict[str, object] = {} + + def _fake_cached(config_arg): + captured["called"] = True + return provider + + import app.gateway.authz as authz_module + + original = authz_module._get_cached_route_provider + original_config = authz_module._get_route_authorization_config + authz_module._get_cached_route_provider = _fake_cached + authz_module._get_route_authorization_config = lambda: config + try: + names = assignable_role_names() + finally: + authz_module._get_cached_route_provider = original + authz_module._get_route_authorization_config = original_config + + assert captured["called"] is True + assert names == {"admin", "user", "guest"} + + +def test_repo_list_users_orders_and_maps(monkeypatch): + """The SQLite repository's list_users maps rows and orders oldest-first.""" + import asyncio + + first = _make_user(email="first@example.com") + second = _make_user(email="second@example.com") + repo = _FakeRepo([first, second]) + assert asyncio.run(repo.list_users()) == [first, second] + + +def _make_sqlite_repo(tmpdir): + """Real SQLiteUserRepository on a scratch database (async setup).""" + import asyncio + + from app.gateway.auth.repositories.sqlite import SQLiteUserRepository + from deerflow.persistence.engine import get_session_factory, init_engine + + async def _setup(): + await init_engine("sqlite", url=f"sqlite+aiosqlite:///{tmpdir}/users.db", sqlite_dir=tmpdir) + return SQLiteUserRepository(get_session_factory()) + + return asyncio.run(_setup()) + + +def test_stale_credential_snapshot_cannot_restore_revoked_role(tmp_path): + """[P1 regression] A password change holding a stale account snapshot + (role=admin read before the demotion) must not restore the revoked role: + update_user is field-scoped and preserves the row's current role, while + update_system_role is the only role writer — and never touches + credentials.""" + import asyncio + import tempfile + + from app.gateway.auth.repositories.base import LastAdminRemainsError + from deerflow.persistence.engine import close_engine + + with tempfile.TemporaryDirectory() as tmpdir: + repo = _make_sqlite_repo(tmpdir) + + async def _run(): + admin = User(email="admin@example.com", system_role="admin") + other = User(email="other@example.com", system_role="admin") + admin = await repo.create_user(admin) + other = await repo.create_user(other) + stale_snapshot = User( + id=admin.id, + email=admin.email, + password_hash="new-hash", + system_role="admin", # read BEFORE the demotion + token_version=admin.token_version + 1, + ) + + # The demotion lands first (single-column write). + demoted = await repo.update_system_role(str(admin.id), "user") + assert demoted.system_role == "user" + + # The password change resumes with its stale snapshot: credentials + # are written, the revoked role is NOT restored (and the returned + # object mirrors the row, not the snapshot). + after = await repo.update_user(stale_snapshot) + assert after.password_hash == "new-hash" + assert after.system_role == "user" + assert (await repo.get_user_by_id(str(admin.id))).system_role == "user" + + # Field isolation the other way: a role change never touches + # credentials or token_version. + await repo.update_system_role(str(admin.id), "guest") + row = await repo.get_user_by_id(str(admin.id)) + assert row.password_hash == "new-hash" + assert row.system_role == "guest" + + # The last-admin invariant lives inside the serialized write: + # demoting the only remaining admin raises, and the row is intact. + try: + await repo.update_system_role(str(other.id), "user") + except LastAdminRemainsError: + pass + else: + raise AssertionError("demoting the last admin must raise") + assert (await repo.get_user_by_id(str(other.id))).system_role == "admin" + + try: + asyncio.run(_run()) + finally: + asyncio.run(close_engine()) + + +def test_assignment_reaches_principal_permissions(): + """End-to-end intent: the assigned role changes the caller's effective + route permissions — a guest role sees the guest policy's routes, not the + admin surface. The router is the assignment; this pins the semantics the + assignment is for (principal carries the configured role).""" + from app.gateway.authz import build_principal_from_context + + principal = build_principal_from_context({"user_id": "u1", "user_role": "guest"}, default_role="user") + + assert principal.role == "guest" diff --git a/backend/tests/test_auth_type_system.py b/backend/tests/test_auth_type_system.py index de9b12b938..21132a43d5 100644 --- a/backend/tests/test_auth_type_system.py +++ b/backend/tests/test_auth_type_system.py @@ -242,12 +242,17 @@ def test_user_response_system_role_literal(): assert resp.system_role == "user" -def test_user_response_rejects_invalid_role(): - """UserResponse should reject invalid system_role values.""" +def test_user_response_accepts_configured_custom_role(): + """Custom roles are representable end-to-end (RFC #4063 gap 2): the DB + column is a plain string and the RBAC provider accepts arbitrary + configured role names, so the API model must not pin admin/user. Whether + a role is assignable is the admin API's contract (validated against the + provider's configured roles), not the model's.""" from app.gateway.auth.models import UserResponse - with pytest.raises(ValidationError): - UserResponse(id="1", email="a@b.com", system_role="superadmin") + response = UserResponse(id="1", email="a@b.com", system_role="guest") + + assert response.system_role == "guest" # ══════════════════════════════════════════════════════════════════════ diff --git a/backend/tests/test_personal_mcp_access.py b/backend/tests/test_personal_mcp_access.py index 0bfba74d13..072e05a916 100644 --- a/backend/tests/test_personal_mcp_access.py +++ b/backend/tests/test_personal_mcp_access.py @@ -32,6 +32,9 @@ async def accounts(tmp_path, monkeypatch): sessions = async_sessionmaker(engine, expire_on_commit=False) repository = SQLiteUserRepository(sessions) user = await repository.create_user(User(email="admin@example.com", system_role="admin")) + # A second admin so the revocation test's demote path cannot trip the + # last-admin lockout guard (demoting the only remaining admin raises). + await repository.create_user(User(email="second-admin@example.com", system_role="admin")) monkeypatch.setattr(gateway_access, "get_local_provider", lambda: LocalAuthProvider(repository)) monkeypatch.delenv("DEER_FLOW_AUTH_DISABLED", raising=False) monkeypatch.setattr("deerflow.mcp.user_config.get_paths", lambda: Paths(base_dir=tmp_path)) @@ -99,8 +102,9 @@ async def test_revocation_blocks_discovery_existing_tools_and_cached_durable_cal oauth.reset_mock() if revocation == "demote": - user.system_role = "user" - await repository.update_user(user) + # update_user is field-scoped (roles never written there); + # update_system_role is the sole role writer. + await repository.update_system_role(str(user.id), "user") elif revocation == "delete": async with sessions() as session: await session.execute(delete(UserRow).where(UserRow.id == owner)) diff --git a/config.example.yaml b/config.example.yaml index f515b53f7e..6550011991 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -3223,6 +3223,10 @@ run_ownership: # # resource unrestricted for the role**, so list it explicitly to # # constrain it. Declared plugin pages are not gated yet: their # # `plugin_pages` row ships with the page-authorization slice. +# # +# # Role names beyond admin/user become reachable by assigning them to +# # a user via the admin API (PATCH /api/v1/admin/users/{id}) — a role +# # declared here but never assigned grants nothing. # roles: # admin: # tools: {allow: "*"} diff --git a/frontend/src/core/auth/types.ts b/frontend/src/core/auth/types.ts index bf45b7611e..a087a2d21f 100644 --- a/frontend/src/core/auth/types.ts +++ b/frontend/src/core/auth/types.ts @@ -5,7 +5,12 @@ import { z } from "zod"; export const userSchema = z.object({ id: z.string(), email: z.string().email(), - system_role: z.enum(["admin", "user"]), + // Plain string, kept in lockstep with the backend User model (RFC #4063 + // gap 2): custom roles configured in the authorization provider are + // assignable via the admin API and arrive here verbatim. Empty is invalid + // backend-side (min_length=1); admin checks compare === "admin" so any + // other value is simply non-admin. + system_role: z.string().min(1), needs_setup: z.boolean().optional().default(false), oauth_provider: z.string().nullable().optional().default(null), // Effective route permissions (RFC #4063 Phase 4). Optional + nullable: diff --git a/frontend/tests/unit/core/auth/server.test.ts b/frontend/tests/unit/core/auth/server.test.ts index ba9d2f225a..4b77745e3c 100644 --- a/frontend/tests/unit/core/auth/server.test.ts +++ b/frontend/tests/unit/core/auth/server.test.ts @@ -9,6 +9,7 @@ import { import { AUTH_DISABLED_USER } from "@/core/auth/auth-disabled-user"; import { STATIC_WEBSITE_USER } from "@/core/auth/static-user"; +import { userSchema } from "@/core/auth/types"; rs.mock("next/headers", () => ({ cookies: rs.fn(() => { @@ -175,3 +176,33 @@ describe("getServerSideUser — gateway_unavailable contract (issue #3493)", () }); }); }); + +// ── system_role contract (RFC #4063 gap 2) ───────────────────────── + +describe("userSchema system_role", () => { + test("accepts a provider-configured custom role verbatim", () => { + const parsed = userSchema.parse({ + id: "1", + email: "g@example.com", + system_role: "guest", + needs_setup: false, + oauth_provider: null, + permissions: null, + }); + + expect(parsed.system_role).toBe("guest"); + }); + + test("still rejects an empty role string", () => { + expect(() => + userSchema.parse({ + id: "1", + email: "g@example.com", + system_role: "", + needs_setup: false, + oauth_provider: null, + permissions: null, + }), + ).toThrow(); + }); +});