From ccf74a4c0be582b3e6bb9b0532aa370cc62e45d5 Mon Sep 17 00:00:00 2001 From: xiafu Date: Thu, 30 Apr 2020 20:52:27 -0700 Subject: [PATCH 1/3] [Azure-Core]Auth Header missing when token credential is not expired --- .../azure-core/azure/core/pipeline/policies/_authentication.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/core/azure-core/azure/core/pipeline/policies/_authentication.py b/sdk/core/azure-core/azure/core/pipeline/policies/_authentication.py index 4af2d9312327..0492fabc47a6 100644 --- a/sdk/core/azure-core/azure/core/pipeline/policies/_authentication.py +++ b/sdk/core/azure-core/azure/core/pipeline/policies/_authentication.py @@ -89,9 +89,10 @@ def on_request(self, request): """ self._enforce_https(request) + # new token is needed if token is None or token is expired if self._need_new_token: self._token = self._credential.get_token(*self._scopes) - self._update_headers(request.http_request.headers, self._token.token) + self._update_headers(request.http_request.headers, self._token.token) # ignore class AzureKeyCredentialPolicy(SansIOHTTPPolicy): From 432624734973364993fdb61747d8ff91d6896d11 Mon Sep 17 00:00:00 2001 From: Laurent Mazuel Date: Fri, 1 May 2020 09:22:23 -0700 Subject: [PATCH 2/3] Fix mypy --- .../azure/core/pipeline/policies/_authentication.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sdk/core/azure-core/azure/core/pipeline/policies/_authentication.py b/sdk/core/azure-core/azure/core/pipeline/policies/_authentication.py index 0492fabc47a6..36edd3cbc020 100644 --- a/sdk/core/azure-core/azure/core/pipeline/policies/_authentication.py +++ b/sdk/core/azure-core/azure/core/pipeline/policies/_authentication.py @@ -89,10 +89,9 @@ def on_request(self, request): """ self._enforce_https(request) - # new token is needed if token is None or token is expired - if self._need_new_token: + if self._token is None or self._need_new_token: self._token = self._credential.get_token(*self._scopes) - self._update_headers(request.http_request.headers, self._token.token) # ignore + self._update_headers(request.http_request.headers, self._token.token) class AzureKeyCredentialPolicy(SansIOHTTPPolicy): From 1354b2f965ad8ae317f17df2bce98cb937a55136 Mon Sep 17 00:00:00 2001 From: Laurent Mazuel Date: Fri, 1 May 2020 09:43:03 -0700 Subject: [PATCH 3/3] Test we put the header even if we didn't tech a new token --- .../tests/azure_core_asynctests/test_authentication.py | 7 ++++++- sdk/core/azure-core/tests/test_authentication.py | 10 ++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/sdk/core/azure-core/tests/azure_core_asynctests/test_authentication.py b/sdk/core/azure-core/tests/azure_core_asynctests/test_authentication.py index 80a5b1333c98..456001b59da5 100644 --- a/sdk/core/azure-core/tests/azure_core_asynctests/test_authentication.py +++ b/sdk/core/azure-core/tests/azure_core_asynctests/test_authentication.py @@ -18,7 +18,8 @@ @pytest.mark.asyncio async def test_bearer_policy_adds_header(): """The bearer token policy should add a header containing a token from its credential""" - expected_token = AccessToken("expected_token", 0) + # 2524608000 == 01/01/2050 @ 12:00am (UTC) + expected_token = AccessToken("expected_token", 2524608000) async def verify_authorization_header(request): assert request.http_request.headers["Authorization"] == "Bearer {}".format(expected_token.token) @@ -37,6 +38,10 @@ async def get_token(_): await pipeline.run(HttpRequest("GET", "https://spam.eggs"), context=None) assert get_token_calls == 1 + await pipeline.run(HttpRequest("GET", "https://spam.eggs"), context=None) + # Didn't need a new token + assert get_token_calls == 1 + @pytest.mark.asyncio async def test_bearer_policy_send(): diff --git a/sdk/core/azure-core/tests/test_authentication.py b/sdk/core/azure-core/tests/test_authentication.py index 0bfeb55a77c8..fb56819b6451 100644 --- a/sdk/core/azure-core/tests/test_authentication.py +++ b/sdk/core/azure-core/tests/test_authentication.py @@ -23,7 +23,8 @@ def test_bearer_policy_adds_header(): """The bearer token policy should add a header containing a token from its credential""" - expected_token = AccessToken("expected_token", 0) + # 2524608000 == 01/01/2050 @ 12:00am (UTC) + expected_token = AccessToken("expected_token", 2524608000) def verify_authorization_header(request): assert request.http_request.headers["Authorization"] == "Bearer {}".format(expected_token.token) @@ -31,10 +32,15 @@ def verify_authorization_header(request): fake_credential = Mock(get_token=Mock(return_value=expected_token)) policies = [BearerTokenCredentialPolicy(fake_credential, "scope"), Mock(send=verify_authorization_header)] - Pipeline(transport=Mock(), policies=policies).run(HttpRequest("GET", "https://spam.eggs")) + pipeline = Pipeline(transport=Mock(), policies=policies) + pipeline.run(HttpRequest("GET", "https://spam.eggs")) assert fake_credential.get_token.call_count == 1 + pipeline.run(HttpRequest("GET", "https://spam.eggs")) + + # Didn't need a new token + assert fake_credential.get_token.call_count == 1 def test_bearer_policy_send(): """The bearer token policy should invoke the next policy's send method and return the result"""