Skip to content

Allow project admin to update template/ISO permissions in projects #14510

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
DaanHoogland wants to merge 1 commit into
base: 4.20
Choose a base branch
from
+48 −3
Open

Allow project admin to update template/ISO permissions in projects #14510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions server/src/main/java/com/cloud/template/TemplateManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand All @@ -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");
}

Expand All @@ -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.
Expand Down
44 changes: 44 additions & 0 deletions server/src/test/java/com/cloud/template/TemplateManagerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -177,6 +181,12 @@ public class TemplateManagerImplTest extends TestCase {
@Mock
AccountManager _accountMgr;

@Mock
ProjectManager _projectMgr;

@Mock
DomainDao _domainDao;

@Mock
VnfTemplateManager vnfTemplateManager;

Expand Down Expand Up @@ -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);
Expand Down
Loading