diff --git a/tests/manager/api/test_authenticator.py b/tests/manager/api/test_authenticator.py index 3bceb56e5..a1a6caa4c 100644 --- a/tests/manager/api/test_authenticator.py +++ b/tests/manager/api/test_authenticator.py @@ -7,6 +7,7 @@ import json import re from collections.abc import Callable +from contextlib import nullcontext from decimal import Decimal from functools import partial from typing import TYPE_CHECKING, Literal, cast @@ -72,7 +73,7 @@ from palace.manager.util.datetime_helpers import utc_now from palace.manager.util.http import RemoteIntegrationException from palace.manager.util.opds_writer import OPDSFeed -from palace.manager.util.problem_detail import ProblemDetail +from palace.manager.util.problem_detail import ProblemDetail, ProblemDetailException from tests.fixtures.announcements import AnnouncementFixture from tests.fixtures.library import LibraryFixture from tests.mocks.analytics_provider import MockAnalyticsProvider @@ -1421,14 +1422,14 @@ def test_authenticated_patron_updates_metadata_if_necessary( True, ), # If we get an incomplete patrondata from remote_authenticate, and enforce_library_identifier_restriction - # returns None, we don't call remote_patron_lookup and get a ProblemDetail + # raises a ProblemDetail, then we don't call remote_patron_lookup and get that ProblemDetail. ( PatronData(authorization_identifier="a", complete=False), - None, + PATRON_OF_ANOTHER_LIBRARY.with_debug("some debug details"), None, 0, False, - PATRON_OF_ANOTHER_LIBRARY, + PATRON_OF_ANOTHER_LIBRARY.with_debug("some debug details"), ), # If we get an incomplete patrondata from remote_authenticate, and enforce_library_identifier_restriction # returns an incomplete patrondata, we call remote_patron_lookup @@ -1489,16 +1490,21 @@ def test_authenticated_patron_only_calls_remote_patron_lookup_once( lookup_return, calls_lookup, create_patron, - expected, + expected: Literal[True] | ProblemDetail, ): # The call to remote_patron_lookup is potentially expensive, so we want to avoid calling it # more than once. This test makes sure that if we have a complete patrondata from remote_authenticate, # or from enforce_library_identifier_restriction, we don't call remote_patron_lookup. provider = mock_basic() provider.remote_authenticate = MagicMock(return_value=auth_return) - provider.enforce_library_identifier_restriction = MagicMock( - return_value=enforce_return - ) + if isinstance(enforce_return, ProblemDetail): + provider.enforce_library_identifier_restriction = MagicMock( + side_effect=ProblemDetailException(enforce_return) + ) + else: + provider.enforce_library_identifier_restriction = MagicMock( + return_value=enforce_return + ) provider.remote_patron_lookup = MagicMock(return_value=lookup_return) username = "a" @@ -1510,8 +1516,16 @@ def test_authenticated_patron_only_calls_remote_patron_lookup_once( db_patron = db.patron() db_patron.authorization_identifier = username - patron = provider.authenticated_patron(db.session, credentials) + context_manager = ( + pytest.raises(ProblemDetailException) + if isinstance(expected, ProblemDetail) + else nullcontext() + ) + with context_manager as ctx: + patron = provider.authenticated_patron(db.session, credentials) + provider.remote_authenticate.assert_called_once_with(username, password) + if auth_return is not None: provider.enforce_library_identifier_restriction.assert_called_once_with( auth_return @@ -1519,7 +1533,11 @@ def test_authenticated_patron_only_calls_remote_patron_lookup_once( else: provider.enforce_library_identifier_restriction.assert_not_called() assert provider.remote_patron_lookup.call_count == calls_lookup - if expected is True: + + if isinstance(expected, ProblemDetail): + problem_detail = ctx.value.problem_detail + assert problem_detail == expected + elif expected is True: # Make sure we get a Patron object back and that the patrondata has been # properly applied to it assert isinstance(patron, Patron) @@ -1576,7 +1594,10 @@ def test_update_patron_metadata_returns_none_different_library( def test_restriction_matches(self): """Test the behavior of the library identifier restriction algorithm.""" - m = BasicAuthenticationProvider._restriction_matches + + def m(*args) -> bool: + result, reason = BasicAuthenticationProvider._restriction_matches(*args) + return result # If restriction is none, we always return True. assert ( @@ -1823,7 +1844,8 @@ def test_enforce_library_identifier_restriction( == patrondata ) else: - assert provider.enforce_library_identifier_restriction(patrondata) is None + with pytest.raises(ProblemDetailException) as exc: + provider.enforce_library_identifier_restriction(patrondata) # Test match applied to library_identifier field on complete patrondata provider.library_identifier_field = "other" @@ -1834,7 +1856,8 @@ def test_enforce_library_identifier_restriction( == patrondata ) else: - assert provider.enforce_library_identifier_restriction(patrondata) is None + with pytest.raises(ProblemDetailException) as exc: + provider.enforce_library_identifier_restriction(patrondata) # Test match applied to library_identifier field on incomplete patrondata provider.library_identifier_field = "other" @@ -1849,10 +1872,8 @@ def test_enforce_library_identifier_restriction( == remote_patrondata ) else: - assert ( + with pytest.raises(ProblemDetailException) as exc: provider.enforce_library_identifier_restriction(local_patrondata) - is None - ) provider.remote_patron_lookup.assert_called_once_with(local_patrondata) def test_enforce_library_identifier_restriction_library_identifier_field_none( diff --git a/tests/manager/api/test_sirsidynix_auth_provider.py b/tests/manager/api/test_sirsidynix_auth_provider.py index 4de582b29..86a839bee 100644 --- a/tests/manager/api/test_sirsidynix_auth_provider.py +++ b/tests/manager/api/test_sirsidynix_auth_provider.py @@ -1,4 +1,5 @@ import json +from contextlib import nullcontext from copy import deepcopy from dataclasses import dataclass from functools import partial @@ -21,6 +22,7 @@ from palace.manager.core.selftest import SelfTestResult from palace.manager.sqlalchemy.model.patron import Patron from palace.manager.util.http import HTTP +from palace.manager.util.problem_detail import ProblemDetail, ProblemDetailException from tests.fixtures.database import DatabaseTransactionFixture from tests.mocks.mock import MockRequestsResponse @@ -376,7 +378,9 @@ def test__request(self, sirsi_auth_fixture: SirsiAuthFixture): ( LibraryIdentifierRestriction.PREFIX, "abc", - PATRON_OF_ANOTHER_LIBRARY, + PATRON_OF_ANOTHER_LIBRARY.with_debug( + "'patronType' does not match library restriction: 'testtype' does not start with 'abc'." + ), ), ], ) @@ -386,7 +390,7 @@ def test_full_auth_request( sirsi_auth_fixture: SirsiAuthFixture, restriction_type: LibraryIdentifierRestriction, restriction: str, - expected: Literal[True] | PatronData, + expected: Literal[True] | ProblemDetail, ): library = db.default_library() library_settings = sirsi_auth_fixture.library_settings( @@ -412,9 +416,15 @@ def test_full_auth_request( library_identifier="testtype", ) ) - patron = provider.authenticated_patron( - db.session, {"username": "testuser", "password": "testpass"} + context_manager = ( + pytest.raises(ProblemDetailException) + if isinstance(expected, ProblemDetail) + else nullcontext() ) + with context_manager as ctx: + patron = provider.authenticated_patron( + db.session, {"username": "testuser", "password": "testpass"} + ) provider.remote_authenticate.assert_called_with("testuser", "testpass") provider.remote_patron_lookup.assert_called() if expected is True: @@ -422,7 +432,8 @@ def test_full_auth_request( assert patron.fines == 50.00 assert patron.block_reason is None else: - assert patron == expected + problem_detail = ctx.value.problem_detail + assert problem_detail == expected def test_blocked_patron_status_info(self, sirsi_auth_fixture: SirsiAuthFixture): provider = sirsi_auth_fixture.provider()