Skip to content

feat: add disable_vk_identity to suppress virtual-key identity in OAuth consent flow - #4751

Merged
Pratham-Mishra04 merged 1 commit into
devfrom
06-28-feat_adds_options_to_disable_vks_in_mcp_oauth_mode
Jun 30, 2026
Merged

feat: add disable_vk_identity to suppress virtual-key identity in OAuth consent flow#4751
Pratham-Mishra04 merged 1 commit into
devfrom
06-28-feat_adds_options_to_disable_vks_in_mcp_oauth_mode

Conversation

@Pratham-Mishra04

Copy link
Copy Markdown
Collaborator

Summary

Adds a disable_vk_identity configuration option that removes virtual-key identity from the OAuth consent flow. When enabled, virtual keys are no longer offered or accepted during OAuth consent, and existing virtual-key grants stop refreshing (they expire within one access-token TTL). The flag is only honored when an identity provider is configured, ensuring the consent flow always retains at least one usable identity path. Anonymous session access remains unaffected and is still governed by EnforceAuthOnInference.

Changes

  • Added DisableVKIdentity bool field to OAuth2ServerConfig with the constraint that it is only honored when an identity provider is configured and mcp_server_auth_mode is oauth.
  • availableModes() in the consent handler now conditionally excludes consentFlowModeVK when DisableVKIdentity is set and user mode is available.
  • Token refresh in the issuance handler rejects VK-mode grants with invalid_grant when DisableVKIdentity is active and user mode is available, causing existing VK sessions to expire within one access-token TTL.
  • OAuth2IssuanceHandler now accepts an OAuth2IdentityResolver so it can apply the same user-mode availability gate as the consent handler.
  • OAuth2IssuanceHandler is promoted to a named field on BifrostHTTPServer (matching the existing pattern for OAuth2ConsentHandler) so it can be injected with a resolver.
  • Config update endpoint validates that disable_vk_identity is only set when mcp_server_auth_mode is oauth, returning a 400 otherwise.
  • JSON schema updated with the new disable_vk_identity boolean field.
  • UI exposes a "Require identity-provider login" toggle in the MCP settings view, visible only in oauth mode when an IdP is configured (enterprise-only, gated via SCIM provider check). A warning alert is shown when the toggle is enabled, explaining that existing VK OAuth sessions will lose access within one access-token lifetime.
  • disable_vk_identity added to the CoreConfig TypeScript interface and to the dirty-state comparison in the MCP settings view.

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

# Core/Transports
go test ./transports/bifrost-http/handlers/...
  1. Configure an identity provider and set mcp_server_auth_mode to oauth.
  2. Enable disable_vk_identity via the API or UI toggle.
  3. Attempt to initiate an OAuth consent flow — verify the virtual-key option is absent from the consent page.
  4. Attempt to refresh an existing VK-mode token — verify the response is 400 invalid_grant with the message "virtual-key identity is no longer accepted; re-authenticate".
  5. Verify that setting disable_vk_identity without an identity provider configured results in the flag being silently ignored (VK remains available).
  6. Verify that setting disable_vk_identity when mcp_server_auth_mode is not oauth returns 400 disable_vk_identity is only valid when mcp_server_auth_mode is oauth.

Breaking changes

  • Yes
  • No

Enabling disable_vk_identity causes existing MCP clients authenticated via virtual-key OAuth grants to lose access within one access-token TTL. They must re-authenticate through the configured identity provider. The flag is opt-in and off by default, so no existing deployments are affected unless explicitly enabled.

Security considerations

This feature is intended to enforce identity-provider authentication for MCP OAuth flows, preventing virtual keys from being used as an OAuth identity mechanism. The guard that requires user mode to be available before the flag takes effect ensures the consent flow can never be left with zero usable identity options. The refresh cutoff uses the same availability gate as the consent handler, so the two surfaces remain consistent.

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

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
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.

@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 55 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 3f382e61-5d9c-4183-9ca9-6aa147f4d1fd

📥 Commits

Reviewing files that changed from the base of the PR and between e1cdee5 and ad3df93.

📒 Files selected for processing (9)
  • transports/bifrost-http/handlers/config.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/server/server.go
  • transports/config.schema.json
  • ui/app/workspace/config/views/mcpView.tsx
  • ui/lib/types/config.ts
📝 Walkthrough

Walkthrough

Adds a disable_vk_identity config flag to block virtual-key OAuth identity. The flag is defined in the JSON schema and TypeScript types, enforced in consent mode selection, token refresh rejection, and config update validation, wired into the server's issuance handler, and exposed in the enterprise UI as an IdP-gated toggle.

