feat: revoke VK-mode OAuth2 grants on VK deletion and add user-liveness checks at refresh and request time

[Pratham-Mishra04]

Summary

When a user identity is deleted or deactivated, their gateway-issued OAuth2 grants and active MCP requests should be cut off immediately rather than continuing to work until the access token naturally expires. This PR adds a user liveness check (IsUserActive) that mirrors the existing virtual-key liveness check, enforcing it at both request time and token refresh time.

Changes

• Added IsUserActive to the OAuth2IdentityResolver interface, returning (false, nil) for a gone/deactivated user and reserving errors for transient failures.
• Added a user liveness check in userScopedServer on the MCP request path — a deleted user is rejected before any virtual key resolution, preventing fallthrough to the global server until the access token expires.
• Added a user liveness check in handleTokenRefresh — a deleted or deactivated user receives invalid_grant on refresh rather than silently receiving a new access token.
• When a virtual key is deleted, any gateway-issued OAuth2 refresh tokens in vk mode bound to that VK are now revoked (setting revoked_at) rather than deleted, so they stop minting access tokens on refresh and fall off the active-grants view while remaining available for reuse detection until the sweep.
• Migrated TableOAuth2RefreshToken in the test setup and added tests covering VK deletion grant revocation, user-inactive refresh rejection, active-user refresh success, and user-inactive MCP request rejection.

Type of change

• Bug fix
• Feature
• Refactor
• Documentation
• Chore/CI

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations
• Plugins
• UI (React)
• Docs

How to test

go test ./framework/configstore/... ./transports/bifrost-http/handlers/...

Key scenarios to verify:

• Deleting a virtual key sets revoked_at on any associated vk-mode refresh tokens rather than leaving them active.
• A refresh request for a user-mode token where IsUserActive returns false receives a 400 invalid_grant response with "user is no longer active".
• A refresh request for a user-mode token where the user is active completes successfully with a 200 and a rotated token.
• An MCP request bearing a user-mode JWT where IsUserActive returns false is rejected with an error rather than falling through to the global server.

Breaking changes

• Yes
• No

Any implementation of the OAuth2IdentityResolver interface must now implement the IsUserActive(ctx context.Context, userID string) (bool, error) method.

Security considerations

This closes a window where a deleted or deactivated user could continue to access MCP resources and silently rotate refresh tokens until their access token expired. The fix ensures revocation is enforced at both the request and refresh layers, consistent with how virtual key deactivation is already handled.

Checklist

• I read docs/contributing/README.md and followed the guidelines
• I added/updated tests where appropriate
• I updated documentation where needed
• I verified builds succeed (Go and UI)
• I verified the CI pipeline passes locally if applicable

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c7e5c18e-5e8f-47e8-9b0b-cd0ea81df64b

📥 Commits

Reviewing files that changed from the base of the PR and between 34cb83c and 1729701.

📒 Files selected for processing (8)

• framework/configstore/rdb.go
• framework/configstore/rdb_test.go
• transports/bifrost-http/handlers/mcpoauth2consent.go
• transports/bifrost-http/handlers/mcpoauth2consent_test.go
• transports/bifrost-http/handlers/mcpoauth2issuance.go
• transports/bifrost-http/handlers/mcpoauth2issuance_test.go
• transports/bifrost-http/handlers/mcpserver.go
• transports/bifrost-http/handlers/mcpserver_auth_test.go

---

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • Added active-user checks for user-based sign-in and token refresh flows.
  • Improved handling so deactivated users are blocked from continuing with expired or refreshed access.

• Bug Fixes

  • Revoking a virtual key now also revokes related refresh tokens, preventing further token renewal.
  • Requests tied to inactive users now stop earlier instead of falling back to unintended access paths.

• Tests

  • Added coverage for virtual key revocation and inactive-user refresh behavior.

Walkthrough

Adds an IsUserActive method to the OAuth2IdentityResolver interface and enforces user-mode liveness at two points: the OAuth2 token refresh handler and the MCP server resolution path. Also revokes VK-bound OAuth2 refresh tokens when a virtual key is deleted.

Liveness enforcement for users and VKs

Layer / File(s)	Summary
IsUserActive contract and fakeResolver support transports/bifrost-http/handlers/mcpoauth2consent.go, transports/bifrost-http/handlers/mcpoauth2consent_test.go	IsUserActive(ctx, userID) (bool, error) added to OAuth2IdentityResolver; fakeResolver extended with userInactive/userActiveErr fields and the corresponding method implementation.
User-mode liveness check in token refresh transports/bifrost-http/handlers/mcpoauth2issuance.go, transports/bifrost-http/handlers/mcpoauth2issuance_test.go	Refresh handler calls IsUserActive for MCPAuthModeUser tokens; returns server_error on lookup failure and invalid_grant with "user is no longer active" for inactive users. Tests seed a user-mode token and verify both rejection and success paths.
User-mode liveness gate in MCP server resolution transports/bifrost-http/handlers/mcpserver.go, transports/bifrost-http/handlers/mcpserver_auth_test.go	userScopedServer calls IsUserActive before resolving a representative VK, returning an error for inactive/deleted users instead of falling through to global fallback. New subtest asserts error when resolver marks user inactive.
VK deletion revokes bound refresh tokens framework/configstore/rdb.go, framework/configstore/rdb_test.go	DeleteVirtualKey now sets revoked_at on TableOAuth2RefreshToken rows where bf_mode = MCPAuthModeVK and bf_sub matches the deleted VK. Test verifies RevokedAt becomes non-nil after deletion.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• akshaydeo

