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

fix: Run createAuthentication on a BLOCKING thread #1847

Merged
merged 4 commits into from
Nov 13, 2024

Conversation

kevin-wise
Copy link
Contributor

@kevin-wise kevin-wise commented Nov 5, 2024

fixes #1846

  • matches behavior of NimbusReactiveJsonWebTokenValidator with JwtTokenProvider, which it replaced
  • so blocking operations such as a RolesFinder that uses a database won't tie up Netty event loop threads

Let me know if you want this to be configurable, or if it's ok to always do it; and what kind of tests you want.

If it needs to be configurable, would something like this be acceptable?

    NimbusReactiveJsonWebTokenValidator(
            List<GenericJwtClaimsValidator<R>> claimsValidators,
            List<SignatureConfiguration> imperativeSignatureConfigurations,
            List<ReactiveSignatureConfiguration<SignedJWT>> reactiveSignatureConfigurations,
            JsonWebTokenParser<JWT> jsonWebTokenParser,
            ReactiveJsonWebTokenSignatureValidator<SignedJWT> signatureValidator,
            JwtAuthenticationFactory jwtAuthenticationFactory,
            BeanContext beanContext,
            @Value("{" + JwtConfigurationProperties.PREFIX + ".reactive-validator.executor:"
                   + TaskExecutors.BLOCKING + "}") String executor) {
        super(claimsValidators, imperativeSignatureConfigurations, reactiveSignatureConfigurations);
        this.jsonWebTokenParser = jsonWebTokenParser;
        this.signatureValidator = signatureValidator;
        this.jwtAuthenticationFactory = jwtAuthenticationFactory;
        this.scheduler = StringUtils.isEmpty(executor) ? null : getScheduler(beanContext, executor);
    }

    private static Scheduler getScheduler(BeanContext beanContext, String executor) {
        try {
            return Schedulers.fromExecutorService(
                    beanContext.getBean(ExecutorService.class, Qualifiers.byName(executor)));
        } catch (NoSuchBeanException e) {
            throw new ConfigurationException(JwtConfigurationProperties.PREFIX +
                    ".reactive-validator.executor is invalid; no executor configured for name: " +
                    executor, e);
        }
    }

    @Override
    @SingleResult
    public Publisher<Authentication> validateToken(String token, R request) {
        Mono<JWT> publisher = Mono.from(validate(token, request));
        if (scheduler != null) {
            publisher = publisher.subscribeOn(scheduler);
        }
        return publisher
                .map(jwtAuthenticationFactory::createAuthentication)
                .filter(Optional::isPresent)
                .map(Optional::get);
    }

- matches behavior of NimbusReactiveJsonWebTokenValidator with JwtTokenProvider, which it replaced
- so blocking operations such as a RolesFinder that uses a database won't tie up Netty event group threads
@kevin-wise
Copy link
Contributor Author

@sdelamo this is coming from the discussion in micronaut-projects/micronaut-core#10924

@sdelamo
Copy link
Contributor

sdelamo commented Nov 13, 2024

I think it is correct that RolesFinder gets called in the Netty thread as the default implementation DefaultRolesFinder is not blocking.

Honestly, I never thought RolesFinder implementation will call a database.

We could make it configurable to opt-in to subscribeOn on the blocking scheduler. But it looks a bit of a hack.

@sdelamo
Copy link
Contributor

sdelamo commented Nov 13, 2024

@kevin-wise I have added a test and a opt-in configuration option. Let me know what you think

@sdelamo sdelamo merged commit fb5416b into micronaut-projects:4.11.x Nov 13, 2024
13 checks passed
@kevin-wise
Copy link
Contributor Author

Thanks for adding the test and the configuration option. I had imagined the configuration would allow you to pick the scheduler, but this is sufficient for my needs.

@kevin-wise kevin-wise deleted the fix/issue-1846 branch November 13, 2024 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

NimbusReactiveJsonWebTokenValidator runs on Netty event group threads
2 participants