diff --git a/src/palace/manager/api/authentication/basic.py b/src/palace/manager/api/authentication/basic.py index 81a1753c2..42274c21e 100644 --- a/src/palace/manager/api/authentication/basic.py +++ b/src/palace/manager/api/authentication/basic.py @@ -41,7 +41,7 @@ from palace.manager.sqlalchemy.model.patron import Patron from palace.manager.sqlalchemy.util import get_one from palace.manager.util.log import elapsed_time_logging -from palace.manager.util.problem_detail import ProblemDetail +from palace.manager.util.problem_detail import ProblemDetail, ProblemDetailException class LibraryIdentifierRestriction(Enum): @@ -412,28 +412,33 @@ def testing_patron_or_bust(self, _db: Session) -> tuple[Patron, str | None]: if self.test_username is None: raise CannotLoadConfiguration("No test patron identifier is configured.") - patron, password = self.testing_patron(_db) + try: + patron, password = self.testing_patron(_db) + except ProblemDetailException as e: + patron = e.problem_detail + if isinstance(patron, Patron): return patron, password + debug_message = None if not patron: message = ( "Remote declined to authenticate the test patron. " "The patron may not exist or its password may be wrong." ) elif isinstance(patron, ProblemDetail): - message = ( - "Test patron lookup returned a problem detail - {}: {} ({})".format( - patron.title, patron.detail, patron.uri - ) - ) + pd = patron + message = f"Test patron lookup returned a problem detail - {pd.title}: {pd.detail} ({pd.uri})" + if pd.debug_message: + message += f" [{pd.debug_message}]" + debug_message = pd.debug_message else: message = ( # type: ignore[unreachable] "Test patron lookup returned invalid value for patron: {!r}".format( patron ) ) - raise IntegrationException(message) + raise IntegrationException(message, debug_message=debug_message) def _run_self_tests(self, _db: Session) -> Generator[SelfTestResult, None, None]: """Verify the credentials of the test patron for this integration, diff --git a/src/palace/manager/api/sip/__init__.py b/src/palace/manager/api/sip/__init__.py index 19166dd53..794544213 100644 --- a/src/palace/manager/api/sip/__init__.py +++ b/src/palace/manager/api/sip/__init__.py @@ -352,6 +352,18 @@ def remote_authenticate( self._enforce_patron_location_restriction(info) return self.info_to_patrondata(info) + def _location_mismatch_message( + self, patron_location: str | None, library_restriction: str | None = None + ) -> str: + library_restriction = library_restriction or self.patron_location_restriction + patron_location = ( + f"'{patron_location}'" if patron_location is not None else "(missing)" + ) + return ( + f"Patron location ({patron_location}) does not match " + f"library location restriction ('{library_restriction}')." + ) + def _enforce_patron_location_restriction( self, info: dict[str, Any] | ProblemDetail ) -> None: @@ -365,9 +377,13 @@ def _enforce_patron_location_restriction( if ( not isinstance(info, ProblemDetail) and self.patron_location_restriction is not None - and info.get("permanent_location") != self.patron_location_restriction + and self.patron_location_restriction != info.get("permanent_location") ): - raise ProblemDetailException(PATRON_OF_ANOTHER_LIBRARY) + raise ProblemDetailException( + PATRON_OF_ANOTHER_LIBRARY.with_debug( + self._location_mismatch_message(info.get("permanent_location")) + ) + ) def _run_self_tests(self, _db): def makeConnection(sip): diff --git a/tests/manager/api/sip/test_authentication_provider.py b/tests/manager/api/sip/test_authentication_provider.py index 0daf048c6..f0a932beb 100644 --- a/tests/manager/api/sip/test_authentication_provider.py +++ b/tests/manager/api/sip/test_authentication_provider.py @@ -346,8 +346,9 @@ def test_remote_authenticate_location_restriction( create_library_settings: Callable[..., SIP2Settings], ): # This patron authentication library instance is configured with "TestLoc". + library_restriction = "TestLoc" library_settings = create_library_settings( - patron_location_restriction="TestLoc" + patron_location_restriction=library_restriction ) provider = create_provider(library_settings=library_settings) client = cast(MockSIPClient, provider.client) @@ -364,14 +365,22 @@ def test_remote_authenticate_location_restriction( client.queue_response(self.end_session_response) with pytest.raises(ProblemDetailException) as exc: provider.remote_authenticate("user", "pass") - assert exc.value.problem_detail == PATRON_OF_ANOTHER_LIBRARY + debug_message = provider._location_mismatch_message(None, library_restriction) + assert exc.value.problem_detail == PATRON_OF_ANOTHER_LIBRARY.with_debug( + debug_message + ) # This patron has the WRONG location. client.queue_response(self.evergreen_patron_with_wrong_loc) client.queue_response(self.end_session_response) with pytest.raises(ProblemDetailException) as exc: provider.remote_authenticate("user", "pass") - assert exc.value.problem_detail == PATRON_OF_ANOTHER_LIBRARY + debug_message = provider._location_mismatch_message( + "OtherLoc", library_restriction + ) + assert exc.value.problem_detail == PATRON_OF_ANOTHER_LIBRARY.with_debug( + debug_message + ) def test_encoding( self,