mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-11-07 19:20:34 +00:00
fix(auth): allow demoting disabled administrators (#7769)
This commit is contained in:
parent
bf9b031127
commit
98b72af732
@ -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.
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
|
||||
@ -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")
|
||||
|
||||
@ -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."""
|
||||
|
||||
116
backend/tests/test_disabled_admin_demotion_auth.py
Normal file
116
backend/tests/test_disabled_admin_demotion_auth.py
Normal file
@ -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()
|
||||
Loading…
x
Reference in New Issue
Block a user