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

Set NullRequestCache when session management disabled #13553

Closed

Conversation

dukcode
Copy link

@dukcode dukcode commented Jul 17, 2023

  @Bean
  public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {

    http
        .authorizeHttpRequests(request -> request.anyRequest().authenticated())
        .sessionManagement(s -> s.sessionCreationPolicy(SessionCreationPolicy.STATELESS));

    return http.build();
  }

When I configure sessionManagement to STATELESS like upper code, spring-security setsRequestCache of ExceptionTranslationFilter to NullRequestCache like next image.

image

  @Bean
  public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {

    http
        .authorizeHttpRequests(request -> request.anyRequest().authenticated())
        .sessionManagement(AbstractHttpConfigurer::disable);

    return http.build();
  }

But when I configure sessionManagement disabled, RequestCache of ExceptionTranslationFilter is HttpSessionRequestCache. Please see image and output from Spring Security debug mode.

image

image

I think that RequestCache should be NullRequestCache.

@pivotal-cla
Copy link

@dukcode Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 17, 2023
@pivotal-cla
Copy link

@dukcode Thank you for signing the Contributor License Agreement!

@marcusdacoregio
Copy link
Contributor

Hi @dukcode, have you discussed this with the team before submitting the PR? Asking because I do not think that we want to do this since sessionManagement() is really only about how the session is handled around authentication. If you disable it, it won't make any configuration, and, therefore, will rely on the other defaults, like the requestCache().

@marcusdacoregio marcusdacoregio self-assigned this Jul 19, 2023
@marcusdacoregio marcusdacoregio added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 19, 2023
@dukcode
Copy link
Author

dukcode commented Jul 20, 2023

Hi @marcusdacoregio!

In ExceptionTranslationFilter, it calls requestCache.saveRequest(request, response)

When we set sessionManagement() to disable, in ExceptionTranslationFilter http session is created because RequestCache is set to HttpSessionRequestCache.

However, when we set sessionManagement() to STATELESS, RequestCache is set to NullRequestCache. So, there is no session creation.

So I think it should work in same way in both config situation.

@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 Jul 20, 2023
@marcusdacoregio
Copy link
Contributor

marcusdacoregio commented Jul 20, 2023

When we set sessionManagement() to disable, in ExceptionTranslationFilter http session is created because RequestCache is set to HttpSessionRequestCache.

This is expected based on what I mentioned before, you are disabling session management but are keeping the defaults of the other configurations. If you do not want to have sessions at all, just set it to STATELESS, otherwise, if you are disabling it, you must also configure the RequestCache to use NullRequestCache.

@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 Jul 20, 2023
@dc-oe
Copy link

dc-oe commented Sep 13, 2023

We are seeing the requestcache draining our form data when looking for continue. It looks similar to what issue 13731 is seeing. Our current solution is to disable the optimization and get it back to 5.8 behavior. You can't drain the input stream looking for something that isn't there. We would like to know if we can fix this through configuration. We use XML and not annotations.

From the Jakarta Javadoc:
If the parameter data was sent in the request body, such as occurs with an HTTP POST request, then reading the body directly via getInputStream() or getReader() can interfere with the execution of this method.

@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 Sep 13, 2023
@marcusdacoregio
Copy link
Contributor

I will close this with what was mentioned in this comment.

@dc-oe it sounds like the bug is not related to this PR, can you add your comments to #13731 if they are related or open a new issue?

@marcusdacoregio marcusdacoregio added status: declined A suggestion or change that we don't feel we should currently apply and removed status: feedback-provided Feedback has been provided labels Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants