Skip to content

Commit 0526891

Browse files
authored
feat: Add consent-info endpoint and redirect HandleAuthorize to UI consent page (#2157)
* feat: Add consent-info endpoint and redirect HandleAuthorize to UI consent page Task 3: Add HandleConsentInfo endpoint that returns client metadata (name, redirect_uri, scopes, is_dynamic) for the UI consent page. Modify HandleCallback to consume consent codes from ConsentCodeStore instead of exchanging Dex authorization codes. Add scopes claim to issued JWTs. Task 4: When ConsentStore is configured, HandleAuthorize redirects to /auth/mcp-consent on the tenant-scoped domain instead of Dex. Default scopes to ["mcp:default"] when no scope parameter is provided. Introduces ConsentCodeConsumer interface to avoid importing api-gateway (which pulls in broken otel transitive deps). Both Dex and consent paths are preserved - consent path activates when ConsentStore is set. * refactor: Extract consent and callback logic into oidc_consent.go Move ConsentEntry, ConsentCodeConsumer, ConsentInfoResponse types, HandleConsentInfo, handleConsentCallback, handleDexCallback, buildConsentRedirect, and buildConsentPageURL into oidc_consent.go to keep oidc.go under the 800-line service convention limit. * fix: Add scope escalation guard in consent callback Add scope subset validation in handleConsentCallback to reject consent codes whose ApprovedScopes exceed the originally RequestedScopes, preventing over-privileged token issuance. Note: /oauth/consent-info mux registration deferred to the wiring task (main.go pre-commit lint fails due to pre-existing otel dep breakage). * fix: guard consent URL construction and fix stale comment - Return error from buildConsentPageURL when both baseURL and baseDomain are empty, preventing malformed "https:///" redirects - Fix stale comment on resolveTenantSlug that referenced handleDexCallback * fix: validate BaseURL scheme/host and filter scopes to mcp: prefix Address CodeRabbit security feedback: - buildConsentPageURL now validates that BaseURL has both scheme and host, rejecting malformed relative URLs - filterAllowedScopes restricts requested scopes to mcp: prefix only, preventing clients from injecting arbitrary scope strings into signed JWTs * fix: validate consent base URL before storing flow state Prevent orphaned state store entries when BaseURL/BaseDomain is misconfigured by validating the consent configuration before writing to the state store. --------- Co-authored-by: Ben Coombs <bjcoombs@users.noreply.github.com>
1 parent d7f5ba1 commit 0526891

3 files changed

Lines changed: 918 additions & 68 deletions

File tree

services/mcp-server/internal/auth/oidc.go

Lines changed: 54 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -248,12 +248,14 @@ type OIDCHandler struct {
248248
oauthCfg OAuthConfig
249249
stateStore *OIDCStateStore
250250
codeStore *CodeStore
251+
consentStore ConsentCodeConsumer
251252
registry *ClientRegistry
252253
signer *platformauth.JWTSigner
253254
tenantResolver TenantSlugResolver
254255
tokenTTL time.Duration
255256
defaultTenantSlug string
256257
baseDomain string
258+
baseURL string
257259
dexAuthBaseURL string // External Dex base URL for browser redirects (derived from BaseURL + DexIssuerURL path).
258260
httpClient *http.Client
259261
logger *slog.Logger
@@ -267,6 +269,11 @@ type OIDCHandlerConfig struct {
267269
CodeStore *CodeStore
268270
Registry *ClientRegistry
269271
Signer *platformauth.JWTSigner
272+
// ConsentStore is the shared consent code store used by both the BFF
273+
// (which issues consent codes after user approval) and the OIDC handler
274+
// (which consumes them in HandleCallback). When nil, HandleCallback
275+
// falls back to Dex token exchange (legacy flow).
276+
ConsentStore ConsentCodeConsumer
270277
// TenantResolver resolves tenant slugs to UUIDs for JWT claims.
271278
// When nil, the raw slug is used as-is (dev/test fallback).
272279
TenantResolver TenantSlugResolver
@@ -341,12 +348,14 @@ func NewOIDCHandler(cfg OIDCHandlerConfig) (*OIDCHandler, error) {
341348
oauthCfg: cfg.OAuth,
342349
stateStore: cfg.StateStore,
343350
codeStore: cfg.CodeStore,
351+
consentStore: cfg.ConsentStore,
344352
registry: cfg.Registry,
345353
signer: cfg.Signer,
346354
tenantResolver: cfg.TenantResolver,
347355
tokenTTL: ttl,
348356
defaultTenantSlug: cfg.DefaultTenantSlug,
349357
baseDomain: cfg.BaseDomain,
358+
baseURL: cfg.BaseURL,
350359
dexAuthBaseURL: dexAuthBaseURL,
351360
httpClient: httpClient,
352361
logger: cfg.Logger,
@@ -470,9 +479,29 @@ func (h *OIDCHandler) HandleAuthorize(w http.ResponseWriter, r *http.Request) {
470479
}
471480

472481
// Capture requested OAuth scopes from the MCP client (space-delimited per RFC 6749).
473-
requestedScopes := strings.Fields(strings.TrimSpace(q.Get("scope")))
482+
// Only allow scopes with the "mcp:" prefix to prevent arbitrary scope injection.
483+
requestedScopes := filterAllowedScopes(strings.Fields(strings.TrimSpace(q.Get("scope"))))
484+
if len(requestedScopes) == 0 {
485+
requestedScopes = []string{"mcp:default"}
486+
}
487+
488+
// When the consent store is configured, redirect to the UI consent page
489+
// instead of Dex. The consent page handles authentication + authorization
490+
// approval in one step.
491+
if h.consentStore != nil {
492+
redirectURL, err := h.buildConsentRedirect(challenge, clientID, redirectURI, q.Get("state"), tenantSlug, requestedScopes)
493+
if err != nil {
494+
h.writeDexRedirectError(w, err)
495+
return
496+
}
497+
h.logger.Info("oidc: redirecting to consent page",
498+
"tenant", tenantSlug,
499+
"client_id", clientID)
500+
http.Redirect(w, r, redirectURL, http.StatusFound)
501+
return
502+
}
474503

475-
// Store OIDC flow state and redirect to Dex for authentication.
504+
// Legacy path: redirect to Dex for authentication.
476505
redirectURL, err := h.buildDexRedirect(challenge, clientID, redirectURI, q.Get("state"), tenantSlug, requestedScopes)
477506
if err != nil {
478507
h.writeDexRedirectError(w, err)
@@ -486,85 +515,29 @@ func (h *OIDCHandler) HandleAuthorize(w http.ResponseWriter, r *http.Request) {
486515
http.Redirect(w, r, redirectURL, http.StatusFound)
487516
}
488517

489-
// HandleCallback handles GET /oauth/callback from Dex.
490-
// It exchanges the Dex authorization code for an ID token, extracts the email,
491-
// signs a Meridian JWT, and redirects back to the MCP client's redirect_uri.
518+
// HandleCallback handles GET /oauth/callback.
519+
// When the consent store is configured, it consumes a consent code issued by
520+
// the BFF after user approval. Otherwise it falls back to Dex token exchange.
521+
// In both cases it signs a Meridian JWT and redirects to the MCP client.
492522
func (h *OIDCHandler) HandleCallback(w http.ResponseWriter, r *http.Request) {
493523
if r.Method != http.MethodGet {
494524
http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
495525
return
496526
}
497527

498-
ctx := r.Context()
499528
q := r.URL.Query()
500-
501-
// Check for Dex error.
502-
if errParam := q.Get("error"); errParam != "" {
503-
desc := q.Get("error_description")
504-
h.logger.Warn("oidc: Dex returned error",
505-
"error", errParam, "description", desc)
506-
http.Error(w, "authentication failed: "+errParam, http.StatusBadRequest)
507-
return
508-
}
509-
510529
stateKey := q.Get("state")
511530
code := q.Get("code")
512-
if stateKey == "" || code == "" {
513-
http.Error(w, "missing state or code parameter", http.StatusBadRequest)
514-
return
515-
}
516531

517-
// Retrieve and consume state (one-time use).
518-
flowState, ok := h.stateStore.Consume(stateKey)
519-
if !ok {
520-
http.Error(w, "invalid or expired state parameter", http.StatusBadRequest)
532+
if h.consentStore != nil {
533+
h.handleConsentCallback(w, r, stateKey, code)
521534
return
522535
}
523-
524-
// Exchange Dex authorization code for tokens.
525-
idToken, err := h.exchangeDexCode(ctx, code, flowState.DexCodeVerifier)
526-
if err != nil {
527-
h.logger.Error("oidc: Dex token exchange failed",
528-
"tenant", flowState.TenantSlug, "error", err)
529-
http.Error(w, "authentication token exchange failed", http.StatusBadGateway)
530-
return
531-
}
532-
533-
// Extract email from Dex ID token (trusted server-to-server response).
534-
email, err := extractEmailFromJWT(idToken)
535-
if err != nil {
536-
h.logger.Error("oidc: failed to extract email from ID token",
537-
"tenant", flowState.TenantSlug, "error", err)
538-
http.Error(w, "failed to process identity", http.StatusBadGateway)
539-
return
540-
}
541-
542-
// Resolve tenant slug to UUID for JWT consistency with BFF-issued tokens.
543-
tenantID, err := h.resolveTenantID(ctx, flowState.TenantSlug)
544-
if err != nil {
545-
http.Error(w, errTenantResolutionFailed.Error(), http.StatusInternalServerError)
546-
return
547-
}
548-
549-
// Defense-in-depth: validate redirect URI scheme before signing tokens.
550-
// The URI was validated at authorize-time, but re-check before redirect.
551-
if !isAllowedRedirectURI(flowState.MCPRedirectURI) {
552-
h.logger.Error("oidc: unsafe redirect URI scheme", "uri", flowState.MCPRedirectURI)
553-
http.Error(w, "invalid redirect_uri", http.StatusBadRequest)
554-
return
555-
}
556-
557-
// Issue MCP authorization code backed by a Meridian JWT.
558-
redirectURL, err := h.issueCodeAndRedirect(email, tenantID, flowState)
559-
if err != nil {
560-
h.logger.Error("oidc: failed to issue auth code", "error", err)
561-
http.Error(w, "internal server error", http.StatusInternalServerError)
562-
return
563-
}
564-
http.Redirect(w, r, redirectURL, http.StatusFound)
536+
h.handleDexCallback(w, r, stateKey, code)
565537
}
566538

567-
// resolveTenantSlug extracts the tenant slug from the request host subdomain, falling back to the default.
539+
// resolveTenantSlug extracts the tenant slug from the request host subdomain,
540+
// falling back to the default.
568541
func (h *OIDCHandler) resolveTenantSlug(host string) (string, error) {
569542
tenantSlug := extractSubdomain(host, h.baseDomain)
570543
if tenantSlug == "" && h.defaultTenantSlug != "" {
@@ -629,11 +602,12 @@ func (h *OIDCHandler) resolveTenantID(ctx context.Context, tenantSlug string) (s
629602
}
630603

631604
// issueCodeAndRedirect signs a Meridian JWT, generates an MCP authorization code, stores it, and returns the redirect URL.
632-
func (h *OIDCHandler) issueCodeAndRedirect(email, tenantID string, flowState OIDCFlowState) (string, error) {
605+
func (h *OIDCHandler) issueCodeAndRedirect(email, tenantID string, scopes []string, flowState OIDCFlowState) (string, error) {
633606
claims := map[string]interface{}{
634607
"sub": email,
635608
"email": email,
636609
"x-tenant-id": tenantID,
610+
"scopes": scopes,
637611
}
638612
tokenStr, err := h.signer.SignClaims(claims, h.tokenTTL)
639613
if err != nil {
@@ -816,3 +790,15 @@ func isAllowedRedirectURI(uri string) bool {
816790
}
817791
return false
818792
}
793+
794+
// filterAllowedScopes returns only scopes with the "mcp:" prefix,
795+
// preventing clients from injecting arbitrary scope strings into signed JWTs.
796+
func filterAllowedScopes(scopes []string) []string {
797+
var allowed []string
798+
for _, s := range scopes {
799+
if strings.HasPrefix(s, "mcp:") {
800+
allowed = append(allowed, s)
801+
}
802+
}
803+
return allowed
804+
}

0 commit comments

Comments
 (0)