Explorar el Código

Enable Null checking in spring-security-cas via JSpecify

Closes gh-16882
Rob Winch hace 1 semana
padre
commit
a4a4908d71
Se han modificado 18 ficheros con 112 adiciones y 25 borrados
  1. 4 0
      cas/spring-security-cas.gradle
  2. 4 2
      cas/src/main/java/org/springframework/security/cas/ServiceProperties.java
  3. 16 7
      cas/src/main/java/org/springframework/security/cas/authentication/CasAuthenticationProvider.java
  4. 4 2
      cas/src/main/java/org/springframework/security/cas/authentication/CasServiceTicketAuthenticationToken.java
  5. 3 1
      cas/src/main/java/org/springframework/security/cas/authentication/NullStatelessTicketCache.java
  6. 2 1
      cas/src/main/java/org/springframework/security/cas/authentication/SpringCacheBasedTicketCache.java
  7. 3 1
      cas/src/main/java/org/springframework/security/cas/authentication/StatelessTicketCache.java
  8. 3 0
      cas/src/main/java/org/springframework/security/cas/authentication/package-info.java
  9. 23 0
      cas/src/main/java/org/springframework/security/cas/jackson2/package-info.java
  10. 3 0
      cas/src/main/java/org/springframework/security/cas/package-info.java
  11. 23 0
      cas/src/main/java/org/springframework/security/cas/userdetails/package-info.java
  12. 4 2
      cas/src/main/java/org/springframework/security/cas/web/CasAuthenticationEntryPoint.java
  13. 5 3
      cas/src/main/java/org/springframework/security/cas/web/CasAuthenticationFilter.java
  14. 4 3
      cas/src/main/java/org/springframework/security/cas/web/authentication/DefaultServiceAuthenticationDetails.java
  15. 3 0
      cas/src/main/java/org/springframework/security/cas/web/authentication/package-info.java
  16. 3 0
      cas/src/main/java/org/springframework/security/cas/web/package-info.java
  17. 4 2
      core/src/main/java/org/springframework/security/authentication/BadCredentialsException.java
  18. 1 1
      core/src/main/java/org/springframework/security/core/AuthenticationException.java

+ 4 - 0
cas/spring-security-cas.gradle

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

+ 4 - 2
cas/src/main/java/org/springframework/security/cas/ServiceProperties.java

@@ -16,6 +16,8 @@
 
 package org.springframework.security.cas;
 
+import org.jspecify.annotations.Nullable;
+
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.util.Assert;
 
@@ -34,7 +36,7 @@ public class ServiceProperties implements InitializingBean {
 
 	public static final String DEFAULT_CAS_SERVICE_PARAMETER = "service";
 
-	private String service;
+	private @Nullable String service;
 
 	private boolean authenticateAllArtifacts;
 
@@ -62,7 +64,7 @@ public class ServiceProperties implements InitializingBean {
 	 * </pre>
 	 * @return the URL of the service the user is authenticating to
 	 */
-	public final String getService() {
+	public final @Nullable String getService() {
 		return this.service;
 	}
 

+ 16 - 7
cas/src/main/java/org/springframework/security/cas/authentication/CasAuthenticationProvider.java

@@ -21,6 +21,8 @@ import org.apache.commons.logging.LogFactory;
 import org.apereo.cas.client.validation.Assertion;
 import org.apereo.cas.client.validation.TicketValidationException;
 import org.apereo.cas.client.validation.TicketValidator;
+import org.jspecify.annotations.NullUnmarked;
+import org.jspecify.annotations.Nullable;
 
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.context.MessageSource;
@@ -62,6 +64,7 @@ public class CasAuthenticationProvider implements AuthenticationProvider, Initia
 
 	private static final Log logger = LogFactory.getLog(CasAuthenticationProvider.class);
 
+	@SuppressWarnings("NullAway.Init")
 	private AuthenticationUserDetailsService<CasAssertionAuthenticationToken> authenticationUserDetailsService;
 
 	private UserDetailsChecker userDetailsChecker = new AccountStatusUserDetailsChecker();
@@ -70,11 +73,13 @@ public class CasAuthenticationProvider implements AuthenticationProvider, Initia
 
 	private StatelessTicketCache statelessTicketCache = new NullStatelessTicketCache();
 
+	@SuppressWarnings("NullAway.Init")
 	private String key;
 
+	@SuppressWarnings("NullAway.Init")
 	private TicketValidator ticketValidator;
 
-	private ServiceProperties serviceProperties;
+	private @Nullable ServiceProperties serviceProperties;
 
 	private GrantedAuthoritiesMapper authoritiesMapper = new NullAuthoritiesMapper();
 
@@ -89,7 +94,7 @@ public class CasAuthenticationProvider implements AuthenticationProvider, Initia
 	}
 
 	@Override
-	public Authentication authenticate(Authentication authentication) throws AuthenticationException {
+	public @Nullable Authentication authenticate(Authentication authentication) throws AuthenticationException {
 		if (!supports(authentication.getClass())) {
 			return null;
 		}
@@ -129,11 +134,14 @@ public class CasAuthenticationProvider implements AuthenticationProvider, Initia
 
 	private CasAuthenticationToken authenticateNow(final Authentication authentication) throws AuthenticationException {
 		try {
-			Assertion assertion = this.ticketValidator.validate(authentication.getCredentials().toString(),
-					getServiceUrl(authentication));
+			Object credentials = authentication.getCredentials();
+			if (credentials == null) {
+				throw new BadCredentialsException("Authentication.getCredentials() cannot be null");
+			}
+			Assertion assertion = this.ticketValidator.validate(credentials.toString(), getServiceUrl(authentication));
 			UserDetails userDetails = loadUserByAssertion(assertion);
 			this.userDetailsChecker.check(userDetails);
-			return new CasAuthenticationToken(this.key, userDetails, authentication.getCredentials(),
+			return new CasAuthenticationToken(this.key, userDetails, credentials,
 					this.authoritiesMapper.mapAuthorities(userDetails.getAuthorities()), userDetails, assertion);
 		}
 		catch (TicketValidationException ex) {
@@ -149,7 +157,8 @@ public class CasAuthenticationProvider implements AuthenticationProvider, Initia
 	 * @param authentication
 	 * @return
 	 */
-	private String getServiceUrl(Authentication authentication) {
+	@NullUnmarked
+	private @Nullable String getServiceUrl(Authentication authentication) {
 		String serviceUrl;
 		if (authentication.getDetails() instanceof ServiceAuthenticationDetails) {
 			return ((ServiceAuthenticationDetails) authentication.getDetails()).getServiceUrl();
@@ -215,7 +224,7 @@ public class CasAuthenticationProvider implements AuthenticationProvider, Initia
 		return this.statelessTicketCache;
 	}
 
-	protected TicketValidator getTicketValidator() {
+	protected @Nullable TicketValidator getTicketValidator() {
 		return this.ticketValidator;
 	}
 

+ 4 - 2
cas/src/main/java/org/springframework/security/cas/authentication/CasServiceTicketAuthenticationToken.java

@@ -19,6 +19,8 @@ package org.springframework.security.cas.authentication;
 import java.io.Serial;
 import java.util.Collection;
 
+import org.jspecify.annotations.Nullable;
+
 import org.springframework.security.authentication.AbstractAuthenticationToken;
 import org.springframework.security.core.GrantedAuthority;
 import org.springframework.util.Assert;
@@ -41,7 +43,7 @@ public class CasServiceTicketAuthenticationToken extends AbstractAuthenticationT
 
 	private final String identifier;
 
-	private Object credentials;
+	private @Nullable Object credentials;
 
 	/**
 	 * This constructor can be safely used by any code that wishes to create a
@@ -86,7 +88,7 @@ public class CasServiceTicketAuthenticationToken extends AbstractAuthenticationT
 	}
 
 	@Override
-	public Object getCredentials() {
+	public @Nullable Object getCredentials() {
 		return this.credentials;
 	}
 

+ 3 - 1
cas/src/main/java/org/springframework/security/cas/authentication/NullStatelessTicketCache.java

@@ -16,6 +16,8 @@
 
 package org.springframework.security.cas.authentication;
 
+import org.jspecify.annotations.Nullable;
+
 /**
  * Implementation of @link {@link StatelessTicketCache} that has no backing cache. Useful
  * in instances where storing of tickets for stateless session management is not required.
@@ -33,7 +35,7 @@ public final class NullStatelessTicketCache implements StatelessTicketCache {
 	 * @return null since we are not storing any tickets.
 	 */
 	@Override
-	public CasAuthenticationToken getByTicketId(final String serviceTicket) {
+	public @Nullable CasAuthenticationToken getByTicketId(final String serviceTicket) {
 		return null;
 	}
 

+ 2 - 1
cas/src/main/java/org/springframework/security/cas/authentication/SpringCacheBasedTicketCache.java

@@ -18,6 +18,7 @@ package org.springframework.security.cas.authentication;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.jspecify.annotations.Nullable;
 
 import org.springframework.cache.Cache;
 import org.springframework.core.log.LogMessage;
@@ -42,7 +43,7 @@ public class SpringCacheBasedTicketCache implements StatelessTicketCache {
 	}
 
 	@Override
-	public CasAuthenticationToken getByTicketId(final String serviceTicket) {
+	public @Nullable CasAuthenticationToken getByTicketId(final String serviceTicket) {
 		final Cache.ValueWrapper element = (serviceTicket != null) ? this.cache.get(serviceTicket) : null;
 		logger.debug(LogMessage.of(() -> "Cache hit: " + (element != null) + "; service ticket: " + serviceTicket));
 		return (element != null) ? (CasAuthenticationToken) element.get() : null;

+ 3 - 1
cas/src/main/java/org/springframework/security/cas/authentication/StatelessTicketCache.java

@@ -16,6 +16,8 @@
 
 package org.springframework.security.cas.authentication;
 
+import org.jspecify.annotations.Nullable;
+
 /**
  * Caches CAS service tickets and CAS proxy tickets for stateless connections.
  *
@@ -69,7 +71,7 @@ public interface StatelessTicketCache {
 	 * </p>
 	 * @return the fully populated authentication token
 	 */
-	CasAuthenticationToken getByTicketId(String serviceTicket);
+	@Nullable CasAuthenticationToken getByTicketId(String serviceTicket);
 
 	/**
 	 * Adds the specified <code>CasAuthenticationToken</code> to the cache.

+ 3 - 0
cas/src/main/java/org/springframework/security/cas/authentication/package-info.java

@@ -18,4 +18,7 @@
  * An {@code AuthenticationProvider} that can process CAS service tickets and proxy
  * tickets.
  */
+@NullMarked
 package org.springframework.security.cas.authentication;
+
+import org.jspecify.annotations.NullMarked;

+ 23 - 0
cas/src/main/java/org/springframework/security/cas/jackson2/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.
+ */
+
+/**
+ * Jackson support for CAS.
+ */
+@NullMarked
+package org.springframework.security.cas.jackson2;
+
+import org.jspecify.annotations.NullMarked;

+ 3 - 0
cas/src/main/java/org/springframework/security/cas/package-info.java

@@ -18,4 +18,7 @@
  * Spring Security support for Apereo's Central Authentication Service
  * (<a href="https://github.com/apereo/cas">CAS</a>).
  */
+@NullMarked
 package org.springframework.security.cas;
+
+import org.jspecify.annotations.NullMarked;

+ 23 - 0
cas/src/main/java/org/springframework/security/cas/userdetails/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.
+ */
+
+/**
+ * {@link org.springframework.security.core.userdetails.UserDetails} abstractions for CAS.
+ */
+@NullMarked
+package org.springframework.security.cas.userdetails;
+
+import org.jspecify.annotations.NullMarked;

+ 4 - 2
cas/src/main/java/org/springframework/security/cas/web/CasAuthenticationEntryPoint.java

@@ -22,6 +22,7 @@ import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
 import org.apereo.cas.client.util.CommonUtils;
 import org.apereo.cas.client.util.WebUtils;
+import org.jspecify.annotations.Nullable;
 
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.security.cas.ServiceProperties;
@@ -47,9 +48,10 @@ import org.springframework.util.Assert;
  */
 public class CasAuthenticationEntryPoint implements AuthenticationEntryPoint, InitializingBean {
 
+	@SuppressWarnings("NullAway.Init")
 	private ServiceProperties serviceProperties;
 
-	private String loginUrl;
+	private @Nullable String loginUrl;
 
 	/**
 	 * Determines whether the Service URL should include the session id for the specific
@@ -117,7 +119,7 @@ public class CasAuthenticationEntryPoint implements AuthenticationEntryPoint, In
 	 * <code>https://www.mycompany.com/cas/login</code>.
 	 * @return the enterprise-wide CAS login URL
 	 */
-	public final String getLoginUrl() {
+	public final @Nullable String getLoginUrl() {
 		return this.loginUrl;
 	}
 

+ 5 - 3
cas/src/main/java/org/springframework/security/cas/web/CasAuthenticationFilter.java

@@ -26,6 +26,7 @@ import jakarta.servlet.http.HttpSession;
 import org.apereo.cas.client.proxy.ProxyGrantingTicketStorage;
 import org.apereo.cas.client.util.WebUtils;
 import org.apereo.cas.client.validation.TicketValidator;
+import org.jspecify.annotations.Nullable;
 
 import org.springframework.core.log.LogMessage;
 import org.springframework.security.authentication.AuthenticationDetailsSource;
@@ -190,12 +191,12 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil
 	/**
 	 * The last portion of the receptor url, i.e. /proxy/receptor
 	 */
-	private RequestMatcher proxyReceptorMatcher;
+	private @Nullable RequestMatcher proxyReceptorMatcher;
 
 	/**
 	 * The backing storage to store ProxyGrantingTicket requests.
 	 */
-	private ProxyGrantingTicketStorage proxyGrantingTicketStorage;
+	private @Nullable ProxyGrantingTicketStorage proxyGrantingTicketStorage;
 
 	private String artifactParameter = ServiceProperties.DEFAULT_CAS_ARTIFACT_PARAMETER;
 
@@ -244,7 +245,7 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil
 	}
 
 	@Override
-	public Authentication attemptAuthentication(HttpServletRequest request, HttpServletResponse response)
+	public @Nullable Authentication attemptAuthentication(HttpServletRequest request, HttpServletResponse response)
 			throws AuthenticationException, IOException {
 		// if the request is a proxy request process it and return null to indicate the
 		// request has been processed
@@ -422,6 +423,7 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil
 	 * @param request
 	 * @return
 	 */
+	@SuppressWarnings("NullAway") // Dataflow analysis limitation
 	private boolean proxyReceptorRequest(HttpServletRequest request) {
 		final boolean result = proxyReceptorConfigured() && this.proxyReceptorMatcher.matches(request);
 		this.logger.debug(LogMessage.format("proxyReceptorRequest = %s", result));

+ 4 - 3
cas/src/main/java/org/springframework/security/cas/web/authentication/DefaultServiceAuthenticationDetails.java

@@ -21,6 +21,7 @@ import java.net.URL;
 import java.util.regex.Pattern;
 
 import jakarta.servlet.http.HttpServletRequest;
+import org.jspecify.annotations.Nullable;
 
 import org.springframework.security.cas.authentication.ServiceAuthenticationDetails;
 import org.springframework.security.web.authentication.WebAuthenticationDetails;
@@ -49,8 +50,8 @@ final class DefaultServiceAuthenticationDetails extends WebAuthenticationDetails
 	 * string from containing the artifact name and value. This can be created using
 	 * {@link #createArtifactPattern(String)}.
 	 */
-	DefaultServiceAuthenticationDetails(String casService, HttpServletRequest request, Pattern artifactPattern)
-			throws MalformedURLException {
+	DefaultServiceAuthenticationDetails(@Nullable String casService, HttpServletRequest request,
+			Pattern artifactPattern) throws MalformedURLException {
 		super(request);
 		URL casServiceUrl = new URL(casService);
 		int port = getServicePort(casServiceUrl);
@@ -104,7 +105,7 @@ final class DefaultServiceAuthenticationDetails extends WebAuthenticationDetails
 	 * @return the query String minus the artifactParameterName and the corresponding
 	 * value.
 	 */
-	private String getQueryString(final HttpServletRequest request, final Pattern artifactPattern) {
+	private @Nullable String getQueryString(final HttpServletRequest request, final Pattern artifactPattern) {
 		final String query = request.getQueryString();
 		if (query == null) {
 			return null;

+ 3 - 0
cas/src/main/java/org/springframework/security/cas/web/authentication/package-info.java

@@ -18,4 +18,7 @@
  * Authentication processing mechanisms which respond to the submission of authentication
  * credentials using CAS.
  */
+@NullMarked
 package org.springframework.security.cas.web.authentication;
+
+import org.jspecify.annotations.NullMarked;

+ 3 - 0
cas/src/main/java/org/springframework/security/cas/web/package-info.java

@@ -17,4 +17,7 @@
 /**
  * Authenticates standard web browser users via CAS.
  */
+@NullMarked
 package org.springframework.security.cas.web;
+
+import org.jspecify.annotations.NullMarked;

+ 4 - 2
core/src/main/java/org/springframework/security/authentication/BadCredentialsException.java

@@ -18,6 +18,8 @@ package org.springframework.security.authentication;
 
 import java.io.Serial;
 
+import org.jspecify.annotations.Nullable;
+
 import org.springframework.security.core.AuthenticationException;
 
 /**
@@ -35,7 +37,7 @@ public class BadCredentialsException extends AuthenticationException {
 	 * Constructs a <code>BadCredentialsException</code> with the specified message.
 	 * @param msg the detail message
 	 */
-	public BadCredentialsException(String msg) {
+	public BadCredentialsException(@Nullable String msg) {
 		super(msg);
 	}
 
@@ -45,7 +47,7 @@ public class BadCredentialsException extends AuthenticationException {
 	 * @param msg the detail message
 	 * @param cause root cause
 	 */
-	public BadCredentialsException(String msg, Throwable cause) {
+	public BadCredentialsException(@Nullable String msg, Throwable cause) {
 		super(msg, cause);
 	}
 

+ 1 - 1
core/src/main/java/org/springframework/security/core/AuthenticationException.java

@@ -50,7 +50,7 @@ public abstract class AuthenticationException extends RuntimeException {
 	 * root cause.
 	 * @param msg the detail message
 	 */
-	public AuthenticationException(String msg) {
+	public AuthenticationException(@Nullable String msg) {
 		super(msg);
 	}