Skip to content

Commit df01859

Browse files
authored
[Core] Read stream challenge error response (#42920)
If the response is not read, the error message that is printed for users is a generic message saying that the "Operation returned an invalid status". To provide improved insight, if the streaming was enabled, read the 401 response body. Signed-off-by: Paul Van Eck <[email protected]>
1 parent c334c85 commit df01859

File tree

4 files changed

+105
-0
lines changed

4 files changed

+105
-0
lines changed

sdk/core/azure-core/azure/core/pipeline/policies/_authentication.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,14 @@ def send(self, request: PipelineRequest[HTTPRequestType]) -> PipelineResponse[HT
170170
try:
171171
request_authorized = self.on_challenge(request, response)
172172
except Exception as ex:
173+
# If the response is streamed, read it so the error message is immediately available to the user.
174+
# Otherwise, a generic error message will be given and the user will have to read the response
175+
# body to see the actual error.
176+
if response.context.options.get("stream"):
177+
try:
178+
response.http_response.read() # type: ignore
179+
except Exception: # pylint:disable=broad-except
180+
pass
173181
# Raise the exception from the token request with the original 401 response
174182
raise ex from HttpResponseError(response=response.http_response)
175183

sdk/core/azure-core/azure/core/pipeline/policies/_authentication_async.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,15 @@ async def send(
114114
try:
115115
request_authorized = await self.on_challenge(request, response)
116116
except Exception as ex:
117+
# If the response is streamed, read it so the error message is immediately available to the user.
118+
# Otherwise, a generic error message will be given and the user will have to read the response
119+
# body to see the actual error.
120+
if response.context.options.get("stream"):
121+
try:
122+
await response.http_response.read() # type: ignore
123+
except Exception: # pylint:disable=broad-except
124+
pass
125+
117126
# Raise the exception from the token request with the original 401 response
118127
raise ex from HttpResponseError(response=response.http_response)
119128

sdk/core/azure-core/tests/async_tests/test_authentication_async.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -689,3 +689,49 @@ async def mock_transport_send(request):
689689
# Verify the HttpResponseError contains the original 401 response
690690
http_response_error = original_exception.__cause__
691691
assert http_response_error.response is response_mock
692+
693+
694+
@pytest.mark.asyncio
695+
@pytest.mark.parametrize("http_request", HTTP_REQUESTS)
696+
async def test_async_bearer_policy_reads_streamed_response_on_challenge_exception(http_request):
697+
"""Test that the async policy reads streamed response body when on_challenge raises exception"""
698+
699+
# Create a credential that will raise an exception when get_token is called with claims
700+
async def failing_get_token(*scopes, **kwargs):
701+
if "claims" in kwargs:
702+
raise ClientAuthenticationError("Failed to get token with claims")
703+
return AccessToken("initial_token", int(time.time()) + 3600)
704+
705+
credential = Mock(spec_set=["get_token"], get_token=failing_get_token)
706+
policy = AsyncBearerTokenCredentialPolicy(credential, "scope")
707+
708+
# Create a 401 response with insufficient_claims challenge that will trigger the exception
709+
test_claims = '{"access_token":{"foo":"bar"}}'
710+
encoded_claims = base64.urlsafe_b64encode(test_claims.encode()).decode().rstrip("=")
711+
challenge_header = f'Bearer error="insufficient_claims", claims="{encoded_claims}"'
712+
713+
# Create the mock HTTP response with stream reading capability
714+
http_response_mock = Mock()
715+
http_response_mock.status_code = 401
716+
http_response_mock.headers = {"WWW-Authenticate": challenge_header}
717+
http_response_mock.read = AsyncMock(return_value=b"Error details from server")
718+
719+
# Mock transport that returns the HTTP response directly (it will be wrapped by Pipeline)
720+
async def mock_transport_send(request, **kwargs):
721+
return http_response_mock
722+
723+
transport = Mock(send=mock_transport_send)
724+
725+
# Create pipeline with stream option
726+
pipeline = AsyncPipeline(transport=transport, policies=[policy])
727+
728+
# Execute the request and verify exception handling
729+
with pytest.raises(ClientAuthenticationError) as exc_info:
730+
await pipeline.run(http_request("GET", "https://example.com"), stream=True)
731+
732+
# Verify that the response.read() was called to consume the stream
733+
http_response_mock.read.assert_called_once()
734+
735+
# Verify the exception chaining
736+
assert exc_info.value.__cause__ is not None
737+
assert isinstance(exc_info.value.__cause__, HttpResponseError)

sdk/core/azure-core/tests/test_authentication.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -885,3 +885,45 @@ def mock_transport_send(request):
885885
# Verify the HttpResponseError contains the original 401 response
886886
http_response_error = original_exception.__cause__
887887
assert http_response_error.response is response_mock
888+
889+
890+
@pytest.mark.parametrize("http_request", HTTP_REQUESTS)
891+
def test_bearer_policy_reads_streamed_response_on_challenge_exception(http_request):
892+
"""Test that the policy reads streamed response body when on_challenge raises exception"""
893+
894+
# Create a credential that will raise an exception when get_token is called with claims
895+
def failing_get_token(*scopes, **kwargs):
896+
if "claims" in kwargs:
897+
raise ClientAuthenticationError("Failed to get token with claims")
898+
return AccessToken("initial_token", int(time.time()) + 3600)
899+
900+
credential = Mock(spec_set=["get_token"], get_token=failing_get_token)
901+
policy = BearerTokenCredentialPolicy(credential, "scope")
902+
903+
# Create a 401 response with insufficient_claims challenge that will trigger the exception
904+
test_claims = '{"access_token":{"foo":"bar"}}'
905+
encoded_claims = base64.urlsafe_b64encode(test_claims.encode()).decode().rstrip("=")
906+
challenge_header = f'Bearer error="insufficient_claims", claims="{encoded_claims}"'
907+
908+
# Create the mock HTTP response with stream reading capability
909+
http_response_mock = Mock()
910+
http_response_mock.status_code = 401
911+
http_response_mock.headers = {"WWW-Authenticate": challenge_header}
912+
http_response_mock.read = Mock(return_value=b"Error details from server")
913+
914+
# Mock transport that returns the HTTP response directly (it will be wrapped by Pipeline)
915+
transport = Mock(send=Mock(return_value=http_response_mock))
916+
917+
# Create pipeline with stream option
918+
pipeline = Pipeline(transport=transport, policies=[policy])
919+
920+
# Execute the request and verify exception handling
921+
with pytest.raises(ClientAuthenticationError) as exc_info:
922+
pipeline.run(http_request("GET", "https://example.com"), stream=True)
923+
924+
# Verify that the response.read() was called to consume the stream
925+
http_response_mock.read.assert_called_once()
926+
927+
# Verify the exception chaining
928+
assert exc_info.value.__cause__ is not None
929+
assert isinstance(exc_info.value.__cause__, HttpResponseError)

0 commit comments

Comments
 (0)