mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
Patrick Buckley
464450b9e2
* 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).
264 lines
9.4 KiB
Python
264 lines
9.4 KiB
Python
"""Storage CRUD tests for the Phase 9 ``mcp_pending_consent`` table.
|
|
|
|
Validates protocol additions backing the dashboard pending-consent badge:
|
|
|
|
- ``upsert_mcp_pending_consent`` — insert + on-conflict refresh
|
|
- ``list_mcp_pending_consent_by_user`` — read path
|
|
- ``delete_mcp_pending_consent`` — single-row clear
|
|
- ``delete_all_mcp_pending_consent_by_user`` — bulk clear
|
|
- ``count_mcp_consented_users_by_server`` — admin status pill
|
|
- ``any_oauth_user_mcp_servers`` — install-level gate
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
|
|
def _iso(ts: str = "2026-05-11T12:00:00") -> str:
|
|
return ts
|
|
|
|
|
|
class TestUpsertAndList:
|
|
def test_insert_round_trip(self, backend) -> None:
|
|
backend.upsert_mcp_pending_consent(
|
|
user_id="user-a",
|
|
server_name="srv-x",
|
|
error_code="mcp_consent_required",
|
|
scopes_required="read write",
|
|
last_ws_id="ws-1",
|
|
last_tool_call_id="tool-1",
|
|
now_iso=_iso(),
|
|
)
|
|
rows = backend.list_mcp_pending_consent_by_user("user-a")
|
|
assert len(rows) == 1
|
|
r = rows[0]
|
|
assert r["user_id"] == "user-a"
|
|
assert r["server_name"] == "srv-x"
|
|
assert r["error_code"] == "mcp_consent_required"
|
|
assert r["scopes_required"] == "read write"
|
|
assert r["last_ws_id"] == "ws-1"
|
|
assert r["last_tool_call_id"] == "tool-1"
|
|
assert r["occurrence_count"] == 1
|
|
assert r["first_seen_at"] == r["last_seen_at"]
|
|
|
|
def test_upsert_bumps_count_and_refreshes_recency(self, backend) -> None:
|
|
backend.upsert_mcp_pending_consent(
|
|
user_id="user-a",
|
|
server_name="srv-x",
|
|
error_code="mcp_consent_required",
|
|
scopes_required=None,
|
|
last_ws_id=None,
|
|
last_tool_call_id=None,
|
|
now_iso="2026-05-11T12:00:00",
|
|
)
|
|
backend.upsert_mcp_pending_consent(
|
|
user_id="user-a",
|
|
server_name="srv-x",
|
|
error_code="mcp_insufficient_scope",
|
|
scopes_required="read",
|
|
last_ws_id="ws-2",
|
|
last_tool_call_id="tool-2",
|
|
now_iso="2026-05-11T13:00:00",
|
|
)
|
|
rows = backend.list_mcp_pending_consent_by_user("user-a")
|
|
assert len(rows) == 1
|
|
r = rows[0]
|
|
# Recency fields refreshed to the second call's values; count bumped.
|
|
assert r["occurrence_count"] == 2
|
|
assert r["error_code"] == "mcp_insufficient_scope"
|
|
assert r["scopes_required"] == "read"
|
|
assert r["last_ws_id"] == "ws-2"
|
|
assert r["last_tool_call_id"] == "tool-2"
|
|
assert r["last_seen_at"] == "2026-05-11T13:00:00"
|
|
# first_seen_at preserved — that's the load-bearing audit value.
|
|
assert r["first_seen_at"] == "2026-05-11T12:00:00"
|
|
|
|
def test_list_orders_by_last_seen_desc(self, backend) -> None:
|
|
backend.upsert_mcp_pending_consent(
|
|
user_id="user-a",
|
|
server_name="srv-old",
|
|
error_code="mcp_consent_required",
|
|
scopes_required=None,
|
|
last_ws_id=None,
|
|
last_tool_call_id=None,
|
|
now_iso="2026-05-11T10:00:00",
|
|
)
|
|
backend.upsert_mcp_pending_consent(
|
|
user_id="user-a",
|
|
server_name="srv-new",
|
|
error_code="mcp_consent_required",
|
|
scopes_required=None,
|
|
last_ws_id=None,
|
|
last_tool_call_id=None,
|
|
now_iso="2026-05-11T11:00:00",
|
|
)
|
|
rows = backend.list_mcp_pending_consent_by_user("user-a")
|
|
assert [r["server_name"] for r in rows] == ["srv-new", "srv-old"]
|
|
|
|
def test_per_user_isolation(self, backend) -> None:
|
|
backend.upsert_mcp_pending_consent(
|
|
user_id="user-a",
|
|
server_name="srv",
|
|
error_code="mcp_consent_required",
|
|
scopes_required=None,
|
|
last_ws_id=None,
|
|
last_tool_call_id=None,
|
|
now_iso=_iso(),
|
|
)
|
|
assert backend.list_mcp_pending_consent_by_user("user-b") == []
|
|
|
|
|
|
class TestDelete:
|
|
def test_delete_single(self, backend) -> None:
|
|
backend.upsert_mcp_pending_consent(
|
|
user_id="user-a",
|
|
server_name="srv-x",
|
|
error_code="mcp_consent_required",
|
|
scopes_required=None,
|
|
last_ws_id=None,
|
|
last_tool_call_id=None,
|
|
now_iso=_iso(),
|
|
)
|
|
assert backend.delete_mcp_pending_consent("user-a", "srv-x") is True
|
|
assert backend.list_mcp_pending_consent_by_user("user-a") == []
|
|
# Second delete returns False (no row).
|
|
assert backend.delete_mcp_pending_consent("user-a", "srv-x") is False
|
|
|
|
def test_delete_missing_returns_false(self, backend) -> None:
|
|
assert backend.delete_mcp_pending_consent("never", "missing") is False
|
|
|
|
def test_delete_all_by_user(self, backend) -> None:
|
|
for name in ("srv-a", "srv-b", "srv-c"):
|
|
backend.upsert_mcp_pending_consent(
|
|
user_id="user-a",
|
|
server_name=name,
|
|
error_code="mcp_consent_required",
|
|
scopes_required=None,
|
|
last_ws_id=None,
|
|
last_tool_call_id=None,
|
|
now_iso=_iso(),
|
|
)
|
|
# Cross-user row that must NOT be touched.
|
|
backend.upsert_mcp_pending_consent(
|
|
user_id="user-b",
|
|
server_name="srv-z",
|
|
error_code="mcp_consent_required",
|
|
scopes_required=None,
|
|
last_ws_id=None,
|
|
last_tool_call_id=None,
|
|
now_iso=_iso(),
|
|
)
|
|
assert backend.delete_all_mcp_pending_consent_by_user("user-a") == 3
|
|
assert backend.list_mcp_pending_consent_by_user("user-a") == []
|
|
assert len(backend.list_mcp_pending_consent_by_user("user-b")) == 1
|
|
|
|
|
|
class TestCountConsentedUsersByServer:
|
|
def _seed_server(self, backend, name: str = "srv-x") -> None:
|
|
backend.create_mcp_server(
|
|
server_id="srv-id-" + name,
|
|
name=name,
|
|
transport="streamable-http",
|
|
command="",
|
|
args="[]",
|
|
url="https://example.com/mcp",
|
|
headers="{}",
|
|
env="{}",
|
|
auto_approve=False,
|
|
enabled=True,
|
|
created_by="admin",
|
|
)
|
|
backend.update_mcp_server("srv-id-" + name, auth_type="oauth_user")
|
|
|
|
def test_counts_distinct_non_expired_users(self, backend) -> None:
|
|
self._seed_server(backend)
|
|
future = "2099-01-01T00:00:00"
|
|
backend.create_mcp_user_token(
|
|
"alice",
|
|
"srv-x",
|
|
access_token_ct=b"ct",
|
|
refresh_token_ct=None,
|
|
expires_at=future,
|
|
scopes=None,
|
|
as_issuer="https://as.example.com",
|
|
audience="https://example.com/mcp",
|
|
)
|
|
backend.create_mcp_user_token(
|
|
"bob",
|
|
"srv-x",
|
|
access_token_ct=b"ct",
|
|
refresh_token_ct=None,
|
|
expires_at=None, # null treated as non-expired
|
|
scopes=None,
|
|
as_issuer="https://as.example.com",
|
|
audience="https://example.com/mcp",
|
|
)
|
|
# Different server — must not count.
|
|
self._seed_server(backend, name="srv-y")
|
|
backend.create_mcp_user_token(
|
|
"carol",
|
|
"srv-y",
|
|
access_token_ct=b"ct",
|
|
refresh_token_ct=None,
|
|
expires_at=future,
|
|
scopes=None,
|
|
as_issuer="https://as.example.com",
|
|
audience="https://example.com/mcp",
|
|
)
|
|
assert backend.count_mcp_consented_users_by_server("srv-x") == 2
|
|
assert backend.count_mcp_consented_users_by_server("srv-y") == 1
|
|
|
|
def test_excludes_expired(self, backend) -> None:
|
|
self._seed_server(backend)
|
|
backend.create_mcp_user_token(
|
|
"alice",
|
|
"srv-x",
|
|
access_token_ct=b"ct",
|
|
refresh_token_ct=None,
|
|
expires_at="2020-01-01T00:00:00", # well in the past
|
|
scopes=None,
|
|
as_issuer="https://as.example.com",
|
|
audience="https://example.com/mcp",
|
|
)
|
|
assert backend.count_mcp_consented_users_by_server("srv-x") == 0
|
|
|
|
def test_zero_when_no_rows(self, backend) -> None:
|
|
assert backend.count_mcp_consented_users_by_server("missing") == 0
|
|
|
|
|
|
class TestInstallGate:
|
|
def test_any_oauth_user_returns_false_on_empty(self, backend) -> None:
|
|
assert backend.any_oauth_user_mcp_servers() is False
|
|
|
|
def test_any_oauth_user_ignores_static_rows(self, backend) -> None:
|
|
backend.create_mcp_server(
|
|
server_id="srv-1",
|
|
name="static-only",
|
|
transport="streamable-http",
|
|
command="",
|
|
args="[]",
|
|
url="https://example.com",
|
|
headers='{"Authorization": "Bearer x"}',
|
|
env="{}",
|
|
auto_approve=False,
|
|
enabled=True,
|
|
created_by="admin",
|
|
)
|
|
assert backend.any_oauth_user_mcp_servers() is False
|
|
|
|
def test_any_oauth_user_returns_true_when_one_exists(self, backend) -> None:
|
|
backend.create_mcp_server(
|
|
server_id="srv-2",
|
|
name="oauth-srv",
|
|
transport="streamable-http",
|
|
command="",
|
|
args="[]",
|
|
url="https://example.com",
|
|
headers="{}",
|
|
env="{}",
|
|
auto_approve=False,
|
|
enabled=True,
|
|
created_by="admin",
|
|
)
|
|
backend.update_mcp_server("srv-2", auth_type="oauth_user")
|
|
assert backend.any_oauth_user_mcp_servers() is True
|