diff --git a/README.md b/README.md index 8c60cdff3f..8f88d25e43 100644 --- a/README.md +++ b/README.md @@ -2236,6 +2236,10 @@ Each message can still select up to 1000 documents. When more than 100 documents 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). +Administrators can revoke a disabled account's admin role while keeping it disabled, +or assign a non-admin role and re-enable it in the same account update. The last +enabled administrator remains protected against demotion and disabling. + 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. For vision-capable agents, `view_image` and the subsequent model-context image read also require `sandbox:execute`. Allowing the tool name alone does not grant image-file access; a role denied sandbox execution cannot reread previously recorded image metadata after its permissions change. External run input and thread-state updates cannot set `viewed_images` or `thread_data`; when a saved image is read from a host copy, its path must resolve to the current user's thread and the recorded virtual image path. diff --git a/backend/app/gateway/AGENTS.md b/backend/app/gateway/AGENTS.md index 1e0d26756a..e1199f7948 100644 --- a/backend/app/gateway/AGENTS.md +++ b/backend/app/gateway/AGENTS.md @@ -254,6 +254,11 @@ Cap edit reads at `MAX_EDITABLE_ARTIFACT_BYTES + 1` despite stale `lstat`. ### Route and skill authorization +Role writes serialize protection of the last **active** admin. Disabled admins may +be demoted before restoration; role-only writes preserve credentials and lifecycle +fields. Regressions: `tests/test_admin_users_router.py` (SQLite/HTTP) and +`tests/test_disabled_admin_demotion_auth.py` (login, JWT, CSRF, old/new sessions). + `authz.authorize_model_use` shares the `model:use` decision between model details and follow-up suggestions, including the factory's first configured model when the caller omits `model_name`. Suggestions check admission before their diff --git a/backend/app/gateway/auth/repositories/base.py b/backend/app/gateway/auth/repositories/base.py index c849971347..29f712103f 100644 --- a/backend/app/gateway/auth/repositories/base.py +++ b/backend/app/gateway/auth/repositories/base.py @@ -6,7 +6,7 @@ from app.gateway.auth.models import User class LastAdminRemainsError(RuntimeError): - """Raised when a role write would demote the last remaining admin. + """Raised when a role write would demote the last remaining active admin. Bootstrap lockout protection: raised inside the same serialized transaction that would perform the write, so the check-then-act window @@ -123,8 +123,9 @@ class UserRepository(ABC): 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 + active admin (the count and the write share one serialized transaction — two concurrent demotions cannot both pass). + An already disabled admin may be assigned a different role. """ raise NotImplementedError diff --git a/backend/app/gateway/auth/repositories/sqlite.py b/backend/app/gateway/auth/repositories/sqlite.py index f0f6e7640b..1551adf65c 100644 --- a/backend/app/gateway/auth/repositories/sqlite.py +++ b/backend/app/gateway/auth/repositories/sqlite.py @@ -316,11 +316,12 @@ class SQLiteUserRepository(UserRepository): 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": + if row.system_role == "admin" and not row.disabled and system_role != "admin": # Count ACTIVE admins only: a disabled admin cannot # authenticate, so counting them would let "disable B, then # demote self" (or a combined role+disable request) strand # the deployment with zero usable management credentials. + # Demoting an already disabled admin removes no active admin. active_admins = await session.scalar(select(func.count()).select_from(UserRow).where(UserRow.system_role == "admin", UserRow.disabled.is_(False))) if active_admins is None or active_admins <= 1: raise LastAdminRemainsError("cannot demote the last remaining admin") diff --git a/backend/tests/test_admin_users_router.py b/backend/tests/test_admin_users_router.py index 7b4c5d52e8..6c21f02bd5 100644 --- a/backend/tests/test_admin_users_router.py +++ b/backend/tests/test_admin_users_router.py @@ -11,6 +11,7 @@ from __future__ import annotations from uuid import uuid4 +import pytest from _router_auth_helpers import make_authed_test_app from fastapi import FastAPI from fastapi.testclient import TestClient @@ -69,7 +70,7 @@ class _FakeRepo: 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 user.system_role == "admin" and not user.disabled and system_role != "admin": # Active admins only (disabled admins cannot authenticate). active = sum(1 for u in self.users.values() if u.system_role == "admin" and not getattr(u, "disabled", False)) if active <= 1: @@ -564,6 +565,65 @@ def test_repo_set_disabled_field_scoped_and_last_active_admin(): asyncio.run(close_engine()) +@pytest.mark.asyncio +@pytest.mark.parametrize("role", ["user", "guest"]) +@pytest.mark.parametrize("reenable", [False, True], ids=["keep-disabled", "reenable"]) +async def test_disabled_admin_can_be_demoted_with_one_active_admin(monkeypatch, tmp_path, role, reenable): + """Exercise HTTP writes against SQL, including combined role/enable updates.""" + from httpx import ASGITransport, AsyncClient + + from app.gateway.auth.repositories.sqlite import SQLiteUserRepository + from deerflow.persistence.engine import close_engine, get_session_factory, init_engine + + await init_engine("sqlite", url=f"sqlite+aiosqlite:///{tmp_path}/users.db", sqlite_dir=str(tmp_path)) + try: + repo = SQLiteUserRepository(get_session_factory()) + admin = await repo.create_user(User(email="active@example.com", system_role="admin")) + target = await repo.create_user(User(email="suspended@example.com", system_role="admin", password_hash="saved-hash", token_version=7, oauth_provider="github", oauth_id="saved-oauth-id")) + app = make_authed_test_app(user_factory=lambda: admin) + app.include_router(admin_users.router) + app.dependency_overrides[get_user_repository] = lambda: repo + monkeypatch.setattr(admin_users, "assignable_role_names", lambda: {"admin", "user", "guest"}) + target_url = f"/api/v1/admin/users/{target.id}" + admin_url = f"/api/v1/admin/users/{admin.id}" + + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: + suspended = await client.patch(target_url, json={"disabled": True}) + assert suspended.status_code == 200 + before = await repo.get_user_by_id(str(target.id)) + assert before is not None and before.disabled + + # A disabled peer must never allow the sole active admin to demote. + blocked = await client.patch(admin_url, json={"system_role": role}) + assert blocked.status_code == 409 + + body = {"system_role": role} + if reenable: + body["disabled"] = False + demoted = await client.patch(target_url, json=body) + assert demoted.status_code == 200, demoted.text + assert demoted.json()["system_role"] == role + assert demoted.json()["disabled"] is (not reenable) + + stored = await repo.get_user_by_id(str(target.id)) + assert stored is not None + assert stored.model_dump() == before.model_copy(update={"system_role": role, "disabled": not reenable}).model_dump() + + if not reenable: + restored = await client.patch(target_url, json={"disabled": False}) + assert restored.status_code == 200 + assert restored.json()["system_role"] == role + assert restored.json()["disabled"] is False + + # Restoring the demoted account adds no administrator; both guards remain. + assert (await client.patch(admin_url, json={"system_role": role})).status_code == 409 + assert (await client.patch(admin_url, json={"disabled": True})).status_code == 409 + remaining = await repo.get_user_by_id(str(admin.id)) + assert remaining is not None and remaining.system_role == "admin" and not remaining.disabled + finally: + await close_engine() + + def test_disabled_account_rejected_at_login_and_flagged_for_resolvers(): """Password login never compares credentials for a disabled account, and lookups return the flag so the JWT resolver can reject.""" diff --git a/backend/tests/test_disabled_admin_demotion_auth.py b/backend/tests/test_disabled_admin_demotion_auth.py new file mode 100644 index 0000000000..f95d3786ef --- /dev/null +++ b/backend/tests/test_disabled_admin_demotion_auth.py @@ -0,0 +1,116 @@ +"""Disabled-admin demotion through production session and CSRF middleware.""" + +import asyncio +from contextlib import ExitStack + +import pytest +from fastapi.testclient import TestClient + + +@pytest.fixture +def auth_app(monkeypatch, tmp_path): + from app.gateway import deps + from app.gateway.auth import config as auth_config + from app.gateway.auth import login_throttle + from app.gateway.routers import auth + from deerflow.config.app_config import pop_current_app_config, push_current_app_config, reload_app_config, reset_app_config + from deerflow.config.extensions_config import reset_extensions_config + from deerflow.extensions import get_loaded_extensions, get_runtime_diagnostics, initialize_runtime_diagnostics, set_loaded_extensions + from deerflow.persistence.engine import close_engine, init_engine + + config_path = tmp_path / "config.yaml" + config_path.write_text("models: []\nsandbox:\n use: deerflow.sandbox.local:LocalSandboxProvider\n", encoding="utf-8") + extensions_path = tmp_path / "extensions_config.json" + extensions_path.write_text("{}\n", encoding="utf-8") + monkeypatch.setenv("DEER_FLOW_CONFIG_PATH", str(config_path)) + monkeypatch.setenv("DEER_FLOW_EXTENSIONS_CONFIG_PATH", str(extensions_path)) + monkeypatch.setenv("DEER_FLOW_HOME", str(tmp_path / "runtime")) + monkeypatch.setenv("DEER_FLOW_AUTH_DISABLED", "0") + monkeypatch.setattr(auth_config, "_auth_config", auth_config.AuthConfig(jwt_secret="disabled-admin-regression-secret-at-least-32")) + monkeypatch.setattr(deps, "_cached_repo", None) + monkeypatch.setattr(deps, "_cached_local_provider", None) + monkeypatch.setattr(login_throttle, "_store", None) + monkeypatch.setattr(auth, "_SETUP_STATUS_CACHE", {}) + monkeypatch.setattr(auth, "_SETUP_STATUS_INFLIGHT", {}) + with ExitStack() as cleanup: + # Environment selectors do not override pinned config or a run-scoped + # ContextVar. Isolate both, including create_app's import-time instance. + cleanup.callback(reset_app_config) + cleanup.callback(reset_extensions_config) + cleanup.callback(set_loaded_extensions, get_loaded_extensions()) + cleanup.callback(initialize_runtime_diagnostics, get_runtime_diagnostics()) + reset_app_config() + reset_extensions_config() + push_current_app_config(reload_app_config()) + cleanup.callback(pop_current_app_config) + cleanup.callback(lambda: asyncio.run(close_engine())) + asyncio.run(init_engine("sqlite", url=f"sqlite+aiosqlite:///{tmp_path}/users.db", sqlite_dir=str(tmp_path))) + + from app.gateway.app import create_app + + # As in the auth integration suite, initialize persistence explicitly; + # the Agent/sandbox lifespan is unrelated to these account operations. + yield create_app() + + +def _csrf(client): + token = client.cookies.get("csrf_token") + assert token + return {"X-CSRF-Token": token} + + +@pytest.mark.parametrize("reenable", [False, True], ids=["restore-later", "restore-in-same-patch"]) +def test_disabled_admin_demotion_preserves_session_authorization(auth_app, reenable): + admin = TestClient(auth_app) + target = TestClient(auth_app) + anonymous = TestClient(auth_app) + try: + first = admin.post("/api/v1/auth/initialize", json={"email": "admin@example.com", "password": "Adm1n!Pass99"}) + assert first.status_code == 201 + registered = target.post("/api/v1/auth/register", json={"email": "target@example.com", "password": "Us3r!Pass99"}) + assert registered.status_code == 201 + assert admin.cookies.get("access_token") and target.cookies.get("access_token") + admin_url = f"/api/v1/admin/users/{first.json()['id']}" + target_url = f"/api/v1/admin/users/{registered.json()['id']}" + + assert anonymous.get("/api/v1/admin/users").status_code == 401 + assert target.patch(target_url, json={"system_role": "admin"}, headers=_csrf(target)).status_code == 403 + assert admin.patch(target_url, json={"system_role": "admin"}, headers=_csrf(admin)).status_code == 200 + assert target.get("/api/v1/admin/users").status_code == 200 + assert admin.patch(target_url, json={"disabled": True}, headers=_csrf(admin)).status_code == 200 + + # Both the existing signed session and a fresh password login are refused. + assert target.get("/api/v1/auth/me").status_code == 401 + denied_login = target.post("/api/v1/auth/login/local", data={"username": "target@example.com", "password": "Us3r!Pass99"}) + assert denied_login.status_code == 401 + assert admin.patch(admin_url, json={"system_role": "user"}, headers=_csrf(admin)).status_code == 409 + + body = {"system_role": "user"} + if reenable: + body["disabled"] = False + assert admin.patch(target_url, json=body).status_code == 403 + changed = admin.patch(target_url, json=body, headers=_csrf(admin)) + assert changed.status_code == 200, changed.text + assert changed.json()["system_role"] == "user" + assert changed.json()["disabled"] is (not reenable) + if not reenable: + assert target.get("/api/v1/auth/me").status_code == 401 + restored = admin.patch(target_url, json={"disabled": False}, headers=_csrf(admin)) + assert restored.status_code == 200 + assert restored.json()["system_role"] == "user" + + # A pre-suspension admin cookie resolves the current database role. + assert target.get("/api/v1/auth/me").json()["system_role"] == "user" + assert target.get("/api/v1/admin/users").status_code == 403 + target.cookies.clear() + logged_in = target.post("/api/v1/auth/login/local", data={"username": "target@example.com", "password": "Us3r!Pass99"}) + assert logged_in.status_code == 200 + assert target.get("/api/v1/auth/me").json()["system_role"] == "user" + assert target.patch(target_url, json={"system_role": "admin"}, headers=_csrf(target)).status_code == 403 + assert admin.patch(admin_url, json={"system_role": "user"}, headers=_csrf(admin)).status_code == 409 + assert admin.patch(admin_url, json={"disabled": True}, headers=_csrf(admin)).status_code == 409 + assert admin.get("/api/v1/admin/users").status_code == 200 + finally: + anonymous.close() + target.close() + admin.close()