Fix for - ReactiveSecurityContextHolder is empty inside @QueryHandler in Spring WebFlux applications #4207

[taniyanandi]

No description provided.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

---

Taniya Nandi seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[taniyanandi]

I have read the CLA Document and I hereby sign the CLA, @AxonFramework I'm unable to see the "I agree" button on the CLA page despite being logged in. Can you assist?

[abuijze]

Apparently, the email address used for the commit doesn't match one of the email addresses registered on your github account. Can you make sure those match?

[smcvb]

I have a pointer to share besides the signing. I am not convinced yet that this should be core AF logic. Wouldn't this be a detail specific for the Reactor Extension that we still need to port to AF5?

[taniyanandi]

@smcvb , Thank you for the feedback! You're right, putting ContextView and Mono.deferContextual() directly in DefaultQueryGateway and SimpleQueryBus introduces a Reactor dependency in core AF which is incorrect. I'll refactor this to introduce a neutral DispatchContextContributor SPI in core that the Reactor Extension can implement, keeping all Reactor-specific logic out of core. Is this the direction you had in mind, or is there an existing extension mechanism I should be using instead?

[smcvb]

@taniyanandi, be sure to check what's happening in this PR. This was provided to use yesterday, and contains the base for this reactor extension port! Anything you'd provide for Reactor specifics would benefit from that PR being merged, I think!

[taniyanandi]

@smcvb Thank you for pointing that out! I've gone through the PR and it's great to see the Reactor extension being ported into AF5, that's exactly the right home for this fix. I'll wait for that PR to merge and then rebase my Reactor Context propagation fix on top of DefaultReactiveQueryGateway.

[theoema]

@taniyanandi @smcvb Is propagating the context all the way into the handlers really something we want to introduce to the Reactor extension? If so I can add that in now that I am working on it. But I think a much simpler solution, thats also more aligned is simply to register a dispatch interceptor that will on Message dispatch take the ReactiveSecurityContext and attach the subject inside the Message Metadata.

[smcvb]

@taniyanandi @smcvb Is propagating the context all the way into the handlers really something we want to introduce to the Reactor extension? If so I can add that in now that I am working on it. But I think a much simpler solution, thats also more aligned is simply to register a dispatch interceptor that will on Message dispatch take the ReactiveSecurityContext and attach the subject inside the Message Metadata.

I'd wager some handler interceptor that upon entering the message handling thread repopulates the ReactiveSecurityContext would be beneficial.
That would allow users for Project Reactor to use (custom) security annotations on their command/query handlers, as is currently supported if you're outside of a Reactor Context.

I don't think this change strictly needs to happen in the PR you're working on, @theoema.
To be frank, from a reviewers perspective, smaller PRs are a lot more helpful. The Axon Framework team has a lot of moving parts and minimizing the scope of PRs is a rule we try to adhere to internally as well.
So, if you, or @taniyanandi is up for it, I'd make a branch off of the branch you made that introduces the Reactor extension.

[taniyanandi]

@smcvb , The dispatch + handler interceptor approach makes much more sense! I'd love to take this on. Should I branch off @theoema's branch or wait for it to merge into axon-5.0.x first?

[smcvb]

Should I branch off @theoema's branch or wait for it to merge into axon-5.0.x first?

Feel free what works for you, @taniyanandi! I am fine if you wait, but similarly fine if you make a branch from his branch.

[smcvb]

@taniyanandi, #4245 has just been merged into main. This means you'd be able to port any ReactiveSecurityContextHolder specifics inside of that extension :-)