Changes

VK Identity Disable Feature

Layer / File(s) Summary
Config schema and TypeScript type
transports/config.schema.json, ui/lib/types/config.ts
disable_vk_identity is added to oauth2_server_config in the JSON schema and as an optional field in CoreConfig.oauth2_server_config.
Config handler validation
transports/bifrost-http/handlers/config.go
updateConfig returns HTTP 400 when disable_vk_identity is set while the effective MCP auth mode is not oauth.
Consent availableModes: VK suppression logic
transports/bifrost-http/handlers/mcpoauth2consent.go, transports/bifrost-http/handlers/mcpoauth2consent_test.go
availableModes() omits VK when DisableVKIdentity is enabled and user mode is available, and the consent tests cover the new combinations.
IssuanceHandler: identityResolver field and VK refresh cutoff
transports/bifrost-http/handlers/mcpoauth2issuance.go, transports/bifrost-http/handlers/mcpoauth2issuance_test.go
OAuth2IssuanceHandler accepts an identityResolver; handleTokenRefresh rejects VK-mode refresh grants with invalid_grant when VK is disabled and user mode is available. The new test covers resolver-present and resolver-absent paths.
Server: singleton IssuanceHandler with identityResolver
transports/bifrost-http/server/server.go
RegisterAPIRoutes constructs the OAuth2 issuance handler with s.OAuth2IdentityResolver.
MCPView: IdP-gated disable_vk_identity toggle
ui/app/workspace/config/views/mcpView.tsx
Queries SCIM providers, derives idpConfigured, tracks disable_vk_identity in change detection, and renders the OAuth2 toggle with a warning alert.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

  • danpiths

Poem

🐇 I hopped through the config with a flick of my toe,
VK keys got a gate, and the IdP got a glow.
If the switch says “no,” then the rabbit says “neigh” —
Re-auth and hop onward to another bright day!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding disable_vk_identity to suppress VK identity in OAuth consent flow.
Description check ✅ Passed The description is well structured and covers summary, changes, testing, security, and breaking change impact.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 06-28-feat_adds_options_to_disable_vks_in_mcp_oauth_mode

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

Pratham-Mishra04 commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator Author

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

@Pratham-Mishra04 Pratham-Mishra04 changed the title Y feat: add disable_vk_identity to suppress virtual-key identity in OAuth consent flow Jun 28, 2026
@greptile-apps

greptile-apps Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
transports/bifrost-http/handlers/config.go Adds validation so disable_vk_identity is only accepted for OAuth MCP auth mode.
transports/bifrost-http/handlers/mcpoauth2consent.go Updates consent mode selection to remove virtual-key identity when the new setting applies.
transports/bifrost-http/handlers/mcpoauth2issuance.go Adds resolver-aware refresh rejection for virtual-key OAuth grants.
transports/bifrost-http/server/server.go Passes the OAuth identity resolver into the issuance handler during route setup.
transports/config.schema.json Adds disable_vk_identity to the OAuth server config schema.
ui/app/workspace/config/views/mcpView.tsx Adds the MCP settings toggle and warning for requiring identity-provider login.
ui/lib/types/config.ts Adds the new optional config field to the TypeScript config type.

Reviews (9): Last reviewed commit: "feat: adds options to disable vks in mcp..." | Re-trigger Greptile

