Browse Source

Enable Null checking in spring-security-taglibs via JSpecify

Closes gh-17828
pull/17191/merge
Rob Winch 4 months ago
parent
commit
5370f1190f
No known key found for this signature in database
  1. 4
      taglibs/spring-security-taglibs.gradle
  2. 28
      taglibs/src/main/java/org/springframework/security/taglibs/authz/AbstractAuthorizeTag.java
  3. 11
      taglibs/src/main/java/org/springframework/security/taglibs/authz/AccessControlListTag.java
  4. 5
      taglibs/src/main/java/org/springframework/security/taglibs/authz/AuthenticationTag.java
  5. 20
      taglibs/src/main/java/org/springframework/security/taglibs/authz/JspAuthorizeTag.java
  6. 3
      taglibs/src/main/java/org/springframework/security/taglibs/authz/package-info.java
  7. 23
      taglibs/src/main/java/org/springframework/security/taglibs/csrf/package-info.java
  8. 3
      taglibs/src/main/java/org/springframework/security/taglibs/package-info.java

4
taglibs/spring-security-taglibs.gradle

@ -1,3 +1,7 @@
plugins {
id 'apply-nullability'
}
apply plugin: 'io.spring.convention.spring-module' apply plugin: 'io.spring.convention.spring-module'
dependencies { dependencies {

28
taglibs/src/main/java/org/springframework/security/taglibs/authz/AbstractAuthorizeTag.java

@ -24,6 +24,8 @@ import jakarta.servlet.ServletContext;
import jakarta.servlet.ServletRequest; import jakarta.servlet.ServletRequest;
import jakarta.servlet.ServletResponse; import jakarta.servlet.ServletResponse;
import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletRequest;
import org.jspecify.annotations.NullUnmarked;
import org.jspecify.annotations.Nullable;
import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContext;
import org.springframework.core.GenericTypeResolver; import org.springframework.core.GenericTypeResolver;
@ -40,6 +42,7 @@ import org.springframework.security.web.FilterInvocation;
import org.springframework.security.web.WebAttributes; import org.springframework.security.web.WebAttributes;
import org.springframework.security.web.access.WebInvocationPrivilegeEvaluator; import org.springframework.security.web.access.WebInvocationPrivilegeEvaluator;
import org.springframework.security.web.context.support.SecurityWebApplicationContextUtils; import org.springframework.security.web.context.support.SecurityWebApplicationContextUtils;
import org.springframework.util.Assert;
import org.springframework.util.StringUtils; import org.springframework.util.StringUtils;
/** /**
@ -60,11 +63,13 @@ import org.springframework.util.StringUtils;
*/ */
public abstract class AbstractAuthorizeTag { public abstract class AbstractAuthorizeTag {
private String access; @SuppressWarnings("NullAway.Init")
private @Nullable String access;
private String url; @SuppressWarnings("NullAway.Init")
private @Nullable String url;
private String method = "GET"; private @Nullable String method = "GET";
/** /**
* This method allows subclasses to provide a way to access the ServletRequest * This method allows subclasses to provide a way to access the ServletRequest
@ -112,14 +117,17 @@ public abstract class AbstractAuthorizeTag {
* @return the result of the authorization decision * @return the result of the authorization decision
* @throws IOException * @throws IOException
*/ */
@SuppressWarnings("NullAway") // Dataflow analysis limitation
public boolean authorizeUsingAccessExpression() throws IOException { public boolean authorizeUsingAccessExpression() throws IOException {
if (getContext().getAuthentication() == null) { if (getContext().getAuthentication() == null) {
return false; return false;
} }
String access = getAccess();
Assert.notNull(access, "access cannot be null");
SecurityExpressionHandler<FilterInvocation> handler = getExpressionHandler(); SecurityExpressionHandler<FilterInvocation> handler = getExpressionHandler();
Expression accessExpression; Expression accessExpression;
try { try {
accessExpression = handler.getExpressionParser().parseExpression(getAccess()); accessExpression = handler.getExpressionParser().parseExpression(access);
} }
catch (ParseException ex) { catch (ParseException ex) {
throw new IOException(ex); throw new IOException(ex);
@ -143,13 +151,16 @@ public abstract class AbstractAuthorizeTag {
* @return the result of the authorization decision * @return the result of the authorization decision
* @throws IOException * @throws IOException
*/ */
@SuppressWarnings("NullAway") // Dataflow analysis limitation
public boolean authorizeUsingUrlCheck() throws IOException { public boolean authorizeUsingUrlCheck() throws IOException {
String url = getUrl();
Assert.notNull(url, "url cannot be null");
String contextPath = ((HttpServletRequest) getRequest()).getContextPath(); String contextPath = ((HttpServletRequest) getRequest()).getContextPath();
Authentication currentUser = getContext().getAuthentication(); Authentication currentUser = getContext().getAuthentication();
return getPrivilegeEvaluator().isAllowed(contextPath, getUrl(), getMethod(), currentUser); return getPrivilegeEvaluator().isAllowed(contextPath, url, getMethod(), currentUser);
} }
public String getAccess() { public @Nullable String getAccess() {
return this.access; return this.access;
} }
@ -157,7 +168,7 @@ public abstract class AbstractAuthorizeTag {
this.access = access; this.access = access;
} }
public String getUrl() { public @Nullable String getUrl() {
return this.url; return this.url;
} }
@ -165,10 +176,11 @@ public abstract class AbstractAuthorizeTag {
this.url = url; this.url = url;
} }
public String getMethod() { public @Nullable String getMethod() {
return this.method; return this.method;
} }
@NullUnmarked
public void setMethod(String method) { public void setMethod(String method) {
this.method = (method != null) ? method.toUpperCase(Locale.ENGLISH) : null; this.method = (method != null) ? method.toUpperCase(Locale.ENGLISH) : null;
} }

11
taglibs/src/main/java/org/springframework/security/taglibs/authz/AccessControlListTag.java

@ -19,6 +19,7 @@ package org.springframework.security.taglibs.authz;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Objects;
import jakarta.servlet.ServletContext; import jakarta.servlet.ServletContext;
import jakarta.servlet.jsp.JspException; import jakarta.servlet.jsp.JspException;
@ -27,6 +28,7 @@ import jakarta.servlet.jsp.tagext.Tag;
import jakarta.servlet.jsp.tagext.TagSupport; import jakarta.servlet.jsp.tagext.TagSupport;
import org.apache.commons.logging.Log; import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory; import org.apache.commons.logging.LogFactory;
import org.jspecify.annotations.Nullable;
import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContext;
import org.springframework.security.access.PermissionEvaluator; import org.springframework.security.access.PermissionEvaluator;
@ -60,14 +62,18 @@ public class AccessControlListTag extends TagSupport {
private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder
.getContextHolderStrategy(); .getContextHolderStrategy();
@SuppressWarnings("NullAway.Init")
private ApplicationContext applicationContext; private ApplicationContext applicationContext;
@SuppressWarnings("NullAway.Init")
private Object domainObject; private Object domainObject;
@SuppressWarnings("NullAway.Init")
private PermissionEvaluator permissionEvaluator; private PermissionEvaluator permissionEvaluator;
private String hasPermission = ""; private String hasPermission = "";
@SuppressWarnings("NullAway.Init")
private String var; private String var;
@Override @Override
@ -148,7 +154,8 @@ public class AccessControlListTag extends TagSupport {
return; return;
} }
this.applicationContext = getContext(this.pageContext); this.applicationContext = getContext(this.pageContext);
this.permissionEvaluator = getBeanOfType(PermissionEvaluator.class); this.permissionEvaluator = Objects.requireNonNull(getBeanOfType(PermissionEvaluator.class),
"PermissionEvaluator Bean is required");
String[] names = this.applicationContext.getBeanNamesForType(SecurityContextHolderStrategy.class); String[] names = this.applicationContext.getBeanNamesForType(SecurityContextHolderStrategy.class);
if (names.length == 1) { if (names.length == 1) {
SecurityContextHolderStrategy strategy = this.applicationContext SecurityContextHolderStrategy strategy = this.applicationContext
@ -157,7 +164,7 @@ public class AccessControlListTag extends TagSupport {
} }
} }
private <T> T getBeanOfType(Class<T> type) throws JspException { private <T> @Nullable T getBeanOfType(Class<T> type) throws JspException {
Map<String, T> map = this.applicationContext.getBeansOfType(type); Map<String, T> map = this.applicationContext.getBeansOfType(type);
for (ApplicationContext context = this.applicationContext.getParent(); context != null; context = context for (ApplicationContext context = this.applicationContext.getParent(); context != null; context = context
.getParent()) { .getParent()) {

5
taglibs/src/main/java/org/springframework/security/taglibs/authz/AuthenticationTag.java

@ -23,6 +23,7 @@ import jakarta.servlet.jsp.JspException;
import jakarta.servlet.jsp.PageContext; import jakarta.servlet.jsp.PageContext;
import jakarta.servlet.jsp.tagext.Tag; import jakarta.servlet.jsp.tagext.Tag;
import jakarta.servlet.jsp.tagext.TagSupport; import jakarta.servlet.jsp.tagext.TagSupport;
import org.jspecify.annotations.Nullable;
import org.springframework.beans.BeanWrapperImpl; import org.springframework.beans.BeanWrapperImpl;
import org.springframework.beans.BeansException; import org.springframework.beans.BeansException;
@ -49,9 +50,9 @@ public class AuthenticationTag extends TagSupport {
private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder
.getContextHolderStrategy(); .getContextHolderStrategy();
private String var; private @Nullable String var;
private String property; private @Nullable String property;
private int scope; private int scope;

20
taglibs/src/main/java/org/springframework/security/taglibs/authz/JspAuthorizeTag.java

@ -25,6 +25,7 @@ import jakarta.servlet.ServletResponse;
import jakarta.servlet.jsp.JspException; import jakarta.servlet.jsp.JspException;
import jakarta.servlet.jsp.PageContext; import jakarta.servlet.jsp.PageContext;
import jakarta.servlet.jsp.tagext.Tag; import jakarta.servlet.jsp.tagext.Tag;
import org.jspecify.annotations.Nullable;
import org.springframework.expression.BeanResolver; import org.springframework.expression.BeanResolver;
import org.springframework.expression.ConstructorResolver; import org.springframework.expression.ConstructorResolver;
@ -49,13 +50,16 @@ import org.springframework.security.web.FilterInvocation;
*/ */
public class JspAuthorizeTag extends AbstractAuthorizeTag implements Tag { public class JspAuthorizeTag extends AbstractAuthorizeTag implements Tag {
private Tag parent; @SuppressWarnings("NullAway.Init")
private @Nullable Tag parent;
@SuppressWarnings("NullAway.Init")
protected PageContext pageContext; protected PageContext pageContext;
protected String id; protected @Nullable String id;
private String var; @SuppressWarnings("NullAway.Init")
private @Nullable String var;
private boolean authorized; private boolean authorized;
@ -104,7 +108,7 @@ public class JspAuthorizeTag extends AbstractAuthorizeTag implements Tag {
return EVAL_PAGE; return EVAL_PAGE;
} }
public String getId() { public @Nullable String getId() {
return this.id; return this.id;
} }
@ -113,7 +117,7 @@ public class JspAuthorizeTag extends AbstractAuthorizeTag implements Tag {
} }
@Override @Override
public Tag getParent() { public @Nullable Tag getParent() {
return this.parent; return this.parent;
} }
@ -122,7 +126,7 @@ public class JspAuthorizeTag extends AbstractAuthorizeTag implements Tag {
this.parent = parent; this.parent = parent;
} }
public String getVar() { public @Nullable String getVar() {
return this.var; return this.var;
} }
@ -205,12 +209,12 @@ public class JspAuthorizeTag extends AbstractAuthorizeTag implements Tag {
} }
@Override @Override
public BeanResolver getBeanResolver() { public @Nullable BeanResolver getBeanResolver() {
return this.delegate.getBeanResolver(); return this.delegate.getBeanResolver();
} }
@Override @Override
public void setVariable(String name, Object value) { public void setVariable(String name, @Nullable Object value) {
this.delegate.setVariable(name, value); this.delegate.setVariable(name, value);
} }

3
taglibs/src/main/java/org/springframework/security/taglibs/authz/package-info.java

@ -17,4 +17,7 @@
/** /**
* JSP Security tag library implementation. * JSP Security tag library implementation.
*/ */
@NullMarked
package org.springframework.security.taglibs.authz; package org.springframework.security.taglibs.authz;
import org.jspecify.annotations.NullMarked;

23
taglibs/src/main/java/org/springframework/security/taglibs/csrf/package-info.java

@ -0,0 +1,23 @@
/*
* Copyright 2004-present the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/**
* JSP Security tag library integration with CSRF protection.
*/
@NullMarked
package org.springframework.security.taglibs.csrf;
import org.jspecify.annotations.NullMarked;

3
taglibs/src/main/java/org/springframework/security/taglibs/package-info.java

@ -17,4 +17,7 @@
/** /**
* Security related tag libraries that can be used in JSPs and templates. * Security related tag libraries that can be used in JSPs and templates.
*/ */
@NullMarked
package org.springframework.security.taglibs; package org.springframework.security.taglibs;
import org.jspecify.annotations.NullMarked;

Loading…
Cancel
Save