Skip to content

Add CORS support - #90

Merged
arobson-ods merged 5 commits into
developfrom
ar/add-cors-support
Sep 23, 2026
Merged

arobson-ods merged 5 commits into
developfrom
ar/add-cors-support

Conversation

@arobson-ods

Copy link
Copy Markdown
Contributor

Addresses issue #88.

The new IATI Dashboard is a browser-based SPA. It authenticates via SSO and calls this API with Authorization: Bearer <token>, from a different origin so the browser's Same-Origin Policy blocks every call unless the API explicitly permits that origin via CORS.

Acceptance criteria from the issue:

  • CORS middleware in place, reading allowed origins from configuration
  • Browser clients can make authenticated Bearer-token requests
  • Preflights return 2xx without credentials, with the expected Access-Control-Allow-* headers
  • Origins outside the allowlist get no Access-Control-Allow-Origin
  • Existing server-to-server clients unaffected

Architecture / scope decisions

  • Origins are listed in a json file referenced from a env variable
  • No middleware is registered at all ifCORS_ALLOWED_ORIGINS_FILE is unset
  • Config is read at import, not through Context. Starlette refuses middleware once the app
    has started, and that includes the lifespan where Context is built so the origins must be
    loaded earlier.
  • An empty allowlist registers nothing, rather than an empty allowlist. Registering
    CORSMiddleware with no origins would still intercept preflights, turning today's 405 on
    OPTIONS into a 400. Guarding the empty case keeps opted-out deployments byte-identical.
  • OPTIONS is not in allow_methods, though spec lists it. It's inert: the list is checked against Access-Control-Request-Method, which never names OPTIONS, because the preflight is the OPTIONS request. Adding or removing it changes only the advertised header string. The behaviour the requirement wants is met and tested.
  • Origin format is strict and unforgiving by design. Entries must be scheme://host[:port], lower case, no trailing slash, no path because CORSMiddleware matches by exact string equality, so a trailing slash would silently never match. Malformed entries are rejected at startup and the error names the offending ones.

Testing

342 passed, 1 skipped (pre-existing). flake8, black --check, isort --check-only, mypy . and bandit
all clean.

New coverage: origins-file loading and validation (shape, format, wildcards, unreadable and malformed files); preflight success for every served verb and refusal for one that isn't; headers present on authenticated requests, absent for unlisted origins, absent entirely when no Origin is sent; CORS headers on 401 and on handled 500s; the no-CORS default; and that src/main.py actually wires the middleware up.

Notes for the reviewer

The third commit touches sentry.py. Sentry and CORS each had their own copy of the environment reader. It's now shared. Duplication resulted from sequencing of the Sentry and CORS work and the need to rebase from develop before committing CORS.

Registers Starlette's CORSMiddleware so that a browser based client can make
Bearer-token calls from another origin. Preflight OPTIONS requests are answered
above the router, so they are not made to authenticate, and error responses
carry the CORS headers a client needs in order to read them.

The allowed origins are read from a JSON file named by the new, optional
CORS_ALLOWED_ORIGINS_FILE variable. With it unset no middleware is registered at
all, so a deployment that has not opted in is unchanged. Deploying with CORS
enabled needs the origins file bind-mounted into the container.
Rebased the CORS branch when Sentry branch was merged to develop. This
refactor removes the resulting code duplication.

Sentry and CORS are both configured before the FastAPI application object exists,
so each read the environment directly rather than through Context, and each had
its own copy of the reader. The CORS copy took the env file as a parameter so that
it could be tested against a temporary file; that version is now the shared one.

No behaviour changes: the precedence is the same as before, and as Context's.
@arobson-ods
arobson-ods merged commit ca1210c into develop Sep 23, 2026
5 checks passed
@arobson-ods
arobson-ods deleted the ar/add-cors-support branch September 23, 2026 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants