diff --git a/server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java b/server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java index 23e757dfe23..c855c9efa30 100644 --- a/server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java @@ -165,6 +165,22 @@ public class TaggedResourceManagerImpl extends ManagerBase implements TaggedReso return new Pair<>(accountId, domainId); } + protected void checkTagsDeletePermission(List tagsToDelete, Account caller) { + for (ResourceTag resourceTag : tagsToDelete) { + if(s_logger.isDebugEnabled()) { + s_logger.debug("Resource Tag Id: " + resourceTag.getResourceId()); + s_logger.debug("Resource Tag AccountId: " + resourceTag.getAccountId()); + } + if (caller.getAccountId() != resourceTag.getAccountId()) { + Account owner = _accountMgr.getAccount(resourceTag.getAccountId()); + if(s_logger.isDebugEnabled()) { + s_logger.debug("Resource Owner: " + owner); + } + _accountMgr.checkAccess(caller, null, false, owner); + } + } + } + @Override @DB @ActionEvent(eventType = EventTypes.EVENT_TAGS_CREATE, eventDescription = "creating resource tags") @@ -241,17 +257,6 @@ public class TaggedResourceManagerImpl extends ManagerBase implements TaggedReso // Finalize which tags should be removed for (ResourceTag resourceTag : resourceTags) { - //1) validate the permissions - if(s_logger.isDebugEnabled()) { - s_logger.debug("Resource Tag Id: " + resourceTag.getResourceId()); - s_logger.debug("Resource Tag AccountId: " + resourceTag.getAccountId()); - } - Account owner = _accountMgr.getAccount(resourceTag.getAccountId()); - if(s_logger.isDebugEnabled()) { - s_logger.debug("Resource Owner: " + owner); - } - _accountMgr.checkAccess(caller, null, false, owner); - //2) Only remove tag if it matches key value pairs if (MapUtils.isEmpty(tags)) { tagsToDelete.add(resourceTag); } else { @@ -278,6 +283,7 @@ public class TaggedResourceManagerImpl extends ManagerBase implements TaggedReso if (tagsToDelete.isEmpty()) { return false; } + checkTagsDeletePermission(tagsToDelete, caller); //Remove the tags Transaction.execute(new TransactionCallbackNoReturn() { diff --git a/server/src/test/java/com/cloud/tags/TaggedResourceManagerImplTest.java b/server/src/test/java/com/cloud/tags/TaggedResourceManagerImplTest.java index d3a76ff429f..6d5b314263c 100644 --- a/server/src/test/java/com/cloud/tags/TaggedResourceManagerImplTest.java +++ b/server/src/test/java/com/cloud/tags/TaggedResourceManagerImplTest.java @@ -19,24 +19,37 @@ package com.cloud.tags; -import com.cloud.server.ResourceTag; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; -import junit.framework.TestCase; + import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; +import com.cloud.exception.PermissionDeniedException; +import com.cloud.server.ResourceTag; +import com.cloud.user.Account; +import com.cloud.user.AccountManager; + +import junit.framework.TestCase; + @RunWith(MockitoJUnitRunner.class) -public class TaggedResourceManagerImplTest extends TestCase{ +public class TaggedResourceManagerImplTest extends TestCase { + + @Mock + AccountManager accountManager; + @Spy + @InjectMocks private final TaggedResourceManagerImpl taggedResourceManagerImplSpy = new TaggedResourceManagerImpl(); private final List listResourceObjectTypes = Arrays.asList(ResourceTag.ResourceObjectType.values()); @@ -84,4 +97,30 @@ public class TaggedResourceManagerImplTest extends TestCase{ Assert.assertEquals(expectedResult, result); }); } + + @Test + public void testCheckTagsDeletePermission() { + long accountId = 1L; + Account caller = Mockito.mock(Account.class); + Mockito.when(caller.getAccountId()).thenReturn(accountId); + ResourceTag resourceTag = Mockito.mock(ResourceTag.class); + Mockito.when(resourceTag.getAccountId()).thenReturn(accountId); + taggedResourceManagerImplSpy.checkTagsDeletePermission(List.of(resourceTag), caller); + } + + @Test(expected = PermissionDeniedException.class) + public void testCheckTagsDeletePermissionFail() { + long callerAccountId = 1L; + long ownerAccountId = 2L; + Account caller = Mockito.mock(Account.class); + Mockito.when(caller.getAccountId()).thenReturn(callerAccountId); + ResourceTag resourceTag1 = Mockito.mock(ResourceTag.class); + Mockito.when(resourceTag1.getAccountId()).thenReturn(callerAccountId); + ResourceTag resourceTag2 = Mockito.mock(ResourceTag.class); + Mockito.when(resourceTag2.getAccountId()).thenReturn(ownerAccountId); + Account owner = Mockito.mock(Account.class); + Mockito.when(accountManager.getAccount(ownerAccountId)).thenReturn(owner); + Mockito.doThrow(PermissionDeniedException.class).when(accountManager).checkAccess(caller, null, false, owner); + taggedResourceManagerImplSpy.checkTagsDeletePermission(List.of(resourceTag1, resourceTag2), caller); + } } diff --git a/ui/src/views/offering/AddComputeOffering.vue b/ui/src/views/offering/AddComputeOffering.vue index 3fa25e869a9..2022a53b186 100644 --- a/ui/src/views/offering/AddComputeOffering.vue +++ b/ui/src/views/offering/AddComputeOffering.vue @@ -885,9 +885,9 @@ export default { }, handleGpuChange (val) { this.vGpuTypes = [] - for (var i in this.gpuTypes) { - if (this.gpuTypes[i].value === val) { - this.vGpuTypes = this.gpuTypes[i].vgpu + for (var gpuType of this.gpuTypes) { + if (gpuType.value === val) { + this.vGpuTypes = gpuType.vgpu break } } @@ -999,9 +999,7 @@ export default { params['serviceofferingdetails[1].key'] = 'pciDevice' params['serviceofferingdetails[1].value'] = values.pcidevice } - if ('vgputype' in values && - this.vGpuTypes !== null && this.vGpuTypes !== undefined && - values.vgputype > this.vGpuTypes.length) { + if ('vgputype' in values && this.arrayHasItems(this.vGpuTypes)) { params['serviceofferingdetails[2].key'] = 'vgpuType' params['serviceofferingdetails[2].value'] = this.vGpuTypes[values.vgputype] }