From 8a592bb300a65462b3493b27d9e2a94549763978 Mon Sep 17 00:00:00 2001 From: Henry Su Date: Wed, 9 Sep 2026 22:16:37 -0500 Subject: [PATCH] fix(auth): raise AuthWeakPasswordError for responses without an error code The legacy branch in handle_exception required data["weak_password"] to be both a dict and a list, so it could never run, and it inspected the weak_password object where it meant to inspect its nested reasons list. Auth responses that report a weak password without a code/error_code were therefore downgraded to a generic AuthApiError and callers lost .reasons. Validate weak_password["reasons"] as a non-empty list of strings instead, matching auth-js. Fixes #1628 --- src/auth/src/supabase_auth/helpers.py | 21 ++++-- src/auth/tests/test_helpers.py | 98 +++++++++++++++++---------- 2 files changed, 76 insertions(+), 43 deletions(-) diff --git a/src/auth/src/supabase_auth/helpers.py b/src/auth/src/supabase_auth/helpers.py index fbaf4dae..ab110a53 100644 --- a/src/auth/src/supabase_auth/helpers.py +++ b/src/auth/src/supabase_auth/helpers.py @@ -163,18 +163,25 @@ def handle_exception(error: HTTPStatusError | RuntimeError) -> AuthError: error_code = data.get("error_code") if error_code is None: + # Legacy support for weak password errors, from before error codes + # existed: the reasons are only carried by the `weak_password` object. + weak_password = ( + data.get("weak_password") if isinstance(data, dict) else None + ) + reasons = ( + weak_password.get("reasons") + if isinstance(weak_password, dict) + else None + ) if ( - isinstance(data, dict) - and data - and isinstance(data.get("weak_password"), dict) - and data.get("weak_password") - and isinstance(data.get("weak_password"), list) - and len(data["weak_password"]) + isinstance(reasons, list) + and reasons + and all(isinstance(reason, str) for reason in reasons) ): return AuthWeakPasswordError( get_error_message(data), error.response.status_code, - data["weak_password"].get("reasons"), + reasons, ) elif error_code == "weak_password": return AuthWeakPasswordError( diff --git a/src/auth/tests/test_helpers.py b/src/auth/tests/test_helpers.py index e8ee6635..f9a31909 100644 --- a/src/auth/tests/test_helpers.py +++ b/src/auth/tests/test_helpers.py @@ -210,11 +210,8 @@ def test_handle_exception_network_error() -> None: def test_handle_exception_with_weak_password_attribute() -> None: - # In the implementation there's a logical error in the code: - # It checks if data.get("weak_password") is BOTH a dict AND a list - # This can never be true. Let's just test the error_code path which works. - - # Test case with error_code=None, so we take the alternate default path + # Test case with error_code=None and no weak_password object, so the + # generic AuthApiError path is taken. mock_response = MagicMock(spec=Response) mock_response.status_code = 400 mock_response.json.return_value = { @@ -234,6 +231,60 @@ def test_handle_exception_with_weak_password_attribute() -> None: assert result.code is None +def test_handle_exception_legacy_weak_password_without_error_code() -> None: + # GoTrue responses that predate error codes report weak passwords only through + # the `weak_password` object, so they must still surface as AuthWeakPasswordError. + mock_response = MagicMock(spec=Response) + mock_response.status_code = 422 + mock_response.json.return_value = { + "message": "Password too weak", + "weak_password": {"reasons": ["length", "characters"]}, + } + + exception = HTTPStatusError( + "Password error", request=MagicMock(), response=mock_response + ) + + with patch("supabase_auth.helpers.parse_response_api_version", return_value=None): + result = handle_exception(exception) + + assert isinstance(result, AuthWeakPasswordError) + assert result.message == "Password too weak" + assert result.status == 422 + assert result.reasons == ["length", "characters"] + + +@pytest.mark.parametrize( + "weak_password", + [ + {"reasons": []}, + {"reasons": "length"}, + {"reasons": ["length", 1]}, + {}, + "length", + ], +) +def test_handle_exception_legacy_weak_password_ignores_malformed_reasons( + weak_password, +) -> None: + mock_response = MagicMock(spec=Response) + mock_response.status_code = 422 + mock_response.json.return_value = { + "message": "Password too weak", + "weak_password": weak_password, + } + + exception = HTTPStatusError( + "Password error", request=MagicMock(), response=mock_response + ) + + with patch("supabase_auth.helpers.parse_response_api_version", return_value=None): + result = handle_exception(exception) + + assert isinstance(result, AuthApiError) + assert not isinstance(result, AuthWeakPasswordError) + + def test_handle_exception_weak_password_with_error_code() -> None: # Test case for weak password identified by error_code mock_response = MagicMock(spec=Response) @@ -332,11 +383,7 @@ def test_is_http_url() -> None: def test_handle_exception_weak_password_branch() -> None: - """Specifically targeting the unreachable branch in handle_exception with weak_password. - - This test attempts to test the branch where weak_password needs to be both a dict and a list, - which is logically impossible, so we'll test it by mocking the implementation details. - """ + """Cover the legacy weak_password branch of handle_exception.""" import httpx from supabase_auth.errors import AuthWeakPasswordError from supabase_auth.helpers import handle_exception @@ -346,13 +393,6 @@ def test_handle_exception_weak_password_branch() -> None: mock_response.status_code = 400 mock_response.headers = {} - # Create a special mock dict that pretends to be both a dict and a list - class WeirdDict(dict): - def __init__(self, *args, **kwargs) -> None: - super().__init__(*args, **kwargs) - self.reasons = ["Password too short"] - - # Mock json response with our special dict mock_response.json.return_value = { "message": "Password too weak", "weak_password": {"reasons": ["Password too short"]}, @@ -363,23 +403,9 @@ def __init__(self, *args, **kwargs) -> None: "Password error", request=MagicMock(spec=httpx.Request), response=mock_response ) - # We need to directly target the specific branch handling weak passwords - # First, we need to monkey patch the implementation temporarily to reach our branch - original_isinstance = isinstance - - def patched_isinstance(obj, cls): # noqa - # Make weak_password appear as both dict and list when needed - if obj == mock_response.json()["weak_password"] and cls in (dict, list): - return True - return original_isinstance(obj, cls) - - with ( - patch("supabase_auth.helpers.isinstance", side_effect=patched_isinstance), - patch("supabase_auth.helpers.len", return_value=1), - ): - result = handle_exception(exception) + result = handle_exception(exception) - # Check if our test coverage reached the AuthWeakPasswordError branch - assert isinstance(result, AuthWeakPasswordError) - assert result.message == "Password too weak" - assert result.status == 400 + assert isinstance(result, AuthWeakPasswordError) + assert result.message == "Password too weak" + assert result.status == 400 + assert result.reasons == ["Password too short"]