fix(ldap): load full user fields on login so teams are not wiped

[joaopamaral]

Summary

LdapAuthenticator.checkAndCreateUser fetched the existing OpenMetadata user with only "id,name,email,roles". The subsequent PUT through UserUtil.addOrUpdateUser -> userRepository.createOrUpdate then ran UserUpdater.entitySpecificUpdate, which unconditionally invokes updateTeams, updatePersonas, etc.

Because the in-memory user had teams = null, updateTeams executed:

deleteTo(original.getId(), USER, Relationship.HAS, Entity.TEAM);
assignTeams(updated, updated.getTeams()); // updated.getTeams() == null

This wiped every manually-assigned team on every LDAP login. The same path wiped personas, defaultPersona, profile, domains, personaPreferences, authenticationMechanism, and isEmailVerified. The deleteTo against a user with many teams also made login visibly slow.

This PR switches the fetch to userRepository.getFieldsWithUserAuth("*") — the same call BasicAuthenticator already uses — so the PUT sees the user's full state and the updater preserves it.

Reproducer

1. Configure LDAP auth.
2. Sign in once over LDAP to materialize the user in OpenMetadata.
3. As admin, manually assign the user to N teams in OM (these are pure-OM teams with no LDAP group/DN/role-mapping backing).
4. Sign the user out and sign back in over LDAP.
5. Observed: the user is now in Organization only — every manually-assigned team was removed. Login latency scales with N (the number of pre-existing team relationships).

After this fix, all manually-assigned teams persist across LDAP logins and login latency no longer scales with team count.

Why this is the right fix

• BasicAuthenticator already loads the user with getFieldsWithUserAuth("*"). The LDAP path silently diverged.
• UserRepository.USER_UPDATE_FIELDS and UserUpdater.entitySpecificUpdate are the source of truth for which fields need to be present on a PUT. Loading them all is the safe, no-special-case fix.
• No protocol or contract change. The fix is one line plus a comment.

Test plan

The original LdapAuthCompleteFlowTest integration test class was deleted from main in #26204 ("Delete Old Integration tests and fix sonar workflow"), so this PR does not add or modify a test in tree.

The fix was verified locally against a 1.12.7 backport branch using a resurrected version of LdapAuthCompleteFlowTest with a new testLoginPreservesManuallyAssignedTeams regression case:

1. Bring up the test class' osixia/openldap:1.5.0 testcontainer and switch OM auth to LDAP.
2. Log the LDAP test user in once so the OM user row is created.
3. Create three pure-OM teams (no LDAP group / DN / role-mapping backing) via TeamRepository.create.
4. Assign all three teams to the LDAP user via UserRepository.createOrUpdate.
5. Trigger a second LDAP login.
6. Assert the user still belongs to all three teams.

Before the fix: the assertion fails — the user ends up in Organization only.

LDAP login wiped the user's manual team assignments.
Expected: [om-manual-team-1-..., om-manual-team-2-..., om-manual-team-3-...]
Got: [Organization]

After the fix: the assertion passes — the three manually-assigned teams persist across the LDAP login. All other tests in the existing flow (testSuccessfulLoginWithValidCredentials, testFailedLoginWithInvalidPassword, testFailedLoginWithNonExistentUser, testLogout, testAdminPrincipalsGrantsAdminPrivileges, testMultipleLoginAttempts) still pass.

I'm happy to add an equivalent test in whatever style the maintainers prefer (unit test with mocks, integration test, etc.) — just let me know.

🤖 Generated with Claude Code

[github-actions]

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

[github-actions]

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

[joaopamaral]

I've tested in openmetadata-service/src/test/java/org/openmetadata/service/security/auth/LdapAuthCompleteFlowTest.java in 1.12.7 but this test file was removed :(

[github-actions]

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

[Copilot]

Pull request overview

Fixes an LDAP login data-loss/performance bug in LdapAuthenticator.checkAndCreateUser caused by fetching an existing user with a sparse field set and then persisting it via UserUtil.addOrUpdateUser, which triggers UserUpdater relationship updates (teams/personas/domains/etc.) and can wipe relationships when those fields are null.

Changes:

• Fetch the existing user using a full-field selector (getFieldsWithUserAuth("*")) instead of only id,name,email,roles.
• Add an explanatory comment documenting why the full fetch is required to prevent relationship clobbering during the subsequent update.

[github-actions]

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

[gitar-bot]

Code Review ✅ Approved 4 resolved / 4 findings

Loads full user fields during LDAP authentication to ensure team and persona associations are preserved on login. No issues found.

✅ 4 resolved

✅ Edge Case: Shell quoting in execLdapAdd breaks if LDIF contains quotes

📄 openmetadata-service/src/test/java/org/openmetadata/service/security/auth/LdapAuthCompleteFlowTest.java:169-183
The execLdapAdd method embeds LDIF content inside single quotes in a bash -c command via string concatenation. If any LDIF value ever contains a single quote, the shell command will break or behave unexpectedly. While current data is safe, this is fragile. Use Transferable / copyFileToContainer to write the LDIF to a temporary file inside the container, then run ldapadd -f with that file.

✅ Quality: Method exceeds 15-line limit (testLoginPreservesManuallyAssignedTeams)

📄 openmetadata-service/src/test/java/org/openmetadata/service/security/auth/LdapAuthCompleteFlowTest.java:445-459
The testLoginPreservesManuallyAssignedTeams method (lines 445-557) is ~112 lines long, well beyond the 15-line method limit. Extract helper methods like loginUser(email, password), createTestTeams(suffix), and assignTeamsToUser(email, teams) to improve readability and keep each method focused.

✅ Quality: Thread.sleep used for synchronization instead of polling

📄 openmetadata-service/src/test/java/org/openmetadata/service/security/auth/LdapAuthCompleteFlowTest.java:72 📄 openmetadata-service/src/test/java/org/openmetadata/service/security/auth/LdapAuthCompleteFlowTest.java:191 📄 openmetadata-service/src/test/java/org/openmetadata/service/security/auth/LdapAuthCompleteFlowTest.java:269
The test uses Thread.sleep(10000) (line 72) and Thread.sleep(3000) (lines 191, 269) for waiting on the LDAP container and config propagation. These are fragile — too short on slow CI, too long otherwise. Consider using Testcontainers' waitingFor(Wait.forLogMessage(...)) for the container readiness and Awaitility or a polling loop for config propagation.

✅ Quality: Wildcard imports and inline fully-qualified names violate style rules

📄 openmetadata-service/src/test/java/org/openmetadata/service/security/auth/LdapAuthCompleteFlowTest.java:3 📄 openmetadata-service/src/test/java/org/openmetadata/service/security/auth/LdapAuthCompleteFlowTest.java:12 📄 openmetadata-service/src/test/java/org/openmetadata/service/security/auth/LdapAuthCompleteFlowTest.java:15 📄 openmetadata-service/src/test/java/org/openmetadata/service/security/auth/LdapAuthCompleteFlowTest.java:468-482
The file uses three wildcard imports (lines 3, 12, 15) and numerous inline fully-qualified class names (lines 468-519) instead of proper imports. The project's coding standards explicitly prohibit wildcard imports and fully-qualified names. The FQNs make the test method extremely hard to read. All classes used inline (TeamRepository, Team, TeamType, EntityReference, Entity) have no naming conflicts with existing imports and should be imported normally.

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact

gitar display:verbose

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