From 0ca4867c1f7d3fd8cb2d0c887d1cb8c4c68de087 Mon Sep 17 00:00:00 2001 From: Shudipto Trafder Date: Sat, 7 Nov 2026 13:20:38 +0600 Subject: [PATCH 01/10] feat(auth): Implement RBAC and permissions registry with tests --- pyproject.toml | 1 + .../src/app/core/auth/permissions_registry.py | 51 ++ tenxgraph_api/src/app/core/auth/rbac.py | 203 ++++++++ .../routers/graph/services/graph_service.py | 7 +- tests/unit_tests/test_permissions_registry.py | 71 +++ tests/unit_tests/test_rbac.py | 455 ++++++++++++++---- tests/unit_tests/test_resume_needs_user_id.py | 19 + 7 files changed, 704 insertions(+), 103 deletions(-) create mode 100644 tenxgraph_api/src/app/core/auth/permissions_registry.py create mode 100644 tenxgraph_api/src/app/core/auth/rbac.py create mode 100644 tests/unit_tests/test_permissions_registry.py diff --git a/pyproject.toml b/pyproject.toml index bbaf207..8ff1914 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -398,6 +398,7 @@ dev = [ "pytest-cov>=7.0.0", "pytest-env>=1.1.5", "pytest-xdist>=3.8.0", + "hypothesis>=6.100.0", "pre-commit>=3.8.0", "mypy>=1.11.0", "ruff==0.5.2", diff --git a/tenxgraph_api/src/app/core/auth/permissions_registry.py b/tenxgraph_api/src/app/core/auth/permissions_registry.py new file mode 100644 index 0000000..47463b3 --- /dev/null +++ b/tenxgraph_api/src/app/core/auth/permissions_registry.py @@ -0,0 +1,51 @@ +"""The one list of permissions (``resource:action``) the API knows about.""" + +from __future__ import annotations + +from collections.abc import Iterable + + +REGISTRY: dict[str, tuple[str, ...]] = { + "graph": ("read", "invoke", "stream", "stop", "fix"), + "threads": ("read", "write", "delete"), + "store": ("read", "write", "delete"), + "files": ("read", "upload", "delete"), + "config": ("read",), + "tasks": ("trigger", "read", "cancel", "pause"), + "admin": ("read", "write"), +} + +ALL_PERMISSIONS: frozenset[str] = frozenset( + f"{resource}:{action}" for resource, actions in REGISTRY.items() for action in actions +) + +DEFAULT_PERMISSIONS: frozenset[str] = frozenset( + p for p in ALL_PERMISSIONS if not p.startswith("admin:") and p != "tasks:pause" +) + + +def expand(patterns: Iterable[str]) -> frozenset[str]: + """Turn ``*``, ``resource:*`` and concrete names into concrete permissions. + + Raises: + ValueError: an entry is malformed or names an unknown resource or action. + """ + result: set[str] = set() + for pattern in patterns: + if not isinstance(pattern, str): + raise ValueError(f"permission {pattern!r} is not a string") + if pattern == "*": + result |= ALL_PERMISSIONS + continue + resource, sep, action = pattern.partition(":") + if not sep or not resource or not action: + raise ValueError(f"permission {pattern!r} is malformed; expected 'resource:action'") + if resource not in REGISTRY: + raise ValueError(f"permission {pattern!r} has unknown resource {resource!r}") + if action == "*": + result |= {f"{resource}:{a}" for a in REGISTRY[resource]} + elif action in REGISTRY[resource]: + result.add(pattern) + else: + raise ValueError(f"permission {pattern!r} has unknown action {action!r}") + return frozenset(result) diff --git a/tenxgraph_api/src/app/core/auth/rbac.py b/tenxgraph_api/src/app/core/auth/rbac.py new file mode 100644 index 0000000..f340489 --- /dev/null +++ b/tenxgraph_api/src/app/core/auth/rbac.py @@ -0,0 +1,203 @@ +"""RBAC config and the pure ``resolve(claims, config) -> Identity`` function.""" + +from __future__ import annotations + +import logging +from typing import Any + +from pydantic import BaseModel, ConfigDict, PrivateAttr, model_validator +from tenxgraph.core.authz import Identity, Reach + +from .permissions_registry import DEFAULT_PERMISSIONS, expand + + +logger = logging.getLogger("tenxgraph_api.auth") + +_REACH_ORDER = {Reach.OWN: 0, Reach.TENANT: 1, Reach.ALL: 2} + + +class InvalidClaimsError(Exception): + """The token claims are malformed for the configured auth; the API answers 401.""" + + +class RoleConfig(BaseModel): + """One entry of ``auth.roles``. ``reach`` is required: it never has a default.""" + + model_config = ConfigDict(extra="forbid") + + permissions: list[str] = [] + inherits: list[str] = [] + reach: Reach + + +class RbacConfig(BaseModel): + """The RBAC keys of the ``auth`` object. Keys it does not own (``method``, ``path``, + ``system_identities``) are ignored here; the full ``auth`` loader owns them.""" + + model_config = ConfigDict(extra="ignore") + + roles_claim: str | None = None + permissions_claim: str | None = None + tenant_claims: list[str] = [] + roles: dict[str, RoleConfig] = {} + default_role: str | None = None + + _effective: dict[str, frozenset[str]] = PrivateAttr(default_factory=dict) + + @model_validator(mode="after") + def _check(self) -> RbacConfig: + if self.permissions_claim and (self.roles_claim or self.roles): + raise ValueError( + "roles_claim/roles and permissions_claim are both set; pick one permission source" + ) + if self.roles_claim and not self.roles: + raise ValueError("roles_claim is set but no roles are defined") + if self.roles and not self.roles_claim: + self.roles_claim = "roles" + if self.default_role is not None and self.default_role not in self.roles: + raise ValueError(f"default_role {self.default_role!r} is not in roles") + own: dict[str, frozenset[str]] = {} + for name, role in self.roles.items(): + try: + own[name] = expand(role.permissions) + except ValueError as exc: + raise ValueError(f"role {name!r}: {exc}") from exc + for parent in role.inherits: + if parent not in self.roles: + raise ValueError(f"role {name!r} inherits {parent!r}, which is not in roles") + if role.reach is Reach.TENANT and not self.tenant_claims: + raise ValueError(f"role {name!r} has reach 'tenant' but tenant_claims is empty") + self._effective = self._inherit(own) + return self + + def _inherit(self, own: dict[str, frozenset[str]]) -> dict[str, frozenset[str]]: + done: dict[str, frozenset[str]] = {} + + def visit(name: str, path: list[str]) -> frozenset[str]: + if name in done: + return done[name] + if name in path: + cycle = " -> ".join([*path[path.index(name) :], name]) + raise ValueError(f"role inheritance cycle: {cycle}") + perms = own[name] + for parent in self.roles[name].inherits: + perms = perms | visit(parent, [*path, name]) + done[name] = perms + return perms + + for name in self.roles: + visit(name, []) + return done + + def role_permissions(self, name: str) -> frozenset[str]: + """Effective permissions of a role: its own plus inherited, computed at parse.""" + return self._effective[name] + + +def roles_with_reach_all(config: RbacConfig | None) -> list[str]: + """Names of roles whose own declared reach is ``all``.""" + if config is None: + return [] + return [name for name, role in config.roles.items() if role.reach is Reach.ALL] + + +def log_reach_all_roles(config: RbacConfig | None) -> None: + """Warn once per role that can see every row.""" + for name in roles_with_reach_all(config): + logger.warning("Role %r has reach 'all': it can read every row of every tenant", name) + + +def resolve(claims: dict[str, Any], config: RbacConfig | None) -> Identity: + """Build the caller's :class:`Identity` from verified token claims. No I/O. + + Raises: + InvalidClaimsError: a claim the config relies on is missing or malformed. + """ + user_id = claims.get("user_id") + if not isinstance(user_id, str) or not user_id: + raise InvalidClaimsError("token has no usable user_id claim") + tenant = _tenant(claims, config.tenant_claims if config else []) + + roles: frozenset[str] = frozenset() + reach = Reach.OWN + if config is not None and config.roles: + roles, permissions, reach = _from_roles(claims, config) + elif config is not None and config.permissions_claim: + entries = _string_list(claims.get(config.permissions_claim), config.permissions_claim) + permissions = _expand_known(entries or []) + else: + permissions = DEFAULT_PERMISSIONS + + permissions = _narrow(claims, permissions) + return Identity( + user_id=user_id, + kind="user", + tenant=tenant, + roles=roles, + permissions=permissions, + reach=reach, + claims=dict(claims), + ) + + +def _tenant(claims: dict[str, Any], names: list[str]) -> dict[str, str]: + tenant: dict[str, str] = {} + for name in names: + value = claims.get(name) + if isinstance(value, int) and not isinstance(value, bool): + value = str(value) + if not isinstance(value, str) or not value: + raise InvalidClaimsError(f"token has no usable tenant claim {name!r}") + tenant[name] = value + return tenant + + +def _string_list(value: Any, claim: str) -> list[str] | None: + """A list of str or a space-separated str; None when the claim is missing.""" + if value is None: + return None + if isinstance(value, str): + return value.split() + if isinstance(value, list) and all(isinstance(v, str) for v in value): + return value + raise InvalidClaimsError( + f"claim {claim!r} must be a list of strings or a space-separated string" + ) + + +def _from_roles( + claims: dict[str, Any], config: RbacConfig +) -> tuple[frozenset[str], frozenset[str], Reach]: + claim = config.roles_claim or "roles" + names = _string_list(claims.get(claim), claim) + if names is None: + names = [config.default_role] if config.default_role else [] + held = [] + for name in names: + if name in config.roles: + held.append(name) + else: + logger.debug("Ignoring unknown role %r in token", name) + permissions: frozenset[str] = frozenset().union(*(config.role_permissions(n) for n in held)) + reach = max((config.roles[n].reach for n in held), key=_REACH_ORDER.get, default=Reach.OWN) + return frozenset(held), permissions, reach + + +def _expand_known(entries: list[str]) -> frozenset[str]: + """Expand entries, skipping (and logging) the ones the registry does not know.""" + known: set[str] = set() + for entry in entries: + try: + known |= expand([entry]) + except ValueError: + logger.debug("Ignoring unknown permission %r in token", entry) + return frozenset(known) + + +def _narrow(claims: dict[str, Any], granted: frozenset[str]) -> frozenset[str]: + """Token ``scope``/``scopes`` only narrow: granted intersected with permission-shaped scopes.""" + entries = (_string_list(claims.get("scope"), "scope") or []) + ( + _string_list(claims.get("scopes"), "scopes") or [] + ) + wanted = _expand_known(entries) + return granted & wanted if wanted else granted diff --git a/tenxgraph_api/src/app/routers/graph/services/graph_service.py b/tenxgraph_api/src/app/routers/graph/services/graph_service.py index b2139ae..296f39d 100644 --- a/tenxgraph_api/src/app/routers/graph/services/graph_service.py +++ b/tenxgraph_api/src/app/routers/graph/services/graph_service.py @@ -877,6 +877,9 @@ async def _prepare_input( # The authenticated user, before any checkpointer read below: PgCheckpointer refuses a # config without ``user_id``. Never taken from the request (``client_config`` strips it). config["user_id"] = user_id if user_id else "anonymous" + # Tenant and extra column values are read from ``config["user"]`` on every storage read. + if user is not None: + config["user"] = user # check recursion limit set or not config["recursion_limit"] = graph_input.recursion_limit or 25 @@ -952,8 +955,6 @@ async def invoke_graph( # noqa: PLR0912, PLR0915 graph_input, user.get("user_id"), user=user ) thread_lock = await self._lock_new_thread(meta) - # add user inside config - config["user"] = user config["user_id"] = user.get("user_id", "anonymous") await self._check_tool_results(input_data["messages"], config) await self._check_resume_target(graph_input, config) @@ -1169,8 +1170,6 @@ async def stream_chunks( # noqa: PLR0912, PLR0915 graph_input, user.get("user_id"), server_config, user=user ) thread_lock = await self._lock_new_thread(meta) - # add user inside config - config["user"] = user config["user_id"] = user.get("user_id", "anonymous") await self._check_tool_results(input_data["messages"], config) await self._check_resume_target(graph_input, config) diff --git a/tests/unit_tests/test_permissions_registry.py b/tests/unit_tests/test_permissions_registry.py new file mode 100644 index 0000000..f085653 --- /dev/null +++ b/tests/unit_tests/test_permissions_registry.py @@ -0,0 +1,71 @@ +import pytest +from hypothesis import given +from hypothesis import strategies as st + +from tenxgraph_api.src.app.core.auth.permissions_registry import ( + ALL_PERMISSIONS, + DEFAULT_PERMISSIONS, + REGISTRY, + expand, +) + + +def test_all_permissions_count(): + assert len(ALL_PERMISSIONS) == 5 + 3 + 3 + 3 + 1 + 4 + 2 + assert "graph:invoke" in ALL_PERMISSIONS + + +def test_default_excludes_admin_and_pause(): + assert not {p for p in DEFAULT_PERMISSIONS if p.startswith("admin:")} + assert "tasks:pause" not in DEFAULT_PERMISSIONS + assert ALL_PERMISSIONS - {"admin:read", "admin:write", "tasks:pause"} == DEFAULT_PERMISSIONS + + +def test_expand_star_resource_and_concrete(): + assert expand(["*"]) == ALL_PERMISSIONS + assert expand(["config:*"]) == {"config:read"} + assert expand(["threads:*", "graph:read"]) == { + "threads:read", + "threads:write", + "threads:delete", + "graph:read", + } + assert expand([]) == frozenset() + + +@pytest.mark.parametrize( + "bad", ["nope:read", "graph:nope", "graph", ":read", "graph:", "", "graph:read:x", 5] +) +def test_expand_rejects_bad_entries(bad): + with pytest.raises(ValueError): + expand([bad]) + + +def test_expand_error_names_entry(): + with pytest.raises(ValueError, match="graph:nope"): + expand(["graph:read", "graph:nope"]) + + +# ---- properties + +PATTERNS = st.lists( + st.sampled_from( + ["*", *sorted(ALL_PERMISSIONS), *(f"{r}:*" for r in REGISTRY)], + ) +) + + +@given(PATTERNS, PATTERNS) +def test_expand_monotone(a, b): + assert expand(a) <= expand(a + b) + + +@given(PATTERNS) +def test_expand_idempotent(a): + assert expand(sorted(expand(a))) == expand(a) + assert expand(a) <= ALL_PERMISSIONS + + +@given(PATTERNS) +def test_star_is_all(a): + assert expand(["*", *a]) == ALL_PERMISSIONS diff --git a/tests/unit_tests/test_rbac.py b/tests/unit_tests/test_rbac.py index b68b953..b565628 100644 --- a/tests/unit_tests/test_rbac.py +++ b/tests/unit_tests/test_rbac.py @@ -1,150 +1,407 @@ -"""Tests for RoleBasedAuthorizationBackend (roles -> scopes) and config wiring.""" - -from __future__ import annotations - import pytest - -from tenxgraph_api.src.app.core.auth.authorization import ( - RoleBasedAuthorizationBackend, - all_scopes, +from hypothesis import given +from hypothesis import strategies as st +from pydantic import ValidationError +from tenxgraph.core.authz import Reach + +from tenxgraph_api.src.app.core.auth.permissions_registry import ( + ALL_PERMISSIONS, + DEFAULT_PERMISSIONS, +) +from tenxgraph_api.src.app.core.auth.rbac import ( + InvalidClaimsError, + RbacConfig, + log_reach_all_roles, + resolve, + roles_with_reach_all, ) +def cfg(**kw) -> RbacConfig: + return RbacConfig.model_validate(kw) + + ROLES = { - "admin": ["*"], - "member": ["graph:invoke", "graph:stream", "checkpointer:read"], - "viewer": ["checkpointer:read"], + "member": {"permissions": ["graph:*", "threads:read"], "reach": "own"}, + "org_admin": {"inherits": ["member"], "permissions": ["admin:read"], "reach": "tenant"}, + "super": {"permissions": ["*"], "reach": "all"}, } +BASE = {"user_id": "u1"} + + +def rbac() -> RbacConfig: + return cfg(roles=ROLES, tenant_claims=["org_id"], default_role="member") + + +# ---- config validation + + +def test_roles_claim_defaults_to_roles(): + assert rbac().roles_claim == "roles" + + +def test_unowned_keys_ignored(): + assert cfg(method="jwt", path=None, system_identities={}).roles == {} + + +def test_inheritance_multi_level_and_diamond(): + c = cfg( + roles={ + "a": {"permissions": ["graph:read"], "reach": "own"}, + "b": {"permissions": ["store:read"], "inherits": ["a"], "reach": "own"}, + "c": {"permissions": ["files:read"], "inherits": ["a"], "reach": "own"}, + "d": {"permissions": ["config:read"], "inherits": ["b", "c"], "reach": "own"}, + } + ) + assert c.role_permissions("d") == {"graph:read", "store:read", "files:read", "config:read"} + assert c.role_permissions("a") == {"graph:read"} + + +def test_inherited_reach_not_changed(): + c = rbac() + assert c.roles["org_admin"].reach is Reach.TENANT + assert c.roles["member"].reach is Reach.OWN + assert "graph:invoke" in c.role_permissions("org_admin") + + +def test_cycle_refused_and_named(): + roles = { + "a": {"inherits": ["b"], "reach": "own"}, + "b": {"inherits": ["c"], "reach": "own"}, + "c": {"inherits": ["a"], "reach": "own"}, + } + with pytest.raises(ValidationError, match=r"a -> b -> c -> a"): + cfg(roles=roles) + + +def test_self_cycle_refused(): + with pytest.raises(ValidationError, match="x -> x"): + cfg(roles={"x": {"inherits": ["x"], "reach": "own"}}) + + +@pytest.mark.parametrize( + ("kwargs", "match"), + [ + ({"roles": {"r": {"permissions": ["nope:read"], "reach": "own"}}}, "nope:read"), + ({"roles": {"r": {"permissions": ["graph:zap"], "reach": "own"}}}, "graph:zap"), + ({"roles": {"r": {"permissions": []}}}, "reach"), + ({"roles": {"r": {"permissions": [], "reach": "galaxy"}}}, "reach"), + ( + {"roles": {"r": {"reach": "own"}}, "default_role": "ghost"}, + "ghost", + ), + ({"roles": {"r": {"inherits": ["ghost"], "reach": "own"}}}, "ghost"), + ( + { + "roles": {"r": {"reach": "own"}}, + "roles_claim": "roles", + "permissions_claim": "perms", + }, + "pick one", + ), + ({"roles": {"r": {"reach": "tenant"}}}, "tenant_claims"), + ({"roles_claim": "roles"}, "no roles"), + ], +) +def test_config_refused(kwargs, match): + with pytest.raises(ValidationError, match=match): + cfg(**kwargs) + + +def test_roles_with_reach_all(): + assert roles_with_reach_all(rbac()) == ["super"] + assert roles_with_reach_all(None) == [] + assert roles_with_reach_all(cfg()) == [] + + +def test_log_reach_all(caplog): + with caplog.at_level("WARNING", logger="tenxgraph_api.auth"): + log_reach_all_roles(rbac()) + assert "super" in caplog.text + + +# ---- resolve: no RBAC config + +@pytest.mark.parametrize("config", [None, cfg(), cfg(tenant_claims=[])]) +def test_default_mode(config): + ident = resolve(dict(BASE), config) + assert ident.permissions == DEFAULT_PERMISSIONS + assert ident.reach is Reach.OWN + assert ident.user_id == "u1" + assert ident.claims == BASE -def _backend(**kw): - return RoleBasedAuthorizationBackend(ROLES, default_scopes=["graph:read"], **kw) +@pytest.mark.parametrize("claims", [{}, {"user_id": ""}, {"user_id": 5}, {"user_id": None}]) +def test_bad_user_id(claims): + with pytest.raises(InvalidClaimsError): + resolve(claims, None) -def test_member_gets_role_scopes_plus_defaults(): - b = _backend() - scopes = b.scopes_for({"user_id": "u1", "roles": ["member"]}) - assert "graph:invoke" in scopes - assert "checkpointer:read" in scopes - assert "graph:read" in scopes # default - assert "checkpointer:delete" not in scopes +# ---- tenant -def test_admin_wildcard_expands_to_all_scopes(): - b = _backend() - assert set(b.scopes_for({"role": "admin"})) == set(all_scopes()) +def test_tenant_values(): + ident = resolve({**BASE, "org_id": 7, "roles": ["member"]}, rbac()) + assert ident.tenant == {"org_id": "7"} -def _required_scopes() -> set[str]: - """Every ``resource:action`` a route asks ``RequirePermission`` for.""" - from tenxgraph_api.src.app.core.auth.permissions import RequirePermission - from tenxgraph_api.src.app.routers.checkpointer.router import router as checkpointer - from tenxgraph_api.src.app.routers.graph.router import router as graph - from tenxgraph_api.src.app.routers.media.router import router as media - from tenxgraph_api.src.app.routers.store.router import router as store - found: set[str] = set() +@pytest.mark.parametrize("value", [None, "", 1.5, ["x"], True]) +def test_bad_tenant_claim(value): + claims = {**BASE, "roles": ["member"]} + if value is not None: + claims["org_id"] = value + with pytest.raises(InvalidClaimsError, match="org_id"): + resolve(claims, rbac()) - def walk(dependant): - for dep in dependant.dependencies: - if isinstance(dep.call, RequirePermission): - found.add(f"{dep.call.resource}:{dep.call.action}") - walk(dep) - for router in (checkpointer, graph, media, store): - for route in router.routes: - walk(route.dependant) - return found +# ---- roles mode -def test_wildcard_role_reaches_every_route(): - required = _required_scopes() - assert {"store:read", "store:write", "store:delete"} <= required - assert required <= all_scopes() +@pytest.mark.parametrize("roles", [["member"], "member"]) +def test_roles_list_and_space_string(roles): + ident = resolve({**BASE, "org_id": "a", "roles": roles}, rbac()) + assert ident.roles == {"member"} + assert ident.permissions == rbac().role_permissions("member") -def test_no_role_gets_only_defaults(): - b = _backend() - assert b.scopes_for({"user_id": "u1"}) == ["graph:read"] +def test_space_string_multiple(): + ident = resolve({**BASE, "org_id": "a", "roles": "member org_admin"}, rbac()) + assert ident.roles == {"member", "org_admin"} + assert ident.reach is Reach.TENANT -def test_multiple_roles_union(): - b = _backend() - scopes = b.scopes_for({"roles": ["viewer", "member"]}) - assert "graph:invoke" in scopes and "checkpointer:read" in scopes +@pytest.mark.parametrize("roles", [5, {"a": 1}, [1, 2], ["member", None], True]) +def test_malformed_roles_claim_not_default(roles): + with pytest.raises(InvalidClaimsError): + resolve({**BASE, "org_id": "a", "roles": roles}, rbac()) -def test_single_role_string_accepted(): - b = _backend() - assert "checkpointer:read" in b.scopes_for({"role": "viewer"}) +def test_missing_roles_claim_uses_default_role(): + ident = resolve({**BASE, "org_id": "a"}, rbac()) + assert ident.roles == {"member"} -def test_isolation_defaults_to_owner_and_is_configurable(): - assert _backend().isolation_scope() == "owner" - assert _backend(isolation="none").isolation_scope() == "none" +def test_missing_roles_claim_no_default_means_nothing(): + c = cfg(roles=ROLES, tenant_claims=["org_id"]) + ident = resolve({**BASE, "org_id": "a"}, c) + assert ident.permissions == frozenset() + assert ident.reach is Reach.OWN +def test_unknown_token_role_ignored(): + ident = resolve({**BASE, "org_id": "a", "roles": ["ghost", "member"]}, rbac()) + assert ident.roles == {"member"} + only_ghost = resolve({**BASE, "org_id": "a", "roles": ["ghost"]}, rbac()) + assert only_ghost.permissions == frozenset() + assert only_ghost.reach is Reach.OWN -@pytest.mark.parametrize("bad", ["bogus", "owners", "OWNER", None, True, ["user_id"]]) -def test_unknown_isolation_raises_at_construction(bad): - """A typo must stop the server from booting, never pick an isolation level quietly.""" - with pytest.raises(ValueError, match="Unknown authorization isolation"): - _backend(isolation=bad) +def test_custom_roles_claim_name(): + c = cfg(roles_claim="groups", roles=ROLES, tenant_claims=["org_id"]) + ident = resolve({**BASE, "org_id": "a", "groups": ["super"], "roles": ["member"]}, c) + assert ident.roles == {"super"} -def test_isolation_accepts_enum_members(): - from tenxgraph.core.authz import Isolation - assert _backend(isolation=Isolation.NONE).isolation_scope() == "none" - assert _backend(isolation=Isolation.OWNER).isolation_scope() == "owner" - assert type(_backend(isolation=Isolation.NONE).isolation_scope()) is str +@pytest.mark.parametrize( + ("held", "expected"), + [ + (["member"], Reach.OWN), + (["member", "org_admin"], Reach.TENANT), + (["org_admin", "super", "member"], Reach.ALL), + (["super"], Reach.ALL), + ], +) +def test_widest_reach(held, expected): + assert resolve({**BASE, "org_id": "a", "roles": held}, rbac()).reach is expected + + +@pytest.mark.parametrize( + "claims", + [ + {}, + {"roles": []}, + {"roles": ["member"]}, + {"roles": ["org_admin"]}, + {"roles": ["ghost"]}, + {"roles": "member org_admin"}, + {"reach": "all", "roles": ["member"]}, + {"scope": "*"}, + ], +) +def test_reach_never_defaults_to_all(claims): + ident = resolve({**BASE, "org_id": "a", **claims}, rbac()) + assert ident.reach is not Reach.ALL + + +def test_no_config_never_all(): + assert resolve({**BASE, "reach": "all"}, None).reach is Reach.OWN + + +# ---- permissions_claim mode + +def test_permissions_claim_mode(): + c = cfg(permissions_claim="perms") + ident = resolve({**BASE, "perms": ["graph:*", "files:read", "bogus", "x:y"]}, c) + assert ident.permissions == { + "graph:read", + "graph:invoke", + "graph:stream", + "graph:stop", + "graph:fix", + "files:read", + } + assert ident.reach is Reach.OWN -def test_backend_without_isolation_scope_gets_owner(): - """A duck-typed backend with no isolation_scope method fails closed (owner).""" - from tenxgraph_api.src.app.core.auth.authorization import resolve_isolation_scope - class _Bare: - async def authorize(self, *a, **k): - return True +def test_permissions_claim_space_string_and_missing(): + c = cfg(permissions_claim="perms") + assert resolve({**BASE, "perms": "config:read store:read"}, c).permissions == { + "config:read", + "store:read", + } + assert resolve(dict(BASE), c).permissions == frozenset() - assert resolve_isolation_scope(_Bare(), {"user_id": "u1"}) == "owner" +@pytest.mark.parametrize("value", [5, {"a": 1}, [1]]) +def test_permissions_claim_malformed(value): + with pytest.raises(InvalidClaimsError): + resolve({**BASE, "perms": value}, cfg(permissions_claim="perms")) -@pytest.mark.asyncio -async def test_rbac_still_enforces_thread_ownership(): - """RBAC inherits owner-only object isolation from OwnershipAuthorizationBackend.""" - class _Cp: - async def aget_thread_owner(self, thread_id): - return "alice" if str(thread_id) == "t1" else None +# ---- narrowing - b = RoleBasedAuthorizationBackend(ROLES, checkpointer=_Cp()) - assert await b.authorize({"user_id": "alice"}, "checkpointer", "read", resource_id="t1") is True - assert await b.authorize({"user_id": "bob"}, "checkpointer", "read", resource_id="t1") is False +def test_scope_narrows(): + ident = resolve({**BASE, "scope": "graph:read threads:read files:read"}, None) + assert ident.permissions == {"graph:read", "threads:read", "files:read"} -def test_loader_builds_rbac_from_dict_config(): - from tenxgraph_api.src.app.loader import _resolve_authorization_backend - b = _resolve_authorization_backend( - {"backend": "rbac", "roles": ROLES, "default_scopes": ["graph:read"]} +def test_scopes_list_and_wildcard(): + ident = resolve({**BASE, "scopes": ["store:*", "admin:read"]}, None) + assert ident.permissions == {"store:read", "store:write", "store:delete"} + + +def test_openid_only_does_not_narrow(): + assert resolve({**BASE, "scope": "openid profile email"}, None).permissions == ( + DEFAULT_PERMISSIONS ) - assert isinstance(b, RoleBasedAuthorizationBackend) - assert "graph:invoke" in b.scopes_for({"roles": ["member"]}) + assert resolve({**BASE, "scopes": []}, None).permissions == DEFAULT_PERMISSIONS + +def test_scope_mixed_with_openid_narrows(): + ident = resolve({**BASE, "scope": "openid graph:read"}, None) + assert ident.permissions == {"graph:read"} + + +@pytest.mark.parametrize( + "claims", [{"scope": 5}, {"scope": {"a": 1}}, {"scopes": 5}, {"scopes": [1]}] +) +def test_malformed_scope(claims): + with pytest.raises(InvalidClaimsError): + resolve({**BASE, **claims}, None) -def test_loader_rejects_unknown_backend_dict(): - from tenxgraph_api.src.app.loader import _resolve_authorization_backend - with pytest.raises(ValueError): - _resolve_authorization_backend({"backend": "nope"}) +@pytest.mark.parametrize("scope", ["*", "admin:*", "admin:write graph:read", "graph:invoke", "x"]) +@pytest.mark.parametrize("roles", [["member"], ["org_admin"], ["super"], []]) +def test_narrowing_never_widens(scope, roles): + base = resolve({**BASE, "org_id": "a", "roles": roles}, rbac()) + narrowed = resolve({**BASE, "org_id": "a", "roles": roles, "scope": scope}, rbac()) + assert narrowed.permissions <= base.permissions + assert narrowed.permissions <= ALL_PERMISSIONS + assert narrowed.reach is base.reach -@pytest.mark.parametrize("bad", ["owners", None]) -def test_loader_refuses_unknown_isolation(bad): - """10xgraph.json with a bad authorization.isolation stops the server from booting.""" - from tenxgraph_api.src.app.loader import _resolve_authorization_backend +def test_scope_cannot_add_to_empty(): + ident = resolve({**BASE, "org_id": "a", "roles": ["ghost"], "scope": "*"}, rbac()) + assert ident.permissions == frozenset() + + +# ---- properties + +PERM_NAMES = sorted(ALL_PERMISSIONS) +JUNK = st.sampled_from(["openid", "profile", "email", "x", "graph", "a:b", "graph:nope"]) +SCOPE_ENTRIES = st.lists(st.one_of(st.sampled_from([*PERM_NAMES, "*", "graph:*", "admin:*"]), JUNK)) + + +@st.composite +def acyclic_roles(draw): + """Role table where role i can only inherit roles with a lower index.""" + count = draw(st.integers(1, 6)) + roles = {} + for i in range(count): + parents = ( + draw(st.lists(st.sampled_from([f"r{j}" for j in range(i)]), max_size=3)) if i else [] + ) + roles[f"r{i}"] = { + "permissions": draw(st.lists(st.sampled_from([*PERM_NAMES, "*", "tasks:*"]))), + "inherits": parents, + "reach": draw(st.sampled_from(["own", "tenant", "all"])), + } + return roles + + +def role_config(roles) -> RbacConfig: + return cfg(roles=roles, tenant_claims=["org_id"], default_role="r0") + + +@given( + roles=acyclic_roles(), + held=st.lists(st.sampled_from([f"r{i}" for i in range(6)])), + scope=SCOPE_ENTRIES, +) +def test_property_narrowing_never_widens(roles, held, scope): + c = role_config(roles) + claims = {**BASE, "org_id": "a", "roles": held} + base = resolve(claims, c) + for key, value in (("scope", " ".join(scope)), ("scopes", scope)): + narrowed = resolve({**claims, key: value}, c) + assert narrowed.permissions <= base.permissions + assert narrowed.reach is base.reach + + +@given(granted=st.sets(st.sampled_from(PERM_NAMES)), scope=SCOPE_ENTRIES) +def test_property_narrowing_permissions_claim(granted, scope): + c = cfg(permissions_claim="perms") + claims = {**BASE, "perms": sorted(granted)} + assert resolve({**claims, "scopes": scope}, c).permissions <= resolve(claims, c).permissions + assert resolve(claims, c).permissions == granted + + +@given(roles=acyclic_roles()) +def test_property_inheritance_superset(roles): + c = role_config(roles) + for name, role in roles.items(): + for parent in role["inherits"]: + assert c.role_permissions(name) >= c.role_permissions(parent) + + +@given(roles=acyclic_roles(), length=st.integers(1, 5)) +def test_property_cycle_refused(roles, length): + cycle = {f"c{i}": {"inherits": [f"c{(i + 1) % length}"], "reach": "own"} for i in range(length)} + with pytest.raises(ValidationError, match="cycle"): + role_config({**roles, **cycle}) + + +@given(roles=acyclic_roles(), held=st.lists(st.sampled_from([f"r{i}" for i in range(6)]))) +def test_property_reach_all_only_if_held_role_declares_it(roles, held): + ident = resolve({**BASE, "org_id": "a", "roles": held}, role_config(roles)) + if ident.reach is Reach.ALL: + assert any(roles[r]["reach"] == "all" for r in ident.roles) + + +MALFORMED = st.one_of( + st.integers(), + st.floats(), + st.booleans(), + st.dictionaries(st.text(), st.text()), + st.lists(st.one_of(st.integers(), st.none(), st.booleans(), st.floats()), min_size=1), + st.tuples(st.text(), st.integers()).map(lambda t: [t[0], t[1]]), +) + - with pytest.raises(ValueError, match=r"authorization\.isolation in 10xgraph\.json"): - _resolve_authorization_backend({"backend": "rbac", "roles": ROLES, "isolation": bad}) +@given(MALFORMED) +def test_property_malformed_roles_claim_raises(value): + with pytest.raises(InvalidClaimsError): + resolve({**BASE, "org_id": "a", "roles": value}, rbac()) diff --git a/tests/unit_tests/test_resume_needs_user_id.py b/tests/unit_tests/test_resume_needs_user_id.py index c43526d..892a262 100644 --- a/tests/unit_tests/test_resume_needs_user_id.py +++ b/tests/unit_tests/test_resume_needs_user_id.py @@ -92,3 +92,22 @@ async def test_client_cannot_supply_user_id(): except Exception: # rejected outright is also fine return assert config["user_id"] == "alice" + + +@pytest.mark.asyncio +async def test_resume_reads_state_with_user_for_tenant_columns(): + # AUTH-S0-01: a tenant-column checkpointer reads tenant values from config["user"], so it + # must be set before the first state read (the static-pause check in _prepare_input). + graph = MagicMock() + graph.ainvoke = AsyncMock(return_value={"messages": [], "state": AgentState()}) + service = _strict_service(graph) + inner = service.checkpointer.aget_state.side_effect + + async def aget_state(config): + if not isinstance(config.get("user"), dict): + raise ValueError("MissingColumnValueError: tenant value missing from config['user']") + return await inner(config) + + service.checkpointer.aget_state = AsyncMock(side_effect=aget_state) + await service.invoke_graph(_resume(interrupt_id="i1"), ALICE) + assert graph.ainvoke.call_args.args[0]["resume"] == {"ok": True} From f5fe6d03720acd820f4423b783b1530ff2333f3e Mon Sep 17 00:00:00 2001 From: Shudipto Trafder Date: Sat, 7 Nov 2026 14:35:36 +0600 Subject: [PATCH 02/10] feat: add /v1/me endpoint to retrieve caller identity and permissions - Implemented the /v1/me endpoint in the me.py router to return the authenticated user's identity, roles, and permissions. - Added tests for the /v1/me endpoint to verify correct behavior with and without authentication. test: add CLI tests for auth rules validation - Created tests for the AuditCommand and BuildCommand to validate shared auth rules in the CLI. - Included assertions for expected outputs and exit codes based on different configurations. test: add integration tests for transport parity - Developed integration tests to ensure consistent permission decisions across different transport methods (HTTP and WebSocket). - Verified that revoked tokens and unauthorized access are handled correctly. test: add unit tests for auth configuration validation - Implemented unit tests for the AuthConfig validation logic to ensure proper handling of various auth configurations. - Included tests for RBAC rules, tenant claims, and custom authorization methods. test: add unit tests for auth dependencies - Created unit tests for RequirePermission and RequireIdentity dependencies to ensure correct behavior in HTTP and WebSocket contexts. - Verified that unauthorized access returns appropriate status codes. test: add unit tests for /v1/me route - Added unit tests for the /v1/me route to check identity retrieval and response structure. - Ensured that the endpoint behaves correctly when authentication is off. --- CLAUDE.md | 5 +- SECURITY.md | 3 +- config-editor-ui/src/icons.js | 2 +- config-editor-ui/src/pages.jsx | 20 +- config-editor-ui/src/store.js | 5 +- config-editor-ui/src/widgets.jsx | 485 +++++++---- tenxgraph_api/__init__.py | 6 + tenxgraph_api/cli/commands/audit.py | 36 +- tenxgraph_api/cli/commands/build.py | 8 +- tenxgraph_api/cli/commands/config.py | 6 +- tenxgraph_api/cli/commands/init.py | 13 +- tenxgraph_api/cli/config_editor/schema.py | 88 +- .../cli/config_editor/static/app.css | 2 +- tenxgraph_api/cli/config_editor/static/app.js | 6 +- tenxgraph_api/cli/config_editor/validation.py | 112 +-- .../cli/templates/prod/10xgraph.json | 8 +- .../cli/templates/prod/auth/agent_auth.py | 17 +- .../references/auth-and-authorization.md | 2 +- .../10xgraph/references/cli-commands.md | 2 +- tenxgraph_api/jobs/arguments.py | 2 +- .../src/app/core/auth/auth_backend.py | 46 - .../src/app/core/auth/auth_config.py | 293 +++++++ .../src/app/core/auth/authorization.py | 816 ------------------ tenxgraph_api/src/app/core/auth/base_auth.py | 41 +- .../src/app/core/auth/identity_cache.py | 77 -- tenxgraph_api/src/app/core/auth/jwt_auth.py | 40 +- .../src/app/core/auth/permissions.py | 502 ++++------- tenxgraph_api/src/app/core/auth/rbac.py | 13 +- .../src/app/core/auth/reserved_identity.py | 14 +- tenxgraph_api/src/app/core/auth/revocation.py | 29 +- .../src/app/core/auth/route_guard.py | 11 +- .../src/app/core/config/graph_config.py | 98 +-- .../src/app/core/exceptions/handle_errors.py | 4 +- .../app/core/middleware/rate_limit/keying.py | 38 +- .../src/app/core/validation/__init__.py | 0 .../{auth => validation}/client_filters.py | 2 +- .../{auth => validation}/request_config.py | 0 tenxgraph_api/src/app/loader.py | 277 ++---- tenxgraph_api/src/app/main.py | 12 +- .../src/app/routers/ag_ui/converter.py | 4 +- tenxgraph_api/src/app/routers/ag_ui/router.py | 32 +- .../src/app/routers/ag_ui/service.py | 37 +- .../src/app/routers/checkpointer/router.py | 119 +-- .../schemas/checkpointer_schemas.py | 25 +- .../services/checkpointer_service.py | 168 ++-- .../src/app/routers/graph/idempotency.py | 9 +- .../src/app/routers/graph/realtime_guard.py | 10 +- tenxgraph_api/src/app/routers/graph/router.py | 210 ++--- .../src/app/routers/graph/run_guard.py | 4 +- .../src/app/routers/graph/run_limits.py | 13 +- .../routers/graph/schemas/graph_schemas.py | 2 +- .../routers/graph/services/graph_service.py | 99 +-- .../graph/services/multimodal_preprocessor.py | 11 +- .../src/app/routers/graph/thread_lock.py | 2 +- tenxgraph_api/src/app/routers/me.py | 32 + .../src/app/routers/media/__init__.py | 36 +- tenxgraph_api/src/app/routers/media/router.py | 40 +- tenxgraph_api/src/app/routers/setup_router.py | 2 + tenxgraph_api/src/app/routers/store/router.py | 38 +- .../routers/store/schemas/store_schemas.py | 2 +- .../routers/store/services/store_service.py | 44 +- tenxgraph_api/src/app/routers/tasks/router.py | 41 +- .../src/app/routers/tasks/schemas.py | 2 +- tests/cli/test_auth_cli.py | 58 ++ tests/cli/test_init_prod.py | 24 +- tests/cli/test_production_readiness.py | 21 +- tests/integration_tests/conftest.py | 48 +- tests/integration_tests/store/conftest.py | 123 +-- tests/integration_tests/test_ag_ui_api.py | 154 +++- tests/integration_tests/test_graph_api.py | 5 +- .../test_graph_introspection.py | 101 +-- .../integration_tests/test_idempotency_key.py | 11 +- .../integration_tests/test_isolation_idor.py | 319 +------ .../test_media_tenant_isolation.py | 64 +- .../test_scope_enforcement.py | 259 ++---- .../test_server_thread_ids.py | 283 ------ .../test_tenant_isolation.py | 332 +++---- .../test_transport_parity.py | 280 ++++++ tests/unit_tests/auth/test_auth_backend.py | 318 ------- .../unit_tests/auth/test_graph_config_auth.py | 417 +-------- tests/unit_tests/auth/test_jwt_hardening.py | 12 +- tests/unit_tests/auth/test_token_claims.py | 113 +-- tests/unit_tests/jobs/test_context.py | 1 - tests/unit_tests/jobs/test_routes.py | 50 +- tests/unit_tests/store/conftest.py | 9 +- .../store/test_store_client_fields.py | 2 +- tests/unit_tests/store/test_store_service.py | 12 +- tests/unit_tests/test_auth_config.py | 214 +++++ tests/unit_tests/test_auth_dependency.py | 265 ++++++ tests/unit_tests/test_cache_store_config.py | 3 +- tests/unit_tests/test_checkpointer_router.py | 55 +- tests/unit_tests/test_checkpointer_service.py | 73 +- .../test_client_config_isolation.py | 35 +- tests/unit_tests/test_client_filter_types.py | 4 +- tests/unit_tests/test_client_input_rules.py | 130 +-- .../test_client_mistakes_not_500.py | 3 +- tests/unit_tests/test_client_tool_calls.py | 17 +- tests/unit_tests/test_closing_and_cancel.py | 15 +- tests/unit_tests/test_concurrency_limiter.py | 31 +- tests/unit_tests/test_config_editor.py | 120 ++- tests/unit_tests/test_fix_graph.py | 27 +- tests/unit_tests/test_g127_leftovers.py | 6 +- tests/unit_tests/test_graph_resume_input.py | 2 +- tests/unit_tests/test_graph_router.py | 21 +- tests/unit_tests/test_graph_service.py | 21 +- tests/unit_tests/test_group3a_storage.py | 24 +- tests/unit_tests/test_group6_nesting_depth.py | 5 +- tests/unit_tests/test_group6_small_fixes.py | 10 +- tests/unit_tests/test_id_generator_config.py | 5 +- tests/unit_tests/test_identity_cache.py | 195 ----- tests/unit_tests/test_loader.py | 155 +--- tests/unit_tests/test_me_route.py | 56 ++ tests/unit_tests/test_media_g123_g125.py | 2 +- tests/unit_tests/test_media_group3b.py | 5 +- tests/unit_tests/test_media_lifecycle.py | 72 +- tests/unit_tests/test_media_ref_ownership.py | 15 +- tests/unit_tests/test_media_router.py | 28 +- tests/unit_tests/test_nc23_media_ownership.py | 46 +- .../test_ownership_authorization.py | 327 ------- tests/unit_tests/test_ownership_resolver.py | 131 --- tests/unit_tests/test_permissions.py | 244 ------ tests/unit_tests/test_permissions_auth.py | 398 --------- tests/unit_tests/test_rbac.py | 11 + tests/unit_tests/test_reserved_user_id.py | 63 +- tests/unit_tests/test_resource_id_rule.py | 25 +- .../test_resume_binding_and_limits.py | 16 +- tests/unit_tests/test_resume_needs_user_id.py | 4 +- tests/unit_tests/test_route_guard.py | 4 +- tests/unit_tests/test_run_entry_guards.py | 27 +- tests/unit_tests/test_run_limits.py | 15 +- tests/unit_tests/test_runs_config.py | 83 +- tests/unit_tests/test_sprint7_review_fixes.py | 268 +----- tests/unit_tests/test_static_pause_resume.py | 2 +- tests/unit_tests/test_store_router.py | 32 +- tests/unit_tests/test_stream_protocol.py | 2 +- tests/unit_tests/test_thread_claiming.py | 13 - tests/unit_tests/test_thread_lock.py | 33 +- tests/unit_tests/test_thread_name_choices.py | 3 +- .../unit_tests/test_thread_scope_resolver.py | 189 ---- .../test_threads_files_live_contract.py | 11 +- tests/unit_tests/test_token_revocation.py | 3 +- tests/unit_tests/test_websocket_auth.py | 26 +- tests/unit_tests/test_websocket_graph.py | 39 +- tests/unit_tests/test_websocket_guard.py | 20 +- tests/unit_tests/test_websocket_limits.py | 11 +- tests/unit_tests/test_websocket_realtime.py | 92 +- 146 files changed, 3433 insertions(+), 7458 deletions(-) delete mode 100644 tenxgraph_api/src/app/core/auth/auth_backend.py create mode 100644 tenxgraph_api/src/app/core/auth/auth_config.py delete mode 100644 tenxgraph_api/src/app/core/auth/authorization.py delete mode 100644 tenxgraph_api/src/app/core/auth/identity_cache.py create mode 100644 tenxgraph_api/src/app/core/validation/__init__.py rename tenxgraph_api/src/app/core/{auth => validation}/client_filters.py (98%) rename tenxgraph_api/src/app/core/{auth => validation}/request_config.py (100%) create mode 100644 tenxgraph_api/src/app/routers/me.py create mode 100644 tests/cli/test_auth_cli.py delete mode 100644 tests/integration_tests/test_server_thread_ids.py create mode 100644 tests/integration_tests/test_transport_parity.py delete mode 100644 tests/unit_tests/auth/test_auth_backend.py create mode 100644 tests/unit_tests/test_auth_config.py create mode 100644 tests/unit_tests/test_auth_dependency.py delete mode 100644 tests/unit_tests/test_identity_cache.py create mode 100644 tests/unit_tests/test_me_route.py delete mode 100644 tests/unit_tests/test_ownership_authorization.py delete mode 100644 tests/unit_tests/test_permissions.py delete mode 100644 tests/unit_tests/test_permissions_auth.py diff --git a/CLAUDE.md b/CLAUDE.md index 94fef42..e0753b6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -100,7 +100,6 @@ Parsed by `tenxgraph_api/src/app/core/config/graph_config.py`. Supported keys: | `runs` | `RunsConfig`: `thread_ids` (`server` default = only existing ids may be named on REST invoke/stream, `/v1/graph/ws` and checkpointer writes, a new thread is started by omitting the id; enforced by the ownership/rbac backends via `OwnershipAuthorizationBackend(server_thread_ids=...)`. AG-UI and `/v1/graph/live` follow the same rule (a blank or `new` id gets a server id). `client` = any request may name a new thread, explicit opt-in), `config_keys` / `state_keys` (allow-lists for request config and `initial_state`/PUT state; 422 on other keys; `thread_id`, `run_id` always allowed), `max_concurrent_per_user` (HTTP invoke/stream/AG-UI runs in flight, default 10, 429 over it; `routers/graph/run_limits.py`), `timeout` (seconds; invoke 504, streams end with an error chunk), `max_parallel_tool_calls` (positive int, default unset = core's default 10 or env `TENXGRAPH_MAX_PARALLEL_TOOL_CALLS`; set into every run's config in `_prepare_input`, reserved from clients) | | `cache_store.token_revocation` | `true`, `false`, `null` or `{"enabled", "token_lifetime", "user_ttl"}` (`graph_config.TokenRevocationConfig`; a string is a config error). Read through `GraphConfig.token_revocation`. Revocations live in the cache store (`core/auth/revocation.py`: `revoke_token`, `revoke_user`, `is_revoked`), checked by `RequirePermission` (401), before every `/v1/graph/ws` run, and before and every 30 s during a `/v1/graph/live` session (`REALTIME_RECHECK_INTERVAL`, close 1008); cache store errors fail closed (503), and an unreachable cache store refuses the boot | | `media` | `delete_with_thread` (default true): delete an uploaded file when the last message or thread that references it is deleted. Links are recorded in the file's `threads` metadata (`MediaService.link_threads` / `unlink_threads`); files without a `threads` key (uploaded before link tracking) and stores without `update_metadata` (core older than the release that added it) are never auto-deleted. `false` still records links. Parsed by `graph_config.MediaConfig` | -| `graph_introspection` | `"open"` or `"scoped"` (default scoped in production): `GET /v1/graph`, `/v1/graph/tools`, `/v1/graph:StateSchema` need an explicit `graph:read` scope (`RequirePermission(..., introspection=True)`) | `cache_store.rate_limit` object (read through `GraphConfig.rate_limit`, None when absent or `enabled: false`): `enabled`, `requests` (default 100), `window` secs (60), `by` (`ip` | @@ -140,7 +139,7 @@ valid ones (`RATE_LIMIT_KEYS`). values) equal the caller's on every column the scope picks; a scope naming a column the checkpointer does not declare denies (fails closed). - **Client `filters`.** `GET /v1/threads?filters=` (at most 8192 chars) and the store routes' - `filters` are checked by `core/auth/client_filters.py` with core's `sql_filter` validator before + `filters` are checked by `core/validation/client_filters.py` with core's `sql_filter` validator before any DB work: malformed JSON, non-filterable or tenant columns / `user_id`, a bad operator, or a checkpointer without filterable columns answer 422 (`ClientInputError`), never echoing values. - **Token claims are validated.** `JwtAuth` requires `user_id` to be a non-empty string or an int @@ -188,7 +187,7 @@ valid ones (`RATE_LIMIT_KEYS`). ownership and size limits still apply. `PUT ... /messages/{message_id}` sets `metadata["feedback"]` only (body `{"feedback": {...}}`, 8 KB, depth 10, `checkpointer:write`; calls `aupdate_message_metadata`, 501 when the checkpointer lacks it, 404 for a missing message). -- **Client input rules** (`core/auth/request_config.py`): `context_summary` and `execution_meta` +- **Client input rules** (`core/validation/request_config.py`): `context_summary` and `execution_meta` are never client-writable (`reject_builtin_state_keys`, 422); client messages are user-only (`client_message_error`: no `assistant`/`system`; a `tool` message only as tool results, which must answer a pending remote call, `GraphService._check_tool_results`, also for PUT state diff --git a/SECURITY.md b/SECURITY.md index 7d7511e..4140566 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -83,8 +83,7 @@ CORS combined with credentials in production. For anything else, in production s - `runs.timeout` and `runs.max_concurrent_per_user` to bound what one account can hold open - `token_revocation` if you need logout or stolen-token revocation before a JWT expires - `rate_limit.trusted_proxies` to your proxy's networks when `trusted_proxy_headers` is on -- Keep `graph_introspection: "scoped"` (the production default) unless every user may see - the graph's structure and tool schemas +- Grant `graph:read` only to users who may see the graph's structure and tool schemas ### Running more than one worker diff --git a/config-editor-ui/src/icons.js b/config-editor-ui/src/icons.js index 27647a4..6aac838 100644 --- a/config-editor-ui/src/icons.js +++ b/config-editor-ui/src/icons.js @@ -40,7 +40,7 @@ export const ICONS = { export const SECTION_ICONS = { "get-running": "flag", environment: "terminal", - core: "flag", auth: "lock", authorization: "shield", cache_store: "gauge", graph_introspection: "code", + core: "flag", auth: "lock", cache_store: "gauge", observability: "activity", runs: "play", websocket: "bolt", ag_ui: "external", remote_tools: "wrench", media: "file", checkpointer: "layers", id_generator: "key", thread_name_generator: "file", store: "box", injectq: "arrows", test: "flask", evaluation: "target", diff --git a/config-editor-ui/src/pages.jsx b/config-editor-ui/src/pages.jsx index 92f48a6..d00fce0 100644 --- a/config-editor-ui/src/pages.jsx +++ b/config-editor-ui/src/pages.jsx @@ -7,7 +7,7 @@ import { } from "./store.js"; import { Callout, Icon, IssueCallout, Segmented } from "./ui.jsx"; import { - AuthWidget, AuthorizationWidget, CacheStoreWidget, ChoiceWidget, RemoteToolsWidget, + AuthWidget, CacheStoreWidget, ChoiceWidget, RemoteToolsWidget, } from "./widgets.jsx"; const Heading = ({ children, code }) => ( @@ -48,10 +48,11 @@ function offText(id) { function authSummary() { const a = store.config.auth; - if (a === undefined || a === null) return { text: offText("auth") }; - if (a === "jwt") return { text: "JWT bearer tokens." }; - if (a && typeof a === "object") return { text: `Custom backend${a.path ? `: ${a.path}` : "."}` }; - return { text: String(a) }; + const method = a === "jwt" ? "jwt" : isObject(a) ? a.method ?? "none" : "none"; + if (method === "none") return { text: offText("auth") }; + const roles = isObject(a) && isObject(a.roles) ? Object.keys(a.roles).length : 0; + const base = method === "jwt" ? "JWT bearer tokens" : `Custom backend${a.path ? `: ${a.path}` : ""}`; + return { text: `${base}${roles ? `, ${roles} ${roles === 1 ? "role" : "roles"}` : ""}.` }; } function cacheSummary() { @@ -150,8 +151,7 @@ function EnvDisclosure({ vars, open }) { } function SectionWidget({ sec }) { - if (sec.widget === "auth") return ; - if (sec.widget === "authorization") return ; + if (sec.widget === "auth") return ; if (sec.widget === "remote_tools") return ; if (sec.widget === "cache_store") return ; if (sec.widget === "id_generator" || sec.widget === "thread_name_generator") return ; @@ -162,7 +162,11 @@ function SectionWidget({ sec }) { export function generalIssues(sec) { const inline = new Set([...sec.fields.map((f) => f.path.join(".")), ...Object.keys(WIDGET_FIELD_IDS)]); const tabbed = new Set(["cache_store.rate_limit", "cache_store.token_revocation"]); - return issuesFor(sec.id).filter((i) => !inline.has(i.path) && !(sec.id === "cache_store" && tabbed.has(i.path))); + // The Auth widget shows issues for its roles and system identities beside their rows. + const authRow = (path) => path.startsWith("auth.roles.") || path.startsWith("auth.system_identities."); + return issuesFor(sec.id).filter( + (i) => !inline.has(i.path) && !(sec.id === "cache_store" && tabbed.has(i.path)) && !(sec.id === "auth" && authRow(i.path)), + ); } // Fields and widget of a section, no heading. Shared by the page and search results. diff --git a/config-editor-ui/src/store.js b/config-editor-ui/src/store.js index b85c61c..a117c5e 100644 --- a/config-editor-ui/src/store.js +++ b/config-editor-ui/src/store.js @@ -7,7 +7,7 @@ // are pages of their own rather than schema sections. export const NAV = [ { label: null, items: ["get-running"] }, - { label: "Before production", items: ["auth", "authorization", "cache_store", "observability", "graph_introspection"] }, + { label: "Before production", items: ["auth", "cache_store", "observability"] }, { label: "Runs and streaming", items: ["runs", "websocket", "ag_ui", "remote_tools", "media"] }, { label: "Project wiring", items: ["checkpointer", "id_generator", "thread_name_generator", "store", "injectq"] }, { label: "Developer tools", items: ["test", "evaluation"] }, @@ -26,7 +26,7 @@ export const PAGES = { }; // Issues a widget renders beside its own input rather than at the top of the section. -export const WIDGET_FIELD_IDS = { "auth.path": "auth-path", authorization: "authz-path" }; +export const WIDGET_FIELD_IDS = { "auth.path": "auth-path" }; // The page token arrives in the URL hash (#token=...). Read it once, keep it in memory, and // mirror it to sessionStorage so a reload still works after the hash switches to page routing. @@ -154,6 +154,7 @@ export const fieldChanged = (f) => { }; export const sectionChanged = (sec) => { if (sec.id === "core") return false; + if (sec.id === "auth") return store.config.auth != null; return sec.optional ? isOn(sec) : sec.fields.some(fieldChanged); }; // Highest-severity issue of a section, or null. diff --git a/config-editor-ui/src/widgets.jsx b/config-editor-ui/src/widgets.jsx index 9a2ae71..37e710c 100644 --- a/config-editor-ui/src/widgets.jsx +++ b/config-editor-ui/src/widgets.jsx @@ -7,36 +7,336 @@ import { // ------------------------------------------------------------------ auth -export function AuthWidget() { - const value = store.config.auth; - const mode = isObject(value) ? "custom" : "jwt"; +const REACH_OPTIONS = [["", "Choose reach"], ["own", "own"], ["tenant", "tenant"], ["all", "all"]]; +const nonEmpty = (v) => (Array.isArray(v) ? v.length > 0 : isObject(v) && Object.keys(v).length > 0); + +// The auth object as the editor edits it, whichever shape the file holds. +function readAuth() { + const a = store.config.auth; + if (a === "jwt") return { method: "jwt" }; + return isObject(a) ? a : { method: "none" }; +} + +// Edit a copy of the auth object. Method none is saved as null. +function editAuth(mutate) { + commit((s) => { + const next = { ...readAuth() }; + mutate(next); + s.config.auth = next.method === "none" ? null : next; + }); +} + +// Set a key, or drop it when the value is empty, so the file only holds what is used. +function put(auth, key, value) { + if (value === null || value === undefined || value === "" || (typeof value !== "string" && !nonEmpty(value))) delete auth[key]; + else auth[key] = value; +} + +function sourceOf(a) { + if (a.roles_claim !== undefined || nonEmpty(a.roles)) return "roles"; + if (a.permissions_claim !== undefined) return "permissions"; + return "default"; +} + +const issuesUnder = (prefix) => store.issues.filter((i) => i.path === prefix || i.path.startsWith(`${prefix}.`)); + +function Issues({ prefix }) { + return issuesUnder(prefix).map((issue, n) => ); +} + +// Renames apply on blur or Enter; the draft lives here so re-renders do not reset it. +function NameInput({ id, label, name, onRename }) { + const [draft, setDraft] = useState(name); + useEffect(() => setDraft(name), [name]); + return ( +
+ + setDraft(e.currentTarget.value)} + onChange={(e) => { if (!onRename(e.currentTarget.value.trim())) setDraft(name); }} + /> +
+ ); +} + +const MiniLabel = ({ children }) => {children}; + +// Rename a key of an object keeping its order; false when the new name is empty or taken. +function renameKey(obj, from, to) { + if (!to || (to !== from && to in obj)) return null; + return Object.fromEntries(Object.entries(obj).map(([k, v]) => [k === from ? to : k, v])); +} + +export function AuthWidget({ sec }) { + const auth = readAuth(); + const method = auth.method ?? "none"; + const source = sourceOf(auth); return (
commit((s) => { s.config.auth = m === "jwt" ? "jwt" : { method: "custom", path: "" }; })} + onPick={(m) => editAuth((a) => { + a.method = m; + if (m === "custom") a.path = a.path ?? ""; + else delete a.path; + })} /> - {mode === "custom" ? ( - + {method === "custom" ? ( +
commit((s) => { s.config.auth = { ...s.config.auth, method: "custom", path: v }; })} + onInput={(v) => editAuth((a) => { a.path = v; })} />
) : null} + {method !== "none" ? ( + <> + + editAuth((a) => { + delete a.roles_claim; delete a.permissions_claim; delete a.roles; delete a.default_role; + if (m === "roles") { a.roles_claim = "roles"; a.roles = { member: { permissions: [], reach: "own" } }; } + if (m === "permissions") a.permissions_claim = "permissions"; + })} + /> + + {source === "roles" ? : null} + {source === "permissions" ? ( + +
+ editAuth((a) => { a.permissions_claim = v; })} + /> +
+ + ) : null} + +
+
+ +
+ editAuth((a) => put(a, "tenant_claims", next))} + /> +
+ +
+ {source === "roles" ? : null} +
+ + + + ) : null}
); } +function DefaultRole({ auth }) { + const names = Object.keys(isObject(auth.roles) ? auth.roles : {}); + return ( + + +
+ setRole(name, { reach: v })} + /> + +
+
+ Permissions + setRole(name, { permissions: next })} + /> +
+
+ Inherits + n !== name)} + onChange={(next) => { const updated = { ...role }; put(updated, "inherits", next); setRoles({ ...roles, [name]: updated }); }} + /> +
+ + + ))} + + ) : ( +

No roles yet. Add one to use roles.

+ )} +
+

