diff --git a/README.adoc b/README.adoc index b342918b26..7d80f5a6c0 100644 --- a/README.adoc +++ b/README.adoc @@ -175,10 +175,10 @@ Refer to webhook documentation for your repository: * link:https://github.com/jenkinsci/gitea-plugin/blob/master/docs/README.md[Gitea] Other git repositories can use a link:https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks[post-receive hook] in the remote repository to notify Jenkins of changes. -Add the following line in your `hooks/post-receive` file on the git server, replacing with the fully qualified URL you use when cloning the repository. +Add the following line in your `hooks/post-receive` file on the git server, replacing with the fully qualified URL you use when cloning the repository, and replacing with a token generated by a Jenkins administrator using the "Git plugin notifyCommit access tokens" section of the "Configure Global Security" page. .... -curl http://yourserver/git/notifyCommit?url= +curl http://yourserver/git/notifyCommit?url=&token= .... This will scan all the jobs that: @@ -191,8 +191,19 @@ If polling finds a change worthy of a build, a build will be triggered. This allows a notify script to remain the same for all Jenkins jobs. Or if you have multiple repositories under a single repository host application (such as Gitosis), you can share a single post-receive hook script with all the repositories. -Finally, this URL doesn't require authentication even for secured Jenkins, because the server doesn't directly use anything that the client is sending. -It polls to verify that there is a change before it actually starts a build. + +The `token` parameter is required by default as a security measure, but can be disabled by the following link:https://www.jenkins.io/doc/book/managing/system-properties/[system property]: + +.... +hudson.plugins.git.GitStatus.NOTIFY_COMMIT_ACCESS_CONTROL +.... + +It has two modes: + +* `disabled-for-polling` - Allows unauthenticated requests as long as they only request polling of the repository supplied in the `url` query parameter. Prohibits unauthenticated requests that attempt to schedule a build immediately by providing a +`sha1` query parameter. +* `disabled` - Fully disables the access token mechanism and allows all requests to `notifyCommit` +to be unauthenticated. *This option is insecure and is not recommended.* When notifyCommit is successful, the list of triggered projects is returned. diff --git a/src/main/java/hudson/plugins/git/ApiTokenPropertyConfiguration.java b/src/main/java/hudson/plugins/git/ApiTokenPropertyConfiguration.java new file mode 100644 index 0000000000..d69a0402c0 --- /dev/null +++ b/src/main/java/hudson/plugins/git/ApiTokenPropertyConfiguration.java @@ -0,0 +1,183 @@ +package hudson.plugins.git; + +import edu.umd.cs.findbugs.annotations.NonNull; +import hudson.Extension; +import hudson.Util; +import hudson.model.PersistentDescriptor; +import hudson.util.HttpResponses; +import jenkins.model.GlobalConfiguration; +import jenkins.model.GlobalConfigurationCategory; +import jenkins.model.Jenkins; +import net.jcip.annotations.GuardedBy; +import net.sf.json.JSONObject; +import org.apache.commons.lang.StringUtils; +import org.jenkinsci.Symbol; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; +import org.kohsuke.stapler.HttpResponse; +import org.kohsuke.stapler.StaplerRequest; +import org.kohsuke.stapler.interceptor.RequirePOST; + +import java.io.Serializable; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.UUID; +import java.util.logging.Level; +import java.util.logging.Logger; + + +@Extension +@Restricted(NoExternalUse.class) +@Symbol("apiTokenProperty") +public class ApiTokenPropertyConfiguration extends GlobalConfiguration implements PersistentDescriptor { + + private static final Logger LOGGER = Logger.getLogger(ApiTokenPropertyConfiguration.class.getName()); + private static final SecureRandom RANDOM = new SecureRandom(); + private static final String HASH_ALGORITHM = "SHA-256"; + + @GuardedBy("this") + private final List apiTokens; + + public ApiTokenPropertyConfiguration() { + this.apiTokens = new ArrayList<>(); + } + + public static ApiTokenPropertyConfiguration get() { + return GlobalConfiguration.all().get(ApiTokenPropertyConfiguration.class); + } + + @NonNull + @Override + public GlobalConfigurationCategory getCategory() { + return GlobalConfigurationCategory.get(GlobalConfigurationCategory.Security.class); + } + + @RequirePOST + public HttpResponse doGenerate(StaplerRequest req) { + Jenkins.get().checkPermission(Jenkins.ADMINISTER); + + String apiTokenName = req.getParameter("apiTokenName"); + JSONObject json = this.generateApiToken(apiTokenName); + save(); + + return HttpResponses.okJSON(json); + } + + public JSONObject generateApiToken(@NonNull String name) { + byte[] random = new byte[16]; + RANDOM.nextBytes(random); + + String plainTextApiToken = Util.toHexString(random); + assert plainTextApiToken.length() == 32; + + String apiTokenValueHashed = Util.toHexString(hashedBytes(plainTextApiToken.getBytes(StandardCharsets.US_ASCII))); + HashedApiToken apiToken = new HashedApiToken(name, apiTokenValueHashed); + + synchronized (this) { + this.apiTokens.add(apiToken); + } + + JSONObject json = new JSONObject(); + json.put("uuid", apiToken.getUuid()); + json.put("name", apiToken.getName()); + json.put("value", plainTextApiToken); + + return json; + } + + @NonNull + private static byte[] hashedBytes(byte[] tokenBytes) { + MessageDigest digest; + try { + digest = MessageDigest.getInstance(HASH_ALGORITHM); + } catch (NoSuchAlgorithmException e) { + throw new AssertionError("There is no " + HASH_ALGORITHM + " available in this system", e); + } + return digest.digest(tokenBytes); + } + + @RequirePOST + public HttpResponse doRevoke(StaplerRequest req) { + Jenkins.get().checkPermission(Jenkins.ADMINISTER); + + String apiTokenUuid = req.getParameter("apiTokenUuid"); + if (StringUtils.isBlank(apiTokenUuid)) { + return HttpResponses.errorWithoutStack(400, "API token UUID cannot be empty"); + } + + synchronized (this) { + this.apiTokens.removeIf(apiToken -> apiToken.getUuid().equals(apiTokenUuid)); + } + save(); + + return HttpResponses.ok(); + } + + public synchronized Collection getApiTokens() { + return Collections.unmodifiableList(new ArrayList<>(this.apiTokens)); + } + + public boolean isValidApiToken(String plainApiToken) { + if (StringUtils.isBlank(plainApiToken)) { + return false; + } + + return this.hasMatchingApiToken(plainApiToken); + } + + public synchronized boolean hasMatchingApiToken(@NonNull String plainApiToken) { + byte[] hash = hashedBytes(plainApiToken.getBytes(StandardCharsets.US_ASCII)); + return this.apiTokens.stream().anyMatch(apiToken -> apiToken.match(hash)); + } + + public static class HashedApiToken implements Serializable { + + private static final long serialVersionUID = 1L; + + private final String uuid; + private final String name; + private final String hash; + + private HashedApiToken(String name, String hash) { + this.uuid = UUID.randomUUID().toString(); + this.name = name; + this.hash = hash; + } + + private HashedApiToken(String uuid, String name, String hash) { + this.uuid = uuid; + this.name = name; + this.hash = hash; + } + + public String getUuid() { + return uuid; + } + + public String getName() { + return name; + } + + public String getHash() { + return hash; + } + + private boolean match(byte[] hashedBytes) { + byte[] hashFromHex; + try { + hashFromHex = Util.fromHexString(hash); + } catch (NumberFormatException e) { + LOGGER.log(Level.INFO, "The API token with name=[{0}] is not in hex-format and so cannot be used", name); + return false; + } + + return MessageDigest.isEqual(hashFromHex, hashedBytes); + } + } +} diff --git a/src/main/java/hudson/plugins/git/GitStatus.java b/src/main/java/hudson/plugins/git/GitStatus.java index 3615c0c5cd..5c889a8733 100644 --- a/src/main/java/hudson/plugins/git/GitStatus.java +++ b/src/main/java/hudson/plugins/git/GitStatus.java @@ -12,14 +12,12 @@ import hudson.security.ACL; import hudson.security.ACLContext; import hudson.triggers.SCMTrigger; -import java.io.IOException; import java.io.PrintWriter; import java.net.URISyntaxException; import java.util.*; import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Pattern; -import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; @@ -27,6 +25,7 @@ import jenkins.model.Jenkins; import jenkins.scm.api.SCMEvent; import jenkins.triggers.SCMTriggerItem; +import jenkins.util.SystemProperties; import org.apache.commons.lang.StringUtils; import static org.apache.commons.lang.StringUtils.isNotEmpty; @@ -39,6 +38,9 @@ */ @Extension public class GitStatus implements UnprotectedRootAction { + static /* not final */ String NOTIFY_COMMIT_ACCESS_CONTROL = + SystemProperties.getString(GitStatus.class.getName() + ".NOTIFY_COMMIT_ACCESS_CONTROL"); + @Override public String getDisplayName() { return "Git"; @@ -113,8 +115,25 @@ public String toString() { } public HttpResponse doNotifyCommit(HttpServletRequest request, @QueryParameter(required=true) String url, - @QueryParameter(required=false) String branches, - @QueryParameter(required=false) String sha1) throws ServletException, IOException { + @QueryParameter() String branches, @QueryParameter() String sha1, + @QueryParameter() String token) { + if (!"disabled".equalsIgnoreCase(NOTIFY_COMMIT_ACCESS_CONTROL) + && !"disabled-for-polling".equalsIgnoreCase(NOTIFY_COMMIT_ACCESS_CONTROL)) { + if (StringUtils.isEmpty(token)) { + return HttpResponses.errorWithoutStack(401, "An access token is required. Please refer to Git plugin documentation for details."); + } + if (!ApiTokenPropertyConfiguration.get().isValidApiToken(token)) { + return HttpResponses.errorWithoutStack(403, "Invalid access token"); + } + } + if ("disabled-for-polling".equalsIgnoreCase(NOTIFY_COMMIT_ACCESS_CONTROL) && StringUtils.isNotEmpty(sha1)) { + if (StringUtils.isEmpty(token)) { + return HttpResponses.errorWithoutStack(401, "An access token is required when using the sha1 parameter. Please refer to Git plugin documentation for details."); + } + if (!ApiTokenPropertyConfiguration.get().isValidApiToken(token)) { + return HttpResponses.errorWithoutStack(403, "Invalid access token"); + } + } lastURL = url; lastBranches = branches; if(StringUtils.isNotBlank(sha1)&&!SHA1_PATTERN.matcher(sha1.trim()).matches()){ @@ -197,7 +216,7 @@ private static String normalizePath(String path) { } /** - * Contributes to a {@link #doNotifyCommit(HttpServletRequest, String, String, String)} response. + * Contributes to a {@link #doNotifyCommit(HttpServletRequest, String, String, String, String)} response. * * @since 1.4.1 */ diff --git a/src/main/resources/hudson/plugins/git/ApiTokenPropertyConfiguration/config.jelly b/src/main/resources/hudson/plugins/git/ApiTokenPropertyConfiguration/config.jelly new file mode 100644 index 0000000000..c047fb9618 --- /dev/null +++ b/src/main/resources/hudson/plugins/git/ApiTokenPropertyConfiguration/config.jelly @@ -0,0 +1,54 @@ + + + + + + +
+ +
+
${%There are no access tokens yet.}
+
+ + + + +
+ + + ${%Revoke} + +
+ + +
+
+ + + + + + + + + + + +
+ +
+ + + +
+ + + diff --git a/src/main/resources/hudson/plugins/git/ApiTokenPropertyConfiguration/help-tokens.html b/src/main/resources/hudson/plugins/git/ApiTokenPropertyConfiguration/help-tokens.html new file mode 100644 index 0000000000..1236735866 --- /dev/null +++ b/src/main/resources/hudson/plugins/git/ApiTokenPropertyConfiguration/help-tokens.html @@ -0,0 +1,15 @@ +
+

