Merge pull request #70 from jenkinsci/improve-premission-checker

Ignore `SYNTHETIC` methods when checking for permissions
jenkinsci · Jan 6, 2021 · 2dcfa6e · 2dcfa6e
2 parents ee7f928 + 7d9c9c1
commit 2dcfa6e
Showing 1 changed file with 5 additions and 30 deletions.
diff --git a/src/test/java/io/jenkins/plugins/util/PluginArchitectureRules.java b/src/test/java/io/jenkins/plugins/util/PluginArchitectureRules.java
@@ -2,11 +2,9 @@
 
 import java.util.Arrays;
 import java.util.List;
-import java.util.Set;
 import java.util.stream.Collectors;
 
 import com.tngtech.archunit.base.DescribedPredicate;
-import com.tngtech.archunit.core.domain.JavaCall;
 import com.tngtech.archunit.core.domain.JavaClass;
 import com.tngtech.archunit.core.domain.JavaMethod;
 import com.tngtech.archunit.core.domain.JavaModifier;
@@ -19,7 +17,6 @@
 import org.kohsuke.stapler.DataBoundSetter;
 import org.kohsuke.stapler.bind.JavaScriptMethod;
 import org.kohsuke.stapler.verb.POST;
-import hudson.model.AbstractProject;
 import hudson.model.Descriptor;
 import hudson.util.ComboBoxModel;
 import hudson.util.FormValidation;
@@ -97,7 +94,6 @@ public final class PluginArchitectureRules {
             methods().that().areDeclaredInClassesThat().areAssignableTo(Descriptor.class)
                     .and().haveNameMatching("doCheck[A-Z].*")
                     .and().haveRawReturnType(FormValidation.class)
-                    .and().haveRawParameterTypes(ofAllowedValidationMethodSignatures())
                     .should().beAnnotatedWith(POST.class)
                     .andShould().bePublic()
                     .andShould(checkPermissions());
@@ -113,10 +109,6 @@ public final class PluginArchitectureRules {
                     .andShould().bePublic()
                     .andShould(checkPermissions());
 
-    private static FormValidationSignaturePredicate ofAllowedValidationMethodSignatures() {
-        return new FormValidationSignaturePredicate();
-    }
-
     private static HavePermissionCheck checkPermissions() {
         return new HavePermissionCheck();
     }
@@ -136,15 +128,17 @@ private static class HavePermissionCheck extends ArchCondition<JavaMethod> {
 
         @Override
         public void check(final JavaMethod item, final ConditionEvents events) {
-            Set<JavaCall<?>> callsFromSelf = item.getCallsFromSelf();
+            if (item.getModifiers().contains(JavaModifier.SYNTHETIC)) {
+                return;
+            }
 
-            if (callsFromSelf.stream().anyMatch(
+            if (item.getCallsFromSelf().stream().anyMatch(
                     javaCall -> javaCall.getTarget().getOwner().getFullName().equals(JenkinsFacade.class.getName())
                             && "hasPermission".equals(javaCall.getTarget().getName()))) {
                 return;
             }
             events.add(SimpleConditionEvent.violated(item,
-                    String.format("JenkinsFacade not called in %s in %s",
+                    String.format("JenkinsFacade.hasPermission() not called in %s in %s",
                             item.getDescription(), item.getSourceCodeLocation())));
         }
     }
@@ -163,23 +157,4 @@ public boolean apply(final JavaClass input) {
             return allowedClassNames.contains(input.getFullName());
         }
     }
-
-    private static class FormValidationSignaturePredicate extends DescribedPredicate<List<JavaClass>> {
-        FormValidationSignaturePredicate() {
-            super("do* method signatures that should be guarded by @POST");
-        }
-
-        @Override
-        public boolean apply(final List<JavaClass> input) {
-            List<String> qualifiedNames = input.stream()
-                    .map(JavaClass::getFullName).collect(Collectors.toList());
-            return qualifiedNames.equals(asList(String.class))
-                    || qualifiedNames.equals(asList(String.class, AbstractProject.class))
-                    || qualifiedNames.equals(asList(AbstractProject.class, String.class));
-        }
-
-        private List<String> asList(final Class<?>... parameterClasses) {
-            return Arrays.stream(parameterClasses).map(Class::getName).collect(Collectors.toList());
-        }
-    }
 }