Skip to content

[PP-3772] Add patron blocking rules per library#3090

Open
dbernstein wants to merge 31 commits intomainfrom
feature/PP-3772-Add-patron-blocking-rules-per-library
Open

[PP-3772] Add patron blocking rules per library#3090
dbernstein wants to merge 31 commits intomainfrom
feature/PP-3772-Add-patron-blocking-rules-per-library

Conversation

@dbernstein
Copy link
Contributor

@dbernstein dbernstein commented Feb 27, 2026

High-level summary

This PR adds a per-library patron blocking rules system for patron authentication, with the first production use wired into the SIP2 authentication provider. The implementation is intentionally structured so the rule model and evaluation hook live at the BasicAuthenticationProvider layer, making it reusable by other basic-auth providers later, even though only SIP2 opts in right now.

At a feature level, the PR introduces a small rules engine for evaluating blocking expressions, stores rule definitions in each library’s existing auth settings, and checks those rules during patron authentication before returning a patron object. It also adds admin-side validation, including validation against live SIP2 test data, so invalid or misleading rules can be caught before they are saved or used in production.

The rules engine is built around safe expression evaluation with a locked-down function set, strict boolean results, placeholder-based field access, and fail-closed behavior: if a rule cannot be evaluated safely at runtime, the patron is blocked rather than accidentally allowed through. Documentation was added for supported rule functions, and extensive tests were added across the rule engine, blocking integration, admin validation flow, and SIP2 provider behavior.

In scope, this PR covers SIP2 plus the shared infrastructure around it; it does not yet enable patron blocking rules for other providers like Millenium, Kansas, or SirsiDynix. The branch also adds the simpleeval dependency and includes follow-up cleanup/fix commits for syntax, lint, mypy, tests, routes, and lockfile updates.

More detailed summary + user flow diagrams

What this PR adds

This PR introduces a library-configurable patron blocking system that lets a library define named blocking rules in its auth settings and enforce them during patron authentication. The shared model and enforcement hook are placed in the basic authentication stack, while SIP2 is the first provider that opts in via supports_patron_blocking_rules = True.

Core design changes

  1. Rules are now part of per-library auth settings

    • A new PatronBlockingRule value object is introduced with:
      • name
      • rule
      • optional message
    • These rules are stored in the existing per-library settings JSON, so no schema migration is needed.
    • BasicAuthProviderLibrarySettings gains a patron_blocking_rules field, with validation for:
      • non-empty name
      • non-empty rule
      • unique rule names within a library
      • max message length validation
    • Validation errors are surfaced in the existing indexed admin error style.
  2. Authentication flow is refactored to make rule enforcement reusable

    • BasicAuthenticationProvider.authenticate is split so the original provider auth logic moves into _do_authenticate.
    • The public authenticate wrapper now:
      • runs provider auth
      • checks whether the provider supports patron blocking rules
      • if a Patron is returned, evaluates the configured rules before returning that patron
    • This is the key architectural move that makes rule enforcement a shared capability rather than SIP2-only glue code.
  3. A real rule engine replaces the earlier sentinel-style check

    • A new rule_engine.py module is added under api/authentication/patron_blocking_rules/.
    • Expressions use {placeholder} syntax and are compiled into safe variable names instead of string interpolation.
    • Expressions must evaluate to a strict bool.
    • Evaluation fails with explicit rule-engine exceptions on:
      • missing placeholders
      • parse/eval errors
      • non-boolean results
    • Validation also rejects empty, too-long, invalid, or non-boolean expressions.
    • The engine runs in a locked-down simpleeval sandbox with an allowlist of functions.
  4. Allowed rule functions are documented

    • The PR adds docs/FUNCTIONS.md, documenting supported rule functions.
    • The documented functions include:
      • age_in_years(date_str, fmt=None)
      • int(value)
    • The docs also explain fail-closed behavior, string method availability, placeholder syntax, and the availability of normalized runtime values such as {fines}.

Runtime enforcement flow

At runtime, the updated patron-blocking path works like this:

  • SIP2 authenticates the patron normally.
  • If authentication returns a Patron, the provider builds runtime values from the patron / SIP2 data.
  • Rules are evaluated one by one with a fresh evaluator instance.
  • If any rule blocks, authentication returns a blocked-credentials result.
  • If a rule errors at runtime, the system fails closed and blocks the patron, while logging internal details server-side rather than exposing them to the patron.

