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.