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.

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

[3.7] Perform security checks on inherited endpoints before payload deserialization in the RESTEasy Reactive #38853

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
}
}
Loading