Poem

🐇 A hop through the code, a check at the gate,
Old tokens revoked before it's too late.
If the user's gone dark, we send them away,
invalid_grant is all they will say.
The bunny guards tokens with vigilant care! 🔒

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 06-30-feat_adds_user_liveliness_checks_in_mcp_oauth

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

_{Comment @coderabbitai help to get the list of available commands.}

[Pratham-Mishra04]

• docs: clarify OAuth revocation behavior, disable_vk_identity flag, and token lifetime semantics #4807
• feat: revoke VK-mode OAuth2 grants on VK deletion and add user-liveness checks at refresh and request time #4806 👈 (View in Graphite)
• feat: scope pre-authenticated user requests to their representative virtual key in headers/both auth modes #4784
• feat: add virtualKeysByID secondary index and cache signing key + VK lookups on /mcp JWT auth path #4783
• feat: push OAuth2 sessions filtering and pagination to SQL with total count #4775
• fix: make MCP sessions table scrollable with sticky header and improved pagination UI #4774
• feat: add interactive MCP test client with header and OAuth auth support #4773
• fix: exclude terminal-status oauth configs from expiring token refresh query #4754
• feat: add disable_vk_identity to suppress virtual-key identity in OAuth consent flow #4751
• refactor: move Circuit Breaker above Pricing Overrides in sidebar, update icons, and fix template literal formatting #4750
• tests: add MCP auth e2e collection covering discovery, credential matrix, OAuth issuance, refresh rotation, revocation window, and runtime mode flip #4525 : 1 other dependent PR (#4744 )
• tests: add MCP auth mode e2e collection and Newman runner for headers/both/oauth #4524
• tests: add OAuth2 issuance, JWT verification, consent, discovery, sessions, and MCP auth unit tests #4523
• feat: add mcp_server_auth_mode and OAuth 2.1 gateway auth docs with gateway-auth page #4522
• ui: add OAuth Grants page and identity exact-match filter for MCP sessions #4511
• ui: add OAuth2 consent page and MCP server auth mode settings #4510
• feat: add OAuth2 session listing, revocation, family-revocation, VK liveness check, and sweep worker #4509
• feat: add JWT Bearer authentication path for /mcp with session validation and context injection #4508
• feat: add OAuth2ConsentHandler with VK, session, and user identity resolution modes #4507
• feat: add OAuth2 issuance endpoints (DCR, authorize, token) with PKCE and refresh token rotation #4506
• feat: add OAuth2 AS discovery endpoints, signing key management, and MCPServerAuthMode config #4505
• refactor: rename MCP handler files to remove underscores and drop redundant prefixes #4747
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[Pratham-Mishra04]

Merge activity

• Jun 30, 1:53 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Jun 30, 2:53 PM UTC: Graphite rebased this pull request as part of a merge.
• Jun 30, 2:54 PM UTC: @Pratham-Mishra04 merged this pull request with Graphite.

[greptile-apps]

Confidence Score: 4/5

Safe to merge with the guard-bypass caveat noted in a prior review comment still unresolved — a deployment where identityResolver is wired but ConfigStore is nil would let a deleted user's JWT fall through to the global MCP server.

The three enforcement points (VK deletion → token revocation, liveness check at refresh, liveness check at request time) are each correctly implemented and tested for their happy and failure paths. The VK-deletion revocation runs atomically within the existing transaction. The one open issue — the combined nil-guard in userScopedServer that short-circuits before IsUserActive runs when ConfigStore is nil — was flagged in a prior review comment and remains unaddressed in this revision; in that specific deployment shape, a removed user could still reach the global server until their access token expires.

transports/bifrost-http/handlers/mcpserver.go — the combined nil-guard on line 808 still bypasses the IsUserActive check when identityResolver is set but ConfigStore is nil.

Important Files Changed

