From 503ac9ae7c084c0c19ac62cbd2e223efc6098e3b Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Fri, 12 Aug 2011 19:42:53 +0100 Subject: [PATCH] SEC-1798: Remove internal evaluation of EL in JSP tag implementations. --- docs/manual/src/docbook/taglibs.xml | 2 +- .../taglibs/authz/AbstractAuthorizeTag.java | 1 + .../taglibs/authz/AccessControlListTag.java | 17 ++--------------- .../taglibs/authz/AuthenticationTag.java | 3 +-- .../security/taglibs/authz/JspAuthorizeTag.java | 5 ----- .../taglibs/authz/AuthorizeTagTests.java | 3 +-- 6 files changed, 6 insertions(+), 25 deletions(-) diff --git a/docs/manual/src/docbook/taglibs.xml b/docs/manual/src/docbook/taglibs.xml index e8b9123b9b..38d2d32790 100644 --- a/docs/manual/src/docbook/taglibs.xml +++ b/docs/manual/src/docbook/taglibs.xml @@ -94,7 +94,7 @@ This content will only be visible to users who are authorized to send requests t comma-separated list of required permissions for a specified domain object. If the current user has any of those permissions, then the tag body will be evaluated. If they don't, it will be skipped. An example might - be<sec:accesscontrollist hasPermission="1,2" domainObject="someObject"> + be<sec:accesscontrollist hasPermission="1,2" domainObject="${someObject}"> This will be shown if the user has either of the permissions represented by the values "1" or "2" on the given object. diff --git a/taglibs/src/main/java/org/springframework/security/taglibs/authz/AbstractAuthorizeTag.java b/taglibs/src/main/java/org/springframework/security/taglibs/authz/AbstractAuthorizeTag.java index 4cea18aa87..e533edbdb0 100644 --- a/taglibs/src/main/java/org/springframework/security/taglibs/authz/AbstractAuthorizeTag.java +++ b/taglibs/src/main/java/org/springframework/security/taglibs/authz/AbstractAuthorizeTag.java @@ -306,6 +306,7 @@ public abstract class AbstractAuthorizeTag { return target; } + @SuppressWarnings("unchecked") private SecurityExpressionHandler getExpressionHandler() throws IOException { ApplicationContext appContext = WebApplicationContextUtils .getRequiredWebApplicationContext(getServletContext()); diff --git a/taglibs/src/main/java/org/springframework/security/taglibs/authz/AccessControlListTag.java b/taglibs/src/main/java/org/springframework/security/taglibs/authz/AccessControlListTag.java index adeebe8ca1..955ffb0320 100644 --- a/taglibs/src/main/java/org/springframework/security/taglibs/authz/AccessControlListTag.java +++ b/taglibs/src/main/java/org/springframework/security/taglibs/authz/AccessControlListTag.java @@ -21,7 +21,6 @@ import org.springframework.security.access.PermissionEvaluator; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.taglibs.TagLibConfig; import org.springframework.web.context.support.WebApplicationContextUtils; -import org.springframework.web.util.ExpressionEvaluationUtils; import javax.servlet.ServletContext; import javax.servlet.jsp.JspException; @@ -67,19 +66,7 @@ public class AccessControlListTag extends TagSupport { initializeIfRequired(); - final String evaledPermissionsString = ExpressionEvaluationUtils.evaluateString("hasPermission", hasPermission, - pageContext); - - Object resolvedDomainObject; - - if (domainObject instanceof String) { - resolvedDomainObject = ExpressionEvaluationUtils.evaluate("domainObject", (String) domainObject, - Object.class, pageContext); - } else { - resolvedDomainObject = domainObject; - } - - if (resolvedDomainObject == null) { + if (domainObject == null) { if (logger.isDebugEnabled()) { logger.debug("domainObject resolved to null, so including tag body"); } @@ -98,7 +85,7 @@ public class AccessControlListTag extends TagSupport { } if (permissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), - resolvedDomainObject, evaledPermissionsString)) { + domainObject, hasPermission)) { return evalBody(); } diff --git a/taglibs/src/main/java/org/springframework/security/taglibs/authz/AuthenticationTag.java b/taglibs/src/main/java/org/springframework/security/taglibs/authz/AuthenticationTag.java index 660c70ae6f..f79cfe0cd6 100644 --- a/taglibs/src/main/java/org/springframework/security/taglibs/authz/AuthenticationTag.java +++ b/taglibs/src/main/java/org/springframework/security/taglibs/authz/AuthenticationTag.java @@ -23,7 +23,6 @@ import org.springframework.security.web.util.TextEscapeUtils; import org.springframework.beans.BeanWrapperImpl; import org.springframework.beans.BeansException; -import org.springframework.web.util.ExpressionEvaluationUtils; import org.springframework.web.util.TagUtils; import java.io.IOException; @@ -144,7 +143,7 @@ public class AuthenticationTag extends TagSupport { * Set HTML escaping for this tag, as boolean value. */ public void setHtmlEscape(String htmlEscape) throws JspException { - this.htmlEscape = ExpressionEvaluationUtils.evaluateBoolean("htmlEscape", htmlEscape, pageContext); + this.htmlEscape = Boolean.valueOf(htmlEscape); } /** diff --git a/taglibs/src/main/java/org/springframework/security/taglibs/authz/JspAuthorizeTag.java b/taglibs/src/main/java/org/springframework/security/taglibs/authz/JspAuthorizeTag.java index 1e39b9eda8..906f052908 100644 --- a/taglibs/src/main/java/org/springframework/security/taglibs/authz/JspAuthorizeTag.java +++ b/taglibs/src/main/java/org/springframework/security/taglibs/authz/JspAuthorizeTag.java @@ -23,7 +23,6 @@ import org.springframework.expression.TypedValue; import org.springframework.security.access.expression.SecurityExpressionHandler; import org.springframework.security.taglibs.TagLibConfig; import org.springframework.security.web.FilterInvocation; -import org.springframework.web.util.ExpressionEvaluationUtils; /** * A JSP {@link Tag} implementation of {@link AbstractAuthorizeTag}. @@ -52,10 +51,6 @@ public class JspAuthorizeTag extends AbstractAuthorizeTag implements Tag { */ public int doStartTag() throws JspException { try { - setIfNotGranted(ExpressionEvaluationUtils.evaluateString("ifNotGranted", getIfNotGranted(), pageContext)); - setIfAllGranted(ExpressionEvaluationUtils.evaluateString("ifAllGranted", getIfAllGranted(), pageContext)); - setIfAnyGranted(ExpressionEvaluationUtils.evaluateString("ifAnyGranted", getIfAnyGranted(), pageContext)); - authorized = super.authorize(); if (!authorized && TagLibConfig.isUiSecurityDisabled()) { diff --git a/taglibs/src/test/java/org/springframework/security/taglibs/authz/AuthorizeTagTests.java b/taglibs/src/test/java/org/springframework/security/taglibs/authz/AuthorizeTagTests.java index ea2615b3d9..f390abd411 100644 --- a/taglibs/src/test/java/org/springframework/security/taglibs/authz/AuthorizeTagTests.java +++ b/taglibs/src/test/java/org/springframework/security/taglibs/authz/AuthorizeTagTests.java @@ -164,8 +164,7 @@ public class AuthorizeTagTests { @Test public void testOutputsBodyWhenNotGrantedSatisfied() throws JspException { authorizeTag.setIfNotGranted("ROLE_BANKER"); - assertEquals(Tag.EVAL_BODY_INCLUDE, - authorizeTag.doStartTag()); + assertEquals(Tag.EVAL_BODY_INCLUDE, authorizeTag.doStartTag()); } @Test