SecurityIdentityAssociation set in a messaging context is not available in outgoing rest calls

[mrickly]

Describe the bug

We have a KafkaIncomingDecorator implements PublisherDecorator where we read a jwt token from the message headers that is then used to set the CurrentIdentityAssociation:

               currentIdentityAssociation.setIdentity(identityProviderManager.authenticate(
                       new TrustedTokenAuthenticationRequest(new JsonWebTokenCredential(jwt))));

We inject the CurrentIdentityAssociation in a SecurityIdentityPropagationFilter implements ResteasyReactiveClientRequestFilter. After a framework update, some users have reported that the injected CurrentIdentityAssociation is now anonymous (i.e. it is not the identity set in the decorator).

As a provisional fix, we have added the jwt to the ContextLocals and we read it from there in the filter when the identity is anonymous.

Expected behavior

The jwt token set in the decorator is available via CurrentIdentityAssociation in the filter when the message processing performs an outgoing rest call.

Actual behavior

The CurrentIdentityAssociation injected in the filter is anonymous.

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

openjdk 21.0.1 2023-10-17 LTS OpenJDK Runtime Environment Zulu21.30+16-SA (build 21.0.1+12-LTS) OpenJDK 64-Bit Server VM Zulu21.30+16-SA (build 21.0.1+12-LTS, mixed mode, sharing)

Quarkus version or git rev

3.6.4

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.9.2

Additional information

Will try to provide a small reproducer in the coming days. It is currently not entirely clear whether there were also changes in the user application that might have impacted the behaviour of our code.

[quarkus-bot]

/cc @sberyozkin (security)

[sberyozkin]

@michalvavrik Hey Michal, can you have a look in the next week or so, I wonder if it may be related to the vertx contexts. Though I'm not sure how it works with the outgoing calls since the identity association represents the incoming call...

[michalvavrik]

sure @sberyozkin I'll have a look when @mrickly provides reproducer as mentioned in Additional information

[sberyozkin]

Thanks @michalvavrik, yeah, we have discussed it with @mrickly. @mrickly, something simple, may be even without having to have Kafka involved will do, may be JAX-RS container request filter picking up a bearer access token from HTTP authorization header, without the security enabled in the frontend and then the frontend endpoint attempting to propagate that to the downstream test endpoint using REST client

[mrickly]

@sberyozkin : I doubt that I can reproduce it in that simple scenario (without messaging involved), unless you're asking for a positive example (where it works).

[michalvavrik]

@mrickly debugging this without reproducer is not feasible, if simple scenario can't be done, just provide a reproducer and I'll try to understand / simplify, let's see. Thank you

[mrickly]

Hi @sberyozkin , @michalvavrik : I invested some time and what I have experienced feels unreal. Sometimes both injection point (Kafka Incoming Filter and Rest Outgoing Filter) will get the same instance of a SecurityIdentityAssociation, sometimes they won't. It apparently matters whether I run or debug the test (even without active breakpoints) in the IDE. Should I expect quantum effects in Quarkus? I have found a thread discussing an almost identical scenario: #21367
The statement from @cescoffier is pretty clear: "The rule is simple: do not use the request scope when dealing with Kafka messages. It will not do what you expect it do to." The issue from our point of view is that we have to consider two different objects when we want to propagate identity: CurrentIdentityAssociation and ContextLocals. A unified mechanism for transporting the user identity would be nice.

[michalvavrik]

I can provide no help without seeing actual code and ideally reproducer. I am not able to make any conclusions or give suggestions based on your comments. I'll let others help you, maybe your description will be sufficient for them. Cheers

[mrickly]

@michalvavrik : I understand that. The problem was not reproducible when getting rid of all superfluous libraries, including our own. I will of course update you if I manage to reproduce this apparent randomness in a simple setting.

[michalvavrik]

@michalvavrik : I understand that. The problem was not reproducible when getting rid of all superfluous libraries, including our own. I will of course update you if I manage to reproduce this apparent randomness in a simple setting.

np @mrickly , there is a chance @cescoffier could answer you in regards on CDI request scope based on your comments when he finds a time. I just wanted to make it clear I can't, take your time.

[mrickly]

glu-test-reproducer2-service.zip
@michalvavrik : I uploaded this reproducer. The example can very likely be simplified, but I wanted to keep some similarity with our usage. At the minimum, two incoming channels are required for the reproducer. The test sends a message to
channel1 only (channel2 receives no message). A jwt token is read from the message header and should be passed to an outgoing rest call. This will sometimes work and sometimes won't. Whether it works depends on the order in which the RequestContextStates associated to the channels via an ActivateRequestContext annotated KafkaIncomingDecorator are created at startup:

1. If RequestContextState1 associated with channel1 is instantiated first, it is used throughout without issues.
2. If the RequestContextState2 associated with channel2 is instantiated first, it will be used throughout, except when the message is received by channel1, at which point RequestContextState1 is activated, used to store the identity and then deactivated again. When the rest call is performed, the rest call filter tries to read the identity from RequestContextState2 (and the outgoing call has no x-identity header).

What I find very strange is that the first RequestContextState to be created will be used globally, for instance whenever the poll method of any kafka consumer thread executes. I wonder whether this behaviour is related to the bug mentioned by @cescoffier in this comment #21367 (comment). It looks wrong to me, even if RequestContext and messaging are not supposed to be used together (which should be documented somewhere).

[michalvavrik]

oki, thanks, I'll check next week

[michalvavrik]

just update, I've had other more urgent tasks, but it's still on my radar. I'll check and let you know when I can.

[michalvavrik]

After little adjustments to your reproducer I was able to reproduce it, indeed it's as described in README.

My conclusion is that:

1. CDI request scope can be used with the Kafka as it is in the Kafka reference https://quarkus.io/guides/kafka#persisting-kafka-messages-with-hibernate-reactive, BUT: you use it as if it was a scope of the Message, which is not how it currently works.
2. If 1. is true, then the way you use it can provide random results when consuming many concurrent messages
3. You can consume Message get payload from it and propagate JWT via Kafka metadata via REST client headers

Hey @cescoffier @ozangunalp , please answer following question because at the very least I can't see this documented (and if I am wrong, answer is as easy as pointing me to the docs section, thanks!):

I need to understand expected behavior, https://quarkus.io/guides/kafka doesn't explain what is expected in regards to the CDI request scope. When @ActivateRequestContext is applied it stores state on the existing context so it can't be safe if the ch.mobi.glu.test.reproducer.messaging.KafkaIncomingDecorator#decorate is not invoked from the duplicated context which has scope of one message. Is the only safe way to propagate metadata in a scope of Kafka message to use Kafka metadata?

[ozangunalp]

It is generally wrong to put @ActivateRequestContext on an arbitrary method and expect things to work, especially when you need want to propagate information between request scoped beans.

For the CDI interceptor to be active the call needs to be made on an injected bean, internal method calls won't get intercepted. Furthermore activating request scope on the KafkaIncomingDecorator#decorate makes no sense as the information retrieval happens on the passed stream's map operation.

BUT, the map operations on that stream will be called on the message's duplicated context so this is effectively what you want.

What was described here is a workaround to make sure a Hibernate Session is available for DB operations, and it works because the request scope and message processing are both terminated at the end of that method call.

We've been holding back on treating a message processing as a "request" (because it isn't) and therefore only providing workarounds for the usage of request scoped beans for message processing, like @ActivateRequestContext. For now, the workaround for your case is using ContextLocals. I haven't tested it with SecurityIdentityAssociation but in theory, it should work.

Going forward, I am planning to add a config flag to be able to opt-in to activate the request scope on messages. It'll also use PublisherDecorator as you did. But I need to find more test cases – like this one to validate the solution.

cc @cescoffier @sberyozkin

[mrickly]

@michalvavrik , @ozangunalp : Thanks for confirming. We are indeed relying solely on ContextLocals now to pass a jwt and have eliminated @ActivateRequestContext and CurrentIdentityAssociation from our decorators. Independently from the issue at hand, the behaviour described above (i.e. RequestContextState that seems to be rather global) looks suspicious to me and I wonder whether you also have an opinion on that.

[ozangunalp]

The PublisherDecorator#decorate method is called at startup time, on the main thread. When you have @ActivateRequestContext on that method you get the global request context state.
Otherwise, the request context state gets associated with the duplicated context when it is available.

[michalvavrik]

Thank you for the explanation. So I take it this is not supported scenario ATM, therefore not a bug.