Repository: slackapi/python-slackclient
Pull request #686 (2020-05-14 05:33:45)

Add SignatureVerifier for request verification
###  Summary

This pull request adds the official implementation of Slack request verification. We have been having this logic as a static method in `BaseClient` but it's not the right place to have it. 

When we release version 2.6.0 with this change, I'll update this part of the document. https://slack.dev/python-slackclient/basic_usage.html#opening-a-modal

### Requirements (place an `x` in each `[ ]`)

* [x] I've read and understood the [Contributing Guidelines](https://github.com/slackapi/python-slackclient/blob/master/.github/contributing.md) and have done my best effort to follow them.
* [x] I've read and agree to the [Code of Conduct](https://slackhq.github.io/code-of-conduct).

codecov[bot]: # [Codecov](https://codecov.io/gh/slackapi/python-slackclient/pull/686?src=pr&el=h1) Report
> Merging [#686](https://codecov.io/gh/slackapi/python-slackclient/pull/686?src=pr&el=desc) into [master](https://codecov.io/gh/slackapi/python-slackclient/commit/af79b19508e115994f8b917ccb5bce06c12cfba2&el=desc) will **decrease** coverage by `0.00%`.
> The diff coverage is `85.71%`.

[![Impacted file tree graph](https://codecov.io/gh/slackapi/python-slackclient/pull/686/graphs/tree.svg?width=650&height=150&src=pr&token=F0O7meYmtD)](https://codecov.io/gh/slackapi/python-slackclient/pull/686?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #686      +/-   ##
==========================================
- Coverage   86.19%   86.19%   -0.01%     
==========================================
  Files          17       19       +2     
  Lines        2413     2441      +28     
  Branches      198      200       +2     
==========================================
+ Hits         2080     2104      +24     
- Misses        262      265       +3     
- Partials       71       72       +1     
```


| [Impacted Files](https://codecov.io/gh/slackapi/python-slackclient/pull/686?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [slack/web/base\_client.py](https://codecov.io/gh/slackapi/python-slackclient/pull/686/diff?src=pr&el=tree#diff-c2xhY2svd2ViL2Jhc2VfY2xpZW50LnB5) | `80.16% <50.00%> (-0.51%)` | :arrow_down: |
| [slack/signature/verifier.py](https://codecov.io/gh/slackapi/python-slackclient/pull/686/diff?src=pr&el=tree#diff-c2xhY2svc2lnbmF0dXJlL3ZlcmlmaWVyLnB5) | `88.00% <88.00%> (ø)` | |
| [slack/signature/\_\_init\_\_.py](https://codecov.io/gh/slackapi/python-slackclient/pull/686/diff?src=pr&el=tree#diff-c2xhY2svc2lnbmF0dXJlL19faW5pdF9fLnB5) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/slackapi/python-slackclient/pull/686?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/slackapi/python-slackclient/pull/686?src=pr&el=footer). Last update [af79b19...0143c25](https://codecov.io/gh/slackapi/python-slackclient/pull/686?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/slack/signature/__init__.py b/slack/signature/__init__.py
new file mode 100644
index 0000000..21b38c0
--- /dev/null
+++ b/slack/signature/__init__.py
@@ -0,0 +1,1 @@
+from .verifier import SignatureVerifier  # noqa
diff --git a/slack/signature/verifier.py b/slack/signature/verifier.py
new file mode 100644
index 0000000..6f8bcf6
--- /dev/null
+++ b/slack/signature/verifier.py
@@ -0,0 +1,60 @@
+import hashlib
+import hmac
+from time import time
+from typing import Dict, Optional
+
+
+class Clock:
+    def now(self) -> float:
+        return time()
+
+
+class SignatureVerifier:
+    def __init__(self, signing_secret: str, clock: Clock = Clock()):
+        """Slack request signature verifier
+
+        Slack signs its requests using a secret that's unique to your app.
+        With the help of signing secrets, your app can more confidently verify
+        whether requests from us are authentic.
+        https://api.slack.com/authentication/verifying-requests-from-slack
+        """
+        self.signing_secret = signing_secret
+        self.clock = clock
+
+    def is_valid_request(self, body: str, headers: Dict[str, str],) -> bool:
+        """Verifies if the given signature is valid"""
+        if headers is None:
+            return False
+        normalized_headers = {k.lower(): v for k, v in headers.items()}
+        return self.is_valid(
+            body=body,
+            timestamp=normalized_headers.get("x-slack-request-timestamp", None),
+            signature=normalized_headers.get("x-slack-signature", None),
+        )
+
+    def is_valid(self, body: str, timestamp: str, signature: str,) -> bool:
+        """Verifies if the given signature is valid"""
+        if timestamp is None or signature is None:
+            return False
+
+        if abs(self.clock.now() - int(timestamp)) > 60 * 5:
+            return False
+
+        if body is None:
+            body = ""
+
+        calculated_signature = self.generate_signature(timestamp=timestamp, body=body)
+        if calculated_signature is None:
+            return False
+        return hmac.compare_digest(calculated_signature, signature)
+
+    def generate_signature(self, *, timestamp: str, body: str) -> Optional[str]:
+        """Generates a signature"""
+        if timestamp is None:
+            return None
+
+        format_req = str.encode(f"v0:{timestamp}:{body}")
+        encoded_secret = str.encode(self.signing_secret)
+        request_hash = hmac.new(encoded_secret, format_req, hashlib.sha256).hexdigest()
+        calculated_signature = f"v0={request_hash}"
+        return calculated_signature
diff --git a/slack/web/base_client.py b/slack/web/base_client.py
index 2467c8a..f3991d8 100644
--- a/slack/web/base_client.py
+++ b/slack/web/base_client.py
@@ -6,7 +6,6 @@ import hashlib
 import hmac
 import io
 import json
-import json as json_module
 import logging
 import mimetypes
 import os
@@ -15,8 +14,8 @@ import sys
 import uuid
 import warnings
 from http.client import HTTPResponse
-from typing import BinaryIO, Dict, List, Union
-from typing import Optional
+from typing import BinaryIO, Dict, List
+from typing import Optional, Union
 from urllib.error import HTTPError
 from urllib.parse import urlencode
 from urllib.parse import urljoin
@@ -333,7 +332,7 @@ class BaseClient:
         return {
             "status_code": int(response["status"]),
             "headers": dict(response["headers"]),
-            "data": json_module.loads(response["body"]),
+            "data": json.loads(response["body"]),
         }
 
     def _urllib_api_call(
@@ -613,6 +612,11 @@ class BaseClient:
         Returns:
             True if signatures matches
         """
+        warnings.warn(
+            "As this method is deprecated since slackclient 2.6.0, "
+            "use `from slack.signature import SignatureVerifier` instead",
+            DeprecationWarning,
+        )
         format_req = str.encode(f"v0:{timestamp}:{data}")
         encoded_secret = str.encode(signing_secret)
         request_hash = hmac.new(encoded_secret, format_req, hashlib.sha256).hexdigest()


Test patch:
diff --git a/tests/signature/test_signature_verifier.py b/tests/signature/test_signature_verifier.py
new file mode 100644
index 0000000..8015733
--- /dev/null
+++ b/tests/signature/test_signature_verifier.py
@@ -0,0 +1,86 @@
+import unittest
+
+from slack.signature import SignatureVerifier
+
+
+class MockClock:
+    def now(self) -> float:
+        return 1531420618
+
+
+class TestSignatureVerifier(unittest.TestCase):
+    def setUp(self):
+        pass
+
+    def tearDown(self):
+        pass
+
+    # https://api.slack.com/authentication/verifying-requests-from-slack
+    signing_secret = "8f742231b10e8888abcd99yyyzzz85a5"
+
+    body = "token=xyzz0WbapA4vBCDEFasx0q6G&team_id=T1DC2JH3J&team_domain=testteamnow&channel_id=G8PSS9T3V&channel_name=foobar&user_id=U2CERLKJA&user_name=roadrunner&command=%2Fwebhook-collect&text=&response_url=https%3A%2F%2Fhooks.slack.com%2Fcommands%2FT1DC2JH3J%2F397700885554%2F96rGlfmibIGlgcZRskXaIFfN&trigger_id=398738663015.47445629121.803a0bc887a14d10d2c447fce8b6703c"
+
+    timestamp = "1531420618"
+    valid_signature = "v0=a2114d57b48eac39b9ad189dd8316235a7b4a8d21a10bd27519666489c69b503"
+
+    headers = {
+        "X-Slack-Request-Timestamp": timestamp,
+        "X-Slack-Signature": valid_signature,
+    }
+
+    def test_generate_signature(self):
+        verifier = SignatureVerifier("8f742231b10e8888abcd99yyyzzz85a5")
+        timestamp = "1531420618"
+        signature = verifier.generate_signature(timestamp=timestamp, body=self.body)
+        self.assertEqual(self.valid_signature, signature)
+
+    def test_is_valid_request(self):
+        verifier = SignatureVerifier(
+            signing_secret=self.signing_secret,
+            clock=MockClock()
+        )
+        self.assertTrue(verifier.is_valid_request(self.body, self.headers))
+
+    def test_is_valid_request_invalid_body(self):
+        verifier = SignatureVerifier(
+            signing_secret=self.signing_secret,
+            clock=MockClock(),
+        )
+        modified_body = self.body + "------"
+        self.assertFalse(verifier.is_valid_request(modified_body, self.headers))
+
+    def test_is_valid_request_expiration(self):
+        verifier = SignatureVerifier(
+            signing_secret=self.signing_secret,
+        )
+        self.assertFalse(verifier.is_valid_request(self.body, self.headers))
+
+    def test_is_valid_request_none(self):
+        verifier = SignatureVerifier(
+            signing_secret=self.signing_secret,
+            clock=MockClock(),
+        )
+        self.assertFalse(verifier.is_valid_request(None, self.headers))
+        self.assertFalse(verifier.is_valid_request(self.body, None))
+        self.assertFalse(verifier.is_valid_request(None, None))
+
+    def test_is_valid(self):
+        verifier = SignatureVerifier(
+            signing_secret=self.signing_secret,
+            clock=MockClock(),
+        )
+        self.assertTrue(verifier.is_valid(self.body, self.timestamp, self.valid_signature))
+        self.assertTrue(verifier.is_valid(self.body, 1531420618, self.valid_signature))
+
+    def test_is_valid_none(self):
+        verifier = SignatureVerifier(
+            signing_secret=self.signing_secret,
+            clock=MockClock(),
+        )
+        self.assertFalse(verifier.is_valid(None, self.timestamp, self.valid_signature))
+        self.assertFalse(verifier.is_valid(self.body, None, self.valid_signature))
+        self.assertFalse(verifier.is_valid(self.body, self.timestamp, None))
+        self.assertFalse(verifier.is_valid(None, None, self.valid_signature))
+        self.assertFalse(verifier.is_valid(None, self.timestamp, None))
+        self.assertFalse(verifier.is_valid(self.body, None, None))
+        self.assertFalse(verifier.is_valid(None, None, None))