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

Looking for guidance on implementing security in WebFlux/reactive environment #5328

Closed
edeandrea opened this issue May 10, 2018 · 10 comments
Closed

Comments

@edeandrea
Copy link
Contributor

edeandrea commented May 10, 2018

I'm looking for a little guidance on how to implement some things within a WebFlux/reactive environment.

More specifically I'm looking for what the best approach is for building reactive equivalents of the RequestHeaderAuthenticationFilter and the AbstractAuthenticationProcessingFilter.

In our organization we offer our application developers (Spring Boot RESTful services) the way to configure their applications to be either protected by siteminder (RequestHeaderAuthenticationFilter) or by JWT tokens passed as Authorization Bearer headers (custom implementation of AbstractAuthenticationProcessingFilter).

When looking at the reactive filters currently available I see the AuthenticationWebFilter which has a ServerHttpBasicAuthenticationConverter as the authenticationConverter as well as HttpBasicServerAuthenticationEntryPoint set within the ServerAuthenticationFailureHandler.

Am I safe to assume that the correct thing to do would be to build custom authenticationConverters for my 2 use cases? I've already added #5325 for allowing an HttpStatusServerEntryPoint, which is what I would set within the ServerAuthenticationFailureHandler.

Or is it a better approach to build a custom WebFilter?

It just seems there's lots of "stuff" in RequestHeaderAuthenticationFilter (and it's AbstractPreAuthenticatedProcessingFilter parent class) as well as in AbstractAuthenticationProcessingFilter that I'd have to replicate. Plus given #4961 the whole event notification things don't work.

@zjengjie
Copy link

I also find it hard to get request detail(like remote IP, header, cookie etc) in the check method, I consider we can set what we want in Authentication.details in AuthenticationConverter, but I find it is unconfigurable.

@rwinch
Copy link
Member

rwinch commented May 11, 2018

@edeandrea You are correct that this is the correct approach. WebFlux is still fairly new so there are certainly pieces that need built out. Can you elaborate on the "stuff" you need? I'd love to see links to some new tickets on additional things you need

@rwinch
Copy link
Member

rwinch commented May 11, 2018

@zjengjie Can you turn your concern into a specific ticket?

@edeandrea
Copy link
Contributor Author

Thanks @rwinch.

By "stuff" I'm talking more about the structure/hierarchy of the existing servlet-based filters: AbstractPreAuthenticatedProcessingFilter -> RequestHeaderAuthenticationFilter (& other siblings - the RequestHeaderAuthenticationFilter is the only one I really care about within my organization) as well as AbstractAuthenticationProcessingFilter -> its direct descendants (the one I care about is a custom implementation that I have).

Do you see foresee Spring Security building base AuthenticationConverter classes that match the existing servlet-based hierarchy? Or is the plan to just leave it as Function<ServerWebExchange, Mono<Authentication>> and let consumers do what they'd like?

In my organization I am going to build some of this stuff that I need and if I have insight into what the longer-term plan is for the reactive-side within Spring Security I can certainly align to it and potentially even contribute it back, as I have been doing with other building blocks.

@rwinch
Copy link
Member

rwinch commented May 11, 2018

Do you see foresee Spring Security building base AuthenticationConverter classes that match the existing servlet-based hierarchy? Or is the plan to just leave it as Function<ServerWebExchange, Mono> and let consumers do what they'd like?

@edeandrea Thanks for the reply. The reactive (WebFlux) support in Spring Security is quite similar to the imperative (servlet) support, but we have tried to make some changes where it made sense. One of these is to favor composition to inheritance. This means we do not have plans for a hiearchy of Filter's but instead will try to provide the Function<ServerWebExchange, Mono<Authentication>> implementations that users need. If you have ideas on implementations you would like to see, please don't hesitate to create a ticket or a pull request.

In my organization I am going to build some of this stuff that I need and if I have insight into what the longer-term plan is for the reactive-side within Spring Security I can certainly align to it and potentially even contribute it back, as I have been doing with other building blocks.

That sounds great! We'd love contributions. Again, please don't hesitate to reach out if you need to.

@edeandrea
Copy link
Contributor Author

edeandrea commented May 11, 2018

Thanks @rwinch for the guidance.

FWIW - One thing I would have loved to have seen was a dedicated interface for an authentication converter and have all the implementations extend/implement that, rather than a generic Function<ServerWebExchange, Mono<Authentication>>, even if that dedicated interface was itself an @FunctionalInterface that just extended Function<ServerWebExchange, Mono<Authentication>>, like

@FunctionalInterface
public interface AuthenticationConverter extends Function<ServerWebExchange, Mono<Authentication>> {

}

Mainly the reason is as a consumer of the framework its really easy in the IDE to open the hierarchy of an interface to see which implementations are available within the framework. The Javadocs would also reflect this hierarchy.

Doing that for a plain Java Function is much harder, plus the IDE will pick up other Function implementations which could be completely unrelated. Looking through the source tree now its difficult to find what the existing implementations actually are outside of searching for the String AuthenticationConverter in the class name.

@rwinch
Copy link
Member

rwinch commented May 11, 2018

@edeandrea I have actually considered that myself. Would you mind creating a ticket for that?

@edeandrea
Copy link
Contributor Author

edeandrea commented May 11, 2018

I certainly can. I would also think that changing

public void setAuthenticationConverter(Function<ServerWebExchange, Mono<Authentication>> authenticationConverter)

to

public void setAuthenticationConverter(AuthenticationConverter authenticationConverter)

as well as retro-fitting ServerFormLoginAuthenticationConverter, ServerHttpBasicAuthenticationConverter, and ServerOAuth2LoginAuthenticationTokenConverter would be a backwards-compatible change. Would it not be?

@rwinch
Copy link
Member

rwinch commented May 11, 2018

It would. However, we can either leave the method as is (it extends Function), add an overloaded method (deprecating the old one), or even add a different setter method like setConverter(AuthenticationConverter) and deprecate the old one.

@edeandrea
Copy link
Contributor Author

Maybe leaving the old one as-is & deprecating it, then adding an overloaded one is better, as well as retrofitting ServerFormLoginAuthenticationConverter, ServerHttpBasicAuthenticationConverter, and ServerOAuth2LoginAuthenticationTokenConverter to implement AuthenticationConverter.

I've added #5338 for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants