diff --git a/itest/web/src/main/resources/log4j.properties b/itest/web/src/main/resources/log4j.properties index 85d49bf473..21e283167b 100644 --- a/itest/web/src/main/resources/log4j.properties +++ b/itest/web/src/main/resources/log4j.properties @@ -7,4 +7,5 @@ log4j.appender.stdout.layout.ConversionPattern=%d %p %c - %m%n log4j.category.org.apache.jasper=INFO log4j.category.org.apache.directory=ERROR log4j.category.org.mortbay.log=INFO +log4j.category.httpclient.wire=INFO log4j.category.org.springframework.security=TRACE diff --git a/itest/web/src/main/webapp/WEB-INF/http-security.xml b/itest/web/src/main/webapp/WEB-INF/http-security.xml index 5ec0441f8c..e68da241d2 100644 --- a/itest/web/src/main/webapp/WEB-INF/http-security.xml +++ b/itest/web/src/main/webapp/WEB-INF/http-security.xml @@ -11,10 +11,10 @@ Needs to be supplemented with authentication provider(s) --> - + - - + + diff --git a/itest/web/src/main/webapp/WEB-INF/in-memory-provider.xml b/itest/web/src/main/webapp/WEB-INF/in-memory-provider.xml index 1f08e2c585..bf4dcd8a02 100644 --- a/itest/web/src/main/webapp/WEB-INF/in-memory-provider.xml +++ b/itest/web/src/main/webapp/WEB-INF/in-memory-provider.xml @@ -12,6 +12,7 @@ + diff --git a/itest/web/src/main/webapp/WEB-INF/security.tld b/itest/web/src/main/webapp/WEB-INF/security.tld index 78ce820d1e..ade20b534c 100644 --- a/itest/web/src/main/webapp/WEB-INF/security.tld +++ b/itest/web/src/main/webapp/WEB-INF/security.tld @@ -9,7 +9,6 @@ http://www.springframework.org/security/tags Spring Security Authorization Tag Library - $Id$ @@ -51,6 +50,15 @@ + + var + false + false + + A page scoped variable into which the boolean result of the tag evaluation will be written, allowing the + same condition to be reused subsequently in the page without re-evaluation. + + ifNotGranted @@ -153,6 +161,15 @@ are being evaluated. + + var + false + false + + A page scoped variable into which the boolean result of the tag evaluation will be written, allowing the + same condition to be reused subsequently in the page without re-evaluation. + + diff --git a/itest/web/src/main/webapp/secure/authorizationTagTestPage.jsp b/itest/web/src/main/webapp/secure/authorizationTagTestPage.jsp new file mode 100644 index 0000000000..b2beb7667c --- /dev/null +++ b/itest/web/src/main/webapp/secure/authorizationTagTestPage.jsp @@ -0,0 +1,21 @@ +<%@ taglib prefix="sec" uri="http://www.springframework.org/security/tags" %> + + +

Authorization Tag Test Page

+ + +Users can see this and 'allowed' variable is ${allowed}. + + + +Role X users (nobody) can see this. + + +Role X expression evaluates to ${allowed}. + + + + + + + diff --git a/itest/web/src/test/java/org/springframework/security/integration/InMemoryProviderWebAppTests.java b/itest/web/src/test/java/org/springframework/security/integration/InMemoryProviderWebAppTests.java index 7d6311008b..1d10f99526 100644 --- a/itest/web/src/test/java/org/springframework/security/integration/InMemoryProviderWebAppTests.java +++ b/itest/web/src/test/java/org/springframework/security/integration/InMemoryProviderWebAppTests.java @@ -94,17 +94,4 @@ public class InMemoryProviderWebAppTests extends AbstractWebServerIntegrationTes tester.gotoPage("secure/index.html"); tester.assertTextPresent("This session has been expired"); } - - @Test - public void authenticationTagEscapingWorksCorrectly() { - beginAt("secure/authenticationTagTestPage.jsp"); - login("theescapist<>&.", "theescapistspassword"); - String response = tester.getServerResponse(); - assertTrue(response.contains("This is the unescaped authentication name: theescapist<>&.")); - assertTrue(response.contains("This is the unescaped principal.username: theescapist<>&.")); - assertTrue(response.contains("This is the authentication name: theescapist<>&.")); - assertTrue(response.contains("This is the principal.username: theescapist<>&.")); - } - - } diff --git a/itest/web/src/test/java/org/springframework/security/integration/JspTaglibTests.java b/itest/web/src/test/java/org/springframework/security/integration/JspTaglibTests.java new file mode 100644 index 0000000000..ef237618f8 --- /dev/null +++ b/itest/web/src/test/java/org/springframework/security/integration/JspTaglibTests.java @@ -0,0 +1,39 @@ +package org.springframework.security.integration; + +import static org.testng.Assert.*; + +import org.testng.annotations.Test; + +/** + * + * @author Luke Taylor + */ +public final class JspTaglibTests extends AbstractWebServerIntegrationTests { + + @Override + protected String getContextConfigLocations() { + return "/WEB-INF/http-security.xml /WEB-INF/in-memory-provider.xml"; + } + + @Test + public void authenticationTagEscapingWorksCorrectly() { + beginAt("secure/authenticationTagTestPage.jsp"); + login("theescapist<>&.", "theescapistspassword"); + String response = tester.getServerResponse(); + assertTrue(response.contains("This is the unescaped authentication name: theescapist<>&.")); + assertTrue(response.contains("This is the unescaped principal.username: theescapist<>&.")); + assertTrue(response.contains("This is the authentication name: theescapist<>&.")); + assertTrue(response.contains("This is the principal.username: theescapist<>&.")); + } + + @Test + public void authorizationTagEvaluatesExpressionCorrectlyAndWritesValueToVariable() { + beginAt("secure/authorizationTagTestPage.jsp"); + login("bessie", "bessiespassword"); + String response = tester.getServerResponse(); + assertTrue(response.contains("Users can see this and 'allowed' variable is true.")); + assertFalse(response.contains("Role X users (nobody) can see this.")); + assertTrue(response.contains("Role X expression evaluates to false")); + } + +} 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 613ff108a1..117778805d 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 @@ -66,6 +66,7 @@ import org.springframework.web.util.ExpressionEvaluationUtils; * implementations are found in the application context. * * @author Ben Alex + * @author Luke Taylor */ public class AccessControlListTag extends TagSupport { //~ Static fields/initializers ===================================================================================== @@ -81,12 +82,13 @@ public class AccessControlListTag extends TagSupport { private SidRetrievalStrategy sidRetrievalStrategy; private PermissionFactory permissionFactory; private String hasPermission = ""; + private String var; //~ Methods ======================================================================================================== public int doStartTag() throws JspException { if ((null == hasPermission) || "".equals(hasPermission)) { - return Tag.SKIP_BODY; + return skipBody(); } initializeIfRequired(); @@ -111,7 +113,7 @@ public class AccessControlListTag extends TagSupport { } // Of course they have access to a null object! - return Tag.EVAL_BODY_INCLUDE; + return evalBody(); } if (SecurityContextHolder.getContext().getAuthentication() == null) { @@ -120,7 +122,7 @@ public class AccessControlListTag extends TagSupport { "SecurityContextHolder did not return a non-null Authentication object, so skipping tag body"); } - return Tag.SKIP_BODY; + return skipBody(); } List sids = sidRetrievalStrategy.getSids(SecurityContextHolder.getContext().getAuthentication()); @@ -131,15 +133,30 @@ public class AccessControlListTag extends TagSupport { Acl acl = aclService.readAclById(oid, sids); if (acl.isGranted(requiredPermissions, sids, false)) { - return Tag.EVAL_BODY_INCLUDE; + return evalBody(); } else { - return Tag.SKIP_BODY; + return skipBody(); } } catch (NotFoundException nfe) { - return Tag.SKIP_BODY; + return skipBody(); } } + private int skipBody() { + if (var != null) { + pageContext.setAttribute(var, Boolean.FALSE, PageContext.PAGE_SCOPE); + } + return SKIP_BODY; + } + + private int evalBody() { + if (var != null) { + pageContext.setAttribute(var, Boolean.TRUE, PageContext.PAGE_SCOPE); + } + return EVAL_BODY_INCLUDE; + } + + /** * Allows test cases to override where application context obtained from. * @@ -233,4 +250,8 @@ public class AccessControlListTag extends TagSupport { public void setHasPermission(String hasPermission) { this.hasPermission = hasPermission; } + + public void setVar(String var) { + this.var = var; + } } diff --git a/taglibs/src/main/java/org/springframework/security/taglibs/authz/AuthorizeTag.java b/taglibs/src/main/java/org/springframework/security/taglibs/authz/AuthorizeTag.java index af28090178..6e45bfeb5a 100644 --- a/taglibs/src/main/java/org/springframework/security/taglibs/authz/AuthorizeTag.java +++ b/taglibs/src/main/java/org/springframework/security/taglibs/authz/AuthorizeTag.java @@ -10,6 +10,7 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.jsp.JspException; +import javax.servlet.jsp.PageContext; import org.springframework.context.ApplicationContext; import org.springframework.expression.Expression; @@ -35,6 +36,7 @@ public class AuthorizeTag extends LegacyAuthorizeTag { private String access; private String url; private String method; + private String var; // If access expression evaluates to "true" return public int doStartTag() throws JspException { @@ -44,13 +46,21 @@ public class AuthorizeTag extends LegacyAuthorizeTag { return SKIP_BODY; } + int result; + if (access != null && access.length() > 0) { - return authorizeUsingAccessExpression(currentUser); + result = authorizeUsingAccessExpression(currentUser); } else if (url != null && url.length() > 0) { - return authorizeUsingUrlCheck(currentUser); + result = authorizeUsingUrlCheck(currentUser); + } else { + result = super.doStartTag(); + } + + if (var != null) { + pageContext.setAttribute(var, Boolean.valueOf(result == EVAL_BODY_INCLUDE), PageContext.PAGE_SCOPE); } - return super.doStartTag(); + return result; } private int authorizeUsingAccessExpression(Authentication currentUser) throws JspException { @@ -91,6 +101,10 @@ public class AuthorizeTag extends LegacyAuthorizeTag { this.method = method; } + public void setVar(String var) { + this.var = var; + } + WebSecurityExpressionHandler getExpressionHandler() throws JspException { ServletContext servletContext = pageContext.getServletContext(); ApplicationContext ctx = WebApplicationContextUtils.getRequiredWebApplicationContext(servletContext); diff --git a/taglibs/src/main/resources/META-INF/security.tld b/taglibs/src/main/resources/META-INF/security.tld index 78ce820d1e..ade20b534c 100644 --- a/taglibs/src/main/resources/META-INF/security.tld +++ b/taglibs/src/main/resources/META-INF/security.tld @@ -9,7 +9,6 @@ http://www.springframework.org/security/tags Spring Security Authorization Tag Library - $Id$ @@ -51,6 +50,15 @@ + + var + false + false + + A page scoped variable into which the boolean result of the tag evaluation will be written, allowing the + same condition to be reused subsequently in the page without re-evaluation. + + ifNotGranted @@ -153,6 +161,15 @@ are being evaluated. + + var + false + false + + A page scoped variable into which the boolean result of the tag evaluation will be written, allowing the + same condition to be reused subsequently in the page without re-evaluation. + + diff --git a/taglibs/src/test/java/org/springframework/security/taglibs/authz/AccessControlListTagTests.java b/taglibs/src/test/java/org/springframework/security/taglibs/authz/AccessControlListTagTests.java index a081746d53..ac53cdd208 100644 --- a/taglibs/src/test/java/org/springframework/security/taglibs/authz/AccessControlListTagTests.java +++ b/taglibs/src/test/java/org/springframework/security/taglibs/authz/AccessControlListTagTests.java @@ -1,6 +1,6 @@ package org.springframework.security.taglibs.authz; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.*; import static org.mockito.Matchers.*; import static org.mockito.Mockito.*; @@ -33,6 +33,7 @@ import org.springframework.web.context.WebApplicationContext; public class AccessControlListTagTests { AccessControlListTag tag; Acl acl; + MockPageContext pageContext; @Before public void setup() { @@ -44,9 +45,6 @@ public class AccessControlListTagTests { ObjectIdentity oid = mock(ObjectIdentity.class); ObjectIdentityRetrievalStrategy oidStrategy = mock(ObjectIdentityRetrievalStrategy.class); when(oidStrategy.getObjectIdentity(anyObject())).thenReturn(oid); -// AclPermissionEvaluator pe = new AclPermissionEvaluator(service); -// pe.setObjectIdentityRetrievalStrategy(oidStrategy); -// pe.setSidRetrievalStrategy(mock(SidRetrievalStrategy.class)); acl = mock(Acl.class); when(service.readAclById(any(ObjectIdentity.class), anyList())).thenReturn(acl); @@ -59,7 +57,8 @@ public class AccessControlListTagTests { MockServletContext servletCtx = new MockServletContext(); servletCtx.setAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE, ctx); - tag.setPageContext(new MockPageContext(servletCtx, new MockHttpServletRequest(), new MockHttpServletResponse())); + pageContext = new MockPageContext(servletCtx, new MockHttpServletRequest(), new MockHttpServletResponse()); + tag.setPageContext(pageContext); } @After @@ -73,8 +72,10 @@ public class AccessControlListTagTests { tag.setDomainObject(new Object()); tag.setHasPermission("READ"); + tag.setVar("allowed"); assertEquals(Tag.EVAL_BODY_INCLUDE, tag.doStartTag()); + assertTrue((Boolean)pageContext.getAttribute("allowed")); } @Test @@ -83,8 +84,10 @@ public class AccessControlListTagTests { tag.setDomainObject(new Object()); tag.setHasPermission("READ"); + tag.setVar("allowed"); assertEquals(Tag.SKIP_BODY, tag.doStartTag()); + assertFalse((Boolean)pageContext.getAttribute("allowed")); } }