Skip to content

Commit

Permalink
Remove recently added SIP2 single-value patron location restriction.
Browse files Browse the repository at this point in the history
  • Loading branch information
tdilauro committed Sep 12, 2024
1 parent 54eca18 commit 8cb4b3c
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 58 deletions.
2 changes: 1 addition & 1 deletion src/palace/manager/api/authentication/basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,7 @@ def _restriction_matches(
case [_, _restriction, _] if not _restriction:
pass
case [_, _, _value] if _value is None or _value == "":
failure_reason = "No value in field."
failure_reason = "No value in field"
case [LibraryIdentifierRestriction.REGEX, *_]:
if not (_pattern := cast(Pattern, restriction)).search(value):
failure_reason = f"{value!r} does not match regular expression {_pattern.pattern!r}"
Expand Down
48 changes: 0 additions & 48 deletions src/palace/manager/api/sip/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,19 +195,6 @@ class SIP2LibrarySettings(BasicAuthProviderLibrarySettings):
description="A specific identifier for the library or branch, if used in patron authentication",
),
)
# Used with SIP2, when it is available in the patron information response.
patron_location_restriction: str | None = FormField(
None,
form=ConfigurationFormItem(
label="Patron Location Restriction",
description=(
"A code for the library or branch, which, when specified, "
"must exactly match the permanent location for the patron."
"<br>If an ILS does not return a location for its patrons, specifying "
"a value here will always result in authentication failure."
),
),
)


class SIP2AuthenticationProvider(
Expand Down Expand Up @@ -257,7 +244,6 @@ def __init__(
self.ssl_verification = settings.ssl_verification
self.dialect = settings.ils
self.institution_id = library_settings.institution_id
self.patron_location_restriction = library_settings.patron_location_restriction
self._client = client

# Check if patrons should be blocked based on SIP status
Expand Down Expand Up @@ -349,42 +335,8 @@ def remote_authenticate(
# passing it on.
password = None
info = self.patron_information(username, password)
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:
"""Raise an exception if patron location does not match the restriction.
If a location restriction is specified for the library against which the
patron is attempting to authenticate, then the authentication will fail
if either (1) the patron does not have an associated location or (2) the
patron's location does not exactly match the one configured.
"""
if (
not isinstance(info, ProblemDetail)
and self.patron_location_restriction is not None
and self.patron_location_restriction != info.get("permanent_location")
):
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):
sip.connect()
Expand Down
21 changes: 12 additions & 9 deletions tests/manager/api/sip/test_authentication_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
from palace.manager.api.authentication.basic import (
BasicAuthProviderLibrarySettings,
Keyboards,
LibraryIdenfitierRestrictionFields,
LibraryIdentifierRestriction,
)
from palace.manager.api.problem_details import (
INVALID_CREDENTIALS,
Expand Down Expand Up @@ -348,7 +350,9 @@ def test_remote_authenticate_location_restriction(
# This patron authentication library instance is configured with "TestLoc".
library_restriction = "TestLoc"
library_settings = create_library_settings(
patron_location_restriction=library_restriction
library_identifier_restriction_type=LibraryIdentifierRestriction.STRING,
library_identifier_field=LibraryIdenfitierRestrictionFields.PATRON_LIBRARY.value,
library_identifier_restriction_criteria=library_restriction,
)
provider = create_provider(library_settings=library_settings)
client = cast(MockSIPClient, provider.client)
Expand All @@ -357,29 +361,28 @@ def test_remote_authenticate_location_restriction(
client.queue_response(self.evergreen_patron_with_location)
client.queue_response(self.end_session_response)
patrondata = provider.remote_authenticate("user", "pass")
patrondata = provider.enforce_library_identifier_restriction(patrondata)
assert isinstance(patrondata, PatronData)
assert "Patron Name" == patrondata.personal_name

# This patron does NOT have an associated location.
client.queue_response(self.evergreen_patron_wo_location)
client.queue_response(self.end_session_response)
patrondata = provider.remote_authenticate("user", "pass")
with pytest.raises(ProblemDetailException) as exc:
provider.remote_authenticate("user", "pass")
debug_message = provider._location_mismatch_message(None, library_restriction)
provider.enforce_library_identifier_restriction(patrondata)
assert exc.value.problem_detail == PATRON_OF_ANOTHER_LIBRARY.with_debug(
debug_message
"'patron location' does not match library restriction: No value in field."
)

# This patron has the WRONG location.
client.queue_response(self.evergreen_patron_with_wrong_loc)
client.queue_response(self.end_session_response)
patrondata = provider.remote_authenticate("user", "pass")
with pytest.raises(ProblemDetailException) as exc:
provider.remote_authenticate("user", "pass")
debug_message = provider._location_mismatch_message(
"OtherLoc", library_restriction
)
provider.enforce_library_identifier_restriction(patrondata)
assert exc.value.problem_detail == PATRON_OF_ANOTHER_LIBRARY.with_debug(
debug_message
"'patron location' does not match library restriction: 'OtherLoc' does not exactly match 'TestLoc'."
)

def test_encoding(
Expand Down

0 comments on commit 8cb4b3c

Please sign in to comment.