mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-25 14:00:40 +00:00
fix(auth): platform administration refuses an API token (BUG-3361)
BREAKING for anyone driving /api/v1/admin/* or admin account creation with a personal access token: those now answer 403 session_required. Use a `pad auth login` CLI session, or on self-host set PAD_ADMIN_API_TOKENS=allow. An admin's PAT carried full platform authority. codex reproduced the chain during BUG-3336's review: register an account with a chosen password, promote it to admin, sign in as it, and reset any account's password. A leaked admin token therefore escalated to interactive sessions for any account. - refuseAPITokenForAdmin guards the whole /api/v1/admin route group, so an admin route added later is refused by construction. TestBUG3361_EveryAdminRouteRefusesAnAdminPAT walks the router to hold it. - handleRegister refuses admin-create signup with an API token. - PAD_ADMIN_API_TOKENS=allow is the self-host escape hatch. It is read at startup, logs a WARN, and is never honoured in cloud mode. Documented in docs/deployment.md, Security. - A browser session and a CLI session (padsess_) are unaffected. So is the Pad Cloud sidecar, which authenticates with the cloud secret, never a PAT. /audit-log already refused a PAT (BUG-3334). Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
This commit is contained in:
@@ -1096,6 +1096,7 @@ degraded, not a peer broken.
|
||||
|----------|---------|-------------|
|
||||
| `PAD_SECURE_COOKIES` | `false` | Set `Secure` flag on session cookies (requires TLS) |
|
||||
| `PAD_CORS_ORIGINS` | — | Comma-separated allowed CORS origins |
|
||||
| `PAD_ADMIN_API_TOKENS` | off | `allow` lets an admin's API token (PAT) use the platform administration API (`/api/v1/admin/*`, and creating accounts). By default these require an interactive session, a browser or `pad auth login`, because a leaked admin token could otherwise create and promote accounts. For operators who script admin tasks; logs a warning at startup; ignored on Pad Cloud. |
|
||||
|
||||
### MCP for agents (self-hosted)
|
||||
|
||||
|
||||
@@ -0,0 +1,110 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"regexp"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// BUG-3361: platform administration requires an interactive session. An
|
||||
// admin's API token is refused on every /api/v1/admin route and on
|
||||
// admin-create signup, unless a self-hosted operator opted in with
|
||||
// PAD_ADMIN_API_TOKENS=allow (never honoured on Pad Cloud).
|
||||
|
||||
type adminPATFixture struct {
|
||||
srv *Server
|
||||
pat string
|
||||
session string
|
||||
}
|
||||
|
||||
func newAdminPATFixture(t *testing.T) adminPATFixture {
|
||||
t.Helper()
|
||||
srv := testServer(t)
|
||||
admin, err := srv.store.CreateUser(models.UserCreate{Email: "admin-3361@example.com", Name: "Admin", Password: "correct-horse-battery-staple", Role: "admin"})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
ws, err := srv.store.CreateWorkspace(models.WorkspaceCreate{Name: "Admin WS", OwnerID: admin.ID})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
pat, err := srv.store.CreateAPIToken(admin.ID, models.APITokenCreate{Name: "admin-pat", WorkspaceID: ws.ID}, 0, 0)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return adminPATFixture{srv: srv, pat: pat.Token, session: loginUser(t, srv, "admin-3361@example.com", "correct-horse-battery-staple")}
|
||||
}
|
||||
|
||||
// Every route the router registers under /api/v1/admin refuses an admin's
|
||||
// PAT, so an admin route added later is refused by construction and by test.
|
||||
func TestBUG3361_EveryAdminRouteRefusesAnAdminPAT(t *testing.T) {
|
||||
t.Setenv("PAD_DISABLE_RATE_LIMITS", "1")
|
||||
f := newAdminPATFixture(t)
|
||||
f.srv.ensureRouter()
|
||||
param := regexp.MustCompile(`\{[^}]*\}`)
|
||||
checked := 0
|
||||
err := chi.Walk(f.srv.router, func(method, route string, _ http.Handler, _ ...func(http.Handler) http.Handler) error {
|
||||
if !strings.HasPrefix(route, "/api/v1/admin/") {
|
||||
return nil
|
||||
}
|
||||
path := param.ReplaceAllString(route, "x")
|
||||
checked++
|
||||
rr := doRequestWithBearer(f.srv, method, path, f.pat, nil)
|
||||
if rr.Code != http.StatusForbidden || !strings.Contains(rr.Body.String(), "session_required") {
|
||||
t.Errorf("admin PAT %s %s: %d %s, want 403 session_required", method, route, rr.Code, rr.Body.String())
|
||||
}
|
||||
return nil
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if checked < 20 {
|
||||
t.Fatalf("only %d admin routes were walked; the walk is not seeing the router", checked)
|
||||
}
|
||||
// The admin's own sessions still administer.
|
||||
if rr := doRequestWithBearer(f.srv, "GET", "/api/v1/admin/settings", f.session, nil); rr.Code != http.StatusOK {
|
||||
t.Fatalf("admin CLI session on /admin/settings: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
if rr := doRequestWithCookie(f.srv, "GET", "/api/v1/admin/settings", nil, f.session); rr.Code != http.StatusOK {
|
||||
t.Fatalf("admin browser session on /admin/settings: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// Admin-create signup is administration too: with a PAT it was the first
|
||||
// step of minting a second admin.
|
||||
func TestBUG3361_AdminCreateSignupRefusesAnAdminPAT(t *testing.T) {
|
||||
f := newAdminPATFixture(t)
|
||||
body := map[string]any{"email": "new-3361@example.com", "name": "New", "password": "a-long-enough-passphrase-3361"}
|
||||
rr := doRequestWithBearer(f.srv, "POST", "/api/v1/auth/register", f.pat, body)
|
||||
if rr.Code != http.StatusForbidden || errorCode(t, rr) != "session_required" {
|
||||
t.Fatalf("admin PAT creating an account: %d %s, want 403 session_required", rr.Code, rr.Body.String())
|
||||
}
|
||||
if u, _ := f.srv.store.GetUserByEmail("new-3361@example.com"); u != nil {
|
||||
t.Fatal("the refused signup created the account")
|
||||
}
|
||||
if rr := doRequestWithBearer(f.srv, "POST", "/api/v1/auth/register", f.session, body); rr.Code != http.StatusCreated {
|
||||
t.Fatalf("admin CLI session creating an account: %d %s, want 201", rr.Code, rr.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// The self-host escape hatch: PAD_ADMIN_API_TOKENS=allow lets an admin PAT
|
||||
// administer, and is ignored on Pad Cloud.
|
||||
func TestBUG3361_EscapeHatchIsSelfHostOnly(t *testing.T) {
|
||||
t.Setenv("PAD_ADMIN_API_TOKENS", "allow")
|
||||
f := newAdminPATFixture(t)
|
||||
if !f.srv.adminAPITokensAllowed {
|
||||
t.Fatal("PAD_ADMIN_API_TOKENS=allow was not read at startup")
|
||||
}
|
||||
if rr := doRequestWithBearer(f.srv, "GET", "/api/v1/admin/settings", f.pat, nil); rr.Code != http.StatusOK {
|
||||
t.Fatalf("admin PAT with the escape hatch on: %d %s, want 200", rr.Code, rr.Body.String())
|
||||
}
|
||||
f.srv.cloudMode = true
|
||||
if rr := doRequestWithBearer(f.srv, "GET", "/api/v1/admin/settings", f.pat, nil); rr.Code != http.StatusForbidden {
|
||||
t.Fatalf("admin PAT with the escape hatch on, in cloud mode: %d, want 403", rr.Code)
|
||||
}
|
||||
}
|
||||
@@ -650,6 +650,12 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
reqUser := currentUser(r)
|
||||
isAdmin := reqUser != nil && reqUser.Role == "admin"
|
||||
switch {
|
||||
case isAdmin && isAPITokenAuth(r) && !s.adminAcceptsAPITokens():
|
||||
// Creating an account is platform administration (BUG-3361):
|
||||
// with a PAT, it was the first step of minting a second admin.
|
||||
writeError(w, http.StatusForbidden, "session_required",
|
||||
"Creating an account requires an interactive session, not an API token")
|
||||
return
|
||||
case isAdmin:
|
||||
// Admin-created account — stays verified.
|
||||
adminCreated = true
|
||||
|
||||
@@ -72,6 +72,10 @@ type Server struct {
|
||||
// load-bearing, not incidental (codex round 3).
|
||||
afterItemPreRead func(itemID string)
|
||||
|
||||
// adminAPITokensAllowed is PAD_ADMIN_API_TOKENS=allow, read at startup
|
||||
// (BUG-3361). See adminAcceptsAPITokens.
|
||||
adminAPITokensAllowed bool
|
||||
|
||||
// visibleCollectionIDsFault is a TEST-ONLY seam, nil in production
|
||||
// (BUG-3334). When set, visibleCollectionIDs calls it first and returns
|
||||
// its error. Every handler that resolves visibility through the helper
|
||||
@@ -654,12 +658,45 @@ func New(s *store.Store) *Server {
|
||||
if disabled, _ := strconv.ParseBool(os.Getenv("PAD_DISABLE_RATE_LIMITS")); disabled {
|
||||
rl = nil
|
||||
}
|
||||
return &Server{
|
||||
store: s,
|
||||
rateLimiters: rl,
|
||||
storageInfoCache: newStorageInfoCache(storageInfoTTL),
|
||||
clientWrites: newClientWriteMarks(),
|
||||
// PAD_ADMIN_API_TOKENS=allow is the self-host escape hatch for operators
|
||||
// who script platform administration with a PAT (BUG-3361). Off by
|
||||
// default, never honoured in cloud mode, and loud when on.
|
||||
adminTokens := os.Getenv("PAD_ADMIN_API_TOKENS") == "allow"
|
||||
if adminTokens {
|
||||
slog.Warn("PAD_ADMIN_API_TOKENS=allow: platform administration accepts API tokens. " +
|
||||
"A leaked admin token can then administer the instance; prefer a CLI session (`pad auth login`). " +
|
||||
"Ignored in cloud mode.")
|
||||
}
|
||||
return &Server{
|
||||
store: s,
|
||||
rateLimiters: rl,
|
||||
storageInfoCache: newStorageInfoCache(storageInfoTTL),
|
||||
clientWrites: newClientWriteMarks(),
|
||||
adminAPITokensAllowed: adminTokens,
|
||||
}
|
||||
}
|
||||
|
||||
// adminAcceptsAPITokens reports whether platform administration accepts an
|
||||
// API token: only when the operator opted in, and never on Pad Cloud.
|
||||
func (s *Server) adminAcceptsAPITokens() bool {
|
||||
return s.adminAPITokensAllowed && !s.cloudMode
|
||||
}
|
||||
|
||||
// refuseAPITokenForAdmin guards every /api/v1/admin route (BUG-3361). An
|
||||
// admin's PAT carries their identity, not their platform authority: one used
|
||||
// to be enough to register an account, promote it to admin, sign in as it,
|
||||
// and reset any password. A browser or `pad auth login` CLI session is
|
||||
// required, the line BUG-2890 drew for token minting. The cloud sidecar
|
||||
// authenticates with the cloud secret, never a PAT, so it is unaffected.
|
||||
func (s *Server) refuseAPITokenForAdmin(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if isAPITokenAuth(r) && !s.adminAcceptsAPITokens() {
|
||||
writeError(w, http.StatusForbidden, "session_required",
|
||||
"Platform administration requires an interactive session, not an API token")
|
||||
return
|
||||
}
|
||||
next.ServeHTTP(w, r)
|
||||
})
|
||||
}
|
||||
|
||||
// Init2FASecret loads the 2FA challenge signing key from platform_settings.
|
||||
@@ -1664,6 +1701,7 @@ func (s *Server) setupRouter() {
|
||||
|
||||
// Admin endpoints (admin-only, handlers check role internally)
|
||||
r.Route("/admin", func(r chi.Router) {
|
||||
r.Use(s.refuseAPITokenForAdmin)
|
||||
r.Get("/settings", s.handleGetPlatformSettings)
|
||||
r.Patch("/settings", s.handleUpdatePlatformSettings)
|
||||
r.Get("/decision-provider", s.handleGetDecisionSettings)
|
||||
|
||||
Reference in New Issue
Block a user