feat(mcp-gateway): durable per-client OAuth grants and Authorized Clients UI#4134
Open
pandemicsyn wants to merge 8 commits into
Open
feat(mcp-gateway): durable per-client OAuth grants and Authorized Clients UI#4134pandemicsyn wants to merge 8 commits into
pandemicsyn wants to merge 8 commits into
Conversation
…ients UI Introduces durable OAuth grants bound per client + user + connect resource + instance + redirect URI + execution context + scopes + config version, with explicit pending/active/revoked lifecycle. Authorization, token issuance, refresh, /userinfo, and Worker runtime now require a non-revoked grant binding. Provider OAuth callbacks finalize the provider grant before issuing the first-level authorization code so clients never receive a code without usable provider credentials. Adds a user-scoped Authorized Clients UI at /cloud/mcp-gateway/authorized-clients that lists active grants, exposes per-grant revoke, and surfaces grant metadata (callback URI, exact context, granted permissions, last use). Lifecycle revocation cascades through one helper (revokeGrantIdsWithTx) so authorization requests, codes, refresh tokens, and pending provider authorizations are invalidated together. Material OAuth client metadata changes, config disable/delete/material change, route rotation, assignment removal, OAuth client deletion, GDPR delete, organization membership removal, and user blocking all funnel through this helper. OAuth client metadata revocation only fires on shrinking changes (removed redirect URIs, scopes, grant types, or response types) and normalizes redirect URIs to avoid mass-revoke on cosmetic re-registration. Worker runtime grant validation also enforces config_version, matching the web token-service binding contract; the executionContext comparison helper now lives in @kilocode/mcp-gateway and is shared by both edges.
…y page
The MCP Gateway list page now has Connections and Authorized clients
tabs instead of a separate route. The Authorized Clients sidebar entry
is removed; users access their authorized clients alongside the
gateway connections, scoped to the current personal or organization
context.
mcpGatewayAuthorizationsRouter.listMine now accepts an optional
{ ownerScope, organizationId } filter so the org-scoped tab only
surfaces grants for that organization.
The card title now shows the reported client name in quotes (e.g. "Kilo") with the client ID and "Reported name" provenance label underneath. The shield icon stays in place but is now a tooltip trigger explaining that the name is self-reported and unverified. Falls back to "Unverified MCP client" when the client did not declare a name during registration.
…t usage Card hierarchy: - Title row keeps the shield + reported name in quotes. - New subline links the connection name (deep-links into the gateway config detail page) and shows the on-behalf-of context inline. - Muted footer renders Authorized + Last used as a single tabular line. - Promoted "What this grants" callout when the grant carries the broad mcp:access scope, so the consequential fact is the most visible one. - Tucked the callback URI and full client ID behind a "Show technical details" disclosure to lower the visual noise floor. - Replaced the Context column with the inline "on behalf of" copy. - Added a focus ring to the unverified-shield tooltip trigger. Backend: - listMine now returns configId so the UI can build the connection link. - New touchOAuthGrantUsage helper updates last_used_at from the Worker runtime path. The update is fired through c.executionCtx.waitUntil() on every authenticated MCP request, with a SQL-side 30s debounce so the same grant is not rewritten on every proxy hop.
Contributor
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Fix these issues in Kilo Cloud Files Reviewed (4 files)
Previous Review Summaries (4 snapshots, latest commit 3c4134a)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 3c4134a)Status: No Issues Found | Recommendation: Merge Files Reviewed (100 files)
Previous review (commit ec6b3b9)Status: 1 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (16 files)
Fix these issues in Kilo Cloud Previous review (commit c2cbf6d)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (32 files)
Fix these issues in Kilo Cloud Previous review (commit fb99d20)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (32 files)
Reviewed by gpt-5.4-20260305 · Input: 85.5K · Output: 10K · Cached: 446.5K Review guidance: REVIEW.md from base branch |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| }, | ||
| table => [ | ||
| uniqueIndex('UQ_mcp_gateway_oauth_grants_active_binding') | ||
| .on(table.oauth_client_id, table.kilo_user_id, table.connect_resource_id, table.redirect_uri) |
Contributor
There was a problem hiding this comment.
WARNING: Active grant uniqueness ignores owner context
The new unique binding only keys on (oauth_client_id, kilo_user_id, connect_resource_id, redirect_uri), so the same user cannot hold separate active grants for the same client/resource pair in different owner contexts. createOrReuseGrant() relies on that same lookup, which means authorizing an org-owned connection after authorizing a personal one can revoke and replace the first grant even though the spec says a grant is bound to one exact execution context and owner tuple.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Contributor
Author
There was a problem hiding this comment.
Reviewed and not warranted. connect_resource_id is the PK of mcp_gateway_connect_resources, and a personal config and an org config produce distinct connect resources with distinct UUIDs even for the same MCP client. The active-binding unique index over (oauth_client_id, kilo_user_id, connect_resource_id, redirect_uri) therefore cannot conflate personal and organization grants. Cross-context revocation in createOrReuseGrant() only happens when the existing grant's (instance_id, config_version, scopes, execution_context) already match the new request, which by definition shares the same owner context as well.
Regenerated migration as 0167_wealthy_eternity.sql against current schema after dropping the branch-local 0166_sloppy_annihilus.sql per AGENTS.md migration-conflict procedure.
…info audience - listMine now verifies organization membership before honoring an organizationId filter so an unrelated org cannot be passed to scope the listing. - userInfo now requires the bearer token's audience to start with the gateway base URL, so access tokens minted for resources outside this gateway cannot be replayed at the userinfo endpoint.
Contributor
Author
|
Addressed bot review feedback in c2cbf6d + d8de963 (merge):
Also merged |
pandemicsyn
force-pushed
the
feat/mcp-gateway-authorized-clients
branch
from
ec6b3b9 to
3c4134a
Compare
Contributor
Author
|
Addressed follow-up PR3 findings in 3c4134a:
|
Contributor
Author
|
Follow-up hardening in db91329:
|
| @@ -15,8 +15,8 @@ export async function POST( | |||
| try { | |||
| const services = createGatewayServices(); | |||
| const route = parseScopedRouteParams(await params); | |||
Contributor
There was a problem hiding this comment.
WARNING: Route-param validation still bypasses the public registration bucket
parseScopedRouteParams(await params) still runs before consumeRegistrationRateLimit(), so requests with syntactically invalid scope/ownerId/configId/routeKey return 400 without incrementing the unauthenticated registration rate limit. If the goal is to throttle every public registration probe, the rate-limit check needs to happen before route-param parsing as well.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| return await exchangeToken(request); | ||
| } catch (error) { | ||
| return gatewayErrorResponse(error); | ||
| return withTokenResponseHeaders(gatewayErrorResponse(error)); |
Contributor
There was a problem hiding this comment.
WARNING: Scoped token errors still miss the new no-store headers
This catch only hardens /api/mcp-gateway/oauth/token. /api/mcp-gateway/oauth/token/[scope]/... still has its own catch that returns gatewayErrorResponse(error) directly, so any thrown GatewayError there remains cacheable. If the intent is "success and error paths" for token endpoints, the header application needs to be shared with the scoped route as well.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hardens MCP Gateway OAuth client lifecycle by introducing durable per-client OAuth grants and a user-facing Authorized Clients tab on the gateway page.
Domain model. Adds
mcp_gateway_oauth_grantswithpending/active/revokedlifecycle and a unique partial index over(oauth_client_id, kilo_user_id, connect_resource_id, redirect_uri)for active bindings. Authorization requests, authorization codes, refresh tokens, pending provider authorizations, and audit events all gain a nullableoauth_grant_idforeign key so a single revocation cascades through every dependent artifact.Token issuance and runtime enforcement. OAuth-client JWTs now embed
token_source,oauth_grant_id, andclient_id; derived connect tokens carrytoken_source=derived_connect. The Worker re-resolves the active grant on every authenticated runtime request (matching(grant_id, client_id, kilo_user_id, instance_id, connect_resource_id, config_version, exec_context, scopes, status=active)) before proxying upstream. Token exchange and refresh rotation lock the grant row inside the same transaction to close TOCTOU windows, and refresh paths surfaceinvalid_grantimmediately on revocation.Lifecycle invalidation. Centralised
revokeGrantIdsWithTx(tx, grantIds, reason)is now the single revocation primitive. It cascades to authorization requests, authorization codes, refresh tokens, pending provider authorizations, and writes audit events. Wired into:revokefrom the Authorized Clients UI.redirect_uris/grant_types/response_types/scopesare preserved; redirect URIs are normalised before comparison).Provider OAuth callback ordering. The dynamic-provider callback path now persists the upstream provider grant before issuing the first-level authorization code, so a client can never receive a code without usable provider credentials. Pending grants are created at authorize time and only promoted to active during callback finalisation.
UI. The MCP Gateway page now uses tabs: Connections (existing) and Authorized clients (new). Each grant card shows the reported client name in quotes with an "unverified" tooltip on the shield, an inline
Authorized to use <connection link> on behalf of <context>subline, a promoted "What this grants" callout for the broadmcp:accessscope, and a<details>disclosure for the raw client ID and callback URI. Revoke confirmation renders the exact callback URI and client ID in code blocks. Per-org gateway pages filter the tab to that org's grants.last_used_athonesty. A newtouchOAuthGrantUsagehelper updateslast_used_atfrom the Worker runtime path throughc.executionCtx.waitUntil()with a SQL-side 30-second debounce, so the displayed timestamp reflects actual MCP traffic rather than just token refresh cycles.Shared invariants.
executionContextsMatchnow lives in@kilocode/mcp-gatewayand is shared by web mint and Worker runtime; the Worker also enforcesconfig_versionto match the web token-service contract, so material config changes invalidate already-issued tokens.Verification
Visual Changes
Reviewer Notes
packages/db/src/migrations/0166_sloppy_annihilus.sqladdsmcp_gateway_oauth_grantsplus nullableoauth_grant_idcolumns and FK constraints on five existing tables. Validation runs in-line; row counts on the affected tables are small (internal users only) so the lock window is negligible.provider-oauth-service.ts:handleProviderCallbackandauthorization-service.ts:completeProviderAuthorizationif you remember the previous order.oauth-client-service.ts:isAdditiveSupersetmatches the intended blast radius on DCR re-registration.last_used_atwrite amplification is bounded by a SQLlast_used_at < NOW() - 30s OR last_used_at IS NULLpredicate on the same row. Worst case is one write per active grant per 30s under heavy traffic.touchOAuthGrantUsagesince the unit harness has noc.executionCtx. The handler also tolerates missingexecutionCtxat runtime.is_adminsidebar gate on the gateway entry is the deploy feature flag, not a permanent ACL. The Authorized clients tab inherits visibility from that gate today; the page is per-user already, so no follow-up is required when the flag opens up.