Comment thread transports/bifrost-http/server/server.go Outdated
Comment thread framework/configstore/tables/mcpoauth2server.go
@Pratham-Mishra04
Pratham-Mishra04 force-pushed the 06-28-feat_adds_options_to_disable_vks_in_mcp_oauth_mode branch from d3f43d6 to 6e6e126 Compare June 28, 2026 11:59

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@transports/bifrost-http/handlers/config.go`:
- Around line 563-571: The validation in config.go only checks
payload.ClientConfig.OAuth2ServerConfig, so it can miss an existing
DisableVKIdentity=true when a partial update changes mcp_server_auth_mode to a
non-oauth mode. Update the guard around the auth-mode check to validate the
merged/configured state used by the handler, not just the incoming payload,
using the existing config fields and effectiveAuthMode logic in the same block.
Ensure the SendError path triggers whenever DisableVKIdentity would remain set
while the final auth mode is not MCPServerAuthModeOAuth.

In `@transports/bifrost-http/server/server.go`:
- Around line 1408-1411: The OAuth2 issuance setup is passing nil into
handlers.NewOAuth2IssuanceHandler, which leaves
OAuth2IssuanceHandler.handleTokenRefresh without an identityResolver and
bypasses the VK-refresh cutoff logic. Update the server initialization in the
code path that creates or registers OAuth2IssuanceHandler so it receives a real
OAuth2IdentityResolver instance from the HTTP server instead of nil, and ensure
RegisterRoutes still uses that configured handler.

In `@ui/app/workspace/config/views/mcpView.tsx`:
- Around line 25-31: The SCIM prerequisite check in mcpView.tsx is treating
loading, missing data, and request errors as the same “not configured” state.
Update the logic around useGetSCIMProvidersQuery and idpConfigured so failures
and in-flight requests are handled separately from an empty provider list, and
only hide the toggle when SCIM is truly unavailable. Make the toggle rendering
in mcpView.tsx distinguish pending/error states from “no IdP configured” so an
already-enabled setting isn’t lost.
- Around line 598-645: The hidden oauth-only flag remains dirty when
`mcp_server_auth_mode` changes away from "oauth", causing `hasChanges` and
`handleSave` in `mcpView.tsx` to submit an invalid `disable_vk_identity` value.
Update the auth-mode change flow (the handler that updates
`localConfig.mcp_server_auth_mode`) to clear or reset
`localConfig.oauth2_server_config.disable_vk_identity` whenever the mode is no
longer "oauth", so the form cannot persist the rejected setting and the save
path stays valid.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 56eb980b-9aea-438e-b281-feaf0c1907c0

📥 Commits

Reviewing files that changed from the base of the PR and between c834625 and 92bc69c.

📒 Files selected for processing (9)
  • transports/bifrost-http/handlers/config.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/server/server.go
  • transports/config.schema.json
  • ui/app/workspace/config/views/mcpView.tsx
  • ui/lib/types/config.ts

Comment thread transports/bifrost-http/handlers/config.go
Comment thread transports/bifrost-http/server/server.go Outdated
Comment thread ui/app/workspace/config/views/mcpView.tsx
Comment thread ui/app/workspace/config/views/mcpView.tsx

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@transports/bifrost-http/server/server.go`:
- Around line 1760-1763: The `OAuth2SweepWorker` started in `Serve`/`Start` is
not cleaned up when the `errChan` branch returns on listen/serve failure. Update
the `Start` or `Serve` error path to stop the worker before returning, mirroring
the existing bootstrap and graceful shutdown cleanup; use the
`OAuth2SweepWorker` and `newOAuth2SweepWorker` flow to locate the fix.
- Around line 1797-1803: The OAuth2 signing-key bootstrap in server startup is
currently warning and continuing when GetOAuth2SigningKey fails, which leaves
discovery enabled before JWKS/JWT signing is ready. Update the bootstrap path in
server.go to fail closed by returning an error or aborting startup from the
surrounding initialization flow instead of just logging in the
s.Config.ConfigStore.GetOAuth2SigningKey(s.Ctx) branch. Keep the check gated by
s.Config.ClientConfig.IsMCPOAuthDiscoveryEnabled(), and ensure the caller of
this startup block propagates the failure so the server does not advertise the
OAuth2 surface until the signing key is successfully loaded.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bb1b8233-7c16-45c9-98db-fd83807576d7

📥 Commits

Reviewing files that changed from the base of the PR and between 92bc69c and e1cdee5.

📒 Files selected for processing (9)
  • transports/bifrost-http/handlers/config.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/server/server.go
  • transports/config.schema.json
  • ui/app/workspace/config/views/mcpView.tsx
  • ui/lib/types/config.ts
