You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Handler.requireAuth (added in #84 via the router-layer enforcement for AuthUser, see internal/api/middleware.go:220) returns only error. It internally resolves the caller's identity via Handler.authenticate() (admin API key → user API key → bearer-token session, internal/api/middleware.go:44-57), but throws that resolved identity away.
Most AuthUser handlers then need the session/principal anyway and re-resolve it inside the handler — a redundant pass through the same code path that was just executed.
AuthUser routes that need the resolved identity (non-exhaustive, from internal/api/router.go):
/api/auth/logout (POST)
/api/auth/me (GET)
/api/auth/profile (PUT)
/api/auth/change-password (POST)
/api/api-keys (GET/POST)
/api/api-keys/:id/revoke (POST)
/api/api-keys/:id (DELETE)
/api/federation/iac (GET)
/api/commitment-options (GET)
Proposal
Refactor authenticate() (or add a sibling) to return the resolved principal alongside the auth verdict, and propagate it through requireAuth so handlers can take it from a single source instead of re-resolving.
Router dispatch keeps its current shape; the handler signatures that need identity gain a *Principal parameter (or read it from a request-scoped context value if a parameter change is too invasive).
Constraints
Must not regress credential acceptance. All three principal kinds (admin API key, user API key, bearer session) must remain accepted on AuthUser routes. PR fix(security): enforce AuthUser at the router layer, not just middleware (closes #60) #178 attempted a similar refactor with a narrower helper (requireUser) and silently dropped user API keys — that's the failure mode to avoid.
No threat-model change. This is purely an ergonomics refactor; it must produce the same auth decision as requireAuth for every input.
Tests: extend internal/api/router_authuser_test.go to assert that the Principal returned by requireAuth is correctly populated for each principal kind, and that handlers wired to use it observe consistent identity.
Out of scope
AuthAdmin / AuthInternal enforcement (already covered by their own helpers).
Changing what counts as a valid AuthUser credential.
Background
Surfaced while triaging PR #178 (which is being closed as superseded by #84). #178's requireUser returned (*Session, error) and was the seed of this idea, but its narrowing of accepted credentials made it a regression rather than an improvement. This issue captures the ergonomic part of #178's intent without the security regression.
Problem
Handler.requireAuth(added in #84 via the router-layer enforcement forAuthUser, seeinternal/api/middleware.go:220) returns onlyerror. It internally resolves the caller's identity viaHandler.authenticate()(admin API key → user API key → bearer-token session,internal/api/middleware.go:44-57), but throws that resolved identity away.Most
AuthUserhandlers then need the session/principal anyway and re-resolve it inside the handler — a redundant pass through the same code path that was just executed.AuthUserroutes that need the resolved identity (non-exhaustive, frominternal/api/router.go):/api/auth/logout(POST)/api/auth/me(GET)/api/auth/profile(PUT)/api/auth/change-password(POST)/api/api-keys(GET/POST)/api/api-keys/:id/revoke(POST)/api/api-keys/:id(DELETE)/api/federation/iac(GET)/api/commitment-options(GET)Proposal
Refactor
authenticate()(or add a sibling) to return the resolved principal alongside the auth verdict, and propagate it throughrequireAuthso handlers can take it from a single source instead of re-resolving.Sketch:
Router dispatch keeps its current shape; the handler signatures that need identity gain a
*Principalparameter (or read it from a request-scoped context value if a parameter change is too invasive).Constraints
AuthUserroutes. PR fix(security): enforce AuthUser at the router layer, not just middleware (closes #60) #178 attempted a similar refactor with a narrower helper (requireUser) and silently dropped user API keys — that's the failure mode to avoid.requireAuthfor every input.internal/api/router_authuser_test.goto assert that thePrincipalreturned byrequireAuthis correctly populated for each principal kind, and that handlers wired to use it observe consistent identity.Out of scope
AuthAdmin/AuthInternalenforcement (already covered by their own helpers).AuthUsercredential.Background
Surfaced while triaging PR #178 (which is being closed as superseded by #84). #178's
requireUserreturned(*Session, error)and was the seed of this idea, but its narrowing of accepted credentials made it a regression rather than an improvement. This issue captures the ergonomic part of #178's intent without the security regression.