-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Fix user creation via UI when SSO authentication is enabled #25212
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.
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
Conversation
- Conditionally add password-related fields only when Basic/LDAP auth is configured - Add tests to verify password fields are excluded for SSO providers - This fixes "Entity already exists" error when creating users via UI with SSO enabled Co-authored-by: mohityadav766 <[email protected]>
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
1 similar comment
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
🔍 CI failure analysis for 2f33a36: 11 Playwright tests failed, but all failures are unrelated to this PR's user creation changes - likely pre-existing issues from main branch merge.IssueThe Root CauseThe test failures are NOT related to this PR's changes. This PR only modifies user creation UI logic in two files:
However, the failing tests are testing completely different features: Failed Tests (11 total):
DetailsNone of these tests touch user creation functionality. They test:
Context: This PR was recently merged with the main branch (commit 2f33a36), which brought in 200+ file changes including:
These test failures are likely:
Previous Analysis ContextEarlier, 9 CI jobs failed due to missing 'safe to test' label. That label was since added, allowing tests to run. Now we're seeing actual test execution, but the failures are in unrelated test suites. Code Review ✅ ApprovedClean, well-targeted bug fix that correctly conditionalizes password field inclusion based on auth provider. Test coverage for both Basic and SSO scenarios is appropriate. What Works WellGood use of conditional spread pattern to cleanly handle the auth provider check. Test cases properly cover both Basic auth and SSO scenarios using appropriate mocking strategies. Tip Comment OptionsAuto-apply is off Gitar will not commit updates to this branch. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | This comment will update automatically (Docs) |
|



Describe your changes:
User creation via UI fails with 409 "Entity already exists" when SSO is enabled because the UI unconditionally sends password-related fields (
password,confirmPassword,createPasswordType) regardless of authentication provider. These fields are only valid for Basic/LDAP auth where OpenMetadata manages passwords directly.Changes:
CreateUser.component.tsx: Conditionally include password fields only when
isAuthProviderBasicis true (Basic or LDAP providers)CreateUser.test.tsx: Added test coverage for both Basic auth (password fields rendered) and SSO auth (password fields excluded)
Impact:
Type of change:
Checklist:
Fixes <issue-number>: <short explanation>Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.