Skip to content

Commit da13165

Browse files
minchen07minchen07
committed
Change AccountManagerImpl.checkAccess to invoke SecurityChecker
interface that takes multiple controlled entities.
1 parent 4c4cfe5 commit da13165

14 files changed

Lines changed: 133 additions & 169 deletions

File tree

‎api/src/com/cloud/user/AccountService.java‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -104,20 +104,14 @@ UserAccount createUserAccount(String userName, String password, String firstName
104104

105105
RoleType getRoleType(Account account);
106106

107-
void checkAccess(Account account, Domain domain) throws PermissionDeniedException;
107+
void checkAccess(Account caller, Domain domain) throws PermissionDeniedException;
108108

109-
void checkAccess(Account account, AccessType accessType, ControlledEntity... entities) throws PermissionDeniedException;
109+
void checkAccess(Account caller, AccessType accessType, ControlledEntity... entities) throws PermissionDeniedException;
110110

111-
void checkAccess(Account account, AccessType accessType, String apiName, ControlledEntity... entities) throws PermissionDeniedException;
112-
113-
// TODO: the following two interfaces will be deprecated by the above two counterparts when securityChecker implementation is in place
114-
void checkAccess(Account account, AccessType accessType, boolean sameOwner, ControlledEntity... entities) throws PermissionDeniedException;
115-
116-
void checkAccess(Account account, AccessType accessType, boolean sameOwner, String apiName,
117-
ControlledEntity... entities) throws PermissionDeniedException;
111+
void checkAccess(Account caller, AccessType accessType, String apiName, ControlledEntity... entities) throws PermissionDeniedException;
118112

119113
//TO be implemented, to check accessibility for an entity owned by domain
120-
void checkAccess(Account account, AccessType accessType, boolean sameOwner, PartOf... entities) throws PermissionDeniedException;
114+
void checkAccess(Account caller, AccessType accessType, boolean sameOwner, PartOf... entities) throws PermissionDeniedException;
121115

122116
Long finalyzeAccountId(String accountName, Long domainId, Long projectId, boolean enabledOnly);
123117
}

‎api/src/org/apache/cloudstack/acl/SecurityChecker.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@
3131
public interface SecurityChecker extends Adapter {
3232

3333
public enum AccessType {
34-
ModifyProject,
35-
OperateEntry,
34+
ListEntry,
3635
UseEntry,
37-
ListEntry
36+
OperateEntry,
37+
ModifyProject,
3838
}
3939

4040
/**

‎api/src/org/apache/cloudstack/api/command/user/nat/DisableStaticNatCmd.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,11 @@
3434
import com.cloud.exception.NetworkRuleConflictException;
3535
import com.cloud.exception.ResourceUnavailableException;
3636
import com.cloud.network.IpAddress;
37+
import com.cloud.network.vpc.Vpc;
38+
import com.cloud.vm.VirtualMachine;
3739

3840
@APICommand(name = "disableStaticNat", description = "Disables static rule for given ip address", responseObject = SuccessResponse.class,
41+
entityType = {IpAddress.class, VirtualMachine.class, Vpc.class},
3942
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
4043
public class DisableStaticNatCmd extends BaseAsyncCmd {
4144
public static final Logger s_logger = Logger.getLogger(DeletePortForwardingRuleCmd.class.getName());
@@ -89,7 +92,7 @@ public void execute() throws ResourceUnavailableException, NetworkRuleConflictEx
8992

9093
if (result) {
9194
SuccessResponse response = new SuccessResponse(getCommandName());
92-
this.setResponseObject(response);
95+
setResponseObject(response);
9396
} else {
9497
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to disable static nat");
9598
}

‎api/src/org/apache/cloudstack/api/command/user/nat/EnableStaticNatCmd.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,13 @@
3535
import com.cloud.exception.NetworkRuleConflictException;
3636
import com.cloud.exception.ResourceUnavailableException;
3737
import com.cloud.network.IpAddress;
38+
import com.cloud.network.vpc.Vpc;
3839
import com.cloud.user.Account;
3940
import com.cloud.uservm.UserVm;
4041
import com.cloud.vm.VirtualMachine;
4142

4243
@APICommand(name = "enableStaticNat", description = "Enables static nat for given ip address", responseObject = SuccessResponse.class,
43-
entityType = {IpAddress.class, VirtualMachine.class},
44+
entityType = {IpAddress.class, VirtualMachine.class, Vpc.class},
4445
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
4546
public class EnableStaticNatCmd extends BaseCmd {
4647
public static final Logger s_logger = Logger.getLogger(CreateIpForwardingRuleCmd.class.getName());

‎api/src/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@
3737
import com.cloud.user.Account;
3838
import com.cloud.vm.VirtualMachine;
3939

40-
@APICommand(name = "attachVolume", description = "Attaches a disk volume to a virtual machine.", responseObject = VolumeResponse.class, responseView = ResponseView.Restricted, entityType = {VirtualMachine.class},
40+
@APICommand(name = "attachVolume", description = "Attaches a disk volume to a virtual machine.", responseObject = VolumeResponse.class, responseView = ResponseView.Restricted, entityType = {
41+
VirtualMachine.class, Volume.class},
4142
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
4243
public class AttachVolumeCmd extends BaseAsyncVolumeCmd {
4344
public static final Logger s_logger = Logger.getLogger(AttachVolumeCmd.class.getName());
@@ -52,6 +53,7 @@ public class AttachVolumeCmd extends BaseAsyncVolumeCmd {
5253
+ "* 4 - /dev/xvde" + "* 5 - /dev/xvdf" + "* 6 - /dev/xvdg" + "* 7 - /dev/xvdh" + "* 8 - /dev/xvdi" + "* 9 - /dev/xvdj")
5354
private Long deviceId;
5455

56+
@ACL(accessType = AccessType.OperateEntry)
5557
@Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = VolumeResponse.class, required = true, description = "the ID of the disk volume")
5658
private Long id;
5759

‎framework/db/test/db.properties‎

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,77 @@
1+
#
12
# Licensed to the Apache Software Foundation (ASF) under one
23
# or more contributor license agreements. See the NOTICE file
34
# distributed with this work for additional information
45
# regarding copyright ownership. The ASF licenses this file
56
# to you under the Apache License, Version 2.0 (the
67
# "License"); you may not use this file except in compliance
78
# with the License. You may obtain a copy of the License at
8-
#
9+
#
910
# http://www.apache.org/licenses/LICENSE-2.0
10-
#
11+
#
1112
# Unless required by applicable law or agreed to in writing,
1213
# software distributed under the License is distributed on an
1314
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
1415
# KIND, either express or implied. See the License for the
1516
# specific language governing permissions and limitations
1617
# under the License.
18+
#
1719

18-
# Just here to make the unit test not blow up
20+
21+
# management server clustering parameters, change cluster.node.IP to the machine IP address
22+
# in which the management server(Tomcat) is running
23+
cluster.node.IP=127.0.0.1
24+
cluster.servlet.port=9090
25+
region.id=1
26+
27+
# CloudStack database settings
28+
db.cloud.username=cloud
29+
db.cloud.password=cloud
30+
db.root.password=
31+
db.cloud.host=localhost
32+
db.cloud.port=3306
33+
db.cloud.name=cloud
34+
35+
# CloudStack database tuning parameters
36+
db.cloud.maxActive=250
37+
db.cloud.maxIdle=30
38+
db.cloud.maxWait=10000
39+
db.cloud.autoReconnect=true
40+
db.cloud.validationQuery=SELECT 1
41+
db.cloud.testOnBorrow=true
42+
db.cloud.testWhileIdle=true
43+
db.cloud.timeBetweenEvictionRunsMillis=40000
44+
db.cloud.minEvictableIdleTimeMillis=240000
45+
db.cloud.poolPreparedStatements=false
46+
db.cloud.url.params=prepStmtCacheSize=517&cachePrepStmts=true&prepStmtCacheSqlLimit=4096
47+
48+
# usage database settings
49+
db.usage.username=cloud
50+
db.usage.password=cloud
51+
db.usage.host=localhost
52+
db.usage.port=3306
53+
db.usage.name=cloud_usage
54+
55+
# usage database tuning parameters
56+
db.usage.maxActive=100
57+
db.usage.maxIdle=30
58+
db.usage.maxWait=10000
59+
db.usage.autoReconnect=true
60+
61+
# awsapi database settings
62+
db.awsapi.username=cloud
63+
db.awsapi.password=cloud
64+
db.awsapi.host=localhost
65+
db.awsapi.port=3306
66+
db.awsapi.name=cloudbridge
67+
68+
# Simulator database settings
69+
db.simulator.username=cloud
70+
db.simulator.password=cloud
71+
db.simulator.host=localhost
72+
db.simulator.port=3306
73+
db.simulator.name=simulator
74+
db.simulator.maxActive=250
75+
db.simulator.maxIdle=30
76+
db.simulator.maxWait=10000
77+
db.simulator.autoReconnect=true

‎plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -95,12 +95,6 @@ public void checkAccess(Account arg0, Domain arg1) throws PermissionDeniedExcept
9595

9696
}
9797

98-
@Override
99-
public void checkAccess(Account arg0, AccessType arg1, boolean arg2, ControlledEntity... arg3) throws PermissionDeniedException {
100-
// TODO Auto-generated method stub
101-
102-
}
103-
10498
@Override
10599
public String[] createApiKeyAndSecretKey(RegisterCmd arg0) {
106100
// TODO Auto-generated method stub
@@ -369,11 +363,6 @@ public void logoutUser(long userId) {
369363

370364
}
371365

372-
@Override
373-
public void checkAccess(Account account, AccessType accessType, boolean sameOwner, String apiName,
374-
ControlledEntity... entities) throws PermissionDeniedException {
375-
// TODO Auto-generated method stub
376-
}
377366

378367
@Override
379368
public Long finalyzeAccountId(String accountName, Long domainId, Long projectId, boolean enabledOnly) {

‎server/src/com/cloud/acl/DomainChecker.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import javax.ejb.Local;
2020
import javax.inject.Inject;
2121

22+
import org.apache.log4j.Logger;
2223
import org.springframework.stereotype.Component;
2324

2425
import org.apache.cloudstack.acl.ControlledEntity;
@@ -50,6 +51,8 @@
5051
@Local(value = SecurityChecker.class)
5152
public class DomainChecker extends AdapterBase implements SecurityChecker {
5253

54+
public static final Logger s_logger = Logger.getLogger(DomainChecker.class);
55+
5356
@Inject
5457
DomainDao _domainDao;
5558
@Inject
@@ -101,6 +104,15 @@ public boolean checkAccess(User user, Domain domain) throws PermissionDeniedExce
101104
@Override
102105
public boolean checkAccess(Account caller, ControlledEntity entity, AccessType accessType)
103106
throws PermissionDeniedException {
107+
108+
if (caller.getId() == Account.ACCOUNT_ID_SYSTEM || _accountService.isRootAdmin(caller.getId())) {
109+
// no need to make permission checks if the system/root admin makes the call
110+
if (s_logger.isTraceEnabled()) {
111+
s_logger.trace("No need to make permission check for System/RootAdmin account, returning true");
112+
}
113+
return true;
114+
}
115+
104116
if (entity instanceof VirtualMachineTemplate) {
105117

106118
VirtualMachineTemplate template = (VirtualMachineTemplate)entity;

‎server/src/com/cloud/api/ApiDispatcher.java‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,6 @@
2323

2424
import org.apache.log4j.Logger;
2525

26-
import org.apache.cloudstack.acl.ControlledEntity;
27-
import org.apache.cloudstack.acl.InfrastructureEntity;
28-
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
29-
import org.apache.cloudstack.api.APICommand;
3026
import org.apache.cloudstack.api.ApiConstants;
3127
import org.apache.cloudstack.api.BaseAsyncCmd;
3228
import org.apache.cloudstack.api.BaseAsyncCreateCmd;
@@ -41,7 +37,6 @@
4137
import com.cloud.api.dispatch.DispatchChainFactory;
4238
import com.cloud.api.dispatch.DispatchTask;
4339
import com.cloud.event.EventTypes;
44-
import com.cloud.user.Account;
4540
import com.cloud.user.AccountManager;
4641
import com.cloud.utils.ReflectUtil;
4742
import com.cloud.vm.VirtualMachine;
@@ -83,23 +78,6 @@ public void dispatchCreateCmd(final BaseAsyncCreateCmd cmd, final Map<String, St
8378
CallContext.current().setEventDisplayEnabled(cmd.isDisplayResourceEnabled());
8479
}
8580

86-
private void doAccessChecks(BaseCmd cmd, Map<Object, AccessType> entitiesToAccess) {
87-
Account caller = CallContext.current().getCallingAccount();
88-
89-
APICommand commandAnnotation = cmd.getClass().getAnnotation(APICommand.class);
90-
String apiName = commandAnnotation != null ? commandAnnotation.name() : null;
91-
92-
if (!entitiesToAccess.isEmpty()) {
93-
for (Object entity : entitiesToAccess.keySet()) {
94-
if (entity instanceof ControlledEntity) {
95-
_accountMgr.checkAccess(caller, entitiesToAccess.get(entity), false, apiName, (ControlledEntity) entity);
96-
} else if (entity instanceof InfrastructureEntity) {
97-
//FIXME: Move this code in adapter, remove code from Account manager
98-
}
99-
}
100-
}
101-
}
102-
10381
public void dispatch(final BaseCmd cmd, final Map<String, String> params, final boolean execute) throws Exception {
10482
// Let the chain of responsibility dispatch gradually
10583
standardDispatchChain.dispatch(new DispatchTask(cmd, params));

‎server/src/com/cloud/api/dispatch/ParamProcessWorker.java‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -231,35 +231,39 @@ public void processParameters(final BaseCmd cmd, final Map params) {
231231

232232
private void doAccessChecks(final BaseCmd cmd, final Map<Object, AccessType> entitiesToAccess) {
233233
Account caller = CallContext.current().getCallingAccount();
234+
Account owner = _accountMgr.getAccount(cmd.getEntityOwnerId());
235+
if (owner == null) {
236+
owner = caller;
237+
}
234238

235239
APICommand commandAnnotation = cmd.getClass().getAnnotation(APICommand.class);
240+
236241
String apiName = commandAnnotation != null ? commandAnnotation.name() : null;
237242

238243
if (!entitiesToAccess.isEmpty()) {
239244
List<ControlledEntity> entitiesToOperate = new ArrayList<ControlledEntity>();
240-
241245
for (Object entity : entitiesToAccess.keySet()) {
242246
if (entity instanceof ControlledEntity) {
243247

244248
if (AccessType.OperateEntry == entitiesToAccess.get(entity)) {
245249
entitiesToOperate.add((ControlledEntity) entity);
246250
} else {
247-
_accountMgr.checkAccess(caller, entitiesToAccess.get(entity), apiName,
251+
_accountMgr.checkAccess(owner, entitiesToAccess.get(entity), apiName,
248252
(ControlledEntity) entity);
249253
}
250254
} else if (entity instanceof InfrastructureEntity) {
251255
if (entity instanceof DataCenter) {
252-
checkZoneAccess(caller, (DataCenter) entity);
256+
checkZoneAccess(owner, (DataCenter)entity);
253257
} else if (entity instanceof ServiceOffering) {
254-
checkServiceOfferingAccess(caller, (ServiceOffering) entity);
258+
checkServiceOfferingAccess(owner, (ServiceOffering)entity);
255259
} else if (entity instanceof DiskOffering) {
256-
checkDiskOfferingAccess(caller, (DiskOffering) entity);
260+
checkDiskOfferingAccess(owner, (DiskOffering)entity);
257261
}
258262
}
259263
}
260264

261265
if (!entitiesToOperate.isEmpty()) {
262-
_accountMgr.checkAccess(caller, AccessType.OperateEntry, false, apiName,
266+
_accountMgr.checkAccess(owner, AccessType.OperateEntry, apiName,
263267
entitiesToOperate.toArray(new ControlledEntity[entitiesToOperate.size()]));
264268
}
265269

0 commit comments

Comments
 (0)