-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid SecurityContextOverrideHandler NPE when user provided custom JAX-RS security context and Quarkus Security is not present #43425
Conversation
Status for workflow
|
I'm using, still getting the error, 2024-10-11 05:11:05,055 ERROR [com.sli.mic.api.map.CommonExceptionMapper] (vert.x-eventloop-thread-3) Failed to handle request [Error Occurred After Shutdown]: java.lang.NullPointerException: Cannot read field "identityAssociation" because "io.quarkus.resteasy.reactive.server.runtime.security.EagerSecurityContext.instance" is null |
Milestone next to this PR says "3.16" and you are using 3.15.1. When the PR gets backported, you will see the milestone changing. |
We always add
SecurityContextOverrideHandler
quarkus/extensions/resteasy-reactive/rest/deployment/src/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java
Line 1344 in 78ff380
But we only add security integration when the Quarkus Security extension is present
quarkus/extensions/resteasy-reactive/rest/deployment/src/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java
Line 1587 in 78ff380
It won't cause any NPE when user doesn't do any security, because we only request
SecurityIdentityAssociation
when user implemented custom JAX-RS security context. However if user does implement custom JAX-RS security context and no Quarkus Security is present, NPE could happen that is fixed by this PR.What is the point of keeping
SecurityContextOverrideHandler
without Quarkus Security? IMO it's extremely edge case, but I have seen before one reproducer where user did all the security himself and relied strictly on JAX-RS concepts. No point stopping them. I have decided to keep this handler becauseCurrentIdentityAssociation
is a separate API from the Quarkus Security and it is always possible that some users decided to provide it themselves, so I don't want to break things.Ad testing - this could only be tested without Quarkus Security in the test scope, so I only tried it locally.