From b82cf72807766dc4617cb89a1a633f906bf1c91d Mon Sep 17 00:00:00 2001 From: j2blake Date: Tue, 26 Apr 2011 18:20:25 +0000 Subject: [PATCH] NIHVIVO-2492 Modify authorizations based on action-classes to be based on actions instead. --- .../webapp/auth/policy/PolicyHelper.java | 199 ------------------ .../controller/FakeSelfEditController.java | 3 +- .../freemarker/SiteAdminController.java | 6 +- .../webapp/auth/policy/PolicyHelperTest.java | 160 -------------- 4 files changed, 4 insertions(+), 364 deletions(-) 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 8f33dd614..bbfd84f75 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 @@ -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 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 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 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 servletClass) - throws PolicyHelperException { - return new ActionClauses( - servletClass.getAnnotation(RequiresAuthorizationFor.class)); - } - - private final List> clauseList; - - 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); - } - - ActionClauses(Collection> actionClasses) - throws PolicyHelperException { - this.clauseList = Collections - .singletonList(buildClause(actionClasses)); - } - - ActionClauses(Class actionClass) - throws PolicyHelperException { - this.clauseList = Collections.singletonList(Collections - .singleton(instantiateAction(actionClass))); - } - - ActionClauses(RequestedAction[] actions) { - HashSet actionSet = new HashSet( - Arrays.asList(actions)); - this.clauseList = Collections.singletonList(Collections - .unmodifiableSet(actionSet)); - } - - private void addClause(List> list, - Class[] actionClasses) - throws PolicyHelperException { - Set clause = buildClause(Arrays - .asList(actionClasses)); - if (!clause.isEmpty()) { - list.add(clause); - } - } - - private Set buildClause( - Collection> actionClasses) - throws PolicyHelperException { - Set clause = new HashSet(); - for (Class 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 actionClass) - throws PolicyHelperException { - try { - Constructor 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; - } - } - - /** 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. */ diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/FakeSelfEditController.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/FakeSelfEditController.java index 46e14ae71..80a50ae42 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/FakeSelfEditController.java +++ b/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; } diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/freemarker/SiteAdminController.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/freemarker/SiteAdminController.java index 34322eccb..730b7a8b5 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/freemarker/SiteAdminController.java +++ b/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. 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 1d060ed9f..0d2cf12fe 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 @@ -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()))); } - /** - * 
-	 * 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)
-	 *
- */ - - // ---------------------------------------------------------------------- - // ---------------------------------------------------------------------- - // ---------------------------------------------------------------------- - // 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 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 authorizedActions;