Admin validation changes

This PR goes beyond static validation and adds live validation support for SIP2-backed rules:

  • During library integration validation, if the integration is SIP2 and has patron blocking rules:
    • the controller loads the provider settings
    • performs a live SIP2 call using the configured test identifier
    • validates each rule expression against real returned values
  • A new admin POST route is added:
    • /admin/patron_auth_service_validate_patron_blocking_rule
  • That endpoint validates a single rule expression against live SIP2 data for a saved service, returning only success/failure rather than the boolean result.

Provider-specific scope

  • SIP2 is the only provider opting into the new shared hook in this PR.
  • The PR explicitly leaves other providers, including Millenium, Kansas, and SirsiDynix, for future follow-up work.

Supporting repo changes

The PR also includes:

  • dependency updates adding simpleeval
  • a new docs page for rule functions
  • route/controller wiring for live rule validation
  • tests for:
    • rule engine behavior
    • patron blocking model/validation
    • SIP2 provider integration
    • admin-side validation logic
  • follow-up commits for syntax fixes, linting, mypy, extra tests, removing inline imports, and lockfile refreshes.

Architectural diagram

flowchart TD
    A[Admin configures per-library patron blocking rules] --> B[BasicAuthProviderLibrarySettings.patron_blocking_rules]
    B --> C[PatronBlockingRule list stored in library settings JSON]

    A --> D[Admin save / validation flow]
    D --> E[Static validation in basic auth settings validator]
    E --> E1[Check required name/rule]
    E --> E2[Check unique rule names]
    E --> E3[Check message/rule constraints]
    E --> E4[validate_rule_expression with test values]

    D --> F[SIP2 live validation path]
    F --> G[Admin controller loads saved service]
    G --> H[fetch_live_rule_validation_values via SIP2 test patron]
    H --> I[validate_rule_expression against real returned values]
    I --> J[Return success or INVALID_CONFIGURATION_OPTION]

    K[Patron login request] --> L[BasicAuthenticationProvider.authenticate]
    L --> M[_do_authenticate provider-specific auth]
    M --> N{Provider supports patron blocking rules?}
    N -- No --> O[Return patron/auth result]
    N -- Yes --> P{Got Patron object back?}
    P -- No --> O
    P -- Yes --> Q[Build runtime values from patron/SIP2 response]
    Q --> R[check_patron_blocking_rules_with_evaluator]
    R --> S[rule_engine.py / simpleeval sandbox]

    S --> T{Rule evaluates to True?}
    T -- Yes --> U[Return BLOCKED_CREDENTIALS / 403]
    T -- No --> V[Continue to next rule]
    V --> T

    S --> W{Rule errors / missing placeholder / non-bool?}
    W -- Yes --> X[Fail closed: block patron and log internal error]
    W -- No --> T

    V --> Y[No blocking rules matched]
    Y --> O
Loading

Motivation and Context

https://ebce-lyrasis.atlassian.net/browse/PP-3772

How Has This Been Tested?

