diff --git a/src/main/java/com/dabsquared/gitlabjenkins/GitLabPushTrigger.java b/src/main/java/com/dabsquared/gitlabjenkins/GitLabPushTrigger.java index 3c2f711..e450520 100644 --- a/src/main/java/com/dabsquared/gitlabjenkins/GitLabPushTrigger.java +++ b/src/main/java/com/dabsquared/gitlabjenkins/GitLabPushTrigger.java @@ -36,6 +36,7 @@ import hudson.triggers.TriggerDescriptor; import hudson.util.FormValidation; import hudson.util.ListBoxModel; import hudson.util.ListBoxModel.Option; +import hudson.util.Secret; import hudson.util.SequentialExecutionQueue; import jenkins.model.Jenkins; import jenkins.model.ParameterizedJobMixIn; @@ -49,9 +50,11 @@ import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.Stapler; import org.kohsuke.stapler.StaplerRequest; +import org.kohsuke.stapler.StaplerResponse; import java.io.IOException; import java.io.ObjectStreamException; +import java.security.SecureRandom; import static com.dabsquared.gitlabjenkins.trigger.filter.BranchFilterConfig.BranchFilterConfigBuilder.branchFilterConfig; import static com.dabsquared.gitlabjenkins.trigger.handler.merge.MergeRequestHookTriggerHandlerFactory.newMergeRequestHookTriggerHandler; @@ -65,7 +68,10 @@ import static com.dabsquared.gitlabjenkins.trigger.handler.push.PushHookTriggerH * @author Daniel Brooks */ public class GitLabPushTrigger extends Trigger> { - private boolean triggerOnPush = true; + + private static final SecureRandom RANDOM = new SecureRandom(); + + private boolean triggerOnPush = true; private boolean triggerOnMergeRequest = true; private final TriggerOpenMergeRequest triggerOpenMergeRequestOnPush; private boolean triggerOnNoteRequest = true; @@ -83,6 +89,7 @@ public class GitLabPushTrigger extends Trigger> { private String excludeBranchesSpec; private String targetBranchRegex; private final MergeRequestLabelFilterConfig mergeRequestLabelFilterConfig; + private volatile Secret secretToken; private transient BranchFilter branchFilter; private transient PushHookTriggerHandler pushHookTriggerHandler; @@ -99,7 +106,7 @@ public class GitLabPushTrigger extends Trigger> { boolean setBuildDescription, boolean addNoteOnMergeRequest, boolean addCiMessage, boolean addVoteOnMergeRequest, boolean acceptMergeRequestOnSuccess, BranchFilterType branchFilterType, String includeBranchesSpec, String excludeBranchesSpec, String targetBranchRegex, - MergeRequestLabelFilterConfig mergeRequestLabelFilterConfig) { + MergeRequestLabelFilterConfig mergeRequestLabelFilterConfig, String secretToken) { this.triggerOnPush = triggerOnPush; this.triggerOnMergeRequest = triggerOnMergeRequest; this.triggerOnNoteRequest = triggerOnNoteRequest; @@ -117,6 +124,7 @@ public class GitLabPushTrigger extends Trigger> { this.targetBranchRegex = targetBranchRegex; this.acceptMergeRequestOnSuccess = acceptMergeRequestOnSuccess; this.mergeRequestLabelFilterConfig = mergeRequestLabelFilterConfig; + this.secretToken = Secret.fromString(secretToken); initializeTriggerHandler(); initializeBranchFilter(); @@ -223,6 +231,15 @@ public class GitLabPushTrigger extends Trigger> { return mergeRequestLabelFilterConfig; } + public String getSecretToken() { + return secretToken == null ? null : secretToken.getPlainText(); + } + + public boolean isWebHookAuthorized(String secretToken) { + String plainText = this.secretToken.getPlainText(); + return StringUtils.isEmpty(plainText) || StringUtils.equals(plainText, secretToken); + } + // executes when the Trigger receives a push request public void onPost(final PushHook hook) { pushHookTriggerHandler.handle(job, hook, ciSkip, branchFilter, mergeRequestLabelFilter); @@ -238,6 +255,12 @@ public class GitLabPushTrigger extends Trigger> { noteHookTriggerHandler.handle(job, hook, ciSkip, branchFilter, mergeRequestLabelFilter); } + private void generateSecretToken() { + byte[] random = new byte[16]; // 16x8=128bit worth of randomness, since we use md5 digest as the API token + RANDOM.nextBytes(random); + secretToken = Secret.fromString(Util.toHexString(random)); + } + private void initializeTriggerHandler() { mergeRequestHookTriggerHandler = newMergeRequestHookTriggerHandler(triggerOnMergeRequest, triggerOpenMergeRequestOnPush, skipWorkInProgressMergeRequest); noteHookTriggerHandler = newNoteHookTriggerHandler(triggerOnNoteRequest, noteRegex); @@ -383,5 +406,11 @@ public class GitLabPushTrigger extends Trigger> { public FormValidation doCheckExcludeMergeRequestLabels(@AncestorInPath final Job project, @QueryParameter final String value) { return ProjectLabelsProvider.instance().doCheckLabels(project, value); } + + public void doGenerateSecretToken(@AncestorInPath final Job project, StaplerResponse response) { + GitLabPushTrigger trigger = getFromJob(project); + trigger.generateSecretToken(); + response.setHeader("script", "document.getElementById('secretToken').value='" + trigger.getSecretToken() + "'"); + } } } diff --git a/src/main/java/com/dabsquared/gitlabjenkins/webhook/ActionResolver.java b/src/main/java/com/dabsquared/gitlabjenkins/webhook/ActionResolver.java index 521b02a..2e833ee 100644 --- a/src/main/java/com/dabsquared/gitlabjenkins/webhook/ActionResolver.java +++ b/src/main/java/com/dabsquared/gitlabjenkins/webhook/ActionResolver.java @@ -1,6 +1,5 @@ package com.dabsquared.gitlabjenkins.webhook; -import com.dabsquared.gitlabjenkins.connection.GitLabConnectionConfig; import com.dabsquared.gitlabjenkins.util.ACLUtil; import com.dabsquared.gitlabjenkins.webhook.build.MergeRequestBuildAction; import com.dabsquared.gitlabjenkins.webhook.build.NoteBuildAction; @@ -16,8 +15,6 @@ import hudson.model.Item; import hudson.model.ItemGroup; import hudson.model.Job; import hudson.security.ACL; -import hudson.security.AccessDeniedException2; -import hudson.security.Permission; import hudson.util.HttpResponses; import jenkins.model.Jenkins; import jenkins.scm.api.SCMSourceOwner; @@ -57,7 +54,6 @@ public class ActionResolver { private WebHookAction resolveAction(Item project, String restOfPath, StaplerRequest request) { String method = request.getMethod(); if (method.equals("POST")) { - checkPermission(Item.BUILD); return onPost(project, request); } else if (method.equals("GET")) { if (project instanceof Job) { @@ -106,14 +102,15 @@ public class ActionResolver { LOGGER.log(Level.FINE, "Missing X-Gitlab-Event header"); return new NoopAction(); } + String tokenHeader = request.getHeader("X-Gitlab-Token"); switch (eventHeader) { case "Merge Request Hook": - return new MergeRequestBuildAction(project, getRequestBody(request)); + return new MergeRequestBuildAction(project, getRequestBody(request), tokenHeader); case "Push Hook": case "Tag Push Hook": - return new PushBuildAction(project, getRequestBody(request)); + return new PushBuildAction(project, getRequestBody(request), tokenHeader); case "Note Hook": - return new NoteBuildAction(project, getRequestBody(request)); + return new NoteBuildAction(project, getRequestBody(request), tokenHeader); default: LOGGER.log(Level.FINE, "Unsupported X-Gitlab-Event header: {0}", eventHeader); return new NoopAction(); @@ -150,16 +147,6 @@ public class ActionResolver { }); } - private void checkPermission(Permission permission) { - if (((GitLabConnectionConfig) Jenkins.getInstance().getDescriptor(GitLabConnectionConfig.class)).isUseAuthenticatedEndpoint()) { - try { - Jenkins.getInstance().checkPermission(permission); - } catch (AccessDeniedException2 e) { - throw HttpResponses.errorWithoutStack(403, e.getMessage()); - } - } - } - static class NoopAction implements WebHookAction { public void execute(StaplerResponse response) { } diff --git a/src/main/java/com/dabsquared/gitlabjenkins/webhook/build/BuildWebHookAction.java b/src/main/java/com/dabsquared/gitlabjenkins/webhook/build/BuildWebHookAction.java index 6bfdc7b..a81c1bd 100644 --- a/src/main/java/com/dabsquared/gitlabjenkins/webhook/build/BuildWebHookAction.java +++ b/src/main/java/com/dabsquared/gitlabjenkins/webhook/build/BuildWebHookAction.java @@ -1,6 +1,15 @@ package com.dabsquared.gitlabjenkins.webhook.build; +import com.dabsquared.gitlabjenkins.GitLabPushTrigger; +import com.dabsquared.gitlabjenkins.connection.GitLabConnectionConfig; import com.dabsquared.gitlabjenkins.webhook.WebHookAction; +import hudson.model.Item; +import hudson.model.Job; +import hudson.security.AccessDeniedException2; +import hudson.security.Permission; +import hudson.util.HttpResponses; +import jenkins.model.Jenkins; +import org.apache.commons.lang.StringUtils; import org.kohsuke.stapler.StaplerResponse; /** @@ -8,10 +17,46 @@ import org.kohsuke.stapler.StaplerResponse; */ abstract class BuildWebHookAction implements WebHookAction { abstract void processForCompatibility(); + abstract void execute(); public final void execute(StaplerResponse response) { processForCompatibility(); execute(); } + + protected abstract static class TriggerNotifier implements Runnable { + + private final Item project; + private final String secretToken; + + public TriggerNotifier(Item project, String secretToken) { + this.project = project; + this.secretToken = secretToken; + } + + public void run() { + GitLabPushTrigger trigger = GitLabPushTrigger.getFromJob((Job) project); + if (trigger != null) { + if (StringUtils.isEmpty(trigger.getSecretToken())) { + checkPermission(Item.BUILD); + } else if (!StringUtils.equals(trigger.getSecretToken(), secretToken)) { + throw HttpResponses.errorWithoutStack(401, "Invalid token"); + } + performOnPost(trigger); + } + } + + private void checkPermission(Permission permission) { + if (((GitLabConnectionConfig) Jenkins.getInstance().getDescriptor(GitLabConnectionConfig.class)).isUseAuthenticatedEndpoint()) { + try { + Jenkins.getInstance().checkPermission(permission); + } catch (AccessDeniedException2 e) { + throw HttpResponses.errorWithoutStack(403, e.getMessage()); + } + } + } + + protected abstract void performOnPost(GitLabPushTrigger trigger); + } } diff --git a/src/main/java/com/dabsquared/gitlabjenkins/webhook/build/MergeRequestBuildAction.java b/src/main/java/com/dabsquared/gitlabjenkins/webhook/build/MergeRequestBuildAction.java index 2822f64..9851a5c 100644 --- a/src/main/java/com/dabsquared/gitlabjenkins/webhook/build/MergeRequestBuildAction.java +++ b/src/main/java/com/dabsquared/gitlabjenkins/webhook/build/MergeRequestBuildAction.java @@ -23,11 +23,13 @@ public class MergeRequestBuildAction extends BuildWebHookAction { private final static Logger LOGGER = Logger.getLogger(MergeRequestBuildAction.class.getName()); private Item project; private MergeRequestHook mergeRequestHook; + private final String secretToken; - public MergeRequestBuildAction(Item project, String json) { + public MergeRequestBuildAction(Item project, String json, String secretToken) { LOGGER.log(Level.FINE, "MergeRequest: {0}", toPrettyPrint(json)); this.project = project; this.mergeRequestHook = JsonUtil.read(json, MergeRequestHook.class); + this.secretToken = secretToken; } void processForCompatibility() { @@ -50,12 +52,10 @@ public class MergeRequestBuildAction extends BuildWebHookAction { if (!(project instanceof Job)) { throw HttpResponses.errorWithoutStack(409, "Merge Request Hook is not supported for this project"); } - ACL.impersonate(ACL.SYSTEM, new Runnable() { - public void run() { - GitLabPushTrigger trigger = GitLabPushTrigger.getFromJob((Job) project); - if (trigger != null) { - trigger.onPost(mergeRequestHook); - } + ACL.impersonate(ACL.SYSTEM, new TriggerNotifier(project, secretToken) { + @Override + protected void performOnPost(GitLabPushTrigger trigger) { + trigger.onPost(mergeRequestHook); } }); throw HttpResponses.ok(); diff --git a/src/main/java/com/dabsquared/gitlabjenkins/webhook/build/NoteBuildAction.java b/src/main/java/com/dabsquared/gitlabjenkins/webhook/build/NoteBuildAction.java index 683a48b..960530a 100644 --- a/src/main/java/com/dabsquared/gitlabjenkins/webhook/build/NoteBuildAction.java +++ b/src/main/java/com/dabsquared/gitlabjenkins/webhook/build/NoteBuildAction.java @@ -23,23 +23,23 @@ public class NoteBuildAction implements WebHookAction { private final static Logger LOGGER = Logger.getLogger(NoteBuildAction.class.getName()); private Item project; private NoteHook noteHook; + private final String secretToken; - public NoteBuildAction(Item project, String json) { + public NoteBuildAction(Item project, String json, String secretToken) { LOGGER.log(Level.FINE, "Note: {0}", toPrettyPrint(json)); this.project = project; this.noteHook = JsonUtil.read(json, NoteHook.class); + this.secretToken = secretToken; } public void execute(StaplerResponse response) { if (!(project instanceof Job)) { throw HttpResponses.errorWithoutStack(409, "Note Hook is not supported for this project"); } - ACL.impersonate(ACL.SYSTEM, new Runnable() { - public void run() { - GitLabPushTrigger trigger = GitLabPushTrigger.getFromJob((Job) project); - if (trigger != null) { - trigger.onPost(noteHook); - } + ACL.impersonate(ACL.SYSTEM, new BuildWebHookAction.TriggerNotifier(project, secretToken) { + @Override + protected void performOnPost(GitLabPushTrigger trigger) { + trigger.onPost(noteHook); } }); throw HttpResponses.ok(); diff --git a/src/main/java/com/dabsquared/gitlabjenkins/webhook/build/PushBuildAction.java b/src/main/java/com/dabsquared/gitlabjenkins/webhook/build/PushBuildAction.java index d2c8ff9..1ad7025 100644 --- a/src/main/java/com/dabsquared/gitlabjenkins/webhook/build/PushBuildAction.java +++ b/src/main/java/com/dabsquared/gitlabjenkins/webhook/build/PushBuildAction.java @@ -30,13 +30,14 @@ public class PushBuildAction extends BuildWebHookAction { private final static Logger LOGGER = Logger.getLogger(PushBuildAction.class.getName()); private final Item project; - private PushHook pushHook; + private final String secretToken; - public PushBuildAction(Item project, String json) { + public PushBuildAction(Item project, String json, String secretToken) { LOGGER.log(Level.FINE, "Push: {0}", toPrettyPrint(json)); this.project = project; this.pushHook = JsonUtil.read(json, PushHook.class); + this.secretToken = secretToken; } void processForCompatibility() { @@ -64,12 +65,10 @@ public class PushBuildAction extends BuildWebHookAction { } if (project instanceof Job) { - ACL.impersonate(ACL.SYSTEM, new Runnable() { - public void run() { - GitLabPushTrigger trigger = GitLabPushTrigger.getFromJob((Job) project); - if (trigger != null) { - trigger.onPost(pushHook); - } + ACL.impersonate(ACL.SYSTEM, new TriggerNotifier(project, secretToken) { + @Override + protected void performOnPost(GitLabPushTrigger trigger) { + trigger.onPost(pushHook); } }); throw HttpResponses.ok(); @@ -104,4 +103,5 @@ public class PushBuildAction extends BuildWebHookAction { } } } + } diff --git a/src/main/resources/com/dabsquared/gitlabjenkins/GitLabPushTrigger/config.jelly b/src/main/resources/com/dabsquared/gitlabjenkins/GitLabPushTrigger/config.jelly index de977a7..6efaf65 100644 --- a/src/main/resources/com/dabsquared/gitlabjenkins/GitLabPushTrigger/config.jelly +++ b/src/main/resources/com/dabsquared/gitlabjenkins/GitLabPushTrigger/config.jelly @@ -69,5 +69,11 @@ + + + + +
+ diff --git a/src/main/webapp/help/help-secretToken.html b/src/main/webapp/help/help-secretToken.html new file mode 100644 index 0000000..bf695d0 --- /dev/null +++ b/src/main/webapp/help/help-secretToken.html @@ -0,0 +1,5 @@ +
+
+

If this is configured only WebHooks that have configured the same token can trigger a build.

+
+
diff --git a/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/MergeRequestBuildActionTest.java b/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/MergeRequestBuildActionTest.java index 2756b40..966ff92 100644 --- a/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/MergeRequestBuildActionTest.java +++ b/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/MergeRequestBuildActionTest.java @@ -75,7 +75,7 @@ public class MergeRequestBuildActionTest { testProject.addTrigger(trigger); exception.expect(HttpResponses.HttpResponseException.class); - new MergeRequestBuildAction(testProject, getJson("MergeRequestEvent.json")).execute(response); + new MergeRequestBuildAction(testProject, getJson("MergeRequestEvent.json"), null).execute(response); verify(trigger).onPost(any(MergeRequestHook.class)); } @@ -86,7 +86,7 @@ public class MergeRequestBuildActionTest { testProject.addTrigger(trigger); exception.expect(HttpResponses.HttpResponseException.class); - new MergeRequestBuildAction(testProject, getJson("MergeRequestEvent_closedMR.json")).execute(response); + new MergeRequestBuildAction(testProject, getJson("MergeRequestEvent_closedMR.json"), null).execute(response); verify(trigger, never()).onPost(any(MergeRequestHook.class)); } @@ -100,7 +100,7 @@ public class MergeRequestBuildActionTest { future.get(); exception.expect(HttpResponses.HttpResponseException.class); - new MergeRequestBuildAction(testProject, getJson("MergeRequestEvent_alreadyBuiltMR.json")).execute(response); + new MergeRequestBuildAction(testProject, getJson("MergeRequestEvent_alreadyBuiltMR.json"), null).execute(response); verify(trigger, never()).onPost(any(MergeRequestHook.class)); } @@ -138,7 +138,7 @@ public class MergeRequestBuildActionTest { future.get(); exception.expect(HttpResponses.HttpResponseException.class); - new MergeRequestBuildAction(testProject, getJson("MergeRequestEvent_alreadyBuiltMR.json")).execute(response); + new MergeRequestBuildAction(testProject, getJson("MergeRequestEvent_alreadyBuiltMR.json"), null).execute(response); verify(trigger).onPost(any(MergeRequestHook.class)); } diff --git a/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/NoteBuildActionTest.java b/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/NoteBuildActionTest.java index 4fdf808..7421e7b 100644 --- a/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/NoteBuildActionTest.java +++ b/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/NoteBuildActionTest.java @@ -74,7 +74,7 @@ public class NoteBuildActionTest { testProject.addTrigger(trigger); exception.expect(HttpResponses.HttpResponseException.class); - new NoteBuildAction(testProject, getJson("NoteEvent.json")).execute(response); + new NoteBuildAction(testProject, getJson("NoteEvent.json"), null).execute(response); verify(trigger).onPost(any(NoteHook.class)); } @@ -88,7 +88,7 @@ public class NoteBuildActionTest { future.get(); exception.expect(HttpResponses.HttpResponseException.class); - new NoteBuildAction(testProject, getJson("NoteEvent_alreadyBuiltMR.json")).execute(response); + new NoteBuildAction(testProject, getJson("NoteEvent_alreadyBuiltMR.json"), null).execute(response); verify(trigger).onPost(any(NoteHook.class)); } @@ -126,7 +126,7 @@ public class NoteBuildActionTest { future.get(); exception.expect(HttpResponses.HttpResponseException.class); - new NoteBuildAction(testProject, getJson("NoteEvent_alreadyBuiltMR.json")).execute(response); + new NoteBuildAction(testProject, getJson("NoteEvent_alreadyBuiltMR.json"), null).execute(response); verify(trigger).onPost(any(NoteHook.class)); } diff --git a/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/PushBuildActionTest.java b/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/PushBuildActionTest.java index 221f01e..6771494 100644 --- a/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/PushBuildActionTest.java +++ b/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/PushBuildActionTest.java @@ -46,7 +46,7 @@ public class PushBuildActionTest { FreeStyleProject testProject = jenkins.createFreeStyleProject(); testProject.addTrigger(trigger); - new PushBuildAction(testProject, getJson("PushEvent_missingRepositoryUrl.json")).execute(response); + new PushBuildAction(testProject, getJson("PushEvent_missingRepositoryUrl.json"), null).execute(response); verify(trigger, never()).onPost(any(PushHook.class)); } @@ -58,11 +58,24 @@ public class PushBuildActionTest { testProject.addTrigger(trigger); exception.expect(HttpResponses.HttpResponseException.class); - new PushBuildAction(testProject, getJson("PushEvent.json")).execute(response); + new PushBuildAction(testProject, getJson("PushEvent.json"), null).execute(response); verify(trigger).onPost(any(PushHook.class)); } + @Test + public void invalidToken() throws IOException { + FreeStyleProject testProject = jenkins.createFreeStyleProject(); + when(trigger.getTriggerOpenMergeRequestOnPush()).thenReturn(TriggerOpenMergeRequest.never); + when(trigger.isWebHookAuthorized("test")).thenReturn(false); + testProject.addTrigger(trigger); + + exception.expect(HttpResponses.HttpResponseException.class); + new PushBuildAction(testProject, getJson("PushEvent.json"), "test").execute(response); + + verify(trigger, never()).onPost(any(PushHook.class)); + } + private String getJson(String name) throws IOException { return IOUtils.toString(getClass().getResourceAsStream(name)); }