Skip to content

Commit

Permalink
Perform security checks eagerly in RR on inherited endpoints
Browse files Browse the repository at this point in the history
  • Loading branch information
michalvavrik committed Feb 19, 2024
1 parent 1dbf5d3 commit 37834cb
Show file tree
Hide file tree
Showing 9 changed files with 106 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -11,4 +12,11 @@ public String defaultSecurityParent() {
return "defaultSecurityParent";
}

@RolesAllowed({ "admin", "user" })
@GET
@Path("/parent-annotated")
public String parentAnnotated() {
return "parent-annotated";
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
}
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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";
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ protected ServerResourceMethod createResourceMethod(MethodInfo methodInfo, Class
}
}
serverResourceMethod.setHandlerChainCustomizers(methodCustomizers);
serverResourceMethod.setActualDeclaringClassName(methodInfo.declaringClass().name().toString());
return serverResourceMethod;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ public class ServerResourceMethod extends ResourceMethod {

private List<HandlerChainCustomizer> handlerChainCustomizers = new ArrayList<>();
private ParameterExtractor customerParameterExtractor;
private String actualDeclaringClassName;

public ServerResourceMethod() {
}
Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -119,4 +131,8 @@ public String getMethodId() {
}
return methodId;
}

public String getActualDeclaringClassName() {
return actualDeclaringClassName;
}
}

0 comments on commit 37834cb

Please sign in to comment.