Support method security annotations for both synchronous and reactive methods in the same app.

Summary

I'd like the ability to use method security annotations (e.g. @PreAuthorize) on both synchronous and reactive methods within the same application.

@EnableGlobalMethodSecurity enables method security annotations (e.g. @PreAuthorize) to be used on synchronous methods. When checks are performed, the SecurityContext is read from SecurityContextHolder.

@EnableReactiveMethodSecurity enables method security annotations to be used on reactive methods (those that return a Publisher). When checks are performed, the SecurityContext is read from ReactiveSecurityContextHolder.

However, currently it is not possible to use both @EnableGlobalMethodSecurity and @EnableReactiveMethodSecurity within the same application.

In my use case, I have a WebFlux application that serves most requests with reactive methods. However, some requests are dispatched to legacy synchronous code (on a bounded elastic scheduler). And that legacy code also uses @PreAuthorize in some places.

When delegating to this other scheduler, I'm copying the SecurityContext from ReactiveSecurityContextHolder to SecurityContextHolder. Theoretically, this could allow @PreAuthorize to continue to work for the synchronous methods.

Actual Behavior

When both @EnableGlobalMethodSecurity and @EnableReactiveMethodSecurity are enabled at within the same app, the following error occurs:

The bean 'methodSecurityInterceptor', defined in class path resource [org/springframework/security/config/annotation/method/configuration/ReactiveMethodSecurityConfiguration.class], could not be registered. A bean with that name has already been defined in class path resource [org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.class] and overriding is disabled.

Expected Behavior

No errors when enabling method security annotations for both synchronous and reactive methods.
Method security annotations work for both synchronous and reactive methods. (assuming the SecurityContext is available in the appropriate holder). It would be fine to require the app to manage propagating the SecurityContext between holders when appropriate. I'm not expecting spring security to have to do that everywhere.
The same MethodSecurityExpressionHandler should be able to be used by both

Configuration

@EnableGlobalMethodSecurity
@EnableReactiveMethodSecurity

Version

5.2.0.RELEASE

Hi, @philsttr. I wonder if this is still an issue if you move from @EnableGlobalMethodSecurity to @EnableMethodSecurity. Is that something that you can try?

The reason I think it will work is that the new @EnableMethodSecurity annotation does not bring in a configuration that publishes a bean named methodSecurityInterceptor.

Hi @jzheaux,

Do you mean replace both @EnableGlobalMethodSecurity and @EnableReactiveMethodSecurity with a single @EnableMethodSecurity annotation?

Or only replace @EnableGlobalMethodSecurity with @EnableMethodSecurity, and continue also using @EnableReactiveMethodSecurity ?

Summary

I'd like the ability to use method security annotations (e.g. @PreAuthorize) on both synchronous and reactive methods within the same application.

@EnableGlobalMethodSecurity enables method security annotations (e.g. @PreAuthorize) to be used on synchronous methods. When checks are performed, the SecurityContext is read from SecurityContextHolder.

@EnableReactiveMethodSecurity enables method security annotations to be used on reactive methods (those that return a Publisher). When checks are performed, the SecurityContext is read from ReactiveSecurityContextHolder.

However, currently it is not possible to use both @EnableGlobalMethodSecurity and @EnableReactiveMethodSecurity within the same application.

In my use case, I have a WebFlux application that serves most requests with reactive methods. However, some requests are dispatched to legacy synchronous code (on a bounded elastic scheduler). And that legacy code also uses @PreAuthorize in some places.

When delegating to this other scheduler, I'm copying the SecurityContext from ReactiveSecurityContextHolder to SecurityContextHolder. Theoretically, this could allow @PreAuthorize to continue to work for the synchronous methods.

Actual Behavior

When both @EnableGlobalMethodSecurity and @EnableReactiveMethodSecurity are enabled at within the same app, the following error occurs:
The bean 'methodSecurityInterceptor', defined in class path resource [org/springframework/security/config/annotation/method/configuration/ReactiveMethodSecurityConfiguration.class], could not be registered. A bean with that name has already been defined in class path resource [org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.class] and overriding is disabled.
Expected Behavior

No errors when enabling method security annotations for both synchronous and reactive methods.

Method security annotations work for both synchronous and reactive methods. (assuming the SecurityContext is available in the appropriate holder). It would be fine to require the app to manage propagating the SecurityContext between holders when appropriate. I'm not expecting spring security to have to do that everywhere.

The same MethodSecurityExpressionHandler should be able to be used by both

Configuration
@EnableGlobalMethodSecurity
@EnableReactiveMethodSecurity
Version

5.2.0.RELEASE

I have the same problem. @philsttr how did you manage to copy the SecurityContext from the ReactiveSecurityContextHolder to the SercurityContextHolder? I'm struggling with it. I tried to use @EnableMethodSecurity, no errors but the @PreAuthorize can't find the Principal.

Reactor's context requires the return types be Mono or Flux and is the limiting factor here, so there isn't much we can do unless Reactor supports this. I know there had been talks of providing automatic transitioning of contexts between ThreadLocal and Reactor Context, but I have not seen that materialize.

Perhaps either @simonbasle or @rstoyanchev have an update to if there is a way to support a Context that works for reactive and imperative within the same call stack.

I have the same problem. @philsttr how did you manage to copy the SecurityContext from the ReactiveSecurityContextHolder to the SercurityContextHolder? I'm struggling with it. I tried to use @EnableMethodSecurity, no errors but the @PreAuthorize can't find the Principal.

As @rwinch mentioned, reactor does not have an automatic way of propagating items from the Context to ThreadLocals. I don't think it ever will, because there are too many boundaries between reactive and synchronous code, and propagating the items from the Context to ThreadLocals at every boundary would be extremely expensive. In our codebase, we identify which specific boundaries where we need this propagation, and we propagate it manually. With the way our codebase is structured for handling these requests, there are only a few boundaries where we need to propagate. So, this manual propagation is not terrible.

Here's how the manual propagation works:

We have a ContextToThreadLocalPropagator interface with the following simplified definition. In reality, it has some other default methods, and some convenience methods for building a propagator that propagates specific Context keys to specific ThreadLocals, but you can get the idea from this simplified definition:

/**
 * Propagates values from a subscriber {@link Context} to various {@link ThreadLocal}s.
 */
@FunctionalInterface
public interface ContextToThreadLocalPropagator {

    /**
     * When closed, removes thread local values that were propagated via
     * {@link #propagateContextToThreadLocals(ContextView)}.
     *
     * <p>For example, if you have a direct reference to the {@link ThreadLocal},
     * then call {@link ThreadLocal#remove()}.</p>
     */
    interface ThreadLocalCloseable extends AutoCloseable {
        void close();
    }

    /**
     * Copies values from the given context view into various {@link ThreadLocal}s,
     * and returns a {@link ThreadLocalCloseable} that will remove the values when closed.
     *
     * <p>The returned {@link ThreadLocalCloseable} is guaranteed to be called on
     * the same thread as the thread on which the {@link #propagateContextToThreadLocals(ContextView)}
     * call that returned it was made.</p>
     *
     * @param contextView the (non-null) subscriber {@link ContextView} from which to obtain values.
     * @return a closable that will reset the thread local to the value prior to propagation when closed
     */
    ThreadLocalCloseable propagateContextToThreadLocals(ContextView contextView);

}

We have various implementations of that interface for the various things we want to propagate. For example, we have implementations for the Spring Security context, the MDC, some metric stuff, some tracing stuff, etc. In addition, we have a composite implementation. And we have a global instance that is a composite implementation to which specific implementations can be registered at runtime via ServiceLoader or explicit registration.

Here is the implementation that propagates the Spring Security context:

class SecurityContextToThreadLocalPropagator implements ContextToThreadLocalPropagator {

    @Override
    public ThreadLocalCloseable propagateContextToThreadLocals(ContextView contextView) {
        // Save the existing contextView so it can be restored later
        SecurityContext existingSecurityContext = SecurityContextHolder.getContext();

        // Propagate reactive SecurityContext to SecurityContextHolder.
        // Unfortunately, ReactiveSecurityContextHolder does not provide a way to
        // retrieve a SecurityContext from a specific ContextView,
        // so this has to retrieve it from the Context directly.
        Mono<SecurityContext> securityContextMono = contextView.getOrDefault(SecurityContext.class, Mono.empty());
        if (securityContextMono != null) {
            // Unfortunately, Spring Security puts a Mono<SecurityContext> into the Context,
            // instead of the SecurityContext itself, so this has to block to get it. 
            SecurityContext securityContext = securityContextMono.block();
            if (securityContext != null) {
                SecurityContextHolder.setContext(copySecurityContext(securityContext));
            } else {
                SecurityContextHolder.clearContext();
            }
        } else {
            SecurityContextHolder.clearContext();
        }

        // Return a closable that restores the previous existing contextView
        if (existingSecurityContext == null) {
            return SecurityContextHolder::clearContext;
        } else {
            return () -> SecurityContextHolder.setContext(existingSecurityContext);
        }
    }
}

Then, in the few reactive -> synchronous boundaries at which we want to propagate items from the Context to ThreadLocals, we do something like this:

return Mono.deferContextual(context -> Mono.fromCallable(() -> {
    try (ContextToThreadLocalPropagator.ThreadLocalCloseable closeable = 
            globalContextToThreadLocalPropagator.propagateContextToThreadLocals(context)) {
        return invokeSynchronousMethodHere();
    }
}).subscribeOn(Schedulers.boundedElastic()));

Anything within invokeSynchronousMethodHere() can access the SecurityContextHolder and other ThreadLocals.

So theoretically, @PreAuthorize on a synchronous method could work here, if Spring Security supported looking up the securtiy context from ReactiveSecurityContextHolder if @PreAuthorize is on a reactive method or looking it up via SecurityContextHolder if @PreAuthorize is on a synchronous method. I have not tried the latest recommendation of using both @EnableReactiveMethodSecurity and @EnableMethodSecurity at the same to see if it actually works.

Another use case is by using the new @AuthorizeReturnObject for a domain object. Consider the following code:

class User {

  @PreAuthorize("hasAuthority('user:read')")
  String getEmail() {
    // ...
  }

}

@Component
class Repository {

  @AuthorizeReturnObject
  Mono<User> getUser(Long id) {
    return findById(id);
  }

}

@Component
class Client {

  private Repository repository;

  Mono<Void> doSomethingWithTheEmail() {
    return this.repository.getUser(1).flatMap(u -> {
      u.getEmail(); // an exception is thrown here
    }).then();
  }

}

An exception will be thrown because the return type of getEmail() is not a reactive type, however this is a valid use case and should be supported.

spring-projects / spring-security