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

#100 JwtIssuerReactiveAuthenticationManagerResolver bean from autoconfiguration bugfix #101

Conversation

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
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
Copy link
Contributor Author

Resolves #100 issue
Sorry for duplicating, didn't plan to create a PR at the beginning

@ch4mpy
Copy link
Owner

ch4mpy commented Feb 17, 2023

There is a 5.x branch and you should be able to create another pull-request, and sure, your patch should be applied there too.

I suggest you git checkout 5.x; git cherry-pick c828d2dd1b806536adb87cf9fa102722ca7d5db1 then create a second pull-request targeting ch4mpy:5.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants