1 Commits

Author	SHA1	Message	Date
Patrick Buckley	adeb10bc2c	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 13:15:09 -07:00