Repository: Shopify/shopify_python_api

Issue:
Update MD5 Signature validation to HMAC
Docs say that MD5 signature validation 'To be removed after June 1st, 2015'

It appears ```Session``` is still doing it this way vs HMAC


Discussion:
gavinballard: The build failures here are the same as those mentioned in PR #86 (issue with `pyactiveresource` and Python v2.7.9).

Patch:
diff --git a/CHANGELOG b/CHANGELOG
index 4301098..00fe323 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,6 +2,7 @@
 
 * Added Checkout resource
 * Updated to pyactiveresource v2.1.1 which includes a test-related bugfix
+* Changed OAuth validation from MD5 to HMAC-SHA256
 
 == Version 2.1.0
 
diff --git a/shopify/session.py b/shopify/session.py
index 9058d55..8741ec4 100644
--- a/shopify/session.py
+++ b/shopify/session.py
@@ -1,8 +1,6 @@
 import time
-try:
-    from hashlib import md5
-except ImportError:
-    from md5 import md5
+import hmac
+from hashlib import sha256
 try:
     import simplejson as json
 except ImportError:
@@ -53,7 +51,7 @@ class Session(object):
             return self.token
 
         if not self.validate_params(params):
-            raise ValidationException('Invalid Signature: Possibly malicious login')
+            raise ValidationException('Invalid HMAC: Possibly malicious login')
 
         code = params['code']
 
@@ -94,18 +92,30 @@ class Session(object):
         if int(params['timestamp']) < time.time() - one_day:
             return False
 
-        return cls.validate_signature(params)
+        return cls.validate_hmac(params)
 
     @classmethod
-    def validate_signature(cls, params):
-        if "signature" not in params:
+    def validate_hmac(cls, params):
+        if 'hmac' not in params:
             return False
 
-        sorted_params = ""
-        signature = params['signature']
+        hmac_calculated = cls.calculate_hmac(params)
+        hmac_to_verify = params['hmac']
 
-        for k in sorted(params.keys()):
-            if k != "signature":
-                sorted_params += k + "=" + str(params[k])
+        # Try to use compare_digest() to reduce vulnerability to timing attacks.
+        # If it's not available, just fall back to regular string comparison.
+        try:
+            return hmac.compare_digest(hmac_calculated, hmac_to_verify)
+        except AttributeError:
+            return hmac_calculated == hmac_to_verify
 
-        return md5((cls.secret + sorted_params).encode('utf-8')).hexdigest() == signature
+    @classmethod
+    def calculate_hmac(cls, params):
+        """
+        Calculate the HMAC of the given parameters in line with Shopify's rules for OAuth authentication.
+        See http://docs.shopify.com/api/authentication/oauth#verification.
+        """
+        # Sort and combine query parameters into a single string, excluding those that should be removed and joining with '&'.
+        sorted_params = '&'.join(['{0}={1}'.format(k, params[k]) for k in sorted(params.keys()) if k not in ['signature', 'hmac']])
+        # Generate the hex digest for the sorted parameters using the secret.
+        return hmac.new(cls.secret.encode(), sorted_params.encode(), sha256).hexdigest()


Test patch:
diff --git a/test/session_test.py b/test/session_test.py
index c8c1643..c85eb4b 100644
--- a/test/session_test.py
+++ b/test/session_test.py
@@ -113,24 +113,54 @@ class SessionTest(TestCase):
         session = shopify.Session("testshop.myshopify.com", "any-token")
         self.assertEqual("https://testshop.myshopify.com/admin", session.site)
 
-    def test_return_token_if_signature_is_valid(self):
+    def test_hmac_calculation(self):
+        # Test using the secret and parameter examples given in the Shopify API documentation.
+        shopify.Session.secret='hush'
+        params = {
+          'shop': 'some-shop.myshopify.com',
+          'code': 'a94a110d86d2452eb3e2af4cfb8a3828',
+          'timestamp': '1337178173',
+          'signature': '6e39a2ea9e497af6cb806720da1f1bf3',
+          'hmac': '2cb1a277650a659f1b11e92a4a64275b128e037f2c3390e3c8fd2d8721dac9e2',
+        }
+        self.assertEqual(shopify.Session.calculate_hmac(params), params['hmac'])
+
+    def test_return_token_if_hmac_is_valid(self):
         shopify.Session.secret='secret'
         params = {'code': 'any-code', 'timestamp': time.time()}
-        sorted_params = self.make_sorted_params(params)
-        signature = md5((shopify.Session.secret + sorted_params).encode('utf-8')).hexdigest()
-        params['signature'] = signature
+        hmac = shopify.Session.calculate_hmac(params)
+        params['hmac'] = hmac
 
         self.fake(None, url='https://localhost.myshopify.com/admin/oauth/access_token', method='POST', body='{"access_token" : "token"}', has_user_agent=False)
         session = shopify.Session('http://localhost.myshopify.com')
         token = session.request_token(params)
         self.assertEqual("token", token)
 
-    def test_raise_error_if_signature_does_not_match_expected(self):
+    def test_return_token_if_hmac_is_valid_but_signature_also_provided(self):
+        shopify.Session.secret='secret'
+        params = {'code': 'any-code', 'timestamp': time.time(), 'signature': '6e39a2'}
+        hmac = shopify.Session.calculate_hmac(params)
+        params['hmac'] = hmac
+
+        self.fake(None, url='https://localhost.myshopify.com/admin/oauth/access_token', method='POST', body='{"access_token" : "token"}', has_user_agent=False)
+        session = shopify.Session('http://localhost.myshopify.com')
+        token = session.request_token(params)
+        self.assertEqual("token", token)
+
+    def test_raise_error_if_hmac_is_invalid(self):
+        shopify.Session.secret='secret'
+        params = {'code': 'any-code', 'timestamp': time.time()}
+        params['hmac'] = 'a94a110d86d2452e92a4a64275b128e9273be3037f2c339eb3e2af4cfb8a3828'
+
+        with self.assertRaises(shopify.ValidationException):
+            session = shopify.Session('http://localhost.myshopify.com')
+            session = session.request_token(params)
+
+    def test_raise_error_if_hmac_does_not_match_expected(self):
         shopify.Session.secret='secret'
         params = {'foo': 'hello', 'timestamp': time.time()}
-        sorted_params = self.make_sorted_params(params)
-        signature = md5((shopify.Session.secret + sorted_params).encode('utf-8')).hexdigest()
-        params['signature'] = signature
+        hmac = shopify.Session.calculate_hmac(params)
+        params['hmac'] = hmac
         params['bar'] = 'world'
         params['code'] = 'code'
 
@@ -142,22 +172,13 @@ class SessionTest(TestCase):
         shopify.Session.secret='secret'
         one_day = 24 * 60 * 60
         params = {'code': 'any-code', 'timestamp': time.time()-(2*one_day)}
-        sorted_params = self.make_sorted_params(params)
-        signature = md5((shopify.Session.secret + sorted_params).encode('utf-8')).hexdigest()
-        params['signature'] = signature
+        hmac = shopify.Session.calculate_hmac(params)
+        params['hmac'] = hmac
 
         with self.assertRaises(shopify.ValidationException):
             session = shopify.Session('http://localhost.myshopify.com')
             session = session.request_token(params)
 
-
-    def make_sorted_params(self, params):
-        sorted_params = ""
-        for k in sorted(params.keys()):
-            if k != "signature":
-                sorted_params += k + "=" + str(params[k])
-        return sorted_params
-
     def normalize_url(self, url):
         scheme, netloc, path, query, fragment = urllib.parse.urlsplit(url)
         query = "&".join(sorted(query.split("&")))