🚧 Files skipped from review as they are similar to previous changes (8)
  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/handlers/mcpoauth2consent.go
  • ui/app/workspace/config/views/mcpView.tsx
  • transports/bifrost-http/handlers/mcpoauth2consent_test.go
  • ui/lib/types/config.ts
  • transports/bifrost-http/handlers/mcpoauth2issuance.go
  • transports/config.schema.json
  • transports/bifrost-http/handlers/mcpoauth2issuance_test.go

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@transports/bifrost-http/server/server.go`:
- Around line 1760-1763: The `OAuth2SweepWorker` started in `Serve`/`Start` is
not cleaned up when the `errChan` branch returns on listen/serve failure. Update
the `Start` or `Serve` error path to stop the worker before returning, mirroring
the existing bootstrap and graceful shutdown cleanup; use the
`OAuth2SweepWorker` and `newOAuth2SweepWorker` flow to locate the fix.
- Around line 1797-1803: The OAuth2 signing-key bootstrap in server startup is
currently warning and continuing when GetOAuth2SigningKey fails, which leaves
discovery enabled before JWKS/JWT signing is ready. Update the bootstrap path in
server.go to fail closed by returning an error or aborting startup from the
surrounding initialization flow instead of just logging in the
s.Config.ConfigStore.GetOAuth2SigningKey(s.Ctx) branch. Keep the check gated by
s.Config.ClientConfig.IsMCPOAuthDiscoveryEnabled(), and ensure the caller of
this startup block propagates the failure so the server does not advertise the
OAuth2 surface until the signing key is successfully loaded.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bb1b8233-7c16-45c9-98db-fd83807576d7

📥 Commits

Reviewing files that changed from the base of the PR and between 92bc69c and e1cdee5.

📒 Files selected for processing (9)
  • transports/bifrost-http/handlers/config.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/server/server.go
  • transports/config.schema.json
  • ui/app/workspace/config/views/mcpView.tsx
  • ui/lib/types/config.ts
🚧 Files skipped from review as they are similar to previous changes (8)
  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/handlers/mcpoauth2consent.go
  • ui/app/workspace/config/views/mcpView.tsx
  • transports/bifrost-http/handlers/mcpoauth2consent_test.go
  • ui/lib/types/config.ts
  • transports/bifrost-http/handlers/mcpoauth2issuance.go
  • transports/config.schema.json
  • transports/bifrost-http/handlers/mcpoauth2issuance_test.go
🛑 Comments failed to post (2)
transports/bifrost-http/server/server.go (2)

1760-1763: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Mirror OAuth2SweepWorker cleanup in the Serve-error path.

This worker is stopped on bootstrap failures and graceful shutdown, but Start() still returns from the errChan branch without stopping it. If the process stays alive after a listen/serve failure, the sweep goroutine keeps running against a server that never came up.

Suggested fix
case err := <-errChan:
	if s.IntegrationHandler != nil {
		s.IntegrationHandler.Close()
	}
+	if s.OAuth2SweepWorker != nil {
+		logger.Info("stopping oauth2 sweep worker...")
+		s.OAuth2SweepWorker.stop()
+		s.OAuth2SweepWorker = nil
+	}
	if s.wsPool != nil {
		s.wsPool.Close()
	}
	return err

Also applies to: 1957-1960

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@transports/bifrost-http/server/server.go` around lines 1760 - 1763, The
`OAuth2SweepWorker` started in `Serve`/`Start` is not cleaned up when the
`errChan` branch returns on listen/serve failure. Update the `Start` or `Serve`
error path to stop the worker before returning, mirroring the existing bootstrap
and graceful shutdown cleanup; use the `OAuth2SweepWorker` and
`newOAuth2SweepWorker` flow to locate the fix.

1797-1803: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Fail startup if OAuth2 signing-key bootstrap fails.

When discovery is enabled, this only logs and continues, so the server can advertise/register the OAuth2 surface before JWKS/JWT signing is actually ready. That should fail closed during bootstrap instead of degrading into first-request auth failures. As per path instructions, "Apply HTTP/API security review: authentication and authorization checks, fail-closed behavior..."

