Skip to content

Commit

Permalink
Merge pull request #14563 from stuartwdouglas/rr-security-fixes
Browse files Browse the repository at this point in the history
Security fixes for RESTEasy Reactive
  • Loading branch information
gsmet authored Jan 26, 2021
2 parents c73723e + 0dbb280 commit e61bc27
Show file tree
Hide file tree
Showing 21 changed files with 648 additions and 5 deletions.
10 changes: 10 additions & 0 deletions bom/application/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2835,6 +2835,16 @@
<groupId>io.quarkus.security</groupId>
<artifactId>quarkus-security</artifactId>
<version>${quarkus-security.version}</version>
<exclusions>
<exclusion>
<groupId>javax.inject</groupId>
<artifactId>javax.inject</artifactId>
</exclusion>
<exclusion>
<groupId>javax.enterprise</groupId>
<artifactId>cdi-api</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.quarkus.resteasy.reactive.common.deployment;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
Expand All @@ -12,6 +13,7 @@

import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.ClassInfo;
import org.jboss.jandex.DotName;
import org.jboss.jandex.IndexView;
import org.jboss.jandex.Type;
import org.jboss.resteasy.reactive.common.model.InterceptorContainer;
Expand All @@ -34,16 +36,39 @@
import io.quarkus.deployment.builditem.CombinedIndexBuildItem;
import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
import io.quarkus.deployment.util.JandexUtil;
import io.quarkus.resteasy.reactive.common.runtime.JaxRsSecurityConfig;
import io.quarkus.resteasy.reactive.spi.AbstractInterceptorBuildItem;
import io.quarkus.resteasy.reactive.spi.ContainerRequestFilterBuildItem;
import io.quarkus.resteasy.reactive.spi.ContainerResponseFilterBuildItem;
import io.quarkus.resteasy.reactive.spi.MessageBodyReaderBuildItem;
import io.quarkus.resteasy.reactive.spi.MessageBodyWriterBuildItem;
import io.quarkus.resteasy.reactive.spi.ReaderInterceptorBuildItem;
import io.quarkus.resteasy.reactive.spi.WriterInterceptorBuildItem;
import io.quarkus.security.spi.AdditionalSecuredClassesBuildIem;
import io.quarkus.security.spi.SecurityTransformerUtils;

public class ResteasyReactiveCommonProcessor {

@BuildStep
void setUpDenyAllJaxRs(CombinedIndexBuildItem index,
JaxRsSecurityConfig config,
ResourceScanningResultBuildItem resteasyDeployment,
BuildProducer<AdditionalSecuredClassesBuildIem> additionalSecuredClasses) {
if (config.denyJaxRs) {
final List<ClassInfo> classes = new ArrayList<>();

Set<DotName> resourceClasses = resteasyDeployment.getResult().getScannedResourcePaths().keySet();
for (DotName className : resourceClasses) {
ClassInfo classInfo = index.getIndex().getClassByName(className);
if (!SecurityTransformerUtils.hasSecurityAnnotation(classInfo)) {
classes.add(classInfo);
}
}

additionalSecuredClasses.produce(new AdditionalSecuredClassesBuildIem(classes));
}
}

@BuildStep
ApplicationResultBuildItem handleApplication(CombinedIndexBuildItem combinedIndexBuildItem,
BuildProducer<ReflectiveClassBuildItem> reflectiveClass) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package io.quarkus.resteasy.reactive.common.runtime;

import io.quarkus.runtime.annotations.ConfigItem;
import io.quarkus.runtime.annotations.ConfigPhase;
import io.quarkus.runtime.annotations.ConfigRoot;

/**
* @author Michal Szynkiewicz, [email protected]
*/
@ConfigRoot(name = "security.jaxrs", phase = ConfigPhase.BUILD_AND_RUN_TIME_FIXED)
public class JaxRsSecurityConfig {
/**
* if set to true, access to all JAX-RS resources will be denied by default
*/
@ConfigItem(name = "deny-unannotated-endpoints")
public boolean denyJaxRs;
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@
import org.jboss.resteasy.reactive.server.model.ContextResolvers;
import org.jboss.resteasy.reactive.server.model.DynamicFeatures;
import org.jboss.resteasy.reactive.server.model.Features;
import org.jboss.resteasy.reactive.server.model.HandlerChainCustomizer;
import org.jboss.resteasy.reactive.server.model.ParamConverterProviders;
import org.jboss.resteasy.reactive.server.processor.scanning.MethodScanner;
import org.jboss.resteasy.reactive.spi.BeanFactory;

import io.quarkus.arc.deployment.AdditionalBeanBuildItem;
Expand Down Expand Up @@ -100,6 +102,7 @@
import io.quarkus.resteasy.reactive.server.runtime.exceptionmappers.AuthenticationRedirectExceptionMapper;
import io.quarkus.resteasy.reactive.server.runtime.exceptionmappers.ForbiddenExceptionMapper;
import io.quarkus.resteasy.reactive.server.runtime.exceptionmappers.UnauthorizedExceptionMapper;
import io.quarkus.resteasy.reactive.server.runtime.security.SecurityContextOverrideHandler;
import io.quarkus.resteasy.reactive.spi.CustomExceptionMapperBuildItem;
import io.quarkus.resteasy.reactive.spi.DynamicFeatureBuildItem;
import io.quarkus.resteasy.reactive.spi.ExceptionMapperBuildItem;
Expand Down Expand Up @@ -532,6 +535,16 @@ public void securityExceptionMappers(BuildProducer<ExceptionMapperBuildItem> exc
Priorities.USER + 1, false));
}

@BuildStep
MethodScannerBuildItem integrateSecurityOverrideSupport() {
return new MethodScannerBuildItem(new MethodScanner() {
@Override
public List<HandlerChainCustomizer> scan(MethodInfo method, Map<String, Object> methodContext) {
return Collections.singletonList(new SecurityContextOverrideHandler.Customizer());
}
});
}

private String determineApplicationPath(IndexView index) {
Collection<AnnotationInstance> applicationPaths = index.getAnnotations(ResteasyReactiveDotNames.APPLICATION_PATH);
if (applicationPaths.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package io.quarkus.resteasy.reactive.server.test.security;

import static io.restassured.RestAssured.given;
import static io.restassured.RestAssured.when;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.security.test.utils.TestIdentityController;
import io.quarkus.security.test.utils.TestIdentityProvider;
import io.quarkus.test.QuarkusUnitTest;

/**
* @author Michal Szynkiewicz, [email protected]
*/
public class DenyAllJaxRsTest {
@RegisterExtension
static QuarkusUnitTest runner = new QuarkusUnitTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClasses(PermitAllResource.class, UnsecuredResource.class,
TestIdentityProvider.class,
TestIdentityController.class,
UnsecuredSubResource.class)
.addAsResource(new StringAsset("quarkus.security.jaxrs.deny-unannotated-endpoints = true\n"),
"application.properties"));

@BeforeAll
public static void setupUsers() {
TestIdentityController.resetRoles()
.add("admin", "admin", "admin")
.add("user", "user", "user");
}

@Test
public void shouldDenyUnannotated() {
String path = "/unsecured/defaultSecurity";
assertStatus(path, 403, 401);
}

@Test
public void shouldDenyDenyAllMethod() {
String path = "/unsecured/denyAll";
assertStatus(path, 403, 401);
}

@Test
public void shouldPermitPermitAllMethod() {
assertStatus("/unsecured/permitAll", 200, 200);
}

@Test
public void shouldDenySubResource() {
String path = "/unsecured/sub/subMethod";
assertStatus(path, 403, 401);
}

@Test
public void shouldAllowPermitAllSubResource() {
String path = "/unsecured/permitAllSub/subMethod";
assertStatus(path, 200, 200);
}

@Test
public void shouldAllowPermitAllClass() {
String path = "/permitAll/sub/subMethod";
assertStatus(path, 200, 200);
}

private void assertStatus(String path, int status, int anonStatus) {
given().auth().preemptive()
.basic("admin", "admin").get(path)
.then()
.statusCode(status);
given().auth().preemptive()
.basic("user", "user").get(path)
.then()
.statusCode(status);
when().get(path)
.then()
.statusCode(anonStatus);

}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package io.quarkus.resteasy.reactive.server.test.security;

import static org.hamcrest.Matchers.is;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.security.test.utils.TestIdentityController;
import io.quarkus.security.test.utils.TestIdentityProvider;
import io.quarkus.test.QuarkusUnitTest;
import io.restassured.RestAssured;

public class LazyAuthRolesAllowedJaxRsTestCase {
@RegisterExtension
static QuarkusUnitTest runner = new QuarkusUnitTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClasses(RolesAllowedResource.class, UserResource.class,
TestIdentityProvider.class,
TestIdentityController.class,
UnsecuredSubResource.class)
.addAsResource(new StringAsset("quarkus.http.auth.proactive=false\n"),
"application.properties"));

@BeforeAll
public static void setupUsers() {
TestIdentityController.resetRoles()
.add("admin", "admin", "admin")
.add("user", "user", "user");
}

@Test
public void testRolesAllowed() {
RestAssured.get("/roles").then().statusCode(401);
RestAssured.given().auth().basic("admin", "admin").get("/roles").then().statusCode(200);
RestAssured.given().auth().basic("admin", "wrong").get("/roles").then().statusCode(401);
RestAssured.given().auth().basic("user", "user").get("/roles").then().statusCode(200);
RestAssured.given().auth().basic("admin", "admin").get("/roles/admin").then().statusCode(200);
RestAssured.given().auth().basic("user", "user").get("/roles/admin").then().statusCode(403);
}

@Test
public void testUser() {
RestAssured.get("/user").then().body(is(""));
RestAssured.given().auth().basic("admin", "admin").get("/user").then().body(is(""));
RestAssured.given().auth().preemptive().basic("admin", "admin").get("/user").then().body(is(""));
RestAssured.given().auth().basic("user", "user").get("/user").then().body(is(""));
RestAssured.given().auth().preemptive().basic("user", "user").get("/user").then().body(is(""));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package io.quarkus.resteasy.reactive.server.test.security;

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

/**
* @author Michal Szynkiewicz, [email protected]
*/
@Path("/permitAll")
@PermitAll
public class PermitAllResource {
@Path("/defaultSecurity")
@GET
public String defaultSecurity() {
return "defaultSecurity";
}

@Path("/sub")
public UnsecuredSubResource sub() {
return new UnsecuredSubResource();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package io.quarkus.resteasy.reactive.server.test.security;

import static org.hamcrest.Matchers.is;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.security.test.utils.TestIdentityController;
import io.quarkus.security.test.utils.TestIdentityProvider;
import io.quarkus.test.QuarkusUnitTest;
import io.restassured.RestAssured;

public class ReplaceIdentityLazyAuthRolesAllowedJaxRsTestCase {
@RegisterExtension
static QuarkusUnitTest runner = new QuarkusUnitTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClasses(RolesAllowedResource.class, UserResource.class,
TestIdentityProvider.class,
TestIdentityController.class,
SecurityOverrideFilter.class,
UnsecuredSubResource.class)
.addAsResource(new StringAsset("quarkus.http.auth.proactive=false\n"),
"application.properties"));

@BeforeAll
public static void setupUsers() {
TestIdentityController.resetRoles()
.add("admin", "admin", "admin")
.add("user", "user", "user");
}

@Test
public void testRolesAllowedModified() {
//make sure that things work as normal when no modification happens
RestAssured.given()
.header("user", "admin")
.header("role", "admin")
.get("/roles").then().statusCode(200);
RestAssured.given()
.auth().basic("user", "user")
.header("user", "admin")
.header("role", "admin").get("/roles/admin").then().statusCode(200);
}

@Test
public void testUser() {
RestAssured.given().auth().basic("user", "user")
.header("user", "admin")
.header("role", "admin").get("/user").then().body(is("admin"));
RestAssured.given().auth().preemptive().basic("user", "user")
.header("user", "admin")
.header("role", "admin").get("/user").then().body(is("admin"));
}
}
Loading

0 comments on commit e61bc27

Please sign in to comment.