fix(auth): require an authorization code on the confidential OIDC callback

[mohityadav766]

Summary

Harden the confidential OIDC login callback so the session is established only from the ID token returned by the back-channel token exchange. The callback now requires an authorization code and rejects a request that carries only a front-channel token.

Fixes #

Tests

• Added unit tests in AuthenticationCodeFlowHandlerTest for the callback with, and without, an authorization code.
• mvn -pl openmetadata-service -am ... -Dtest=AuthenticationCodeFlowHandlerTest package → green (85 tests).

🤖 Generated with Claude Code

Confidence Score: 3/5

The PR should not merge until implicit callbacks cannot create sessions from forged identity claims.

Findings

1.   Forged tokens create sessions ▶

Summary

The PR requires an authorization code before exchanging tokens for code-based OIDC login.

• OIDC code-flow callbacks now require an authorization code.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[OIDC callback] --> B{Configured response type includes code?}
  B -->|Yes| C{Authorization code present?}
  C -->|No| D[Reject with HTTP 400]
  C -->|Yes| E[Exchange code with provider]
  B -->|No| F[Keep browser-supplied token]
  E --> G[Read identity claims]
  F --> G
  G --> H[Find user and activate session]

Loading

_{Reviews (2) · Last reviewed commit: "fix(auth): scope the callback code requi..." · Reviewed by Greptile}

[github-actions]

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

• No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

[github-actions]

✅ SSO Login Nightly — PASSED

View workflow run · Commit 608db34

Provider	Status	Pass	Fail	Flaky	Skip	Duration
basic	✅ pass	28	0	0	0	6m 50s
ldap	✅ pass	7	0	0	0	0m 44s
keycloak-oidc-confidential	✅ pass	12	0	0	0	1m 19s
keycloak-oidc-public	✅ pass	11	0	0	0	1m 29s
msal-mock	✅ pass	7	0	0	0	0m 41s
auth0-mock	✅ pass	6	0	0	0	0m 54s
okta	⏩ skipped	-	-	-	-	-
keycloak-azure-saml	✅ pass	23	0	0	0	6m 38s
keycloak-azure-saml-crosssite	⏩ skipped	-	-	-	-	-

Totals: 94 passed · 0 failed · 0 flaky · 0 skipped · duration 18m 39s

[gitar-bot]

Code Review ✅ Approved 3 closed / 3 findings

🔴 High risk · Requiring a code on confidential OIDC callbacks changes authentication decisions; providers returning incompatible responses could block legitimate logins.

Hardens the confidential OIDC callback to require an authorization code and establish sessions only from the back-channel ID token. Resolved findings: confidential clients with responseType id_token no longer fail every login, the back-channel rule is now recorded, and tests exercise the callback instead of calling the private guard by reflection. No open issues remain.

✅ 3 closed

✅ Edge Case: Confidential clients set to responseType id_token now fail every login

📄 openmetadata-service/src/main/java/org/openmetadata/service/security/AuthenticationCodeFlowHandler.java:619 📄 openmetadata-service/src/main/java/org/openmetadata/service/security/AuthenticationCodeFlowHandler.java:922-929
oidcConfiguration.responseType can still be set by an operator (conf/openmetadata.yaml:541 takes ${OIDC_RESPONSE_TYPE:-"code"}, and oidcClientConfig.json allows any string). buildLoginParams sends it to the provider unchanged (line 914). A deployment that sets OIDC_RESPONSE_TYPE=id_token gets a callback with no code. After this change, requireAuthorizationCode throws on that callback and handleCallback returns a 400 "Authorization code is required" on every sign-in. Nothing catches the setting earlier: validateConfig and OidcDiscoveryValidator.validateResponseType both still accept id_token, so an upgrade turns a working config into a total login outage that only shows up at runtime.

Fix: reject any non-code response type for confidential clients in validateConfig/buildOidcClient, or force code there, so the bad config fails at startup or when saved, with a clear message. Add a release note for upgrades.

✅ Quality: New rule "confidential login only via back-channel ID token" is unrecorded

📄 openmetadata-service/src/main/java/org/openmetadata/service/security/AuthenticationCodeFlowHandler.java:616-619
The PR description states a rule: "the session is established only from the ID token returned by the back-channel token exchange. The callback now requires an authorization code and rejects a request that carries only a front-channel token." No record in docs/decisions/ says this. The repo squash-merges, so the description will not reach git log. The rule also changes a configuration contract: responseType: id_token (still in the schema enum and in OIDC_RESPONSE_TYPE) stops being valid for confidential clients. The record would say: "A confidential OIDC client signs a user in only from the ID token returned by the token endpoint for an authorization code. A callback without a code is rejected with 400, and responseType must be code." Its Guard would be the callback tests.

✅ Quality: Tests call the private guard by reflection, not the callback

📄 openmetadata-service/src/test/java/org/openmetadata/service/security/AuthenticationCodeFlowHandlerTest.java:1808-1822
Both new tests reach requireAuthorizationCode by reflection and only check that it throws. They would still pass if the call were removed from handleCallback, or moved after validateAndSendTokenRequest/toIdToken(). In either case the code-less callback would go back to creating a session from the unverified front-channel token. That breaks the repo rule to test observable behavior, and the bug-fix rule that a test must fail without the fix.

Fix: drive handleCallback with callback parameters that hold only id_token and a valid state, using the existing createLoginHandler/stubProviderMetadataForLogin helpers. Assert a 400 response, and that sessionService.activatePendingSession was never called and no token request was sent.

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

Auto-approval Not enabled

Set up

_{For more verification details, comment gitar display:verbose.}

Options

Display: compact → Showing compact verification results.

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

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source}

[greptile-apps]

Forged tokens create sessions

When responseType is id_token, the new expectsAuthorizationCode() exemption lets a token-only callback skip the token exchange. handleCallback then reads the supplied token’s claims without checking its signature and uses them to select a user and activate a session. Someone can start login, copy the state and nonce from the login redirect, and submit a forged token naming another user.

Keep the authorization-code requirement for this handler, or verify front-channel tokens before trusting their claims.

How this was verified: The exempt callback copies the supplied token into credentials, skips the code exchange, and passes its unchecked claims through user lookup to session activation.

Knowledge Base Used: Platform security and governance