diff --git a/core/mcp/credstore/per_user_headers.go b/core/mcp/credstore/per_user_headers.go index a40b3ba442..70f0dc7dca 100644 --- a/core/mcp/credstore/per_user_headers.go +++ b/core/mcp/credstore/per_user_headers.go @@ -108,9 +108,14 @@ func (r *perUserHeadersResolver) buildAuthRequiredError(ctx *schemas.BifrostCont } // missingRequiredHeaderKeys returns the names of any required header key -// that's absent or whose stored value is empty in storedHeaders. Comparison -// is case-insensitive at the wire level but the schema is the source of -// truth — we look up by the exact key the admin declared. +// that's absent or whose stored value is empty in storedHeaders. +// +// Both inputs are assumed to be in canonical form (lowercase + trimmed) — +// see the invariant doc on mcputils.CanonicalizeHeaderKey. All write +// boundaries (HTTP create/update, flow submit, config.json load) run +// the inputs through that helper, so exact map lookup here is correct. +// Do NOT add defensive case-folding inside this function: it would mask +// a missed write-side canonicalization rather than catching it. func missingRequiredHeaderKeys(required []string, storedHeaders map[string]string) []string { if len(storedHeaders) == 0 { return append([]string(nil), required...) @@ -128,6 +133,12 @@ func missingRequiredHeaderKeys(required []string, storedHeaders map[string]strin // user-submitted credential values for the required keys. Keys not declared // by the current schema are dropped on purpose so a stale row that still // stores a deprecated key cannot leak it onto the wire. +// +// Required keys and storedHeaders keys are both canonical (lowercase + +// trimmed) by the write-side invariant — see missingRequiredHeaderKeys +// above and mcputils.CanonicalizeHeaderKey. http.Header.Set runs its own +// MIME canonicalization on the way out (so "authorization" becomes +// "Authorization" on the wire), which is what upstream servers expect. func buildPerUserHeaderValues(required []string, storedHeaders map[string]string) http.Header { out := http.Header{} for _, key := range required { diff --git a/core/mcp/utils/utils.go b/core/mcp/utils/utils.go index 5605c556d8..0325a7a9f9 100644 --- a/core/mcp/utils/utils.go +++ b/core/mcp/utils/utils.go @@ -25,9 +25,16 @@ func FlattenHeaders(h http.Header) map[string]string { // BuildMCPCallbackBaseURL extracts the base URL set on the BifrostContext by // the HTTP middleware (e.g. "https://host"). Per-user OAuth and per-user // headers resolvers append their respective paths on top. +// +// Trailing slashes are stripped defensively. The sole writer today +// (lib/ctx.go BuildBaseURL) already normalizes, but OAuth providers match +// redirect URIs exactly — a `https://host//api/oauth/callback` produced by +// a future writer that forgets to trim would silently break every per-user +// OAuth flow. Guarding once on the read side keeps that invariant local +// to this function rather than spread across every potential writer. func BuildMCPCallbackBaseURL(ctx *schemas.BifrostContext) string { if base, ok := ctx.Value(schemas.BifrostContextKeyMCPCallbackBaseURL).(string); ok && base != "" { - return base + return strings.TrimRight(base, "/") } return "" } @@ -91,6 +98,69 @@ func matchesPerUserHeaderKey(name string, perUserKeys []string) bool { return false } +// Canonical-form invariant for per-user-headers data +// ===================================================== +// HTTP header names are case-insensitive on the wire (RFC 7230 §3.2), +// so anywhere the per-user-headers feature compares a schema key against +// a stored or submitted header name we'd need EqualFold lookups. Doing +// that defensively at every read site is fragile — a single missed call +// site re-introduces the bug (stored `authorization` looking missing +// against schema `Authorization`, etc.). +// +// Instead we enforce a write-time invariant: every external boundary +// that accepts a header key (or a credential header map) lowercases and +// trims via the helpers below before persisting. Downstream code can +// then assume canonical form and use plain map lookups. +// +// Write boundaries that MUST call these: +// - createMCPClient / updateMCPClient / resolvePerUserHeaderKeys +// (handlers/mcp.go) for MCPClientConfig.PerUserHeaderKeys +// - flowSubmit (handlers/mcp_per_user_headers.go) for the +// user-submitted credential.Headers map +// - loadMCPClientConfigFromFile (lib/config.go) for the config.json +// load path +// +// New write paths added in the future must canonicalize too — there is +// no defensive case-folding on the read side anymore. + +// CanonicalizeHeaderKey returns the canonical lowercase + trimmed form +// of a single header key. Empty input returns empty. +func CanonicalizeHeaderKey(key string) string { + return strings.ToLower(strings.TrimSpace(key)) +} + +// CanonicalizeHeaderKeys returns a new slice with every entry passed +// through CanonicalizeHeaderKey. Nil in → nil out so a caller that +// uses "nil means preserve existing" semantics (e.g. +// resolvePerUserHeaderKeys, UpdateMCPClientConfig) keeps that signal. +// The input slice is not mutated. +func CanonicalizeHeaderKeys(keys []string) []string { + if keys == nil { + return nil + } + out := make([]string, len(keys)) + for i, k := range keys { + out[i] = CanonicalizeHeaderKey(k) + } + return out +} + +// CanonicalizeHeaderMap returns a new map whose keys are passed through +// CanonicalizeHeaderKey. On collision (e.g. "Authorization" and +// "authorization" both present in the input), the last value wins — +// callers that need duplicate detection should run it on the raw input +// before calling this. Nil in → nil out. +func CanonicalizeHeaderMap(m map[string]string) map[string]string { + if m == nil { + return nil + } + out := make(map[string]string, len(m)) + for k, v := range m { + out[CanonicalizeHeaderKey(k)] = v + } + return out +} + // ExtractFilteredExtras returns just the per-request "extra" headers carried // in the BifrostContext (BifrostContextKeyMCPExtraHeaders), scoped by the // client's AllowedExtraHeaders. Static config headers are NOT included here — diff --git a/framework/configstore/rdb.go b/framework/configstore/rdb.go index c73a966572..178f5e51a6 100644 --- a/framework/configstore/rdb.go +++ b/framework/configstore/rdb.go @@ -1864,6 +1864,7 @@ func (s *RDBConfigStore) UpdateMCPClientConfig(ctx context.Context, id string, c updates["stdio_config_json"] = stdioConfigJSON updates["auth_type"] = clientConfigCopy.AuthType updates["oauth_config_id"] = clientConfigCopy.OauthConfigID + updates["per_user_header_keys_json"] = perUserHeaderKeysJSON } // Only update is_ping_available if explicitly provided (non-nil) @@ -2762,11 +2763,7 @@ func (s *RDBConfigStore) DeleteVirtualKey(ctx context.Context, id string, tx ... if err := txDB.WithContext(ctx).Where("virtual_key_id = ?", id).Delete(&tables.TableMCPPerUserHeaderCredential{}).Error; err != nil { return err } - // Delete per-user MCP header flows tied to this VK — mirrors the - // OAuth session purge above. A pending flow's temp-token is valid - // for ~15 min; without this delete, a submission in flight could - // upsert a credential row pointing at the just-deleted VK and - // re-grant access after explicit revocation. + // Delete per-user MCP header submission flows tied to this VK if err := txDB.WithContext(ctx).Where("virtual_key_id = ?", id).Delete(&tables.TableMCPPerUserHeaderFlow{}).Error; err != nil { return err } @@ -5392,7 +5389,11 @@ func (s *RDBConfigStore) GetMCPPerUserHeaderFlowByID(ctx context.Context, id str var flow tables.TableMCPPerUserHeaderFlow if err := s.ScopedDB(ctx). Preload("MCPClient", func(db *gorm.DB) *gorm.DB { - return db.Select("client_id, name, headers_json, allowed_extra_headers_json, per_user_header_keys_json") + // encryption_status is required for TableMCPClient.AfterFind to + // decrypt headers_json in encrypted deployments — omitting it + // makes the preload return ciphertext and breaks flowSubmit's + // VerifyHeadersConnection path which reads config.Headers. + return db.Select("client_id, name, headers_json, allowed_extra_headers_json, per_user_header_keys_json, encryption_status") }). Preload("VirtualKey", func(db *gorm.DB) *gorm.DB { return db.Select("id, name") }). Where("id = ?", id).First(&flow).Error; err != nil { @@ -5485,9 +5486,9 @@ func (s *RDBConfigStore) DeleteMCPPerUserHeaderFlowsByModeIdentityAndMCPClient(c } // ListAllPendingMCPPerUserHeaderFlows returns all pending header-submission -// flow rows whose expiry is in the future. Mirrors -// ListAllPendingOauthUserSessions. Visibility scoping is handled by the -// enterprise configstore layer via DAC; OSS sees everything. +// flow rows whose expiry is in the future. Uses ScopedDB so a query-scope +// stashed on ctx (if any) narrows the result; otherwise returns every row. +// Mirrors ListAllPendingOauthUserSessions. func (s *RDBConfigStore) ListAllPendingMCPPerUserHeaderFlows(ctx context.Context) ([]tables.TableMCPPerUserHeaderFlow, error) { var flows []tables.TableMCPPerUserHeaderFlow if err := s.ScopedDB(ctx). @@ -5513,3 +5514,257 @@ func (s *RDBConfigStore) DeleteExpiredMCPPerUserHeaderFlows(ctx context.Context) } return result.RowsAffected, nil } + +// ----- Per-user credential reconciliation ----- +// +// The Reconcile* methods orphan/reactivate vk-keyed credentials whose MCP +// grant changed (allowlist edit, AllowOnAllVirtualKeys toggle, VK delete). +// Pending flow rows whose MCP lost the grant are hard-deleted — they're +// transient in-flight attempts that can't complete without the grant. +// +// "Effective allowlist" for a VK = explicit rows in +// governance_virtual_key_mcp_configs ∪ MCPs with +// config_mcp_clients.allow_on_all_virtual_keys = true. Mirrors the runtime +// check in plugins/governance/main.go isMCPToolAllowedByVKWith. +// +// Runtime lookups filter status='active', so orphaned rows are invisible +// until reactivation. 'needs_reauth' (OAuth) and 'needs_update' (headers) +// rows are left alone — their problem isn't grant state. +// +// Wrappers that maintain user-keyed credentials (rows keyed by user_id +// rather than virtual_key_id) layer on top of these methods. + +// vkEffectiveMCPClientIDs returns the set of MCP client_ids the given VK +// can access — union of explicit per-VK allowlist and MCPs marked +// AllowOnAllVirtualKeys=true. +func vkEffectiveMCPClientIDs(tx *gorm.DB, vkID string) ([]string, error) { + var explicit []string + if err := tx.Table("governance_virtual_key_mcp_configs vkmc"). + Distinct("mcp.client_id"). + Joins("JOIN config_mcp_clients mcp ON mcp.id = vkmc.mcp_client_id"). + Where("vkmc.virtual_key_id = ?", vkID). + Pluck("mcp.client_id", &explicit).Error; err != nil { + return nil, fmt.Errorf("read VK %s explicit allowlist: %w", vkID, err) + } + var implicit []string + if err := tx.Table("config_mcp_clients"). + Where("allow_on_all_virtual_keys = ?", true). + Pluck("client_id", &implicit).Error; err != nil { + return nil, fmt.Errorf("read AllowOnAllVirtualKeys MCPs: %w", err) + } + if len(implicit) == 0 { + return explicit, nil + } + seen := make(map[string]struct{}, len(explicit)+len(implicit)) + out := make([]string, 0, len(explicit)+len(implicit)) + for _, id := range explicit { + if _, ok := seen[id]; ok { + continue + } + seen[id] = struct{}{} + out = append(out, id) + } + for _, id := range implicit { + if _, ok := seen[id]; ok { + continue + } + seen[id] = struct{}{} + out = append(out, id) + } + return out, nil +} + +// reconcileVKDirectTokensDB orphans/reactivates vk-keyed OAuth token rows +// against the VK's effective allowlist. Pure DB; runs inside the caller's +// transaction. +func reconcileVKDirectTokensDB(tx *gorm.DB, vkID string) error { + allowedClientIDs, err := vkEffectiveMCPClientIDs(tx, vkID) + if err != nil { + return err + } + + // Defense-in-depth: every other query in this file that touches these + // tables filters on auth_mode/flow_mode (see GetByModeIdentity*, + // UpsertCredential, ListPending* etc). Reconciliation matched on + // virtual_key_id alone; align with the convention so a stray non-VK + // row with virtual_key_id set can never be touched here. + orphanQ := tx.Model(&tables.TableOauthUserToken{}). + Where("auth_mode = ? AND virtual_key_id = ? AND status = ?", string(schemas.MCPAuthModeVK), vkID, "active") + if len(allowedClientIDs) > 0 { + orphanQ = orphanQ.Where("mcp_client_id NOT IN ?", allowedClientIDs) + } + if err := orphanQ.Update("status", "orphaned").Error; err != nil { + return fmt.Errorf("orphan vk-keyed tokens for vk %s: %w", vkID, err) + } + + if len(allowedClientIDs) > 0 { + if err := tx.Model(&tables.TableOauthUserToken{}). + Where("auth_mode = ? AND virtual_key_id = ? AND status = ? AND mcp_client_id IN ?", string(schemas.MCPAuthModeVK), vkID, "orphaned", allowedClientIDs). + Update("status", "active").Error; err != nil { + return fmt.Errorf("reactivate vk-keyed tokens for vk %s: %w", vkID, err) + } + } + + // Pending-only: an 'authorized' session means the upstream OAuth callback + // already arrived and the code-for-token exchange is mid-flight; deleting + // it would surface a confusing "session not found" error to the user. + // Leave non-pending statuses ('authorized', 'failed', 'expired') alone — + // the next sweep pass cleans up finished/expired rows, and any token the + // in-flight exchange produces becomes immediately-orphaned (tidier than + // a hard error). + flowsQ := tx.Where("flow_mode = ? AND virtual_key_id = ? AND status = ?", string(schemas.MCPAuthModeVK), vkID, "pending") + if len(allowedClientIDs) > 0 { + flowsQ = flowsQ.Where("mcp_client_id NOT IN ?", allowedClientIDs) + } + if err := flowsQ.Delete(&tables.TableOauthUserSession{}).Error; err != nil { + return fmt.Errorf("delete vk-keyed flow rows for vk %s: %w", vkID, err) + } + return nil +} + +// reconcileVKDirectHeaderRowsDB is the headers counterpart of +// reconcileVKDirectTokensDB. +func reconcileVKDirectHeaderRowsDB(tx *gorm.DB, vkID string) error { + allowedClientIDs, err := vkEffectiveMCPClientIDs(tx, vkID) + if err != nil { + return err + } + + // Defense-in-depth: see reconcileVKDirectTokensDB above — filter on + // auth_mode/flow_mode so non-VK rows with virtual_key_id set can + // never be touched here. + orphanQ := tx.Model(&tables.TableMCPPerUserHeaderCredential{}). + Where("auth_mode = ? AND virtual_key_id = ? AND status = ?", string(schemas.MCPAuthModeVK), vkID, "active") + if len(allowedClientIDs) > 0 { + orphanQ = orphanQ.Where("mcp_client_id NOT IN ?", allowedClientIDs) + } + if err := orphanQ.Update("status", "orphaned").Error; err != nil { + return fmt.Errorf("orphan vk-keyed header credentials for vk %s: %w", vkID, err) + } + + if len(allowedClientIDs) > 0 { + if err := tx.Model(&tables.TableMCPPerUserHeaderCredential{}). + Where("auth_mode = ? AND virtual_key_id = ? AND status = ? AND mcp_client_id IN ?", string(schemas.MCPAuthModeVK), vkID, "orphaned", allowedClientIDs). + Update("status", "active").Error; err != nil { + return fmt.Errorf("reactivate vk-keyed header credentials for vk %s: %w", vkID, err) + } + } + + // Pending-only — same rationale as the OAuth flow delete above. Headers + // flows complete by row-deletion, so 'completed' rows shouldn't normally + // exist, but expired rows do and shouldn't be touched here either. + flowsQ := tx.Where("flow_mode = ? AND virtual_key_id = ? AND status = ?", string(schemas.MCPAuthModeVK), vkID, "pending") + if len(allowedClientIDs) > 0 { + flowsQ = flowsQ.Where("mcp_client_id NOT IN ?", allowedClientIDs) + } + if err := flowsQ.Delete(&tables.TableMCPPerUserHeaderFlow{}).Error; err != nil { + return fmt.Errorf("delete vk-keyed header flow rows for vk %s: %w", vkID, err) + } + return nil +} + +// readVKsHoldingOauthCredsForMCP returns the distinct virtual_key_ids +// with an active or pending OAuth row (token or session) for the given +// MCP client. Used by ReconcileOauthAfterMCPChange to know which VKs +// need re-evaluation. +func readVKsHoldingOauthCredsForMCP(tx *gorm.DB, mcpClientID string) ([]string, error) { + var vkIDs []string + if err := tx.Raw(` + SELECT DISTINCT virtual_key_id FROM oauth_user_tokens + WHERE mcp_client_id = ? AND virtual_key_id IS NOT NULL AND virtual_key_id <> '' + UNION + SELECT DISTINCT virtual_key_id FROM oauth_user_sessions + WHERE mcp_client_id = ? AND virtual_key_id IS NOT NULL AND virtual_key_id <> '' + `, mcpClientID, mcpClientID).Scan(&vkIDs).Error; err != nil { + return nil, fmt.Errorf("read VK owners of OAuth creds for mcp %s: %w", mcpClientID, err) + } + return vkIDs, nil +} + +// readVKsHoldingHeaderCredsForMCP is the headers counterpart of +// readVKsHoldingOauthCredsForMCP. +func readVKsHoldingHeaderCredsForMCP(tx *gorm.DB, mcpClientID string) ([]string, error) { + var vkIDs []string + if err := tx.Raw(` + SELECT DISTINCT virtual_key_id FROM mcp_per_user_header_credentials + WHERE mcp_client_id = ? AND virtual_key_id IS NOT NULL AND virtual_key_id <> '' + UNION + SELECT DISTINCT virtual_key_id FROM mcp_per_user_header_flows + WHERE mcp_client_id = ? AND virtual_key_id IS NOT NULL AND virtual_key_id <> '' + `, mcpClientID, mcpClientID).Scan(&vkIDs).Error; err != nil { + return nil, fmt.Errorf("read VK owners of header creds for mcp %s: %w", mcpClientID, err) + } + return vkIDs, nil +} + +// ReconcileOauthAfterVKChange orphans/reactivates vk-keyed OAuth rows +// against the VK's current effective allowlist. Called whenever a VK's +// MCP grants might have changed (AP propagation, direct dashboard edit, +// SCIM auto-assign). +func (s *RDBConfigStore) ReconcileOauthAfterVKChange(ctx context.Context, vkID string) error { + if vkID == "" { + return nil + } + return s.DB().WithContext(ctx).Transaction(func(tx *gorm.DB) error { + return reconcileVKDirectTokensDB(tx, vkID) + }) +} + +// ReconcileMCPHeadersAfterVKChange is the headers counterpart of +// ReconcileOauthAfterVKChange. +func (s *RDBConfigStore) ReconcileMCPHeadersAfterVKChange(ctx context.Context, vkID string) error { + if vkID == "" { + return nil + } + return s.DB().WithContext(ctx).Transaction(func(tx *gorm.DB) error { + return reconcileVKDirectHeaderRowsDB(tx, vkID) + }) +} + +// ReconcileOauthAfterMCPChange re-evaluates every VK that holds an OAuth +// credential for the given MCP. Called when an MCP edit mutates who can +// access it (vk_configs diff or AllowOnAllVirtualKeys toggle). +func (s *RDBConfigStore) ReconcileOauthAfterMCPChange(ctx context.Context, mcpClientID string) error { + if mcpClientID == "" { + return nil + } + return s.DB().WithContext(ctx).Transaction(func(tx *gorm.DB) error { + vkIDs, err := readVKsHoldingOauthCredsForMCP(tx, mcpClientID) + if err != nil { + return err + } + // Sort so concurrent MCP edits lock the same VKs in the same order; + // the UNION returned by readVKsHoldingOauthCredsForMCP is unordered, + // which can deadlock two overlapping reconciliations otherwise. + sort.Strings(vkIDs) + for _, vkID := range vkIDs { + if err := reconcileVKDirectTokensDB(tx, vkID); err != nil { + return err + } + } + return nil + }) +} + +// ReconcileMCPHeadersAfterMCPChange is the headers counterpart of +// ReconcileOauthAfterMCPChange. +func (s *RDBConfigStore) ReconcileMCPHeadersAfterMCPChange(ctx context.Context, mcpClientID string) error { + if mcpClientID == "" { + return nil + } + return s.DB().WithContext(ctx).Transaction(func(tx *gorm.DB) error { + vkIDs, err := readVKsHoldingHeaderCredsForMCP(tx, mcpClientID) + if err != nil { + return err + } + // See ReconcileOauthAfterMCPChange — deterministic lock order + // across concurrent MCP edits. + sort.Strings(vkIDs) + for _, vkID := range vkIDs { + if err := reconcileVKDirectHeaderRowsDB(tx, vkID); err != nil { + return err + } + } + return nil + }) +} diff --git a/framework/configstore/store.go b/framework/configstore/store.go index 57f14581da..cf42373d3a 100644 --- a/framework/configstore/store.go +++ b/framework/configstore/store.go @@ -431,16 +431,39 @@ type ConfigStore interface { DeleteMCPPerUserHeaderFlowsByModeIdentityAndMCPClient(ctx context.Context, mode schemas.MCPAuthMode, identity, mcpClientID string) error DeleteMCPPerUserHeaderFlow(ctx context.Context, id string) error // ListAllPendingMCPPerUserHeaderFlows returns every non-expired flow row - // with status='pending', regardless of caller identity. Visibility scoping - // happens at the enterprise configstore layer via DAC scope; OSS sees - // everything. Used by the sessions list endpoint to surface pending - // submission flows alongside completed credentials. Mirrors - // ListAllPendingOauthUserSessions on the OAuth side. + // with status='pending'. Used by the sessions list endpoint to surface + // pending submission flows alongside completed credentials. Mirrors + // ListAllPendingOauthUserSessions on the OAuth side. The implementation + // reads via ScopedDB(ctx), so a query-scope stashed on ctx (e.g. by + // enterprise DAC) narrows the result; with no scope, every pending + // row is returned. ListAllPendingMCPPerUserHeaderFlows(ctx context.Context) ([]tables.TableMCPPerUserHeaderFlow, error) // DeleteExpiredMCPPerUserHeaderFlows hard-deletes pending flow rows whose // ExpiresAt has passed. Returns the number of rows removed. DeleteExpiredMCPPerUserHeaderFlows(ctx context.Context) (int64, error) + // Per-user credential reconciliation. + // + // Called whenever a VK ↔ MCP grant might have changed (direct + // dashboard edit, AP propagation, SCIM auto-assign). Orphans + // vk-keyed credentials whose MCP is no longer in the VK's effective + // allowlist (explicit per-VK row ∪ MCPs with + // AllowOnAllVirtualKeys=true) and reactivates orphaned rows when the + // grant returns. Pending flow rows for lost grants are hard-deleted. + // + // Session-keyed rows are never touched — they carry no notion of + // "lost access". + // + // Handlers should invoke both surfaces (OAuth + headers) after every + // grant-change so both stay consistent. + ReconcileOauthAfterVKChange(ctx context.Context, vkID string) error + ReconcileMCPHeadersAfterVKChange(ctx context.Context, vkID string) error + // MCP-side variants: called when the change originates on the MCP + // client (vk_configs edit OR AllowOnAllVirtualKeys toggle). Each + // re-evaluates every VK that holds a credential for the changed MCP. + ReconcileOauthAfterMCPChange(ctx context.Context, mcpClientID string) error + ReconcileMCPHeadersAfterMCPChange(ctx context.Context, mcpClientID string) error + // Not found retry wrapper RetryOnNotFound(ctx context.Context, fn func(ctx context.Context) (any, error), maxRetries int, retryDelay time.Duration) (any, error) diff --git a/transports/bifrost-http/handlers/governance.go b/transports/bifrost-http/handlers/governance.go index e1c536110c..019ede327f 100644 --- a/transports/bifrost-http/handlers/governance.go +++ b/transports/bifrost-http/handlers/governance.go @@ -1478,6 +1478,21 @@ func (h *GovernanceHandler) updateVirtualKey(ctx *fasthttp.RequestCtx) { SendError(ctx, 500, "Virtual key updated in database but failed to reload in-memory state") return } + + // Per-user credential reconciliation when the VK's MCP allowlist + // changed. Mirrors the AP-propagation path: enterprise orphans / + // reactivates credentials keyed to this VK (vk-keyed creds) and to the + // VK's owner (user-keyed creds) against the new effective allowlist + // (explicit rows ∪ MCPs with AllowOnAllVirtualKeys=true). OSS no-ops. + if req.MCPConfigs != nil && h.configStore != nil { + if err := h.configStore.ReconcileOauthAfterVKChange(ctx, vk.ID); err != nil { + logger.Error("reconcile OAuth credentials after VK %s update failed: %v", vk.ID, err) + } + if err := h.configStore.ReconcileMCPHeadersAfterVKChange(ctx, vk.ID); err != nil { + logger.Error("reconcile per-user-headers credentials after VK %s update failed: %v", vk.ID, err) + } + } + SendJSON(ctx, map[string]interface{}{ "message": "Virtual key updated successfully", "virtual_key": preloadedVk, diff --git a/transports/bifrost-http/handlers/mcp.go b/transports/bifrost-http/handlers/mcp.go index aa095e9c37..f32dab2f63 100644 --- a/transports/bifrost-http/handlers/mcp.go +++ b/transports/bifrost-http/handlers/mcp.go @@ -16,6 +16,7 @@ import ( "github.com/google/uuid" bifrost "github.com/maximhq/bifrost/core" "github.com/maximhq/bifrost/core/mcp" + mcputils "github.com/maximhq/bifrost/core/mcp/utils" "github.com/maximhq/bifrost/core/schemas" "github.com/maximhq/bifrost/framework/configstore" configstoreTables "github.com/maximhq/bifrost/framework/configstore/tables" @@ -406,22 +407,34 @@ func (h *MCPHandler) addMCPClient(ctx *fasthttp.RequestCtx) { SendError(ctx, fasthttp.StatusBadRequest, "per_user_header_keys must be a non-empty list when auth_type is 'per_user_headers'") return } - normalisedHeaders := make([]string, 0, len(req.PerUserHeaderKeys)) - for i, key := range req.PerUserHeaderKeys { - if strings.TrimSpace(key) == "" { + // Canonicalize (lowercase + trim) at the request boundary so the + // stored schema, credential rows, and runtime comparisons all + // agree on one form. See the invariant doc on + // mcputils.CanonicalizeHeaderKey — defensive case-folding on the + // read side was removed in favor of write-side normalization, so + // every key that enters this handler MUST go through here before + // it reaches the schemas/store layer. + canonHeaderKeys := mcputils.CanonicalizeHeaderKeys(req.PerUserHeaderKeys) + for i, key := range canonHeaderKeys { + if key == "" { SendError(ctx, fasthttp.StatusBadRequest, fmt.Sprintf("per_user_header_keys[%d] is empty", i)) return } - normalisedHeaders = append(normalisedHeaders, strings.ToLower(strings.TrimSpace(key))) } // HTTP header names are case-insensitive on the wire — reject duplicates // like ["X-Api-Key", "x-api-key"] so downstream change-detection and - // credential storage stay correct. - if lib.HasDuplicates(normalisedHeaders) { + // credential storage stay correct. Run the dup check on the canon + // form so case-only collisions are caught. + if lib.HasDuplicates(canonHeaderKeys) { SendError(ctx, fasthttp.StatusBadRequest, "per_user_header_keys contains duplicate entries") return } - if missing := missingPerUserHeaderValues(req.PerUserHeaderKeys, req.UserHeaders); len(missing) > 0 { + // Canonicalize the admin's sample header values too so the + // "missing values for required keys" check matches by canonical + // form. Without this, a UI that sends "Authorization" as a key + // and "authorization" as a value-map entry would spuriously fail. + canonUserHeaders := mcputils.CanonicalizeHeaderMap(req.UserHeaders) + if missing := missingPerUserHeaderValues(canonHeaderKeys, canonUserHeaders); len(missing) > 0 { SendError(ctx, fasthttp.StatusBadRequest, fmt.Sprintf("sample user_headers missing values for required keys: %s", strings.Join(missing, ", "))) return } @@ -451,7 +464,7 @@ func (h *MCPHandler) addMCPClient(ctx *fasthttp.RequestCtx) { ConnectionString: req.ConnectionString, StdioConfig: req.StdioConfig, AuthType: schemas.MCPAuthTypePerUserHeaders, - PerUserHeaderKeys: req.PerUserHeaderKeys, + PerUserHeaderKeys: canonHeaderKeys, ToolsToExecute: req.ToolsToExecute, ToolsToAutoExecute: req.ToolsToAutoExecute, ToolPricing: req.ToolPricing, @@ -463,8 +476,9 @@ func (h *MCPHandler) addMCPClient(ctx *fasthttp.RequestCtx) { // Verify connection and discover tools using the admin's sample // header values. Discovered tools land on schemasConfig before we // persist so the DB row includes them from the start — same - // convention as the per-user OAuth branch below. - tools, toolNameMapping, verifyErr := h.mcpManager.VerifyHeadersConnection(ctx, schemasConfig, req.UserHeaders) + // convention as the per-user OAuth branch below. Pass the canon + // form so the verify path sees the same keys the schema declares. + tools, toolNameMapping, verifyErr := h.mcpManager.VerifyHeadersConnection(ctx, schemasConfig, canonUserHeaders) if verifyErr != nil { SendError(ctx, fasthttp.StatusUnprocessableEntity, fmt.Sprintf("Verification failed: %v", verifyErr)) return @@ -483,7 +497,6 @@ func (h *MCPHandler) addMCPClient(ctx *fasthttp.RequestCtx) { SendError(ctx, fasthttp.StatusInternalServerError, fmt.Sprintf("Failed to register MCP client: %v", err)) return } - h.mcpManager.SetClientTools(schemasConfig.ID, tools, toolNameMapping) SendJSON(ctx, map[string]any{ "status": "success", @@ -771,6 +784,18 @@ func (h *MCPHandler) updateMCPClient(ctx *fasthttp.RequestCtx) { SendError(ctx, fasthttp.StatusNotFound, "MCP client not found") return } + // Snapshot fields we need to diff against the request AFTER UpdateMCPClient + // runs further below — UpdateMCPClient mutates the *MCPClientConfig in + // place (it's the same pointer the manager holds in MCPConfig.ClientConfigs), + // so post-update reads would already reflect the new value and the diff + // would always be false. + // + // PerUserHeaderKeys is snapshotted via append (independent backing array) + // rather than a bare slice-header copy, so we're safe if a future change + // mutates the slice contents in-place instead of reassigning the header. + existingAllowOnAllVirtualKeys := existingConfig.AllowOnAllVirtualKeys + existingPerUserHeaderKeys := append([]string(nil), existingConfig.PerUserHeaderKeys...) + // connection_type and auth_type and connection string are permanently immutable if req.ConnectionType != "" && req.ConnectionType != string(existingConfig.ConnectionType) { SendError(ctx, fasthttp.StatusBadRequest, "connection_type cannot be changed for an existing MCP client") @@ -822,7 +847,11 @@ func (h *MCPHandler) updateMCPClient(ctx *fasthttp.RequestCtx) { } // Validate per_user_header_keys only when the request explicitly provides // the field — otherwise resolvePerUserHeaderKeys carries the existing list - // forward unchanged (already validated at create time). + // forward unchanged (already validated at create time). Canonicalization + // happens here AND inside resolvePerUserHeaderKeys; doing it twice is + // cheap and keeps the validation error messages aligned with the canon + // form that ultimately gets persisted (see invariant doc on + // mcputils.CanonicalizeHeaderKey). if req.PerUserHeaderKeys != nil { // Reject an explicit empty list for per_user_headers clients. // AuthType is immutable on update (enforced at clientmanager.go:911), @@ -830,22 +859,16 @@ func (h *MCPHandler) updateMCPClient(ctx *fasthttp.RequestCtx) { // auth types may legitimately carry no per_user_header_keys, but for // per_user_headers an empty schema means the auth mode has nothing // to collect or validate, which violates the feature contract. - // Without this guard, resolvePerUserHeaderKeys returns [] and the - // resolver errors on every subsequent tool call with "no PerUser- - // HeaderKeys declared" — and MarkMCPPerUserHeaderCredentialsNeedsUpdate - // fires first, flipping all active credentials to needs_update for - // nothing. if existingConfig.AuthType == schemas.MCPAuthTypePerUserHeaders && len(req.PerUserHeaderKeys) == 0 { SendError(ctx, fasthttp.StatusBadRequest, "per_user_header_keys must be a non-empty list for per_user_headers clients") return } - canonHeaderKeys := make([]string, 0, len(req.PerUserHeaderKeys)) - for i, key := range req.PerUserHeaderKeys { - if strings.TrimSpace(key) == "" { + canonHeaderKeys := mcputils.CanonicalizeHeaderKeys(req.PerUserHeaderKeys) + for i, key := range canonHeaderKeys { + if key == "" { SendError(ctx, fasthttp.StatusBadRequest, fmt.Sprintf("per_user_header_keys[%d] is empty", i)) return } - canonHeaderKeys = append(canonHeaderKeys, strings.ToLower(strings.TrimSpace(key))) } if lib.HasDuplicates(canonHeaderKeys) { SendError(ctx, fasthttp.StatusBadRequest, "per_user_header_keys contains duplicate entries") @@ -1013,18 +1036,6 @@ func (h *MCPHandler) updateMCPClient(ctx *fasthttp.RequestCtx) { PerUserHeaderKeys: resolvePerUserHeaderKeys(existingConfig, req), } - // If the per-user-headers schema changed, flip every existing active row - // to 'needs_update' so callers are forced to resubmit on next tool use. - // The rows are preserved (status only flips) so the submission UI can - // prefill known values. - if existingConfig.AuthType == schemas.MCPAuthTypePerUserHeaders && - perUserHeaderKeysChanged(existingConfig.PerUserHeaderKeys, schemasConfig.PerUserHeaderKeys) && - h.store.ConfigStore != nil { - if err := h.store.ConfigStore.MarkMCPPerUserHeaderCredentialsNeedsUpdate(ctx, existingConfig.ID); err != nil { - logger.Error(fmt.Sprintf("failed to flip per-user header credentials to needs_update for client %s: %v", existingConfig.ID, err)) - } - } - // Update MCP client config in memory (always — applies name/tools/header changes, if err := h.mcpManager.UpdateMCPClient(ctx, id, schemasConfig); err != nil { // Rollback DB update to keep DB and memory in sync @@ -1038,6 +1049,26 @@ func (h *MCPHandler) updateMCPClient(ctx *fasthttp.RequestCtx) { return } + // If the per-user-headers schema now requires additional keys, flip every + // existing active row to 'needs_update' so callers are forced to submit the + // new values on next tool use. Removed-only schema changes do not need a + // resubmission: runtime resolution and flow-submit both filter stored + // credentials to the current schema before using/persisting them. + // + // Runs AFTER the in-memory UpdateMCPClient succeeds — if we flipped + // credentials first and the runtime update then failed, the rollback + // above would revert the DB row but leave every credential stuck in + // needs_update, even though the old schema is still the active one. + // Users would see a spurious "resubmit" prompt with no actual schema + // change to reconcile. + if existingConfig.AuthType == schemas.MCPAuthTypePerUserHeaders && + perUserHeaderKeysAdded(existingPerUserHeaderKeys, schemasConfig.PerUserHeaderKeys) && + h.store.ConfigStore != nil { + if err := h.store.ConfigStore.MarkMCPPerUserHeaderCredentialsNeedsUpdate(ctx, existingConfig.ID); err != nil { + logger.Error(fmt.Sprintf("failed to flip per-user header credentials to needs_update for client %s: %v", existingConfig.ID, err)) + } + } + // Reload every VK currently referencing this MCP client so the governance // cache's preloaded MCPClient relation picks up the rename / tool / header // changes. The VK-assignment-change block below does its own targeted @@ -1189,6 +1220,28 @@ func (h *MCPHandler) updateMCPClient(ctx *fasthttp.RequestCtx) { // return // } + // Per-user credential reconciliation for changes that mutate who can + // access this MCP. Two trigger conditions: + // 1. vk_configs explicitly diffed (rows added/removed/updated). + // 2. AllowOnAllVirtualKeys flipped — the implicit fallback toggled, + // every VK with a credential for this MCP needs re-evaluation. + // + // Reconcile is enterprise-only behavior (no-op in OSS). It orphans + // credentials whose MCP just lost the grant and reactivates orphaned + // ones whose MCP regained the grant. Both surfaces (OAuth + headers) + // are reconciled — they share the same VK→MCP allowlist model. + if h.store.ConfigStore != nil { + shouldReconcile := req.VKConfigs != nil || req.AllowOnAllVirtualKeys != existingAllowOnAllVirtualKeys + if shouldReconcile { + if err := h.store.ConfigStore.ReconcileOauthAfterMCPChange(ctx, id); err != nil { + logger.Error(fmt.Sprintf("reconcile OAuth credentials after MCP %s update failed: %v", id, err)) + } + if err := h.store.ConfigStore.ReconcileMCPHeadersAfterMCPChange(ctx, id); err != nil { + logger.Error(fmt.Sprintf("reconcile per-user-headers credentials after MCP %s update failed: %v", id, err)) + } + } + } + SendJSON(ctx, map[string]any{ "status": "success", "message": "MCP client edited successfully", @@ -1612,12 +1665,20 @@ func (h *MCPHandler) completeMCPClientOAuth(ctx *fasthttp.RequestCtx) { } // resolvePerUserHeaderKeys returns the per-user-header-key list to persist on -// the updated MCP client. If the request explicitly sets the field (even to -// an empty list when the caller is removing all keys), the request wins; -// otherwise the existing schema is preserved. +// the updated MCP client. If the request explicitly sets the field, the +// request wins; otherwise the existing schema is preserved. The handler +// rejects an explicit empty list for per_user_headers clients upstream +// (see updateMCPClient validation), so this function cannot be invoked +// with an empty slice for that auth type. +// +// Request-supplied keys are canonicalized (lowercase + trim) here so the +// persisted slice matches the canon form already in stored credential rows +// — see mcputils.CanonicalizeHeaderKey for the invariant. Existing values +// are already canon (they came through this path on create/update), so +// they pass through untouched. func resolvePerUserHeaderKeys(existing *schemas.MCPClientConfig, req MCPClientUpdateRequest) []string { if req.PerUserHeaderKeys != nil { - return req.PerUserHeaderKeys + return mcputils.CanonicalizeHeaderKeys(req.PerUserHeaderKeys) } if existing != nil { return existing.PerUserHeaderKeys @@ -1625,14 +1686,13 @@ func resolvePerUserHeaderKeys(existing *schemas.MCPClientConfig, req MCPClientUp return nil } -// perUserHeaderKeysChanged reports whether the new key set differs from the -// old set (order-insensitive). Used by updateMCPClient to decide whether to -// flip existing user credentials to 'needs_update'. -func perUserHeaderKeysChanged(oldKeys, newKeys []string) bool { - if len(oldKeys) != len(newKeys) { - return true - } - if len(oldKeys) == 0 { +// perUserHeaderKeysAdded reports whether the new schema introduces any key +// absent from the old schema (order-insensitive). Used by updateMCPClient to +// decide whether existing user credentials must be marked 'needs_update'. +// Removed-only changes do not require resubmission because stale stored keys +// are filtered out before use. +func perUserHeaderKeysAdded(oldKeys, newKeys []string) bool { + if len(newKeys) == 0 { return false } seen := make(map[string]struct{}, len(oldKeys)) diff --git a/transports/bifrost-http/handlers/mcp_per_user_headers.go b/transports/bifrost-http/handlers/mcp_per_user_headers.go index 82dbe64028..0bcaebb061 100644 --- a/transports/bifrost-http/handlers/mcp_per_user_headers.go +++ b/transports/bifrost-http/handlers/mcp_per_user_headers.go @@ -10,7 +10,9 @@ import ( "github.com/fasthttp/router" "github.com/google/uuid" + mcputils "github.com/maximhq/bifrost/core/mcp/utils" "github.com/maximhq/bifrost/core/schemas" + configstoreTables "github.com/maximhq/bifrost/framework/configstore/tables" "github.com/maximhq/bifrost/framework/temptoken" "github.com/maximhq/bifrost/transports/bifrost-http/lib" "github.com/valyala/fasthttp" @@ -142,21 +144,7 @@ func (h *MCPPerUserHeadersHandler) flowDetail(ctx *fasthttp.RequestCtx) { // edit affordance instead of a fresh form. Identity is the flow row's own // identity column — same convention as OAuth's flowDetail. if h.store.MCPHeadersProvider != nil { - mode := schemas.MCPAuthMode(flow.FlowMode) - identity := "" - switch mode { - case schemas.MCPAuthModeUser: - if flow.UserID != nil { - identity = *flow.UserID - } - case schemas.MCPAuthModeVK: - if flow.VirtualKeyID != nil { - identity = *flow.VirtualKeyID - } - case schemas.MCPAuthModeSession: - identity = flow.SessionID - } - if identity != "" { + if mode, identity, ok := headersFlowIdentity(flow); ok { // GetCredentialByMode returns 'active' and 'needs_update' rows // (orphaned is filtered at the store). SubmittedKeys carries the // previously-submitted key NAMES — useful regardless of status @@ -220,16 +208,30 @@ func (h *MCPPerUserHeadersHandler) flowSubmit(ctx *fasthttp.RequestCtx) { SendError(ctx, fasthttp.StatusInternalServerError, cfgErr.Error()) return } - if missing := missingPerUserHeaderValues(config.PerUserHeaderKeys, req.Headers); len(missing) > 0 { + // Canonicalize incoming header keys (lowercase + trim) before comparing + // against the schema. The schema is already canon by the write-time + // invariant (see mcputils.CanonicalizeHeaderKey doc), so once we + // normalize the request side the exact map lookups below "just work" + // regardless of whether the client UI sent "Authorization" or + // "authorization". A pre-normalization mismatch would otherwise force + // users into unnecessary re-submission loops. + canonHeaders := mcputils.CanonicalizeHeaderMap(req.Headers) + mergedHeaders := canonHeaders + if mode, identity, ok := headersFlowIdentity(flow); ok { + mergedHeaders = mergeExistingPerUserHeaders(ctx, h.store.MCPHeadersProvider, mode, identity, flow.MCPClientID, canonHeaders) + } + if missing := missingPerUserHeaderValues(config.PerUserHeaderKeys, mergedHeaders); len(missing) > 0 { SendError(ctx, fasthttp.StatusBadRequest, fmt.Sprintf("missing values for required keys: %s", strings.Join(missing, ", "))) return } // Filter to declared keys only — extras get dropped on purpose so a stale - // UI cannot persist values that would never be sent on the wire. + // UI cannot persist values that would never be sent on the wire. Both + // the schema and the lookup map are canon at this point, so exact-key + // lookup is correct. filtered := make(map[string]string, len(config.PerUserHeaderKeys)) for _, key := range config.PerUserHeaderKeys { - if v, ok := req.Headers[key]; ok { + if v, ok := mergedHeaders[key]; ok { filtered[key] = v } } @@ -314,6 +316,31 @@ func (h *MCPPerUserHeadersHandler) revoke(ctx *fasthttp.RequestCtx) { SendError(ctx, fasthttp.StatusNotFound, "credential not found") return } + // Drop pending submission flow rows for the same binding BEFORE the + // credential. A holder of the 15-min temp-token who has the auth-page + // URL open in another tab can still PUT /flows/{id}; if the submit + // lands after the credential is gone, the upsert would mint a fresh + // credential and silently undo the revoke. Mirrors mcp_sessions.go. + credMode := schemas.MCPAuthMode(cred.AuthMode) + credIdentity := "" + switch credMode { + case schemas.MCPAuthModeUser: + if cred.UserID != nil { + credIdentity = *cred.UserID + } + case schemas.MCPAuthModeVK: + if cred.VirtualKeyID != nil { + credIdentity = *cred.VirtualKeyID + } + case schemas.MCPAuthModeSession: + credIdentity = cred.SessionID + } + if credIdentity != "" { + if err := h.store.ConfigStore.DeleteMCPPerUserHeaderFlowsByModeIdentityAndMCPClient(ctx, credMode, credIdentity, cred.MCPClientID); err != nil { + SendError(ctx, fasthttp.StatusInternalServerError, fmt.Sprintf("Failed to clear pending submission flows: %v", err)) + return + } + } if err := h.store.MCPHeadersProvider.DeleteCredential(ctx, cred.ID); err != nil { SendError(ctx, fasthttp.StatusInternalServerError, fmt.Sprintf("Failed to revoke credential: %v", err)) return @@ -321,6 +348,54 @@ func (h *MCPPerUserHeadersHandler) revoke(ctx *fasthttp.RequestCtx) { ctx.SetStatusCode(fasthttp.StatusNoContent) } +func headersFlowIdentity(flow *configstoreTables.TableMCPPerUserHeaderFlow) (schemas.MCPAuthMode, string, bool) { + if flow == nil { + return "", "", false + } + mode := schemas.MCPAuthMode(flow.FlowMode) + switch mode { + case schemas.MCPAuthModeUser: + if flow.UserID != nil && *flow.UserID != "" { + return mode, *flow.UserID, true + } + case schemas.MCPAuthModeVK: + if flow.VirtualKeyID != nil && *flow.VirtualKeyID != "" { + return mode, *flow.VirtualKeyID, true + } + case schemas.MCPAuthModeSession: + if flow.SessionID != "" { + return mode, flow.SessionID, true + } + } + return mode, "", false +} + +func mergeExistingPerUserHeaders( + ctx context.Context, + provider schemas.MCPHeadersProvider, + mode schemas.MCPAuthMode, + identity string, + mcpClientID string, + submitted map[string]string, +) map[string]string { + merged := make(map[string]string, len(submitted)) + if provider != nil && identity != "" { + if cred, err := provider.GetCredentialByMode(ctx, mode, identity, mcpClientID); err == nil && cred != nil { + for key, value := range cred.Headers { + if strings.TrimSpace(value) != "" { + merged[key] = value + } + } + } + } + for key, value := range submitted { + if strings.TrimSpace(value) != "" { + merged[key] = value + } + } + return merged +} + // loadMCPClientConfig fetches the MCP client config and verifies it is a // per-user-headers client. Returns a typed error so the handler can pick the // right HTTP status. diff --git a/transports/bifrost-http/handlers/mcp_sessions.go b/transports/bifrost-http/handlers/mcp_sessions.go index a4792c2262..182a2d836c 100644 --- a/transports/bifrost-http/handlers/mcp_sessions.go +++ b/transports/bifrost-http/handlers/mcp_sessions.go @@ -214,7 +214,17 @@ func (h *MCPSessionsHandler) reauth(ctx *fasthttp.RequestCtx) { // Header credential rows and OAuth token rows are both UUIDs in // separate tables. Try headers first; on miss fall through to OAuth. - if headerCred, _ := h.store.ConfigStore.GetMCPPerUserHeaderCredentialByID(ctx, rowID); headerCred != nil { + // Don't swallow store errors here — a real DB outage would otherwise + // surface as a misleading 404 from the OAuth fallback path, which + // hides the outage from the caller and from any retry logic that + // switches on status code. + headerCred, headerCredErr := h.store.ConfigStore.GetMCPPerUserHeaderCredentialByID(ctx, rowID) + if headerCredErr != nil { + logger.Error("[mcp/sessions] load header credential failed: id=%s err=%v", rowID, headerCredErr) + SendError(ctx, fasthttp.StatusInternalServerError, "Failed to load MCP session") + return + } + if headerCred != nil { h.reauthHeaderCredential(ctx, bfCtx, headerCred) return } @@ -351,8 +361,16 @@ func (h *MCPSessionsHandler) revoke(ctx *fasthttp.RequestCtx) { // Row IDs from three tables (OAuth tokens, header credentials, header // flows) are all UUIDs. Try headers first (credential, then pending // flow); on miss fall through to OAuth tokens. Each branch returns; - // only one delete runs per request. - if headerCred, _ := h.store.ConfigStore.GetMCPPerUserHeaderCredentialByID(ctx, rowID); headerCred != nil { + // only one delete runs per request. Store errors must surface as 500 + // (not be swallowed into a miss) so a DB outage doesn't manifest as + // a misleading 404 from the OAuth fallback. + headerCred, headerCredErr := h.store.ConfigStore.GetMCPPerUserHeaderCredentialByID(ctx, rowID) + if headerCredErr != nil { + logger.Error("[mcp/sessions] load header credential failed: id=%s err=%v", rowID, headerCredErr) + SendError(ctx, fasthttp.StatusInternalServerError, "Failed to delete MCP session") + return + } + if headerCred != nil { // Drop pending submission flow rows for the same binding BEFORE // the credential. If a flow finishes (submit lands) after the // credential is gone, the upsert would mint a fresh credential and @@ -387,7 +405,13 @@ func (h *MCPSessionsHandler) revoke(ctx *fasthttp.RequestCtx) { ctx.SetStatusCode(fasthttp.StatusNoContent) return } - if headerFlow, _ := h.store.ConfigStore.GetMCPPerUserHeaderFlowByID(ctx, rowID); headerFlow != nil { + headerFlow, headerFlowErr := h.store.ConfigStore.GetMCPPerUserHeaderFlowByID(ctx, rowID) + if headerFlowErr != nil { + logger.Error("[mcp/sessions] load header flow failed: id=%s err=%v", rowID, headerFlowErr) + SendError(ctx, fasthttp.StatusInternalServerError, "Failed to delete MCP session") + return + } + if headerFlow != nil { if err := h.store.ConfigStore.DeleteMCPPerUserHeaderFlow(ctx, headerFlow.ID); err != nil { logger.Error("[mcp/sessions] delete header flow failed: id=%s err=%v", rowID, err) SendError(ctx, fasthttp.StatusInternalServerError, "Failed to delete MCP session") diff --git a/transports/bifrost-http/lib/config.go b/transports/bifrost-http/lib/config.go index b6adac74cf..deb3d3891e 100644 --- a/transports/bifrost-http/lib/config.go +++ b/transports/bifrost-http/lib/config.go @@ -23,6 +23,7 @@ import ( "github.com/google/uuid" bifrost "github.com/maximhq/bifrost/core" "github.com/maximhq/bifrost/core/mcp" + mcputils "github.com/maximhq/bifrost/core/mcp/utils" "github.com/maximhq/bifrost/core/schemas" "github.com/maximhq/bifrost/framework" "github.com/maximhq/bifrost/framework/configstore" @@ -1466,7 +1467,7 @@ func mcpClientConfigToTable(clientConfig *schemas.MCPClientConfig) (configstoreT Disabled: clientConfig.Disabled, DiscoveredTools: clientConfig.DiscoveredTools, DiscoveredToolNameMapping: clientConfig.DiscoveredToolNameMapping, - PerUserHeaderKeys: clientConfig.PerUserHeaderKeys, + PerUserHeaderKeys: mcputils.CanonicalizeHeaderKeys(clientConfig.PerUserHeaderKeys), ConfigHash: clientConfig.ConfigHash, }, nil } diff --git a/transports/bifrost-http/lib/config_test.go b/transports/bifrost-http/lib/config_test.go index 77afb4a99c..e933d6f0a3 100644 --- a/transports/bifrost-http/lib/config_test.go +++ b/transports/bifrost-http/lib/config_test.go @@ -1325,6 +1325,18 @@ func (m *MockConfigStore) ListAllPendingMCPPerUserHeaderFlows(ctx context.Contex func (m *MockConfigStore) DeleteExpiredMCPPerUserHeaderFlows(ctx context.Context) (int64, error) { return 0, nil } +func (m *MockConfigStore) ReconcileOauthAfterVKChange(ctx context.Context, vkID string) error { + return nil +} +func (m *MockConfigStore) ReconcileMCPHeadersAfterVKChange(ctx context.Context, vkID string) error { + return nil +} +func (m *MockConfigStore) ReconcileOauthAfterMCPChange(ctx context.Context, mcpClientID string) error { + return nil +} +func (m *MockConfigStore) ReconcileMCPHeadersAfterMCPChange(ctx context.Context, mcpClientID string) error { + return nil +} // Routing rules func (m *MockConfigStore) GetRoutingRules(ctx context.Context) ([]tables.TableRoutingRule, error) {