fix(web): gate /docs, /redoc, /openapi.json behind login
FastAPI auto-registers these with no auth of its own -- every other data route is individually wrapped in Depends(require_user), but these three are wired up internally by the FastAPI() constructor and were never covered by that. Confirmed live: all three returned 200 with zero session, handing anyone who could reach the port a full map of every route -- VM remove, cache purge, template delete, service control included -- without logging in. Disabled FastAPI's built-in docs_url/redoc_url/openapi_url and re-added all three as ordinary routes behind the same require_user dependency every other route uses. Verified against the live deployment: all three now 303 to /login instead of 200.
This commit is contained in:
@@ -152,6 +152,12 @@ reinstalls the `[web]` extra and restarts the service in one step (steps
|
||||
address while `[web.oidc]` is absent — an unauthenticated dashboard that
|
||||
can destroy VMs and purge caches must never be reachable from the LAN.
|
||||
Configure `[web.oidc]` first, or bind `127.0.0.1`.
|
||||
- **`/docs`, `/redoc`, `/openapi.json` are gated behind login too.** FastAPI
|
||||
auto-registers these with no auth of their own; the app disables its
|
||||
built-in ones (`docs_url=None` etc.) and re-adds them behind
|
||||
`require_user`, same as every data route — otherwise anyone who can reach
|
||||
the port gets a full map of every route (including every destructive one)
|
||||
with no session needed.
|
||||
- Every mutating route (VM remove/cleanup, cache purge, template backup,
|
||||
template delete, backup delete) is behind a synchronizer-token CSRF check
|
||||
and a two-step confirm gate (a preview/dry-run, then an explicit "Confirm"
|
||||
|
||||
@@ -32,9 +32,10 @@ from __future__ import annotations
|
||||
import secrets
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from typing import TYPE_CHECKING
|
||||
from typing import TYPE_CHECKING, Any
|
||||
|
||||
from fastapi import Depends, FastAPI, Request
|
||||
from fastapi.openapi.docs import get_redoc_html, get_swagger_ui_html
|
||||
from fastapi.responses import HTMLResponse
|
||||
from fastapi.staticfiles import StaticFiles
|
||||
from fastapi.templating import Jinja2Templates
|
||||
@@ -391,6 +392,29 @@ def healthz() -> dict[str, str]:
|
||||
return {"status": "ok"}
|
||||
|
||||
|
||||
def openapi_json(request: Request) -> dict[str, Any]:
|
||||
"""``GET /openapi.json`` -- the raw schema, gated behind ``require_user``.
|
||||
|
||||
FastAPI's own ``openapi_url`` is disabled in :func:`create_app`
|
||||
(it ships with no auth of its own -- every mutating route's path,
|
||||
parameters and docstring would otherwise be readable by anyone who can
|
||||
reach the port, logged in or not). This route re-adds it behind the same
|
||||
dependency every other data route uses.
|
||||
"""
|
||||
app: FastAPI = request.app
|
||||
return app.openapi()
|
||||
|
||||
|
||||
def swagger_docs() -> HTMLResponse:
|
||||
"""``GET /docs`` -- Swagger UI, gated the same way as :func:`openapi_json`."""
|
||||
return get_swagger_ui_html(openapi_url="/openapi.json", title="ci-orchestrator web - Swagger UI")
|
||||
|
||||
|
||||
def redoc_docs() -> HTMLResponse:
|
||||
"""``GET /redoc`` -- ReDoc, gated the same way as :func:`openapi_json`."""
|
||||
return get_redoc_html(openapi_url="/openapi.json", title="ci-orchestrator web - ReDoc")
|
||||
|
||||
|
||||
async def dashboard(
|
||||
request: Request, backend: VmBackend | None = Depends(get_backend)
|
||||
) -> HTMLResponse:
|
||||
@@ -568,7 +592,10 @@ def create_app(config: Config) -> FastAPI:
|
||||
register_action_routes,
|
||||
)
|
||||
|
||||
app = FastAPI(title="ci-orchestrator web")
|
||||
# docs_url/redoc_url/openapi_url are FastAPI's own auto-registered routes
|
||||
# with no auth of their own -- disabled here and re-added below behind
|
||||
# require_user, same as every other data route.
|
||||
app = FastAPI(title="ci-orchestrator web", docs_url=None, redoc_url=None, openapi_url=None)
|
||||
app.state.config = config
|
||||
app.state.templates = Jinja2Templates(directory=str(_WEB_DIR / "templates"))
|
||||
filters.register(app.state.templates)
|
||||
@@ -597,6 +624,21 @@ def create_app(config: Config) -> FastAPI:
|
||||
app.mount("/static", StaticFiles(directory=str(_WEB_DIR / "static")), name="static")
|
||||
|
||||
app.get("/healthz")(healthz)
|
||||
app.get("/openapi.json", dependencies=[Depends(require_user)], include_in_schema=False)(
|
||||
openapi_json
|
||||
)
|
||||
app.get(
|
||||
"/docs",
|
||||
response_class=HTMLResponse,
|
||||
dependencies=[Depends(require_user)],
|
||||
include_in_schema=False,
|
||||
)(swagger_docs)
|
||||
app.get(
|
||||
"/redoc",
|
||||
response_class=HTMLResponse,
|
||||
dependencies=[Depends(require_user)],
|
||||
include_in_schema=False,
|
||||
)(redoc_docs)
|
||||
app.get("/", response_class=HTMLResponse, dependencies=[Depends(require_user)])(dashboard)
|
||||
app.get(
|
||||
"/executions", response_class=HTMLResponse, dependencies=[Depends(require_user)]
|
||||
|
||||
@@ -94,6 +94,30 @@ def test_healthz_returns_ok(tmp_path: Path) -> None:
|
||||
assert resp.json() == {"status": "ok"}
|
||||
|
||||
|
||||
# ── /docs, /redoc, /openapi.json ────────────────────────────────────────
|
||||
# Gated behind require_user like every other data route (test_web_auth.py's
|
||||
# _PROTECTED_ROUTES proves the redirect-when-unauthenticated half); this
|
||||
# proves the positive path still works once past that gate.
|
||||
|
||||
|
||||
def test_openapi_json_lists_a_mutating_route(tmp_path: Path) -> None:
|
||||
app = create_app(_make_config(tmp_path))
|
||||
client = TestClient(app)
|
||||
|
||||
resp = client.get("/openapi.json")
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert "/vms/{vmx_id}/remove" in resp.json()["paths"]
|
||||
|
||||
|
||||
def test_docs_and_redoc_render(tmp_path: Path) -> None:
|
||||
app = create_app(_make_config(tmp_path))
|
||||
client = TestClient(app)
|
||||
|
||||
assert client.get("/docs").status_code == 200
|
||||
assert client.get("/redoc").status_code == 200
|
||||
|
||||
|
||||
# ── / (dashboard) ────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
|
||||
@@ -47,7 +47,7 @@ _OIDC_CRED_NAME = "TestWebOidcClientSecret"
|
||||
_FAKE_SESSION_SECRET = "sess-secret-do-not-leak-abc123"
|
||||
_FAKE_OIDC_CLIENT_SECRET = "oidc-client-secret-do-not-leak-xyz987"
|
||||
|
||||
_PROTECTED_ROUTES = ["/", "/executions", "/cache", "/host"]
|
||||
_PROTECTED_ROUTES = ["/", "/executions", "/cache", "/host", "/docs", "/redoc", "/openapi.json"]
|
||||
|
||||
|
||||
# ── shared fixtures / fakes ─────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user