From f8fcecd640660e22773e60cfdd1053351a15f14a Mon Sep 17 00:00:00 2001 From: Ilker Manap Date: Sun, 4 Oct 2026 16:25:56 +0200 Subject: [PATCH 01/17] Audit log: every change through the admin API and the administration pages, with ctl audit Co-Authored-By: Claude Opus 5.5 (1M context) --- internal/audit/audit.go | 152 ++++++++++++++++++ internal/audit/audit_test.go | 60 +++++++ internal/cli/audit.go | 67 ++++++++ internal/cli/ctl.go | 1 + internal/cli/server.go | 14 +- internal/control/adminapi/api.go | 7 + internal/control/adminapi/audit.go | 77 +++++++++ internal/control/adminapi/guard_test.go | 25 +++ internal/router/handlers.go | 3 + internal/store/audit.go | 72 +++++++++ internal/store/audit_test.go | 34 ++++ internal/store/copy.go | 4 +- .../store/migrations/postgres/0020_audit.sql | 15 ++ .../store/migrations/sqlite/0020_audit.sql | 14 ++ internal/web/account.go | 11 +- internal/web/admin_tenancy_test.go | 41 +++++ internal/web/admin_users.go | 6 +- internal/web/audit.go | 85 ++++++++++ 18 files changed, 681 insertions(+), 7 deletions(-) create mode 100644 internal/audit/audit.go create mode 100644 internal/audit/audit_test.go create mode 100644 internal/cli/audit.go create mode 100644 internal/control/adminapi/audit.go create mode 100644 internal/store/audit.go create mode 100644 internal/store/audit_test.go create mode 100644 internal/store/migrations/postgres/0020_audit.sql create mode 100644 internal/store/migrations/sqlite/0020_audit.sql create mode 100644 internal/web/audit.go diff --git a/internal/audit/audit.go b/internal/audit/audit.go new file mode 100644 index 0000000..1270003 --- /dev/null +++ b/internal/audit/audit.go @@ -0,0 +1,152 @@ +// Package audit records who changed what: every change made through the +// admin API and the administration pages, written to the store with the +// request's fields — anything secret taken out — and how it ended. +package audit + +import ( + "bytes" + "context" + "encoding/json" + "io" + "log/slog" + "net/http" + "net/url" + "regexp" + "sort" + "strings" + "time" + + "gitea.mynodes.xyz/ilker/llmgw/internal/store" +) + +// Sink is where events go: the store. +type Sink interface { + WriteAudit(ctx context.Context, e store.AuditEvent) error +} + +// secretName is a field whose value is never recorded. +// The password form's fields are current and new. +var secretName = regexp.MustCompile(`(?i)(pass|secret|token|api_?key|^key$|dsn|private|credential|csrf|cookie|^current$|^new$|^confirm)`) + +// Redact is a request's fields with secrets replaced, as JSON. +func Redact(fields map[string]any) string { + clean := redactValue(fields) + b, _ := json.Marshal(clean) + if len(b) > 8192 { + b = append(b[:8192], []byte("…")...) + } + return string(b) +} + +func redactValue(v any) any { + switch t := v.(type) { + case map[string]any: + out := make(map[string]any, len(t)) + for k, val := range t { + if secretName.MatchString(k) { + out[k] = "***" + continue + } + out[k] = redactValue(val) + } + return out + case []any: + out := make([]any, len(t)) + for i, val := range t { + out[i] = redactValue(val) + } + return out + case string: + // A secret pasted where a name was expected. + if strings.HasPrefix(t, "sk-") || strings.Contains(t, "PRIVATE KEY") { + return "***" + } + return t + } + return v +} + +// FormFields is a form as fields, for Redact. +func FormFields(v url.Values) map[string]any { + out := map[string]any{} + keys := make([]string, 0, len(v)) + for k := range v { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + if len(v[k]) == 1 { + out[k] = v[k][0] + } else { + out[k] = v[k] + } + } + return out +} + +// statusWriter keeps the status a handler answered with. +type statusWriter struct { + http.ResponseWriter + status int +} + +func (w *statusWriter) WriteHeader(code int) { + if w.status == 0 { + w.status = code + } + w.ResponseWriter.WriteHeader(code) +} + +func (w *statusWriter) Write(b []byte) (int, error) { + if w.status == 0 { + w.status = http.StatusOK + } + return w.ResponseWriter.Write(b) +} + +func (w *statusWriter) Flush() { + if f, ok := w.ResponseWriter.(http.Flusher); ok { + f.Flush() + } +} + +// API records every change to the admin API: a request that is not a GET or +// HEAD, under /api/. actor says who made it. +func API(sink Sink, actor func(*http.Request) string, log *slog.Logger, next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet || r.Method == http.MethodHead || !strings.HasPrefix(r.URL.Path, "/api/") { + next.ServeHTTP(w, r) + return + } + var body []byte + if r.Body != nil { + body, _ = io.ReadAll(io.LimitReader(r.Body, 64<<10)) + r.Body = io.NopCloser(bytes.NewReader(body)) + } + sw := &statusWriter{ResponseWriter: w} + next.ServeHTTP(sw, r) + e := store.AuditEvent{At: time.Now(), Actor: actor(r), Source: "api", + Action: r.Method + " " + r.URL.Path, Status: sw.status} + var fields map[string]any + if json.Unmarshal(body, &fields) == nil { + e.Detail = Redact(fields) + } + for _, k := range []string{"name", "target", "scope", "model"} { + if s, ok := fields[k].(string); ok && e.Target == "" { + e.Target = s + } + } + Write(r.Context(), sink, log, e) + }) +} + +// Write records an event, saying so in the log when the store will not take +// it: a change that happened and is not in the audit is worth a warning. +func Write(ctx context.Context, sink Sink, log *slog.Logger, e store.AuditEvent) { + if sink == nil { + return + } + if err := sink.WriteAudit(context.WithoutCancel(ctx), e); err != nil && log != nil { + log.Warn("a change could not be written to the audit log", "action", e.Action, "actor", e.Actor, "err", err) + } +} diff --git a/internal/audit/audit_test.go b/internal/audit/audit_test.go new file mode 100644 index 0000000..8286806 --- /dev/null +++ b/internal/audit/audit_test.go @@ -0,0 +1,60 @@ +package audit + +import ( + "context" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "gitea.mynodes.xyz/ilker/llmgw/internal/store" +) + +func TestNothingSecretIsRecorded(t *testing.T) { + got := Redact(FormFields(url.Values{ + "current": {"old-pw"}, "new": {"new-pw"}, "password": {"pw"}, "csrf": {"tok"}, + "name": {"ok-name"}, "note": {"sk-proj-abcdefgh"}, + })) + for _, secret := range []string{"old-pw", "new-pw", `"pw"`, "tok", "sk-proj"} { + if strings.Contains(got, secret) { + t.Errorf("%s recorded: %s", secret, got) + } + } + if !strings.Contains(got, "ok-name") { + t.Errorf("an ordinary field was dropped: %s", got) + } + nested := Redact(map[string]any{"external": map[string]any{"api_key": "x1", "url": "https://p"}}) + if strings.Contains(nested, "x1") || !strings.Contains(nested, "https://p") { + t.Errorf("nested: %s", nested) + } +} + +type memSink struct{ got []store.AuditEvent } + +func (m *memSink) WriteAudit(_ context.Context, e store.AuditEvent) error { + m.got = append(m.got, e) + return nil +} + +func TestEveryChangeToTheAdminAPIIsRecorded(t *testing.T) { + sink := &memSink{} + h := API(sink, func(*http.Request) string { return "ilker" }, nil, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusCreated) + })) + for _, req := range []*http.Request{ + httptest.NewRequest("GET", "/api/v1/models", nil), + httptest.NewRequest("POST", "/api/v1/models", strings.NewReader(`{"name":"m1","external":{"api_key":"sk-1"}}`)), + httptest.NewRequest("DELETE", "/api/v1/nodes/n1", nil), + } { + h.ServeHTTP(httptest.NewRecorder(), req) + } + if len(sink.got) != 2 { + t.Fatalf("recorded %d: %+v", len(sink.got), sink.got) + } + e := sink.got[0] + if e.Actor != "ilker" || e.Action != "POST /api/v1/models" || e.Target != "m1" || e.Status != 201 || + strings.Contains(e.Detail, "sk-1") || e.Source != "api" { + t.Errorf("%+v", e) + } +} diff --git a/internal/cli/audit.go b/internal/cli/audit.go new file mode 100644 index 0000000..7e2fbf9 --- /dev/null +++ b/internal/cli/audit.go @@ -0,0 +1,67 @@ +package cli + +import ( + "encoding/json" + "fmt" + "net/url" + "strconv" + + "github.com/spf13/cobra" + + "gitea.mynodes.xyz/ilker/llmgw/internal/store" +) + +func newAuditCmd(client func() *ctlClient, asJSON *bool) *cobra.Command { + var ( + since, actor, contains string + limit int + detail bool + ) + c := &cobra.Command{ + Use: "audit", + Short: "Who changed what: every change through the admin API and the administration pages", + Long: "Each change made with `llmgw ctl`, the console, or the pages under /admin —\n" + + "and a person's own keys and password — is recorded: who, when, through\n" + + "what, the fields it was given with anything secret taken out, and how it\n" + + "ended. The newest first.", + Example: " llmgw ctl audit --since 24h\n llmgw ctl audit --actor ilker --contains models\n llmgw ctl audit --contains bob@example.com --detail", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + q := url.Values{"since": {since}, "limit": {strconv.Itoa(limit)}} + if actor != "" { + q.Set("actor", actor) + } + if contains != "" { + q.Set("contains", contains) + } + var ev []store.AuditEvent + if err := client().get(cmd.Context(), "/api/v1/audit?"+q.Encode(), &ev); err != nil { + return err + } + if *asJSON { + return json.NewEncoder(cmd.OutOrStdout()).Encode(ev) + } + if len(ev) == 0 { + fmt.Fprintln(cmd.OutOrStdout(), "No changes recorded in that time.") + return nil + } + w := tab() + fmt.Fprintln(w, "WHEN (UTC)\tWHO\tVIA\tWHAT\tTARGET\tSTATUS") + for _, e := range ev { + fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%d\n", e.At.UTC().Format("2006-01-02 15:04:05"), + e.Actor, e.Source, e.Action, dash(e.Target), e.Status) + if detail && e.Detail != "" && e.Detail != "{}" { + fmt.Fprintf(w, "\t\t\t %s\t\t\n", e.Detail) + } + } + return w.Flush() + }, + } + f := c.Flags() + f.StringVar(&since, "since", "7d", "how far back: 90m, 24h, 30d") + f.StringVar(&actor, "actor", "", "only changes by this person") + f.StringVar(&contains, "contains", "", "only changes whose action or target contains this") + f.IntVar(&limit, "limit", 200, "at most this many") + f.BoolVar(&detail, "detail", false, "show the fields each change was given") + return c +} diff --git a/internal/cli/ctl.go b/internal/cli/ctl.go index 83fbee4..4986a3f 100644 --- a/internal/cli/ctl.go +++ b/internal/cli/ctl.go @@ -161,6 +161,7 @@ func newCtlCmd() *cobra.Command { newCapacityCmd(client, &asJSON), newQuotaCmd(client, &asJSON), newGuardCmd(client, &asJSON), + newAuditCmd(client, &asJSON), newUsageCmd(client, &asJSON), newCertCmd(), ) diff --git a/internal/cli/server.go b/internal/cli/server.go index 2c2421c..b524e23 100644 --- a/internal/cli/server.go +++ b/internal/cli/server.go @@ -31,6 +31,7 @@ import ( "golang.org/x/sync/errgroup" llmgwv1 "gitea.mynodes.xyz/ilker/llmgw/api/gen/llmgw/v1" + "gitea.mynodes.xyz/ilker/llmgw/internal/audit" "gitea.mynodes.xyz/ilker/llmgw/internal/config" "gitea.mynodes.xyz/ilker/llmgw/internal/control" "gitea.mynodes.xyz/ilker/llmgw/internal/control/adminapi" @@ -269,6 +270,7 @@ func newServerCmd() *cobra.Command { if st := rt.Store(); st != nil { // Removing a node revokes its certificate. api.SetRevoker(st.RevokeNode) + api.SetAudit(st) } // Catalog changes need somewhere to persist, so this is wired // only when there is a store. Without one, the endpoints say so @@ -404,7 +406,7 @@ func newServerCmd() *cobra.Command { return err } adminSrv := withLimits(&http.Server{ - Handler: router.AdminGuard(router.AdminAuth(adminToken, cfg.DebugPprof, adminMux)), + Handler: router.AdminGuard(router.AdminAuth(adminToken, cfg.DebugPprof, auditAPI(rt, adminMux, log))), ReadTimeout: time.Minute, }) @@ -660,6 +662,16 @@ func startControlPlane(ctx context.Context, cfg config.Server, cat *catalog.Live return ctl, svc, nil } +// auditAPI records every change made through the admin API, when there is a +// store to record it in. +func auditAPI(rt *router.Router, next http.Handler, log *slog.Logger) http.Handler { + st := rt.Store() + if st == nil { + return next + } + return audit.API(st, adminapi.Actor, log, next) +} + // metricsAddr renders the metrics listener for the startup line, saying so // plainly when there is none rather than printing an empty string. func metricsAddr(ln net.Listener) string { diff --git a/internal/control/adminapi/api.go b/internal/control/adminapi/api.go index ee28abe..f705223 100644 --- a/internal/control/adminapi/api.go +++ b/internal/control/adminapi/api.go @@ -48,6 +48,8 @@ type API struct { serve *modelserve.Server // history is the Prometheus the console's charts read from. history History + // audit reads the audit log. + audit AuditReader // guard is the guardrail policies' library and attachments. guard *GuardAdmin // revoke records a removed node, so its certificate is refused. @@ -80,6 +82,7 @@ func (a *API) Register(mux *http.ServeMux) { mux.HandleFunc("POST /api/v1/models/{name}/resources", a.setResources) a.registerQuotas(mux) a.registerGuard(mux) + mux.HandleFunc("GET /api/v1/audit", a.auditEvents) mux.HandleFunc("POST /api/v1/models/{name}/adapters", a.setAdapters) mux.HandleFunc("GET /api/v1/models/{name}/target", a.target) mux.HandleFunc("POST /api/v1/models/{name}/class", func(w http.ResponseWriter, r *http.Request) { @@ -1635,6 +1638,10 @@ func (a *API) planModel(w http.ResponseWriter, r *http.Request) { // the admin API answers only this machine and trusts whoever reaches it. Kept // to a short printable name, so a claim cannot pass for more than one — no // line breaks to forge a second log line, no length to bury the rest. +// Actor is who an admin API request says it comes from: the person who ran +// the command holding the admin token, as the CLI reports it. +func Actor(r *http.Request) string { return actorOf(r) } + func actorOf(r *http.Request) string { claimed := r.Header.Get("X-Llmgw-Actor") var b strings.Builder diff --git a/internal/control/adminapi/audit.go b/internal/control/adminapi/audit.go new file mode 100644 index 0000000..ddbff60 --- /dev/null +++ b/internal/control/adminapi/audit.go @@ -0,0 +1,77 @@ +package adminapi + +import ( + "context" + "net/http" + "strconv" + "time" + + "gitea.mynodes.xyz/ilker/llmgw/internal/store" +) + +// AuditReader lists recorded changes. +type AuditReader interface { + AuditEvents(ctx context.Context, f store.AuditFilter) ([]store.AuditEvent, error) +} + +// SetAudit enables GET /api/v1/audit. +func (a *API) SetAudit(r AuditReader) { a.audit = r } + +// audit answers GET /api/v1/audit?since=24h&actor=&contains=&limit=. +func (a *API) auditEvents(w http.ResponseWriter, r *http.Request) { + if a.audit == nil { + writeError(w, http.StatusNotImplemented, "this control plane keeps no audit log: it has no store") + return + } + q := r.URL.Query() + since := 7 * 24 * time.Hour + if s := q.Get("since"); s != "" { + d, err := parseSince(s) + if err != nil { + writeError(w, http.StatusBadRequest, err.Error()) + return + } + since = d + } + limit, _ := strconv.Atoi(q.Get("limit")) + ev, err := a.audit.AuditEvents(r.Context(), store.AuditFilter{ + Since: time.Now().Add(-since), Actor: q.Get("actor"), Contains: q.Get("contains"), Limit: limit, + }) + if err != nil { + writeError(w, http.StatusInternalServerError, err.Error()) + return + } + if ev == nil { + ev = []store.AuditEvent{} + } + writeJSON(w, http.StatusOK, ev) +} + +// parseSince reads 90m, 24h, 7d. +func parseSince(s string) (time.Duration, error) { + if n, ok := cutSuffix(s, "d"); ok { + days, err := strconv.Atoi(n) + if err != nil || days < 0 { + return 0, errBadSince(s) + } + return time.Duration(days) * 24 * time.Hour, nil + } + d, err := time.ParseDuration(s) + if err != nil || d < 0 { + return 0, errBadSince(s) + } + return d, nil +} + +type errBadSince string + +func (e errBadSince) Error() string { + return "since " + string(e) + " is not a duration like 90m, 24h or 7d" +} + +func cutSuffix(s, suf string) (string, bool) { + if len(s) > len(suf) && s[len(s)-len(suf):] == suf { + return s[:len(s)-len(suf)], true + } + return "", false +} diff --git a/internal/control/adminapi/guard_test.go b/internal/control/adminapi/guard_test.go index 5f7c18b..d441041 100644 --- a/internal/control/adminapi/guard_test.go +++ b/internal/control/adminapi/guard_test.go @@ -4,8 +4,10 @@ import ( "context" "slices" "testing" + "time" "gitea.mynodes.xyz/ilker/llmgw/internal/guard" + "gitea.mynodes.xyz/ilker/llmgw/internal/store" ) type memGuard struct{ att []guard.Attachment } @@ -57,3 +59,26 @@ func TestGuardPoliciesAreAttachedByTarget(t *testing.T) { t.Errorf("detach again: %d", code) } } + +type memAudit struct{ f store.AuditFilter } + +func (m *memAudit) AuditEvents(_ context.Context, f store.AuditFilter) ([]store.AuditEvent, error) { + m.f = f + return []store.AuditEvent{{Actor: "ilker", Action: "POST /api/v1/models"}}, nil +} + +func TestTheAuditLogIsReadWithFilters(t *testing.T) { + a, _ := setup(t, testModels()) + m := &memAudit{} + a.SetAudit(m) + var ev []store.AuditEvent + if code := call(t, a, "GET", "/api/v1/audit?since=2d&actor=ilker&contains=models&limit=5", "", &ev); code != 200 || len(ev) != 1 { + t.Fatalf("%d %v", code, ev) + } + if m.f.Actor != "ilker" || m.f.Contains != "models" || m.f.Limit != 5 || time.Since(m.f.Since) < 47*time.Hour { + t.Errorf("filter %+v", m.f) + } + if code := call(t, a, "GET", "/api/v1/audit?since=soon", "", nil); code != 400 { + t.Errorf("a bad since: %d", code) + } +} diff --git a/internal/router/handlers.go b/internal/router/handlers.go index 8cf302f..9fd938e 100644 --- a/internal/router/handlers.go +++ b/internal/router/handlers.go @@ -180,6 +180,9 @@ func (r *Router) Handler() http.Handler { pages.SetPersonalOff(r.personalOff) pages.SetLlamaCppVersion(r.llamaVersion) pages.SetPersonalDefaults(r.personalDefaults) + if r.store != nil { + pages.SetAudit(r.store) + } if pol, ok := r.guard.(interface { Policy(guard.Subject) (guard.Policy, string) }); ok { diff --git a/internal/store/audit.go b/internal/store/audit.go new file mode 100644 index 0000000..9cecdd4 --- /dev/null +++ b/internal/store/audit.go @@ -0,0 +1,72 @@ +package store + +import ( + "context" + "database/sql" + "strings" + "time" +) + +// AuditEvent is one change: who made it, through what, to what, and how it +// ended. Detail never holds a secret; the writer takes them out. +type AuditEvent struct { + ID int64 `json:"id"` + At time.Time `json:"at"` + Actor string `json:"actor"` + Source string `json:"source"` // api, web + Action string `json:"action"` // "POST /api/v1/models", "POST /admin/users/{id}/suspend" + Target string `json:"target,omitempty"` + Detail string `json:"detail,omitempty"` + Status int `json:"status"` +} + +// WriteAudit records one change. +func (s *DB) WriteAudit(ctx context.Context, e AuditEvent) error { + if e.At.IsZero() { + e.At = time.Now() + } + _, err := s.db.ExecContext(ctx, `INSERT INTO audit_events (ts, actor, source, action, target, detail, status) + VALUES (?, ?, ?, ?, ?, ?, ?)`, millis(e.At), e.Actor, e.Source, e.Action, e.Target, e.Detail, e.Status) + return err +} + +// AuditFilter narrows a listing; empty fields match everything. +type AuditFilter struct { + Since, Until time.Time + Actor string + // Contains matches the action or the target. + Contains string + Limit int +} + +// AuditEvents lists changes, the newest first. +func (s *DB) AuditEvents(ctx context.Context, f AuditFilter) ([]AuditEvent, error) { + q := `SELECT id, ts, actor, source, action, target, detail, status FROM audit_events WHERE ts >= ?` + args := []any{millis(f.Since)} + if !f.Until.IsZero() { + q += ` AND ts < ?` + args = append(args, millis(f.Until)) + } + if f.Actor != "" { + q += ` AND actor = ?` + args = append(args, f.Actor) + } + if f.Contains != "" { + q += ` AND (LOWER(action) LIKE ? OR LOWER(target) LIKE ?)` + like := "%" + strings.ToLower(f.Contains) + "%" + args = append(args, like, like) + } + limit := f.Limit + if limit <= 0 || limit > 10000 { + limit = 200 + } + q += ` ORDER BY ts DESC, id DESC LIMIT ?` + args = append(args, limit) + return queryAll(ctx, s.db, q, args, func(rows *sql.Rows) (AuditEvent, error) { + var e AuditEvent + var ts int64 + err := rows.Scan(&e.ID, &ts, &e.Actor, &e.Source, &e.Action, &e.Target, &e.Detail, &e.Status) + e.At = fromMillis(ts) + return e, err + }) +} diff --git a/internal/store/audit_test.go b/internal/store/audit_test.go new file mode 100644 index 0000000..a855307 --- /dev/null +++ b/internal/store/audit_test.go @@ -0,0 +1,34 @@ +package store + +import ( + "testing" + "time" +) + +func TestAuditEventsAreKeptAndFound(t *testing.T) { + db := capacityStore(t) + ctx := t.Context() + now := time.Now() + for i, e := range []AuditEvent{ + {At: now.Add(-2 * time.Hour), Actor: "ilker", Source: "api", Action: "POST /api/v1/models", Target: "m1", Status: 200}, + {At: now.Add(-time.Hour), Actor: "ann@example.com", Source: "web", Action: "POST /admin/users/u1/suspend", Target: "bob@example.com", Status: 200}, + {At: now, Actor: "ilker", Source: "api", Action: "DELETE /api/v1/nodes/n1", Status: 409}, + } { + if err := db.WriteAudit(ctx, e); err != nil { + t.Fatal(i, err) + } + } + all, err := db.AuditEvents(ctx, AuditFilter{Since: now.Add(-24 * time.Hour)}) + if err != nil || len(all) != 3 || all[0].Action != "DELETE /api/v1/nodes/n1" { + t.Fatalf("%+v %v", all, err) + } + if got, _ := db.AuditEvents(ctx, AuditFilter{Since: now.Add(-24 * time.Hour), Actor: "ilker"}); len(got) != 2 { + t.Errorf("by actor: %d", len(got)) + } + if got, _ := db.AuditEvents(ctx, AuditFilter{Since: now.Add(-24 * time.Hour), Contains: "BOB@"}); len(got) != 1 { + t.Errorf("by target: %d", len(got)) + } + if got, _ := db.AuditEvents(ctx, AuditFilter{Since: now.Add(-90 * time.Minute)}); len(got) != 2 { + t.Errorf("since: %d", len(got)) + } +} diff --git a/internal/store/copy.go b/internal/store/copy.go index af46262..353a549 100644 --- a/internal/store/copy.go +++ b/internal/store/copy.go @@ -55,6 +55,8 @@ var copyOrder = []string{ // Or a removed node's certificate would work again in the new store. "revoked_nodes", "guard_attachments", + // Who changed what is not lost by moving the store. + "audit_events", } // notCopied are tables deliberately left behind. @@ -227,7 +229,7 @@ func (s *DB) resetIdentities(ctx context.Context) error { return nil } // Only the tables with a generated id. SQLite's rowid needs nothing. - for _, t := range []string{"memory_notes", "usage"} { + for _, t := range []string{"memory_notes", "usage", "audit_events"} { var has bool if err := s.db.QueryRowContext(ctx, `SELECT EXISTS (SELECT 1 FROM information_schema.columns diff --git a/internal/store/migrations/postgres/0020_audit.sql b/internal/store/migrations/postgres/0020_audit.sql new file mode 100644 index 0000000..1dc3bed --- /dev/null +++ b/internal/store/migrations/postgres/0020_audit.sql @@ -0,0 +1,15 @@ +-- Ported from the SQLite migration of the same number. +-- Who changed what, from where: every change through the admin API and the +-- administration pages. Detail is the request's fields with anything secret +-- taken out; status is how it ended. +CREATE TABLE audit_events ( + id BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, + ts BIGINT NOT NULL, + actor TEXT NOT NULL, + source TEXT NOT NULL, + action TEXT NOT NULL, + target TEXT NOT NULL DEFAULT '', + detail TEXT NOT NULL DEFAULT '', + status INTEGER NOT NULL DEFAULT 0 +); +CREATE INDEX audit_events_ts ON audit_events (ts); diff --git a/internal/store/migrations/sqlite/0020_audit.sql b/internal/store/migrations/sqlite/0020_audit.sql new file mode 100644 index 0000000..bd7492b --- /dev/null +++ b/internal/store/migrations/sqlite/0020_audit.sql @@ -0,0 +1,14 @@ +-- Who changed what, from where: every change through the admin API and the +-- administration pages. Detail is the request's fields with anything secret +-- taken out; status is how it ended. +CREATE TABLE audit_events ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + ts INTEGER NOT NULL, + actor TEXT NOT NULL, + source TEXT NOT NULL, + action TEXT NOT NULL, + target TEXT NOT NULL DEFAULT '', + detail TEXT NOT NULL DEFAULT '', + status INTEGER NOT NULL DEFAULT 0 +); +CREATE INDEX audit_events_ts ON audit_events (ts); diff --git a/internal/web/account.go b/internal/web/account.go index 229a859..96eef85 100644 --- a/internal/web/account.go +++ b/internal/web/account.go @@ -14,6 +14,7 @@ import ( "time" "gitea.mynodes.xyz/ilker/llmgw/internal/account" + "gitea.mynodes.xyz/ilker/llmgw/internal/audit" "gitea.mynodes.xyz/ilker/llmgw/internal/clientip" "gitea.mynodes.xyz/ilker/llmgw/internal/guard" "gitea.mynodes.xyz/ilker/llmgw/internal/store" @@ -65,6 +66,8 @@ type AccountPages struct { hasMirror bool // hasQuotas is set when /admin/quotas is registered. hasQuotas bool + // audit records every change made through these pages; nil records none. + audit audit.Sink // hasGuard is set when /admin/guard is registered. hasGuard bool // guardFor names the guardrail policies in force for someone, and @@ -150,10 +153,10 @@ func (p *AccountPages) Register(mux *http.ServeMux) { mux.HandleFunc("POST /login", rationed(newRation(30, 30), p.login)) mux.HandleFunc("POST /logout", p.logout) mux.HandleFunc("GET /profile", p.profile) - mux.HandleFunc("POST /profile/keys", p.createKey) - mux.HandleFunc("POST /profile/keys/{id}/revoke", p.revokeKey) - mux.HandleFunc("POST /profile/password", p.changePassword) - mux.HandleFunc("POST /profile/connect", p.connect) + mux.HandleFunc("POST /profile/keys", p.selfAudited(p.createKey)) + mux.HandleFunc("POST /profile/keys/{id}/revoke", p.selfAudited(p.revokeKey)) + mux.HandleFunc("POST /profile/password", p.selfAudited(p.changePassword)) + mux.HandleFunc("POST /profile/connect", p.selfAudited(p.connect)) mux.HandleFunc("GET /setup/{tool}", p.setupScript) } diff --git a/internal/web/admin_tenancy_test.go b/internal/web/admin_tenancy_test.go index 42566c9..21abb9d 100644 --- a/internal/web/admin_tenancy_test.go +++ b/internal/web/admin_tenancy_test.go @@ -90,9 +90,22 @@ func (f *fakeAdmin) AllKeys(context.Context) ([]store.KeyRow, error) func (f *fakeAdmin) RevokeKey(context.Context, string, time.Time) error { return f.err } func tenancyPage(t *testing.T, s *fakeAdmin, admin bool) *http.ServeMux { + t.Helper() + return tenancyPageAs(t, s, admin, nil) +} + +func tenancyPageWith(t *testing.T, s *fakeAdmin, with func(*AccountPages)) *http.ServeMux { + t.Helper() + return tenancyPageAs(t, s, true, with) +} + +func tenancyPageAs(t *testing.T, s *fakeAdmin, admin bool, with func(*AccountPages)) *http.ServeMux { t.Helper() a := fakeAccounts{user: store.User{ID: "u1", Email: "a@example.com", IsAdmin: admin}} p := NewAccountPages(a, nil, nil, "v0", slog.New(slog.NewTextHandler(io.Discard, nil))) + if with != nil { + with(p) + } mux := http.NewServeMux() p.RegisterAdminUsers(mux, s) return mux @@ -199,3 +212,31 @@ func TestThisPageSetsNoQuotas(t *testing.T) { } } } + +type auditMem struct{ got []store.AuditEvent } + +func (a *auditMem) WriteAudit(_ context.Context, e store.AuditEvent) error { + a.got = append(a.got, e) + return nil +} + +func TestAChangeOnTheAdminPagesIsAudited(t *testing.T) { + s := &fakeAdmin{} + am := &auditMem{} + mux := tenancyPageWith(t, s, func(p *AccountPages) { p.SetAudit(am) }) + mirrorPost(t, mux, "/admin/tenancy/org", url.Values{"csrf": {"tok"}, "name": {"Scania"}}) + s.err = store.ErrDuplicateName + mirrorPost(t, mux, "/admin/tenancy/org", url.Values{"csrf": {"tok"}, "name": {"Scania"}}) + if len(am.got) != 2 { + t.Fatalf("recorded %d", len(am.got)) + } + ok, refused := am.got[0], am.got[1] + if ok.Source != "web" || ok.Action != "POST /admin/tenancy/org" || ok.Actor == "" || + !strings.Contains(ok.Detail, `"name":"Scania"`) || !strings.Contains(ok.Detail, "created") || + strings.Contains(ok.Detail, "tok") { + t.Errorf("%+v", ok) + } + if !strings.Contains(refused.Detail, "refused") { + t.Errorf("a refusal reads as one: %+v", refused) + } +} diff --git a/internal/web/admin_users.go b/internal/web/admin_users.go index fd82d3b..cba7456 100644 --- a/internal/web/admin_users.go +++ b/internal/web/admin_users.go @@ -420,11 +420,13 @@ func (p *AccountPages) tenancyAction(s AccountAdmin, err = plainError(err) p.log.Warn("a tenancy change was refused", "admin", admin.Email, "path", r.URL.Path, "err", err) + noteAudit(r, "", "", err.Error()) p.renderUsers(w, r, s, admin, "", err.Error(), r.FormValue("open")) return } p.log.Info("the tenancy tree was changed", "admin", admin.Email, "path", r.URL.Path, "what", msg) + noteAudit(r, "", msg, "") p.renderUsers(w, r, s, admin, msg, "", r.FormValue("open")) }) } @@ -445,7 +447,7 @@ func (p *AccountPages) adminOnly( http.NotFound(w, r) return } - next(w, p.withNav(r, user), user) + p.audited(w, p.withNav(r, user), user, func(w http.ResponseWriter, r *http.Request) { next(w, r, user) }) } } @@ -474,11 +476,13 @@ func (p *AccountPages) adminAction(s AccountAdmin, // action is as worth knowing about as one that went through. p.log.Warn("an administrative action was refused", "admin", admin.Email, "subject", subject, "err", err) + noteAudit(r, subject, "", err.Error()) p.renderUsers(w, r, s, admin, "", subject+": "+err.Error(), r.FormValue("open")) return } p.log.Info("an account was changed", "admin", admin.Email, "subject", subject, "what", msg) + noteAudit(r, subject, msg, "") p.renderUsers(w, r, s, admin, subject+" "+msg, "", r.FormValue("open")) }) } diff --git a/internal/web/audit.go b/internal/web/audit.go new file mode 100644 index 0000000..5aa4b31 --- /dev/null +++ b/internal/web/audit.go @@ -0,0 +1,85 @@ +package web + +import ( + "context" + "net/http" + "time" + + "gitea.mynodes.xyz/ilker/llmgw/internal/audit" + "gitea.mynodes.xyz/ilker/llmgw/internal/store" +) + +// SetAudit has every change made through these pages recorded. +func (p *AccountPages) SetAudit(s audit.Sink) { p.audit = s } + +// auditNote is what a handler says about the change it made: whom it was +// about, and how it ended in words — pages answer 200 to a refusal too. +type auditNote struct{ subject, result, refused string } + +type auditNoteKey struct{} + +// noteAudit lets the handler of a change say whom it concerned and how it +// ended. +func noteAudit(r *http.Request, subject, result, refused string) { + if n, ok := r.Context().Value(auditNoteKey{}).(*auditNote); ok { + n.subject, n.result, n.refused = subject, result, refused + } +} + +type statusRecorder struct { + http.ResponseWriter + status int +} + +func (w *statusRecorder) WriteHeader(code int) { + if w.status == 0 { + w.status = code + } + w.ResponseWriter.WriteHeader(code) +} + +func (w *statusRecorder) Write(b []byte) (int, error) { + if w.status == 0 { + w.status = http.StatusOK + } + return w.ResponseWriter.Write(b) +} + +// audited runs a change and records it: who, which page, the form with its +// secrets taken out, and how it ended. +func (p *AccountPages) audited(w http.ResponseWriter, r *http.Request, actor store.User, + run func(w http.ResponseWriter, r *http.Request), +) { + if p.audit == nil || r.Method == http.MethodGet || r.Method == http.MethodHead { + run(w, r) + return + } + note := &auditNote{} + r = r.WithContext(context.WithValue(r.Context(), auditNoteKey{}, note)) + sw := &statusRecorder{ResponseWriter: w} + run(sw, r) + fields := audit.FormFields(r.PostForm) + if note.result != "" { + fields["result"] = note.result + } + if note.refused != "" { + fields["refused"] = note.refused + } + audit.Write(r.Context(), p.audit, p.log, store.AuditEvent{ + At: time.Now(), Actor: actor.Email, Source: "web", Action: r.Method + " " + r.URL.Path, + Target: note.subject, Detail: audit.Redact(fields), Status: sw.status, + }) +} + +// selfAudited records a change a person makes to their own account: a key +// made or revoked, a password changed. +func (p *AccountPages) selfAudited(h http.HandlerFunc) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + _, user, ok := p.session(r) + if !ok { + h(w, r) + return + } + p.audited(w, r, user, h) + } +} From b60cf45adffa99ca56d2167629453d19a2898b46 Mon Sep 17 00:00:00 2001 From: Ilker Manap Date: Sun, 4 Oct 2026 16:30:56 +0200 Subject: [PATCH 02/17] Staff roles (viewer, operator, billing) and the console's Audit view Co-Authored-By: Claude Opus 5.5 (1M context) --- BACKLOG.md | 8 ++- CHANGELOG.md | 14 +++++ docs/console.md | 14 +++++ internal/cli/tui_actions_test.go | 14 +++++ internal/cli/tui_audit.go | 47 +++++++++++++++ internal/cli/tui_console.go | 13 ++++- internal/cli/tui_people.go | 38 +++++++++---- internal/cli/users.go | 39 ++++++++++++- internal/store/audit_test.go | 21 +++++++ .../migrations/postgres/0021_staff_role.sql | 5 ++ .../migrations/sqlite/0021_staff_role.sql | 4 ++ internal/store/sessions.go | 4 +- internal/store/users.go | 57 +++++++++++++++++-- internal/web/admin_page.go | 2 +- internal/web/admin_users.go | 2 +- internal/web/nav.go | 7 ++- internal/web/roles.go | 33 +++++++++++ internal/web/roles_test.go | 48 ++++++++++++++++ internal/web/templates/nav.html | 6 +- 19 files changed, 352 insertions(+), 24 deletions(-) create mode 100644 internal/cli/tui_audit.go create mode 100644 internal/store/migrations/postgres/0021_staff_role.sql create mode 100644 internal/store/migrations/sqlite/0021_staff_role.sql create mode 100644 internal/web/roles.go create mode 100644 internal/web/roles_test.go diff --git a/BACKLOG.md b/BACKLOG.md index 0562b49..9a4deb2 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -372,7 +372,13 @@ portal is not done — the page shows; the console and ctl change. ## C. Features, ranked by value × feasibility -1. **Audit log and roles (M).** Today "audit" is slog lines +1. **Done (branch `features-c`):** `audit_events`, written by the admin + API middleware and the /admin pages (with each change's result), `ctl + audit`, the console's Audit view; staff roles viewer / operator / + billing / admin. Left: before/after values (the request's fields are + recorded, not the state they replaced); a tenant-admin role that changes + its own organization (managers still only view it); hiding the forms a + viewer cannot submit. **Audit log and roles (M).** Today "audit" is slog lines (`internal/web/admin_users.go:403-480`) and roles are the `IsAdmin` flag plus an org "manager" who can view. Add `audit_events` (who, what, before/after, source) written by the `adminAction`/`tenancyAction` diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ca8b63..f7df86d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,20 @@ APIs change between minors without a deprecation period. ## Unreleased +### Administration + +- **An audit log.** Every change made through the admin API (`llmgw ctl`, + the console) or the pages under /admin is recorded: who, when, through + what, the fields it was given with passwords, keys and tokens taken out, + and how it ended — refusals included, in words. So are a person's own new + keys, revoked keys and password changes. Read it with `llmgw ctl audit` + (`--since`, `--actor`, `--contains`, `--detail`), `GET /api/v1/audit`, or + the console's **Audit** view. It moves with the store. +- **Staff roles** besides administrator: `viewer` looks at every + administration page and changes nothing, `operator` also runs the cluster + from them, `billing` sees usage and quotas. Set with `llmgw users role` or + the console's **Staff role…**. + ### Guardrails - **Hooks for guardrails on the request path** (nothing judges yet; B2 adds diff --git a/docs/console.md b/docs/console.md index 92e86e2..ef24734 100644 --- a/docs/console.md +++ b/docs/console.md @@ -160,6 +160,20 @@ project, their limits, a new password (their sessions close), delete. issues one; the key is shown once, in the result, and never again. On a row: its limits, or revoke it. +**Accounts** rows also have **Staff role…**: +- `viewer` looks at every page under /admin and changes nothing; +- `operator` also runs the cluster from them (models through the mirror, a + node's engines); +- `billing` sees usage and quotas; +- `admin` may do anything. + +The STAFF column shows each person's role. + +**Audit** lists every change of the last week — through `ctl`, this console +or the /admin pages, and people's own keys and passwords — with who made +it, through what, and how it ended. Enter shows the fields it was given, +with anything secret taken out. `llmgw ctl audit` is the same, with filters. + **Organizations** — the tenancy tree: organizations, their teams, the teams' projects, with members, keys and limits. **A** adds an organization. On a row: add a team or project inside it, rename, move, limits for the level and diff --git a/internal/cli/tui_actions_test.go b/internal/cli/tui_actions_test.go index 485974f..99e92fa 100644 --- a/internal/cli/tui_actions_test.go +++ b/internal/cli/tui_actions_test.go @@ -649,3 +649,17 @@ func TestGuardrailsSayWhereTheyApplyFrom(t *testing.T) { t.Error("no Guardrails… in the menu") } } + +func TestTheAuditViewListsChangesAndOpensOne(t *testing.T) { + m, _ := consoleWith(t, cAudit) + m.width, m.height = 160, 40 + m.data.audit = []store.AuditEvent{{At: time.Now(), Actor: "ilker", Source: "api", + Action: "POST /api/v1/models", Target: "qwen", Status: 200, Detail: `{"name":"qwen"}`}} + if v := m.View(); !strings.Contains(v, "POST /api/v1/models") || !strings.Contains(v, "ilker") { + t.Fatalf("not listed:\n%s", v) + } + tm := press(m, "enter") + if v := tm.View(); !strings.Contains(v, `"name":"qwen"`) { + t.Errorf("not opened:\n%s", v) + } +} diff --git a/internal/cli/tui_audit.go b/internal/cli/tui_audit.go new file mode 100644 index 0000000..e505d3a --- /dev/null +++ b/internal/cli/tui_audit.go @@ -0,0 +1,47 @@ +package cli + +import ( + "fmt" + "strconv" + "strings" + "time" + + "gitea.mynodes.xyz/ilker/llmgw/internal/store" +) + +// The audit view: who changed what, newest first — the same as `llmgw ctl +// audit`. + +func (m consoleModel) auditPanel(pw int) (string, string, string) { + keys := "→ or Enter open · ↑↓ choose · Q leave" + if m.inPanel { + keys = "↑↓ row · Enter what it was given · ← back · Q leave" + } + head := "\n " + sDim.Render("Every change through ctl, this console and the /admin pages, the last week.") + "\n\n" + var rows [][]string + for _, e := range m.data.audit { + status := strconv.Itoa(e.Status) + if e.Status >= 400 { + status = sBad.Render(status) + } + rows = append(rows, []string{ageShort(e.At), e.Actor, e.Source, e.Action, dash(e.Target), status}) + } + if len(rows) == 0 { + return "Audit", head + " " + sDim.Render("Nothing changed in the last week, or this control plane has no store."), keys + } + return "Audit", head + table([]string{"WHEN", "WHO", "VIA", "WHAT", "TARGET", "STATUS"}, rows, m.row, pw, m.inPanel), keys +} + +// auditDialog is one change in full. +func auditDialog(e store.AuditEvent) (string, string) { + var b strings.Builder + fmt.Fprintf(&b, "%s, by %s through %s\nstatus %d\n", e.At.Format(time.DateTime), e.Actor, e.Source, e.Status) + if e.Target != "" { + fmt.Fprintf(&b, "target %s\n", e.Target) + } + if e.Detail != "" && e.Detail != "{}" { + fmt.Fprintf(&b, "\n%s\n", wrap(e.Detail, 80, "")) + } + b.WriteString("\n" + sDim.Render("any key")) + return e.Action, b.String() +} diff --git a/internal/cli/tui_console.go b/internal/cli/tui_console.go index 6d2fbae..2c1097c 100644 --- a/internal/cli/tui_console.go +++ b/internal/cli/tui_console.go @@ -24,6 +24,7 @@ import ( "gitea.mynodes.xyz/ilker/llmgw/internal/docgen" "gitea.mynodes.xyz/ilker/llmgw/internal/hf" "gitea.mynodes.xyz/ilker/llmgw/internal/settings" + "gitea.mynodes.xyz/ilker/llmgw/internal/store" ) // The console: what a bare `sudo llmgw` opens on a control plane. Down the @@ -46,6 +47,7 @@ const ( cAccounts cKeys cOrgs + cAudit cManual cSettings cAddNode @@ -58,7 +60,7 @@ const ( cCount ) -var consoleLabels = [cCount]string{"Overview", "Monitor", "Nodes", "GPUs", "Models", "Replicas", "Usage", "Quotas", "Mirror", "Jobs", "Logs", "Accounts", "API keys", "Organizations", "Manual", +var consoleLabels = [cCount]string{"Overview", "Monitor", "Nodes", "GPUs", "Models", "Replicas", "Usage", "Quotas", "Mirror", "Jobs", "Logs", "Accounts", "API keys", "Organizations", "Audit", "Manual", "server.yaml", "Add a node", "Replicate the store", "Serve a model", "Upgrade the cluster", "Restart all replicas", "Back up now", "Leave"} @@ -75,6 +77,7 @@ type consoleData struct { models []adminapi.ModelView replicas []adminapi.ReplicaView jobs []modelserve.Job + audit []store.AuditEvent err error } @@ -228,6 +231,7 @@ func (m consoleModel) fetch() tea.Cmd { _ = c.get(ctx, "/api/v1/models", &d.models) _ = c.get(ctx, "/api/v1/replicas", &d.replicas) _ = c.get(ctx, "/api/v1/jobs", &d.jobs) + _ = c.get(ctx, "/api/v1/audit?since=7d&limit=300", &d.audit) return consoleDataMsg(d) } } @@ -474,6 +478,9 @@ func (m consoleModel) key(k tea.KeyMsg) (tea.Model, tea.Cmd) { case cJobs: m.dialog = "error" m.dialogTitle, m.dialogText = jobDialog(m.data.jobs[m.row]) + case cAudit: + m.dialog = "error" + m.dialogTitle, m.dialogText = auditDialog(m.data.audit[m.row]) case cServe: return m, m.fetchRepo(m.hf.models[m.row].ID) case cAccounts: @@ -873,6 +880,8 @@ func (m consoleModel) rowCount() int { return len(m.mirrorGroups()) case cJobs: return len(m.data.jobs) + case cAudit: + return len(m.data.audit) case cServe: return len(m.hf.models) case cAccounts: @@ -1108,6 +1117,8 @@ func (m consoleModel) panel() (title, body, keys string) { return m.mirrorPanel(pw) case cJobs: return m.jobsPanel(pw) + case cAudit: + return m.auditPanel(pw) case cMonitor: return m.monitorPanel(pw) case cServe: diff --git a/internal/cli/tui_people.go b/internal/cli/tui_people.go index 34c071d..4bd33f4 100644 --- a/internal/cli/tui_people.go +++ b/internal/cli/tui_people.go @@ -204,11 +204,10 @@ func (m consoleModel) userMenu(u store.User) *menuDialog { return nil }}) } - if u.IsAdmin { - items = append(items, cli("Take away the cluster dashboard", "users", "demote", u.ID)) - } else { - items = append(items, cli("Give the cluster dashboard (/admin)", "users", "promote", u.ID)) - } + items = append(items, menuItem{"Staff role (" + or(u.StaffRole(), "none") + ")…", func(m *consoleModel) tea.Cmd { + m.menuD = staffRoleMenu(u) + return nil + }}) items = append(items, menuItem{"Add to an organization, team or project…", func(m *consoleModel) tea.Cmd { m.openForm(&formDialog{title: "Membership for " + who, @@ -264,6 +263,28 @@ func (m consoleModel) userMenu(u store.User) *menuDialog { return &menuDialog{title: who, items: items} } +// staffRoleMenu sets what a person may do on the pages under /admin. +func staffRoleMenu(u store.User) *menuDialog { + about := map[string]string{ + store.RoleViewer: "looks at every administration page, changes nothing", + store.RoleOperator: "also runs the cluster: models, the mirror, nodes", + store.RoleBilling: "sees usage and quotas", + store.RoleAdmin: "may do anything", + "none": "no part in running the gateway", + } + var items []menuItem + for _, role := range append(slices.Clone(store.StaffRoles), "none") { + label := role + " — " + about[role] + if role == or(u.StaffRole(), "none") { + label = "✓ " + label + } + items = append(items, menuItem{label, func(m *consoleModel) tea.Cmd { + return m.runCLI("Staff role for "+u.Email, "users", "role", u.ID, role) + }}) + } + return &menuDialog{title: "Staff role · " + u.Email, items: items} +} + func (m *consoleModel) createUserForm() { m.openForm(&formDialog{title: "New account", note: "Created already approved. The password should be changed by its owner.", @@ -565,16 +586,13 @@ func (m consoleModel) peoplePanel(pw int) (string, string, string) { var rows [][]string switch m.menu { case cAccounts: - head = []string{"EMAIL", "STATE", "ADMIN", "MEMBER OF", "SIGNED UP", "LAST LOGIN"} + head = []string{"EMAIL", "STATE", "STAFF", "MEMBER OF", "SIGNED UP", "LAST LOGIN"} for _, u := range m.people.users { state := string(u.State) if u.State == store.UserPending { state = sWarn.Render(state) } - admin := "" - if u.IsAdmin { - admin = "yes" - } + admin := u.StaffRole() where := strings.Join(m.people.where[u.ID], "; ") if where == "" { where = sDim.Render("on their own") diff --git a/internal/cli/users.go b/internal/cli/users.go index 43fb597..17c9191 100644 --- a/internal/cli/users.go +++ b/internal/cli/users.go @@ -37,7 +37,8 @@ func newUsersCmd() *cobra.Command { newUserCreateCmd(open), newUserResetPasswordCmd(open), newUserJoinCmd(open), newUserLeaveCmd(open), newUserDeleteCmd(open), newUserAdminCmd(open, "promote", true, "Let an account see the cluster dashboard at /admin"), - newUserAdminCmd(open, "demote", false, "Take away access to the cluster dashboard")) + newUserAdminCmd(open, "demote", false, "Take away access to the cluster dashboard"), + newUserRoleCmd(open)) return cmd } @@ -177,6 +178,42 @@ func newUserStateCmd(open userOpener, verb string, state store.UserState, short } } +func newUserRoleCmd(open userOpener) *cobra.Command { + return &cobra.Command{ + Use: "role ", + Short: "Give an account a part in running the gateway, from the pages under /admin", + Long: "viewer looks at every administration page and changes nothing. operator\n" + + "also runs the cluster from them: models through the mirror, a node's\n" + + "engines. billing sees usage and quotas. admin may do anything. none takes\n" + + "the role away. Every change made from those pages is in `llmgw ctl audit`.", + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + role := args[1] + if role == "none" { + role = "" + } + s, err := open(cmd) + if err != nil { + return err + } + defer s.Close() + u, err := resolveUser(cmd.Context(), s, args[0]) + if err != nil { + return err + } + if err := s.SetStaffRole(cmd.Context(), u.ID, role); err != nil { + return err + } + if role == "" { + fmt.Fprintf(cmd.OutOrStdout(), "%s has no part in running the gateway now\n", u.Email) + } else { + fmt.Fprintf(cmd.OutOrStdout(), "%s is now %s\n", u.Email, role) + } + return nil + }, + } +} + func newUserAdminCmd(open userOpener, verb string, admin bool, short string) *cobra.Command { return &cobra.Command{ Use: verb + " ", diff --git a/internal/store/audit_test.go b/internal/store/audit_test.go index a855307..3a61341 100644 --- a/internal/store/audit_test.go +++ b/internal/store/audit_test.go @@ -32,3 +32,24 @@ func TestAuditEventsAreKeptAndFound(t *testing.T) { t.Errorf("since: %d", len(got)) } } + +func TestAStaffRoleIsGivenAndTakenAway(t *testing.T) { + db := capacityStore(t) + ctx := t.Context() + if err := db.CreateUser(ctx, User{ID: "u1", Email: "v@example.com", Name: "v", PasswordHash: "x", + State: UserActive, CreatedAt: time.Now()}); err != nil { + t.Fatal(err) + } + for _, role := range []string{RoleViewer, RoleOperator, RoleBilling, RoleAdmin, ""} { + if err := db.SetStaffRole(ctx, "u1", role); err != nil { + t.Fatal(role, err) + } + u, err := db.UserByID(ctx, "u1") + if err != nil || u.StaffRole() != role || u.Staff() != (role != "") || u.IsAdmin != (role == RoleAdmin) { + t.Errorf("%q: %+v %v", role, u, err) + } + } + if db.SetStaffRole(ctx, "u1", "god") == nil { + t.Error("an unknown role was taken") + } +} diff --git a/internal/store/migrations/postgres/0021_staff_role.sql b/internal/store/migrations/postgres/0021_staff_role.sql new file mode 100644 index 0000000..7634ad2 --- /dev/null +++ b/internal/store/migrations/postgres/0021_staff_role.sql @@ -0,0 +1,5 @@ +-- Ported from the SQLite migration of the same number. +-- What a person may do on the administration pages besides administering: +-- viewer (look at everything), operator (and run the cluster: models, +-- mirror, nodes), billing (usage and quotas). Administrators keep is_admin. +ALTER TABLE users ADD COLUMN staff_role TEXT NOT NULL DEFAULT ''; diff --git a/internal/store/migrations/sqlite/0021_staff_role.sql b/internal/store/migrations/sqlite/0021_staff_role.sql new file mode 100644 index 0000000..edd1cd9 --- /dev/null +++ b/internal/store/migrations/sqlite/0021_staff_role.sql @@ -0,0 +1,4 @@ +-- What a person may do on the administration pages besides administering: +-- viewer (look at everything), operator (and run the cluster: models, +-- mirror, nodes), billing (usage and quotas). Administrators keep is_admin. +ALTER TABLE users ADD COLUMN staff_role TEXT NOT NULL DEFAULT ''; diff --git a/internal/store/sessions.go b/internal/store/sessions.go index 7b70868..9df041d 100644 --- a/internal/store/sessions.go +++ b/internal/store/sessions.go @@ -58,7 +58,7 @@ func (s *DB) SessionByToken(ctx context.Context, token string) (Session, User, e row := s.db.QueryRowContext(ctx, `SELECT s.user_id, s.csrf_hash, s.expires_at, s.last_seen, u.id, u.email, u.name, u.password_hash, u.state, COALESCE(u.project_id, ''), - u.created_at, u.approved_at, u.approved_by, u.last_login_at, u.is_admin + u.created_at, u.approved_at, u.approved_by, u.last_login_at, u.is_admin, u.staff_role FROM sessions s JOIN users u ON u.id = s.user_id WHERE s.token_hash = ?`, HashKey(token)) @@ -72,7 +72,7 @@ func (s *DB) SessionByToken(ctx context.Context, token string) (Session, User, e if err := row.Scan(&sess.UserID, &sess.CSRFHash, &expires, &lastSeen, &u.ID, &u.Email, &u.Name, &u.PasswordHash, &state, &u.ProjectID, - &created, &approved, &approvedBy, &lastLogin, &u.IsAdmin); err != nil { + &created, &approved, &approvedBy, &lastLogin, &u.IsAdmin, &u.Role); err != nil { if errors.Is(err, sql.ErrNoRows) { return Session{}, User{}, ErrNoSession } diff --git a/internal/store/users.go b/internal/store/users.go index 72bd5a1..ca5b90f 100644 --- a/internal/store/users.go +++ b/internal/store/users.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "errors" + "fmt" "strings" "time" @@ -42,9 +43,36 @@ type User struct { ApprovedAt time.Time ApprovedBy string LastLoginAt time.Time - // IsAdmin lets this account see the cluster dashboard. It is not a role - // system: one bit, and everything that changes the cluster stays on the CLI. + // IsAdmin lets this account administer: every page under /admin, and + // every change there. IsAdmin bool + // Role is a lesser part in running the gateway: RoleViewer looks at + // every administration page, RoleOperator also runs the cluster from them + // (models, the mirror, nodes), RoleBilling sees usage and quotas. Empty + // for everybody else. + Role string +} + +// Staff roles. +const ( + RoleViewer = "viewer" + RoleOperator = "operator" + RoleBilling = "billing" + RoleAdmin = "admin" +) + +// StaffRoles are the roles SetStaffRole takes; "" takes them all away. +var StaffRoles = []string{RoleViewer, RoleOperator, RoleBilling, RoleAdmin} + +// Staff reports whether a person has any part in running the gateway. +func (u User) Staff() bool { return u.IsAdmin || u.Role != "" } + +// StaffRole is the role a person holds, admin included. +func (u User) StaffRole() string { + if u.IsAdmin { + return RoleAdmin + } + return u.Role } // CanSignIn reports whether this account may open a session. @@ -58,7 +86,7 @@ func NormalizeEmail(email string) string { } const userColumns = `id, email, name, password_hash, state, COALESCE(project_id, ''), - created_at, approved_at, approved_by, last_login_at, is_admin` + created_at, approved_at, approved_by, last_login_at, is_admin, staff_role` // CreateUser records a new, unapproved account. func (s *DB) CreateUser(ctx context.Context, u User) error { @@ -192,6 +220,27 @@ func (s *DB) SetAdmin(ctx context.Context, id string, admin bool) error { return nil } +// SetStaffRole gives a person a part in running the gateway — viewer, +// operator, billing or admin — or, with "", none. +func (s *DB) SetStaffRole(ctx context.Context, id, role string) error { + admin, rest := 0, role + switch role { + case RoleAdmin: + admin, rest = 1, "" + case "", RoleViewer, RoleOperator, RoleBilling: + default: + return fmt.Errorf("role %q is not one of viewer, operator, billing, admin", role) + } + res, err := s.db.ExecContext(ctx, `UPDATE users SET is_admin = ?, staff_role = ? WHERE id = ?`, admin, rest, id) + if err != nil { + return err + } + if n, _ := res.RowsAffected(); n == 0 { + return ErrNotFound + } + return nil +} + // TouchLogin records a successful sign-in. func (s *DB) TouchLogin(ctx context.Context, id string, at time.Time) error { _, err := s.db.ExecContext(ctx, `UPDATE users SET last_login_at = ? WHERE id = ?`, @@ -252,7 +301,7 @@ func (s *DB) scanUser(sc scanner) (User, error) { var approvedBy sql.NullString if err := sc.Scan(&u.ID, &u.Email, &u.Name, &u.PasswordHash, &state, &u.ProjectID, - &created, &approved, &approvedBy, &lastLogin, &u.IsAdmin); err != nil { + &created, &approved, &approvedBy, &lastLogin, &u.IsAdmin, &u.Role); err != nil { if errors.Is(err, sql.ErrNoRows) { return User{}, ErrNotFound } diff --git a/internal/web/admin_page.go b/internal/web/admin_page.go index e3ade09..eb76dc4 100644 --- a/internal/web/admin_page.go +++ b/internal/web/admin_page.go @@ -20,7 +20,7 @@ func (p *AccountPages) AdminPage(dashboard http.HandlerFunc) http.HandlerFunc { http.Redirect(w, r, "/login", http.StatusSeeOther) return } - if !user.IsAdmin { + if !staffMay(user, r) { http.NotFound(w, r) return } diff --git a/internal/web/admin_users.go b/internal/web/admin_users.go index cba7456..f87673f 100644 --- a/internal/web/admin_users.go +++ b/internal/web/admin_users.go @@ -442,7 +442,7 @@ func (p *AccountPages) adminOnly( http.Redirect(w, r, "/login", http.StatusSeeOther) return } - if !user.IsAdmin { + if !staffMay(user, r) { // 404 rather than 403: a 403 confirms the page exists. http.NotFound(w, r) return diff --git a/internal/web/nav.go b/internal/web/nav.go index 92cb4ce..5bf7d08 100644 --- a/internal/web/nav.go +++ b/internal/web/nav.go @@ -17,8 +17,10 @@ type Nav struct { // Current is the page being shown, so its link is marked rather than // offered: "cluster", "people", "quotas", "usage", "mirror", "chat", "keys". Current string - // Admin shows the administration links. + // Admin shows the administration links, and Role which of them: a + // billing role sees usage and quotas only. Admin bool + Role string // Mirror and Quotas are whether those pages were mounted at all; usage is // mounted with quotas. Mirror, Quotas bool @@ -37,7 +39,8 @@ type navKey struct{} func (p *AccountPages) navFor(r *http.Request, user store.User, current string) Nav { n := Nav{ Current: current, - Admin: user.IsAdmin, + Admin: user.Staff(), + Role: user.StaffRole(), Mirror: p.hasMirror, Quotas: p.hasQuotas, Guard: p.hasGuard, diff --git a/internal/web/roles.go b/internal/web/roles.go new file mode 100644 index 0000000..aaf378d --- /dev/null +++ b/internal/web/roles.go @@ -0,0 +1,33 @@ +package web + +import ( + "net/http" + "strings" + + "gitea.mynodes.xyz/ilker/llmgw/internal/store" +) + +// staffMay reports whether a person may do this to an administration page. +// +// An administrator may do anything. A viewer may look at every page and +// change nothing. An operator may also run the cluster from them: models +// through the mirror, and a node's engines. Billing may look at usage and +// quotas. Everything that touches people, the tree, limits or keys stays +// with administrators. +func staffMay(u store.User, r *http.Request) bool { + if u.IsAdmin { + return true + } + read := r.Method == http.MethodGet || r.Method == http.MethodHead + path := r.URL.Path + switch u.Role { + case store.RoleViewer: + return read + case store.RoleOperator: + return read || strings.HasPrefix(path, "/admin/mirror/") || + strings.HasPrefix(path, "/admin/nodes/") && strings.HasSuffix(path, "/engines") + case store.RoleBilling: + return read && (path == "/admin/usage" || path == "/admin/quotas") + } + return false +} diff --git a/internal/web/roles_test.go b/internal/web/roles_test.go new file mode 100644 index 0000000..b66c5c7 --- /dev/null +++ b/internal/web/roles_test.go @@ -0,0 +1,48 @@ +package web + +import ( + "net/http/httptest" + "strings" + "testing" + + "gitea.mynodes.xyz/ilker/llmgw/internal/store" +) + +func TestEachStaffRoleMayDoWhatItSays(t *testing.T) { + cases := []struct { + role, method, path string + may bool + }{ + {store.RoleViewer, "GET", "/admin/users", true}, + {store.RoleViewer, "POST", "/admin/users/u1/suspend", false}, + {store.RoleViewer, "POST", "/admin/mirror/fetch", false}, + {store.RoleOperator, "GET", "/admin", true}, + {store.RoleOperator, "POST", "/admin/mirror/fetch", true}, + {store.RoleOperator, "POST", "/admin/nodes/n1/engines", true}, + {store.RoleOperator, "POST", "/admin/quotas", false}, + {store.RoleOperator, "POST", "/admin/tenancy/org", false}, + {store.RoleBilling, "GET", "/admin/usage", true}, + {store.RoleBilling, "GET", "/admin/quotas", true}, + {store.RoleBilling, "GET", "/admin/users", false}, + {store.RoleBilling, "POST", "/admin/quotas", false}, + {"", "GET", "/admin", false}, + } + for _, c := range cases { + u := store.User{Role: c.role} + if got := staffMay(u, httptest.NewRequest(c.method, c.path, nil)); got != c.may { + t.Errorf("%s %s %s: %v", c.role, c.method, c.path, got) + } + } + if !staffMay(store.User{IsAdmin: true}, httptest.NewRequest("POST", "/admin/users/u1/delete", nil)) { + t.Error("an administrator may do anything") + } +} + +func TestABillingRoleIsShownUsageAndQuotasOnly(t *testing.T) { + rec := httptest.NewRecorder() + render(rec, "admin_guard", guardPageData{Nav: Nav{Admin: true, Role: store.RoleBilling}}) + body := rec.Body.String() + if !strings.Contains(body, `href="/admin/usage"`) || strings.Contains(body, `href="/admin/users"`) { + t.Errorf("menu: %s", body) + } +} diff --git a/internal/web/templates/nav.html b/internal/web/templates/nav.html index 193d250..0b2b43f 100644 --- a/internal/web/templates/nav.html +++ b/internal/web/templates/nav.html @@ -6,7 +6,11 @@ Organization {{- end}} -{{- if .Admin}} +{{- if and .Admin (eq .Role "billing")}} + Usage + Quotas + +{{- else if .Admin}} Cluster {{- /* Limits and usage are tabs of the people page, not pages of their own. */}} People From 4cf40644c9e647d98f2a0557f0aed93386dd73a7 Mon Sep 17 00:00:00 2001 From: Ilker Manap Date: Sun, 4 Oct 2026 16:36:15 +0200 Subject: [PATCH 03/17] Single sign-on through OpenID Connect: accounts, staff roles and memberships from the provider Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 7 + docs/install.md | 38 +++ docs/reference.md | 99 ++++++++ go.mod | 5 +- go.sum | 6 + internal/config/config.go | 6 + internal/config/engines_test.go | 18 ++ internal/config/oidc.go | 88 +++++++ internal/docgen/reference_gen.go | 14 ++ internal/docgen/render.go | 1 + internal/router/handlers.go | 5 + internal/router/router.go | 3 + internal/web/account.go | 29 ++- internal/web/oidc.go | 366 ++++++++++++++++++++++++++++ internal/web/oidc_test.go | 171 +++++++++++++ internal/web/templates/account.html | 1 + 16 files changed, 853 insertions(+), 4 deletions(-) create mode 100644 internal/config/oidc.go create mode 100644 internal/web/oidc.go create mode 100644 internal/web/oidc_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index f7df86d..8246872 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,13 @@ APIs change between minors without a deprecation period. administration page and changes nothing, `operator` also runs the cluster from them, `billing` sees usage and quotas. Set with `llmgw users role` or the console's **Staff role…**. +- **Single sign-on through OpenID Connect** (`auth.oidc`). A "Sign in + with …" button sends people to the identity provider and back, with PKCE, + state and nonce. The ID token is verified, and the account is found by + the verified email address, or made when `create_accounts` is on. Groups + give staff roles (`roles`, set at every sign-in) and places in the tree + (`memberships`). `no_passwords` leaves the provider as the only way in, + except for administrators. ### Guardrails diff --git a/docs/install.md b/docs/install.md index 64aa014..93190ec 100644 --- a/docs/install.md +++ b/docs/install.md @@ -1080,6 +1080,44 @@ thing to get right is the health check: point it at `/ready`, not `/health`. so a load balancer using it would send half the traffic to something that refuses every request. +## Single sign-on + +People can sign in through your identity provider — Keycloak, Entra ID, +Google, Okta, Authentik, anything that speaks OpenID Connect. + +1. Register a client there with the redirect address + `https:///login/oidc/callback`. +2. Put its secret in a file only root reads. +3. Add to `server.yaml`: + +```yaml +auth: + oidc: + issuer: https://login.example.com/realms/acme + client_id: llmgw + client_secret_file: /etc/llmgw/secrets/oidc-client-secret + label: Acme SSO # the button says "Sign in with Acme SSO" + create_accounts: true # make an active account for someone new + roles: # staff roles from groups, set at each sign-in + admin: [llmgw-admins] + operator: [llmgw-ops] + memberships: # groups to places in the tree, joined at sign-in + ml-engineers: "team:Acme/ml" + no_passwords: true # the provider only; administrators keep a password way in +``` + +How the sign-in works: + +- **Request:** it uses PKCE, a state and a nonce. +- **Identity:** the ID token is verified against the provider's keys, and + the account is found by the verified email address. +- **Unknown addresses:** without `create_accounts`, only people who already + have an account can sign in. +- **Roles:** a person taken out of a group loses the role the next time + they sign in. +- **Memberships:** they are added, never taken away. +- **Audit:** each sign-in is recorded in the audit log. + ## The firewall `server init`, `node add`, `server add` and `mirror init` each keep that diff --git a/docs/reference.md b/docs/reference.md index 337a519..4eb3ba3 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -33,6 +33,7 @@ rather than repeated. | `default_rpm` | `int` | [Auth](#auth) | | `default_tpm` | `int` | [Auth](#auth) | | `signup_open` | `bool` | [Auth](#auth) | +| `oidc` | `OIDC` | [Auth](#auth) | | `personal` | `Personal` | [Auth](#auth) | | `pricing` | `map[string]Price` | [Auth](#auth) | | `default_price` | `Price` | [Auth](#auth) | @@ -106,6 +107,17 @@ rather than repeated. | `prometheus` | `string` | [Monitoring](#monitoring) | | `window` | `time.Duration` | [Monitoring](#monitoring) | | `timeout` | `time.Duration` | [Monitoring](#monitoring) | +| `issuer` | `string` | [OIDC](#oidc) | +| `client_id` | `string` | [OIDC](#oidc) | +| `client_secret_file` | `string` | [OIDC](#oidc) | +| `redirect_url` | `string` | [OIDC](#oidc) | +| `label` | `string` | [OIDC](#oidc) | +| `scopes` | `[]string` | [OIDC](#oidc) | +| `groups_claim` | `string` | [OIDC](#oidc) | +| `create_accounts` | `bool` | [OIDC](#oidc) | +| `roles` | `map[string][]string` | [OIDC](#oidc) | +| `memberships` | `map[string]string` | [OIDC](#oidc) | +| `no_passwords` | `bool` | [OIDC](#oidc) | | `nvidia_smi` | `string` | [Options](#options) | | `ib_root` | `string` | [Options](#options) | | `prefer_nvml` | `bool` | [Options](#options) | @@ -413,6 +425,14 @@ SignupOpen serves the signup form. Accounts it creates are pending until an operator approves them, so this is not a way in — but turning it off means accounts are made with `llmgw users create` instead. +### `oidc` + +`OIDC` + +OIDC is sign-in through an identity provider. + +Takes a [OIDC](#oidc) block. + ### `personal` `Personal` @@ -959,6 +979,85 @@ Window is how far back the charts look. Zero takes an hour. Timeout bounds one query. It runs while somebody waits for a page, so a monitoring system that is slow today is skipped rather than waited for. +## OIDC + +### `issuer` + +`string` + +Issuer is the provider's address; its /.well-known/openid-configuration +says the rest. + +### `client_id` + +`string` + +ClientID and the file holding its secret, as registered with the +provider. + +### `client_secret_file` + +`string` + +### `redirect_url` + +`string` + +RedirectURL is where the provider sends people back: +https:///login/oidc/callback. Empty works it out from the +request, which is right unless a proxy changes the address. + +### `label` + +`string` + +Label is what the sign-in button says: "Sign in with