Skip to content

Commit

Permalink
Merge pull request #28820 from michalvavrik/feature/only-secure-endpo…
Browse files Browse the repository at this point in the history 
…ints

Apply additional JAX-RS security only for endpoints and not all resource methods
  • Loading branch information
@sberyozkin
sberyozkin authored Nov 2, 2022
2 parents 4aca253 + 5e670e4 commit 0506e81
Show file tree
Hide file tree
Showing 14 changed files with 405 additions and 150 deletions.
27 changes: 12 additions & 15 deletions .../deployment/src/main/java/io/quarkus/resteasy/deployment/RestPathAnnotationProcessor.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package io.quarkus.resteasy.deployment;

import java.util.List;
import java.util.Optional;
import java.util.regex.Pattern;

Expand Down Expand Up @@ -81,18 +80,15 @@ public void transform(TransformationContext ctx) {
MethodInfo methodInfo = target.asMethod();
ClassInfo classInfo = methodInfo.declaringClass();

AnnotationInstance annotation = methodInfo.annotation(REST_PATH);
if (annotation == null) {
// Check for @Path on class and not method
if (!isRestEndpointMethod(methodInfo.annotations())) {
return;
}
if (!isRestEndpointMethod(methodInfo)) {
return;
}
// Don't create annotations for rest clients
if (classInfo.classAnnotation(REGISTER_REST_CLIENT) != null) {
return;
}

AnnotationInstance annotation = methodInfo.annotation(REST_PATH);
StringBuilder stringBuilder;
if (annotation != null) {
stringBuilder = new StringBuilder(slashify(annotation.value().asString()));
Expand Down Expand Up @@ -138,17 +134,18 @@ String slashify(String path) {
return '/' + path;
}

boolean isRestEndpointMethod(List<AnnotationInstance> annotations) {
boolean isRestEndpointMethod = false;
static boolean isRestEndpointMethod(MethodInfo methodInfo) {

for (AnnotationInstance annotation : annotations) {
if (ResteasyDotNames.JAXRS_METHOD_ANNOTATIONS.contains(annotation.name())) {
isRestEndpointMethod = true;
break;
if (!methodInfo.hasAnnotation(REST_PATH)) {
// Check for @Path on class and not method
for (AnnotationInstance annotation : methodInfo.annotations()) {
if (ResteasyDotNames.JAXRS_METHOD_ANNOTATIONS.contains(annotation.name())) {
return true;
}
}
return false;
}

return isRestEndpointMethod;
return true;
}

private boolean notRequired(Capabilities capabilities,
Expand Down
33 changes: 18 additions & 15 deletions ...sy/deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyBuiltinsProcessor.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.quarkus.resteasy.deployment;

import static io.quarkus.deployment.annotations.ExecutionTime.STATIC_INIT;
import static io.quarkus.resteasy.deployment.RestPathAnnotationProcessor.isRestEndpointMethod;
import static io.quarkus.security.spi.SecurityTransformerUtils.hasSecurityAnnotation;

import java.util.ArrayList;
Expand All @@ -10,6 +11,7 @@

import org.jboss.jandex.ClassInfo;
import org.jboss.jandex.DotName;
import org.jboss.jandex.MethodInfo;

import io.quarkus.arc.deployment.AdditionalBeanBuildItem;
import io.quarkus.deployment.Capabilities;
Expand All @@ -36,7 +38,7 @@
import io.quarkus.resteasy.runtime.vertx.JsonObjectReader;
import io.quarkus.resteasy.runtime.vertx.JsonObjectWriter;
import io.quarkus.resteasy.server.common.deployment.ResteasyDeploymentBuildItem;
import io.quarkus.security.spi.AdditionalSecuredClassesBuildItem;
import io.quarkus.security.spi.AdditionalSecuredMethodsBuildItem;
import io.quarkus.vertx.http.deployment.HttpRootPathBuildItem;
import io.quarkus.vertx.http.deployment.devmode.NotFoundPageDisplayableEndpointBuildItem;
import io.quarkus.vertx.http.deployment.devmode.RouteDescriptionBuildItem;
Expand All @@ -51,30 +53,31 @@ public class ResteasyBuiltinsProcessor {
void setUpDenyAllJaxRs(CombinedIndexBuildItem index,
JaxRsSecurityConfig config,
ResteasyDeploymentBuildItem resteasyDeployment,
BuildProducer<AdditionalSecuredClassesBuildItem> additionalSecuredClasses) {
if ((config.denyJaxRs) && (resteasyDeployment != null)) {
final List<ClassInfo> classes = new ArrayList<>();
BuildProducer<AdditionalSecuredMethodsBuildItem> additionalSecuredClasses) {
if (resteasyDeployment != null && (config.denyJaxRs || config.defaultRolesAllowed.isPresent())) {
final List<MethodInfo> methods = new ArrayList<>();

// add endpoints
List<String> resourceClasses = resteasyDeployment.getDeployment().getScannedResourceClasses();
for (String className : resourceClasses) {
ClassInfo classInfo = index.getIndex().getClassByName(DotName.createSimple(className));
if (!hasSecurityAnnotation(classInfo)) {
classes.add(classInfo);
for (MethodInfo methodInfo : classInfo.methods()) {
if (isRestEndpointMethod(methodInfo) && !hasSecurityAnnotation(methodInfo)) {
methods.add(methodInfo);
}
}
}
}

additionalSecuredClasses.produce(new AdditionalSecuredClassesBuildItem(classes));
} else if (config.defaultRolesAllowed.isPresent() && resteasyDeployment != null) {
final List<ClassInfo> classes = new ArrayList<>();

List<String> resourceClasses = resteasyDeployment.getDeployment().getScannedResourceClasses();
for (String className : resourceClasses) {
ClassInfo classInfo = index.getIndex().getClassByName(DotName.createSimple(className));
if (!hasSecurityAnnotation(classInfo)) {
classes.add(classInfo);
if (!methods.isEmpty()) {
if (config.denyJaxRs) {
additionalSecuredClasses.produce(new AdditionalSecuredMethodsBuildItem(methods));
} else {
additionalSecuredClasses
.produce(new AdditionalSecuredMethodsBuildItem(methods, config.defaultRolesAllowed));
}
}
additionalSecuredClasses.produce(new AdditionalSecuredClassesBuildItem(classes, config.defaultRolesAllowed));
}
}

Expand Down
33 changes: 32 additions & 1 deletion ...loyment/src/test/java/io/quarkus/resteasy/test/security/DefaultRolesAllowedJaxRsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
import static io.restassured.RestAssured.given;
import static io.restassured.RestAssured.when;

import javax.annotation.security.RolesAllowed;
import javax.ws.rs.GET;
import javax.ws.rs.Path;

import org.hamcrest.Matchers;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
Expand All @@ -19,7 +24,7 @@ public class DefaultRolesAllowedJaxRsTest {
.addClasses(PermitAllResource.class, UnsecuredResource.class,
TestIdentityProvider.class,
TestIdentityController.class,
UnsecuredSubResource.class)
UnsecuredSubResource.class, HelloResource.class)
.addAsResource(new StringAsset("quarkus.security.jaxrs.default-roles-allowed = admin\n"),
"application.properties"));

Expand Down Expand Up @@ -65,6 +70,17 @@ public void shouldAllowPermitAllClass() {
assertStatus(path, 200, 200, 200);
}

@Test
public void testNonEndpointMethodAreNotDenied() {
// ensure io.quarkus.resteasy.test.security.DefaultRolesAllowedJaxRsTest.HelloResource.getHello is not secured with RolesAllowedInterceptor
given().auth().preemptive()
.basic("user", "user")
.get("/hello")
.then()
.statusCode(200)
.body(Matchers.equalTo("hello"));
}

private void assertStatus(String path, int adminStatus, int userStatus, int anonStatus) {
given().auth().preemptive()
.basic("admin", "admin").get(path)
Expand All @@ -80,4 +96,19 @@ private void assertStatus(String path, int adminStatus, int userStatus, int anon

}

@Path("/hello")
public static class HelloResource {

@RolesAllowed("**")
@GET
public String hello() {
return getHello();
}

public String getHello() {
return "hello";
}

}

}
32 changes: 31 additions & 1 deletion ...resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/DenyAllJaxRsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
import static io.restassured.RestAssured.when;
import static org.hamcrest.Matchers.emptyString;

import javax.annotation.security.PermitAll;
import javax.ws.rs.GET;
import javax.ws.rs.Path;

import org.hamcrest.Matchers;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
Expand All @@ -23,7 +28,7 @@ public class DenyAllJaxRsTest {
.addClasses(PermitAllResource.class, UnsecuredResource.class,
TestIdentityProvider.class,
TestIdentityController.class,
UnsecuredSubResource.class)
UnsecuredSubResource.class, HelloResource.class)
.addAsResource(new StringAsset("quarkus.security.jaxrs.deny-unannotated-endpoints = true\n"),
"application.properties"));

Expand Down Expand Up @@ -82,6 +87,16 @@ public void shouldAllowPermitAllClass() {
assertStatus(path, 200, 200);
}

@Test
public void testNonEndpointMethodAreNotDenied() {
// ensure io.quarkus.resteasy.test.security.DenyAllJaxRsTest.HelloResource.getHello is not secured with DenyAllInterceptor
given()
.get("/hello")
.then()
.statusCode(200)
.body(Matchers.equalTo("hello"));
}

private void assertStatus(String path, int status, int anonStatus) {
given().auth().preemptive()
.basic("admin", "admin").get(path)
Expand All @@ -97,4 +112,19 @@ private void assertStatus(String path, int status, int anonStatus) {

}

@Path("/hello")
public static class HelloResource {

@PermitAll
@GET
public String hello() {
return getHello();
}

public String getHello() {
return "hello";
}

}

}
48 changes: 29 additions & 19 deletions .../java/io/quarkus/resteasy/reactive/common/deployment/ResteasyReactiveCommonProcessor.java
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package io.quarkus.resteasy.reactive.common.deployment;

import static io.quarkus.security.spi.SecurityTransformerUtils.hasSecurityAnnotation;
import static org.jboss.resteasy.reactive.common.model.ResourceInterceptor.FILTER_SOURCE_METHOD_METADATA_KEY;
import static org.jboss.resteasy.reactive.common.processor.EndpointIndexer.collectClassEndpoints;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
Expand All @@ -18,7 +21,6 @@
import javax.ws.rs.ext.RuntimeDelegate;

import org.jboss.jandex.AnnotationTarget;
import org.jboss.jandex.AnnotationTarget.Kind;
import org.jboss.jandex.ClassInfo;
import org.jboss.jandex.CompositeIndex;
import org.jboss.jandex.DotName;
Expand Down Expand Up @@ -61,8 +63,7 @@
import io.quarkus.resteasy.reactive.spi.MessageBodyWriterOverrideBuildItem;
import io.quarkus.resteasy.reactive.spi.ReaderInterceptorBuildItem;
import io.quarkus.resteasy.reactive.spi.WriterInterceptorBuildItem;
import io.quarkus.security.spi.AdditionalSecuredClassesBuildItem;
import io.quarkus.security.spi.SecurityTransformerUtils;
import io.quarkus.security.spi.AdditionalSecuredMethodsBuildItem;

public class ResteasyReactiveCommonProcessor {

Expand All @@ -72,33 +73,42 @@ public class ResteasyReactiveCommonProcessor {
@BuildStep
void setUpDenyAllJaxRs(
CombinedIndexBuildItem index,
ResteasyReactiveConfig rrConfig,
JaxRsSecurityConfig securityConfig,
Optional<ResourceScanningResultBuildItem> resteasyDeployment,
BuildProducer<AdditionalSecuredClassesBuildItem> additionalSecuredClasses) {
BeanArchiveIndexBuildItem beanArchiveIndexBuildItem,
ApplicationResultBuildItem applicationResultBuildItem,
BuildProducer<AdditionalSecuredMethodsBuildItem> additionalSecuredClasses) {

if (securityConfig.denyJaxRs() && resteasyDeployment.isPresent()) {
List<ClassInfo> classes = new ArrayList<>();
if (resteasyDeployment.isPresent()
&& (securityConfig.denyJaxRs() || securityConfig.defaultRolesAllowed().isPresent())) {
final List<MethodInfo> methods = new ArrayList<>();
Map<DotName, String> httpAnnotationToMethod = resteasyDeployment.get().getResult().getHttpAnnotationToMethod();
Set<DotName> resourceClasses = resteasyDeployment.get().getResult().getScannedResourcePaths().keySet();

for (DotName className : resourceClasses) {
ClassInfo classInfo = index.getIndex().getClassByName(className);
if (!SecurityTransformerUtils.hasSecurityAnnotation(classInfo)) {
classes.add(classInfo);
if (!hasSecurityAnnotation(classInfo)) {
// collect class endpoints
Collection<MethodInfo> classEndpoints = collectClassEndpoints(classInfo, httpAnnotationToMethod,
beanArchiveIndexBuildItem.getIndex(), applicationResultBuildItem.getResult());

// add endpoints
for (MethodInfo classEndpoint : classEndpoints) {
if (!hasSecurityAnnotation(classEndpoint)) {
methods.add(classEndpoint);
}
}
}
}

additionalSecuredClasses.produce(new AdditionalSecuredClassesBuildItem(classes));
} else if (securityConfig.defaultRolesAllowed().isPresent() && resteasyDeployment.isPresent()) {
List<ClassInfo> classes = new ArrayList<>();
Set<DotName> resourceClasses = resteasyDeployment.get().getResult().getScannedResourcePaths().keySet();
for (DotName className : resourceClasses) {
ClassInfo classInfo = index.getIndex().getClassByName(className);
if (!SecurityTransformerUtils.hasSecurityAnnotation(classInfo)) {
classes.add(classInfo);
if (!methods.isEmpty()) {
if (securityConfig.denyJaxRs()) {
additionalSecuredClasses.produce(new AdditionalSecuredMethodsBuildItem(methods));
} else {
additionalSecuredClasses
.produce(new AdditionalSecuredMethodsBuildItem(methods, securityConfig.defaultRolesAllowed()));
}
}
additionalSecuredClasses
.produce(new AdditionalSecuredClassesBuildItem(classes, securityConfig.defaultRolesAllowed()));
}
}

Expand Down
12 changes: 11 additions & 1 deletion .../java/io/quarkus/resteasy/reactive/server/test/security/DefaultRolesAllowedJaxRsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static io.restassured.RestAssured.given;
import static io.restassured.RestAssured.when;

import org.hamcrest.Matchers;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
Expand All @@ -19,7 +20,7 @@ public class DefaultRolesAllowedJaxRsTest {
.addClasses(PermitAllResource.class, UnsecuredResource.class,
TestIdentityProvider.class,
TestIdentityController.class,
UnsecuredSubResource.class)
UnsecuredSubResource.class, HelloResource.class)
.addAsResource(new StringAsset("quarkus.security.jaxrs.default-roles-allowed=admin\n"),
"application.properties"));

Expand Down Expand Up @@ -77,6 +78,15 @@ public void shouldAllowPermitAllClass() {
assertStatus(path, 200, 200, 200);
}

@Test
public void testServerExceptionMapper() {
given()
.get("/hello")
.then()
.statusCode(200)
.body(Matchers.equalTo("unauthorizedExceptionMapper"));
}

private void assertStatus(String path, int adminStatus, int userStatus, int anonStatus) {
given().auth().preemptive()
.basic("admin", "admin").get(path)
Expand Down
Loading

0 comments on commit 0506e81

Please sign in to comment.