Suggested fix
if s.Config.ConfigStore != nil && s.Config.ClientConfig.IsMCPOAuthDiscoveryEnabled() {
-	if _, keyErr := s.Config.ConfigStore.GetOAuth2SigningKey(s.Ctx); keyErr != nil {
-		logger.Warn("oauth2: failed to bootstrap signing key: %v", keyErr)
-	}
+	if _, keyErr := s.Config.ConfigStore.GetOAuth2SigningKey(s.Ctx); keyErr != nil {
+		return fmt.Errorf("oauth2: failed to bootstrap signing key: %w", keyErr)
+	}
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

	// Bootstrap OAuth2 signing key when discovery is enabled — ensures JWKS
	// and JWT signing are ready before the first request arrives.
	if s.Config.ConfigStore != nil && s.Config.ClientConfig.IsMCPOAuthDiscoveryEnabled() {
		if _, keyErr := s.Config.ConfigStore.GetOAuth2SigningKey(s.Ctx); keyErr != nil {
			return fmt.Errorf("oauth2: failed to bootstrap signing key: %w", keyErr)
		}
	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@transports/bifrost-http/server/server.go` around lines 1797 - 1803, The
OAuth2 signing-key bootstrap in server startup is currently warning and
continuing when GetOAuth2SigningKey fails, which leaves discovery enabled before
JWKS/JWT signing is ready. Update the bootstrap path in server.go to fail closed
by returning an error or aborting startup from the surrounding initialization
flow instead of just logging in the
s.Config.ConfigStore.GetOAuth2SigningKey(s.Ctx) branch. Keep the check gated by
s.Config.ClientConfig.IsMCPOAuthDiscoveryEnabled(), and ensure the caller of
this startup block propagates the failure so the server does not advertise the
OAuth2 surface until the signing key is successfully loaded.

Source: Path instructions

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 29, 2026
Comment thread transports/config.schema.json

Pratham-Mishra04 commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator Author

Merge activity

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

@Pratham-Mishra04
Pratham-Mishra04 changed the base branch from 06-28-fix_ui_fixes to graphite-base/4751 June 30, 2026 14:28
@Pratham-Mishra04
Pratham-Mishra04 changed the base branch from graphite-base/4751 to dev June 30, 2026 14:31
@Pratham-Mishra04
Pratham-Mishra04 dismissed coderabbitai[bot]’s stale review June 30, 2026 14:31

The base branch was changed.

@Pratham-Mishra04
Pratham-Mishra04 requested a review from a team as a code owner June 30, 2026 14:31
@Pratham-Mishra04
Pratham-Mishra04 force-pushed the 06-28-feat_adds_options_to_disable_vks_in_mcp_oauth_mode branch from 6673c1a to 4624785 Compare June 30, 2026 14:31
@Pratham-Mishra04
Pratham-Mishra04 merged commit 19494bd into dev Jun 30, 2026
15 of 16 checks passed
@Pratham-Mishra04
Pratham-Mishra04 deleted the 06-28-feat_adds_options_to_disable_vks_in_mcp_oauth_mode branch June 30, 2026 14:34
akshaydeo pushed a commit that referenced this pull request Jul 1, 2026
…Auth consent flow (#4751)

## Summary

Adds a `disable_vk_identity` configuration option that removes virtual-key identity from the OAuth consent flow. When enabled, virtual keys are no longer offered or accepted during OAuth consent, and existing virtual-key grants stop refreshing (they expire within one access-token TTL). The flag is only honored when an identity provider is configured, ensuring the consent flow always retains at least one usable identity path. Anonymous session access remains unaffected and is still governed by `EnforceAuthOnInference`.

## Changes

- Added `DisableVKIdentity bool` field to `OAuth2ServerConfig` with the constraint that it is only honored when an identity provider is configured and `mcp_server_auth_mode` is `oauth`.
- `availableModes()` in the consent handler now conditionally excludes `consentFlowModeVK` when `DisableVKIdentity` is set and user mode is available.
- Token refresh in the issuance handler rejects VK-mode grants with `invalid_grant` when `DisableVKIdentity` is active and user mode is available, causing existing VK sessions to expire within one access-token TTL.
- `OAuth2IssuanceHandler` now accepts an `OAuth2IdentityResolver` so it can apply the same user-mode availability gate as the consent handler.
- `OAuth2IssuanceHandler` is promoted to a named field on `BifrostHTTPServer` (matching the existing pattern for `OAuth2ConsentHandler`) so it can be injected with a resolver.
- Config update endpoint validates that `disable_vk_identity` is only set when `mcp_server_auth_mode` is `oauth`, returning a `400` otherwise.
- JSON schema updated with the new `disable_vk_identity` boolean field.
- UI exposes a "Require identity-provider login" toggle in the MCP settings view, visible only in `oauth` mode when an IdP is configured (enterprise-only, gated via SCIM provider check). A warning alert is shown when the toggle is enabled, explaining that existing VK OAuth sessions will lose access within one access-token lifetime.
- `disable_vk_identity` added to the `CoreConfig` TypeScript interface and to the dirty-state comparison in the MCP settings view.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (React)
- [ ] Docs

## How to test

```sh
# Core/Transports
go test ./transports/bifrost-http/handlers/...
```

1. Configure an identity provider and set `mcp_server_auth_mode` to `oauth`.
2. Enable `disable_vk_identity` via the API or UI toggle.
3. Attempt to initiate an OAuth consent flow — verify the virtual-key option is absent from the consent page.
4. Attempt to refresh an existing VK-mode token — verify the response is `400 invalid_grant` with the message "virtual-key identity is no longer accepted; re-authenticate".
5. Verify that setting `disable_vk_identity` without an identity provider configured results in the flag being silently ignored (VK remains available).
6. Verify that setting `disable_vk_identity` when `mcp_server_auth_mode` is not `oauth` returns `400 disable_vk_identity is only valid when mcp_server_auth_mode is oauth`.

## Breaking changes

- [x] Yes
- [ ] No

Enabling `disable_vk_identity` causes existing MCP clients authenticated via virtual-key OAuth grants to lose access within one access-token TTL. They must re-authenticate through the configured identity provider. The flag is opt-in and off by default, so no existing deployments are affected unless explicitly enabled.

## Security considerations

This feature is intended to enforce identity-provider authentication for MCP OAuth flows, preventing virtual keys from being used as an OAuth identity mechanism. The guard that requires user mode to be available before the flag takes effect ensures the consent flow can never be left with zero usable identity options. The refresh cutoff uses the same availability gate as the consent handler, so the two surfaces remain consistent.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
akhsaul pushed a commit to akhsaul/bifrost that referenced this pull request Aug 27, 2026
…Auth consent flow (maximhq#4751)

## Summary

Adds a `disable_vk_identity` configuration option that removes virtual-key identity from the OAuth consent flow. When enabled, virtual keys are no longer offered or accepted during OAuth consent, and existing virtual-key grants stop refreshing (they expire within one access-token TTL). The flag is only honored when an identity provider is configured, ensuring the consent flow always retains at least one usable identity path. Anonymous session access remains unaffected and is still governed by `EnforceAuthOnInference`.

## Changes

- Added `DisableVKIdentity bool` field to `OAuth2ServerConfig` with the constraint that it is only honored when an identity provider is configured and `mcp_server_auth_mode` is `oauth`.
- `availableModes()` in the consent handler now conditionally excludes `consentFlowModeVK` when `DisableVKIdentity` is set and user mode is available.
- Token refresh in the issuance handler rejects VK-mode grants with `invalid_grant` when `DisableVKIdentity` is active and user mode is available, causing existing VK sessions to expire within one access-token TTL.
- `OAuth2IssuanceHandler` now accepts an `OAuth2IdentityResolver` so it can apply the same user-mode availability gate as the consent handler.
- `OAuth2IssuanceHandler` is promoted to a named field on `BifrostHTTPServer` (matching the existing pattern for `OAuth2ConsentHandler`) so it can be injected with a resolver.
- Config update endpoint validates that `disable_vk_identity` is only set when `mcp_server_auth_mode` is `oauth`, returning a `400` otherwise.
- JSON schema updated with the new `disable_vk_identity` boolean field.
- UI exposes a "Require identity-provider login" toggle in the MCP settings view, visible only in `oauth` mode when an IdP is configured (enterprise-only, gated via SCIM provider check). A warning alert is shown when the toggle is enabled, explaining that existing VK OAuth sessions will lose access within one access-token lifetime.
- `disable_vk_identity` added to the `CoreConfig` TypeScript interface and to the dirty-state comparison in the MCP settings view.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (React)
- [ ] Docs

## How to test

```sh
# Core/Transports
go test ./transports/bifrost-http/handlers/...
```

1. Configure an identity provider and set `mcp_server_auth_mode` to `oauth`.
2. Enable `disable_vk_identity` via the API or UI toggle.
3. Attempt to initiate an OAuth consent flow — verify the virtual-key option is absent from the consent page.
4. Attempt to refresh an existing VK-mode token — verify the response is `400 invalid_grant` with the message "virtual-key identity is no longer accepted; re-authenticate".
5. Verify that setting `disable_vk_identity` without an identity provider configured results in the flag being silently ignored (VK remains available).
6. Verify that setting `disable_vk_identity` when `mcp_server_auth_mode` is not `oauth` returns `400 disable_vk_identity is only valid when mcp_server_auth_mode is oauth`.

## Breaking changes

- [x] Yes
- [ ] No

Enabling `disable_vk_identity` causes existing MCP clients authenticated via virtual-key OAuth grants to lose access within one access-token TTL. They must re-authenticate through the configured identity provider. The flag is opt-in and off by default, so no existing deployments are affected unless explicitly enabled.

## Security considerations

This feature is intended to enforce identity-provider authentication for MCP OAuth flows, preventing virtual keys from being used as an OAuth identity mechanism. The guard that requires user mode to be available before the flag takes effect ensures the consent flow can never be left with zero usable identity options. The refresh cutoff uses the same availability gate as the consent handler, so the two surfaces remain consistent.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants