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

Clarify that spring.security.filter properties only apply to servlet-based web apps #33551

Closed
tgeens opened this issue Dec 20, 2022 · 4 comments
Labels
type: documentation A documentation update
Milestone

Comments

@tgeens
Copy link

tgeens commented Dec 20, 2022

Should it be documented (or fixed) that spring.security.filter.order is ignored in the reactive stack ?

Context: I'm trying to convert an auto-configuration-enabled library to the reactive-stack. It uses a filter that should be registered before the spring-security-filter. The servlet variant is using SecurityProperties.getFilter().getOrder() as a best-effort attempt to find out the order of the spring-security-filter.

WebFluxSecurityConfiguration uses a fixed @Order(WEB_FILTER_CHAIN_FILTER_ORDER) - but the WEB_FILTER_CHAIN_FILTER_ORDER is a magic number, with only package visiblity.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 20, 2022
@philwebb philwebb changed the title spring.security.filter.order is ignored in the reactive stack spring.security.filter.order is ignored in the reactive stack Dec 20, 2022
@philwebb philwebb added the for: team-attention An issue we'd like other members of the team to review label Dec 20, 2022
@wilkinsona
Copy link
Member

This feels like an oversight to me. I think we should explore applying the property to the reactive stack.

@wilkinsona
Copy link
Member

It occurs to me that applying spring.security.filter.order to the reactive stack will create confusion with the spring.security.filter.dispatcher-types property which is Servlet-specific. If we want the order property to apply to be web stacks, we may need to rename spring.security.filter.dispatcher-types to make it clear that it's Servlet-specific.

@philwebb philwebb added type: bug A general bug and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Jan 11, 2023
@philwebb philwebb added this to the 2.7.x milestone Jan 11, 2023
@wilkinsona
Copy link
Member

As things stand, I don't think we can implement this. As @tgeens has noted above, WebFluxSecurityConfiguration hardcodes the order of the filter on the @Bean method. The order specified on a bean definition's factory method (the @Bean method in this case) is the primary source of its order. This method exists on a package-private class that is part of Spring Security so there's no much we can do about it.

Without changes to Spring Security, I think the best that we can do here is to make it clearer that the spring.security.filter properties only apply to Servlet-based apps. We could do that just be updating their descriptions. We could also consider deprecating the current properties and adding servlet-specific replacements, for example by moving them to spring.security.servlet.filter. Flagging for team discussion so that we can decide what to do.

@tgeens In your situation, I believe you can safely assume that the security filter in a reactive app will have the hardcoded order -100 as there's no way to change it at the moment.

@wilkinsona wilkinsona added the for: team-meeting An issue we'd like to discuss as a team to make progress label Jan 18, 2023
@philwebb philwebb added type: documentation A documentation update and removed type: bug A general bug for: team-meeting An issue we'd like to discuss as a team to make progress labels Jan 18, 2023
@bclozel
Copy link
Member

bclozel commented Jan 18, 2023

There's a section in the reference documentation that we can revisit as part of this issue if we want to do so.

@wilkinsona wilkinsona changed the title spring.security.filter.order is ignored in the reactive stack Clarify that spring.security.filter properties only apply to servlet-based web apps Jan 20, 2023
@wilkinsona wilkinsona modified the milestones: 2.7.x, 2.7.15 Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation A documentation update
Projects
None yet
Development

No branches or pull requests

5 participants