Skip to content

Commit

Permalink
Merge pull request #40448 from michalvavrik/feature/secure-field-warn…
Browse files Browse the repository at this point in the history
…ing-logs

Avoid "Failed to index" warnings produced during @SecureField annotation detection
  • Loading branch information
gsmet authored May 10, 2024
2 parents 9e35db9 + f473250 commit 143e083
Show file tree
Hide file tree
Showing 5 changed files with 513 additions and 6 deletions.
21 changes: 21 additions & 0 deletions docs/src/main/asciidoc/rest.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1458,8 +1458,12 @@ A very simple Jakarta REST Resource that uses `Person` could be:
----
package org.acme.rest;
import static jakarta.ws.rs.core.MediaType.APPLICATION_JSON;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.Response;
@Path("person")
public class Person {
Expand All @@ -1469,8 +1473,25 @@ public class Person {
public Person getPerson(Long id) {
return new Person(id, "foo", "bar", "Brick Lane");
}
@Produces(APPLICATION_JSON) <1>
@Path("/friend/{id}")
@GET
public Response getPersonFriend(Long id) {
var person = new Person(id, "foo", "bar", "Brick Lane");
return Response.ok(person).build();
}
}
----
<1> The `@SecureField` annotation is only effective when Quarkus recognizes that produced content type is the 'application/json' type.

WARNING: Currently you cannot use the `@SecureField` annotation to secure your data returned from resource methods returning the `io.smallrye.mutiny.Multi` reactive type.

[IMPORTANT]
====
All resource methods returning data secured with the `@SecureField` annotation should be tested.
Please make sure data are secured as you intended.
====

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
performs an HTTP GET on `/person/1` they will receive:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.quarkus.resteasy.reactive.jackson.deployment.processor;

import static io.quarkus.resteasy.reactive.common.deployment.QuarkusResteasyReactiveDotNames.JSON_IGNORE;
import static io.quarkus.security.spi.RolesAllowedConfigExpResolverBuildItem.isSecurityConfigExpressionCandidate;
import static org.jboss.resteasy.reactive.common.util.RestMediaType.APPLICATION_NDJSON;
import static org.jboss.resteasy.reactive.common.util.RestMediaType.APPLICATION_STREAM_JSON;
Expand All @@ -15,6 +16,7 @@
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.BiConsumer;
import java.util.function.Predicate;
import java.util.function.Supplier;

import jakarta.inject.Singleton;
Expand Down Expand Up @@ -59,6 +61,7 @@
import io.quarkus.deployment.builditem.ShutdownContextBuildItem;
import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
import io.quarkus.resteasy.reactive.common.deployment.JaxRsResourceIndexBuildItem;
import io.quarkus.resteasy.reactive.common.deployment.QuarkusResteasyReactiveDotNames;
import io.quarkus.resteasy.reactive.common.deployment.ResourceScanningResultBuildItem;
import io.quarkus.resteasy.reactive.common.deployment.ServerDefaultProducesHandlerBuildItem;
import io.quarkus.resteasy.reactive.jackson.CustomDeserialization;
Expand Down Expand Up @@ -372,7 +375,12 @@ public void handleFieldSecurity(ResteasyReactiveResourceMethodEntriesBuildItem r
JaxRsResourceIndexBuildItem index,
BuildProducer<ResourceMethodCustomSerializationBuildItem> producer) {
IndexView indexView = index.getIndexView();
Map<String, Boolean> typeToHasSecureField = new HashMap<>();
boolean noSecureFieldDetected = indexView.getAnnotations(SECURE_FIELD).isEmpty();
if (noSecureFieldDetected) {
return;
}

Map<String, Boolean> typeToHasSecureField = new HashMap<>(getTypesWithSecureField());
List<ResourceMethodCustomSerializationBuildItem> result = new ArrayList<>();
for (ResteasyReactiveResourceMethodEntriesBuildItem.Entry entry : resourceMethodEntries.getEntries()) {
MethodInfo methodInfo = entry.getMethodInfo();
Expand Down Expand Up @@ -425,7 +433,7 @@ public void handleFieldSecurity(ResteasyReactiveResourceMethodEntriesBuildItem r
}

ClassInfo effectiveReturnClassInfo = indexView.getClassByName(effectiveReturnType.name());
if ((effectiveReturnClassInfo == null) || effectiveReturnClassInfo.name().equals(ResteasyReactiveDotNames.OBJECT)) {
if (effectiveReturnClassInfo == null) {
continue;
}
AtomicBoolean needToDeleteCache = new AtomicBoolean(false);
Expand All @@ -443,6 +451,7 @@ public void handleFieldSecurity(ResteasyReactiveResourceMethodEntriesBuildItem r
}
if (needToDeleteCache.get()) {
typeToHasSecureField.clear();
typeToHasSecureField.putAll(getTypesWithSecureField());
}
}
if (!result.isEmpty()) {
Expand All @@ -452,6 +461,13 @@ public void handleFieldSecurity(ResteasyReactiveResourceMethodEntriesBuildItem r
}
}

private static Map<String, Boolean> getTypesWithSecureField() {
// if any of following types is detected as an endpoint return type or a field of endpoint return type,
// we always need to apply security serialization as any type can be represented with them
return Map.of(ResteasyReactiveDotNames.OBJECT.toString(), Boolean.TRUE, ResteasyReactiveDotNames.RESPONSE.toString(),
Boolean.TRUE);
}

private static boolean hasSecureFields(IndexView indexView, ClassInfo currentClassInfo,
Map<String, Boolean> typeToHasSecureField, AtomicBoolean needToDeleteCache) {
// use cached result if there is any
Expand Down Expand Up @@ -479,10 +495,20 @@ private static boolean hasSecureFields(IndexView indexView, ClassInfo currentCla
.anyMatch(ci -> hasSecureFields(indexView, ci, typeToHasSecureField, needToDeleteCache));
} else {
// figure if any field or parent / subclass field is secured
hasSecureFields = hasSecureFields(currentClassInfo)
|| anyFieldHasSecureFields(indexView, currentClassInfo, typeToHasSecureField, needToDeleteCache)
|| anySubclassHasSecureFields(indexView, currentClassInfo, typeToHasSecureField, needToDeleteCache)
|| anyParentClassHasSecureFields(indexView, currentClassInfo, typeToHasSecureField, needToDeleteCache);
if (hasSecureFields(currentClassInfo)) {
hasSecureFields = true;
} else {
Predicate<DotName> ignoredTypesPredicate = QuarkusResteasyReactiveDotNames.IGNORE_TYPE_FOR_REFLECTION_PREDICATE;
if (ignoredTypesPredicate.test(currentClassInfo.name())) {
hasSecureFields = false;
} else {
hasSecureFields = anyFieldHasSecureFields(indexView, currentClassInfo, typeToHasSecureField,
needToDeleteCache)
|| anySubclassHasSecureFields(indexView, currentClassInfo, typeToHasSecureField, needToDeleteCache)
|| anyParentClassHasSecureFields(indexView, currentClassInfo, typeToHasSecureField,
needToDeleteCache);
}
}
}
typeToHasSecureField.put(className, hasSecureFields);
return hasSecureFields;
Expand Down Expand Up @@ -513,6 +539,7 @@ private static boolean anyFieldHasSecureFields(IndexView indexView, ClassInfo cu
return currentClassInfo
.fields()
.stream()
.filter(fieldInfo -> !fieldInfo.hasAnnotation(JSON_IGNORE))
.map(FieldInfo::type)
.anyMatch(fieldType -> fieldTypeHasSecureFields(fieldType, indexView, typeToHasSecureField, needToDeleteCache));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
import org.jboss.resteasy.reactive.RestForm;
import org.jboss.resteasy.reactive.multipart.FileUpload;

import io.quarkus.resteasy.reactive.jackson.DisableSecureSerialization;
import io.smallrye.common.annotation.Blocking;

@Path("/multipart")
public class MultipartResource {

@DisableSecureSerialization // Person has @SecureField but we want to inspect all data
@POST
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.MULTIPART_FORM_DATA)
Expand All @@ -45,6 +47,7 @@ public Map<String, Object> greeting(@Valid @BeanParam FormData formData) {
return result;
}

@DisableSecureSerialization // Person has @SecureField but we want to inspect all data
@POST
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.MULTIPART_FORM_DATA)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package io.quarkus.resteasy.reactive.jackson.deployment.test;

import jakarta.ws.rs.core.Response;

import org.jboss.resteasy.reactive.RestResponse;

import io.quarkus.resteasy.reactive.jackson.SecureField;
import io.smallrye.mutiny.Multi;
import io.smallrye.mutiny.Uni;

public enum ResponseType {
/**
* Returns DTOs directly.
*/
PLAIN(true, "plain"),
/**
* Returns {@link Multi} with DTOs.
*/
// TODO: enable when https://github.com/quarkusio/quarkus/issues/40447 gets fixed
//MULTI(true, "multi"),
/**
* Returns {@link Uni} with DTOs.
*/
UNI(true, "uni"),
/**
* Returns {@link Object} that is either DTO with a {@link SecureField} or not.
*/
OBJECT(false, "object"), // we must always assume it can contain SecureField
/**
* Returns {@link Response} that is either DTO with a {@link SecureField} or not.
*/
RESPONSE(false, "response"), // we must always assume it can contain SecureField
/**
* Returns {@link RestResponse} with DTOs.
*/
REST_RESPONSE(true, "rest-response"),
/**
* Returns {@link RestResponse} with DTOs.
*/
COLLECTION(true, "collection");

private final boolean secureFieldDetectable;
private final String resourceSubPath;

ResponseType(boolean secureFieldDetectable, String resourceSubPath) {
this.secureFieldDetectable = secureFieldDetectable;
this.resourceSubPath = resourceSubPath;
}

boolean isSecureFieldDetectable() {
return secureFieldDetectable;
}

String getResourceSubPath() {
return resourceSubPath;
}
}
Loading

0 comments on commit 143e083

Please sign in to comment.