Merge pull request #70 from tfennelly/JENKINS-26363

[FIXED JENKINS-26363] Input step should permit cancellation from anyone with Job.CANCEL permission
jenkinsci · Mar 4, 2015 · eaa8f80 · eaa8f80
2 parents 729984d + 026ff61
commit eaa8f80
Show file tree

Hide file tree

Showing 3 changed files with 126 additions and 2 deletions.
diff --git a/aggregator/src/test/java/org/jenkinsci/plugins/workflow/steps/input/InputStepTest.java b/aggregator/src/test/java/org/jenkinsci/plugins/workflow/steps/input/InputStepTest.java
@@ -2,8 +2,16 @@
 
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
 import hudson.model.BooleanParameterDefinition;
+import hudson.model.Job;
+import hudson.model.User;
 import hudson.model.queue.QueueTaskFuture;
+
 import java.util.Arrays;
+import java.util.List;
+
+import hudson.security.ACL;
+import hudson.security.GlobalMatrixAuthorizationStrategy;
+import jenkins.model.Jenkins;
 import org.apache.commons.lang.StringUtils;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowExecution;
@@ -14,6 +22,7 @@
 import org.junit.Assert;
 import org.junit.Rule;
 import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 
 /**
@@ -65,4 +74,77 @@ public void parameter() throws Exception {
         System.out.println(b.getLog());
         assertTrue(b.getLog().contains("after: false"));
     }
+
+    @Test
+    @Issue("JENKINS-26363")
+    public void test_cancel_run_by_input() throws Exception {
+        JenkinsRule.WebClient webClient = j.createWebClient();
+        JenkinsRule.DummySecurityRealm dummySecurityRealm = j.createDummySecurityRealm();
+        GlobalMatrixAuthorizationStrategy authorizationStrategy = new GlobalMatrixAuthorizationStrategy();
+
+        j.jenkins.setSecurityRealm(dummySecurityRealm);
+
+        // Only give "alice" basic privs. That's normally not enough to Job.CANCEL, only for the fact that "alice"
+        // is listed as the submitter.
+        addUserWithPrivs("alice", authorizationStrategy);
+        // Only give "bob" basic privs.  That's normally not enough to Job.CANCEL and "bob" is not the submitter,
+        // so they should be rejected.
+        addUserWithPrivs("bob", authorizationStrategy);
+        // Give "charlie" basic privs + Job.CANCEL.  That should allow user3 cancel.
+        addUserWithPrivs("charlie", authorizationStrategy);
+        authorizationStrategy.add(Job.CANCEL, "charlie");
+
+        j.jenkins.setAuthorizationStrategy(authorizationStrategy);
+
+        final WorkflowJob foo = j.jenkins.createProject(WorkflowJob.class, "foo");
+        ACL.impersonate(User.get("alice").impersonate(), new Runnable() {
+            @Override
+            public void run() {
+                foo.setDefinition(new CpsFlowDefinition("input id: 'InputX', message: 'OK?', ok: 'Yes', submitter: 'alice'"));
+            }
+        });
+
+        runAndAbort(webClient, foo, "alice", true);   // alice should work coz she's declared as 'submitter'
+        runAndAbort(webClient, foo, "bob", false);    // bob shouldn't work coz he's not declared as 'submitter' and doesn't have Job.CANCEL privs
+        runAndAbort(webClient, foo, "charlie", true); // charlie should work coz he has Job.CANCEL privs
+    }
+
+    private void runAndAbort(JenkinsRule.WebClient webClient, WorkflowJob foo, String loginAs, boolean expectAbortOk) throws Exception {
+        // get the build going, and wait until workflow pauses
+        QueueTaskFuture<WorkflowRun> queueTaskFuture = foo.scheduleBuild2(0);
+        WorkflowRun run = queueTaskFuture.getStartCondition().get();
+        CpsFlowExecution execution = (CpsFlowExecution) run.getExecutionPromise().get();
+
+        while (run.getAction(InputAction.class) == null) {
+            execution.waitForSuspension();
+        }
+
+        webClient.login(loginAs);
+
+        InputAction inputAction = run.getAction(InputAction.class);
+        InputStepExecution is = inputAction.getExecution("InputX");
+        HtmlPage p = webClient.getPage(run, inputAction.getUrlName());
+
+        try {
+            j.submit(p.getFormByName(is.getId()), "abort");
+            assertEquals(0, inputAction.getExecutions().size());
+            queueTaskFuture.get();
+
+            List<String> log = run.getLog(1000);
+            System.out.println(log);
+            assertTrue(expectAbortOk);
+            assertEquals("Finished: ABORTED", log.get(log.size() - 1)); // Should be aborted
+        } catch (Exception e) {
+            List<String> log = run.getLog(1000);
+            System.out.println(log);
+            assertFalse(expectAbortOk);
+            assertEquals("Yes or Abort", log.get(log.size() - 1));  // Should still be paused at input
+        }
+    }
+
+    private void addUserWithPrivs(String username, GlobalMatrixAuthorizationStrategy authorizationStrategy) {
+        authorizationStrategy.add(Jenkins.READ, username);
+        authorizationStrategy.add(Jenkins.RUN_SCRIPTS, username);
+        authorizationStrategy.add(Job.READ, username);
+    }
 }
diff --git a/support/src/main/java/org/jenkinsci/plugins/workflow/support/steps/input/InputStep.java b/support/src/main/java/org/jenkinsci/plugins/workflow/support/steps/input/InputStep.java
@@ -105,6 +105,7 @@ public String getMessage() {
         return message;
     }
 
+    @Deprecated
     public boolean canSubmit() {
         Authentication a = Jenkins.getAuthentication();
         return canSettle(a);
@@ -113,6 +114,7 @@ public boolean canSubmit() {
     /**
      * Checks if the given user can settle this input.
      */
