Merge pull request #39564 from michalvavrik/feature/secure-field-on-a…

…bstract-class-quarkus-rest

Fix Quarkus REST Jackson @SecureField detection on subclasses, interface implementors, fileds of the fields, parametrized types and arrays

extensions/resteasy-reactive/rest-jackson/deployment/src/main/java/io/quarkus/resteasy/reactive/jackson/deployment/processor/ResteasyReactiveJacksonProcessor.java

@@ -6,11 +6,14 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.BiConsumer;
 import java.util.function.Supplier;
 
@@ -25,6 +28,7 @@
 import org.jboss.jandex.AnnotationValue;
 import org.jboss.jandex.ClassInfo;
 import org.jboss.jandex.DotName;
+import org.jboss.jandex.FieldInfo;
 import org.jboss.jandex.IndexView;
 import org.jboss.jandex.MethodInfo;
 import org.jboss.jandex.Type;
@@ -368,6 +372,7 @@ public void handleFieldSecurity(ResteasyReactiveResourceMethodEntriesBuildItem r
             JaxRsResourceIndexBuildItem index,
             BuildProducer<ResourceMethodCustomSerializationBuildItem> producer) {
         IndexView indexView = index.getIndexView();
+        Map<String, Boolean> typeToHasSecureField = new HashMap<>();
         List<ResourceMethodCustomSerializationBuildItem> result = new ArrayList<>();
         for (ResteasyReactiveResourceMethodEntriesBuildItem.Entry entry : resourceMethodEntries.getEntries()) {
             MethodInfo methodInfo = entry.getMethodInfo();
@@ -423,22 +428,8 @@ public void handleFieldSecurity(ResteasyReactiveResourceMethodEntriesBuildItem r
             if ((effectiveReturnClassInfo == null) || effectiveReturnClassInfo.name().equals(ResteasyReactiveDotNames.OBJECT)) {
                 continue;
             }
-            ClassInfo currentClassInfo = effectiveReturnClassInfo;
-            boolean hasSecureFields = false;
-            while (true) {
-                if (currentClassInfo.annotationsMap().containsKey(SECURE_FIELD)) {
-                    hasSecureFields = true;
-                    break;
-                }
-                if (currentClassInfo.superName().equals(ResteasyReactiveDotNames.OBJECT)) {
-                    break;
-                }
-                currentClassInfo = indexView.getClassByName(currentClassInfo.superName());
-                if (currentClassInfo == null) {
-                    break;
-                }
-            }
-            if (hasSecureFields) {
+            AtomicBoolean needToDeleteCache = new AtomicBoolean(false);
+            if (hasSecureFields(indexView, effectiveReturnClassInfo, typeToHasSecureField, needToDeleteCache)) {
                 AnnotationInstance customSerializationAtClassAnnotation = methodInfo.declaringClass()
                         .declaredAnnotation(CUSTOM_SERIALIZATION);
                 AnnotationInstance customSerializationAtMethodAnnotation = methodInfo.annotation(CUSTOM_SERIALIZATION);
@@ -450,6 +441,9 @@ public void handleFieldSecurity(ResteasyReactiveResourceMethodEntriesBuildItem r
                             SecurityCustomSerialization.class));
                 }
             }
+            if (needToDeleteCache.get()) {
+                typeToHasSecureField.clear();
+            }
         }
         if (!result.isEmpty()) {
             for (ResourceMethodCustomSerializationBuildItem bi : result) {
@@ -458,6 +452,95 @@ public void handleFieldSecurity(ResteasyReactiveResourceMethodEntriesBuildItem r
         }
     }
 
+    private static boolean hasSecureFields(IndexView indexView, ClassInfo currentClassInfo,
+            Map<String, Boolean> typeToHasSecureField, AtomicBoolean needToDeleteCache) {
+        // use cached result if there is any
+        final String className = currentClassInfo.name().toString();
+        if (typeToHasSecureField.containsKey(className)) {
+            Boolean hasSecureFields = typeToHasSecureField.get(className);
+            if (hasSecureFields == null) {
+                // this is to avoid false negative for scenario like:
+                // when 'a' declares field of type 'b' which declares field of type 'a' and both 'a' and 'b'
+                // are returned from an endpoint and 'b' is detected based on 'a' and processed after 'a'
+                needToDeleteCache.set(true);
+                return false;
+            }
+            return hasSecureFields;
+        }
+
+        // prevent cyclic check of the same type
+        // for example when a field has a same type as the current class has
+        typeToHasSecureField.put(className, null);
+
+        final boolean hasSecureFields;
+        if (currentClassInfo.isInterface()) {
+            // check interface implementors as anyone of them can be returned
+            hasSecureFields = indexView.getAllKnownImplementors(currentClassInfo.name()).stream()
+                    .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);
+        }
+        typeToHasSecureField.put(className, hasSecureFields);
+        return hasSecureFields;
+    }
+
+    private static boolean hasSecureFields(ClassInfo classInfo) {
+        return classInfo.annotationsMap().containsKey(SECURE_FIELD);
+    }
+
+    private static boolean anyParentClassHasSecureFields(IndexView indexView, ClassInfo currentClassInfo,
+            Map<String, Boolean> typeToHasSecureField, AtomicBoolean needToDeleteCache) {
+        if (!currentClassInfo.superName().equals(ResteasyReactiveDotNames.OBJECT)) {
+            final ClassInfo parentClassInfo = indexView.getClassByName(currentClassInfo.superName());
+            return parentClassInfo != null
+                    && hasSecureFields(indexView, parentClassInfo, typeToHasSecureField, needToDeleteCache);
+        }
+        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));
+    }
+
+    private static boolean anyFieldHasSecureFields(IndexView indexView, ClassInfo currentClassInfo,
+            Map<String, Boolean> typeToHasSecureField, AtomicBoolean needToDeleteCache) {
+        return currentClassInfo
+                .fields()
+                .stream()
+                .map(FieldInfo::type)
+                .anyMatch(fieldType -> fieldTypeHasSecureFields(fieldType, indexView, typeToHasSecureField, needToDeleteCache));
+    }
+
+    private static boolean fieldTypeHasSecureFields(Type fieldType, IndexView indexView,
+            Map<String, Boolean> typeToHasSecureField, AtomicBoolean needToDeleteCache) {
+        // this is the best effort and does not cover every possibility (e.g. type variables, wildcards)
+        if (fieldType.kind() == Type.Kind.CLASS) {
+            if (fieldType.name().packagePrefix() != null && fieldType.name().packagePrefix().startsWith("java.")) {
+                return false;
+            }
+            final ClassInfo fieldClass = indexView.getClassByName(fieldType.name());
+            return fieldClass != null && hasSecureFields(indexView, fieldClass, typeToHasSecureField, needToDeleteCache);
+        }
+        if (fieldType.kind() == Type.Kind.ARRAY) {
+            return fieldTypeHasSecureFields(fieldType.asArrayType().constituent(), indexView, typeToHasSecureField,
+                    needToDeleteCache);
+        }
+        if (fieldType.kind() == Type.Kind.PARAMETERIZED_TYPE) {
+            return fieldType
+                    .asParameterizedType()
+                    .arguments()
+                    .stream()
+                    .anyMatch(t -> fieldTypeHasSecureFields(t, indexView, typeToHasSecureField, needToDeleteCache));
+        }
+        return false;
+    }
+
     private String getTargetId(AnnotationTarget target) {
         if (target.kind() == AnnotationTarget.Kind.CLASS) {
             return getClassId(target.asClass());

extensions/resteasy-reactive/rest-jackson/deployment/src/test/java/io/quarkus/resteasy/reactive/jackson/deployment/test/AbstractNamedPet.java

@@ -0,0 +1,17 @@
+package io.quarkus.resteasy.reactive.jackson.deployment.test;
+
+import io.quarkus.resteasy.reactive.jackson.SecureField;
+
+public abstract class AbstractNamedPet extends AbstractPet {
+
+    @SecureField(rolesAllowed = "admin")
+    private String privateName;
+
+    public String getPrivateName() {
+        return privateName;
+    }
+
+    public void setPrivateName(String privateName) {
+        this.privateName = privateName;
+    }
+}

extensions/resteasy-reactive/rest-jackson/deployment/src/test/java/io/quarkus/resteasy/reactive/jackson/deployment/test/AbstractPet.java

@@ -0,0 +1,23 @@
+package io.quarkus.resteasy.reactive.jackson.deployment.test;
+
+public abstract class AbstractPet implements SecuredPersonInterface {
+
+    private String publicName;
+    private Veterinarian veterinarian;
+
+    public String getPublicName() {
+        return publicName;
+    }
+
+    public void setPublicName(String publicName) {
+        this.publicName = publicName;
+    }
+
+    public Veterinarian getVeterinarian() {
+        return veterinarian;
+    }
+
+    public void setVeterinarian(Veterinarian veterinarian) {
+        this.veterinarian = veterinarian;
+    }
+}

extensions/resteasy-reactive/rest-jackson/deployment/src/test/java/io/quarkus/resteasy/reactive/jackson/deployment/test/AbstractUnsecuredPet.java

@@ -0,0 +1,15 @@
+package io.quarkus.resteasy.reactive.jackson.deployment.test;
+
+public abstract class AbstractUnsecuredPet {
+
+    private String publicName;
+
+    public String getPublicName() {
+        return publicName;
+    }
+
+    public void setPublicName(String publicName) {
+        this.publicName = publicName;
+    }
+
+}

extensions/resteasy-reactive/rest-jackson/deployment/src/test/java/io/quarkus/resteasy/reactive/jackson/deployment/test/Cat.java

@@ -0,0 +1,17 @@
+package io.quarkus.resteasy.reactive.jackson.deployment.test;
+
+import io.quarkus.resteasy.reactive.jackson.SecureField;
+
+public class Cat extends AbstractNamedPet {
+
+    @SecureField(rolesAllowed = "admin")
+    private int privateAge;
+
+    public int getPrivateAge() {
+        return privateAge;
+    }
+
+    public void setPrivateAge(int privateAge) {
+        this.privateAge = privateAge;
+    }
+}

extensions/resteasy-reactive/rest-jackson/deployment/src/test/java/io/quarkus/resteasy/reactive/jackson/deployment/test/Dog.java

@@ -0,0 +1,14 @@
+package io.quarkus.resteasy.reactive.jackson.deployment.test;
+
+public class Dog extends AbstractNamedPet {
+
+    private int publicAge;
+
+    public int getPublicAge() {
+        return publicAge;
+    }
+
+    public void setPublicAge(int publicAge) {
+        this.publicAge = publicAge;
+    }
+}

extensions/resteasy-reactive/rest-jackson/deployment/src/test/java/io/quarkus/resteasy/reactive/jackson/deployment/test/Frog.java

@@ -0,0 +1,28 @@
+package io.quarkus.resteasy.reactive.jackson.deployment.test;
+
+import java.util.List;
+
+public class Frog {
+
+    // check no cycle when field has the same type as the class which declares it
+    private Frog partner;
+
+    private List<Pond> ponds;
+
+    public Frog getPartner() {
+        return partner;
+    }
+
+    public void setPartner(Frog partner) {
+        this.partner = partner;
+    }
+
+    public List<Pond> getPonds() {
+        return ponds;
+    }
+
+    public void setPonds(List<Pond> ponds) {
+        this.ponds = ponds;
+    }
+
+}

extensions/resteasy-reactive/rest-jackson/deployment/src/test/java/io/quarkus/resteasy/reactive/jackson/deployment/test/FrogBodyParts.java

@@ -0,0 +1,45 @@
+package io.quarkus.resteasy.reactive.jackson.deployment.test;
+
+import io.quarkus.resteasy.reactive.jackson.SecureField;
+
+public class FrogBodyParts {
+
+    public FrogBodyParts() {
+    }
+
+    public FrogBodyParts(String bodyPartName) {
+        this.parts = new BodyPart[] { new BodyPart(bodyPartName) };
+    }
+
+    private BodyPart[] parts;
+
+    public BodyPart[] getParts() {
+        return parts;
+    }
+
+    public void setParts(BodyPart[] parts) {
+        this.parts = parts;
+    }
+
+    public static class BodyPart {
+
+        public BodyPart(String name) {
+            this.name = name;
+        }
+
+        public BodyPart() {
+        }
+
+        @SecureField(rolesAllowed = "admin")
+        private String name;
+
+        public String getName() {
+            return name;
+        }
+
+        public void setName(String name) {
+            this.name = name;
+        }
+    }
+
+}

extensions/resteasy-reactive/rest-jackson/deployment/src/test/java/io/quarkus/resteasy/reactive/jackson/deployment/test/Pond.java

@@ -0,0 +1,33 @@
+package io.quarkus.resteasy.reactive.jackson.deployment.test;
+
+import io.quarkus.resteasy.reactive.jackson.SecureField;
+
+public class Pond {
+
+    @SecureField(rolesAllowed = "admin")
+    private WaterQuality waterQuality;
+
+    private String name;
+
+    public WaterQuality getWaterQuality() {
+        return waterQuality;
+    }
+
+    public void setWaterQuality(WaterQuality waterQuality) {
+        this.waterQuality = waterQuality;
+    }
+
+    public String getName() {
+        return name;
+    }
+
+    public void setName(String name) {
+        this.name = name;
+    }
+
+    public enum WaterQuality {
+        CLEAR,
+        DIRTY
+    }
+
+}

extensions/resteasy-reactive/rest-jackson/deployment/src/test/java/io/quarkus/resteasy/reactive/jackson/deployment/test/SecuredPersonInterface.java

@@ -0,0 +1,7 @@
+package io.quarkus.resteasy.reactive.jackson.deployment.test;
+
+public interface SecuredPersonInterface {
+
+    String getPublicName();
+
+}