From 93c2851cf225c480ffd43494b33e38ca543ef993 Mon Sep 17 00:00:00 2001 From: Ulli Hafner Date: Wed, 6 Jan 2021 21:09:39 +0100 Subject: [PATCH 1/2] Ignore `SYNTHETIC` methods when checking for permissions. --- .../plugins/util/PluginArchitectureRules.java | 29 +++---------------- 1 file changed, 4 insertions(+), 25 deletions(-) diff --git a/src/test/java/io/jenkins/plugins/util/PluginArchitectureRules.java b/src/test/java/io/jenkins/plugins/util/PluginArchitectureRules.java index 438b53a..0e6d0a9 100644 --- a/src/test/java/io/jenkins/plugins/util/PluginArchitectureRules.java +++ b/src/test/java/io/jenkins/plugins/util/PluginArchitectureRules.java @@ -19,7 +19,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 +96,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 +111,6 @@ public final class PluginArchitectureRules { .andShould().bePublic() .andShould(checkPermissions()); - private static FormValidationSignaturePredicate ofAllowedValidationMethodSignatures() { - return new FormValidationSignaturePredicate(); - } - private static HavePermissionCheck checkPermissions() { return new HavePermissionCheck(); } @@ -138,6 +132,10 @@ private static class HavePermissionCheck extends ArchCondition { public void check(final JavaMethod item, final ConditionEvents events) { Set> callsFromSelf = item.getCallsFromSelf(); + if (item.getModifiers().contains(JavaModifier.SYNTHETIC)) { + return; + } + if (callsFromSelf.stream().anyMatch( javaCall -> javaCall.getTarget().getOwner().getFullName().equals(JenkinsFacade.class.getName()) && "hasPermission".equals(javaCall.getTarget().getName()))) { @@ -163,23 +161,4 @@ public boolean apply(final JavaClass input) { return allowedClassNames.contains(input.getFullName()); } } - - private static class FormValidationSignaturePredicate extends DescribedPredicate> { - FormValidationSignaturePredicate() { - super("do* method signatures that should be guarded by @POST"); - } - - @Override - public boolean apply(final List input) { - List 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 asList(final Class... parameterClasses) { - return Arrays.stream(parameterClasses).map(Class::getName).collect(Collectors.toList()); - } - } } From 7d9c9c1bd71983d8f9519c4cae6c9b428d9d1704 Mon Sep 17 00:00:00 2001 From: Ulli Hafner Date: Wed, 6 Jan 2021 21:26:22 +0100 Subject: [PATCH 2/2] Improve logging. --- .../io/jenkins/plugins/util/PluginArchitectureRules.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/test/java/io/jenkins/plugins/util/PluginArchitectureRules.java b/src/test/java/io/jenkins/plugins/util/PluginArchitectureRules.java index 0e6d0a9..f216d36 100644 --- 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; @@ -130,19 +128,17 @@ private static class HavePermissionCheck extends ArchCondition { @Override public void check(final JavaMethod item, final ConditionEvents events) { - Set> 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()))); } }