Skip to content
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.

Sign up for GitHub

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

Jump to bottom

NPE during custom bean invocation via PreAuthorize annotation in Quarkus RESTEasy #44511

Closed
deidzen opened this issue Nov 14, 2024 · 16 comments · Fixed by #44667
Closed

NPE during custom bean invocation via PreAuthorize annotation in Quarkus RESTEasy #44511

deidzen opened this issue Nov 14, 2024 · 16 comments · Fixed by #44667
Labels
area/resteasy-classic env/windows Impacts Windows machines kind/bug Something isn't working
Milestone
3.17.2

Comments

@deidzen
Copy link

deidzen commented Nov 14, 2024

Describe the bug

According to documentation (https://quarkus.io/guides/spring-security#preauthorize), Quarkus supports custom bean invocation via PreAuthorize annotation before method executing

Image

But starting from 3.6.8 version (I personally checked 3.6.8, 3.9.5 and current 3.16.3), this functionality in Quarkus RESTEasy is not working, it throws NullPointerException.

I see that EagerSecurityFilter class mentioned in error stacktrace was changed in 3.6.8 version (6d76eaf#diff-fbf38214479c02a3e1bae50b5b595d13c12c28e64d34246398151ef3b521e5db). Maybe that caused the issue.

Expected behavior

Custom bean is invoked through @PreAuthorize annotation succesfully

Actual behavior

NPE is thrown during custom bean invocation

java.lang.NullPointerException: Cannot load from object array because "<parameter2>" is null
	at io.quarkus.spring.security.check.PersonChecker_2d2da3d8eb389fe1c70f7b93d91d35c2a8f20142_CheckFor_check.check(Unknown Source)
	at io.quarkus.spring.security.runtime.interceptor.check.AbstractBeanMethodSecurityCheck.doApply(AbstractBeanMethodSecurityCheck.java:30)
	at io.quarkus.spring.security.runtime.interceptor.check.AbstractBeanMethodSecurityCheck.apply(AbstractBeanMethodSecurityCheck.java:26)
	at io.quarkus.resteasy.runtime.EagerSecurityFilter.applySecurityChecks(EagerSecurityFilter.java:127)
	at io.quarkus.resteasy.runtime.EagerSecurityFilter.filter(EagerSecurityFilter.java:78)
	at org.jboss.resteasy.core.interception.jaxrs.PreMatchContainerRequestContext.filter(PreMatchContainerRequestContext.java:276)
	at org.jboss.resteasy.core.ResourceMethodInvoker.invokeOnTarget(ResourceMethodInvoker.java:415)
	at org.jboss.resteasy.core.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:378)
	at org.jboss.resteasy.core.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:356)
	at org.jboss.resteasy.core.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:70)
	at org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:429)
	at org.jboss.resteasy.core.SynchronousDispatcher.lambda$invokePropagateNotFound$6(SynchronousDispatcher.java:275)
	at org.jboss.resteasy.core.SynchronousDispatcher.lambda$preprocess$0(SynchronousDispatcher.java:154)
	at org.jboss.resteasy.core.interception.jaxrs.PreMatchContainerRequestContext.filter(PreMatchContainerRequestContext.java:321)
	at org.jboss.resteasy.core.SynchronousDispatcher.preprocess(SynchronousDispatcher.java:157)
	at org.jboss.resteasy.core.SynchronousDispatcher.invokePropagateNotFound(SynchronousDispatcher.java:260)
	at io.quarkus.resteasy.runtime.standalone.RequestDispatcher.service(RequestDispatcher.java:86)
	at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler.dispatch(VertxRequestHandler.java:151)
	at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler$1.run(VertxRequestHandler.java:97)
	at io.quarkus.vertx.core.runtime.VertxCoreRecorder$14.runWith(VertxCoreRecorder.java:627)
	at org.jboss.threads.EnhancedQueueExecutor$Task.doRunWith(EnhancedQueueExecutor.java:2675)
	at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2654)
	at org.jboss.threads.EnhancedQueueExecutor.runThreadBody(EnhancedQueueExecutor.java:1627)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1594)
	at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:11)
	at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:11)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:842)
Resulted in: org.jboss.resteasy.spi.UnhandledException: java.lang.NullPointerException: Cannot load from object array because "<parameter2>" is null
	at org.jboss.resteasy.core.ExceptionHandler.handleException(ExceptionHandler.java:357)
	at org.jboss.resteasy.core.SynchronousDispatcher.writeException(SynchronousDispatcher.java:205)
	at org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:452)
	... 17 more

How to Reproduce?

Reproducer: https://github.com/deidzen/quarkus-preauthorized-test

Steps to reproduce:

  1. Pull the reproducer.
  2. Run mvn clean test . 2 tests will fail with 500 error
    java.lang.NullPointerException: Cannot load from object array because "<parameter2>" is null

For compare you can switch to 3.6.7 Quarkus version (quarkus-3.6.7-resteasy branch) and try once again - tests will pass

Output of uname -a or ver

Microsoft Windows [Version 10.0.26100.2033]

Output of java -version

17.0.10, vendor: Oracle OpenJDK

Quarkus version or git rev

3.6.8+ (I personally checked 3.6.8, 3.9.5 and current 3.16.3)

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

Apache Maven 3.9.9 (8e8579a9e76f7d015ee5ec7bfcdc97d260186937)

Additional information

No response
@deidzen deidzen added the kind/bug Something isn't working label Nov 14, 2024
@quarkus-bot quarkus-bot bot added env/windows Impacts Windows machines triage/needs-triage labels Nov 14, 2024
@sberyozkin
Copy link
Member

@deidzen While this issue is being addressed, you may want to to have a look at @PermissionChecker in Quarkus 3.17.0.CR1:

https://quarkus.io/version/main/guides/security-authorize-web-endpoints-reference#permission-checker

We believe it offers a simpler alternative to @PreAuthorize

@michalvavrik
Copy link
Member

I see that EagerSecurityFilter class mentioned in error stacktrace was changed in 3.6.8 version (6d76eaf#diff-fbf38214479c02a3e1bae50b5b595d13c12c28e64d34246398151ef3b521e5db). Maybe that caused the issue.

We keep changing it, that is nothing out of ordinary. I will put this issue on my (extended) list.

@michalvavrik michalvavrik mentioned this issue Nov 23, 2024
Merged
@michalvavrik
Copy link
Member

So what has changed is that we now perform this security check eagerly (before serialization etc.). This should help #44667, but you can only do this for authenticated requests for postponing authorization for anonymous users is not supported in Quarkus.

@quarkus-bot quarkus-bot bot added this to the 3.18 - main milestone Nov 25, 2024
@gsmet gsmet modified the milestones: 3.18 - main, 3.17.1 Nov 27, 2024
@Kirill-Belko
Copy link

Could you please add this fix in 3.9.5 version?

@michalvavrik
Copy link
Member

Could you please add this fix in 3.9.5 version?

A great lot has changed in this implementation, I think it may require custom backport. Anyway, 3.9.x is not supported anymore, it's not LTS. The latest version is now 3.17 and the latest LTS is 3.15. See https://code.quarkus.io/ streams.

@geoand
Copy link
Contributor

geoand commented Dec 4, 2024

Exactly what @michalvavrik said.

Is there a reason why you have not moved up from 3.9?

@Kirill-Belko
Copy link

Is there another way instead of @PreAuthorize to call custom checker with parameters to check availability of endpoint in quarkus 3.9.5?

@deidzen
Copy link
Author

deidzen commented Dec 5, 2024

A great lot has changed in this implementation, I think it may require custom backport. Anyway, 3.9.x is not supported anymore, it's not LTS. The latest version is now 3.17 and the latest LTS is 3.15. See https://code.quarkus.io/ streams.

Please consider backporting this fix to 3.15

@michalvavrik
Copy link
Member

A great lot has changed in this implementation, I think it may require custom backport. Anyway, 3.9.x is not supported anymore, it's not LTS. The latest version is now 3.17 and the latest LTS is 3.15. See https://code.quarkus.io/ streams.

Please consider backporting this fix to 3.15

I was being careful because I didn't analyze how much Quarkus Spring Security had changed, but I think this fix is isolated and safe as long as tests will pass in backport CI. I'll propose it by adding respective label to the PR.

@deidzen
Copy link
Author

deidzen commented Dec 5, 2024

Thanks a lot!

@Kirill-Belko
Copy link

A great lot has changed in this implementation, I think it may require custom backport. Anyway, 3.9.x is not supported anymore, it's not LTS. The latest version is now 3.17 and the latest LTS is 3.15. See https://code.quarkus.io/ streams.

Please consider backporting this fix to 3.15

I was being careful because I didn't analyze how much Quarkus Spring Security had changed, but I think this fix is isolated and safe as long as tests will pass in backport CI. I'll propose it by adding respective label to the PR.

Could you please write when approximately the fix is planned to backport in 3.15 ?

@michalvavrik
Copy link
Member

Could you please write when approximately the fix is planned to backport in 3.15 ?

I'd expect it to be included in 3.15.3, considering 3.15.2 was released 3 weeks ago, I'd expect to see 3.15.3 in December. I have no internal release planning knowledge, as far as I know, only source of the truth is https://github.com/quarkusio/quarkus/wiki/Release-Planning. @gsmet will know more, but I am sure he is very busy.

@Kirill-Belko
Copy link

@gsmet, do you know when the fix is going to be backported in 3.15.3?

@gsmet
Copy link
Member

gsmet commented Dec 9, 2024

The next 3.15 LTS micro is currently planned for mid-January.

@Kirill-Belko
Copy link

The next 3.15 LTS micro is currently planned for mid-January.

@gsmet , are you going to backport this fix in 3.15 in the nearest time ?

@geoand
Copy link
Contributor

geoand commented Feb 10, 2025

That's the plan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/resteasy-classic env/windows Impacts Windows machines kind/bug Something isn't working
Projects
None yet
Milestone
3.17.2
6 participants
@sberyozkin @gsmet @geoand @Kirill-Belko @deidzen @michalvavrik