hataa 0032579017
feat(authz): admin role-assignment API for configured custom roles (#4063 gap 2) (#6382)
* feat(authz): admin role-assignment API for configured custom roles (#4063 gap 2)

The RBAC provider accepts arbitrary configured role names (guest, ...),
but nothing could assign them to real users: User.system_role was pinned
to Literal["admin", "user"] at the API layer while the DB column stayed
a plain String(16) for exactly this future, and zhfeng's #3462 audit
listed the missing assignment surface as gap 2 — custom roles were
declarable in YAML but unreachable for authenticated web users.

- User/UserResponse.system_role widens to a non-empty string; whether a
  role is *assignable* is the admin API's contract, not the model's.
- RbacAuthorizationProvider exposes known_roles; the Gateway's new
  assignable_role_names() unions the built-ins with the provider's
  configured role names (custom providers: built-ins only).
- New admin-only router /api/v1/admin/users: GET lists users with their
  role; PATCH assigns — role validated against assignable names (an
  unknown role would silently fall through to default_role on every
  decision), 404 for missing users, and the last remaining admin cannot
  be demoted (bootstrap lockout protection). Enforcement is unchanged:
  the assignment only changes which role the principal carries; the
  @require_permission route guards and runtime provider decisions remain
  the single enforcement points.
- SQLite repository gains list_users (ordered oldest-first).

Tests: router guard/422/404/409/no-op/happy-path, last-admin invariant
(both directions), assignable names union, principal carries the
configured role; the two old Literal-rejection contract tests updated to
the new contract (custom role representable; empty string still
rejected).

* fix(auth): widen the frontend user schema for custom roles (review-lessons catch)

The role-assignment API's review-lessons sweep found the one consumer
class the implementation had not enumerated: the FRONTEND parses /me
with a strict userSchema whose system_role was z.enum(["admin",
"user"]) — a user assigned a provider-configured custom role (guest)
would fail userSchema.parse in server.ts and break the auth flow before
rendering anything.

- types.ts: system_role widens to z.string().min(1), kept in lockstep
  with the backend model (min_length=1). All existing consumers compare
  === "admin", so any other value is simply non-admin — no behavior
  change for admin/user.
- server.test.ts: contract tests — custom role parses verbatim; empty
  string still rejected.
- config.example.yaml: the authorization comment now says custom roles
  become reachable via PATCH /api/v1/admin/users/{id} and that a
  declared-but-unassigned role grants nothing.

* fix(auth): commit the admin_users router, drop the stale type-ignore, document the surface

Review catch on #6382: the router module was never staged — app.py and
the tests import it, so the pushed branch could not boot (ImportError in
create_app). The local tree kept it as an untracked file, which is why
every check passed before the push.

- Actually commit app/gateway/routers/admin_users.py.
- _row_to_user: drop the now-stale '# type: ignore[arg-type]' on
  system_role — the model field is a plain string now (keep the comma).
- README: the authorization paragraph now mentions the admin
  role-assignment surface (GET/PATCH /api/v1/admin/users, admin-only,
  last-admin demotion protected, declared-but-unassigned grants nothing).
- server.test.ts import-order lint fix from the frontend sweep.

* fix(auth): field-scoped role writes close the demotion-undo race

Review round 2 on #6382 — three findings, one design:

- [P1] update_user copied system_role from the caller's account snapshot,
  so a password change that read the user as admin could restore a role
  concurrently revoked by the demotion route (stale-snapshot write-back).
  Account writes are now field-scoped against each other: update_user is
  the credential/profile writer (never touches the role, mirrors the
  row's current role back), update_system_role is the only role writer
  (single-column, never touches credentials or token_version).
- The last-admin lockout check moved inside the role write's serialized
  transaction: count-then-write across two sessions let two concurrent
  demotions of the only two admins both pass. Same idiom as
  create_first_admin — BEGIN IMMEDIATE on SQLite, pg_advisory_xact_lock
  on a distinct key on Postgres, raise on unknown dialects.
- UserNotFoundError from the racing-delete window maps to the route's
  404 instead of a 500.

Tests 11->12: real-repo regression covering the P1 interleaving
(demotion lands -> stale credential snapshot writes -> role stays
revoked, credentials applied), field isolation both directions, and the
last-admin invariant raised from inside the write with the row intact.
Mutation-verified: restoring the role write-back in update_user fails
it.

* style(auth): format the server auth tests

* fix(tests): migrate the personal-MCP demote revocation to update_system_role

The role-write field-scoping (66e2cec23) made update_user a
credential/profile-only writer, so the demote branch of
test_revocation_blocks_discovery... silently no-oped and CI failed on
all four shards (assert 2 == 1 — the role never changed).

- The demote now goes through update_system_role, the sole role writer.
- The fixture gains a second admin: the revocation test demotes its sole
  synthetic admin, which the new last-admin lockout guard correctly
  blocks — a second admin keeps the scenario legal without weakening
  either invariant.

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
2026-10-07 14:59:16 +08:00

281 lines
12 KiB
Python

"""Built-in RBAC authorization provider.
Reads a role→resource policy from config and compiles it into immutable
structures at construction time. Deny always wins over allow. Unknown or
missing roles raise ``ValueError`` (not a silent allow) so that the execution
layer's ``fail_closed`` can make the final decision.
See ``docs/plans/2026-07-15-authz-phase1a-implementation-plan.md`` §3.3 for
the full semantic table.
"""
from __future__ import annotations
from typing import Any
from deerflow.authz.provider import (
AuthzDecision,
AuthzReason,
AuthzRequest,
Principal,
)
# Explicit resource-type → config-key mapping. Prevents silent mis-lookup
# when ``AuthzRequest.resource`` (singular, e.g. "tool") doesn't match the
# config key (plural, e.g. "tools").
_RESOURCE_POLICY_KEYS: dict[str, str] = {
"tool": "tools",
"model": "models",
"skill": "skills",
"sandbox": "sandbox",
"mcp_server": "mcp_servers",
"route": "routes",
# Plugin resources. The left side is the request ``resource`` and the right
# side is the ``config.yaml`` key. ``plugin_management`` is genuinely
# self-mapped (like ``sandbox``), so a config key of the same name is legal;
# ``plugin_action`` is not, and using it as a config key is rejected at
# construction. A resource with no policy key is unrestricted, which is the
# documented non-breaking path for deployments that enable authorization
# without listing these keys.
"plugin_action": "plugin_actions",
"plugin_management": "plugin_management",
}
_ALL = object() # sentinel meaning "allow all candidates"
_ABSENT = object() # sentinel meaning "key not present in dict"
# The only supported keys in a resource policy dict. Any other key (typos,
# unknown fields) is rejected at construction to prevent silent mis-grants.
_SUPPORTED_POLICY_KEYS: frozenset[str] = frozenset({"allow", "deny"})
def _require_non_empty_string(value: object, *, field: str) -> str:
"""Return a validated request identifier or raise a stable boundary error."""
if not isinstance(value, str) or not value:
raise ValueError(f"{field} must be a non-empty string, got {value!r}")
return value
class _CompiledPolicy:
"""Immutable, pre-validated policy for a single (role, resource_type) pair."""
__slots__ = ("allowed", "denied")
def __init__(self, *, allowed: frozenset[str] | object, denied: frozenset[str]):
self.allowed = allowed
self.denied = denied
def is_allowed(self, target: str) -> bool:
# Deny always wins.
if target in self.denied:
return False
if self.allowed is _ALL:
return True
return target in self.allowed
class RbacAuthorizationProvider:
"""Built-in role-based authorization provider.
Configured via ``roles`` mapping where each role maps resource-type keys
to ``{allow: ..., deny: [...]}`` policies. Policy configuration is fully
validated at construction; the request path validates identifiers before
performing membership checks.
Policies are scoped by role, resource, and target. ``AuthzRequest.action``
is accepted for protocol compatibility but is not a rule dimension in this
built-in provider.
Example config::
roles:
admin:
tools: {allow: "*"}
user:
tools: {allow: "*", deny: ["update_agent"]}
guest:
tools: {allow: ["web_search", "read_file"]}
"""
name = "rbac"
def __init__(self, *, roles: dict[str, Any] | object = _ABSENT, **kwargs: Any) -> None:
if kwargs:
raise ValueError(f"unknown provider config keys {sorted(kwargs, key=repr)}; supported: ['roles']")
if roles is _ABSENT:
raise ValueError("missing required provider config key 'roles'")
if not isinstance(roles, dict):
raise ValueError(f"roles must be a dict, got {type(roles).__name__}")
# Compile all policies up front.
self._policies: dict[tuple[str, str], _CompiledPolicy] = {}
self._known_roles: frozenset[str] = frozenset(roles.keys())
for role_name, role_config in roles.items():
if not isinstance(role_name, str) or not role_name:
raise ValueError(f"role name must be a non-empty string, got {role_name!r}")
if not isinstance(role_config, dict):
raise ValueError(f"role '{role_name}' config must be a dict, got {type(role_config).__name__}")
for resource_key, resource_policy in role_config.items():
if not isinstance(resource_key, str) or not resource_key:
raise ValueError(f"role '{role_name}' has invalid resource key {resource_key!r}")
mapped_resource_key = _RESOURCE_POLICY_KEYS.get(resource_key)
if mapped_resource_key is not None and mapped_resource_key != resource_key:
raise ValueError(f"role '{role_name}' resource key '{resource_key}' is a reserved request alias; use '{mapped_resource_key}' in RBAC config")
if not isinstance(resource_policy, dict):
raise ValueError(f"role '{role_name}' resource '{resource_key}' must be a dict, got {type(resource_policy).__name__}")
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)
if role not in self._known_roles:
if field == "role":
raise ValueError(f"Unknown role '{role}'; known roles: {sorted(self._known_roles)}")
raise ValueError(f"{field} '{role}' is not defined; known roles: {sorted(self._known_roles)}")
@staticmethod
def _compile_resource_policy(
role_name: str,
resource_key: str,
policy: dict[str, Any],
) -> _CompiledPolicy:
"""Validate and compile a single resource policy into immutable structures.
Distinguishes "key absent" (use default) from "key present but null"
(invalid — reject). Unknown keys (typos) are rejected to prevent
silent mis-grants.
"""
# --- reject unknown keys (catch typos like "alow") ---
unknown_keys = set(policy.keys()) - _SUPPORTED_POLICY_KEYS
if unknown_keys:
raise ValueError(f"role '{role_name}' resource '{resource_key}': unknown policy keys {sorted(unknown_keys, key=repr)}; supported: {sorted(_SUPPORTED_POLICY_KEYS)}")
# --- allow ---
raw_allow = policy.get("allow", _ABSENT)
if raw_allow is _ABSENT:
allowed: frozenset[str] | object = _ALL # missing allow = allow all (deny still applies)
elif raw_allow is None:
raise ValueError(f"role '{role_name}' resource '{resource_key}': allow must not be null; omit the key, or use '*' / bool / list")
elif raw_allow is True:
allowed = _ALL
elif raw_allow is False:
allowed = frozenset() # allow: false = deny all
elif isinstance(raw_allow, str):
if raw_allow == "*":
allowed = _ALL
else:
raise ValueError(f"role '{role_name}' resource '{resource_key}': allow string must be '*', got {raw_allow!r}")
elif isinstance(raw_allow, (list, tuple)):
for item in raw_allow:
if not isinstance(item, str) or not item:
raise ValueError(f"role '{role_name}' resource '{resource_key}': allow list contains non-string or empty item {item!r}")
allowed = frozenset(raw_allow)
else:
raise ValueError(f"role '{role_name}' resource '{resource_key}': allow must be '*', bool, or list of strings, got {type(raw_allow).__name__}")
# --- deny ---
raw_deny = policy.get("deny", _ABSENT)
if raw_deny is _ABSENT or raw_deny is None:
if raw_deny is None:
raise ValueError(f"role '{role_name}' resource '{resource_key}': deny must not be null; omit the key for no deny list")
denied: frozenset[str] = frozenset()
elif isinstance(raw_deny, (list, tuple)):
for item in raw_deny:
if not isinstance(item, str) or not item:
raise ValueError(f"role '{role_name}' resource '{resource_key}': deny list contains non-string or empty item {item!r}")
denied = frozenset(raw_deny)
else:
raise ValueError(f"role '{role_name}' resource '{resource_key}': deny must be a list of strings, got {type(raw_deny).__name__}")
return _CompiledPolicy(allowed=allowed, denied=denied)
def _resolve_policy(
self,
principal: Principal,
resource: str,
*,
resource_field: str = "resource",
) -> _CompiledPolicy | None:
"""Look up the compiled policy for (role, resource_type).
Returns ``None`` if no policy is configured for this role+resource
(meaning: unrestricted). Raises ``ValueError`` for invalid resource
identifiers and unknown or missing roles.
"""
role = principal.role
if role is None or role == "":
raise ValueError("Principal has no role; cannot evaluate RBAC policy")
self.validate_role(role)
resource = _require_non_empty_string(resource, field=resource_field)
resource_key = _RESOURCE_POLICY_KEYS.get(resource, resource)
return self._policies.get((role, resource_key))
def authorize(self, request: AuthzRequest) -> AuthzDecision:
"""Evaluate a single authorization request."""
policy = self._resolve_policy(request.principal, request.resource)
target = _require_non_empty_string(request.target, field="target")
if policy is None:
# No policy for this role+resource → unrestricted.
return AuthzDecision(
allow=True,
reasons=[AuthzReason(code="authz.no_policy", message="no policy configured")],
policy_id="rbac:unrestricted",
)
if policy.is_allowed(target):
return AuthzDecision(
allow=True,
reasons=[AuthzReason(code="authz.allowed")],
policy_id="rbac:allow",
)
return AuthzDecision(
allow=False,
reasons=[
AuthzReason(
code="authz.denied",
message=f"role '{request.principal.role}' is denied '{target}' on resource '{request.resource}'",
)
],
policy_id="rbac:deny",
)
async def aauthorize(self, request: AuthzRequest) -> AuthzDecision:
return self.authorize(request)
def filter_resources(
self,
principal: Principal,
resource_type: str,
candidates: list[str],
) -> list[str]:
"""Batch visibility filter.
Preserves candidate order and duplicates, never adds items, and raises
the same role/resource errors as :meth:`authorize`.
"""
policy = self._resolve_policy(principal, resource_type, resource_field="resource_type")
if not isinstance(candidates, list):
raise ValueError(f"candidates must be a list, got {type(candidates).__name__}")
validated_candidates = [_require_non_empty_string(candidate, field=f"candidates[{index}]") for index, candidate in enumerate(candidates)]
if policy is None:
return validated_candidates
return [candidate for candidate in validated_candidates if policy.is_allowed(candidate)]