Summary

I'm trying to use graphql-java on a Reactive Spring Boot (2.0.0.RC2) codebase with Spring WebFlux (5.0.4.RELEASE).

When the GraphQL HTTP endpoint gets called, I delegate to the GraphQL engine, which executes asynchronously behind the scene. During the execution, some @Services of mine get called, where some methods are annotated with @PreAuthorize.

Spring Security fails at authorizing the calls, even if the authenticated user should have been authorized. Note that Spring Security does get called (as I'm receiving a "Denied" response), it just cannot authorize the call.

My requests are authenticated via Basic Auth for now.

Actual Behavior

When SecurityExpressionRoot#hasAnyAuthorityName(...) is called, the authorities retrieved with getAuthoritySet() only contains one single ROLE_ANONYMOUS entry, which then forbids the access to the resource.

The cause might be that, when PrePostAdviceReactiveMethodInterceptor#invoke(...) is run, ReactiveSecurityContextHolder.getContext() seems to return no authentication (i.e. an empty Mono).

Expected Behavior

I would have expected the ReactiveSecurityContextHolder to still somehow hold the authentication generated when the HTTP endpoint got called.

Configuration

See my following comment for an MVCE.

Version

5.0.2.RELEASE

Notes

  • This cannot be reproduced when doing vanilla Spring WebFlux HTTP calls (i.e. without that additional GraphQL asynchronous layer).
  • Might be related to https://github.com/spring-projects/spring-security/issues/4047.
  • I wanted to give a try to SecurityContextHolder.MODE_INHERITABLETHREADLOCAL, but I couldn't find the reactive alternative (if any).

Comment From: sp00m

Just made an MVCE that replicates the issue: https://github.com/sp00m/spring-security-5034.

If you GET http://username:password@localhost:8080/sync-hello, you'll receive:

hello

But if you GET http://username:password@localhost:8080/async-hello, you'll receive:

{
    "timestamp": "2018-02-22T10:26:30.495+0000",
    "path": "/async-hello",
    "status": 500,
    "error": "Internal Server Error",
    "message": "org.springframework.security.access.AccessDeniedException: Denied"
}

See the HelloHandler.

Comment From: sp00m

I may have found a solution:

public Mono<ServerResponse> asyncHello(ServerRequest request) {
    return ReactiveSecurityContextHolder
            .getContext()
            .map(SecurityContext::getAuthentication)
            .map(ReactiveSecurityContextHolder::withAuthentication)
            .flatMap(context -> Mono.fromFuture(CompletableFuture.supplyAsync(() -> service.getProtectedHello().subscriberContext(context).block())))
            .transform(hello -> ok().body(hello, String.class));
}

Is that the right approach? If yes, just need to find how make this possible with graphql-java...

Comment From: rwinch

I'm not sure how to do this in the context of graphql-java as I'm not familiar with it. However, using block() is not the recommended approach. Anytime this is done the reactive context will be lost. Instead, I'd recommend doing something like this:

public Mono<ServerResponse> asyncHello(ServerRequest request) {
    CompletableFuture<String> future = service.getProtectedHello().toFuture();
    return Mono.fromFuture(future)
            .transform(hello -> ok().body(hello, String.class));
}

Does that help?

Comment From: rwinch

As an FYI you can simplify (and improve security slightly) of your sample from using

@Bean
public ReactiveUserDetailsService userDetailsService() {
    UserDetails username = withUsername("username").password("password").roles("USER").build();
    return new MapReactiveUserDetailsService(username);
}

@Bean
public ReactiveAuthenticationManager authenticationManager(ReactiveUserDetailsService userDetailsService) {
    UserDetailsRepositoryReactiveAuthenticationManager authenticationManager = new UserDetailsRepositoryReactiveAuthenticationManager(userDetailsService);
    authenticationManager.setPasswordEncoder(NoOpPasswordEncoder.getInstance());
    return authenticationManager;
}

with just

@Bean
public ReactiveUserDetailsService userDetailsService() {
    UserDetails username = withDefaultPasswordEncoder().username("username").password("password").roles("USER").build();
    return new MapReactiveUserDetailsService(username);
}

If you must use NoOpPasswordEncoder you just need to expose it as a @Bean (no need to create ReactiveAuthenticationManager explicitly.

Comment From: sp00m

Thanks for you feedback.

I .block()-ed in the MVCE, but my real world scenario does not .block() (maybe I failed the MVCE then).

Basically:

  1. My reactive handler receives an HTTP call with a GraphQL query;
  2. It delegates the execution of the query to the GraphQL engine of graphql-java, which handles asynchronicity thanks to CompletableFutures;
  3. During the execution, some of my reactive services are being called, where the returned values of type Mono/Flux are transformed to CompletableFutures to make the GraphQL engine happy;
  4. The whole result of the execution is being transformed back into a Mono to be WebFlux-compatible.

Quite a lot of conversions I admit, but works quite good so far.

The problem is that during the execution of the query, the reactive security context is lost, I guess because of the fact that is has been executed through CompletableFutures by a third-party lib (graphql-java) that isn't WebFlux-aware.

In other words, when a third-party lib calls your reactive services from within a CompletableFuture execution, how to keep the reactive security context so that the services can be authorised?

The idea of the solution I posted is to first get the context, then launch the query execution, but manually passing the context inside the CompletableFuture execution thanks to .subscriberContext(context).

Did I manage to better explain my issue and solution? If yes, the remaining question is, is my solution valid?

Comment From: rwinch

It is valid. Another option is that if you have another way of obtaining the SecurityContext dynamically, then you can create an Hooks.onLastOperator to populate the SecurityContext automatically. You want to ensure that the hook is registered only once and that there is no way for it to discover the incorrect SecurityContext (it must be thread safe).

cc @smaldini

Comment From: rwinch

I'm closing this as resolved as I don't think there is anything additional we can do from a Spring Security perspective. Please create a separate issue if you find a specific thing we can do to make this work better.