AxonFramework / AxonFramework

Framework for Evolutionary Message-Driven Microservices on the JVM
https://axoniq.io/
Apache License 2.0
3.32k stars 790 forks source link

`SecurityContext` lost when using `AxonServerCommandBus` with `DelegatingSecurityContextExecutorService` #3115

Closed CptCheesebeard closed 1 month ago

CptCheesebeard commented 1 month ago

Basic information

Steps to reproduce

Check linked repository for example implementation.

Expected behaviour

Configured `DelegatingSecurityContextExecutorService` should work with AxonServerCommand-/AxonServerQuery-Bus and delegate the context to the returned Future.

SimpleQueryBus or DefaultQueryGateway should have some way to specify the used Executor

Actual behaviour

When using a CompletableFuture, the SecurityContext is lost in the `thenApply` function (or others). Default behavior so far.
. It is possible to use the `DelegatingSecurityContextExecutorService` to solve this problem.

When using Axon Command- or Query-Gateway, the same issue comes up. In case of the CommandGateway, it is possible to configure a `AsynchronousCommandBus` and add the `DelegatingSecurityContextExecutorService` to solve the issue, but when AxonServer is used, there is no solution to this problem.

In case of the QueryBus, there is no `AsynchronousCommandBus` equivalent that could be used when not using AxonServer.

smcvb commented 1 month ago

First and foremost, thanks for filing this issue with us, @CptCheesebeard!

Secondly, I have a hunch this issue can be solved rather easily. Have you seen the ExecutorServiceBuilder configuration option on the AxonServerCommandBus and AxonServerQueryBus?

The ExecutorServiceBuilder is a functional interface that provides you the required building blocks to construct an ExecutorService that adheres to the requirements of the AxonServerCommandBus and AxonServerQueryBus. If you want to configure a custom ExecutorServiceBuilder you will be required to provide your own instances of the AxonServerCommandBus and/or AxonServerQueryBus by using the Builder#executorServiceBuilder(ExecutorServiceBuilder) method.

By setting this property and having it return the DelegatingSecurityContextExecutorService (taking the given AxonServerConfiguration and using the BlockingQueue<Runnable> as the workQueue of the executor), I am pretty certain you should be good.

So, please tell us whether this would solve your predicament, @CptCheesebeard! If that's the case, I think we can close this issue as resolved.

CptCheesebeard commented 1 month ago

Hi @smcvb, thanks for your reply.

Yes, i saw the ExecutorServiceBuilder. My provided example is using this to demonstrate the problem. https://github.com/CptCheesebeard/axon-security-context/blob/main/src/test/java/cpt/cheesbeard/axonsecuritycontextdemo/command/ServerContextDelegatingCommandBusTest.java

The bean configuration is an exact copy of those defined in the spring-boot-starter project. I added comments so my changes can easily be detected.

As i have seen in the default implementation, AxonServerCommandBus uses a qualified CommandBus bean with name localSegment. I also created a custom bean to make it use DelegatingSecurityContextExecutorService as well. But without success.

smcvb commented 1 month ago

Thanks for expanding here, @CptCheesebeard. My first reply was purely based on your text and what I know of Axon Framework.

As I didn't see a clear mention of the ExecutorServiceBuilder, I assumed you weren't using this My bad for making that wrong assumption.

However, now I have checked your sample project and investigated the DelegatingSecurityContextExecutorService. I think your hoped-for solution doesn't work because Axon Server is in the middle.

Differently put, the command is passed on the Axon Server, which then returns it to an Axon Framework instance. A DelegatingSecurityContextExecutorService wouldn't automatically know how to set the current SecurityContext based on the message it receives as it went outside the JVM. The "local context" version that only uses the AsynchronousCommandBus thus works because it still stays within the same JVM.

To pass along the SecurityContext between message dispatching and handling, both in a local and distributed environment, I have always used Axon Framework's MessageDispatchInterceptor and MessageHandlerInterceptor. The MessageDispatchInterceptor would add information to the Message's MetaData, which ensures that the MessageHandlerInterceptor is able to reconstruct and set the SecurityContext upon receiving the message to handle.

Granted, this solution goes beyond the use of the DelegatingSecurityContextExecutorService entirely. Nonetheless, would you mind given that approach a try? Supporting the DelegatingSecurityContextExecutorService, if anything, would be an enhancement/feature request. Marking it as a bug would suggest Axon Framework promoted the use of the DelegatingSecurityContextExecutorService somewhere in the documentation or on the forum, which isn't the case as far as I know.

Note that doesn't mean we can't leave the idea as a feature request, of course. It mainly changes the priority of this. Especially so if the dispatch-and-handler-interceptor solution would work in your scenario too.

If you need some help with drafting the interceptor-approach, be sure to say so! I should be able to find an internal sample somewhere.

CptCheesebeard commented 1 month ago

Thanks for having another look at my issue @smcvb. Maybe my explanation could have been a bit better.

As AxonServerCommandBus provides a way to add a ExecutorService i thought it could be a problem with the implementation. I tried already to figure this out by myself, but after a few layers within the code it is easy to get lost :laughing:

In our use-case the SecurityContext is indirectly needed for follow up calls. Like, query some data first and use the result for a follow up command. And all of that non-blocking. For now the easiest thing is to have a little workaround and simply store the context in a variable between the calls and set it again before the next call. Not too big of a problem, but something that can easily be missed during development, as we are testing with "local context" and Axon Server is only used in production. On the other hand this issue is really easy to detect when it runs on production, so it is not too concerning for me.

I will have a look into the possibilities of the MessageDispatchInterceptor/MessageHandlerInterceptor combo. We are using the interceptor already to extract some needed information from the security context and add it as MetaData. If we can achieve it in some way, that the MetaData is "reused" for every follow up command, we get what we actually want to achieve. The SecurityContext is not really necessary, but the MetaData we excract out of it for the commands/queries is.

I got the information i needed, thank you very much for your time! I will close this bug.

smcvb commented 1 month ago

Happy to hear you have a path forward, @CptCheesebeard!

If switching your solution towards the suggested interceptor approach does not turn out as expected, be sure to open up this thread again. As stated, although I don't think it's necessarily a bug in Axon Framework, it might very well make sense as an enhancement for Axon Framework 4.