diff --git a/CLAUDE.md b/CLAUDE.md index 2732f12..8d8d6d3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -746,6 +746,15 @@ purpose - the collection is `api_tokens`, the prefix is `vt_`, the routes are `/api/tokens`, and renaming a published endpoint to match a nav label would break every script already written against it. +**Every `/api` route must be declared twice or the server refuses to boot**: +once in `routeScopes` (or `sessionOnlyRoutes`) and once in `serverScopedRoutes`. +`sessionOnlyRoutes` holds the MFA routes, which no API token may reach - a +token that could enrol a passkey or satisfy step-up would be a way around MFA. +They have no scope on purpose, and `RequireScopes` refuses any unmapped route +to a token. `TestRegisteredRoutesPassBootAssertions` registers the real routes +and runs both boot assertions, so a missing declaration fails CI rather than +production startup. + `server/internal/api/docs/openapi.json` is a **generated, committed** OpenAPI 3.1 document - `swag v2` reading `@…` annotations off the handlers - served at `GET /api/openapi.json` and rendered as a reference page by a vendored Scalar diff --git a/server/internal/api/bootassert_test.go b/server/internal/api/bootassert_test.go new file mode 100644 index 0000000..fd5d222 --- /dev/null +++ b/server/internal/api/bootassert_test.go @@ -0,0 +1,42 @@ +package api + +import ( + "strings" + "testing" + + "github.com/gin-gonic/gin" +) + +// The two boot assertions only run against a real engine in main.go, so a +// route added without its declarations compiled, passed every unit test and +// then refused to start in production. This registers the real routes and +// runs both assertions, so that failure lands in CI instead. +func TestRegisteredRoutesPassBootAssertions(t *testing.T) { + gin.SetMode(gin.TestMode) + r := gin.New() + RegisterRoutes(r) + + if err := AssertScopeMapComplete(r); err != nil { + t.Fatalf("scope map: %v", err) + } + + var routes []string + for _, route := range r.Routes() { + if strings.HasPrefix(route.Path, "/api/") { + routes = append(routes, route.Method+" "+route.Path) + } + } + if err := AssertServerScopeMapComplete(routes); err != nil { + t.Fatalf("server scope map: %v", err) + } +} + +// A route cannot be both session-only and token-reachable: an entry in both +// maps would silently hand the MFA routes to any token holding that scope. +func TestSessionOnlyRoutesHaveNoTokenScope(t *testing.T) { + for route := range sessionOnlyRoutes { + if scope, ok := routeScopes[route]; ok { + t.Errorf("%s is session-only but routeScopes grants it to tokens with %q", route, scope) + } + } +} diff --git a/server/internal/api/scopes.go b/server/internal/api/scopes.go index d0a939d..e7ad2e3 100644 --- a/server/internal/api/scopes.go +++ b/server/internal/api/scopes.go @@ -238,6 +238,30 @@ var routesOutsideAPIGroup = map[string]bool{ "GET /api/secrets/:group/values": true, } +// sessionOnlyRoutes are /api routes that an API token must never reach, so they +// deliberately have no entry in routeScopes. RequireScopes already refuses any +// unmapped route to a token; this set is what tells AssertScopeMapComplete the +// absence is a decision rather than an omission. +// +// Every entry manages the caller's own second factors or proves the caller is +// present. A token that could enrol a passkey, regenerate recovery codes or +// satisfy step-up would turn one leaked token into a way around MFA itself. +var sessionOnlyRoutes = map[string]bool{ + "GET /api/me/mfa": true, + "POST /api/me/mfa/totp/setup": true, + "POST /api/me/mfa/totp/confirm": true, + "DELETE /api/me/mfa/totp": true, + "POST /api/me/mfa/recovery/regenerate": true, + "POST /api/me/passkeys/begin": true, + "POST /api/me/passkeys/finish": true, + "PATCH /api/me/passkeys/:id": true, + "DELETE /api/me/passkeys/:id": true, + "POST /api/me/step-up": true, + "POST /api/me/step-up/webauthn/begin": true, + "POST /api/me/step-up/webauthn/finish": true, + "DELETE /api/org/users/:id/mfa": true, +} + // AssertScopeMapComplete fails boot when a registered /api route has no scope. // // Without it, adding a route silently makes it unreachable by every token, and @@ -253,6 +277,10 @@ func AssertScopeMapComplete(r *gin.Engine) error { if routesOutsideAPIGroup[route.Method+" "+route.Path] { continue } + // Session-only routes are refused to every token by having no scope. + if sessionOnlyRoutes[route.Method+" "+route.Path] { + continue + } if _, ok := routeScopes[route.Method+" "+route.Path]; !ok { missing = append(missing, route.Method+" "+route.Path) } diff --git a/server/internal/api/serverscope.go b/server/internal/api/serverscope.go index 46b4785..166f4e8 100644 --- a/server/internal/api/serverscope.go +++ b/server/internal/api/serverscope.go @@ -424,6 +424,23 @@ var serverScopedRoutes = map[string]scopeDecl{ "GET /api/patch-runs": scoped, "GET /api/patch-runs/:runId": scoped, "POST /api/patch-runs/:runId/cancel": scoped, + + // Multi-factor authentication. These act on the caller's own factors (or, + // for the reset, on another member's), never on a server, and no API token + // can reach them at all - see sessionOnlyRoutes. + "GET /api/me/mfa": exempt, + "POST /api/me/mfa/totp/setup": exempt, + "POST /api/me/mfa/totp/confirm": exempt, + "DELETE /api/me/mfa/totp": exempt, + "POST /api/me/mfa/recovery/regenerate": exempt, + "POST /api/me/passkeys/begin": exempt, + "POST /api/me/passkeys/finish": exempt, + "PATCH /api/me/passkeys/:id": exempt, + "DELETE /api/me/passkeys/:id": exempt, + "POST /api/me/step-up": exempt, + "POST /api/me/step-up/webauthn/begin": exempt, + "POST /api/me/step-up/webauthn/finish": exempt, + "DELETE /api/org/users/:id/mfa": exempt, } // AssertServerScopeMapComplete refuses to boot when any registered /api route diff --git a/server/internal/api/serverscope_test.go b/server/internal/api/serverscope_test.go index b538151..497b959 100644 --- a/server/internal/api/serverscope_test.go +++ b/server/internal/api/serverscope_test.go @@ -16,7 +16,7 @@ func TestServerScopeMapCoversEveryScopedRoute(t *testing.T) { if _, ok := routeScopes[r]; ok { continue } - if routesOutsideAPIGroup[r] { + if routesOutsideAPIGroup[r] || sessionOnlyRoutes[r] { continue } t.Errorf("route %q is declared in serverScopedRoutes but is not a registered route", r)