These access tokens serve as a way of authenticating requests to the notifyCommit endpoint. +

By default, all requests to notifyCommit must include a valid token in the token query parameter. However, it is possible to disable + that requirement with the system property: + 

hudson.plugins.git.GitStatus.NOTIFY_COMMIT_ACCESS_CONTROL
+
+ It has two modes: +
    +
  • disabled-for-polling - Allows unauthenticated requests as long as they only request polling of the repository supplied in the + url query parameter. Prohibits unauthenticated requests that attempt to schedule a build immediately by providing a + sha1 query parameter.
    • +
  • disabled - Fully disables the access token mechanism and allows all requests to notifyCommit + to be unauthenticated. This option is insecure and is not recommended.
    • +
+
diff --git a/src/main/resources/hudson/plugins/git/ApiTokenPropertyConfiguration/resources.css b/src/main/resources/hudson/plugins/git/ApiTokenPropertyConfiguration/resources.css new file mode 100644 index 0000000000..c55ad88ba1 --- /dev/null +++ b/src/main/resources/hudson/plugins/git/ApiTokenPropertyConfiguration/resources.css @@ -0,0 +1,18 @@ +.api-token-list .api-token-list-item-row { + display: flex; + align-items: center; + max-width: 700px; +} +.api-token-list .api-token-list-item-row.api-token-list-existing-api-token { + justify-content: space-between; +} +.api-token-list .api-token-list-item .hidden, .api-token-list .api-token-list-empty-item.hidden { + display: none; +} + +.api-token-list .api-token-revoke-button, .api-token-list .new-api-token-value { + padding: 0 0.5rem; +} +.api-token-list .api-token-warning-message, .api-token-list .api-token-save-button { + margin: 0.5rem 0; +} diff --git a/src/main/resources/hudson/plugins/git/ApiTokenPropertyConfiguration/resources.js b/src/main/resources/hudson/plugins/git/ApiTokenPropertyConfiguration/resources.js new file mode 100644 index 0000000000..8eba6882e5 --- /dev/null +++ b/src/main/resources/hudson/plugins/git/ApiTokenPropertyConfiguration/resources.js @@ -0,0 +1,81 @@ +function revokeApiToken(anchorRevoke) { + const repeatedChunk = anchorRevoke.up('.repeated-chunk'); + const apiTokenList = repeatedChunk.up('.api-token-list'); + const confirmMessage = anchorRevoke.getAttribute('data-confirm'); + const targetUrl = anchorRevoke.getAttribute('data-target-url'); + const inputUuid = repeatedChunk.querySelector('.api-token-uuid-input'); + const apiTokenUuid = inputUuid.value; + + if (confirm(confirmMessage)) { + new Ajax.Request(targetUrl, { + method: "post", + parameters: {apiTokenUuid: apiTokenUuid}, + onSuccess: function(res, _) { + repeatedChunk.remove(); + adjustEmptyListMessage(apiTokenList); + } + }); + } + + return false; +} + +function saveApiToken(button){ + if (button.hasClassName('request-pending')) { + // avoid multiple requests to be sent if user is clicking multiple times + return; + } + button.addClassName('request-pending'); + const targetUrl = button.getAttribute('data-target-url'); + const repeatedChunk = button.up('.repeated-chunk'); + const apiTokenList = repeatedChunk.up('.api-token-list'); + const nameInput = repeatedChunk.querySelector('.api-token-name-input'); + const apiTokenName = nameInput.value; + + new Ajax.Request(targetUrl, { + method: "post", + parameters: {apiTokenName: apiTokenName}, + onSuccess: function(res, _) { + const { name, value, uuid } = res.responseJSON.data; + nameInput.value = name; + + const apiTokenValueSpan = repeatedChunk.querySelector('.new-api-token-value'); + apiTokenValueSpan.innerText = value; + apiTokenValueSpan.removeClassName('hidden'); + + const apiTokenCopyButton = repeatedChunk.querySelector('.copy-button'); + apiTokenCopyButton.setAttribute('text', value); + apiTokenCopyButton.removeClassName('hidden'); + + const uuidInput = repeatedChunk.querySelector('.api-token-uuid-input'); + uuidInput.value = uuid; + + const warningMessage = repeatedChunk.querySelector('.api-token-warning-message'); + warningMessage.removeClassName('hidden'); + + // we do not want to allow user to create twice api token using same name by mistake + button.remove(); + + const revokeButton = repeatedChunk.querySelector('.api-token-revoke-button'); + revokeButton.removeClassName('hidden'); + + const cancelButton = repeatedChunk.querySelector('.api-token-cancel-button'); + cancelButton.addClassName('hidden'); + + repeatedChunk.addClassName('api-token-list-fresh-item'); + + adjustEmptyListMessage(apiTokenList); + } + }); +} + +function adjustEmptyListMessage(apiTokenList) { + const emptyListMessageClassList = apiTokenList.querySelector('.api-token-list-empty-item').classList; + + const apiTokenListLength = apiTokenList.querySelectorAll('.api-token-list-existing-item, .api-token-list-fresh-item').length; + if (apiTokenListLength >= 1) { + emptyListMessageClassList.add("hidden"); + } else { + emptyListMessageClassList.remove("hidden"); + } +} diff --git a/src/test/java/hudson/plugins/git/AbstractGitProject.java b/src/test/java/hudson/plugins/git/AbstractGitProject.java index 57688f2024..acb9252715 100644 --- a/src/test/java/hudson/plugins/git/AbstractGitProject.java +++ b/src/test/java/hudson/plugins/git/AbstractGitProject.java @@ -59,6 +59,7 @@ import org.junit.Rule; import org.jvnet.hudson.test.CaptureEnvironmentBuilder; +import org.jvnet.hudson.test.FlagRule; import org.jvnet.hudson.test.JenkinsRule; /** @@ -70,6 +71,10 @@ public class AbstractGitProject extends AbstractGitRepository { @Rule public JenkinsRule jenkins = new JenkinsRule(); + @Rule + public FlagRule notifyCommitAccessControl = + new FlagRule<>(() -> GitStatus.NOTIFY_COMMIT_ACCESS_CONTROL, x -> GitStatus.NOTIFY_COMMIT_ACCESS_CONTROL = x); + protected FreeStyleProject setupProject(List branches, boolean authorOrCommitter) throws Exception { FreeStyleProject project = jenkins.createFreeStyleProject(); GitSCM scm = new GitSCM(remoteConfigs(), branches, diff --git a/src/test/java/hudson/plugins/git/GitStatusCrumbExclusionTest.java b/src/test/java/hudson/plugins/git/GitStatusCrumbExclusionTest.java index cc0ee07f47..2f64477128 100644 --- a/src/test/java/hudson/plugins/git/GitStatusCrumbExclusionTest.java +++ b/src/test/java/hudson/plugins/git/GitStatusCrumbExclusionTest.java @@ -39,7 +39,10 @@ public class GitStatusCrumbExclusionTest { private final String branchArgument; private final byte[] branchArgumentBytes; - public GitStatusCrumbExclusionTest() throws IOException { + private final String notifyCommitApiToken; + private final byte[] notifyCommitApiTokenBytes; + + public GitStatusCrumbExclusionTest() throws Exception { String jenkinsUrl = r.getURL().toExternalForm(); if (!jenkinsUrl.endsWith("/")) { jenkinsUrl = jenkinsUrl + "/"; @@ -56,6 +59,9 @@ public GitStatusCrumbExclusionTest() throws IOException { branchArgument = "branches=origin/some-branch-name"; branchArgumentBytes = branchArgument.getBytes(StandardCharsets.UTF_8); + + notifyCommitApiToken = "token=" + ApiTokenPropertyConfiguration.get().generateApiToken("test").getString("value"); + notifyCommitApiTokenBytes = notifyCommitApiToken.getBytes(StandardCharsets.UTF_8); } private HttpURLConnection connectionPOST; @@ -86,6 +92,8 @@ public void testPOSTValidPathNoArgument() throws Exception { public void testPOSTValidPathMandatoryArgument() throws Exception { try (OutputStream os = connectionPOST.getOutputStream()) { os.write(urlArgumentBytes); + os.write(separatorBytes); + os.write(notifyCommitApiTokenBytes); } assertThat(connectionPOST.getResponseCode(), is(HttpURLConnection.HTTP_OK)); assertThat(connectionPOST.getResponseMessage(), is("OK")); @@ -97,6 +105,8 @@ public void testPOSTValidPathEmptyMandatoryArgument() throws Exception { String urlEmptyArgument = "url="; // Empty argument is not a valid URL byte[] urlEmptyArgumentBytes = urlEmptyArgument.getBytes(StandardCharsets.UTF_8); os.write(urlEmptyArgumentBytes); + os.write(separatorBytes); + os.write(notifyCommitApiTokenBytes); } assertThat(connectionPOST.getResponseCode(), is(HttpURLConnection.HTTP_BAD_REQUEST)); assertThat(connectionPOST.getResponseMessage(), is("Bad Request")); @@ -108,6 +118,8 @@ public void testPOSTValidPathBadURLInMandatoryArgument() throws Exception { String urlBadArgument = "url=" + "http://256.256.256.256/"; // Not a valid URI per Java 8 javadoc byte[] urlBadArgumentBytes = urlBadArgument.getBytes(StandardCharsets.UTF_8); os.write(urlBadArgumentBytes); + os.write(separatorBytes); + os.write(notifyCommitApiTokenBytes); } assertThat(connectionPOST.getResponseCode(), is(HttpURLConnection.HTTP_OK)); assertThat(connectionPOST.getResponseMessage(), is("OK")); @@ -119,6 +131,8 @@ public void testPOSTValidPathMandatoryAndOptionalArgument() throws Exception { os.write(urlArgumentBytes); os.write(separatorBytes); os.write(branchArgumentBytes); + os.write(separatorBytes); + os.write(notifyCommitApiTokenBytes); } assertThat(connectionPOST.getResponseCode(), is(HttpURLConnection.HTTP_OK)); assertThat(connectionPOST.getResponseMessage(), is("OK")); @@ -231,7 +245,7 @@ public void testGETValidPathNoArgument() throws Exception { @Test public void testGETValidPathMandatoryArgument() throws Exception { - URL getURL = new URL(notifyCommitURL + "?" + urlArgument); + URL getURL = new URL(notifyCommitURL + "?" + urlArgument + separator + notifyCommitApiToken); HttpURLConnection connectionGET = (HttpURLConnection) getURL.openConnection(); connectionGET.setRequestMethod("GET"); connectionGET.connect(); @@ -242,7 +256,7 @@ public void testGETValidPathMandatoryArgument() throws Exception { @Test public void testGETValidPathMandatoryAndOptionalArgument() throws Exception { - URL getURL = new URL(notifyCommitURL + "?" + urlArgument + separator + branchArgument); + URL getURL = new URL(notifyCommitURL + "?" + urlArgument + separator + branchArgument + separator + notifyCommitApiToken); HttpURLConnection connectionGET = (HttpURLConnection) getURL.openConnection(); connectionGET.setRequestMethod("GET"); connectionGET.connect(); diff --git a/src/test/java/hudson/plugins/git/GitStatusTest.java b/src/test/java/hudson/plugins/git/GitStatusTest.java index 4789626ec0..738ac23286 100644 --- a/src/test/java/hudson/plugins/git/GitStatusTest.java +++ b/src/test/java/hudson/plugins/git/GitStatusTest.java @@ -54,6 +54,7 @@ public class GitStatusTest extends AbstractGitProject { private String repoURL; private String branch; private String sha1; + private String notifyCommitApiToken; @Before public void setUp() throws Exception { @@ -65,6 +66,9 @@ public void setUp() throws Exception { this.repoURL = new File(".").getAbsolutePath(); this.branch = "**"; this.sha1 = "7bb68ef21dc90bd4f7b08eca876203b2e049198d"; + if (jenkins.jenkins != null) { + this.notifyCommitApiToken = ApiTokenPropertyConfiguration.get().generateApiToken("test").getString("value"); + } } @After @@ -154,7 +158,7 @@ public void testDoNotifyCommitWithNoBranches() throws Exception { SCMTrigger bMasterTrigger = setupProjectWithTrigger("b", "master", false); SCMTrigger bTopicTrigger = setupProjectWithTrigger("b", "topic", false); - this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "", null); + this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "", null, notifyCommitApiToken); Mockito.verify(aMasterTrigger).run(); Mockito.verify(aTopicTrigger).run(); Mockito.verify(bMasterTrigger, Mockito.never()).run(); @@ -170,7 +174,7 @@ public void testDoNotifyCommitWithNoMatchingUrl() throws Exception { SCMTrigger bMasterTrigger = setupProjectWithTrigger("b", "master", false); SCMTrigger bTopicTrigger = setupProjectWithTrigger("b", "topic", false); - this.gitStatus.doNotifyCommit(requestWithNoParameter, "nonexistent", "", null); + this.gitStatus.doNotifyCommit(requestWithNoParameter, "nonexistent", "", null, notifyCommitApiToken); Mockito.verify(aMasterTrigger, Mockito.never()).run(); Mockito.verify(aTopicTrigger, Mockito.never()).run(); Mockito.verify(bMasterTrigger, Mockito.never()).run(); @@ -186,7 +190,7 @@ public void testDoNotifyCommitWithOneBranch() throws Exception { SCMTrigger bMasterTrigger = setupProjectWithTrigger("b", "master", false); SCMTrigger bTopicTrigger = setupProjectWithTrigger("b", "topic", false); - this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "master", null); + this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "master", null, notifyCommitApiToken); Mockito.verify(aMasterTrigger).run(); Mockito.verify(aTopicTrigger, Mockito.never()).run(); Mockito.verify(bMasterTrigger, Mockito.never()).run(); @@ -204,7 +208,7 @@ public void testDoNotifyCommitWithTwoBranches() throws Exception { SCMTrigger bTopicTrigger = setupProjectWithTrigger("b", "topic", false); SCMTrigger bFeatureTrigger = setupProjectWithTrigger("b", "feature/def", false); - this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "master,topic,feature/def", null); + this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "master,topic,feature/def", null, notifyCommitApiToken); Mockito.verify(aMasterTrigger).run(); Mockito.verify(aTopicTrigger).run(); Mockito.verify(aFeatureTrigger).run(); @@ -223,7 +227,7 @@ public void testDoNotifyCommitWithNoMatchingBranches() throws Exception { SCMTrigger bMasterTrigger = setupProjectWithTrigger("b", "master", false); SCMTrigger bTopicTrigger = setupProjectWithTrigger("b", "topic", false); - this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "nonexistent", null); + this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "nonexistent", null, notifyCommitApiToken); Mockito.verify(aMasterTrigger, Mockito.never()).run(); Mockito.verify(aTopicTrigger, Mockito.never()).run(); Mockito.verify(bMasterTrigger, Mockito.never()).run(); @@ -239,7 +243,7 @@ public void testDoNotifyCommitWithSlashesInBranchNames() throws Exception { SCMTrigger aSlashesTrigger = setupProjectWithTrigger("a", "name/with/slashes", false); - this.gitStatus.doNotifyCommit(requestWithParameter, "a", "name/with/slashes", null); + this.gitStatus.doNotifyCommit(requestWithParameter, "a", "name/with/slashes", null, notifyCommitApiToken); Mockito.verify(aSlashesTrigger).run(); Mockito.verify(bMasterTrigger, Mockito.never()).run(); @@ -252,7 +256,7 @@ public void testDoNotifyCommitWithParametrizedBranch() throws Exception { SCMTrigger bMasterTrigger = setupProjectWithTrigger("b", "master", false); SCMTrigger bTopicTrigger = setupProjectWithTrigger("b", "topic", false); - this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "master", null); + this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "master", null, notifyCommitApiToken); Mockito.verify(aMasterTrigger).run(); Mockito.verify(bMasterTrigger, Mockito.never()).run(); Mockito.verify(bTopicTrigger, Mockito.never()).run(); @@ -264,7 +268,7 @@ public void testDoNotifyCommitWithParametrizedBranch() throws Exception { public void testDoNotifyCommitWithIgnoredRepository() throws Exception { SCMTrigger aMasterTrigger = setupProjectWithTrigger("a", "master", true); - this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", null, ""); + this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", null, "", notifyCommitApiToken); Mockito.verify(aMasterTrigger, Mockito.never()).run(); assertEquals("URL: a SHA1: ", this.gitStatus.toString()); @@ -273,7 +277,7 @@ public void testDoNotifyCommitWithIgnoredRepository() throws Exception { @Test public void testDoNotifyCommitWithNoScmTrigger() throws Exception { setupProject("a", "master", null); - this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", null, ""); + this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", null, "", notifyCommitApiToken); // no expectation here, however we shouldn't have a build triggered, and no exception assertEquals("URL: a SHA1: ", this.gitStatus.toString()); @@ -321,7 +325,7 @@ private void doNotifyCommitWithTwoBranchesAndAdditionalParameter(final boolean a parameterMap.put("paramKey1", new String[] {"paramValue1"}); when(requestWithParameter.getParameterMap()).thenReturn(parameterMap); - this.gitStatus.doNotifyCommit(requestWithParameter, "a", "master,topic", null); + this.gitStatus.doNotifyCommit(requestWithParameter, "a", "master,topic", null, notifyCommitApiToken); Mockito.verify(aMasterTrigger).run(); Mockito.verify(aTopicTrigger).run(); Mockito.verify(bMasterTrigger, Mockito.never()).run(); @@ -345,7 +349,7 @@ private void doNotifyCommitWithTwoBranchesAndAdditionalParameter(final boolean a @Theory public void testDoNotifyCommitBranchWithSlash(@FromDataPoints("branchSpecPrefixes") String branchSpecPrefix) throws Exception { SCMTrigger trigger = setupProjectWithTrigger("remote", branchSpecPrefix + "feature/awesome-feature", false); - this.gitStatus.doNotifyCommit(requestWithNoParameter, "remote", "feature/awesome-feature", null); + this.gitStatus.doNotifyCommit(requestWithNoParameter, "remote", "feature/awesome-feature", null, notifyCommitApiToken); Mockito.verify(trigger).run(); } @@ -353,7 +357,7 @@ public void testDoNotifyCommitBranchWithSlash(@FromDataPoints("branchSpecPrefixe @Theory public void testDoNotifyCommitBranchWithoutSlash(@FromDataPoints("branchSpecPrefixes") String branchSpecPrefix) throws Exception { SCMTrigger trigger = setupProjectWithTrigger("remote", branchSpecPrefix + "awesome-feature", false); - this.gitStatus.doNotifyCommit(requestWithNoParameter, "remote", "awesome-feature", null); + this.gitStatus.doNotifyCommit(requestWithNoParameter, "remote", "awesome-feature", null, notifyCommitApiToken); Mockito.verify(trigger).run(); } @@ -361,7 +365,7 @@ public void testDoNotifyCommitBranchWithoutSlash(@FromDataPoints("branchSpecPref @Theory public void testDoNotifyCommitBranchByBranchRef(@FromDataPoints("branchSpecPrefixes") String branchSpecPrefix) throws Exception { SCMTrigger trigger = setupProjectWithTrigger("remote", branchSpecPrefix + "awesome-feature", false); - this.gitStatus.doNotifyCommit(requestWithNoParameter, "remote", "refs/heads/awesome-feature", null); + this.gitStatus.doNotifyCommit(requestWithNoParameter, "remote", "refs/heads/awesome-feature", null, notifyCommitApiToken); Mockito.verify(trigger).run(); } @@ -369,7 +373,7 @@ public void testDoNotifyCommitBranchByBranchRef(@FromDataPoints("branchSpecPrefi @Test public void testDoNotifyCommitBranchWithRegex() throws Exception { SCMTrigger trigger = setupProjectWithTrigger("remote", ":[^/]*/awesome-feature", false); - this.gitStatus.doNotifyCommit(requestWithNoParameter, "remote", "feature/awesome-feature", null); + this.gitStatus.doNotifyCommit(requestWithNoParameter, "remote", "feature/awesome-feature", null, notifyCommitApiToken); Mockito.verify(trigger).run(); } @@ -377,7 +381,7 @@ public void testDoNotifyCommitBranchWithRegex() throws Exception { @Test public void testDoNotifyCommitBranchWithWildcard() throws Exception { SCMTrigger trigger = setupProjectWithTrigger("remote", "origin/feature/*", false); - this.gitStatus.doNotifyCommit(requestWithNoParameter, "remote", "feature/awesome-feature", null); + this.gitStatus.doNotifyCommit(requestWithNoParameter, "remote", "feature/awesome-feature", null, notifyCommitApiToken); Mockito.verify(trigger).run(); } @@ -488,7 +492,7 @@ private Map setupParameterMap(String extraValue) { @Test public void testDoNotifyCommit() throws Exception { /* No parameters */ setupNotifyProject(); - this.gitStatus.doNotifyCommit(requestWithNoParameter, repoURL, branch, sha1); + this.gitStatus.doNotifyCommit(requestWithNoParameter, repoURL, branch, sha1, notifyCommitApiToken); assertEquals("URL: " + repoURL + " SHA1: " + sha1 + " Branches: " + branch, this.gitStatus.toString()); @@ -529,7 +533,7 @@ private void doNotifyCommitWithExtraParameterAllowed(final boolean allowed, Stri setupNotifyProject(); String extraValue = "An-extra-value"; when(requestWithParameter.getParameterMap()).thenReturn(setupParameterMap(extraValue)); - this.gitStatus.doNotifyCommit(requestWithParameter, repoURL, branch, sha1); + this.gitStatus.doNotifyCommit(requestWithParameter, repoURL, branch, sha1, notifyCommitApiToken); String expected = "URL: " + repoURL + " SHA1: " + sha1 @@ -543,7 +547,7 @@ private void doNotifyCommitWithExtraParameterAllowed(final boolean allowed, Stri public void testDoNotifyCommitWithNullValueExtraParameter() throws Exception { setupNotifyProject(); when(requestWithParameter.getParameterMap()).thenReturn(setupParameterMap(null)); - this.gitStatus.doNotifyCommit(requestWithParameter, repoURL, branch, sha1); + this.gitStatus.doNotifyCommit(requestWithParameter, repoURL, branch, sha1, notifyCommitApiToken); assertEquals("URL: " + repoURL + " SHA1: " + sha1 + " Branches: " + branch, this.gitStatus.toString()); @@ -610,7 +614,7 @@ private void doNotifyCommitWithDefaultParameter(final boolean allowed, String sa String extraValue = "An-extra-value"; when(requestWithParameter.getParameterMap()).thenReturn(setupParameterMap(extraValue)); - this.gitStatus.doNotifyCommit(requestWithParameter, repoURL, branch, sha1); + this.gitStatus.doNotifyCommit(requestWithParameter, repoURL, branch, sha1, notifyCommitApiToken); String expected = "URL: " + repoURL + " SHA1: " + sha1 @@ -650,7 +654,7 @@ public void testDoNotifyCommitTriggeredHeadersLimited() throws Exception { projectTriggers[i] = setupProjectWithTrigger("a", "master", false); } - HttpResponse rsp = this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "master", null); + HttpResponse rsp = this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "master", null, notifyCommitApiToken); // Up to 10 "Triggered" headers + 1 extra warning are returned. StaplerRequest sReq = mock(StaplerRequest.class); @@ -674,11 +678,104 @@ public void testDoNotifyCommitWithWrongSha1Content() throws Exception { String content = ""; - HttpResponse rsp = this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "master", content); + HttpResponse rsp = this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "master", content, notifyCommitApiToken); HttpResponses.HttpResponseException responseException = ((HttpResponses.HttpResponseException) rsp); assertEquals(IllegalArgumentException.class, responseException.getCause().getClass()); assertEquals("Illegal SHA1", responseException.getCause().getMessage()); } + + @Test + @Issue("SECURITY-284") + public void testDoNotifyCommitWithValidSha1AndValidApiToken() throws Exception { + // when sha1 is provided build is scheduled right away instead of repo polling, so we do not check for trigger + FreeStyleProject project = setupNotifyProject(); + + this.gitStatus.doNotifyCommit(requestWithParameter, repoURL, branch, sha1, notifyCommitApiToken); + + jenkins.waitUntilNoActivity(); + FreeStyleBuild lastBuild = project.getLastBuild(); + + assertNotNull(lastBuild); + assertEquals(lastBuild.getNumber(), 1); + } + + @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 + @Issue("SECURITY-284") + public void testDoNotifyCommitWithUnauthenticatedPollingAllowed() throws Exception { + GitStatus.NOTIFY_COMMIT_ACCESS_CONTROL = "disabled-for-polling"; + SCMTrigger trigger = setupProjectWithTrigger("a", "master", false); + + this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "master", null, null); + + Mockito.verify(trigger).run(); + } + + @Test + @Issue("SECURITY-284") + public void testDoNotifyCommitWithAllowModeRandomValue() throws Exception { + GitStatus.NOTIFY_COMMIT_ACCESS_CONTROL = "random"; + setupProjectWithTrigger("a", "master", false); + StaplerResponse res = mock(StaplerResponse.class); + + HttpResponse httpResponse = this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "master", null, null); + httpResponse.generateResponse(null, res, null); + + Mockito.verify(res).sendError(401, "An access token is required. Please refer to Git plugin documentation for details."); + } + + @Test + @Issue("SECURITY-284") + public void testDoNotifyCommitWithSha1AndAllowModePoll() throws Exception { + GitStatus.NOTIFY_COMMIT_ACCESS_CONTROL = "disabled-for-polling"; + setupProjectWithTrigger("a", "master", false); + StaplerResponse res = mock(StaplerResponse.class); + + HttpResponse httpResponse = this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "master", sha1, null); + httpResponse.generateResponse(null, res, null); + + Mockito.verify(res).sendError(401, "An access token is required when using the sha1 parameter. Please refer to Git plugin documentation for details."); + } + + @Test + @Issue("SECURITY-284") + public void testDoNotifyCommitWithSha1AndAllowModePollWithInvalidToken() throws Exception { + GitStatus.NOTIFY_COMMIT_ACCESS_CONTROL = "disabled-for-polling"; + setupProjectWithTrigger("a", "master", false); + StaplerResponse res = mock(StaplerResponse.class); + + HttpResponse httpResponse = this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "master", sha1, "invalid"); + httpResponse.generateResponse(null, res, null); + + Mockito.verify(res).sendError(403, "Invalid access token"); + } + + @Test + @Issue("SECURITY-284") + public void testDoNotifyCommitWithAllowModeSha1() throws Exception { + GitStatus.NOTIFY_COMMIT_ACCESS_CONTROL = "disabled"; + // when sha1 is provided build is scheduled right away instead of repo polling, so we do not check for trigger + FreeStyleProject project = setupNotifyProject(); + + this.gitStatus.doNotifyCommit(requestWithParameter, repoURL, branch, sha1, null); + + jenkins.waitUntilNoActivity(); + FreeStyleBuild lastBuild = project.getLastBuild(); + + assertNotNull(lastBuild); + assertEquals(lastBuild.getNumber(), 1); + } } diff --git a/src/test/java/hudson/plugins/git/security/ApiTokenPropertyConfigurationTest.java b/src/test/java/hudson/plugins/git/security/ApiTokenPropertyConfigurationTest.java new file mode 100644 index 0000000000..9d6a73fc5f --- /dev/null +++ b/src/test/java/hudson/plugins/git/security/ApiTokenPropertyConfigurationTest.java @@ -0,0 +1,122 @@ +package hudson.plugins.git.security; + +import com.gargoylesoftware.htmlunit.HttpMethod; +import com.gargoylesoftware.htmlunit.WebRequest; +import com.gargoylesoftware.htmlunit.WebResponse; +import com.gargoylesoftware.htmlunit.util.NameValuePair; +import hudson.plugins.git.ApiTokenPropertyConfiguration; +import jenkins.model.Jenkins; +import net.sf.json.JSONObject; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.MockAuthorizationStrategy; + +import java.util.Collection; +import java.util.Collections; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +public class ApiTokenPropertyConfigurationTest { + + @Rule + public JenkinsRule j = new JenkinsRule(); + + @Before + public void init() { + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + MockAuthorizationStrategy authorizationStrategy = new MockAuthorizationStrategy(); + authorizationStrategy.grant(Jenkins.ADMINISTER).everywhere().to("alice"); + authorizationStrategy.grant(Jenkins.READ).everywhere().to("bob"); + j.jenkins.setAuthorizationStrategy(authorizationStrategy); + } + + @Test + public void testAdminPermissionRequiredToGenerateNewApiTokens() throws Exception { + try (JenkinsRule.WebClient wc = j.createWebClient()) { + wc.login("bob"); + WebRequest req = new WebRequest( + wc.createCrumbedUrl(ApiTokenPropertyConfiguration.get().getDescriptorUrl() + "/generate"), HttpMethod.POST); + req.setRequestBody("{\"apiTokenName\":\"test\"}"); + wc.setThrowExceptionOnFailingStatusCode(false); + + WebResponse res = wc.getPage(req).getWebResponse(); + + assertEquals(403, res.getStatusCode()); + assertTrue(res.getContentAsString().contains("bob is missing the Overall/Administer permission")); + } + } + + @Test + public void adminPermissionsRequiredToRevokeApiTokens() throws Exception { + try (JenkinsRule.WebClient wc = j.createWebClient()) { + wc.login("bob"); + WebRequest req = new WebRequest(wc.createCrumbedUrl(ApiTokenPropertyConfiguration.get().getDescriptorUrl() + "/revoke"), HttpMethod.POST); + wc.setThrowExceptionOnFailingStatusCode(false); + + WebResponse res = wc.getPage(req).getWebResponse(); + + assertEquals(403, res.getStatusCode()); + assertTrue(res.getContentAsString().contains("bob is missing the Overall/Administer permission")); + } + } + + @Test + public void testBasicGenerationAndRevocation() throws Exception { + try (JenkinsRule.WebClient wc = j.createWebClient()) { + wc.login("alice"); + WebRequest generateReq = new WebRequest( + wc.createCrumbedUrl(ApiTokenPropertyConfiguration.get().getDescriptorUrl() + "/generate"), HttpMethod.POST); + generateReq.setRequestParameters(Collections.singletonList(new NameValuePair("apiTokenName", "token"))); + String uuid = JSONObject.fromObject(wc.getPage(generateReq).getWebResponse().getContentAsString()).getJSONObject("data").getString("uuid"); + + generateReq.setRequestParameters(Collections.singletonList(new NameValuePair("apiTokenName", "nekot"))); + String uuid2 = JSONObject.fromObject(wc.getPage(generateReq).getWebResponse().getContentAsString()).getJSONObject("data").getString("uuid"); + + Collection apiTokens = ApiTokenPropertyConfiguration.get().getApiTokens(); + assertThat(apiTokens, allOf( + iterableWithSize(2), + hasItem( + allOf( + hasProperty("name", is("token")), + hasProperty("uuid", is(uuid)) + ) + ), + hasItem( + allOf( + hasProperty("name", is("nekot")), + hasProperty("uuid", is(uuid2)) + ) + ) + )); + + WebRequest revokeReq = new WebRequest( + wc.createCrumbedUrl(ApiTokenPropertyConfiguration.get().getDescriptorUrl() + "/revoke"), HttpMethod.POST); + revokeReq.setRequestParameters(Collections.singletonList(new NameValuePair("apiTokenUuid", uuid))); + wc.getPage(revokeReq); + + apiTokens = ApiTokenPropertyConfiguration.get().getApiTokens(); + assertThat(apiTokens, allOf( + iterableWithSize(1), + hasItem( + allOf( + hasProperty("name", is("nekot")), + hasProperty("uuid", is(uuid2)) + ) + ) + )); + } + } + + @Test + public void isValidApiTokenReturnsTrueIfGivenApiTokenExists() { + JSONObject json = ApiTokenPropertyConfiguration.get().generateApiToken("test"); + + assertTrue(ApiTokenPropertyConfiguration.get().isValidApiToken(json.getString("value"))); + } + +} diff --git a/src/test/java/jenkins/plugins/git/GitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/GitSCMSourceTest.java index 1749701650..9fdd3666d2 100644 --- a/src/test/java/jenkins/plugins/git/GitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/GitSCMSourceTest.java @@ -11,6 +11,7 @@ import hudson.FilePath; import hudson.plugins.git.GitStatus; import hudson.plugins.git.GitTool; +import hudson.plugins.git.ApiTokenPropertyConfiguration; import hudson.scm.SCMDescriptor; import hudson.tools.CommandInstaller; import hudson.tools.InstallSourceProperty; @@ -92,11 +93,12 @@ public void setup() { @Test @Deprecated public void testSourceOwnerTriggeredByDoNotifyCommit() throws Exception { + String notifyCommitApiToken = ApiTokenPropertyConfiguration.get().generateApiToken("test").getString("value"); GitSCMSource gitSCMSource = new GitSCMSource("id", REMOTE, "", "*", "", false); GitSCMSourceOwner scmSourceOwner = setupGitSCMSourceOwner(gitSCMSource); jenkins.getInstance().add(scmSourceOwner, "gitSourceOwner"); - gitStatus.doNotifyCommit(mock(HttpServletRequest.class), REMOTE, "master", ""); + gitStatus.doNotifyCommit(mock(HttpServletRequest.class), REMOTE, "master", "", notifyCommitApiToken); SCMHeadEvent event = jenkins.getInstance().getExtensionList(SCMEventListener.class).get(SCMEventListenerImpl.class) diff --git a/src/test/java/jenkins/plugins/git/GitSampleRepoRule.java b/src/test/java/jenkins/plugins/git/GitSampleRepoRule.java index c935ad3da4..877f69c32e 100644 --- a/src/test/java/jenkins/plugins/git/GitSampleRepoRule.java +++ b/src/test/java/jenkins/plugins/git/GitSampleRepoRule.java @@ -29,6 +29,7 @@ import hudson.Launcher; import hudson.model.TaskListener; import hudson.plugins.git.GitSCM; +import hudson.plugins.git.ApiTokenPropertyConfiguration; import hudson.util.StreamTaskListener; import java.io.ByteArrayOutputStream; import java.io.File; @@ -88,7 +89,9 @@ public final boolean mkdirs(String rel) throws IOException { public void notifyCommit(JenkinsRule r) throws Exception { synchronousPolling(r); - WebResponse webResponse = r.createWebClient().goTo("git/notifyCommit?url=" + bareUrl(), "text/plain").getWebResponse(); + 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()); for (NameValuePair pair : webResponse.getResponseHeaders()) { if (pair.getName().equals("Triggered")) {