Filename	Overview
framework/configstore/rdb.go	Adds revocation of vk-mode OAuth2 refresh tokens inside the existing DeleteVirtualKey transaction; uses correct WHERE clause (bf_mode=vk, bf_sub=id, revoked_at IS NULL) and sets revoked_at atomically with the rest of the VK cleanup.
framework/configstore/rdb_test.go	Adds TableOAuth2RefreshToken to the test migration setup and a new test that verifies revoked_at is set (not nil) after DeleteVirtualKey; test correctly seeds a vk-mode token and asserts revocation without deletion.
transports/bifrost-http/handlers/mcpoauth2consent.go	Adds IsUserActive to the OAuth2IdentityResolver interface with clear semantics: (false, nil) for gone/deactivated user, error only for transient failures. Breaking change well-documented in the PR.
transports/bifrost-http/handlers/mcpoauth2issuance.go	Adds user-mode liveness check in handleTokenRefresh after the VK-mode check; correctly differentiates transient errors (500 server_error) from confirmed-gone users (400 invalid_grant) without revoking the token row, matching the expected design.
transports/bifrost-http/handlers/mcpoauth2issuance_test.go	Adds two sub-tests for user liveness at refresh time (inactive → invalid_grant; active → 200). The transient-error path (IsUserActive returns a non-nil error → server_error) is not covered, leaving that branch untested.
transports/bifrost-http/handlers/mcpserver.go	Adds IsUserActive call in userScopedServer before VK resolution; however the combined nil-guard (identityResolver == nil
transports/bifrost-http/handlers/mcpserver_auth_test.go	Adds a test confirming getMCPServerForRequest returns an error when IsUserActive returns false; test correctly sets up a store with a valid VK to prove rejection is from the liveness check and not VK resolution.
transports/bifrost-http/handlers/mcpoauth2consent_test.go	Extends fakeResolver with userInactive and userActiveErr fields and implements IsUserActive; implementation correctly inverts the boolean and propagates the error, satisfying the updated interface.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant C as OAuth2 Client
    participant H as IssuanceHandler
    participant IR as IdentityResolver
    participant DB as ConfigStore

    Note over C,DB: Refresh token flow (user-mode)
    C->>H: "POST /token (grant_type=refresh_token)"
    H->>DB: GetOAuth2RefreshToken(hash)
    DB-->>H: "rt (BfMode=user, BfSub=user-1)"
    H->>IR: IsUserActive(user-1)
    alt user deleted/deactivated
        IR-->>H: (false, nil)
        H-->>C: 400 invalid_grant user is no longer active
    else transient error
        IR-->>H: (_, err)
        H-->>C: 500 server_error failed to verify user
    else user active
        IR-->>H: (true, nil)
        H->>DB: RotateOAuth2RefreshToken(old to new)
        H-->>C: 200 access_token + refresh_token
    end

    Note over C,DB: MCP request path (user-mode JWT)
    C->>H: "GET /mcp Bearer JWT bf_mode=user"
    H->>IR: IsUserActive(claims.sub)
    alt user deleted/deactivated
        IR-->>H: (false, nil)
        H-->>C: error user is no longer active
    else user active
        IR-->>H: (true, nil)
        H->>IR: ResolveUserVirtualKey(user-id)
        IR-->>H: vkID
        H->>DB: GetVirtualKey(vkID)
        DB-->>H: VK
        H-->>C: MCP response scoped server
    end

    Note over C,DB: VK deletion grant revocation
    C->>H: "DELETE /virtual-key/{id}"
    H->>DB: DeleteVirtualKey(id) in txn
    DB->>DB: "UPDATE oauth2_refresh_tokens SET revoked_at=now WHERE bf_mode=vk AND bf_sub=id AND revoked_at IS NULL"
    DB-->>H: ok
    H-->>C: 200

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant C as OAuth2 Client
    participant H as IssuanceHandler
    participant IR as IdentityResolver
    participant DB as ConfigStore

    Note over C,DB: Refresh token flow (user-mode)
    C->>H: "POST /token (grant_type=refresh_token)"
    H->>DB: GetOAuth2RefreshToken(hash)
    DB-->>H: "rt (BfMode=user, BfSub=user-1)"
    H->>IR: IsUserActive(user-1)
    alt user deleted/deactivated
        IR-->>H: (false, nil)
        H-->>C: 400 invalid_grant user is no longer active
    else transient error
        IR-->>H: (_, err)
        H-->>C: 500 server_error failed to verify user
    else user active
        IR-->>H: (true, nil)
        H->>DB: RotateOAuth2RefreshToken(old to new)
        H-->>C: 200 access_token + refresh_token
    end

    Note over C,DB: MCP request path (user-mode JWT)
    C->>H: "GET /mcp Bearer JWT bf_mode=user"
    H->>IR: IsUserActive(claims.sub)
    alt user deleted/deactivated
        IR-->>H: (false, nil)
        H-->>C: error user is no longer active
    else user active
        IR-->>H: (true, nil)
        H->>IR: ResolveUserVirtualKey(user-id)
        IR-->>H: vkID
        H->>DB: GetVirtualKey(vkID)
        DB-->>H: VK
        H-->>C: MCP response scoped server
    end

    Note over C,DB: VK deletion grant revocation
    C->>H: "DELETE /virtual-key/{id}"
    H->>DB: DeleteVirtualKey(id) in txn
    DB->>DB: "UPDATE oauth2_refresh_tokens SET revoked_at=now WHERE bf_mode=vk AND bf_sub=id AND revoked_at IS NULL"
    DB-->>H: ok
    H-->>C: 200

Loading

_{Reviews (2): Last reviewed commit: "feat: adds user liveliness checks in mcp..." | Re-trigger Greptile}