From ba4d43bbbac9128f386b57e4745b29b2be76c858 Mon Sep 17 00:00:00 2001 From: msyyc <70930885+msyyc@users.noreply.github.com> Date: Thu, 14 Nov 2024 12:35:58 +0800 Subject: [PATCH 1/7] update cae policy --- .../mgmt/core/policies/_authentication.py | 59 +------------------ .../core/policies/_authentication_async.py | 27 +-------- sdk/core/azure-mgmt-core/setup.py | 2 +- .../tests/test_authentication.py | 32 ---------- 4 files changed, 3 insertions(+), 117 deletions(-) diff --git a/sdk/core/azure-mgmt-core/azure/mgmt/core/policies/_authentication.py b/sdk/core/azure-mgmt-core/azure/mgmt/core/policies/_authentication.py index c7015b819c70..d2b3316dec5d 100644 --- a/sdk/core/azure-mgmt-core/azure/mgmt/core/policies/_authentication.py +++ b/sdk/core/azure-mgmt-core/azure/mgmt/core/policies/_authentication.py @@ -23,14 +23,13 @@ # IN THE SOFTWARE. # # -------------------------------------------------------------------------- -import base64 import time from typing import Optional, Union, MutableMapping, List, Any, Sequence, TypeVar, Generic from azure.core.credentials import AccessToken, TokenCredential from azure.core.credentials_async import AsyncTokenCredential from azure.core.pipeline.policies import BearerTokenCredentialPolicy, SansIOHTTPPolicy -from azure.core.pipeline import PipelineRequest, PipelineResponse +from azure.core.pipeline import PipelineRequest from azure.core.exceptions import ServiceRequestError from azure.core.pipeline.transport import ( HttpRequest as LegacyHttpRequest, @@ -49,34 +48,8 @@ class ARMChallengeAuthenticationPolicy(BearerTokenCredentialPolicy): This policy internally handles Continuous Access Evaluation (CAE) challenges. When it can't complete a challenge, it will return the 401 (unauthorized) response from ARM. - - :param ~azure.core.credentials.TokenCredential credential: credential for authorizing requests - :param str scopes: required authentication scopes """ - def on_challenge( - self, - request: PipelineRequest[HTTPRequestType], - response: PipelineResponse[HTTPRequestType, HTTPResponseType], - ) -> bool: - """Authorize request according to an ARM authentication challenge - - :param ~azure.core.pipeline.PipelineRequest request: the request which elicited an authentication challenge - :param ~azure.core.pipeline.PipelineResponse response: ARM's response - :returns: a bool indicating whether the policy should send the request - :rtype: bool - """ - - challenge = response.http_response.headers.get("WWW-Authenticate") - if challenge: - claims = _parse_claims_challenge(challenge) - if claims: - self.authorize_request(request, *self._scopes, claims=claims) - return True - - return False - - # pylint:disable=too-few-public-methods class _AuxiliaryAuthenticationPolicyBase(Generic[TokenCredentialType]): """Adds auxiliary authorization token header to requests. @@ -150,33 +123,3 @@ def on_request(self, request: PipelineRequest[HTTPRequestType]) -> None: self._aux_tokens = self._get_auxiliary_tokens(*self._scopes) self._update_headers(request.http_request.headers) - - -def _parse_claims_challenge(challenge: str) -> Optional[str]: - """Parse the "claims" parameter from an authentication challenge - - Example challenge with claims: - Bearer authorization_uri="https://login.windows-ppe.net/", error="invalid_token", - error_description="User session has been revoked", - claims="eyJhY2Nlc3NfdG9rZW4iOnsibmJmIjp7ImVzc2VudGlhbCI6dHJ1ZSwgInZhbHVlIjoiMTYwMzc0MjgwMCJ9fX0=" - - :param str challenge: The authentication challenge - :return: the challenge's "claims" parameter or None, if it doesn't contain that parameter - """ - encoded_claims = None - for parameter in challenge.split(","): - if "claims=" in parameter: - if encoded_claims: - # multiple claims challenges, e.g. for cross-tenant auth, would require special handling - return None - encoded_claims = parameter[parameter.index("=") + 1 :].strip(" \"'") - - if not encoded_claims: - return None - - padding_needed = -len(encoded_claims) % 4 - try: - decoded_claims = base64.urlsafe_b64decode(encoded_claims + "=" * padding_needed).decode() - return decoded_claims - except Exception: # pylint:disable=broad-except - return None diff --git a/sdk/core/azure-mgmt-core/azure/mgmt/core/policies/_authentication_async.py b/sdk/core/azure-mgmt-core/azure/mgmt/core/policies/_authentication_async.py index bd93b112a902..936760af860e 100644 --- a/sdk/core/azure-mgmt-core/azure/mgmt/core/policies/_authentication_async.py +++ b/sdk/core/azure-mgmt-core/azure/mgmt/core/policies/_authentication_async.py @@ -40,7 +40,7 @@ from azure.core.credentials_async import AsyncTokenCredential -from ._authentication import _parse_claims_challenge, _AuxiliaryAuthenticationPolicyBase +from ._authentication import _AuxiliaryAuthenticationPolicyBase HTTPRequestType = Union[LegacyHttpRequest, HttpRequest] @@ -66,33 +66,8 @@ class AsyncARMChallengeAuthenticationPolicy(AsyncBearerTokenCredentialPolicy): This policy internally handles Continuous Access Evaluation (CAE) challenges. When it can't complete a challenge, it will return the 401 (unauthorized) response from ARM. - - :param ~azure.core.credentials.TokenCredential credential: credential for authorizing requests - :param str scopes: required authentication scopes """ - # pylint:disable=unused-argument - async def on_challenge( - self, - request: PipelineRequest[HTTPRequestType], - response: PipelineResponse[HTTPRequestType, AsyncHTTPResponseType], - ) -> bool: - """Authorize request according to an ARM authentication challenge - - :param ~azure.core.pipeline.PipelineRequest request: the request which elicited an authentication challenge - :param ~azure.core.pipeline.PipelineResponse response: the resource provider's response - :returns: a bool indicating whether the policy should send the request - :rtype: bool - """ - # Casting, as the code seems to be certain that on_challenge this header will be present - challenge: str = cast(str, response.http_response.headers.get("WWW-Authenticate")) - claims = _parse_claims_challenge(challenge) - if claims: - await self.authorize_request(request, *self._scopes, claims=claims) - return True - - return False - class AsyncAuxiliaryAuthenticationPolicy( _AuxiliaryAuthenticationPolicyBase[AsyncTokenCredential], diff --git a/sdk/core/azure-mgmt-core/setup.py b/sdk/core/azure-mgmt-core/setup.py index 5203a92c70ea..e55baec158a7 100644 --- a/sdk/core/azure-mgmt-core/setup.py +++ b/sdk/core/azure-mgmt-core/setup.py @@ -69,7 +69,7 @@ "pytyped": ["py.typed"], }, install_requires=[ - "azure-core>=1.31.0", + "azure-core>=1.32.0", ], python_requires=">=3.8", ) diff --git a/sdk/core/azure-mgmt-core/tests/test_authentication.py b/sdk/core/azure-mgmt-core/tests/test_authentication.py index 3f6df0b7dd8a..4f04d3d75274 100644 --- a/sdk/core/azure-mgmt-core/tests/test_authentication.py +++ b/sdk/core/azure-mgmt-core/tests/test_authentication.py @@ -29,7 +29,6 @@ from azure.core.credentials import AccessToken from azure.core.pipeline import Pipeline from azure.mgmt.core.policies._authentication import ( - _parse_claims_challenge, ARMChallengeAuthenticationPolicy, AuxiliaryAuthenticationPolicy, ) @@ -47,37 +46,6 @@ ip_claim = b'{"access_token":{"nbf":{"essential":true,"value":"1610563006"},"xms_rp_ipaddr":{"value":"1.2.3.4"}}}' CLAIM_IP = base64.b64encode(ip_claim).decode()[:-2] # Trim off padding = characters - -@pytest.mark.parametrize( - "challenge,expected_claims", - ( - # CAE - insufficient claims - ( - f'Bearer realm="", authorization_uri="https://login.microsoftonline.com/common/oauth2/authorize", client_id="00000003-0000-0000-c000-000000000000", error="insufficient_claims", claims="{CLAIM_TOKEN}"', - '{"access_token": {"foo": "bar"}}', - ), - # CAE - sessions revoked - ( - f'Bearer authorization_uri="https://login.windows-ppe.net/", error="invalid_token", error_description="User session has been revoked", claims={CLAIM_NBF}', - '{"access_token":{"nbf":{"essential":true, "value":"1603742800"}}}', - ), - # CAE - IP policy - ( - f'Bearer authorization_uri="https://login.windows.net/", error="invalid_token", error_description="Tenant IP Policy validate failed.", claims={CLAIM_IP}', - '{"access_token":{"nbf":{"essential":true,"value":"1610563006"},"xms_rp_ipaddr":{"value":"1.2.3.4"}}}', - ), - # ARM - ( - 'Bearer authorization_uri="https://login.windows.net/", error="invalid_token", error_description="The authentication failed because of missing \'Authorization\' header."', - None, - ), - ), -) -def test_challenge_parsing(challenge, expected_claims): - claims = _parse_claims_challenge(challenge) - assert claims == expected_claims - - def test_auxiliary_authentication_policy(): """The auxiliary authentication policy should add a header containing a token from its credential""" first_token = AccessToken("first", int(time.time()) + 3600) From 29b1bc64c9607e7202d7d197227f6dae4134a602 Mon Sep 17 00:00:00 2001 From: msyyc <70930885+msyyc@users.noreply.github.com> Date: Tue, 26 Nov 2024 16:16:21 +0800 Subject: [PATCH 2/7] update test --- .../tests/test_authentication.py | 58 +++++++++++++++---- 1 file changed, 46 insertions(+), 12 deletions(-) diff --git a/sdk/core/azure-mgmt-core/tests/test_authentication.py b/sdk/core/azure-mgmt-core/tests/test_authentication.py index 4f04d3d75274..8b39cd7aa43a 100644 --- a/sdk/core/azure-mgmt-core/tests/test_authentication.py +++ b/sdk/core/azure-mgmt-core/tests/test_authentication.py @@ -87,7 +87,7 @@ def test_claims_challenge(): expected_claims = '{"access_token": {"essential": "true"}' expected_scope = "scope" - challenge = 'Bearer authorization_uri="https://localhost", error=".", error_description=".", claims="{}"'.format( + challenge = 'Bearer authorization_uri="https://localhost", error="insufficient_claims", error_description=".", claims="{}"'.format( base64.b64encode(expected_claims.encode()).decode() ) responses = ( @@ -134,28 +134,62 @@ def get_token(*scopes, **kwargs): def test_multiple_claims_challenges(): - """ARMChallengeAuthenticationPolicy should not attempt to handle a response having multiple claims challenges""" + """ARMChallengeAuthenticationPolicy handle a response having multiple claims challenges""" + + first_token = AccessToken("first", int(time.time()) + 3600) + second_token = AccessToken("second", int(time.time()) + 3600) + tokens = (t for t in (first_token, second_token)) + + expected_claims = '{"access_token": {"essential": "true"}' + expected_scope = "scope" + + claims = base64.b64encode(expected_claims.encode()).decode() expected_header = ",".join( ( - 'Bearer realm="", authorization_uri="https://login.microsoftonline.com/common/oauth2/authorize", client_id="00000003-0000-0000-c000-000000000000", error="insufficient_claims", claims="eyJhY2Nlc3NfdG9rZW4iOiB7ImZvbyI6ICJiYXIifX0="', - 'Bearer authorization_uri="https://login.windows-ppe.net/", error="invalid_token", error_description="User session has been revoked", claims="eyJhY2Nlc3NfdG9rZW4iOnsibmJmIjp7ImVzc2VudGlhbCI6dHJ1ZSwgInZhbHVlIjoiMTYwMzc0MjgwMCJ9fX0="', + 'Bearer realm="", authorization_uri="https://localhost", client_id="00", error="insufficient_claims", claims="{}"'.format(claims), + 'Bearer authorization_uri="https://login.windows-ppe.net/", error="invalid_token", error_description="User session has been revoked", claims="{}"'.format(claims), + ) + ) + + responses = ( + r + for r in ( + Mock(status_code=401, headers={"WWW-Authenticate": expected_header}), + Mock(status_code=200), ) ) def send(request): - return Mock(status_code=401, headers={"WWW-Authenticate": expected_header}) + res = next(responses) + if res.status_code == 401: + expected_token = first_token.token + else: + expected_token = second_token.token + assert request.headers["Authorization"] == "Bearer " + expected_token + + return res + + def get_token(*scopes, **kwargs): + assert scopes == (expected_scope,) + return next(tokens) + credential = Mock(spec_set=["get_token"], get_token=Mock(wraps=get_token)) transport = Mock(send=Mock(wraps=send)) - credential = FakeTokenCredential() - policies = [ARMChallengeAuthenticationPolicy(credential, "scope")] + policies = [ARMChallengeAuthenticationPolicy(credential, expected_scope)] pipeline = Pipeline(transport=transport, policies=policies) response = pipeline.run(HttpRequest("GET", "https://localhost")) - assert transport.send.call_count == 1 - assert credential.get_token_count == 1 + assert response.http_response.status_code == 200 + assert transport.send.call_count == 2 + assert credential.get_token.call_count == 2 - # the policy should have returned the error response because it was unable to handle the challenge - assert response.http_response.status_code == 401 - assert response.http_response.headers["WWW-Authenticate"] == expected_header + args, kwargs = credential.get_token.call_args + assert expected_scope in args + assert kwargs["claims"] == expected_claims + + with pytest.raises(StopIteration): + next(tokens) + with pytest.raises(StopIteration): + next(responses) \ No newline at end of file From 92944e19dbdba090692cc54b77aeb35327e1a2c7 Mon Sep 17 00:00:00 2001 From: msyyc <70930885+msyyc@users.noreply.github.com> Date: Tue, 26 Nov 2024 16:20:32 +0800 Subject: [PATCH 3/7] update changelog --- sdk/core/azure-mgmt-core/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sdk/core/azure-mgmt-core/CHANGELOG.md b/sdk/core/azure-mgmt-core/CHANGELOG.md index f83890f3b1d8..57eaf8cea642 100644 --- a/sdk/core/azure-mgmt-core/CHANGELOG.md +++ b/sdk/core/azure-mgmt-core/CHANGELOG.md @@ -1,5 +1,11 @@ # Release History +## 1.5.1 (2023-XX-XX) + +### Other Changes + +- `ARMChallengeAuthenticationPolicy` adopt `on_challenge` in `BearerTokenCredentialPolicy` of `azure-core` to support complete CAE challenges. + ## 1.5.0 (2024-10-31) ### Features Added From cae655ad21434509737f476390382802afb35b70 Mon Sep 17 00:00:00 2001 From: msyyc <70930885+msyyc@users.noreply.github.com> Date: Tue, 26 Nov 2024 16:20:43 +0800 Subject: [PATCH 4/7] update version --- sdk/core/azure-mgmt-core/azure/mgmt/core/_version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/azure-mgmt-core/azure/mgmt/core/_version.py b/sdk/core/azure-mgmt-core/azure/mgmt/core/_version.py index 1948e34ac9b0..1d786afec45d 100644 --- a/sdk/core/azure-mgmt-core/azure/mgmt/core/_version.py +++ b/sdk/core/azure-mgmt-core/azure/mgmt/core/_version.py @@ -9,4 +9,4 @@ # regenerated. # -------------------------------------------------------------------------- -VERSION = "1.5.0" +VERSION = "1.5.1" From 03e7d57bd4d085e67dd9ddce0dcd8d0a15308343 Mon Sep 17 00:00:00 2001 From: msyyc <70930885+msyyc@users.noreply.github.com> Date: Tue, 26 Nov 2024 16:22:36 +0800 Subject: [PATCH 5/7] format --- .../azure/mgmt/core/policies/_authentication.py | 1 + sdk/core/azure-mgmt-core/tests/test_authentication.py | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/sdk/core/azure-mgmt-core/azure/mgmt/core/policies/_authentication.py b/sdk/core/azure-mgmt-core/azure/mgmt/core/policies/_authentication.py index d2b3316dec5d..dc824406b80d 100644 --- a/sdk/core/azure-mgmt-core/azure/mgmt/core/policies/_authentication.py +++ b/sdk/core/azure-mgmt-core/azure/mgmt/core/policies/_authentication.py @@ -50,6 +50,7 @@ class ARMChallengeAuthenticationPolicy(BearerTokenCredentialPolicy): it will return the 401 (unauthorized) response from ARM. """ + # pylint:disable=too-few-public-methods class _AuxiliaryAuthenticationPolicyBase(Generic[TokenCredentialType]): """Adds auxiliary authorization token header to requests. diff --git a/sdk/core/azure-mgmt-core/tests/test_authentication.py b/sdk/core/azure-mgmt-core/tests/test_authentication.py index 8b39cd7aa43a..9b70b2cec07b 100644 --- a/sdk/core/azure-mgmt-core/tests/test_authentication.py +++ b/sdk/core/azure-mgmt-core/tests/test_authentication.py @@ -46,6 +46,7 @@ ip_claim = b'{"access_token":{"nbf":{"essential":true,"value":"1610563006"},"xms_rp_ipaddr":{"value":"1.2.3.4"}}}' CLAIM_IP = base64.b64encode(ip_claim).decode()[:-2] # Trim off padding = characters + def test_auxiliary_authentication_policy(): """The auxiliary authentication policy should add a header containing a token from its credential""" first_token = AccessToken("first", int(time.time()) + 3600) @@ -147,8 +148,12 @@ def test_multiple_claims_challenges(): expected_header = ",".join( ( - 'Bearer realm="", authorization_uri="https://localhost", client_id="00", error="insufficient_claims", claims="{}"'.format(claims), - 'Bearer authorization_uri="https://login.windows-ppe.net/", error="invalid_token", error_description="User session has been revoked", claims="{}"'.format(claims), + 'Bearer realm="", authorization_uri="https://localhost", client_id="00", error="insufficient_claims", claims="{}"'.format( + claims + ), + 'Bearer authorization_uri="https://login.windows-ppe.net/", error="invalid_token", error_description="User session has been revoked", claims="{}"'.format( + claims + ), ) ) @@ -192,4 +197,4 @@ def get_token(*scopes, **kwargs): with pytest.raises(StopIteration): next(tokens) with pytest.raises(StopIteration): - next(responses) \ No newline at end of file + next(responses) From 044dbf2075139224db02f3df4d7c1e0b49ef8c24 Mon Sep 17 00:00:00 2001 From: msyyc <70930885+msyyc@users.noreply.github.com> Date: Tue, 26 Nov 2024 16:32:24 +0800 Subject: [PATCH 6/7] update test --- .../asynctests/test_authentication_async.py | 62 ++++++++++++++----- 1 file changed, 48 insertions(+), 14 deletions(-) diff --git a/sdk/core/azure-mgmt-core/tests/asynctests/test_authentication_async.py b/sdk/core/azure-mgmt-core/tests/asynctests/test_authentication_async.py index 8be0b0576818..a9f6e0337b9d 100644 --- a/sdk/core/azure-mgmt-core/tests/asynctests/test_authentication_async.py +++ b/sdk/core/azure-mgmt-core/tests/asynctests/test_authentication_async.py @@ -50,7 +50,7 @@ async def test_claims_challenge(): expected_claims = '{"access_token": {"essential": "true"}' expected_scope = "scope" - challenge = 'Bearer authorization_uri="https://localhost", error=".", error_description=".", claims="{}"'.format( + challenge = 'Bearer authorization_uri="https://localhost", error="insufficient_claims", error_description=".", claims="{}"'.format( base64.b64encode(expected_claims.encode()).decode() ) responses = ( @@ -97,34 +97,68 @@ async def get_token(*scopes, **kwargs): async def test_multiple_claims_challenges(): - """ARMChallengeAuthenticationPolicy should not attempt to handle a response having multiple claims challenges""" + """ARMChallengeAuthenticationPolicy handle a response having multiple claims challenges""" + first_token = AccessToken("first", int(time.time()) + 3600) + second_token = AccessToken("second", int(time.time()) + 3600) + tokens = (t for t in (first_token, second_token)) + + expected_claims = '{"access_token": {"essential": "true"}' + expected_scope = "scope" + + claims = base64.b64encode(expected_claims.encode()).decode() expected_header = ",".join( ( - 'Bearer realm="", authorization_uri="https://login.microsoftonline.com/common/oauth2/authorize", client_id="00000003-0000-0000-c000-000000000000", error="insufficient_claims", claims="eyJhY2Nlc3NfdG9rZW4iOiB7ImZvbyI6ICJiYXIifX0="', - 'Bearer authorization_uri="https://login.windows-ppe.net/", error="invalid_token", error_description="User session has been revoked", claims="eyJhY2Nlc3NfdG9rZW4iOnsibmJmIjp7ImVzc2VudGlhbCI6dHJ1ZSwgInZhbHVlIjoiMTYwMzc0MjgwMCJ9fX0="', + 'Bearer realm="", authorization_uri="https://localhost", client_id="00", error="insufficient_claims", claims="{}"'.format( + claims + ), + 'Bearer authorization_uri="https://login.windows-ppe.net/", error="invalid_token", error_description="User session has been revoked", claims="{}"'.format( + claims + ), + ) + ) + + responses = ( + r + for r in ( + Mock(status_code=401, headers={"WWW-Authenticate": expected_header}), + Mock(status_code=200), ) ) async def send(request): - return Mock(status_code=401, headers={"WWW-Authenticate": expected_header}) + res = next(responses) + if res.status_code == 401: + expected_token = first_token.token + else: + expected_token = second_token.token + assert request.headers["Authorization"] == "Bearer " + expected_token - async def get_token(*_, **__): - return AccessToken("***", 42) + return res + + async def get_token(*scopes, **kwargs): + assert scopes == (expected_scope,) + return next(tokens) - transport = Mock(send=Mock(wraps=send)) credential = Mock(spec_set=["get_token"], get_token=Mock(wraps=get_token)) - policies = [AsyncARMChallengeAuthenticationPolicy(credential, "scope")] + transport = Mock(send=Mock(wraps=send)) + policies = [AsyncARMChallengeAuthenticationPolicy(credential, expected_scope)] pipeline = AsyncPipeline(transport=transport, policies=policies) response = await pipeline.run(HttpRequest("GET", "https://localhost")) - assert transport.send.call_count == 1 - assert credential.get_token.call_count == 1 + assert response.http_response.status_code == 200 + assert transport.send.call_count == 2 + assert credential.get_token.call_count == 2 + + args, kwargs = credential.get_token.call_args + assert expected_scope in args + assert kwargs["claims"] == expected_claims - # the policy should have returned the error response because it was unable to handle the challenge - assert response.http_response.status_code == 401 - assert response.http_response.headers["WWW-Authenticate"] == expected_header + with pytest.raises(StopIteration): + next(tokens) + with pytest.raises(StopIteration): + next(responses) async def test_auxiliary_authentication_policy(): From 5212f210ff265d054ab9f7beb0c23228f01b7fa8 Mon Sep 17 00:00:00 2001 From: msyyc <70930885+msyyc@users.noreply.github.com> Date: Tue, 26 Nov 2024 17:34:50 +0800 Subject: [PATCH 7/7] fix pylint --- .../azure/mgmt/core/policies/_authentication_async.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/azure-mgmt-core/azure/mgmt/core/policies/_authentication_async.py b/sdk/core/azure-mgmt-core/azure/mgmt/core/policies/_authentication_async.py index 936760af860e..4707db700229 100644 --- a/sdk/core/azure-mgmt-core/azure/mgmt/core/policies/_authentication_async.py +++ b/sdk/core/azure-mgmt-core/azure/mgmt/core/policies/_authentication_async.py @@ -23,7 +23,7 @@ # IN THE SOFTWARE. # # -------------------------------------------------------------------------- -from typing import cast, Awaitable, Optional, List, Union, Any +from typing import Awaitable, Optional, List, Union, Any import inspect from azure.core.pipeline.policies import (