Skip to content

Commit

Permalink
Default Require Explicit Session Management = true
Browse files Browse the repository at this point in the history
Closes gh-11763
  • Loading branch information
rwinch committed Oct 1, 2022
1 parent 0d58c51 commit 4479cef
Show file tree
Hide file tree
Showing 20 changed files with 105 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import jakarta.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpSession;
Expand Down Expand Up @@ -135,7 +137,9 @@ public final class SessionManagementConfigurer<H extends HttpSecurityBuilder<H>>

private AuthenticationFailureHandler sessionAuthenticationFailureHandler;

private boolean requireExplicitAuthenticationStrategy;
private Set<String> propertiesThatRequireImplicitAuthentication = new HashSet<>();

private Boolean requireExplicitAuthenticationStrategy;

/**
* Creates a new instance
Expand All @@ -154,6 +158,7 @@ public SessionManagementConfigurer() {
*/
public SessionManagementConfigurer<H> invalidSessionUrl(String invalidSessionUrl) {
this.invalidSessionUrl = invalidSessionUrl;
this.propertiesThatRequireImplicitAuthentication.add("invalidSessionUrl = " + invalidSessionUrl);
return this;
}

Expand Down Expand Up @@ -181,6 +186,7 @@ public SessionManagementConfigurer<H> requireExplicitAuthenticationStrategy(
public SessionManagementConfigurer<H> invalidSessionStrategy(InvalidSessionStrategy invalidSessionStrategy) {
Assert.notNull(invalidSessionStrategy, "invalidSessionStrategy");
this.invalidSessionStrategy = invalidSessionStrategy;
this.propertiesThatRequireImplicitAuthentication.add("invalidSessionStrategy = " + invalidSessionStrategy);
return this;
}

Expand All @@ -195,6 +201,8 @@ public SessionManagementConfigurer<H> invalidSessionStrategy(InvalidSessionStrat
*/
public SessionManagementConfigurer<H> sessionAuthenticationErrorUrl(String sessionAuthenticationErrorUrl) {
this.sessionAuthenticationErrorUrl = sessionAuthenticationErrorUrl;
this.propertiesThatRequireImplicitAuthentication
.add("sessionAuthenticationErrorUrl = " + sessionAuthenticationErrorUrl);
return this;
}

Expand All @@ -210,6 +218,8 @@ public SessionManagementConfigurer<H> sessionAuthenticationErrorUrl(String sessi
public SessionManagementConfigurer<H> sessionAuthenticationFailureHandler(
AuthenticationFailureHandler sessionAuthenticationFailureHandler) {
this.sessionAuthenticationFailureHandler = sessionAuthenticationFailureHandler;
this.propertiesThatRequireImplicitAuthentication
.add("sessionAuthenticationFailureHandler = " + sessionAuthenticationFailureHandler);
return this;
}

Expand Down Expand Up @@ -245,6 +255,7 @@ public SessionManagementConfigurer<H> enableSessionUrlRewriting(boolean enableSe
public SessionManagementConfigurer<H> sessionCreationPolicy(SessionCreationPolicy sessionCreationPolicy) {
Assert.notNull(sessionCreationPolicy, "sessionCreationPolicy cannot be null");
this.sessionPolicy = sessionCreationPolicy;
this.propertiesThatRequireImplicitAuthentication.add("sessionCreationPolicy = " + sessionCreationPolicy);
return this;
}

Expand All @@ -266,6 +277,8 @@ public SessionManagementConfigurer<H> sessionCreationPolicy(SessionCreationPolic
public SessionManagementConfigurer<H> sessionAuthenticationStrategy(
SessionAuthenticationStrategy sessionAuthenticationStrategy) {
this.providedSessionAuthenticationStrategy = sessionAuthenticationStrategy;
this.propertiesThatRequireImplicitAuthentication
.add("sessionAuthenticationStrategy = " + sessionAuthenticationStrategy);
return this;
}

Expand Down Expand Up @@ -309,6 +322,7 @@ public SessionManagementConfigurer<H> sessionFixation(
*/
public ConcurrencyControlConfigurer maximumSessions(int maximumSessions) {
this.maximumSessions = maximumSessions;
this.propertiesThatRequireImplicitAuthentication.add("maximumSessions = " + maximumSessions);
return new ConcurrencyControlConfigurer();
}

Expand Down Expand Up @@ -384,8 +398,26 @@ public void configure(H http) {
}
}

private boolean shouldRequireExplicitAuthenticationStrategy() {
boolean defaultRequireExplicitAuthenticationStrategy = this.propertiesThatRequireImplicitAuthentication
.isEmpty();
if (this.requireExplicitAuthenticationStrategy == null) {
// explicit is not set, use default
return defaultRequireExplicitAuthenticationStrategy;
}
if (this.requireExplicitAuthenticationStrategy && !defaultRequireExplicitAuthenticationStrategy) {
// explicit disabled and implicit requires it
throw new IllegalStateException(
"Invalid configuration that explicitly sets requireExplicitAuthenticationStrategy to "
+ this.requireExplicitAuthenticationStrategy
+ " but implicitly requires it due to the following properties being set: "
+ this.propertiesThatRequireImplicitAuthentication);
}
return this.requireExplicitAuthenticationStrategy;
}

private SessionManagementFilter createSessionManagementFilter(H http) {
if (this.requireExplicitAuthenticationStrategy) {
if (shouldRequireExplicitAuthenticationStrategy()) {
return null;
}
SecurityContextRepository securityContextRepository = http.getSharedObject(SecurityContextRepository.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -539,9 +539,10 @@ else if (StringUtils.hasText(invalidSessionStrategyRef)) {
sessionMgmtFilter.addPropertyReference("invalidSessionStrategy", invalidSessionStrategyRef);
}
sessionMgmtFilter.addConstructorArgReference(sessionAuthStratRef);
boolean registerSessionMgmtFilter = (sessionMgmtElt == null
|| !"true".equals(sessionMgmtElt.getAttribute(ATT_AUTHENTICATION_STRATEGY_EXPLICIT_INVOCATION)));
if (registerSessionMgmtFilter) {
boolean registerSessionMgmtFilter = (sessionMgmtElt != null
&& "false".equals(sessionMgmtElt.getAttribute(ATT_AUTHENTICATION_STRATEGY_EXPLICIT_INVOCATION)));
if (registerSessionMgmtFilter || StringUtils.hasText(errorUrl) || StringUtils.hasText(invalidSessionUrl)
|| StringUtils.hasText(invalidSessionStrategyRef)) {
this.sfpf = (RootBeanDefinition) sessionMgmtFilter.getBeanDefinition();
}
this.sessionStrategyRef = new RuntimeBeanReference(sessionAuthStratRef);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
import org.springframework.security.web.header.HeaderWriterFilter;
import org.springframework.security.web.savedrequest.RequestCacheAwareFilter;
import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter;
import org.springframework.security.web.session.SessionManagementFilter;
import org.springframework.security.web.util.matcher.AnyRequestMatcher;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -112,7 +111,6 @@ public void filterChainProxyBuilderIgnoringResources() {
assertThat(classes.contains(RequestCacheAwareFilter.class)).isTrue();
assertThat(classes.contains(SecurityContextHolderAwareRequestFilter.class)).isTrue();
assertThat(classes.contains(AnonymousAuthenticationFilter.class)).isTrue();
assertThat(classes.contains(SessionManagementFilter.class)).isTrue();
assertThat(classes.contains(ExceptionTranslationFilter.class)).isTrue();
assertThat(classes.contains(FilterSecurityInterceptor.class)).isTrue();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,17 @@ private static SessionResultMatcher session() {
@EnableWebSecurity
static class SessionManagementConfig extends WebSecurityConfigurerAdapter {

@Override
protected void configure(HttpSecurity http) throws Exception {
// @formatter:off
super.configure(http);
http
.sessionManagement((sessions) -> sessions
.requireExplicitAuthenticationStrategy(false)
);
// @formatter:on
}

}

@Configuration
Expand Down Expand Up @@ -364,6 +375,7 @@ protected void configure(HttpSecurity http) throws Exception {
// @formatter:off
http
.sessionManagement()
.requireExplicitAuthenticationStrategy(false)
.and()
.httpBasic();
// @formatter:on
Expand All @@ -379,8 +391,9 @@ static class SFPPostProcessedConfig extends WebSecurityConfigurerAdapter {
protected void configure(HttpSecurity http) throws Exception {
// @formatter:off
http
.sessionManagement()
.and()
.sessionManagement((sessions) -> sessions
.requireExplicitAuthenticationStrategy(false)
)
.httpBasic();
// @formatter:on
}
Expand All @@ -400,9 +413,10 @@ static class SFPNewSessionSessionManagementConfig extends WebSecurityConfigurerA
protected void configure(HttpSecurity http) throws Exception {
// @formatter:off
http
.sessionManagement()
.sessionManagement((sessions) -> sessions
.sessionFixation().newSession()
.and()
.requireExplicitAuthenticationStrategy(false)
)
.httpBasic();
// @formatter:on
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,7 @@ protected void configure(HttpSecurity http) throws Exception {
http
.sessionManagement((sessionManagement) ->
sessionManagement
.requireExplicitAuthenticationStrategy(false)
.sessionFixation((sessionFixation) ->
sessionFixation.newSession()
)
Expand Down Expand Up @@ -583,9 +584,12 @@ static class SharedTrustResolverConfig extends WebSecurityConfigurerAdapter {
static AuthenticationTrustResolver TR;

@Override
protected void configure(HttpSecurity http) {
protected void configure(HttpSecurity http) throws Exception {
// @formatter:off
http
.sessionManagement((sessions) -> sessions
.requireExplicitAuthenticationStrategy(false)
)
.setSharedObject(AuthenticationTrustResolver.class, TR);
// @formatter:on
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ public void explicitDeferHttpSession() throws Exception {

this.springSecurityFilterChain.doFilter(mockRequest, response, chain);

verify(mockRequest, never()).isRequestedSessionIdValid();
verify(mockRequest, never()).changeSessionId();
verify(mockRequest, never()).getSession(anyBoolean());
verify(mockRequest, never()).getSession();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@
import org.springframework.security.web.savedrequest.RequestCacheAwareFilter;
import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter;
import org.springframework.security.web.session.DisableEncodeUrlFilter;
import org.springframework.security.web.session.SessionManagementFilter;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.MvcResult;
import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
Expand Down Expand Up @@ -479,8 +478,6 @@ public void getWhenAuthenticatingThenConsultsCustomSecurityContextRepository() t
.andReturn();
// @formatter:on
assertThat(result.getRequest().getSession(false)).isNotNull();
verify(repository, atLeastOnce()).saveContext(any(SecurityContext.class), any(HttpServletRequest.class),
any(HttpServletResponse.class));
}

@Test
Expand Down Expand Up @@ -851,7 +848,6 @@ private void assertThatFiltersMatchExpectedAutoConfigList(String url) {
assertThat(filters.next()).isInstanceOf(RequestCacheAwareFilter.class);
assertThat(filters.next()).isInstanceOf(SecurityContextHolderAwareRequestFilter.class);
assertThat(filters.next()).isInstanceOf(AnonymousAuthenticationFilter.class);
assertThat(filters.next()).isInstanceOf(SessionManagementFilter.class);
assertThat(filters.next()).isInstanceOf(ExceptionTranslationFilter.class);
assertThat(filters.next()).isInstanceOf(FilterSecurityInterceptor.class)
.hasFieldOrPropertyWithValue("observeOncePerRequest", false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ class SessionFixationDslTests {
open fun securityFilterChain(http: HttpSecurity): SecurityFilterChain {
http {
sessionManagement {
requireExplicitAuthenticationStrategy = false
sessionFixation {
newSession()
}
Expand Down Expand Up @@ -111,6 +112,7 @@ class SessionFixationDslTests {
open fun securityFilterChain(http: HttpSecurity): SecurityFilterChain {
http {
sessionManagement {
requireExplicitAuthenticationStrategy = false
sessionFixation {
migrateSession()
}
Expand Down Expand Up @@ -147,6 +149,7 @@ class SessionFixationDslTests {
open fun securityFilterChain(http: HttpSecurity): SecurityFilterChain {
http {
sessionManagement {
requireExplicitAuthenticationStrategy = false
sessionFixation {
changeSessionId()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
<http auto-config="true"
use-authorization-manager="true">
<intercept-url pattern="/**" access="permitAll"/>
<session-management authentication-strategy-explicit-invocation="true"/>
</http>

<b:import resource="CsrfConfigTests-shared-userservice.xml"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
https://www.springframework.org/schema/beans/spring-beans.xsd">

<http auto-config="true">
<session-management>
<session-management
authentication-strategy-explicit-invocation="false">
<concurrency-control/>
</session-management>
<remember-me services-ref="customRememberMeServices"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
https://www.springframework.org/schema/beans/spring-beans.xsd">

<http auto-config="true">
<session-management>
<session-management
authentication-strategy-explicit-invocation="false">
<concurrency-control session-registry-alias="sessionRegistry"/>
</session-management>
<csrf disabled="true"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
https://www.springframework.org/schema/beans/spring-beans.xsd">

<http auto-config="true">
<session-management>
<session-management
authentication-strategy-explicit-invocation="false">
<concurrency-control session-registry-ref="sessionRegistry"/>
</session-management>
<csrf disabled="true"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@

<http auto-config="true" create-session="ifRequired" use-expressions="true">
<intercept-url pattern="/auth/**" access="authenticated"/>
<session-management>
<session-management
authentication-strategy-explicit-invocation="false">
<concurrency-control max-sessions="1" error-if-maximum-exceeded="true"/>
</session-management>
<csrf disabled="true"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@

<http auto-config="true" use-expressions="true">
<intercept-url pattern="/auth/**" access="authenticated"/>
<session-management session-authentication-strategy-ref="teapotSessionAuthenticationStrategy"/>
<session-management
authentication-strategy-explicit-invocation="false"
session-authentication-strategy-ref="teapotSessionAuthenticationStrategy"/>
</http>

<b:bean name="basicController"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@

<http auto-config="true" use-expressions="true">
<intercept-url pattern="/auth/**" access="authenticated"/>
<session-management session-fixation-protection="migrateSession"/>
<session-management
authentication-strategy-explicit-invocation="false"
session-fixation-protection="migrateSession"/>
</http>

<b:bean name="basicController"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,13 @@
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.http.MediaType;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
import org.springframework.security.core.userdetails.User;
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.core.userdetails.UserDetailsService;
import org.springframework.security.provisioning.InMemoryUserDetailsManager;
import org.springframework.security.web.DefaultSecurityFilterChain;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.test.context.web.WebAppConfiguration;
Expand All @@ -38,6 +39,7 @@
import org.springframework.web.context.WebApplicationContext;
import org.springframework.web.servlet.config.annotation.EnableWebMvc;

import static org.springframework.security.config.Customizer.withDefaults;
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestBuilders.formLogin;
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.httpBasic;
import static org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers.authenticated;
Expand Down Expand Up @@ -89,11 +91,26 @@ public void authenticationFailed() throws Exception {
@Configuration
@EnableWebSecurity
@EnableWebMvc
static class Config extends WebSecurityConfigurerAdapter {
static class Config {

@Override
@Bean
public UserDetailsService userDetailsService() {
DefaultSecurityFilterChain springSecurity(HttpSecurity http) throws Exception {
// @formatter:off
http
.authorizeHttpRequests((requests) -> requests
.anyRequest().authenticated()
)
.sessionManagement((sessions) -> sessions
.requireExplicitAuthenticationStrategy(false)
)
.httpBasic(withDefaults())
.formLogin(withDefaults());
// @formatter:on
return http.build();
}

@Bean
UserDetailsService userDetailsService() {
// @formatter:off
UserDetails user = User.withDefaultPasswordEncoder().username("user").password("password").roles("USER").build();
return new InMemoryUserDetailsManager(user);
Expand Down
Loading

0 comments on commit 4479cef

Please sign in to comment.