Skip to content

refactor: remove deferred-fill user-mode OAuth flow support #3839

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
akshaydeo merged 1 commit into from
May 28, 2026
Merged

refactor: remove deferred-fill user-mode OAuth flow support #3839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion framework/configstore/tables/oauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:"-"`
Expand Down
47 changes: 15 additions & 32 deletions framework/oauth2/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 == "" {
Expand Down Expand Up @@ -978,22 +974,19 @@ 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
// state/PKCE here would invalidate that callback's state token and
// 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
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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 == "" {
Expand Down
27 changes: 3 additions & 24 deletions transports/bifrost-http/handlers/mcp_sessions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
Expand Down Expand Up @@ -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 == "" {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
Loading