From e7a4542a3285fdbc5f605c3337b1a6d7e997bdc8 Mon Sep 17 00:00:00 2001 From: HawkForge <75696486+HawkForge@users.noreply.github.com> Date: Fri, 16 Oct 2026 23:37:47 +0600 Subject: [PATCH] server: allow project admins to update permissions of project templates (#14174) updateTemplateOrIsoPermissions() only treated the caller as owner when the caller's account id matched the template owner. For project owned templates/ISOs the owner is the project account, so project admins were rejected even though they can set the same flags at registration. Treat callers that can modify the project account (project admins) as owners. Fixes: #14174 --- .../cloud/template/TemplateManagerImpl.java | 10 ++- .../template/TemplateManagerImplTest.java | 79 +++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index d88bbd09bc24..06392e8f7289 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -1640,8 +1640,14 @@ public boolean updateTemplateOrIsoPermissions(BaseUpdateTemplateOrIsoPermissions } } + // For project owned templates/isos, the project admins are treated as owners + boolean isOwner = caller.getId() == ownerId; + if (!isOwner && !isAdmin && owner.getType() == Account.Type.PROJECT) { + isOwner = _projectMgr.canModifyProjectAccount(caller, ownerId); + } + //Only admin or owner of the template should be able to change its permissions - if (caller.getId() != ownerId && !isAdmin) { + if (!isOwner && !isAdmin) { throw new InvalidParameterValueException("Unable to grant permission to Account " + caller.getAccountName() + " as it is neither admin nor owner or the Template"); } @@ -1657,7 +1663,7 @@ public boolean updateTemplateOrIsoPermissions(BaseUpdateTemplateOrIsoPermissions if (isExtractable != null) { // Only Root admins and owners are allowed to change it for templates - if (!template.getFormat().equals(ImageFormat.ISO) && caller.getId() != ownerId && !isAdmin) { + if (!template.getFormat().equals(ImageFormat.ISO) && !isOwner && !isAdmin) { throw new InvalidParameterValueException("Only ROOT admins and Template owners are allowed to modify isExtractable attribute."); } else { // For Isos normal user can change it, as their are no derivatives. diff --git a/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java b/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java index 8ffa031f28f4..f26090ef941e 100755 --- a/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java +++ b/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java @@ -19,11 +19,13 @@ package com.cloud.template; +import com.cloud.domain.dao.DomainDao; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.ResourceAllocationException; import com.cloud.host.Status; import com.cloud.hypervisor.Hypervisor; import com.cloud.hypervisor.HypervisorGuruManager; +import com.cloud.projects.ProjectManager; import com.cloud.resourcelimit.CheckedReservation; import com.cloud.storage.DataStoreRole; import com.cloud.storage.GuestOSVO; @@ -66,6 +68,7 @@ import org.apache.cloudstack.api.command.user.template.RegisterTemplateCmd; import org.apache.cloudstack.api.command.user.template.RegisterVnfTemplateCmd; import org.apache.cloudstack.api.command.user.template.UpdateTemplateCmd; +import org.apache.cloudstack.api.command.user.template.UpdateTemplatePermissionsCmd; import org.apache.cloudstack.api.command.user.template.UpdateVnfTemplateCmd; import org.apache.cloudstack.api.command.user.userdata.LinkUserDataToTemplateCmd; import org.apache.cloudstack.context.CallContext; @@ -189,6 +192,12 @@ public class TemplateManagerImplTest extends TestCase { @Mock private UserDataVO userDataMock; + @Mock + ProjectManager projectManagerMock; + + @Mock + DomainDao domainDaoMock; + public class CustomThreadPoolExecutor extends ThreadPoolExecutor { AtomicInteger ai = new AtomicInteger(0); public CustomThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long keepAliveTime, TimeUnit unit, @@ -773,4 +782,74 @@ public void verifyHeuristicRulesForZoneTestTemplateNotISOFormatShouldCheckForTem Mockito.verify(heuristicRuleHelperMock, Mockito.times(1)).getImageStoreIfThereIsHeuristicRule(1L, HeuristicType.TEMPLATE, vmTemplateVOMock); } + private Account mockUpdateTemplatePermissionsCall(Account.Type ownerType, long callerId, boolean callerIsAdmin) { + Account caller = Mockito.mock(Account.class); + when(caller.getId()).thenReturn(callerId); + CallContext.unregister(); + CallContext.register(Mockito.mock(User.class), caller); + + Account owner = Mockito.mock(Account.class); + when(owner.getType()).thenReturn(ownerType); + when(_accountMgr.getAccount(2L)).thenReturn(owner); + when(_accountMgr.isAdmin(callerId)).thenReturn(callerIsAdmin); + + VMTemplateVO template = Mockito.mock(VMTemplateVO.class); + when(template.getAccountId()).thenReturn(2L); + when(template.getFormat()).thenReturn(Storage.ImageFormat.QCOW2); + when(vmTemplateDao.findById(10L)).thenReturn(template); + return caller; + } + + private UpdateTemplatePermissionsCmd mockUpdateTemplatePermissionsCmd() { + UpdateTemplatePermissionsCmd cmd = Mockito.mock(UpdateTemplatePermissionsCmd.class); + when(cmd.getId()).thenReturn(10L); + when(cmd.getAccountNames()).thenReturn(null); + when(cmd.getProjectIds()).thenReturn(null); + when(cmd.isPublic()).thenReturn(true); + when(cmd.isExtractable()).thenReturn(true); + return cmd; + } + + @Test + public void updateTemplateOrIsoPermissionsTestProjectAdminCanUpdateProjectTemplate() { + Account caller = mockUpdateTemplatePermissionsCall(Account.Type.PROJECT, 5L, false); + when(projectManagerMock.canModifyProjectAccount(caller, 2L)).thenReturn(true); + when(vmTemplateDao.createForUpdate()).thenReturn(Mockito.mock(VMTemplateVO.class)); + + Assert.assertTrue(templateManager.updateTemplateOrIsoPermissions(mockUpdateTemplatePermissionsCmd())); + Mockito.verify(vmTemplateDao, Mockito.times(1)).update(Mockito.eq(0L), Mockito.any(VMTemplateVO.class)); + } + + @Test(expected = InvalidParameterValueException.class) + public void updateTemplateOrIsoPermissionsTestProjectMemberCannotUpdateProjectTemplate() { + Account caller = mockUpdateTemplatePermissionsCall(Account.Type.PROJECT, 5L, false); + when(projectManagerMock.canModifyProjectAccount(caller, 2L)).thenReturn(false); + + templateManager.updateTemplateOrIsoPermissions(mockUpdateTemplatePermissionsCmd()); + } + + @Test + public void updateTemplateOrIsoPermissionsTestOwnerCanUpdateTemplate() { + mockUpdateTemplatePermissionsCall(Account.Type.NORMAL, 2L, false); + when(vmTemplateDao.createForUpdate()).thenReturn(Mockito.mock(VMTemplateVO.class)); + + Assert.assertTrue(templateManager.updateTemplateOrIsoPermissions(mockUpdateTemplatePermissionsCmd())); + Mockito.verify(projectManagerMock, Mockito.never()).canModifyProjectAccount(Mockito.any(Account.class), Mockito.anyLong()); + } + + @Test + public void updateTemplateOrIsoPermissionsTestAdminCanUpdateProjectTemplate() { + mockUpdateTemplatePermissionsCall(Account.Type.PROJECT, 5L, true); + when(vmTemplateDao.createForUpdate()).thenReturn(Mockito.mock(VMTemplateVO.class)); + + Assert.assertTrue(templateManager.updateTemplateOrIsoPermissions(mockUpdateTemplatePermissionsCmd())); + Mockito.verify(projectManagerMock, Mockito.never()).canModifyProjectAccount(Mockito.any(Account.class), Mockito.anyLong()); + } + + @Test(expected = InvalidParameterValueException.class) + public void updateTemplateOrIsoPermissionsTestNonOwnerCannotUpdateTemplate() { + mockUpdateTemplatePermissionsCall(Account.Type.NORMAL, 5L, false); + + templateManager.updateTemplateOrIsoPermissions(mockUpdateTemplatePermissionsCmd()); + } }