From b7d3a8c4231a3c0a8c779dc7e678f92052026d2a Mon Sep 17 00:00:00 2001 From: "ai-fixes[bot]" <321365310+ai-fixes[bot]@users.noreply.github.com> Date: Wed, 21 Oct 2026 13:04:57 +0000 Subject: [PATCH] CS-3498: Allow project admin to update template/ISO permissions on project-owned templates Project-owned templates can only have their permissions (ispublic, isextractable, launch permissions) changed by a CloudStack admin or by an account whose id equals the template's owner account id. For a project-owned template the owner is the project's pseudo-account, which no human account id can ever equal, so every project member - including the Project Admin who originally registered the template - was rejected by updateTemplateOrIsoPermissions(). Fix: compute whether the caller is the Project Admin of the owning project via the existing ProjectManager.canModifyProjectAccount() check (the same check DomainChecker already uses for AccessType.ModifyProject) and let that satisfy both ownership checks in TemplateManagerImpl.updateTemplateOrIsoPermissions(), alongside the existing owner/admin checks. Added TemplateManagerImplTest#testUpdateTemplatePermissionsAllowsProjectAdminToActAsOwner, which fails with the old code (InvalidParameterValueException) and passes with the fix. Fixes CS-3498 / apache/cloudstack#14174 --- .../cloud/template/TemplateManagerImpl.java | 7 +-- .../template/TemplateManagerImplTest.java | 44 +++++++++++++++++++ 2 files changed, 48 insertions(+), 3 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..fe98fe1b6391 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -1627,6 +1627,7 @@ public boolean updateTemplateOrIsoPermissions(BaseUpdateTemplateOrIsoPermissions // check configuration parameter(allow.public.user.templates) value for // the template owner boolean isAdmin = _accountMgr.isAdmin(caller.getId()); + boolean isProjectTemplateOwnerAdmin = owner.getType() == Account.Type.PROJECT && _projectMgr.canModifyProjectAccount(caller, ownerId); boolean allowPublicUserTemplates = AllowPublicUserTemplates.valueIn(template.getAccountId()); if (!isAdmin && !allowPublicUserTemplates && isPublic != null && isPublic) { throw new InvalidParameterValueException("Only private " + mediaType + "s can be created."); @@ -1640,8 +1641,8 @@ public boolean updateTemplateOrIsoPermissions(BaseUpdateTemplateOrIsoPermissions } } - //Only admin or owner of the template should be able to change its permissions - if (caller.getId() != ownerId && !isAdmin) { + //Only admin, owner, or the project admin of a project-owned template should be able to change its permissions + if (caller.getId() != ownerId && !isAdmin && !isProjectTemplateOwnerAdmin) { throw new InvalidParameterValueException("Unable to grant permission to Account " + caller.getAccountName() + " as it is neither admin nor owner or the Template"); } @@ -1657,7 +1658,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) && caller.getId() != ownerId && !isAdmin && !isProjectTemplateOwnerAdmin) { 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..508d321bea9f 100755 --- a/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java +++ b/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java @@ -19,11 +19,14 @@ package com.cloud.template; +import com.cloud.domain.DomainVO; +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 +69,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; @@ -177,6 +181,12 @@ public class TemplateManagerImplTest extends TestCase { @Mock AccountManager _accountMgr; + @Mock + ProjectManager _projectMgr; + + @Mock + DomainDao _domainDao; + @Mock VnfTemplateManager vnfTemplateManager; @@ -763,6 +773,40 @@ public void verifyHeuristicRulesForZoneTestTemplateIsISOFormatShouldCheckForISOH Mockito.verify(heuristicRuleHelperMock, Mockito.times(1)).getImageStoreIfThereIsHeuristicRule(1L, HeuristicType.ISO, vmTemplateVOMock); } + @Test + public void testUpdateTemplatePermissionsAllowsProjectAdminToActAsOwner() { + UpdateTemplatePermissionsCmd cmd = mock(UpdateTemplatePermissionsCmd.class); + when(cmd.getId()).thenReturn(10L); + when(cmd.getAccountNames()).thenReturn(null); + when(cmd.getProjectIds()).thenReturn(null); + when(cmd.isFeatured()).thenReturn(null); + when(cmd.isPublic()).thenReturn(null); + when(cmd.isExtractable()).thenReturn(false); + when(cmd.getOperation()).thenReturn(null); + + VMTemplateVO template = mock(VMTemplateVO.class); + when(template.getId()).thenReturn(10L); + when(template.getFormat()).thenReturn(Storage.ImageFormat.QCOW2); + when(template.getRemoved()).thenReturn(null); + when(template.getAccountId()).thenReturn(5L); + when(vmTemplateDao.findById(10L)).thenReturn(template); + when(vmTemplateDao.createForUpdate()).thenReturn(mock(VMTemplateVO.class)); + + Account projectOwnerAccount = mock(Account.class); + when(projectOwnerAccount.getType()).thenReturn(Account.Type.PROJECT); + when(projectOwnerAccount.getDomainId()).thenReturn(1L); + when(_accountMgr.getAccount(5L)).thenReturn(projectOwnerAccount); + when(_domainDao.findById(1L)).thenReturn(mock(DomainVO.class)); + + // caller is a project member who is not the template's owner account (the project) and not a + // CloudStack admin, but is the Project Admin of the project that owns the template + when(_projectMgr.canModifyProjectAccount(any(Account.class), eq(5L))).thenReturn(true); + + boolean result = templateManager.updateTemplateOrIsoPermissions(cmd); + + Assert.assertTrue("Project admin should be allowed to update permissions on a project-owned template", result); + } + @Test public void verifyHeuristicRulesForZoneTestTemplateNotISOFormatShouldCheckForTemplateHeuristicType() { VMTemplateVO vmTemplateVOMock = Mockito.mock(VMTemplateVO.class);