server: fix NPE on first LDAP login

[xujiantop-crypto]

Description

On a user's first LDAP login, getActiveUserAccount returns null before loginUser authenticates the LDAP user and creates the CloudStack user. After the successful login, the OAuth login-detail cleanup dereferences that stale null value and turns the successful login into an NPE.

Only remove the OAuth login detail when the user already existed before authentication. A newly created LDAP user cannot have that pre-existing detail, so its successful login response is now returned normally.

Fixes #14095

Testing

Added DefaultLoginAPIAuthenticatorCmdTest.testAuthenticateFirstLdapLoginWithoutExistingUser to reproduce the first-login flow and verify that OAuth-detail cleanup is skipped for a user that did not exist before LDAP authentication.

mvn -pl server -am -Dtest=DefaultLoginAPIAuthenticatorCmdTest -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false test

Result: 1 test passed; all 26 reactor modules succeeded; Checkstyle reported 0 violations.

[boring-cyborg]

Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
Here are some useful points:

• In case of a new feature add useful documentation (raise doc PR at https://github.com/apache/cloudstack-documentation)
• Be patient and persistent. It might take some time to get a review or get the final approval from the committers.
• Pay attention to the quality of your code, ensure tests are passing and your PR doesn't have conflicts.
• Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Issues, Mailing list and Slack.
• Be sure to read the CloudStack Coding Conventions.
  Apache CloudStack is a community-driven project and together we are making it better 🚀.
  In case of doubts contact the developers at:
  Mailing List: dev@cloudstack.apache.org (https://cloudstack.apache.org/mailing-lists.html)
  Slack: https://apachecloudstack.slack.com/

[sudo87]

clgtm

[sureshanaparti]

hi @xujiantop-crypto can you rebase/change the target branch to 4.20 if the fix is relevant there.

[sureshanaparti]

hi @xujiantop-crypto can you rebase/change the target branch to 4.20 if the fix is relevant there.

ignore this, the change is added in 4.23 (in this PR: #12294)

[sureshanaparti]

cgltm

[sureshanaparti]

@blueorangutan package

[blueorangutan]

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 19.80%. Comparing base (cddd2c6) to head (061f1c9).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...loud/api/auth/DefaultLoginAPIAuthenticatorCmd.java	0.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##               main   #14098   +/-   ##
=========================================
  Coverage     19.79%   19.80%           
- Complexity    20021    20023    +2     
=========================================
  Files          6371     6371           
  Lines        575954   575955    +1     
  Branches      70521    70522    +1     
=========================================
+ Hits         114035   114056   +21     
+ Misses       449483   449456   -27     
- Partials      12436    12443    +7     

Flag	Coverage Δ
uitests	3.52% <ø> (ø)
unittests	21.07% <0.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[xujiantop-crypto]

Thanks for the clarification. I will keep the target branch as main. The package and CI checks are running now; I will keep an eye on the results.

[blueorangutan]

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19178