Skip to content

AuthenticationWebFilter's ReactiveAuthenticationManagerResolver should take a ServerWebExchange #7872

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

Closed
jzheaux opened this issue Jan 29, 2020 · 1 comment
Assignees
Labels
in: web An issue in web modules (web, webmvc) type: breaks-passivity A change that breaks passivity with the previous release type: enhancement A general enhancement
Milestone

Comments

@jzheaux
Copy link
Contributor

jzheaux commented Jan 29, 2020

AuthenticationWebFilter uses the exchange object throughout its implementation. It uses it for its ServerWebExchangeMatcher, ServerAuthenticationConverter, ServerSecurityContextRepository and all its other HTTP-based collaborators.

It would be cleaner for AuthenticationWebFilter to take a ReactiveAuthenticationManagerResolver<ServerWebExchange> instead of a ReactiveAuthenticationManagerResolver<ServerHttpRequest> to align with the rest of the API.

One way to achieve this might be to add an interface like:

public interface ServerWebExchangeReactiveAuthenticationManagerResolver
		extends ReactiveAuthenticationManagerResolver<ServerWebExchange> {
}

And then add a constructor:

public AuthenticationWebFilter
        (ServerWebExchangeReactiveAuthenticationManagerResolver resolver)  {
    // ...
}

The downside here is that we'd have an interface that we would not otherwise have introduced.

Or, since this is a very new feature, it might be best to simply change the constructor parameter generic type to alleviate confusion. That is, change:

public AuthenticationWebFilter
        (ReactiveAuthenticationManagerResolver<ServerHttpRequest> resolver)  {
    // ...
}

to

public AuthenticationWebFilter
        (ReactiveAuthenticationManagerResolver<ServerWebExchange> resolver)  {
    // ...
}

And then document the change in the 5.3 release notes.

@jzheaux jzheaux added in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement labels Jan 29, 2020
@jzheaux jzheaux added this to the 5.3.0.RC1 milestone Jan 29, 2020
@jzheaux jzheaux self-assigned this Jan 29, 2020
@jzheaux
Copy link
Contributor Author

jzheaux commented Jan 29, 2020

After chatting a bit with @rwinch about this, I think we should do the second option. It is a new enough feature that doing so will cause the least confusion in the long term.

@jzheaux jzheaux added the type: breaks-passivity A change that breaks passivity with the previous release label Jan 31, 2020
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: breaks-passivity A change that breaks passivity with the previous release type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

1 participant