NIHVIVO-2492 Improve method names
This commit is contained in:
parent
d126241bcd
commit
f2269120d7
5 changed files with 26 additions and 32 deletions
|
@ -76,7 +76,7 @@ public class PolicyHelper {
|
||||||
/**
|
/**
|
||||||
* Does this servlet require authorization?
|
* Does this servlet require authorization?
|
||||||
*/
|
*/
|
||||||
public static boolean isRestrictedPage(VitroHttpServlet servlet) {
|
public static boolean isServletRestricted(VitroHttpServlet servlet) {
|
||||||
Class<? extends VitroHttpServlet> servletClass = servlet.getClass();
|
Class<? extends VitroHttpServlet> servletClass = servlet.getClass();
|
||||||
try {
|
try {
|
||||||
return !ActionClauses.forServletClass(servletClass).isEmpty();
|
return !ActionClauses.forServletClass(servletClass).isEmpty();
|
||||||
|
@ -89,20 +89,19 @@ public class PolicyHelper {
|
||||||
* Are the actions that this servlet requires authorized for the current
|
* Are the actions that this servlet requires authorized for the current
|
||||||
* user by the current policies?
|
* user by the current policies?
|
||||||
*/
|
*/
|
||||||
public static boolean areRequiredAuthorizationsSatisfied(
|
public static boolean isAuthorizedForServlet(HttpServletRequest req,
|
||||||
HttpServletRequest req, VitroHttpServlet servlet) {
|
VitroHttpServlet servlet) {
|
||||||
return areRequiredAuthorizationsSatisfied(req, servlet.getClass());
|
return isAuthorizedForServlet(req, servlet.getClass());
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Are the actions that this servlet class requires authorized for the
|
* Are the actions that this servlet class requires authorized for the
|
||||||
* current user by the current policies?
|
* current user by the current policies?
|
||||||
*/
|
*/
|
||||||
public static boolean areRequiredAuthorizationsSatisfied(
|
public static boolean isAuthorizedForServlet(HttpServletRequest req,
|
||||||
HttpServletRequest req,
|
|
||||||
Class<? extends VitroHttpServlet> servletClass) {
|
Class<? extends VitroHttpServlet> servletClass) {
|
||||||
try {
|
try {
|
||||||
return isActionClausesAuthorized(req,
|
return isAuthorizedForActionClauses(req,
|
||||||
ActionClauses.forServletClass(servletClass));
|
ActionClauses.forServletClass(servletClass));
|
||||||
} catch (PolicyHelperException e) {
|
} catch (PolicyHelperException e) {
|
||||||
return false;
|
return false;
|
||||||
|
@ -113,11 +112,10 @@ public class PolicyHelper {
|
||||||
* Are these action classes authorized for the current user by the current
|
* Are these action classes authorized for the current user by the current
|
||||||
* policies?
|
* policies?
|
||||||
*/
|
*/
|
||||||
public static boolean areRequiredAuthorizationsSatisfied(
|
public static boolean isAuthorizedForActions(HttpServletRequest req,
|
||||||
HttpServletRequest req,
|
|
||||||
Collection<Class<? extends RequestedAction>> actionClasses) {
|
Collection<Class<? extends RequestedAction>> actionClasses) {
|
||||||
try {
|
try {
|
||||||
return isActionClausesAuthorized(req, new ActionClauses(
|
return isAuthorizedForActionClauses(req, new ActionClauses(
|
||||||
actionClasses));
|
actionClasses));
|
||||||
} catch (PolicyHelperException e) {
|
} catch (PolicyHelperException e) {
|
||||||
return false;
|
return false;
|
||||||
|
@ -128,11 +126,11 @@ public class PolicyHelper {
|
||||||
* Is this action class authorized for the current user by the current
|
* Is this action class authorized for the current user by the current
|
||||||
* policies?
|
* policies?
|
||||||
*/
|
*/
|
||||||
public static boolean isActionAuthorized(HttpServletRequest req,
|
public static boolean isAuthorizedForAction(HttpServletRequest req,
|
||||||
Class<? extends RequestedAction> actionClass) {
|
Class<? extends RequestedAction> actionClass) {
|
||||||
try {
|
try {
|
||||||
return isActionClausesAuthorized(req,
|
return isAuthorizedForActionClauses(req, new ActionClauses(
|
||||||
new ActionClauses(actionClass));
|
actionClass));
|
||||||
} catch (PolicyHelperException e) {
|
} catch (PolicyHelperException e) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
@ -142,20 +140,20 @@ public class PolicyHelper {
|
||||||
* Actions must be authorized for the current user by the current policies.
|
* Actions must be authorized for the current user by the current policies.
|
||||||
* If no actions, no problem.
|
* If no actions, no problem.
|
||||||
*/
|
*/
|
||||||
private static boolean isActionClausesAuthorized(HttpServletRequest req,
|
private static boolean isAuthorizedForActionClauses(HttpServletRequest req,
|
||||||
ActionClauses actionClauses) {
|
ActionClauses actionClauses) {
|
||||||
PolicyIface policy = ServletPolicyList.getPolicies(req);
|
PolicyIface policy = ServletPolicyList.getPolicies(req);
|
||||||
IdentifierBundle ids = RequestIdentifiers.getIdBundleForRequest(req);
|
IdentifierBundle ids = RequestIdentifiers.getIdBundleForRequest(req);
|
||||||
|
|
||||||
return actionClauses.isEmpty()
|
return actionClauses.isEmpty()
|
||||||
|| isActionClausesAuthorized(policy, ids, actionClauses);
|
|| isAuthorizedForActionClauses(policy, ids, actionClauses);
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Any clause in an ActionClauses may be authorized. */
|
/** Any clause in an ActionClauses may be authorized. */
|
||||||
private static boolean isActionClausesAuthorized(PolicyIface policy,
|
private static boolean isAuthorizedForActionClauses(PolicyIface policy,
|
||||||
IdentifierBundle ids, ActionClauses actionClauses) {
|
IdentifierBundle ids, ActionClauses actionClauses) {
|
||||||
for (Set<RequestedAction> clause : actionClauses.getClauseList()) {
|
for (Set<RequestedAction> clause : actionClauses.getClauseList()) {
|
||||||
if (isClauseAuthorized(policy, ids, clause)) {
|
if (isAuthorizedForClause(policy, ids, clause)) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -163,10 +161,10 @@ public class PolicyHelper {
|
||||||
}
|
}
|
||||||
|
|
||||||
/** All actions in a clause must be authorized. */
|
/** All actions in a clause must be authorized. */
|
||||||
private static boolean isClauseAuthorized(PolicyIface policy,
|
private static boolean isAuthorizedForClause(PolicyIface policy,
|
||||||
IdentifierBundle ids, Set<RequestedAction> clause) {
|
IdentifierBundle ids, Set<RequestedAction> clause) {
|
||||||
for (RequestedAction action : clause) {
|
for (RequestedAction action : clause) {
|
||||||
if (!isActionAuthorized(policy, ids, action)) {
|
if (!isAuthorizedForAction(policy, ids, action)) {
|
||||||
log.debug("not authorized");
|
log.debug("not authorized");
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
@ -177,7 +175,7 @@ public class PolicyHelper {
|
||||||
/**
|
/**
|
||||||
* Is this action authorized for these IDs by this policy?
|
* 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) {
|
IdentifierBundle ids, RequestedAction action) {
|
||||||
PolicyDecision decision = policy.isAuthorized(ids, action);
|
PolicyDecision decision = policy.isAuthorized(ids, action);
|
||||||
log.debug("decision for '" + action.getClass().getName() + "' was: "
|
log.debug("decision for '" + action.getClass().getName() + "' was: "
|
||||||
|
|
|
@ -62,13 +62,12 @@ public class VitroHttpServlet extends HttpServlet {
|
||||||
}
|
}
|
||||||
|
|
||||||
// Record restricted pages so we won't return to them on logout
|
// Record restricted pages so we won't return to them on logout
|
||||||
if (PolicyHelper.isRestrictedPage(this)) {
|
if (PolicyHelper.isServletRestricted(this)) {
|
||||||
LogoutRedirector.recordRestrictedPageUri(hreq);
|
LogoutRedirector.recordRestrictedPageUri(hreq);
|
||||||
}
|
}
|
||||||
|
|
||||||
// If the @RequiresAuthenticationFor actions are not authorized,
|
// If the user isn't authorized for this servlet, don't show it.
|
||||||
// don't show them the page.
|
if (!PolicyHelper.isAuthorizedForServlet(hreq, this)) {
|
||||||
if (!PolicyHelper.areRequiredAuthorizationsSatisfied(hreq, this)) {
|
|
||||||
if (LoginStatusBean.getBean(hreq).isLoggedIn()) {
|
if (LoginStatusBean.getBean(hreq).isLoggedIn()) {
|
||||||
redirectToInsufficientAuthorizationPage(hreq, hresp);
|
redirectToInsufficientAuthorizationPage(hreq, hresp);
|
||||||
return;
|
return;
|
||||||
|
|
|
@ -72,10 +72,10 @@ public class SiteAdminController extends FreemarkerHttpServlet {
|
||||||
if (loginBean.isLoggedInAtLeast(LoginStatusBean.CURATOR)) {
|
if (loginBean.isLoggedInAtLeast(LoginStatusBean.CURATOR)) {
|
||||||
body.put("siteConfig", getSiteConfigurationData(vreq, urlBuilder));
|
body.put("siteConfig", getSiteConfigurationData(vreq, urlBuilder));
|
||||||
}
|
}
|
||||||
if (PolicyHelper.isActionAuthorized(vreq, UseOntologyEditorPages.class)) {
|
if (PolicyHelper.isAuthorizedForAction(vreq, UseOntologyEditorPages.class)) {
|
||||||
body.put("ontologyEditor", getOntologyEditorData(vreq, urlBuilder));
|
body.put("ontologyEditor", getOntologyEditorData(vreq, urlBuilder));
|
||||||
}
|
}
|
||||||
if (PolicyHelper.isActionAuthorized(vreq, UseAdvancedDataToolsPages.class)) {
|
if (PolicyHelper.isAuthorizedForAction(vreq, UseAdvancedDataToolsPages.class)) {
|
||||||
body.put("dataTools", getDataToolsData(vreq, urlBuilder));
|
body.put("dataTools", getDataToolsData(vreq, urlBuilder));
|
||||||
|
|
||||||
// Only for DataStar. Should handle without needing a DataStar-specific version of this controller.
|
// 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"));
|
urls.put("tabs", urlBuilder.getPortalUrl("/listTabs"));
|
||||||
|
|
||||||
if (PolicyHelper.areRequiredAuthorizationsSatisfied(vreq, UsersListingController.class)) {
|
if (PolicyHelper.isAuthorizedForServlet(vreq, UsersListingController.class)) {
|
||||||
urls.put("users", urlBuilder.getPortalUrl("/listUsers"));
|
urls.put("users", urlBuilder.getPortalUrl("/listUsers"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -60,8 +60,7 @@ public class RequiresAuthorizationFor extends BodyTagSupport {
|
||||||
if (classes == null) {
|
if (classes == null) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
return PolicyHelper.areRequiredAuthorizationsSatisfied(getRequest(),
|
return PolicyHelper.isAuthorizedForActions(getRequest(), classes);
|
||||||
classes);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
|
@ -3,7 +3,6 @@
|
||||||
package edu.cornell.mannlib.vitro.webapp.auth.policy;
|
package edu.cornell.mannlib.vitro.webapp.auth.policy;
|
||||||
|
|
||||||
import static org.junit.Assert.assertEquals;
|
import static org.junit.Assert.assertEquals;
|
||||||
import static org.junit.Assert.fail;
|
|
||||||
|
|
||||||
import java.util.Arrays;
|
import java.util.Arrays;
|
||||||
import java.util.HashSet;
|
import java.util.HashSet;
|
||||||
|
@ -158,8 +157,7 @@ public class PolicyHelperTest extends AbstractTestClass {
|
||||||
|
|
||||||
private void assertExpectedAuthorization(String label,
|
private void assertExpectedAuthorization(String label,
|
||||||
Class<? extends VitroHttpServlet> servletClass, boolean expected) {
|
Class<? extends VitroHttpServlet> servletClass, boolean expected) {
|
||||||
boolean actual = PolicyHelper.areRequiredAuthorizationsSatisfied(req,
|
boolean actual = PolicyHelper.isAuthorizedForServlet(req, servletClass);
|
||||||
servletClass);
|
|
||||||
assertEquals(label, expected, actual);
|
assertEquals(label, expected, actual);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Add table
Reference in a new issue