Add Generic AuthenticationFilter

[sbespalov]

The proposal is to have generic Authentication filter which will be able to provide single authentication strategy regardless of authentication type.

The Servlet API based authentication process commonly consist of following stages:

1. Expose Authentication from request (supply);
2. If there is no supported Authentication provided then proceed filter chain;
3. If there is supported Authentication then try to authenticate it with AuthenticationManager;
4. If the authentication succeed then proceed filter chain with "authenticated" Authentication;
5. If the authentication failed then provide appropriate response with AuthenticationEntryPoint;

The first stage, from above, can be done with AuthenticationSupplier interface:

public interface AuthenticationSupplier<T extends Authentication> extends AuthenticationEntryPoint {
	
	T supply(HttpServletRequest request) throws AuthenticationException;

	AuthenticationType getAuthenticationType();

}

This issue targets to clarify proper implementation previously suggested within PR #6496.

[sbespalov]

@rwinch let me clarify why AuthenticationSupplier was extended from AuthenticationEntryPoint

but let's not combine AuthenticationSupplier and AuthenticationEntryPoint

the reason is that in case of failed authentication, which was supplied, we should commence same authentication scheme, that was supplied. So probably there is not much sense to separate this interfaces and it might be logical to have one interface for this.

[rwinch]

Thank you for getting the conversation going. I can see value in a generic Filter. However, I think that we need to discuss this with the team as it is a big change for users (it would deprecate a lot of existing Filters). I do like the idea in principal, and it aligns with how the reactive part works.

I do think we need to separate the entry point from the supplier. The entry point is typically used to indicate that no authentication/credentials are present so prompt for some credentials so adding a supplier to the entry point seems counter intuitive to me.

[sbespalov]

@rwinch thanks, will wait for your response

[sbespalov]

I do think we need to separate the entry point from the supplier. The entry point is typically used to indicate that no authentication/credentials are present so prompt for some credentials so adding a supplier to the entry point seems counter intuitive to me.

In this case ExceptionTranslationFilter will use regular AuthenticationEntryPoint (BasicAuthenticationEntryPoint or any other) if there was AccessDeniedException.

I mean the case when we receive invalid credentials, provided with specific AuthenticationSupplier, then we should prompt for another credentials with same authentication type that was supplied.

so the point is that if we are able to supply authentication from request, then we also should be able to prompt appropriate authentication in response.

Maybe AuthenticationSupplier name confusing here? If so then it can be renamed to something like WebAuthenticationSupport.

[rwinch]

That is a separate API too. In that case you use an AuthenticationFailureHandler. That handler should not be tied to how the authentication is obtained from the request.

[sbespalov]

yeah, but there was no AuthenticationFailureHandler interface changes proposed, this handler is just used to call appropriate AuthenticationEntryPoint.

I can try to think how to split AuthenticationSupplier from AuthenticationEntryPoint, but it's also important to know would you accept AuthenticationType and AuthenticationSupplierRegistry?

[sbespalov]

@rwinch have you bean able to decide something on AuthenticationSupplier feature in general?

would you accept AuthenticationType and AuthenticationSupplierRegistry?

this also needs your response.

[sbespalov]

@rwinch can you please provide a feedback, so I can proceed with this task?

[carlspring]

@rwinch ,

We're quite interested in contributing this work to your project and we believe that many people will benefit from @sbespalov 's changes. Could you please give us an update?

Many thanks in advance!

Martin

[carlspring]

@rwinch ,

In addition, we're already using this in our project (Strongbox). It was introduced via strongbox/strongbox#935.

We thought we it would be a great idea to improve the Spring Security project with these fixes, which is why @sbespalov extracted this implementation in a pull request that we would like to contribute to you guys.

We understand that these changes could cause deprecations of existing code and that if accepted, they may have to be introduced in a future major release. However, we believe this would improve the way things are currently done in Spring Security and would like to continue this conversation and understand:

• What your concerns are
• How to address them
• How to get them included in an upcoming release

It would be great to hear your thoughts!

Kind regards,

Martin

[steve-todorov]

@rwinch ping, are there any updates on this? :)

[rwinch]

Thanks for the ping everyone.

I'm ok with this happening. However, we need to first decide on a design. I'd take a look at how the reactive bits work and model after that.

[carlspring]

@rwinch :

Would you mind elaborating on what you mean by that? Are you looking for some diagrams and flow charts, or what? Or, do you need to go over the code again?