New unit test coverage.
It was also manually tested locally with the front end changes (ThePalaceProject/circulation-admin#201)

Checklist

  • I have updated the documentation accordingly.
  • All new and existing tests passed.

@codecov
Copy link

codecov bot commented Feb 27, 2026

Codecov Report

❌ Patch coverage is 97.13262% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.27%. Comparing base (cba8c0d) to head (fe09100).

Files with missing lines Patch % Lines
...nager/api/admin/controller/patron_auth_services.py 95.45% 2 Missing ⚠️
...uthentication/patron_blocking_rules/rule_engine.py 98.38% 1 Missing and 1 partial ⚠️
...manager/integration/patron_auth/patron_blocking.py 95.65% 2 Missing ⚠️
src/palace/manager/api/admin/routes.py 83.33% 1 Missing ⚠️
src/palace/manager/api/authentication/basic.py 96.96% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3090      +/-   ##
==========================================
+ Coverage   93.24%   93.27%   +0.02%     
==========================================
  Files         492      494       +2     
  Lines       45455    45731     +276     
  Branches     6252     6284      +32     
==========================================
+ Hits        42386    42654     +268     
- Misses       1983     1990       +7     
- Partials     1086     1087       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dbernstein dbernstein force-pushed the feature/PP-3772-Add-patron-blocking-rules-per-library branch 2 times, most recently from fcbea27 to 3a302fd Compare March 3, 2026 20:20
@dbernstein dbernstein marked this pull request as ready for review March 3, 2026 20:22
@dbernstein dbernstein force-pushed the feature/PP-3772-Add-patron-blocking-rules-per-library branch 4 times, most recently from 40c4ef5 to 98d9c5e Compare March 5, 2026 21:18
@dbernstein dbernstein requested review from a team and removed request for a team March 6, 2026 18:03
@dbernstein dbernstein force-pushed the feature/PP-3772-Add-patron-blocking-rules-per-library branch 2 times, most recently from a1a477c to fb6544e Compare March 10, 2026 23:42
@dbernstein dbernstein requested a review from a team March 11, 2026 15:47
@dbernstein dbernstein added the feature New feature label Mar 11, 2026
@dbernstein dbernstein force-pushed the feature/PP-3772-Add-patron-blocking-rules-per-library branch from c9e38c4 to fe09100 Compare March 11, 2026 16:49
Copy link
Member

@jonathangreen jonathangreen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dbernstein I haven't made it completely though this monster of a pull request yet, but I have some initial feedback that I thought I'd put on the PR so you can start looking at it while I finish my review.

from datetime import date
from typing import Any

from simpleeval import ( # type: ignore[import-untyped]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: instead of doing a type ignore like this, lets put this in the pyproject.toml file like we normally do


# ---------------------------------------------------------------------------
# Constants
# ---------------------------------------------------------------------------
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I'm not a big fan of section comments like this. I find they tend to get out of date. If a file is big enough it needs sections, IMO generally it should just be broken up into separate files instead of just trying to organize it via comments like this.

# remote_authenticate() stores the raw info here so that _build_blocking_rule_values()
# can use the full SIP2 payload for rule evaluation without a second network call.
# Each worker thread has its own slot, so concurrent requests do not interfere.
_sip2_thread_local: threading.local = threading.local()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a code smell to me. The core problem is that remote_authenticate() has the rich SIP2 response dict, but by the time authenticate() runs blocking rules, that data has been discarded and only a Patron DB object remains. The thread-local smuggles data across that gap.

This seems like a structural problem, we will face with all of these sort of integrations, so we will need to proliferate this sort of hack in all of the providers to get data where it needs to be.

I think we should modify the API contract, so that the extra info we need gets explicitly provided, instead of implicitly smuggling it in like we do here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't change the API, I also have some real security concerns about how we are using _sip2_thread_local, and how that could leak patron details across requests, but I'm going to save those, because I think we should just replace this structure anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point. I will modify.

Comment on lines +67 to +74
"""Raised when a required placeholder key is absent from the values dict.

Attributes:
key: The name of the missing placeholder (without braces).
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.
"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect doc string style. Not going to note all of these, but from what I see, all the new code in this PR uses google style doc comments where this project uses reST style.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Legit: I need to remember to use the prompt boiler plate that specifies this kind of thing.

Comment on lines +98 to +100
# NOTE: "dob" is intentionally NOT included here yet. Rules that
# reference {dob} will fail-closed (block) until a future version
# populates it from the patron record or SIP2 response.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: I don't foresee us ever wanting to store patron DOB in the record. So does this comment actually make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is an out of date comment from an earlier iteration of the code that should have been removed. Good catch.

Comment on lines +133 to +155
# Live SIP2 rule validation — only runs for SIP2 integrations with rules.
protocol_class = self.get_protocol_class(integration.parent.protocol)
if not issubclass(protocol_class, SIP2AuthenticationProvider):
return
library_settings = protocol_class.library_settings_load(integration)
if not library_settings.patron_blocking_rules:
return

settings = protocol_class.settings_load(integration.parent)
# fetch_live_rule_validation_values raises ProblemDetailException on
# any SIP2 failure (missing test_identifier, network error, etc.).
live_values = protocol_class.fetch_live_rule_validation_values(settings)

evaluator = make_evaluator()
for i, rule in enumerate(library_settings.patron_blocking_rules):
try:
validate_rule_expression(rule.rule, live_values, evaluator)
except RuleValidationError as exc:
raise ProblemDetailException(
INVALID_CONFIGURATION_OPTION.detailed(
f"Rule at index {i} ('{rule.name}'): {exc.message}"
)
) from exc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PatronAuthServicesController is a generic controller for all patron auth protocols, but library_integration_validation contains an explicit issubclass(protocol_class, SIP2AuthenticationProvider) check and directly calls fetch_live_rule_validation_values, which is a SIP2-only method. This violates the open/closed principle, when a second provider (e.g. Millenium, SirsiDynix) gains blocking-rule support, this controller method must be modified with another issubclass branch.

The controller should not need to know which providers support live rule validation. A cleaner approach would be to define fetch_live_rule_validation_values (or a more generic name like
fetch_rule_validation_values) as an optional method/interface on BasicAuthenticationProvider, or create a mixin class HasRuleValidation, that subclasses can implement. Then the controller can ask "does this provider support blocking rules?" and "can it provide live validation values?" without knowing anything about SIP2 specifically.

Comment on lines +278 to +294
def validate_message(message: str) -> None:
"""Validate a patron blocking rule *message* string.

Args:
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)})."
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validate_message is defined and tested but never called from production code. This should either be wired in or removed.

Comment on lines +263 to +274
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,
),
] = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The patron_blocking_rules field is defined on BasicAuthProviderLibrarySettings, which means every basic-auth provider (SimpleAuthentication, Millenium, SirsiDynix, Kansas, Minimal) inherits this field and will accept rules in their settings JSON. But supports_patron_blocking_rules is False for all providers except SIP2, so these rules would be silently ignored at runtime.

The field should either live on SIP2LibrarySettings specifically (since it's the only provider that supports it today), or there should be validation that rejects rules when supports_patron_blocking_rules is False. The current approach creates a confusing gap between "accepts configuration" and "actually uses configuration."

Comment on lines +146 to +149
evaluator = make_evaluator()
for i, rule in enumerate(library_settings.patron_blocking_rules):
try:
validate_rule_expression(rule.rule, live_values, evaluator)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The admin controller creates one evaluator and reuses it across all rules in the validation loop.

Meanwhile, check_patron_blocking_rules_with_evaluator (the runtime path) creates a fresh evaluator per call. Both validate_rule_expression and evaluate_rule_expression_strict_bool mutate evaluator.names and reset it in finally.

While this works today because the calls are sequential, the admin path and runtime path have inconsistent patterns for evaluator lifecycle, which is confusing for anyone reading or maintaining either path. And the mutate then reset in finally pattern is brittle, and easy to break in the future.

@@ -0,0 +1,135 @@
# Patron Blocking Rules — Allowed Functions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doc is created, but its not referenced anywhere. At the very least this should be referenced in the README so people can find this documentation.

settings = protocol_class.settings_load(integration.parent)
# fetch_live_rule_validation_values raises ProblemDetailException on
# any SIP2 failure (missing test_identifier, network error, etc.).
live_values = protocol_class.fetch_live_rule_validation_values(settings)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The live SIP2 call on every admin save has several workflow problems:

  • Every save triggers a SIP2 call, even when rules haven't changed. process_updated_libraries calls library_integration_validation for every library in the submission unconditionally. If an admin edits institution_id or library_identifier_restriction_type on a library that already has blocking rules, the live SIP2 validation fires again for the exact same unchanged rules.
  • SIP2 server unavailability blocks all unrelated config changes. If the SIP2 server is down, the admin cannot save any changes to that integration's library settings — even changes completely unrelated to blocking rules. The failure rolls back the entire transaction. This couples SIP2 server availability to the admin's ability to manage configuration.
  • Test patrons are frequently invalid or absent. In practice, test patrons expire or are removed by libraries regularly — a significant portion of our integrations have no working test patron at any given time. This validation assumes the test patron is valid, which means an admin with blocking rules configured could be locked out of saving any settings changes until the library provisions a new test patron. The fetch_live_rule_validation_values method explicitly raises a ProblemDetailException when test_identifier is missing, so even the absence of a configured test patron blocks the save entirely.
  • The test patron's response may not be representative. Validation uses a single test patron's SIP2 response to check placeholder availability. Different patrons can return different fields depending on their account type, branch, or ILS configuration. A rule referencing {polaris_patron_birthdate} would fail validation if the test patron's response doesn't include that field — even though the field exists for the patrons the rule is intended to target.
  • Validation results are non-deterministic. The test patron's data can change over time (fines increase, account status changes, fields appear or disappear). A set of rules that validates successfully today might fail tomorrow without any configuration change, potentially preventing an admin from re-saving a previously working configuration.
  • Sequential SIP2 calls for multi-library integrations. If a SIP2 integration is associated with multiple libraries that all have blocking rules, each library triggers a separate SIP2 round-trip (connect/login/sc_status/patron_information/end_session/disconnect) during a single synchronous HTTP request. This will almost certainly cause timeouts for patron auth integrtions that have hundreds of libraries.

Some of these are code concerns, but my broader concern is the product level concern. I think this codes impact on configuration workflow needs to be validated by David or Carissa before this actually gets merged. To be clear I'm not questioning the overall PR here, just the validation workflow. I personally don't think we should be doing SIP requests on settings changes at all, and especially not failing settings saves based on them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important considerations: the issues around saving and validation I think can addressed. The issues of non-deterministic results and test patron not being representative is clearly a bigger issue. I just assumed that fields would be present for all users that were present for one (whether or not the fields had values). I also assumed that if a field contained a date value, the format of that value would be consistent across all responses. It is not a problem to have empty or None values in fields, but it is a problem if we can't assume a deterministic set of fields. I will bring up with Carissa and David.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RE: SIP2 server unavailability blocks all unrelated config changes: Now that we are doing live validations on "blur" in the Circulation Admin, the user will be alerted that there is an issue before they try to save. Nevertheless I'll check with Carissa if it is acceptable that a user would need to abandon blocking rules until the SIP server is available and the test user actually valid.

Copy link
Member

@jonathangreen jonathangreen Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the validation also runs on save, if they are editing any setting for a auth integration with block rules won't a sip server being offline mean you can't edit that provider settings at all?

Comment on lines +190 to +193
return BLOCKED_CREDENTIALS.detailed(_DEFAULT_BLOCK_MESSAGE)

if blocked:
return BLOCKED_CREDENTIALS.detailed(rule.message or _DEFAULT_BLOCK_MESSAGE)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is somewhat of a pre-existing problem, but I think the BLOCKED_CREDENTIALS problem detail is not exactly the right one for this (or its poorly named and should be renamed).

BLOCKED_CREDENTIALS = pd(
    "http://librarysimplified.org/terms/problem/credentials-suspended",
    403,
    _("Suspended credentials."),
    _("Your library card has been suspended. Contact your branch library."),
)

The title and URI both reference suspended credentials, but that isn't correct here, they are blocked for some reason. I think it might be worth renaming BLOCKED_CREDENTIALS -> SUSPENDED_CREDENTIALS, since the current name is misleading. And creating a new problem detail BLOCKED_BY_POLICY or something for a credential blocked by library policy.

Extends SIP2LibrarySettings with an optional patron_blocking_rules field
(list of PatronBlockingRule objects). Each rule has a required name, a
required rule expression, and an optional patron-facing message.

Validation is integrated into the existing SettingsValidationError /
BaseSettings pattern: empty name, empty rule expression, and duplicate
rule names within a library all surface as INVALID_CONFIGURATION_OPTION
ProblemDetails with the array index in the message, consistent with how
all other LibrarySettings validation errors are reported.

The authentication hook stub overrides authenticate() in
SIP2AuthenticationProvider. After the base class authenticates a patron
successfully, check_patron_blocking_rules() is called. Any rule whose
rule expression equals the literal "BLOCK" returns a BLOCKED_CREDENTIALS
(HTTP 403) ProblemDetail, using the rule's message if provided. All other
expressions are pass-throughs (stub for a future rule engine).

Storage requires no migration: settings are already a JSON blob in
IntegrationLibraryConfiguration.settings_dict, and the new field
defaults to an empty list that is excluded from model_dump(), so
existing library rows continue to load without change.

No changes to PatronAuthServicesController or IntegrationSettingsController
are needed; the field round-trips through the existing libraries JSON
payload path automatically.

30 new tests cover: PatronBlockingRule model, SIP2LibrarySettings
round-trip and all validation failure cases, check_patron_blocking_rules
pure-function behaviour, and the authenticate() override (blocked,
unblocked, no rules, None pass-through, ProblemDetail pass-through).
Non-SIP2 providers are confirmed unaffected.
    MissingPlaceholderError now accepts an available dict and embeds all available keys + values in the message (e.g. "Available fields: fee_amount='$5.00', fines=5.0, ..."). The available attribute is also accessible for programmatic use.
* build_names now passes the full values mapping to MissingPlaceholderError.
* Both validate_rule_expression and evaluate_rule_expression_strict_bool now catch FunctionNotDefined separately and include the list of allowed functions in the error (e.g. "Allowed functions: age_in_years").
* Added _format_available_keys and _format_allowed_functions helpers. Imported FunctionNotDefined from simpleeval.
* patron_blocking.py — Removed RULE_VALIDATION_TEST_VALUES; build_values_from_sip2_info now returns all raw SIP2 fields verbatim plus the normalised fines float — the old explicit patron_type and dob mappings are gone since those fields appear naturally in the raw dict.
* basic.py — Pydantic static validator now only enforces structural constraints (empty name, empty rule, duplicate names, rule length ≤ 1000, message length ≤ 1000). The syntax/semantic check against live values is done exclusively in library_integration_validation.
* Tests — Updated test_rule_engine.py to assert available keys appear in missing-placeholder errors and allowed functions appear in function-not-found errors. Updated test_patron_blocking.py to remove TestRuleValidationTestValues, drop tests that relied on static Pydantic expression validation, and rewrite TestBuildValuesFromSip2Info to verify all raw fields pass through. Updated test_patron_auth.py to use {custom_field} instead of {dob} in the missing-placeholder test.
* check_patron_blocking_rules (the legacy "BLOCK" string sentinel) was deleted from patron_blocking.py along with its entire docblock.
* TestCheckPatronBlockingRules (and the check_patron_blocking_rules import) were removed from test_patron_blocking.py.
* Runtime auth flow wired up to the full SIP2 response
* basic.py — added the _build_blocking_rule_values(patron, credentials) hook method. The base implementation calls build_runtime_values_from_patron (DB-backed fallback for any future non-SIP2 provider). authenticate() was updated to use the hook and also short-circuits when patron_blocking_rules is empty (avoids calling _build_blocking_rule_values unnecessarily).
* sip2/provider.py — thread-local module variable _sip2_thread_local was added. remote_authenticate() now caches the raw SIP2 patron_information dict in _sip2_thread_local.last_info immediately after the network call, at zero extra cost. _build_blocking_rule_values() reads that cache and calls build_values_from_sip2_info(info) to expose every SIP2 field to rule expressions. If the cache is None (e.g. when _do_authenticate is patched in tests), it falls back to the patron-model path.
Tests
* TestSIP2AuthenticateWithBlockingRules received:
* An autouse fixture that zeroes _sip2_thread_local.last_info before and after every test, preventing cross-test pollution.
* Updated test_missing_placeholder_fails_closed — comment now correctly describes the patron-fallback path taken when remote_authenticate is bypassed.
* Four new tests that directly set _sip2_thread_local.last_info to simulate the real runtime SIP2 path:
* test_raw_sip2_field_blocks_when_rule_matches / …_allows_when_…
* test_fines_normalised_from_sip2_fee_amount
* test_sip2_info_takes_precedence_over_patron_model
…rocess.

Also fixes poetry lock file merge conflict.
patron_auth_services.py — Added process_validate_patron_blocking_rule(): loads integration by ID, calls fetch_live_rule_validation_values() for a live SIP2 auth against test credentials, runs validate_rule_expression() on the result, returns 200 or INVALID_CONFIGURATION_OPTION with the error detail. Returns a clear "save the service first" message when no service ID or service not found.
routes.py — Registered POST /admin/patron_auth_service_validate_patron_blocking_rule with the standard @requires_admin/@requires_csrf_token decorators.
test_patron_auth.py — Added TestProcessValidatePatronBlockingRule (10 tests): missing/nonexistent service ID, non-SIP2 service, valid rule returning True, valid rule returning False (still 200), bad syntax, missing placeholder, SIP2 connection error, missing test identifier, and non-admin access.
Remove thread-local smuggling of SIP2 response data by introducing a
RemoteAuthResult dataclass returned from remote_authenticate(). The
extra_context field carries provider-specific data (e.g. raw SIP2 dict)
through the auth flow so _build_blocking_rule_values() receives it
explicitly instead of via thread-local storage.
Changes:
- Add RemoteAuthResult(patron_data, extra_context) in basic.py
- Change remote_authenticate() return type to RemoteAuthResult
- Change _do_authenticate() to return (patron, extra_context) tuple
- Update authenticate() and _build_blocking_rule_values() to use
  extra_context instead of credentials
- SIP2: return RemoteAuthResult with info dict in extra_context;
  remove _sip2_thread_local and threading import
- Wrap existing returns in RemoteAuthResult in simple, minimal,
  kansas, sirsidynix, and millenium providers
- Update tests and bin/informational/patron_information
Addresses code review feedback on PR #3090 (PP-3772).
Call validate_message from BasicAuthProviderLibrarySettings
validate_patron_blocking_rules when a rule has a non-None message.
Validates non-empty/non-whitespace and length <= 1000 chars.
RuleValidationError is converted to SettingsValidationError with
INVALID_CONFIGURATION_OPTION for consistency with other validation
errors in that method.
- Add supports_patron_blocking_rules: ClassVar[bool] = False to
  BasicAuthProviderLibrarySettings
- SIP2LibrarySettings overrides with True
- validate_patron_blocking_rules rejects rules when the provider does
  not support them
- Add test_rules_rejected_when_provider_does_not_support
- Switch rule tests to SIP2LibrarySettings (only SIP2 supports rules)
- Remove issubclass(..., SIP2AuthenticationProvider) from
  PatronAuthServicesController
- Use getattr(protocol_class, "supports_patron_blocking_rules", False)
  in library_integration_validation and process_validate_patron_blocking_rule
- Drop SIP2 import from patron_auth_services.py
- Update error message to "authentication services that support patron
  blocking rules"
- Point _FETCH_PATCH at sip2.provider module instead of controller
- Rename test_non_sip2_provider_skips_live_validation to
  test_non_supporting_provider_rejects_rules_at_save
- Adjust test_non_sip2_service_returns_error to assert "patron blocking
  rules" in error detail
- Add get_evaluator() with per-thread cached evaluator in rule_engine.py
- Admin and runtime both use get_evaluator() instead of make_evaluator()
- Keep make_evaluator() for tests and custom evaluators
- Evaluator reuse avoids repeated allocation while staying thread-safe
…Y_POLICY (#12)

- Rename BLOCKED_CREDENTIALS to SUSPENDED_CREDENTIALS (ILS-sourced blocks)
- Add BLOCKED_BY_POLICY for patron blocking rule blocks
- Use BLOCKED_BY_POLICY in check_patron_blocking_rules_with_evaluator
- Use SUSPENDED_CREDENTIALS in AuthorizationBlocked
- Update tests to use BLOCKED_BY_POLICY
@dbernstein dbernstein force-pushed the feature/PP-3772-Add-patron-blocking-rules-per-library branch from fe09100 to 49d27dc Compare March 16, 2026 20:59
- Switch patron blocking from fail-closed to fail-open: rules that error
  (missing placeholder, parse error, non-bool result) are logged and
  ignored instead of blocking the patron
- Add INFO log "Patron blocking rules evaluation attempted" for
  CloudWatch error-rate denominator
- Update error log to "Patron blocking rule evaluation failed" for
  CloudWatch metric filter
- Remove live SIP2 rule validation at admin save time; rules are
  validated only at auth time
- Update tests for fail-open behavior and removed live validation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants