diff --git a/docs/content/Extensions/personal-access-tokens.mdx b/docs/content/Extensions/personal-access-tokens.mdx index 24ab642d..c533b528 100644 --- a/docs/content/Extensions/personal-access-tokens.mdx +++ b/docs/content/Extensions/personal-access-tokens.mdx @@ -131,7 +131,7 @@ For a restricted family the token: - can read, update and delete only the listed resources, and listings show only those; - **cannot create** new resources of that family, since a new resource would be outside the list; - cannot attach a resource outside the list to something else, for example set an agent's source to a source the token may not use; -- is refused (`403`, `"error": "resource_not_allowed"`) wherever DocsGPT cannot prove the request stays inside the list. Schedules, conversations and analytics are closed to agent-restricted tokens for that reason, and `/api/sources/paginated` is closed to source-restricted tokens (use `/api/sources`). +- is refused (`403`, `"error": "resource_not_allowed"`) wherever DocsGPT cannot prove the request stays inside the list. Schedules, conversations and analytics are closed to agent-restricted tokens for that reason, the message replay endpoints (`/api/messages//tail` and `/api/messages//events`) are closed to every restricted token, and `/api/sources/paginated` is closed to source-restricted tokens (use `/api/sources`). Restrictions and chat (`chat:run`): diff --git a/docsgpt/api/asgi_auth.py b/docsgpt/api/asgi_auth.py index aa2ac5bd..a65e83ab 100644 --- a/docsgpt/api/asgi_auth.py +++ b/docsgpt/api/asgi_auth.py @@ -31,7 +31,8 @@ async def authenticate( request: The incoming Starlette request. pat_scope: Scope a personal access token needs for this route. Left unset, the route rejects PATs outright (deny by default, matching - the Flask rule table in ``docsgpt/api/pat/rules.py``). + the Flask rule table in ``docsgpt/api/pat/rules.py``). A token with + a resource filter is always rejected. Returns: tuple: ``(claims, None)`` for an authenticated caller, ``(None, None)`` @@ -52,6 +53,17 @@ async def authenticate( {"success": False, "message": "Token lacks the required scope", "error": "insufficient_scope"}, status_code=403, ) + if decoded.get("resource_filter"): + # These routes sit outside the Flask rule table and cannot tie what + # they serve to an allowlist, so a restricted token is kept out. + return None, JSONResponse( + { + "success": False, + "message": "This endpoint is not available to a resource-restricted token", + "error": "resource_not_allowed", + }, + status_code=403, + ) return decoded, None # The denylist is a sync Redis read; keep it off the event loop. if settings.AUTH_TYPE == "oidc" and await anyio.to_thread.run_sync(oidc_session_denied, decoded): diff --git a/docsgpt/api/pat/routes.py b/docsgpt/api/pat/routes.py index 84360a1b..ad22cf6d 100644 --- a/docsgpt/api/pat/routes.py +++ b/docsgpt/api/pat/routes.py @@ -113,7 +113,11 @@ class PersonalAccessTokens(Resource): if not auth_type_supports_pats(): return _error("Personal access tokens are not available on this server", 403) - body = request.get_json(silent=True) or {} + body = request.get_json(silent=True) + if body is None: + body = {} + if not isinstance(body, dict): + return _error("Request body must be a JSON object", 400) name = body.get("name") if not isinstance(name, str) or not name.strip(): return _error("name is required", 400) diff --git a/docsgpt/api/pat/rules.py b/docsgpt/api/pat/rules.py index f340d698..6794390b 100644 --- a/docsgpt/api/pat/rules.py +++ b/docsgpt/api/pat/rules.py @@ -53,6 +53,8 @@ def _rule(scope: Optional[str] = None, *ids: Locator, any_of: tuple[str, ...] = return Rule(scopes=scopes, family=family, ids=tuple(ids), **kwargs) +_ALL_FAMILIES = ("agents", "sources", "prompts", "tools", "workflows") + # Ids of other families that agent create/update accept in their JSON-or-form body. _AGENT_BODY_REFS = ( ("sources", (BODY, "source")), @@ -216,8 +218,9 @@ RULES: dict[tuple[str, str], Rule] = { ("/api/get_conversations", "GET"): _rule("conversations:read", blocked_by=("agents",)), ("/api/search_conversations", "GET"): _rule("conversations:read", blocked_by=("agents",)), ("/api/get_single_conversation", "GET"): _rule("conversations:read", blocked_by=("agents",)), + # A message cannot be tied to an allowlist from here, so any restricted token is kept out. ("/api/messages//tail", "GET"): _rule( - any_of=("conversations:read", "chat:run"), family=None + any_of=("conversations:read", "chat:run"), family=None, blocked_by=_ALL_FAMILIES ), ("/api/delete_conversation", "POST"): _rule("conversations:write", blocked_by=("agents",)), ("/api/delete_all_conversations", "GET"): _rule("conversations:write", blocked_by=("agents",)), diff --git a/docsgpt/api/user/tools/routes.py b/docsgpt/api/user/tools/routes.py index 8b70abac..ddc56916 100644 --- a/docsgpt/api/user/tools/routes.py +++ b/docsgpt/api/user/tools/routes.py @@ -266,9 +266,6 @@ class GetTools(Resource): shaped = _shape_tool(row, ownership="team", force_strip_secret=True) shaped["team_access"] = team_shared.get(str(row["id"])) user_tools.append(shaped) - # A resource-restricted token sees only its allowed tools; the - # default and builtin rows appended below belong to no user. - user_tools = filter_listing(request, "tools", user_tools) # ``scheduler`` is dual-registered (default chat tool + agent- # selectable builtin) and resolves to the same synthetic uuid5 id. @@ -298,6 +295,9 @@ class GetTools(Resource): builtin_copy.get("name") in WORKFLOW_ONLY_BUILTINS ) user_tools.append(builtin_copy) + # A resource-restricted token sees only its allowed tools. Default + # and builtin rows have ids too, so they follow the same allowlist. + user_tools = filter_listing(request, "tools", user_tools) except Exception as err: current_app.logger.error(f"Error getting user tools: {err}", exc_info=True) return make_response(jsonify({"success": False}), 400) diff --git a/tests/api/test_asgi_auth.py b/tests/api/test_asgi_auth.py index 341ded0d..d40fae0e 100644 --- a/tests/api/test_asgi_auth.py +++ b/tests/api/test_asgi_auth.py @@ -156,3 +156,11 @@ class TestPersonalAccessTokens: decoded, error = await asgi_auth.authenticate(_request(), pat_scope="chat:run") denied.assert_not_called() assert error is None and decoded is not None + + async def test_restricted_token_is_refused_even_with_the_scope(self): + claims = dict(_PAT_CLAIMS, resource_filter={"agents": ["a1"]}) + with patch.object(asgi_auth, "handle_auth", return_value=claims): + decoded, error = await asgi_auth.authenticate(_request(), pat_scope="chat:run") + assert decoded is None + assert error.status_code == 403 + assert json.loads(error.body)["error"] == "resource_not_allowed" diff --git a/tests/api/test_pat_routes.py b/tests/api/test_pat_routes.py index cd157284..be07b7d5 100644 --- a/tests/api/test_pat_routes.py +++ b/tests/api/test_pat_routes.py @@ -118,6 +118,11 @@ class TestCreate: def test_validation(self, client, db, body): assert _create(client, **body).status_code == 400 + @pytest.mark.parametrize("body", [[1], "text", 5]) + def test_non_object_body_is_a_client_error(self, client, db, body): + with _session(): + assert client.post("/api/user/tokens", json=body).status_code == 400 + def test_duplicate_name_conflicts(self, client, db): assert _create(client).status_code == 201 assert _create(client).status_code == 409 diff --git a/tests/api/test_pat_rules.py b/tests/api/test_pat_rules.py index 3a3cd75a..c6c9f0b8 100644 --- a/tests/api/test_pat_rules.py +++ b/tests/api/test_pat_rules.py @@ -230,6 +230,12 @@ class TestResourceRestrictions: assert _denied(_call(client, "GET", f"/api/agents/{AGENT_A}/schedules", claims)) is None assert _denied(_call(client, "GET", f"/api/agents/{AGENT_B}/schedules", claims)) == "resource_not_allowed" + @pytest.mark.parametrize("family", ["agents", "sources", "prompts", "tools", "workflows"]) + def test_message_tail_is_closed_to_any_restricted_token(self, client, family): + claims = _claims(["chat:run"], {family: [AGENT_A]}) + assert _denied(_call(client, "GET", "/api/messages/m1/tail", claims)) == "resource_not_allowed" + assert _denied(_call(client, "GET", "/api/messages/m1/tail", _claims(["chat:run"]))) is None + def test_sql_paged_listing_is_closed_to_restricted_tokens(self, client): claims = self._restricted(["sources:read"], sources=[SOURCE_A]) assert _denied(_call(client, "GET", "/api/sources/paginated", claims)) == "resource_not_allowed"