mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
adeb10bc2c
* feat(mcp): admin status, deferred-consent persistence, operator docs (Phase 9)
Completes the OAuth-MCP build-out (Phases 0-8 shipped) by closing the
operator + deferred-consent gaps:
1. **Per-(user, server) deferred-consent persistence** — when a
non-interactive run (scheduled / channel) hits ``mcp_consent_required``
or ``mcp_insufficient_scope``, the sync pool dispatchers now upsert a
row into a new ``mcp_pending_consent`` table. The dashboard hydrates
the gear-icon badge from this table on load, so users who weren't
online to see the in-flight SSE prompt still surface the deferred
work on next login. Cleared automatically by the OAuth callback
handler on consent completion; manual user dismiss via new DELETE
endpoints. Composite PK ``(user_id, server_name)`` collapses repeat
occurrences for the same server — no NULLs-not-distinct trap.
2. **Admin status pill + bulk-revoke** — the MCP Servers admin row now
shows ``consented_users_count`` for ``auth_type=oauth_user`` rows
when ≥1, with a two-step-confirm ``bulk-revoke`` button that drops
every user's token for the server via the existing
``delete_mcp_oauth_rows_by_server_name`` primitive. Upstream RFC
7009 revoke is intentionally NOT attempted in bulk (avoids N
upstream HTTP calls per admin click); audit detail records
``upstream_revoke_outcome=bulk_admin_no_upstream``. A "last
refresh" pill (age + outcome) renders on each row, sourced from a
new ``_last_refresh`` dict populated by ``_refresh_server`` on every
call (both manual ``refresh_sync`` and the ``_cb_auto_reconnect``
follow-up).
3. **ClientType.SCHEDULED** added to the prompts module + scheduler
passes it through to ``create_workstream``. ``ChatSession`` now
computes ``_is_interactive_for_consent`` at construction (WEB / CLI
are interactive; CHAT / SCHEDULED are not) and plumbs the flag
through ``call_tool_sync`` / ``read_resource_sync`` /
``get_prompt_sync`` to the three sync dispatchers. The wrap at the
``_is_structured_error`` gate routes consent codes to the new
``_record_pending_consent_best_effort`` helper for non-interactive
callers only; interactive sessions stay on the in-flight SSE path
Phase 8 ships unchanged.
4. **Operator docs** — ``docs/mcp-oauth.md`` (operator guide, parallel
to ``docs/oidc.md``: ``auth_type`` choice, OAuth client setup,
encryption-key rotation, troubleshooting matrix) and
``docs/operations/mcp-oauth-headless.md`` (one-paragraph runbook
per ``feedback_runbook_trust_llm.md``: pre-consent recipe for
scheduled / channel-driven runs).
Schema
- Migration 054_mcp_pending_consent.py — composite PK
``(user_id, server_name)``, ``occurrence_count`` + ``first_seen_at`` /
``last_seen_at`` for recency metadata, ``idx_mcp_pending_consent_user``
for the badge-load query. No FKs (matches the rest of the
oauth_user schema).
- Migration 055_mcp_user_tokens_server_index.py — adds
``idx_mcp_user_tokens_server`` on ``(server_name, expires_at)`` so
the admin pill's ``count_mcp_consented_users_*`` queries don't
full-scan against the leading-``user_id`` composite PK.
- Cross-backend: works on SQLite + PostgreSQL via dialect-specific
``on_conflict_do_update`` (PG ``postgresql.insert`` / SQLite
``sqlalchemy.dialects.sqlite.insert``). No ``NULLS NOT DISTINCT``
needed — the simplified PK eliminates the cross-version trap.
Endpoints
- ``GET /v1/api/mcp/oauth/pending`` — list deferred-consent records for
the authenticated user. Install-level gate via cached
``any_oauth_user_mcp_servers`` short-circuits to ``{pending: 0}`` on
installs with no oauth_user MCP servers — local-auth deployments
exercise zero new storage queries on this path. The gate result is
cached on ``app.state`` with a 60s TTL to spare repeat dashboard
loads.
- ``DELETE /v1/api/mcp/oauth/pending/{server_name}`` — single dismiss.
Returns 204 in both existed-and-deleted and never-existed cases
(no cross-tenant existence leak); audits
``mcp_server.oauth.pending_consent_dismissed`` with
``mode=single`` + ``cleared=0|1`` so a session-hijack attacker
scrubbing breadcrumbs leaves an audit trail.
- ``DELETE /v1/api/mcp/oauth/pending`` — bulk dismiss; audits
``mode=bulk`` + ``cleared=N``.
- ``POST /v1/api/admin/mcp-servers/{name}/bulk-revoke`` — admin
bulk-revoke for the named server's per-user tokens. Requires
``admin.mcp`` permission + 400s when the row isn't ``oauth_user``.
All four registered on both ``turnstone-server`` and
``turnstone-console`` (mirrors the Phase 8 ``/connections`` endpoint
shape).
Performance
- Admin list handler now uses a single ``GROUP BY`` bulk-count query
(``count_mcp_consented_users_grouped_by_server``) wrapped in
``asyncio.to_thread`` rather than N per-row sync DB round-trips
inside the async handler. Skipped entirely when no row is
oauth_user.
Frontend
- ``ui/static/app.js``: ``loadPendingConsents()`` hydrates the
existing ``_pendingConsentServers`` set on dashboard init + after
the user opens the settings modal. Endpoint failures stay silent
— the badge will be re-driven by the next in-flight tool error.
- ``console/static/admin.js``: ``consented_users_count`` pill +
``bulk-revoke`` button on each MCP row (only when ≥1 consented),
two-step confirm matching the existing delete pattern. ``last-
refresh`` age + outcome pill in the per-row status cell, sourced
from the freshest per-node entry in ``status[*].last_refresh_at`` /
``last_refresh_outcome``. CSS for the pills in ``style.css``.
Tests
- ``test_mcp_pending_consent_storage`` — 13 tests covering upsert
idempotency, list ordering, per-user isolation, single/bulk delete,
count-by-server + grouped variant, install-level gate.
- ``test_mcp_pending_consent_dispatch`` — 9 tests, including the
boundary-cross gate per ``feedback_tests_through_boundaries.md``:
drives the real ``call_tool_sync`` → ``_dispatch_pool_sync`` →
``_is_structured_error`` → ``_record_pending_consent_best_effort``
with a mocked classified-lookup so the structural plumb-through is
verified end-to-end. Includes a storage-failure test that pins
the docstring's "envelope unchanged on storage failure" promise.
- ``test_mcp_pending_consent_endpoints`` — 11 tests: install gate,
list-for-self, no-cross-user-leak, single/bulk delete, idempotent
not-found, audit emission on single + bulk + cross-tenant dismiss.
- ``test_chat_session_interactivity_flag`` — 7 tests pinning the
``ClientType`` → ``_is_interactive_for_consent`` mapping against
the module-level ``INTERACTIVE_CONSENT_CLIENT_TYPES`` frozenset.
- ``test_mcp_admin_bulk_revoke`` — 7 tests covering admin.mcp
permission gate, 404 on missing, 400 on non-oauth_user, 200 with
``rows_deleted`` + ``consented_users_before``, audit row with
``upstream_revoke_outcome=bulk_admin_no_upstream``, cross-server
isolation.
- ``test_mcp_oauth_handlers`` — 2 new callback tests pin the post-
callback ``delete_mcp_pending_consent`` invocation: success-clears
+ storage-failure-still-redirects.
- 636 tests pass on the impacted surface (47 new + Phase 0-8 OAuth-MCP
+ session + prompts + storage admin). ruff + mypy clean.
Hard invariants honored
- Static path byte-identical for ``auth_type ∈ {none, static}`` — the
flag flows only through the pool dispatchers, which only fire when
the row resolves to ``oauth_user``.
- ``asyncio.timeout`` (not ``asyncio.wait_for``) preserved on every
AS / SDK / pool-loop await — no new awaits added to the hot path.
- Install-level gate on the badge endpoint: cached
``any_oauth_user_mcp_servers`` returns False on a row-less
deployment → endpoint short-circuits without touching the pending-
consent table; 60s TTL bounds the staleness window after admin
flips ``auth_type``.
- Operator-actionable codes (key-unknown, url-insecure, *_forbidden)
explicitly filtered out of persistence — they're outside the
user-facing consent badge scope.
- Best-effort write: the structured-error envelope returned to the
agent is identical whether the persistence write succeeds or fails
(storage exception is logged with type name only — no chained
context that could carry an ``httpx.Request`` bearer header).
- No ``exc_info=True`` on any new path that can chain a bearer-bearing
``httpx.Request``.
- Defensive parsing: ``_parse_pending_consent_envelope`` mirrors
``_is_structured_error``'s ``isinstance(decoded, dict)`` guard plus
filters scope tokens through ``is_valid_scope_token`` capped at
``MAX_INSUFFICIENT_SCOPE_REPORTED`` — defense-in-depth even though
production callers already validate upstream.
- Audit events on every dismiss endpoint so a session-control attacker
scrubbing dashboard breadcrumbs still leaves a trail.
Cross-backend
- Tested on SQLite via the conftest backend fixture.
- PostgreSQL path uses ``postgresql.insert(...).on_conflict_do_update``
parallel to the existing ``mcp_user_tokens`` upsert in Phase 3.
Deferred (not Phase 9 blockers)
- Multi-node pool eviction on bulk-revoke: only local-node sessions
would be evicted if we built it, and there's no bulk-by-server
primitive on MCPClientManager today; remote nodes will surface as
a 401 on next dispatch which refreshes through the (now empty)
token row.
- RFC 8693 / Azure OBO ``auth_type=oauth_token_exchange`` — captured
in the design doc as a future architectural direction (~600 LOC +
IdP-side admin work); requires OIDC token capture and per-MCP-server
resource-trust configuration that v1 does not ship.
* docs(mcp): address Copilot review feedback on Phase 9
- Fix misleading admin.js comment that claimed the refresh pill rendered
"<short-relative> <outcome>" — the pill actually renders only the short
age, with outcome reflected via CSS class and tooltip.
- Replace broken feedback_secrets_not_in_env.md repo-root link in
mcp-oauth.md with the inlined rationale (env-borne secrets reachable
via shell tools / os.environ; TOML secrets are not).
212 lines
7.1 KiB
Python
212 lines
7.1 KiB
Python
"""Integration tests for the Phase 9 admin bulk-revoke endpoint.
|
|
|
|
POST /v1/api/admin/mcp-servers/{name}/bulk-revoke clears every user's
|
|
OAuth token for a server (admin-side counterpart to the per-user
|
|
DELETE /v1/api/mcp/oauth/connections/{server_name} that shipped in
|
|
Phase 8).
|
|
|
|
Coverage:
|
|
- requires ``admin.mcp`` permission (401/403 without).
|
|
- 404 when the named server is missing.
|
|
- 400 when the server's ``auth_type`` is not ``oauth_user``.
|
|
- 200 + ``rows_deleted`` + ``consented_users_before`` on success.
|
|
- Audit row written with
|
|
``upstream_revoke_outcome="bulk_admin_no_upstream"``.
|
|
- Token rows are gone from ``mcp_user_tokens`` post-call.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from typing import TYPE_CHECKING, Any
|
|
|
|
import pytest
|
|
from starlette.applications import Starlette
|
|
from starlette.middleware import Middleware
|
|
from starlette.middleware.base import BaseHTTPMiddleware
|
|
from starlette.routing import Mount, Route
|
|
from starlette.testclient import TestClient
|
|
|
|
from turnstone.console.server import admin_mcp_bulk_revoke
|
|
from turnstone.core.auth import AuthResult
|
|
from turnstone.core.storage._sqlite import SQLiteBackend
|
|
|
|
if TYPE_CHECKING:
|
|
from starlette.requests import Request
|
|
from starlette.responses import Response
|
|
|
|
|
|
class _InjectAdminMcp(BaseHTTPMiddleware):
|
|
async def dispatch(self, request: Request, call_next: Any) -> Response:
|
|
request.state.auth_result = AuthResult(
|
|
user_id="admin-user",
|
|
scopes=frozenset({"approve"}),
|
|
token_source="config",
|
|
permissions=frozenset({"read", "write", "approve", "admin.mcp"}),
|
|
)
|
|
return await call_next(request)
|
|
|
|
|
|
class _InjectNoAdminMcp(BaseHTTPMiddleware):
|
|
async def dispatch(self, request: Request, call_next: Any) -> Response:
|
|
request.state.auth_result = AuthResult(
|
|
user_id="regular-user",
|
|
scopes=frozenset({"approve"}),
|
|
token_source="jwt",
|
|
permissions=frozenset({"read", "write", "approve"}),
|
|
)
|
|
return await call_next(request)
|
|
|
|
|
|
def _build_app(storage: SQLiteBackend, *, with_admin_mcp: bool = True) -> Starlette:
|
|
mw = _InjectAdminMcp if with_admin_mcp else _InjectNoAdminMcp
|
|
app = Starlette(
|
|
routes=[
|
|
Mount(
|
|
"/v1",
|
|
routes=[
|
|
Route(
|
|
"/api/admin/mcp-servers/{name}/bulk-revoke",
|
|
admin_mcp_bulk_revoke,
|
|
methods=["POST"],
|
|
),
|
|
],
|
|
),
|
|
],
|
|
middleware=[Middleware(mw)],
|
|
)
|
|
app.state.auth_storage = storage
|
|
return app
|
|
|
|
|
|
@pytest.fixture
|
|
def storage(tmp_path: Any) -> SQLiteBackend:
|
|
return SQLiteBackend(str(tmp_path / "test.db"))
|
|
|
|
|
|
def _seed_oauth_server(
|
|
backend: SQLiteBackend,
|
|
*,
|
|
name: str = "srv-oauth",
|
|
server_id: str = "srv-oauth-id",
|
|
) -> None:
|
|
backend.create_mcp_server(
|
|
server_id=server_id,
|
|
name=name,
|
|
transport="streamable-http",
|
|
url="https://example.com/mcp",
|
|
auth_type="oauth_user",
|
|
)
|
|
|
|
|
|
def _seed_static_server(
|
|
backend: SQLiteBackend,
|
|
*,
|
|
name: str = "srv-static",
|
|
server_id: str = "srv-static-id",
|
|
) -> None:
|
|
backend.create_mcp_server(
|
|
server_id=server_id,
|
|
name=name,
|
|
transport="streamable-http",
|
|
url="https://example.com/mcp",
|
|
auth_type="static",
|
|
)
|
|
|
|
|
|
def _seed_user_tokens(backend: SQLiteBackend, server_name: str, users: int) -> None:
|
|
for i in range(users):
|
|
backend.create_mcp_user_token(
|
|
f"user-{i}",
|
|
server_name,
|
|
access_token_ct=b"ct",
|
|
refresh_token_ct=None,
|
|
expires_at=None,
|
|
scopes=None,
|
|
as_issuer="https://as.example.com",
|
|
audience="https://example.com/mcp",
|
|
)
|
|
|
|
|
|
def test_requires_admin_mcp_permission(storage: SQLiteBackend) -> None:
|
|
_seed_oauth_server(storage)
|
|
client = TestClient(_build_app(storage, with_admin_mcp=False))
|
|
resp = client.post("/v1/api/admin/mcp-servers/srv-oauth/bulk-revoke")
|
|
assert resp.status_code == 403
|
|
|
|
|
|
def test_404_on_missing_server(storage: SQLiteBackend) -> None:
|
|
client = TestClient(_build_app(storage))
|
|
resp = client.post("/v1/api/admin/mcp-servers/never-existed/bulk-revoke")
|
|
assert resp.status_code == 404
|
|
assert resp.json() == {"error": "No such server"}
|
|
|
|
|
|
def test_400_on_static_server(storage: SQLiteBackend) -> None:
|
|
_seed_static_server(storage)
|
|
client = TestClient(_build_app(storage))
|
|
resp = client.post("/v1/api/admin/mcp-servers/srv-static/bulk-revoke")
|
|
assert resp.status_code == 400
|
|
body = resp.json()
|
|
assert "oauth_user" in body["error"]
|
|
|
|
|
|
def test_400_on_invalid_server_name(storage: SQLiteBackend) -> None:
|
|
# double-underscore is reserved for the prefixed-tool-name encoding.
|
|
client = TestClient(_build_app(storage))
|
|
resp = client.post("/v1/api/admin/mcp-servers/bad__name/bulk-revoke")
|
|
assert resp.status_code == 400
|
|
|
|
|
|
def test_200_on_success_with_no_consented_users(storage: SQLiteBackend) -> None:
|
|
_seed_oauth_server(storage)
|
|
client = TestClient(_build_app(storage))
|
|
resp = client.post("/v1/api/admin/mcp-servers/srv-oauth/bulk-revoke")
|
|
assert resp.status_code == 200
|
|
body = resp.json()
|
|
assert body["status"] == "ok"
|
|
assert body["rows_deleted"] == 0
|
|
assert body["consented_users_before"] == 0
|
|
|
|
|
|
def test_200_clears_all_user_tokens(storage: SQLiteBackend) -> None:
|
|
_seed_oauth_server(storage)
|
|
_seed_user_tokens(storage, "srv-oauth", users=3)
|
|
# Token for another server must survive the bulk-revoke.
|
|
_seed_oauth_server(storage, name="srv-other", server_id="srv-other-id")
|
|
_seed_user_tokens(storage, "srv-other", users=2)
|
|
|
|
client = TestClient(_build_app(storage))
|
|
resp = client.post("/v1/api/admin/mcp-servers/srv-oauth/bulk-revoke")
|
|
assert resp.status_code == 200
|
|
body = resp.json()
|
|
assert body["status"] == "ok"
|
|
assert body["rows_deleted"] == 3
|
|
assert body["consented_users_before"] == 3
|
|
|
|
# Target server's tokens are gone; bystander's tokens survive.
|
|
assert storage.count_mcp_consented_users_by_server("srv-oauth") == 0
|
|
assert storage.count_mcp_consented_users_by_server("srv-other") == 2
|
|
|
|
|
|
def test_audits_with_bulk_admin_no_upstream(storage: SQLiteBackend) -> None:
|
|
_seed_oauth_server(storage)
|
|
_seed_user_tokens(storage, "srv-oauth", users=2)
|
|
client = TestClient(_build_app(storage))
|
|
resp = client.post("/v1/api/admin/mcp-servers/srv-oauth/bulk-revoke")
|
|
assert resp.status_code == 200
|
|
|
|
# Pull the most-recent audit row for the bulk_revoked action and
|
|
# verify it carries the deferral marker.
|
|
events = storage.list_audit_events(limit=10)
|
|
bulk_rows = [e for e in events if e.get("action") == "mcp_server.oauth.bulk_revoked"]
|
|
assert len(bulk_rows) == 1
|
|
detail = bulk_rows[0].get("detail")
|
|
if isinstance(detail, str):
|
|
import json as _json
|
|
|
|
detail = _json.loads(detail)
|
|
assert detail.get("upstream_revoke_outcome") == "bulk_admin_no_upstream"
|
|
assert detail.get("rows_deleted") == 2
|
|
assert detail.get("consented_users_before") == 2
|
|
assert detail.get("name") == "srv-oauth"
|