Cherry pic/1.13 sso fix by chirag-madlani · Pull Request #31573 · open-metadata/OpenMetadata

chirag-madlani · 2026-08-15T06:31:06Z

Describe your changes:

Fixes #

I worked on ... because ...

Type of change:

High-level design:

N/A — small change.

Tests:

Use cases covered

Unit tests

Backend integration tests

Ingestion integration tests

Playwright (UI) tests

Manual testing performed

UI screen recording / screenshots:

Not applicable.

Checklist:

I have read the CONTRIBUTING document.
My PR title is Fixes <issue-number>: <short explanation>
My PR is linked to a GitHub issue via Fixes #<issue-number> above.
I have commented on my code, particularly in hard-to-understand areas.
For JSON Schema changes: I updated the migration scripts or explained why it is not needed.
For UI changes: I attached a screen recording and/or screenshots above.
I have added tests (unit / integration / Playwright as applicable) and listed them above.

Summary by Gitar

Authentication fixes:
- Serve persisted responseType, publicKeyUrls, and tokenValidationAlgorithm from /config/auth (Bug: /api/v1/system/config/auth serves a frozen/stale authentication config that does not update regardless of env vars, database state, or provider changes — breaks SSO login end-to-end #29597)
- Forward server-configured OIDC responseType across all providers in AuthProvider.util.ts (Bug: /api/v1/system/config/auth serves a frozen/stale authentication config that does not update regardless of env vars, database state, or provider changes — breaks SSO login end-to-end #29597)
Tests:
- Added unit and integration tests for ConfigResource and AuthProvider utilities
- Added Playwright e2e test for SSO login response type validation

_{This will update automatically on new commits.}

…ionAlgorithm from /config/auth (#29597) (#31516) ConfigResource.getAuthConfig() builds a fresh AuthenticationConfiguration and hand-copies only a subset of fields from the persisted config. It never copied responseType, publicKeyUrls, or tokenValidationAlgorithm, so those fell back to their JSON-schema POJO defaults (responseType="id_token", publicKeyUrls=[]) regardless of what was configured. For AWS Cognito (and any provider) configured with responseType="code", the public /api/v1/system/config/auth endpoint therefore served "id_token", pushing the browser onto the implicit flow — no server-side session, the user bounces back to /signin, and login never completes. Copy the three login-relevant public fields from the persisted config. Tests: - ConfigResourceTest (unit): mocks the config singleton and proves getAuthConfig reflects persisted responseType/publicKeyUrls/tokenValidationAlgorithm; fails before the fix (code->id_token, [url]->[]). - ConfigResourceIT: adds a real-server invariant cross-checking /config/auth against the admin /security/config; drops the stale assertion that required publicKeyUrls to be empty. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit d0aeedd)

…ders (#29597) (#31517) * fix(ui): respect server-configured OIDC responseType across all providers (#29597) getUserManagerConfig — the config handed to the oidc-client UserManager — never forwarded responseType, so oidc-client fell back to its built-in 'id_token' default. Every front-channel OIDC login (AWS Cognito, Google, Custom OIDC public clients) therefore sent response_type=id_token regardless of the server config, forcing the implicit flow: no server session, bounce back to /signin, login never completes. Changes: - getUserManagerConfig: forward response_type from the resolved config (defaulting to the authorization-code flow, not id_token). - getAuthConfig: every provider now honors the server-provided responseType instead of a hardcoded value; AWS Cognito no longer hardcodes 'code' and falls back to 'code' only when the server omits it (Google/Custom OIDC keep their historical 'id_token' fallback). Combined with the backend fix, the persisted responseType flows through end-to-end. - getCandidateUserManagerConfig (SSO test-login popup): respect responseType instead of hardcoding 'id_token'. Tests: - AuthProvider.util.test.ts (Jest): 9 cases proving getUserManagerConfig, getAuthConfig (Cognito/Google/Custom OIDC), and getCandidateUserManagerConfig respect the server responseType. Verified RED before the fix (response_type undefined; Cognito 'id_token'->'code' hardcode), GREEN after. - Playwright SSO harness: new public-client provider 'keycloak-oidc-public' (the only path that traverses getUserManagerConfig) plus a keycloak realm public client with implicit flow disabled, and a response_type assertion on the IdP authorize redirect in SSOLogin.spec.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(ui): keep id_token as the responseType default (review) Address review: don't change any default from id_token to code. Restore the blanket `responseType = 'id_token'` destructure default in getAuthConfig (matches the authenticationConfiguration schema default) and drop the per-provider `?? 'code'`/`?? 'id_token'` overrides. AWS Cognito now simply passes `responseType` through (respecting the server value) instead of hardcoding 'code' — the hardcoded value never reached the browser anyway, since getUserManagerConfig dropped the field. The two UserManager builders keep 'id_token' as the fallback. Net change is now just: forward responseType in getUserManagerConfig / getCandidateUserManagerConfig, and stop Cognito hardcoding 'code'. Tests updated: absent-responseType cases now assert the id_token schema default. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit e4abbec)

github-actions · 2026-08-15T06:31:15Z

❌ 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.

gitar-bot · 2026-08-15T06:32:02Z

+import {
+  getAuthConfig,
+  getCandidateUserManagerConfig,
+  getUserManagerConfig,


🚨 Bug: Test imports non-existent getCandidateUserManagerConfig

AuthProvider.util.test.ts imports getCandidateUserManagerConfig from ./AuthProvider.util, but that function is not defined or exported anywhere in the codebase (only referenced in the test). The import resolves to undefined, so the getCandidateUserManagerConfig(...) call in the 'SSO test-login popup respects responseType' test throws TypeError: getCandidateUserManagerConfig is not a function, failing the suite. Either add and export getCandidateUserManagerConfig in AuthProvider.util.ts or remove that describe block and import.

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

gitar-bot · 2026-08-15T06:32:03Z

Code Review 🚫 Blocked 0 resolved / 1 findings

Updates OIDC authentication configuration and UI providers to respect server-configured response types, but the new tests fail due to importing a non-existent getCandidateUserManagerConfig from AuthProvider.

🚨 Bug: Test imports non-existent getCandidateUserManagerConfig

📄 openmetadata-ui/src/main/resources/ui/src/utils/AuthProvider.util.test.ts:23 📄 openmetadata-ui/src/main/resources/ui/src/utils/AuthProvider.util.test.ts:131-138

AuthProvider.util.test.ts imports getCandidateUserManagerConfig from ./AuthProvider.util, but that function is not defined or exported anywhere in the codebase (only referenced in the test). The import resolves to undefined, so the getCandidateUserManagerConfig(...) call in the 'SSO test-login popup respects responseType' test throws TypeError: getCandidateUserManagerConfig is not a function, failing the suite. Either add and export getCandidateUserManagerConfig in AuthProvider.util.ts or remove that describe block and import.

🤖 Prompt for agents

Code Review: Updates OIDC authentication configuration and UI providers to respect server-configured response types, but the new tests fail due to importing a non-existent getCandidateUserManagerConfig from AuthProvider.

1. 🚨 Bug: Test imports non-existent getCandidateUserManagerConfig
   Files: openmetadata-ui/src/main/resources/ui/src/utils/AuthProvider.util.test.ts:23, openmetadata-ui/src/main/resources/ui/src/utils/AuthProvider.util.test.ts:131-138

   AuthProvider.util.test.ts imports `getCandidateUserManagerConfig` from `./AuthProvider.util`, but that function is not defined or exported anywhere in the codebase (only referenced in the test). The import resolves to `undefined`, so the `getCandidateUserManagerConfig(...)` call in the 'SSO test-login popup respects responseType' test throws `TypeError: getCandidateUserManagerConfig is not a function`, failing the suite. Either add and export `getCandidateUserManagerConfig` in AuthProvider.util.ts or remove that describe block and import.

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 👍 / 👎 | Powered by Gitar — free for open source}

github-actions · 2026-08-15T06:36:15Z

❌ UI Checkstyle Failed

❌ ESLint + Prettier + Organise Imports (src)

One or more source files have linting or formatting issues.

❌ Core Components - I18n Sync

Core-components t() keys, locale files, or language-set are out of sync. Run yarn check-i18n-all locally.

❌ Tailwind Audit

Hardcoded Tailwind values found. Use a design-system utility (run yarn tw-audit:report for the token each value maps to).

❌ Antd + Less Deprecation Guard

A new antd import or new .less file was added. Use UntitledUI + Tailwind for new work.

Affected files

at Function._resolveFilename (node:internal/modules/cjs/loader:1401:15)
at defaultResolveImpl (node:internal/modules/cjs/loader:1057:19)
at resolveForCJSWithHooks (node:internal/modules/cjs/loader:1062:22)
at Function._load (node:internal/modules/cjs/loader:1211:37)
at TracingChannel.traceSync (node:diagnostics_channel:322:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:235:24)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:171:5)
at node:internal/main/run_main_module:36:49 {

Fix locally (fast - only checks files changed in this branch):

make ui-checkstyle-changed

chirag-madlani and others added 2 commits August 15, 2026 11:20

chirag-madlani requested review from a team, akash-jain-10, harshach and tutte as code owners August 15, 2026 06:31

github-actions Bot added safe to test Add this label to run secure Github workflows on PRs UI UI specific issues labels Aug 15, 2026

gitar-bot Bot reviewed Aug 15, 2026

View reviewed changes

chirag-madlani temporarily deployed to test August 15, 2026 06:41 — with GitHub Actions Inactive

chirag-madlani had a problem deploying to test August 15, 2026 06:41 — with GitHub Actions Failure

Navigation Menu

Uh oh!

Cherry pic/1.13 sso fix - #31573

Cherry pic/1.13 sso fix#31573
chirag-madlani wants to merge 2 commits into
1.13from
cherry-pic/1.13-sso-fix

chirag-madlani commented Aug 15, 2026 •

edited by gitar-bot Bot

Loading

Uh oh!

github-actions Bot commented Aug 15, 2026

Uh oh!

gitar-bot Bot Aug 15, 2026

Uh oh!

gitar-bot Bot commented Aug 15, 2026

Uh oh!

github-actions Bot commented Aug 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

chirag-madlani commented Aug 15, 2026 • edited by gitar-bot Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes:

Type of change:

High-level design:

Tests:

Use cases covered

Unit tests

Backend integration tests

Ingestion integration tests

Playwright (UI) tests

Manual testing performed

UI screen recording / screenshots:

Checklist:

Summary by Gitar

Uh oh!

github-actions Bot commented Aug 15, 2026

❌ PR checklist incomplete

Uh oh!

gitar-bot Bot Aug 15, 2026

Choose a reason for hiding this comment

Uh oh!

gitar-bot Bot commented Aug 15, 2026

Uh oh!

github-actions Bot commented Aug 15, 2026

❌ UI Checkstyle Failed

❌ ESLint + Prettier + Organise Imports (src)

❌ Core Components - I18n Sync

❌ Tailwind Audit

❌ Antd + Less Deprecation Guard

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

chirag-madlani commented Aug 15, 2026 •

edited by gitar-bot Bot

Loading