diff --git a/taglibs/src/main/java/org/springframework/security/taglibs/TagLibConfig.java b/taglibs/src/main/java/org/springframework/security/taglibs/TagLibConfig.java index dfbe370fde..64083a679a 100644 --- a/taglibs/src/main/java/org/springframework/security/taglibs/TagLibConfig.java +++ b/taglibs/src/main/java/org/springframework/security/taglibs/TagLibConfig.java @@ -33,19 +33,18 @@ public final class TagLibConfig { static Log logger = LogFactory.getLog("spring-security-taglibs"); static final boolean DISABLE_UI_SECURITY; + static final String SECURED_UI_PREFIX; + static final String SECURED_UI_SUFFIX; static { String db = System.getProperty("spring.security.disableUISecurity"); String prefix = System.getProperty("spring.security.securedUIPrefix"); String suffix = System.getProperty("spring.security.securedUISuffix"); - SECURED_UI_PREFIX = (prefix != null) ? prefix : ""; SECURED_UI_SUFFIX = (suffix != null) ? suffix : ""; - DISABLE_UI_SECURITY = "true".equals(db); - if (DISABLE_UI_SECURITY) { logger.warn("***** UI security is disabled. All unauthorized content will be displayed *****"); } @@ -60,11 +59,7 @@ public final class TagLibConfig { * @param authorized whether the user is authorized to see the content or not */ public static int evalOrSkip(boolean authorized) { - if (authorized || DISABLE_UI_SECURITY) { - return Tag.EVAL_BODY_INCLUDE; - } - - return Tag.SKIP_BODY; + return (authorized || DISABLE_UI_SECURITY) ? Tag.EVAL_BODY_INCLUDE : Tag.SKIP_BODY; } public static boolean isUiSecurityDisabled() { 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 803f569d89..95fb17928f 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 @@ -93,22 +93,13 @@ public abstract class AbstractAuthorizeTag { * @throws IOException */ public boolean authorize() throws IOException { - boolean isAuthorized; - if (StringUtils.hasText(getAccess())) { - isAuthorized = authorizeUsingAccessExpression(); - + return authorizeUsingAccessExpression(); } - else if (StringUtils.hasText(getUrl())) { - isAuthorized = authorizeUsingUrlCheck(); - + if (StringUtils.hasText(getUrl())) { + return authorizeUsingUrlCheck(); } - else { - isAuthorized = false; - - } - - return isAuthorized; + return false; } /** @@ -122,18 +113,14 @@ public abstract class AbstractAuthorizeTag { if (SecurityContextHolder.getContext().getAuthentication() == null) { return false; } - SecurityExpressionHandler handler = getExpressionHandler(); - Expression accessExpression; try { accessExpression = handler.getExpressionParser().parseExpression(getAccess()); - } catch (ParseException ex) { throw new IOException(ex); } - return ExpressionUtils.evaluateAsBoolean(accessExpression, createExpressionEvaluationContext(handler)); } @@ -144,7 +131,6 @@ public abstract class AbstractAuthorizeTag { FilterInvocation f = new FilterInvocation(getRequest(), getResponse(), (request, response) -> { throw new UnsupportedOperationException(); }); - return handler.createEvaluationContext(SecurityContextHolder.getContext().getAuthentication(), f); } @@ -184,21 +170,17 @@ public abstract class AbstractAuthorizeTag { this.method = (method != null) ? method.toUpperCase() : null; } - /*------------- Private helper methods -----------------*/ - @SuppressWarnings({ "unchecked", "rawtypes" }) private SecurityExpressionHandler getExpressionHandler() throws IOException { ApplicationContext appContext = SecurityWebApplicationContextUtils .findRequiredWebApplicationContext(getServletContext()); Map handlers = appContext.getBeansOfType(SecurityExpressionHandler.class); - - for (SecurityExpressionHandler h : handlers.values()) { - if (FilterInvocation.class - .equals(GenericTypeResolver.resolveTypeArgument(h.getClass(), SecurityExpressionHandler.class))) { - return h; + for (SecurityExpressionHandler handler : handlers.values()) { + if (FilterInvocation.class.equals( + GenericTypeResolver.resolveTypeArgument(handler.getClass(), SecurityExpressionHandler.class))) { + return handler; } } - throw new IOException("No visible WebSecurityExpressionHandler instance could be found in the application " + "context. There must be at least one in order to support expressions in JSP 'authorize' tags."); } @@ -209,17 +191,14 @@ public abstract class AbstractAuthorizeTag { if (privEvaluatorFromRequest != null) { return privEvaluatorFromRequest; } - ApplicationContext ctx = SecurityWebApplicationContextUtils .findRequiredWebApplicationContext(getServletContext()); Map wipes = ctx.getBeansOfType(WebInvocationPrivilegeEvaluator.class); - if (wipes.size() == 0) { throw new IOException( "No visible WebInvocationPrivilegeEvaluator instance could be found in the application " + "context. There must be at least one in order to support the use of URL access checks in 'authorize' tags."); } - return (WebInvocationPrivilegeEvaluator) wipes.values().toArray()[0]; } 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 0b8ce61b4e..16904672a6 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 @@ -72,35 +72,23 @@ public class AccessControlListTag extends TagSupport { if ((null == this.hasPermission) || "".equals(this.hasPermission)) { return skipBody(); } - initializeIfRequired(); - if (this.domainObject == null) { - if (logger.isDebugEnabled()) { - logger.debug("domainObject resolved to null, so including tag body"); - } - + logger.debug("domainObject resolved to null, so including tag body"); // Of course they have access to a null object! return evalBody(); } - Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); if (authentication == null) { - if (logger.isDebugEnabled()) { - logger.debug( - "SecurityContextHolder did not return a non-null Authentication object, so skipping tag body"); - } - + logger.debug("SecurityContextHolder did not return a non-null Authentication object, so skipping tag body"); return skipBody(); } - List requiredPermissions = parseHasPermission(this.hasPermission); for (Object requiredPermission : requiredPermissions) { if (!this.permissionEvaluator.hasPermission(authentication, this.domainObject, requiredPermission)) { return skipBody(); } } - return evalBody(); } @@ -112,7 +100,7 @@ public class AccessControlListTag extends TagSupport { try { parsedPermission = Integer.parseInt(permissionToParse); } - catch (NumberFormatException notBitMask) { + catch (NumberFormatException ex) { } parsedPermissions.add(parsedPermission); } @@ -141,7 +129,6 @@ public class AccessControlListTag extends TagSupport { */ protected ApplicationContext getContext(PageContext pageContext) { ServletContext servletContext = pageContext.getServletContext(); - return SecurityWebApplicationContextUtils.findRequiredWebApplicationContext(servletContext); } @@ -157,27 +144,22 @@ public class AccessControlListTag extends TagSupport { if (this.applicationContext != null) { return; } - this.applicationContext = getContext(this.pageContext); - this.permissionEvaluator = getBeanOfType(PermissionEvaluator.class); } private T getBeanOfType(Class type) throws JspException { Map map = this.applicationContext.getBeansOfType(type); - for (ApplicationContext context = this.applicationContext.getParent(); context != null; context = context .getParent()) { map.putAll(context.getBeansOfType(type)); } - if (map.size() == 0) { return null; } - else if (map.size() == 1) { + if (map.size() == 1) { return map.values().iterator().next(); } - throw new JspException("Found incorrect number of " + type.getSimpleName() + " instances in " + "application context - you must have only have one!"); } 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 e65e4e00b7..d7fd43627c 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 @@ -91,13 +91,10 @@ public class AuthenticationTag extends TagSupport { || (SecurityContextHolder.getContext().getAuthentication() == null)) { return Tag.EVAL_PAGE; } - Authentication auth = SecurityContextHolder.getContext().getAuthentication(); - if (auth.getPrincipal() == null) { return Tag.EVAL_PAGE; } - try { BeanWrapperImpl wrapper = new BeanWrapperImpl(auth); result = wrapper.getPropertyValue(this.property); @@ -106,7 +103,6 @@ public class AuthenticationTag extends TagSupport { throw new JspException(ex); } } - if (this.var != null) { /* * Store the result, letting an IllegalArgumentException propagate back if the 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 f1ae9512d8..e5b4cedb78 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 @@ -68,17 +68,13 @@ public class JspAuthorizeTag extends AbstractAuthorizeTag implements Tag { public int doStartTag() throws JspException { try { this.authorized = super.authorize(); - if (!this.authorized && TagLibConfig.isUiSecurityDisabled()) { this.pageContext.getOut().write(TagLibConfig.getSecuredUiPrefix()); } - if (this.var != null) { this.pageContext.setAttribute(this.var, this.authorized, PageContext.PAGE_SCOPE); } - return TagLibConfig.evalOrSkip(this.authorized); - } catch (IOException ex) { throw new JspException(ex); @@ -105,7 +101,6 @@ public class JspAuthorizeTag extends AbstractAuthorizeTag implements Tag { catch (IOException ex) { throw new JspException(ex); } - return EVAL_PAGE; } @@ -222,7 +217,6 @@ public class JspAuthorizeTag extends AbstractAuthorizeTag implements Tag { @Override public Object lookupVariable(String name) { Object result = this.delegate.lookupVariable(name); - if (result == null) { result = JspAuthorizeTag.this.pageContext.findAttribute(name); } diff --git a/taglibs/src/main/java/org/springframework/security/taglibs/csrf/AbstractCsrfTag.java b/taglibs/src/main/java/org/springframework/security/taglibs/csrf/AbstractCsrfTag.java index 65509ddba4..d9ad9145d2 100644 --- a/taglibs/src/main/java/org/springframework/security/taglibs/csrf/AbstractCsrfTag.java +++ b/taglibs/src/main/java/org/springframework/security/taglibs/csrf/AbstractCsrfTag.java @@ -33,7 +33,6 @@ abstract class AbstractCsrfTag extends TagSupport { @Override public int doEndTag() throws JspException { - CsrfToken token = (CsrfToken) this.pageContext.getRequest().getAttribute(CsrfToken.class.getName()); if (token != null) { try { @@ -43,7 +42,6 @@ abstract class AbstractCsrfTag extends TagSupport { throw new JspException(ex); } } - return EVAL_PAGE; }