From 917cfeeae19c0b2ccefa1ae759a52dc31feaabca Mon Sep 17 00:00:00 2001
From: Luis Toledo <luis@variacode.com>
Date: Fri, 21 Oct 2022 16:22:21 -0300
Subject: [PATCH 1/4] check Artifact permissions in OptionProvider add tests

---
 pom.xml                                       |  31 ++++
 .../plugins/rundeck/OptionProvider.java       |  32 +++-
 .../plugins/rundeck/OptionProviderSpec.groovy | 158 ++++++++++++++++++
 3 files changed, 220 insertions(+), 1 deletion(-)
 create mode 100644 src/test/groovy/jenkinsci/plugins/rundeck/OptionProviderSpec.groovy

diff --git a/pom.xml b/pom.xml
index 76a846a6..ed562f87 100644
--- a/pom.xml
+++ b/pom.xml
@@ -173,12 +173,43 @@
                 </exclusion>
             </exclusions>
         </dependency>
+        <!-- https://mvnrepository.com/artifact/cglib/cglib-nodep -->
+        <dependency>
+            <groupId>cglib</groupId>
+            <artifactId>cglib-nodep</artifactId>
+            <version>3.3.0</version>
+            <scope>test</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>cglib</groupId>
+            <artifactId>cglib</artifactId>
+            <version>3.3.0</version>
+            <scope>test</scope>
+        </dependency>
+
+        <!-- https://mvnrepository.com/artifact/org.objenesis/objenesis -->
+        <dependency>
+            <groupId>org.objenesis</groupId>
+            <artifactId>objenesis</artifactId>
+            <version>3.3</version>
+            <scope>test</scope>
+        </dependency>
+
         <dependency>
             <groupId>org.spockframework</groupId>
             <artifactId>spock-core</artifactId>
             <version>1.3-groovy-2.5</version>
             <scope>test</scope>
         </dependency>
+
+        <dependency>
+            <groupId>com.homeaway.devtools.jenkins</groupId>
+            <artifactId>jenkins-spock</artifactId>
+            <scope>test</scope>
+            <version>2.1.5</version>
+        </dependency>
+
         <dependency>
             <groupId>com.google.code.gson</groupId>
             <artifactId>gson</artifactId>
diff --git a/src/main/java/org/jenkinsci/plugins/rundeck/OptionProvider.java b/src/main/java/org/jenkinsci/plugins/rundeck/OptionProvider.java
index 68c6901f..6a442ed6 100644
--- a/src/main/java/org/jenkinsci/plugins/rundeck/OptionProvider.java
+++ b/src/main/java/org/jenkinsci/plugins/rundeck/OptionProvider.java
@@ -1,6 +1,5 @@
 package org.jenkinsci.plugins.rundeck;
 
-import hudson.model.AbstractProject;
 import hudson.model.Hudson;
 import hudson.model.Item;
 import hudson.model.ItemGroup;
