mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-11 03:12:55 +00:00
fix(pat): align replay scopes, keep 404/405, retire expired names, document the PAT_ENABLED switch
The ASGI message events route now accepts the same scopes as its Flask sibling (conversations:read or chat:run) through a shared constant. A token request that fails routing gets Flask's 404/405 instead of a 403. Creating a token retires an expired token that still held the name. allowed_ids uses is_pat instead of a bare literal. PAT_ENABLED is documented as the master switch it is: turning it off stops every existing token from authenticating. The docs explain that sources are matched by name (oldest wins) and point CI flows at sources upload --replace.
This commit is contained in:
1 parent
934ae1afb3
commit
7ddb5f6400
12 files changed
+105
-15
No files matched your search
@@ -136,7 +136,7 @@ Bearer token for IdP SCIM clients (required when SCIM is enabled).
|
||||
|
||||
Type `bool`, default `true`.
|
||||
|
||||
Allow users to create personal access tokens. Tokens are only issued under AUTH_TYPE=oidc or unset (None); simple_jwt and session_jwt have no stable user identity to bind a token to.
|
||||
Master switch for personal access tokens. When false, no token can be created AND every existing token stops authenticating immediately (pipelines using them get 401); tokens are kept and work again when re-enabled. Tokens are only available under AUTH_TYPE=oidc or unset (None); switching to simple_jwt or session_jwt disables them the same way.
|
||||
|
||||
### `PAT_DEFAULT_LIFETIME_DAYS`
|
||||
|
||||
|
||||
@@ -69,6 +69,8 @@ curl -X POST "$DOCSGPT_URL/api/import_agent/plan" \
|
||||
-d "$(jq -Rs '{yaml: .}' support-bot.agent.yaml)"
|
||||
```
|
||||
|
||||
Sources are matched by **name**, and when several of your sources share a name the oldest one wins. A pipeline that re-uploads documentation on every push should therefore upload with `docsgpt-cli sources upload ... --wait --replace` (which removes the older same-named sources) and run `agents apply` afterwards, or the agent stays bound to the first upload.
|
||||
|
||||
`/api/import_agent/plan` is a dry run that reports whether the agent would be created or updated and how each referenced source, tool and prompt resolves. `/api/import_agent` applies it. An agent is matched by `metadata.id`, then `metadata.slug`; when nothing matches, a new draft agent is created.
|
||||
|
||||
### GitHub Actions example
|
||||
@@ -83,7 +85,9 @@ jobs:
|
||||
env:
|
||||
DOCSGPT_URL: ${{ vars.DOCSGPT_URL }}
|
||||
DOCSGPT_TOKEN: ${{ secrets.DOCSGPT_TOKEN }}
|
||||
run: docsgpt-cli agents apply -f agents/
|
||||
run: |
|
||||
docsgpt-cli sources upload docs/*.md --name "Product docs" --wait --replace
|
||||
docsgpt-cli agents apply -f agents/
|
||||
```
|
||||
|
||||
## Scopes
|
||||
@@ -149,19 +153,23 @@ Restrictions and `agents apply`: a token restricted to specific agents can apply
|
||||
- Tokens of a deactivated user (through the admin API or SCIM) stop working immediately and work again if the user is reactivated.
|
||||
- Token creation and revocation are recorded in the authentication audit log (`pat_created`, `pat_revoked`), visible to admins.
|
||||
|
||||
An expired token's name can be reused: creating a token with that name retires the expired one.
|
||||
|
||||
Each user may hold up to `PAT_MAX_PER_USER` live tokens (25 by default). The token list shows when and from which IP address each token was last used.
|
||||
|
||||
## Operator settings
|
||||
|
||||
| Setting | Default | Purpose |
|
||||
| --- | --- | --- |
|
||||
| `PAT_ENABLED` | `true` | Allow users to create and use personal access tokens |
|
||||
| `PAT_ENABLED` | `true` | Master switch. When `false`, tokens cannot be created **and every existing token stops authenticating immediately** |
|
||||
| `PAT_DEFAULT_LIFETIME_DAYS` | `90` | Lifetime of a token created without an explicit expiry |
|
||||
| `PAT_MAX_LIFETIME_DAYS` | `365` | Longest lifetime a user may request |
|
||||
| `PAT_ALLOW_NON_EXPIRING` | `false` | Let users create tokens that never expire |
|
||||
| `PAT_MAX_PER_USER` | `25` | Maximum number of live tokens per user |
|
||||
|
||||
Personal access tokens need a stable user identity, so they are available with `AUTH_TYPE=oidc` and with authentication disabled (single-user self-hosting). They are not available with `simple_jwt` or `session_jwt`. See the [Settings Reference](/Deploying/Settings-Reference) for details.
|
||||
Personal access tokens need a stable user identity, so they are available with `AUTH_TYPE=oidc` and with authentication disabled (single-user self-hosting). They are not available with `simple_jwt` or `session_jwt`.
|
||||
|
||||
Turning `PAT_ENABLED` off, or switching `AUTH_TYPE` to `simple_jwt` or `session_jwt`, is not limited to the settings page: every pipeline that uses a token starts getting `401` right away. Tokens are not deleted and work again once the setting is restored. See the [Settings Reference](/Deploying/Settings-Reference) for details.
|
||||
|
||||
## Management API
|
||||
|
||||
|
||||
@@ -9,7 +9,7 @@ from __future__ import annotations
|
||||
|
||||
import uuid
|
||||
from contextvars import Token
|
||||
from typing import Optional, Tuple
|
||||
from typing import Optional, Sequence, Tuple, Union
|
||||
|
||||
import anyio
|
||||
from starlette.requests import Request
|
||||
@@ -23,13 +23,13 @@ from docsgpt.core.settings import settings
|
||||
|
||||
|
||||
async def authenticate(
|
||||
request: Request, *, pat_scope: Optional[str] = None
|
||||
request: Request, *, pat_scope: Union[str, Sequence[str], None] = None
|
||||
) -> Tuple[Optional[dict], Optional[JSONResponse]]:
|
||||
"""Decode the caller's JWT the way Flask's ``authenticate_request`` does.
|
||||
|
||||
Args:
|
||||
request: The incoming Starlette request.
|
||||
pat_scope: Scope a personal access token needs for this route. Left
|
||||
pat_scope: Scope (or any-of scopes) 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``). A token with
|
||||
a resource filter is always rejected.
|
||||
@@ -48,7 +48,8 @@ async def authenticate(
|
||||
if is_pat(decoded):
|
||||
# A PAT lookup already excludes revoked tokens and deactivated users,
|
||||
# so the session denylist below does not apply to it.
|
||||
if pat_scope is None or pat_scope not in (decoded.get("scopes") or []):
|
||||
accepted = (pat_scope,) if isinstance(pat_scope, str) else tuple(pat_scope or ())
|
||||
if not set(accepted).intersection(decoded.get("scopes") or []):
|
||||
return None, JSONResponse(
|
||||
{"success": False, "message": "Token lacks the required scope", "error": "insufficient_scope"},
|
||||
status_code=403,
|
||||
|
||||
@@ -25,6 +25,7 @@ from starlette.responses import Response
|
||||
from starlette.routing import Route
|
||||
|
||||
from docsgpt.api.asgi_auth import authenticate, bind_log_context, json_error
|
||||
from docsgpt.api.pat.rules import MESSAGE_REPLAY_SCOPES
|
||||
from docsgpt.api.asgi_stream import sse_response
|
||||
from docsgpt.core.settings import settings
|
||||
from docsgpt.storage.db.session import db_readonly
|
||||
@@ -94,7 +95,8 @@ async def stream_message_events(request: Request) -> Response:
|
||||
"""
|
||||
# Same JWT decoder and OIDC revocation check as the Flask routes. With
|
||||
# AUTH_TYPE unset the caller resolves to ``{"sub": "local"}``.
|
||||
decoded, error = await authenticate(request, pat_scope="chat:run")
|
||||
# Same scopes as its Flask sibling GET /api/messages/<id>/tail.
|
||||
decoded, error = await authenticate(request, pat_scope=MESSAGE_REPLAY_SCOPES)
|
||||
if error is not None:
|
||||
return error
|
||||
user_id = decoded.get("sub") if isinstance(decoded, dict) else None
|
||||
|
||||
@@ -139,6 +139,7 @@ class PersonalAccessTokens(Resource):
|
||||
return _error(
|
||||
f"Token limit reached ({settings.PAT_MAX_PER_USER}); revoke one first", 409
|
||||
)
|
||||
repo.retire_expired_name(user_id, name)
|
||||
if repo.name_in_use(user_id, name):
|
||||
return _error("A token with this name already exists", 409)
|
||||
row = repo.create(
|
||||
|
||||
@@ -25,6 +25,8 @@ import uuid
|
||||
from dataclasses import dataclass
|
||||
from typing import Any, Callable, Iterable, Optional
|
||||
|
||||
from docsgpt.api.pat.tokens import is_pat
|
||||
|
||||
VIEW, QUERY, JSON, FORM, BODY = "view", "query", "json", "form", "body"
|
||||
|
||||
Locator = tuple[str, str]
|
||||
@@ -53,6 +55,11 @@ def _rule(scope: Optional[str] = None, *ids: Locator, any_of: tuple[str, ...] =
|
||||
return Rule(scopes=scopes, family=family, ids=tuple(ids), **kwargs)
|
||||
|
||||
|
||||
#: Scopes that admit a token to message replay. Shared by the Flask tail route
|
||||
#: below and its ASGI sibling GET /api/messages/<id>/events (docsgpt/api/async_sse.py),
|
||||
#: which sits outside this table.
|
||||
MESSAGE_REPLAY_SCOPES = ("conversations:read", "chat:run")
|
||||
|
||||
_ALL_FAMILIES = ("agents", "sources", "prompts", "tools", "workflows")
|
||||
|
||||
# Ids of other families that agent create/update accept in their JSON-or-form body.
|
||||
@@ -220,7 +227,7 @@ RULES: dict[tuple[str, str], Rule] = {
|
||||
("/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, blocked_by=_ALL_FAMILIES
|
||||
any_of=MESSAGE_REPLAY_SCOPES, 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",)),
|
||||
@@ -360,7 +367,11 @@ def _all_allowed(ids: Iterable[str], allowed: Iterable[str]) -> bool:
|
||||
def authorize(request, decoded_token: dict) -> Optional[tuple[dict, int]]:
|
||||
"""Check a PAT request against the table. ``None`` allows; otherwise ``(body, status)``."""
|
||||
url_rule = getattr(request, "url_rule", None)
|
||||
rule = RULES.get((url_rule.rule, request.method)) if url_rule is not None else None
|
||||
if url_rule is None:
|
||||
# Routing failed (unknown path or wrong method): no view will run, so
|
||||
# let Flask answer 404/405 instead of masking it with a 403.
|
||||
return None
|
||||
rule = RULES.get((url_rule.rule, request.method))
|
||||
if rule is None:
|
||||
return (
|
||||
{
|
||||
@@ -420,8 +431,8 @@ def allowed_ids(request, family: str) -> Optional[set[str]]:
|
||||
|
||||
Used by listing routes (``listing=True``) and by ``in_route`` handlers.
|
||||
"""
|
||||
decoded = getattr(request, "decoded_token", None) or {}
|
||||
if decoded.get("auth_method") != "pat":
|
||||
decoded = getattr(request, "decoded_token", None)
|
||||
if not is_pat(decoded):
|
||||
return None
|
||||
ids = (decoded.get("resource_filter") or {}).get(family)
|
||||
if ids is None:
|
||||
|
||||
@@ -89,8 +89,10 @@ class AuthSettings(SettingsGroup):
|
||||
PAT_ENABLED: bool = Field(
|
||||
default=True,
|
||||
description=(
|
||||
"Allow users to create personal access tokens. Tokens are only issued under AUTH_TYPE=oidc or "
|
||||
"unset (None); simple_jwt and session_jwt have no stable user identity to bind a token to."
|
||||
"Master switch for personal access tokens. When false, no token can be created AND every existing "
|
||||
"token stops authenticating immediately (pipelines using them get 401); tokens are kept and work "
|
||||
"again when re-enabled. Tokens are only available under AUTH_TYPE=oidc or unset (None); switching "
|
||||
"to simple_jwt or session_jwt disables them the same way."
|
||||
),
|
||||
)
|
||||
PAT_DEFAULT_LIFETIME_DAYS: int = Field(
|
||||
|
||||
@@ -87,6 +87,23 @@ class PersonalAccessTokensRepository:
|
||||
{"user_id": user_id},
|
||||
).scalar_one()
|
||||
|
||||
def retire_expired_name(self, user_id: str, name: str) -> int:
|
||||
"""Revoke an expired token holding ``name`` so the name can be reused.
|
||||
|
||||
An expired token can no longer authenticate but keeps ``status = 'active'``,
|
||||
and the unique index on live names would otherwise reserve its name forever.
|
||||
"""
|
||||
result = self._conn.execute(
|
||||
text(
|
||||
"UPDATE personal_access_tokens "
|
||||
"SET status = 'revoked', revoked_at = now(), revoke_reason = 'expired' "
|
||||
"WHERE user_id = :user_id AND name = :name AND status = 'active' "
|
||||
"AND expires_at IS NOT NULL AND expires_at <= now()"
|
||||
),
|
||||
{"user_id": user_id, "name": name},
|
||||
)
|
||||
return result.rowcount
|
||||
|
||||
def name_in_use(self, user_id: str, name: str) -> bool:
|
||||
return (
|
||||
self._conn.execute(
|
||||
|
||||
@@ -164,3 +164,19 @@ class TestPersonalAccessTokens:
|
||||
assert decoded is None
|
||||
assert error.status_code == 403
|
||||
assert json.loads(error.body)["error"] == "resource_not_allowed"
|
||||
|
||||
async def test_any_of_several_scopes_admits_the_token(self):
|
||||
claims = dict(_PAT_CLAIMS, scopes=["conversations:read"])
|
||||
with patch.object(asgi_auth, "handle_auth", return_value=claims):
|
||||
decoded, error = await asgi_auth.authenticate(
|
||||
_request(), pat_scope=("conversations:read", "chat:run")
|
||||
)
|
||||
assert error is None and decoded["sub"] == "alice"
|
||||
|
||||
async def test_message_events_accepts_the_same_scopes_as_message_tail(self):
|
||||
from docsgpt.api import async_sse
|
||||
from docsgpt.api.pat import rules
|
||||
|
||||
tail = rules.RULES[("/api/messages/<string:message_id>/tail", "GET")]
|
||||
assert tail.scopes == rules.MESSAGE_REPLAY_SCOPES
|
||||
assert async_sse.MESSAGE_REPLAY_SCOPES is rules.MESSAGE_REPLAY_SCOPES
|
||||
@@ -127,6 +127,15 @@ class TestCreate:
|
||||
assert _create(client).status_code == 201
|
||||
assert _create(client).status_code == 409
|
||||
|
||||
def test_expired_token_does_not_reserve_its_name(self, client, db):
|
||||
assert _create(client).status_code == 201
|
||||
db.execute(text("UPDATE personal_access_tokens SET expires_at = now() - interval '1 day'"))
|
||||
assert _create(client).status_code == 201
|
||||
rows = db.execute(
|
||||
text("SELECT status, revoke_reason FROM personal_access_tokens ORDER BY created_at")
|
||||
).all()
|
||||
assert [tuple(r) for r in rows] == [("revoked", "expired"), ("active", None)]
|
||||
|
||||
def test_per_user_cap(self, client, db, monkeypatch):
|
||||
monkeypatch.setattr(pat_tokens.settings, "PAT_MAX_PER_USER", 1)
|
||||
assert _create(client, name="one").status_code == 201
|
||||
|
||||
@@ -114,6 +114,11 @@ class TestScopeEnforcement:
|
||||
response = _call(client, "GET", "/api/admin/users", _claims(list(SCOPES)))
|
||||
assert _denied(response) == "not_available_to_tokens"
|
||||
|
||||
def test_unknown_path_and_wrong_method_keep_their_own_status(self, client):
|
||||
claims = _claims(list(SCOPES))
|
||||
assert _call(client, "GET", "/api/no_such_route", claims).status_code == 404
|
||||
assert _call(client, "DELETE", "/api/get_agents", claims).status_code == 405
|
||||
|
||||
def test_missing_scope_is_refused_and_names_the_scope(self, client):
|
||||
response = _call(client, "GET", "/api/get_agents", _claims(["sources:read"]))
|
||||
assert _denied(response) == "insufficient_scope"
|
||||
|
||||
@@ -64,6 +64,24 @@ class TestUniqueness:
|
||||
assert not repo.name_in_use("u1", "ci")
|
||||
assert _create(repo, token_hash="h2")["name"] == "ci"
|
||||
|
||||
def test_retire_expired_name_only_touches_expired_tokens_with_that_name(self, pg_conn):
|
||||
repo = PersonalAccessTokensRepository(pg_conn)
|
||||
past = datetime.now(timezone.utc) - timedelta(days=1)
|
||||
_create(repo, name="ci", token_hash="h1", expires_at=past)
|
||||
_create(repo, name="other", token_hash="h2", expires_at=past)
|
||||
_create(repo, user_id="u2", name="ci", token_hash="h3", expires_at=past)
|
||||
assert repo.retire_expired_name("u1", "ci") == 1
|
||||
assert not repo.name_in_use("u1", "ci")
|
||||
assert repo.name_in_use("u1", "other") and repo.name_in_use("u2", "ci")
|
||||
|
||||
def test_retire_expired_name_leaves_live_tokens(self, pg_conn):
|
||||
repo = PersonalAccessTokensRepository(pg_conn)
|
||||
_create(repo, token_hash="h1")
|
||||
_create(repo, name="later", token_hash="h2", expires_at=datetime.now(timezone.utc) + timedelta(days=1))
|
||||
assert repo.retire_expired_name("u1", "ci") == 0
|
||||
assert repo.retire_expired_name("u1", "later") == 0
|
||||
assert repo.name_in_use("u1", "ci") and repo.name_in_use("u1", "later")
|
||||
|
||||
def test_same_name_for_other_user_is_fine(self, pg_conn):
|
||||
repo = PersonalAccessTokensRepository(pg_conn)
|
||||
_create(repo, user_id="u1", token_hash="h1")
|
||||
|
||||
Reference in new issue
Block a user