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

Disabling logout keeps LogoutPageGeneratingWebFilter registered at /logout #9475

Closed
kkroner8451 opened this issue Feb 25, 2021 · 2 comments
Closed
Assignees
Labels
in: config An issue in spring-security-config status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone

Comments

@kkroner8451
Copy link

Describe the bug
When configuring SecurityWebFilterChain with http.logout().disable() the default LogoutPageGeneratingWebFilter is still created, registered and listening to GET calls to /logout. It seems there is no way to disable this or change the route matcher to anything other than GET on /logout.

To Reproduce
Register a SecurityWebFilterChain bean similar to:

@Bean
 public SecurityWebFilterChain springSecurityFilterChain(ServerHttpSecurity http){
   http
                .oauth2Login()
                .and()
                .logout().disable()
}

Navigate in browser to http://hostname/logout and the default logout page is shown asking if you want to logout.

Expected behavior
I would expect that if logout is disabled then the logout page generating filter would not be registered/listening at /logout. Alternatively, I would have expected that if changing the logout.logoutUrl("/someOtherLogout") then the page generating filter would have been changed to that location as well or even better that the matcher could be set explicitly on the LogoutPageGeneratingWebFilter in a similar way as the logout().requiresLogout(ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, "/customLogoutPath"));

Sample

A link to a GitHub repository with a minimal, reproducible sample.

Reports that include a sample will take priority over reports that do not.
At times, we may require a sample, so it is good to try and include a sample up front.

@kkroner8451 kkroner8451 added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Feb 25, 2021
@eleftherias eleftherias self-assigned this Feb 25, 2021
@rwinch
Copy link
Member

rwinch commented Feb 26, 2021

Thanks for the report. A few additional thoughts:

  • I think we should also ensure it is easy to disable the default logout page but yet still support log out.
  • I think anything that disables the default login page should probably disable the default log out page.

@eleftherias eleftherias added in: config An issue in spring-security-config and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 1, 2021
@eleftherias
Copy link
Contributor

I have pushed a fix in dfd0047.
This fix simply changes the default behaviour, to not create the logout page if logout is disabled.

I have created gh-9938 to capture the additional feature of disabling the default logout page while still supporting log out.

@eleftherias eleftherias added this to the 5.6.0-M1 milestone Jun 17, 2021
@spring-projects-issues spring-projects-issues added the status: backported An issue that has been backported to maintenance branches label Jun 17, 2021
akohli96 pushed a commit to akohli96/spring-security that referenced this issue Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: config An issue in spring-security-config status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants