Skip to content

Commit

Permalink
Add more detail to location restriction self-test errors.
Browse files Browse the repository at this point in the history
  • Loading branch information
tdilauro committed Sep 9, 2024
1 parent 81b7ce9 commit 6b2380b
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 13 deletions.
21 changes: 13 additions & 8 deletions src/palace/manager/api/authentication/basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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

Check warning on line 418 in src/palace/manager/api/authentication/basic.py

View check run for this annotation

Codecov / codecov/patch

src/palace/manager/api/authentication/basic.py#L417-L418

Added lines #L417 - L418 were not covered by tests

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

Check warning on line 434 in src/palace/manager/api/authentication/basic.py

View check run for this annotation

Codecov / codecov/patch

src/palace/manager/api/authentication/basic.py#L433-L434

Added lines #L433 - L434 were not covered by tests
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,
Expand Down
20 changes: 18 additions & 2 deletions src/palace/manager/api/sip/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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):
Expand Down
15 changes: 12 additions & 3 deletions tests/manager/api/sip/test_authentication_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand Down

0 comments on commit 6b2380b

Please sign in to comment.