From 146b105046f03c6a0d8dd5c119f991497bc2f5b9 Mon Sep 17 00:00:00 2001 From: Joe Fong <127404525+joefong-nhs@users.noreply.github.com> Date: Tue, 7 Nov 2023 11:18:31 +0000 Subject: [PATCH] [HOTFIX] [PRMDR-445] (#138) * [PRMDR-445] Fix incorrect logic in token expiry time comparison, add unit test to verify new logic * [PRMDR-445] add one unit test to verify logic --- lambdas/services/pds_api_service.py | 3 +- .../unit/services/test_pds_api_service.py | 179 +++++++++++++++++- 2 files changed, 177 insertions(+), 5 deletions(-) diff --git a/lambdas/services/pds_api_service.py b/lambdas/services/pds_api_service.py index 69bf48a67..9a95a0b73 100644 --- a/lambdas/services/pds_api_service.py +++ b/lambdas/services/pds_api_service.py @@ -28,7 +28,8 @@ def pds_request(self, nhs_number: str, retry_on_expired: bool): + int(access_token_response["issued_at"]) / 1000 ) time_safety_margin_seconds = 10 - if time.time() - access_token_expiration > time_safety_margin_seconds: + remaining_time_before_expiration = access_token_expiration - time.time() + if remaining_time_before_expiration < time_safety_margin_seconds: access_token = self.get_new_access_token() x_request_id = str(uuid.uuid4()) diff --git a/lambdas/tests/unit/services/test_pds_api_service.py b/lambdas/tests/unit/services/test_pds_api_service.py index 3b3337289..6f6c83983 100644 --- a/lambdas/tests/unit/services/test_pds_api_service.py +++ b/lambdas/tests/unit/services/test_pds_api_service.py @@ -169,15 +169,35 @@ def test_get_new_access_token_raise_PdsErrorException(mocker): mock_update_ssm.assert_not_called() +def mock_pds_token_response_issued_at(timestamp_in_sec: float) -> dict: + response_token = { + "access_token": "Sr5PGv19wTEHJdDr2wx2f7IGd0cw", + "expires_in": "599", + "token_type": "Bearer", + "issued_at": str(int(timestamp_in_sec * 1000)), + } + + return response_token + + def test_pds_request_valid_token(mocker): response = Response() response.status_code = 200 response._content = json.dumps(PDS_PATIENT).encode("utf-8") - mock_api_request_parameters = ("api.test/endpoint/", json.dumps(RESPONSE_TOKEN)) + mock_post = mocker.patch("requests.get", return_value=response) + + time_now = 1600000000 + mocker.patch("time.time", return_value=time_now) + mock_response_token = mock_pds_token_response_issued_at(time_now) + + mock_api_request_parameters = ( + "api.test/endpoint/", + json.dumps(mock_response_token), + ) nhs_number = "1111111111" mock_url_endpoint = "api.test/endpoint/Patient/" + nhs_number mock_authorization_header = { - "Authorization": f"Bearer {RESPONSE_TOKEN['access_token']}", + "Authorization": f"Bearer {mock_response_token['access_token']}", "X-Request-ID": "123412342", } @@ -185,12 +205,10 @@ def test_pds_request_valid_token(mocker): "services.pds_api_service.PdsApiService.get_parameters_for_pds_api_request", return_value=mock_api_request_parameters, ) - mocker.patch("time.time", return_value=1600000000.953031) mock_new_access_token = mocker.patch( "services.pds_api_service.PdsApiService.get_new_access_token" ) mocker.patch("uuid.uuid4", return_value="123412342") - mock_post = mocker.patch("requests.get", return_value=response) pds_service.pds_request(nhs_number="1111111111", retry_on_expired=True) @@ -201,6 +219,159 @@ def test_pds_request_valid_token(mocker): ) +def test_pds_request_not_refresh_token_if_more_than_10_seconds_before_expiry(mocker): + response = Response() + response.status_code = 200 + response._content = json.dumps(PDS_PATIENT).encode("utf-8") + mocker.patch("requests.get", return_value=response) + + time_now = 1600000000 + mocker.patch("time.time", return_value=time_now) + mock_response_token = mock_pds_token_response_issued_at(time_now - 599 + 11) + + mock_api_request_parameters = ( + "api.test/endpoint/", + json.dumps(mock_response_token), + ) + + mock_get_parameters = mocker.patch( + "services.pds_api_service.PdsApiService.get_parameters_for_pds_api_request", + return_value=mock_api_request_parameters, + ) + mock_new_access_token = mocker.patch( + "services.pds_api_service.PdsApiService.get_new_access_token" + ) + mocker.patch("uuid.uuid4", return_value="123412342") + + pds_service.pds_request(nhs_number="1111111111", retry_on_expired=True) + + mock_get_parameters.assert_called_once() + mock_new_access_token.assert_not_called() + + +def test_pds_request_refresh_token_9_seconds_before_expiration(mocker): + response = Response() + response.status_code = 200 + response._content = json.dumps(PDS_PATIENT).encode("utf-8") + + time_now = 1600000000 + mocker.patch("time.time", return_value=time_now) + mock_response_token = mock_pds_token_response_issued_at(time_now - 599 + 9) + new_mock_access_token = "mock_access_token" + + mock_api_request_parameters = ( + "api.test/endpoint/", + json.dumps(mock_response_token), + ) + nhs_number = "1111111111" + mock_url_endpoint = "api.test/endpoint/Patient/" + nhs_number + mock_authorization_header = { + "Authorization": f"Bearer {new_mock_access_token}", + "X-Request-ID": "123412342", + } + + mock_get_parameters = mocker.patch( + "services.pds_api_service.PdsApiService.get_parameters_for_pds_api_request", + return_value=mock_api_request_parameters, + ) + mock_new_access_token = mocker.patch( + "services.pds_api_service.PdsApiService.get_new_access_token", + return_value=new_mock_access_token, + ) + mocker.patch("uuid.uuid4", return_value="123412342") + mock_post = mocker.patch("requests.get", return_value=response) + + pds_service.pds_request(nhs_number="1111111111", retry_on_expired=True) + + mock_get_parameters.assert_called_once() + mock_new_access_token.assert_called_once() + mock_post.assert_called_with( + url=mock_url_endpoint, headers=mock_authorization_header + ) + + +def test_pds_request_refresh_token_if_already_expired(mocker): + response = Response() + response.status_code = 200 + response._content = json.dumps(PDS_PATIENT).encode("utf-8") + + time_now = 1600000000 + mocker.patch("time.time", return_value=time_now) + mock_response_token = mock_pds_token_response_issued_at(time_now - 599) + new_mock_access_token = "mock_access_token" + + mock_api_request_parameters = ( + "api.test/endpoint/", + json.dumps(mock_response_token), + ) + nhs_number = "1111111111" + mock_url_endpoint = "api.test/endpoint/Patient/" + nhs_number + mock_authorization_header = { + "Authorization": f"Bearer {new_mock_access_token}", + "X-Request-ID": "123412342", + } + + mock_get_parameters = mocker.patch( + "services.pds_api_service.PdsApiService.get_parameters_for_pds_api_request", + return_value=mock_api_request_parameters, + ) + mock_new_access_token = mocker.patch( + "services.pds_api_service.PdsApiService.get_new_access_token", + return_value=new_mock_access_token, + ) + mocker.patch("uuid.uuid4", return_value="123412342") + mock_post = mocker.patch("requests.get", return_value=response) + + pds_service.pds_request(nhs_number="1111111111", retry_on_expired=True) + + mock_get_parameters.assert_called_once() + mock_new_access_token.assert_called_once() + mock_post.assert_called_with( + url=mock_url_endpoint, headers=mock_authorization_header + ) + + +def test_pds_request_refresh_token_if_already_expired_11_seconds_ago(mocker): + response = Response() + response.status_code = 200 + response._content = json.dumps(PDS_PATIENT).encode("utf-8") + + time_now = 1600000000 + mocker.patch("time.time", return_value=time_now) + mock_response_token = mock_pds_token_response_issued_at(time_now - 599 - 11) + new_mock_access_token = "mock_access_token" + + mock_api_request_parameters = ( + "api.test/endpoint/", + json.dumps(mock_response_token), + ) + nhs_number = "1111111111" + mock_url_endpoint = "api.test/endpoint/Patient/" + nhs_number + mock_authorization_header = { + "Authorization": f"Bearer {new_mock_access_token}", + "X-Request-ID": "123412342", + } + + mock_get_parameters = mocker.patch( + "services.pds_api_service.PdsApiService.get_parameters_for_pds_api_request", + return_value=mock_api_request_parameters, + ) + mock_new_access_token = mocker.patch( + "services.pds_api_service.PdsApiService.get_new_access_token", + return_value=new_mock_access_token, + ) + mocker.patch("uuid.uuid4", return_value="123412342") + mock_post = mocker.patch("requests.get", return_value=response) + + pds_service.pds_request(nhs_number="1111111111", retry_on_expired=True) + + mock_get_parameters.assert_called_once() + mock_new_access_token.assert_called_once() + mock_post.assert_called_with( + url=mock_url_endpoint, headers=mock_authorization_header + ) + + def test_pds_request_expired_token(mocker): response = Response() response.status_code = 200