Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ WORKDIR /app
# Pull the uv binary from its published image and point it at the base
# image's interpreter (UV_PYTHON_DOWNLOADS=0 — no managed-Python download).
# The project venv lives at /app/.venv; putting it on PATH exposes both
# `gunicorn` and the `access` console script (entry point api.manage:cli)
# `gunicorn` and the `access` console script (entry point api.cli:cli)
# for CronJobs and other CLI invocations.
COPY --from=ghcr.io/astral-sh/uv:0.11.10 /uv /bin/uv
ENV UV_PYTHON_DOWNLOADS=0 \
Expand Down
45 changes: 0 additions & 45 deletions POST_MIGRATION_TODO.md

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Removing these as they're not necessary/low priority/bad ideas

Original file line number Diff line number Diff line change
Expand Up @@ -26,30 +26,6 @@ with the schema. 281/281 tests green.

---

## Auth

### 9. Cache JWKS lookups smarter

Today: `cachetools.TTLCache(maxsize=1, ttl=3600)` per process. Production
scale may want:
- Redis-backed cache shared across replicas, or
- A sidecar that pre-fetches and refreshes
- Move CF / OIDC verification entirely into middleware so dependencies don't
re-verify on each call

---

## Background Work / Long-Running Operations

(Note: the previous "Replace the in-process syncer.py loop with a
proper task runner" entry was removed — the syncer already runs as a
Kubernetes CronJob via `examples/kubernetes/cron-job-syncer.yaml`,
invoking `access sync` every 15 minutes. The Dockerfile's default
`gunicorn` CMD is overridable in the K8s manifest. README.md §
Kubernetes Deployment and CronJobs already documents this pattern.)

---

## Tooling

### 14. Strict type checking on routers + schemas
Expand All @@ -75,16 +51,6 @@ Either:
generates fixtures from Pydantic models — keeps test data and request
schemas in sync automatically.

### 17. Golden-file response snapshots

Add snapshot tests for the major endpoints (`GET /api/groups`,
`GET /api/groups/{id}`, `GET /api/users/{id}`, `GET /api/requests`,
`GET /api/audit/users`). Lets future changes to schemas surface as
diffable test failures rather than runtime regressions for clients.

Recommend `syrupy` for the snapshot framework — JSON-aware diffs and
clean `--snapshot-update` ergonomics.

---

## Security follow-ups (out of scope for the migration PR)
Expand All @@ -96,14 +62,3 @@ Drop `'unsafe-inline'` from `script-src` and `style-src` in
`SecurityHeadersMiddleware` and thread it through `build/index.html`
+ the React build pipeline so every inline `<script>` / `<style>`
carries the nonce. Touches the frontend; not a same-PR fix.

### 21. Trust proxy `X-Forwarded-*` only from an allowlist

