feat(admin): what has been done on this server, across every project
The other half of the error log. That one is what broke; this is what was done, and nobody could ask it: `ListActivity` takes one project id, and an administrator is not necessarily a member of the project they need to look into. Paged with a keyset cursor rather than an offset. The log is written to constantly and read rarely, so a page fetched a minute after the one before it would be shifted by everything written in between — one row shown twice and the row behind it skipped, which is the single failure an audit log must not have. Both halves of the cursor matter: the log is written in whole seconds, so a timestamp alone cannot separate two rows written in the same one. Migration 0009 makes `activity.user_id` SET NULL for the same reason 0008 did it to `created_by`. A log whose rows vanish with the account they describe answers "what has happened here" with everything except the part somebody had a reason to remove. `project_id` keeps cascading on purpose: the trash is the undo, and past it the deletion was deliberate. The event names moved to `web/src/lib/events.js`, because two surfaces read the same log now and two copies of nineteen strings is two places for them to drift. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Kristian Wind
co-authored by
Claude Opus 5
Claude Opus 5
parent
ebdc8eacf2
commit
85f7b2488d
@@ -0,0 +1,111 @@
|
||||
package httpapi
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/kristianwind/verdande/internal/store"
|
||||
)
|
||||
|
||||
// What has happened on this server, across every project.
|
||||
//
|
||||
// The per-project history under /projects/{id}/activity is the same rows read by
|
||||
// the people who work in that project. This is the administrator's version of the
|
||||
// question, and it is a different one: an administrator is not necessarily a member
|
||||
// of the project they need to look into, and "which account did that" cannot be
|
||||
// asked one project at a time.
|
||||
//
|
||||
// It sits beside the error log for a reason. That one is the half about what broke;
|
||||
// this is the half about what was done, and an instance had only ever had the first.
|
||||
|
||||
type auditEntryJSON struct {
|
||||
ID string `json:"id"`
|
||||
At string `json:"at"`
|
||||
ProjectID string `json:"project_id"`
|
||||
ProjectName string `json:"project_name"`
|
||||
TaskID string `json:"task_id,omitempty"`
|
||||
// UserID and UserName are absent when the account has been deleted. The row
|
||||
// stays — an audit log that lost its rows when somebody left would be missing
|
||||
// exactly the part worth keeping.
|
||||
UserID string `json:"user_id,omitempty"`
|
||||
UserName string `json:"user_name,omitempty"`
|
||||
Event string `json:"event"`
|
||||
Payload map[string]any `json:"payload,omitempty"`
|
||||
}
|
||||
|
||||
func (s *Server) handleAuditLog(w http.ResponseWriter, r *http.Request) {
|
||||
q := r.URL.Query()
|
||||
f := store.AuditFilter{
|
||||
UserID: q.Get("user_id"),
|
||||
ProjectID: q.Get("project_id"),
|
||||
Event: q.Get("event"),
|
||||
Limit: parseLimit(q.Get("limit"), 100, 500),
|
||||
}
|
||||
if cursor := q.Get("before"); cursor != "" {
|
||||
at, id, ok := parseAuditCursor(cursor)
|
||||
if !ok {
|
||||
writeError(w, http.StatusBadRequest, CodeBadRequest,
|
||||
"before must be <unix>:<id>, as next_cursor returns it")
|
||||
return
|
||||
}
|
||||
f.BeforeAt, f.BeforeID = at, id
|
||||
}
|
||||
|
||||
entries, err := s.db.ListAuditLog(r.Context(), f)
|
||||
if err != nil {
|
||||
s.internal(w, r, "list audit log", err)
|
||||
return
|
||||
}
|
||||
|
||||
out := make([]auditEntryJSON, 0, len(entries))
|
||||
for _, a := range entries {
|
||||
out = append(out, auditEntryJSON{
|
||||
ID: a.ID, At: a.CreatedAt.Format(time.RFC3339),
|
||||
ProjectID: a.ProjectID, ProjectName: a.ProjectName, TaskID: a.TaskID,
|
||||
UserID: a.UserID, UserName: a.UserName, Event: a.Event, Payload: a.Payload,
|
||||
})
|
||||
}
|
||||
|
||||
body := map[string]any{"activity": out}
|
||||
// A cursor only when the page was full. A short page is the end of the log, and
|
||||
// a cursor there invites one more request that can only come back empty.
|
||||
if len(entries) == f.Limit && f.Limit > 0 {
|
||||
last := entries[len(entries)-1]
|
||||
body["next_cursor"] = strconv.FormatInt(last.CreatedAt.Unix(), 10) + ":" + last.ID
|
||||
}
|
||||
writeJSON(w, http.StatusOK, body)
|
||||
}
|
||||
|
||||
// handleAuditEvents is what the event filter is drawn from: the names that occur,
|
||||
// with counts. Offered rather than typed — "member.role_changed" is an internal
|
||||
// vocabulary, and a text field for it is a field that returns nothing until you
|
||||
// have read the source.
|
||||
func (s *Server) handleAuditEvents(w http.ResponseWriter, r *http.Request) {
|
||||
events, err := s.db.AuditEvents(r.Context())
|
||||
if err != nil {
|
||||
s.internal(w, r, "list audit events", err)
|
||||
return
|
||||
}
|
||||
out := make([]map[string]any, 0, len(events))
|
||||
for _, e := range events {
|
||||
out = append(out, map[string]any{"event": e.Event, "count": e.Count})
|
||||
}
|
||||
writeJSON(w, http.StatusOK, map[string]any{"events": out})
|
||||
}
|
||||
|
||||
// parseAuditCursor reads "<unix>:<id>" — the pair the keyset walk needs. Both
|
||||
// halves are required: a cursor carrying only a timestamp cannot separate two rows
|
||||
// written in the same second, which is the case it exists for.
|
||||
func parseAuditCursor(s string) (int64, string, bool) {
|
||||
at, id, found := strings.Cut(s, ":")
|
||||
if !found || id == "" {
|
||||
return 0, "", false
|
||||
}
|
||||
unix, err := strconv.ParseInt(at, 10, 64)
|
||||
if err != nil || unix <= 0 {
|
||||
return 0, "", false
|
||||
}
|
||||
return unix, id, true
|
||||
}
|
||||
@@ -0,0 +1,264 @@
|
||||
package httpapi
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// The point of the instance-wide log is that it crosses projects the caller is not
|
||||
// a member of. The per-project endpoint cannot answer this question, and an
|
||||
// administrator asking "what has happened on this server" was previously told to
|
||||
// go and look in each project they happen to belong to.
|
||||
func TestTheAuditLogCrossesProjectsTheAdminIsNotInAllOfThem(t *testing.T) {
|
||||
ts := newTestServer(t)
|
||||
ts.bootstrap(t)
|
||||
other := ts.newUser(t, "anden@example.dk", "Anden")
|
||||
|
||||
// A project the administrator is not a member of.
|
||||
_, theirs := other.do(t, "POST", "/api/v1/projects", map[string]any{"name": "Deres eget"})
|
||||
theirID := theirs["id"].(string)
|
||||
other.do(t, "POST", "/api/v1/tasks", map[string]any{
|
||||
"content": "noget i deres eget", "project_id": theirID,
|
||||
})
|
||||
|
||||
// The administrator cannot read that project's own history at all.
|
||||
if resp, _ := ts.do(t, "GET", "/api/v1/projects/"+theirID+"/activity", nil); resp.StatusCode != http.StatusNotFound {
|
||||
t.Fatalf("a project the admin is not in: status %d, want 404", resp.StatusCode)
|
||||
}
|
||||
|
||||
// The instance log has it.
|
||||
resp, body := ts.do(t, "GET", "/api/v1/activity", nil)
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
t.Fatalf("audit log: status %d", resp.StatusCode)
|
||||
}
|
||||
entries, _ := body["activity"].([]any)
|
||||
var found map[string]any
|
||||
for _, raw := range entries {
|
||||
e := raw.(map[string]any)
|
||||
if e["project_id"] == theirID && e["event"] == "task.created" {
|
||||
found = e
|
||||
}
|
||||
}
|
||||
if found == nil {
|
||||
t.Fatalf("nothing from a project the admin is not in: %v", entries)
|
||||
}
|
||||
if found["project_name"] != "Deres eget" {
|
||||
t.Errorf("project_name = %v — a row that does not name its project says nothing",
|
||||
found["project_name"])
|
||||
}
|
||||
if found["user_name"] != "Anden" {
|
||||
t.Errorf("user_name = %v", found["user_name"])
|
||||
}
|
||||
}
|
||||
|
||||
// Administrators only, and sessions only. An audit log readable with a bearer token
|
||||
// would let a stolen token read the record of the theft — and of everything else on
|
||||
// the instance.
|
||||
func TestTheAuditLogIsAdminsOnlyAndSessionsOnly(t *testing.T) {
|
||||
ts := newTestServer(t)
|
||||
ts.bootstrap(t)
|
||||
other := ts.newUser(t, "anden@example.dk", "Anden")
|
||||
|
||||
for _, path := range []string{"/api/v1/activity", "/api/v1/activity/events"} {
|
||||
if resp, _ := other.do(t, "GET", path, nil); resp.StatusCode != http.StatusForbidden {
|
||||
t.Errorf("%s as an ordinary user: status %d, want 403", path, resp.StatusCode)
|
||||
}
|
||||
}
|
||||
|
||||
admin, err := ts.db.UserByEmail(t.Context(), "kristian@example.dk")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
token := ts.apiToken(t, admin.ID)
|
||||
for _, path := range []string{"/api/v1/activity", "/api/v1/activity/events"} {
|
||||
req, _ := http.NewRequest("GET", ts.URL+path, nil)
|
||||
req.Header.Set("Authorization", "Bearer "+token)
|
||||
resp, err := (&http.Client{}).Do(req)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
resp.Body.Close()
|
||||
if resp.StatusCode != http.StatusForbidden {
|
||||
t.Errorf("%s with an admin's API token: status %d, want 403", path, resp.StatusCode)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The cursor is keyed on (created_at, id) because the log is written in whole
|
||||
// seconds. A page boundary that landed inside a busy second would, with a
|
||||
// timestamp-only cursor, either repeat those rows or skip them — and an audit log
|
||||
// that skips rows is worse than no audit log.
|
||||
func TestTheAuditLogPagesWithoutRepeatingOrSkipping(t *testing.T) {
|
||||
ts := newTestServer(t)
|
||||
ts.bootstrap(t)
|
||||
|
||||
_, project := ts.do(t, "POST", "/api/v1/projects", map[string]any{"name": "Meget"})
|
||||
projectID := project["id"].(string)
|
||||
// Written as fast as the loop goes, so they share timestamps.
|
||||
const written = 25
|
||||
for i := range written {
|
||||
ts.do(t, "POST", "/api/v1/tasks", map[string]any{
|
||||
"content": fmt.Sprintf("opgave %d", i), "project_id": projectID,
|
||||
})
|
||||
}
|
||||
|
||||
seen := map[string]bool{}
|
||||
cursor := ""
|
||||
for page := 0; page < 20; page++ {
|
||||
path := "/api/v1/activity?limit=4"
|
||||
if cursor != "" {
|
||||
path += "&before=" + cursor
|
||||
}
|
||||
resp, body := ts.do(t, "GET", path, nil)
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
t.Fatalf("page %d: status %d", page, resp.StatusCode)
|
||||
}
|
||||
entries, _ := body["activity"].([]any)
|
||||
for _, raw := range entries {
|
||||
id := raw.(map[string]any)["id"].(string)
|
||||
if seen[id] {
|
||||
t.Fatalf("row %s came back on two pages", id)
|
||||
}
|
||||
seen[id] = true
|
||||
}
|
||||
next, _ := body["next_cursor"].(string)
|
||||
if next == "" {
|
||||
break
|
||||
}
|
||||
if next == cursor {
|
||||
t.Fatalf("the cursor did not move: %s", next)
|
||||
}
|
||||
cursor = next
|
||||
}
|
||||
|
||||
// Everything that was written is in the walk exactly once: the task creations,
|
||||
// plus the project creation.
|
||||
if len(seen) != written+1 {
|
||||
t.Errorf("the walk saw %d rows, want %d — a page boundary lost some",
|
||||
len(seen), written+1)
|
||||
}
|
||||
}
|
||||
|
||||
// The filters, and the list the event filter is drawn from.
|
||||
func TestTheAuditLogFiltersAndListsItsEvents(t *testing.T) {
|
||||
ts := newTestServer(t)
|
||||
ts.bootstrap(t)
|
||||
other := ts.newUser(t, "anden@example.dk", "Anden")
|
||||
otherUser, err := ts.db.UserByEmail(t.Context(), "anden@example.dk")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
_, mine := ts.do(t, "POST", "/api/v1/projects", map[string]any{"name": "Mit"})
|
||||
mineID := mine["id"].(string)
|
||||
ts.do(t, "POST", "/api/v1/tasks", map[string]any{"content": "min", "project_id": mineID})
|
||||
|
||||
_, theirs := other.do(t, "POST", "/api/v1/projects", map[string]any{"name": "Deres"})
|
||||
theirID := theirs["id"].(string)
|
||||
other.do(t, "POST", "/api/v1/tasks", map[string]any{"content": "deres", "project_id": theirID})
|
||||
|
||||
_, byUser := ts.do(t, "GET", "/api/v1/activity?user_id="+otherUser.ID, nil)
|
||||
for _, raw := range byUser["activity"].([]any) {
|
||||
if got := raw.(map[string]any)["user_id"]; got != otherUser.ID {
|
||||
t.Errorf("user_id filter returned a row by %v", got)
|
||||
}
|
||||
}
|
||||
if len(byUser["activity"].([]any)) == 0 {
|
||||
t.Error("the user filter returned nothing")
|
||||
}
|
||||
|
||||
_, byProject := ts.do(t, "GET", "/api/v1/activity?project_id="+mineID, nil)
|
||||
for _, raw := range byProject["activity"].([]any) {
|
||||
if got := raw.(map[string]any)["project_id"]; got != mineID {
|
||||
t.Errorf("project_id filter returned a row from %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
_, byEvent := ts.do(t, "GET", "/api/v1/activity?event=project.created", nil)
|
||||
rows := byEvent["activity"].([]any)
|
||||
if len(rows) != 2 {
|
||||
t.Errorf("event filter found %d project.created rows, want 2", len(rows))
|
||||
}
|
||||
for _, raw := range rows {
|
||||
if got := raw.(map[string]any)["event"]; got != "project.created" {
|
||||
t.Errorf("event filter returned %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
// And the vocabulary the filter is built from.
|
||||
_, events := ts.do(t, "GET", "/api/v1/activity/events", nil)
|
||||
counts := map[string]float64{}
|
||||
for _, raw := range events["events"].([]any) {
|
||||
e := raw.(map[string]any)
|
||||
counts[e["event"].(string)] = e["count"].(float64)
|
||||
}
|
||||
if counts["project.created"] != 2 || counts["task.created"] != 2 {
|
||||
t.Errorf("event counts = %v", counts)
|
||||
}
|
||||
|
||||
// A malformed cursor is a 400, not a silent first page: paging that quietly
|
||||
// restarts is how an audit walk reads the same rows forever.
|
||||
if resp, _ := ts.do(t, "GET", "/api/v1/activity?before=igår", nil); resp.StatusCode != http.StatusBadRequest {
|
||||
t.Errorf("a malformed cursor: status %d, want 400", resp.StatusCode)
|
||||
}
|
||||
}
|
||||
|
||||
// A deleted account leaves its trail behind. This is the reason migration 0009
|
||||
// exists: an audit log that lost its rows when somebody left would be missing
|
||||
// exactly the part worth keeping.
|
||||
func TestTheAuditLogOutlivesTheAccountItDescribes(t *testing.T) {
|
||||
ts := newTestServer(t)
|
||||
ts.bootstrap(t)
|
||||
other := ts.newUser(t, "anden@example.dk", "Anden")
|
||||
otherUser, err := ts.db.UserByEmail(t.Context(), "anden@example.dk")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// In a project that outlives them, so the row is not carried off by the
|
||||
// project cascade instead.
|
||||
_, mine := ts.do(t, "POST", "/api/v1/projects", map[string]any{"name": "Fælles"})
|
||||
sharedID := mine["id"].(string)
|
||||
ts.do(t, "POST", "/api/v1/projects/"+sharedID+"/invites", map[string]any{
|
||||
"email": "anden@example.dk", "role": "editor",
|
||||
})
|
||||
other.do(t, "POST", "/api/v1/tasks", map[string]any{
|
||||
"content": "deres bidrag", "project_id": sharedID,
|
||||
})
|
||||
|
||||
_, before := ts.do(t, "GET", "/api/v1/activity?user_id="+otherUser.ID+"&event=task.created", nil)
|
||||
if len(before["activity"].([]any)) != 1 {
|
||||
t.Fatalf("expected one row before the delete: %v", before["activity"])
|
||||
}
|
||||
|
||||
if resp, _ := ts.do(t, "DELETE", "/api/v1/users/"+otherUser.ID, nil); resp.StatusCode != http.StatusNoContent {
|
||||
t.Fatalf("delete: status %d", resp.StatusCode)
|
||||
}
|
||||
|
||||
_, after := ts.do(t, "GET", "/api/v1/activity?project_id="+sharedID+"&event=task.created", nil)
|
||||
rows := after["activity"].([]any)
|
||||
if len(rows) != 1 {
|
||||
t.Fatalf("the row went with the account: %v", rows)
|
||||
}
|
||||
row := rows[0].(map[string]any)
|
||||
if _, ok := row["user_id"]; ok {
|
||||
t.Errorf("user_id is still there after the account went: %v", row["user_id"])
|
||||
}
|
||||
if _, ok := row["user_name"]; ok {
|
||||
t.Errorf("user_name is still there after the account went: %v", row["user_name"])
|
||||
}
|
||||
|
||||
// And the project's own history still shows it, rather than dropping the row
|
||||
// on an inner join to a user that is gone.
|
||||
_, project := ts.do(t, "GET", "/api/v1/projects/"+sharedID+"/activity", nil)
|
||||
var kept int
|
||||
for _, raw := range project["activity"].([]any) {
|
||||
if raw.(map[string]any)["event"] == "task.created" {
|
||||
kept++
|
||||
}
|
||||
}
|
||||
if kept != 1 {
|
||||
t.Errorf("the project history dropped the authorless row: %d", kept)
|
||||
}
|
||||
}
|
||||
@@ -144,6 +144,7 @@ func New(cfg *config.Config, db *store.DB, log *slog.Logger, web fs.FS) *Server
|
||||
r.Use(s.requireAuth)
|
||||
r.Get("/me", s.handleMe)
|
||||
r.Patch("/me", s.handleUpdateProfile)
|
||||
r.Put("/sidebar-sections", s.handleSetSidebarSections)
|
||||
r.Post("/logout", s.handleLogout)
|
||||
r.Post("/password/change", s.handleChangePassword)
|
||||
r.Post("/totp/setup", s.handleTOTPSetup)
|
||||
@@ -236,6 +237,17 @@ func New(cfg *config.Config, db *store.DB, log *slog.Logger, web fs.FS) *Server
|
||||
r.Delete("/", s.handleDisconnectGmail)
|
||||
r.Post("/authorize", s.handleGmailAuthorize)
|
||||
r.Post("/sync", s.handleGmailSyncNow)
|
||||
|
||||
// The OAuth client itself is the instance's registration with
|
||||
// Google, not anybody's mailbox — so unlike everything above it, it
|
||||
// is administrators only, and behind a session as well: a leaked
|
||||
// token must not be able to read or replace this instance's
|
||||
// identity to Google.
|
||||
r.Group(func(r chi.Router) {
|
||||
r.Use(s.requireSession, s.requireAdmin)
|
||||
r.Get("/client", s.handleGetGmailClient)
|
||||
r.Put("/client", s.handleSetGmailClient)
|
||||
})
|
||||
})
|
||||
|
||||
r.Route("/import", func(r chi.Router) {
|
||||
@@ -306,6 +318,17 @@ func New(cfg *config.Config, db *store.DB, log *slog.Logger, web fs.FS) *Server
|
||||
r.Group(func(r chi.Router) {
|
||||
r.Use(s.requireSession, s.requireAdmin)
|
||||
r.Get("/errors", s.handleListErrors)
|
||||
// A backup file is a complete copy of the database, so this whole
|
||||
// group is sessions-only as well as administrators-only: a leaked
|
||||
// token must not be able to download everybody's data.
|
||||
r.Get("/backups", s.handleListBackups)
|
||||
r.Post("/backups", s.handleRunBackup)
|
||||
r.Get("/backups/{backupID}", s.handleDownloadBackup)
|
||||
// The instance-wide history. Above the per-project one in every
|
||||
// sense: it crosses projects the caller is not a member of, which
|
||||
// is the whole reason it is here and not there.
|
||||
r.Get("/activity", s.handleAuditLog)
|
||||
r.Get("/activity/events", s.handleAuditEvents)
|
||||
r.Get("/users", s.handleListUsers)
|
||||
r.Post("/users", s.handleCreateUser)
|
||||
r.Patch("/users/{userID}", s.handleUpdateUser)
|
||||
|
||||
Reference in New Issue
Block a user