Skip to content

Commit

Permalink
Fixing authorizations of empty objects (MID-4369)
Browse files Browse the repository at this point in the history
  • Loading branch information
semancik committed Feb 12, 2018
1 parent ff82167 commit 88efae4
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 7 deletions.
Expand Up @@ -20,6 +20,7 @@
import static org.testng.AssertJUnit.assertNotNull;
import static org.testng.AssertJUnit.assertNull;
import static org.testng.AssertJUnit.assertTrue;
import static org.testng.AssertJUnit.assertFalse;

import java.io.File;
import java.io.FileInputStream;
Expand Down Expand Up @@ -76,11 +77,14 @@
import com.evolveum.midpoint.schema.util.MiscSchemaUtil;
import com.evolveum.midpoint.security.api.Authorization;
import com.evolveum.midpoint.security.api.MidPointPrincipal;
import com.evolveum.midpoint.security.api.OwnerResolver;
import com.evolveum.midpoint.security.enforcer.api.AuthorizationParameters;
import com.evolveum.midpoint.task.api.Task;
import com.evolveum.midpoint.test.util.TestUtil;
import com.evolveum.midpoint.util.FailableProcessor;
import com.evolveum.midpoint.util.Holder;
import com.evolveum.midpoint.util.MiscUtil;
import com.evolveum.midpoint.util.QNameUtil;
import com.evolveum.midpoint.util.exception.CommunicationException;
import com.evolveum.midpoint.util.exception.ConfigurationException;
import com.evolveum.midpoint.util.exception.ExpressionEvaluationException;
Expand All @@ -96,6 +100,7 @@
import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentPolicyEnforcementType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.AuthorizationDecisionType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.AuthorizationPhaseType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.AuthorizationType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.CredentialsType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.FocusType;
Expand Down Expand Up @@ -1374,6 +1379,24 @@ protected <O extends ObjectType> void logError(String action, Class<O> type, Str
LOGGER.info(msg);
}

protected <O extends ObjectType, T extends ObjectType> void assertIsAuthorized(String operationUrl, AuthorizationPhaseType phase, AuthorizationParameters<O,T> params, OwnerResolver ownerResolver) throws SchemaException, ObjectNotFoundException, ExpressionEvaluationException, CommunicationException, ConfigurationException, SecurityViolationException {
Task task = taskManager.createTaskInstance(AbstractSecurityTest.class.getName() + ".assertIsAuthorized");
OperationResult result = task.getResult();
boolean authorized = securityEnforcer.isAuthorized(operationUrl, phase, params, ownerResolver, task, result);
assertTrue("Expected isAuthorized for "+QNameUtil.uriToQName(operationUrl).getLocalPart()+" with "+params+", but we are not authorized", authorized);
result.computeStatus();
TestUtil.assertSuccess(result);
}

protected <O extends ObjectType, T extends ObjectType> void assertIsNotAuthorized(String operationUrl, AuthorizationPhaseType phase, AuthorizationParameters<O,T> params, OwnerResolver ownerResolver) throws SchemaException, ObjectNotFoundException, ExpressionEvaluationException, CommunicationException, ConfigurationException, SecurityViolationException {
Task task = taskManager.createTaskInstance(AbstractSecurityTest.class.getName() + ".assertIsAuthorized");
OperationResult result = task.getResult();
boolean authorized = securityEnforcer.isAuthorized(operationUrl, phase, params, ownerResolver, task, result);
assertFalse("Expected not isAuthorized for "+QNameUtil.uriToQName(operationUrl).getLocalPart()+" with "+params+", but we are authorized", authorized);
result.computeStatus();
TestUtil.assertSuccess(result);
}

protected <O extends ObjectType> void asAdministrator(Attempt attempt) throws Exception {
Task task = taskManager.createTaskInstance(AbstractSecurityTest.class.getName() + ".asAdministrator");
OperationResult result = task.getResult();
Expand Down
Expand Up @@ -15,6 +15,8 @@
*/
package com.evolveum.midpoint.model.intest.security;

import static org.testng.AssertJUnit.assertFalse;
import static org.testng.AssertJUnit.assertTrue;
import static org.testng.AssertJUnit.assertNotNull;
import static org.testng.AssertJUnit.assertNull;
import static org.testng.AssertJUnit.assertEquals;
Expand Down Expand Up @@ -52,6 +54,7 @@
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.schema.util.MiscSchemaUtil;
import com.evolveum.midpoint.security.api.MidPointPrincipal;
import com.evolveum.midpoint.security.enforcer.api.AuthorizationParameters;
import com.evolveum.midpoint.task.api.Task;
import com.evolveum.midpoint.test.IntegrationTestTools;
import com.evolveum.midpoint.util.exception.CommunicationException;
Expand All @@ -66,6 +69,7 @@
import com.evolveum.midpoint.xml.ns._public.common.common_3.ActivationType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentPolicyEnforcementType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.AuthorizationPhaseType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ConstructionType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ExclusionPolicyConstraintType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.FocusType;
Expand Down Expand Up @@ -2124,7 +2128,6 @@ public void test262AutzJackLimitedRoleAdministratorAndAssignApplicationRoles() t
(task, result) -> deleteObject(RoleType.class, ROLE_EMPTY_OID));

// MID-4443
display("TTT1");
assertAddAllow(ROLE_EMPTY_FILE);

