CVE-2024-22234: Broken Access Control in Spring Security With Direct Use of isFullyAuthenticated

@@ -27,8 +27,9 @@
 import org.apereo.cas.client.validation.TicketValidator;
 
 import org.springframework.core.log.LogMessage;
-import org.springframework.security.authentication.AnonymousAuthenticationToken;
 import org.springframework.security.authentication.AuthenticationDetailsSource;
+import org.springframework.security.authentication.AuthenticationTrustResolver;
+import org.springframework.security.authentication.AuthenticationTrustResolverImpl;
 import org.springframework.security.authentication.event.InteractiveAuthenticationSuccessEvent;
 import org.springframework.security.cas.ServiceProperties;
 import org.springframework.security.cas.authentication.CasServiceTicketAuthenticationToken;
@@ -195,6 +196,8 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil
 
 	private SecurityContextRepository securityContextRepository = new HttpSessionSecurityContextRepository();
 
+	private final AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl();
+
 	public CasAuthenticationFilter() {
 		super("/login/cas");
 		setAuthenticationFailureHandler(new SimpleUrlAuthenticationFailureHandler());
@@ -337,8 +340,7 @@ private boolean proxyTicketRequest(boolean serviceTicketRequest, HttpServletRequ
 	 */
 	private boolean authenticated() {
 		Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
-		return authentication != null && authentication.isAuthenticated()
-				&& !(authentication instanceof AnonymousAuthenticationToken);
+		return this.trustResolver.isAuthenticated(authentication);
 	}
 
 	/**

@@ -29,6 +29,7 @@
 import jakarta.servlet.http.HttpSession;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Answers;
 
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Bean;
@@ -82,6 +83,7 @@
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.withSettings;
 import static org.springframework.security.config.Customizer.withDefaults;
 import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf;
 import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.httpBasic;
@@ -304,7 +306,8 @@ public void configureWhenRegisteringObjectPostProcessorThenInvokedOnChangeSessio
 	@Test
 	public void getWhenAnonymousRequestAndTrustResolverSharedObjectReturnsAnonymousFalseThenSessionIsSaved()
 			throws Exception {
-		SharedTrustResolverConfig.TR = mock(AuthenticationTrustResolver.class);
+		SharedTrustResolverConfig.TR = mock(AuthenticationTrustResolver.class,
+				withSettings().defaultAnswer(Answers.CALLS_REAL_METHODS));
 		given(SharedTrustResolverConfig.TR.isAnonymous(any())).willReturn(false);
 		this.spring.register(SharedTrustResolverConfig.class).autowire();
 		MvcResult mvcResult = this.mvc.perform(get("/")).andReturn();

@@ -28,6 +28,7 @@ import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.context.annotation.Bean
 import org.springframework.context.annotation.Configuration
 import org.springframework.mock.web.MockHttpSession
+import org.springframework.security.authentication.TestingAuthenticationToken
 import org.springframework.security.config.annotation.web.builders.HttpSecurity
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity
 import org.springframework.security.config.http.SessionCreationPolicy
@@ -118,7 +119,7 @@ class SessionManagementDslTests {
     @Test
     fun `session management when session authentication error url then redirected to url`() {
         this.spring.register(SessionAuthenticationErrorUrlConfig::class.java).autowire()
-        val authentication: Authentication = mockk()
+        val authentication: Authentication = TestingAuthenticationToken("user", "password", "ROLE_USER")
         val session: MockHttpSession = mockk(relaxed = true)
         every { session.changeSessionId() } throws SessionAuthenticationException("any SessionAuthenticationException")
         every<Any?> { session.getAttribute(any()) } returns null
@@ -150,7 +151,7 @@ class SessionManagementDslTests {
     @Test
     fun `session management when session authentication failure handler then handler used`() {
         this.spring.register(SessionAuthenticationFailureHandlerConfig::class.java).autowire()
-        val authentication: Authentication = mockk()
+        val authentication: Authentication = TestingAuthenticationToken("user", "password", "ROLE_USER")
         val session: MockHttpSession = mockk(relaxed = true)
         every { session.changeSessionId() } throws SessionAuthenticationException("any SessionAuthenticationException")
         every<Any?> { session.getAttribute(any()) } returns null
@@ -210,7 +211,7 @@ class SessionManagementDslTests {
     fun `session management when session authentication strategy then strategy used`() {
         this.spring.register(SessionAuthenticationStrategyConfig::class.java).autowire()
         mockkObject(SessionAuthenticationStrategyConfig.STRATEGY)
-        val authentication: Authentication = mockk(relaxed = true)
+        val authentication: Authentication = TestingAuthenticationToken("user", "password", "ROLE_USER")
         val session: MockHttpSession = mockk(relaxed = true)
         every { session.changeSessionId() } throws SessionAuthenticationException("any SessionAuthenticationException")
         every<Any?> { session.getAttribute(any()) } returns null

@@ -147,7 +147,7 @@ public final boolean isAnonymous() {
 
 	@Override
 	public final boolean isAuthenticated() {
-		return !isAnonymous();
+		return this.trustResolver.isAuthenticated(getAuthentication());
 	}
 
 	@Override

@@ -61,13 +61,25 @@ public interface AuthenticationTrustResolver {
 	 * <p>
 	 * @param authentication to test (may be <code>null</code> in which case the method
 	 * will always return <code>false</code>)
-	 * @return <code>true</code> the passed authentication token represented an anonymous
-	 * principal and is authenticated using a remember-me token, <code>false</code>
-	 * otherwise
+	 * @return <code>true</code> the passed authentication token represented an
+	 * authenticated user ({@link #isAuthenticated(Authentication)} and not
+	 * {@link #isRememberMe(Authentication)}, <code>false</code> otherwise
 	 * @since 6.1
 	 */
 	default boolean isFullyAuthenticated(Authentication authentication) {
-		return !isAnonymous(authentication) && !isRememberMe(authentication);
+		return isAuthenticated(authentication) && !isRememberMe(authentication);
+	}
+
+	/**
+	 * Checks if the {@link Authentication} is not null, authenticated, and not anonymous.
+	 * @param authentication the {@link Authentication} to check.
+	 * @return true if the {@link Authentication} is not null,
+	 * {@link #isAnonymous(Authentication)} returns false, &
+	 * {@link Authentication#isAuthenticated()} is true.
+	 * @since 6.1.7
+	 */
+	default boolean isAuthenticated(Authentication authentication) {
+		return authentication != null && authentication.isAuthenticated() && !isAnonymous(authentication);
 	}
 
 }

@@ -133,8 +133,7 @@ private static class AuthenticatedAuthorizationStrategy extends AbstractAuthoriz
 
 		@Override
 		boolean isGranted(Authentication authentication) {
-			return authentication != null && !this.trustResolver.isAnonymous(authentication)
-					&& authentication.isAuthenticated();
+			return this.trustResolver.isAuthenticated(authentication);
 		}
 
 	}
@@ -143,7 +142,7 @@ private static final class FullyAuthenticatedAuthorizationStrategy extends Authe
 
 		@Override
 		boolean isGranted(Authentication authentication) {
-			return authentication != null && this.trustResolver.isFullyAuthenticated(authentication);
+			return this.trustResolver.isFullyAuthenticated(authentication);
 		}
 
 	}

@@ -25,6 +25,7 @@
 import org.springframework.security.core.authority.AuthorityUtils;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
 
@@ -134,4 +135,27 @@ public void hasAuthorityDoesNotAddDefaultPrefix() {
 		assertThat(this.root.hasAnyAuthority("ROLE_A", "NOT")).isTrue();
 	}
 
+	@Test
+	void isAuthenticatedWhenAuthenticatedNullThenException() {
+		this.root = new SecurityExpressionRoot((Authentication) null) {
+		};
+		assertThatIllegalArgumentException().isThrownBy(() -> this.root.isAuthenticated());
+	}
+
+	@Test
+	void isAuthenticatedWhenTrustResolverFalseThenFalse() {
+		AuthenticationTrustResolver atr = mock(AuthenticationTrustResolver.class);
+		given(atr.isAuthenticated(JOE)).willReturn(false);
+		this.root.setTrustResolver(atr);
+		assertThat(this.root.isAuthenticated()).isFalse();
+	}
+
+	@Test
+	void isAuthenticatedWhenTrustResolverTrueThenTrue() {
+		AuthenticationTrustResolver atr = mock(AuthenticationTrustResolver.class);
+		given(atr.isAuthenticated(JOE)).willReturn(true);
+		this.root.setTrustResolver(atr);
+		assertThat(this.root.isAuthenticated()).isTrue();
+	}
+
 }

@@ -18,6 +18,7 @@
 
 import org.junit.jupiter.api.Test;
 
+import org.springframework.security.core.Authentication;
 import org.springframework.security.core.authority.AuthorityUtils;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -63,4 +64,56 @@ public void testGettersSetters() {
 		assertThat(trustResolver.getRememberMeClass()).isEqualTo(TestingAuthenticationToken.class);
 	}
 
+	@Test
+	void isAuthenticatedWhenAuthenticationNullThenFalse() {
+		AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl();
+		Authentication authentication = null;
+		assertThat(trustResolver.isAuthenticated(authentication)).isFalse();
+	}
+
+	@Test
+	void isAuthenticatedWhenAuthenticationNotAuthenticatedThenFalse() {
+		AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl();
+		TestingAuthenticationToken authentication = new TestingAuthenticationToken("user", "password");
+		assertThat(trustResolver.isAuthenticated(authentication)).isFalse();
+	}
+
+	@Test
+	void isAuthenticatedWhenAnonymousThenFalse() {
+		AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl();
+		AnonymousAuthenticationToken authentication = new AnonymousAuthenticationToken("key", "principal",
+				AuthorityUtils.createAuthorityList("ROLE_ANONYMOUS"));
+		assertThat(trustResolver.isAuthenticated(authentication)).isFalse();
+	}
+
+	@Test
+	void isFullyAuthenticatedWhenAuthenticationNullThenFalse() {
+		AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl();
+		Authentication authentication = null;
+		assertThat(trustResolver.isFullyAuthenticated(authentication)).isFalse();
+	}
+
+	@Test
+	void isFullyAuthenticatedWhenAuthenticationNotAuthenticatedThenFalse() {
+		AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl();
+		TestingAuthenticationToken authentication = new TestingAuthenticationToken("user", "password");
+		assertThat(trustResolver.isFullyAuthenticated(authentication)).isFalse();
+	}
+
+	@Test
+	void isFullyAuthenticatedWhenAnonymousThenFalse() {
+		AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl();
+		AnonymousAuthenticationToken authentication = new AnonymousAuthenticationToken("key", "principal",
+				AuthorityUtils.createAuthorityList("ROLE_ANONYMOUS"));
+		assertThat(trustResolver.isFullyAuthenticated(authentication)).isFalse();
+	}
+
+	@Test
+	void isFullyAuthenticatedWhenRememberMeThenFalse() {
+		AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl();
+		RememberMeAuthenticationToken authentication = new RememberMeAuthenticationToken("key", "user",
+				AuthorityUtils.createAuthorityList("ROLE_USER"));
+		assertThat(trustResolver.isFullyAuthenticated(authentication)).isFalse();
+	}
+
 }

@@ -22,6 +22,7 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Answers;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 
@@ -50,7 +51,7 @@
 @ExtendWith(MockitoExtension.class)
 public class DefaultMessageSecurityExpressionHandlerTests {
 
-	@Mock
+	@Mock(answer = Answers.CALLS_REAL_METHODS)
 	AuthenticationTrustResolver trustResolver;
 
 	@Mock

@@ -107,8 +107,7 @@ public void removeAuthorizedClient(String clientRegistrationId, Authentication p
 	}
 
 	private boolean isPrincipalAuthenticated(Authentication authentication) {
-		return authentication != null && !this.authenticationTrustResolver.isAnonymous(authentication)
-				&& authentication.isAuthenticated();
+		return this.authenticationTrustResolver.isAuthenticated(authentication);
 	}
 
 }

@@ -106,8 +106,7 @@ public Mono<Void> removeAuthorizedClient(String clientRegistrationId, Authentica
 	}
 
 	private boolean isPrincipalAuthenticated(Authentication authentication) {
-		return authentication != null && !this.authenticationTrustResolver.isAnonymous(authentication)
-				&& authentication.isAuthenticated();
+		return this.authenticationTrustResolver.isAuthenticated(authentication);
 	}
 
 }

@@ -52,7 +52,7 @@ public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
 		return chain.filter(exchange)
 			.onErrorResume(AccessDeniedException.class, (denied) -> exchange.getPrincipal()
 				.filter((principal) -> (!(principal instanceof Authentication) || (principal instanceof Authentication
-						&& !(this.authenticationTrustResolver.isAnonymous((Authentication) principal)))))
+						&& (this.authenticationTrustResolver.isAuthenticated((Authentication) principal)))))
 				.switchIfEmpty(commenceAuthentication(exchange,
 						new InsufficientAuthenticationException(
 								"Full authentication is required to access this resource")))

@@ -93,7 +93,7 @@ public SecurityContextHolderAwareRequestWrapper(HttpServletRequest request,
 	 */
 	private Authentication getAuthentication() {
 		Authentication auth = this.securityContextHolderStrategy.getContext().getAuthentication();
-		return (!this.trustResolver.isAnonymous(auth)) ? auth : null;
+		return (this.trustResolver.isAuthenticated(auth)) ? auth : null;
 	}
 
 	/**

@@ -94,7 +94,7 @@ private void doFilter(HttpServletRequest request, HttpServletResponse response,
 		request.setAttribute(FILTER_APPLIED, Boolean.TRUE);
 		if (!this.securityContextRepository.containsContext(request)) {
 			Authentication authentication = this.securityContextHolderStrategy.getContext().getAuthentication();
-			if (authentication != null && !this.trustResolver.isAnonymous(authentication)) {
+			if (this.trustResolver.isAuthenticated(authentication)) {
 				// The user has been authenticated during the current request, so call the
 				// session strategy
 				try {

@@ -140,7 +140,7 @@ public void testGetRemoteUserStringWithAuthenticatedPrincipal() {
 		String username = "authPrincipalUsername";
 		AuthenticatedPrincipal principal = mock(AuthenticatedPrincipal.class);
 		given(principal.getName()).willReturn(username);
-		Authentication auth = new TestingAuthenticationToken(principal, "user");
+		Authentication auth = new TestingAuthenticationToken(principal, "user", "ROLE_USER");
 		SecurityContextHolder.getContext().setAuthentication(auth);
 		MockHttpServletRequest request = new MockHttpServletRequest();
 		request.setRequestURI("/");

@@ -21,6 +21,7 @@
 import jakarta.servlet.http.HttpServletResponse;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Test;
+import org.mockito.Answers;
 
 import org.springframework.mock.web.MockFilterChain;
 import org.springframework.mock.web.MockHttpServletRequest;
@@ -42,6 +43,7 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.withSettings;
 
 /**
  * @author Luke Taylor
@@ -176,7 +178,8 @@ public void responseIsRedirectedToRequestedUrlIfSetAndSessionIsInvalid() throws
 
 	@Test
 	public void customAuthenticationTrustResolver() throws Exception {
-		AuthenticationTrustResolver trustResolver = mock(AuthenticationTrustResolver.class);
+		AuthenticationTrustResolver trustResolver = mock(AuthenticationTrustResolver.class,
+				withSettings().defaultAnswer(Answers.CALLS_REAL_METHODS));
 		SecurityContextRepository repo = mock(SecurityContextRepository.class);
 		SessionManagementFilter filter = new SessionManagementFilter(repo);
 		filter.setTrustResolver(trustResolver);
@@ -194,7 +197,8 @@ public void setTrustResolverNull() {
 	}
 
 	private void authenticateUser() {
-		SecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken("user", "pass"));
+		SecurityContextHolder.getContext()
+			.setAuthentication(new TestingAuthenticationToken("user", "pass", "ROLE_USER"));
 	}
 
 }