mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-13 02:14:23 +00:00
fix(pat): close message replay to restricted tokens, filter builtin tools, reject non-object bodies
Message tail and the ASGI reconnect stream cannot tie a message to an allowlist, so any token with a resource filter is refused there. The tools listing now applies the allowlist to default and builtin rows as well. Token creation answers 400 instead of 500 for a JSON body that is not an object.
This commit is contained in:
1 parent
17d66c061d
commit
934ae1afb3
8 files changed
+45
-7
No files matched your search
@@ -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/<id>/tail` and `/api/messages/<id>/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`):
|
||||
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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/<string:message_id>/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",)),
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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"
|
||||
@@ -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
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in new issue
Block a user