Skip to content

Commit 00dc78e

Browse files
fix update user workflow
1 parent 96eacbc commit 00dc78e

File tree

2 files changed

+31
-25
lines changed

2 files changed

+31
-25
lines changed

server/src/main/java/com/cloud/user/AccountManagerImpl.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1937,7 +1937,7 @@ protected Account getCurrentCallingAccount() {
19371937
}
19381938

19391939
/**
1940-
* Validates user API and Secret keys. If a new pair of keys is provided, we update them in the user POJO.
1940+
* Validates user API and Secret keys. If a new pair of keys is provided, we update them in the key pair POJO.
19411941
* <ul>
19421942
* <li>When updating the keys, it must be provided a pair (API and Secret keys); otherwise, an {@link InvalidParameterValueException} is thrown.
19431943
* <li>If a pair of keys is provided, we validate to see if there is an user already using the provided API key. If there is someone else using, we throw an {@link InvalidParameterValueException} because two users cannot have the same API key.
@@ -1950,25 +1950,25 @@ protected void validateAndUpdateApiAndSecretKeyIfNeeded(UpdateUserCmd updateUser
19501950
boolean isApiKeyBlank = StringUtils.isBlank(apiKey);
19511951
boolean isSecretKeyBlank = StringUtils.isBlank(secretKey);
19521952
if (isApiKeyBlank ^ isSecretKeyBlank) {
1953-
throw new InvalidParameterValueException("Please provide a userApiKey/userSecretKey pair");
1953+
throw new InvalidParameterValueException("Please provide a valid API and secret key pair.");
19541954
}
19551955
if (isApiKeyBlank && isSecretKeyBlank) {
19561956
return;
19571957
}
1958-
Ternary<User, Account, ApiKeyPair> keyPairTernary = findUserByApiKey(apiKey);
1959-
if (keyPairTernary == null) {
1960-
throw new InvalidParameterValueException(String.format("The API key [%s] does not exist in the system. Please provide a valid key.", apiKey));
1958+
1959+
ApiKeyPairVO lastUserKeyPair = apiKeyPairDao.getLastApiKeyCreatedByUser(user.getId());
1960+
if (lastUserKeyPair == null) {
1961+
throw new InvalidParameterValueException(String.format("User [%s] has no active API key pairs to be updated.", user.getUsername()));
19611962
}
19621963

1963-
User userThatHasTheProvidedApiKey = keyPairTernary.first();
1964-
if (userThatHasTheProvidedApiKey.getId() != user.getId()) {
1964+
Ternary<User, Account, ApiKeyPair> keyPairTernary = findUserByApiKey(apiKey);
1965+
if (keyPairTernary != null) {
19651966
throw new InvalidParameterValueException(String.format("The API key [%s] already exists in the system. Please provide a unique key.", apiKey));
19661967
}
19671968

1968-
ApiKeyPairVO keyPair = (ApiKeyPairVO) keyPairTernary.third();
1969-
keyPair.setApiKey(apiKey);
1970-
keyPair.setSecretKey(secretKey);
1971-
apiKeyPairDao.update(keyPair.getId(), keyPair);
1969+
lastUserKeyPair.setApiKey(apiKey);
1970+
lastUserKeyPair.setSecretKey(secretKey);
1971+
apiKeyPairDao.update(lastUserKeyPair.getId(), lastUserKeyPair);
19721972
}
19731973

19741974
protected void validateAndUpdateUserApiKeyAccess(UpdateUserCmd updateUserCmd, UserVO user) {

server/src/test/java/com/cloud/user/AccountManagerImplTest.java

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,6 @@ private void prepareMockAndExecuteUpdateUserTest(int numberOfExpectedCallsForSet
513513
Mockito.doReturn("password").when(UpdateUserCmdMock).getPassword();
514514
Mockito.doReturn("newpassword").when(UpdateUserCmdMock).getCurrentPassword();
515515
Mockito.doReturn(userVoMock).when(accountManagerImpl).retrieveAndValidateUser(UpdateUserCmdMock);
516-
Mockito.doReturn(apiKeyPairVOMock).when(accountManagerImpl).validateAndUpdateApiAndSecretKeyIfNeeded(UpdateUserCmdMock, userVoMock);
517516
Mockito.doReturn(accountMock).when(accountManagerImpl).retrieveAndValidateAccount(userVoMock);
518517

519518
Mockito.doNothing().when(accountManagerImpl).validateAndUpdateFirstNameIfNeeded(UpdateUserCmdMock, userVoMock);
@@ -564,33 +563,35 @@ public void retrieveAndValidateUserTestUserIsFound() {
564563
}
565564

566565
@Test
567-
public void validateAndUpdatApiAndSecretKeyIfNeededTestNoKeys() {
566+
public void validateAndUpdateApiAndSecretKeyIfNeededTestNoKeys() {
568567
accountManagerImpl.validateAndUpdateApiAndSecretKeyIfNeeded(UpdateUserCmdMock, userVoMock);
569568

569+
Mockito.verify(apiKeyPairDaoMock, Mockito.times(0)).getLastApiKeyCreatedByUser(Mockito.anyLong());
570570
Mockito.verify(accountManagerImpl, Mockito.times(0)).findUserByApiKey(Mockito.anyString());
571571
}
572572

573573
@Test(expected = InvalidParameterValueException.class)
574-
public void validateAndUpdatApiAndSecretKeyIfNeededTestOnlyApiKeyInformed() {
574+
public void validateAndUpdateApiAndSecretKeyIfNeededTestOnlyApiKeyInformed() {
575575
Mockito.doReturn("apiKey").when(UpdateUserCmdMock).getApiKey();
576576

577577
accountManagerImpl.validateAndUpdateApiAndSecretKeyIfNeeded(UpdateUserCmdMock, userVoMock);
578578
}
579579

580580
@Test(expected = InvalidParameterValueException.class)
581-
public void validateAndUpdatApiAndSecretKeyIfNeededTestOnlySecretKeyInformed() {
581+
public void validateAndUpdateApiAndSecretKeyIfNeededTestOnlySecretKeyInformed() {
582582
Mockito.doReturn("secretKey").when(UpdateUserCmdMock).getSecretKey();
583583

584584
accountManagerImpl.validateAndUpdateApiAndSecretKeyIfNeeded(UpdateUserCmdMock, userVoMock);
585585
}
586586

587587
@Test(expected = InvalidParameterValueException.class)
588-
public void validateAndUpdatApiAndSecretKeyIfNeededTestApiKeyAlreadyUsedBySomeoneElse() {
588+
public void validateAndUpdateApiAndSecretKeyIfNeededTestApiKeyAlreadyUsedBySomeoneElse() {
589589
String apiKey = "apiKey";
590590
Mockito.doReturn(apiKey).when(UpdateUserCmdMock).getApiKey();
591591
Mockito.doReturn("secretKey").when(UpdateUserCmdMock).getSecretKey();
592592

593593
Mockito.doReturn(1L).when(userVoMock).getId();
594+
Mockito.doReturn(apiKeyPairVOMock).when(apiKeyPairDaoMock).getLastApiKeyCreatedByUser(1L);
594595

595596
User otherUserMock = Mockito.mock(User.class);
596597

@@ -600,6 +601,15 @@ public void validateAndUpdatApiAndSecretKeyIfNeededTestApiKeyAlreadyUsedBySomeon
600601
accountManagerImpl.validateAndUpdateApiAndSecretKeyIfNeeded(UpdateUserCmdMock, userVoMock);
601602
}
602603

604+
@Test(expected = InvalidParameterValueException.class)
605+
public void validateAndUpdateApiAndSecretKeyIfNeededTestUserHasNoActiveApiKeyPair() {
606+
String apiKey = "apiKey";
607+
Mockito.doReturn(apiKey).when(UpdateUserCmdMock).getApiKey();
608+
Mockito.doReturn("secretKey").when(UpdateUserCmdMock).getSecretKey();
609+
Mockito.doReturn(1L).when(userVoMock).getId();
610+
accountManagerImpl.validateAndUpdateApiAndSecretKeyIfNeeded(UpdateUserCmdMock, userVoMock);
611+
}
612+
603613
@Test
604614
public void validateAndUpdateApiAndSecretKeyIfNeededTest() {
605615
String apiKey = "apiKey";
@@ -608,12 +618,8 @@ public void validateAndUpdateApiAndSecretKeyIfNeededTest() {
608618
String secretKey = "secretKey";
609619
Mockito.doReturn(secretKey).when(UpdateUserCmdMock).getSecretKey();
610620

611-
User otherUserMock = Mockito.mock(User.class);
612-
Mockito.doReturn(userVoIdMock).when(otherUserMock).getId();
613-
614-
Ternary<User, Account, ApiKeyPair> pairUserAccountMock = new Ternary<>(otherUserMock, Mockito.mock(Account.class), apiKeyPairVOMock);
615-
Mockito.doReturn(pairUserAccountMock).when(accountManagerImpl).findUserByApiKey(apiKey);
616-
Mockito.doReturn(accountVoMock).when(_accountDao).findById(Mockito.anyLong());
621+
Mockito.doReturn(1L).when(userVoMock).getId();
622+
Mockito.doReturn(apiKeyPairVOMock).when(apiKeyPairDaoMock).getLastApiKeyCreatedByUser(1L);
617623

618624
accountManagerImpl.validateAndUpdateApiAndSecretKeyIfNeeded(UpdateUserCmdMock, userVoMock);
619625

@@ -623,7 +629,7 @@ public void validateAndUpdateApiAndSecretKeyIfNeededTest() {
623629
}
624630

625631
@Test
626-
public void validateAndUpdatUserApiKeyAccess() {
632+
public void validateAndUpdateUserApiKeyAccess() {
627633
Mockito.doReturn("Enabled").when(UpdateUserCmdMock).getApiKeyAccess();
628634
try (MockedStatic<ActionEventUtils> eventUtils = Mockito.mockStatic(ActionEventUtils.class)) {
629635
Mockito.when(ActionEventUtils.onActionEvent(Mockito.anyLong(), Mockito.anyLong(),
@@ -643,7 +649,7 @@ public void validateAndUpdatUserApiKeyAccessInvalidParameter() {
643649
}
644650

645651
@Test
646-
public void validateAndUpdatAccountApiKeyAccess() {
652+
public void validateAndUpdateAccountApiKeyAccess() {
647653
Mockito.doReturn("Inherit").when(UpdateAccountCmdMock).getApiKeyAccess();
648654
try (MockedStatic<ActionEventUtils> eventUtils = Mockito.mockStatic(ActionEventUtils.class)) {
649655
Mockito.when(ActionEventUtils.onActionEvent(Mockito.anyLong(), Mockito.anyLong(),
@@ -657,7 +663,7 @@ public void validateAndUpdatAccountApiKeyAccess() {
657663
}
658664

659665
@Test(expected = InvalidParameterValueException.class)
660-
public void validateAndUpdatAccountApiKeyAccessInvalidParameter() {
666+
public void validateAndUpdateAccountApiKeyAccessInvalidParameter() {
661667
Mockito.doReturn("False").when(UpdateAccountCmdMock).getApiKeyAccess();
662668
accountManagerImpl.validateAndUpdateAccountApiKeyAccess(UpdateAccountCmdMock, accountVoMock);
663669
}

0 commit comments

Comments
 (0)