Fixed the refresh token flow

Author: tamirse

CHANGELOG.rst

@@ -1,6 +1,8 @@
 Changelog
 =========
 
+* Fixed a bug where the authentication refresh token flow did not update the token values
+
 v1.1.0 (2023-01-20)
 -------------------
 

datacrunch/authentication/authentication.py

@@ -40,7 +40,7 @@ def authenticate(self) -> dict:
         }
 
         response = requests.post(
-            url, data=payload, headers=self._generate_headers())
+            url, json=payload, headers=self._generate_headers())
         handle_error(response)
 
         auth_data = response.json()
@@ -56,6 +56,7 @@ def authenticate(self) -> dict:
     def refresh(self) -> dict:
         """Authenticate the client using the refresh token - refresh the access token.
 
+        updates the object's tokens, and:
         returns an authentication data dictionary with the following schema:
         {
             "access_token": token str,
@@ -76,10 +77,18 @@ def refresh(self) -> dict:
         }
 
         response = requests.post(
-            url, data=payload, headers=self._generate_headers())
+            url, json=payload, headers=self._generate_headers())
         handle_error(response)
 
-        return response.json()
+        auth_data = response.json()
+
+        self._access_token = auth_data['access_token']
+        self._refresh_token = auth_data['refresh_token']
+        self._scope = auth_data['scope']
+        self._token_type = auth_data['token_type']
+        self._expires_at = time.time() + auth_data['expires_in']
+
+        return auth_data
 
     def _generate_headers(self):
         # get the first 10 chars of the client id

datacrunch/http_client/http_client.py

@@ -148,7 +148,7 @@ def _refresh_token_if_expired(self) -> None:
         :raises APIException: an api exception with message and error type code
         """
         if(self._auth_service.is_expired()):
-            # to to refresh. if refresh token has expired, reauthenticate
+            # try to refresh. if refresh token has expired, reauthenticate
             try:
                 self._auth_service.refresh()
             except Exception:
@@ -181,8 +181,8 @@ def _generate_user_agent(self) -> str:
         :return: user agent string
         :rtype: str
         """
-        client_id_truncated = self._auth_service._client_id[:
-                                                            10]  # get the first 10 chars of the client id
+        # get the first 10 chars of the client id
+        client_id_truncated = self._auth_service._client_id[:10]  
 
         return f'datacrunch-python-v{self._version}-{client_id_truncated}'
 

tests/unit_tests/authentication/test_authentication.py

@@ -1,5 +1,6 @@
 import pytest
 import responses # https://github.com/getsentry/responses
+from responses import matchers
 import time
 
 from datacrunch.exceptions import APIException
@@ -18,6 +19,9 @@
 TOKEN_TYPE = 'Bearer'
 EXPIRES_IN = 3600
 
+ACCESS_TOKEN2 = 'access2'
+REFRESH_TOKEN2 = 'refresh2'
+
 class TestAuthenticationService:
 
     @pytest.fixture
@@ -74,9 +78,10 @@ def test_authenticate_failed(self, authentication_service, endpoint):
         assert excinfo.value.code == INVALID_REQUEST
         assert excinfo.value.message == INVALID_REQUEST_MESSAGE
         assert responses.assert_call_count(endpoint, 1) is True
-        assert responses.calls[0].request.body == f'grant_type=client_credentials&client_id={CLIENT_ID}&client_secret={CLIENT_SECRET}'
+        assert responses.calls[0].request.body == f'{{"grant_type": "client_credentials", "client_id": "{CLIENT_ID}", "client_secret": "{CLIENT_SECRET}"}}'.encode()
 
     def test_refresh_successful(self, authentication_service, endpoint):
+        # add a response for the client credentials grant
         responses.add(
             responses.POST,
             endpoint,
@@ -87,12 +92,27 @@ def test_refresh_successful(self, authentication_service, endpoint):
                 'token_type': TOKEN_TYPE,
                 'expires_in': EXPIRES_IN
             },
+            match=[matchers.json_params_matcher({"grant_type":"client_credentials", "client_id": CLIENT_ID, "client_secret": CLIENT_SECRET})],
+            status=200
+        )
+
+        # add another response for the refresh token grant
+        responses.add(
+            responses.POST,
+            endpoint,
+            json={
+                'access_token': ACCESS_TOKEN2,
+                'refresh_token': REFRESH_TOKEN2,
+                'scope': SCOPE,
+                'token_type': TOKEN_TYPE,
+                'expires_in': EXPIRES_IN
+            },
+            match=[matchers.json_params_matcher({"grant_type":"refresh_token", "refresh_token": REFRESH_TOKEN})],
             status=200
         )
 
         # act
-        authentication_service.authenticate() # authenticate first
-        auth_data = authentication_service.refresh() # refresh
+        auth_data = authentication_service.authenticate() # authenticate first
 
         # assert
         assert type(auth_data) == dict
@@ -101,8 +121,17 @@ def test_refresh_successful(self, authentication_service, endpoint):
         assert authentication_service._scope == SCOPE
         assert authentication_service._token_type == TOKEN_TYPE
         assert authentication_service._expires_at != None
-        assert responses.calls[0].request.body == f'grant_type=client_credentials&client_id={CLIENT_ID}&client_secret={CLIENT_SECRET}'
-        assert responses.calls[1].request.body == f'grant_type=refresh_token&refresh_token={REFRESH_TOKEN}'
+        assert responses.calls[0].request.body == f'{{"grant_type": "client_credentials", "client_id": "{CLIENT_ID}", "client_secret": "{CLIENT_SECRET}"}}'.encode()
+
+        auth_data2 = authentication_service.refresh() # refresh
+
+        assert type(auth_data2) == dict
+        assert authentication_service._access_token == ACCESS_TOKEN2
+        assert authentication_service._refresh_token == REFRESH_TOKEN2
+        assert authentication_service._scope == SCOPE
+        assert authentication_service._token_type == TOKEN_TYPE
+        assert authentication_service._expires_at != None
+        assert responses.calls[1].request.body == f'{{"grant_type": "refresh_token", "refresh_token": "{REFRESH_TOKEN}"}}'.encode()
         assert responses.assert_call_count(endpoint, 2) is True
 
     def test_refresh_failed(self, authentication_service, endpoint):
@@ -118,6 +147,7 @@ def test_refresh_failed(self, authentication_service, endpoint):
                 'token_type': TOKEN_TYPE,
                 'expires_in': EXPIRES_IN
             },
+            match=[matchers.json_params_matcher({"grant_type":"client_credentials", "client_id": CLIENT_ID, "client_secret": CLIENT_SECRET})],
             status=200
         )
 
@@ -126,6 +156,7 @@ def test_refresh_failed(self, authentication_service, endpoint):
             responses.POST,
             endpoint,
             json={"code": INVALID_REQUEST, "message": INVALID_REQUEST_MESSAGE},
+            match=[matchers.json_params_matcher({"grant_type":"refresh_token", "refresh_token": REFRESH_TOKEN})],
             status=400
         )
 
@@ -139,8 +170,8 @@ def test_refresh_failed(self, authentication_service, endpoint):
         assert excinfo.value.code == INVALID_REQUEST
         assert excinfo.value.message == INVALID_REQUEST_MESSAGE
         assert responses.assert_call_count(endpoint, 2) is True
-        assert responses.calls[0].request.body == f'grant_type=client_credentials&client_id={CLIENT_ID}&client_secret={CLIENT_SECRET}'
-        assert responses.calls[1].request.body == f'grant_type=refresh_token&refresh_token={REFRESH_TOKEN}'
+        assert responses.calls[0].request.body == f'{{"grant_type": "client_credentials", "client_id": "{CLIENT_ID}", "client_secret": "{CLIENT_SECRET}"}}'.encode()
+        assert responses.calls[1].request.body == f'{{"grant_type": "refresh_token", "refresh_token": "{REFRESH_TOKEN}"}}'.encode()
 
     def test_is_expired(self, authentication_service, endpoint):
         # arrange