Move testDoNotifyCommitWithInvalidApiToken and use JenkinsRule (#1621)

* Remove unused GitSCMTest methods

* Add notifyCommit method with token argument

Allow callers to provide their own access token

* Move testDoNotifyCommitWithInvalidApiToken and use JenkinsRule

The GitStepTest already includes logic that uses notifyCommit to start
jobs and perform other tests of notifyCommit.  Use that logic to
replace the mocked testDoNotifyCommitWithInvalidApiToken with a test
that uses web requests to a JenkinsRule based test.

Prepare for Jetty 12 EE 9 transition.

* Move new test to end of file

src/test/java/hudson/plugins/git/GitSCMTest.java

@@ -2641,36 +2641,6 @@ public void testPolling_environmentValueAsEnvironmentContributingAction() throws
         assertNotSame("Environment path should not be broken path", environment.get("PATH"), brokenPath);
     }
 
-    /**
-     * Method performs HTTP get on "notifyCommit" URL, passing it commit by SHA1
-     * and tests for custom SCM name build data consistency.
-     * @param project project to build
-     * @param commit commit to build
-     * @param expectedScmName Expected SCM name for commit.
-     * @param ordinal number of commit to log into errors, if any
-     * @param git git SCM
-     * @throws Exception on error
-     */
-    private int notifyAndCheckScmName(FreeStyleProject project, ObjectId commit,
-            String expectedScmName, int ordinal, GitSCM git, ObjectId... priorCommits) throws Exception {
-        StringBuilder priorCommitIDs = new StringBuilder();
-        for (ObjectId priorCommit : priorCommits) {
-            priorCommitIDs.append(" ").append(priorCommit);
-        }
-        assertTrue("scm polling should detect commit " + ordinal, notifyCommit(project, commit));
-
-        final Build build = project.getLastBuild();
-        final BuildData buildData = git.getBuildData(build);
-        assertEquals("Expected SHA1 != built SHA1 for commit " + ordinal + " priors:" + priorCommitIDs, commit, buildData
-                .getLastBuiltRevision().getSha1());
-        assertEquals("Expected SHA1 != retrieved SHA1 for commit " + ordinal + " priors:" + priorCommitIDs, commit, buildData.getLastBuild(commit).getSHA1());
-        assertTrue("Commit " + ordinal + " not marked as built", buildData.hasBeenBuilt(commit));
-
-        assertEquals("Wrong SCM Name for commit " + ordinal, expectedScmName, buildData.getScmName());
-
-        return build.getNumber();
-    }
-
     private void checkNumberedBuildScmName(FreeStyleProject project, int buildNumber,
             String expectedScmName, GitSCM git) throws Exception {
 
@@ -2945,67 +2915,6 @@ public void testCommitMessageIsPrintedToLogs() throws Exception {
         r.waitForMessage("Commit message: \"test commit\"", run);
     }
 
-    /**
-     * Method performs HTTP get on "notifyCommit" URL, passing it commit by SHA1
-     * and tests for build data consistency.
-     * @param project project to build
-     * @param commit commit to build
-     * @param expectedBranch branch, that is expected to be built
-     * @param ordinal number of commit to log into errors, if any
-     * @param git git SCM
-     * @throws Exception on error
-     */
-    private void notifyAndCheckBranch(FreeStyleProject project, ObjectId commit,
-            String expectedBranch, int ordinal, GitSCM git) throws Exception {
-        assertTrue("scm polling should detect commit " + ordinal, notifyCommit(project, commit));
-        final BuildData buildData = git.getBuildData(project.getLastBuild());
-        final Collection<Branch> builtBranches = buildData.lastBuild.getRevision().getBranches();
-        assertEquals("Commit " + ordinal + " should be built", commit, buildData
-                .getLastBuiltRevision().getSha1());
-
-        final String expectedBranchString = "origin/" + expectedBranch;
-        assertFalse("Branches should be detected for the build", builtBranches.isEmpty());
-        assertEquals(expectedBranch + " branch should be detected", expectedBranchString,
-                     builtBranches.iterator().next().getName());
-        assertEquals(expectedBranchString, getEnvVars(project).get(GitSCM.GIT_BRANCH));
-    }
-
-    /**
-     * Method performs commit notification for the last committed SHA1 using
-     * notifyCommit URL.
-     * @param project project to trigger
-     * @return whether the new build has been triggered (<code>true</code>) or
-     *         not (<code>false</code>).
-     * @throws Exception on error
-     */
-    private boolean notifyCommit(FreeStyleProject project, ObjectId commitId) throws Exception {
-        final int initialBuildNumber = project.getLastBuild().getNumber();
-        final String commit1 = ObjectId.toString(commitId);
-
-        final String notificationPath = r.getURL().toExternalForm()
-                + "git/notifyCommit?url=" + testRepo.gitDir.toString() + "&sha1=" + commit1;
-        final URL notifyUrl = new URL(notificationPath);
-        String notifyContent;
-        try (final InputStream is = notifyUrl.openStream()) {
-            notifyContent = new String(is.readAllBytes(), StandardCharsets.UTF_8);
-        }
-        assertThat(notifyContent, containsString("No Git consumers using SCM API plugin for: " + testRepo.gitDir.toString()));
-
-        if ((project.getLastBuild().getNumber() == initialBuildNumber)
-                && (r.jenkins.getQueue().isEmpty())) {
-            return false;
-        } else {
-            while (!r.jenkins.getQueue().isEmpty()) {
-                Thread.sleep(100);
-            }
-            final FreeStyleBuild build = project.getLastBuild();
-            while (build.isBuilding()) {
-                Thread.sleep(100);
-            }
-            return true;
-        }
-    }
-
     private void setupJGit(GitSCM git) {
         git.gitTool="jgit";
         r.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(new JGitTool(Collections.emptyList()));

src/test/java/hudson/plugins/git/GitStatusTest.java

@@ -583,13 +583,7 @@ public void testDoNotifyCommitWithValidSha1AndValidApiToken() throws Exception {
     @Test
     @Issue("SECURITY-284")
     public void testDoNotifyCommitWithInvalidApiToken() throws Exception {
-        setupProjectWithTrigger("a", "master", false);
-        StaplerResponse res = mock(StaplerResponse.class);
-
-        HttpResponse httpResponse = this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "master", null, "invalid");
-        httpResponse.generateResponse(null, res, null);
-
-        Mockito.verify(res).sendError(403, "Invalid access token");
+        // Test moved to GitStepTest#testDoNotifyCommitWithInvalidApiToken()
     }
 
     @Test

src/test/java/jenkins/plugins/git/GitSampleRepoRule.java

@@ -50,6 +50,8 @@ public final class GitSampleRepoRule extends AbstractSampleDVCSRepoRule {
 
     private static final Logger LOGGER = Logger.getLogger(GitSampleRepoRule.class.getName());
 
+    public static final String INVALID_NOTIFY_COMMIT_TOKEN = "invalid-notifyCommit-token";
+
     @Override
     public void before() throws Throwable {
         super.before();
@@ -98,18 +100,44 @@ public final boolean mkdirs(String rel) throws IOException {
         return new File(this.sampleRepo, rel).mkdirs();
     }
 
-    public void notifyCommit(JenkinsRule r) throws Exception {
-        synchronousPolling(r);
+    public String notifyCommit(JenkinsRule r) throws Exception {
         String notifyCommitToken = ApiTokenPropertyConfiguration.get().generateApiToken("notifyCommit").getString("value");
-        WebResponse webResponse = r.createWebClient()
-                .goTo("git/notifyCommit?url=" + bareUrl() + "&token=" + notifyCommitToken, "text/plain").getWebResponse();
-        LOGGER.log(Level.FINE, webResponse.getContentAsString());
+        return notifyCommit(r, notifyCommitToken);
+    }
+
+    public String notifyCommit(JenkinsRule r, String notifyCommitToken) throws Exception {
+        /* If the caller expects an error and does not want an
+         * exception thrown by the web response, the notifyCommitToken
+         * must contain the invalid notifyCommit token string */
+        boolean expectError = notifyCommitToken.contains(INVALID_NOTIFY_COMMIT_TOKEN);
+        synchronousPolling(r);
+        JenkinsRule.WebClient webClient = r.createWebClient();
+        if (expectError) {
+            /* Return without exception on failing status code */
+            webClient.getOptions().setThrowExceptionOnFailingStatusCode(false);
+            /* Do not clutter output with failures that are expected and checked by the caller */
+            webClient.getOptions().setPrintContentOnFailingStatusCode(false);
+        }
+        String responseFormat = expectError ? "text/html" : "text/plain";
+
+        WebResponse webResponse = webClient.goTo("git/notifyCommit?url=" + bareUrl() + "&token=" + notifyCommitToken, responseFormat).getWebResponse();
+        StringBuilder sb = new StringBuilder(webResponse.getContentAsString());
+        if (!expectError) {
+            LOGGER.log(Level.FINE, sb.toString());
+        }
+
         for (NameValuePair pair : webResponse.getResponseHeaders()) {
             if (pair.getName().equals("Triggered")) {
-                LOGGER.log(Level.FINE, "Triggered: " + pair.getValue());
+                sb.append('\n');
+                sb.append("Triggered: ");
+                sb.append(pair.getValue());
+                if (!expectError) {
+                    LOGGER.log(Level.FINE, "Triggered: " + pair.getValue());
+                }
             }
         }
         r.waitUntilNoActivity();
+        return sb.toString();
     }
 
     public String head() throws Exception {

src/test/java/jenkins/plugins/git/GitStepTest.java

@@ -44,7 +44,12 @@
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
 import org.jenkinsci.plugins.workflow.steps.Step;
 import org.jenkinsci.plugins.workflow.steps.StepConfigTester;
-import static org.junit.Assert.*;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assume.assumeTrue;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
@@ -292,4 +297,19 @@ public void commitToWorkspace() throws Exception {
         r.waitForMessage("+edited by build", b);
     }
 
+    @Test
+    @Issue("SECURITY-284")
+    public void testDoNotifyCommitWithInvalidApiToken() throws Exception {
+        assumeTrue("Test class max time " + MAX_SECONDS_FOR_THESE_TESTS + " exceeded", isTimeAvailable());
+        sampleRepo.init();
+        WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "demo");
+        p.addTrigger(new SCMTrigger("")); // no schedule, use notifyCommit only
+        p.setDefinition(new CpsFlowDefinition(
+            "node {\n" +
+            "    error('this echo should never be called')\n" +
+            "}", true));
+        String response = sampleRepo.notifyCommit(r, GitSampleRepoRule.INVALID_NOTIFY_COMMIT_TOKEN);
+        assertThat(response, containsString("Invalid access token"));
+    }
+
 }