asAdministrator(
Expand All @@ -2139,9 +2142,24 @@ public void test262AutzJackLimitedRoleAdministratorAndAssignApplicationRoles() t
roleEmpty2.asObjectable().getAssignment().add(appliationRoleAssignment);

// MID-4443
display("TTT2");
assertAllow("Add empty role with application role assignment",
(task, result) -> addObject(roleEmpty2));

// MID-4369
// Empty role as object. Really empty: no items there at all.
PrismObject<RoleType> testRoleObject = prismContext.getSchemaRegistry().findObjectDefinitionByCompileTimeClass(RoleType.class).instantiate();
// There are no items in this empty role. Therefore there is no item that is allowed. But also no item that
// is denied or not allowed. This is a corner case. But this approach is often used by GUI to determine if
// a specific class of object is allowed, e.g. if it is allowed to create (some) roles. This is used to
// determine whether to display a particular menu item.
assertTrue(testRoleObject.isEmpty());
assertIsAuthorized(ModelAuthorizationAction.ADD.getUrl(), AuthorizationPhaseType.REQUEST,
AuthorizationParameters.Builder.buildObject(testRoleObject), null);

testRoleObject.asObjectable().setRiskLevel("hazardous");
assertFalse(testRoleObject.isEmpty());
assertIsNotAuthorized(ModelAuthorizationAction.ADD.getUrl(), AuthorizationPhaseType.REQUEST,
AuthorizationParameters.Builder.buildObject(testRoleObject), null);

assertGlobalStateUntouched();
}
Expand Down
8 changes: 4 additions & 4 deletions model/model-intest/src/test/resources/logback-test.xml
Expand Up @@ -82,7 +82,7 @@
<logger name="com.evolveum.midpoint.provisioning" level="DEBUG" />
<logger name="com.evolveum.midpoint.provisioning.impl.ResourceManager" level="DEBUG" />
<logger name="com.evolveum.midpoint.provisioning.impl.ShadowCache" level="DEBUG" />
<logger name="com.evolveum.midpoint.provisioning.impl.ShadowManager" level="TRACE" />
<logger name="com.evolveum.midpoint.provisioning.impl.ShadowManager" level="DEBUG" />
<logger name="com.evolveum.midpoint.provisioning.consistency" level="DEBUG" />
<logger name="com.evolveum.midpoint.expression" level="DEBUG" />
<logger name="com.evolveum.midpoint.model.common.expression" level="DEBUG" />
Expand All @@ -94,8 +94,8 @@
<logger name="com.evolveum.midpoint.model.common.stringpolicy.ValuePolicyGenerator" level="DEBUG" />
<logger name="com.evolveum.midpoint.model.impl.controller.ObjectMerger" level="DEBUG" />
<logger name="com.evolveum.midpoint.notifications" level="DEBUG" />
<logger name="com.evolveum.midpoint.security" level="TRACE" />
<logger name="com.evolveum.midpoint.security.enforcer.impl.SecurityEnforcerImpl" level="TRACE" />
<logger name="com.evolveum.midpoint.security" level="DEBUG" />
<logger name="com.evolveum.midpoint.security.enforcer.impl.SecurityEnforcerImpl" level="DEBUG" />
<logger name="com.evolveum.midpoint.model.impl.security" level="DEBUG" />
<logger name="com.evolveum.midpoint.model.impl.util.AbstractSearchIterativeTaskHandler" level="DEBUG" />
<logger name="com.evolveum.midpoint.model.impl.sync.SynchronizationServiceImpl" level="DEBUG" />
Expand All @@ -105,7 +105,7 @@
<logger name="com.evolveum.midpoint.model.impl.sync.SynchronizeAccountResultHandler" level="DEBUG" />
<logger name="com.evolveum.midpoint.model.impl.controller.ModelController" level="DEBUG" />
<logger name="com.evolveum.midpoint.model.impl.controller.ModelInteractionServiceImpl" level="DEBUG" />
<logger name="com.evolveum.midpoint.model.impl.controller.SchemaTransformer" level="TRACE" />
<logger name="com.evolveum.midpoint.model.impl.controller.SchemaTransformer" level="DEBUG" />
<logger name="com.evolveum.midpoint.model.impl.importer" level="DEBUG" />
<logger name="com.evolveum.midpoint.common.validator" level="DEBUG" />
<logger name="com.evolveum.icf.dummy" level="INFO" />
Expand Down
Expand Up @@ -298,7 +298,17 @@ private AccessDecision decideAllowedItems(final ItemPath itemPath, final AutzIte
}

private <O extends ObjectType> AccessDecision determineObjectDecision(PrismContainer<O> object, ItemDecisionFunction itemDecitionFunction) {
return determineContainerDecision(object.getValue(), itemDecitionFunction, false, "object");
AccessDecision containerDecision = determineContainerDecision(object.getValue(), itemDecitionFunction, false, "object");
if (containerDecision == null && object.isEmpty()) {
// There are no items in the object. Therefore there is no item that is allowed. Therefore decision is DEFAULT.
// But also there is no item that is denied or not allowed.
// This is a corner case. But this approach is often used by GUI to determine if
// a specific class of object is allowed, e.g. if it is allowed to create (some) roles. This is used to
// determine whether to display a particular menu item.
// Therefore we should allow such cases.
return AccessDecision.ALLOW;
}
return containerDecision;
}

private <C extends Containerable, O extends ObjectType> AccessDecision determineContainerDeltaDecision(ContainerDelta<C> cdelta, PrismObject<O> currentObject, ItemDecisionFunction itemDecitionFunction) {
Expand Down

0 comments on commit 88efae4

Please sign in to comment.