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

Add BearerTokenAuthenticationConverter #14750

Closed
CrazyParanoid opened this issue Mar 14, 2024 · 9 comments · May be fixed by #14791
Closed

Add BearerTokenAuthenticationConverter #14750

CrazyParanoid opened this issue Mar 14, 2024 · 9 comments · May be fixed by #14791
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement

Comments

@CrazyParanoid
Copy link

Need to add BearerTokenAuthenticationConverter that implements AuthenticationConverter. Perhaps, it is worth extract some of the functionality from ServerBearerTokenAuthenticationConverter into AbstractBearerTokenAuthenticationConverter for example. This will be a template method pattern.

@CrazyParanoid CrazyParanoid added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Mar 14, 2024
CrazyParanoid pushed a commit to CrazyParanoid/spring-security that referenced this issue Mar 22, 2024
@sjohnr sjohnr self-assigned this Mar 28, 2024
@sjohnr sjohnr added the in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) label Mar 28, 2024
@sjohnr
Copy link
Member

sjohnr commented Mar 28, 2024

Hi @CrazyParanoid! Thanks for reaching out. Can you please provide a bit more information about what this issue relates to? For example, are you attempting to use AuthenticationFilter configured to support bearer tokens? If so, is there a reason you are unable to use the existing BearerTokenAuthenticationFilter? Can you please describe your use case?

@sjohnr sjohnr added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 28, 2024
@CrazyParanoid
Copy link
Author

CrazyParanoid commented Mar 29, 2024

Hi @sjohnr ! Thanks for your feedback. Support for multiple authentication schemes in spring security is very important to me. It seems to me now that it is not entirely complete. I started several tasks to add support for delegating components. For example, #14644 a DelegatingBearerTokenResolver would solve my problem, but if the AuthenticationFilter is in the configurer, then it would be nice to have a set of delegating components. In that case, I'm interested in the appearance of delegating components (DelegatingServerLogoutSuccessHandler, DelegatingServerAuthenticationConverter, RequestMatcherDelegatingAuthenticationSuccessHandler), as this will help solve issues with several authentication schemes.
Recently, some of these components have been implemented (#14655 #14654 ), but this is still not enough.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Mar 29, 2024
@sjohnr
Copy link
Member

sjohnr commented Mar 29, 2024

Greetings @CrazyParanoid! Thanks for providing clarification. However, I still have questions about your use case.

Support for multiple authentication schemes in spring security is very important to me.

Are you trying to support multiple authentication schemes through a single AuthenticationFilter? Otherwise, I'm not clear on how delegating implementations will help in your particular case.

I'm interested in the appearance of delegating components (DelegatingServerLogoutSuccessHandler, DelegatingServerAuthenticationConverter, RequestMatcherDelegatingAuthenticationSuccessHandler), as this will help solve issues with several authentication schemes.

Adding some of these components may be very helpful for various scenarios so I don't see a problem with it in general. However, I can't help but feel like you may be choosing a complicated path for solving the problem you are having where a simpler solution may exist. For example, based on the name RequestMatcherDelegatingAuthenticationSuccessHandler seems like it would aim to solve a problem that can be solved by using HttpSecurity#securityMatcher.

Can you please describe your use case in more detail? What authentication schemes are you trying to implement, what types of endpoints are you trying to secure with multiple schemes, what types of clients do you have, etc.?

@sjohnr sjohnr added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Mar 29, 2024
@CrazyParanoid
Copy link
Author

CrazyParanoid commented Mar 29, 2024

@sjohnr Thanks for your comments!

Are you trying to support multiple authentication schemes through a single AuthenticationFilter?

Yes, initially I tried to do this through BearerTokenAuthenticationFilter and I was missing DelegatingBearerTokenResolver. I described this situation #14644. AuthenticationWebFilter has all the necessary components, but I needed to implement #14521 I know many cases when legacy jwt-based authentication schemes work for many years and sooner or later there is a need to migrate to Oauth 2.0.

Adding some of these components may be very helpful for various scenarios so I don't see a problem with it in general. However, I can't help but feel like you may be choosing a complicated path for solving the problem you are having where a simpler solution may exist.

Maybe I'm wrong, but it seems to me that such support in BearerTokenAuthenticationFilter or AuthenticationWebFilter is the simplest solution both from the point of view of code and application overhead. The solution with two securityFilterChain did not suit me because I see it as redundancy and overhead. @sjohnr I will be very grateful if you help me to find a simpler and more suitable solution for my case.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Mar 29, 2024
@sjohnr
Copy link
Member

sjohnr commented Mar 29, 2024

it seems to me that such support in BearerTokenAuthenticationFilter or AuthenticationWebFilter is the simplest solution both from the point of view of code and application overhead.

That depends on the use case, and is also somewhat opinion-based so different developers might come to different perspectives on the same solution. I still am not sure I understand your case. When you say "multiple schemes" are you referring to different authentication mechanisms (e.g. HTTP Basic, SAML, OAuth2), multiple ways of presenting the same credential (e.g. Authorization and X-MyCustomHeader headers) or both?

The solution with two securityFilterChain did not suit me because I see it as redundancy and overhead.

I think this is an opinion-based preference, which is fine. However, optimizing the framework for developer preference isn't my highest priority. We first want to ensure there's a way to do the right things, and that it's reasonable. Afterwards, we might address convenience and preference but it's low priority.

I will be very grateful if you help me to find a simpler and more suitable solution for my case.

I'd be happy to, however this may not be the best place to find an actual solution. We could do that through stack overflow if needed. Here, I'm more interested in whether your situation is possible to solve in the framework. And yes, I would recommend using multiple SecurityFilterChain beans targeting different sets of endpoints. If the only difference between two access patterns is a header name, then I can definitely see the appeal of delegating implementations, which is why I mentioned that is totally reasonable.

Initially I tried to do this through BearerTokenAuthenticationFilter and I was missing DelegatingBearerTokenResolver. I described this situation #14644.

Regarding this issue, I'm still not sure why you can't use BearerTokenAuthenticationFilter. It seems you should be able to use it now. Can you address that question?

@CrazyParanoid
Copy link
Author

CrazyParanoid commented Mar 29, 2024

We first want to ensure there's a way to do the right things, and that it's reasonable. Afterwards, we might address convenience and preference but it's low priority.

I agree with you!

I still am not sure I understand your case.

I can describe my case on stackoverflow, this issue is not the best place for this.

Regarding this issue, I'm still not sure why you can't use BearerTokenAuthenticationFilter. It seems you should be able to use it now. Can you address that question?

Of course I can use it, I didn't say that this is a problem for me now, since a solution was proposed in #14644 :

@Bean 
BearerTokenResolver bearerTokenResolver() {
    BearerTokenResolver one = ...;
    BearerTokenResolver two = ...;
    return (request) -> Optional.ofNullable(one.resolve(request)).orElseGet(() -> two.resolve(request));
}

@sjohnr
Copy link
Member

sjohnr commented Apr 4, 2024

I still am not sure I understand your case.

I can describe my case on stackoverflow, this issue is not the best place for this.

Sadly I don't 100% agree with you here. While I appreciate not wanting to go into deep detail such as describing the actual application (no need to do this) I feel I need to understand the basic premise of your use case, as it is what would drive the need for a BearerTokenAuthenticationConverter. Can you please describe the specific reason you need this component? You have said you need multiple authentication schemes, but have not elaborated on that.

If you don't have details, I think we can either close this issue as not needed or perhaps leave it open for others to weigh in on their use case before proceeding.

@sjohnr sjohnr added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Apr 4, 2024
CrazyParanoid pushed a commit to CrazyParanoid/spring-security that referenced this issue Apr 7, 2024
CrazyParanoid pushed a commit to CrazyParanoid/spring-security that referenced this issue Apr 7, 2024
CrazyParanoid pushed a commit to CrazyParanoid/spring-security that referenced this issue Apr 8, 2024
@spring-projects-issues
Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Apr 11, 2024
@spring-projects-issues
Copy link

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

@spring-projects-issues spring-projects-issues closed this as not planned Won't fix, can't repro, duplicate, stale Apr 18, 2024
@spring-projects-issues spring-projects-issues removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants