Skip to content

Support delegating BearerTokenResolver #14644

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
franticticktick opened this issue Feb 22, 2024 · 8 comments · Fixed by #14654 or #14655
Closed

Support delegating BearerTokenResolver #14644

franticticktick opened this issue Feb 22, 2024 · 8 comments · Fixed by #14654 or #14655
Assignees
Labels
in: web An issue in web modules (web, webmvc) status: duplicate A duplicate of another issue type: enhancement A general enhancement

Comments

@franticticktick
Copy link
Contributor

franticticktick commented Feb 22, 2024

Need to add support for retrieving tokens from different headers in one component. This is a common case when a project has multiple authentication schemes. For example, in ProviderManager there are two providers, but one gets the token through DefaultBearerTokenResolver and the other from a non-standard header like -X-Authorization.
For example, component DelegatingBearerTokenResolver:

    @Override
    public String resolve(HttpServletRequest request) {
        return delegates.stream()
                .map(d -> d.resolve(request))
                .filter(Objects::nonNull)
                .findAny()
                .orElse(null);
    }
@franticticktick franticticktick added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Feb 22, 2024
@franticticktick
Copy link
Contributor Author

It also makes sense to make a DelegatingServerAuthenticationConverter for different authentication schemes. What’s interesting is there is no way to change the authorizationPattern in the ServerBearerTokenAuthenticationConverter, which prevents this component from being used for non-standard authentication schemes.

@jzheaux
Copy link
Contributor

jzheaux commented Feb 26, 2024

I think a DelegatingServerAuthenticationConverter would be nice, @CrazyParanoid. Can you submit a PR for it?

@jzheaux jzheaux added status: ideal-for-contribution An issue that we actively are looking for someone to help us with and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 26, 2024
@jzheaux jzheaux self-assigned this Feb 26, 2024
kse-music added a commit to kse-music/spring-security that referenced this issue Feb 27, 2024
kse-music added a commit to kse-music/spring-security that referenced this issue Feb 27, 2024
kse-music added a commit to kse-music/spring-security that referenced this issue Feb 27, 2024
franticticktick pushed a commit to franticticktick/spring-security that referenced this issue Feb 27, 2024
franticticktick pushed a commit to franticticktick/spring-security that referenced this issue Feb 27, 2024
@franticticktick
Copy link
Contributor Author

Added DelegatingBearerTokenResolver. I see that DelegatingServerAuthenticationConverter was added in another PR.

kse-music added a commit to kse-music/spring-security that referenced this issue Feb 28, 2024
kse-music added a commit to kse-music/spring-security that referenced this issue Feb 29, 2024
kse-music added a commit to kse-music/spring-security that referenced this issue Feb 29, 2024
kse-music added a commit to kse-music/spring-security that referenced this issue Feb 29, 2024
kse-music added a commit to kse-music/spring-security that referenced this issue Feb 29, 2024
franticticktick pushed a commit to franticticktick/spring-security that referenced this issue Feb 29, 2024
jzheaux pushed a commit that referenced this issue Mar 5, 2024
@jzheaux jzheaux added status: duplicate A duplicate of another issue in: web An issue in web modules (web, webmvc) and removed status: ideal-for-contribution An issue that we actively are looking for someone to help us with labels Mar 5, 2024
@jzheaux
Copy link
Contributor

jzheaux commented Mar 5, 2024

I'm reopening this since there is more to discuss to address the servlet side of this enhancement.

@jzheaux jzheaux reopened this Mar 5, 2024
@jzheaux
Copy link
Contributor

jzheaux commented Mar 5, 2024

I don't prefer to add a delegating bearer token resolver as the use case is quite narrow. Instead, I'd prefer to add DelegatingAuthenticationConverter so that it can service more use cases. If you are interested a PR is welcome.

I realize that BearerTokenAuthenticationFilter doesn't use AuthenticationConverter currently, so that doesn't address your concern directly. I believe #11983 will. In the meantime, you can do:

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

@jzheaux jzheaux added the status: waiting-for-feedback We need additional information before we can continue label Mar 5, 2024
franticticktick pushed a commit to franticticktick/spring-security that referenced this issue Mar 5, 2024
@franticticktick
Copy link
Contributor Author

Hi @jzheaux. Thanks for your feedback! I added support DelegatingAuthenticationConverter to my PR.

@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 5, 2024
@franticticktick
Copy link
Contributor Author

franticticktick commented Mar 6, 2024

@jzheaux if the AuthenticationFilter is going to be placed in the configurer, maybe it would be worth implementing a BearerTokenAuthenticationConverter? In this case, in the configurer you can implement:

	@Override
	public void configure(H http) {
		AuthenticationManagerResolver resolver = this.authenticationManagerResolver;
		if (resolver == null) {
			AuthenticationManager authenticationManager = getAuthenticationManager(http);
			resolver = (request) -> authenticationManager;
		}

		BearerTokenAuthenticationConverter converter = new BearerTokenAuthenticationConverter();

		AuthenticationFilter filter = new AuthenticationFilter(resolver, converter);
		filter.setSecurityContextHolderStrategy(getSecurityContextHolderStrategy());
		filter = postProcess(filter);
		http.addFilter(filter);
	}

franticticktick pushed a commit to franticticktick/spring-security that referenced this issue Mar 7, 2024
franticticktick pushed a commit to franticticktick/spring-security that referenced this issue Mar 8, 2024
@jzheaux jzheaux removed the status: feedback-provided Feedback has been provided label Mar 13, 2024
@jzheaux
Copy link
Contributor

jzheaux commented Mar 13, 2024

@CrazyParanoid, I think it is worth looking into so long as #9576 is not regressed. Will you please open a separate ticket for now, now that this ticket because the work for adding delegating authentication converters?

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) status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
3 participants