@@ -21,6 +20,8 @@
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 
+import static hudson.model.Run.ARTIFACTS;
+
 /**
  * Option provider for Rundeck - see http://rundeck.org/docs/manual/jobs.html#option-model-provider
  *
@@ -60,6 +61,13 @@ public void doArtifact(StaplerRequest request, StaplerResponse response) throws
             return;
         }
 
+        try {
+            build.checkPermission(ARTIFACTS);
+        }catch (Exception e){
+            response.sendError(HttpServletResponse.SC_BAD_REQUEST, e.getMessage());
+            return;
+        }
+
         List<Option> options = new ArrayList<OptionProvider.Option>();
         for (Artifact artifact : build.getArtifacts()) {
             if (artifactPattern == null || artifactPattern.matcher(artifact.getFileName()).matches()) {
@@ -147,7 +155,14 @@ public void doBuild(StaplerRequest request, StaplerResponse response) throws IOE
         List<Option> options = new ArrayList<OptionProvider.Option>();
         RunList<?> builds = project.getBuilds();
         for (Run<?, ?> build : builds) {
+            try {
+                build.checkPermission(ARTIFACTS);
+            }catch (Exception e){
+                continue;
+            }
+
             Artifact artifact = findArtifact(artifactName, artifactPattern, build);
+
             if (artifact != null) {
                 String buildName = build.getDisplayName();
                 options.add(new Option(buildName, buildArtifactUrl(build, artifact)));
@@ -161,6 +176,11 @@ public void doBuild(StaplerRequest request, StaplerResponse response) throws IOE
         // add optional references to last / lastStable / lastSuccessful builds
         if (Boolean.valueOf(request.getParameter("includeLastStableBuild"))) {
             Run<?, ?> build = project.getLastStableBuild();
+            try {
+                build.checkPermission(ARTIFACTS);
+            }catch (Exception e){
+                response.sendError(HttpServletResponse.SC_BAD_REQUEST, e.getMessage());
+            }
             Artifact artifact = findArtifact(artifactName, artifactPattern, build);
             if (build != null && artifact != null) {
                 options.add(0, new Option("lastStableBuild", buildArtifactUrl(build, artifact)));
@@ -168,6 +188,11 @@ public void doBuild(StaplerRequest request, StaplerResponse response) throws IOE
         }
         if (Boolean.valueOf(request.getParameter("includeLastSuccessfulBuild"))) {
             Run<?, ?> build = project.getLastSuccessfulBuild();
+            try {
+                build.checkPermission(ARTIFACTS);
+            }catch (Exception e){
+                response.sendError(HttpServletResponse.SC_BAD_REQUEST, e.getMessage());
+            }
             Artifact artifact = findArtifact(artifactName, artifactPattern, build);
             if (build != null && artifact != null) {
                 options.add(0, new Option("lastSuccessfulBuild", buildArtifactUrl(build, artifact)));
@@ -175,6 +200,11 @@ public void doBuild(StaplerRequest request, StaplerResponse response) throws IOE
         }
         if (Boolean.valueOf(request.getParameter("includeLastBuild"))) {
             Run<?, ?> build = project.getLastBuild();
+            try {
+                build.checkPermission(ARTIFACTS);
+            }catch (Exception e){
+                response.sendError(HttpServletResponse.SC_BAD_REQUEST, e.getMessage());
+            }
             Artifact artifact = findArtifact(artifactName, artifactPattern, build);
             if (build != null && artifact != null) {
                 options.add(0, new Option("lastBuild", buildArtifactUrl(build, artifact)));
diff --git a/src/test/groovy/jenkinsci/plugins/rundeck/OptionProviderSpec.groovy b/src/test/groovy/jenkinsci/plugins/rundeck/OptionProviderSpec.groovy
new file mode 100644
index 00000000..16ec8ac1
--- /dev/null
+++ b/src/test/groovy/jenkinsci/plugins/rundeck/OptionProviderSpec.groovy
@@ -0,0 +1,158 @@
+package jenkinsci.plugins.rundeck
+
+import hudson.model.FreeStyleBuild
+import hudson.model.Hudson
+import hudson.model.Job
+import hudson.model.Run
+import hudson.security.AccessDeniedException2
+import hudson.util.RunList
+import jenkins.model.Jenkins
+import org.jenkinsci.plugins.rundeck.OptionProvider
+import org.kohsuke.stapler.StaplerRequest
+import org.kohsuke.stapler.StaplerResponse
+import spock.lang.Specification
+
+import javax.servlet.http.HttpServletResponse
+
+import static hudson.model.Run.ARTIFACTS
+
+class OptionProviderSpec extends Specification {
+
+
+    def "test option artifact without permissions"(){
+
+        given:
+
+        final Hudson jenkins = Mock()
+
+        jenkins.getInstanceOrNull() >> jenkins
+        jenkins.getInstance() >> jenkins
+
+        Jenkins.HOLDER = new Jenkins.JenkinsHolder() {
+            Jenkins getInstance() {
+                return jenkins
+            }
+        }
+
+        jenkins.getItemByFullName("test", _)>>Mock(Job) {
+            getLastSuccessfulBuild() >> Mock(FreeStyleBuild) {
+                checkPermission(ARTIFACTS) >> { throw new AccessDeniedException2(Jenkins.getAuthentication(), ARTIFACTS) }
+            }
+        }
+
+        def response = Mock(StaplerResponse)
+
+        when:
+        OptionProvider optionProvider = new OptionProvider()
+        def request = Mock(StaplerRequest){
+            getParameter("project")>>"test"
+            getParameter("build")>>"lastSuccessful"
+        }
+        def result = optionProvider.doArtifact(request,response )
+
+        then:
+        1 * response.sendError(HttpServletResponse.SC_BAD_REQUEST, {message-> message == "anonymous is missing the Run/Artifacts permission" });
+
+    }
+
+    def "test option artifact with permissions"(){
+
+        given:
+
+        final Hudson jenkins = Mock(){
+            getRootUrl()>>"http://localhost:8080"
+        }
+
+        jenkins.getInstanceOrNull() >> jenkins
+        jenkins.getInstance() >> jenkins
+
+        Jenkins.HOLDER = new Jenkins.JenkinsHolder() {
+            Jenkins getInstance() {
+                return jenkins
+            }
+        }
+
+        jenkins.getItemByFullName("test", _)>>Mock(Job) {
+            getLastSuccessfulBuild() >> Mock(FreeStyleBuild) {
+                getArtifacts()>> [
+                        Mock(Run.Artifact){
+                            getFileName()>>"test-1.0.jar"
+                            getHref()>>"artifact/test-1.0.jar"
+                        }
+                ]
+                getUrl()>>"/rundeck/"
+            }
+        }
+
+        def writer = Mock(PrintWriter)
+        def response = Mock(StaplerResponse){
+            getWriter()>>writer
+        }
+
+        when:
+        OptionProvider optionProvider = new OptionProvider()
+        def request = Mock(StaplerRequest){
+            getParameter("project")>>"test"
+            getParameter("build")>>"lastSuccessful"
+        }
+        optionProvider.doArtifact(request,response )
+
+        then:
+        1*writer.append({json-> json == "[{\"name\":\"test-1.0.jar\",\"value\":\"http://localhost:8080/rundeck/artifact/artifact/test-1.0.jar\"}]"})
+
+    }
+
+    def "test option build without permissions"(){
+
+        given:
+
+        final Hudson jenkins = Mock()
+
+        jenkins.getInstanceOrNull() >> jenkins
+        jenkins.getInstance() >> jenkins
+
+        Jenkins.HOLDER = new Jenkins.JenkinsHolder() {
+            Jenkins getInstance() {
+                return jenkins
+            }
+        }
+
+        List builds = [
+                Mock(FreeStyleBuild) {
+                    getArtifacts()>> [
+                            Mock(Run.Artifact){
+                                getFileName()>>"test-1.0.jar"
+                                getHref()>>"artifact/test-1.0.jar"
+                            }
+                    ]
+                    getUrl()>>"/rundeck/"
+                    checkPermission(ARTIFACTS) >> { throw new AccessDeniedException2(Jenkins.getAuthentication(), ARTIFACTS) }
+
+                }
+        ]
+
+        jenkins.getItemByFullName("test", _)>>Mock(Job) {
+            getBuilds()>> Mock(RunList){
+                iterator()>>builds.iterator()
+            }
+        }
+
+        def writer = Mock(PrintWriter)
+        def response = Mock(StaplerResponse){
+            getWriter()>>writer
+        }
+
+        when:
+        OptionProvider optionProvider = new OptionProvider()
+        def request = Mock(StaplerRequest){
+            getParameter("project")>>"test"
+            getParameter("artifactRegex")>>".jar"
+            getParameter("build")>>"lastSuccessful"
+        }
+        def result = optionProvider.doBuild(request,response )
+
+        then:
+        1*writer.append({json-> json == "[]"})
+
+    }
+}
\ No newline at end of file

From 73ce5bf872857dda47db0984ffea8bbe7ea856cf Mon Sep 17 00:00:00 2001
From: Luis Toledo <luis@variacode.com>
Date: Fri, 21 Oct 2022 16:27:46 -0300
Subject: [PATCH 2/4] cleaning pom.xml

---
 pom.xml | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/pom.xml b/pom.xml
index ed562f87..ac2d3d72 100644
--- a/pom.xml
+++ b/pom.xml
@@ -203,13 +203,6 @@
             <scope>test</scope>
         </dependency>
 
-        <dependency>
-            <groupId>com.homeaway.devtools.jenkins</groupId>
-            <artifactId>jenkins-spock</artifactId>
-            <scope>test</scope>
-            <version>2.1.5</version>
-        </dependency>
-
         <dependency>
             <groupId>com.google.code.gson</groupId>
             <artifactId>gson</artifactId>

From 9a7f7364cc68622b0b6ec1daa77efaabc04a92b4 Mon Sep 17 00:00:00 2001
From: Luis Toledo <luis@variacode.com>
Date: Fri, 21 Oct 2022 16:56:24 -0300
Subject: [PATCH 3/4] fix test

---
 .../plugins/rundeck/OptionProviderSpec.groovy   | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/test/groovy/jenkinsci/plugins/rundeck/OptionProviderSpec.groovy b/src/test/groovy/jenkinsci/plugins/rundeck/OptionProviderSpec.groovy
index 16ec8ac1..0a36114e 100644
--- a/src/test/groovy/jenkinsci/plugins/rundeck/OptionProviderSpec.groovy
+++ b/src/test/groovy/jenkinsci/plugins/rundeck/OptionProviderSpec.groovy
@@ -18,16 +18,26 @@ import static hudson.model.Run.ARTIFACTS
 
 class OptionProviderSpec extends Specification {
 
+    def originalHolder
+
+    def setup() {
+         originalHolder = Jenkins.HOLDER
+    }
+
+    def cleanup() {
+        Jenkins.HOLDER = originalHolder
+    }
 
     def "test option artifact without permissions"(){
 
         given:
 
         final Hudson jenkins = Mock()
-
         jenkins.getInstanceOrNull() >> jenkins
         jenkins.getInstance() >> jenkins
 
+        def originalHolder = Jenkins.HOLDER
+
         Jenkins.HOLDER = new Jenkins.JenkinsHolder() {
             Jenkins getInstance() {
                 return jenkins
@@ -48,11 +58,13 @@ class OptionProviderSpec extends Specification {
             getParameter("project")>>"test"
             getParameter("build")>>"lastSuccessful"
         }
-        def result = optionProvider.doArtifact(request,response )
+
+        optionProvider.doArtifact(request,response )
 
         then:
         1 * response.sendError(HttpServletResponse.SC_BAD_REQUEST, {message-> message == "anonymous is missing the Run/Artifacts permission" });
 
+
     }
 
     def "test option artifact with permissions"(){
@@ -155,4 +167,5 @@ class OptionProviderSpec extends Specification {
         1*writer.append({json-> json == "[]"})
 
     }
+
 }
\ No newline at end of file

From dd97ee27e4efa91bdee6ffdc11e4caa31b5cd796 Mon Sep 17 00:00:00 2001
From: Luis Toledo <luis@variacode.com>
Date: Wed, 26 Oct 2022 10:22:42 -0300
Subject: [PATCH 4/4] refactoring

---
 .../plugins/rundeck/OptionProvider.java       | 36 ++++++++-----------
 .../plugins/rundeck/OptionProviderSpec.groovy |  1 +
 2 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/src/main/java/org/jenkinsci/plugins/rundeck/OptionProvider.java b/src/main/java/org/jenkinsci/plugins/rundeck/OptionProvider.java
index 6a442ed6..1e1faba8 100644
--- a/src/main/java/org/jenkinsci/plugins/rundeck/OptionProvider.java
+++ b/src/main/java/org/jenkinsci/plugins/rundeck/OptionProvider.java
@@ -1,5 +1,6 @@
 package org.jenkinsci.plugins.rundeck;
 
+import hudson.Functions;
 import hudson.model.Hudson;
 import hudson.model.Item;
 import hudson.model.ItemGroup;
@@ -62,7 +63,7 @@ public void doArtifact(StaplerRequest request, StaplerResponse response) throws
         }
 
         try {
-            build.checkPermission(ARTIFACTS);
+            this.checkArtifactPermissions(build);
         }catch (Exception e){
             response.sendError(HttpServletResponse.SC_BAD_REQUEST, e.getMessage());
             return;
@@ -155,12 +156,6 @@ public void doBuild(StaplerRequest request, StaplerResponse response) throws IOE
         List<Option> options = new ArrayList<OptionProvider.Option>();
         RunList<?> builds = project.getBuilds();
         for (Run<?, ?> build : builds) {
-            try {
-                build.checkPermission(ARTIFACTS);
-            }catch (Exception e){
-                continue;
-            }
-
             Artifact artifact = findArtifact(artifactName, artifactPattern, build);
 
             if (artifact != null) {
@@ -176,11 +171,6 @@ public void doBuild(StaplerRequest request, StaplerResponse response) throws IOE
         // add optional references to last / lastStable / lastSuccessful builds
         if (Boolean.valueOf(request.getParameter("includeLastStableBuild"))) {
             Run<?, ?> build = project.getLastStableBuild();
-            try {
-                build.checkPermission(ARTIFACTS);
-            }catch (Exception e){
-                response.sendError(HttpServletResponse.SC_BAD_REQUEST, e.getMessage());
-            }
             Artifact artifact = findArtifact(artifactName, artifactPattern, build);
             if (build != null && artifact != null) {
                 options.add(0, new Option("lastStableBuild", buildArtifactUrl(build, artifact)));
@@ -188,11 +178,6 @@ public void doBuild(StaplerRequest request, StaplerResponse response) throws IOE
         }
         if (Boolean.valueOf(request.getParameter("includeLastSuccessfulBuild"))) {
             Run<?, ?> build = project.getLastSuccessfulBuild();
-            try {
-                build.checkPermission(ARTIFACTS);
-            }catch (Exception e){
-                response.sendError(HttpServletResponse.SC_BAD_REQUEST, e.getMessage());
-            }
             Artifact artifact = findArtifact(artifactName, artifactPattern, build);
             if (build != null && artifact != null) {
                 options.add(0, new Option("lastSuccessfulBuild", buildArtifactUrl(build, artifact)));
@@ -200,11 +185,6 @@ public void doBuild(StaplerRequest request, StaplerResponse response) throws IOE
         }
         if (Boolean.valueOf(request.getParameter("includeLastBuild"))) {
             Run<?, ?> build = project.getLastBuild();
-            try {
-                build.checkPermission(ARTIFACTS);
-            }catch (Exception e){
-                response.sendError(HttpServletResponse.SC_BAD_REQUEST, e.getMessage());
-            }
             Artifact artifact = findArtifact(artifactName, artifactPattern, build);
             if (build != null && artifact != null) {
                 options.add(0, new Option("lastBuild", buildArtifactUrl(build, artifact)));
@@ -278,6 +258,12 @@ private Artifact findArtifact(String artifactName, Pattern artifactPattern, Run<
             return null;
         }
 
+        try{
+            this.checkArtifactPermissions(build);
+        }catch (Exception e){
+            return null;
+        }
+
         for (Artifact artifact : build.getArtifacts()) {
             if (StringUtils.equals(artifactName, artifact.getFileName())) {
                 return artifact;
@@ -320,6 +306,12 @@ private void writeJson(List<Option> options, StaplerResponse response) throws IO
         response.getWriter().append(json);
     }
 
+    private void checkArtifactPermissions(Run<?, ?> build){
+        if(Functions.isArtifactsPermissionEnabled()){
+            build.checkPermission(ARTIFACTS);
+        }
+    }
+
     /**
      * Javabean representation of an option
      */
diff --git a/src/test/groovy/jenkinsci/plugins/rundeck/OptionProviderSpec.groovy b/src/test/groovy/jenkinsci/plugins/rundeck/OptionProviderSpec.groovy
index 0a36114e..750fd5b2 100644
--- a/src/test/groovy/jenkinsci/plugins/rundeck/OptionProviderSpec.groovy
+++ b/src/test/groovy/jenkinsci/plugins/rundeck/OptionProviderSpec.groovy
@@ -22,6 +22,7 @@ class OptionProviderSpec extends Specification {
 
     def setup() {
          originalHolder = Jenkins.HOLDER
+        System.properties['hudson.security.ArtifactsPermission'] = "true"
     }
 
     def cleanup() {