From 258e194777d5deb9c534e3701cd42740f2b24920 Mon Sep 17 00:00:00 2001 From: Anton Horodchuk Date: Wed, 7 Oct 2020 20:23:59 +0300 Subject: [PATCH 01/11] Signature verification --- build.gradle | 2 +- config/pluginspec.yaml | 22 +++-- dsl/properties/ec_webhook/script.groovy | 112 +++++++++++++++++------- 3 files changed, 93 insertions(+), 43 deletions(-) diff --git a/build.gradle b/build.gradle index fbd6a6a..9ab7b7e 100644 --- a/build.gradle +++ b/build.gradle @@ -16,7 +16,7 @@ dependencies { implementation 'com.electriccloud.plugins:flowpdf-groovy-lib:1.1.1.0' //That's ours - //implementation 'com.electriccloud:commander-api-bindings:9.0.0-SNAPSHOT' + implementation 'com.electriccloud:commander-api-bindings:9.0.0-SNAPSHOT' } diff --git a/config/pluginspec.yaml b/config/pluginspec.yaml index 54638a0..6ea6436 100644 --- a/config/pluginspec.yaml +++ b/config/pluginspec.yaml @@ -27,18 +27,16 @@ properties: - propertyName: ec_webhook credentialProtected: true properties: - - propertyName: displayName - value: GitHub - - propertyName: procedureName - value: webhook - - propertyName: script - value: - path: dsl/properties/ec_webhook/script.groovy - - propertyName: setupProcedure - value: SetupWebhook - - propertyName: ec_polling - properties: - displayName: GitHub + - propertyName: default + properties: + - propertyName: displayName + value: GitHub + - propertyName: procedureName + value: webhook + - propertyName: script + value: '$[/myProject/ec_webhook/script.groovy]' + - propertyName: setupProcedure + value: SetupWebhook procedures: - name: SetupWebhook diff --git a/dsl/properties/ec_webhook/script.groovy b/dsl/properties/ec_webhook/script.groovy index cfe91a2..3c491e2 100644 --- a/dsl/properties/ec_webhook/script.groovy +++ b/dsl/properties/ec_webhook/script.groovy @@ -1,50 +1,102 @@ +import groovy.json.JsonSlurper +import org.apache.commons.codec.binary.Hex + +import javax.crypto.Mac +import javax.crypto.spec.SecretKeySpec +import static java.nio.charset.StandardCharsets.UTF_8 + println args def trigger = args.trigger def headers = args.headers -def method = args.method -def body = args.body -def url = args.url +String method = args.method +String body = args.body +String url = args.url def query = args.query -// do something +// Parsing headers +def event = headers.get('X-GitHub-Event') +def signature = headers.get('X-Hub-Signature') -def event = '' -def signature = '' - -headers.each { k, v -> - if (k.toLowerCase() == 'X-GitHub-Event') { - event = v - } - if (k.toLowerCase() == 'X-Hub-Signature') { - signature = v - } +//validating signature +if (!verifySignedPayload(signature, trigger.webhookSecret, body)) { + // Todo: change to agreed exception + throw new RuntimeException("Signatures does not match. Please recheck the shared secret") } -//validate signature +// Receiving trigger parameters +//Map pluginParameters = trigger.pluginParameters +//throw new RuntimeException("params:" + pluginParameters) + +// Check branches selected if (event == 'ping') { return [ - eventType : 'ping', - webhookData : ['some data': 'some data'], - commitId : null, - commitAuthorName : null, - commitAuthorEmail: null, - branch : null, - launchWebhook : false + eventType : 'ping', + webhookData : ['some data': 'some data'], + commitId : null, + commitAuthorName : null, + commitAuthorEmail: null, + branch : null, + launchWebhook : false, + responseMessage : 'Pong' ] } else if (event == 'push') { def payload = new JsonSlurper().parseText(body) def commits = payload.commits def repo = payload.repository +} else if (event == 'pull_request') { +// opened +// edited +// closed +// assigned +// unassigned +// review_requested +// review_request_removed +// ready_for_review +// labeled +// unlabeled +// synchronize +// locked +// unlocked +// reopened + +} else if (event == 'check_run') { +// created +// completed +// rerequested +} else if (event == 'status') { +// pending, +// success, +// failure, +// error } + return [ - eventType : 'push', - webhookData : ['some data': 'some data'], - commitId : null, - commitAuthorName : null, - commitAuthorEmail: null, - branch : null, - launchWebhook : true -] \ No newline at end of file + eventType : 'push', + webhookData : ['some data': 'some data'], + commitId : null, + commitAuthorName : null, + commitAuthorEmail: null, + branch : null, + launchWebhook : false +] + +boolean verifySignedPayload(String remoteSignature, String secretToken, String payload) { + def signature = 'sha1=' + hmacSignature(payload, secretToken) + return signature.equals(remoteSignature) +} + +String hmacSignature(String data, String key) { + try { + final SecretKeySpec keySpec = new SecretKeySpec(key.getBytes(UTF_8), "HmacSHA1"); + final Mac mac = Mac.getInstance("HmacSHA1"); + mac.init(keySpec); + final byte[] rawHMACBytes = mac.doFinal(data.getBytes(UTF_8) as byte[]); + + return Hex.encodeHexString(rawHMACBytes); + } catch (Exception e) { + throw new RuntimeException("Computed invalid signature: " + e.getMessage()) + } +} \ No newline at end of file From dc8ff0293274de7e3486636869d2623d615351cf Mon Sep 17 00:00:00 2001 From: Anton Horodchuk Date: Sat, 10 Oct 2020 01:10:53 +0300 Subject: [PATCH 02/11] webhook script design and logic --- config/pluginspec.yaml | 74 ++++- dsl/properties/ec_webhook/script.groovy | 396 ++++++++++++++++++++---- 2 files changed, 399 insertions(+), 71 deletions(-) diff --git a/config/pluginspec.yaml b/config/pluginspec.yaml index 6ea6436..5d3edec 100644 --- a/config/pluginspec.yaml +++ b/config/pluginspec.yaml @@ -61,28 +61,78 @@ procedures: # this is a webhook backing procedure (gives form for the webhook) parameters: - name: repositories + label: Repositories + documentation: List of repositories, separated by a newline. Leave empty to process events for all the repositories where the webhook is set up. type: textarea - documentation: List of repositories, separated by a newline. required: false - name: pushEvent + label: Process Push Events? type: checkbox - label: Process Push? - documentation: Processes push events - - name: prEvent - type: checkbox - label: Process Pull Request? - - name: prAction - type: textarea - dependsOn: prEvent - condition: ${prEvent == "true"} - documentation: The action that was performed. Can be one of opened, edited, closed, assigned, unassigned, review_requested, review_request_removed, ready_for_review, labeled, unlabeled, synchronize, locked, unlocked, or reopened. If the action is closed and the merged key is false, the pull request was closed with unmerged commits. If the action is closed and the merged key is true, the pull request was merged. + documentation: Check this if you want trigger to be run when the new commit appears in one of the monitored branches. - name: includeBranches + label: Include Branches + documentation: | + List of branch names, separated by a comma. + Incoming events will be discarded if not relate to one of the specified branches. + Leave empty to process events for all branches except ones specified in the “excludeBranches” parameter. type: textarea dependsOn: pushEvent - condition: ${pushEvent == "true"} + condition: ${pushEvent} == "true" - name: excludeBranches + label: Exclude Branches + type: textarea + documentation: | + List of branch names, separated by a comma. + Incoming events will be discarded if relate to one of the specified branches. + Leave empty to process events for all branches or only for the specified in the "Include Branches". + required: false + dependsOn: pushEvent + condition: ${pushEvent} == "true" + - name: prEvent + label: Process Pull Requests? + documentation: | + Check this if you want the trigger to be run when Pull Request event occurs + type: checkbox + required: false + - name: includePrActions type: textarea + dependsOn: prEvent + condition: ${prEvent} == "true" + documentation: | + The action that was performed. Can be one of: +
  • opened
    • +
  • edited
    • +
  • closed
    • +
  • closed_merged
    • +
  • closed_discarded
    • +
  • assigned
    • +
  • unassigned
    • +
  • review_requested
    • +
  • review_request_removed
    • +
  • ready_for_review
    • +
  • labeled
    • +
  • unlabeled
    • +
  • synchronize
    • +
  • locked
  • +
  • unlocked
    • +
  • reopened
    • +
+ - name: commitStatusEvent + label: Process Commit Status Events? + type: checkbox + documentation: Check this if you want trigger to be run when a commit status has been changed. required: false + - name: includeCommitStatuses + label: Include Commit Status Events + type: textarea + documentation: | + Limit to following commit statuses. Comma-separated list with following statuses: +
    +
  • pending
    • +
  • success
    • +
  • failure
    • +
  • error
    • +
- name: Create Repository description: Creates a GitHub Repository diff --git a/dsl/properties/ec_webhook/script.groovy b/dsl/properties/ec_webhook/script.groovy index 3c491e2..34718a2 100644 --- a/dsl/properties/ec_webhook/script.groovy +++ b/dsl/properties/ec_webhook/script.groovy @@ -3,92 +3,141 @@ import org.apache.commons.codec.binary.Hex import javax.crypto.Mac import javax.crypto.spec.SecretKeySpec +import java.util.regex.Pattern + import static java.nio.charset.StandardCharsets.UTF_8 println args def trigger = args.trigger -def headers = args.headers +Map headers = args.headers String method = args.method String body = args.body String url = args.url def query = args.query +final ArrayList SUPPORTED_EVENTS = ['push', 'pull_request', 'status', 'ping'] + // Parsing headers -def event = headers.get('X-GitHub-Event') -def signature = headers.get('X-Hub-Signature') +String event = headers.get('X-GitHub-Event') +String signature = headers.get('X-Hub-Signature') +if (!signature) { + throw new RuntimeException("Request does not contain the signature header") +} +if (!event) { + throw new RuntimeException("Request does not contain the event header") +} +// As we do not use other restrictions, every trigger should have a signature secret +if (!trigger.webhookSecret) { + throw new RuntimeException("Trigger '${trigger.getName()}' does not have webhookSecret set up") +} //validating signature -if (!verifySignedPayload(signature, trigger.webhookSecret, body)) { +if (!verifySignedPayload(signature, (String) trigger.webhookSecret, body)) { // Todo: change to agreed exception - throw new RuntimeException("Signatures does not match. Please recheck the shared secret") + throw new RuntimeException("Signatures does not match. Please recheck the shared secrets.") } // Receiving trigger parameters -//Map pluginParameters = trigger.pluginParameters -//throw new RuntimeException("params:" + pluginParameters) +Map pluginParameters = trigger.getPluginParameters() +throw new RuntimeException("params:" + pluginParameters) -// Check branches selected +WebhookEvent webhookEvent = WebhookEvent.getForType(event, body, pluginParameters) +if (webhookEvent == null) { + return [ + launchWebhook : false, + responseMessage: "Ignoring unsupported '${event}' event" + ] +} + +// Check repository +String repositoryName = webhookEvent.getRepositoryName() +if (!repositoryName) { + throw new RuntimeException("Webhook event '${event}' doesn't contain 'repository' object") +} +if (!doCheckRepositoryIncluded(pluginParameters.get('repositories'), repositoryName)) { + return [ + eventType : event, + responseMessage: "Ignoring ${repositoryName} repository event", + launchWebhook : false + ] +} +// We can respond to ping immediately if (event == 'ping') { return [ - eventType : 'ping', - webhookData : ['some data': 'some data'], - commitId : null, - commitAuthorName : null, - commitAuthorEmail: null, - branch : null, - launchWebhook : false, - responseMessage : 'Pong' + eventType : 'ping', + responseMessage: 'pong', + launchWebhook : false ] -} else if (event == 'push') { - def payload = new JsonSlurper().parseText(body) - def commits = payload.commits - def repo = payload.repository -} else if (event == 'pull_request') { -// opened -// edited -// closed -// assigned -// unassigned -// review_requested -// review_request_removed -// ready_for_review -// labeled -// unlabeled -// synchronize -// locked -// unlocked -// reopened - -} else if (event == 'check_run') { -// created -// completed -// rerequested -} else if (event == 'status') { -// pending, -// success, -// failure, -// error -} - - -return [ - eventType : 'push', - webhookData : ['some data': 'some data'], - commitId : null, - commitAuthorName : null, - commitAuthorEmail: null, - branch : null, - launchWebhook : false +} + +if (!webhookEvent.isEnabled()) { + return [ + responseMessage: "Processing for the '${webhookEvent.getName()}' event is disabled", + launchWebhook : false + ] +} + +if (!webhookEvent.isActionEnabled()) { + String action = webhookEvent.getAction() + return [ + responseMessage: "Processing for the '${action}' of the '${event}' is disabled", + launchWebhook : false + ] +} + +String includeBranches = pluginParameters.get('includeBranches') +String excludeBranches = pluginParameters.get('excludeBranches') + +if (includeBranches) { + ArrayList branches = includeBranches.tokenize(/,\s+?/) + if (!webhookEvent.isCorrespondingToAnyBranchIn(branches)) { + String branchName = webhookEvent.getBranchNames().join(', ') + return [ + launchWebhook : false, + responseMessage: "Ignoring '${event}' event for branch '${branchName}'" + ] + } +} +if (excludeBranches) { + ArrayList branches = includeBranches.tokenize(/,\s+?/) + if (webhookEvent.isCorrespondingToAnyBranchIn(branches)) { + String branchName = webhookEvent.getBranchNames().join(', ') + return [ + launchWebhook : false, + responseMessage: "Ignoring '${event}' event for exluded branch '${branchName}'" + ] + } +} + +Map webhookData = webhookEvent.collectWebhookData() +Map recentCommit = webhookEvent.getRecentCommit() + +def response = [ + eventType : 'push', + launchWebhook: true, + branch : webhookEvent.getBranchNames().join(', ') ] -boolean verifySignedPayload(String remoteSignature, String secretToken, String payload) { +if (webhookData) { + response['webhookData'] = webhookData +} + +if (recentCommit) { + response['commitId'] = recentCommit['commitId'] + response['commitAuthorName'] = recentCommit['commitAuthorName'] + response['commitAuthorEmail'] = recentCommit['commitAuthorEmail'] +} + +return response + +private boolean verifySignedPayload(String remoteSignature, String secretToken, String payload) { def signature = 'sha1=' + hmacSignature(payload, secretToken) return signature.equals(remoteSignature) } -String hmacSignature(String data, String key) { +private String hmacSignature(String data, String key) { try { final SecretKeySpec keySpec = new SecretKeySpec(key.getBytes(UTF_8), "HmacSHA1"); final Mac mac = Mac.getInstance("HmacSHA1"); @@ -99,4 +148,233 @@ String hmacSignature(String data, String key) { } catch (Exception e) { throw new RuntimeException("Computed invalid signature: " + e.getMessage()) } -} \ No newline at end of file +} + +private static boolean doCheckRepositoryIncluded(String parameterValue, String checked) { + ArrayList list = parameterValue.tokenize(/\n/).collect({ it.trim() }) + return listContainsStrictMatch(list, checked) +} + +private static boolean doCheckActionIncluded(String parameterValue, String checked) { + if (!parameterValue) return true + ArrayList list = parameterValue.tokenize(/,\s+?/).collect({ it.trim() }) + return listContainsStrictMatch(list, checked) +} + +private static boolean doCheckBranchIncluded(String parameterValue, String checked) { + ArrayList list = parameterValue.tokenize(/,\s+?/).collect({ it.trim() }) + return listContainsGlobMatch(list, checked) +} + +private static boolean listContainsStrictMatch(ArrayList list, String checked) { + return list.contains(checked) +} + +private static boolean listContainsGlobMatch(ArrayList list, String checked) { + for (String l : list) { + def pattern = Pattern.compile(l) + if (checked ==~ pattern) { + return true + } + } + return false +} + +abstract class WebhookEvent { + abstract String name + + @Lazy + String action = { + payload.get('action') + }() + + Map payload + boolean enabled + + abstract static String enabledParameterName + abstract static String includedActionsParameterName + + WebhookEvent(String payload, Map triggerPluginParameters) { + this.payload = (new JsonSlurper()).parseText(payload) as Map + this.enabled = isEnabled(triggerPluginParameters) + } + + static WebhookEvent getForType(String event, String payload, Map triggerPluginParameters) { + if (event == 'pull_request') { + return new PullRequestEvent(payload, triggerPluginParameters) + } else if (event == 'push') { + return new PushEvent(payload, triggerPluginParameters) + } else if (event == 'status') { + return new CommitStatusEvent(payload, triggerPluginParameters) + } else { + // This should be handled by the SUPPORTED_EVENTS check, but just in case + throw new RuntimeException("Yep, there is no handling for '${event}' event yet.") + } + } + + boolean isEnabled() { enabled } + + private boolean checkEnabled(Map triggerPluginParameters) { + if (triggerPluginParameters[enabledParameterName] == 'false') { + return false + } + if (includedActionsParameterName) { + String actionsIncluded = triggerPluginParameters[includedActionsParameterName] + doCheckActionIncluded(actionsIncluded, this.action) + } + } + + String getRepositoryName() { return payload?.get('repository')?.get('full_name') } + + abstract ArrayList getBranchNames() + + abstract ArrayList> getCommits() + + abstract Map getRecentCommit() + + abstract Map collectWebhookData() + + boolean isCorrespondingToBranch(String branchName) { + return getBranchNames().contains(branchName) + } + + boolean isCorrespondingToAnyBranchIn(ArrayList branchNames) { + // TODO: Rewrite to map + constant check + for (String branchName : branchNames) { + if (this.isCorrespondingToBranch(branchName)) { + return true + } + } + return false + } + +} + +class PullRequestEvent extends WebhookEvent { + + static String name = 'pull_request' + static String enabledParameterName = 'prEvent' + static String includedActionsParameterName = 'includePrActions' + + PullRequestEvent(String payload, Map triggerPluginParameters) { + super(payload, triggerPluginParameters) + } + + @Lazy + ArrayList branchNames = { + String refName = payload.get('pull_request')?.get('head')?.get('ref') + if (!refName) { + return null + } + return [refName.replace('/refs/heads/', '')] + }() + + @Lazy + ArrayList> commits = { + Map prHead = payload.get('head') + return [ + [ + commitId : prHead['sha'], + branch : prHead['ref'], + commitAuthorName : prHead['user']['login'], + //TODO: check if we should request additionally + commitAuthorEmail: null, + ] as Map + ] + }() + + @Lazy + Map recentCommit = { + if (!commits || !commits.size()) { + return null + } + return commits.first() + }() + + @Override + Map collectWebhookData() { + return null + } +} + +class PushEvent extends WebhookEvent { + + static String name = 'push' + static String enabledParameterName = 'pushEvent' + static String includedActionsParameterName = null + + PushEvent(String payload, Map triggerPluginParameters) { + super(payload, triggerPluginParameters) + } + + @Lazy + ArrayList branchNames = { + String refName = payload.get('ref') + if (!refName) { + return null + } + if (!refName.matches('/refs/heads')) { + // This is not a branch push + throw new RuntimeException("Only the branch 'push' events are supported.") + } + return [refName.replace('/refs/heads/', '')] + }() + + @Lazy + ArrayList> commits = { + ArrayList> res = new ArrayList<>() + payload.get('commits').each { Map commit -> + return [ + commitId : commit['sha'], + commitMessage : commit['message'], + commitAuthorName : commit['author']['name'], + commitAuthorEmail: commit['author']['email'], + ] + } + return res + }() + + @Override + Map getRecentCommit() { + return null + } + + @Override + Map collectWebhookData() { + return null + } +} + +class CommitStatusEvent extends WebhookEvent { + + static String enabledParameterName = 'commitStatusEvent' + static String includedActions = 'includeCommitStatuses' + + CommitStatusEvent(String payload, Map triggerPluginParameters) { + super(payload, triggerPluginParameters) + } + + @Lazy + ArrayList branchNames = { + ArrayList> commitBranches = payload.get('branches') as ArrayList> + if (!commitBranches || !commitBranches.size()) { + return null + } + return commitBranches.collect({ it.get('name') }) + }() + + @Override + ArrayList> getCommits() { + return null + } + + @Override + Map getRecentCommit() { + return null + } + + @Override + Map collectWebhookData() { + return null + } +} From 7b11f933d50f86b10f4af1a72168f97e463aa85f Mon Sep 17 00:00:00 2001 From: Anton Horodchuk Date: Sat, 10 Oct 2020 14:13:56 +0300 Subject: [PATCH 03/11] Moving the business logic out from the WebhookEvent classes --- dsl/properties/ec_webhook/script.groovy | 172 ++++++++++++------------ 1 file changed, 84 insertions(+), 88 deletions(-) diff --git a/dsl/properties/ec_webhook/script.groovy b/dsl/properties/ec_webhook/script.groovy index 34718a2..b54ce3c 100644 --- a/dsl/properties/ec_webhook/script.groovy +++ b/dsl/properties/ec_webhook/script.groovy @@ -16,7 +16,24 @@ String body = args.body String url = args.url def query = args.query -final ArrayList SUPPORTED_EVENTS = ['push', 'pull_request', 'status', 'ping'] +final Map SUPPORTED_EVENTS = [ + 'push' : [ + enabledParamName: 'pushEvent', + actionsParamName: null + ], + 'pull_request': [ + enabledParamName: 'prEvent', + actionsParamName: 'includePrActions' + ], + 'status' : [ + enabledParamName: 'commitStatusEvent', + actionsParamName: 'includeCommitStatuses' + ], + 'ping' : [ + enabledParamName: null, + actionsParamName: null + ], +] // Parsing headers String event = headers.get('X-GitHub-Event') @@ -35,14 +52,14 @@ if (!trigger.webhookSecret) { //validating signature if (!verifySignedPayload(signature, (String) trigger.webhookSecret, body)) { // Todo: change to agreed exception - throw new RuntimeException("Signatures does not match. Please recheck the shared secrets.") + throw new RuntimeException("Signatures do not match. Please check that the trigger's 'webhookSecret' field value matches one specified in the Github repository webhook settings.") } // Receiving trigger parameters Map pluginParameters = trigger.getPluginParameters() throw new RuntimeException("params:" + pluginParameters) -WebhookEvent webhookEvent = WebhookEvent.getForType(event, body, pluginParameters) +WebhookEvent webhookEvent = WebhookEvent.getForType(event, body) if (webhookEvent == null) { return [ launchWebhook : false, @@ -72,28 +89,35 @@ if (event == 'ping') { ] } -if (!webhookEvent.isEnabled()) { +// Check event enabled +boolean eventEnabled = pluginParameters.get(SUPPORTED_EVENTS[event]['enabledParamName']) != 'false' +if (!eventEnabled) { return [ responseMessage: "Processing for the '${webhookEvent.getName()}' event is disabled", launchWebhook : false ] } -if (!webhookEvent.isActionEnabled()) { - String action = webhookEvent.getAction() +// Check action enabled +String includedActions = pluginParameters.get(SUPPORTED_EVENTS[event]['actionsParamName']) +String action = webhookEvent.getAction() +boolean actionEnabled = doCheckActionIncluded(includedActions, action) +if (!actionEnabled) { return [ - responseMessage: "Processing for the '${action}' of the '${event}' is disabled", + responseMessage: "Processing for the '${action}' action of the '${event}' event is disabled", launchWebhook : false ] } +// Check that branch is included and not excluded String includeBranches = pluginParameters.get('includeBranches') String excludeBranches = pluginParameters.get('excludeBranches') +ArrayList eventBranches = webhookEvent.getBranchNames() +String branchName = eventBranches.join(', ') + if (includeBranches) { - ArrayList branches = includeBranches.tokenize(/,\s+?/) - if (!webhookEvent.isCorrespondingToAnyBranchIn(branches)) { - String branchName = webhookEvent.getBranchNames().join(', ') + if (!doCheckBranchIncluded(includeBranches, eventBranches)) { return [ launchWebhook : false, responseMessage: "Ignoring '${event}' event for branch '${branchName}'" @@ -101,9 +125,7 @@ if (includeBranches) { } } if (excludeBranches) { - ArrayList branches = includeBranches.tokenize(/,\s+?/) - if (webhookEvent.isCorrespondingToAnyBranchIn(branches)) { - String branchName = webhookEvent.getBranchNames().join(', ') + if (doCheckBranchIncluded(excludeBranches, eventBranches)) { return [ launchWebhook : false, responseMessage: "Ignoring '${event}' event for exluded branch '${branchName}'" @@ -111,14 +133,15 @@ if (excludeBranches) { } } +// Collect data for response Map webhookData = webhookEvent.collectWebhookData() Map recentCommit = webhookEvent.getRecentCommit() -def response = [ - eventType : 'push', +Map response = [ + eventType : event, launchWebhook: true, branch : webhookEvent.getBranchNames().join(', ') -] +] as Map if (webhookData) { response['webhookData'] = webhookData @@ -132,12 +155,41 @@ if (recentCommit) { return response -private boolean verifySignedPayload(String remoteSignature, String secretToken, String payload) { +/** + * These methods depend on the form declaration + */ + +private static boolean doCheckRepositoryIncluded(String parameterValue, String checked) { + ArrayList list = parameterValue.tokenize(/\n/).collect({ it.trim() }) + return list.contains(checked) +} + +private static boolean doCheckActionIncluded(String parameterValue, String checked) { + if (!parameterValue) return true + ArrayList list = parameterValue.tokenize(/,\s+?/).collect({ it.trim() }) + return list.contains(checked) +} + +private static boolean doCheckBranchIncluded(String parameterValue, ArrayList checked) { + ArrayList list = parameterValue.tokenize(/,\s+?/).collect({ it.trim() }) + for (String b : checked) { + if (listContainsGlobMatch(list, b)) { + return true + } + } + return false +} + +////////////////////////////////////////////////////////////////////////////////////////////// +// End of business logic +////////////////////////////////////////////////////////////////////////////////////////////// + +static boolean verifySignedPayload(String remoteSignature, String secretToken, String payload) { def signature = 'sha1=' + hmacSignature(payload, secretToken) return signature.equals(remoteSignature) } -private String hmacSignature(String data, String key) { +static String hmacSignature(String data, String key) { try { final SecretKeySpec keySpec = new SecretKeySpec(key.getBytes(UTF_8), "HmacSHA1"); final Mac mac = Mac.getInstance("HmacSHA1"); @@ -150,27 +202,7 @@ private String hmacSignature(String data, String key) { } } -private static boolean doCheckRepositoryIncluded(String parameterValue, String checked) { - ArrayList list = parameterValue.tokenize(/\n/).collect({ it.trim() }) - return listContainsStrictMatch(list, checked) -} - -private static boolean doCheckActionIncluded(String parameterValue, String checked) { - if (!parameterValue) return true - ArrayList list = parameterValue.tokenize(/,\s+?/).collect({ it.trim() }) - return listContainsStrictMatch(list, checked) -} - -private static boolean doCheckBranchIncluded(String parameterValue, String checked) { - ArrayList list = parameterValue.tokenize(/,\s+?/).collect({ it.trim() }) - return listContainsGlobMatch(list, checked) -} - -private static boolean listContainsStrictMatch(ArrayList list, String checked) { - return list.contains(checked) -} - -private static boolean listContainsGlobMatch(ArrayList list, String checked) { +static boolean listContainsGlobMatch(ArrayList list, String checked) { for (String l : list) { def pattern = Pattern.compile(l) if (checked ==~ pattern) { @@ -189,42 +221,25 @@ abstract class WebhookEvent { }() Map payload - boolean enabled - - abstract static String enabledParameterName - abstract static String includedActionsParameterName - WebhookEvent(String payload, Map triggerPluginParameters) { + WebhookEvent(String payload) { this.payload = (new JsonSlurper()).parseText(payload) as Map - this.enabled = isEnabled(triggerPluginParameters) } - static WebhookEvent getForType(String event, String payload, Map triggerPluginParameters) { + static WebhookEvent getForType(String event, String payload) { if (event == 'pull_request') { - return new PullRequestEvent(payload, triggerPluginParameters) + return new PullRequestEvent(payload) } else if (event == 'push') { - return new PushEvent(payload, triggerPluginParameters) + return new PushEvent(payload) } else if (event == 'status') { - return new CommitStatusEvent(payload, triggerPluginParameters) + return new CommitStatusEvent(payload) } else { // This should be handled by the SUPPORTED_EVENTS check, but just in case throw new RuntimeException("Yep, there is no handling for '${event}' event yet.") } } - boolean isEnabled() { enabled } - - private boolean checkEnabled(Map triggerPluginParameters) { - if (triggerPluginParameters[enabledParameterName] == 'false') { - return false - } - if (includedActionsParameterName) { - String actionsIncluded = triggerPluginParameters[includedActionsParameterName] - doCheckActionIncluded(actionsIncluded, this.action) - } - } - - String getRepositoryName() { return payload?.get('repository')?.get('full_name') } + String getRepositoryName() { return payload.get('repository')?.get('full_name') } abstract ArrayList getBranchNames() @@ -234,30 +249,14 @@ abstract class WebhookEvent { abstract Map collectWebhookData() - boolean isCorrespondingToBranch(String branchName) { - return getBranchNames().contains(branchName) - } - - boolean isCorrespondingToAnyBranchIn(ArrayList branchNames) { - // TODO: Rewrite to map + constant check - for (String branchName : branchNames) { - if (this.isCorrespondingToBranch(branchName)) { - return true - } - } - return false - } - } class PullRequestEvent extends WebhookEvent { static String name = 'pull_request' - static String enabledParameterName = 'prEvent' - static String includedActionsParameterName = 'includePrActions' - PullRequestEvent(String payload, Map triggerPluginParameters) { - super(payload, triggerPluginParameters) + PullRequestEvent(String payload) { + super(payload) } @Lazy @@ -300,11 +299,9 @@ class PullRequestEvent extends WebhookEvent { class PushEvent extends WebhookEvent { static String name = 'push' - static String enabledParameterName = 'pushEvent' - static String includedActionsParameterName = null - PushEvent(String payload, Map triggerPluginParameters) { - super(payload, triggerPluginParameters) + PushEvent(String payload) { + super(payload) } @Lazy @@ -347,11 +344,10 @@ class PushEvent extends WebhookEvent { class CommitStatusEvent extends WebhookEvent { - static String enabledParameterName = 'commitStatusEvent' - static String includedActions = 'includeCommitStatuses' + static String name = 'status' - CommitStatusEvent(String payload, Map triggerPluginParameters) { - super(payload, triggerPluginParameters) + CommitStatusEvent(String payload) { + super(payload) } @Lazy From f5692070ecc594f6787a0df57c34af435375c844 Mon Sep 17 00:00:00 2001 From: Anton Horodchuk Date: Mon, 12 Oct 2020 15:02:46 +0300 Subject: [PATCH 04/11] Adding webhook data and cleaning code --- dsl/properties/ec_webhook/script.groovy | 119 ++++++++++++++++++------ 1 file changed, 88 insertions(+), 31 deletions(-) diff --git a/dsl/properties/ec_webhook/script.groovy b/dsl/properties/ec_webhook/script.groovy index b54ce3c..89c8f58 100644 --- a/dsl/properties/ec_webhook/script.groovy +++ b/dsl/properties/ec_webhook/script.groovy @@ -16,6 +16,7 @@ String body = args.body String url = args.url def query = args.query +// This map corresponds to the procedure form final Map SUPPORTED_EVENTS = [ 'push' : [ enabledParamName: 'pushEvent', @@ -90,23 +91,34 @@ if (event == 'ping') { } // Check event enabled -boolean eventEnabled = pluginParameters.get(SUPPORTED_EVENTS[event]['enabledParamName']) != 'false' -if (!eventEnabled) { - return [ - responseMessage: "Processing for the '${webhookEvent.getName()}' event is disabled", - launchWebhook : false - ] +String eventEnabledParamName = SUPPORTED_EVENTS[event]['enabledParamName'] +if (eventEnabledParamName != null) { + + boolean eventEnabled = pluginParameters.get(eventEnabledParamName) != 'false' + if (!eventEnabled) { + return [ + responseMessage: "Processing for the '${webhookEvent.getName()}' event is disabled", + launchWebhook : false + ] + } } + // Check action enabled -String includedActions = pluginParameters.get(SUPPORTED_EVENTS[event]['actionsParamName']) -String action = webhookEvent.getAction() -boolean actionEnabled = doCheckActionIncluded(includedActions, action) -if (!actionEnabled) { - return [ - responseMessage: "Processing for the '${action}' action of the '${event}' event is disabled", - launchWebhook : false - ] +String includedActionParamName = SUPPORTED_EVENTS[event]['actionsParamName'] +if (includedActionParamName != null) { + + String includedActions = pluginParameters.get(includedActionParamName) + String eventAction = webhookEvent.getAction() + + boolean actionEnabled = doCheckActionIncluded(includedActions, eventAction) + + if (!actionEnabled) { + return [ + responseMessage: "Processing for the '${eventAction}' action of the '${event}' event is disabled", + launchWebhook : false + ] + } } // Check that branch is included and not excluded @@ -191,12 +203,12 @@ static boolean verifySignedPayload(String remoteSignature, String secretToken, S static String hmacSignature(String data, String key) { try { - final SecretKeySpec keySpec = new SecretKeySpec(key.getBytes(UTF_8), "HmacSHA1"); - final Mac mac = Mac.getInstance("HmacSHA1"); - mac.init(keySpec); - final byte[] rawHMACBytes = mac.doFinal(data.getBytes(UTF_8) as byte[]); + final SecretKeySpec keySpec = new SecretKeySpec(key.getBytes(UTF_8), "HmacSHA1") + final Mac mac = Mac.getInstance("HmacSHA1") + mac.init(keySpec) + final byte[] rawHMACBytes = mac.doFinal(data.getBytes(UTF_8) as byte[]) - return Hex.encodeHexString(rawHMACBytes); + return Hex.encodeHexString(rawHMACBytes) } catch (Exception e) { throw new RuntimeException("Computed invalid signature: " + e.getMessage()) } @@ -214,6 +226,7 @@ static boolean listContainsGlobMatch(ArrayList list, String checked) { abstract class WebhookEvent { abstract String name + abstract String body @Lazy String action = { @@ -223,7 +236,8 @@ abstract class WebhookEvent { Map payload WebhookEvent(String payload) { - this.payload = (new JsonSlurper()).parseText(payload) as Map + this.body = payload + this.payload = (new JsonSlurper()).parseText(this.body) as Map } static WebhookEvent getForType(String event, String payload) { @@ -255,10 +269,6 @@ class PullRequestEvent extends WebhookEvent { static String name = 'pull_request' - PullRequestEvent(String payload) { - super(payload) - } - @Lazy ArrayList branchNames = { String refName = payload.get('pull_request')?.get('head')?.get('ref') @@ -270,12 +280,13 @@ class PullRequestEvent extends WebhookEvent { @Lazy ArrayList> commits = { - Map prHead = payload.get('head') + def prHead = payload['head'] return [ [ commitId : prHead['sha'], branch : prHead['ref'], commitAuthorName : prHead['user']['login'], + //TODO: check if we should request additionally commitAuthorEmail: null, ] as Map @@ -290,9 +301,39 @@ class PullRequestEvent extends WebhookEvent { return commits.first() }() + PullRequestEvent(String payload) { + super(payload) + } + @Override Map collectWebhookData() { - return null + def pullRequest = payload['pull_request'] + return [ + number : pullRequest['number'], + title : pullRequest['title'], + body : pullRequest['body'], + state : pullRequest['state'], + url : pullRequest['html_url'], + payload: this.body + ] as Map + } + + /** + * We are adding two additional virtual actions: closed_merged, closed_discarded + */ + @Override + String getAction() { + String action = payload.get('action') + if (action == 'closed') { + def pullRequest = payload.get('pull_request') + if (pullRequest['merged'] == 'true') { + return 'closed_merged' + } else { + return 'closed_discarded' + } + + } + return action } } @@ -333,12 +374,17 @@ class PushEvent extends WebhookEvent { @Override Map getRecentCommit() { - return null + if (!commits || !commits.size()) return null + return commits.first() } @Override Map collectWebhookData() { - return null + return [ + ref : payload['ref'], + branch : getBranchNames().join(', '), + payload: this.body + ] as Map } } @@ -361,16 +407,27 @@ class CommitStatusEvent extends WebhookEvent { @Override ArrayList> getCommits() { - return null + // Single commit in an array + return [getRecentCommit()] } @Override Map getRecentCommit() { - return null + def commit = payload['commit'] + return [ + commitId : commit['sha'], + commitMessage : commit['message'], + commitAuthorName : commit['commiter']['name'], + commitAuthorEmail: commit['commiter']['email'], + ] as Map } @Override Map collectWebhookData() { - return null + return [ + sha : payload['sha'], + state : payload['state'], + payload: this.body + ] as Map } } From e0c4a1e43c0038a6d345af664c8a7e19186a793b Mon Sep 17 00:00:00 2001 From: Anton Horodchuk Date: Mon, 12 Oct 2020 15:16:16 +0300 Subject: [PATCH 05/11] Processing the trigger parameters Adding 'ping' event to a scheme --- dsl/properties/ec_webhook/script.groovy | 60 +++++++++++++++---------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/dsl/properties/ec_webhook/script.groovy b/dsl/properties/ec_webhook/script.groovy index 89c8f58..7ae800b 100644 --- a/dsl/properties/ec_webhook/script.groovy +++ b/dsl/properties/ec_webhook/script.groovy @@ -57,8 +57,12 @@ if (!verifySignedPayload(signature, (String) trigger.webhookSecret, body)) { } // Receiving trigger parameters -Map pluginParameters = trigger.getPluginParameters() -throw new RuntimeException("params:" + pluginParameters) +def paramsPropertySheet = trigger.pluginParameters +Map pluginParameters = [:] +paramsPropertySheet['properties'].each { String k, Map v -> + pluginParameters[k] = v['value'] +} + WebhookEvent webhookEvent = WebhookEvent.getForType(event, body) if (webhookEvent == null) { @@ -81,15 +85,6 @@ if (!doCheckRepositoryIncluded(pluginParameters.get('repositories'), repositoryN ] } -// We can respond to ping immediately -if (event == 'ping') { - return [ - eventType : 'ping', - responseMessage: 'pong', - launchWebhook : false - ] -} - // Check event enabled String eventEnabledParamName = SUPPORTED_EVENTS[event]['enabledParamName'] if (eventEnabledParamName != null) { @@ -241,15 +236,16 @@ abstract class WebhookEvent { } static WebhookEvent getForType(String event, String payload) { - if (event == 'pull_request') { + if (event == 'ping') { + return new PingEvent(payload) + } else if (event == 'pull_request') { return new PullRequestEvent(payload) } else if (event == 'push') { return new PushEvent(payload) } else if (event == 'status') { return new CommitStatusEvent(payload) } else { - // This should be handled by the SUPPORTED_EVENTS check, but just in case - throw new RuntimeException("Yep, there is no handling for '${event}' event yet.") + return null } } @@ -257,14 +253,35 @@ abstract class WebhookEvent { abstract ArrayList getBranchNames() - abstract ArrayList> getCommits() - abstract Map getRecentCommit() abstract Map collectWebhookData() } +class PingEvent extends WebhookEvent { + static String name = 'ping' + + PingEvent(String payload) { + super(payload) + } + + @Override + ArrayList getBranchNames() { + return null + } + + @Override + Map getRecentCommit() { + return null + } + + @Override + Map collectWebhookData() { + return null + } +} + class PullRequestEvent extends WebhookEvent { static String name = 'pull_request' @@ -293,13 +310,11 @@ class PullRequestEvent extends WebhookEvent { ] }() - @Lazy - Map recentCommit = { - if (!commits || !commits.size()) { - return null - } + @Override + Map getRecentCommit() { + if (!commits || !commits.size()) return null return commits.first() - }() + } PullRequestEvent(String payload) { super(payload) @@ -405,7 +420,6 @@ class CommitStatusEvent extends WebhookEvent { return commitBranches.collect({ it.get('name') }) }() - @Override ArrayList> getCommits() { // Single commit in an array return [getRecentCommit()] From e7e7a3ec396cf6b76fc88603c005d230d6fd7461 Mon Sep 17 00:00:00 2001 From: Anton Horodchuk Date: Mon, 12 Oct 2020 16:36:43 +0300 Subject: [PATCH 06/11] Fixing some of the checks --- dsl/properties/ec_webhook/script.groovy | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/dsl/properties/ec_webhook/script.groovy b/dsl/properties/ec_webhook/script.groovy index 7ae800b..abe0c17 100644 --- a/dsl/properties/ec_webhook/script.groovy +++ b/dsl/properties/ec_webhook/script.groovy @@ -67,8 +67,8 @@ paramsPropertySheet['properties'].each { String k, Map v -> WebhookEvent webhookEvent = WebhookEvent.getForType(event, body) if (webhookEvent == null) { return [ - launchWebhook : false, - responseMessage: "Ignoring unsupported '${event}' event" + responseMessage: "Ignoring unsupported '${event}' event", + launchWebhook : false ] } @@ -79,7 +79,6 @@ if (!repositoryName) { } if (!doCheckRepositoryIncluded(pluginParameters.get('repositories'), repositoryName)) { return [ - eventType : event, responseMessage: "Ignoring ${repositoryName} repository event", launchWebhook : false ] @@ -126,16 +125,16 @@ String branchName = eventBranches.join(', ') if (includeBranches) { if (!doCheckBranchIncluded(includeBranches, eventBranches)) { return [ - launchWebhook : false, - responseMessage: "Ignoring '${event}' event for branch '${branchName}'" + responseMessage: "Ignoring '${event}' event for branch '${branchName}'", + launchWebhook : false ] } } if (excludeBranches) { if (doCheckBranchIncluded(excludeBranches, eventBranches)) { return [ - launchWebhook : false, - responseMessage: "Ignoring '${event}' event for exluded branch '${branchName}'" + responseMessage: "Ignoring '${event}' event for exluded branch '${branchName}'", + launchWebhook : false ] } } @@ -167,7 +166,7 @@ return response */ private static boolean doCheckRepositoryIncluded(String parameterValue, String checked) { - ArrayList list = parameterValue.tokenize(/\n/).collect({ it.trim() }) + ArrayList list = parameterValue.tokenize("\n").collect({ it.trim() }) return list.contains(checked) } @@ -366,9 +365,9 @@ class PushEvent extends WebhookEvent { if (!refName) { return null } - if (!refName.matches('/refs/heads')) { + if (!refName.contains('refs/heads/')) { // This is not a branch push - throw new RuntimeException("Only the branch 'push' events are supported.") + throw new RuntimeException("Only the branch 'push' events are supported, got '${refName}'") } return [refName.replace('/refs/heads/', '')] }() From d391b89935e67910d54b4a088c850d08ea869c55 Mon Sep 17 00:00:00 2001 From: Anton Horodchuk Date: Mon, 12 Oct 2020 16:50:22 +0300 Subject: [PATCH 07/11] Fixing branch name glob check --- dsl/properties/ec_webhook/script.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dsl/properties/ec_webhook/script.groovy b/dsl/properties/ec_webhook/script.groovy index abe0c17..1304b19 100644 --- a/dsl/properties/ec_webhook/script.groovy +++ b/dsl/properties/ec_webhook/script.groovy @@ -211,7 +211,7 @@ static String hmacSignature(String data, String key) { static boolean listContainsGlobMatch(ArrayList list, String checked) { for (String l : list) { def pattern = Pattern.compile(l) - if (checked ==~ pattern) { + if (pattern.matcher(checked).lookingAt()) { return true } } @@ -296,7 +296,7 @@ class PullRequestEvent extends WebhookEvent { @Lazy ArrayList> commits = { - def prHead = payload['head'] + def prHead = payload['pull_request']['head'] return [ [ commitId : prHead['sha'], From c1a2cbb104bc52b8358502d763660885787f2932 Mon Sep 17 00:00:00 2001 From: Anton Horodchuk Date: Mon, 12 Oct 2020 17:44:45 +0300 Subject: [PATCH 08/11] Fixing commit status author JSON path --- dsl/properties/ec_webhook/script.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dsl/properties/ec_webhook/script.groovy b/dsl/properties/ec_webhook/script.groovy index 1304b19..6b4064c 100644 --- a/dsl/properties/ec_webhook/script.groovy +++ b/dsl/properties/ec_webhook/script.groovy @@ -430,8 +430,8 @@ class CommitStatusEvent extends WebhookEvent { return [ commitId : commit['sha'], commitMessage : commit['message'], - commitAuthorName : commit['commiter']['name'], - commitAuthorEmail: commit['commiter']['email'], + commitAuthorName : commit['commit']['author']['name'], + commitAuthorEmail: commit['commit']['author']['email'], ] as Map } From a1a2ab91c3bca8b3e4461d97fc48b919309a810e Mon Sep 17 00:00:00 2001 From: Anton Horodchuk Date: Mon, 12 Oct 2020 17:54:02 +0300 Subject: [PATCH 09/11] Version bump --- config/pluginspec.yaml | 3 +-- help/changelog.yaml | 4 +++- help/metadata.yaml | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/config/pluginspec.yaml b/config/pluginspec.yaml index c62643d..e6ecd4f 100644 --- a/config/pluginspec.yaml +++ b/config/pluginspec.yaml @@ -1,6 +1,6 @@ pluginInfo: pluginName: EC-Github - version: 3.3.0 + version: 3.4.0 description: CloudBees CD integration for Github API author: Polina authorUrl: pshubina@cloudbees.com @@ -102,7 +102,6 @@ procedures: The action that was performed. Can be one of:
  • opened
  • edited
    • -
  • closed
  • closed_merged
  • closed_discarded
  • assigned
    • diff --git a/help/changelog.yaml b/help/changelog.yaml index 11448ed..cda7698 100644 --- a/help/changelog.yaml +++ b/help/changelog.yaml @@ -19,4 +19,6 @@ 3.0.1: - Fixed setup procedure (with proper classpath calculation) 3.3.0: - - Added Creat Pull Request procedure. + - Added Create Pull Request procedure. +3.4.0: + - Added webhooks support. diff --git a/help/metadata.yaml b/help/metadata.yaml index 73c2394..2a5603e 100644 --- a/help/metadata.yaml +++ b/help/metadata.yaml @@ -1,5 +1,5 @@ overview: | - This plugin integates with Github API. + This plugin integrates with Github API. excludeProcedures: - flowpdk-setup From 70b5dc6444299ee8660ab9ce73c1f8030a43c9c2 Mon Sep 17 00:00:00 2001 From: Anton Horodchuk Date: Mon, 12 Oct 2020 18:04:40 +0300 Subject: [PATCH 10/11] Commenting bindings back --- build.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index 9ab7b7e..6babed9 100644 --- a/build.gradle +++ b/build.gradle @@ -15,8 +15,8 @@ dependencies { implementation 'org.kohsuke:github-api:1.95' implementation 'com.electriccloud.plugins:flowpdf-groovy-lib:1.1.1.0' - //That's ours - implementation 'com.electriccloud:commander-api-bindings:9.0.0-SNAPSHOT' + //That's ours (you can uncomment this for local testing) +// implementation 'com.electriccloud:commander-api-bindings:9.0.0-SNAPSHOT' } From d9f2a3d2c5fc8dbb5cf63927c15d9edd73c57e58 Mon Sep 17 00:00:00 2001 From: Anton Horodchuk Date: Tue, 13 Oct 2020 13:57:09 +0300 Subject: [PATCH 11/11] Fixing the Push event branch name computing --- dsl/properties/ec_webhook/script.groovy | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dsl/properties/ec_webhook/script.groovy b/dsl/properties/ec_webhook/script.groovy index 6b4064c..abadedf 100644 --- a/dsl/properties/ec_webhook/script.groovy +++ b/dsl/properties/ec_webhook/script.groovy @@ -369,7 +369,11 @@ class PushEvent extends WebhookEvent { // This is not a branch push throw new RuntimeException("Only the branch 'push' events are supported, got '${refName}'") } - return [refName.replace('/refs/heads/', '')] + + // Stripping the ref path + String branchName = refName.replaceAll(/^refs\/heads\//, '') + + return [branchName] }() @Lazy