From 37834cb44a0bc5101cdf60922c8dd0301efac822 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Vav=C5=99=C3=ADk?= <mvavrik@redhat.com> Date: Sat, 17 Feb 2024 13:19:08 +0100 Subject: [PATCH] Perform security checks eagerly in RR on inherited endpoints --- .../test/security/DenyAllJaxRsTest.java | 33 +++++++++++++++++++ .../security/UnsecuredParentResource.java | 8 +++++ .../test/security/UnsecuredResource.java | 12 +++++++ .../security/UnsecuredResourceInterface.java | 24 ++++++++++++++ .../security/EagerSecurityHandler.java | 2 +- .../processor/ServerEndpointIndexer.java | 1 + .../startup/RuntimeResourceDeployment.java | 2 +- .../server/model/ServerResourceMethod.java | 9 +++++ .../spi/ResteasyReactiveResourceInfo.java | 18 +++++++++- 9 files changed, 106 insertions(+), 3 deletions(-) diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DenyAllJaxRsTest.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DenyAllJaxRsTest.java index 76f23ddcc8056..7c394c783545e 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DenyAllJaxRsTest.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/DenyAllJaxRsTest.java @@ -91,6 +91,39 @@ public void shouldDenyUnannotatedOnParentClass() { assertStatus(path, 403, 401); } + @Test + public void shouldAllowAnnotatedParentEndpoint() { + // the endpoint has @RolesAllowed, therefore default JAX-RS security should not be applied + String path = "/unsecured/parent-annotated"; + assertStatus(path, 200, 401); + } + + @Test + public void shouldAllowAnnotatedEndpointOnInterface() { + // the endpoint has @RolesAllowed, therefore default JAX-RS security should not be applied + String path = "/unsecured/interface-annotated"; + assertStatus(path, 200, 401); + } + + @Test + public void shouldDenyUnannotatedOverriddenOnInterfaceImplementor() { + // @RolesAllowed on interface, however implementor overridden the endpoint method with @Path @GET + String path = "/unsecured/interface-overridden-declared-on-implementor"; + assertStatus(path, 403, 401); + } + + @Test + public void shouldAllowAnnotatedOverriddenEndpointDeclaredOnInterface() { + // @RolesAllowed on interface and implementor didn't declare endpoint declaring annotations @GET + String path = "/unsecured/interface-overridden-declared-on-interface"; + assertStatus(path, 200, 401); + // check that response comes from the overridden method + given().auth().preemptive() + .basic("admin", "admin").get(path) + .then() + .body(Matchers.is("implementor-response")); + } + @Test public void shouldDenyUnannotatedOnInterface() { String path = "/unsecured/defaultSecurityInterface"; diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredParentResource.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredParentResource.java index 8250d5a9bf9a6..d92e24a57d685 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredParentResource.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredParentResource.java @@ -1,5 +1,6 @@ package io.quarkus.resteasy.reactive.server.test.security; +import jakarta.annotation.security.RolesAllowed; import jakarta.ws.rs.GET; import jakarta.ws.rs.Path; @@ -11,4 +12,11 @@ public String defaultSecurityParent() { return "defaultSecurityParent"; } + @RolesAllowed({ "admin", "user" }) + @GET + @Path("/parent-annotated") + public String parentAnnotated() { + return "parent-annotated"; + } + } diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResource.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResource.java index 82622c3e585de..0a7cf03414d81 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResource.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResource.java @@ -57,4 +57,16 @@ public UnsecuredSubResource sub() { public UnsecuredSubResource permitAllSub() { return new UnsecuredSubResource(); } + + @Override + public String interfaceOverriddenDeclaredOnInterface() { + return "implementor-response"; + } + + @GET + @Path("/interface-overridden-declared-on-implementor") + @Override + public String interfaceOverriddenDeclaredOnImplementor() { + return "implementor-response"; + } } diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResourceInterface.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResourceInterface.java index 7be652f15ef8d..d08cedd9f40fb 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResourceInterface.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/security/UnsecuredResourceInterface.java @@ -1,5 +1,6 @@ package io.quarkus.resteasy.reactive.server.test.security; +import jakarta.annotation.security.RolesAllowed; import jakarta.ws.rs.GET; import jakarta.ws.rs.Path; @@ -11,4 +12,27 @@ default String defaultSecurityInterface() { return "defaultSecurityInterface"; } + @RolesAllowed({ "admin", "user" }) + @GET + @Path("/interface-annotated") + default String interfaceAnnotated() { + return "interface-annotated"; + } + + @RolesAllowed({ "admin", "user" }) + @GET + @Path("/interface-overridden-declared-on-interface") + default String interfaceOverriddenDeclaredOnInterface() { + // this interface is overridden without @GET and @Path + return "interface-overridden-declared-on-interface"; + } + + @RolesAllowed({ "admin", "user" }) + @GET + @Path("/interface-overridden-declared-on-implementor") + default String interfaceOverriddenDeclaredOnImplementor() { + // this interface is overridden with @GET and @Path + return "interface-overridden-declared-on-implementor"; + } + } diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/security/EagerSecurityHandler.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/security/EagerSecurityHandler.java index f94c369070cf0..0285e4a1a2ad3 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/security/EagerSecurityHandler.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/security/EagerSecurityHandler.java @@ -187,7 +187,7 @@ private static Map<String, Object> createEventPropsWithRoutingCtx(ResteasyReacti } static MethodDescription lazyMethodToMethodDescription(ResteasyReactiveResourceInfo lazyMethod) { - return new MethodDescription(lazyMethod.getResourceClass().getName(), + return new MethodDescription(lazyMethod.getActualDeclaringClassName(), lazyMethod.getName(), MethodDescription.typesAsStrings(lazyMethod.getParameterTypes())); } diff --git a/independent-projects/resteasy-reactive/server/processor/src/main/java/org/jboss/resteasy/reactive/server/processor/ServerEndpointIndexer.java b/independent-projects/resteasy-reactive/server/processor/src/main/java/org/jboss/resteasy/reactive/server/processor/ServerEndpointIndexer.java index 5044f7eb4b3ee..07fb3fba149d4 100644 --- a/independent-projects/resteasy-reactive/server/processor/src/main/java/org/jboss/resteasy/reactive/server/processor/ServerEndpointIndexer.java +++ b/independent-projects/resteasy-reactive/server/processor/src/main/java/org/jboss/resteasy/reactive/server/processor/ServerEndpointIndexer.java @@ -183,6 +183,7 @@ protected ServerResourceMethod createResourceMethod(MethodInfo methodInfo, Class } } serverResourceMethod.setHandlerChainCustomizers(methodCustomizers); + serverResourceMethod.setActualDeclaringClassName(methodInfo.declaringClass().name().toString()); return serverResourceMethod; } diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/startup/RuntimeResourceDeployment.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/startup/RuntimeResourceDeployment.java index 455355d6a1521..e1bb21511e916 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/startup/RuntimeResourceDeployment.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/startup/RuntimeResourceDeployment.java @@ -189,7 +189,7 @@ public RuntimeResource buildResourceMethod(ResourceClass clazz, ResteasyReactiveResourceInfo lazyMethod = new ResteasyReactiveResourceInfo(method.getName(), resourceClass, parameterDeclaredUnresolvedTypes, classAnnotationNames, method.getMethodAnnotationNames(), - !defaultBlocking && !method.isBlocking()); + !defaultBlocking && !method.isBlocking(), method.getActualDeclaringClassName()); RuntimeInterceptorDeployment.MethodInterceptorContext interceptorDeployment = runtimeInterceptorDeployment .forMethod(method, lazyMethod); diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/model/ServerResourceMethod.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/model/ServerResourceMethod.java index 9c8620e9da341..e56dd9e8836f3 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/model/ServerResourceMethod.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/model/ServerResourceMethod.java @@ -18,6 +18,7 @@ public class ServerResourceMethod extends ResourceMethod { private List<HandlerChainCustomizer> handlerChainCustomizers = new ArrayList<>(); private ParameterExtractor customerParameterExtractor; + private String actualDeclaringClassName; public ServerResourceMethod() { } @@ -70,4 +71,12 @@ public ServerResourceMethod setCustomerParameterExtractor(ParameterExtractor cus this.customerParameterExtractor = customerParameterExtractor; return this; } + + public String getActualDeclaringClassName() { + return actualDeclaringClassName; + } + + public void setActualDeclaringClassName(String actualDeclaringClassName) { + this.actualDeclaringClassName = actualDeclaringClassName; + } } diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/spi/ResteasyReactiveResourceInfo.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/spi/ResteasyReactiveResourceInfo.java index 5e8b3377e4d77..014668cab4ebe 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/spi/ResteasyReactiveResourceInfo.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/spi/ResteasyReactiveResourceInfo.java @@ -26,21 +26,33 @@ public class ResteasyReactiveResourceInfo implements ResourceInfo { * If it's non-blocking method within the runtime that won't always default to blocking */ public final boolean isNonBlocking; - + /** + * This class name will only differ from {@link this#declaringClass} name when the {@link this#method} was inherited. + */ + private final String actualDeclaringClassName; private volatile Annotation[] classAnnotations; private volatile Method method; private volatile Annotation[] annotations; private volatile Type returnType; private volatile String methodId; + @Deprecated public ResteasyReactiveResourceInfo(String name, Class<?> declaringClass, Class[] parameterTypes, Set<String> classAnnotationNames, Set<String> methodAnnotationNames, boolean isNonBlocking) { + this(name, declaringClass, parameterTypes, classAnnotationNames, methodAnnotationNames, isNonBlocking, + declaringClass.getName()); + } + + public ResteasyReactiveResourceInfo(String name, Class<?> declaringClass, Class[] parameterTypes, + Set<String> classAnnotationNames, Set<String> methodAnnotationNames, boolean isNonBlocking, + String actualDeclaringClassName) { this.name = name; this.declaringClass = declaringClass; this.parameterTypes = parameterTypes; this.classAnnotationNames = classAnnotationNames; this.methodAnnotationNames = methodAnnotationNames; this.isNonBlocking = isNonBlocking; + this.actualDeclaringClassName = actualDeclaringClassName; } public String getName() { @@ -119,4 +131,8 @@ public String getMethodId() { } return methodId; } + + public String getActualDeclaringClassName() { + return actualDeclaringClassName; + } }