From c234336ddcc89a3c1ec464d074d23536abc83a11 Mon Sep 17 00:00:00 2001 From: Rohan Bansal Date: Mon, 13 Jan 2020 15:02:43 +0530 Subject: [PATCH 1/3] feat: add security headers to webhook --- .../integrations/doctype/webhook/webhook.js | 10 ++++++- .../integrations/doctype/webhook/webhook.json | 29 ++++++++++++++++++- .../integrations/doctype/webhook/webhook.py | 10 +++++++ 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/frappe/integrations/doctype/webhook/webhook.js b/frappe/integrations/doctype/webhook/webhook.js index 90b5b12dc6..8154cf82a0 100644 --- a/frappe/integrations/doctype/webhook/webhook.js +++ b/frappe/integrations/doctype/webhook/webhook.js @@ -66,7 +66,15 @@ frappe.ui.form.on('Webhook', { webhook_doctype: (frm) => { frappe.webhook.set_fieldname_select(frm); - } + }, + + enable_security: (frm) => { + frm.toggle_reqd('webhook_secret', frm.doc.enable_security); + }, + + generate_secret: (frm) => { + frm.call("generate_secret"); + }, }); frappe.ui.form.on("Webhook Data", { diff --git a/frappe/integrations/doctype/webhook/webhook.json b/frappe/integrations/doctype/webhook/webhook.json index 8aa96e6859..4c02a4403e 100644 --- a/frappe/integrations/doctype/webhook/webhook.json +++ b/frappe/integrations/doctype/webhook/webhook.json @@ -19,6 +19,10 @@ "request_url", "cb_webhook", "request_structure", + "sb_security", + "enable_security", + "webhook_secret", + "generate_secret", "sb_webhook_headers", "webhook_headers", "sb_webhook_data", @@ -127,10 +131,33 @@ "fieldtype": "Select", "label": "Naming Series", "options": "\nHOOK-.####" + }, + { + "fieldname": "sb_security", + "fieldtype": "Section Break", + "label": "Webhook Security" + }, + { + "default": "0", + "fieldname": "enable_security", + "fieldtype": "Check", + "label": "Enable Security" + }, + { + "depends_on": "eval:doc.enable_security == 1", + "fieldname": "webhook_secret", + "fieldtype": "Password", + "label": "Webhook Secret" + }, + { + "depends_on": "eval:doc.enable_security == 1", + "fieldname": "generate_secret", + "fieldtype": "Button", + "label": "Generate Secret" } ], "links": [], - "modified": "2020-01-06 02:51:07.997566", + "modified": "2020-01-13 01:11:29.392078", "modified_by": "Administrator", "module": "Integrations", "name": "Webhook", diff --git a/frappe/integrations/doctype/webhook/webhook.py b/frappe/integrations/doctype/webhook/webhook.py index 0a97022f66..070d0177b9 100644 --- a/frappe/integrations/doctype/webhook/webhook.py +++ b/frappe/integrations/doctype/webhook/webhook.py @@ -16,6 +16,8 @@ from frappe import _ from frappe.model.document import Document from frappe.utils.jinja import validate_template +WEBHOOK_SECRET_HEADER = "X-Frappe-Webhook-Key" + class Webhook(Document): def validate(self): @@ -67,6 +69,9 @@ class Webhook(Document): if len(webhook_data) != len(set(webhook_data)): frappe.throw(_("Same Field is entered more than once")) + def generate_secret(self): + self.webhook_secret = frappe.generate_hash(length=32) + def get_context(doc): return {"doc": doc, "utils": frappe.utils} @@ -94,10 +99,15 @@ def enqueue_webhook(doc, webhook): def get_webhook_headers(doc, webhook): headers = {} + + if webhook.enable_security: + headers[WEBHOOK_SECRET_HEADER] = webhook.get_password("webhook_secret") + if webhook.webhook_headers: for h in webhook.webhook_headers: if h.get("key") and h.get("value"): headers[h.get("key")] = h.get("value") + return headers From ed71b275a26c895009ac7a97009e7b279e5da2a3 Mon Sep 17 00:00:00 2001 From: Rohan Bansal Date: Mon, 13 Jan 2020 15:24:11 +0530 Subject: [PATCH 2/3] fix: remove auto-generation of secret --- frappe/integrations/doctype/webhook/webhook.js | 6 +----- frappe/integrations/doctype/webhook/webhook.json | 9 +-------- frappe/integrations/doctype/webhook/webhook.py | 3 --- 3 files changed, 2 insertions(+), 16 deletions(-) diff --git a/frappe/integrations/doctype/webhook/webhook.js b/frappe/integrations/doctype/webhook/webhook.js index 8154cf82a0..09c296113a 100644 --- a/frappe/integrations/doctype/webhook/webhook.js +++ b/frappe/integrations/doctype/webhook/webhook.js @@ -70,11 +70,7 @@ frappe.ui.form.on('Webhook', { enable_security: (frm) => { frm.toggle_reqd('webhook_secret', frm.doc.enable_security); - }, - - generate_secret: (frm) => { - frm.call("generate_secret"); - }, + } }); frappe.ui.form.on("Webhook Data", { diff --git a/frappe/integrations/doctype/webhook/webhook.json b/frappe/integrations/doctype/webhook/webhook.json index 4c02a4403e..9f979099c9 100644 --- a/frappe/integrations/doctype/webhook/webhook.json +++ b/frappe/integrations/doctype/webhook/webhook.json @@ -22,7 +22,6 @@ "sb_security", "enable_security", "webhook_secret", - "generate_secret", "sb_webhook_headers", "webhook_headers", "sb_webhook_data", @@ -148,16 +147,10 @@ "fieldname": "webhook_secret", "fieldtype": "Password", "label": "Webhook Secret" - }, - { - "depends_on": "eval:doc.enable_security == 1", - "fieldname": "generate_secret", - "fieldtype": "Button", - "label": "Generate Secret" } ], "links": [], - "modified": "2020-01-13 01:11:29.392078", + "modified": "2020-01-13 01:53:04.459968", "modified_by": "Administrator", "module": "Integrations", "name": "Webhook", diff --git a/frappe/integrations/doctype/webhook/webhook.py b/frappe/integrations/doctype/webhook/webhook.py index 070d0177b9..152ffada97 100644 --- a/frappe/integrations/doctype/webhook/webhook.py +++ b/frappe/integrations/doctype/webhook/webhook.py @@ -69,9 +69,6 @@ class Webhook(Document): if len(webhook_data) != len(set(webhook_data)): frappe.throw(_("Same Field is entered more than once")) - def generate_secret(self): - self.webhook_secret = frappe.generate_hash(length=32) - def get_context(doc): return {"doc": doc, "utils": frappe.utils} From 43ccae0327cf8d9b81ee527778a5d86793592722 Mon Sep 17 00:00:00 2001 From: Rohan Bansal Date: Mon, 13 Jan 2020 15:35:15 +0530 Subject: [PATCH 3/3] fix: hash secret before assigning to header --- frappe/integrations/doctype/webhook/webhook.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/frappe/integrations/doctype/webhook/webhook.py b/frappe/integrations/doctype/webhook/webhook.py index 152ffada97..5cbe7c0a02 100644 --- a/frappe/integrations/doctype/webhook/webhook.py +++ b/frappe/integrations/doctype/webhook/webhook.py @@ -4,7 +4,10 @@ from __future__ import unicode_literals +import base64 import datetime +import hashlib +import hmac import json from time import sleep @@ -16,7 +19,7 @@ from frappe import _ from frappe.model.document import Document from frappe.utils.jinja import validate_template -WEBHOOK_SECRET_HEADER = "X-Frappe-Webhook-Key" +WEBHOOK_SECRET_HEADER = "X-Frappe-Webhook-Signature" class Webhook(Document): @@ -98,7 +101,15 @@ def get_webhook_headers(doc, webhook): headers = {} if webhook.enable_security: - headers[WEBHOOK_SECRET_HEADER] = webhook.get_password("webhook_secret") + data = get_webhook_data(doc, webhook) + signature = base64.b64encode( + hmac.new( + webhook.get_password("webhook_secret").encode("utf8"), + json.dumps(data).encode("utf8"), + hashlib.sha256 + ).digest() + ) + headers[WEBHOOK_SECRET_HEADER] = signature if webhook.webhook_headers: for h in webhook.webhook_headers: