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.

An after_request hook back-fills the wildcard header on responses the decorator never sees, such as a raising view converted by the app-level error handler, or an outer decorator returning a problem detail. Without it, cross-origin clients would get an opaque CORS failure instead of a readable error body.

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, including preflight, error responses, and decorator stacking.
• 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 6m 36s —— View job

---

Summary

The decorator itself is sound: send_wildcard=True with supports_credentials=False is the right combination (a wildcard origin plus no Access-Control-Allow-Credentials means browsers won't hand these responses to credentials: 'include' requests), flask-cors 6.0.2 defaults allow_private_network to false so no private-network preflight is advertised, and the Vary: Origin assertion correctly locks in cacheability. The mutual-exclusion guard propagates in both stacking orders because functools.wraps/update_wrapper copy __dict__, and the tests cover that. Two things are worth tightening before the follow-up PR applies this to real routes: the after_request back-fill silently does nothing for preflight failures, and nothing asserts the hook is actually registered on the production app.

Details

Minor: src/palace/manager/api/routes.py:164-178

The back-fill only sets Access-Control-Allow-Origin, which rescues simple GETs but not preflights. Existing routes put allows_patron_web underneath @has_library, and if allows_public_cors takes that same slot, an OPTIONS preflight for an unknown library short name is answered by has_library with a 404 and never reaches the cross_origin wrapper — the hook then adds Access-Control-Allow-Origin: * but no Access-Control-Allow-Methods/Access-Control-Allow-Headers, and a preflight with a non-2xx status fails regardless. So any request that preflights (e.g. one carrying Authorization, which the docstring says is how patron-specific behavior reaches these routes) still gets an opaque CORS error rather than the readable problem detail the hook exists to deliver. Putting this decorator outside has_library fixes it — the wrapper then answers OPTIONS before has_library runs, and also CORS-decorates has_library's short-circuit response — leaving the hook responsible only for the ErrorHandler path; worth saying so in the docstring so the routes PR gets the ordering right.

@library_route("/feed/<lane_identifier>")
@allows_public_cors   # outside has_library, unlike allows_patron_web
@has_library
@returns_problem_detail
def feed(lane_identifier): ...

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

Lines 164 to 178 in 1dae183

	@app.after_request
	def add_public_cors_to_error_responses(response: Response) -> Response:
	"""Back-fill the wildcard CORS header on public routes.
	The allows_public_cors decorator cannot add headers to a response it
	never sees. A view that raises gets its response built by the app-level
	error handler, and an outer decorator such as has_library can return a
	problem detail without calling the view. Cross-origin clients need the
	header on those responses to read the error body.
	"""
	if "Access-Control-Allow-Origin" not in response.headers and request.endpoint:
	view = current_app.view_functions.get(request.endpoint)
	if view is not None and getattr(view, "allows_public_cors", False):
	response.headers["Access-Control-Allow-Origin"] = "*"
	return response

Minor: tests/manager/api/test_routes.py:95-100

Every test builds a throwaway Flask app and attaches the hook by hand with app.after_request(routes.add_public_cors_to_error_responses), so nothing verifies that the @app.after_request in routes.py registered it on the real app. Deleting that decorator from the source would leave the entire suite green while silently disabling the back-fill in production. TestAppConfiguration already asserts against routes.app, so a one-liner there closes the gap:

assert routes.add_public_cors_to_error_responses in routes.app.after_request_funcs[None]

circulation/tests/manager/api/test_routes.py

Lines 95 to 100 in 1dae183

	def test_error_responses_keep_cors(self) -> None:
	# A raising view never returns through the decorator, so the
	# after_request hook must back-fill the wildcard header. Routes
	# without the decorator must stay untouched.
	app = flask.Flask(__name__)
	app.after_request(routes.add_public_cors_to_error_responses)

---

• Gather context (diff, routes.py, app_server.py, tests, pinned flask-cors version)
• Analyze the CORS decorator & after_request hook
• Review test coverage
• Post review

Review only — no files changed.
| feature/public-cors-decorator

[greptile-apps]

Greptile Summary

The PR introduces a decorator for wildcard CORS on unauthenticated public routes, including preflight handling and an application hook that preserves CORS headers on error and short-circuit responses.

• Rejects stacking public and patron-web CORS decorators.
• Advertises only GET, HEAD, and OPTIONS during preflight.
• Adds coverage for normal, preflight, error, short-circuit, and decorator-stacking behavior.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/palace/manager/api/routes.py	Adds public wildcard-CORS decoration, incompatible-decorator checks, preflight configuration, and error-response header backfilling.
tests/manager/api/test_routes.py	Adds broad functional coverage for public CORS behavior, including preflight and response paths that bypass the decorated view.

Sequence Diagram

sequenceDiagram
    participant Browser
    participant Flask
    participant Route
    participant ErrorHandler
    Browser->>Flask: Cross-origin request
    Flask->>Route: Dispatch decorated public route
    alt View returns normally
        Route-->>Flask: Response with wildcard CORS header
    else View raises or outer decorator short-circuits
        Route-->>ErrorHandler: Error or alternate response
        ErrorHandler-->>Flask: Response without CORS header
        Flask->>Flask: after_request checks endpoint marker
        Flask->>Flask: Add wildcard CORS header
    end
    Flask-->>Browser: Readable cross-origin response

Loading

_{Reviews (3): Last reviewed commit: "CI 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 (1dae183).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3680   +/-   ##
=======================================
  Coverage   93.55%   93.55%           
=======================================
  Files         513      513           
  Lines       46907    46927   +20     
  Branches     6405     6409    +4     
=======================================
+ Hits        43884    43904   +20     
  Misses       1954     1954           
  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.

[tdilauro]

More Claude / greptile beef. And I'm here for it! 😂

Note on the earlier Greptile comment
I'd push back on the TestAllowsPublicCors naming complaint. CLAUDE.md says to name test classes after "the class or module under test" — allows_public_cors is the unit under test here, not a behavior or scenario, so the name follows the convention rather than violating it.