NIHVIVO-2492 Modify authorizations based on action-classes to be based on actions instead.

webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyHelper.java

@@ -6,16 +6,7 @@ import java.lang.annotation.ElementType;
 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;
 
-import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 
 import org.apache.commons.logging.Log;
@@ -23,10 +14,6 @@ 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;
 import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.Actions;
 import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.ifaces.RequestedAction;
@@ -102,192 +89,6 @@ public class PolicyHelper {
 		Or[] or() default @Or();
 	}
 
-	/**
-	 * Is this action class authorized for the current user by the current
-	 * policies?
-	 */
-	public static boolean isAuthorizedForAction(HttpServletRequest req,
-			Class<? extends RequestedAction> actionClass) {
-		try {
-			return isAuthorizedForActionClauses(req, new ActionClauses(
-					actionClass));
-		} catch (PolicyHelperException e) {
-			return false;
-		}
-	}
-
-	/**
-	 * Are these actions authorized for the current user by the current
-	 * policies?
-	 */
-	public static boolean isAuthorizedForAction(HttpServletRequest req,
-			RequestedAction... actions) {
-		return isAuthorizedForActionClauses(req, new ActionClauses(actions));
-	}
-
-	/**
-	 * Actions must be authorized for the current user by the current policies.
-	 * If no actions, no problem.
-	 */
-	private static boolean isAuthorizedForActionClauses(HttpServletRequest req,
-			ActionClauses actionClauses) {
-		PolicyIface policy = ServletPolicyList.getPolicies(req);
-		IdentifierBundle ids = RequestIdentifiers.getIdBundleForRequest(req);
-
-		return actionClauses.isEmpty()
-				|| isAuthorizedForActionClauses(policy, ids, actionClauses);
-	}
-
-	/** Any clause in an ActionClauses may be authorized. */
-	private static boolean isAuthorizedForActionClauses(PolicyIface policy,
-			IdentifierBundle ids, ActionClauses actionClauses) {
-		for (Set<RequestedAction> clause : actionClauses.getClauseList()) {
-			if (isAuthorizedForClause(policy, ids, clause)) {
-				return true;
-			}
-		}
-		return false;
-	}
-
-	/** All actions in a clause must be authorized. */
-	private static boolean isAuthorizedForClause(PolicyIface policy,
-			IdentifierBundle ids, Set<RequestedAction> clause) {
-		for (RequestedAction action : clause) {
-			if (!isAuthorizedForAction(policy, ids, action)) {
-				log.debug("not authorized");
-				return false;
-			}
-		}
-		return true;
-	}
-
-	/**
-	 * Is this action authorized for these IDs by this policy?
-	 */
-	private static boolean isAuthorizedForAction(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);
-	}
-
-	/**
-	 * 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.
-	 * 
-	 * 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 class ActionClauses {
-		static ActionClauses forServletClass(
-				Class<? extends HttpServlet> servletClass)
-				throws PolicyHelperException {
-			return new ActionClauses(
-					servletClass.getAnnotation(RequiresAuthorizationFor.class));
-		}
-
-		private final List<Set<RequestedAction>> clauseList;
-
-		ActionClauses(RequiresAuthorizationFor annotation)
-				throws PolicyHelperException {
-			List<Set<RequestedAction>> list = new ArrayList<Set<RequestedAction>>();
-			if (annotation != null) {
-				addClause(list, annotation.value());
-				for (Or orAnnotation : annotation.or()) {
-					addClause(list, orAnnotation.value());
-				}
-			}
-			this.clauseList = Collections.unmodifiableList(list);
-		}
-
-		ActionClauses(Collection<Class<? extends RequestedAction>> actionClasses)
-				throws PolicyHelperException {
-			this.clauseList = Collections
-					.singletonList(buildClause(actionClasses));
-		}
-
-		ActionClauses(Class<? extends RequestedAction> actionClass)
-				throws PolicyHelperException {
-			this.clauseList = Collections.singletonList(Collections
-					.singleton(instantiateAction(actionClass)));
-		}
-
-		ActionClauses(RequestedAction[] actions) {
-			HashSet<RequestedAction> actionSet = new HashSet<RequestedAction>(
-					Arrays.asList(actions));
-			this.clauseList = Collections.singletonList(Collections
-					.unmodifiableSet(actionSet));
-		}
-
-		private void addClause(List<Set<RequestedAction>> list,
-				Class<? extends RequestedAction>[] actionClasses)
-				throws PolicyHelperException {
-			Set<RequestedAction> clause = buildClause(Arrays
-					.asList(actionClasses));
-			if (!clause.isEmpty()) {
-				list.add(clause);
-			}
-		}
-
-		private Set<RequestedAction> buildClause(
-				Collection<Class<? extends RequestedAction>> actionClasses)
-				throws PolicyHelperException {
-			Set<RequestedAction> clause = new HashSet<RequestedAction>();
-			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<Set<RequestedAction>> getClauseList() {
-			return this.clauseList;
-		}
-	}
-
-	/** We failed to instantiate a RequestedAction */
-	private static class PolicyHelperException extends Exception {
-		// no members
-	}
-
 	/**
 	 * No need to instantiate this helper class - all methods are static.
 	 */

