Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SwitchUserFilter not working in Spring Security 6 #12504

Closed
RobertBleyl opened this issue Jan 7, 2023 · 16 comments
Closed

SwitchUserFilter not working in Spring Security 6 #12504

RobertBleyl opened this issue Jan 7, 2023 · 16 comments
Assignees
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug
Milestone

Comments

@RobertBleyl
Copy link

Describe the bug
When using Spring Security 6 (via the Spring Boot 3 BOM) the SwitchUserFilter is not working anymore. The currently logged in user is redirected to the SwitchUserUrl (that is configured in the SwitchUserFilter), but the user is not switched.

The attached log file shows the following line:
"Failed to find original user"

To Reproduce

  • Have a Spring Boot 3 project with Spring Security
  • Define a SwitchUserFilter bean in a configuration class:
	@Bean
	public SwitchUserFilter switchUserFilter() {
		SwitchUserFilter filter = new SwitchUserFilter();
		filter.setUserDetailsService(userDetailsService);
		filter.setUsernameParameter("username");
		filter.setSwitchUserUrl("/admin/switch_user");
		filter.setExitUserUrl("/admin/switch_user_exit");
		filter.setTargetUrl("/");
		return filter;
	}
  • Use this bean in a SecurityFilterChain:
.addFilterAfter(switchUserFilter(), AuthorizationFilter.class)
  • Login as an admin user and try to switch to a different user

Expected behavior
The user performing the switch should be logged in as the selected user.

Sample
While I don't have a minimal example, I have an open source project that reproduces the issue. The relevant config is here:
https://gitlab.com/skrupeltng/skrupel-tng/-/blob/issue-531_spring_boot_3/src/main/java/org/skrupeltng/config/SecurityConfig.java

The javadoc of the SwitchUserFilter still states:
"Note that the filter must come after the FilterSecurityInteceptor in the chain"
However, FilterSecurityIntercepter is deprecated. The deprecation text says one should use AuthorizationFilter, so I used this.
Using the AuthorizationFilter was in fact working when using Spring Boot 2.7 and Spring Security 5.8.
Maybe we have to put the SwitchUserFilter before/after a different Filter now?

switch_user.log

@RobertBleyl RobertBleyl added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Jan 7, 2023
@mdeinum
Copy link
Contributor

mdeinum commented Jan 9, 2023

Not sure if it is related but when defining a filter in Spring Boot as an @Bean it will be picked up by Spring Boot and automatically registered in the filter-chain. When using Spring Security this will lead to the filter being accessed twice. When defining a Filter as an @Bean for Spring Security make sure you are also adding an FilterRegistrationBean for said filter and mark it is setEnabled(false); to prevent the registration of this filter in the regular filter chain.

@marcusdacoregio marcusdacoregio self-assigned this Jan 9, 2023
@marcusdacoregio marcusdacoregio added in: web An issue in web modules (web, webmvc) and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 9, 2023
@marcusdacoregio
Copy link
Contributor

marcusdacoregio commented Jan 9, 2023

Hi @RobertBleyl, thanks for the report.

I think that the SwitchUserFilter is not explicitly saving the security context in the SecurityContextRepository.

Can you add the following configuration and see if it solves the problem? If so, that confirms my suspicion.

public SecurityFilterChain filterChain(HttpSecurity http) {
	http
		// ...
		.securityContext((securityContext) -> securityContext
			.requireExplicitSave(false)
		);
	return http.build();
}

@RobertBleyl
Copy link
Author

RobertBleyl commented Jan 9, 2023

Not sure if it is related but when defining a filter in Spring Boot as an @Bean it will be picked up by Spring Boot and automatically registered in the filter-chain. When using Spring Security this will lead to the filter being accessed twice. When defining a Filter as an @Bean for Spring Security make sure you are also adding an FilterRegistrationBean for said filter and mark it is setEnabled(false); to prevent the registration of this filter in the regular filter chain.

Thank you for your reply!
I am trying to get this to work using the FilterRegistrationBean but I am unsure about the proper use. This is what I am trying:

