Skip to content

Commit 3ccd65d

Browse files
committed
changes
Signed-off-by: Abhishek Kumar <[email protected]>
1 parent de8a1c2 commit 3ccd65d

File tree

2 files changed

+13
-10
lines changed

2 files changed

+13
-10
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1291,7 +1291,7 @@ public Pair<Long, Account> doInTransaction(TransactionStatus status) {
12911291
Role change should follow the below conditions:
12921292
- Caller should not be of Unknown role type
12931293
- New role's type should not be Unknown
1294-
- Caller should not be able to escalate or de-escalate an account's role which is of same or higher role type
1294+
- Caller should not be able to escalate or de-escalate an account's role which is of higher role type
12951295
- New role should not be of type Admin with domain other than ROOT domain
12961296
*/
12971297
protected void validateRoleChange(Account account, Role role, Account caller) {
@@ -1305,10 +1305,10 @@ protected void validateRoleChange(Account account, Role role, Account caller) {
13051305
throw new PermissionDeniedException(String.format("%s as the new role privileges are unknown", errorMsg));
13061306
}
13071307
if (!callerRole.getRoleType().equals(RoleType.Admin) &&
1308-
(role.getRoleType().ordinal() <= callerRole.getRoleType().ordinal() ||
1309-
currentRole.getRoleType().ordinal() <= callerRole.getRoleType().ordinal())) { // Same type caller
1308+
(role.getRoleType().ordinal() < callerRole.getRoleType().ordinal() ||
1309+
currentRole.getRoleType().ordinal() < callerRole.getRoleType().ordinal())) {
13101310
throw new PermissionDeniedException(String.format("%s as either current or new role has higher " +
1311-
"or same privileges than the caller", errorMsg));
1311+
"privileges than the caller", errorMsg));
13121312
}
13131313
if (role.getRoleType().equals(RoleType.Admin) && account.getDomainId() != Domain.ROOT_DOMAIN) {
13141314
throw new PermissionDeniedException(String.format("%s as the user does not belong to the ROOT domain",

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,10 +1021,13 @@ public void testValidateRoleChangeUnknownNewRole() {
10211021
accountManagerImpl.validateRoleChange(account, newRole, caller);
10221022
}
10231023

1024-
@Test(expected = PermissionDeniedException.class)
1025-
public void testValidateRoleNewRoleSame() {
1024+
@Test
1025+
public void testValidateRoleNewRoleSameCaller() {
10261026
Account account = Mockito.mock(Account.class);
10271027
Mockito.when(account.getRoleId()).thenReturn(1L);
1028+
Role currentRole = Mockito.mock(Role.class);
1029+
Mockito.when(currentRole.getRoleType()).thenReturn(RoleType.User);
1030+
Mockito.when(roleService.findRole(1L)).thenReturn(currentRole);
10281031
Role newRole = Mockito.mock(Role.class);
10291032
Mockito.when(newRole.getRoleType()).thenReturn(RoleType.DomainAdmin);
10301033
Role callerRole = Mockito.mock(Role.class);
@@ -1035,8 +1038,8 @@ public void testValidateRoleNewRoleSame() {
10351038
accountManagerImpl.validateRoleChange(account, newRole, caller);
10361039
}
10371040

1038-
@Test(expected = PermissionDeniedException.class)
1039-
public void testValidateRoleCurrentRoleSame() {
1041+
@Test
1042+
public void testValidateRoleCurrentRoleSameCaller() {
10401043
Account account = Mockito.mock(Account.class);
10411044
Mockito.when(account.getRoleId()).thenReturn(1L);
10421045
Role accountRole = Mockito.mock(Role.class);
@@ -1053,7 +1056,7 @@ public void testValidateRoleCurrentRoleSame() {
10531056
}
10541057

10551058
@Test(expected = PermissionDeniedException.class)
1056-
public void testValidateRoleNewRoleHigher() {
1059+
public void testValidateRoleNewRoleHigherCaller() {
10571060
Account account = Mockito.mock(Account.class);
10581061
Mockito.when(account.getRoleId()).thenReturn(1L);
10591062
Role newRole = Mockito.mock(Role.class);
@@ -1067,7 +1070,7 @@ public void testValidateRoleNewRoleHigher() {
10671070
}
10681071

10691072
@Test
1070-
public void testValidateRoleNewRoleLower() {
1073+
public void testValidateRoleNewRoleLowerCaller() {
10711074
Account account = Mockito.mock(Account.class);
10721075
Mockito.when(account.getRoleId()).thenReturn(1L);
10731076
Role newRole = Mockito.mock(Role.class);

0 commit comments

Comments
 (0)