Skip to content

Conversation

@DaanHoogland
Copy link
Contributor

Description

This PR implements the reporters suggestion as requested...

Fixes: #12612

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

@codecov
Copy link

codecov bot commented Feb 11, 2026

Codecov Report

❌ Patch coverage is 5.00000% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 16.25%. Comparing base (34f6f41) to head (9f0443a).

Files with missing lines Patch % Lines
...c/main/java/com/cloud/user/dao/AccountDaoImpl.java 0.00% 19 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               4.20   #12632   +/-   ##
=========================================
  Coverage     16.25%   16.25%           
- Complexity    13421    13423    +2     
=========================================
  Files          5662     5662           
  Lines        500141   500141           
  Branches      60728    60728           
=========================================
+ Hits          81299    81310   +11     
+ Misses       409759   409747   -12     
- Partials       9083     9084    +1     
Flag Coverage Δ
uitests 4.15% <ø> (ø)
unittests 17.11% <5.00%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses issue #12612 where API-key authentication could yield a different (random) user UUID per request, by ensuring the user’s UUID is read from the database and propagated into the User object used in auth/context.

Changes:

  • Update the FIND_USER_ACCOUNT_BY_API_KEY query and result mapping to include and set u.uuid.
  • Adjust UserVO(long id) initialization to delegate to the no-arg constructor.
  • Extend the com.cloud.user.User interface to include setUuid(String).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
engine/schema/src/main/java/com/cloud/user/dao/AccountDaoImpl.java Adds UUID to the API-key lookup query and maps it into the User instance.
engine/schema/src/main/java/com/cloud/user/UserVO.java Changes the UserVO(long id) constructor to call this() before setting id.
api/src/main/java/com/cloud/user/User.java Adds setUuid(String uuid) to the User interface.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

DaanHoogland and others added 2 commits February 11, 2026 18:23
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 40%)

See analysis details on SonarQube Cloud

Copy link
Collaborator

@abh1sar abh1sar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code LGTM

Copy link
Contributor

@sureshanaparti sureshanaparti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm

@weizhouapache
Copy link
Member

@DaanHoogland
can we use this method UserAccountDao.getUserByApiKey instead ?

@DaanHoogland
Copy link
Contributor Author

@DaanHoogland can we use this method UserAccountDao.getUserByApiKey instead ?

no doubt, but can you be more specific? Do you mean, bypass findUserAccountByApiKey altogether? Or call UserAccountDao.getUserByApiKey from within it. I am sure you mean to suggest a cleanup refactor, but that is as far as my understanding goes.

Note findUserAccountByApiKey is called in three places. Happy to make this PR more elaborate.

@weizhouapache
Copy link
Member

@DaanHoogland can we use this method UserAccountDao.getUserByApiKey instead ?

no doubt, but can you be more specific? Do you mean, bypass findUserAccountByApiKey altogether? Or call UserAccountDao.getUserByApiKey from within it. I am sure you mean to suggest a cleanup refactor, but that is as far as my understanding goes.

Note findUserAccountByApiKey is called in three places. Happy to make this PR more elaborate.

@DaanHoogland
I had a quick look, I think remove the method accountDao.findUserAccountByApiKey(apiKey), and use this method instead

--- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java
+++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java
@@ -3051,7 +3051,9 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
 
     @Override
     public Pair<User, Account> findUserByApiKey(String apiKey) {
-        return _accountDao.findUserAccountByApiKey(apiKey);
+        UserAccount userAccount = _userAccountDao.getUserByApiKey(apiKey);
+        // TODO: get User and Account from userAccount or from DB
+        return new Pair<>(user, account);
     }
 
     @Override

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AccountDaoImpl.findUserAccountByApiKey() generates random UUID instead of reading from database

4 participants