diff --git a/README.md b/README.md index ea0f89dd43..d8565b2973 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,11 @@ Palace Manager is a backend service for digital library systems, maintained by [The Palace Project](https://thepalaceproject.org). +## Documentation + +- [Patron Blocking Rules — Allowed Functions](docs/FUNCTIONS.md) — + Reference for the functions available in patron blocking rule expressions. + ## Installation Docker images created from this code are available at: diff --git a/bin/informational/patron_information b/bin/informational/patron_information index 4350e1a518..a9e1cea491 100755 --- a/bin/informational/patron_information +++ b/bin/informational/patron_information @@ -30,8 +30,8 @@ class PatronInformationScript(LibraryInputScript): auth = LibraryAuthenticator.from_config( _db, args.libraries[0] ).basic_auth_provider - patron_data = auth.remote_authenticate(args.barcode, args.pin) - self.explain(patron_data) + result = auth.remote_authenticate(args.barcode, args.pin) + self.explain(result.patron_data) def explain(self, patron_data): if patron_data is None or patron_data is False: diff --git a/docs/FUNCTIONS.md b/docs/FUNCTIONS.md new file mode 100644 index 0000000000..fe84875d73 --- /dev/null +++ b/docs/FUNCTIONS.md @@ -0,0 +1,135 @@ +# Patron Blocking Rules — Allowed Functions + +Patron blocking rule expressions are evaluated by a locked-down +[simpleeval](https://github.com/danthedeckie/simpleeval) sandbox. +Only the functions listed below may be called inside a rule expression. +Any reference to an unlisted function causes the rule to **fail closed** +(the patron is blocked at runtime; the rule is rejected at admin-save time). + +--- + +## `age_in_years` + +Calculates the age of a person in **whole years** from a date string. +Use this to write rules that gate access by age (e.g. block minors or +enforce senior-only services). + +### Signature + +```text +age_in_years(date_str, fmt=None) -> int +``` + +### Parameters + +| Parameter | Type | Required | +|------------|-----------------|----------| +| `date_str` | `str` | Yes | +| `fmt` | `str` or `None` | No | + +- **`date_str`** — A date string representing the person's date of birth. + ISO 8601 format (`YYYY-MM-DD`) is tried first; if that fails, + `dateutil.parser` is used as a fallback, accepting most common + human-readable formats (e.g. `"Jan 1, 1990"`, `"01/01/1990"`). +- **`fmt`** — An explicit + [`strptime`](https://docs.python.org/3/library/datetime.html#datetime.datetime.strptime) + format string (e.g. `"%d/%m/%Y"`). When supplied, no automatic parsing + is attempted. + +### Returns + +`int` — The person's age in complete years (fractional years are truncated, +not rounded). + +### Raises + +`ValueError` — If `date_str` cannot be parsed (either by ISO 8601, the +supplied `fmt`, or `dateutil`). At runtime this causes the rule to +**fail closed**. + +### Examples + +```python +# Block patrons under 18 (field returned verbatim from the SIP2 server) +age_in_years({polaris_patron_birthdate}) < 18 + +# Block patrons under 18 using an explicit strptime format +age_in_years({dob_field}, "%d/%m/%Y") < 18 + +# Block patrons aged 65 or over (e.g. senior-only restriction) +age_in_years({polaris_patron_birthdate}) >= 65 +``` + +--- + +## `int` + +Converts a value to a Python `int`. Useful when the SIP2 server returns +a numeric field as a string (a common occurrence) and you need to compare +it numerically rather than lexicographically. + +### Signature + +```text +int(value) -> int +``` + +### Parameters + +| Parameter | Type | Required | +|-----------|-------|----------| +| `value` | `Any` | Yes | + +- **`value`** — The value to convert. Typically a string such as `"3"` or + a float such as `2.9`. Any value accepted by Python's built-in `int()` is + valid. Passing a non-numeric string (e.g. `"adult"`) raises a `ValueError` + and causes the rule to **fail closed**. + +### Returns + +`int` — The integer representation of `value`. Floating-point values are +**truncated** toward zero (e.g. `int("2.9")` raises `ValueError`; pass a +float literal or cast via `{field} * 1` first if you need truncation of +floats). + +### Raises + +`ValueError` — If `value` cannot be converted to an integer. At runtime +this causes the rule to **fail closed**. + +### Examples + +```python +# Block patron class codes above 2 (SIP2 returns the code as a string) +int({sipserver_patron_class}) > 2 + +# Block if a numeric expiry-year field indicates an expired account +int({expire_year}) < 2025 +``` + +--- + +## Notes + +- **String methods are available** — methods on Python `str` values can be + called directly on string-valued placeholders. For example, to check + whether a patron identifier starts with a certain prefix: + + ```python + {patron_identifier}.startswith("1234") + ``` + +- **Fail-closed behaviour** — any function call that raises an exception + (e.g. an unparseable date or a non-numeric string passed to `int()`) + causes the patron to be **blocked** at runtime and the rule to be + **rejected** at admin-save time. Write test rules carefully using + representative patron data before enabling them in production. +- **No other builtins** — Python builtins such as `len`, `str`, `float`, + `abs`, and `round` are **not** available. If you need additional + functions, request them via the standard feature-request process so they + can be reviewed and added to `DEFAULT_ALLOWED_FUNCTIONS` in + `rule_engine.py`. +- **Placeholder syntax** — field values from the SIP2 response are + referenced as `{field_name}`. All fields returned by the SIP2 + `patron_information` command are available, plus the normalised `{fines}` + key (a `float` derived from `fee_amount`). diff --git a/poetry.lock b/poetry.lock index 228d3d58e5..6003006ccc 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2063,8 +2063,6 @@ files = [ {file = "greenlet-3.2.4-cp310-cp310-manylinux_2_24_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:c2ca18a03a8cfb5b25bc1cbe20f3d9a4c80d8c3b13ba3df49ac3961af0b1018d"}, {file = "greenlet-3.2.4-cp310-cp310-musllinux_1_1_aarch64.whl", hash = "sha256:9fe0a28a7b952a21e2c062cd5756d34354117796c6d9215a87f55e38d15402c5"}, {file = "greenlet-3.2.4-cp310-cp310-musllinux_1_1_x86_64.whl", hash = "sha256:8854167e06950ca75b898b104b63cc646573aa5fef1353d4508ecdd1ee76254f"}, - {file = "greenlet-3.2.4-cp310-cp310-musllinux_1_2_aarch64.whl", hash = "sha256:f47617f698838ba98f4ff4189aef02e7343952df3a615f847bb575c3feb177a7"}, - {file = "greenlet-3.2.4-cp310-cp310-musllinux_1_2_x86_64.whl", hash = "sha256:af41be48a4f60429d5cad9d22175217805098a9ef7c40bfef44f7669fb9d74d8"}, {file = "greenlet-3.2.4-cp310-cp310-win_amd64.whl", hash = "sha256:73f49b5368b5359d04e18d15828eecc1806033db5233397748f4ca813ff1056c"}, {file = "greenlet-3.2.4-cp311-cp311-macosx_11_0_universal2.whl", hash = "sha256:96378df1de302bc38e99c3a9aa311967b7dc80ced1dcc6f171e99842987882a2"}, {file = "greenlet-3.2.4-cp311-cp311-manylinux2014_aarch64.manylinux_2_17_aarch64.whl", hash = "sha256:1ee8fae0519a337f2329cb78bd7a8e128ec0f881073d43f023c7b8d4831d5246"}, @@ -2074,8 +2072,6 @@ files = [ {file = "greenlet-3.2.4-cp311-cp311-manylinux_2_24_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:2523e5246274f54fdadbce8494458a2ebdcdbc7b802318466ac5606d3cded1f8"}, {file = "greenlet-3.2.4-cp311-cp311-musllinux_1_1_aarch64.whl", hash = "sha256:1987de92fec508535687fb807a5cea1560f6196285a4cde35c100b8cd632cc52"}, {file = "greenlet-3.2.4-cp311-cp311-musllinux_1_1_x86_64.whl", hash = "sha256:55e9c5affaa6775e2c6b67659f3a71684de4c549b3dd9afca3bc773533d284fa"}, - {file = "greenlet-3.2.4-cp311-cp311-musllinux_1_2_aarch64.whl", hash = "sha256:c9c6de1940a7d828635fbd254d69db79e54619f165ee7ce32fda763a9cb6a58c"}, - {file = "greenlet-3.2.4-cp311-cp311-musllinux_1_2_x86_64.whl", hash = "sha256:03c5136e7be905045160b1b9fdca93dd6727b180feeafda6818e6496434ed8c5"}, {file = "greenlet-3.2.4-cp311-cp311-win_amd64.whl", hash = "sha256:9c40adce87eaa9ddb593ccb0fa6a07caf34015a29bf8d344811665b573138db9"}, {file = "greenlet-3.2.4-cp312-cp312-macosx_11_0_universal2.whl", hash = "sha256:3b67ca49f54cede0186854a008109d6ee71f66bd57bb36abd6d0a0267b540cdd"}, {file = "greenlet-3.2.4-cp312-cp312-manylinux2014_aarch64.manylinux_2_17_aarch64.whl", hash = "sha256:ddf9164e7a5b08e9d22511526865780a576f19ddd00d62f8a665949327fde8bb"}, @@ -2085,8 +2081,6 @@ files = [ {file = "greenlet-3.2.4-cp312-cp312-manylinux_2_24_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:3b3812d8d0c9579967815af437d96623f45c0f2ae5f04e366de62a12d83a8fb0"}, {file = "greenlet-3.2.4-cp312-cp312-musllinux_1_1_aarch64.whl", hash = "sha256:abbf57b5a870d30c4675928c37278493044d7c14378350b3aa5d484fa65575f0"}, {file = "greenlet-3.2.4-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:20fb936b4652b6e307b8f347665e2c615540d4b42b3b4c8a321d8286da7e520f"}, - {file = "greenlet-3.2.4-cp312-cp312-musllinux_1_2_aarch64.whl", hash = "sha256:ee7a6ec486883397d70eec05059353b8e83eca9168b9f3f9a361971e77e0bcd0"}, - {file = "greenlet-3.2.4-cp312-cp312-musllinux_1_2_x86_64.whl", hash = "sha256:326d234cbf337c9c3def0676412eb7040a35a768efc92504b947b3e9cfc7543d"}, {file = "greenlet-3.2.4-cp312-cp312-win_amd64.whl", hash = "sha256:a7d4e128405eea3814a12cc2605e0e6aedb4035bf32697f72deca74de4105e02"}, {file = "greenlet-3.2.4-cp313-cp313-macosx_11_0_universal2.whl", hash = "sha256:1a921e542453fe531144e91e1feedf12e07351b1cf6c9e8a3325ea600a715a31"}, {file = "greenlet-3.2.4-cp313-cp313-manylinux2014_aarch64.manylinux_2_17_aarch64.whl", hash = "sha256:cd3c8e693bff0fff6ba55f140bf390fa92c994083f838fece0f63be121334945"}, @@ -2096,8 +2090,6 @@ files = [ {file = "greenlet-3.2.4-cp313-cp313-manylinux_2_24_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:23768528f2911bcd7e475210822ffb5254ed10d71f4028387e5a99b4c6699671"}, {file = "greenlet-3.2.4-cp313-cp313-musllinux_1_1_aarch64.whl", hash = "sha256:00fadb3fedccc447f517ee0d3fd8fe49eae949e1cd0f6a611818f4f6fb7dc83b"}, {file = "greenlet-3.2.4-cp313-cp313-musllinux_1_1_x86_64.whl", hash = "sha256:d25c5091190f2dc0eaa3f950252122edbbadbb682aa7b1ef2f8af0f8c0afefae"}, - {file = "greenlet-3.2.4-cp313-cp313-musllinux_1_2_aarch64.whl", hash = "sha256:6e343822feb58ac4d0a1211bd9399de2b3a04963ddeec21530fc426cc121f19b"}, - {file = "greenlet-3.2.4-cp313-cp313-musllinux_1_2_x86_64.whl", hash = "sha256:ca7f6f1f2649b89ce02f6f229d7c19f680a6238af656f61e0115b24857917929"}, {file = "greenlet-3.2.4-cp313-cp313-win_amd64.whl", hash = "sha256:554b03b6e73aaabec3745364d6239e9e012d64c68ccd0b8430c64ccc14939a8b"}, {file = "greenlet-3.2.4-cp314-cp314-macosx_11_0_universal2.whl", hash = "sha256:49a30d5fda2507ae77be16479bdb62a660fa51b1eb4928b524975b3bde77b3c0"}, {file = "greenlet-3.2.4-cp314-cp314-manylinux2014_aarch64.manylinux_2_17_aarch64.whl", hash = "sha256:299fd615cd8fc86267b47597123e3f43ad79c9d8a22bebdce535e53550763e2f"}, @@ -2105,8 +2097,6 @@ files = [ {file = "greenlet-3.2.4-cp314-cp314-manylinux2014_s390x.manylinux_2_17_s390x.whl", hash = "sha256:b4a1870c51720687af7fa3e7cda6d08d801dae660f75a76f3845b642b4da6ee1"}, {file = "greenlet-3.2.4-cp314-cp314-manylinux2014_x86_64.manylinux_2_17_x86_64.whl", hash = "sha256:061dc4cf2c34852b052a8620d40f36324554bc192be474b9e9770e8c042fd735"}, {file = "greenlet-3.2.4-cp314-cp314-manylinux_2_24_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:44358b9bf66c8576a9f57a590d5f5d6e72fa4228b763d0e43fee6d3b06d3a337"}, - {file = "greenlet-3.2.4-cp314-cp314-musllinux_1_2_aarch64.whl", hash = "sha256:2917bdf657f5859fbf3386b12d68ede4cf1f04c90c3a6bc1f013dd68a22e2269"}, - {file = "greenlet-3.2.4-cp314-cp314-musllinux_1_2_x86_64.whl", hash = "sha256:015d48959d4add5d6c9f6c5210ee3803a830dce46356e3bc326d6776bde54681"}, {file = "greenlet-3.2.4-cp314-cp314-win_amd64.whl", hash = "sha256:e37ab26028f12dbb0ff65f29a8d3d44a765c61e729647bf2ddfbbed621726f01"}, {file = "greenlet-3.2.4-cp39-cp39-macosx_11_0_universal2.whl", hash = "sha256:b6a7c19cf0d2742d0809a4c05975db036fdff50cd294a93632d6a310bf9ac02c"}, {file = "greenlet-3.2.4-cp39-cp39-manylinux2014_aarch64.manylinux_2_17_aarch64.whl", hash = "sha256:27890167f55d2387576d1f41d9487ef171849ea0359ce1510ca6e06c8bece11d"}, @@ -2116,8 +2106,6 @@ files = [ {file = "greenlet-3.2.4-cp39-cp39-manylinux_2_24_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:c9913f1a30e4526f432991f89ae263459b1c64d1608c0d22a5c79c287b3c70df"}, {file = "greenlet-3.2.4-cp39-cp39-musllinux_1_1_aarch64.whl", hash = "sha256:b90654e092f928f110e0007f572007c9727b5265f7632c2fa7415b4689351594"}, {file = "greenlet-3.2.4-cp39-cp39-musllinux_1_1_x86_64.whl", hash = "sha256:81701fd84f26330f0d5f4944d4e92e61afe6319dcd9775e39396e39d7c3e5f98"}, - {file = "greenlet-3.2.4-cp39-cp39-musllinux_1_2_aarch64.whl", hash = "sha256:28a3c6b7cd72a96f61b0e4b2a36f681025b60ae4779cc73c1535eb5f29560b10"}, - {file = "greenlet-3.2.4-cp39-cp39-musllinux_1_2_x86_64.whl", hash = "sha256:52206cd642670b0b320a1fd1cbfd95bca0e043179c1d8a045f2c6109dfe973be"}, {file = "greenlet-3.2.4-cp39-cp39-win32.whl", hash = "sha256:65458b409c1ed459ea899e939f0e1cdb14f58dbc803f2f93c5eab5694d32671b"}, {file = "greenlet-3.2.4-cp39-cp39-win_amd64.whl", hash = "sha256:d2e685ade4dafd447ede19c31277a224a239a0a1a4eca4e6390efedf20260cfb"}, {file = "greenlet-3.2.4.tar.gz", hash = "sha256:0dca0d95ff849f9a364385f36ab49f50065d76964944638be9691e1832e9f86d"}, @@ -3663,10 +3651,8 @@ files = [ {file = "psycopg2_binary-2.9.11-cp310-cp310-manylinux2014_aarch64.manylinux_2_17_aarch64.whl", hash = "sha256:c47676e5b485393f069b4d7a811267d3168ce46f988fa602658b8bb901e9e64d"}, {file = "psycopg2_binary-2.9.11-cp310-cp310-manylinux2014_ppc64le.manylinux_2_17_ppc64le.whl", hash = "sha256:a28d8c01a7b27a1e3265b11250ba7557e5f72b5ee9e5f3a2fa8d2949c29bf5d2"}, {file = "psycopg2_binary-2.9.11-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl", hash = "sha256:5f3f2732cf504a1aa9e9609d02f79bea1067d99edf844ab92c247bbca143303b"}, - {file = "psycopg2_binary-2.9.11-cp310-cp310-manylinux_2_38_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:865f9945ed1b3950d968ec4690ce68c55019d79e4497366d36e090327ce7db14"}, {file = "psycopg2_binary-2.9.11-cp310-cp310-musllinux_1_2_aarch64.whl", hash = "sha256:91537a8df2bde69b1c1db01d6d944c831ca793952e4f57892600e96cee95f2cd"}, {file = "psycopg2_binary-2.9.11-cp310-cp310-musllinux_1_2_ppc64le.whl", hash = "sha256:4dca1f356a67ecb68c81a7bc7809f1569ad9e152ce7fd02c2f2036862ca9f66b"}, - {file = "psycopg2_binary-2.9.11-cp310-cp310-musllinux_1_2_riscv64.whl", hash = "sha256:0da4de5c1ac69d94ed4364b6cbe7190c1a70d325f112ba783d83f8440285f152"}, {file = "psycopg2_binary-2.9.11-cp310-cp310-musllinux_1_2_x86_64.whl", hash = "sha256:37d8412565a7267f7d79e29ab66876e55cb5e8e7b3bbf94f8206f6795f8f7e7e"}, {file = "psycopg2_binary-2.9.11-cp310-cp310-win_amd64.whl", hash = "sha256:c665f01ec8ab273a61c62beeb8cce3014c214429ced8a308ca1fc410ecac3a39"}, {file = "psycopg2_binary-2.9.11-cp311-cp311-macosx_10_9_x86_64.whl", hash = "sha256:0e8480afd62362d0a6a27dd09e4ca2def6fa50ed3a4e7c09165266106b2ffa10"}, @@ -3674,10 +3660,8 @@ files = [ {file = "psycopg2_binary-2.9.11-cp311-cp311-manylinux2014_aarch64.manylinux_2_17_aarch64.whl", hash = "sha256:2e164359396576a3cc701ba8af4751ae68a07235d7a380c631184a611220d9a4"}, {file = "psycopg2_binary-2.9.11-cp311-cp311-manylinux2014_ppc64le.manylinux_2_17_ppc64le.whl", hash = "sha256:d57c9c387660b8893093459738b6abddbb30a7eab058b77b0d0d1c7d521ddfd7"}, {file = "psycopg2_binary-2.9.11-cp311-cp311-manylinux2014_x86_64.manylinux_2_17_x86_64.whl", hash = "sha256:2c226ef95eb2250974bf6fa7a842082b31f68385c4f3268370e3f3870e7859ee"}, - {file = "psycopg2_binary-2.9.11-cp311-cp311-manylinux_2_38_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:a311f1edc9967723d3511ea7d2708e2c3592e3405677bf53d5c7246753591fbb"}, {file = "psycopg2_binary-2.9.11-cp311-cp311-musllinux_1_2_aarch64.whl", hash = "sha256:ebb415404821b6d1c47353ebe9c8645967a5235e6d88f914147e7fd411419e6f"}, {file = "psycopg2_binary-2.9.11-cp311-cp311-musllinux_1_2_ppc64le.whl", hash = "sha256:f07c9c4a5093258a03b28fab9b4f151aa376989e7f35f855088234e656ee6a94"}, - {file = "psycopg2_binary-2.9.11-cp311-cp311-musllinux_1_2_riscv64.whl", hash = "sha256:00ce1830d971f43b667abe4a56e42c1e2d594b32da4802e44a73bacacb25535f"}, {file = "psycopg2_binary-2.9.11-cp311-cp311-musllinux_1_2_x86_64.whl", hash = "sha256:cffe9d7697ae7456649617e8bb8d7a45afb71cd13f7ab22af3e5c61f04840908"}, {file = "psycopg2_binary-2.9.11-cp311-cp311-win_amd64.whl", hash = "sha256:304fd7b7f97eef30e91b8f7e720b3db75fee010b520e434ea35ed1ff22501d03"}, {file = "psycopg2_binary-2.9.11-cp312-cp312-macosx_10_13_x86_64.whl", hash = "sha256:be9b840ac0525a283a96b556616f5b4820e0526addb8dcf6525a0fa162730be4"}, @@ -3685,10 +3669,8 @@ files = [ {file = "psycopg2_binary-2.9.11-cp312-cp312-manylinux2014_aarch64.manylinux_2_17_aarch64.whl", hash = "sha256:ab8905b5dcb05bf3fb22e0cf90e10f469563486ffb6a96569e51f897c750a76a"}, {file = "psycopg2_binary-2.9.11-cp312-cp312-manylinux2014_ppc64le.manylinux_2_17_ppc64le.whl", hash = "sha256:bf940cd7e7fec19181fdbc29d76911741153d51cab52e5c21165f3262125685e"}, {file = "psycopg2_binary-2.9.11-cp312-cp312-manylinux2014_x86_64.manylinux_2_17_x86_64.whl", hash = "sha256:fa0f693d3c68ae925966f0b14b8edda71696608039f4ed61b1fe9ffa468d16db"}, - {file = "psycopg2_binary-2.9.11-cp312-cp312-manylinux_2_38_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:a1cf393f1cdaf6a9b57c0a719a1068ba1069f022a59b8b1fe44b006745b59757"}, {file = "psycopg2_binary-2.9.11-cp312-cp312-musllinux_1_2_aarch64.whl", hash = "sha256:ef7a6beb4beaa62f88592ccc65df20328029d721db309cb3250b0aae0fa146c3"}, {file = "psycopg2_binary-2.9.11-cp312-cp312-musllinux_1_2_ppc64le.whl", hash = "sha256:31b32c457a6025e74d233957cc9736742ac5a6cb196c6b68499f6bb51390bd6a"}, - {file = "psycopg2_binary-2.9.11-cp312-cp312-musllinux_1_2_riscv64.whl", hash = "sha256:edcb3aeb11cb4bf13a2af3c53a15b3d612edeb6409047ea0b5d6a21a9d744b34"}, {file = "psycopg2_binary-2.9.11-cp312-cp312-musllinux_1_2_x86_64.whl", hash = "sha256:62b6d93d7c0b61a1dd6197d208ab613eb7dcfdcca0a49c42ceb082257991de9d"}, {file = "psycopg2_binary-2.9.11-cp312-cp312-win_amd64.whl", hash = "sha256:b33fabeb1fde21180479b2d4667e994de7bbf0eec22832ba5d9b5e4cf65b6c6d"}, {file = "psycopg2_binary-2.9.11-cp313-cp313-macosx_10_13_x86_64.whl", hash = "sha256:b8fb3db325435d34235b044b199e56cdf9ff41223a4b9752e8576465170bb38c"}, @@ -3696,10 +3678,8 @@ files = [ {file = "psycopg2_binary-2.9.11-cp313-cp313-manylinux2014_aarch64.manylinux_2_17_aarch64.whl", hash = "sha256:8c55b385daa2f92cb64b12ec4536c66954ac53654c7f15a203578da4e78105c0"}, {file = "psycopg2_binary-2.9.11-cp313-cp313-manylinux2014_ppc64le.manylinux_2_17_ppc64le.whl", hash = "sha256:c0377174bf1dd416993d16edc15357f6eb17ac998244cca19bc67cdc0e2e5766"}, {file = "psycopg2_binary-2.9.11-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.whl", hash = "sha256:5c6ff3335ce08c75afaed19e08699e8aacf95d4a260b495a4a8545244fe2ceb3"}, - {file = "psycopg2_binary-2.9.11-cp313-cp313-manylinux_2_38_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:84011ba3109e06ac412f95399b704d3d6950e386b7994475b231cf61eec2fc1f"}, {file = "psycopg2_binary-2.9.11-cp313-cp313-musllinux_1_2_aarch64.whl", hash = "sha256:ba34475ceb08cccbdd98f6b46916917ae6eeb92b5ae111df10b544c3a4621dc4"}, {file = "psycopg2_binary-2.9.11-cp313-cp313-musllinux_1_2_ppc64le.whl", hash = "sha256:b31e90fdd0f968c2de3b26ab014314fe814225b6c324f770952f7d38abf17e3c"}, - {file = "psycopg2_binary-2.9.11-cp313-cp313-musllinux_1_2_riscv64.whl", hash = "sha256:d526864e0f67f74937a8fce859bd56c979f5e2ec57ca7c627f5f1071ef7fee60"}, {file = "psycopg2_binary-2.9.11-cp313-cp313-musllinux_1_2_x86_64.whl", hash = "sha256:04195548662fa544626c8ea0f06561eb6203f1984ba5b4562764fbeb4c3d14b1"}, {file = "psycopg2_binary-2.9.11-cp313-cp313-win_amd64.whl", hash = "sha256:efff12b432179443f54e230fdf60de1f6cc726b6c832db8701227d089310e8aa"}, {file = "psycopg2_binary-2.9.11-cp314-cp314-macosx_10_13_x86_64.whl", hash = "sha256:92e3b669236327083a2e33ccfa0d320dd01b9803b3e14dd986a4fc54aa00f4e1"}, @@ -3707,10 +3687,8 @@ files = [ {file = "psycopg2_binary-2.9.11-cp314-cp314-manylinux2014_aarch64.manylinux_2_17_aarch64.whl", hash = "sha256:9b52a3f9bb540a3e4ec0f6ba6d31339727b2950c9772850d6545b7eae0b9d7c5"}, {file = "psycopg2_binary-2.9.11-cp314-cp314-manylinux2014_ppc64le.manylinux_2_17_ppc64le.whl", hash = "sha256:db4fd476874ccfdbb630a54426964959e58da4c61c9feba73e6094d51303d7d8"}, {file = "psycopg2_binary-2.9.11-cp314-cp314-manylinux2014_x86_64.manylinux_2_17_x86_64.whl", hash = "sha256:47f212c1d3be608a12937cc131bd85502954398aaa1320cb4c14421a0ffccf4c"}, - {file = "psycopg2_binary-2.9.11-cp314-cp314-manylinux_2_38_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:e35b7abae2b0adab776add56111df1735ccc71406e56203515e228a8dc07089f"}, {file = "psycopg2_binary-2.9.11-cp314-cp314-musllinux_1_2_aarch64.whl", hash = "sha256:fcf21be3ce5f5659daefd2b3b3b6e4727b028221ddc94e6c1523425579664747"}, {file = "psycopg2_binary-2.9.11-cp314-cp314-musllinux_1_2_ppc64le.whl", hash = "sha256:9bd81e64e8de111237737b29d68039b9c813bdf520156af36d26819c9a979e5f"}, - {file = "psycopg2_binary-2.9.11-cp314-cp314-musllinux_1_2_riscv64.whl", hash = "sha256:32770a4d666fbdafab017086655bcddab791d7cb260a16679cc5a7338b64343b"}, {file = "psycopg2_binary-2.9.11-cp314-cp314-musllinux_1_2_x86_64.whl", hash = "sha256:c3cb3a676873d7506825221045bd70e0427c905b9c8ee8d6acd70cfcbd6e576d"}, {file = "psycopg2_binary-2.9.11-cp314-cp314-win_amd64.whl", hash = "sha256:4012c9c954dfaccd28f94e84ab9f94e12df76b4afb22331b1f0d3154893a6316"}, {file = "psycopg2_binary-2.9.11-cp39-cp39-macosx_10_9_x86_64.whl", hash = "sha256:20e7fb94e20b03dcc783f76c0865f9da39559dcc0c28dd1a3fce0d01902a6b9c"}, @@ -3718,10 +3696,8 @@ files = [ {file = "psycopg2_binary-2.9.11-cp39-cp39-manylinux2014_aarch64.manylinux_2_17_aarch64.whl", hash = "sha256:9d3a9edcfbe77a3ed4bc72836d466dfce4174beb79eda79ea155cc77237ed9e8"}, {file = "psycopg2_binary-2.9.11-cp39-cp39-manylinux2014_ppc64le.manylinux_2_17_ppc64le.whl", hash = "sha256:44fc5c2b8fa871ce7f0023f619f1349a0aa03a0857f2c96fbc01c657dcbbdb49"}, {file = "psycopg2_binary-2.9.11-cp39-cp39-manylinux2014_x86_64.manylinux_2_17_x86_64.whl", hash = "sha256:9c55460033867b4622cda1b6872edf445809535144152e5d14941ef591980edf"}, - {file = "psycopg2_binary-2.9.11-cp39-cp39-manylinux_2_38_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:2d11098a83cca92deaeaed3d58cfd150d49b3b06ee0d0852be466bf87596899e"}, {file = "psycopg2_binary-2.9.11-cp39-cp39-musllinux_1_2_aarch64.whl", hash = "sha256:691c807d94aecfbc76a14e1408847d59ff5b5906a04a23e12a89007672b9e819"}, {file = "psycopg2_binary-2.9.11-cp39-cp39-musllinux_1_2_ppc64le.whl", hash = "sha256:8b81627b691f29c4c30a8f322546ad039c40c328373b11dff7490a3e1b517855"}, - {file = "psycopg2_binary-2.9.11-cp39-cp39-musllinux_1_2_riscv64.whl", hash = "sha256:b637d6d941209e8d96a072d7977238eea128046effbf37d1d8b2c0764750017d"}, {file = "psycopg2_binary-2.9.11-cp39-cp39-musllinux_1_2_x86_64.whl", hash = "sha256:41360b01c140c2a03d346cec3280cf8a71aa07d94f3b1509fa0161c366af66b4"}, {file = "psycopg2_binary-2.9.11-cp39-cp39-win_amd64.whl", hash = "sha256:875039274f8a2361e5207857899706da840768e2a775bf8c65e82f60b197df02"}, ] @@ -5163,6 +5139,18 @@ files = [ {file = "sgmllib3k-1.0.0.tar.gz", hash = "sha256:7868fb1c8bfa764c1ac563d3cf369c381d1325d36124933a726f29fcdaa812e9"}, ] +[[package]] +name = "simpleeval" +version = "1.0.3" +description = "A simple, safe single expression evaluator library." +optional = false +python-versions = ">=3.9" +groups = ["main"] +files = [ + {file = "simpleeval-1.0.3-py3-none-any.whl", hash = "sha256:e3bdbb8c82c26297c9a153902d0fd1858a6c3774bf53ff4f134788c3f2035c38"}, + {file = "simpleeval-1.0.3.tar.gz", hash = "sha256:67bbf246040ac3b57c29cf048657b9cf31d4e7b9d6659684daa08ca8f1e45829"}, +] + [[package]] name = "six" version = "1.17.0" @@ -5993,4 +5981,4 @@ lxml = ">=3.8" [metadata] lock-version = "2.1" python-versions = ">=3.12,<4" -content-hash = "dad42c175162339ff5c8549891773f022e2abd0b49e14282dff06b324c2bd51c" +content-hash = "9deb8cf9b3f6595fb0d4b8e53e790f8f999db3c88265e8c5fe5e305b570fa147" diff --git a/pyproject.toml b/pyproject.toml index 1d0548f19d..c9709bc587 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -212,6 +212,7 @@ module = [ # contains a couple version strings it can be safely ignored "palace.manager._version", "pyld", + "simpleeval", "textblob.*", "unicodecsv", "uwsgi", @@ -284,6 +285,7 @@ pyyaml = "^6.0" redis = "^6.4.0" redmail = "^0.6.0" requests = "^2.29" +simpleeval = "^1.0.3" sqlalchemy = {version = "^1.4", extras = ["mypy"]} tenacity = "^9.0.0" textblob = "0.19.0" diff --git a/src/palace/manager/api/admin/controller/patron_auth_services.py b/src/palace/manager/api/admin/controller/patron_auth_services.py index a652f591d3..9fc3587c29 100644 --- a/src/palace/manager/api/admin/controller/patron_auth_services.py +++ b/src/palace/manager/api/admin/controller/patron_auth_services.py @@ -1,5 +1,6 @@ from __future__ import annotations +from collections.abc import Callable from typing import Any import flask @@ -13,10 +14,16 @@ from palace.manager.api.admin.form_data import ProcessFormData from palace.manager.api.admin.problem_details import ( FAILED_TO_RUN_SELF_TESTS, + INVALID_CONFIGURATION_OPTION, MULTIPLE_BASIC_AUTH_SERVICES, ) from palace.manager.api.authentication.base import AuthenticationProviderType from palace.manager.api.authentication.basic import BasicAuthenticationProvider +from palace.manager.api.authentication.patron_blocking_rules.rule_engine import ( + RuleValidationError, + get_evaluator, + validate_rule_expression, +) from palace.manager.integration.goals import Goals from palace.manager.integration.settings import BaseSettings from palace.manager.sqlalchemy.listeners import site_configuration_has_changed @@ -90,8 +97,11 @@ def process_post(self) -> Response | ProblemDetail: def library_integration_validation( self, integration: IntegrationLibraryConfiguration ) -> None: - """Check that the library didn't end up with multiple basic auth services.""" + """Validate a library integration after its settings have been saved. + Ensures the library does not end up with more than one basic-auth + patron authentication service. + """ library = integration.library basic_auth_integrations = ( self._db.query(IntegrationConfiguration) @@ -120,6 +130,63 @@ def process_updated_libraries( for integration, _ in libraries: self.library_integration_validation(integration) + def process_validate_patron_blocking_rule(self) -> Response | ProblemDetail: + """Validate a single patron blocking rule expression against live ILS data. + + Loads the saved service by ID, makes a live patron_information() call + using the configured test_identifier/test_password via + fetch_live_rule_validation_values, then evaluates the rule expression + against the real values returned. Only parse/eval success or failure + is reported — the boolean result (blocked vs. not blocked) is discarded. + """ + self.require_system_admin() + try: + form_data = flask.request.form + service_id_str = form_data.get("service_id", "") + rule_expr = form_data.get("rule", "") + + if not service_id_str: + return INVALID_CONFIGURATION_OPTION.detailed("service_id is required.") + + try: + service_id = int(service_id_str) + except ValueError: + return INVALID_CONFIGURATION_OPTION.detailed( + "service_id must be an integer." + ) + + integration = self._db.get(IntegrationConfiguration, service_id) + if integration is None: + return INVALID_CONFIGURATION_OPTION.detailed( + "Patron auth service not found. Save the service before validating rules." + ) + + protocol_class = self.get_protocol_class(integration.protocol) + if not getattr(protocol_class, "supports_patron_blocking_rules", False): + return INVALID_CONFIGURATION_OPTION.detailed( + "Rule validation is only supported for authentication " + "services that support patron blocking rules." + ) + + settings = protocol_class.settings_load(integration) + # fetch_live_rule_validation_values raises ProblemDetailException on + # missing test_identifier, network error, or SIP2 error response. + fetch_values: Callable[[Any], dict[str, Any]] = getattr( + protocol_class, "fetch_live_rule_validation_values", lambda s: {} + ) + live_values = fetch_values(settings) + + evaluator = get_evaluator() + try: + validate_rule_expression(rule_expr, live_values, evaluator) + except RuleValidationError as exc: + return INVALID_CONFIGURATION_OPTION.detailed(exc.message) + + except ProblemDetailException as e: + return e.problem_detail + + return Response(status=200) + def process_delete(self, service_id: int) -> Response | ProblemDetail: self.require_system_admin() try: diff --git a/src/palace/manager/api/admin/routes.py b/src/palace/manager/api/admin/routes.py index 289397bd4a..b648f89667 100644 --- a/src/palace/manager/api/admin/routes.py +++ b/src/palace/manager/api/admin/routes.py @@ -495,6 +495,16 @@ def patron_auth_self_tests(identifier): ) +@app.route("/admin/patron_auth_service_validate_patron_blocking_rule", methods=["POST"]) +@returns_json_or_response_or_problem_detail +@requires_admin +@requires_csrf_token +def patron_auth_service_validate_patron_blocking_rule(): + return ( + app.manager.admin_patron_auth_services_controller.process_validate_patron_blocking_rule() + ) + + @library_route("/admin/manage_patrons", methods=["POST"]) @has_library @returns_json_or_response_or_problem_detail diff --git a/src/palace/manager/api/authentication/basic.py b/src/palace/manager/api/authentication/basic.py index cb03cabd48..63c4fca24d 100644 --- a/src/palace/manager/api/authentication/basic.py +++ b/src/palace/manager/api/authentication/basic.py @@ -3,10 +3,11 @@ import re from abc import ABC, abstractmethod from collections.abc import Generator +from dataclasses import dataclass, field from enum import Enum from functools import partial from re import Pattern -from typing import Annotated, Any, cast +from typing import Annotated, Any, ClassVar, cast from flask import url_for from pydantic import PositiveInt, field_validator @@ -15,6 +16,7 @@ from werkzeug.datastructures import Authorization from palace.manager.api.admin.problem_details import ( + INVALID_CONFIGURATION_OPTION, INVALID_LIBRARY_IDENTIFIER_RESTRICTION_REGULAR_EXPRESSION, ) from palace.manager.api.authentication.base import ( @@ -25,6 +27,11 @@ PatronData, PatronLookupNotSupported, ) +from palace.manager.api.authentication.patron_blocking_rules.rule_engine import ( + MAX_RULE_LENGTH, + RuleValidationError, + validate_message, +) from palace.manager.api.problem_details import ( PATRON_OF_ANOTHER_LIBRARY, UNSUPPORTED_AUTHENTICATION_MECHANISM, @@ -33,6 +40,11 @@ from palace.manager.core.config import CannotLoadConfiguration from palace.manager.core.exceptions import IntegrationException from palace.manager.core.selftest import SelfTestResult +from palace.manager.integration.patron_auth.patron_blocking import ( + PatronBlockingRule, + build_runtime_values_from_patron, + check_patron_blocking_rules_with_evaluator, +) from palace.manager.integration.settings import ( FormFieldType, FormMetadata, @@ -44,6 +56,12 @@ from palace.manager.util.problem_detail import ProblemDetail, ProblemDetailException +@dataclass +class RemoteAuthResult: + patron_data: PatronData | ProblemDetail | None + extra_context: dict[str, Any] = field(default_factory=dict) + + class LibraryIdentifierRestriction(Enum): NONE = "none" REGEX = "regex" @@ -191,6 +209,11 @@ class BasicAuthProviderSettings(AuthProviderSettings): class BasicAuthProviderLibrarySettings(AuthProviderLibrarySettings): + """Library-scoped settings for basic auth providers.""" + + # Subclasses that support patron blocking rules (e.g. SIP2) override to True. + supports_patron_blocking_rules: ClassVar[bool] = False + # When multiple libraries share an ILS, a person may be able to # authenticate with the ILS but not be considered a patron of # _this_ library. This setting contains the rule for determining @@ -250,6 +273,19 @@ class BasicAuthProviderLibrarySettings(AuthProviderLibrarySettings): ), ] = None + patron_blocking_rules: Annotated[ + list[PatronBlockingRule], + FormMetadata( + label="Patron Blocking Rules", + description=( + "A list of rules that can block patron access after successful ILS " + "authentication. Each rule has a name, a rule expression, and an " + "optional message shown to the patron when blocked." + ), + hidden=True, + ), + ] = [] + @field_validator("library_identifier_restriction_criteria") @classmethod def validate_restriction_criteria( @@ -269,6 +305,74 @@ def validate_restriction_criteria( ) return restriction_criteria + @field_validator("patron_blocking_rules") + @classmethod + def validate_patron_blocking_rules( + cls, rules: list[PatronBlockingRule] + ) -> list[PatronBlockingRule]: + """Validate patron blocking rules: non-empty name/rule, no duplicate names, + rule length <= 1000, and message length <= 1000. + + Rejects rules when this settings class does not support blocking rules + (supports_patron_blocking_rules is False). + + Full expression validation (syntax, placeholder resolution, bool result) + is deferred to admin-save time via a live SIP2 call in + ``PatronAuthServicesController.library_integration_validation``, where + real patron values are available. + """ + if rules and not cls.supports_patron_blocking_rules: + raise SettingsValidationError( + INVALID_CONFIGURATION_OPTION.detailed( + "Patron blocking rules are not supported by this authentication " + "provider. Rules are ignored at runtime." + ) + ) + + names_seen: set[str] = set() + for i, rule in enumerate(rules): + if not rule.name: + raise SettingsValidationError( + INVALID_CONFIGURATION_OPTION.detailed( + f"Rule at index {i}: 'name' must not be empty" + ) + ) + if not rule.rule: + raise SettingsValidationError( + INVALID_CONFIGURATION_OPTION.detailed( + f"Rule at index {i}: 'rule' expression must not be empty" + ) + ) + if rule.name in names_seen: + raise SettingsValidationError( + INVALID_CONFIGURATION_OPTION.detailed( + f"Rule at index {i}: duplicate rule name '{rule.name}'" + ) + ) + names_seen.add(rule.name) + + # Enforce rule length limit. + if len(rule.rule) > MAX_RULE_LENGTH: + raise SettingsValidationError( + INVALID_CONFIGURATION_OPTION.detailed( + f"Rule at index {i} ('{rule.name}'): rule expression must not " + f"exceed {MAX_RULE_LENGTH} characters." + ) + ) + + # Validate optional message when provided. + if rule.message is not None: + try: + validate_message(rule.message) + except RuleValidationError as exc: + raise SettingsValidationError( + INVALID_CONFIGURATION_OPTION.detailed( + f"Rule at index {i} ('{rule.name}'): {exc.message}" + ) + ) from exc + + return rules + class BasicAuthenticationProvider[ SettingsType: BasicAuthProviderSettings, @@ -278,6 +382,12 @@ class BasicAuthenticationProvider[ a remote source of truth. """ + # Subclasses that connect to an ILS and wish to honour patron blocking rules + # should override this to True. Non-ILS providers (e.g. SimpleAuthentication, + # MinimalAuthentication) leave it as False so that the blocking-rules check + # in authenticate() is skipped entirely for those providers. + supports_patron_blocking_rules: ClassVar[bool] = False + def __init__( self, library_id: int, @@ -315,6 +425,7 @@ def __init__( library_settings.library_identifier_restriction_criteria ) ) + self.patron_blocking_rules = library_settings.patron_blocking_rules def process_library_identifier_restriction_criteria( self, criteria: str | None @@ -354,17 +465,22 @@ def flow_type(self) -> str: @abstractmethod def remote_authenticate( self, username: str, password: str | None - ) -> PatronData | ProblemDetail | None: + ) -> RemoteAuthResult: """Does the source of truth approve of these credentials? - If the credentials are valid, return a PatronData object. The PatronData object - has a `complete` field. This field on the returned PatronData object will be used - to determine if we need to call `remote_patron_lookup` later to get the complete + If the credentials are valid, return a RemoteAuthResult with patron_data + set to a PatronData object. The PatronData object has a `complete` field. + This field on the returned PatronData object will be used to determine + if we need to call `remote_patron_lookup` later to get the complete information about the patron. - If the credentials are invalid, return None. + If the credentials are invalid, return RemoteAuthResult(patron_data=None). - If there is a problem communicating with the remote, return a ProblemDetail. + If there is a problem communicating with the remote, return + RemoteAuthResult(patron_data=ProblemDetail). + + Providers with richer upstream data (e.g. SIP2) may populate + extra_context for use in patron blocking rule evaluation. """ ... @@ -494,15 +610,68 @@ def scrub_credential(self, value: str | None) -> str | None: return value return value.strip() + def _build_blocking_rule_values( + self, patron: Patron, extra_context: dict[str, Any] + ) -> dict[str, Any]: + """Build the values dict used when evaluating patron blocking rules. + + The base implementation extracts the small set of values available on + the :class:`~palace.manager.sqlalchemy.model.patron.Patron` DB model + (``fines``, ``patron_type``). + + Provider subclasses that have access to richer upstream data (e.g. the + full SIP2 response dict) should override this to return a more complete + mapping, using data from extra_context when available. + + :param patron: The successfully authenticated patron. + :param extra_context: Provider-specific data from :meth:`remote_authenticate` + (e.g. raw SIP2 response dict). Empty for providers that do not populate it. + :returns: Dict mapping placeholder key to resolved value for + :func:`~palace.manager.integration.patron_auth.patron_blocking + .check_patron_blocking_rules_with_evaluator`. + """ + return build_runtime_values_from_patron(patron) + def authenticate( self, _db: Session, credentials: dict ) -> Patron | ProblemDetail | None: """Turn a set of credentials into a Patron object. + After the ILS successfully authenticates the patron, any configured + patron_blocking_rules are evaluated. If a rule triggers a block, + a ProblemDetail is returned instead of the Patron object. + :param credentials: A dictionary with keys `username` and `password`. :return: A Patron if one can be authenticated; a ProblemDetail - if an error occurs; None if the credentials are missing or wrong. + if an error occurs or a blocking rule fires; None if the + credentials are missing or wrong. + """ + result, extra_context = self._do_authenticate(_db, credentials) + if ( + self.supports_patron_blocking_rules + and self.patron_blocking_rules + and isinstance(result, Patron) + ): + # NOTE: This log is used by CloudWatch to count total patron blocking + # evaluations for error-rate calculation. Do not remove. + self.log.info("Patron blocking rules evaluation attempted") + values = self._build_blocking_rule_values(result, extra_context) + blocked = check_patron_blocking_rules_with_evaluator( + self.patron_blocking_rules, values, log=self.log + ) + if blocked is not None: + return blocked + return result + + def _do_authenticate( + self, _db: Session, credentials: dict + ) -> tuple[Patron | ProblemDetail | None, dict[str, Any]]: + """Core authentication logic (ILS round-trip + local patron lookup/create). + + Separated from :meth:`authenticate` so that the blocking-rules hook + in that method has a single, clean intercept point regardless of + which code path inside this method resolves the patron. """ username = self.scrub_credential(credentials.get("username")) or "" password = self.scrub_credential(credentials.get("password")) @@ -513,13 +682,15 @@ def authenticate( username, result.details, ) - return None + return None, {} # Check these credentials with the source of truth. - patrondata = self.remote_authenticate(username, password) + auth_result = self.remote_authenticate(username, password) + patrondata = auth_result.patron_data + extra_context = auth_result.extra_context if patrondata is None or isinstance(patrondata, ProblemDetail): # Either an error occurred or the credentials did not correspond to any patron. - return patrondata + return patrondata, extra_context # Check that the patron belongs to this library. patrondata = self.enforce_library_identifier_restriction(patrondata) @@ -542,11 +713,11 @@ def authenticate( if not isinstance(patrondata, PatronData): # Something went wrong, we can't get the patron's information. # so we fail the authentication process. - return patrondata + return patrondata, extra_context # Apply the information we have to the patron and return it. patrondata.apply(patron) - return patron + return patron, extra_context # At this point we didn't find the patron, so we want to look up the patron # with the remote, in case this allows us to find an existing patron, based @@ -556,18 +727,18 @@ def authenticate( if not isinstance(patrondata, PatronData): # Something went wrong, we can't get the patron's information. # so we fail the authentication process. - return patrondata + return patrondata, extra_context patron = self.local_patron_lookup(_db, username, patrondata) if patron: # We found the patron, so we apply the information we have to the patron and return it. patrondata.apply(patron) - return patron + return patron, extra_context # We didn't find the patron, so we create a new patron with the information we have. patron, _ = patrondata.get_or_create_patron( _db, self.library_id, analytics=self.analytics ) - return patron + return patron, extra_context def get_credential_from_header(self, auth: Authorization) -> str | None: """Extract a password credential from a WWW-Authenticate header diff --git a/src/palace/manager/api/authentication/patron_blocking_rules/__init__.py b/src/palace/manager/api/authentication/patron_blocking_rules/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/palace/manager/api/authentication/patron_blocking_rules/rule_engine.py b/src/palace/manager/api/authentication/patron_blocking_rules/rule_engine.py new file mode 100644 index 0000000000..76fd3a7837 --- /dev/null +++ b/src/palace/manager/api/authentication/patron_blocking_rules/rule_engine.py @@ -0,0 +1,392 @@ +from __future__ import annotations + +import re +import threading +from collections.abc import Callable, Mapping +from dataclasses import dataclass, field +from datetime import date +from typing import Any + +from simpleeval import ( + EvalWithCompoundTypes, + FunctionNotDefined, + NameNotDefined, +) + +from palace.manager.core.exceptions import BasePalaceException + +MAX_RULE_LENGTH = 1000 +MAX_MESSAGE_LENGTH = 1000 + +# Placeholder keys: alphanumeric + underscore, e.g. {patron_type}, {fines} +_PLACEHOLDER_RE = re.compile(r"\{([A-Za-z0-9_]+)\}") + +# Variable name prefix used when compiling placeholders to safe identifiers. +_VAR_PREFIX = "__v_" + + +def _format_available_keys(available: Mapping[str, Any]) -> str: + """Return a sorted, human-readable listing of available keys and values.""" + if not available: + return "(none)" + return ", ".join( + f"{k}={v!r}" for k, v in sorted(available.items(), key=lambda kv: kv[0]) + ) + + +def _format_allowed_functions(functions: Mapping[str, Callable[..., Any]]) -> str: + """Return a sorted, human-readable list of allowed function names.""" + if not functions: + return "(none)" + return ", ".join(sorted(functions.keys())) + + +class RuleValidationError(BasePalaceException): + """Raised when a patron blocking rule fails admin-save validation.""" + + message: str + + def __init__(self, message: str) -> None: + super().__init__(message) + + +class MissingPlaceholderError(BasePalaceException): + """Raised when a required placeholder key is absent from the values dict. + + :ivar key: The name of the missing placeholder (without braces). + :ivar available: The keys (and their values) that *were* available at the + time the error was raised. Included in the error message so that + operators can identify the correct field name to use. + """ + + def __init__(self, key: str, available: Mapping[str, Any] | None = None) -> None: + avail = available or {} + available_str = _format_available_keys(avail) + msg = ( + f"Placeholder {{{key}}} is not available. " + f"Available fields: {available_str}" + ) + super().__init__(msg) + self.key = key + self.available: dict[str, Any] = dict(avail) + + +class RuleEvaluationError(BasePalaceException): + """Raised at runtime when a rule cannot be evaluated safely. + + All callers should treat this as a block (fail-closed). + """ + + def __init__(self, message: str, rule_name: str | None = None) -> None: + super().__init__(message) + self.rule_name = rule_name + + +@dataclass(frozen=True) +class CompiledRule: + """The result of compiling a rule expression. + + :ivar original: The original rule expression string (with {key} placeholders). + :ivar compiled: The expression with placeholders replaced by safe variable + names (e.g. ``__v_key``), ready for simpleeval. + :ivar var_map: Mapping from original placeholder key to its safe variable name. + """ + + original: str + compiled: str + var_map: dict[str, str] = field(default_factory=dict) + + +def compile_rule_expression(expr: str) -> CompiledRule: + """Compile a rule expression by replacing ``{key}`` placeholders with safe + variable identifiers. + + The resulting expression string is suitable for passing to simpleeval. + Placeholder values are injected via the ``names`` dict (see + :func:`build_names`). + + :param expr: The raw rule expression, e.g. ``"fines > 10.0"``. + :returns: A :class:`CompiledRule` instance. + """ + var_map: dict[str, str] = {} + + def _replace(m: re.Match[str]) -> str: + key = m.group(1) + var_name = f"{_VAR_PREFIX}{key}" + var_map[key] = var_name + return var_name + + compiled = _PLACEHOLDER_RE.sub(_replace, expr) + return CompiledRule(original=expr, compiled=compiled, var_map=var_map) + + +def build_names(compiled: CompiledRule, values: Mapping[str, Any]) -> dict[str, Any]: + """Build the simpleeval ``names`` dict for a compiled rule. + + Raises :class:`MissingPlaceholderError` if any placeholder referenced in + *compiled* is absent from *values*. The error message includes a listing + of all available keys and their values to help operators fix the rule. + + :param compiled: A :class:`CompiledRule` produced by + :func:`compile_rule_expression`. + :param values: Mapping of placeholder key to concrete value, e.g. + ``{"fines": 5.0, "sipserver_patron_class": "adult"}``. + :returns: Dict mapping safe variable names to their values, ready to pass + as ``evaluator.names``. + :raises MissingPlaceholderError: If a required key is absent from *values*. + """ + names: dict[str, Any] = {} + for key, var_name in compiled.var_map.items(): + if key not in values: + raise MissingPlaceholderError(key, available=values) + names[var_name] = values[key] + return names + + +def age_in_years( + date_str: str, + fmt: str | None = None, + *, + today: date | None = None, +) -> int: + """Return the age in whole years relative to *today*. + + :param date_str: A date string to parse. + :param fmt: Optional :func:`~datetime.datetime.strptime` format string. When + omitted the function attempts ISO 8601 parsing first, then falls back to + ``dateutil.parser`` if available. + :param today: Keyword-only override for the reference date; defaults to + :func:`datetime.date.today`. Intended for deterministic tests. + :returns: Age in whole years (floor). + :raises ValueError: If *date_str* cannot be parsed. + """ + from datetime import datetime + + ref = today if today is not None else date.today() + + birth: date + if fmt is not None: + birth = datetime.strptime(date_str, fmt).date() + else: + # Try ISO 8601 first. + try: + birth = date.fromisoformat(date_str) + except ValueError: + # Fall back to dateutil if available. + try: + from dateutil import parser as _dateutil_parser + + birth = _dateutil_parser.parse(date_str).date() + except Exception: + raise ValueError(f"Cannot parse date string: {date_str!r}") from None + + years = ref.year - birth.year + if (ref.month, ref.day) < (birth.month, birth.day): + years -= 1 + return years + + +#: Default set of functions available in rule expressions. +DEFAULT_ALLOWED_FUNCTIONS: dict[str, Callable[..., Any]] = { + "age_in_years": age_in_years, + "int": int, +} + + +_evaluator_local: threading.local = threading.local() + + +def get_evaluator( + allowed_functions: dict[str, Callable[..., Any]] | None = None, +) -> EvalWithCompoundTypes: + """Return a per-thread evaluator for rule expression evaluation. + + Admin validation and runtime blocking both use this so the evaluator + lifecycle is unified. The evaluator is cached per thread and reused; + :func:`validate_rule_expression` and :func:`evaluate_rule_expression_strict_bool` + clear ``evaluator.names`` after each call so no data leaks between rules. + + :param allowed_functions: Override the function whitelist. Pass an empty + dict to disallow all functions. Only the first call per thread uses + this; subsequent calls ignore it. + :returns: A configured :class:`~simpleeval.EvalWithCompoundTypes`. + """ + evaluator = getattr(_evaluator_local, "evaluator", None) + if evaluator is None: + functions = ( + allowed_functions + if allowed_functions is not None + else DEFAULT_ALLOWED_FUNCTIONS + ) + evaluator = EvalWithCompoundTypes(functions=functions, names={}) + _evaluator_local.evaluator = evaluator + return _evaluator_local.evaluator + + +def make_evaluator( + allowed_functions: dict[str, Callable[..., Any]] | None = None, +) -> EvalWithCompoundTypes: + """Create a fresh locked-down :class:`~simpleeval.EvalWithCompoundTypes`. + + Prefer :func:`get_evaluator` for production use so admin and runtime share + the same per-thread evaluator. Use this when you need an isolated + evaluator (e.g. tests, or custom function sets). + + :param allowed_functions: Override the function whitelist. Pass an empty + dict to disallow all functions. + :returns: A configured :class:`~simpleeval.EvalWithCompoundTypes`. + """ + functions = ( + allowed_functions + if allowed_functions is not None + else DEFAULT_ALLOWED_FUNCTIONS + ) + return EvalWithCompoundTypes(functions=functions, names={}) + + +def validate_message(message: str) -> None: + """Validate a patron blocking rule *message* string. + + :param message: The human-readable message shown when a patron is blocked. + :raises RuleValidationError: If the message is empty/whitespace or exceeds + :data:`MAX_MESSAGE_LENGTH` characters. + """ + if not message or not message.strip(): + raise RuleValidationError("Message must not be empty or whitespace.") + if len(message) > MAX_MESSAGE_LENGTH: + raise RuleValidationError( + f"Message must not exceed {MAX_MESSAGE_LENGTH} characters " + f"(got {len(message)})." + ) + + +def validate_rule_expression( + expr: str, + test_values: Mapping[str, Any], + evaluator: EvalWithCompoundTypes, +) -> None: + """Validate a rule expression at admin-save time. + + Checks performed (in order): + 1. Non-empty / non-whitespace. + 2. Length ≤ :data:`MAX_RULE_LENGTH`. + 3. All placeholders present in *test_values*. + 4. Expression parses and evaluates without error. + 5. Result is a strict :class:`bool`. + + When a placeholder is missing or an unsupported function is referenced the + error message includes a listing of all available keys/functions so + operators can correct the rule without guessing. + + :param expr: The raw rule expression string. + :param test_values: Mapping of placeholder key → test value used for the + trial evaluation. For live validation this is the dict returned by + ``fetch_live_rule_validation_values``; it contains all raw SIP2 + response fields plus the normalised ``fines`` key. + :param evaluator: A locked-down evaluator from :func:`get_evaluator` or + :func:`make_evaluator`. + :raises RuleValidationError: On any validation failure. + """ + if not expr or not expr.strip(): + raise RuleValidationError("Rule expression must not be empty or whitespace.") + if len(expr) > MAX_RULE_LENGTH: + raise RuleValidationError( + f"Rule expression must not exceed {MAX_RULE_LENGTH} characters " + f"(got {len(expr)})." + ) + + compiled = compile_rule_expression(expr) + + try: + names = build_names(compiled, test_values) + except MissingPlaceholderError as exc: + raise RuleValidationError(str(exc.message)) from exc + + evaluator.names = names + try: + result = evaluator.eval(compiled.compiled) + except FunctionNotDefined as exc: + allowed_fns = _format_allowed_functions(evaluator.functions) + raise RuleValidationError( + f"Rule expression references an unsupported function: {exc}. " + f"Allowed functions: {allowed_fns}" + ) from exc + except Exception as exc: + raise RuleValidationError(f"Rule expression failed to evaluate: {exc}") from exc + finally: + evaluator.names = {} + + if not isinstance(result, bool): + raise RuleValidationError( + f"Rule expression must evaluate to a boolean, got {type(result).__name__!r}." + ) + + +def evaluate_rule_expression_strict_bool( + expr: str, + values: Mapping[str, Any], + evaluator: EvalWithCompoundTypes, + rule_name: str | None = None, +) -> bool: + """Evaluate a rule expression at runtime. + + This function is **fail-closed**: any error raises + :class:`RuleEvaluationError` rather than silently allowing access. + + When a placeholder is missing the error message lists all available keys + and their values. When an unsupported function is referenced the error + message lists all allowed functions. This information is logged + server-side (never exposed to patrons) so operators can diagnose and fix + the rule. + + :param expr: The raw rule expression string (same as stored). + :param values: Mapping of placeholder key → runtime value. + :param evaluator: A locked-down evaluator from :func:`get_evaluator` or + :func:`make_evaluator`. + :param rule_name: Optional identifier for the rule, included in error messages. + :returns: ``True`` if the patron should be *blocked*, ``False`` otherwise. + :raises RuleEvaluationError: On missing placeholders, parse/eval errors, or + non-boolean result. + """ + compiled = compile_rule_expression(expr) + + try: + names = build_names(compiled, values) + except MissingPlaceholderError as exc: + raise RuleEvaluationError( + f"Missing placeholder {{{exc.key}}} for rule {rule_name!r}. " + f"Available fields: {_format_available_keys(exc.available)}", + rule_name=rule_name, + ) from exc + + evaluator.names = names + try: + result = evaluator.eval(compiled.compiled) + except FunctionNotDefined as exc: + allowed_fns = _format_allowed_functions(evaluator.functions) + raise RuleEvaluationError( + f"Rule {rule_name!r} references an unsupported function: {exc}. " + f"Allowed functions: {allowed_fns}", + rule_name=rule_name, + ) from exc + except NameNotDefined as exc: + raise RuleEvaluationError( + f"Undefined name in rule {rule_name!r}: {exc}", + rule_name=rule_name, + ) from exc + except Exception as exc: + raise RuleEvaluationError( + f"Rule {rule_name!r} could not be evaluated.", + rule_name=rule_name, + ) from exc + finally: + evaluator.names = {} + + if not isinstance(result, bool): + raise RuleEvaluationError( + f"Rule {rule_name!r} did not return a boolean " + f"(got {type(result).__name__!r}).", + rule_name=rule_name, + ) + + return result diff --git a/src/palace/manager/api/circulation/exceptions.py b/src/palace/manager/api/circulation/exceptions.py index 3d9dd47f4f..b42d1eb9b4 100644 --- a/src/palace/manager/api/circulation/exceptions.py +++ b/src/palace/manager/api/circulation/exceptions.py @@ -4,7 +4,6 @@ from palace.manager.api.problem_details import ( BAD_DELIVERY_MECHANISM, - BLOCKED_CREDENTIALS, CANNOT_FULFILL, CANNOT_RELEASE_HOLD, CHECKOUT_FAILED, @@ -24,6 +23,7 @@ RENEW_FAILED, SPECIFIC_HOLD_LIMIT_MESSAGE, SPECIFIC_LOAN_LIMIT_MESSAGE, + SUSPENDED_CREDENTIALS, ) from palace.manager.core.exceptions import IntegrationException from palace.manager.core.problem_details import ( @@ -199,7 +199,7 @@ class AuthorizationBlocked(CannotLoan): @property def base(self) -> ProblemDetail: - return BLOCKED_CREDENTIALS + return SUSPENDED_CREDENTIALS class LimitReached(CirculationException, ABC): diff --git a/src/palace/manager/api/circulation_manager.py b/src/palace/manager/api/circulation_manager.py index 7f92d46d17..efbd8f49fb 100644 --- a/src/palace/manager/api/circulation_manager.py +++ b/src/palace/manager/api/circulation_manager.py @@ -149,6 +149,7 @@ class CirculationManager(LoggerMixin): admin_announcement_service: AnnouncementSettings admin_search_controller: AdminSearchController admin_view_controller: ViewController + admin_quicksight_controller: QuickSightController admin_report_controller: ReportController diff --git a/src/palace/manager/api/problem_details.py b/src/palace/manager/api/problem_details.py index 19cb31d3df..159ed0a982 100644 --- a/src/palace/manager/api/problem_details.py +++ b/src/palace/manager/api/problem_details.py @@ -34,13 +34,20 @@ _("Your library card has expired. You need to renew it."), ) -BLOCKED_CREDENTIALS = pd( +SUSPENDED_CREDENTIALS = pd( "http://librarysimplified.org/terms/problem/credentials-suspended", 403, _("Suspended credentials."), _("Your library card has been suspended. Contact your branch library."), ) +BLOCKED_BY_POLICY = pd( + "http://librarysimplified.org/terms/problem/credentials-blocked-by-policy", + 403, + _("Blocked by library policy."), + _("Your access is restricted by library policy. Contact your branch library."), +) + NO_LICENSES = pd( "http://librarysimplified.org/terms/problem/no-licenses", 404, diff --git a/src/palace/manager/integration/patron_auth/kansas_patron.py b/src/palace/manager/integration/patron_auth/kansas_patron.py index b1a3ebd7de..b3ddd292b6 100644 --- a/src/palace/manager/integration/patron_auth/kansas_patron.py +++ b/src/palace/manager/integration/patron_auth/kansas_patron.py @@ -8,6 +8,7 @@ BasicAuthenticationProvider, BasicAuthProviderLibrarySettings, BasicAuthProviderSettings, + RemoteAuthResult, ) from palace.manager.integration.settings import FormMetadata from palace.manager.sqlalchemy.model.patron import Patron @@ -62,7 +63,7 @@ def __init__( def remote_authenticate( self, username: str, password: str | None - ) -> PatronData | None: + ) -> RemoteAuthResult: # Create XML doc for request authorization_request = self.create_authorize_request(username, password) # Post request to the server @@ -72,14 +73,16 @@ def remote_authenticate( response.content ) if not authorized: - return None + return RemoteAuthResult(patron_data=None) # Kansas auth gives very little data about the patron. Only name and a library identifier. - return PatronData( - permanent_id=username, - authorization_identifier=username, - personal_name=patron_name, - library_identifier=library_identifier, - complete=True, + return RemoteAuthResult( + patron_data=PatronData( + permanent_id=username, + authorization_identifier=username, + personal_name=patron_name, + library_identifier=library_identifier, + complete=True, + ) ) def remote_patron_lookup( diff --git a/src/palace/manager/integration/patron_auth/millenium_patron.py b/src/palace/manager/integration/patron_auth/millenium_patron.py index 8170094cb9..2b84ab24b1 100644 --- a/src/palace/manager/integration/patron_auth/millenium_patron.py +++ b/src/palace/manager/integration/patron_auth/millenium_patron.py @@ -15,6 +15,7 @@ BasicAuthenticationProvider, BasicAuthProviderLibrarySettings, BasicAuthProviderSettings, + RemoteAuthResult, ) from palace.manager.api.authentication.patron_debug import HasPatronDebug from palace.manager.core.selftest import SelfTestResult @@ -219,42 +220,45 @@ def _request(self, path): def remote_authenticate( self, username: str, password: str | None - ) -> PatronData | None: + ) -> RemoteAuthResult: """Does the Millenium Patron API approve of these credentials? - :return: False if the credentials are invalid. If they are - valid, a PatronData that serves only to indicate which - authorization identifier the patron prefers. + :return: RemoteAuthResult with patron_data=None if the credentials + are invalid. If they are valid, patron_data is a PatronData that + serves only to indicate which authorization identifier the patron + prefers. """ if not username: - return None + return RemoteAuthResult(patron_data=None) if not self.collects_password: # We don't even look at the password. If the patron exists, they # are authenticated. patrondata = self.remote_patron_lookup(username) if not patrondata: - return None - return patrondata + return RemoteAuthResult(patron_data=None) + return RemoteAuthResult(patron_data=patrondata) if self.auth_mode == AuthenticationMode.PIN: - return self._remote_authenticate_pintest( - username=username, password=password + return RemoteAuthResult( + patron_data=self._remote_authenticate_pintest( + username=username, password=password + ) ) elif self.auth_mode == AuthenticationMode.FAMILY_NAME: # Patrons are authenticated by their family name. patrondata = self.remote_patron_lookup(username) if not patrondata: # The patron doesn't even exist. - return None + return RemoteAuthResult(patron_data=None) # The patron exists; but do the last names match? if self.family_name_match(patrondata.personal_name, password): # Since this is a complete PatronData, we'll be able # to update their account without making a separate # call to /dump. - return patrondata - return None + return RemoteAuthResult(patron_data=patrondata) + return RemoteAuthResult(patron_data=None) def _pintest_request(self, username: str, password: str | None) -> dict[str, str]: """Perform a PIN test HTTP request and return the parsed response. diff --git a/src/palace/manager/integration/patron_auth/minimal_authentication.py b/src/palace/manager/integration/patron_auth/minimal_authentication.py index 9ea72c8d6d..06194ba734 100644 --- a/src/palace/manager/integration/patron_auth/minimal_authentication.py +++ b/src/palace/manager/integration/patron_auth/minimal_authentication.py @@ -7,6 +7,7 @@ BasicAuthenticationProvider, BasicAuthProviderLibrarySettings, BasicAuthProviderSettings, + RemoteAuthResult, ) from palace.manager.sqlalchemy.model.patron import Patron @@ -47,12 +48,12 @@ def library_settings_class(cls) -> type[BasicAuthProviderLibrarySettings]: def remote_authenticate( self, username: str, password: str | None - ) -> PatronData | None: + ) -> RemoteAuthResult: """No Auth authentication: Allow everyone through as long as they have a username.""" if not username: - return None + return RemoteAuthResult(patron_data=None) - return self.generate_patrondata(username) + return RemoteAuthResult(patron_data=self.generate_patrondata(username)) @classmethod def generate_patrondata(cls, authorization_identifier: str) -> PatronData: diff --git a/src/palace/manager/integration/patron_auth/patron_blocking.py b/src/palace/manager/integration/patron_auth/patron_blocking.py new file mode 100644 index 0000000000..1650065a4f --- /dev/null +++ b/src/palace/manager/integration/patron_auth/patron_blocking.py @@ -0,0 +1,163 @@ +"""Patron blocking rules — shared across all patron authentication protocols. + +A blocking rule is a simple named predicate attached to a library's +per-protocol settings. At authentication time, the rules are evaluated +after the remote ILS has successfully authenticated the patron. If any +rule triggers a block, a ProblemDetail is returned instead of the Patron +object and the request is rejected. + +Rule expressions are evaluated with the simpleeval-based rule engine +(see palace.manager.api.authentication.patron_blocking_rules.rule_engine). +""" + +from __future__ import annotations + +import logging +from typing import TYPE_CHECKING, Any + +from pydantic import BaseModel, ConfigDict + +from palace.manager.api.authentication.patron_blocking_rules.rule_engine import ( + RuleEvaluationError, + evaluate_rule_expression_strict_bool, + get_evaluator, +) +from palace.manager.api.problem_details import BLOCKED_BY_POLICY +from palace.manager.util import MoneyUtility +from palace.manager.util.problem_detail import ProblemDetail + +if TYPE_CHECKING: + from palace.manager.sqlalchemy.model.patron import Patron + + +class PatronBlockingRule(BaseModel): + """A single patron blocking rule stored in a library's per-protocol settings. + + :ivar name: A human-readable identifier for the rule; must be non-empty and + unique within a library's rule list. + :ivar rule: A simpleeval rule expression evaluated at authentication time. + The expression must evaluate to a strict bool. Placeholder values may + be embedded as ``{key}`` and are resolved at runtime from the patron's + profile (see :func:`build_runtime_values_from_patron`). + :ivar message: Optional text shown to the patron when this rule blocks + access. If omitted a generic default is used. + """ + + model_config = ConfigDict(frozen=True, str_strip_whitespace=True) + + name: str + rule: str + message: str | None = None + + +def build_runtime_values_from_patron(patron: Patron) -> dict[str, Any]: + """Build the simpleeval ``names`` dict for a patron at authentication time. + + Keys produced here correspond to the placeholder names supported at + runtime. Any placeholder key *not* present in the returned dict will + cause :func:`check_patron_blocking_rules_with_evaluator` to log and ignore + that rule if it references the missing key. + + :param patron: The authenticated + :class:`~palace.manager.sqlalchemy.model.patron.Patron`. + :returns: Dict mapping placeholder key to resolved value. + """ + values: dict[str, Any] = {} + + # fines — always populated; None or unparseable → 0.0 + try: + fines_raw = getattr(patron, "fines", None) + values["fines"] = float(fines_raw) if fines_raw is not None else 0.0 + except (ValueError, TypeError): + values["fines"] = 0.0 + + # patron_type — always populated; None → empty string + try: + pt = getattr(patron, "external_type", None) + values["patron_type"] = str(pt) if pt is not None else "" + except Exception: + values["patron_type"] = "" + + return values + + +def build_values_from_sip2_info(info: dict[str, Any]) -> dict[str, Any]: + """Build a simpleeval values dict from a raw SIP2 ``patron_information`` dict. + + Returns **all** fields present in the SIP2 response so that operators have + the widest possible set of keys to reference in blocking rules, plus a + normalised ``fines`` key derived from ``fee_amount``. + + This is used at admin-save validation time (live SIP2 call) and may also + be used at runtime once richer SIP2 data is available in the auth flow. + + :param info: The dict returned by + :meth:`~palace.manager.integration.patron_auth.sip2.client.SIPClient.patron_information`. + :returns: Dict mapping placeholder key to resolved value. All raw SIP2 + fields are included verbatim; the additional ``fines`` key is a parsed + :class:`float` derived from ``fee_amount``. + """ + # Include every raw SIP2 field so rules can reference any server-returned key. + values: dict[str, Any] = dict(info) + + # Add normalised 'fines' from fee_amount (BV); may be "$5.00", "5.00", or absent. + try: + values["fines"] = float(MoneyUtility.parse(info.get("fee_amount") or "0")) + except (ValueError, TypeError): + values["fines"] = 0.0 + + return values + + +_DEFAULT_BLOCK_MESSAGE = "Patron is blocked by library policy." + + +def check_patron_blocking_rules_with_evaluator( + rules: list[PatronBlockingRule], + values: dict[str, Any], + log: logging.Logger | logging.LoggerAdapter[logging.Logger] | None = None, +) -> ProblemDetail | None: + """Evaluate blocking rules using the simpleeval rule engine. + + This function is **fail-open**: rules that cannot be parsed or evaluated + (missing placeholder, parse error, non-bool result) are logged and ignored; + evaluation continues with the next rule. Only rules that successfully + evaluate to ``True`` cause a block. + + Uses a per-thread evaluator from :func:`~palace.manager.api.authentication + .patron_blocking_rules.rule_engine.get_evaluator` so the function is safe + to call from concurrent requests. + + :param rules: The list of :class:`PatronBlockingRule` objects for the library. + :param values: Runtime placeholder values; for SIP2 providers this is the + full raw SIP2 response dict (see :func:`build_values_from_sip2_info`). + :param log: Optional logger for server-side error diagnostics. + :returns: A :class:`~palace.manager.util.problem_detail.ProblemDetail` + (HTTP 403) if the patron should be blocked, or ``None`` if + authentication should proceed normally. + """ + evaluator = get_evaluator() + + for rule in rules: + try: + blocked = evaluate_rule_expression_strict_bool( + rule.rule, values, evaluator, rule_name=rule.name + ) + except RuleEvaluationError as exc: + if log: + # NOTE: This particular error string can be used by Cloudwatch or + # other monitoring tools. Be aware that changing it may cause + # the alarm to fail silently. + log.error( + "Patron blocking rule evaluation failed (ignored): " + "rule=%r, reason=%s: %s", + exc.rule_name, + type(exc.__cause__).__name__ if exc.__cause__ else "unknown", + exc, + ) + continue + + if blocked: + return BLOCKED_BY_POLICY.detailed(rule.message or _DEFAULT_BLOCK_MESSAGE) + + return None diff --git a/src/palace/manager/integration/patron_auth/simple_authentication.py b/src/palace/manager/integration/patron_auth/simple_authentication.py index 03cab31b24..e6274e4c02 100644 --- a/src/palace/manager/integration/patron_auth/simple_authentication.py +++ b/src/palace/manager/integration/patron_auth/simple_authentication.py @@ -5,6 +5,7 @@ BasicAuthenticationProvider, BasicAuthProviderLibrarySettings, BasicAuthProviderSettings, + RemoteAuthResult, ) from palace.manager.integration.settings import ( FormFieldType, @@ -96,15 +97,15 @@ def __init__( def remote_authenticate( self, username: str, password: str | None - ) -> PatronData | None: + ) -> RemoteAuthResult: """Fake 'remote' authentication.""" if not username or (self.collects_password and not password): - return None + return RemoteAuthResult(patron_data=None) if not self.valid_patron(username, password): - return None + return RemoteAuthResult(patron_data=None) - return self.generate_patrondata(username) + return RemoteAuthResult(patron_data=self.generate_patrondata(username)) @classmethod def generate_patrondata(cls, authorization_identifier: str) -> PatronData: diff --git a/src/palace/manager/integration/patron_auth/sip2/client.py b/src/palace/manager/integration/patron_auth/sip2/client.py index 557b93c89c..9ef877267d 100644 --- a/src/palace/manager/integration/patron_auth/sip2/client.py +++ b/src/palace/manager/integration/patron_auth/sip2/client.py @@ -209,6 +209,7 @@ class Named: sipserver_patron_class = NamedField("PC") sipserver_internet_privileges = NamedField("PI") sipserver_internal_id = NamedField("XI") + sipserver_patron_birthdate = NamedField("PB") # SIP extensions defined by Polaris. polaris_patron_birthdate = NamedField("BC") diff --git a/src/palace/manager/integration/patron_auth/sip2/provider.py b/src/palace/manager/integration/patron_auth/sip2/provider.py index 67c9a97939..78052b4c97 100644 --- a/src/palace/manager/integration/patron_auth/sip2/provider.py +++ b/src/palace/manager/integration/patron_auth/sip2/provider.py @@ -4,21 +4,27 @@ import socket from collections.abc import Callable, Generator from datetime import datetime -from typing import Annotated, Any +from typing import Annotated, Any, ClassVar from annotated_types import Ge, Le from pydantic import PositiveInt from sqlalchemy.orm import Session +from palace.manager.api.admin.problem_details import INVALID_CONFIGURATION_OPTION from palace.manager.api.authentication.base import PatronAuthResult, PatronData from palace.manager.api.authentication.basic import ( BasicAuthenticationProvider, BasicAuthProviderLibrarySettings, BasicAuthProviderSettings, + RemoteAuthResult, ) from palace.manager.api.authentication.patron_debug import HasPatronDebug from palace.manager.api.problem_details import INVALID_CREDENTIALS from palace.manager.core.selftest import SelfTestResult +from palace.manager.integration.patron_auth.patron_blocking import ( + build_runtime_values_from_patron, + build_values_from_sip2_info, +) from palace.manager.integration.patron_auth.sip2.client import Sip2Encoding, SIPClient from palace.manager.integration.patron_auth.sip2.dialect import Dialect as Sip2Dialect from palace.manager.integration.settings import ( @@ -29,7 +35,7 @@ from palace.manager.sqlalchemy.model.patron import Patron from palace.manager.util import MoneyUtility from palace.manager.util.network_diagnostics import run_network_diagnostics -from palace.manager.util.problem_detail import ProblemDetail +from palace.manager.util.problem_detail import ProblemDetail, ProblemDetailException class SIP2Settings(BasicAuthProviderSettings): @@ -200,6 +206,8 @@ class SIP2Settings(BasicAuthProviderSettings): class SIP2LibrarySettings(BasicAuthProviderLibrarySettings): + supports_patron_blocking_rules: ClassVar[bool] = True + # Used as the SIP2 AO field. institution_id: Annotated[ str | None, @@ -214,6 +222,8 @@ class SIP2AuthenticationProvider( BasicAuthenticationProvider[SIP2Settings, SIP2LibrarySettings], HasPatronDebug, ): + supports_patron_blocking_rules: ClassVar[bool] = True + DATE_FORMATS = ["%Y%m%d", "%Y%m%d%Z%H%M%S", "%Y%m%d %H%M%S"] # Map the reasons why SIP2 might report a patron is blocked to the @@ -310,6 +320,77 @@ def settings_class(cls) -> type[SIP2Settings]: def library_settings_class(cls) -> type[SIP2LibrarySettings]: return SIP2LibrarySettings + @classmethod + def fetch_live_rule_validation_values( + cls, settings: SIP2Settings + ) -> dict[str, Any]: + """Make a live SIP2 call and return placeholder values for rule validation. + + Called at admin-save time when the library settings include patron blocking + rules. Uses the provider's configured test identifier to fetch real patron + information from the SIP server and builds a values dict suitable for + passing to :func:`~palace.manager.api.authentication.patron_blocking_rules + .rule_engine.validate_rule_expression`. + + This is intentionally a classmethod so it can be called without a + database session — the temporary provider instance it creates is never + persisted. + + Args: + settings: The validated :class:`SIP2Settings` for this integration. + + Returns: + A dict mapping placeholder key to resolved value, produced by + :func:`~palace.manager.integration.patron_auth.patron_blocking + .build_values_from_sip2_info`. + + Raises: + :class:`~palace.manager.util.problem_detail.ProblemDetailException`: + If ``test_identifier`` is not configured, the SIP2 server cannot + be reached, or the server returns an error response. + """ + if not settings.test_identifier: + raise ProblemDetailException( + INVALID_CONFIGURATION_OPTION.detailed( + "A test identifier must be configured on this authentication " + "service before patron blocking rules can be validated against " + "live patron data." + ) + ) + + # Instantiate a temporary provider to reuse patron_information(). + # library_id=0 and integration_id=0 are placeholders — they are never + # written to the database. + provider = cls( + library_id=0, + integration_id=0, + settings=settings, + library_settings=cls.library_settings_class()(), + ) + + try: + info = provider.patron_information( + settings.test_identifier, settings.test_password or "" + ) + except Exception as exc: + raise ProblemDetailException( + INVALID_CONFIGURATION_OPTION.detailed( + f"Could not contact the SIP2 server while validating patron " + f"blocking rules: {exc}" + ) + ) from exc + + if isinstance(info, ProblemDetail): + raise ProblemDetailException( + INVALID_CONFIGURATION_OPTION.detailed( + f"SIP2 server returned an error while validating patron blocking " + f"rules for test identifier {settings.test_identifier!r}: " + f"{info.detail}" + ) + ) + + return build_values_from_sip2_info(info) + def patron_information( self, username: str, password: str | None ) -> dict[str, Any] | ProblemDetail: @@ -341,9 +422,13 @@ def remote_patron_lookup( def remote_authenticate( self, username: str, password: str | None - ) -> PatronData | None | ProblemDetail: + ) -> RemoteAuthResult: """Authenticate a patron with the SIP2 server. + Returns the raw SIP2 response dict in extra_context so that + :meth:`_build_blocking_rule_values` can use the full payload for + rule evaluation without requiring a second network call. + :param username: The patron's username/barcode/card number/authorization identifier. :param password: The patron's password/pin/access code. @@ -353,7 +438,32 @@ def remote_authenticate( # passing it on. password = None info = self.patron_information(username, password) - return self.info_to_patrondata(info) + extra_context = info if isinstance(info, dict) else {} + return RemoteAuthResult( + patron_data=self.info_to_patrondata(info), + extra_context=extra_context, + ) + + def _build_blocking_rule_values( + self, patron: Patron, extra_context: dict[str, Any] + ) -> dict[str, Any]: + """Return the full SIP2 response dict as rule-evaluation values. + + Uses the raw SIP2 ``patron_information`` payload passed via + extra_context from :meth:`remote_authenticate`. If extra_context is + empty (e.g. in tests that bypass ``remote_authenticate``), falls back + to the patron-model values produced by + :func:`~palace.manager.integration.patron_auth.patron_blocking + .build_runtime_values_from_patron`. + + The returned dict contains **every field** returned by the SIP2 server + plus a normalised ``fines`` float key. This allows blocking rules to + reference any SIP2 field by name (e.g. ``{sipserver_patron_class}``, + ``{fee_amount}``, ``{polaris_patron_birthdate}``). + """ + if extra_context: + return build_values_from_sip2_info(extra_context) + return build_runtime_values_from_patron(patron) def patron_debug( self, username: str, password: str | None diff --git a/src/palace/manager/integration/patron_auth/sirsidynix_authentication_provider.py b/src/palace/manager/integration/patron_auth/sirsidynix_authentication_provider.py index 1b0b0496a6..05675f627f 100644 --- a/src/palace/manager/integration/patron_auth/sirsidynix_authentication_provider.py +++ b/src/palace/manager/integration/patron_auth/sirsidynix_authentication_provider.py @@ -14,6 +14,7 @@ BasicAuthenticationProvider, BasicAuthProviderLibrarySettings, BasicAuthProviderSettings, + RemoteAuthResult, ) from palace.manager.core.config import Configuration from palace.manager.core.exceptions import BasePalaceException, PalaceValueError @@ -171,21 +172,23 @@ def __init__( def remote_authenticate( self, username: str, password: str | None - ) -> PatronData | None: + ) -> RemoteAuthResult: """Authenticate this user with the remote server.""" if password is None: - return None + return RemoteAuthResult(patron_data=None) data = self.api_patron_login(username, password) if not data: - return None - - return SirsiDynixPatronData( - username=username, - authorization_identifier=username, - permanent_id=data.get("patronKey"), - session_token=data.get("sessionToken"), - complete=False, + return RemoteAuthResult(patron_data=None) + + return RemoteAuthResult( + patron_data=SirsiDynixPatronData( + username=username, + authorization_identifier=username, + permanent_id=data.get("patronKey"), + session_token=data.get("sessionToken"), + complete=False, + ) ) def remote_patron_lookup( diff --git a/tests/manager/api/admin/controller/test_patron_auth.py b/tests/manager/api/admin/controller/test_patron_auth.py index ebf46e908e..abf0aa37da 100644 --- a/tests/manager/api/admin/controller/test_patron_auth.py +++ b/tests/manager/api/admin/controller/test_patron_auth.py @@ -58,7 +58,7 @@ from palace.manager.sqlalchemy.model.integration import IntegrationConfiguration from palace.manager.sqlalchemy.model.library import Library from palace.manager.sqlalchemy.util import get_one -from palace.manager.util.problem_detail import ProblemDetail +from palace.manager.util.problem_detail import ProblemDetail, ProblemDetailException from tests.fixtures.flask import FlaskAppFixture from tests.fixtures.services import ServicesFixture from tests.mocks.saml_strings import CORRECT_XML_WITH_ONE_SP @@ -926,3 +926,455 @@ def test_patron_auth_self_tests_test_post( assert mock.call_args.args[1] is None assert mock.call_args.args[2] == library.id assert mock.call_args.args[3] == auth_service.id + + # Shared for live rule validation and validate-rule endpoint tests. + _FETCH_PATCH = ( + "palace.manager.integration.patron_auth.sip2.provider" + ".SIP2AuthenticationProvider.fetch_live_rule_validation_values" + ) + _SIP2_BASE_ARGS = [ + ("url", "sip.example.com"), + ("test_identifier", "patron1"), + ("test_password", "pass"), + ("identifier_keyboard", Keyboards.DEFAULT.value), + ("password_keyboard", Keyboards.DEFAULT.value), + ("identifier_barcode_format", BarcodeFormats.CODABAR.value), + ] + + def _post_sip2( + self, + controller_fixture: ControllerFixture, + flask_app_fixture: FlaskAppFixture, + library: Library, + rules: list[dict] | None = None, + extra_args: list[tuple[str, str]] | None = None, + service_name: str = "live-sip2-test", + ) -> ProblemDetail | Response: + """Submit a SIP2 patron-auth service POST and return the response.""" + library_data: dict = { + "short_name": library.short_name, + "library_identifier_restriction_type": LibraryIdentifierRestriction.NONE.value, + "library_identifier_field": "barcode", + } + if rules is not None: + library_data["patron_blocking_rules"] = rules + + form_args: list[tuple[str, str]] = ( + [ + ("name", service_name), + ( + "protocol", + controller_fixture.get_protocol(SIP2AuthenticationProvider), + ), + ("libraries", json.dumps([library_data])), + ] + + self._SIP2_BASE_ARGS + + (extra_args or []) + ) + + with flask_app_fixture.test_request_context_system_admin("/", method="POST"): + flask.request.form = ImmutableMultiDict(form_args) + return controller_fixture.controller.process_patron_auth_services() + + def test_no_rules_skips_live_sip2_call( + self, + controller_fixture: ControllerFixture, + flask_app_fixture: FlaskAppFixture, + db: DatabaseTransactionFixture, + monkeypatch: MonkeyPatch, + ) -> None: + """When no patron blocking rules are configured the live SIP2 call must + not be made and the save must succeed.""" + mock_fetch = MagicMock() + monkeypatch.setattr(self._FETCH_PATCH, mock_fetch) + + response = self._post_sip2( + controller_fixture, flask_app_fixture, db.default_library(), rules=None + ) + + assert isinstance(response, Response) + assert response.status_code in (200, 201) + mock_fetch.assert_not_called() + + def test_rule_passes_against_live_values_allows_save( + self, + controller_fixture: ControllerFixture, + flask_app_fixture: FlaskAppFixture, + db: DatabaseTransactionFixture, + monkeypatch: MonkeyPatch, + ) -> None: + """When the live SIP2 call succeeds and every rule validates against the + real values, the save must succeed.""" + monkeypatch.setattr( + self._FETCH_PATCH, + MagicMock( + return_value={ + "fines": 2.50, + "patron_type": "adult", + } + ), + ) + + response = self._post_sip2( + controller_fixture, + flask_app_fixture, + db.default_library(), + rules=[{"name": "fine-check", "rule": "{fines} > 10.0"}], + ) + + assert isinstance(response, Response) + assert response.status_code in (200, 201) + + def test_rule_with_invalid_placeholder_allows_save( + self, + controller_fixture: ControllerFixture, + flask_app_fixture: FlaskAppFixture, + db: DatabaseTransactionFixture, + ) -> None: + """Rules are not validated on save; invalid rules are allowed and + ignored at auth time.""" + response = self._post_sip2( + controller_fixture, + flask_app_fixture, + db.default_library(), + rules=[{"name": "custom-check", "rule": "{custom_field} == 'expected'"}], + ) + + assert isinstance(response, Response) + assert response.status_code in (200, 201) + + def test_non_supporting_provider_rejects_rules_at_save( + self, + controller_fixture: ControllerFixture, + flask_app_fixture: FlaskAppFixture, + db: DatabaseTransactionFixture, + ) -> None: + """Providers that do not support patron blocking rules reject rules + at settings validation time; the live validation path is never reached.""" + with flask_app_fixture.test_request_context_system_admin("/", method="POST"): + flask.request.form = ImmutableMultiDict( + [ + ("name", "simple-auth-with-rules"), + ( + "protocol", + controller_fixture.get_protocol(SimpleAuthenticationProvider), + ), + ("test_identifier", "user"), + ("test_password", "pass"), + ("identifier_keyboard", Keyboards.DEFAULT.value), + ("password_keyboard", Keyboards.DEFAULT.value), + ("identifier_barcode_format", BarcodeFormats.CODABAR.value), + ( + "libraries", + json.dumps( + [ + { + "short_name": db.default_library().short_name, + "library_identifier_restriction_type": LibraryIdentifierRestriction.NONE.value, + "library_identifier_field": "barcode", + "patron_blocking_rules": [ + {"name": "fine-check", "rule": "{fines} > 10.0"} + ], + } + ] + ), + ), + ] + ) + response = controller_fixture.controller.process_patron_auth_services() + + assert isinstance(response, ProblemDetail) + assert response.uri == INVALID_CONFIGURATION_OPTION.uri + assert response.detail is not None + assert "not supported" in response.detail.lower() + + def _create_sip2_integration( + self, + controller_fixture: ControllerFixture, + flask_app_fixture: FlaskAppFixture, + db: DatabaseTransactionFixture, + service_name: str = "validate-test-sip2", + ) -> int: + """Create a SIP2 integration without patron blocking rules and return its ID. + + No rules means library_integration_validation skips the live SIP2 call, + so no mock is needed during creation. + """ + library_data = { + "short_name": db.default_library().short_name, + "library_identifier_restriction_type": LibraryIdentifierRestriction.NONE.value, + "library_identifier_field": "barcode", + } + with flask_app_fixture.test_request_context_system_admin("/", method="POST"): + flask.request.form = ImmutableMultiDict( + [ + ("name", service_name), + ( + "protocol", + controller_fixture.get_protocol(SIP2AuthenticationProvider), + ), + ("libraries", json.dumps([library_data])), + ] + + self._SIP2_BASE_ARGS + ) + response = controller_fixture.controller.process_patron_auth_services() + assert isinstance(response, Response) + return int(response.get_data(as_text=True)) + + def _create_simple_integration( + self, + controller_fixture: ControllerFixture, + flask_app_fixture: FlaskAppFixture, + service_name: str = "validate-test-simple", + ) -> int: + """Create a SimpleAuthenticationProvider integration (no library) and return its ID.""" + with flask_app_fixture.test_request_context_system_admin("/", method="POST"): + flask.request.form = ImmutableMultiDict( + [ + ("name", service_name), + ( + "protocol", + controller_fixture.get_protocol(SimpleAuthenticationProvider), + ), + ("test_identifier", "user"), + ("test_password", "pass"), + ("identifier_keyboard", Keyboards.DEFAULT.value), + ("password_keyboard", Keyboards.DEFAULT.value), + ("identifier_barcode_format", BarcodeFormats.CODABAR.value), + ("libraries", json.dumps([])), + ] + ) + response = controller_fixture.controller.process_patron_auth_services() + assert isinstance(response, Response) + return int(response.get_data(as_text=True)) + + def _post_validate( + self, + controller_fixture: ControllerFixture, + flask_app_fixture: FlaskAppFixture, + service_id: int | str, + rule: str = "{fines} > 10.0", + ) -> ProblemDetail | Response: + """Call process_validate_patron_blocking_rule with the given service_id and rule.""" + with flask_app_fixture.test_request_context_system_admin("/", method="POST"): + flask.request.form = ImmutableMultiDict( + [ + ("service_id", str(service_id)), + ("rule", rule), + ] + ) + return controller_fixture.controller.process_validate_patron_blocking_rule() + + def test_missing_service_id_returns_error( + self, + controller_fixture: ControllerFixture, + flask_app_fixture: FlaskAppFixture, + ) -> None: + """Omitting service_id returns INVALID_CONFIGURATION_OPTION.""" + with flask_app_fixture.test_request_context_system_admin("/", method="POST"): + flask.request.form = ImmutableMultiDict([("rule", "{fines} > 0")]) + response = ( + controller_fixture.controller.process_validate_patron_blocking_rule() + ) + assert isinstance(response, ProblemDetail) + assert response.uri == INVALID_CONFIGURATION_OPTION.uri + + def test_service_not_found_returns_error( + self, + controller_fixture: ControllerFixture, + flask_app_fixture: FlaskAppFixture, + ) -> None: + """A nonexistent service_id returns an error that tells the user to save first.""" + response = self._post_validate( + controller_fixture, flask_app_fixture, 999999, "{fines} > 0" + ) + assert isinstance(response, ProblemDetail) + assert response.uri == INVALID_CONFIGURATION_OPTION.uri + assert response.detail is not None + assert "save" in response.detail.lower() + + def test_non_sip2_service_returns_error( + self, + controller_fixture: ControllerFixture, + flask_app_fixture: FlaskAppFixture, + ) -> None: + """A service that does not support patron blocking rules returns an error.""" + simple_id = self._create_simple_integration( + controller_fixture, flask_app_fixture + ) + response = self._post_validate( + controller_fixture, flask_app_fixture, simple_id, "{fines} > 0" + ) + assert isinstance(response, ProblemDetail) + assert response.uri == INVALID_CONFIGURATION_OPTION.uri + assert response.detail is not None + assert "patron blocking rules" in response.detail + + def test_valid_rule_returns_200( + self, + controller_fixture: ControllerFixture, + flask_app_fixture: FlaskAppFixture, + db: DatabaseTransactionFixture, + monkeypatch: MonkeyPatch, + ) -> None: + """A valid rule that evaluates to True against live values returns 200.""" + monkeypatch.setattr( + self._FETCH_PATCH, + MagicMock(return_value={"fines": 2.50, "patron_type": "adult"}), + ) + service_id = self._create_sip2_integration( + controller_fixture, flask_app_fixture, db + ) + response = self._post_validate( + controller_fixture, flask_app_fixture, service_id, "{fines} > 1.0" + ) + assert isinstance(response, Response) + assert response.status_code == 200 + + def test_rule_with_false_result_still_returns_200( + self, + controller_fixture: ControllerFixture, + flask_app_fixture: FlaskAppFixture, + db: DatabaseTransactionFixture, + monkeypatch: MonkeyPatch, + ) -> None: + """A valid rule that evaluates to False against live values still returns 200. + + The boolean result (blocked vs. not blocked) is intentionally ignored — + only parse/eval success or failure is reported. + """ + monkeypatch.setattr( + self._FETCH_PATCH, + MagicMock(return_value={"fines": 0.0, "patron_type": "adult"}), + ) + service_id = self._create_sip2_integration( + controller_fixture, flask_app_fixture, db + ) + response = self._post_validate( + controller_fixture, flask_app_fixture, service_id, "{fines} > 100.0" + ) + assert isinstance(response, Response) + assert response.status_code == 200 + + def test_invalid_expression_returns_error( + self, + controller_fixture: ControllerFixture, + flask_app_fixture: FlaskAppFixture, + db: DatabaseTransactionFixture, + monkeypatch: MonkeyPatch, + ) -> None: + """An unparseable rule expression returns INVALID_CONFIGURATION_OPTION.""" + monkeypatch.setattr( + self._FETCH_PATCH, + MagicMock(return_value={"fines": 0.0}), + ) + service_id = self._create_sip2_integration( + controller_fixture, flask_app_fixture, db + ) + response = self._post_validate( + controller_fixture, + flask_app_fixture, + service_id, + "not a valid python expression !!!", + ) + assert isinstance(response, ProblemDetail) + assert response.uri == INVALID_CONFIGURATION_OPTION.uri + + def test_missing_placeholder_returns_error( + self, + controller_fixture: ControllerFixture, + flask_app_fixture: FlaskAppFixture, + db: DatabaseTransactionFixture, + monkeypatch: MonkeyPatch, + ) -> None: + """A rule referencing a placeholder not in the live values returns an error + whose detail mentions the missing field name.""" + monkeypatch.setattr( + self._FETCH_PATCH, + MagicMock(return_value={"fines": 0.0, "patron_type": "adult"}), + ) + service_id = self._create_sip2_integration( + controller_fixture, flask_app_fixture, db + ) + response = self._post_validate( + controller_fixture, + flask_app_fixture, + service_id, + "{unknown_field} == 'expected'", + ) + assert isinstance(response, ProblemDetail) + assert response.uri == INVALID_CONFIGURATION_OPTION.uri + assert response.detail is not None + assert "unknown_field" in response.detail + + def test_sip2_connection_error_propagates( + self, + controller_fixture: ControllerFixture, + flask_app_fixture: FlaskAppFixture, + db: DatabaseTransactionFixture, + monkeypatch: MonkeyPatch, + ) -> None: + """When fetch_live_rule_validation_values raises ProblemDetailException + (e.g. network error), that ProblemDetail is returned to the caller.""" + monkeypatch.setattr( + self._FETCH_PATCH, + MagicMock( + side_effect=ProblemDetailException( + INVALID_CONFIGURATION_OPTION.detailed( + "Could not contact the SIP2 server: Connection refused" + ) + ) + ), + ) + service_id = self._create_sip2_integration( + controller_fixture, flask_app_fixture, db + ) + response = self._post_validate( + controller_fixture, flask_app_fixture, service_id, "{fines} > 0" + ) + assert isinstance(response, ProblemDetail) + assert response.uri == INVALID_CONFIGURATION_OPTION.uri + assert response.detail is not None + assert "SIP2" in response.detail + + def test_missing_test_identifier_propagates( + self, + controller_fixture: ControllerFixture, + flask_app_fixture: FlaskAppFixture, + db: DatabaseTransactionFixture, + monkeypatch: MonkeyPatch, + ) -> None: + """When fetch_live_rule_validation_values raises because no test_identifier + is configured, the error is propagated as INVALID_CONFIGURATION_OPTION.""" + monkeypatch.setattr( + self._FETCH_PATCH, + MagicMock( + side_effect=ProblemDetailException( + INVALID_CONFIGURATION_OPTION.detailed( + "A test identifier must be configured on this authentication " + "service before patron blocking rules can be validated." + ) + ) + ), + ) + service_id = self._create_sip2_integration( + controller_fixture, flask_app_fixture, db + ) + response = self._post_validate( + controller_fixture, flask_app_fixture, service_id, "{fines} > 0" + ) + assert isinstance(response, ProblemDetail) + assert response.uri == INVALID_CONFIGURATION_OPTION.uri + + def test_requires_system_admin( + self, + controller_fixture: ControllerFixture, + flask_app_fixture: FlaskAppFixture, + ) -> None: + """A request without system-admin privileges raises AdminNotAuthorized.""" + with flask_app_fixture.test_request_context("/", method="POST"): + flask.request.form = ImmutableMultiDict( + [("service_id", "1"), ("rule", "{fines} > 0")] + ) + with pytest.raises(AdminNotAuthorized): + controller_fixture.controller.process_validate_patron_blocking_rule() diff --git a/tests/manager/api/authentication/patron_blocking_rules/__init__.py b/tests/manager/api/authentication/patron_blocking_rules/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/manager/api/authentication/patron_blocking_rules/test_rule_engine.py b/tests/manager/api/authentication/patron_blocking_rules/test_rule_engine.py new file mode 100644 index 0000000000..f898e75df6 --- /dev/null +++ b/tests/manager/api/authentication/patron_blocking_rules/test_rule_engine.py @@ -0,0 +1,472 @@ +from __future__ import annotations + +from datetime import date + +import pytest + +from palace.manager.api.authentication.patron_blocking_rules.rule_engine import ( + MAX_MESSAGE_LENGTH, + MAX_RULE_LENGTH, + CompiledRule, + MissingPlaceholderError, + RuleEvaluationError, + RuleValidationError, + age_in_years, + build_names, + compile_rule_expression, + evaluate_rule_expression_strict_bool, + make_evaluator, + validate_message, + validate_rule_expression, +) + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture() +def evaluator(): + return make_evaluator() + + +# --------------------------------------------------------------------------- +# compile_rule_expression +# --------------------------------------------------------------------------- + + +class TestCompileRuleExpression: + def test_single_placeholder_replaced(self): + result = compile_rule_expression("age_in_years({dob}) >= 18") + assert "__v_dob" in result.compiled + assert "{dob}" not in result.compiled + assert result.var_map == {"dob": "__v_dob"} + + def test_original_is_preserved(self): + expr = "age_in_years({dob}) >= 18" + result = compile_rule_expression(expr) + assert result.original == expr + + def test_repeated_placeholder_maps_consistently(self): + result = compile_rule_expression("{x} > 0 and {x} < 100") + assert result.var_map == {"x": "__v_x"} + # Both occurrences should be replaced with the same var name. + assert result.compiled == "__v_x > 0 and __v_x < 100" + + def test_multiple_distinct_placeholders(self): + result = compile_rule_expression("{fines} > 5 and age_in_years({dob}) < 18") + assert result.var_map == {"fines": "__v_fines", "dob": "__v_dob"} + + def test_no_placeholders(self): + result = compile_rule_expression("True") + assert result.var_map == {} + assert result.compiled == "True" + + def test_returns_compiled_rule_instance(self): + result = compile_rule_expression("{a} == {b}") + assert isinstance(result, CompiledRule) + + def test_underscore_in_key(self): + result = compile_rule_expression("{patron_type} == 'student'") + assert "__v_patron_type" in result.compiled + assert result.var_map == {"patron_type": "__v_patron_type"} + + # TODO: enforce rejection of invalid placeholder key formats if stricter + # validation is added (e.g. {123invalid}). + + +# --------------------------------------------------------------------------- +# build_names +# --------------------------------------------------------------------------- + + +class TestBuildNames: + def test_maps_key_to_var_name(self): + compiled = compile_rule_expression("{fines} >= 5") + names = build_names(compiled, {"fines": 5.0}) + assert names == {"__v_fines": 5.0} + + def test_missing_key_raises(self): + compiled = compile_rule_expression("{fines} > 5") + with pytest.raises(MissingPlaceholderError) as exc_info: + build_names(compiled, {}) + assert exc_info.value.key == "fines" + + def test_missing_key_error_lists_available_keys(self): + """Error message must list all available keys and their values.""" + compiled = compile_rule_expression("{amount_owed} > 10") + with pytest.raises(MissingPlaceholderError) as exc_info: + build_names(compiled, {"fines": 5.0, "patron_type": "adult"}) + msg = str(exc_info.value) + assert "amount_owed" in msg + assert "fines" in msg + assert "patron_type" in msg + + def test_missing_key_available_dict_is_stored(self): + """The .available attribute holds the dict that was passed in.""" + compiled = compile_rule_expression("{missing} > 0") + avail = {"x": 1, "y": 2} + with pytest.raises(MissingPlaceholderError) as exc_info: + build_names(compiled, avail) + assert exc_info.value.available == avail + + def test_missing_key_with_empty_available_shows_none(self): + """When no keys are available the message says '(none)'.""" + compiled = compile_rule_expression("{fines} > 5") + with pytest.raises(MissingPlaceholderError) as exc_info: + build_names(compiled, {}) + assert "(none)" in str(exc_info.value) + + def test_missing_key_message_contains_key(self): + compiled = compile_rule_expression("{amount_owed} > 10") + with pytest.raises(MissingPlaceholderError) as exc_info: + build_names(compiled, {"other": "x"}) + assert "amount_owed" in str(exc_info.value) + + def test_no_placeholders_returns_empty(self): + compiled = compile_rule_expression("True") + names = build_names(compiled, {}) + assert names == {} + + def test_extra_keys_in_values_are_ignored(self): + compiled = compile_rule_expression("{x} > 0") + names = build_names(compiled, {"x": 5, "y": 99}) + assert "__v_x" in names + assert "__v_y" not in names + + +# --------------------------------------------------------------------------- +# validate_message +# --------------------------------------------------------------------------- + + +class TestValidateMessage: + def test_valid_message_passes(self): + validate_message("Your account has outstanding fines.") + + def test_empty_string_raises(self): + with pytest.raises(RuleValidationError): + validate_message("") + + def test_whitespace_only_raises(self): + with pytest.raises(RuleValidationError): + validate_message(" ") + + def test_exactly_max_length_passes(self): + validate_message("x" * MAX_MESSAGE_LENGTH) + + def test_exceeds_max_length_raises(self): + with pytest.raises(RuleValidationError): + validate_message("x" * (MAX_MESSAGE_LENGTH + 1)) + + +# --------------------------------------------------------------------------- +# validate_rule_expression +# --------------------------------------------------------------------------- + + +class TestValidateRuleExpression: + def test_valid_expression_passes(self, evaluator): + validate_rule_expression( + "{fines} > 5", + test_values={"fines": 10}, + evaluator=evaluator, + ) + + def test_valid_age_expression_passes(self, evaluator): + validate_rule_expression( + "age_in_years({dob}) >= 18", + test_values={"dob": "1990-01-01"}, + evaluator=evaluator, + ) + + def test_valid_int_expression_passes(self, evaluator): + validate_rule_expression( + "int({patron_class}) > 2", + test_values={"patron_class": "3"}, + evaluator=evaluator, + ) + + def test_empty_expression_raises(self, evaluator): + with pytest.raises(RuleValidationError): + validate_rule_expression("", test_values={}, evaluator=evaluator) + + def test_whitespace_only_raises(self, evaluator): + with pytest.raises(RuleValidationError): + validate_rule_expression(" ", test_values={}, evaluator=evaluator) + + def test_exceeds_max_length_raises(self, evaluator): + long_expr = "True and " * 200 # well over 1000 chars + with pytest.raises(RuleValidationError): + validate_rule_expression(long_expr, test_values={}, evaluator=evaluator) + + def test_exactly_max_length_expression_content_is_validated(self, evaluator): + # Build a valid 1000-char expression: "{x}" padded with " and True". + base = "{x} == 1" + padding = " and True" + expr = base + padding * ((MAX_RULE_LENGTH - len(base)) // len(padding)) + # May be slightly under 1000 chars due to integer division; that's fine. + assert len(expr) <= MAX_RULE_LENGTH + validate_rule_expression(expr, test_values={"x": 1}, evaluator=evaluator) + + def test_missing_placeholder_raises(self, evaluator): + with pytest.raises(RuleValidationError) as exc_info: + validate_rule_expression( + "{missing_key} > 0", + test_values={"fines": 5.0, "patron_type": "adult"}, + evaluator=evaluator, + ) + msg = str(exc_info.value) + assert "missing_key" in msg + # Available keys should be listed in the error. + assert "fines" in msg + assert "patron_type" in msg + + def test_unknown_function_error_lists_allowed_functions(self, evaluator): + """A call to an unsupported function raises RuleValidationError listing + the allowed functions so the operator knows what to use.""" + with pytest.raises(RuleValidationError) as exc_info: + validate_rule_expression( + "bad_func({x}) > 0", + test_values={"x": 1}, + evaluator=evaluator, + ) + msg = str(exc_info.value) + assert "age_in_years" in msg + assert "int" in msg + + def test_syntax_error_raises(self, evaluator): + with pytest.raises(RuleValidationError): + validate_rule_expression( + "{x} >>>??? invalid", + test_values={"x": 1}, + evaluator=evaluator, + ) + + def test_non_bool_result_raises(self, evaluator): + with pytest.raises(RuleValidationError) as exc_info: + validate_rule_expression( + "{fines} + 1", + test_values={"fines": 5}, + evaluator=evaluator, + ) + assert "bool" in str(exc_info.value).lower() + + def test_string_result_raises(self, evaluator): + with pytest.raises(RuleValidationError): + validate_rule_expression( + "'{x}'", + test_values={"x": "hello"}, + evaluator=evaluator, + ) + + def test_integer_zero_raises(self, evaluator): + with pytest.raises(RuleValidationError): + validate_rule_expression( + "{n} * 0", + test_values={"n": 5}, + evaluator=evaluator, + ) + + def test_returns_none_on_success(self, evaluator): + result = validate_rule_expression( + "{active} == True", + test_values={"active": True}, + evaluator=evaluator, + ) + assert result is None + + +# --------------------------------------------------------------------------- +# evaluate_rule_expression_strict_bool +# --------------------------------------------------------------------------- + + +class TestEvaluateRuleExpressionStrictBool: + def test_true_result_returned(self, evaluator): + result = evaluate_rule_expression_strict_bool( + "{fines} > 5", + values={"fines": 10}, + evaluator=evaluator, + ) + assert result is True + + def test_false_result_returned(self, evaluator): + result = evaluate_rule_expression_strict_bool( + "{fines} > 5", + values={"fines": 3}, + evaluator=evaluator, + ) + assert result is False + + def test_missing_placeholder_raises_rule_evaluation_error(self, evaluator): + with pytest.raises(RuleEvaluationError) as exc_info: + evaluate_rule_expression_strict_bool( + "{some_field} == 'expected'", + values={"fines": 5.0, "patron_type": "adult"}, + evaluator=evaluator, + ) + msg = str(exc_info.value) + assert "some_field" in msg + # Available fields should be listed so operators can fix the rule. + assert "fines" in msg + assert "patron_type" in msg + + def test_missing_placeholder_error_includes_rule_name(self, evaluator): + with pytest.raises(RuleEvaluationError) as exc_info: + evaluate_rule_expression_strict_bool( + "{some_field} == 'expected'", + values={}, + evaluator=evaluator, + rule_name="underage_check", + ) + assert exc_info.value.rule_name == "underage_check" + + def test_unknown_function_error_lists_allowed_functions(self, evaluator): + """A call to an unsupported function raises RuleEvaluationError and + the message includes the list of allowed functions.""" + with pytest.raises(RuleEvaluationError) as exc_info: + evaluate_rule_expression_strict_bool( + "bad_func({x}) > 0", + values={"x": 1}, + evaluator=evaluator, + rule_name="bad-rule", + ) + msg = str(exc_info.value) + assert "age_in_years" in msg + assert "int" in msg + + def test_int_function_converts_string_field(self, evaluator): + """int() converts a string-valued SIP2 field before comparison.""" + result = evaluate_rule_expression_strict_bool( + "int({patron_class}) > 2", + values={"patron_class": "3"}, + evaluator=evaluator, + ) + assert result is True + + def test_int_function_does_not_block_below_threshold(self, evaluator): + result = evaluate_rule_expression_strict_bool( + "int({patron_class}) > 2", + values={"patron_class": "1"}, + evaluator=evaluator, + ) + assert result is False + + def test_invalid_expression_raises_rule_evaluation_error(self, evaluator): + with pytest.raises(RuleEvaluationError): + evaluate_rule_expression_strict_bool( + "{x} >>>??? bad", + values={"x": 1}, + evaluator=evaluator, + ) + + def test_non_bool_result_raises_rule_evaluation_error(self, evaluator): + with pytest.raises(RuleEvaluationError) as exc_info: + evaluate_rule_expression_strict_bool( + "{fines} + 1", + values={"fines": 5}, + evaluator=evaluator, + ) + assert "bool" in str(exc_info.value).lower() + + def test_rule_name_preserved_in_non_bool_error(self, evaluator): + with pytest.raises(RuleEvaluationError) as exc_info: + evaluate_rule_expression_strict_bool( + "{x} + 0", + values={"x": 1}, + evaluator=evaluator, + rule_name="my_rule", + ) + assert exc_info.value.rule_name == "my_rule" + + def test_rule_name_none_by_default(self, evaluator): + with pytest.raises(RuleEvaluationError) as exc_info: + evaluate_rule_expression_strict_bool( + "{x} + 0", + values={"x": 1}, + evaluator=evaluator, + ) + assert exc_info.value.rule_name is None + + def test_age_in_years_integration(self, evaluator): + result = evaluate_rule_expression_strict_bool( + "age_in_years({dob}) < 18", + values={"dob": "2015-06-01"}, + evaluator=evaluator, + ) + assert result is True + + def test_disallows_arbitrary_names(self, evaluator): + with pytest.raises(RuleEvaluationError): + evaluate_rule_expression_strict_bool( + "undefined_var > 0", + values={}, + evaluator=evaluator, + ) + + +# --------------------------------------------------------------------------- +# age_in_years +# --------------------------------------------------------------------------- + + +class TestAgeInYears: + _REF_DATE = date(2025, 6, 15) + + def test_exact_birthday_today(self): + result = age_in_years("1990-06-15", today=self._REF_DATE) + assert result == 35 + + def test_birthday_passed_this_year(self): + result = age_in_years("1990-01-01", today=self._REF_DATE) + assert result == 35 + + def test_birthday_not_yet_this_year(self): + result = age_in_years("1990-12-31", today=self._REF_DATE) + assert result == 34 + + def test_iso_format(self): + result = age_in_years("2000-06-15", today=self._REF_DATE) + assert result == 25 + + def test_fmt_parsing(self): + result = age_in_years("15/06/1990", fmt="%d/%m/%Y", today=self._REF_DATE) + assert result == 35 + + def test_fmt_us_format(self): + result = age_in_years("06-15-1990", fmt="%m-%d-%Y", today=self._REF_DATE) + assert result == 35 + + def test_dateutil_fallback(self): + # dateutil can parse "June 15, 1990" + result = age_in_years("June 15, 1990", today=self._REF_DATE) + assert result == 35 + + def test_unparseable_date_raises_value_error(self): + with pytest.raises(ValueError): + age_in_years("not-a-date", today=self._REF_DATE) + + def test_unparseable_date_with_fmt_raises_value_error(self): + with pytest.raises(ValueError): + age_in_years("not-a-date", fmt="%Y-%m-%d", today=self._REF_DATE) + + def test_returns_int(self): + result = age_in_years("1990-01-01", today=self._REF_DATE) + assert isinstance(result, int) + + def test_newborn(self): + result = age_in_years("2025-06-15", today=self._REF_DATE) + assert result == 0 + + def test_negative_age_not_possible_for_future_birthday_same_year(self): + # Born in the future relative to today (same year, later month) + result = age_in_years("2025-12-31", today=self._REF_DATE) + assert result == -1 + + def test_injectable_today_is_deterministic(self): + today_a = date(2024, 1, 1) + today_b = date(2025, 1, 1) + result_a = age_in_years("2000-06-01", today=today_a) + result_b = age_in_years("2000-06-01", today=today_b) + assert result_b == result_a + 1 diff --git a/tests/manager/api/test_authenticator.py b/tests/manager/api/test_authenticator.py index 1101cd1c89..d8143b1300 100644 --- a/tests/manager/api/test_authenticator.py +++ b/tests/manager/api/test_authenticator.py @@ -33,6 +33,7 @@ Keyboards, LibraryIdenfitierRestrictionField, LibraryIdentifierRestriction, + RemoteAuthResult, ) from palace.manager.api.authentication.basic_token import ( BasicTokenAuthenticationProvider, @@ -154,7 +155,7 @@ def login_button_image(self) -> str | None: return "login.png" def remote_authenticate(self, username, password): - return self.patrondata + return RemoteAuthResult(patron_data=self.patrondata) def remote_patron_lookup(self, patrondata): return self.lookup_patrondata @@ -1894,7 +1895,9 @@ def test_authenticated_patron_only_calls_remote_patron_lookup_once( # 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.remote_authenticate = MagicMock( + return_value=RemoteAuthResult(patron_data=auth_return) + ) if isinstance(enforce_return, ProblemDetail): provider.enforce_library_identifier_restriction = MagicMock( side_effect=ProblemDetailException(enforce_return) diff --git a/tests/manager/integration/patron_auth/sip2/test_provider.py b/tests/manager/integration/patron_auth/sip2/test_provider.py index f1fe70a19f..57b8a1c078 100644 --- a/tests/manager/integration/patron_auth/sip2/test_provider.py +++ b/tests/manager/integration/patron_auth/sip2/test_provider.py @@ -4,9 +4,11 @@ from decimal import Decimal from functools import partial from typing import cast +from unittest.mock import MagicMock, patch import pytest +from palace.manager.api.admin.problem_details import INVALID_CONFIGURATION_OPTION from palace.manager.api.authentication.base import PatronData from palace.manager.api.authentication.basic import ( BasicAuthProviderLibrarySettings, @@ -27,6 +29,7 @@ SIP2LibrarySettings, SIP2Settings, ) +from palace.manager.sqlalchemy.model.patron import Patron from palace.manager.util.problem_detail import ProblemDetail, ProblemDetailException from tests.fixtures.database import DatabaseTransactionFixture from tests.mocks.sip import MockSIPClient @@ -225,7 +228,8 @@ def test_remote_authenticate( # Some examples taken from a Sierra SIP API. client.queue_response(self.sierra_valid_login) client.queue_response(self.end_session_response) - patrondata = provider.remote_authenticate("user", "pass") + result = provider.remote_authenticate("user", "pass") + patrondata = result.patron_data assert isinstance(patrondata, PatronData) assert "12345" == patrondata.authorization_identifier assert "foo@example.com" == patrondata.email_address @@ -237,7 +241,7 @@ def test_remote_authenticate( client.queue_response(self.sierra_invalid_login) client.queue_response(self.end_session_response) - assert provider.remote_authenticate("user", "pass") is None + assert provider.remote_authenticate("user", "pass").patron_data is None # Since Sierra provides both the patron's fine amount and the # maximum allowable amount, we can determine just by looking @@ -245,14 +249,16 @@ def test_remote_authenticate( # fines. client.queue_response(self.sierra_excessive_fines) client.queue_response(self.end_session_response) - patrondata = provider.remote_authenticate("user", "pass") + result = provider.remote_authenticate("user", "pass") + patrondata = result.patron_data assert isinstance(patrondata, PatronData) assert PatronData.EXCESSIVE_FINES == patrondata.block_reason # A patron with an expired card. client.queue_response(self.evergreen_expired_card) client.queue_response(self.end_session_response) - patrondata = provider.remote_authenticate("user", "pass") + result = provider.remote_authenticate("user", "pass") + patrondata = result.patron_data assert isinstance(patrondata, PatronData) assert "12345" == patrondata.authorization_identifier # SIP extension field XI becomes sipserver_internal_id which @@ -266,7 +272,8 @@ def test_remote_authenticate( # A patron with excessive fines client.queue_response(self.evergreen_excessive_fines) client.queue_response(self.end_session_response) - patrondata = provider.remote_authenticate("user", "pass") + result = provider.remote_authenticate("user", "pass") + patrondata = result.patron_data assert isinstance(patrondata, PatronData) assert "12345" == patrondata.authorization_identifier assert "863718" == patrondata.permanent_id @@ -288,20 +295,23 @@ def test_remote_authenticate( # still borrow books. client.queue_response(self.evergreen_hold_privileges_denied) client.queue_response(self.end_session_response) - patrondata = provider.remote_authenticate("user", "pass") + result = provider.remote_authenticate("user", "pass") + patrondata = result.patron_data assert isinstance(patrondata, PatronData) assert PatronData.NO_VALUE == patrondata.block_reason client.queue_response(self.evergreen_card_reported_lost) client.queue_response(self.end_session_response) - patrondata = provider.remote_authenticate("user", "pass") + result = provider.remote_authenticate("user", "pass") + patrondata = result.patron_data assert isinstance(patrondata, PatronData) assert PatronData.CARD_REPORTED_LOST == patrondata.block_reason # Some examples taken from a Polaris instance. client.queue_response(self.polaris_valid_pin) client.queue_response(self.end_session_response) - patrondata = provider.remote_authenticate("user", "pass") + result = provider.remote_authenticate("user", "pass") + patrondata = result.patron_data assert isinstance(patrondata, PatronData) assert "25891000331441" == patrondata.authorization_identifier assert "foo@bar.com" == patrondata.email_address @@ -311,18 +321,20 @@ def test_remote_authenticate( client.queue_response(self.polaris_wrong_pin) client.queue_response(self.end_session_response) - patrondata = provider.remote_authenticate("user", "pass") - assert patrondata is None + result = provider.remote_authenticate("user", "pass") + assert result.patron_data is None client.queue_response(self.polaris_expired_card) client.queue_response(self.end_session_response) - patrondata = provider.remote_authenticate("user", "pass") + result = provider.remote_authenticate("user", "pass") + patrondata = result.patron_data assert isinstance(patrondata, PatronData) assert datetime(2016, 10, 25, 23, 59, 59) == patrondata.authorization_expires client.queue_response(self.polaris_excess_fines) client.queue_response(self.end_session_response) - patrondata = provider.remote_authenticate("user", "pass") + result = provider.remote_authenticate("user", "pass") + patrondata = result.patron_data assert isinstance(patrondata, PatronData) assert 11.50 == patrondata.fines @@ -331,15 +343,13 @@ def test_remote_authenticate( # valid_patron_password='N' when that happens. client.queue_response(self.polaris_no_such_patron) client.queue_response(self.end_session_response) - patrondata = provider.remote_authenticate("user", "pass") - assert patrondata is None + assert provider.remote_authenticate("user", "pass").patron_data is None # And once on an ILS that leaves valid_patron_password blank # when that happens. client.queue_response(self.tlc_no_such_patron) client.queue_response(self.end_session_response) - patrondata = provider.remote_authenticate("user", "pass") - assert patrondata is None + assert provider.remote_authenticate("user", "pass").patron_data is None def test_remote_authenticate_no_password( self, @@ -354,7 +364,8 @@ def test_remote_authenticate_no_password( # This Evergreen instance doesn't use passwords. client.queue_response(self.evergreen_active_user) client.queue_response(self.end_session_response) - patrondata = provider.remote_authenticate("user", None) + result = provider.remote_authenticate("user", None) + patrondata = result.patron_data assert isinstance(patrondata, PatronData) assert "12345" == patrondata.authorization_identifier assert "863715" == patrondata.permanent_id @@ -366,7 +377,8 @@ def test_remote_authenticate_no_password( # If a password is specified, it is not sent over the wire. client.queue_response(self.evergreen_active_user) client.queue_response(self.end_session_response) - patrondata = provider.remote_authenticate("user2", "some password") + result = provider.remote_authenticate("user2", "some password") + patrondata = result.patron_data assert isinstance(patrondata, PatronData) assert "12345" == patrondata.authorization_identifier request = client.requests[-1] @@ -391,7 +403,8 @@ def test_remote_authenticate_location_restriction( # This patron has the CORRECT location. client.queue_response(self.evergreen_patron_with_location) client.queue_response(self.end_session_response) - patrondata = provider.remote_authenticate("user", "pass") + result = provider.remote_authenticate("user", "pass") + patrondata = result.patron_data assert isinstance(patrondata, PatronData) patrondata = provider.enforce_library_identifier_restriction(patrondata) assert isinstance(patrondata, PatronData) @@ -400,7 +413,8 @@ def test_remote_authenticate_location_restriction( # 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") + result = provider.remote_authenticate("user", "pass") + patrondata = result.patron_data assert isinstance(patrondata, PatronData) with pytest.raises(ProblemDetailException) as exc: provider.enforce_library_identifier_restriction(patrondata) @@ -411,7 +425,8 @@ def test_remote_authenticate_location_restriction( # 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") + result = provider.remote_authenticate("user", "pass") + patrondata = result.patron_data assert isinstance(patrondata, PatronData) with pytest.raises(ProblemDetailException) as exc: provider.enforce_library_identifier_restriction(patrondata) @@ -438,7 +453,8 @@ def test_encoding( # as opposed to the CP850 version. client.queue_response(self.sierra_valid_login_utf8) client.queue_response(self.end_session_response) - patrondata = provider.remote_authenticate("user", "pass") + result = provider.remote_authenticate("user", "pass") + patrondata = result.patron_data # We're able to parse the message from the server and parse # out patron data, including the É character, with the proper @@ -470,10 +486,11 @@ def connect(self): ) provider = create_provider(client=CannotConnect, settings=settings) - response = provider.remote_authenticate( + result = provider.remote_authenticate( "username", "password", ) + response = result.patron_data assert isinstance(response, ProblemDetail) assert response.status_code == INVALID_CREDENTIALS.status_code @@ -501,10 +518,11 @@ def do_send(self, data): ) provider = create_provider(client=CannotSend, settings=settings) - response = provider.remote_authenticate( + result = provider.remote_authenticate( "username", "password", ) + response = result.patron_data assert isinstance(response, ProblemDetail) assert response.status_code == INVALID_CREDENTIALS.status_code @@ -997,3 +1015,578 @@ def test_patron_debug_no_such_patron( labels = [r.label for r in results] assert "Library Identifier Restriction" not in labels assert "Parsed Patron Data" not in labels + + +class TestSIP2AuthenticateWithBlockingRules: + """Tests that the blocking-rules hook fires correctly through a concrete + SIP2 provider instance. + + Rules must be valid simpleeval boolean expressions, e.g. ``True``, + ``False``, or expressions such as ``{fines} > 10``. + + **Test isolation**: the tests that patch ``_do_authenticate`` bypass + ``remote_authenticate``, so extra_context is empty and the provider falls + back to ``build_runtime_values_from_patron``. Tests that want to exercise + the full SIP2-response path pass the SIP2 dict as the second element of + the ``_do_authenticate`` return tuple. + """ + + _PATCH_TARGET = ( + "palace.manager.api.authentication.basic." + "BasicAuthenticationProvider._do_authenticate" + ) + + def test_authenticate_blocked_when_rule_is_true( + self, + create_provider: Callable[..., SIP2AuthenticationProvider], + create_library_settings: Callable[..., SIP2LibrarySettings], + ) -> None: + """When the ILS authenticates the patron and a rule evaluates True, + authenticate returns a ProblemDetail (HTTP 403) instead of the Patron.""" + library_settings = create_library_settings( + patron_blocking_rules=[ + {"name": "block-all", "rule": "True", "message": "Library A blocks."} + ] + ) + provider = create_provider(library_settings=library_settings) + + mock_patron = MagicMock(spec=Patron) + with patch(self._PATCH_TARGET, return_value=(mock_patron, {})): + result = provider.authenticate( + MagicMock(), {"username": "u", "password": "p"} + ) + + assert isinstance(result, ProblemDetail) + assert result.status_code == 403 + assert result.detail == "Library A blocks." + + def test_authenticate_not_blocked_when_rule_is_false( + self, + create_provider: Callable[..., SIP2AuthenticationProvider], + create_library_settings: Callable[..., SIP2LibrarySettings], + ) -> None: + """When a rule evaluates False, authenticate returns the Patron unchanged.""" + library_settings = create_library_settings( + patron_blocking_rules=[{"name": "never-block", "rule": "False"}] + ) + provider = create_provider(library_settings=library_settings) + + mock_patron = MagicMock(spec=Patron) + with patch(self._PATCH_TARGET, return_value=(mock_patron, {})): + result = provider.authenticate( + MagicMock(), {"username": "u", "password": "p"} + ) + + assert result is mock_patron + + def test_authenticate_no_rules_returns_patron( + self, + create_provider: Callable[..., SIP2AuthenticationProvider], + ) -> None: + """With no blocking rules configured, authenticate returns the Patron normally.""" + provider = create_provider() + assert provider.patron_blocking_rules == [] + + mock_patron = MagicMock(spec=Patron) + with patch(self._PATCH_TARGET, return_value=(mock_patron, {})): + result = provider.authenticate( + MagicMock(), {"username": "u", "password": "p"} + ) + + assert result is mock_patron + + def test_authenticate_passes_through_none( + self, + create_provider: Callable[..., SIP2AuthenticationProvider], + create_library_settings: Callable[..., SIP2LibrarySettings], + ) -> None: + """If the ILS rejects credentials (None), blocking rules are skipped.""" + library_settings = create_library_settings( + patron_blocking_rules=[{"name": "block-all", "rule": "True"}] + ) + provider = create_provider(library_settings=library_settings) + + with patch(self._PATCH_TARGET, return_value=(None, {})): + result = provider.authenticate( + MagicMock(), {"username": "u", "password": "p"} + ) + + assert result is None + + def test_authenticate_passes_through_problem_detail( + self, + create_provider: Callable[..., SIP2AuthenticationProvider], + create_library_settings: Callable[..., SIP2LibrarySettings], + ) -> None: + """If the ILS returns a ProblemDetail (e.g. server error), + blocking rules are skipped and the original ProblemDetail is returned.""" + library_settings = create_library_settings( + patron_blocking_rules=[{"name": "block-all", "rule": "True"}] + ) + provider = create_provider(library_settings=library_settings) + + base_problem = INVALID_CREDENTIALS.detailed("Server error.") + with patch(self._PATCH_TARGET, return_value=(base_problem, {})): + result = provider.authenticate( + MagicMock(), {"username": "u", "password": "p"} + ) + + assert result is base_problem + + def test_patron_blocking_rules_stored_on_provider( + self, + create_provider: Callable[..., SIP2AuthenticationProvider], + create_library_settings: Callable[..., SIP2LibrarySettings], + ) -> None: + """patron_blocking_rules attribute is populated from library_settings.""" + rules = [ + {"name": "r1", "rule": "True", "message": "Msg"}, + {"name": "r2", "rule": "False"}, + ] + library_settings = create_library_settings(patron_blocking_rules=rules) + provider = create_provider(library_settings=library_settings) + + assert len(provider.patron_blocking_rules) == 2 + assert provider.patron_blocking_rules[0].name == "r1" + assert provider.patron_blocking_rules[0].rule == "True" + assert provider.patron_blocking_rules[1].name == "r2" + + # ------------------------------------------------------------------ + # simpleeval runtime evaluation tests + # ------------------------------------------------------------------ + + def test_rule_true_with_message_returns_that_message( + self, + create_provider: Callable[..., SIP2AuthenticationProvider], + create_library_settings: Callable[..., SIP2LibrarySettings], + ) -> None: + """Library A rule evaluates True → ProblemDetail carries the rule's message.""" + library_settings = create_library_settings( + patron_blocking_rules=[ + { + "name": "always-block", + "rule": "True", + "message": "You are not permitted to borrow at this branch.", + } + ] + ) + provider = create_provider(library_settings=library_settings) + + mock_patron = MagicMock(spec=Patron) + mock_patron.fines = None + mock_patron.external_type = None + with patch(self._PATCH_TARGET, return_value=(mock_patron, {})): + result = provider.authenticate( + MagicMock(), {"username": "u", "password": "p"} + ) + + assert isinstance(result, ProblemDetail) + assert result.status_code == 403 + assert result.detail == "You are not permitted to borrow at this branch." + + def test_rule_true_without_message_uses_default( + self, + create_provider: Callable[..., SIP2AuthenticationProvider], + create_library_settings: Callable[..., SIP2LibrarySettings], + ) -> None: + """When the blocking rule has no message, the default message is used.""" + library_settings = create_library_settings( + patron_blocking_rules=[{"name": "always-block", "rule": "True"}] + ) + provider = create_provider(library_settings=library_settings) + + mock_patron = MagicMock(spec=Patron) + mock_patron.fines = None + mock_patron.external_type = None + with patch(self._PATCH_TARGET, return_value=(mock_patron, {})): + result = provider.authenticate( + MagicMock(), {"username": "u", "password": "p"} + ) + + assert isinstance(result, ProblemDetail) + assert result.detail == "Patron is blocked by library policy." + + def test_library_with_no_rules_does_not_block( + self, + create_provider: Callable[..., SIP2AuthenticationProvider], + ) -> None: + """Library B configured with no rules: patron passes through unchanged.""" + provider_b = create_provider() + assert provider_b.patron_blocking_rules == [] + + mock_patron = MagicMock(spec=Patron) + mock_patron.fines = None + mock_patron.external_type = None + with patch(self._PATCH_TARGET, return_value=(mock_patron, {})): + result = provider_b.authenticate( + MagicMock(), {"username": "u", "password": "p"} + ) + + assert result is mock_patron + + def test_per_library_isolation( + self, + create_provider: Callable[..., SIP2AuthenticationProvider], + create_library_settings: Callable[..., SIP2LibrarySettings], + ) -> None: + """Rules from Library A do not affect Library B (separate provider instances).""" + # Library A: has a blocking rule + settings_a = create_library_settings( + patron_blocking_rules=[ + {"name": "block-all", "rule": "True", "message": "Library A blocks."} + ] + ) + provider_a = create_provider(library_settings=settings_a) + + # Library B: no rules + provider_b = create_provider() + + mock_patron = MagicMock(spec=Patron) + mock_patron.fines = None + mock_patron.external_type = None + + with patch(self._PATCH_TARGET, return_value=(mock_patron, {})): + result_a = provider_a.authenticate( + MagicMock(), {"username": "u", "password": "p"} + ) + result_b = provider_b.authenticate( + MagicMock(), {"username": "u", "password": "p"} + ) + + assert isinstance(result_a, ProblemDetail) + assert result_a.detail == "Library A blocks." + assert result_b is mock_patron + + def test_fines_placeholder_blocks_high_fines( + self, + create_provider: Callable[..., SIP2AuthenticationProvider], + create_library_settings: Callable[..., SIP2LibrarySettings], + ) -> None: + """A rule using {fines} blocks a patron with high fines (patron-fallback path). + + ``_do_authenticate`` is patched with empty extra_context; + ``fines`` is resolved via ``build_runtime_values_from_patron`` from the + Patron model's ``fines`` attribute. + """ + library_settings = create_library_settings( + patron_blocking_rules=[ + { + "name": "fines-check", + "rule": "{fines} > 10.0", + "message": "Outstanding fines too high.", + } + ] + ) + provider = create_provider(library_settings=library_settings) + + mock_patron = MagicMock(spec=Patron) + mock_patron.fines = "25.00" + mock_patron.external_type = None + with patch(self._PATCH_TARGET, return_value=(mock_patron, {})): + result = provider.authenticate( + MagicMock(), {"username": "u", "password": "p"} + ) + + assert isinstance(result, ProblemDetail) + assert result.detail == "Outstanding fines too high." + + def test_fines_placeholder_allows_low_fines( + self, + create_provider: Callable[..., SIP2AuthenticationProvider], + create_library_settings: Callable[..., SIP2LibrarySettings], + ) -> None: + """A rule using {fines} allows a patron with low fines (patron-fallback path).""" + library_settings = create_library_settings( + patron_blocking_rules=[ + { + "name": "fines-check", + "rule": "{fines} > 10.0", + } + ] + ) + provider = create_provider(library_settings=library_settings) + + mock_patron = MagicMock(spec=Patron) + mock_patron.fines = "3.50" + mock_patron.external_type = None + with patch(self._PATCH_TARGET, return_value=(mock_patron, {})): + result = provider.authenticate( + MagicMock(), {"username": "u", "password": "p"} + ) + + assert result is mock_patron + + # ------------------------------------------------------------------ + # Raw SIP2 response fields accessible in rules + # ------------------------------------------------------------------ + + def test_raw_sip2_field_blocks_when_rule_matches( + self, + create_provider: Callable[..., SIP2AuthenticationProvider], + create_library_settings: Callable[..., SIP2LibrarySettings], + ) -> None: + """When extra_context carries the full SIP2 response dict, any raw + SIP2 field can be referenced in a blocking rule. + + This simulates what happens at runtime when ``remote_authenticate`` + has already been called and cached the info dict: the rule engine + receives every field returned by the ILS, not just the subset that + exists on the Patron DB model. + """ + library_settings = create_library_settings( + patron_blocking_rules=[ + { + "name": "patron-class-check", + "rule": "{sipserver_patron_class} == 'restricted'", + "message": "Restricted patrons may not borrow.", + } + ] + ) + provider = create_provider(library_settings=library_settings) + mock_patron = MagicMock(spec=Patron) + + sip2_info = { + "sipserver_patron_class": "restricted", + "fee_amount": "0.00", + } + + with patch( + self._PATCH_TARGET, + return_value=(mock_patron, sip2_info), + ): + result = provider.authenticate( + MagicMock(), {"username": "u", "password": "p"} + ) + + assert isinstance(result, ProblemDetail) + assert result.status_code == 403 + assert result.detail == "Restricted patrons may not borrow." + + def test_raw_sip2_field_allows_when_rule_does_not_match( + self, + create_provider: Callable[..., SIP2AuthenticationProvider], + create_library_settings: Callable[..., SIP2LibrarySettings], + ) -> None: + """When extra_context is populated and the rule evaluates False, + the patron is allowed through unchanged.""" + library_settings = create_library_settings( + patron_blocking_rules=[ + { + "name": "patron-class-check", + "rule": "{sipserver_patron_class} == 'restricted'", + "message": "Restricted patrons may not borrow.", + } + ] + ) + provider = create_provider(library_settings=library_settings) + mock_patron = MagicMock(spec=Patron) + + sip2_info = { + "sipserver_patron_class": "adult", + "fee_amount": "0.00", + } + + with patch( + self._PATCH_TARGET, + return_value=(mock_patron, sip2_info), + ): + result = provider.authenticate( + MagicMock(), {"username": "u", "password": "p"} + ) + + assert result is mock_patron + + def test_fines_normalised_from_sip2_fee_amount( + self, + create_provider: Callable[..., SIP2AuthenticationProvider], + create_library_settings: Callable[..., SIP2LibrarySettings], + ) -> None: + """When extra_context is populated, {fines} is derived from the + ``fee_amount`` field of the SIP2 response via ``build_values_from_sip2_info`` + rather than from the Patron DB model. + + This verifies that the normalised ``fines`` float takes precedence over + whatever might be stored on the Patron record. + """ + library_settings = create_library_settings( + patron_blocking_rules=[ + { + "name": "fines-check", + "rule": "{fines} > 10.0", + "message": "Fines too high (SIP2 path).", + } + ] + ) + provider = create_provider(library_settings=library_settings) + mock_patron = MagicMock(spec=Patron) + # The patron model has no fines — only the SIP2 response carries them. + mock_patron.fines = None + mock_patron.external_type = None + + sip2_info = {"fee_amount": "$25.00"} + + with patch( + self._PATCH_TARGET, + return_value=(mock_patron, sip2_info), + ): + result = provider.authenticate( + MagicMock(), {"username": "u", "password": "p"} + ) + + assert isinstance(result, ProblemDetail) + assert result.detail == "Fines too high (SIP2 path)." + + def test_sip2_info_takes_precedence_over_patron_model( + self, + create_provider: Callable[..., SIP2AuthenticationProvider], + create_library_settings: Callable[..., SIP2LibrarySettings], + ) -> None: + """When extra_context is populated, its ``fee_amount`` is used + to compute ``{fines}`` even when the Patron model has a different value.""" + library_settings = create_library_settings( + patron_blocking_rules=[ + { + "name": "fines-check", + "rule": "{fines} > 10.0", + "message": "High fines.", + } + ] + ) + provider = create_provider(library_settings=library_settings) + mock_patron = MagicMock(spec=Patron) + # Patron model says 50 — well above the threshold. + mock_patron.fines = "50.00" + mock_patron.external_type = None + + sip2_info = {"fee_amount": "$3.00"} + + with patch( + self._PATCH_TARGET, + return_value=(mock_patron, sip2_info), + ): + result = provider.authenticate( + MagicMock(), {"username": "u", "password": "p"} + ) + + # SIP2 data wins; rule evaluates False → patron allowed. + assert result is mock_patron + + +class TestFetchLiveRuleValidationValues: + """Unit tests for SIP2AuthenticationProvider.fetch_live_rule_validation_values. + + Each test uses a real (non-mocked) SIP2Settings instance and patches only + ``patron_information`` on the class to avoid any real network calls. + """ + + _PATRON_INFO_PATCH = ( + "palace.manager.integration.patron_auth.sip2.provider" + ".SIP2AuthenticationProvider.patron_information" + ) + + @pytest.fixture + def settings_with_identifier( + self, create_settings: Callable[..., SIP2Settings] + ) -> SIP2Settings: + return create_settings(test_identifier="patron1", test_password="secret") + + @pytest.fixture + def settings_without_identifier( + self, create_settings: Callable[..., SIP2Settings] + ) -> SIP2Settings: + return create_settings(test_identifier=None) + + def test_raises_when_test_identifier_not_configured( + self, + settings_without_identifier: SIP2Settings, + ) -> None: + """When test_identifier is absent, ProblemDetailException is raised before + any network call is made.""" + with pytest.raises(ProblemDetailException) as exc_info: + SIP2AuthenticationProvider.fetch_live_rule_validation_values( + settings_without_identifier + ) + + problem = exc_info.value.problem_detail + assert problem.uri == INVALID_CONFIGURATION_OPTION.uri + assert "test identifier" in (problem.detail or "").lower() + + def test_raises_when_patron_information_raises( + self, + settings_with_identifier: SIP2Settings, + ) -> None: + """When patron_information raises any exception (e.g. network error), + it is wrapped in a ProblemDetailException.""" + with patch( + self._PATRON_INFO_PATCH, + side_effect=OSError("Connection refused"), + ): + with pytest.raises(ProblemDetailException) as exc_info: + SIP2AuthenticationProvider.fetch_live_rule_validation_values( + settings_with_identifier + ) + + problem = exc_info.value.problem_detail + assert problem.uri == INVALID_CONFIGURATION_OPTION.uri + assert "Connection refused" in (problem.detail or "") + + def test_raises_when_patron_information_returns_problem_detail( + self, + settings_with_identifier: SIP2Settings, + ) -> None: + """When patron_information returns a ProblemDetail (e.g. bad credentials), + it is raised as a ProblemDetailException with a descriptive message.""" + sip2_error = INVALID_CREDENTIALS.detailed("Patron not found") + with patch(self._PATRON_INFO_PATCH, return_value=sip2_error): + with pytest.raises(ProblemDetailException) as exc_info: + SIP2AuthenticationProvider.fetch_live_rule_validation_values( + settings_with_identifier + ) + + problem = exc_info.value.problem_detail + assert problem.uri == INVALID_CONFIGURATION_OPTION.uri + assert "patron1" in (problem.detail or "") + assert "Patron not found" in (problem.detail or "") + + def test_returns_values_dict_on_success( + self, + settings_with_identifier: SIP2Settings, + ) -> None: + """When patron_information succeeds, the raw SIP2 info dict is passed through + build_values_from_sip2_info and returned.""" + sip2_info = {"fee_amount": "$5.00", "sipserver_patron_class": "adult"} + with patch(self._PATRON_INFO_PATCH, return_value=sip2_info): + result = SIP2AuthenticationProvider.fetch_live_rule_validation_values( + settings_with_identifier + ) + + assert result["fee_amount"] == "$5.00" + assert result["sipserver_patron_class"] == "adult" + assert result["fines"] == pytest.approx(5.0) + + def test_uses_test_password_when_present( + self, + create_settings: Callable[..., SIP2Settings], + ) -> None: + """patron_information is called with test_identifier and test_password.""" + settings = create_settings(test_identifier="user1", test_password="mypass") + sip2_info = {"fee_amount": "0.00"} + + with patch(self._PATRON_INFO_PATCH, return_value=sip2_info) as mock_pi: + SIP2AuthenticationProvider.fetch_live_rule_validation_values(settings) + + mock_pi.assert_called_once_with("user1", "mypass") + + def test_uses_empty_string_when_no_test_password( + self, + create_settings: Callable[..., SIP2Settings], + ) -> None: + """When test_password is not set, patron_information is called with an + empty string rather than None.""" + settings = create_settings(test_identifier="user1", test_password=None) + sip2_info = {"fee_amount": "0.00"} + + with patch(self._PATRON_INFO_PATCH, return_value=sip2_info) as mock_pi: + SIP2AuthenticationProvider.fetch_live_rule_validation_values(settings) + + mock_pi.assert_called_once_with("user1", "") diff --git a/tests/manager/integration/patron_auth/test_kansas_patron.py b/tests/manager/integration/patron_auth/test_kansas_patron.py index 7d25d469b7..de0e1c397b 100644 --- a/tests/manager/integration/patron_auth/test_kansas_patron.py +++ b/tests/manager/integration/patron_auth/test_kansas_patron.py @@ -153,7 +153,8 @@ def test_parse_response(self, create_provider: Callable[..., MockAPI]): def test_remote_authenticate(self, create_provider: Callable[..., MockAPI]): provider = create_provider() provider.enqueue("authorization_response_good.xml") - patrondata = provider.remote_authenticate("1234", "4321") + result = provider.remote_authenticate("1234", "4321") + patrondata = result.patron_data assert isinstance(patrondata, PatronData) assert patrondata.authorization_identifier == "1234" assert patrondata.permanent_id == "1234" @@ -161,15 +162,14 @@ def test_remote_authenticate(self, create_provider: Callable[..., MockAPI]): assert patrondata.personal_name == "Montgomery Burns" provider.enqueue("authorization_response_bad.xml") - patrondata = provider.remote_authenticate("1234", "4321") - assert patrondata is None + assert provider.remote_authenticate("1234", "4321").patron_data is None provider.enqueue("authorization_response_no_status.xml") - patrondata = provider.remote_authenticate("1234", "4321") - assert patrondata is None + assert provider.remote_authenticate("1234", "4321").patron_data is None provider.enqueue("authorization_response_no_id.xml") - patrondata = provider.remote_authenticate("1234", "4321") + result = provider.remote_authenticate("1234", "4321") + patrondata = result.patron_data assert isinstance(patrondata, PatronData) assert patrondata.authorization_identifier == "1234" assert patrondata.permanent_id == "1234" @@ -177,12 +177,10 @@ def test_remote_authenticate(self, create_provider: Callable[..., MockAPI]): assert patrondata.personal_name == "Gee" provider.enqueue("authorization_response_empty_tag.xml") - patrondata = provider.remote_authenticate("1234", "4321") - assert patrondata is None + assert provider.remote_authenticate("1234", "4321").patron_data is None provider.enqueue("authorization_response_malformed.xml") - patrondata = provider.remote_authenticate("1234", "4321") - assert patrondata is None + assert provider.remote_authenticate("1234", "4321").patron_data is None def test_remote_patron_lookup( self, create_provider: Callable[..., MockAPI], db: DatabaseTransactionFixture diff --git a/tests/manager/integration/patron_auth/test_millenium_patron.py b/tests/manager/integration/patron_auth/test_millenium_patron.py index 5046997621..d1e6f59a0e 100644 --- a/tests/manager/integration/patron_auth/test_millenium_patron.py +++ b/tests/manager/integration/patron_auth/test_millenium_patron.py @@ -309,7 +309,7 @@ def test_remote_authenticate_no_such_barcode( ): provider = create_provider() provider.enqueue("pintest.no such barcode.html") - assert provider.remote_authenticate("wrong barcode", "pin") is None + assert provider.remote_authenticate("wrong barcode", "pin").patron_data is None def test_remote_authenticate_wrong_pin( self, @@ -317,7 +317,7 @@ def test_remote_authenticate_wrong_pin( ): provider = create_provider() provider.enqueue("pintest.bad.html") - assert provider.remote_authenticate("barcode", "wrong pin") is None + assert provider.remote_authenticate("barcode", "wrong pin").patron_data is None def test_remote_authenticate_correct_pin( self, @@ -327,7 +327,8 @@ def test_remote_authenticate_correct_pin( api.enqueue("pintest.good.html") barcode = "barcode1234567!" pin = "!correct pin<>@/" - patrondata = api.remote_authenticate(barcode, pin) + result = api.remote_authenticate(barcode, pin) + patrondata = result.patron_data assert isinstance(patrondata, PatronData) # The return value includes everything we know about the # authenticated patron, which isn't much. @@ -357,7 +358,8 @@ def test_remote_authenticate_correct_pin_POST( api.enqueue("pintest.good.html") barcode = "barcode1234567!" pin = "!correct pin<>@/" - patrondata = api.remote_authenticate(barcode, pin) + result = api.remote_authenticate(barcode, pin) + patrondata = result.patron_data # The return value includes everything we know about the # authenticated patron, which isn't much. assert patrondata is not None @@ -757,14 +759,14 @@ def test_authorization_without_password( # If the patron lookup succeeds, the user is authenticated # as that patron. api.enqueue("dump.success.html") - patrondata = api.remote_authenticate("44444444444447", None) + result = api.remote_authenticate("44444444444447", None) + patrondata = result.patron_data assert isinstance(patrondata, PatronData) assert "44444444444447" == patrondata.authorization_identifier # If it fails, the user is not authenticated. api.enqueue("dump.no such barcode.html") - patrondata = api.remote_authenticate("44444444444447", None) - assert patrondata is None + assert api.remote_authenticate("44444444444447", None).patron_data is None def test_authorization_family_name_success( self, @@ -777,7 +779,8 @@ def test_authorization_family_name_success( settings = create_settings(authentication_mode=AuthenticationMode.FAMILY_NAME) api = create_provider(settings=settings) api.enqueue("dump.success.html") - patrondata = api.remote_authenticate("44444444444447", "Sheldon") + result = api.remote_authenticate("44444444444447", "Sheldon") + patrondata = result.patron_data assert isinstance(patrondata, PatronData) assert "44444444444447" == patrondata.authorization_identifier @@ -796,7 +799,9 @@ def test_authorization_family_name_failure( settings = create_settings(authentication_mode=AuthenticationMode.FAMILY_NAME) api = create_provider(settings=settings) api.enqueue("dump.success.html") - assert api.remote_authenticate("44444444444447", "wrong name") is None + assert ( + api.remote_authenticate("44444444444447", "wrong name").patron_data is None + ) def test_authorization_family_name_no_such_patron( self, @@ -809,7 +814,7 @@ def test_authorization_family_name_no_such_patron( settings = create_settings(authentication_mode=AuthenticationMode.FAMILY_NAME) api = create_provider(settings=settings) api.enqueue("dump.no such barcode.html") - assert api.remote_authenticate("44444444444447", "somebody") is None + assert api.remote_authenticate("44444444444447", "somebody").patron_data is None def test_extract_postal_code(self): # Test our heuristics for extracting postal codes from address fields. diff --git a/tests/manager/integration/patron_auth/test_minimal_authentication.py b/tests/manager/integration/patron_auth/test_minimal_authentication.py index 6c471e9d8a..73b1686cca 100644 --- a/tests/manager/integration/patron_auth/test_minimal_authentication.py +++ b/tests/manager/integration/patron_auth/test_minimal_authentication.py @@ -73,15 +73,16 @@ def test_remote_authenticate( provider = minimal_auth_fixture.provider() result = provider.remote_authenticate(username, password) + patrondata = result.patron_data if expect_success: - assert isinstance(result, PatronData) - assert result.authorization_identifier is not None - assert result.permanent_id is not None - assert result.username is not None - assert result.personal_name is not None + assert isinstance(patrondata, PatronData) + assert patrondata.authorization_identifier is not None + assert patrondata.permanent_id is not None + assert patrondata.username is not None + assert patrondata.personal_name is not None else: - assert result is None + assert patrondata is None @pytest.mark.parametrize( "authorization_identifier", @@ -200,11 +201,12 @@ def test_remote_authenticate_creates_consistent_patrondata( username = "testuser" result = provider.remote_authenticate(username, "any_password") + patrondata = result.patron_data # Should match what generate_patrondata produces expected = MinimalAuthenticationProvider.generate_patrondata(username) - assert result.authorization_identifier == expected.authorization_identifier - assert result.username == expected.username - assert result.permanent_id == expected.permanent_id - assert result.personal_name == expected.personal_name + assert patrondata.authorization_identifier == expected.authorization_identifier + assert patrondata.username == expected.username + assert patrondata.permanent_id == expected.permanent_id + assert patrondata.personal_name == expected.personal_name diff --git a/tests/manager/integration/patron_auth/test_patron_blocking.py b/tests/manager/integration/patron_auth/test_patron_blocking.py new file mode 100644 index 0000000000..d4b3b37378 --- /dev/null +++ b/tests/manager/integration/patron_auth/test_patron_blocking.py @@ -0,0 +1,551 @@ +"""Tests for the shared patron blocking-rules infrastructure. + +Covers: +- PatronBlockingRule model (patron_blocking.py) +- check_patron_blocking_rules() pure function (patron_blocking.py) +- check_patron_blocking_rules_with_evaluator() (patron_blocking.py) +- build_runtime_values_from_patron() (patron_blocking.py) +- patron_blocking_rules field on BasicAuthProviderLibrarySettings (basic.py) + including simpleeval validation and message-length checks +- supports_patron_blocking_rules flag on BasicAuthenticationProvider (basic.py) +- blocking-rules hook in BasicAuthenticationProvider.authenticate (basic.py) +""" + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +import pytest + +from palace.manager.api.authentication.basic import ( + BasicAuthenticationProvider, + BasicAuthProviderLibrarySettings, +) +from palace.manager.api.problem_details import BLOCKED_BY_POLICY, INVALID_CREDENTIALS +from palace.manager.integration.patron_auth.patron_blocking import ( + PatronBlockingRule, + build_runtime_values_from_patron, + build_values_from_sip2_info, + check_patron_blocking_rules_with_evaluator, +) +from palace.manager.integration.patron_auth.sip2.provider import ( + SIP2AuthenticationProvider, + SIP2LibrarySettings, +) +from palace.manager.sqlalchemy.model.patron import Patron +from palace.manager.util.problem_detail import ProblemDetail +from tests.fixtures.problem_detail import raises_problem_detail + +# --------------------------------------------------------------------------- +# PatronBlockingRule value object +# --------------------------------------------------------------------------- + + +class TestPatronBlockingRule: + """Unit tests for the PatronBlockingRule value object.""" + + def test_basic_construction(self) -> None: + rule = PatronBlockingRule(name="rule1", rule="True") + assert rule.name == "rule1" + assert rule.rule == "True" + assert rule.message is None + + def test_with_message(self) -> None: + rule = PatronBlockingRule(name="rule1", rule="True", message="You are blocked.") + assert rule.message == "You are blocked." + + def test_strips_whitespace(self) -> None: + rule = PatronBlockingRule(name=" rule1 ", rule=" True ") + assert rule.name == "rule1" + assert rule.rule == "True" + + def test_frozen(self) -> None: + rule = PatronBlockingRule(name="rule1", rule="True") + with pytest.raises(Exception): + rule.name = "other" # type: ignore[misc] + + def test_round_trip_dict(self) -> None: + rule = PatronBlockingRule(name="r", rule="True", message="msg") + restored = PatronBlockingRule.model_validate(rule.model_dump()) + assert restored == rule + + +# --------------------------------------------------------------------------- +# check_patron_blocking_rules_with_evaluator +# --------------------------------------------------------------------------- + + +class TestCheckPatronBlockingRulesWithEvaluator: + """Tests for the simpleeval-based check_patron_blocking_rules_with_evaluator().""" + + def test_empty_rules_returns_none(self) -> None: + assert check_patron_blocking_rules_with_evaluator([], {}) is None + + def test_true_rule_blocks(self) -> None: + rules = [PatronBlockingRule(name="always-block", rule="True")] + result = check_patron_blocking_rules_with_evaluator(rules, {}) + assert isinstance(result, ProblemDetail) + assert result.status_code == 403 + assert result.uri == BLOCKED_BY_POLICY.uri + + def test_true_rule_uses_custom_message(self) -> None: + rules = [ + PatronBlockingRule( + name="always-block", rule="True", message="Library A blocks you." + ) + ] + result = check_patron_blocking_rules_with_evaluator(rules, {}) + assert isinstance(result, ProblemDetail) + assert result.detail == "Library A blocks you." + + def test_true_rule_uses_default_message_when_none(self) -> None: + rules = [PatronBlockingRule(name="always-block", rule="True")] + result = check_patron_blocking_rules_with_evaluator(rules, {}) + assert isinstance(result, ProblemDetail) + assert result.detail == "Patron is blocked by library policy." + + def test_false_rule_does_not_block(self) -> None: + rules = [PatronBlockingRule(name="never-block", rule="False")] + assert check_patron_blocking_rules_with_evaluator(rules, {}) is None + + def test_expression_with_placeholder(self) -> None: + rules = [PatronBlockingRule(name="high-fines", rule="{fines} > 10.0")] + assert ( + check_patron_blocking_rules_with_evaluator(rules, {"fines": 15.0}) + is not None + ) + assert check_patron_blocking_rules_with_evaluator(rules, {"fines": 5.0}) is None + + def test_missing_placeholder_ignored_fail_open(self) -> None: + """A rule referencing a missing placeholder is ignored (fail-open).""" + rules = [PatronBlockingRule(name="needs-dob", rule="{dob} == '2000-01-01'")] + result = check_patron_blocking_rules_with_evaluator(rules, {}) + assert result is None + + def test_invalid_expression_ignored_fail_open(self) -> None: + """A syntactically invalid rule is ignored (fail-open).""" + rules = [PatronBlockingRule(name="bad-rule", rule="{fines} >>>!!! invalid")] + result = check_patron_blocking_rules_with_evaluator(rules, {"fines": 5.0}) + assert result is None + + def test_non_bool_result_ignored_fail_open(self) -> None: + """A rule that does not evaluate to bool is ignored (fail-open).""" + rules = [PatronBlockingRule(name="non-bool", rule="{fines} + 1")] + result = check_patron_blocking_rules_with_evaluator(rules, {"fines": 5.0}) + assert result is None + + def test_error_rule_then_blocking_rule_blocks(self) -> None: + """When an error rule is followed by a blocking rule, the blocking rule applies.""" + rules = [ + PatronBlockingRule(name="bad", rule="{missing} == 1"), + PatronBlockingRule(name="block", rule="True", message="Blocked."), + ] + result = check_patron_blocking_rules_with_evaluator(rules, {}) + assert isinstance(result, ProblemDetail) + assert result.detail == "Blocked." + + def test_first_matching_rule_wins(self) -> None: + rules = [ + PatronBlockingRule(name="r1", rule="True", message="First."), + PatronBlockingRule(name="r2", rule="True", message="Second."), + ] + result = check_patron_blocking_rules_with_evaluator(rules, {}) + assert isinstance(result, ProblemDetail) + assert result.detail == "First." + + def test_false_then_true_rule(self) -> None: + rules = [ + PatronBlockingRule(name="r1", rule="False"), + PatronBlockingRule(name="r2", rule="True", message="Second rule."), + ] + result = check_patron_blocking_rules_with_evaluator(rules, {}) + assert isinstance(result, ProblemDetail) + assert result.detail == "Second rule." + + def test_error_is_logged(self) -> None: + """Evaluation errors are logged server-side.""" + rules = [PatronBlockingRule(name="bad", rule="{missing_key} == 1")] + mock_log = MagicMock() + check_patron_blocking_rules_with_evaluator(rules, {}, log=mock_log) + assert mock_log.error.called + + +# --------------------------------------------------------------------------- +# build_runtime_values_from_patron +# --------------------------------------------------------------------------- + + +class TestBuildRuntimeValuesFromPatron: + def _make_patron(self, fines=None, external_type=None): + patron = MagicMock(spec=Patron) + patron.fines = fines + patron.external_type = external_type + return patron + + def test_fines_none_becomes_zero(self) -> None: + patron = self._make_patron(fines=None) + values = build_runtime_values_from_patron(patron) + assert values["fines"] == 0.0 + + def test_fines_string_converted_to_float(self) -> None: + patron = self._make_patron(fines="12.50") + values = build_runtime_values_from_patron(patron) + assert values["fines"] == 12.50 + + def test_fines_unparseable_becomes_zero(self) -> None: + patron = self._make_patron(fines="not-a-number") + values = build_runtime_values_from_patron(patron) + assert values["fines"] == 0.0 + + def test_patron_type_present(self) -> None: + patron = self._make_patron(external_type="student") + values = build_runtime_values_from_patron(patron) + assert values["patron_type"] == "student" + + def test_patron_type_none_becomes_empty_string(self) -> None: + patron = self._make_patron(external_type=None) + values = build_runtime_values_from_patron(patron) + assert values["patron_type"] == "" + + def test_returns_dict(self) -> None: + patron = self._make_patron() + values = build_runtime_values_from_patron(patron) + assert isinstance(values, dict) + + +# --------------------------------------------------------------------------- +# BasicAuthProviderLibrarySettings — patron_blocking_rules field validation +# --------------------------------------------------------------------------- + + +class TestBasicAuthLibrarySettingsBlockingRules: + """Tests for patron_blocking_rules on BasicAuthProviderLibrarySettings. + + The field lives on the base class so every basic-auth protocol (SIP2, + Millennium, SirsiDynix, …) inherits validation for free. + """ + + def test_default_is_empty_list(self) -> None: + settings = BasicAuthProviderLibrarySettings() + assert settings.patron_blocking_rules == [] + + def test_rules_rejected_when_provider_does_not_support(self) -> None: + """Providers that do not support blocking rules cannot have rules configured.""" + with raises_problem_detail() as info: + BasicAuthProviderLibrarySettings( + patron_blocking_rules=[{"name": "r", "rule": "True"}] + ) + assert info.value.detail is not None + assert "not supported" in info.value.detail.lower() + + def test_round_trip_with_valid_rules(self) -> None: + settings = SIP2LibrarySettings( + patron_blocking_rules=[ + {"name": "block-all", "rule": "True", "message": "Sorry"}, + {"name": "no-op", "rule": "False"}, + ] + ) + assert len(settings.patron_blocking_rules) == 2 + assert settings.patron_blocking_rules[0].name == "block-all" + assert settings.patron_blocking_rules[0].rule == "True" + assert settings.patron_blocking_rules[0].message == "Sorry" + assert settings.patron_blocking_rules[1].name == "no-op" + assert settings.patron_blocking_rules[1].message is None + + def test_model_dump_excludes_empty_list_by_default(self) -> None: + """Empty list (the default) is omitted from model_dump so we don't + store defaults in the JSON blob.""" + settings = BasicAuthProviderLibrarySettings() + assert "patron_blocking_rules" not in settings.model_dump() + + def test_model_dump_includes_non_empty_list(self) -> None: + settings = SIP2LibrarySettings( + patron_blocking_rules=[{"name": "r", "rule": "True"}] + ) + dumped = settings.model_dump() + assert "patron_blocking_rules" in dumped + assert len(dumped["patron_blocking_rules"]) == 1 + assert dumped["patron_blocking_rules"][0]["name"] == "r" + + def test_model_validate_missing_field_produces_default(self) -> None: + """A settings dict without the key deserialises to an empty list.""" + settings = BasicAuthProviderLibrarySettings.model_validate({}) + assert settings.patron_blocking_rules == [] + + def test_validate_empty_name_raises(self) -> None: + with raises_problem_detail() as info: + SIP2LibrarySettings(patron_blocking_rules=[{"name": "", "rule": "True"}]) + assert info.value.detail is not None + assert "index 0" in info.value.detail + assert "'name' must not be empty" in info.value.detail + + def test_validate_whitespace_only_name_raises(self) -> None: + # str_strip_whitespace=True on PatronBlockingRule strips " " to "" + with raises_problem_detail() as info: + SIP2LibrarySettings(patron_blocking_rules=[{"name": " ", "rule": "True"}]) + assert info.value.detail is not None + assert "index 0" in info.value.detail + assert "'name' must not be empty" in info.value.detail + + def test_validate_empty_rule_raises(self) -> None: + with raises_problem_detail() as info: + SIP2LibrarySettings( + patron_blocking_rules=[{"name": "valid-name", "rule": ""}] + ) + assert info.value.detail is not None + assert "index 0" in info.value.detail + assert "'rule' expression must not be empty" in info.value.detail + + def test_validate_duplicate_name_raises(self) -> None: + with raises_problem_detail() as info: + SIP2LibrarySettings( + patron_blocking_rules=[ + {"name": "same", "rule": "True"}, + {"name": "same", "rule": "False"}, + ] + ) + assert info.value.detail is not None + assert "index 1" in info.value.detail + assert "duplicate rule name" in info.value.detail + assert "'same'" in info.value.detail + + def test_validate_duplicate_at_higher_index(self) -> None: + """The error message cites the index of the second occurrence.""" + with raises_problem_detail() as info: + SIP2LibrarySettings( + patron_blocking_rules=[ + {"name": "a", "rule": "True"}, + {"name": "b", "rule": "False"}, + {"name": "a", "rule": "True"}, + ] + ) + assert info.value.detail is not None + assert "index 2" in info.value.detail + + # ------------------------------------------------------------------ + # New: simpleeval-based rule expression validation + # ------------------------------------------------------------------ + + def test_validate_rule_length_over_1000_raises(self) -> None: + """rule text > 1000 characters is rejected at validation time.""" + long_rule = "True and " * 200 # well over 1000 chars + with raises_problem_detail() as info: + SIP2LibrarySettings( + patron_blocking_rules=[{"name": "r", "rule": long_rule}] + ) + assert info.value.detail is not None + assert "index 0" in info.value.detail + + def test_validate_message_length_over_1000_raises(self) -> None: + """message > 1000 characters is rejected at validation time.""" + with raises_problem_detail() as info: + SIP2LibrarySettings( + patron_blocking_rules=[ + { + "name": "r", + "rule": "True", + "message": "x" * 1001, + } + ] + ) + assert info.value.detail is not None + assert "index 0" in info.value.detail + assert "message" in info.value.detail.lower() + + def test_validate_message_exactly_1000_chars_passes(self) -> None: + """message of exactly 1000 chars is accepted.""" + SIP2LibrarySettings( + patron_blocking_rules=[ + { + "name": "r", + "rule": "True", + "message": "x" * 1000, + } + ] + ) + + def test_validate_any_rule_expression_passes_static_check(self) -> None: + """Any non-empty rule expression that fits within 1000 chars is accepted + by static Pydantic validation.""" + settings = SIP2LibrarySettings( + patron_blocking_rules=[ + {"name": "fines-check", "rule": "{fines} > 10.0"}, + {"name": "any-field", "rule": "{totally_unknown_key} > 0"}, + {"name": "non-bool", "rule": "{fines} + 1"}, + ] + ) + assert len(settings.patron_blocking_rules) == 3 + + def test_validate_rule_exactly_1000_chars_passes(self) -> None: + """rule text of exactly 1000 chars is accepted.""" + rule = "T" * 1000 + settings = SIP2LibrarySettings( + patron_blocking_rules=[{"name": "r", "rule": rule}] + ) + assert settings.patron_blocking_rules[0].rule == rule + + +# --------------------------------------------------------------------------- +# supports_patron_blocking_rules flag + authenticate hook +# --------------------------------------------------------------------------- + + +class TestBasicAuthenticationProvider: + """Tests for BasicAuthenticationProvider.authenticate with patron blocking rules.""" + + _PATCH_TARGET = ( + "palace.manager.api.authentication.basic." + "BasicAuthenticationProvider._do_authenticate" + ) + + def test_base_class_flag_is_false(self) -> None: + assert BasicAuthenticationProvider.supports_patron_blocking_rules is False + + def test_sip2_flag_is_true(self) -> None: + assert SIP2AuthenticationProvider.supports_patron_blocking_rules is True + + def test_blocking_skipped_when_flag_false(self) -> None: + """When supports_patron_blocking_rules is False, a True rule is ignored + and the Patron object is returned unchanged.""" + mock_patron = MagicMock(spec=Patron) + + with patch(self._PATCH_TARGET, return_value=(mock_patron, {})): + provider = MagicMock(spec=BasicAuthenticationProvider) + provider.supports_patron_blocking_rules = False + provider.patron_blocking_rules = [ + PatronBlockingRule(name="block-all", rule="True") + ] + provider._do_authenticate = MagicMock(return_value=(mock_patron, {})) + result = BasicAuthenticationProvider.authenticate(provider, MagicMock(), {}) + + assert result is mock_patron + + def test_blocking_applied_when_flag_true(self) -> None: + """When supports_patron_blocking_rules is True, a True rule intercepts + the authenticated Patron and returns a 403 ProblemDetail.""" + mock_patron = MagicMock(spec=Patron) + + provider = MagicMock(spec=BasicAuthenticationProvider) + provider.supports_patron_blocking_rules = True + provider.patron_blocking_rules = [ + PatronBlockingRule( + name="block-all", rule="True", message="Blocked by policy." + ) + ] + provider._do_authenticate = MagicMock(return_value=(mock_patron, {})) + provider.log = MagicMock() + + result = BasicAuthenticationProvider.authenticate(provider, MagicMock(), {}) + + assert isinstance(result, ProblemDetail) + assert result.status_code == 403 + assert result.uri == BLOCKED_BY_POLICY.uri + assert result.detail == "Blocked by policy." + provider.log.info.assert_any_call("Patron blocking rules evaluation attempted") + + def test_blocking_not_applied_when_do_authenticate_returns_none(self) -> None: + """When _do_authenticate returns None (bad credentials), the flag has no + effect — None is passed through.""" + provider = MagicMock(spec=BasicAuthenticationProvider) + provider.supports_patron_blocking_rules = True + provider.patron_blocking_rules = [ + PatronBlockingRule(name="block-all", rule="True") + ] + provider._do_authenticate = MagicMock(return_value=(None, {})) + + result = BasicAuthenticationProvider.authenticate(provider, MagicMock(), {}) + + assert result is None + + def test_blocking_not_applied_when_do_authenticate_returns_problem_detail( + self, + ) -> None: + """When _do_authenticate itself returns a ProblemDetail (e.g. connection + failure), blocking rules are not evaluated — the original error is returned.""" + provider = MagicMock(spec=BasicAuthenticationProvider) + provider.supports_patron_blocking_rules = True + provider.patron_blocking_rules = [ + PatronBlockingRule(name="block-all", rule="True") + ] + provider._do_authenticate = MagicMock(return_value=(INVALID_CREDENTIALS, {})) + + result = BasicAuthenticationProvider.authenticate(provider, MagicMock(), {}) + + assert result is INVALID_CREDENTIALS + + def test_false_rule_does_not_block(self) -> None: + """A False rule does not block the patron.""" + mock_patron = MagicMock(spec=Patron) + + provider = MagicMock(spec=BasicAuthenticationProvider) + provider.supports_patron_blocking_rules = True + provider.patron_blocking_rules = [ + PatronBlockingRule(name="never-block", rule="False") + ] + provider._do_authenticate = MagicMock(return_value=(mock_patron, {})) + provider.log = MagicMock() + + result = BasicAuthenticationProvider.authenticate(provider, MagicMock(), {}) + + assert result is mock_patron + provider.log.info.assert_any_call("Patron blocking rules evaluation attempted") + + +# --------------------------------------------------------------------------- +# build_values_from_sip2_info +# --------------------------------------------------------------------------- + + +class TestBuildValuesFromSip2Info: + """Tests for build_values_from_sip2_info().""" + + def test_fee_amount_plain_float_string(self) -> None: + """fee_amount like '5.00' is parsed to a float under the 'fines' key.""" + values = build_values_from_sip2_info({"fee_amount": "5.00"}) + assert values["fines"] == pytest.approx(5.0) + + def test_fee_amount_dollar_prefix(self) -> None: + """fee_amount like '$12.50' (dollar sign prefix) is parsed correctly.""" + values = build_values_from_sip2_info({"fee_amount": "$12.50"}) + assert values["fines"] == pytest.approx(12.5) + + def test_fee_amount_missing_defaults_to_zero(self) -> None: + """Absent fee_amount → fines = 0.0.""" + values = build_values_from_sip2_info({}) + assert values["fines"] == pytest.approx(0.0) + + def test_fee_amount_unparseable_defaults_to_zero(self) -> None: + """Unparseable fee_amount → fines = 0.0 (no exception raised).""" + values = build_values_from_sip2_info({"fee_amount": "not-a-number"}) + assert values["fines"] == pytest.approx(0.0) + + def test_all_raw_sip2_fields_are_included(self) -> None: + """Every key in the raw info dict is present verbatim in the result.""" + info = { + "fee_amount": "3.50", + "sipserver_patron_class": "student", + "polaris_patron_birthdate": "2000-06-15", + "patron_status": "active", + "personal_name": "Jane Doe", + } + values = build_values_from_sip2_info(info) + # All raw keys must be present. + for k, v in info.items(): + assert values[k] == v + + def test_normalized_fines_added_alongside_raw_fee_amount(self) -> None: + """The 'fines' key (float) is added IN ADDITION to the raw fee_amount.""" + info = {"fee_amount": "3.50"} + values = build_values_from_sip2_info(info) + assert "fee_amount" in values # raw field preserved + assert values["fines"] == pytest.approx(3.5) # normalised key added + + def test_empty_info_dict_has_only_fines_key(self) -> None: + """An empty SIP2 response still produces the 'fines' key (defaulting to 0).""" + values = build_values_from_sip2_info({}) + assert values == {"fines": pytest.approx(0.0)} + + def test_polaris_patron_birthdate_accessible_directly(self) -> None: + """polaris_patron_birthdate is accessible under its own raw key.""" + values = build_values_from_sip2_info({"polaris_patron_birthdate": "1990-01-01"}) + assert values["polaris_patron_birthdate"] == "1990-01-01" diff --git a/tests/manager/integration/patron_auth/test_simple_auth.py b/tests/manager/integration/patron_auth/test_simple_auth.py index 0a76387934..446c30c26c 100644 --- a/tests/manager/integration/patron_auth/test_simple_auth.py +++ b/tests/manager/integration/patron_auth/test_simple_auth.py @@ -69,9 +69,10 @@ def test_simple( settings = create_settings() provider = create_provider(settings=settings) - assert provider.remote_authenticate("user", "wrongpass") is None - assert provider.remote_authenticate("user", None) is None - user = provider.remote_authenticate("barcode", "pass") + assert provider.remote_authenticate("user", "wrongpass").patron_data is None + assert provider.remote_authenticate("user", None).patron_data is None + result = provider.remote_authenticate("barcode", "pass") + user = result.patron_data assert isinstance(user, PatronData) assert "barcode" == user.authorization_identifier assert "barcode_id" == user.permanent_id @@ -91,15 +92,17 @@ def test_no_password_authentication( provider = create_provider(settings=settings) # If you don't provide a password, you're in. - user = provider.remote_authenticate("barcode", None) + result = provider.remote_authenticate("barcode", None) + user = result.patron_data assert isinstance(user, PatronData) - user2 = provider.remote_authenticate("barcode", "") + result2 = provider.remote_authenticate("barcode", "") + user2 = result2.patron_data assert isinstance(user2, PatronData) assert user2.authorization_identifier == user.authorization_identifier # If you provide any password, you're out. - assert provider.remote_authenticate("barcode", "pass") is None + assert provider.remote_authenticate("barcode", "pass").patron_data is None def test_additional_identifiers( self, @@ -111,31 +114,36 @@ def test_additional_identifiers( ) provider = create_provider(settings=settings) - assert provider.remote_authenticate("a", None) is None + assert provider.remote_authenticate("a", None).patron_data is None - user = provider.remote_authenticate("a", "pass") + result = provider.remote_authenticate("a", "pass") + user = result.patron_data assert isinstance(user, PatronData) assert "a" == user.authorization_identifier assert "a_id" == user.permanent_id assert "a_username" == user.username - user2 = provider.remote_authenticate("b", "pass") + result2 = provider.remote_authenticate("b", "pass") + user2 = result2.patron_data assert isinstance(user2, PatronData) assert "b" == user2.authorization_identifier assert "b_id" == user2.permanent_id assert "b_username" == user2.username # Users can also authenticate by their 'username' - user3 = provider.remote_authenticate("a_username", "pass") + result3 = provider.remote_authenticate("a_username", "pass") + user3 = result3.patron_data assert isinstance(user3, PatronData) assert "a" == user3.authorization_identifier - user4 = provider.remote_authenticate("b_username", "pass") + result4 = provider.remote_authenticate("b_username", "pass") + user4 = result4.patron_data assert isinstance(user4, PatronData) assert "b" == user4.authorization_identifier # The main user can still authenticate too. - user5 = provider.remote_authenticate("barcode", "pass") + result5 = provider.remote_authenticate("barcode", "pass") + user5 = result5.patron_data assert isinstance(user5, PatronData) assert "barcode" == user5.authorization_identifier diff --git a/tests/manager/integration/patron_auth/test_sirsidynix_auth_provider.py b/tests/manager/integration/patron_auth/test_sirsidynix_auth_provider.py index 63a0d5b4bd..f209d6c74f 100644 --- a/tests/manager/integration/patron_auth/test_sirsidynix_auth_provider.py +++ b/tests/manager/integration/patron_auth/test_sirsidynix_auth_provider.py @@ -11,7 +11,10 @@ from _pytest._code import ExceptionInfo from palace.manager.api.authentication.base import PatronData -from palace.manager.api.authentication.basic import LibraryIdentifierRestriction +from palace.manager.api.authentication.basic import ( + LibraryIdentifierRestriction, + RemoteAuthResult, +) from palace.manager.api.config import Configuration from palace.manager.api.problem_details import PATRON_OF_ANOTHER_LIBRARY from palace.manager.core.selftest import SelfTestResult @@ -195,7 +198,8 @@ def test_remote_authenticate(self, sirsi_auth_fixture: SirsiAuthFixture): 200, content=response_dict ) - response = provider.remote_authenticate("username", "pwd") + result = provider.remote_authenticate("username", "pwd") + response = result.patron_data assert type(response) == SirsiDynixPatronData assert response.authorization_identifier == "username" assert response.username == "username" @@ -204,15 +208,15 @@ def test_remote_authenticate(self, sirsi_auth_fixture: SirsiAuthFixture): sirsi_auth_fixture.mock_request.return_value = MockRequestsResponse( 401, content=response_dict ) - assert provider.remote_authenticate("username", "pwd") is None + assert provider.remote_authenticate("username", "pwd").patron_data is None def test_remote_authenticate_username_password_none( self, sirsi_auth_fixture: SirsiAuthFixture ): provider = sirsi_auth_fixture.provider() - response = provider.remote_authenticate("username", None) - assert response is None + result = provider.remote_authenticate("username", None) + assert result.patron_data is None def test_remote_patron_lookup(self, sirsi_auth_fixture: SirsiAuthFixture): provider_mock = sirsi_auth_fixture.provider_mocked_api() @@ -699,8 +703,10 @@ def test_full_auth_request( library_settings=library_settings, ) provider.remote_authenticate = MagicMock( - return_value=SirsiDynixPatronData( - permanent_id="xxxx", session_token="xxx", complete=False + return_value=RemoteAuthResult( + patron_data=SirsiDynixPatronData( + permanent_id="xxxx", session_token="xxx", complete=False + ) ) ) provider.remote_patron_lookup = MagicMock(