Skip to content

Commit

Permalink
Polish spring-security-taglibs main code
Browse files Browse the repository at this point in the history
Manually polish `spring-security-taglibs` following the formatting
and checkstyle fixes.

Issue gh-8945
  • Loading branch information
philwebb authored and rwinch committed Aug 24, 2020
1 parent 1f03608 commit 2ca6256
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 71 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 : "<span class=\"securityHiddenUI\">";
SECURED_UI_SUFFIX = (suffix != null) ? suffix : "</span>";

DISABLE_UI_SECURITY = "true".equals(db);

if (DISABLE_UI_SECURITY) {
logger.warn("***** UI security is disabled. All unauthorized content will be displayed *****");
}
Expand All @@ -60,11 +59,7 @@ private 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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

}
else {
isAuthorized = false;

if (StringUtils.hasText(getUrl())) {
return authorizeUsingUrlCheck();
}

return isAuthorized;
return false;
}

/**
Expand All @@ -122,18 +113,14 @@ public boolean authorizeUsingAccessExpression() throws IOException {
if (SecurityContextHolder.getContext().getAuthentication() == null) {
return false;
}

SecurityExpressionHandler<FilterInvocation> handler = getExpressionHandler();

Expression accessExpression;
try {
accessExpression = handler.getExpressionParser().parseExpression(getAccess());

}
catch (ParseException ex) {
throw new IOException(ex);
}

return ExpressionUtils.evaluateAsBoolean(accessExpression, createExpressionEvaluationContext(handler));
}

Expand All @@ -144,7 +131,6 @@ protected EvaluationContext createExpressionEvaluationContext(SecurityExpression
FilterInvocation f = new FilterInvocation(getRequest(), getResponse(), (request, response) -> {
throw new UnsupportedOperationException();
});

return handler.createEvaluationContext(SecurityContextHolder.getContext().getAuthentication(), f);
}

Expand Down Expand Up @@ -184,21 +170,17 @@ public void setMethod(String method) {
this.method = (method != null) ? method.toUpperCase() : null;
}

/*------------- Private helper methods -----------------*/

@SuppressWarnings({ "unchecked", "rawtypes" })
private SecurityExpressionHandler<FilterInvocation> getExpressionHandler() throws IOException {
ApplicationContext appContext = SecurityWebApplicationContextUtils
.findRequiredWebApplicationContext(getServletContext());
Map<String, SecurityExpressionHandler> 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.");
}
Expand All @@ -209,17 +191,14 @@ private WebInvocationPrivilegeEvaluator getPrivilegeEvaluator() throws IOExcepti
if (privEvaluatorFromRequest != null) {
return privEvaluatorFromRequest;
}

ApplicationContext ctx = SecurityWebApplicationContextUtils
.findRequiredWebApplicationContext(getServletContext());
Map<String, WebInvocationPrivilegeEvaluator> 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];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,35 +72,23 @@ public int doStartTag() throws JspException {
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<Object> requiredPermissions = parseHasPermission(this.hasPermission);
for (Object requiredPermission : requiredPermissions) {
if (!this.permissionEvaluator.hasPermission(authentication, this.domainObject, requiredPermission)) {
return skipBody();
}
}

return evalBody();
}

Expand All @@ -112,7 +100,7 @@ private List<Object> parseHasPermission(String hasPermission) {
try {
parsedPermission = Integer.parseInt(permissionToParse);
}
catch (NumberFormatException notBitMask) {
catch (NumberFormatException ex) {
}
parsedPermissions.add(parsedPermission);
}
Expand Down Expand Up @@ -141,7 +129,6 @@ private int evalBody() {
*/
protected ApplicationContext getContext(PageContext pageContext) {
ServletContext servletContext = pageContext.getServletContext();

return SecurityWebApplicationContextUtils.findRequiredWebApplicationContext(servletContext);
}

Expand All @@ -157,27 +144,22 @@ private void initializeIfRequired() throws JspException {
if (this.applicationContext != null) {
return;
}

this.applicationContext = getContext(this.pageContext);

this.permissionEvaluator = getBeanOfType(PermissionEvaluator.class);
}

private <T> T getBeanOfType(Class<T> type) throws JspException {
Map<String, T> 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!");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,10 @@ public int doEndTag() throws JspException {
|| (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);
Expand All @@ -106,7 +103,6 @@ public int doEndTag() throws JspException {
throw new JspException(ex);
}
}

if (this.var != null) {
/*
* Store the result, letting an IllegalArgumentException propagate back if the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -105,7 +101,6 @@ public int doEndTag() throws JspException {
catch (IOException ex) {
throw new JspException(ex);
}

return EVAL_PAGE;
}

Expand Down Expand Up @@ -222,7 +217,6 @@ public void setVariable(String name, Object value) {
@Override
public Object lookupVariable(String name) {
Object result = this.delegate.lookupVariable(name);

if (result == null) {
result = JspAuthorizeTag.this.pageContext.findAttribute(name);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -43,7 +42,6 @@ public int doEndTag() throws JspException {
throw new JspException(ex);
}
}

return EVAL_PAGE;
}

Expand Down

0 comments on commit 2ca6256

Please sign in to comment.