`api/middleware.py:_client_ip` reads `X-Forwarded-For` / `X-Real-IP`
from any caller, so an attacker that can reach the FastAPI service
directly can forge `RequestContext.ip` (audit-log only — no
auth/rate-limit decision uses it). Configure
`uvicorn --forwarded-allow-ips=<LB CIDR>` in the production
deployment (or use Starlette's `ProxyHeadersMiddleware` with an
explicit `TRUSTED_PROXIES`). Document the setting in the K8s example
manifests.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ The `.env.production` file is where you configure the application.
- `OIDC_CLIENT_SECRETS`: Specifies the path to your client_secrets.json file or if you prefer, inline the entire JSON string.
- `ALLOWED_HOSTS`: **[REQUIRED for OIDC deployments outside development/test; recommended otherwise]** Comma-separated allowlist of `Host` header values accepted by the app (wildcards like `*.example.com` supported). Rejects spoofed `Host` headers, which would otherwise poison URLs derived from the request (notably the OIDC `redirect_uri`). Cloudflare Access deployments don't hit that path and aren't required to set it, but it remains useful defense-in-depth. Set to your public host, e.g. `access.example.com`.
- `OIDC_OVERWRITE_REDIRECT_URI`: **[OPTIONAL, recommended for OIDC behind a proxy]** Pins the OIDC callback URL handed to your IdP instead of deriving it from the request `Host` header. Set to your registered sign-in redirect URI, e.g. `https://access.example.com/oidc/authorize`.
- `ENABLE_API_DOCS`: **[OPTIONAL]** Set to `true` to expose the auto-generated OpenAPI docs (`/api/docs`) and schema (`/api/openapi.json`) in staging/production. Off by default; the docs are always available in development (`ENV=development`). Both routes stay behind the app's authentication gate, so they're only reachable by authenticated users.
- `ENABLE_MCP`: **[OPTIONAL]** Set to `true` to mount the embedded Model Context Protocol server at `/mcp`. Off by default. See [MCP Server (optional)](#mcp-server-optional) below.
- `MCP_FALLBACK_SCOPES`: **[OPTIONAL]** Comma-separated scopes granted to MCP tokens that carry no `scope` claim. Defaults to `read_all,create_requests` (read + filing requests). Set to `read_all` for read-only MCP sessions, or `""` to fail closed. Only relevant when `ENABLE_MCP=true`.
- `OIDC_MCP_AUDIENCE`: **[REQUIRED when `ENABLE_MCP=true` and `OIDC_SERVER_METADATA_URL` is set]** The OAuth audience to validate against the `aud` claim on incoming MCP bearer tokens. Typically the OAuth client identifier of the MCP application registered with your IdP, e.g. `access-mcp`.
Expand Down
4 changes: 2 additions & 2 deletions api/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,9 @@ async def lifespan(_fast_app: FastAPI) -> AsyncIterator[None]:
app = FastAPI(
title=settings.APP_NAME,
version=settings.APP_VERSION,
docs_url="/api/docs" if settings.DEBUG else None,
docs_url="/api/docs" if settings.expose_api_docs else None,
redoc_url=None,
openapi_url="/api/openapi.json" if settings.DEBUG else None,
openapi_url="/api/openapi.json" if settings.expose_api_docs else None,
# Defense-in-depth: every endpoint goes through the auth gate.
# `require_authenticated` short-circuits for the small allowlist
# (health, OIDC login). Endpoints still declare `CurrentUserId`
Expand Down
5 changes: 3 additions & 2 deletions api/auth/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,9 @@ async def get_current_user(
async def require_authenticated(request: Request, db: DbSession) -> None:
"""Enforce authentication on every request except `/api/healthz` and
the OIDC login endpoints. `/api/docs` and `/api/openapi.json` are
intentionally inside the gate even though they're DEBUG-only, as is
the catch-all SPA route."""
intentionally inside the gate — they're served in development and,
when `ENABLE_API_DOCS` is set, in staging/production, but always to
authenticated users only, as is the catch-all SPA route."""
path = request.url.path
if any(path == p.rstrip("/") or path.startswith(p) for p in AUTH_ALLOWLIST_PREFIXES):
return
Expand Down
2 changes: 1 addition & 1 deletion api/manage.py → api/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
access init <admin_email>
access sync
access notify
python -m api.manage <command>
python -m api.cli <command>
"""

from __future__ import annotations
Expand Down
16 changes: 16 additions & 0 deletions api/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,13 @@ def DEBUG(self) -> bool:
def TESTING(self) -> bool:
return self.ENV == "test"

@property
def expose_api_docs(self) -> bool:
"""Whether to mount `/api/docs` and `/api/openapi.json`.

Always on in development; otherwise gated behind `ENABLE_API_DOCS`."""
return self.DEBUG or self.ENABLE_API_DOCS

CLIENT_ORIGIN_URL: Optional[str] = None

# Comma-separated Host header allowlist for TrustedHostMiddleware
Expand Down Expand Up @@ -157,6 +164,15 @@ def TESTING(self) -> bool:
# Behavior toggles
REQUIRE_DESCRIPTIONS: bool = False

# Expose the auto-generated OpenAPI docs (`/api/docs`) and schema
# (`/api/openapi.json`) outside development. These are always served in
# development (`DEBUG`); flipping this on additionally exposes them in
# staging/production. Off by default so deployments don't publish their
# API surface unless an operator opts in. Both routes stay behind the
# app-wide `require_authenticated` gate regardless, so enabling this
# exposes the docs to authenticated users only, not the public internet.
ENABLE_API_DOCS: bool = False

# MCP server. Off-by-default; flipping this to True mounts the FastMCP
# server at /mcp and activates the MCP auth middleware. Most operators
# of the open-source distribution don't run LLM tooling and shouldn't
Expand Down
2 changes: 1 addition & 1 deletion examples/plugins/health_check_plugin/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def health_command() -> None:
Plugin-contributed commands are registered on the CLI group as-is, so
they drive their own event loop: the command body is an ``async def``
executed with one ``asyncio.run`` per invocation, mirroring the
``_with_app_context`` pattern in ``api/manage.py``.
``_with_app_context`` pattern in ``api/cli.py``.
"""
from api.config import settings
from api.database import build_async_engine
Expand Down
4 changes: 2 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ dependencies = [
]

[project.scripts]
access = "api.manage:cli"
access = "api.cli:cli"

# ----------------------------------------------------------------------
# Dependency groups (PEP 735) — installed by `uv sync`, excluded from the
Expand Down Expand Up @@ -134,7 +134,7 @@ include = [
"api/auth/**",
"api/mcp/**",
"api/exception_handlers.py",
"api/manage.py",
"api/cli.py",
"api/app.py",
]

Expand Down
11 changes: 11 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@ def test_app_group_deleter_ids() -> None:
assert Settings(APP_GROUP_DELETER_ID="test1,test2").app_group_deleter_ids == ["test1", "test2"]


def test_expose_api_docs() -> None:
# Always exposed in development, regardless of the toggle.
assert Settings(ENV="development", ENABLE_API_DOCS=False).expose_api_docs is True
# Off by default outside development.
assert Settings(ENV="staging").expose_api_docs is False
assert Settings(ENV="production").expose_api_docs is False
# The toggle opts staging/production in.
assert Settings(ENV="staging", ENABLE_API_DOCS=True).expose_api_docs is True
assert Settings(ENV="production", ENABLE_API_DOCS=True).expose_api_docs is True


def test_trusted_hosts() -> None:
assert Settings(ALLOWED_HOSTS="").trusted_hosts == []
assert Settings(ALLOWED_HOSTS="access.example.com").trusted_hosts == ["access.example.com"]
Expand Down
76 changes: 76 additions & 0 deletions tests/test_oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,64 @@ def oidc_app(
_restore_oidc_registry(saved_clients, saved_registry)


# `oidc_app` / `dev_oidc_app` are sync fixtures with async dependencies
# (`db`), so — like `oidc_client` — they must be pulled through an async
# fixture to be consumable by the async docs-wiring tests below.
@pytest.fixture
async def staging_app(oidc_app: FastAPI) -> FastAPI:
return oidc_app


@pytest.fixture
async def dev_app(dev_oidc_app: FastAPI) -> FastAPI:
return dev_oidc_app


@pytest.fixture
async def oidc_app_with_docs(
monkeypatch: pytest.MonkeyPatch,
db: Db,
seed_oidc_user: OktaUser,
oidc_mock: SimpleNamespace,
stub_build_dir: Path,
) -> AsyncGenerator[FastAPI, None]:
# Same as `oidc_app` (staging), but with ENABLE_API_DOCS flipped on so
# the OpenAPI docs are mounted outside development.
saved = (
settings.ENV,
settings.SECRET_KEY,
settings.OIDC_CLIENT_SECRETS,
settings.CLOUDFLARE_TEAM_DOMAIN,
settings.SQLALCHEMY_DATABASE_URI,
settings.CLOUDSQL_CONNECTION_NAME,
settings.ALLOWED_HOSTS,
settings.ENABLE_MCP,
settings.ENABLE_API_DOCS,
)
saved_clients = dict(oidc_module.oauth._clients)
saved_registry = dict(oidc_module.oauth._registry)
monkeypatch.setenv("ENV", "staging")
try:
_install_oidc_settings("staging")
settings.ENABLE_API_DOCS = True
_install_oidc_mock(oidc_mock)
app = create_app(testing=False)
yield app
finally:
(
settings.ENV,
settings.SECRET_KEY,
settings.OIDC_CLIENT_SECRETS,
settings.CLOUDFLARE_TEAM_DOMAIN,
settings.SQLALCHEMY_DATABASE_URI,
settings.CLOUDSQL_CONNECTION_NAME,
settings.ALLOWED_HOSTS,
settings.ENABLE_MCP,
settings.ENABLE_API_DOCS,
) = saved
_restore_oidc_registry(saved_clients, saved_registry)


@pytest.fixture
async def oidc_client(oidc_app: FastAPI) -> AsyncGenerator[httpx.AsyncClient, None]:
# Use https so the cookie jar will replay a Secure-flagged session cookie
Expand Down Expand Up @@ -267,6 +325,24 @@ def _read_session_from_set_cookie(set_cookie_header: str) -> dict[str, Any] | No
# ---------------------------------------------------------------------------


async def test_dev_app_exposes_api_docs(dev_app: FastAPI) -> None:
# Development always mounts the OpenAPI docs and schema.
assert dev_app.docs_url == "/api/docs"
assert dev_app.openapi_url == "/api/openapi.json"


async def test_staging_app_hides_api_docs_by_default(staging_app: FastAPI) -> None:
# Outside development the docs are off unless ENABLE_API_DOCS is set.
assert staging_app.docs_url is None
assert staging_app.openapi_url is None


async def test_staging_app_exposes_api_docs_when_enabled(oidc_app_with_docs: FastAPI) -> None:
# ENABLE_API_DOCS=true mounts the docs even in staging/production.
assert oidc_app_with_docs.docs_url == "/api/docs"
assert oidc_app_with_docs.openapi_url == "/api/openapi.json"


async def test_unauthenticated_protected_endpoint_redirects_to_oidc_login(
oidc_client: httpx.AsyncClient,
) -> None:
Expand Down