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

Switching users creates user sessions #13435

Open
filiphr opened this issue Jun 28, 2023 · 4 comments
Open

Switching users creates user sessions #13435

filiphr opened this issue Jun 28, 2023 · 4 comments
Labels
in: web An issue in web modules (web, webmvc) status: feedback-provided Feedback has been provided type: bug A general bug

Comments

@filiphr
Copy link
Contributor

filiphr commented Jun 28, 2023

When upgrading to Spring Boot 2.7.11 and later we started having sessions even though we have explicitly configured the use of the SecurityContextRepository to be the NullSecurityContextRepository. We traced this to #12834.

From my analysis I can see that for #12504 a SecurityContextRepository was added to the SwitchUserFilter with the default being RequestAttributeSecurityContextRepository. Although this is different than the other filters than use the NullSecurityContextRepository as a default, it is still fine since it is not creating sessions. The other filters are:

  • AbstractAuthenticationProcessingFilter
  • AuthenticationFilter
  • AbstractPreAuthenticatedProcessingFilter
  • RememberMeAuthenticationFilter
  • BasicAuthenticationFilter
  • DigestAuthenticationFilter

In my opinion the change done in #12834 is backwards incompatible change since all of a sudden sessions get created. I think that the default should be the same as the other filters i.e. use NullSecurityContextRepository and if a user wants to use something else they can set the SecurityContextRepository that they want to use.

e.g. We have our own SwitchUserConfigurer and we use:

http.apply(new SwitchUserConfigurer<>())

to apply the switch user. This means that people can get the configured security context repository and set it in their own configurer.

@filiphr filiphr added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Jun 28, 2023
@marcusdacoregio marcusdacoregio self-assigned this Jun 28, 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 Jun 28, 2023
@marcusdacoregio
Copy link
Contributor

marcusdacoregio commented Jun 28, 2023

Hi @filiphr, thanks for the report.

Although this is different than the other filters than use the NullSecurityContextRepository as a default

The filters that you mentioned are stateless filters or filters meant to be extended, that is one reason that they cannot assume you are using sessions.

I think that the default should be the same as the other filters i.e. use NullSecurityContextRepository and if a user wants to use something else they can set the SecurityContextRepository that they want to use.

I don't think I agree here because switching users typically implies a user is logged in and has an authenticated session that needs to change across a period of requests. In a stateless arrangement, there is no need to "switch the user", one simply provides different creds on each request.

It is not intuitive if I need to set the SecurityContextRepository to HttpSessionSecurityContextRepository if the filter is meant to be used with sessions, this is also aligned with the reactive implementation of SwitchUserWebFilter.

If you have your own SwitchUserConfigurer you can either get the SecurityContextConfigurer and use the repository configured there or check if there is a SecurityContextRepository bean.

What do you think?

@marcusdacoregio marcusdacoregio added the status: waiting-for-feedback We need additional information before we can continue label Jun 28, 2023
@filiphr
Copy link
Contributor Author

filiphr commented Jun 29, 2023

The filters that you mentioned are stateless filters or filters meant to be extended, that is one reason that they cannot assume you are using sessions.

What is the difference between the BasicAuthenticationFilter and the SwitchUserFilter? Both are doing some kind of authentication. In my opinion the SwitcherUserFilter should be stateless as well, it is the responsibility of the user of the filter to configure it in the way that they need to.

I don't think I agree here because switching users typically implies a user is logged in and has an authenticated session that needs to change across a period of requests. In a stateless arrangement, there is no need to "switch the user", one simply provides different creds on each request.

We can agree to disagree on this one. Switching users is usually done for impersonating someone, which means that you do not have the credentials for the user you are trying to impersonate, but you have the permissions. There are additional audit functionalities which you get using the switching user / impersonate filter.

In addition to all of that a user being logged in doesn't mean that there should be an authenticated session. There are other mechanisms that you can provide a consistent logging without using sessions. e.g. you can use a JWT token passed by the FE for each request or use some other form of token authentication.

It is not intuitive if I need to set the SecurityContextRepository to HttpSessionSecurityContextRepository if the filter is meant to be used with sessions, this is also aligned with the reactive implementation of SwitchUserWebFilter.

Not sure why you are saying that the SwitchUserFilter is meant to be used with sessions, from what I can see prior to 5.7.7 (when the SecurityContextRepository was added) the filter was not using it and was not using any sessions. The introduction of sessions in 5.7.8 is a breaking change for all the users of the SwitchUserFilter. I don't use the reactive implementation and I am not sure why the reactive implementation was not implemented to match the implementation of the SwitchUserFilter, but that is an independent topic. You can also argue that the reactive implementation is the one that should be aligned with the older servlet implement of the SwitchUserFilter

If you have your own SwitchUserConfigurer you can either get the SecurityContextConfigurer and use the repository configured there or check if there is a SecurityContextRepository bean.

Yes I can, but I do not expect a behaviour change from a patch release in Spring Security, especially not one as crucial as a session creation. I can argue the same thing about the reverse, if you want sessions then go ahead and make sure that the SwitchUserFilter is using sessions. However, there is no need to force that on everyone.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jun 29, 2023
@marcusdacoregio
Copy link
Contributor

I am not understanding in what scenario you would need SwitchUserFilter to be stateless. Before 5.7, the filter didn't have the SecurityContextRepository because the SecurityContextPersistenceFilter was taking care of persisting the context in the session. If you configured it to use NullSecurityContextRepository, switching users won't work at all, or am I missing something?

Switching users is usually done for impersonating someone, which means that you do not have the credentials for the user you are trying to impersonate, but you have the permissions.

If the SwitchUserFilter is stateless, how can the changed security context be remembered for subsequent requests?

@marcusdacoregio marcusdacoregio added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jun 29, 2023
@filiphr
Copy link
Contributor Author

filiphr commented Jun 30, 2023

If you configured it to use NullSecurityContextRepository, switching users won't work at all, or am I missing something?

It does work, we have our own mechanism. We provide our own AuthenticationFailureHandler, AuthenticationSuccessHandler and SwitchUserAuthorityChanger to the SwitchUserFilter. In addition to that we have our own RunAsManager that knows how to handle a stateless switching of users. We use a cookie to persist the fact that a user needs to run in the context of another user. We configure the FilterSecurityInterceptor with this RunAsManager

The security context is created appropriately every time.

I am not sure how it worked prior to 5.7. What I know is that prior to 5.7.7 the SwitchUserFilter was not creating sessions, since 5.7.8 the SwitchUserFilter started creating sessions due to the use of theHttpSessionSecurityContextRepository.

In my opinion the SwitchUserFilter should be the same as the rest of the authentication filters and use the NullSecurityContextRepository. It is the responsibility of the user configuring the SwitchUserFilter to configure it in the way that they want. If you want to make sure that things work properly I would suggest that you provide a SwitchUserConfigurer to the HttpSecurity and people can then use that to configure it, this way you can get the configured SecurityContextRepository and provide it to the SwitchUserFilter.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jun 30, 2023
@marcusdacoregio marcusdacoregio removed their assignment Sep 17, 2024
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) status: feedback-provided Feedback has been provided type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants