Skip to content

feat: add OAuth2 AS discovery endpoints, signing key management, and MCPServerAuthMode config#4505

Open
Pratham-Mishra04 wants to merge 1 commit into
devfrom
06-16-feat_adds_mcp_server_oauth_tables
Open

feat: add OAuth2 AS discovery endpoints, signing key management, and MCPServerAuthMode config#4505
Pratham-Mishra04 wants to merge 1 commit into
devfrom
06-16-feat_adds_mcp_server_oauth_tables

Conversation

@Pratham-Mishra04

Copy link
Copy Markdown
Collaborator

Summary

This PR introduces the foundational OAuth 2.1 authorization server infrastructure for Bifrost's /mcp endpoint. It adds a configurable mcp_server_auth_mode that controls how inbound MCP clients are authenticated, enabling Bifrost to act as a spec-compliant OAuth 2.1 AS with RFC-mandated discovery endpoints, a JWKS endpoint, and a persistent RS256 signing keypair.

Changes

  • MCPServerAuthMode — new varchar column on TableClientConfig with three modes:
    • headers (default): existing VK/api-key/session header auth only; discovery endpoints return 404.
    • both: accepts both header credentials and Bifrost-issued JWTs; discovery endpoints are live.
    • oauth: Bifrost JWTs only; header credentials are rejected on /mcp. Breaking for existing VK-based MCP integrations.
  • OAuth2ServerConfig — new JSON blob column on TableClientConfig holding AS-specific settings (IssuerURL, AuthCodeTTL, AccessTokenTTL). Serialized via BeforeSave/AfterFind hooks. Only meaningful when mode is both or oauth.
  • OAuth2SigningKey — RS2048 keypair generated on first use and persisted in governance_config under oauth2_signing_key. The private key PEM is encrypted at rest via framework/encrypt when encryption is enabled.
  • GetOAuth2SigningKey — new ConfigStore interface method that lazily generates and persists the signing keypair on first call, always returning a usable key.
  • OAuth2DiscoveryHandler — serves the three well-known discovery endpoints:
    • GET /.well-known/oauth-protected-resource[/{path}] (RFC 9728)
    • GET /.well-known/oauth-authorization-server[/{path}] (RFC 8414)
    • GET /.well-known/jwks.json (RFC 7517)
      All three return 404 when MCPServerAuthMode is headers. Routes are always registered; the mode flag is the feature toggle.
  • oauth2IssuerURL / oauth2ServerCfg — utility helpers that resolve the effective issuer URL (configured IssuerURL or request-derived fallback) and AS config defaults.
  • OAuth2ConsentScopeName — new temp-token scope for binding browser sessions to pending authorization requests on the public consent page.
  • Database migration add_oauth2_server_tables — adds mcp_server_auth_mode and oauth2_server_config_json columns to config_client.
  • Config schemamcp_server_auth_mode and oauth2_server_config added to config.schema.json with full descriptions and validation.
  • IsMCPOAuthDiscoveryEnabled — helper on ClientConfig that returns true when mode is both or oauth.

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/...
  1. Start Bifrost with mcp_server_auth_mode unset (or "headers"). Confirm GET /.well-known/oauth-authorization-server returns 404.
  2. Set mcp_server_auth_mode to "both" and restart. Confirm:
    • GET /.well-known/oauth-authorization-server returns a valid JSON document with issuer, authorization_endpoint, token_endpoint, etc.
    • GET /.well-known/oauth-protected-resource returns a document pointing to /mcp.
    • GET /.well-known/jwks.json returns a JWKS with one RS256 key entry.
  3. Confirm the signing keypair is persisted in governance_config and survives a restart (same kid returned).
  4. Set mcp_server_auth_mode to "oauth" and confirm header-credential MCP requests are rejected.

New config fields:

Field Type Default Description
mcp_server_auth_mode "headers" | "both" | "oauth" "headers" Inbound MCP client auth mode
oauth2_server_config.issuer_url string / env var (request host) Stable AS issuer URL
oauth2_server_config.auth_code_ttl int (seconds) 600 Authorization code lifetime
oauth2_server_config.access_token_ttl int (seconds) 600 JWT Bearer token lifetime

Breaking changes

  • Yes
  • No

Setting mcp_server_auth_mode to "oauth" disables VK/api-key/session header authentication on /mcp. Existing virtual-key MCP integrations will stop working. Use "both" for a non-breaking migration path that accepts both credential types simultaneously.

Security considerations

  • The RS256 private key is encrypted at rest using framework/encrypt when encryption is enabled. The plaintext key is only held in memory during the signing operation.
  • The OAuth2ConsentScopeName temp token is the sole credential binding a browser session to a pending authorization request on the public (unauthenticated) consent page — it must be treated as a short-lived secret.
  • Refresh tokens have no timer-based expiry; they are invalidated only by rotation on use, subject liveness checks, explicit revocation, or enforcement policy changes.
  • Multi-host / reverse-proxy deployments must set a stable issuer_url; omitting it causes token verification failures when the Host header differs across nodes.

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

CLAassistant commented Jun 17, 2026

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 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8ec510fa-0e12-4aff-a943-fc689184ea60

📥 Commits

Reviewing files that changed from the base of the PR and between d9c0026 and 04ec54f.

📒 Files selected for processing (15)
  • framework/configstore/clientconfig.go
  • framework/configstore/migrations.go
  • framework/configstore/rdb.go
  • framework/configstore/store.go
  • framework/configstore/tables/clientconfig.go
  • framework/configstore/tables/oauth2_server.go
  • framework/temptoken/scope.go
  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/handlers/governance_test.go
  • transports/bifrost-http/handlers/middlewares.go
  • transports/bifrost-http/handlers/oauth2_discovery.go
  • transports/bifrost-http/handlers/oauth2_utils.go
  • transports/bifrost-http/lib/config_test.go
  • transports/bifrost-http/server/server.go
  • transports/config.schema.json
💤 Files with no reviewable changes (9)
  • transports/bifrost-http/handlers/oauth2_utils.go
  • transports/bifrost-http/handlers/governance_test.go
  • transports/bifrost-http/server/server.go
  • transports/bifrost-http/handlers/middlewares.go
  • transports/config.schema.json
  • framework/configstore/migrations.go
  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/lib/config_test.go
  • transports/bifrost-http/handlers/oauth2_discovery.go
✅ Files skipped from review due to trivial changes (1)
  • framework/temptoken/scope.go
🚧 Files skipped from review as they are similar to previous changes (5)
  • framework/configstore/store.go
  • framework/configstore/clientconfig.go
  • framework/configstore/tables/oauth2_server.go
  • framework/configstore/tables/clientconfig.go
  • framework/configstore/rdb.go

📝 Walkthrough

Summary by CodeRabbit

  • New Features
    • Added inbound /mcp authentication modes (headers/both/oauth) and OAuth2 server settings (issuer URL plus token lifetimes).
    • Introduced OAuth2 discovery endpoints for protected-resource metadata, authorization-server metadata, and JWKS (served only when discovery is enabled).
    • Added automatic OAuth2 signing-key generation and persistence for JWKS.
  • Bug Fixes
    • Improved config updates by validating MCP/OAuth2 settings and preserving existing auth-related values during partial updates.
  • Chores
    • Added database migration to store the new MCP/OAuth2 configuration fields.

Walkthrough

Adds MCP OAuth2 authentication support across config, persistence, discovery endpoints, and HTTP wiring, including signing-key storage and well-known metadata/JWKS routes.

Changes

MCP OAuth2 Discovery Infrastructure

Layer / File(s) Summary
OAuth2 types and config fields
framework/configstore/tables/oauth2_server.go, framework/configstore/tables/clientconfig.go, framework/configstore/clientconfig.go, framework/temptoken/scope.go, transports/config.schema.json
Defines MCP auth modes, OAuth2 server config and signing-key types, extends client config models and schema fields, and adds MCP OAuth discovery and consent-scope helpers.
Migration and config-store persistence
framework/configstore/migrations.go, framework/configstore/store.go, framework/configstore/rdb.go
Adds the OAuth2 client-config migration, updates config-store read/write mapping, extends the config-store interface, and implements persisted OAuth2 signing-key retrieval and generation.
OAuth2 discovery handlers and helpers
transports/bifrost-http/handlers/oauth2_discovery.go, transports/bifrost-http/handlers/oauth2_utils.go
Adds well-known discovery routes for protected-resource metadata, authorization-server metadata, and JWKS, plus issuer and resource URL helpers and RSA public-key parsing/JWK conversion.
Config update, middleware, and server wiring
transports/bifrost-http/handlers/config.go, transports/bifrost-http/handlers/middlewares.go, transports/bifrost-http/server/server.go, transports/bifrost-http/lib/config_test.go, transports/bifrost-http/handlers/governance_test.go
Validates MCP auth mode and OAuth2 server config during config updates, widens the auth skip list for well-known routes, registers discovery routes during API setup, and updates the mock config store and governance test data for the new interface and config paths.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant OAuth2DiscoveryHandler
  participant ClientConfig
  participant RDBConfigStore
  participant GovernanceConfigDB

  Client->>OAuth2DiscoveryHandler: GET /.well-known/jwks.json
  OAuth2DiscoveryHandler->>ClientConfig: IsMCPOAuthDiscoveryEnabled()
  alt discovery disabled
    OAuth2DiscoveryHandler-->>Client: 404
  else discovery enabled
    OAuth2DiscoveryHandler->>RDBConfigStore: GetOAuth2SigningKey(ctx)
    RDBConfigStore->>GovernanceConfigDB: load signing key
    alt key exists
      GovernanceConfigDB-->>RDBConfigStore: persisted key
      RDBConfigStore->>RDBConfigStore: Decrypt()
    else key missing
      RDBConfigStore->>RDBConfigStore: generate RSA-2048 keypair
      RDBConfigStore->>GovernanceConfigDB: persist key material
    end
    RDBConfigStore-->>OAuth2DiscoveryHandler: OAuth2SigningKey
    OAuth2DiscoveryHandler->>OAuth2DiscoveryHandler: parseRSAPublicKeyPEM
    OAuth2DiscoveryHandler->>OAuth2DiscoveryHandler: rsaPublicKeyToJWK
    OAuth2DiscoveryHandler-->>Client: JWKS JSON
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • maximhq/bifrost#4398: Also changes framework/configstore/clientconfig.go hash/reconciliation behavior in the same code path.
  • maximhq/bifrost#4595: Also updates ClientConfig.GenerateClientConfigHash, overlapping with the hash logic changed here.
  • maximhq/bifrost#4653: Also modifies ClientConfig and its hash-based reconciliation inputs.

Suggested reviewers

  • akshaydeo
  • danpiths
  • roroghost17

Poem

🐇 I hopped by .well-known/ in the moonlit air,
With RSA keys and TTLs to spare.
Headers, both, or oauth now sing,
While discovery routes wake up in spring.
A nibble of config, a JWKS glow—
Hooray for MCP’s shiny new flow!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main OAuth2 discovery, signing key, and MCP auth-mode changes.
Description check ✅ Passed The PR description follows the template well and includes summary, changes, testing, breaking changes, security, and checklist sections.
Docstring Coverage ✅ Passed Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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-16-feat_adds_mcp_server_oauth_tables

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.

@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 4/5

Safe to merge after fixing the issuer URL trailing-slash normalization; all other changes are well-structured foundational work.

The discovery handlers concatenate base + "/oauth2/authorize" etc. without stripping a potential trailing slash from the configured issuer URL, producing invalid double-slash endpoint URLs in the AS metadata document that would break OAuth clients in any deployment where the operator includes a trailing slash in issuer_url. The one-liner fix in oauth2IssuerURL keeps this from reaching production in a broken state. The signing-key upsert logic, migration, encryption mirroring, and auth-mode gating are all correctly implemented.

transports/bifrost-http/handlers/oauth2_utils.go — the issuer URL normalization fix belongs here; transports/bifrost-http/handlers/oauth2_discovery.go is the consumer that exposes the invalid URLs.

Important Files Changed

Filename Overview
transports/bifrost-http/handlers/oauth2_utils.go oauth2IssuerURL does not strip trailing slash from configured IssuerURL; oauth2MCPResourceURL correctly trims but callers of the raw issuer URL do not.
transports/bifrost-http/handlers/oauth2_discovery.go New AS metadata / PRM / JWKS handlers; trailing-slash in configured IssuerURL produces double-slash endpoint URLs in handleASMetadata and authorization_servers in handlePRM.
framework/configstore/rdb.go Adds GetOAuth2SigningKey with upsert-on-conflict to handle concurrent first-use safely; TOCTOU race addressed and post-conflict reload is correct.
framework/configstore/tables/oauth2_server.go New types for MCPServerAuthMode, OAuth2ServerConfig, and OAuth2SigningKey with encrypt/decrypt mirroring existing BeforeSave/AfterFind patterns.
transports/bifrost-http/handlers/config.go Adds MCPServerAuthMode enum validation and OAuth2ServerConfig guarding for headers mode; zero-TTL storage from partial updates still unaddressed per prior review.
framework/configstore/migrations.go Adds add_oauth2_server_tables migration with has-column guards and a rollback; insertion order between two existing steps is safe because the migrator tracks by ID, not position.
framework/configstore/tables/clientconfig.go Adds MCPServerAuthMode varchar column and OAuth2ServerConfigJSON text column with proper BeforeSave/AfterFind serialization hooks.
framework/configstore/clientconfig.go Adds MCPServerAuthMode and OAuth2ServerConfig fields with IsMCPOAuthDiscoveryEnabled helper; hash skips empty auth mode to avoid hash churn on upgrade.
transports/bifrost-http/handlers/middlewares.go Adds /.well-known/ as an unauthenticated prefix; broad by convention but any future /.well-known/ route will also bypass auth automatically.
transports/config.schema.json Adds mcp_server_auth_mode enum and oauth2_server_config object with minimum:1 on TTL fields; schema correctly aligns with Go types and handler validation.
transports/bifrost-http/server/server.go Registers OAuth2DiscoveryHandler routes alongside existing handlers; dependency injection follows existing pattern.
framework/configstore/store.go Extends ConfigStore interface with GetOAuth2SigningKey; mock in config_test.go satisfies the interface correctly.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Client as MCP Client
    participant BF as Bifrost
    participant DB as governance_config

    Client->>BF: GET /.well-known/oauth-authorization-server
    BF->>BF: discoveryEnabled()?
    alt "mode == headers"
        BF-->>Client: 404
    else "mode == both or oauth"
        BF->>BF: oauth2IssuerURL(ctx, store)
        BF-->>Client: 200 AS metadata (issuer, endpoints, jwks_uri)
    end

    Client->>BF: GET /.well-known/jwks.json
    BF->>DB: GetOAuth2SigningKey
    alt key exists
        DB-->>BF: decrypt(private_key_pem)
    else first call
        BF->>BF: rsa.GenerateKey(2048)
        BF->>DB: INSERT ON CONFLICT DO NOTHING
        alt won insert
            DB-->>BF: persisted key
        else lost race
            DB-->>BF: reload winner key
        end
    end
    BF-->>Client: 200 JWKS keys RSA

    Client->>BF: GET /.well-known/oauth-protected-resource
    BF-->>Client: 200 resource /mcp authorization_servers issuer
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 Client as MCP Client
    participant BF as Bifrost
    participant DB as governance_config

    Client->>BF: GET /.well-known/oauth-authorization-server
    BF->>BF: discoveryEnabled()?
    alt "mode == headers"
        BF-->>Client: 404
    else "mode == both or oauth"
        BF->>BF: oauth2IssuerURL(ctx, store)
        BF-->>Client: 200 AS metadata (issuer, endpoints, jwks_uri)
    end

    Client->>BF: GET /.well-known/jwks.json
    BF->>DB: GetOAuth2SigningKey
    alt key exists
        DB-->>BF: decrypt(private_key_pem)
    else first call
        BF->>BF: rsa.GenerateKey(2048)
        BF->>DB: INSERT ON CONFLICT DO NOTHING
        alt won insert
            DB-->>BF: persisted key
        else lost race
            DB-->>BF: reload winner key
        end
    end
    BF-->>Client: 200 JWKS keys RSA

    Client->>BF: GET /.well-known/oauth-protected-resource
    BF-->>Client: 200 resource /mcp authorization_servers issuer
Loading

Reviews (9): Last reviewed commit: "feat: adds mcp server oauth tables" | Re-trigger Greptile

Comment thread framework/configstore/rdb.go
Comment thread framework/configstore/tables/oauth2_server.go
Comment thread transports/bifrost-http/handlers/oauth2_discovery.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.

Actionable comments posted: 10

🧹 Nitpick comments (1)
transports/config.schema.json (1)

276-280: ⚡ Quick win

Declare the default for mcp_server_auth_mode in schema.

The description documents 'headers' as the default, but the schema does not set "default": "headers". Add it so schema-driven tooling and generated config stay aligned with runtime expectations.

Suggested fix
         "mcp_server_auth_mode": {
           "type": "string",
           "enum": ["headers", "both", "oauth"],
+          "default": "headers",
           "description": "How /mcp authenticates inbound MCP clients. 'headers' (default): VK/api-key/session headers only, discovery disabled. 'both': accepts header credentials and Bifrost-issued JWTs, discovery enabled. 'oauth': Bifrost JWTs only — WARNING: disables VK/header MCP access."
         },
As per coding guidelines, `transports/config.schema.json` is the source of truth for config fields.
🤖 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/config.schema.json` around lines 276 - 280, The
mcp_server_auth_mode property in the schema documents 'headers' as the default
in its description, but the schema object itself is missing the "default" field
declaration. Add "default": "headers" to the mcp_server_auth_mode object
definition to ensure schema-driven tooling and generated configurations are
aligned with the documented runtime expectations.

Source: Coding guidelines

🤖 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 `@framework/configstore/clientconfig.go`:
- Around line 101-102: The GenerateClientConfigHash function does not include
the newly added MCPServerAuthMode and OAuth2ServerConfig fields in its hash
calculation, which means configuration changes to OAuth2 authentication settings
will not trigger reconciliation. Update the GenerateClientConfigHash function to
include both the MCPServerAuthMode field and the OAuth2ServerConfig field in the
hash computation to ensure config drift is properly detected.

In `@framework/configstore/migrations.go`:
- Around line 10905-10924: The migration with ID migrationName only defines the
Migrate function but lacks explicit rollback behavior. Add a Rollback function
to the migration object that removes the two columns (MCPServerAuthMode and
OAuth2ServerConfigJSON) from tables.TableClientConfig using the migrator's
DropColumn method, or if the migration is non-rollbackable, explicitly return an
error indicating that downgrade is not supported so operators have clear
expectations.

In `@framework/configstore/rdb.go`:
- Around line 6701-6707: The read path decrypts PrivateKeyPEM whenever
encrypt.IsEnabled() is true, but keys created before encryption was enabled are
stored as plaintext and will fail decryption, breaking OAuth operations. To fix
this, add an encryption status marker or version field to track whether each key
was encrypted when stored, then in the decrypt logic (around the
encrypt.IsEnabled() check in the code block starting at line 6701), only attempt
decryption if the encryption marker indicates the key was actually encrypted, or
detect plaintext keys and re-encrypt them on first read when encryption becomes
enabled. Apply the same logic to all similar decrypt paths mentioned in the
comment (also applies to lines 6731-6743).
- Around line 6688-6710: The current lazy signing-key creation has a race
condition where multiple concurrent callers can both miss the row, each
generating and saving different RSA keys, leading to inconsistency between
signed tokens and advertised keys. Fix this by making the creation atomic in the
createOAuth2SigningKey method: use an insert-on-conflict clause with DoNothing
set to true and Columns set to the key field, then check the RowsAffected count.
If RowsAffected is 0 (indicating a conflict where another process inserted
first), reload and decrypt the existing row before returning instead of
returning the newly generated key. Apply this fix to both the
createOAuth2SigningKey call path within GetOAuth2SigningKey and the other
similar signing-key creation location mentioned in the comment range.

In `@framework/configstore/tables/oauth2_server.go`:
- Around line 53-55: The comment for AccessTokenTTL in the OAuth2ServerConfig
incorrectly states the default value as 900 seconds, but the actual
DefaultAccessTokenTTL constant and schema both define it as 600 seconds. Update
the comment for AccessTokenTTL to correct the default value from 900 to 600
seconds to match the actual implementation and prevent confusion during
operational troubleshooting.

In `@transports/bifrost-http/handlers/config.go`:
- Around line 503-504: The assignments to updatedConfig.MCPServerAuthMode and
updatedConfig.OAuth2ServerConfig always overwrite existing values with zero
values when those fields are omitted from the payload during partial updates.
Instead of unconditionally assigning these fields, add conditional checks to
only update updatedConfig.MCPServerAuthMode and updatedConfig.OAuth2ServerConfig
when the corresponding payload.ClientConfig fields are actually provided (not
empty or zero values). This preserves existing runtime and database state when
these OAuth settings are not included in the update request.
- Around line 503-504: The MCPServerAuthMode and OAuth2ServerConfig fields in
the config handler are being assigned directly from the payload without
validating against the schema constraints defined in
transports/config.schema.json. Add validation logic before the assignments to
ensure MCPServerAuthMode conforms to the allowed enum values (headers, both,
oauth) and that OAuth2ServerConfig has the correct shape and applicability
according to the schema. If validation fails, return an appropriate error
response instead of persisting invalid configuration.

In `@transports/bifrost-http/handlers/plugins.go`:
- Around line 589-591: The isEnvVarObject function at line 589 is too permissive
in detecting EnvVar objects, treating any map containing the keys value,
env_var, and from_env as an EnvVar placeholder, even if additional fields are
present. This causes the function to incorrectly return the existing
configuration at line 591, discarding user updates. Tighten the isEnvVarObject
detection logic to only return true for objects that are exclusively EnvVar
objects with only the expected fields and no additional properties. Apply this
same fix to the similar check at lines 633-637 to prevent the same config
rollback issue from occurring in both locations.

In `@transports/bifrost-http/server/server.go`:
- Line 1397: The OAuth2 discovery routes registered by NewOAuth2DiscoveryHandler
are being blocked by APIMiddleware because the `/.well-known/` endpoint prefixes
are not whitelisted. Requests to these endpoints are rejected before reaching
the handler's discoveryEnabled() check. Add "/.well-known/" as a new entry to
the whitelistedPrefixes list in handlers/middlewares.go to allow these endpoints
through the APIMiddleware while letting the individual discovery handlers
continue to control access through their own discoveryEnabled() validation
logic.

In `@transports/config.schema.json`:
- Around line 299-306: The schema definitions for auth_code_ttl and
access_token_ttl fields currently only specify type as integer without enforcing
positive values, allowing zero or negative TTL values that would create invalid
credentials. Add a minimum constraint property to both auth_code_ttl and
access_token_ttl field definitions in the JSON schema to enforce that values
must be greater than zero, ensuring only valid positive integer values are
accepted.

---

Nitpick comments:
In `@transports/config.schema.json`:
- Around line 276-280: The mcp_server_auth_mode property in the schema documents
'headers' as the default in its description, but the schema object itself is
missing the "default" field declaration. Add "default": "headers" to the
mcp_server_auth_mode object definition to ensure schema-driven tooling and
generated configurations are aligned with the documented runtime expectations.
🪄 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: 0b0d9090-5ec7-407d-ace5-a4ae23b1050b

📥 Commits

Reviewing files that changed from the base of the PR and between 8806020 and 53a04f6.

📒 Files selected for processing (14)
  • framework/configstore/clientconfig.go
  • framework/configstore/migrations.go
  • framework/configstore/rdb.go
  • framework/configstore/store.go
  • framework/configstore/tables/clientconfig.go
  • framework/configstore/tables/oauth2_server.go
  • framework/temptoken/scope.go
  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/handlers/oauth2_discovery.go
  • transports/bifrost-http/handlers/oauth2_utils.go
  • transports/bifrost-http/handlers/plugins.go
  • transports/bifrost-http/handlers/plugins_test.go
  • transports/bifrost-http/server/server.go
  • transports/config.schema.json

Comment thread framework/configstore/clientconfig.go
Comment thread framework/configstore/migrations.go
Comment thread framework/configstore/rdb.go Outdated
Comment thread framework/configstore/rdb.go Outdated
Comment thread framework/configstore/tables/oauth2_server.go
Comment thread transports/bifrost-http/handlers/config.go Outdated
Comment thread transports/bifrost-http/handlers/plugins.go Outdated
Comment thread transports/bifrost-http/server/server.go
Comment thread transports/config.schema.json
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-16-feat_adds_mcp_server_oauth_tables branch 2 times, most recently from 226ccda to bd05880 Compare June 18, 2026 07:37

@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: 1

🤖 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 `@framework/configstore/rdb.go`:
- Around line 6709-6710: In the createOAuth2SigningKey function, before the
final reload attempt (the call to loadOAuth2SigningKey after handling insert
conflicts), add logic to repair any existing empty rows in the database. When an
insert conflict occurs, check if an empty row already exists (where Value is an
empty string or NULL), and if so, update that row with the newly generated key
data before reloading. This repair step should be applied in both locations
mentioned in the issue (around line 6709-6710 in the loadOAuth2SigningKey check
and also around line 6775), preventing the infinite retry loop that occurs when
loadOAuth2SigningKey returns ErrNotFound for empty rows.
🪄 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: 110527b2-c815-4a08-8d76-630f8379c0ad

📥 Commits

Reviewing files that changed from the base of the PR and between 53a04f6 and 226ccda.

📒 Files selected for processing (14)
  • framework/configstore/clientconfig.go
  • framework/configstore/migrations.go
  • framework/configstore/rdb.go
  • framework/configstore/store.go
  • framework/configstore/tables/clientconfig.go
  • framework/configstore/tables/oauth2_server.go
  • framework/temptoken/scope.go
  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/handlers/middlewares.go
  • transports/bifrost-http/handlers/oauth2_discovery.go
  • transports/bifrost-http/handlers/oauth2_utils.go
  • transports/bifrost-http/lib/config_test.go
  • transports/bifrost-http/server/server.go
  • transports/config.schema.json
🚧 Files skipped from review as they are similar to previous changes (7)
  • transports/bifrost-http/handlers/oauth2_utils.go
  • transports/bifrost-http/server/server.go
  • transports/config.schema.json
  • framework/configstore/store.go
  • framework/temptoken/scope.go
  • transports/bifrost-http/handlers/oauth2_discovery.go
  • framework/configstore/tables/clientconfig.go

Comment thread framework/configstore/rdb.go
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 18, 2026

@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: 1

🤖 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/oauth2_utils.go`:
- Around line 27-29: The oauth2MCPResourceURL function concatenates the issuer
URL with "/mcp" without normalizing trailing slashes, which results in "//mcp"
when the issuer URL is configured with a trailing slash. This causes resource
URL mismatches that break token validation. Fix this by trimming any trailing
slash from the result of oauth2IssuerURL(ctx, store) before appending "/mcp" to
ensure consistent, canonical resource URLs across discovery, authorization, and
verification paths.
🪄 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: f8dbb913-0854-4528-8915-13626b1d3311

📥 Commits

Reviewing files that changed from the base of the PR and between 226ccda and 67e3649.

📒 Files selected for processing (14)
  • framework/configstore/clientconfig.go
  • framework/configstore/migrations.go
  • framework/configstore/rdb.go
  • framework/configstore/store.go
  • framework/configstore/tables/clientconfig.go
  • framework/configstore/tables/oauth2_server.go
  • framework/temptoken/scope.go
  • transports/bifrost-http/handlers/config.go
  • transports/bifrost-http/handlers/middlewares.go
  • transports/bifrost-http/handlers/oauth2_discovery.go
  • transports/bifrost-http/handlers/oauth2_utils.go
  • transports/bifrost-http/lib/config_test.go
  • transports/bifrost-http/server/server.go
  • transports/config.schema.json
🚧 Files skipped from review as they are similar to previous changes (13)
  • framework/temptoken/scope.go
  • transports/bifrost-http/handlers/middlewares.go
  • framework/configstore/clientconfig.go
  • transports/bifrost-http/server/server.go
  • framework/configstore/migrations.go
  • transports/config.schema.json
  • framework/configstore/store.go
  • transports/bifrost-http/lib/config_test.go
  • transports/bifrost-http/handlers/config.go
  • framework/configstore/tables/clientconfig.go
  • framework/configstore/tables/oauth2_server.go
  • framework/configstore/rdb.go
  • transports/bifrost-http/handlers/oauth2_discovery.go

Comment thread transports/bifrost-http/handlers/oauth2_utils.go
@akshaydeo akshaydeo requested a review from a team as a code owner June 18, 2026 12:09
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-16-feat_adds_mcp_server_oauth_tables branch from 67e3649 to 46280b1 Compare June 18, 2026 12:22
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 18, 2026
@akshaydeo akshaydeo dismissed coderabbitai[bot]’s stale review June 19, 2026 07:23

The merge-base changed after approval.

@akshaydeo akshaydeo force-pushed the dev branch 2 times, most recently from fa15f50 to ca190fc Compare June 21, 2026 11:44
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-16-feat_adds_mcp_server_oauth_tables branch 2 times, most recently from 9f1fff1 to 0cbfee6 Compare June 22, 2026 16:58
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 22, 2026
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-16-feat_adds_mcp_server_oauth_tables branch from 0cbfee6 to d9c0026 Compare June 23, 2026 05:30
@coderabbitai coderabbitai Bot requested a review from roroghost17 June 23, 2026 05:31
@akshaydeo akshaydeo dismissed coderabbitai[bot]’s stale review June 24, 2026 08:37

The merge-base changed after approval.

@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 06-16-feat_adds_mcp_server_oauth_tables branch from d9c0026 to 04ec54f Compare June 25, 2026 05:10
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