From f2269120d7f7a7da35c13e4ba209088ac96fce5c Mon Sep 17 00:00:00 2001 From: j2blake Date: Tue, 19 Apr 2011 14:12:51 +0000 Subject: [PATCH] NIHVIVO-2492 Improve method names --- .../webapp/auth/policy/PolicyHelper.java | 38 +++++++++---------- .../webapp/controller/VitroHttpServlet.java | 7 ++-- .../freemarker/SiteAdminController.java | 6 +-- .../web/jsptags/RequiresAuthorizationFor.java | 3 +- .../webapp/auth/policy/PolicyHelperTest.java | 4 +- 5 files changed, 26 insertions(+), 32 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 090c5053f..4041b7a1a 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 @@ -76,7 +76,7 @@ public class PolicyHelper { /** * Does this servlet require authorization? */ - public static boolean isRestrictedPage(VitroHttpServlet servlet) { + public static boolean isServletRestricted(VitroHttpServlet servlet) { Class servletClass = servlet.getClass(); try { return !ActionClauses.forServletClass(servletClass).isEmpty(); @@ -89,20 +89,19 @@ public class PolicyHelper { * Are the actions that this servlet requires authorized for the current * user by the current policies? */ - public static boolean areRequiredAuthorizationsSatisfied( - HttpServletRequest req, VitroHttpServlet servlet) { - return areRequiredAuthorizationsSatisfied(req, servlet.getClass()); + public static boolean isAuthorizedForServlet(HttpServletRequest req, + VitroHttpServlet servlet) { + return isAuthorizedForServlet(req, servlet.getClass()); } /** * Are the actions that this servlet class requires authorized for the * current user by the current policies? */ - public static boolean areRequiredAuthorizationsSatisfied( - HttpServletRequest req, + public static boolean isAuthorizedForServlet(HttpServletRequest req, Class servletClass) { try { - return isActionClausesAuthorized(req, + return isAuthorizedForActionClauses(req, ActionClauses.forServletClass(servletClass)); } catch (PolicyHelperException e) { return false; @@ -113,11 +112,10 @@ public class PolicyHelper { * Are these action classes authorized for the current user by the current * policies? */ - public static boolean areRequiredAuthorizationsSatisfied( - HttpServletRequest req, + public static boolean isAuthorizedForActions(HttpServletRequest req, Collection> actionClasses) { try { - return isActionClausesAuthorized(req, new ActionClauses( + return isAuthorizedForActionClauses(req, new ActionClauses( actionClasses)); } catch (PolicyHelperException e) { return false; @@ -128,11 +126,11 @@ public class PolicyHelper { * Is this action class authorized for the current user by the current * policies? */ - public static boolean isActionAuthorized(HttpServletRequest req, + public static boolean isAuthorizedForAction(HttpServletRequest req, Class actionClass) { try { - return isActionClausesAuthorized(req, - new ActionClauses(actionClass)); + return isAuthorizedForActionClauses(req, new ActionClauses( + actionClass)); } catch (PolicyHelperException e) { return false; } @@ -142,20 +140,20 @@ public class PolicyHelper { * Actions must be authorized for the current user by the current policies. * If no actions, no problem. */ - private static boolean isActionClausesAuthorized(HttpServletRequest req, + private static boolean isAuthorizedForActionClauses(HttpServletRequest req, ActionClauses actionClauses) { PolicyIface policy = ServletPolicyList.getPolicies(req); IdentifierBundle ids = RequestIdentifiers.getIdBundleForRequest(req); return actionClauses.isEmpty() - || isActionClausesAuthorized(policy, ids, actionClauses); + || isAuthorizedForActionClauses(policy, ids, actionClauses); } /** Any clause in an ActionClauses may be authorized. */ - private static boolean isActionClausesAuthorized(PolicyIface policy, + private static boolean isAuthorizedForActionClauses(PolicyIface policy, IdentifierBundle ids, ActionClauses actionClauses) { for (Set clause : actionClauses.getClauseList()) { - if (isClauseAuthorized(policy, ids, clause)) { + if (isAuthorizedForClause(policy, ids, clause)) { return true; } } @@ -163,10 +161,10 @@ public class PolicyHelper { } /** All actions in a clause must be authorized. */ - private static boolean isClauseAuthorized(PolicyIface policy, + private static boolean isAuthorizedForClause(PolicyIface policy, IdentifierBundle ids, Set clause) { for (RequestedAction action : clause) { - if (!isActionAuthorized(policy, ids, action)) { + if (!isAuthorizedForAction(policy, ids, action)) { log.debug("not authorized"); return false; } @@ -177,7 +175,7 @@ public class PolicyHelper { /** * Is this action authorized for these IDs by this policy? */ - private static boolean isActionAuthorized(PolicyIface 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: " 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 d192df95f..c62ab7f4b 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/VitroHttpServlet.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/VitroHttpServlet.java @@ -62,13 +62,12 @@ public class VitroHttpServlet extends HttpServlet { } // Record restricted pages so we won't return to them on logout - if (PolicyHelper.isRestrictedPage(this)) { + if (PolicyHelper.isServletRestricted(this)) { LogoutRedirector.recordRestrictedPageUri(hreq); } - // If the @RequiresAuthenticationFor actions are not authorized, - // don't show them the page. - if (!PolicyHelper.areRequiredAuthorizationsSatisfied(hreq, this)) { + // If the user isn't authorized for this servlet, don't show it. + if (!PolicyHelper.isAuthorizedForServlet(hreq, this)) { if (LoginStatusBean.getBean(hreq).isLoggedIn()) { redirectToInsufficientAuthorizationPage(hreq, hresp); return; 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 ec8d64c6f..8f3311357 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 @@ -72,10 +72,10 @@ public class SiteAdminController extends FreemarkerHttpServlet { if (loginBean.isLoggedInAtLeast(LoginStatusBean.CURATOR)) { body.put("siteConfig", getSiteConfigurationData(vreq, urlBuilder)); } - if (PolicyHelper.isActionAuthorized(vreq, UseOntologyEditorPages.class)) { + if (PolicyHelper.isAuthorizedForAction(vreq, UseOntologyEditorPages.class)) { body.put("ontologyEditor", getOntologyEditorData(vreq, urlBuilder)); } - if (PolicyHelper.isActionAuthorized(vreq, UseAdvancedDataToolsPages.class)) { + if (PolicyHelper.isAuthorizedForAction(vreq, UseAdvancedDataToolsPages.class)) { body.put("dataTools", getDataToolsData(vreq, urlBuilder)); // Only for DataStar. Should handle without needing a DataStar-specific version of this controller. @@ -128,7 +128,7 @@ public class SiteAdminController extends FreemarkerHttpServlet { urls.put("tabs", urlBuilder.getPortalUrl("/listTabs")); - if (PolicyHelper.areRequiredAuthorizationsSatisfied(vreq, UsersListingController.class)) { + if (PolicyHelper.isAuthorizedForServlet(vreq, UsersListingController.class)) { urls.put("users", urlBuilder.getPortalUrl("/listUsers")); } 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 055f40329..34cba764f 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 @@ -60,8 +60,7 @@ public class RequiresAuthorizationFor extends BodyTagSupport { if (classes == null) { return false; } - return PolicyHelper.areRequiredAuthorizationsSatisfied(getRequest(), - classes); + return PolicyHelper.isAuthorizedForActions(getRequest(), classes); } /** 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 91c3ab376..d068de151 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 @@ -3,7 +3,6 @@ package edu.cornell.mannlib.vitro.webapp.auth.policy; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.fail; import java.util.Arrays; import java.util.HashSet; @@ -158,8 +157,7 @@ public class PolicyHelperTest extends AbstractTestClass { private void assertExpectedAuthorization(String label, Class servletClass, boolean expected) { - boolean actual = PolicyHelper.areRequiredAuthorizationsSatisfied(req, - servletClass); + boolean actual = PolicyHelper.isAuthorizedForServlet(req, servletClass); assertEquals(label, expected, actual); }