diff --git a/framework/configstore/tables/oauth.go b/framework/configstore/tables/oauth.go index 144c20a586..980479c24c 100644 --- a/framework/configstore/tables/oauth.go +++ b/framework/configstore/tables/oauth.go @@ -165,7 +165,7 @@ type TableOauthUserSession struct { CodeVerifier string `gorm:"type:text" json:"-"` // PKCE code verifier (kept secret) SessionID string `gorm:"type:varchar(255);index" json:"session_id,omitempty"` // Session-mode identity: client-asserted x-bf-mcp-session-id. Empty for vk/user mode rows. Stored plaintext (not a bearer credential; same trust model as a VK value). VirtualKeyID *string `gorm:"type:varchar(255);index" json:"virtual_key_id"` // VK identity (propagated to oauth_user_tokens) - UserID *string `gorm:"type:varchar(255);index" json:"user_id"` // Enterprise user identity (propagated to oauth_user_tokens); nullable for deferred-fill user-mode flows + UserID *string `gorm:"type:varchar(255);index" json:"user_id"` // User identity (propagated to oauth_user_tokens); populated only for user-mode rows, nil for vk/session-mode FlowMode string `gorm:"type:varchar(20);not null;default:'vk'" json:"flow_mode"` // 'user' | 'vk' | 'session' — mirrors the token row's AuthMode; immutable after creation Status string `gorm:"type:varchar(50);not null;index" json:"status"` // "pending", "authorized", "failed", "expired" EncryptionStatus string `gorm:"type:varchar(20);default:'plain_text'" json:"-"` diff --git a/framework/oauth2/main.go b/framework/oauth2/main.go index eb113d3a66..7749cfafb2 100644 --- a/framework/oauth2/main.go +++ b/framework/oauth2/main.go @@ -940,14 +940,10 @@ func (p *OAuth2Provider) InitiateUserOAuthFlow(ctx context.Context, oauthConfigI case schemas.MCPAuthModeUser: v, _ := ctx.Value(schemas.BifrostContextKeyUserID).(string) if v == "" { - // Deferred-fill: external MCP client OAuth init where the user - // identity is stamped at completion time. No identity → no - // existing-row lookup; we always insert a fresh row. - uid = nil - } else { - uid = &v - lookupID = v + return nil, "", fmt.Errorf("user-mode flow requires a user identity in context") } + uid = &v + lookupID = v case schemas.MCPAuthModeVK: v, _ := ctx.Value(schemas.BifrostContextKeyGovernanceVirtualKeyID).(string) if v == "" { @@ -978,7 +974,6 @@ func (p *OAuth2Provider) InitiateUserOAuthFlow(ctx context.Context, oauthConfigI expiresAt := time.Now().Add(15 * time.Minute) // 4. Single canonical lookup: one flow row per (mode, identity, mcp_client). - // Deferred-fill user-mode (lookupID empty) always inserts a fresh row. // // Only treat a 'pending' hit as reusable. A 'claiming' row means an // upstream callback is mid-flight for this binding — rotating its @@ -986,14 +981,12 @@ func (p *OAuth2Provider) InitiateUserOAuthFlow(ctx context.Context, oauthConfigI // leave the user with an "OAuth flow not found" error. Falling through // to insert a fresh row lets both flows complete independently. var existing *tables.TableOauthUserSession - if lookupID != "" { - found, lookupErr := p.configStore.GetOauthUserSessionByModeIdentityAndMCPClient(ctx, flowMode, lookupID, mcpClientID) - if lookupErr != nil { - return nil, "", fmt.Errorf("failed to look up existing flow row: %w", lookupErr) - } - if found != nil && found.Status == "pending" { - existing = found - } + found, lookupErr := p.configStore.GetOauthUserSessionByModeIdentityAndMCPClient(ctx, flowMode, lookupID, mcpClientID) + if lookupErr != nil { + return nil, "", fmt.Errorf("failed to look up existing flow row: %w", lookupErr) + } + if found != nil && found.Status == "pending" { + existing = found } var rowID string @@ -1011,10 +1004,10 @@ func (p *OAuth2Provider) InitiateUserOAuthFlow(ctx context.Context, oauthConfigI } rowID = existing.ID } else { - // First-time auth (or deferred-fill user): insert a new row. SessionID - // is populated only for session-mode (the caller's x-bf-mcp-session-id); - // vk/user-mode rows have an empty SessionID — their identity lives in - // virtual_key_id / user_id. + // First-time auth: insert a new row. SessionID is populated only for + // session-mode (the caller's x-bf-mcp-session-id); vk/user-mode rows + // have an empty SessionID — their identity lives in virtual_key_id / + // user_id. row := &tables.TableOauthUserSession{ ID: uuid.New().String(), MCPClientID: mcpClientID, @@ -1144,9 +1137,7 @@ func (p *OAuth2Provider) CompleteUserOAuthFlow(ctx context.Context, state string // Resolve identity columns from the flow row, honoring its flow_mode. Only // the column that matches the mode is populated on the token row; the others - // stay nil to maintain the single-identity invariant. For user-mode flows - // where the row's UserID was deferred at init time, stamp from completer - // context here. + // stay nil to maintain the single-identity invariant. flowMode := schemas.MCPAuthMode(session.FlowMode) var ( tokenVKID *string @@ -1155,18 +1146,10 @@ func (p *OAuth2Provider) CompleteUserOAuthFlow(ctx context.Context, state string switch flowMode { case schemas.MCPAuthModeUser: tokenUserID = session.UserID - if tokenUserID == nil || *tokenUserID == "" { - // Deferred fill: pull from completer context. - if v, _ := ctx.Value(schemas.BifrostContextKeyUserID).(string); v != "" { - tokenUserID = &v - } - } if tokenUserID == nil || *tokenUserID == "" { p.cleanupFlow(ctx, session.ID) - return "", fmt.Errorf("user-mode oauth flow has no user_id at completion (neither flow nor completer context)") + return "", fmt.Errorf("user-mode oauth flow has no user_id at completion") } - // Stamp the resolved user_id back on the flow for audit. - session.UserID = tokenUserID case schemas.MCPAuthModeVK: tokenVKID = session.VirtualKeyID if tokenVKID == nil || *tokenVKID == "" { diff --git a/transports/bifrost-http/handlers/mcp_sessions.go b/transports/bifrost-http/handlers/mcp_sessions.go index 1575ebeb14..d34b0e7cd4 100644 --- a/transports/bifrost-http/handlers/mcp_sessions.go +++ b/transports/bifrost-http/handlers/mcp_sessions.go @@ -279,12 +279,6 @@ func (h *MCPSessionsHandler) list(ctx *fasthttp.RequestCtx) { if _, hasToken := tokenBindings[bindingKeyFromFlow(f)]; hasToken { continue } - // Skip deferred-fill user-mode flow rows (user_id not yet stamped). - // They have no concrete binding to render, and surfacing them in - // the table forces an ambiguous label. - if f.FlowMode == string(schemas.MCPAuthModeUser) && (f.UserID == nil || *f.UserID == "") { - continue - } rows = append(rows, flowRow(f)) } } @@ -696,10 +690,8 @@ type mcpFlowDetailResponse struct { // flowDetail returns the pending flow row's metadata so the frontend sessions // auth page can render a "you're about to authenticate X" view. // -// Permission model: deferred-fill flows (flow_mode='user' with user_id=nil) -// are visible to any user-mode caller — the first SCIM-authenticated user to -// open the URL will become the row's user_id at completion time. For all -// other modes the caller's identity must match the row's identity column. +// Permission model: the caller's identity must match the row's identity column +// for the row's flow_mode. Enforced at the configstore layer via DAC scope. func (h *MCPSessionsHandler) flowDetail(ctx *fasthttp.RequestCtx) { flowID, ok := ctx.UserValue("id").(string) if !ok || flowID == "" { @@ -748,17 +740,6 @@ func (h *MCPSessionsHandler) flowDetail(ctx *fasthttp.RequestCtx) { case schemas.MCPAuthModeUser: if flow.UserID != nil && *flow.UserID != "" { identity = *flow.UserID - } else if v, _ := ctx.UserValue(schemas.BifrostContextKeyUserID).(string); v != "" { - // Deferred-fill: flow.UserID is nil until completion. Fall - // back to the signed-in caller's user_id so HasActiveToken - // reflects whether THIS user already has a credential for - // the MCP client and we don't prompt an unnecessary re-auth. - // - // Use UserValue (not Value) — auth middleware stores via - // SetUserValue, and fasthttp's Value() only handles bare - // string keys, so a typed BifrostContextKey lookup via - // Value() always returns nil. - identity = v } case schemas.MCPAuthModeVK: if flow.VirtualKeyID != nil { @@ -817,9 +798,7 @@ func (h *MCPSessionsHandler) flowStart(ctx *fasthttp.RequestCtx) { // loadAuthorizedFlow looks up the pending flow row. Visibility is enforced // at the enterprise configstore layer via DAC scope on // GetOauthUserSessionByID: if the caller is not allowed to see this row, -// the store returns (nil, nil) and we surface 404. Deferred-fill user-mode -// flows (user_id IS NULL) are visible to any SCIM-authenticated caller by -// the scope builder so they can claim the auth URL. Writes the appropriate +// the store returns (nil, nil) and we surface 404. Writes the appropriate // HTTP error response and returns a sentinel error on failure. func (h *MCPSessionsHandler) loadAuthorizedFlow(ctx *fasthttp.RequestCtx, flowID string) (*tables.TableOauthUserSession, error) { flow, err := h.store.ConfigStore.GetOauthUserSessionByID(ctx, flowID)