Skip to content

Commit

Permalink
fix(rest-jackson,security): improve @SecureField annotation detection
Browse files Browse the repository at this point in the history
  • Loading branch information
michalvavrik committed Nov 23, 2024
1 parent 1e7e874 commit e86c7f3
Show file tree
Hide file tree
Showing 9 changed files with 230 additions and 33 deletions.
3 changes: 3 additions & 0 deletions docs/src/main/asciidoc/rest.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1535,6 +1535,9 @@ WARNING: Currently you cannot use the `@SecureField` annotation to secure your d
====
All resource methods returning data secured with the `@SecureField` annotation should be tested.
Please make sure data are secured as you intended.
Quarkus always attempts to detect fields annotated with the `@SecureField` annotation,
however it may fail to infer returned type and miss the `@SecureField` annotation instance.
If that happens, please explicitly enable secure serialization with the `@EnableSecureSerialization` annotation.
====

Assuming security has been set up for the application (see our xref:security-overview.adoc[guide] for more details), when a user with the `admin` role
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,8 @@ public void handleFieldSecurity(ResteasyReactiveResourceMethodEntriesBuildItem r
continue;
}
}
boolean secureSerializationExplicitlyEnabled = methodInfo.hasAnnotation(ENABLE_SECURE_SERIALIZATION)
|| entry.getActualClassInfo().hasDeclaredAnnotation(ENABLE_SECURE_SERIALIZATION);

ResourceMethod resourceInfo = entry.getResourceMethod();
boolean isJsonResponse = false;
Expand All @@ -470,12 +472,31 @@ public void handleFieldSecurity(ResteasyReactiveResourceMethodEntriesBuildItem r
continue;
}

ClassInfo effectiveReturnClassInfo = getEffectiveClassInfo(methodInfo.returnType(), indexView);
var methodReturnType = methodInfo.returnType();
ClassInfo effectiveReturnClassInfo = getEffectiveClassInfo(methodReturnType, indexView);
if (effectiveReturnClassInfo == null) {
continue;
}

final Map<String, Type> typeParamIdentifierToParameterizedType;
if (methodReturnType.kind() == Type.Kind.PARAMETERIZED_TYPE) {
typeParamIdentifierToParameterizedType = new HashMap<>();
var parametrizedReturnType = methodReturnType.asParameterizedType();
for (int i = 0; i < parametrizedReturnType.arguments().size(); i++) {
if (i < effectiveReturnClassInfo.typeParameters().size()) {
var identifier = effectiveReturnClassInfo.typeParameters().get(i).identifier();
var parametrizedTypeArg = parametrizedReturnType.arguments().get(i);
typeParamIdentifierToParameterizedType.put(identifier, parametrizedTypeArg);
}
}
} else {
typeParamIdentifierToParameterizedType = null;
}

AtomicBoolean needToDeleteCache = new AtomicBoolean(false);
if (hasSecureFields(indexView, effectiveReturnClassInfo, typeToHasSecureField, needToDeleteCache)) {
if (secureSerializationExplicitlyEnabled
|| hasSecureFields(indexView, effectiveReturnClassInfo, typeToHasSecureField, needToDeleteCache,
typeParamIdentifierToParameterizedType)) {
AnnotationInstance customSerializationAtClassAnnotation = methodInfo.declaringClass()
.declaredAnnotation(CUSTOM_SERIALIZATION);
AnnotationInstance customSerializationAtMethodAnnotation = methodInfo.annotation(CUSTOM_SERIALIZATION);
Expand Down Expand Up @@ -539,7 +560,8 @@ private static Map<String, Boolean> getTypesWithSecureField() {
}

private static boolean hasSecureFields(IndexView indexView, ClassInfo currentClassInfo,
Map<String, Boolean> typeToHasSecureField, AtomicBoolean needToDeleteCache) {
Map<String, Boolean> typeToHasSecureField, AtomicBoolean needToDeleteCache,
Map<String, Type> typeParamIdentifierToParameterizedType) {
// use cached result if there is any
final String className = currentClassInfo.name().toString();
if (typeToHasSecureField.containsKey(className)) {
Expand All @@ -565,7 +587,7 @@ private static boolean hasSecureFields(IndexView indexView, ClassInfo currentCla
} else {
// check interface implementors as anyone of them can be returned
hasSecureFields = indexView.getAllKnownImplementors(currentClassInfo.name()).stream()
.anyMatch(ci -> hasSecureFields(indexView, ci, typeToHasSecureField, needToDeleteCache));
.anyMatch(ci -> hasSecureFields(indexView, ci, typeToHasSecureField, needToDeleteCache, null));
}
} else {
// figure if any field or parent / subclass field is secured
Expand All @@ -576,7 +598,7 @@ private static boolean hasSecureFields(IndexView indexView, ClassInfo currentCla
hasSecureFields = false;
} else {
hasSecureFields = anyFieldHasSecureFields(indexView, currentClassInfo, typeToHasSecureField,
needToDeleteCache)
needToDeleteCache, typeParamIdentifierToParameterizedType)
|| anySubclassHasSecureFields(indexView, currentClassInfo, typeToHasSecureField, needToDeleteCache)
|| anyParentClassHasSecureFields(indexView, currentClassInfo, typeToHasSecureField,
needToDeleteCache);
Expand All @@ -600,24 +622,34 @@ private static boolean anyParentClassHasSecureFields(IndexView indexView, ClassI
if (!currentClassInfo.superName().equals(ResteasyReactiveDotNames.OBJECT)) {
final ClassInfo parentClassInfo = indexView.getClassByName(currentClassInfo.superName());
return parentClassInfo != null
&& hasSecureFields(indexView, parentClassInfo, typeToHasSecureField, needToDeleteCache);
&& hasSecureFields(indexView, parentClassInfo, typeToHasSecureField, needToDeleteCache, null);
}
return false;
}

private static boolean anySubclassHasSecureFields(IndexView indexView, ClassInfo currentClassInfo,
Map<String, Boolean> typeToHasSecureField, AtomicBoolean needToDeleteCache) {
return indexView.getAllKnownSubclasses(currentClassInfo.name()).stream()
.anyMatch(subclass -> hasSecureFields(indexView, subclass, typeToHasSecureField, needToDeleteCache));
.anyMatch(subclass -> hasSecureFields(indexView, subclass, typeToHasSecureField, needToDeleteCache, null));
}

private static boolean anyFieldHasSecureFields(IndexView indexView, ClassInfo currentClassInfo,
Map<String, Boolean> typeToHasSecureField, AtomicBoolean needToDeleteCache) {
Map<String, Boolean> typeToHasSecureField, AtomicBoolean needToDeleteCache,
Map<String, Type> typeParamIdentifierToParameterizedType) {
return currentClassInfo
.fields()
.stream()
.filter(fieldInfo -> !fieldInfo.hasAnnotation(JSON_IGNORE))
.map(FieldInfo::type)
.map(fieldType -> {
if (typeParamIdentifierToParameterizedType != null && fieldType.kind() == Type.Kind.TYPE_VARIABLE) {
var typeVariable = typeParamIdentifierToParameterizedType.get(fieldType.asTypeVariable().identifier());
if (typeVariable != null) {
return typeVariable;
}
}
return fieldType;
})
.anyMatch(fieldType -> fieldTypeHasSecureFields(fieldType, indexView, typeToHasSecureField, needToDeleteCache));
}

Expand All @@ -629,7 +661,7 @@ private static boolean fieldTypeHasSecureFields(Type fieldType, IndexView indexV
return false;
}
final ClassInfo fieldClass = indexView.getClassByName(fieldType.name());
return fieldClass != null && hasSecureFields(indexView, fieldClass, typeToHasSecureField, needToDeleteCache);
return fieldClass != null && hasSecureFields(indexView, fieldClass, typeToHasSecureField, needToDeleteCache, null);
}
if (fieldType.kind() == Type.Kind.ARRAY) {
return fieldTypeHasSecureFields(fieldType.asArrayType().constituent(), indexView, typeToHasSecureField,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package io.quarkus.resteasy.reactive.jackson.deployment.test;

import jakarta.ws.rs.Consumes;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;

import org.hamcrest.Matchers;
import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.resteasy.reactive.jackson.DisableSecureSerialization;
import io.quarkus.resteasy.reactive.jackson.EnableSecureSerialization;
import io.quarkus.resteasy.reactive.jackson.SecureField;
import io.quarkus.security.test.utils.TestIdentityController;
import io.quarkus.security.test.utils.TestIdentityProvider;
import io.quarkus.test.QuarkusUnitTest;
import io.restassured.RestAssured;
import io.restassured.response.ValidatableResponse;

public class DisableSecureSerializationTest {

@RegisterExtension
static QuarkusUnitTest test = new QuarkusUnitTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClasses(TestIdentityProvider.class, TestIdentityController.class));

@Test
public void testDisablingOfSecureSerialization() {
request("disabled", "user").body("secretField", Matchers.is("secret"));
request("disabled", "admin").body("secretField", Matchers.is("secret"));
request("enabled", "user").body("secretField", Matchers.nullValue());
request("enabled", "admin").body("secretField", Matchers.is("secret"));
}

private static ValidatableResponse request(String subPath, String user) {
TestIdentityController.resetRoles().add(user, user, user);
return RestAssured
.with()
.auth().preemptive().basic(user, user)
.get("/test/" + subPath)
.then()
.statusCode(200)
.body("publicField", Matchers.is("public"));
}

@DisableSecureSerialization
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.APPLICATION_JSON)
@Path("test")
public static class GreetingsResource {

@Path("disabled")
@GET
public Dto disabled() {
return Dto.createDto();
}

@EnableSecureSerialization
@Path("enabled")
@GET
public Dto enabled() {
return Dto.createDto();
}
}

public static class Dto {

public Dto(String secretField, String publicField) {
this.secretField = secretField;
this.publicField = publicField;
}

@SecureField(rolesAllowed = "admin")
private final String secretField;

private final String publicField;

public String getSecretField() {
return secretField;
}

public String getPublicField() {
return publicField;
}

private static Dto createDto() {
return new Dto("secret", "public");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package io.quarkus.resteasy.reactive.jackson.deployment.test;

import java.util.List;

public class Fruit {

public String name;

public List<Price> prices;

public Fruit(String name, Float price) {
this.name = name;
this.prices = List.of(new Price("USD", price));
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package io.quarkus.resteasy.reactive.jackson.deployment.test;

public class GenericWrapper<T> {

public String name;

public T entity;

public GenericWrapper(String name, T entity) {
this.name = name;
this.entity = entity;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package io.quarkus.resteasy.reactive.jackson.deployment.test;

import io.quarkus.resteasy.reactive.jackson.SecureField;

public class Price {

@SecureField(rolesAllowed = "admin")
public Float price;

public String currency;

public Price(String currency, Float price) {
this.currency = currency;
this.price = price;
}

}
Loading

0 comments on commit e86c7f3

Please sign in to comment.