mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-26 14:30:13 +00:00
fix(server): minting or rotating an API token requires an interactive session (BUG-2890) (#1267)
A PAT that could reach POST /auth/tokens minted further tokens with
independent names and expiries. Those survive the revocation of the token
that created them, and nothing in the token list records which token
minted which -- so revoking a leaked credential did not end the access it
had been used to establish.
Ruled on the item's trail day 57: create and rotate require session auth
and refuse a PAT-authenticated call with 403 session_required; list and
revoke stay PAT-reachable.
Population is THREE doors, not the two the filing named. Enumerating every
route that reaches store.CreateAPIToken / RotateAPIToken turns up the
workspace-scoped mint at POST /workspaces/{ws}/tokens, which had only
requireMinRole("owner") in front of it -- and a user-owned PAT held by an
owner satisfies that. Measured, not read: with the fixture's owner also
written into workspace_members, that door returned 201 with a live token
on the unfixed tree. All three doors are gated; list and revoke on both
the user and workspace routes are deliberately untouched, because neither
extends access and revocation is the compromised-credential response.
The gate is on the CREDENTIAL, not on the door: isAPITokenAuth is false
for a session cookie AND for a padsess_ CLI bearer, a distinction that
predates this fix (see ctxValidatedSessionBearer's note in
middleware_auth.go) because a CLI session IS an interactive session. At
the workspace door the check runs before requireMinRole, so a PAT-borne
caller cannot learn its own membership status from the difference between
the two 403s.
Five mutants run, each detected by its own leg: dropping the gate at each
of the three doors fails that door's test; gating on Authorization != ""
instead of the credential kind fails the CLI-session leg AND NOTHING ELSE;
over-applying the gate to list and revoke fails both PAT-still-works
controls. The fourth is why the CLI leg exists -- a header-shaped gate
passes every PAT assertion and breaks every logged-in CLI.
Also documents the refusal on the CLAUDE.md route line: the routes were
listed without saying anything about credential kind, so an agent holding
a PAT would have met an unexplained 403.
Claude-Session: https://claude.ai/code/session_01HeChkgZVYb3NTgTcckF5KR
This commit is contained in:
xarmian
@@ -107,7 +107,7 @@ Both SSE endpoints share one admission budget, enforced **per instance**: `PAD_S
|
||||
- `POST /api/v1/auth/forgot-password` — request password reset email
|
||||
- `POST /api/v1/auth/reset-password` — reset password with token
|
||||
- `POST /api/v1/auth/local-reset` — localhost-only account recovery (self-host, non-cloud). Loopback-gated, no auth — the bootstrap trust model. Returns a single-use reset link, or a temp password with `{"temp_password": true}`. Backs `pad auth reset-password`.
|
||||
- `GET/POST/DELETE /api/v1/auth/tokens` — user-scoped API tokens
|
||||
- `GET/POST/DELETE /api/v1/auth/tokens` — user-scoped API tokens. **Minting and rotating require an INTERACTIVE SESSION** (BUG-2890): a call authenticated by a PAT is refused `403 session_required` on `POST /auth/tokens`, `POST /auth/tokens/{id}/rotate` and `POST /workspaces/{ws}/tokens`, because a token that can mint tokens outlives its own revocation. A session cookie and a `padsess_` CLI bearer both count as interactive; LIST and REVOKE stay PAT-reachable, deliberately — revocation is the compromised-credential response
|
||||
- `GET/PATCH /api/v1/admin/settings` — platform settings (admin-only)
|
||||
- `POST /api/v1/admin/test-email` — send test email (admin-only)
|
||||
- `POST /api/v1/invitations/{code}/accept` — accept workspace invitation
|
||||
|
||||
@@ -32,9 +32,51 @@ func (s *Server) getTokenExpirySettings() (defaultDays, maxDays int) {
|
||||
return
|
||||
}
|
||||
|
||||
// requireInteractiveSession refuses a request authenticated by a long-lived
|
||||
// API token, and is the gate on every door that MINTS or ROTATES one
|
||||
// (BUG-2890, ruled on that item's trail day 57).
|
||||
//
|
||||
// The escalation it closes: a PAT that can reach a mint door issues further
|
||||
// tokens with independent names and expiries, which outlive the revocation
|
||||
// of the token that created them. Revoking a leaked credential then does
|
||||
// not end the access it was used to establish, and nothing in the token
|
||||
// list says which token minted which.
|
||||
//
|
||||
// It gates the CREDENTIAL, not the door. `isAPITokenAuth` is false for a
|
||||
// session cookie AND for a `padsess_` CLI bearer — that distinction is
|
||||
// deliberate and predates this fix (see ctxValidatedSessionBearer's note in
|
||||
// middleware_auth.go): a CLI session IS an interactive session. A gate
|
||||
// written against "carries an Authorization: Bearer header" would look
|
||||
// identical on every PAT test and break every logged-in CLI.
|
||||
//
|
||||
// LIST and REVOKE deliberately stay reachable by a PAT. Neither extends
|
||||
// access, and revocation is the compromised-credential response — gating it
|
||||
// behind a browser would put a session in the way of the incident path.
|
||||
//
|
||||
// Same shape as the 2FA handlers' gate; the code differs because this one
|
||||
// is `session_required` rather than the generic `forbidden`, so a client
|
||||
// can tell "wrong credential kind" from "insufficient permissions".
|
||||
func requireInteractiveSession(w http.ResponseWriter, r *http.Request) bool {
|
||||
if isAPITokenAuth(r) {
|
||||
writeError(w, http.StatusForbidden, "session_required",
|
||||
"Creating or rotating API tokens requires an interactive session, not an API token")
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// handleCreateToken creates a new API token scoped to a workspace.
|
||||
// The token is owned by the authenticated user (if any).
|
||||
func (s *Server) handleCreateToken(w http.ResponseWriter, r *http.Request) {
|
||||
// Before requireMinRole, not after: this door mints through the same
|
||||
// store call as /auth/tokens and is reachable by an OWNER's PAT
|
||||
// (measured on BUG-2890's trail — the filing named only the two
|
||||
// /auth/tokens doors). Checking the credential kind first also stops
|
||||
// a PAT-borne caller learning its own membership status from the
|
||||
// difference between the two 403s.
|
||||
if !requireInteractiveSession(w, r) {
|
||||
return
|
||||
}
|
||||
if !requireMinRole(w, r, "owner") {
|
||||
return
|
||||
}
|
||||
@@ -144,6 +186,10 @@ func (s *Server) handleListUserTokens(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
// handleCreateUserToken creates a new API token owned by the authenticated user.
|
||||
func (s *Server) handleCreateUserToken(w http.ResponseWriter, r *http.Request) {
|
||||
if !requireInteractiveSession(w, r) {
|
||||
return
|
||||
}
|
||||
|
||||
userID := currentUserID(r)
|
||||
if userID == "" {
|
||||
writeError(w, http.StatusUnauthorized, "unauthorized", "Not logged in")
|
||||
@@ -205,6 +251,10 @@ func (s *Server) handleDeleteUserToken(w http.ResponseWriter, r *http.Request) {
|
||||
// handleRotateUserToken generates a new secret for an existing token,
|
||||
// invalidating the old one. The token metadata is preserved.
|
||||
func (s *Server) handleRotateUserToken(w http.ResponseWriter, r *http.Request) {
|
||||
if !requireInteractiveSession(w, r) {
|
||||
return
|
||||
}
|
||||
|
||||
userID := currentUserID(r)
|
||||
if userID == "" {
|
||||
writeError(w, http.StatusUnauthorized, "unauthorized", "Not logged in")
|
||||
|
||||
@@ -0,0 +1,188 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// BUG-2890: minting and rotating API tokens requires an INTERACTIVE
|
||||
// SESSION. A PAT that can reach the create door mints further tokens with
|
||||
// independent names and expiries, which survive the revocation of the
|
||||
// token that made them — so revoking a leaked credential does not end the
|
||||
// access it was used to establish.
|
||||
//
|
||||
// Dave ruled the shape on day 57: create and rotate require session auth
|
||||
// and refuse a PAT-authenticated call with 403 `session_required`; list
|
||||
// and revoke stay PAT-reachable, because neither extends access.
|
||||
//
|
||||
// The controls matter more than usual here. A PAT-refused leg alone is
|
||||
// satisfied by a handler that refuses EVERYONE, and a gate written against
|
||||
// "carries an Authorization header" rather than against "is a PAT" would
|
||||
// pass every leg below except the CLI one — `padsess_` Bearer credentials
|
||||
// are interactive sessions and must keep working.
|
||||
|
||||
type tokenAuthEnv struct {
|
||||
srv *Server
|
||||
user *models.User
|
||||
ws *models.Workspace
|
||||
sessTok string // padsess_ — cookie AND CLI bearer
|
||||
patTok string // pad_ — the credential under test
|
||||
patID string
|
||||
spareID string // a second PAT, so revoke/rotate legs need not eat the one they authenticate with
|
||||
}
|
||||
|
||||
func setupTokenAuthEnv(t *testing.T) *tokenAuthEnv {
|
||||
t.Helper()
|
||||
srv := testServer(t)
|
||||
|
||||
user, err := srv.store.CreateUser(models.UserCreate{
|
||||
Email: "tokens@example.com", Name: "Token Owner",
|
||||
Password: "correct-horse-battery-staple", Role: "member",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("create user: %v", err)
|
||||
}
|
||||
ws, err := srv.store.CreateWorkspace(models.WorkspaceCreate{Name: "Tokens WS", OwnerID: user.ID})
|
||||
if err != nil {
|
||||
t.Fatalf("create workspace: %v", err)
|
||||
}
|
||||
// MEMBERSHIP IS LOAD-BEARING FOR THE WORKSPACE-DOOR LEG, not setup
|
||||
// boilerplate. CreateWorkspace records an owner_id; it does not add a
|
||||
// workspace_members row, and RequireWorkspaceAccess gates on
|
||||
// membership. Without this the workspace-door test gets its 403 from
|
||||
// "not a member" and discriminates nothing about the session gate —
|
||||
// measured, not assumed: the first run of this file failed on exactly
|
||||
// that, with `You are not a member of this workspace`.
|
||||
if err := srv.store.AddWorkspaceMember(ws.ID, user.ID, "owner"); err != nil {
|
||||
t.Fatalf("add owner as member: %v", err)
|
||||
}
|
||||
|
||||
pat, err := srv.store.CreateAPIToken(user.ID, models.APITokenCreate{
|
||||
Name: "the-pat", WorkspaceID: ws.ID,
|
||||
}, 0, 0)
|
||||
if err != nil {
|
||||
t.Fatalf("CreateAPIToken: %v", err)
|
||||
}
|
||||
spare, err := srv.store.CreateAPIToken(user.ID, models.APITokenCreate{
|
||||
Name: "spare", WorkspaceID: ws.ID,
|
||||
}, 0, 0)
|
||||
if err != nil {
|
||||
t.Fatalf("CreateAPIToken (spare): %v", err)
|
||||
}
|
||||
|
||||
return &tokenAuthEnv{
|
||||
srv: srv,
|
||||
user: user,
|
||||
ws: ws,
|
||||
sessTok: loginUser(t, srv, "tokens@example.com", "correct-horse-battery-staple"),
|
||||
patTok: pat.Token,
|
||||
patID: pat.ID,
|
||||
spareID: spare.ID,
|
||||
}
|
||||
}
|
||||
|
||||
// THE RULE, create: a PAT cannot mint a token.
|
||||
func TestCreateUserToken_PATIsRefused(t *testing.T) {
|
||||
t.Parallel()
|
||||
env := setupTokenAuthEnv(t)
|
||||
|
||||
rr := doRequestWithBearer(env.srv, "POST", "/api/v1/auth/tokens", env.patTok,
|
||||
map[string]any{"name": "minted-by-a-pat"})
|
||||
if rr.Code != http.StatusForbidden {
|
||||
t.Fatalf("PAT minted a token: got %d, want 403; body=%s", rr.Code, rr.Body.String())
|
||||
}
|
||||
if got := errorCode(t, rr); got != "session_required" {
|
||||
t.Errorf("error code = %v, want session_required; body=%s", got, rr.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// THE RULE, rotate: a PAT cannot rotate a token. Rotation re-issues a
|
||||
// secret and can extend an expiry, so it establishes access exactly the
|
||||
// way create does.
|
||||
func TestRotateUserToken_PATIsRefused(t *testing.T) {
|
||||
t.Parallel()
|
||||
env := setupTokenAuthEnv(t)
|
||||
|
||||
rr := doRequestWithBearer(env.srv, "POST", "/api/v1/auth/tokens/"+env.spareID+"/rotate", env.patTok, nil)
|
||||
if rr.Code != http.StatusForbidden {
|
||||
t.Fatalf("PAT rotated a token: got %d, want 403; body=%s", rr.Code, rr.Body.String())
|
||||
}
|
||||
if got := errorCode(t, rr); got != "session_required" {
|
||||
t.Errorf("error code = %v, want session_required; body=%s", got, rr.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// THE THIRD DOOR (population, not the two the filing named): the
|
||||
// workspace-scoped mint at POST /workspaces/{ws}/tokens reaches the same
|
||||
// store call with only requireMinRole("owner") in front of it, and a
|
||||
// user-owned PAT held by an owner satisfies that.
|
||||
func TestCreateWorkspaceToken_PATIsRefused(t *testing.T) {
|
||||
t.Parallel()
|
||||
env := setupTokenAuthEnv(t)
|
||||
|
||||
rr := doRequestWithBearer(env.srv, "POST", "/api/v1/workspaces/"+env.ws.Slug+"/tokens", env.patTok,
|
||||
map[string]any{"name": "minted-by-a-pat-workspace-door"})
|
||||
if rr.Code != http.StatusForbidden {
|
||||
t.Fatalf("PAT minted a workspace-scoped token: got %d, want 403; body=%s", rr.Code, rr.Body.String())
|
||||
}
|
||||
if got := errorCode(t, rr); got != "session_required" {
|
||||
t.Errorf("error code = %v, want session_required; body=%s", got, rr.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// CONTROL — the gate is on the CREDENTIAL, not on the door. A browser
|
||||
// session still mints. Without this leg, a handler that refuses everyone
|
||||
// passes every assertion above.
|
||||
func TestCreateUserToken_SessionCookieStillWorks(t *testing.T) {
|
||||
t.Parallel()
|
||||
env := setupTokenAuthEnv(t)
|
||||
|
||||
rr := doRequestWithCookie(env.srv, "POST", "/api/v1/auth/tokens",
|
||||
map[string]any{"name": "minted-by-a-session"}, env.sessTok)
|
||||
if rr.Code != http.StatusCreated {
|
||||
t.Fatalf("session could not mint a token: got %d, want 201; body=%s", rr.Code, rr.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// CONTROL — and the sharpest one. A `padsess_` CLI bearer IS an
|
||||
// interactive session: ctxValidatedSessionBearer, not ctxIsAPIToken. A
|
||||
// gate written against "has an Authorization: Bearer header" would pass
|
||||
// every other leg in this file and break every logged-in CLI.
|
||||
func TestCreateUserToken_CLISessionBearerStillWorks(t *testing.T) {
|
||||
t.Parallel()
|
||||
env := setupTokenAuthEnv(t)
|
||||
|
||||
rr := doRequestWithBearer(env.srv, "POST", "/api/v1/auth/tokens", env.sessTok,
|
||||
map[string]any{"name": "minted-by-a-cli-session"})
|
||||
if rr.Code != http.StatusCreated {
|
||||
t.Fatalf("a CLI session bearer was refused: got %d, want 201; body=%s", rr.Code, rr.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// CONTROL — list stays PAT-reachable. Reading which tokens exist extends
|
||||
// no access, and gating it would break every agent that inventories its
|
||||
// own credentials.
|
||||
func TestListUserTokens_PATStillWorks(t *testing.T) {
|
||||
t.Parallel()
|
||||
env := setupTokenAuthEnv(t)
|
||||
|
||||
rr := doRequestWithBearer(env.srv, "GET", "/api/v1/auth/tokens", env.patTok, nil)
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("PAT could not list tokens: got %d, want 200; body=%s", rr.Code, rr.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// CONTROL — revoke stays PAT-reachable, and this is the load-bearing one:
|
||||
// a compromised-credential response is REVOCATION, so gating it behind a
|
||||
// session would make the incident path need a browser.
|
||||
func TestDeleteUserToken_PATStillWorks(t *testing.T) {
|
||||
t.Parallel()
|
||||
env := setupTokenAuthEnv(t)
|
||||
|
||||
rr := doRequestWithBearer(env.srv, "DELETE", "/api/v1/auth/tokens/"+env.spareID, env.patTok, nil)
|
||||
if rr.Code != http.StatusNoContent {
|
||||
t.Fatalf("PAT could not revoke a token: got %d, want 204; body=%s", rr.Code, rr.Body.String())
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user