Skip to content

Commit

Permalink
Merge branch '6.1.x' into 6.2.x
Browse files Browse the repository at this point in the history
  • Loading branch information
marcusdacoregio committed Feb 16, 2024
2 parents 94f885c + 750cb30 commit 15306c1
Show file tree
Hide file tree
Showing 16 changed files with 123 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -199,6 +200,8 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil
private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder
.getContextHolderStrategy();

private final AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl();

public CasAuthenticationFilter() {
super("/#/cas");
setAuthenticationFailureHandler(new SimpleUrlAuthenticationFailureHandler());
Expand Down Expand Up @@ -348,8 +351,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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public final boolean isAnonymous() {

@Override
public final boolean isAuthenticated() {
return !isAnonymous();
return this.trustResolver.isAuthenticated(getAuthentication());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

}
Expand All @@ -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);
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -50,7 +51,7 @@
@ExtendWith(MockitoExtension.class)
public class DefaultMessageSecurityExpressionHandlerTests {

@Mock
@Mock(answer = Answers.CALLS_REAL_METHODS)
AuthenticationTrustResolver trustResolver;

@Mock
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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")))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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("/");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Answers;

import org.springframework.http.HttpStatus;
import org.springframework.mock.web.MockFilterChain;
Expand All @@ -45,6 +46,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
Expand Down Expand Up @@ -244,7 +246,8 @@ public void responseIsRedirectedToRequestedUrlIfStatusCodeIsSetAndSessionIsInval

@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);
Expand All @@ -262,7 +265,8 @@ public void setTrustResolverNull() {
}

private void authenticateUser() {
SecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken("user", "pass"));
SecurityContextHolder.getContext()
.setAuthentication(new TestingAuthenticationToken("user", "pass", "ROLE_USER"));
}

}

0 comments on commit 15306c1

Please # to comment.