webapp/src/edu/cornell/mannlib/vitro/webapp/controller/FakeSelfEditController.java

@@ -18,7 +18,6 @@ import edu.cornell.mannlib.vitro.webapp.auth.policy.PolicyHelper;
 import edu.cornell.mannlib.vitro.webapp.auth.policy.PolicyHelper.RequiresAuthorizationFor;
 import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.usepages.UseMiscellaneousAdminPages;
 
-@RequiresAuthorizationFor(/* restricted page, but checking is done internally. */)
 /**
  * TODO This is caught in the middle of the transition from LoginFormBean to LoginStatusBean.
  */
@@ -60,7 +59,7 @@ public class FakeSelfEditController extends VitroHttpServlet {
 
 	private boolean isAuthorized(VitroRequest vreq, HttpSession session) {
 		boolean isFakingAlready = (session.getAttribute(ATTRIBUTE_LOGIN_STATUS_SAVE) != null);
-		boolean isAdmin = PolicyHelper.isAuthorizedForAction(vreq, UseMiscellaneousAdminPages.class);
+		boolean isAdmin = PolicyHelper.isAuthorizedForActions(vreq, new UseMiscellaneousAdminPages());
 		log.debug("isFakingAlready: " + isFakingAlready + ", isAdmin: "	+ isAdmin);
 		return isAdmin || isFakingAlready;
 	}

webapp/src/edu/cornell/mannlib/vitro/webapp/controller/freemarker/SiteAdminController.java

@@ -56,7 +56,7 @@ public class SiteAdminController extends FreemarkerHttpServlet {
 
         UrlBuilder urlBuilder = new UrlBuilder(vreq.getPortal());
         
-    	if (PolicyHelper.isAuthorizedForAction(vreq, UseIndividualEditorPages.class)) {
+    	if (PolicyHelper.isAuthorizedForActions(vreq, new UseIndividualEditorPages())) {
     		body.put("dataInput", getDataInputData(vreq));
     	}
 
@@ -66,10 +66,10 @@ public class SiteAdminController extends FreemarkerHttpServlet {
         // of step with the levels required by the pages themselves. We should implement a 
         // mechanism similar to what's used on the front end to display links to Site Admin
         // and Revision Info iff the user has access to those pages.
-        if (PolicyHelper.isAuthorizedForAction(vreq, UseOntologyEditorPages.class)) {
+        if (PolicyHelper.isAuthorizedForActions(vreq, new UseOntologyEditorPages())) {
         	body.put("ontologyEditor", getOntologyEditorData(vreq, urlBuilder));
         }
-		if (PolicyHelper.isAuthorizedForAction(vreq, UseAdvancedDataToolsPages.class)) {
+		if (PolicyHelper.isAuthorizedForActions(vreq, new UseAdvancedDataToolsPages())) {
             body.put("dataTools", getDataToolsData(vreq, urlBuilder));
             
             // Only for DataStar. Should handle without needing a DataStar-specific version of this controller.

webapp/test/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyHelperTest.java

@@ -17,14 +17,11 @@ import stubs.javax.servlet.http.HttpServletRequestStub;
 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;
 import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.Actions;
 import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.ifaces.RequestedAction;
-import edu.cornell.mannlib.vitro.webapp.controller.VitroHttpServlet;
 
 /**
  * Test the basic top-level function of PolicyHelper.
@@ -96,125 +93,6 @@ public class PolicyHelperTest extends AbstractTestClass {
 						new Action1(), new Action2()).or(new Action3())));
 	}
 
-	/**
-	 * <pre>
-	 * actions is null, 
-	 * actions is empty
-	 * actions has one clause with multiple actions
-	 * 	   all pass
-	 *     some pass
-	 * action has multiple clauses
-	 *     one passes
-	 *     none pass (but partial passes)
-	 * </pre>
-	 */
-
-	// ----------------------------------------------------------------------
-	// ----------------------------------------------------------------------
-	// ----------------------------------------------------------------------
-	// Obsolete???
-	// ----------------------------------------------------------------------
-	// ----------------------------------------------------------------------
-	// ----------------------------------------------------------------------
-
-//	@Test
-//	public void noAnnotation() {
-//		createPolicy();
-//		assertExpectedAuthorization("no actions required",
-//				NoAnnotationServlet.class, true);
-//	}
-//
-//	@Test
-//	public void noRequirements() {
-//		createPolicy();
-//		assertExpectedAuthorization("no actions required",
-//				NoRequirementsServlet.class, true);
-//	}
-//
-//	@Test
-//	public void oneRequirementFail() {
-//		createPolicy();
-//		assertExpectedAuthorization("requires Action1", Action1Servlet.class,
-//				false);
-//	}
-//
-//	@Test
-//	public void oneRequirementSucceed() {
-//		createPolicy(new Action1());
-//		assertExpectedAuthorization("requires Action1", Action1Servlet.class,
-//				true);
-//	}
-//
-//	@Test
-//	public void twoRequirementsFailOne() {
-//		createPolicy(new Action1());
-//		assertExpectedAuthorization("requires Actions 1 and 2",
-//				Action1AndAction2Servlet.class, false);
-//	}
-//
-//	@Test
-//	public void twoRequirementsFailTwo() {
-//		createPolicy(new Action2());
-//		assertExpectedAuthorization("requires Actions 1 and 2",
-//				Action1AndAction2Servlet.class, false);
-//	}
-//
-//	@Test
-//	public void twoRequirementsSucceed() {
-//		createPolicy(new Action2(), new Action1());
-//		assertExpectedAuthorization("requires Actions 1 and 2",
-//				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);
-//	}
-
 	// ----------------------------------------------------------------------
 	// Helper methods
 	// ----------------------------------------------------------------------
@@ -223,12 +101,6 @@ public class PolicyHelperTest extends AbstractTestClass {
 		ServletPolicyList.addPolicy(ctx, new MySimplePolicy(authorizedActions));
 	}
 
-//	private void assertExpectedAuthorization(String label,
-//			Class<? extends VitroHttpServlet> servletClass, boolean expected) {
-//		boolean actual = PolicyHelper.isAuthorizedForServlet(req, servletClass);
-//		assertEquals(label, expected, actual);
-//	}
-
 	// ----------------------------------------------------------------------
 	// Helper Classes
 	// ----------------------------------------------------------------------
@@ -245,38 +117,6 @@ public class PolicyHelperTest extends AbstractTestClass {
 		// actions must be public, with public constructor
 	}
 
-	// no annotation
-	private static class NoAnnotationServlet extends VitroHttpServlet {
-		/* no body */
-	}
-
-	@RequiresAuthorizationFor
-	private static class NoRequirementsServlet extends VitroHttpServlet {
-		/* no body */
-	}
-
-	@RequiresAuthorizationFor(Action1.class)
-	private static class Action1Servlet extends VitroHttpServlet {
-		/* no body */
-	}
-
-	@RequiresAuthorizationFor({ Action1.class, Action2.class })
-	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 */
-	}
-
 	private static class MySimplePolicy implements PolicyIface {
 		private final Set<RequestedAction> authorizedActions;