redmine_webhook
redmine_webhook copied to clipboard
Added HMAC signature to webhook payloads
This implemented the feature requested in #9 .
Changes
- NEW: "secret_key" text field in Webhook model.
- NEW: Webhook view & controller modified to include secret_key text field (tagged as password field in the view).
- NEW: Webhook posts contain two new headers.
-
X-RedmineWebhook-HMAC-Alg: The algorithm used for the HMAC signature. Currently hard-coded assha1. -
X-RedmineWebhook-HMAC-Signature: The HMAC signature.
-
Testing
The changes were tested against the latest docker redmine container (5.0.3 at the time of this PR). A small Python Flask server was written to perform the HMAC validation that clients are expected to do (can be found in this gist).
I've tested with three configurations: (1) an incorrect key, (2) a correct key, and (3) no key.

The Flask web server output for this is:
Project: examplebadkey
Alg: sha1
Signature: 89b3f8de7175044d4f772ec3ce7b6741aa852431
Calculated signature: b9ad1a2263b192d7d8978d13f0cb8d8ed68175d8
HMAC verification failed, payload is malformed or tampered!
HMAC verification succeeded: False
172.18.0.2 - - [23/Oct/2022 13:42:28] "POST /redminewebhook/examplebadkey HTTP/1.1" 200 -
Project: goodkey
Alg: sha1
Signature: b9ad1a2263b192d7d8978d13f0cb8d8ed68175d8
Calculated signature: b9ad1a2263b192d7d8978d13f0cb8d8ed68175d8
HMAC verification succeeded: True
172.18.0.2 - - [23/Oct/2022 13:42:28] "POST /redminewebhook/goodkey HTTP/1.1" 200 -
Project: nokey
Alg: sha1
Signature: 2880f307aa1ba774c716279b26542d6011e5fe6b
Calculated signature: b9ad1a2263b192d7d8978d13f0cb8d8ed68175d8
HMAC verification failed, payload is malformed or tampered!
HMAC verification succeeded: False
172.18.0.2 - - [23/Oct/2022 13:42:28] "POST /redminewebhook/nokey HTTP/1.1" 200 -
Potential issues / improvements
- The algorithm used for the digest is hard-coded to use SHA1. This should be configurable to some extent.
- This is potentially a breaking change with existing webhook setups. I'm not sure how database migrations work with existing entries. If the secret_key is not a proper text entry (ie. null), I expect this code could break.
Additional notes
- I followed the naming convention (somewhat) for db migrations from the main redmine project, prepending the date at the front in YYYYMMDD format.
- I do not have much experience with Ruby development, verification from a maintainer regarding code quality would be much appreciated.
I just realized this may require a version increment potentially? Let me know if that's required (or add it on top).