Public CORS decorator (PP-5045)

[tdilauro]

Description

Adds an allows_public_cors route decorator that sends open CORS headers and handles preflight requests. The decorator comment documents that it must not be stacked with allows_patron_web and must only go on GET/HEAD routes.

• Access-Control-Allow-Origin: *,
• no credentials, and
• preflight advertises only GET, HEAD, and OPTIONS methods.
  No routes use it yet.

Motivation and Context

Public catalog data is readable by anyone without credentials, so an origin allowlist adds no protection there. What it does do is break browser-based catalog clients (like CPW) when their hosts are not configured, which creates friction for dev instances and new deployments. This decorator is the first step. A follow-up PR will apply it to the public routes.

[Jira PP-5045]

How Has This Been Tested?

• New tests to cover the new functionality.
• All tests pass locally and in CI.

Checklist

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

[claude]

Claude finished @tdilauro's task in 5m 43s —— View job

---

Code Review

• Read PR diff
• Read surrounding code in routes.py (existing allows_patron_web CORS handling, route decorator stacking)
• Analyze flask_cors.cross_origin semantics for this configuration (flask-cors 6.0.2, no CORS_* app config in play)
• Check app-level error handling path (api/app.py:114, ErrorHandler.handle)
• Post review

---

Summary

The approach is sound and the rationale is documented unusually well — the wildcard-origin-plus-no-credentials reasoning is correct, and I confirmed Palace has no IP/ambient-credential patron authentication that would undermine it (remote_addr is only used for logging and in controller/loan.py, never for auth). Delegating to flask_cors.cross_origin instead of hand-rolling is the right call, and test_stacked_with_wrapping_decorator is a good instinct — it pins the one thing that would silently break when this lands under @has_library. My findings are all minor and about polish for the follow-up PR that actually applies the decorator: preflight caching, error responses, and one coverage gap.

Details

Minor: src/palace/manager/api/routes.py:125-129

No max_age is configured, so no Access-Control-Max-Age is advertised and browsers fall back to their default preflight cache of ~5 seconds. This only bites the case the PR exists to serve: a browser catalog client sending Authorization on a public feed route makes that request non-simple, so essentially every catalog fetch pays an extra OPTIONS round trip — and once this sits under @has_library (as allows_patron_web does today), each preflight also runs a library_for_request() database lookup before the CORS decorator short-circuits it. Setting an explicit max age costs nothing:

allows_public_cors = cross_origin(
    methods=["GET", "HEAD", "OPTIONS"],
    max_age=3600,
    send_wildcard=True,
    supports_credentials=False,
)

circulation/src/palace/manager/api/routes.py

Lines 125 to 129 in 888d545

	allows_public_cors = cross_origin(
	methods=["GET", "HEAD", "OPTIONS"],
	send_wildcard=True,
	supports_credentials=False,
	)

Minor: src/palace/manager/api/routes.py:113-129

Error responses on decorated routes will carry no CORS headers. cross_origin's wrapper only reaches set_cors_headers when the view returns normally; anything that raises — a BaseProblemDetailException from a controller, a werkzeug HTTPException, or an unexpected 500 — is converted to a response by the app-level handler registered at api/app.py:114, entirely outside the decorator. flask_cors's intercept_exceptions option only applies to the CORS(app) extension, not to cross_origin, so it does not cover this. The practical effect is that a cross-origin catalog client gets an opaque CORS failure instead of the problem detail whenever a request fails, which is exactly when it most needs to read the body. Worth handling in ErrorHandler.handle or a small after_request, or at minimum noting the limitation in the comment block so the follow-up PR does not assume error bodies are readable.

circulation/src/palace/manager/api/routes.py

Lines 113 to 129 in 888d545

	# The allows_public_cors decorator adds permissive CORS headers to read-only
	# routes that serve public data. Anyone can already read these routes without
	# authenticating, so every web origin is allowed and no configuration is
	# needed. The Access-Control-Allow-Credentials header is never sent, so
	# browsers refuse to share these responses with cross-origin scripts that make
	# cookie-authenticated requests, which is what keeps the wildcard origin safe.
	# Patron-specific behavior on these routes works only through the
	# Authorization header, which a cross-origin script must set explicitly.
	# Use either this decorator or allows_patron_web on a route, never both;
	# stacking them produces broken CORS responses. The methods list below only
	# limits what a preflight advertises; it does not block other methods on the
	# actual response, so apply this decorator only to GET/HEAD routes.
	allows_public_cors = cross_origin(
	methods=["GET", "HEAD", "OPTIONS"],
	send_wildcard=True,
	supports_credentials=False,
	)

Minor: tests/manager/api/test_routes.py:57-77

test_preflight only exercises the allowed-method path. The comment block's most safety-relevant claim — that write methods are never advertised — is untested, so a future change to the methods list (or a flask-cors upgrade that changes how it filters Access-Control-Request-Method) would not fail anything. A few lines in the same test class would pin it:

def test_preflight_does_not_advertise_write_methods(self, client: FlaskClient) -> None:
    response = client.options(
        "/public",
        headers={
            "Origin": "http://any.web.client",
            "Access-Control-Request-Method": "POST",
        },
    )
    assert "Access-Control-Allow-Methods" not in response.headers

circulation/tests/manager/api/test_routes.py

Lines 57 to 77 in 888d545

	def test_preflight(self, client: FlaskClient) -> None:
	response = client.options(
	"/public",
	headers={
	"Origin": "http://any.web.client",
	"Access-Control-Request-Method": "GET",
	"Access-Control-Request-Headers": "Authorization",
	},
	)
	assert response.status_code == 200
	assert response.headers["Access-Control-Allow-Origin"] == "*"
	# Only the read-only methods are advertised.
	assert set(response.headers["Access-Control-Allow-Methods"].split(", ")) == {
	"GET",
	"HEAD",
	"OPTIONS",
	}
	assert (
	"authorization" in response.headers["Access-Control-Allow-Headers"].lower()
	)
	assert "Access-Control-Allow-Credentials" not in response.headers

---

No code changes were made — this was a review-only request. I did not run the test suite (no Python environment is installed in this checkout); the findings above come from reading the diff against origin/main and the surrounding routes.py / app_server.py / app.py code.
• Branch: feature/public-cors-decorator

[greptile-apps]

Greptile Summary

Adds a reusable permissive CORS decorator for future public read-only routes, without applying it to any existing endpoint.

• Advertises GET, HEAD, and OPTIONS during preflight.
• Sends a wildcard origin without credential support.
• Adds isolated tests for simple requests, preflight behavior, and wrapper composition.

Confidence Score: 4/5

The PR appears safe to merge, with only a non-blocking test-organization issue.

The new decorator is not yet attached to production routes, its intended CORS behavior is covered by tests, and the only accepted concern is the repository-inconsistent test class organization.

Files Needing Attention: tests/manager/api/test_routes.py

Important Files Changed

Filename	Overview
src/palace/manager/api/routes.py	Adds the currently unused public CORS decorator and documents its authentication, stacking, and HTTP-method constraints.
tests/manager/api/test_routes.py	Covers the decorator's headers and wrapper composition, but places the tests in a behavior-specific class contrary to the repository convention.

_{Reviews (1): Last reviewed commit: "Local AI code review feedback" | Re-trigger Greptile}

[greptile-apps]

Behavior-specific test class

TestAllowsPublicCors organizes these tests around one behavior rather than the module under test, contrary to the repository's module-oriented test-class convention and making related route tests less consistent to locate.

Context Used: CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.55%. Comparing base (f93b325) to head (888d545).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3680   +/-   ##
=======================================
  Coverage   93.55%   93.55%           
=======================================
  Files         513      513           
  Lines       46907    46909    +2     
  Branches     6405     6405           
=======================================
+ Hits        43884    43887    +3     
+ Misses       1954     1953    -1     
  Partials     1069     1069           

☔ View full report in Codecov by Harness.
📢 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.