ReactiveSecurityContextHolder.getContext() is broken when used with Mono.toFuture().

[dave-fl]

Summary

ReactiveSecurityContextHolder is broken when used with Futures. It does not always provide results and sometimes just fires the onComplete signal.

Actual Behavior

Executes onComplete()

Expected Behavior

Should execute onNext()

Version

5.0.7 Release

Sample

@Test
public void testWorkingContext() {
	Authentication authentication = new PreAuthenticatedAuthenticationToken("TEST", "");
	Mono<String> working = ReactiveSecurityContextHolder.getContext()
			.map(securityContext -> (String)securityContext.getAuthentication().getPrincipal());

	Mono<String> stringMono = working.subscriberContext(ReactiveSecurityContextHolder.withAuthentication(authentication));
	StepVerifier.create(stringMono).expectNext("TEST").verifyComplete();
}


@Test
public void testBrokenContext() {
	Authentication authentication = new PreAuthenticatedAuthenticationToken("TEST", "");
	Mono<String> working = ReactiveSecurityContextHolder.getContext()
			.map(securityContext -> (String)securityContext.getAuthentication().getPrincipal());
	Mono<String> broken = Mono.fromFuture(working.toFuture());
	Mono<String> stringMono = broken.subscriberContext(ReactiveSecurityContextHolder.withAuthentication(authentication));
	StepVerifier.create(stringMono).expectNext("TEST").verifyComplete();
}

[rwinch]

Thanks for the report. If it is just an arbitrary Future I would not expect it to work. However, if the Future was produced from a Mono I would think it could work. If it were to work it would be a Project Reactor change.

What are your thoughts @simonbasle and @smaldini?

[dave-fl]

@rwinch In my test, the CompletableFuture is produced from a Mono.

[rwinch]

@dave-fl Right. This is why I didn't immediately close the issue since this seems somewhat reasonable to support to me.

In any case, this is something we will need Project Reactor to support. which is why I pinged Simon and Stephane. We will wait to see what their thoughts are.

[dave-fl]

Another simple example.

@Test
public void simple() {
	Authentication authentication = new PreAuthenticatedAuthenticationToken("SIMPLE SIMON", "");
	Mono<String> context = ReactiveSecurityContextHolder.getContext().flatMap(securityContext -> {
		return Mono.just((String)securityContext.getAuthentication().getPrincipal());
	});

	Mono<String> wrappedWithSubscribe = Mono.create(monoSink -> context.subscribe(s -> monoSink.success(s), e -> monoSink.error(e), () -> monoSink.success()));
	Mono<String> addedContext = wrappedWithSubscribe
			.subscriberContext(ReactiveSecurityContextHolder.withAuthentication(authentication));

	StepVerifier.create(addedContext).expectNext("SIMPLE SIMON").verifyComplete();
}

[simonbasle]

@dave-fl you are switching to a Future and then back to a Mono. The Context is purely a Reactor concept, and is propagated along Reactive Streams signals by Flux and Mono as long as you don't break the chain of operators (your second example with a subscribe breaks that assumption).

I gather you did that first test to simulate using an external Future-based API in the middle?

The MonoCompletionStage can probably be made aware of the Context of a MonoToFuture instance, but that would only help if you're guaranteed that external API doesn't wrap the Mono-produced Future, or change it in any way.

[dave-fl]

@simonbasle Yes using Caffeine Async Cache - which uses futures. I don't believe it changes the future as its just storing what its being provided.

[dave-fl]

Is there anything that can be done with the subscription to preserve the context?

[simonbasle]

Is there anything that can be done with the subscription to preserve the context?

How do you mean? Context is accessed by an operator's internal Subscriber by looking at its immediate downstream Subscriber. If there is a stopgap in the reactive chain and there's no such Subscriber, then the Context cannot be accessed.

[dave-fl]

@simonbasle I guess what I am asking you is that if one wanted to introduce their own Mono as I have done in this example, is there not a means to patch in that context so that chain is not broken e.g. do whatever it is that is normally done in the completion stage.

[simonbasle]

The sink has a currentContext() method so you could do sink -> context.subscriberContext(sink.currentContext()).subscribe(...).

That said, subscribe inside create is an anti-pattern to me, and it achieves nothing. Probably just an attempt at reproducing the issue, but it fails by nature and not as a byproduct of a bug. (bit like saying "this code throws a NPE: throw new NullPointerException();" 😄).

[sepanniemi]

The same problem seems to exist if I try to use SecurityContext in RxJava2 flow.

Single.fromPublisher(ReactiveSecurityContextHolder.getContext())

Is there any way to access the current security context if using RxJava data types in WebFlux RestController?

I tried also with RxJava2Adapter and with Mono.toFuture, as said above the context is visible only when staying purely in reactor flow.

[simonbasle]

Nope, the reactive metadata Context only works as long as the reactive sequence is made of Reactor operators

[sepanniemi]

I was able to transfer the SecurityContext to RxJava flow using reactor Hooks and custom operator which copies the SecurityContext from reactor context to ThreadLocal as Single created from Mono. Needs some more testing, but looks promising for some of our existing services heavily using RxJava and soon migrating to SpringBoot2.

[lee-gilbert]

Might this also provide a solution for retrieving the SecurityContext within Spring Data AuditorAware implementations?

[rwinch]

@lee-gilbert I'm not sure. Can you provide more information?

[lee-gilbert]

@rwinch It appears the issue is discussed without a solution here:
https://jira.spring.io/browse/DATACMNS-1231

[mgumerov]

Sorry, this might be a bit off topic but not by much because it questions if ReactiveSecurityContextHolder and Future are related at all.
So, in my project I subscribe to ReactiveSecurityContextHolder.getContext in a bean (i.e. the subscription outlives a single chain), like @bean Mono convertedPrincipal { return ReactiveSecurityContextHolder.getContext().map(...) }
Turns out that for every new request the map() gets called again. Meaning, that desprite getContext() being Mono<>, it emits multiple values to the subscriber - although each chain only observes one value.

Doesn't that violate a Mono<> protocol?
And anyway, clearly that is not compatible with Future<> is it?

[simonbasle]

(sounds indeed a bit off-topic and potentially a separate issue / question, but here goes)

emits multiple values to the subscriber - although each chain only observes one value.

these statements seem to contradict each other. are you subscribing to the Mono multiple times?

the Mono contract of at most one onNext signal is like similar Reactive Streams contracts: it is only true in the context of a single Subscription - ie. for a specific Subscriber-Publisher relationship. A Mono/Publisher is like a Subscription factory, multiple subscriptions can happen.

Bear in mind that some Mono will explicitly reject more than one subscription (e.g. Monos representing a transient source that can be only read once, like a unique response in a Network I/O).
Others will conceptually re-generate the source data ("cold" Monos - e.g. higher level "remote call" => perform the request again) and other will cache or just complete empty to late subscribers.

You should definitely not disconnect the getContext Mono from the request chain and make it a Bean IMHO. If you're already doing that, I assume you have implemented another kind of scope yourself, but it better be solid. You might have overlooked the potential cold and deferred nature of the getContext Mono (if someone from the @spring-projects/spring-security team can confirm it is cold).