Skip to content

Commit 9962cf1

Browse files
author
Prachi Damle
committed
SecurityChecker can accept multiple ControlledEntity
1 parent 685d664 commit 9962cf1

5 files changed

Lines changed: 186 additions & 23 deletions

File tree

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,26 @@ boolean checkAccess(Account caller, ControlledEntity entity, AccessType accessTy
100100
*/
101101
boolean checkAccess(Account caller, ControlledEntity entity, AccessType accessType, String action) throws PermissionDeniedException;
102102

103+
/**
104+
* Checks if the account can access multiple objects.
105+
*
106+
* @param caller
107+
* account to check against.
108+
* @param entities
109+
* objects that the account is trying to access.
110+
* @param accessType
111+
* TODO
112+
* @param action
113+
* name of the API
114+
* @return true if access allowed. false if this adapter cannot provide
115+
* permission.
116+
* @throws PermissionDeniedException
117+
* if this adapter is suppose to authenticate ownership and the
118+
* check failed.
119+
*/
120+
boolean checkAccess(Account caller, AccessType accessType, String action, ControlledEntity... entities)
121+
throws PermissionDeniedException;
122+
103123

104124
/**
105125
* Checks if the user belongs to an account that can access the object.

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,4 +341,17 @@ public boolean checkAccess(Account caller, ControlledEntity entity, AccessType a
341341
}
342342
return checkAccess(caller, entity, accessType);
343343
}
344+
345+
@Override
346+
public boolean checkAccess(Account caller, AccessType accessType, String action, ControlledEntity... entities)
347+
throws PermissionDeniedException {
348+
349+
// returns true only if access to all entities is granted
350+
for (ControlledEntity entity : entities) {
351+
if (!checkAccess(caller, entity, accessType, action)) {
352+
return false;
353+
}
354+
}
355+
return true;
356+
}
344357
}

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

Lines changed: 98 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,10 @@
3737

3838
import org.apache.cloudstack.acl.ControlledEntity;
3939
import org.apache.cloudstack.acl.InfrastructureEntity;
40+
import org.apache.cloudstack.acl.SecurityChecker;
4041
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
4142
import org.apache.cloudstack.api.ACL;
43+
import org.apache.cloudstack.api.APICommand;
4244
import org.apache.cloudstack.api.ApiErrorCode;
4345
import org.apache.cloudstack.api.BaseAsyncCreateCmd;
4446
import org.apache.cloudstack.api.BaseCmd;
@@ -54,7 +56,12 @@
5456
import org.apache.cloudstack.api.command.user.event.ListEventsCmd;
5557
import org.apache.cloudstack.context.CallContext;
5658

59+
import com.cloud.configuration.ConfigurationManager;
60+
import com.cloud.dc.DataCenter;
5761
import com.cloud.exception.InvalidParameterValueException;
62+
import com.cloud.exception.PermissionDeniedException;
63+
import com.cloud.offering.DiskOffering;
64+
import com.cloud.offering.ServiceOffering;
5865
import com.cloud.user.Account;
5966
import com.cloud.user.AccountManager;
6067
import com.cloud.utils.DateUtil;
@@ -71,6 +78,17 @@ public class ParamProcessWorker implements DispatchWorker {
7178
@Inject
7279
protected EntityManager _entityMgr;
7380

81+
List<SecurityChecker> _secChecker;
82+
83+
public List<SecurityChecker> getSecChecker() {
84+
return _secChecker;
85+
}
86+
87+
@Inject
88+
public void setSecChecker(List<SecurityChecker> secChecker) {
89+
_secChecker = secChecker;
90+
}
91+
7492
@Override
7593
public void handle(final DispatchTask task) {
7694
processParameters(task.getCmd(), task.getParams());
@@ -214,27 +232,96 @@ public void processParameters(final BaseCmd cmd, final Map params) {
214232

215233

216234
private void doAccessChecks(final BaseCmd cmd, final Map<Object, AccessType> entitiesToAccess) {
217-
final Account caller = CallContext.current().getCallingAccount();
218-
final Account owner = _accountMgr.getActiveAccountById(cmd.getEntityOwnerId());
235+
Account caller = CallContext.current().getCallingAccount();
219236

220-
if (cmd instanceof BaseAsyncCreateCmd) {
221-
//check that caller can access the owner account.
222-
_accountMgr.checkAccess(caller, null, true, owner);
223-
}
237+
APICommand commandAnnotation = cmd.getClass().getAnnotation(APICommand.class);
238+
String apiName = commandAnnotation != null ? commandAnnotation.name() : null;
224239

225240
if (!entitiesToAccess.isEmpty()) {
226-
//check that caller can access the owner account.
227-
_accountMgr.checkAccess(caller, null, true, owner);
228-
for (final Object entity : entitiesToAccess.keySet()) {
241+
List<ControlledEntity> entitiesToOperate = new ArrayList<ControlledEntity>();
242+
243+
for (Object entity : entitiesToAccess.keySet()) {
229244
if (entity instanceof ControlledEntity) {
230-
_accountMgr.checkAccess(caller, entitiesToAccess.get(entity), true, (ControlledEntity)entity);
245+
246+
if (AccessType.OperateEntry == entitiesToAccess.get(entity)) {
247+
entitiesToOperate.add((ControlledEntity) entity);
248+
} else {
249+
_accountMgr.checkAccess(caller, entitiesToAccess.get(entity), false, apiName,
250+
(ControlledEntity) entity);
251+
}
231252
} else if (entity instanceof InfrastructureEntity) {
232-
//FIXME: Move this code in adapter, remove code from Account manager
253+
if (entity instanceof DataCenter) {
254+
checkZoneAccess(caller, (DataCenter) entity);
255+
} else if (entity instanceof ServiceOffering) {
256+
checkServiceOfferingAccess(caller, (ServiceOffering) entity);
257+
} else if (entity instanceof DiskOffering) {
258+
checkDiskOfferingAccess(caller, (DiskOffering) entity);
259+
}
233260
}
234261
}
262+
263+
if (!entitiesToOperate.isEmpty()) {
264+
_accountMgr.checkAccess(caller, AccessType.OperateEntry, false, apiName,
265+
(ControlledEntity[]) entitiesToOperate.toArray());
266+
}
267+
235268
}
236269
}
237270

271+
private void checkDiskOfferingAccess(Account caller, DiskOffering dof) {
272+
for (SecurityChecker checker : _secChecker) {
273+
if (checker.checkAccess(caller, dof)) {
274+
if (s_logger.isDebugEnabled()) {
275+
s_logger.debug("Access granted to " + caller + " to disk offering:" + dof.getId() + " by "
276+
+ checker.getName());
277+
}
278+
return;
279+
} else {
280+
throw new PermissionDeniedException("Access denied to " + caller + " by " + checker.getName());
281+
}
282+
}
283+
284+
assert false : "How can all of the security checkers pass on checking this caller?";
285+
throw new PermissionDeniedException("There's no way to confirm " + caller + " has access to disk offering:"
286+
+ dof.getId());
287+
}
288+
289+
private void checkServiceOfferingAccess(Account caller, ServiceOffering sof) {
290+
for (SecurityChecker checker : _secChecker) {
291+
if (checker.checkAccess(caller, sof)) {
292+
if (s_logger.isDebugEnabled()) {
293+
s_logger.debug("Access granted to " + caller + " to service offering:" + sof.getId() + " by "
294+
+ checker.getName());
295+
}
296+
return;
297+
} else {
298+
throw new PermissionDeniedException("Access denied to " + caller + " by " + checker.getName());
299+
}
300+
}
301+
302+
assert false : "How can all of the security checkers pass on checking this caller?";
303+
throw new PermissionDeniedException("There's no way to confirm " + caller + " has access to service offering:"
304+
+ sof.getId());
305+
}
306+
307+
private void checkZoneAccess(Account caller, DataCenter zone) {
308+
for (SecurityChecker checker : _secChecker) {
309+
if (checker.checkAccess(caller, zone)) {
310+
if (s_logger.isDebugEnabled()) {
311+
s_logger.debug("Access granted to " + caller + " to zone:" + zone.getId() + " by "
312+
+ checker.getName());
313+
}
314+
return;
315+
} else {
316+
throw new PermissionDeniedException("Access denied to " + caller + " by " + checker.getName()
317+
+ " for zone " + zone.getId());
318+
}
319+
}
320+
321+
assert false : "How can all of the security checkers pass on checking this caller?";
322+
throw new PermissionDeniedException("There's no way to confirm " + caller + " has access to zone:"
323+
+ zone.getId());
324+
}
238325

239326
@SuppressWarnings({"unchecked", "rawtypes"})
240327
private void setFieldValue(final Field field, final BaseCmd cmdObj, final Object paramObj, final Parameter annotation) throws IllegalArgumentException, ParseException {

‎services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedAPIAccessChecker.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,6 @@ public void setServices(List<PluggableService> services) {
216216
}
217217

218218
private void addDefaultAclPolicyPermission(String apiName, Class<?> cmdClass, RoleType role) {
219-
220219
AccessType accessType = null;
221220
Class<?>[] entityTypes = null;
222221
if (cmdClass != null) {

‎services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java‎

Lines changed: 55 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.apache.cloudstack.acl.ControlledEntity;
2828
import org.apache.cloudstack.acl.PermissionScope;
2929
import org.apache.cloudstack.acl.SecurityChecker;
30+
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
3031
import org.apache.cloudstack.api.InternalIdentity;
3132
import org.apache.cloudstack.iam.api.IAMGroup;
3233
import org.apache.cloudstack.iam.api.IAMPolicy;
@@ -107,14 +108,22 @@ public boolean checkAccess(Account caller, ControlledEntity entity, AccessType a
107108
permissions = _iamSrv.listPolicyPermissionByActionAndEntity(policy.getId(), action, entityType);
108109
if (permissions.isEmpty()) {
109110
if (accessType != null) {
110-
permissions.addAll(_iamSrv.listPolicyPermissionByAccessAndEntity(policy.getId(),
111-
accessType.toString(), entityType));
111+
for (AccessType type : AccessType.values()) {
112+
if (type.ordinal() >= accessType.ordinal()) {
113+
permissions.addAll(_iamSrv.listPolicyPermissionByAccessAndEntity(policy.getId(),
114+
type.toString(), entityType));
115+
}
116+
}
112117
}
113118
}
114119
} else {
115120
if (accessType != null) {
116-
permissions.addAll(_iamSrv.listPolicyPermissionByAccessAndEntity(policy.getId(),
117-
accessType.toString(), entityType));
121+
for (AccessType type : AccessType.values()) {
122+
if (type.ordinal() >= accessType.ordinal()) {
123+
permissions.addAll(_iamSrv.listPolicyPermissionByAccessAndEntity(policy.getId(),
124+
type.toString(), entityType));
125+
}
126+
}
118127
}
119128
}
120129
for (IAMPolicyPermission permission : permissions) {
@@ -145,6 +154,48 @@ public boolean checkAccess(Account caller, ControlledEntity entity, AccessType a
145154
return false;
146155
}
147156

157+
@Override
158+
public boolean checkAccess(Account caller, AccessType accessType, String action, ControlledEntity... entities)
159+
throws PermissionDeniedException {
160+
161+
// operate access on multiple entities?
162+
if (accessType != null && accessType == AccessType.OperateEntry) {
163+
// In this case caller MUST own n-1 entities.
164+
165+
for (ControlledEntity entity : entities) {
166+
checkAccess(caller, entity, accessType, action);
167+
168+
boolean otherEntitiesAccess = true;
169+
170+
for (ControlledEntity otherEntity : entities) {
171+
if (otherEntity.getAccountId() == caller.getAccountId()
172+
|| (checkAccess(caller, otherEntity, accessType, action) && otherEntity.getAccountId() == entity
173+
.getAccountId())) {
174+
continue;
175+
} else {
176+
otherEntitiesAccess = false;
177+
break;
178+
}
179+
}
180+
181+
if (otherEntitiesAccess) {
182+
return true;
183+
}
184+
}
185+
186+
throw new PermissionDeniedException(caller
187+
+ " does not have permission to perform this operation on these resources");
188+
189+
} else {
190+
for (ControlledEntity entity : entities) {
191+
if (!checkAccess(caller, entity, accessType, action)) {
192+
return false;
193+
}
194+
}
195+
return true;
196+
}
197+
}
198+
148199
private boolean checkPermissionScope(Account caller, String scope, Long scopeId, ControlledEntity entity) {
149200

150201
if(scopeId != null && !scopeId.equals(new Long(IAMPolicyPermission.PERMISSION_SCOPE_ID_CURRENT_CALLER))){
@@ -181,15 +232,8 @@ private boolean checkPermissionScope(Account caller, String scope, Long scopeId,
181232

182233
private List<IAMPolicy> getEffectivePolicies(Account caller, ControlledEntity entity) {
183234

184-
// Get the static Policies of the Caller
185235
List<IAMPolicy> policies = _iamSrv.listIAMPolicies(caller.getId());
186236

187-
// add any dynamic policies w.r.t the entity
188-
if (caller.getId() == entity.getAccountId()) {
189-
// The caller owns the entity
190-
policies.add(_iamSrv.getResourceOwnerPolicy());
191-
}
192-
193237
List<IAMGroup> groups = _iamSrv.listIAMGroups(caller.getId());
194238
for (IAMGroup group : groups) {
195239
// for each group find the grand parent groups.

0 commit comments

Comments
 (0)