+ Reach: own the user's rows, tenant every row + of their tenant, all every row. +

+ +
+ + + ); +} + +function SystemIdentities({ auth }) { + const identities = isObject(auth.system_identities) ? auth.system_identities : {}; + const entries = Object.entries(identities); + const roleNames = Object.keys(isObject(auth.roles) ? auth.roles : {}); + const setAll = (next) => editAuth((a) => put(a, "system_identities", next)); + const setOne = (name, patch) => setAll({ ...identities, [name]: { ...identities[name], ...patch } }); + const add = () => { + let n = entries.length + 1; + while (`job${n}` in identities) n++; + setAll({ ...identities, [`job${n}`]: { roles: [], tenant: {} } }); + }; + // Tenant is {claim: value}; the chips hold "claim=value" text. + const tenantChips = (tenant) => Object.entries(tenant ?? {}).map(([k, v]) => `${k}=${v}`); + const parseTenant = (chips) => Object.fromEntries( + chips.filter((c) => c.includes("=")).map((c) => [c.slice(0, c.indexOf("=")).trim(), c.slice(c.indexOf("=") + 1).trim()]), + ); + + return ( + +
+ {entries.length ? ( +
+ {entries.map(([name, identity], index) => ( +
+
+ { const next = renameKey(identities, name, to); if (next) setAll(next); return !!next; }} + /> + +
+
+ Roles + setOne(name, { roles: next })} + /> +
+
+ Tenant, as claim=value + setOne(name, { tenant: parseTenant(next) })} + /> +
+ +
+ ))} +
+ ) : ( +

None. Scheduled jobs and other non-user callers can run as one.

+ )} +
+ +
+
+ + ); +} + // ------------------------------------------------------------------ cache store const CACHE_CARDS = [ @@ -341,171 +641,6 @@ export function ChoiceWidget({ sec }) { ); } -// ------------------------------------------------------------------ authorization - -function authzModeOf(v) { - if (isObject(v)) return "rbac"; - if (typeof v === "string") { - const key = v.trim().toLowerCase(); - if (key === "ownership") return "ownership"; - if (["allow_all", "default", "none"].includes(key)) return "allow_all"; - } - return v === undefined || v === null ? null : "custom"; -} - -const AUTHZ_PRESETS = { - ownership: () => "ownership", - allow_all: () => "allow_all", - custom: () => "", - rbac: () => ({ - backend: "rbac", - roles: { admin: ["*"], member: ["graph:invoke", "graph:stream", "graph:read", "checkpointer:read"] }, - default_scopes: [], - isolation: "owner", - }), -}; - -export function AuthorizationWidget({ sec }) { - const value = store.config.authorization; - const mode = authzModeOf(value) ?? "ownership"; - return ( -
- - commit((s) => { s.config.authorization = AUTHZ_PRESETS[m](); })} - /> - - {mode === "custom" ? ( - -
- commit((s) => { s.config.authorization = v; })} - /> -
- - ) : null} - {mode === "rbac" ? : null} -
- ); -} - -// Renames apply on blur or Enter; the draft lives here so re-renders do not reset it. -function RoleName({ id, name, onRename }) { - const [draft, setDraft] = useState(name); - useEffect(() => setDraft(name), [name]); - return ( -
- - setDraft(e.currentTarget.value)} - onChange={(e) => { if (!onRename(e.currentTarget.value.trim())) setDraft(name); }} - /> -
- ); -} - -function Rbac({ sec, value }) { - const roleKey = "roles" in value || !("role_scopes" in value) ? "roles" : "role_scopes"; - const roles = isObject(value[roleKey]) ? value[roleKey] : {}; - const scopeOptions = ["*", ...(sec.scopes || [])]; - const entries = Object.entries(roles); - const setRoles = (next) => commit(() => { value[roleKey] = next; }); - const addRole = () => { - let n = entries.length + 1; - while (`role${n}` in roles) n++; - setRoles({ ...roles, [`role${n}`]: [] }); - }; - - return ( - <> - -
- - {entries.length ? ( -
- {entries.map(([name, scopes], index) => ( -
- { - if (!renamed || (renamed !== name && renamed in roles)) return false; - setRoles(Object.fromEntries(Object.entries(roles).map(([k, v]) => [k === name ? renamed : k, v]))); - return true; - }} - /> - setRoles({ ...roles, [name]: next })} - /> - -
- ))} -
- ) : ( -

No roles yet. Users only get the default scopes.

- )} -
-

- Read from the user's roles (or role) claim. * grants every scope. -

- -
-
- - -
-
- -
- commit(() => { value.default_scopes = next; })} - /> -
-
- - -
-