FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

add access validation for the deleteUserData, linkUserDataToTemplate … · apache/cloudstack@72bdf59 · GitHub

/ cloudstack Public

Commit 72bdf59

Browse files
bernardodemarco
authored and
shwstppr
committed
add access validation for the deleteUserData, linkUserDataToTemplate and resetUserDataForVirtualMachine APIs
1 parent 9b9d0cf commit 72bdf59

6 files changed

Lines changed: 51 additions & 29 deletions

File tree

‎api/src/main/java/org/apache/cloudstack/api/command/user/userdata/DeleteUserDataCmd.java‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
package org.apache.cloudstack.api.command.user.userdata;
1818

1919
import org.apache.cloudstack.acl.RoleType;
20+
import org.apache.cloudstack.acl.SecurityChecker;
21+
import org.apache.cloudstack.api.ACL;
2022
import org.apache.cloudstack.api.APICommand;
2123
import org.apache.cloudstack.api.ApiConstants;
2224
import org.apache.cloudstack.api.ApiErrorCode;
@@ -27,7 +29,6 @@
2729
import org.apache.cloudstack.api.response.ProjectResponse;
2830
import org.apache.cloudstack.api.response.SuccessResponse;
2931
import org.apache.cloudstack.api.response.UserDataResponse;
30-
import org.apache.cloudstack.context.CallContext;
3132

3233
import com.cloud.user.Account;
3334
import com.cloud.user.UserData;
@@ -43,6 +44,7 @@ public class DeleteUserDataCmd extends BaseCmd {
4344
//////////////// API parameters /////////////////////
4445
/////////////////////////////////////////////////////
4546

47+
@ACL(accessType = SecurityChecker.AccessType.OperateEntry)
4648
@Parameter(name = ApiConstants.ID, type = CommandType.UUID, required = true, entityType = UserDataResponse.class, description = "The ID of the Userdata")
4749
private Long id;
4850

@@ -97,18 +99,13 @@ public void execute() {
9799

98100
@Override
99101
public long getEntityOwnerId() {
100-
Account account = CallContext.current().getCallingAccount();
101-
if ((account == null || _accountService.isAdmin(account.getId())) && (domainId != null && accountName != null)) {
102-
Account userAccount = _responseGenerator.findAccountByNameDomain(accountName, domainId);
103-
if (userAccount != null) {
104-
return userAccount.getId();
102+
if (id != null) {
103+
UserData userData = _entityMgr.findById(UserData.class, id);
104+
if (userData != null) {
105+
return userData.getAccountId();
105106
}
106107
}
107108

108-
if (account != null) {
109-
return account.getId();
110-
}
111-
112109
return Account.ACCOUNT_ID_SYSTEM; // no account info given, parent this command to SYSTEM so ERROR events are tracked
113110
}
114111
}

‎api/src/test/java/org/apache/cloudstack/api/command/user/userdata/DeleteUserDataCmdTest.java‎

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,18 @@
1717
package org.apache.cloudstack.api.command.user.userdata;
1818

1919
import com.cloud.server.ManagementService;
20-
import com.cloud.user.Account;
2120
import com.cloud.user.AccountService;
21+
import com.cloud.user.UserData;
22+
import com.cloud.utils.db.EntityManager;
2223
import org.apache.cloudstack.api.ServerApiException;
2324
import org.apache.cloudstack.api.response.SuccessResponse;
24-
import org.apache.cloudstack.context.CallContext;
2525
import org.junit.After;
2626
import org.junit.Assert;
2727
import org.junit.Before;
2828
import org.junit.Test;
2929
import org.junit.runner.RunWith;
3030
import org.mockito.InjectMocks;
3131
import org.mockito.Mock;
32-
import org.mockito.MockedStatic;
3332
import org.mockito.Mockito;
3433
import org.mockito.MockitoAnnotations;
3534
import org.mockito.junit.MockitoJUnitRunner;
@@ -46,6 +45,12 @@ public class DeleteUserDataCmdTest {
4645
@Mock
4746
ManagementService _mgr;
4847

48+
@Mock
49+
private EntityManager entityManagerMock;
50+
51+
@Mock
52+
private UserData userDataMock;
53+
4954
private static final long DOMAIN_ID = 5L;
5055
private static final long PROJECT_ID = 10L;
5156
private static final String ACCOUNT_NAME = "user";
@@ -84,19 +89,13 @@ public void testDeleteFailure() {
8489
}
8590

8691
@Test
87-
public void validateArgsCmd() {
88-
try (MockedStatic<CallContext> callContextMocked = Mockito.mockStatic(CallContext.class)) {
89-
CallContext callContextMock = Mockito.mock(CallContext.class);
90-
callContextMocked.when(CallContext::current).thenReturn(callContextMock);
91-
Account accountMock = Mockito.mock(Account.class);
92-
Mockito.when(callContextMock.getCallingAccount()).thenReturn(accountMock);
93-
Mockito.when(accountMock.getId()).thenReturn(2L);
94-
Mockito.doReturn(false).when(_accountService).isAdmin(2L);
95-
96-
ReflectionTestUtils.setField(cmd, "id", 1L);
92+
public void getEntityOwnerIdTestReturnUserDataOwnerWhenUserDataIdIsProvided() {
93+
long userDataId = 1L;
94+
long userDataOwnerId = 2L;
95+
ReflectionTestUtils.setField(cmd, "id", userDataId);
96+
Mockito.when(entityManagerMock.findById(UserData.class, userDataId)).thenReturn(userDataMock);
97+
Mockito.when(userDataMock.getAccountId()).thenReturn(userDataOwnerId);
9798

98-
Assert.assertEquals(1L, (long) cmd.getId());
99-
Assert.assertEquals(2L, cmd.getEntityOwnerId());
100-
}
99+
Assert.assertEquals(userDataOwnerId, cmd.getEntityOwnerId());
101100
}
102101
}

‎server/src/main/java/com/cloud/template/TemplateManagerImpl.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@
197197
import com.cloud.user.User;
198198
import com.cloud.user.UserData;
199199
import com.cloud.user.dao.AccountDao;
200+
import com.cloud.user.dao.UserDataDao;
200201
import com.cloud.uservm.UserVm;
201202
import com.cloud.utils.DateUtil;
202203
import com.cloud.utils.EncryptionUtil;
@@ -328,6 +329,9 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
328329

329330
protected boolean backupSnapshotAfterTakingSnapshot = SnapshotInfo.BackupSnapshotAfterTakingSnapshot.value();
330331

332+
@Inject
333+
private UserDataDao userDataDao;
334+
331335
private TemplateAdapter getAdapter(HypervisorType type) {
332336
TemplateAdapter adapter = null;
333337
if (type == HypervisorType.BareMetal) {
@@ -2589,12 +2593,17 @@ public VirtualMachineTemplate linkUserDataToTemplate(LinkUserDataToTemplateCmd c
25892593

25902594
_accountMgr.checkAccess(caller, AccessType.OperateEntry, true, template);
25912595

2592-
template.setUserDataId(userDataId);
25932596
if (userDataId != null) {
2597+
UserData userData = userDataDao.findById(userDataId);
2598+
if (userData == null) {
2599+
throw new InvalidParameterValueException("Unable to find user data with the specified ID.");
2600+
}
2601+
_accountMgr.checkAccess(caller, null, false, userData);
25942602
template.setUserDataLinkPolicy(overridePolicy);
25952603
} else {
25962604
template.setUserDataLinkPolicy(null);
25972605
}
2606+
template.setUserDataId(userDataId);
25982607
_tmpltDao.update(template.getId(), template);
25992608

26002609
return _tmpltDao.findById(template.getId());

‎server/src/main/java/com/cloud/vm/UserVmManagerImpl.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -996,8 +996,16 @@ public UserVm resetVMUserData(ResetVMUserDataCmd cmd) throws ResourceUnavailable
996996
throw new InvalidParameterValueException(String.format("VM %s should be stopped to do UserData reset", userVm));
997997
}
998998

999-
String userData = cmd.getUserData();
1000999
Long userDataId = cmd.getUserdataId();
1000+
if (userDataId != null) {
1001+
UserData userData = userDataDao.findById(userDataId);
1002+
if (userData == null) {
1003+
throw new InvalidParameterValueException("Unable to find user data with the specified ID.");
1004+
}
1005+
_accountMgr.checkAccess(caller, null, false, userData);
1006+
}
1007+
1008+
String userData = cmd.getUserData();
10011009
String userDataDetails = null;
10021010
if (MapUtils.isNotEmpty(cmd.getUserdataDetails())) {
10031011
userDataDetails = cmd.getUserdataDetails().toString();

‎server/src/test/java/com/cloud/template/TemplateManagerImplTest.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,10 @@
6262
import com.cloud.user.ResourceLimitService;
6363
import com.cloud.user.User;
6464
import com.cloud.user.UserData;
65+
import com.cloud.user.UserDataVO;
6566
import com.cloud.user.UserVO;
6667
import com.cloud.user.dao.AccountDao;
68+
import com.cloud.user.dao.UserDataDao;
6769
import com.cloud.utils.concurrency.NamedThreadFactory;
6870
import com.cloud.utils.exception.CloudRuntimeException;
6971
import com.cloud.vm.VMInstanceVO;
@@ -220,6 +222,12 @@ public class TemplateManagerImplTest extends TestCase {
220222
@Mock
221223
HeuristicRuleHelper heuristicRuleHelperMock;
222224

225+
@Mock
226+
private UserDataDao userDataDaoMock;
227+
228+
@Mock
229+
private UserDataVO userDataMock;
230+
223231
public class CustomThreadPoolExecutor extends ThreadPoolExecutor {
224232
AtomicInteger ai = new AtomicInteger(0);
225233
public CustomThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long keepAliveTime, TimeUnit unit,
@@ -524,6 +532,8 @@ public void testLinkUserDataToTemplate() {
524532
VMTemplateVO template = Mockito.mock(VMTemplateVO.class);
525533
when(vmTemplateDao.findById(anyLong())).thenReturn(template);
526534

535+
when(userDataDaoMock.findById(anyLong())).thenReturn(userDataMock);
536+
527537
VirtualMachineTemplate resultTemplate = templateManager.linkUserDataToTemplate(cmd);
528538

529539
Assert.assertEquals(template, resultTemplate);

‎server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1029,7 +1029,6 @@ public void testResetVMUserDataDontAcceptBothUserdataAndUserdataId() {
10291029

10301030
when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Stopped);
10311031

1032-
when(cmd.getUserData()).thenReturn("testUserdata");
10331032
when(cmd.getUserdataId()).thenReturn(1L);
10341033

10351034
try {

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL