Skip to content

Fix NPE with ApiKeyPair during listApis call (from cmk) #13149

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.

Sign up for GitHub

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

Open
sureshanaparti wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix NPE with ApiKeyPair during listApis call (from cmk) #13149

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aa6f16b
Fix NPE with ApiKeyPair during listApis call (from cmk)
sureshanaparti May 12, 2026
17fca41
Return FALSE when no key pair for the api key
sureshanaparti Jul 8, 2026
5d8d7ef
addressed review comments
sureshanaparti Jul 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions server/src/main/java/com/cloud/user/AccountManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3202,7 +3202,7 @@ public Pair<Boolean, Map<String, String>> getKeys(GetUserKeysCmd cmd) {
ApiKeyPair keyPair;
if (accessingApiKey != null) {
ApiKeyPair accessingKeyPair = apiKeyPairService.findByApiKey(accessingApiKey);
if (userId == accessingKeyPair.getUserId()) {
if (accessingKeyPair != null && userId == accessingKeyPair.getUserId()) {

@weizhouapache weizhouapache Jul 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It might be better to throw an exception here, if apikeypair is not found
similar to change with getAllKeypairPermissions method

@bernardodemarco bernardodemarco Jul 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, it would be nice

keyPair = apiKeyPairService.findByApiKey(accessingApiKey);
Comment thread
sureshanaparti marked this conversation as resolved.
Comment on lines 3204 to 3206

@bernardodemarco bernardodemarco Jul 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As for this suggestion, we could apply it in this PR, if you're willing to, @sureshanaparti. But it is nitpicking IMO

} else {
keyPair = _accountService.getLatestUserKeyPair(userId);
Expand Down Expand Up @@ -3320,6 +3320,10 @@ private Boolean isAccessingKeypairSuperset(ApiKeyPair accessedKeyPair, BaseCmd c
return Boolean.TRUE;
}
ApiKeyPair accessingKeyPair = apiKeyPairService.findByApiKey(apiKey);
if (accessingKeyPair == null) {
logger.warn("Unable to find API key pair for the accessing API key.");
return Boolean.FALSE;
}
return isApiKeySupersetOfPermission(new ArrayList<>(getAllKeypairPermissions(accessingKeyPair.getApiKey())), new ArrayList<>(getAllKeypairPermissions(accessedKeyPair.getApiKey())));
}

Expand All @@ -3334,8 +3338,8 @@ public String getAccessingApiKey(BaseCmd cmd) {
if (accessedByApiKey) {
return accessingApiKey;
}
} catch (NullPointerException e) {
logger.info("Accessing API through session.");
} catch (Exception e) {
logger.info("Error accessing API through session.", e);
}
Comment on lines +3341 to 3343

@bernardodemarco bernardodemarco Jul 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This Copilot suggestion does not make sense in the context of this method

return null;
}
Expand Down Expand Up @@ -3582,6 +3586,10 @@ public List<RolePermissionEntity> getAllKeypairPermissions(String apiKey) {
throw new InvalidParameterValueException("API key not present in the request's URL and, thus, unable to fetch API key rules.");
}
ApiKeyPair apiKeyPair = keyPairManager.findByApiKey(apiKey);
if (apiKeyPair == null) {
logger.warn("Unable to find API key pair by API key.");

@weizhouapache weizhouapache Jul 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

throw an exception ?

return new ArrayList<>();
}
Comment on lines +3589 to +3592
Account account = _accountDao.findById(apiKeyPair.getAccountId());
List<ApiKeyPairPermission> keyPairPermissions = keyPairManager.findAllPermissionsByKeyPairId(apiKeyPair.getId(), account.getRoleId());
return new ArrayList<>(keyPairPermissions);
Expand Down Expand Up @@ -3848,12 +3856,11 @@ public void buildACLViewSearchCriteria(SearchCriteria<? extends ControlledViewEn
@Override
public UserAccount getUserByApiKey(String apiKey) {
ApiKeyPairVO keyPair = apiKeyPairDao.findByApiKey(apiKey);

if (keyPair == null) {
return null;
if (keyPair != null) {
return userAccountDao.findById(keyPair.getUserId());
}

return userAccountDao.findById(keyPair.getUserId());
return null;
}

@Override
Expand Down
Loading