diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyHelper.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyHelper.java
index 61d35c43b..090c5053f 100644
--- a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyHelper.java
+++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyHelper.java
@@ -7,8 +7,10 @@ import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.lang.reflect.Constructor;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
@@ -21,6 +23,7 @@ import org.apache.commons.logging.LogFactory;
import edu.cornell.mannlib.vitro.webapp.auth.identifier.IdentifierBundle;
import edu.cornell.mannlib.vitro.webapp.auth.identifier.RequestIdentifiers;
import edu.cornell.mannlib.vitro.webapp.auth.policy.PolicyHelper.RequiresAuthorizationFor.NoAction;
+import edu.cornell.mannlib.vitro.webapp.auth.policy.PolicyHelper.RequiresAuthorizationFor.Or;
import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.Authorization;
import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.PolicyDecision;
import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.PolicyIface;
@@ -41,6 +44,17 @@ public class PolicyHelper {
*
* Any RequestedAction can be specified, but the most common use will be to
* specify implementations of UsePagesRequestedAction.
+ *
+ * Note that a combination of AND and OR relationships can be created
+ * (at-signs converted to #-signs, so Javadoc won't try to actually apply
+ * the annotations):
+ *
+ *
+ * #RequiresAuthorizationFor(This.class)
+ * #RequiresAuthorizationFor({This.class, That.class})
+ * #RequiresAuthorizationFor(value=This.class, or=#Or(That.class))
+ * #RequiresAuthorizationFor(or={#Or(One_A.class, One_B.class), #Or(Two.class)})
+ *
*/
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@@ -49,7 +63,14 @@ public class PolicyHelper {
/* no fields */
}
+ @Retention(RetentionPolicy.RUNTIME)
+ public static @interface Or {
+ Class extends RequestedAction>[] value() default NoAction.class;
+ }
+
Class extends RequestedAction>[] value() default NoAction.class;
+
+ Or[] or() default @Or();
}
/**
@@ -57,7 +78,11 @@ public class PolicyHelper {
*/
public static boolean isRestrictedPage(VitroHttpServlet servlet) {
Class extends VitroHttpServlet> servletClass = servlet.getClass();
- return !getRequiredAuthorizationsForServlet(servletClass).isEmpty();
+ try {
+ return !ActionClauses.forServletClass(servletClass).isEmpty();
+ } catch (PolicyHelperException e) {
+ return true;
+ }
}
/**
@@ -66,8 +91,7 @@ public class PolicyHelper {
*/
public static boolean areRequiredAuthorizationsSatisfied(
HttpServletRequest req, VitroHttpServlet servlet) {
- Class extends VitroHttpServlet> servletClass = servlet.getClass();
- return areRequiredAuthorizationsSatisfied(req, servletClass);
+ return areRequiredAuthorizationsSatisfied(req, servlet.getClass());
}
/**
@@ -77,8 +101,12 @@ public class PolicyHelper {
public static boolean areRequiredAuthorizationsSatisfied(
HttpServletRequest req,
Class extends VitroHttpServlet> servletClass) {
- return areRequiredAuthorizationsSatisfied(req,
- getRequiredAuthorizationsForServlet(servletClass));
+ try {
+ return isActionClausesAuthorized(req,
+ ActionClauses.forServletClass(servletClass));
+ } catch (PolicyHelperException e) {
+ return false;
+ }
}
/**
@@ -87,132 +115,183 @@ public class PolicyHelper {
*/
public static boolean areRequiredAuthorizationsSatisfied(
HttpServletRequest req,
- Class extends RequestedAction>... actionClasses) {
- List> classList = Arrays
- .asList(actionClasses);
-
- Set actions = instantiateActions(classList);
- if (actions == null) {
- log.debug("not authorized: failed to instantiate actions");
+ Collection> actionClasses) {
+ try {
+ return isActionClausesAuthorized(req, new ActionClauses(
+ actionClasses));
+ } catch (PolicyHelperException e) {
return false;
}
-
- return areRequiredAuthorizationsSatisfied(req, actions);
- }
-
- /**
- * Are these actions authorized for the current user by the current
- * policies?
- */
- public static boolean areRequiredAuthorizationsSatisfied(
- HttpServletRequest req,
- Collection extends RequestedAction> actions) {
- PolicyIface policy = ServletPolicyList.getPolicies(req);
- IdentifierBundle ids = RequestIdentifiers.getIdBundleForRequest(req);
-
- for (RequestedAction action : actions) {
- if (isAuthorized(policy, ids, action)) {
- log.debug("not authorized");
- return false;
- }
- }
-
- log.debug("authorized");
- return true;
}
/**
* Is this action class authorized for the current user by the current
* policies?
*/
- @SuppressWarnings("unchecked")
- public static boolean isAuthorized(HttpServletRequest req,
+ public static boolean isActionAuthorized(HttpServletRequest req,
Class extends RequestedAction> actionClass) {
- return areRequiredAuthorizationsSatisfied(req, actionClass);
+ try {
+ return isActionClausesAuthorized(req,
+ new ActionClauses(actionClass));
+ } catch (PolicyHelperException e) {
+ return false;
+ }
+ }
+
+ /**
+ * Actions must be authorized for the current user by the current policies.
+ * If no actions, no problem.
+ */
+ private static boolean isActionClausesAuthorized(HttpServletRequest req,
+ ActionClauses actionClauses) {
+ PolicyIface policy = ServletPolicyList.getPolicies(req);
+ IdentifierBundle ids = RequestIdentifiers.getIdBundleForRequest(req);
+
+ return actionClauses.isEmpty()
+ || isActionClausesAuthorized(policy, ids, actionClauses);
+ }
+
+ /** Any clause in an ActionClauses may be authorized. */
+ private static boolean isActionClausesAuthorized(PolicyIface policy,
+ IdentifierBundle ids, ActionClauses actionClauses) {
+ for (Set clause : actionClauses.getClauseList()) {
+ if (isClauseAuthorized(policy, ids, clause)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /** All actions in a clause must be authorized. */
+ private static boolean isClauseAuthorized(PolicyIface policy,
+ IdentifierBundle ids, Set clause) {
+ for (RequestedAction action : clause) {
+ if (!isActionAuthorized(policy, ids, action)) {
+ log.debug("not authorized");
+ return false;
+ }
+ }
+ return true;
}
/**
* Is this action authorized for these IDs by this policy?
*/
- private static boolean isAuthorized(PolicyIface policy,
+ private static boolean isActionAuthorized(PolicyIface policy,
IdentifierBundle ids, RequestedAction action) {
PolicyDecision decision = policy.isAuthorized(ids, action);
log.debug("decision for '" + action.getClass().getName() + "' was: "
+ decision);
- return (decision == null)
- || (decision.getAuthorized() != Authorization.AUTHORIZED);
+ return (decision != null)
+ && (decision.getAuthorized() == Authorization.AUTHORIZED);
}
/**
- * What RequestedActions does this servlet require authorization for?
+ * This helper class embodies the list of OR and AND relationships for the
+ * required authorizations. A group of AND relationships is a "clause", and
+ * the list of clauses are in an OR relationship.
*
- * Keep this private, since it reveals how the Annotation is implemented. If
- * we change the Annotation to include "or" and "and", then this method
- * becomes meaningless with its current return type.
+ * Authorization is successful if ALL of the actions in ANY of the clauses
+ * are authorized, or if there are NO clauses.
+ *
+ * If any action can't be instantiated, throw an exception so authorization
+ * will fail.
*/
- private static Set getRequiredAuthorizationsForServlet(
- Class extends VitroHttpServlet> clazz) {
- Set result = new HashSet();
+ private static class ActionClauses {
+ static ActionClauses forServletClass(
+ Class extends VitroHttpServlet> servletClass)
+ throws PolicyHelperException {
+ return new ActionClauses(
+ servletClass.getAnnotation(RequiresAuthorizationFor.class));
+ }
- RequiresAuthorizationFor annotation = clazz
- .getAnnotation(RequiresAuthorizationFor.class);
+ private final List> clauseList;
- if (annotation != null) {
- for (Class extends RequestedAction> actionClass : annotation
- .value()) {
- if (NoAction.class != actionClass) {
- RequestedAction action = instantiateAction(actionClass);
- if (action != null) {
- result.add(action);
- }
+ ActionClauses(RequiresAuthorizationFor annotation)
+ throws PolicyHelperException {
+ List> list = new ArrayList>();
+ if (annotation != null) {
+ addClause(list, annotation.value());
+ for (Or orAnnotation : annotation.or()) {
+ addClause(list, orAnnotation.value());
}
}
+ this.clauseList = Collections.unmodifiableList(list);
}
- return result;
- }
- /**
- * Instantiate actions from their classes. If any one of the classes cannot
- * be instantiated, return null.
- */
- private static Set instantiateActions(
- Collection> actionClasses) {
- Set actions = new HashSet();
- for (Class extends RequestedAction> actionClass : actionClasses) {
- RequestedAction action = instantiateAction(actionClass);
- if (action == null) {
- return null;
- } else {
- actions.add(action);
+ ActionClauses(Collection> actionClasses)
+ throws PolicyHelperException {
+ this.clauseList = Collections
+ .singletonList(buildClause(actionClasses));
+ }
+
+ ActionClauses(Class extends RequestedAction> actionClass)
+ throws PolicyHelperException {
+ this.clauseList = Collections.singletonList(Collections
+ .singleton(instantiateAction(actionClass)));
+ }
+
+ private void addClause(List> list,
+ Class extends RequestedAction>[] actionClasses)
+ throws PolicyHelperException {
+ Set clause = buildClause(Arrays
+ .asList(actionClasses));
+ if (!clause.isEmpty()) {
+ list.add(clause);
}
}
- return actions;
+
+ private Set buildClause(
+ Collection> actionClasses)
+ throws PolicyHelperException {
+ Set clause = new HashSet();
+ for (Class extends RequestedAction> actionClass : actionClasses) {
+ if (!NoAction.class.equals(actionClass)) {
+ clause.add(instantiateAction(actionClass));
+ }
+ }
+ return Collections.unmodifiableSet(clause);
+ }
+
+ /**
+ * Get an instance of the RequestedAction, from its class, or throw an
+ * exception.
+ */
+ private RequestedAction instantiateAction(
+ Class extends RequestedAction> actionClass)
+ throws PolicyHelperException {
+ try {
+ Constructor extends RequestedAction> constructor = actionClass
+ .getConstructor();
+ RequestedAction instance = constructor.newInstance();
+ return instance;
+ } catch (NoSuchMethodException e) {
+ log.error("'" + actionClass.getName()
+ + "' does not have a no-argument constructor.");
+ throw new PolicyHelperException();
+ } catch (IllegalAccessException e) {
+ log.error("The no-argument constructor for '"
+ + actionClass.getName() + "' is not public.");
+ throw new PolicyHelperException();
+ } catch (Exception e) {
+ log.error("Failed to instantiate '" + actionClass.getName()
+ + "'", e);
+ throw new PolicyHelperException();
+ }
+ }
+
+ boolean isEmpty() {
+ return this.clauseList.isEmpty();
+ }
+
+ List> getClauseList() {
+ return this.clauseList;
+ }
}
- /**
- * Get an instance of the RequestedAction, from its class. If the class
- * cannot be instantiated, return null.
- */
- private static RequestedAction instantiateAction(
- Class extends RequestedAction> actionClass) {
- try {
- Constructor extends RequestedAction> constructor = actionClass
- .getConstructor();
- RequestedAction instance = constructor.newInstance();
- return instance;
- } catch (NoSuchMethodException e) {
- log.error("'" + actionClass.getName()
- + "' does not have a no-argument constructor.");
- return null;
- } catch (IllegalAccessException e) {
- log.error("The no-argument constructor for '"
- + actionClass.getName() + "' is not public.");
- return null;
- } catch (Exception e) {
- log.error("Failed to instantiate '" + actionClass.getName() + "'",
- e);
- return null;
- }
+ /** We failed to instantiate a RequestedAction */
+ private static class PolicyHelperException extends Exception {
+ // no members
}
/**
diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/VitroHttpServlet.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/VitroHttpServlet.java
index 76a3ba9a4..22dbd5c8e 100644
--- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/VitroHttpServlet.java
+++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/VitroHttpServlet.java
@@ -69,8 +69,10 @@ public class VitroHttpServlet extends HttpServlet {
if (!PolicyHelper.areRequiredAuthorizationsSatisfied(hreq, this)) {
if (LoginStatusBean.getBean(hreq).isLoggedIn()) {
redirectToInsufficientAuthorizationPage(hreq, hresp);
+ return;
} else {
redirectToLoginPage(hreq, hresp);
+ return;
}
}
}
@@ -164,37 +166,6 @@ public class VitroHttpServlet extends HttpServlet {
}
}
- /**
- * If none of these actions are authorized by the current policy, redirect
- * them to the appropriate page.
- *
- * Currently the RequiresAuthorizationFor annotation can't handle "or"
- * situations, so we need to do an explicit call to this method. You should
- * still use the annotation with no actions, so we know this is a restricted
- * page when we logout.
- */
- public static boolean checkIfAnyActionsAreAuthorized(
- HttpServletRequest request, HttpServletResponse response,
- Class extends RequestedAction>... actionClasses) {
- for (Class extends RequestedAction> actionClass : actionClasses) {
- if (PolicyHelper.isAuthorized(request, actionClass)) {
- log.trace("Authorized for '" + actionClass.getSimpleName()
- + "'");
- return true;
- }
- }
- LoginStatusBean statusBean = LoginStatusBean.getBean(request);
- if (statusBean.isLoggedIn()) {
- log.trace("Authorization is insufficient for requested actions");
- redirectToInsufficientAuthorizationPage(request, response);
- return false;
- } else {
- log.trace("Not logged in; not sufficient for requested actions");
- redirectToLoginPage(request, response);
- return false;
- }
- }
-
/**
* Logged in, but with insufficent authorization. Send them to the home page
* with a message. They won't be coming back.
diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/jena/JenaExportController.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/jena/JenaExportController.java
index 3476b63a8..47fa100ff 100644
--- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/jena/JenaExportController.java
+++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/jena/JenaExportController.java
@@ -20,6 +20,7 @@ import com.hp.hpl.jena.shared.Lock;
import edu.cornell.mannlib.vedit.controller.BaseEditController;
import edu.cornell.mannlib.vitro.webapp.auth.policy.PolicyHelper.RequiresAuthorizationFor;
+import edu.cornell.mannlib.vitro.webapp.auth.policy.PolicyHelper.RequiresAuthorizationFor.Or;
import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.usepages.UseAdvancedDataToolsPages;
import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.usepages.UseOntologyEditorPages;
import edu.cornell.mannlib.vitro.webapp.controller.Controllers;
@@ -28,17 +29,13 @@ import edu.cornell.mannlib.vitro.webapp.dao.jena.JenaModelUtils;
import edu.cornell.mannlib.vitro.webapp.dao.jena.ModelContext;
import edu.cornell.mannlib.vitro.webapp.servlet.setup.JenaDataSourceSetupBase;
-@RequiresAuthorizationFor(/* either-or; see call to checkIfAnyActionsAreAuthorized */)
+@RequiresAuthorizationFor(or={@Or(UseAdvancedDataToolsPages.class), @Or(UseOntologyEditorPages.class)})
public class JenaExportController extends BaseEditController {
+ @Override
public void doGet( HttpServletRequest request, HttpServletResponse response ) {
VitroRequest vreq = new VitroRequest(request);
- if (!checkIfAnyActionsAreAuthorized(vreq, response,
- UseAdvancedDataToolsPages.class, UseOntologyEditorPages.class)) {
- return;
- }
-
if ( vreq.getRequestURL().indexOf("/download/") > -1 ) {
outputRDF( vreq, response );
return;
diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/web/jsptags/RequiresAuthorizationFor.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/web/jsptags/RequiresAuthorizationFor.java
index 5d780452c..055f40329 100644
--- a/webapp/src/edu/cornell/mannlib/vitro/webapp/web/jsptags/RequiresAuthorizationFor.java
+++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/web/jsptags/RequiresAuthorizationFor.java
@@ -2,7 +2,6 @@
package edu.cornell.mannlib.vitro.webapp.web.jsptags;
-import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
@@ -57,12 +56,12 @@ public class RequiresAuthorizationFor extends BodyTagSupport {
* are authorized for those actions.
*/
private boolean isAuthorized() {
- Collection actions = instantiateActions();
- if (actions == null) {
+ Set> classes = instantiateActionClasses();
+ if (classes == null) {
return false;
}
return PolicyHelper.areRequiredAuthorizationsSatisfied(getRequest(),
- actions);
+ classes);
}
/**
@@ -71,18 +70,13 @@ public class RequiresAuthorizationFor extends BodyTagSupport {
*
* If we can't do all of that, complain and return null.
*/
- private Set instantiateActions() {
+ private Set> instantiateActionClasses() {
Set classNames = parseClassNames();
if (classNames.isEmpty()) {
return Collections.emptySet();
}
- Set> actionClasses = loadClassesAndCheckTypes(classNames);
- if (actionClasses == null) {
- return null;
- }
-
- return getInstancesFromClasses(actionClasses);
+ return loadClassesAndCheckTypes(classNames);
}
private Set parseClassNames() {
diff --git a/webapp/test/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyHelperTest.java b/webapp/test/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyHelperTest.java
index 3cead99c1..91c3ab376 100644
--- a/webapp/test/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyHelperTest.java
+++ b/webapp/test/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyHelperTest.java
@@ -2,7 +2,8 @@
package edu.cornell.mannlib.vitro.webapp.auth.policy;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
import java.util.Arrays;
import java.util.HashSet;
@@ -18,6 +19,7 @@ import stubs.javax.servlet.http.HttpSessionStub;
import edu.cornell.mannlib.vitro.testing.AbstractTestClass;
import edu.cornell.mannlib.vitro.webapp.auth.identifier.IdentifierBundle;
import edu.cornell.mannlib.vitro.webapp.auth.policy.PolicyHelper.RequiresAuthorizationFor;
+import edu.cornell.mannlib.vitro.webapp.auth.policy.PolicyHelper.RequiresAuthorizationFor.Or;
import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.Authorization;
import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.PolicyDecision;
import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.PolicyIface;
@@ -80,21 +82,70 @@ public class PolicyHelperTest extends AbstractTestClass {
public void twoRequirementsFailOne() {
createPolicy(new Action1());
assertExpectedAuthorization("requires Actions 1 and 2",
- Action1Action2Servlet.class, false);
+ Action1AndAction2Servlet.class, false);
}
@Test
public void twoRequirementsFailTwo() {
createPolicy(new Action2());
assertExpectedAuthorization("requires Actions 1 and 2",
- Action1Action2Servlet.class, false);
+ Action1AndAction2Servlet.class, false);
}
@Test
public void twoRequirementsSucceed() {
createPolicy(new Action2(), new Action1());
assertExpectedAuthorization("requires Actions 1 and 2",
- Action1Action2Servlet.class, true);
+ Action1AndAction2Servlet.class, true);
+ }
+
+ @Test
+ public void oneOrTwoFail() {
+ createPolicy();
+ assertExpectedAuthorization("requires Action 1 or 2",
+ Action1OrAction2Servlet.class, false);
+ }
+
+ @Test
+ public void oneOrTwoSucceedOne() {
+ createPolicy(new Action1());
+ assertExpectedAuthorization("requires Action 1 or 2",
+ Action1OrAction2Servlet.class, true);
+ }
+
+ @Test
+ public void oneOrTwoSucceedTwo() {
+ createPolicy(new Action2());
+ assertExpectedAuthorization("requires Action 1 or 2",
+ Action1OrAction2Servlet.class, true);
+ }
+
+ @Test
+ public void oneOrTwoOrThreeFail() {
+ createPolicy();
+ assertExpectedAuthorization("requires Action 1 or 2 or 3",
+ Action1OrAction2OrAction3Servlet.class, false);
+ }
+
+ @Test
+ public void oneOrTwoOrThreeSucceedOne() {
+ createPolicy(new Action1());
+ assertExpectedAuthorization("requires Action 1 or 2 or 3",
+ Action1OrAction2OrAction3Servlet.class, true);
+ }
+
+ @Test
+ public void oneOrTwoOrThreeSucceedTwo() {
+ createPolicy(new Action2());
+ assertExpectedAuthorization("requires Action 1 or 2 or 3",
+ Action1OrAction2OrAction3Servlet.class, true);
+ }
+
+ @Test
+ public void oneOrTwoOrThreeSucceedThree() {
+ createPolicy(new Action3());
+ assertExpectedAuthorization("requires Action 1 or 2 or 3",
+ Action1OrAction2OrAction3Servlet.class, true);
}
// ----------------------------------------------------------------------
@@ -124,6 +175,10 @@ public class PolicyHelperTest extends AbstractTestClass {
// actions must be public, with public constructor
}
+ public static class Action3 extends RequestedAction {
+ // actions must be public, with public constructor
+ }
+
// no annotation
private static class NoAnnotationServlet extends VitroHttpServlet {
/* no body */
@@ -140,7 +195,19 @@ public class PolicyHelperTest extends AbstractTestClass {
}
@RequiresAuthorizationFor({ Action1.class, Action2.class })
- private static class Action1Action2Servlet extends VitroHttpServlet {
+ private static class Action1AndAction2Servlet extends VitroHttpServlet {
+ /* no body */
+ }
+
+ @RequiresAuthorizationFor(value = Action1.class, or = @Or(Action2.class))
+ private static class Action1OrAction2Servlet extends VitroHttpServlet {
+ /* no body */
+ }
+
+ @RequiresAuthorizationFor(value = Action1.class, or = { @Or(Action2.class),
+ @Or(Action3.class) })
+ private static class Action1OrAction2OrAction3Servlet extends
+ VitroHttpServlet {
/* no body */
}