@Bean
public FilterRegistrationBean<SwitchUserFilter> switchUserFilterRegistration() {
  FilterRegistrationBean<SwitchUserFilter> registration = new FilterRegistrationBean<>();
  registration.setFilter(switchUserFilter());
  registration.addUrlPatterns("/admin/switch_user");
  registration.setName("switchUserFilter");
  registration.setOrder(1);
  return registration;
}

public SwitchUserFilter switchUserFilter() {
  SwitchUserFilter filter = new SwitchUserFilter();
  filter.setUserDetailsService(userDetailsService);
  filter.setUsernameParameter("username");
  filter.setSwitchUserUrl("/admin/switch_user");
  filter.setExitUserUrl("/admin/switch_user_exit");
  filter.setTargetUrl("/");
  return filter;
}

But when trying to perform a user switch an exception is thrown:

java.lang.NullPointerException: Cannot invoke "org.springframework.security.web.authentication.AuthenticationSuccessHandler.onAuthenticationSuccess(jakarta.servlet.http.HttpServletRequest, jakarta.servlet.http.HttpServletResponse, org.springframework.security.core.Authentication)" because "this.successHandler" is null
	at org.springframework.security.web.authentication.switchuser.SwitchUserFilter.doFilter(SwitchUserFilter.java:187) ~[spring-security-web-6.0.1.jar:6.0.1]

I am either doing it wrong or the SwitchUserFilter is not meant to be used with a FilterRegistrationBean? :)

@RobertBleyl
Copy link
Author

Hi @RobertBleyl, thanks for the report.

I think that the SwitchUserFilter is not explicitly saving the security context in the SecurityContextRepository.

Can you add the following configuration and see if it solves the problem? If so, that confirms my suspicion.

public SecurityFilterChain filterChain(HttpSecurity http) {
	http
		// ...
		.securityContext((securityContext) -> securityContext
			.requireExplicitSave(false)
		);
	return http.build();
}

Thank you for your response! I tried it and it unfortunately did not fix the problem. For transparency, this is what the filter chain looked like:

@Bean
public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
  http.authorizeHttpRequests()
	  .requestMatchers("/", "/error", "/favicon.ico", "/sitemap.xml", "/logo.png", "/dist/**", "/images/**", "/factions/**",
		  "/webfonts/**", "/login", "/credits", "/init-setup", "/register", "/register-successfull", "/legal", "/data-privacy",
		  "/activate-account/**", "/activation-failure/**", "/reset-password/**", "/infos/**", "/guest-account/**", "/admin/switch_user_exit")
	  .permitAll()
	  .requestMatchers("/admin/**", "/debug/**").access(new WebExpressionAuthorizationManager("hasRole('%s')".formatted(Roles.ADMIN)))
	  .requestMatchers("/**").access(new WebExpressionAuthorizationManager("not( hasRole('%s') ) and hasAnyRole('%s', '%s')".formatted(Roles.AI, Roles.ADMIN, Roles.PLAYER)))
	  .and()
	  .csrf()
	  .csrfTokenRepository(CookieCsrfTokenRepository.withHttpOnlyFalse())
	  .and()
	  .securityContext(c -> c.requireExplicitSave(true))
	  .addFilterAfter(switchUserFilter(), AuthorizationFilter.class)
	  .formLogin()
	  .loginPage("/login").failureHandler(handleAuthenticationFailure()).defaultSuccessUrl("/my-games")
	  .and().rememberMe().key("skrupel-tng-remember-me")
	  .and().logout().logoutSuccessUrl("/").permitAll();
  
  return http.build();
}

@marcusdacoregio
Copy link
Contributor

@RobertBleyl, I think you should do requireExplicitSave(false) instead of requireExplicitSave(true), can you confirm that?

@mdeinum
Copy link
Contributor

mdeinum commented Jan 9, 2023

You need to disable it, not enable it!

@Bean
public FilterRegistrationBean<SwitchUserFilter> switchUserFilterRegistration(SwitchUserFilter switchUserFilter) {
  FilterRegistrationBean<SwitchUserFilter> registration = new FilterRegistrationBean<>(switchUserFilter);
  registration.setEnabled(false);
  return registration;
}

You need to disable it so it won't be added to the regular filter chain, if it would it would execute twice and also far too early in the process.

@RobertBleyl
Copy link
Author

RobertBleyl commented Jan 9, 2023

@RobertBleyl, I think you should do requireExplicitSave(false) instead of requireExplicitSave(true), can you confirm that?

Oh wow, my bad! Using false does indeed work! Thank you!
I am not clear yet on whether or not this is intended behavior and using requireExplicitSave(false) is required now when using the SwitchUserFilter, or if this is considered a bug. In other words: should I close this issue now? :)

@RobertBleyl
Copy link
Author

You need to disable it, not enable it!

@Bean
public FilterRegistrationBean<SwitchUserFilter> switchUserFilterRegistration(SwitchUserFilter switchUserFilter) {
  FilterRegistrationBean<SwitchUserFilter> registration = new FilterRegistrationBean<>(switchUserFilter);
  registration.setEnabled(false);
  return registration;
}

You need to disable it so it won't be added to the regular filter chain, if it would it would execute twice and also far too early in the process.

It does appear that requireExplicitSave(false) solves the issue and the usage of FilterRegistrationBean is not required here.

@mdeinum
Copy link
Contributor

mdeinum commented Jan 9, 2023

Well you want to prevent this filter from being executed twice, so it is always a good idea to add it for filters that are meant only for the security filter chain.

@RobertBleyl
Copy link
Author

I just noticed that when trying to access a URL that is not the defaultSuccessUrl and you are not logged in, then you are redirected to the URL you requested before the login (as expected), but additionally now these log warnings appear:

2023-01-09T17:48:32.836+01:00  WARN 25933 --- [nio-8081-exec-9] w.c.HttpSessionSecurityContextRepository : Failed to create a session, as response has been committed. Unable to store SecurityContext.

I guess this has something to do with requireExplicitSave(false)?

I also added the FilterRegistrationBean, this does not change anything in my case apparently.

@marcusdacoregio
Copy link
Contributor

I am not clear yet on whether or not this is intended behavior and using requireExplicitSave(false) is required now when using the SwitchUserFilter, or if this is considered a bug. In other words: should I close this issue now? :)

I think it is a bug, I'll just confirm with the team first and proceed with the fix.

@RobertBleyl
Copy link
Author

I am still experiencing the issue with Spring Boot 3.0.3 (that comes with Spring Security 6.0.2 which appears to include the changes made for this issue).

Attached a log file with trace information:
switch_user.log

The same debug output appears as before:
Failed to find original user

The current version of my SecurityConfig can be found here.

I removed the previously added line
.securityContext(c -> c.requireExplicitSave(false))
again because this caused a lot of other issues related to the login process. It produced a lot of output like this:

w.c.HttpSessionSecurityContextRepository : Failed to create a session, as response has been committed. Unable to store SecurityContext.

Is this "requireExplicitSave" necessary now or was this just for debugging purposes?
Am I using the SwitchUserFilter wrong somehow or is there still a bug?

@RobertBleyl
Copy link
Author

I created a minimal test project demonstrating the issue: https://gitlab.com/robertbleyl/switch-user-test/-/tree/main
The readme contains the full description of how to set it up.
I hope this helps in either debugging or finding the issue in how I set it up :)

@imtiazShakil
Copy link

After upgrading spring boot from 2.7 to 3.0.4 I'm experiencing the same issue. After switching the user doesn't get changed.

My code is very similar to @RobertBleyl so not sharing it here. Can this issue be reopened?

Thanks.

@marcusdacoregio
Copy link
Contributor

marcusdacoregio commented Mar 7, 2023

Hello folks, this was an oversight and I'm working on the fix, I've opened #12834 for that.

As a workaround for now, you can configure the SwitchUserFilter to use another SecurityContextRepository, like so:
filter.setSecurityContextRepository(new DelegatingSecurityContextRepository(new HttpSessionSecurityContextRepository(), new RequestAttributeSecurityContextRepository()));

Can you try this workaround and see if it works?

@RobertBleyl
Copy link
Author

Thank you for your response! I just tried this workaround and it works!
I am now able to continue with the migration to Spring 3 and Spring Security 6 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants