Fix MCP OAuth login with OIDC SSO

[Vishnuujain]

Fixes #30445

Problem

ChatGPT (or other MCP tool) login break for companies using SSO login (OIDC). Server check id from wrong source expect Google, get company's own login server. Login fail, 500 error.

Fix

• MCP login flow now always fetch identity fresh from company's own SSO server, not reuse old web session shortcut.
• Track login state (state/nonce/PKCE) properly through round trip so callback can match.

Web login path is untouched (guarded behind !isMcpFlow).

Greptile Summary

Updates the MCP OIDC login flow to persist callback correlation state before redirecting to the identity provider.

• Registers an MCP pending-state persistence hook during server initialization.
• Bypasses the OpenMetadata active-session shortcut for MCP logins so identity comes from the configured OIDC provider.
• Centralizes MCP session attribute names and updates callback handling accordingly.
• Adds focused tests for MCP redirect detection, state persistence, and unchanged web-login behavior.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failures remain in the fix for the previously reported callback-state persistence race.

Important Files Changed

Filename	Overview
openmetadata-mcp/src/main/java/org/openmetadata/mcp/McpServer.java	Registers the pending-state persistence bridge used before the OIDC provider redirect.
openmetadata-mcp/src/main/java/org/openmetadata/mcp/server/auth/provider/UserSSOOAuthProvider.java	Carries the MCP request identifier through the shared session and verifies that OIDC state was linked.
openmetadata-mcp/src/main/java/org/openmetadata/mcp/server/auth/handlers/McpCallbackServlet.java	Reuses the centralized MCP authorization-request session key in the direct token callback path.
openmetadata-service/src/main/java/org/openmetadata/service/security/AuthenticationCodeFlowHandler.java	Forces MCP requests through the external OIDC round trip and invokes state persistence before redirecting.
openmetadata-service/src/test/java/org/openmetadata/service/security/AuthenticationCodeFlowHandlerTest.java	Covers MCP flow detection, pre-redirect persistence, and preservation of the existing web-login shortcut.

Sequence Diagram

sequenceDiagram
  participant Client as MCP Client
  participant Provider as UserSSOOAuthProvider
  participant Handler as AuthenticationCodeFlowHandler
  participant Store as MCP Pending Request Store
  participant IdP as OIDC Provider
  participant Callback as MCP Callback

  Client->>Provider: Start OAuth authorization
  Provider->>Store: Create pending request
  Provider->>Handler: handleLogin(MCP redirect URI)
  Handler->>Store: Persist state, nonce, and PKCE verifier
  Handler->>Handler: Create pending OIDC session
  Handler-->>IdP: Redirect authorization request
  IdP-->>Callback: Return authorization code and state
  Callback->>Store: Resolve pending request by state
  Callback->>Handler: Exchange code and validate identity
  Callback-->>Client: Complete MCP authorization

Loading

_{Reviews (7): Last reviewed commit: "Merge branch 'main' into fix-mcp-oauth-i..." | Re-trigger Greptile}

Context used:

• Knowledge Base — OpenMetadata MCP Server
• Knowledge Base — Auth and Security: Authentication, Authorization, Secrets, and SCIM

[github-actions]

🔴 Playwright Results — workflow failed

Validated commit 2c9b71b9caa802eeac1854fa3563227b203c7bc4 in Playwright run 30242227154, attempt 1.

✅ 0 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Pipeline and setup failures (5)

• Duration-aware shard planning finished with status failure.
• Fixture cache restoration finished with status skipped.
• Seeded fixture preparation finished with status skipped.
• The Playwright shard matrix was unexpectedly skipped.
• No expected Playwright shards were declared.

Performance

⚪ Performance metrics unavailable; see the CI and reporting failures above.

Shard	Passed	Failed	Flaky	Skipped	Lifecycle failed	Lifecycle flaky

📦 Download artifacts

How to debug locally

# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

[github-actions]

✅ PR checks passed

The linked issue has a description and all required Shipping project fields set. Thanks!

[gitar-bot]

Code Review ✅ Approved 2 resolved / 2 findings

Refactors MCP OAuth to use the full OIDC round-trip and bypass the internal active-session shortcut, resolving the ID token issuer mismatch. Clean implementation with robust test coverage and no open findings.

✅ 2 resolved

✅ Quality: Misleading log/dead path after removing early return in MCP flow

📄 openmetadata-mcp/src/main/java/org/openmetadata/mcp/server/auth/provider/UserSSOOAuthProvider.java:359-373
This commit moved the pending-request linking into handleLogin() via the persister and removed the early return that previously followed it. Control now falls through to the pac4j session-attribute scan (lines 359-431). Since AuthenticationCodeFlowHandler stores state via sessionService.createPendingSession (DB), not the HttpSession, pac4jState is always null for the MCP round-trip, so the code lands in the else branch and logs the 'active-session shortcut detected ... id_token in URL fragment' message — which is now factually wrong because MCP explicitly bypasses the active-session shortcut (AuthenticationCodeFlowHandler.java:387). This will mislead operators debugging MCP OAuth. Consider re-adding an early return for the MCP flow after handleLogin(), or removing/adjusting this now-dead pac4j-extraction block and its log message.

✅ Edge Case: Persister silently no-ops if session/auth-request id missing

📄 openmetadata-mcp/src/main/java/org/openmetadata/mcp/McpServer.java:194-207 📄 openmetadata-service/src/main/java/org/openmetadata/service/security/AuthenticationCodeFlowHandler.java:439-444
The registered persister only calls updatePac4jSession when request.getSession(false) is non-null and MCP_AUTH_REQUEST_ID is a String; otherwise it silently does nothing, and persistMcpPendingState also swallows a null persister. If linking fails, handleSSOAuthorization still returns 'SSO_REDIRECT_INITIATED' (the pac4j-extraction else-branch logs success on a committed response), so the returning /callback?state= will later fail with 'state not found or expired' with no earlier signal. Given setAttribute runs just before handleLogin the happy path is fine, but consider logging a warning when the persister cannot resolve the auth-request id so this failure mode is diagnosable.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}