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

JwtIssuerReactiveAuthenticationManagerResolver bean from autoconfiguration has a bug #100

Closed
lArtiquel opened this issue Feb 17, 2023 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@lArtiquel
Copy link
Contributor

Description
Hello 👋

I spotted a small bug while testing a multi-tentancy feature.

Expected Result
In my use-case scenario, REST API endpoint call to the resource server with auth token issued by Realm not specified in the com.c4-soft.springaddons.security.issuers[*].location list, should return 401 (Unauthorized) Status Code instead of 500 SC.

Debugging process explained

  • In the logs, I found a good-old NPE
2023-02-17 20:35:11.442 ERROR 214332 --- [ctor-http-nio-7] a.w.r.e.AbstractErrorWebExceptionHandler : [65251a12-1]  500 Server Error for HTTP GET "/dummy"

java.lang.NullPointerException: null
	at org.springframework.security.oauth2.server.resource.authentication.JwtIssuerReactiveAuthenticationManagerResolver$ResolvingAuthenticationManager.lambda$authenticate$1(JwtIssuerReactiveAuthenticationManagerResolver.java:145) ~[spring-security-oauth2-resource-server-5.5.4.jar:5.5.4]
	Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException: 
Error has been observed at the following site(s):
	*__checkpoint ⇢ org.springframework.security.web.server.authentication.AuthenticationWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.context.ReactorContextWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.header.HttpHeaderWriterWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.config.web.server.ServerHttpSecurity$ServerWebExchangeReactorContextWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.WebFilterChainProxy [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.boot.actuate.metrics.web.reactive.server.MetricsWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ HTTP GET "/dummy" [ExceptionHandlingWebHandler]
  • After checking
    org.springframework.security.oauth2.server.resource.authentication.JwtIssuerReactiveAuthenticationManagerResolver$ResolvingAuthenticationManager.ResolvingAuthenticationManager#authenticate(), I found out that the issue is on the following line when we try to resolve AuthenticationManager against non-existent manager in the map.
return this.issuerAuthenticationManagerResolver.resolve(issuer).switchIfEmpty(Mono.error(() -> {
                    return new InvalidBearerTokenException("Invalid issuer " + issuer);
                }));
  • Then after realizing that it's a Spring Security dependency, I tracked down where that issuerAuthenticationManagerResolver bean gets created that is used here. I found it in the AddonsWebSecurityBeans.java config class, here it is:
    @ConditionalOnMissingBean
    @Bean
    ReactiveAuthenticationManagerResolver<ServerWebExchange> authenticationManagerResolver(
            OAuth2ResourceServerProperties auth2ResourceServerProperties,
            SpringAddonsSecurityProperties addonsProperties,
            Converter<Jwt, Mono<AbstractAuthenticationToken>> jwtAuthenticationConverter) {
        final var jwtProps = Optional.ofNullable(auth2ResourceServerProperties)
                .map(OAuth2ResourceServerProperties::getJwt);
        // @formatter:off
		Optional.ofNullable(jwtProps.map(OAuth2ResourceServerProperties.Jwt::getIssuerUri)).orElse(jwtProps.map(OAuth2ResourceServerProperties.Jwt::getJwkSetUri))
		    .filter(StringUtils::hasLength)
		    .ifPresent(jwtConf -> {
				log.warn("spring.security.oauth2.resourceserver configuration will be ignored in favor of com.c4-soft.springaddons.security");
			});
		// @formatter:on

        final Map<String, Mono<ReactiveAuthenticationManager>> jwtManagers = Stream.of(addonsProperties.getIssuers())
                .collect(Collectors.toMap(issuer -> issuer.getLocation().toString(), issuer -> {
                    ReactiveJwtDecoder decoder = issuer.getJwkSetUri() != null
                            && StringUtils.hasLength(issuer.getJwkSetUri().toString())
                                    ? NimbusReactiveJwtDecoder.withJwkSetUri(issuer.getJwkSetUri().toString()).build()
                                    : ReactiveJwtDecoders.fromIssuerLocation(issuer.getLocation().toString());
                    var provider = new JwtReactiveAuthenticationManager(decoder);
                    provider.setJwtAuthenticationConverter(jwtAuthenticationConverter);
                    return Mono.just(provider);
                }));

        log.debug(
                "Building default JwtIssuerReactiveAuthenticationManagerResolver with: {} {}",
                auth2ResourceServerProperties.getJwt(),
                Stream.of(addonsProperties.getIssuers()).toList());
        return new JwtIssuerReactiveAuthenticationManagerResolver(
                (ReactiveAuthenticationManagerResolver<String>) jwtManagers::get);
    }
  • As we can see, Resolver does return a null in case if Manager with such issuer URI does not exist in the map
        return new JwtIssuerReactiveAuthenticationManagerResolver(
                (ReactiveAuthenticationManagerResolver<String>) jwtManagers::get);

Therefore, I just patched that line to:

        return new JwtIssuerReactiveAuthenticationManagerResolver(managerName ->
                jwtManagers.getOrDefault(managerName, Mono.empty()));

And used that Bean instead and after that I got expected 401 SC in the response 🎉 .

Additional info
Reactive (WebFlux) application.
Spring Addon dependency used:
implementation "com.c4-soft.springaddons:spring-addons-webflux-jwt-resource-server:$5.4.0"

I could not use 6.x versions b/c of my Spring Boot (2.5.9) version. But I see in the source code that the bug is still there ;)

Questions
Can open a PR with that quickfix?
In the case of approval, could you please release that fix for 5.x version (e.g. 5.4.1) for me so that I could use it without hacking this autocofiguration bean?

Thanks 🙏

@lArtiquel lArtiquel added the bug Something isn't working label Feb 17, 2023
lArtiquel added a commit to lArtiquel/spring-security-addons that referenced this issue Feb 17, 2023
@ch4mpy
Copy link
Owner

ch4mpy commented Feb 17, 2023

Good catch.

Would you create a PR for that? If I just copy your code, you would not get the authoring.

@lArtiquel
Copy link
Contributor Author

@ch4mpy yep, just created one #101

ch4mpy pushed a commit that referenced this issue Feb 17, 2023
…ssuer is not listed in configuration

Thank you for your contribution!
lArtiquel added a commit to lArtiquel/spring-security-addons that referenced this issue Feb 17, 2023
ch4mpy pushed a commit that referenced this issue Feb 17, 2023
@ch4mpy
Copy link
Owner

ch4mpy commented Feb 17, 2023

@lArtiquel I just released

  • 5.4.2 (assembled with JDK-8, to be used with boot 2)
  • 6.0.15 (assembled with JDK-17, to be used with boot 3)

Both are now available from maven-central (I just checked)

I also updated the main README (after the releases so that there is no risk one finds it in README but not on maven-central)

Thank you again for reporting the bug, finding a fix and submitting a PR for each branch!

Do not hesitate to submit a feature request if your are missing something on the 5.x branch that would be only on the 6.x. I'd handle the backport.

@ch4mpy ch4mpy closed this as completed Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants