Skip to content

Commit

Permalink
Fixing assign/modify authorization combination (MID-4517)
Browse files Browse the repository at this point in the history
  • Loading branch information
@semancik
semancik committed Mar 22, 2018
1 parent 6a7c4e6 commit 68683eb
Show file tree
Hide file tree
Showing 9 changed files with 297 additions and 36 deletions.
11 changes: 11 additions & 0 deletions ...i/src/main/java/com/evolveum/midpoint/web/security/MidPointGuiAuthorizationEvaluator.java
View file
Expand Up @@ -16,8 +16,11 @@

package com.evolveum.midpoint.web.security;

import com.evolveum.midpoint.prism.Containerable;
import com.evolveum.midpoint.prism.PrismContainerValue;
import com.evolveum.midpoint.prism.PrismObject;
import com.evolveum.midpoint.prism.delta.ObjectDelta;
import com.evolveum.midpoint.prism.delta.PlusMinusZero;
import com.evolveum.midpoint.prism.path.ItemPath;
import com.evolveum.midpoint.prism.query.ObjectFilter;
import com.evolveum.midpoint.schema.result.OperationResult;
Expand Down Expand Up @@ -347,4 +350,12 @@ public <O extends ObjectType> AccessDecision determineSubitemDecision(
return securityEnforcer.determineSubitemDecision(securityConstraints, delta, currentObject, operationUrl, phase, subitemRootPath);
}

@Override
public <C extends Containerable> AccessDecision determineSubitemDecision(
ObjectSecurityConstraints securityConstraints, PrismContainerValue<C> containerValue,
String operationUrl, AuthorizationPhaseType phase, ItemPath subitemRootPath,
PlusMinusZero plusMinusZero, String decisionContextDesc) {
return securityEnforcer.determineSubitemDecision(securityConstraints, containerValue, operationUrl, phase, subitemRootPath, plusMinusZero, decisionContextDesc);
}

}
75 changes: 49 additions & 26 deletions model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/Clockwork.java
View file
Expand Up @@ -1369,22 +1369,22 @@ private <F extends ObjectType, O extends ObjectType> ObjectSecurityConstraints a
LOGGER.debug("Deny assignment/unassignment to {} becasue access to assignment container/properties is explicitly denied", object);
throw new AuthorizationException("Access denied");
} else {
AuthorizationDecisionType actionDecision = securityConstraints.getActionDecision(operationUrl, getRequestAuthorizationPhase(context));
if (actionDecision == AuthorizationDecisionType.ALLOW) {
AuthorizationDecisionType allItemsDecision = securityConstraints.findAllItemsDecision(operationUrl, getRequestAuthorizationPhase(context));
if (allItemsDecision == AuthorizationDecisionType.ALLOW) {
// Nothing to do, operation is allowed for all values
} else if (actionDecision == AuthorizationDecisionType.DENY) {
} else if (allItemsDecision == AuthorizationDecisionType.DENY) {
throw new AuthorizationException("Access denied");
} else {
// No explicit decision for assignment modification yet
// No blank decision for assignment modification yet
// process each assignment individually
authorizeAssignmentRequest(context, ModelAuthorizationAction.ASSIGN.getUrl(),
object, ownerResolver, PlusMinusZero.PLUS, true, task, result);
authorizeAssignmentRequest(context, operationUrl, ModelAuthorizationAction.ASSIGN.getUrl(),
object, ownerResolver, securityConstraints, PlusMinusZero.PLUS, true, task, result);

if (!primaryDelta.isAdd()) {
// We want to allow unassignment even if there are policies. Otherwise we would not be able to get
// rid of that assignment
authorizeAssignmentRequest(context, ModelAuthorizationAction.UNASSIGN.getUrl(),
object, ownerResolver,PlusMinusZero.MINUS, false, task, result);
authorizeAssignmentRequest(context, operationUrl, ModelAuthorizationAction.UNASSIGN.getUrl(),
object, ownerResolver, securityConstraints, PlusMinusZero.MINUS, false, task, result);
}
}
}
Expand Down Expand Up @@ -1473,8 +1473,18 @@ private <F extends ObjectType> AuthorizationDecisionType evaluateCredentialDecis
ModelAuthorizationAction.CHANGE_CREDENTIALS.getUrl(), getRequestAuthorizationPhase(context));
}

private <F extends ObjectType,O extends ObjectType> void authorizeAssignmentRequest(LensContext<F> context, String assignActionUrl, PrismObject<O> object,
OwnerResolver ownerResolver, PlusMinusZero plusMinusZero, boolean prohibitPolicies, Task task, OperationResult result) throws SecurityViolationException, SchemaException, ObjectNotFoundException, ExpressionEvaluationException, CommunicationException, ConfigurationException {
private <F extends ObjectType,O extends ObjectType> void authorizeAssignmentRequest(
LensContext<F> context,
String operationUrl,
String assignActionUrl,
PrismObject<O> object,
OwnerResolver ownerResolver,
ObjectSecurityConstraints securityConstraints,
PlusMinusZero plusMinusZero,
boolean prohibitPolicies,
Task task,
OperationResult result)
throws SecurityViolationException, SchemaException, ObjectNotFoundException, ExpressionEvaluationException, CommunicationException, ConfigurationException {
// This is *request* authorization. Therefore we care only about primary delta.
ObjectDelta<F> focusPrimaryDelta = context.getFocusContext().getPrimaryDelta();
if (focusPrimaryDelta == null) {
Expand All @@ -1484,15 +1494,25 @@ private <F extends ObjectType,O extends ObjectType> void authorizeAssignmentRequ
if (focusAssignmentDelta == null) {
return;
}
String operationDesc = assignActionUrl.substring(assignActionUrl.lastIndexOf('#') + 1);
Collection<PrismContainerValue<AssignmentType>> changedAssignmentValues = determineChangedAssignmentValues(context.getFocusContext(), focusAssignmentDelta, plusMinusZero);
for (PrismContainerValue<AssignmentType> changedAssignmentValue: changedAssignmentValues) {
AssignmentType changedAssignment = changedAssignmentValue.getRealValue();
ObjectReferenceType targetRef = changedAssignment.getTargetRef();
if (targetRef == null || targetRef.getOid() == null) {
String operationDesc = assignActionUrl.substring(assignActionUrl.lastIndexOf('#') + 1);
LOGGER.debug("{} of non-target assignment not allowed", operationDesc);
securityEnforcer.failAuthorization(operationDesc, getRequestAuthorizationPhase(context), AuthorizationParameters.Builder.buildObject(object), result);
assert false; // just to keep static checkers happy
// This may still be allowed by #add and #modify authorizations. We have already checked these, but there may be combinations of
// assignments, one of the assignments allowed by #assign, other allowed by #modify (e.g. MID-4517).
// Therefore check the items again. This is not very efficient to check it twice. But this is not a common case
// so there should not be any big harm in suffering this inefficiency.
AccessDecision subitemDecision = securityEnforcer.determineSubitemDecision(securityConstraints, changedAssignmentValue, operationUrl,
getRequestAuthorizationPhase(context), null, plusMinusZero, operationDesc);
if (subitemDecision == AccessDecision.ALLOW) {
LOGGER.debug("{} of policy assignment to {} allowed with {} authorization", operationDesc, object, operationUrl);
continue;
} else {
LOGGER.debug("{} of non-target assignment not allowed", operationDesc);
securityEnforcer.failAuthorization(operationDesc, getRequestAuthorizationPhase(context), AuthorizationParameters.Builder.buildObject(object), result);
}
}
// We do not worry about performance here too much. The target was already evaluated. This will be retrieved from repo cache anyway.
PrismObject<ObjectType> target = objectResolver.resolve(targetRef.asReferenceValue(), "resolving assignment target", task, result);
Expand All @@ -1514,32 +1534,35 @@ private <F extends ObjectType,O extends ObjectType> void authorizeAssignmentRequ

if (prohibitPolicies) {
if (changedAssignment.getPolicyRule() != null || !changedAssignment.getPolicyException().isEmpty() || !changedAssignment.getPolicySituation().isEmpty() || !changedAssignment.getTriggeredPolicyRule().isEmpty()) {
securityEnforcer.failAuthorization("with assignment because of policies in the assignment", getRequestAuthorizationPhase(context), autzParams, result);
// This may still be allowed by #add and #modify authorizations. We have already checked these, but there may be combinations of
// assignments, one of the assignments allowed by #assign, other allowed by #modify (e.g. MID-4517).
// Therefore check the items again. This is not very efficient to check it twice. But this is not a common case
// so there should not be any big harm in suffering this inefficiency.
AccessDecision subitemDecision = securityEnforcer.determineSubitemDecision(securityConstraints, changedAssignmentValue, operationUrl,
getRequestAuthorizationPhase(context), null, plusMinusZero, operationDesc);
if (subitemDecision == AccessDecision.ALLOW) {
LOGGER.debug("{} of policy assignment to {} allowed with {} authorization", operationDesc, object, operationUrl);
continue;
} else {
securityEnforcer.failAuthorization("with assignment because of policies in the assignment", getRequestAuthorizationPhase(context), autzParams, result);
}
}
}

if (securityEnforcer.isAuthorized(assignActionUrl, getRequestAuthorizationPhase(context), autzParams, ownerResolver, task, result)) {
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("{} of target {} to {} allowed with {} authorization",
assignActionUrl.substring(assignActionUrl.lastIndexOf('#') + 1),
target, object, assignActionUrl);
}
LOGGER.debug("{} of target {} to {} allowed with {} authorization", operationDesc, target, object, assignActionUrl);
continue;
}
if (ObjectTypeUtil.isDelegationRelation(relation)) {
if (securityEnforcer.isAuthorized(ModelAuthorizationAction.DELEGATE.getUrl(), getRequestAuthorizationPhase(context), autzParams, ownerResolver, task, result)) {
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("{} of target {} to {} allowed with {} authorization",
assignActionUrl.substring(assignActionUrl.lastIndexOf('#') + 1),
target, object, ModelAuthorizationAction.DELEGATE.getUrl());
LOGGER.debug("{} of target {} to {} allowed with {} authorization", operationDesc, target, object, ModelAuthorizationAction.DELEGATE.getUrl());
}
continue;
}
}
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("{} of target {} to {} denied",
assignActionUrl.substring(assignActionUrl.lastIndexOf('#')),
target, object);
LOGGER.debug("{} of target {} to {} denied", operationDesc, target, object);
}
securityEnforcer.failAuthorization("with assignment", getRequestAuthorizationPhase(context), autzParams, result);
}
Expand Down
119 changes: 118 additions & 1 deletion ...ntest/src/test/java/com/evolveum/midpoint/model/intest/security/TestSecurityAdvanced.java
View file
Expand Up @@ -134,6 +134,9 @@ public class TestSecurityAdvanced extends AbstractSecurityTest {

protected static final File ROLE_PROP_EXCEPT_ADMINISTRATIVE_STATUS_FILE = new File(TEST_DIR, "role-prop-except-administrative-status.xml");
protected static final String ROLE_PROP_EXCEPT_ADMINISTRATIVE_STATUS_OID = "cc549256-02a5-11e8-994e-43c307e2a819";

protected static final File ROLE_ASSIGN_ORG_FILE = new File(TEST_DIR, "role-assign-org.xml");
protected static final String ROLE_ASSIGN_ORG_OID = "be96f834-2dbb-11e8-b29d-7f5de07e7995";


@Override
Expand All @@ -152,11 +155,12 @@ public void initSystem(Task initTask, OperationResult initResult) throws Excepti
repoAddObjectFromFile(ROLE_MODIFY_DESCRIPTION_FILE, initResult);
repoAddObjectFromFile(ROLE_PROP_EXCEPT_ASSIGNMENT_FILE, initResult);
repoAddObjectFromFile(ROLE_PROP_EXCEPT_ADMINISTRATIVE_STATUS_FILE, initResult);
repoAddObjectFromFile(ROLE_ASSIGN_ORG_FILE, initResult);

setDefaultObjectTemplate(UserType.COMPLEX_TYPE, USER_TEMPLATE_SECURITY_OID, initResult);
}

protected static final int NUMBER_OF_IMPORTED_ROLES = 9;
protected static final int NUMBER_OF_IMPORTED_ROLES = 10;

protected int getNumberOfRoles() {
return super.getNumberOfRoles() + NUMBER_OF_IMPORTED_ROLES;
Expand Down Expand Up @@ -2433,6 +2437,119 @@ public void test274AutzJackModifyPolicyExceptionSituation() throws Exception {

assertGlobalStateUntouched();
}

/**
* MID-4517
*/
@Test
public void test280AutzJackModifyPolicyExceptionAndAssignOrg() throws Exception {
final String TEST_NAME = "test280AutzJackModifyPolicyExceptionAndAssignOrg";
displayTestTitle(TEST_NAME);
// GIVEN
cleanupAutzTest(USER_JACK_OID);
assignRole(USER_JACK_OID, ROLE_LIMITED_ROLE_ADMINISTRATOR_OID);
assignRole(USER_JACK_OID, ROLE_ASSIGN_ORG_OID);
login(USER_JACK_USERNAME);

// WHEN
displayWhen(TEST_NAME);

assertGetAllow(UserType.class, USER_JACK_OID);
assertReadDenyRaw();
assertAddDeny();
assertDeleteDeny();

PrismObject<RoleType> roleEmpty = assertGetAllow(RoleType.class, ROLE_EMPTY_OID);
display("Empty role", roleEmpty);

assertAllow("add exclusion & assign org (1)",
(task, result) -> modifyRoleAddExclusionAndAssignOrg(ROLE_EMPTY_OID, ROLE_PIRATE_OID, ORG_MINISTRY_OF_RUM_OID, task, result));

PrismObject<RoleType> roleEmptyException = assertGetAllow(RoleType.class, ROLE_EMPTY_OID);
display("Empty role with exclusion and org", roleEmptyException);
assertAssignments(roleEmptyException, 2);

// TODO: delete the assignments

assertGlobalStateUntouched();
}

/**
* Partial check related to test280AutzJackModifyPolicyExceptionAndAssignOrg.
* Make sure that the operation is denied if the user does not have all the roles.
*/
@Test
public void test282AutzJackModifyPolicyExceptionAndAssignOrgDeny() throws Exception {
final String TEST_NAME = "test282AutzJackModifyPolicyExceptionAndAssignOrgDeny";
displayTestTitle(TEST_NAME);
// GIVEN
cleanupAutzTest(USER_JACK_OID);
assignRole(USER_JACK_OID, ROLE_LIMITED_ROLE_ADMINISTRATOR_OID);
login(USER_JACK_USERNAME);

// WHEN
displayWhen(TEST_NAME);

assertGetAllow(UserType.class, USER_JACK_OID);
assertReadDenyRaw();
assertAddDeny();
assertDeleteDeny();

PrismObject<RoleType> roleEmpty = assertGetAllow(RoleType.class, ROLE_EMPTY_OID);
display("Empty role", roleEmpty);

assertDeny("add policyException & assign org (1)",
(task, result) -> modifyRoleAddExclusionAndAssignOrg(ROLE_EMPTY_OID, ROLE_PIRATE_OID, ORG_MINISTRY_OF_RUM_OID, task, result));

PrismObject<RoleType> roleEmptyException = assertGetAllow(RoleType.class, ROLE_EMPTY_OID);
display("Empty role ", roleEmptyException);
assertAssignments(roleEmptyException, 0);

assertGlobalStateUntouched();
}

/**
* Partial check related to test280AutzJackModifyPolicyExceptionAndAssignOrg.
* Make sure that org assignment is allowed with just the org assign role.
*/
@Test
public void test283AutzJackModifyPolicyAssignOrg() throws Exception {
final String TEST_NAME = "test283AutzJackModifyPolicyAssignOrg";
displayTestTitle(TEST_NAME);
// GIVEN
cleanupAutzTest(USER_JACK_OID);
assignRole(USER_JACK_OID, ROLE_ASSIGN_ORG_OID);
login(USER_JACK_USERNAME);

// WHEN
displayWhen(TEST_NAME);

assertGetAllow(UserType.class, USER_JACK_OID);
assertReadDenyRaw();
assertAddDeny();
assertDeleteDeny();

PrismObject<RoleType> roleEmpty = assertGetAllow(RoleType.class, ROLE_EMPTY_OID);
display("Empty role", roleEmpty);

assertAllow("assign org (1)",
(task, result) -> assignOrg(RoleType.class, ROLE_EMPTY_OID, ORG_MINISTRY_OF_RUM_OID, task, result));

PrismObject<RoleType> roleEmptyException = assertGetAllow(RoleType.class, ROLE_EMPTY_OID);
display("Empty role ", roleEmptyException);
assertAssignments(roleEmptyException, 1);

assertGlobalStateUntouched();
}

protected void modifyRoleAddExclusionAndAssignOrg(String roleOid, String excludedRoleOid, String orgOid, Task task, OperationResult result) throws SchemaException, ObjectAlreadyExistsException, ObjectNotFoundException, ExpressionEvaluationException, CommunicationException, ConfigurationException, PolicyViolationException, SecurityViolationException {
ObjectDelta<RoleType> roleDelta = createAssignmentFocusDelta(RoleType.class, roleOid, orgOid, OrgType.COMPLEX_TYPE, null, null, null, true);
PolicyRuleType exclusionPolicyRule = createExclusionPolicyRule(excludedRoleOid);
AssignmentType assignment = new AssignmentType();
assignment.setPolicyRule(exclusionPolicyRule);
roleDelta.addModificationAddContainer(new ItemPath(RoleType.F_ASSIGNMENT), assignment);
executeChanges(roleDelta, null, task, result);
}

@Test
public void test300AutzJackExceptAssignment() throws Exception {
Expand Down
57 changes: 57 additions & 0 deletions model/model-intest/src/test/resources/security/role-assign-org.xml
View file
@@ -0,0 +1,57 @@
<!--
~ Copyright (c) 2014-2018 Evolveum
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<role oid="be96f834-2dbb-11e8-b29d-7f5de07e7995"
xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3"
xmlns:q="http://prism.evolveum.com/xml/ns/public/query-3"
xmlns:t="http://prism.evolveum.com/xml/ns/public/types-3">
<name>Assign Orgs</name>
<description>Assign any organization to any object</description>
<authorization>
<action>http://midpoint.evolveum.com/xml/ns/public/security/authorization-model-3#read</action>
<phase>request</phase>
<object>
<type>RoleType</type>
</object>
</authorization>
<authorization>
<action>http://midpoint.evolveum.com/xml/ns/public/security/authorization-model-3#read</action>
<phase>request</phase>
<object>
<type>UserType</type>
</object>
</authorization>
<authorization>
<action>http://midpoint.evolveum.com/xml/ns/public/security/authorization-model-3#assign</action>
<phase>request</phase>
<target>
<type>OrgType</type>
</target>
</authorization>
<authorization>
<action>http://midpoint.evolveum.com/xml/ns/public/security/authorization-model-3#unassign</action>
<phase>request</phase>
<target>
<type>OrgType</type>
</target>
</authorization>
<authorization>
<action>http://midpoint.evolveum.com/xml/ns/public/security/authorization-model-3#read</action>
<action>http://midpoint.evolveum.com/xml/ns/public/security/authorization-model-3#add</action>
<action>http://midpoint.evolveum.com/xml/ns/public/security/authorization-model-3#modify</action>
<action>http://midpoint.evolveum.com/xml/ns/public/security/authorization-model-3#delete</action>
<phase>execution</phase>
</authorization>
</role>

0 comments on commit 68683eb

Please sign in to comment.