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

Push security checks further up the handler chain #19598

Merged
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 @@ -72,6 +72,7 @@
import io.quarkus.arc.runtime.BeanContainer;
import io.quarkus.arc.runtime.ClientProxyUnwrapper;
import io.quarkus.deployment.Capabilities;
import io.quarkus.deployment.Capability;
import io.quarkus.deployment.Feature;
import io.quarkus.deployment.GeneratedClassGizmoAdaptor;
import io.quarkus.deployment.annotations.BuildProducer;
Expand All @@ -80,6 +81,7 @@
import io.quarkus.deployment.annotations.Record;
import io.quarkus.deployment.builditem.ApplicationClassPredicateBuildItem;
import io.quarkus.deployment.builditem.BytecodeTransformerBuildItem;
import io.quarkus.deployment.builditem.CombinedIndexBuildItem;
import io.quarkus.deployment.builditem.FeatureBuildItem;
import io.quarkus.deployment.builditem.GeneratedClassBuildItem;
import io.quarkus.deployment.builditem.LaunchModeBuildItem;
Expand Down Expand Up @@ -108,6 +110,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.EagerSecurityHandler;
import io.quarkus.resteasy.reactive.server.runtime.security.SecurityContextOverrideHandler;
import io.quarkus.resteasy.reactive.server.spi.MethodScannerBuildItem;
import io.quarkus.resteasy.reactive.spi.CustomExceptionMapperBuildItem;
Expand Down Expand Up @@ -544,6 +547,8 @@ private boolean hasAnnotation(MethodInfo method, short paramPosition, DotName an
.setDynamicFeatures(dynamicFeats)
.setSerialisers(serialisers)
.setApplicationPath(applicationPath)
.setGlobalHandlerCustomers(
new ArrayList<>(Collections.singletonList(new SecurityContextOverrideHandler.Customizer()))) //TODO: should be pluggable
.setResourceClasses(resourceClasses)
.setLocatableResourceClasses(subResourceClasses)
.setParamConverterProviders(paramConverterProviders);
Expand Down Expand Up @@ -632,12 +637,26 @@ public void securityExceptionMappers(BuildProducer<ExceptionMapperBuildItem> exc
}

@BuildStep
MethodScannerBuildItem integrateSecurityOverrideSupport() {
MethodScannerBuildItem integrateEagerSecurity(Capabilities capabilities, CombinedIndexBuildItem indexBuildItem) {
if (!capabilities.isPresent(Capability.SECURITY)) {
return null;
}
var index = indexBuildItem.getComputingIndex();
return new MethodScannerBuildItem(new MethodScanner() {
@Override
public List<HandlerChainCustomizer> scan(MethodInfo method, ClassInfo actualEndpointClass,
Map<String, Object> methodContext) {
return Collections.singletonList(new SecurityContextOverrideHandler.Customizer());
if (SecurityTransformerUtils.hasStandardSecurityAnnotation(method)) {
return Collections.singletonList(new EagerSecurityHandler.Customizer());
}
ClassInfo c = actualEndpointClass;
while (c.superName() != null) {
if (SecurityTransformerUtils.hasStandardSecurityAnnotation(c)) {
return Collections.singletonList(new EagerSecurityHandler.Customizer());
}
c = index.getClassByName(c.superName());
}
return Collections.emptyList();
}
});
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package io.quarkus.resteasy.reactive.server.deployment;

import java.util.Collection;
import java.util.HashSet;
import java.util.Optional;
import java.util.Set;

import javax.annotation.security.DenyAll;
import javax.annotation.security.PermitAll;
import javax.annotation.security.RolesAllowed;

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

import io.quarkus.security.Authenticated;

public class SecurityTransformerUtils {
public static final Set<DotName> SECURITY_BINDINGS = new HashSet<>();

static {
// keep the contents the same as in io.quarkus.resteasy.deployment.SecurityTransformerUtils
SECURITY_BINDINGS.add(DotName.createSimple(RolesAllowed.class.getName()));
SECURITY_BINDINGS.add(DotName.createSimple(Authenticated.class.getName()));
SECURITY_BINDINGS.add(DotName.createSimple(DenyAll.class.getName()));
SECURITY_BINDINGS.add(DotName.createSimple(PermitAll.class.getName()));
}

public static boolean hasStandardSecurityAnnotation(MethodInfo methodInfo) {
return hasStandardSecurityAnnotation(methodInfo.annotations());
}

public static boolean hasStandardSecurityAnnotation(ClassInfo classInfo) {
return hasStandardSecurityAnnotation(classInfo.classAnnotations());
}

private static boolean hasStandardSecurityAnnotation(Collection<AnnotationInstance> instances) {
for (AnnotationInstance instance : instances) {
if (SECURITY_BINDINGS.contains(instance.name())) {
return true;
}
}
return false;
}

public static Optional<AnnotationInstance> findFirstStandardSecurityAnnotation(MethodInfo methodInfo) {
return findFirstStandardSecurityAnnotation(methodInfo.annotations());
}

public static Optional<AnnotationInstance> findFirstStandardSecurityAnnotation(ClassInfo classInfo) {
return findFirstStandardSecurityAnnotation(classInfo.classAnnotations());
}

private static Optional<AnnotationInstance> findFirstStandardSecurityAnnotation(Collection<AnnotationInstance> instances) {
for (AnnotationInstance instance : instances) {
if (SECURITY_BINDINGS.contains(instance.name())) {
return Optional.of(instance);
}
}
return Optional.empty();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import static org.hamcrest.Matchers.is;

import java.util.Arrays;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
Expand All @@ -18,7 +20,7 @@ public class LazyAuthRolesAllowedJaxRsTestCase {
@RegisterExtension
static QuarkusUnitTest runner = new QuarkusUnitTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClasses(RolesAllowedResource.class, UserResource.class,
.addClasses(RolesAllowedResource.class, RolesAllowedBlockingResource.class, UserResource.class,
TestIdentityProvider.class,
TestIdentityController.class,
UnsecuredSubResource.class)
Expand All @@ -34,12 +36,14 @@ public static void setupUsers() {

@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);
Arrays.asList("/roles", "/roles-blocking").forEach((path) -> {
RestAssured.get(path).then().statusCode(401);
RestAssured.given().auth().basic("admin", "admin").get(path).then().statusCode(200);
RestAssured.given().auth().basic("admin", "wrong").get(path).then().statusCode(401);
RestAssured.given().auth().basic("user", "user").get(path).then().statusCode(200);
RestAssured.given().auth().basic("admin", "admin").get(path + "/admin").then().statusCode(200);
RestAssured.given().auth().basic("user", "user").get(path + "/admin").then().statusCode(403);
});
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import static org.hamcrest.Matchers.is;

import java.util.Arrays;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
Expand All @@ -18,7 +20,7 @@ public class ReplaceIdentityLazyAuthRolesAllowedJaxRsTestCase {
@RegisterExtension
static QuarkusUnitTest runner = new QuarkusUnitTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClasses(RolesAllowedResource.class, UserResource.class,
.addClasses(RolesAllowedResource.class, UserResource.class, RolesAllowedBlockingResource.class,
TestIdentityProvider.class,
TestIdentityController.class,
SecurityOverrideFilter.class,
Expand All @@ -36,14 +38,17 @@ public static void setupUsers() {
@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);

Arrays.asList("/roles", "/roles-blocking").forEach((path) -> {
RestAssured.given()
.header("user", "admin")
.header("role", "admin")
.get(path).then().statusCode(200);
RestAssured.given()
.auth().basic("user", "user")
.header("user", "admin")
.header("role", "admin").get(path + "/admin").then().statusCode(200);
});
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package io.quarkus.resteasy.reactive.server.test.security;

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

import io.smallrye.common.annotation.Blocking;

/**
* @author Michal Szynkiewicz, [email protected]
*/
@Path("/roles-blocking")
@PermitAll
@Blocking
public class RolesAllowedBlockingResource {
@GET
@RolesAllowed({ "user", "admin" })
public String defaultSecurity() {
return "default";
}

@Path("/admin")
@RolesAllowed("admin")
@GET
public String admin() {
return "admin";
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,21 @@

import static org.hamcrest.Matchers.is;

import java.io.IOException;
import java.io.InputStream;
import java.lang.annotation.Annotation;
import java.lang.reflect.Type;
import java.util.Arrays;

import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.ext.MessageBodyReader;
import javax.ws.rs.ext.Provider;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
Expand All @@ -17,7 +30,8 @@ public class RolesAllowedJaxRsTestCase {
@RegisterExtension
static QuarkusUnitTest runner = new QuarkusUnitTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClasses(RolesAllowedResource.class, UserResource.class,
.addClasses(RolesAllowedResource.class, UserResource.class, RolesAllowedBlockingResource.class,
SerializationEntity.class, SerializationRolesResource.class,
TestIdentityProvider.class,
TestIdentityController.class,
UnsecuredSubResource.class));
Expand All @@ -31,11 +45,13 @@ public static void setupUsers() {

@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("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);
Arrays.asList("/roles", "/roles-blocking").forEach((path) -> {
RestAssured.get(path).then().statusCode(401);
RestAssured.given().auth().basic("admin", "admin").get(path).then().statusCode(200);
RestAssured.given().auth().basic("user", "user").get(path).then().statusCode(200);
RestAssured.given().auth().basic("admin", "admin").get(path + "/admin").then().statusCode(200);
RestAssured.given().auth().basic("user", "user").get(path + "/admin").then().statusCode(403);
});
}

@Test
Expand All @@ -46,4 +62,47 @@ public void testUser() {
RestAssured.given().auth().basic("user", "user").get("/user").then().body(is(""));
RestAssured.given().auth().preemptive().basic("user", "user").get("/user").then().body(is("user"));
}

@Test
public void testSecurityRunsBeforeValidation() {
read = false;
RestAssured.given().body(new SerializationEntity()).post("/roles-validate").then().statusCode(401);
Assertions.assertFalse(read);
RestAssured.given().body(new SerializationEntity()).auth().basic("admin", "admin").post("/roles-validate").then()
.statusCode(200);
Assertions.assertTrue(read);
read = false;
RestAssured.given().body(new SerializationEntity()).auth().basic("user", "user").post("/roles-validate").then()
.statusCode(200);
Assertions.assertTrue(read);
read = false;
RestAssured.given().body(new SerializationEntity()).auth().basic("admin", "admin").post("/roles-validate/admin").then()
.statusCode(200);
Assertions.assertTrue(read);
read = false;
RestAssured.given().body(new SerializationEntity()).auth().basic("user", "user").post("/roles-validate/admin").then()
.statusCode(403);
Assertions.assertFalse(read);
}

static volatile boolean read = false;

@Provider
public static class Reader implements MessageBodyReader<SerializationEntity> {

@Override
public boolean isReadable(Class<?> type, Type genericType, Annotation[] annotations, MediaType mediaType) {
return true;
}

@Override
public SerializationEntity readFrom(Class<SerializationEntity> type, Type genericType, Annotation[] annotations,
MediaType mediaType, MultivaluedMap<String, String> httpHeaders, InputStream entityStream)
throws IOException, WebApplicationException {
read = true;
SerializationEntity entity = new SerializationEntity();
entity.setName("read");
return entity;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,11 @@
import javax.ws.rs.GET;
import javax.ws.rs.Path;

import io.smallrye.common.annotation.Blocking;

/**
* @author Michal Szynkiewicz, [email protected]
*/
@Path("/roles")
@PermitAll
@Blocking
public class RolesAllowedResource {
@GET
@RolesAllowed({ "user", "admin" })
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package io.quarkus.resteasy.reactive.server.test.security;

public class SerializationEntity {
private String name;

public String getName() {
return name;
}

public SerializationEntity setName(String name) {
this.name = name;
return this;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package io.quarkus.resteasy.reactive.server.test.security;

import javax.annotation.security.PermitAll;
import javax.annotation.security.RolesAllowed;
import javax.ws.rs.POST;
import javax.ws.rs.Path;

import io.smallrye.common.annotation.Blocking;

@Path("/roles-validate")
@PermitAll
@Blocking
public class SerializationRolesResource {
@POST
@RolesAllowed({ "user", "admin" })
public String defaultSecurity(SerializationEntity entity) {
return entity.getName();
}

@Path("/admin")
@RolesAllowed("admin")
@POST
public String admin(SerializationEntity entity) {
return entity.getName();
}

}
Loading