+    @Deprecated
     public boolean canSettle(Authentication a) {
         if (submitter==null || a.getName().equals(submitter))
             return true;

diff --git a/.../src/main/java/org/jenkinsci/plugins/workflow/support/steps/input/InputStepExecution.java b/.../src/main/java/org/jenkinsci/plugins/workflow/support/steps/input/InputStepExecution.java
@@ -6,6 +6,7 @@
 import hudson.console.HyperlinkNote;
 import hudson.model.Failure;
 import hudson.model.FileParameterValue;
+import hudson.model.Job;
 import hudson.model.ModelObject;
 import hudson.model.ParameterDefinition;
 import hudson.model.ParameterValue;
@@ -14,8 +15,11 @@
 import hudson.model.TaskListener;
 import hudson.model.User;
 import hudson.util.HttpResponses;
+import jenkins.model.Jenkins;
 import net.sf.json.JSONArray;
 import net.sf.json.JSONObject;
+import org.acegisecurity.Authentication;
+import org.acegisecurity.GrantedAuthority;
 import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl;
 import org.jenkinsci.plugins.workflow.support.actions.PauseAction;
 import org.jenkinsci.plugins.workflow.graph.FlowNode;
@@ -166,7 +170,7 @@ public HttpResponse doProceedEmpty() throws IOException {
      */
     @RequirePOST
     public HttpResponse doAbort() throws IOException, ServletException {
-        preSubmissionCheck();
+        preAbortCheck();
 
         FlowInterruptedException e = new FlowInterruptedException(Result.ABORTED, new Rejection(User.current()));
         outcome = new Outcome(null,e);
@@ -179,13 +183,24 @@ public HttpResponse doAbort() throws IOException, ServletException {
         return HttpResponses.ok();
     }
 
+    /**
+     * Check if the current user can abort/cancel the run from the input.
+     */
+    private void preAbortCheck() {
+        if (isSettled()) {
+            throw new Failure("This input has been already given");
+        } if (!canCancel() && !canSubmit()) {
+            throw new Failure("You need to be '"+ input.getSubmitter() +"' (or have Job CANCEL permissions) to cancel this.");
+        }
+    }
+
     /**
      * Check if the current user can submit the input.
      */
     private void preSubmissionCheck() {
         if (isSettled())
             throw new Failure("This input has been already given");
-        if (!input.canSubmit()) {
+        if (!canSubmit()) {
             throw new Failure("You need to be "+ input.getSubmitter() +" to submit this");
         }
     }
@@ -199,6 +214,31 @@ private void postSettlement() throws IOException {
         }
     }
 
+    private boolean canCancel() {
+        return getRun().getParent().hasPermission(Job.CANCEL);
+    }
+
+    private boolean canSubmit() {
+        Authentication a = Jenkins.getAuthentication();
+        return canSettle(a);
+    }
+
+    /**
+     * Checks if the given user can settle this input.
+     */
+    private boolean canSettle(Authentication a) {
+        String submitter = input.getSubmitter();
+        if (submitter==null || a.getName().equals(submitter)) {
+            return true;
+        }
+        for (GrantedAuthority ga : a.getAuthorities()) {
+            if (ga.getAuthority().equals(submitter)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     /**
      * Parse the submitted {@link ParameterValue}s
      */