Skip to content

Consider making BearerTokenServerWebExchangeMatcher public and more generic #8824

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
qavid opened this issue Jul 10, 2020 · 1 comment · Fixed by #8854
Closed

Consider making BearerTokenServerWebExchangeMatcher public and more generic #8824

qavid opened this issue Jul 10, 2020 · 1 comment · Fixed by #8854
Assignees
Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement
Milestone

Comments

@qavid
Copy link
Contributor

qavid commented Jul 10, 2020

Expected Behavior

Rename bearerTokenConverter (and setter) to more generic name, eg. serverAuthenticationConverter
Rename BearerTokenServerWebExchangeMatcher to more generic name eg. AuthenticationServerWebExchangeMatcher and make it public.

Then, AuthenticationServerWebExchangeMatcher can be used as a securityMatcher when configuring SecurityFilterChain and also as matcher for authentication entry point, access denied handler and CSRF protection in OAuth2ResourceServerSpec.

Current Behavior

BearerTokenServerWebExchangeMatcher is private class in OAuth2ResourceServerSpec and is used as a matcher only for authentication entry point, access denied handler and CSRF protection.

It's necessary to create custom ServerWebExchangeMatcher which is almost identical with BearerTokenServerWebExchangeMatcher.

Context

We have defined multiple SecurityFilterChains, each has security matcher based on currently used authentication method.

private SecurityWebFilterChain httpBasicSecurityFilterChain(ServerHttpSecurity http) {
    http.securityMatcher(new AuthenticationServerWebExchangeMatcher(new ServerHttpBasicAuthenticationConverter()));
    // some configuration
    return http.build();
}

private SecurityWebFilterChain oauth2SecurityFilterChain(ServerHttpSecurity http) {
    http.securityMatcher(new AuthenticationServerWebExchangeMatcher(new ServerBearerTokenAuthenticationConverter()));
    // some configuration
    return http.build();
}

private SecurityWebFilterChain x509SecurityFilterChain(ServerHttpSecurity http) {
    http.securityMatcher(new AuthenticationServerWebExchangeMatcher(new ServerX509AuthenticationConverter(extractor)));
    // some configuration
    return http.build();
}
@qavid qavid added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Jul 10, 2020
@jzheaux jzheaux added in: web An issue in web modules (web, webmvc) and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 15, 2020
@jzheaux
Copy link
Contributor

jzheaux commented Jul 16, 2020

I think something like this could be useful, though I'd recommend AuthenticationServerWebExchangeMatcher take the ServerAuthenticationConverter in its constructor, matching your code sample. I'd also recommend it be called AuthenticationConverterServerWebExchangeMatcher.

Are you able to submit a PR that introduces that class as well as refactors the resource server DSL to use it?

jzheaux pushed a commit that referenced this issue Jul 21, 2020
AuthenticationConverterServerWebExchangeMatcher is ServerWebExchangeMatcher implementation based on AuthenticationConverter which matches if ServerWebExchange can be converted to Authentication.
It can be used as a matcher where SecurityFilterChain should be matched based on used authentication method.
BearerTokenServerWebExchangeMatcher was replaced by this matcher.

Closes gh-8824
jzheaux added a commit that referenced this issue Jul 21, 2020
@jzheaux jzheaux added this to the 5.4.0-RC1 milestone Aug 5, 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: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants