mirror of
https://github.com/Zproger/Aurum.git
synced 2026-10-13 02:56:39 +07:00
fix(security): stop granting every origin credentialed access to the API (#5)
`AURUM_CORS_ORIGINS` defaults to `*`, and the middleware was configured with
`allow_credentials=True`. Starlette does not answer that combination with a
literal `*` — it echoes back whatever `Origin` the caller sent and adds
`Access-Control-Allow-Credentials: true`, so any page open in the same
browser can call the instance, with the user's Basic Auth attached, and read
the response:
GET /api/accounts Origin: https://evil.example
-> Access-Control-Allow-Origin: https://evil.example
Access-Control-Allow-Credentials: true
That undercuts the README's advice that leaving Basic Auth off is fine while
the instance is only reachable from localhost: reachable-from-localhost
includes every website the user visits.
Nothing in the shipped setup is cross-origin. Compose serves the UI and the
API from the same nginx, and the Vite dev server proxies `/api`
(frontend/vite.config.ts), so neither deployment nor development needs CORS
at all. The default is therefore empty — no cross-origin access — and the
middleware is only installed when origins are configured deliberately.
An explicit `*` is still honoured for a genuinely public read-only setup, but
never together with credentials, which is what the spec intends `*` to mean
anyway.
This commit is contained in:
@@ -8,8 +8,13 @@ AURUM_POSTGRES_HOST=db
|
|||||||
AURUM_POSTGRES_PORT=5432
|
AURUM_POSTGRES_PORT=5432
|
||||||
|
|
||||||
# --- Backend ---
|
# --- Backend ---
|
||||||
# Comma-separated list of allowed origins, or * for any (fine for local self-hosting).
|
# Comma-separated list of browser origins allowed to call the API. Leave it
|
||||||
AURUM_CORS_ORIGINS=*
|
# empty unless you serve the frontend from a different origin than the API:
|
||||||
|
# docker compose serves both from the same nginx, and the Vite dev server
|
||||||
|
# proxies /api, so neither needs this. "*" is honoured but then credentials
|
||||||
|
# are never granted, because "*" plus credentials lets any site you visit
|
||||||
|
# read this instance.
|
||||||
|
AURUM_CORS_ORIGINS=
|
||||||
AURUM_DEFAULT_CURRENCY=USD
|
AURUM_DEFAULT_CURRENCY=USD
|
||||||
|
|
||||||
# --- Crypto tab (live prices via CoinGecko) ---
|
# --- Crypto tab (live prices via CoinGecko) ---
|
||||||
|
|||||||
@@ -22,8 +22,11 @@ class Settings(BaseSettings):
|
|||||||
# Default currency shown across the UI when an account doesn't override it
|
# Default currency shown across the UI when an account doesn't override it
|
||||||
default_currency: str = "USD"
|
default_currency: str = "USD"
|
||||||
|
|
||||||
# Comma-separated list of origins allowed to call the API (frontend dev server, etc.)
|
# Comma-separated list of browser origins allowed to call the API. Empty
|
||||||
cors_origins: str = "*"
|
# by default, which allows none: the shipped compose serves the UI and the
|
||||||
|
# API from one nginx, and the Vite dev server proxies /api, so neither is
|
||||||
|
# a cross-origin caller. Set it only for a genuinely separate frontend.
|
||||||
|
cors_origins: str = ""
|
||||||
|
|
||||||
# CoinGecko Demo API key (free, no card required — https://www.coingecko.com/en/api/pricing)
|
# CoinGecko Demo API key (free, no card required — https://www.coingecko.com/en/api/pricing)
|
||||||
# for services/crypto_service.py's price lookups. Empty by default; the
|
# for services/crypto_service.py's price lookups. Empty by default; the
|
||||||
|
|||||||
@@ -40,10 +40,18 @@ async def lifespan(app: FastAPI):
|
|||||||
|
|
||||||
app = FastAPI(title="Aurum API", version=APP_VERSION, lifespan=lifespan)
|
app = FastAPI(title="Aurum API", version=APP_VERSION, lifespan=lifespan)
|
||||||
|
|
||||||
|
# CORS stays off unless someone deliberately opens it, and credentials are
|
||||||
|
# only granted to a pinned list. Starlette answers a credentialed "*" by
|
||||||
|
# echoing back whatever Origin asked instead of a literal "*" — so the two
|
||||||
|
# together turn any page the user happens to have open into an authenticated
|
||||||
|
# client of their instance, which is exactly what the README's "fine if it's
|
||||||
|
# only reachable from localhost" advice assumes can't happen.
|
||||||
|
cors_origins = settings.cors_origins_list
|
||||||
|
if cors_origins:
|
||||||
app.add_middleware(
|
app.add_middleware(
|
||||||
CORSMiddleware,
|
CORSMiddleware,
|
||||||
allow_origins=settings.cors_origins_list,
|
allow_origins=cors_origins,
|
||||||
allow_credentials=True,
|
allow_credentials="*" not in cors_origins,
|
||||||
allow_methods=["*"],
|
allow_methods=["*"],
|
||||||
allow_headers=["*"],
|
allow_headers=["*"],
|
||||||
)
|
)
|
||||||
|
|||||||
37
backend/tests/test_cors.py
Normal file
37
backend/tests/test_cors.py
Normal file
@@ -0,0 +1,37 @@
|
|||||||
|
"""CORS defaults.
|
||||||
|
|
||||||
|
The README tells self-hosters that leaving basic auth off is fine as long as
|
||||||
|
the instance is only reachable from localhost. That promise only holds if a
|
||||||
|
random page open in the same browser can't call the API on their behalf —
|
||||||
|
which is what these tests pin down.
|
||||||
|
"""
|
||||||
|
from httpx import AsyncClient
|
||||||
|
|
||||||
|
from app.core.config import Settings
|
||||||
|
|
||||||
|
EVIL = "https://evil.example"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_a_foreign_origin_is_never_echoed_back(client: AsyncClient):
|
||||||
|
"""Starlette answers a credentialed wildcard by echoing whatever Origin
|
||||||
|
asked, which is what makes the wildcard dangerous rather than merely
|
||||||
|
permissive — the answer must be `*` or nothing, never the caller's own
|
||||||
|
origin."""
|
||||||
|
resp = await client.get("/accounts", headers={"Origin": EVIL})
|
||||||
|
|
||||||
|
assert resp.headers.get("access-control-allow-origin") != EVIL
|
||||||
|
|
||||||
|
|
||||||
|
async def test_credentials_are_not_granted_to_every_origin(client: AsyncClient):
|
||||||
|
"""With allow-credentials, a page the user visits can call the instance
|
||||||
|
with their Basic Auth attached and read the response."""
|
||||||
|
resp = await client.get("/accounts", headers={"Origin": EVIL})
|
||||||
|
|
||||||
|
assert resp.headers.get("access-control-allow-credentials") != "true"
|
||||||
|
|
||||||
|
|
||||||
|
def test_shipped_default_allows_no_cross_origin_access():
|
||||||
|
"""Nothing in the shipped setup needs CORS: compose serves UI and API
|
||||||
|
from one origin, and the Vite dev server proxies /api. So the default
|
||||||
|
should be closed, not `*`."""
|
||||||
|
assert Settings.model_fields["cors_origins"].default == ""
|
||||||
@@ -23,7 +23,7 @@ services:
|
|||||||
AURUM_POSTGRES_DB: ${AURUM_POSTGRES_DB:-aurum}
|
AURUM_POSTGRES_DB: ${AURUM_POSTGRES_DB:-aurum}
|
||||||
AURUM_POSTGRES_HOST: db
|
AURUM_POSTGRES_HOST: db
|
||||||
AURUM_POSTGRES_PORT: 5432
|
AURUM_POSTGRES_PORT: 5432
|
||||||
AURUM_CORS_ORIGINS: ${AURUM_CORS_ORIGINS:-*}
|
AURUM_CORS_ORIGINS: ${AURUM_CORS_ORIGINS:-}
|
||||||
AURUM_DEFAULT_CURRENCY: ${AURUM_DEFAULT_CURRENCY:-USD}
|
AURUM_DEFAULT_CURRENCY: ${AURUM_DEFAULT_CURRENCY:-USD}
|
||||||
# Free CoinGecko Demo key (coingecko.com/en/api/pricing) — the Crypto
|
# Free CoinGecko Demo key (coingecko.com/en/api/pricing) — the Crypto
|
||||||
# tab's price sync 400s with a clear message until this is set.
|
# tab's price sync 400s with a clear message until this is set.
|
||||||
|
|||||||
Reference in New Issue
Block a user