Files
turnstone/tests/test_mcp_admin_bulk_revoke.py
Patrick Buckley 464450b9e2 feat(mcp): admin status, deferred-consent persistence, operator docs (Phase 9) (#516)
* 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).
2026-05-12 20:55:01 -07:00

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"