Repository: globus/globus-sdk-python
Pull request #305 (2018-08-14 19:30:56)

Replace token suffixes in debug logging with SHA-256 hash value
This is my first time working with this repository and submitting a pull request, so feel free to tear things apart. This is addressing #299 

I added `access_token_hash` data member to the `AccessTokenAuthorizer` and `RenewingAuthorizer` classes to hold a SHA-256 hash value. Additionally, I added a utility function for calculating the hash value if that functionality is needed elsewhere.

Again, feel free to suggest things to tweak and move around, tests to add, etc. 


Patch:
diff --git a/globus_sdk/authorizers/access_token.py b/globus_sdk/authorizers/access_token.py
index b129813a..d9c6d4d2 100644
--- a/globus_sdk/authorizers/access_token.py
+++ b/globus_sdk/authorizers/access_token.py
@@ -1,6 +1,7 @@
 import logging
 
 from globus_sdk.authorizers.base import GlobusAuthorizer
+from globus_sdk.utils.string_hashing import sha256_string
 
 logger = logging.getLogger(__name__)
 
@@ -19,17 +20,19 @@ class AccessTokenAuthorizer(GlobusAuthorizer):
     def __init__(self, access_token):
         logger.info(("Setting up an AccessTokenAuthorizer. It will use an "
                      "auth type of Bearer and cannot handle 401s."))
-        logger.debug('Bearer token ends in "...{}" (last 5 chars)'
-                     .format(access_token[-5:]))
         self.access_token = access_token
         self.header_val = "Bearer %s" % access_token
 
+        self.access_token_hash = sha256_string(self.access_token)
+        logger.debug('Bearer token has hash "{}"'
+                     .format(self.access_token_hash))
+
     def set_authorization_header(self, header_dict):
         """
         Sets the ``Authorization`` header to
         "Bearer <access_token>"
         """
         logger.debug(("Setting AccessToken Authorization Header: "
-                      '"Bearer ...{}" (last 5 chars)')
-                     .format(self.header_val[-5:]))
+                      '"Bearer token has hash "{}"')
+                     .format(self.access_token_hash))
         header_dict['Authorization'] = self.header_val
diff --git a/globus_sdk/authorizers/renewing.py b/globus_sdk/authorizers/renewing.py
index 8a47f22e..e8a7f968 100644
--- a/globus_sdk/authorizers/renewing.py
+++ b/globus_sdk/authorizers/renewing.py
@@ -4,6 +4,7 @@ import logging
 import time
 
 from globus_sdk.authorizers.base import GlobusAuthorizer
+from globus_sdk.utils.string_hashing import sha256_string
 
 
 logger = logging.getLogger(__name__)
@@ -48,11 +49,11 @@ class RenewingAuthorizer(GlobusAuthorizer):
         # check access_token too -- it's not clear what it would mean to set
         # expiration without an access token
         if expires_at is not None and self.access_token is not None:
+            self.access_token_hash = sha256_string(self.access_token)
             logger.info(("Got both expires_at and access_token. "
                          "Will start by using "
-                         "RenewingAuthorizer.access_token = ...{} "
-                         "(last 5 chars)")
-                        .format(self.access_token[-5:]))
+                         'RenewingAuthorizer.access_token with hash "{}"')
+                        .format(self.access_token_hash))
             self._set_expiration_time(expires_at)
 
         # if these were unspecified, fetch a new access token
@@ -89,7 +90,8 @@ class RenewingAuthorizer(GlobusAuthorizer):
     def _get_new_access_token(self):
         """
         Given token data from _get_token_response and _extract_token_data,
-        set the access token and expiration time, and call on_refresh
+        set the access token and expiration time, calculate the new token
+        hash, and call on_refresh
         """
         # get the first (and only) token
         res = self._get_token_response()
@@ -97,10 +99,11 @@ class RenewingAuthorizer(GlobusAuthorizer):
 
         self._set_expiration_time(token_data['expires_at_seconds'])
         self.access_token = token_data['access_token']
+        self.access_token_hash = sha256_string(self.access_token)
 
-        logger.info(("RenewingAuthorizer.access_token updated to "
-                     '"...{}" (last 5 chars)')
-                    .format(self.access_token[-5:]))
+        logger.info(("RenewingAuthorizer.access_token updated to token"
+                     "with hash" '"{}"')
+                    .format(self.access_token_hash))
 
         if callable(self.on_refresh):
             self.on_refresh(res)
@@ -128,8 +131,8 @@ class RenewingAuthorizer(GlobusAuthorizer):
         """
         self._check_expiration_time()
         logger.debug(("Setting RefreshToken Authorization Header:"
-                      '"Bearer ...{}" (last 5 chars)')
-                     .format(self.access_token[-5:]))
+                      'Bearer token has hash "{}"')
+                     .format(self.access_token_hash))
         header_dict['Authorization'] = "Bearer %s" % self.access_token
 
     def handle_missing_authorization(self, *args, **kwargs):
diff --git a/globus_sdk/utils/string_hashing.py b/globus_sdk/utils/string_hashing.py
new file mode 100644
index 00000000..867f4f73
--- /dev/null
+++ b/globus_sdk/utils/string_hashing.py
@@ -0,0 +1,5 @@
+import hashlib
+
+
+def sha256_string(s):
+    return hashlib.sha256(s.encode('utf-8')).hexdigest()


Test patch:
diff --git a/tests/unit/test_renewing_authorizer.py b/tests/unit/test_renewing_authorizer.py
index c3146b93..ed227b92 100644
--- a/tests/unit/test_renewing_authorizer.py
+++ b/tests/unit/test_renewing_authorizer.py
@@ -86,6 +86,9 @@ class RenewingAuthorizerTests(CapturedIOTestCase):
         # confirm starting with original access_token
         self.assertEqual(self.authorizer.access_token, self.access_token)
 
+        # take note of original access_token_hash
+        original_hash = self.authorizer.access_token_hash
+
         # get new_access_token
         self.authorizer._get_new_access_token()
         # confirm side effects
@@ -94,6 +97,8 @@ class RenewingAuthorizerTests(CapturedIOTestCase):
         self.assertEqual(self.authorizer.expires_at,
                          self.token_data["expires_at_seconds"] -
                          EXPIRES_ADJUST_SECONDS)
+        self.assertNotEqual(self.authorizer.access_token_hash,
+                            original_hash)
         self.on_refresh.assert_called_once()
 
     def test_check_expiration_time_valid(self):