From 64bbd0ea4f7fc215c732059c9754b63b42d0c664 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 30 Sep 2025 11:53:50 +0530 Subject: [PATCH 1/2] server,ui: prevent role change for default accounts Fixes #10931 Role for default accounts shouldn't be changed. Appropriate error should be returned by the server and UI should not present option for them. Signed-off-by: Abhishek Kumar --- .../java/com/cloud/user/AccountManagerImpl.java | 6 ++++++ .../com/cloud/user/AccountManagerImplTest.java | 16 ++++++++++++++++ ui/src/views/iam/EditAccount.vue | 6 ++++-- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 04a64fbfc8c9..5c9fe0497d1f 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -1398,6 +1398,9 @@ public Pair doInTransaction(TransactionStatus status) { - New role should not be of type Admin with domain other than ROOT domain */ protected void validateRoleChange(Account account, Role role, Account caller) { + if (account.getRoleId() == role.getId()) { + return; + } Role currentRole = roleService.findRole(account.getRoleId()); Role callerRole = roleService.findRole(caller.getRoleId()); String errorMsg = String.format("Unable to update account role to %s, ", role.getName()); @@ -1413,6 +1416,9 @@ protected void validateRoleChange(Account account, Role role, Account caller) { throw new PermissionDeniedException(String.format("%s as either current or new role has higher " + "privileges than the caller", errorMsg)); } + if (account.isDefault()) { + throw new PermissionDeniedException(String.format("%s as the account is a default account", errorMsg)); + } if (role.getRoleType().equals(RoleType.Admin) && account.getDomainId() != Domain.ROOT_DOMAIN) { throw new PermissionDeniedException(String.format("%s as the user does not belong to the ROOT domain", errorMsg)); diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java index 09a2d73080d6..2aeb43469d19 100644 --- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java +++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java @@ -1365,6 +1365,22 @@ public void testValidateRoleAdminCannotEscalateAdminFromNonRootDomain() { accountManagerImpl.validateRoleChange(account, newRole, caller); } + @Test(expected = PermissionDeniedException.class) + public void testValidateRoleAdminCannotChangeDefaultAdmin() { + Account account = Mockito.mock(Account.class); + Mockito.when(account.isDefault()).thenReturn(true); + Mockito.when(account.getRoleId()).thenReturn(1L); + Role newRole = Mockito.mock(Role.class); + Mockito.when(newRole.getRoleType()).thenReturn(RoleType.User); + Role callerRole = Mockito.mock(Role.class); + Mockito.when(callerRole.getRoleType()).thenReturn(RoleType.Admin); + Account caller = Mockito.mock(Account.class); + Mockito.when(caller.getRoleId()).thenReturn(2L); + Mockito.when(roleService.findRole(1L)).thenReturn(Mockito.mock(Role.class)); + Mockito.when(roleService.findRole(2L)).thenReturn(callerRole); + accountManagerImpl.validateRoleChange(account, newRole, caller); + } + @Test public void checkIfAccountManagesProjectsTestNotThrowExceptionWhenTheAccountIsNotAProjectAdministrator() { long accountId = 1L; diff --git a/ui/src/views/iam/EditAccount.vue b/ui/src/views/iam/EditAccount.vue index 7775bb2b8259..efaf9924a244 100644 --- a/ui/src/views/iam/EditAccount.vue +++ b/ui/src/views/iam/EditAccount.vue @@ -40,7 +40,7 @@ v-model:value="form.networkdomain" :placeholder="apiParams.networkdomain.description" /> - + @@ -145,11 +145,13 @@ export default { const params = { newname: values.newname, networkdomain: values.networkdomain, - roleid: values.roleid, apikeyaccess: values.apikeyaccess, account: this.account, domainid: this.domainId } + if (values.roleid) { + params.roleid = values.roleid + } if (this.isValidValueForKey(values, 'networkdomain') && values.networkdomain.length > 0) { params.networkdomain = values.networkdomain } From 56d80afc6ad6c50ce9abdaa0b15ea1c8bc779aad Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 1 Oct 2025 14:10:13 +0530 Subject: [PATCH 2/2] Update server/src/main/java/com/cloud/user/AccountManagerImpl.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- server/src/main/java/com/cloud/user/AccountManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 5c9fe0497d1f..db7eadc71669 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -1398,7 +1398,7 @@ public Pair doInTransaction(TransactionStatus status) { - New role should not be of type Admin with domain other than ROOT domain */ protected void validateRoleChange(Account account, Role role, Account caller) { - if (account.getRoleId() == role.getId()) { + if (account.getRoleId() != null && account.getRoleId().equals(role.getId())) { return; } Role currentRole = roleService.findRole(account.getRoleId());