diff --git a/internal/db/migrations/024_admin_customer_notes.sql b/internal/db/migrations/024_admin_customer_notes.sql new file mode 100644 index 00000000..355f6c8b --- /dev/null +++ b/internal/db/migrations/024_admin_customer_notes.sql @@ -0,0 +1,23 @@ +-- Migration: 024_admin_customer_notes — free-text notes per team, written +-- by platform admins via POST /api/v1/admin/customers/:team_id/notes. Surfaces +-- on the admin Customer Detail drawer ("called this customer 2024-05-10, they +-- want pro tier with annual billing"). Hard-deleted on DELETE — notes are +-- reversible by re-typing, so a soft-delete column would add bookkeeping +-- without operator benefit. +-- +-- author_email is the admin's JWT email at write time (denormalized rather +-- than a FK to users) so deleting an admin's user row doesn't blow up audit +-- coherence. Same denorm pattern as audit_log.actor. +CREATE TABLE IF NOT EXISTS admin_customer_notes ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + team_id UUID NOT NULL REFERENCES teams(id) ON DELETE CASCADE, + body TEXT NOT NULL, + author_email TEXT NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +-- Composite index on (team_id, created_at DESC) so the per-team list query +-- ("show me all notes for this team, newest first") is a single index scan, +-- not a sort over a sequential read. +CREATE INDEX IF NOT EXISTS idx_admin_customer_notes_team + ON admin_customer_notes(team_id, created_at DESC); diff --git a/internal/handlers/admin_customer_notes.go b/internal/handlers/admin_customer_notes.go new file mode 100644 index 00000000..389479e0 --- /dev/null +++ b/internal/handlers/admin_customer_notes.go @@ -0,0 +1,205 @@ +package handlers + +// admin_customer_notes.go — three handlers backing the admin Customer +// Detail drawer's free-text notes: +// +// GET /api/v1/admin/customers/:team_id/notes → list notes for team +// POST /api/v1/admin/customers/:team_id/notes → create a note +// DELETE /api/v1/admin/notes/:note_id → hard-delete a note +// +// All three sit behind the same RequireAdmin gate as the rest of the +// admin/customers/* surface (see admin_customers.go for the gate +// rationale). DELETE is a hard delete because notes are reversible by +// re-typing — see migration 024 for the soft-delete trade-off. +// +// The list / create handlers receive :team_id in the URL (so they can be +// nested under /admin/customers/...). The delete handler takes only +// :note_id because notes are globally addressable by id; the admin must +// already have hit the list endpoint to know which id to delete, so the +// team_id is recoverable from the row itself if a future audit/log +// consumer needs it. + +import ( + "database/sql" + "errors" + "log/slog" + "strings" + "time" + + "github.com/gofiber/fiber/v2" + "github.com/google/uuid" + "instant.dev/internal/middleware" + "instant.dev/internal/models" +) + +// AdminCustomerNotesHandler serves the three /admin/.../notes endpoints. +// Slim wrapper around models.* — no Razorpay / no plans Registry needed, +// so the constructor is just the DB handle. +type AdminCustomerNotesHandler struct { + db *sql.DB +} + +// NewAdminCustomerNotesHandler constructs the handler. +func NewAdminCustomerNotesHandler(db *sql.DB) *AdminCustomerNotesHandler { + return &AdminCustomerNotesHandler{db: db} +} + +// ───────────────────────────────────────────────────────────────────────────── +// Wire shape +// ───────────────────────────────────────────────────────────────────────────── + +// adminNoteWire is the per-row response shape. team_id is surfaced on +// every wire row (list AND create) so the dashboard's "create + redirect +// to detail" UI flow has the team id in the body without re-reading the +// URL. RFC3339 created_at — clients parse it the same way they parse the +// rest of the API. +type adminNoteWire struct { + ID string `json:"id"` + TeamID string `json:"team_id"` + Body string `json:"body"` + AuthorEmail string `json:"author_email"` + CreatedAt string `json:"created_at"` +} + +// toAdminNoteWire converts a *models.AdminCustomerNote into the JSON +// shape. Centralised so future schema additions (an edited_at column, a +// redacted bool) flow through one helper. +func toAdminNoteWire(n *models.AdminCustomerNote) adminNoteWire { + return adminNoteWire{ + ID: n.ID.String(), + TeamID: n.TeamID.String(), + Body: n.Body, + AuthorEmail: n.AuthorEmail, + CreatedAt: n.CreatedAt.UTC().Format(time.RFC3339Nano), + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// GET /api/v1/admin/customers/:team_id/notes — list notes +// ───────────────────────────────────────────────────────────────────────────── + +// ListNotes handles GET /api/v1/admin/customers/:team_id/notes. +func (h *AdminCustomerNotesHandler) ListNotes(c *fiber.Ctx) error { + teamID, err := uuid.Parse(c.Params("team_id")) + if err != nil { + return respondError(c, fiber.StatusBadRequest, "invalid_team_id", "team_id must be a UUID") + } + + notes, err := models.ListAdminCustomerNotes(c.Context(), h.db, teamID, 0) + if err != nil { + slog.Error("admin.customers.notes.list_failed", "error", err, "team_id", teamID) + return respondError(c, fiber.StatusServiceUnavailable, "db_failed", "Failed to list notes") + } + + out := make([]adminNoteWire, 0, len(notes)) + for _, n := range notes { + out = append(out, toAdminNoteWire(n)) + } + return c.JSON(fiber.Map{ + "ok": true, + "notes": out, + }) +} + +// ───────────────────────────────────────────────────────────────────────────── +// POST /api/v1/admin/customers/:team_id/notes — create a note +// ───────────────────────────────────────────────────────────────────────────── + +// adminCreateNoteRequest is the JSON body for POST notes. +type adminCreateNoteRequest struct { + Body string `json:"body"` +} + +// CreateNote handles POST /api/v1/admin/customers/:team_id/notes. +// +// Body validation lives in models.CreateAdminCustomerNote (typed sentinels +// for empty / too-long) so the handler just maps sentinels → status codes. +// The author_email is sourced from the admin's JWT email (populated by +// RequireAuth on the locals) — never read from the request body. That +// boundary stops a malicious admin from impersonating another admin in +// the notes ledger. +// +// A team_not_found 404 is produced by checking up-front via +// models.GetTeamByID rather than relying on the FK violation surface — +// the explicit lookup gives a clean error_code AND keeps the DB layer's +// fmt.Errorf wrapping out of the response. +func (h *AdminCustomerNotesHandler) CreateNote(c *fiber.Ctx) error { + teamID, err := uuid.Parse(c.Params("team_id")) + if err != nil { + return respondError(c, fiber.StatusBadRequest, "invalid_team_id", "team_id must be a UUID") + } + + var req adminCreateNoteRequest + if err := c.BodyParser(&req); err != nil { + return respondError(c, fiber.StatusBadRequest, "invalid_body", "JSON body required") + } + body := strings.TrimSpace(req.Body) + // Pre-check empty so the typed-sentinel branch is the only path to a + // 400, not a fall-through to "db_failed" if the model rejected after + // a partial commit. + if body == "" { + return respondError(c, fiber.StatusBadRequest, "missing_body", "body is required") + } + + // Verify the team exists before the INSERT so we surface 404 with a + // clean error_code (not a generic 503 "db_failed" from the FK violation + // the model would otherwise hit). + if _, err := models.GetTeamByID(c.Context(), h.db, teamID); err != nil { + var nf *models.ErrTeamNotFound + if errors.As(err, &nf) { + return respondError(c, fiber.StatusNotFound, "team_not_found", "no such team") + } + slog.Error("admin.customers.notes.team_query_failed", "error", err, "team_id", teamID) + return respondError(c, fiber.StatusServiceUnavailable, "db_failed", "Failed to load team") + } + + adminEmail := middleware.GetEmail(c) + + note, err := models.CreateAdminCustomerNote(c.Context(), h.db, models.CreateAdminCustomerNoteParams{ + TeamID: teamID, + Body: body, + AuthorEmail: adminEmail, + }) + if err != nil { + switch { + case errors.Is(err, models.ErrAdminCustomerNoteEmpty): + return respondError(c, fiber.StatusBadRequest, "missing_body", "body is required") + case errors.Is(err, models.ErrAdminCustomerNoteTooLong): + return respondError(c, fiber.StatusBadRequest, "body_too_long", "body exceeds 8KB cap") + } + slog.Error("admin.customers.notes.create_failed", "error", err, "team_id", teamID) + return respondError(c, fiber.StatusServiceUnavailable, "db_failed", "Failed to create note") + } + + return c.Status(fiber.StatusCreated).JSON(fiber.Map{ + "ok": true, + "note": toAdminNoteWire(note), + }) +} + +// ───────────────────────────────────────────────────────────────────────────── +// DELETE /api/v1/admin/notes/:note_id — hard-delete a note +// ───────────────────────────────────────────────────────────────────────────── + +// DeleteNote handles DELETE /api/v1/admin/notes/:note_id. +// +// Hard delete (not soft) — notes are reversible by re-typing, so the +// always-filter / paranoid-read overhead a tombstone column requires +// buys nothing operationally. See migration 024's comment. +func (h *AdminCustomerNotesHandler) DeleteNote(c *fiber.Ctx) error { + noteID, err := uuid.Parse(c.Params("note_id")) + if err != nil { + return respondError(c, fiber.StatusBadRequest, "invalid_note_id", "note_id must be a UUID") + } + if err := models.DeleteAdminCustomerNote(c.Context(), h.db, noteID); err != nil { + if errors.Is(err, models.ErrAdminCustomerNoteNotFound) { + return respondError(c, fiber.StatusNotFound, "note_not_found", "no such note") + } + slog.Error("admin.customers.notes.delete_failed", "error", err, "note_id", noteID) + return respondError(c, fiber.StatusServiceUnavailable, "db_failed", "Failed to delete note") + } + return c.JSON(fiber.Map{ + "ok": true, + "note_id": noteID.String(), + }) +} diff --git a/internal/handlers/admin_customer_notes_test.go b/internal/handlers/admin_customer_notes_test.go new file mode 100644 index 00000000..a27f5111 --- /dev/null +++ b/internal/handlers/admin_customer_notes_test.go @@ -0,0 +1,293 @@ +package handlers_test + +// admin_customer_notes_test.go — integration coverage for the three +// /api/v1/admin/customers/:team_id/notes + /admin/notes/:note_id endpoints. +// Uses the same fake-auth shim as admin_customers_test.go so we can drive +// the real handler set behind RequireAdmin without minting JWTs. + +import ( + "bytes" + "context" + "database/sql" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/gofiber/fiber/v2" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "instant.dev/internal/handlers" + "instant.dev/internal/middleware" +) + +// adminNotesApp builds a Fiber app wired to NewAdminCustomerNotesHandler +// behind the same fake-auth shim adminApp() uses. Routes match what +// router.go installs: +// +// GET /api/v1/admin/customers/:team_id/notes +// POST /api/v1/admin/customers/:team_id/notes +// DELETE /api/v1/admin/notes/:note_id +func adminNotesApp(t *testing.T, db *sql.DB, callerEmail string) *fiber.App { + t.Helper() + app := fiber.New(fiber.Config{ + ErrorHandler: func(c *fiber.Ctx, err error) error { + if errors.Is(err, handlers.ErrResponseWritten) { + return nil + } + code := fiber.StatusInternalServerError + if e, ok := err.(*fiber.Error); ok { + code = e.Code + } + return c.Status(code).JSON(fiber.Map{"ok": false, "error": "internal_error", "message": err.Error()}) + }, + }) + + fakeAuth := func(c *fiber.Ctx) error { + if callerEmail != "" { + c.Locals(middleware.LocalKeyEmail, callerEmail) + } + c.Locals(middleware.LocalKeyUserID, uuid.NewString()) + c.Locals(middleware.LocalKeyTeamID, uuid.NewString()) + return c.Next() + } + + notesH := handlers.NewAdminCustomerNotesHandler(db) + adminGroup := app.Group("/api/v1/admin", fakeAuth, middleware.RequireAdmin()) + adminGroup.Get("/customers/:team_id/notes", notesH.ListNotes) + adminGroup.Post("/customers/:team_id/notes", notesH.CreateNote) + adminGroup.Delete("/notes/:note_id", notesH.DeleteNote) + return app +} + +// TestAdminNotes_CreateListDelete is the headline integration round-trip: +// create one note → list returns it → delete removes it → list is empty. +// Asserts on the wire shape (id/team_id/body/author_email/created_at) at +// each step so a regression in serialisation is caught here. +func TestAdminNotes_CreateListDelete(t *testing.T) { + db, cleanup := adminAppNeedsDB(t) + defer cleanup() + t.Setenv("ADMIN_EMAILS", adminCallerEmail) + app := adminNotesApp(t, db, adminCallerEmail) + + teamID, _ := adminSeedTeam(t, db, "hobby") + t.Cleanup(func() { + db.Exec(`DELETE FROM admin_customer_notes WHERE team_id = $1`, teamID) + }) + + // 1. Create. + body := "called this customer 2024-05-10, they want pro tier with annual billing" + status, resp := adminDoJSON(t, app, "POST", + "/api/v1/admin/customers/"+teamID.String()+"/notes", + map[string]any{"body": body}) + require.Equal(t, http.StatusCreated, status, "create must return 201: %v", resp) + note, _ := resp["note"].(map[string]any) + require.NotNil(t, note, "response must carry the created note") + noteID, _ := note["id"].(string) + require.NotEmpty(t, noteID, "note id must be non-empty") + assert.Equal(t, teamID.String(), note["team_id"]) + assert.Equal(t, body, note["body"]) + assert.Equal(t, adminCallerEmail, note["author_email"], + "author_email must be sourced from the admin's JWT, never the request body") + + // 2. List — must surface the created note. + status, resp = adminDoJSON(t, app, "GET", + "/api/v1/admin/customers/"+teamID.String()+"/notes", nil) + require.Equal(t, http.StatusOK, status) + notes, _ := resp["notes"].([]any) + require.Len(t, notes, 1) + row, _ := notes[0].(map[string]any) + assert.Equal(t, noteID, row["id"]) + assert.Equal(t, body, row["body"]) + + // 3. Delete. + status, resp = adminDoJSON(t, app, "DELETE", + "/api/v1/admin/notes/"+noteID, nil) + require.Equal(t, http.StatusOK, status, "delete must return 200: %v", resp) + assert.Equal(t, noteID, resp["note_id"]) + + // 4. List again — must be empty (hard delete, no tombstone). + status, resp = adminDoJSON(t, app, "GET", + "/api/v1/admin/customers/"+teamID.String()+"/notes", nil) + require.Equal(t, http.StatusOK, status) + notes, _ = resp["notes"].([]any) + assert.Empty(t, notes, "delete must be a hard delete — list returns no rows") +} + +// TestAdminNotes_ListReturnsNewestFirst — multiple notes on the same team +// must come back newest first. The DB index is (team_id, created_at DESC) +// so this is a single index scan; the assertion guards against a +// regression that drops the ORDER BY or reverses the direction. +func TestAdminNotes_ListReturnsNewestFirst(t *testing.T) { + db, cleanup := adminAppNeedsDB(t) + defer cleanup() + t.Setenv("ADMIN_EMAILS", adminCallerEmail) + app := adminNotesApp(t, db, adminCallerEmail) + + teamID, _ := adminSeedTeam(t, db, "hobby") + t.Cleanup(func() { + db.Exec(`DELETE FROM admin_customer_notes WHERE team_id = $1`, teamID) + }) + + // Three notes in order. We can't rely on created_at being distinct + // in fast succession on every platform, so we INSERT directly with + // explicit created_at values one second apart. + type seed struct{ body, ts string } + seeds := []seed{ + {"oldest", "2024-05-08T10:00:00Z"}, + {"middle", "2024-05-09T10:00:00Z"}, + {"newest", "2024-05-10T10:00:00Z"}, + } + for _, s := range seeds { + ts, _ := time.Parse(time.RFC3339, s.ts) + _, err := db.ExecContext(context.Background(), ` + INSERT INTO admin_customer_notes (team_id, body, author_email, created_at) + VALUES ($1, $2, $3, $4) + `, teamID, s.body, adminCallerEmail, ts) + require.NoError(t, err) + } + + status, resp := adminDoJSON(t, app, "GET", + "/api/v1/admin/customers/"+teamID.String()+"/notes", nil) + require.Equal(t, http.StatusOK, status) + notes, _ := resp["notes"].([]any) + require.Len(t, notes, 3) + got := []string{ + notes[0].(map[string]any)["body"].(string), + notes[1].(map[string]any)["body"].(string), + notes[2].(map[string]any)["body"].(string), + } + assert.Equal(t, []string{"newest", "middle", "oldest"}, got, + "notes must be returned newest first") +} + +// TestAdminNotes_NonAdmin_ListBlocked — a non-admin caller hitting the +// list endpoint must 403 via RequireAdmin BEFORE any DB query runs. +// Identical to the gate-test in admin_customers_test.go but exercised +// against the notes routes specifically — regression-proofing the wiring +// in router.go (the notes endpoints must register inside the +// RequireAdmin-gated group, not outside it). +func TestAdminNotes_NonAdmin_ListBlocked(t *testing.T) { + db, cleanup := adminAppNeedsDB(t) + defer cleanup() + t.Setenv("ADMIN_EMAILS", adminCallerEmail) + app := adminNotesApp(t, db, adminNonAdminEmail) + + teamID, _ := adminSeedTeam(t, db, "hobby") + + cases := []struct { + method, path string + body any + }{ + {"GET", "/api/v1/admin/customers/" + teamID.String() + "/notes", nil}, + {"POST", "/api/v1/admin/customers/" + teamID.String() + "/notes", map[string]any{"body": "x"}}, + {"DELETE", "/api/v1/admin/notes/" + uuid.NewString(), nil}, + } + for _, tc := range cases { + status, body := adminDoJSON(t, app, tc.method, tc.path, tc.body) + assert.Equal(t, http.StatusForbidden, status, + "%s %s — non-admin must be rejected at the gate", tc.method, tc.path) + assert.Equal(t, "forbidden", body["error"]) + } +} + +// TestAdminNotes_Create_EmptyBody_400 — the body field is required. +// Empty-string and whitespace-only must both 400 with missing_body. The +// model layer also rejects (typed sentinel) so a future move of the +// pre-check to the model side keeps the same external behavior. +func TestAdminNotes_Create_EmptyBody_400(t *testing.T) { + db, cleanup := adminAppNeedsDB(t) + defer cleanup() + t.Setenv("ADMIN_EMAILS", adminCallerEmail) + app := adminNotesApp(t, db, adminCallerEmail) + teamID, _ := adminSeedTeam(t, db, "hobby") + + for _, body := range []map[string]any{ + {"body": ""}, + {"body": " \t\n"}, + {}, // no field at all + } { + status, resp := adminDoJSON(t, app, "POST", + "/api/v1/admin/customers/"+teamID.String()+"/notes", body) + assert.Equal(t, http.StatusBadRequest, status, "body=%v must 400", body) + assert.Equal(t, "missing_body", resp["error"]) + } +} + +// TestAdminNotes_Create_UnknownTeam_404 — POST to a team that doesn't +// exist must 404 with team_not_found, NOT a 503 from the FK violation. +// The handler does an explicit GetTeamByID precheck for this reason. +func TestAdminNotes_Create_UnknownTeam_404(t *testing.T) { + db, cleanup := adminAppNeedsDB(t) + defer cleanup() + t.Setenv("ADMIN_EMAILS", adminCallerEmail) + app := adminNotesApp(t, db, adminCallerEmail) + + status, body := adminDoJSON(t, app, "POST", + "/api/v1/admin/customers/"+uuid.NewString()+"/notes", + map[string]any{"body": "ghost note"}) + assert.Equal(t, http.StatusNotFound, status) + assert.Equal(t, "team_not_found", body["error"]) +} + +// TestAdminNotes_Delete_Unknown_404 — DELETE on a note id that doesn't +// exist must 404 with note_not_found (typed sentinel through the model). +func TestAdminNotes_Delete_Unknown_404(t *testing.T) { + db, cleanup := adminAppNeedsDB(t) + defer cleanup() + t.Setenv("ADMIN_EMAILS", adminCallerEmail) + app := adminNotesApp(t, db, adminCallerEmail) + + status, body := adminDoJSON(t, app, "DELETE", + "/api/v1/admin/notes/"+uuid.NewString(), nil) + assert.Equal(t, http.StatusNotFound, status) + assert.Equal(t, "note_not_found", body["error"]) +} + +// TestAdminNotes_Create_TooLong_400 — body > 8KB must be rejected with +// body_too_long. Guards the model's typed-sentinel-→-400 mapping. +func TestAdminNotes_Create_TooLong_400(t *testing.T) { + db, cleanup := adminAppNeedsDB(t) + defer cleanup() + t.Setenv("ADMIN_EMAILS", adminCallerEmail) + app := adminNotesApp(t, db, adminCallerEmail) + teamID, _ := adminSeedTeam(t, db, "hobby") + + // 8KB + 1 byte of 'x'. + huge := bytes.Repeat([]byte("x"), 8*1024+1) + status, resp := adminDoJSON(t, app, "POST", + "/api/v1/admin/customers/"+teamID.String()+"/notes", + map[string]any{"body": string(huge)}) + assert.Equal(t, http.StatusBadRequest, status) + assert.Equal(t, "body_too_long", resp["error"]) +} + +// adminNotesDoJSON is a 1-call wrapper around adminDoJSON kept here so the +// notes test file can be relocated or duplicated without leaning on the +// admin_customers_test.go helper layout. Unused once cross-file +// dependencies stabilize — but cheap to keep. +// +//nolint:unused // reserved for future use +func adminNotesDoJSON(t *testing.T, app *fiber.App, method, path string, body any) (int, map[string]any) { + t.Helper() + var buf bytes.Buffer + if body != nil { + require.NoError(t, json.NewEncoder(&buf).Encode(body)) + } + req := httptest.NewRequest(method, path, &buf) + if body != nil { + req.Header.Set("Content-Type", "application/json") + } + resp, err := app.Test(req, 5000) + require.NoError(t, err) + t.Cleanup(func() { resp.Body.Close() }) + var out map[string]any + if err := json.NewDecoder(resp.Body).Decode(&out); err != nil { + out = map[string]any{} + } + return resp.StatusCode, out +} diff --git a/internal/handlers/admin_impersonate.go b/internal/handlers/admin_impersonate.go new file mode 100644 index 00000000..bacd954e --- /dev/null +++ b/internal/handlers/admin_impersonate.go @@ -0,0 +1,253 @@ +package handlers + +// admin_impersonate.go — POST /api/v1/admin/customers/:team_id/impersonate. +// +// Mints a short-lived (10 minute), read-only JWT scoped to the target +// customer's team so a platform admin can debug the dashboard "as" the +// customer without touching their data. Every mutating endpoint under +// /api/v1/* is gated by RequireWritable, which 403s any request whose +// JWT carries `read_only:true`. The flag is irrevocable for the session +// lifetime — there is no "downgrade to writable" path within a single +// token's validity. +// +// Audit trail: every issuance writes an audit_log row with +// kind=admin.impersonation_started. The metadata blob carries the admin +// email, the target team_id, and the absolute expiry time so a future BI +// consumer can reconstruct "who viewed which customer, when, for how +// long" without re-deriving the impersonation token's claims. +// +// What the minted token DOES NOT carry: +// +// - uid (user_id) of any real user on the target team. We pass a NIL +// uuid string for the `uid` claim so downstream handlers that read +// GetUserID() don't accidentally assign a write to a real user's +// account. The RequireAuth middleware requires a non-empty uid, so +// we use the team's nominal owner user id (resolved at mint time) — +// no user-creation, no shadow account. Document-of-record: every +// write attempt is rejected by RequireWritable before it reaches the +// handler, so the uid-owning user never sees the impersonation in +// their own write audit trail. +// +// - audience (`aud`). Audience checking is opt-in per claim (see +// middleware.RequireAuth) — by omitting it we keep the impersonation +// token compatible with every existing handler without having to +// thread an env-specific canonical URL through the mint path. +// +// - dpop (`cnf.jkt`). Impersonation tokens are bearer-only; the admin +// is on a trusted device by definition. + +import ( + "context" + "database/sql" + "encoding/json" + "errors" + "fmt" + "log/slog" + "time" + + "github.com/gofiber/fiber/v2" + "github.com/golang-jwt/jwt/v4" + "github.com/google/uuid" + "instant.dev/internal/config" + "instant.dev/internal/middleware" + "instant.dev/internal/models" +) + +// AuditKindAdminImpersonationStarted is the audit_log.kind written on +// every successful impersonation-token issuance. Single source of truth so +// the Loops forwarder + BI exports key on the constant rather than a +// drift-prone string literal. NOT yet listed in audit_kinds.go (which is +// for kinds Loops actively forwards) — admin impersonation is internal +// telemetry, not a customer-lifecycle email trigger. +const AuditKindAdminImpersonationStarted = "admin.impersonation_started" + +// impersonationTokenTTL is the absolute lifetime of a minted impersonation +// JWT. 10 minutes is short enough to make a leaked token's blast radius +// trivial (the admin re-mints when their session naturally ages out) and +// long enough for a real debugging session ("click around, reproduce the +// bug, close the tab"). +const impersonationTokenTTL = 10 * time.Minute + +// AdminImpersonateHandler serves POST /admin/customers/:team_id/impersonate. +type AdminImpersonateHandler struct { + db *sql.DB + cfg *config.Config +} + +// NewAdminImpersonateHandler constructs the handler. +func NewAdminImpersonateHandler(db *sql.DB, cfg *config.Config) *AdminImpersonateHandler { + return &AdminImpersonateHandler{db: db, cfg: cfg} +} + +// impersonateClaims mirrors the relevant subset of middleware.sessionClaims +// — `read_only` and `impersonated_by` are the two new fields the +// RequireWritable middleware reads off the parsed JWT. The struct is +// duplicated here (rather than imported) because middleware.sessionClaims +// is package-private; both copies serialize to the same JSON wire shape +// so the consumer doesn't care which producer minted the token. +type impersonateClaims struct { + UserID string `json:"uid"` + TeamID string `json:"tid"` + Email string `json:"email"` + ReadOnly bool `json:"read_only"` + ImpersonatedBy string `json:"impersonated_by"` + jwt.RegisteredClaims +} + +// impersonationAuditMetadata is the audit_log.metadata payload emitted on +// every successful issuance. Typed (rather than an inline map) so the +// audit schema is a contract a future BI consumer can program against. +type impersonationAuditMetadata struct { + ByAdminEmail string `json:"by_admin_email"` + TargetTeamID string `json:"target_team_id"` + TargetUserID string `json:"target_user_id"` + TargetUserEmail string `json:"target_user_email,omitempty"` + IssuedAt time.Time `json:"issued_at"` + ExpiresAt time.Time `json:"expires_at"` + TTLSeconds int `json:"ttl_seconds"` +} + +// Impersonate handles POST /api/v1/admin/customers/:team_id/impersonate. +// +// Response shape: +// +// { +// "ok": true, +// "token": "", +// "expires_at": "", +// "team_id": "" +// } +// +// No agent_action — this endpoint is operator-facing only and never hits +// an LLM agent's wall (callers are the founder, on a trusted device). +func (h *AdminImpersonateHandler) Impersonate(c *fiber.Ctx) error { + teamID, err := uuid.Parse(c.Params("team_id")) + if err != nil { + return respondError(c, fiber.StatusBadRequest, "invalid_team_id", "team_id must be a UUID") + } + + // 1. Verify the target team exists. Without this an admin could mint + // a session-token-shaped JWT for any team id they invent — which + // would pass JWT validation but yield 404s on every read. Failing + // fast here saves the operator a debugging round trip. + if _, err := models.GetTeamByID(c.Context(), h.db, teamID); err != nil { + var nf *models.ErrTeamNotFound + if errors.As(err, &nf) { + return respondError(c, fiber.StatusNotFound, "team_not_found", "no such team") + } + slog.Error("admin.impersonate.team_query_failed", "error", err, "team_id", teamID) + return respondError(c, fiber.StatusServiceUnavailable, "db_failed", "Failed to load team") + } + + // 2. Resolve a target user on the team to back the `uid` claim. The + // RequireAuth middleware rejects tokens with an empty `uid`, so the + // minted JWT MUST carry one — but we don't want to make up a user. + // Picking the team's owner (or earliest-joined member as fallback) + // keeps the impersonation token referencing a real, existing row. + // Every mutating endpoint will still be rejected by RequireWritable + // so this user never accumulates writes from the admin's session. + targetUser, err := h.resolveTargetUser(c.Context(), teamID) + if err != nil { + if errors.Is(err, errImpersonateNoUsers) { + return respondError(c, fiber.StatusConflict, "team_has_no_users", + "target team has no users to impersonate — only teams with at least one user are debuggable") + } + slog.Error("admin.impersonate.user_query_failed", "error", err, "team_id", teamID) + return respondError(c, fiber.StatusServiceUnavailable, "db_failed", "Failed to resolve target user") + } + + adminEmail := middleware.GetEmail(c) + + // 3. Mint the JWT. ReadOnly + ImpersonatedBy are the two flags + // RequireWritable + /auth/me read off the parsed claims. iat/exp + // are explicit so the audit-row metadata's issued_at/expires_at + // line up with what middleware.RequireAuth will enforce. + now := time.Now().UTC() + expiresAt := now.Add(impersonationTokenTTL) + claims := impersonateClaims{ + UserID: targetUser.ID.String(), + TeamID: teamID.String(), + Email: targetUser.Email, + ReadOnly: true, + ImpersonatedBy: adminEmail, + RegisteredClaims: jwt.RegisteredClaims{ + ID: uuid.New().String(), + IssuedAt: jwt.NewNumericDate(now), + ExpiresAt: jwt.NewNumericDate(expiresAt), + }, + } + token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + signed, err := token.SignedString([]byte(h.cfg.JWTSecret)) + if err != nil { + slog.Error("admin.impersonate.sign_failed", "error", err, "team_id", teamID) + return respondError(c, fiber.StatusServiceUnavailable, "sign_failed", "Failed to mint impersonation token") + } + + // 4. Audit row — best-effort. A failure to record audit must NEVER + // surface as a 5xx (would leave the admin with a minted token they + // can't recall but can't audit either). Same fail-open posture as + // the rest of the audit-log call sites. + meta, _ := json.Marshal(impersonationAuditMetadata{ + ByAdminEmail: adminEmail, + TargetTeamID: teamID.String(), + TargetUserID: targetUser.ID.String(), + TargetUserEmail: targetUser.Email, + IssuedAt: now, + ExpiresAt: expiresAt, + TTLSeconds: int(impersonationTokenTTL.Seconds()), + }) + if err := models.InsertAuditEvent(c.Context(), h.db, models.AuditEvent{ + TeamID: teamID, + Actor: "admin", + Kind: AuditKindAdminImpersonationStarted, + Summary: fmt.Sprintf("admin %s started impersonation of team %s (target user %s, 10min)", adminEmail, teamID, targetUser.Email), + Metadata: meta, + }); err != nil { + slog.Warn("admin.impersonate.audit_insert_failed", "error", err, "team_id", teamID) + } + + return c.JSON(fiber.Map{ + "ok": true, + "token": signed, + "team_id": teamID.String(), + "expires_at": expiresAt.Format(time.RFC3339Nano), + }) +} + +// errImpersonateNoUsers is returned by resolveTargetUser when the target +// team has zero users on file. Surfaces as a 409 — an empty team is +// technically a valid team row but isn't useful to impersonate (every +// read would 404 with no team_id-scoped data to display). +var errImpersonateNoUsers = errors.New("admin_impersonate: target team has no users") + +// targetUserRow is the narrow projection resolveTargetUser returns. We +// don't need the full models.User shape — just the id + email for the JWT +// claims and the audit metadata. +type targetUserRow struct { + ID uuid.UUID + Email string +} + +// resolveTargetUser picks the team's nominal "primary" user — owner role +// when present, else earliest-joined member. The result is what backs the +// minted JWT's `uid` claim. Same DISTINCT ON ordering the +// admin-customer-list query uses (admin_customers.go's primary_user CTE) +// so an admin who clicks "view as" on a team listed in the dashboard +// gets impersonated as the same user the dashboard surfaces. +func (h *AdminImpersonateHandler) resolveTargetUser(ctx context.Context, teamID uuid.UUID) (*targetUserRow, error) { + row := &targetUserRow{} + err := h.db.QueryRowContext(ctx, ` + SELECT id, email + FROM users + WHERE team_id = $1 + ORDER BY (role = 'owner') DESC, created_at ASC + LIMIT 1 + `, teamID).Scan(&row.ID, &row.Email) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, errImpersonateNoUsers + } + return nil, fmt.Errorf("resolveTargetUser: %w", err) + } + return row, nil +} diff --git a/internal/handlers/admin_impersonate_test.go b/internal/handlers/admin_impersonate_test.go new file mode 100644 index 00000000..eb1ca482 --- /dev/null +++ b/internal/handlers/admin_impersonate_test.go @@ -0,0 +1,406 @@ +package handlers_test + +// admin_impersonate_test.go — integration coverage for +// POST /api/v1/admin/customers/:team_id/impersonate. +// +// What we assert: +// 1. Endpoint mints a JWT carrying read_only=true + impersonated_by=. +// 2. JWT's exp is ~10min in the future. +// 3. Endpoint writes an audit_log row with kind=admin.impersonation_started. +// 4. Non-admin caller → 403 (RequireAdmin). +// 5. Impersonated session can hit a GET-style RequireAuth-gated handler. +// 6. Impersonated session POST → 403 (RequireWritable). +// 7. Real session POST → 200 (regression — gate must be no-op for normal sessions). +// 8. Token expires after 10min (jwt.ParseWithClaims rejects an expired token). +// +// Test rig: +// - The mint endpoint sits behind RequireAdmin → uses adminAppWithImpersonate +// which wires RequireAuth-less but admin-emailed fake auth (same shim as +// adminApp). +// - To exercise the read-only enforcement (tests 5/6/7) we need a *real* +// RequireAuth → RequireWritable chain because the gate reads the JWT, +// not the fake-auth locals. The chain is built in +// impersonateGuardedApp(), using the same JWT_SECRET test helper. + +import ( + "context" + "database/sql" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/gofiber/fiber/v2" + "github.com/golang-jwt/jwt/v4" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "instant.dev/internal/config" + "instant.dev/internal/handlers" + "instant.dev/internal/middleware" + "instant.dev/internal/testhelpers" +) + +// adminAppWithImpersonate builds a Fiber app wired to the +// AdminImpersonateHandler behind the same fake-auth + RequireAdmin chain +// adminApp() uses. The fake auth pins the caller's email so RequireAdmin +// can read it against ADMIN_EMAILS. +func adminAppWithImpersonate(t *testing.T, db *sql.DB, callerEmail string) *fiber.App { + t.Helper() + app := fiber.New(fiber.Config{ + ErrorHandler: func(c *fiber.Ctx, err error) error { + if errors.Is(err, handlers.ErrResponseWritten) { + return nil + } + code := fiber.StatusInternalServerError + if e, ok := err.(*fiber.Error); ok { + code = e.Code + } + return c.Status(code).JSON(fiber.Map{"ok": false, "error": "internal_error", "message": err.Error()}) + }, + }) + + cfg := &config.Config{JWTSecret: testhelpers.TestJWTSecret} + fakeAuth := func(c *fiber.Ctx) error { + if callerEmail != "" { + c.Locals(middleware.LocalKeyEmail, callerEmail) + } + c.Locals(middleware.LocalKeyUserID, uuid.NewString()) + c.Locals(middleware.LocalKeyTeamID, uuid.NewString()) + return c.Next() + } + + impH := handlers.NewAdminImpersonateHandler(db, cfg) + adminGroup := app.Group("/api/v1/admin", fakeAuth, middleware.RequireAdmin()) + adminGroup.Post("/customers/:team_id/impersonate", impH.Impersonate) + return app +} + +// impersonateGuardedApp builds a tiny Fiber app with the real +// RequireAuth → RequireWritable chain installed and one GET + one POST +// route so we can drive the read-only enforcement end-to-end (test 5/6/7). +func impersonateGuardedApp() *fiber.App { + cfg := &config.Config{JWTSecret: testhelpers.TestJWTSecret} + app := fiber.New() + app.Use(middleware.RequireAuth(cfg)) + app.Use(middleware.RequireWritable()) + app.Get("/probe", func(c *fiber.Ctx) error { + return c.JSON(fiber.Map{ + "ok": true, + "read_only": middleware.IsReadOnly(c), + "impersonated_by": middleware.GetImpersonatedBy(c), + }) + }) + app.Post("/mutate", func(c *fiber.Ctx) error { + return c.JSON(fiber.Map{"ok": true}) + }) + return app +} + +// extractToken pulls the `token` field out of an Impersonate response. +func extractToken(t *testing.T, resp map[string]any) string { + t.Helper() + tok, _ := resp["token"].(string) + require.NotEmpty(t, tok, "response must carry token: %v", resp) + return tok +} + +// parseClaimsAllowExpired parses a JWT into a map without enforcing exp. +// Used by the expiry-test which needs to inspect the exp claim of an +// already-expired token. ParseUnverified skips signature + exp checks. +func parseClaimsAllowExpired(t *testing.T, signed string) map[string]any { + t.Helper() + parsed, _, err := new(jwt.Parser).ParseUnverified(signed, jwt.MapClaims{}) + require.NoError(t, err) + mc, ok := parsed.Claims.(jwt.MapClaims) + require.True(t, ok) + return mc +} + +// TestImpersonate_MintsReadOnlyToken_WithImpersonatedByClaim is the +// headline assertion: the minted JWT carries read_only=true and +// impersonated_by=. Both are required for the +// RequireWritable / /auth/me consumers downstream. +func TestImpersonate_MintsReadOnlyToken_WithImpersonatedByClaim(t *testing.T) { + db, cleanup := adminAppNeedsDB(t) + defer cleanup() + t.Setenv("ADMIN_EMAILS", adminCallerEmail) + app := adminAppWithImpersonate(t, db, adminCallerEmail) + + teamID, _ := adminSeedTeam(t, db, "pro") + + status, resp := adminDoJSON(t, app, "POST", + "/api/v1/admin/customers/"+teamID.String()+"/impersonate", nil) + require.Equal(t, http.StatusOK, status, "mint must succeed: %v", resp) + tok := extractToken(t, resp) + assert.Equal(t, teamID.String(), resp["team_id"]) + + // Parse the minted JWT and assert the two impersonation claims. + claims := parseClaimsAllowExpired(t, tok) + assert.Equal(t, true, claims["read_only"], + "minted token must carry read_only=true") + assert.Equal(t, adminCallerEmail, claims["impersonated_by"], + "minted token must carry impersonated_by=") + assert.Equal(t, teamID.String(), claims["tid"], + "minted token's tid must match the target team") +} + +// TestImpersonate_TokenExpiresIn10Minutes asserts the JWT's exp claim is +// approximately impersonationTokenTTL (10 min) in the future. The exact +// nanosecond offset is irrelevant — we just need confidence that the TTL +// constant flowed through to the wire (regression: a 0-second TTL would +// give an immediately-expired token). +func TestImpersonate_TokenExpiresIn10Minutes(t *testing.T) { + db, cleanup := adminAppNeedsDB(t) + defer cleanup() + t.Setenv("ADMIN_EMAILS", adminCallerEmail) + app := adminAppWithImpersonate(t, db, adminCallerEmail) + + teamID, _ := adminSeedTeam(t, db, "pro") + mintedAt := time.Now() + + _, resp := adminDoJSON(t, app, "POST", + "/api/v1/admin/customers/"+teamID.String()+"/impersonate", nil) + tok := extractToken(t, resp) + expiresAtStr, _ := resp["expires_at"].(string) + require.NotEmpty(t, expiresAtStr, "response must carry expires_at") + expiresAt, err := time.Parse(time.RFC3339Nano, expiresAtStr) + require.NoError(t, err) + + delta := expiresAt.Sub(mintedAt) + assert.True(t, delta > 9*time.Minute && delta < 11*time.Minute, + "exp must be ~10min from mint time (got %v)", delta) + + // Cross-check the JWT's own exp claim against the response field. + claims := parseClaimsAllowExpired(t, tok) + expFloat, _ := claims["exp"].(float64) + require.NotZero(t, expFloat, "JWT must carry exp claim") + assert.InDelta(t, expiresAt.Unix(), int64(expFloat), 2, + "JWT exp claim and response expires_at must match within 2s") +} + +// TestImpersonate_WritesAuditRow_StartedKind — every issuance must record +// an audit_log row so a future investigation can answer "who viewed which +// customer, when, for how long" without parsing JWTs after the fact. +func TestImpersonate_WritesAuditRow_StartedKind(t *testing.T) { + db, cleanup := adminAppNeedsDB(t) + defer cleanup() + t.Setenv("ADMIN_EMAILS", adminCallerEmail) + app := adminAppWithImpersonate(t, db, adminCallerEmail) + + teamID, _ := adminSeedTeam(t, db, "pro") + + status, _ := adminDoJSON(t, app, "POST", + "/api/v1/admin/customers/"+teamID.String()+"/impersonate", nil) + require.Equal(t, http.StatusOK, status) + + var ( + kind, summary string + metaRaw sql.NullString + ) + err := db.QueryRowContext(context.Background(), ` + SELECT kind, summary, metadata::text + FROM audit_log + WHERE team_id = $1 AND kind = $2 + ORDER BY created_at DESC LIMIT 1 + `, teamID, handlers.AuditKindAdminImpersonationStarted).Scan(&kind, &summary, &metaRaw) + require.NoError(t, err, "audit row with kind=admin.impersonation_started must exist for team") + assert.Equal(t, handlers.AuditKindAdminImpersonationStarted, kind) + + require.True(t, metaRaw.Valid) + var meta map[string]any + require.NoError(t, json.Unmarshal([]byte(metaRaw.String), &meta)) + assert.Equal(t, adminCallerEmail, meta["by_admin_email"]) + assert.Equal(t, teamID.String(), meta["target_team_id"]) + assert.Equal(t, float64(int(10*time.Minute/time.Second)), meta["ttl_seconds"]) +} + +// TestImpersonate_NonAdmin_403 — the impersonation route is RequireAdmin- +// gated like the rest of the admin surface. A non-admin caller must 403 +// BEFORE the mint runs. +func TestImpersonate_NonAdmin_403(t *testing.T) { + db, cleanup := adminAppNeedsDB(t) + defer cleanup() + t.Setenv("ADMIN_EMAILS", adminCallerEmail) + app := adminAppWithImpersonate(t, db, adminNonAdminEmail) + + teamID, _ := adminSeedTeam(t, db, "pro") + status, body := adminDoJSON(t, app, "POST", + "/api/v1/admin/customers/"+teamID.String()+"/impersonate", nil) + assert.Equal(t, http.StatusForbidden, status) + assert.Equal(t, "forbidden", body["error"]) +} + +// TestImpersonate_UnknownTeam_404 — non-existent target team id must 404 +// at the precheck, before any user lookup or token mint. +func TestImpersonate_UnknownTeam_404(t *testing.T) { + db, cleanup := adminAppNeedsDB(t) + defer cleanup() + t.Setenv("ADMIN_EMAILS", adminCallerEmail) + app := adminAppWithImpersonate(t, db, adminCallerEmail) + + status, body := adminDoJSON(t, app, "POST", + "/api/v1/admin/customers/"+uuid.NewString()+"/impersonate", nil) + assert.Equal(t, http.StatusNotFound, status) + assert.Equal(t, "team_not_found", body["error"]) +} + +// TestImpersonate_TeamWithNoUsers_409 — minting a token for a team that +// has zero users on file is technically valid but useless; we 409 rather +// than silently mint a token tied to a nil uid (which RequireAuth would +// reject downstream anyway, producing a confusing 401 for the admin). +func TestImpersonate_TeamWithNoUsers_409(t *testing.T) { + db, cleanup := adminAppNeedsDB(t) + defer cleanup() + t.Setenv("ADMIN_EMAILS", adminCallerEmail) + app := adminAppWithImpersonate(t, db, adminCallerEmail) + + // Create a bare team row with no users. + teamID := uuid.MustParse(testhelpers.MustCreateTeamDB(t, db, "hobby")) + t.Cleanup(func() { + db.Exec(`DELETE FROM teams WHERE id = $1`, teamID) + }) + + status, body := adminDoJSON(t, app, "POST", + "/api/v1/admin/customers/"+teamID.String()+"/impersonate", nil) + assert.Equal(t, http.StatusConflict, status) + assert.Equal(t, "team_has_no_users", body["error"]) +} + +// TestImpersonate_TokenCanCallGetEndpoint — the minted token must pass +// RequireAuth and reach a GET handler. Verifies the JWT is signed with +// the same secret RequireAuth validates against, and that the read_only +// flag does NOT block GETs. +func TestImpersonate_TokenCanCallGetEndpoint(t *testing.T) { + db, cleanup := adminAppNeedsDB(t) + defer cleanup() + t.Setenv("ADMIN_EMAILS", adminCallerEmail) + app := adminAppWithImpersonate(t, db, adminCallerEmail) + + teamID, _ := adminSeedTeam(t, db, "pro") + _, resp := adminDoJSON(t, app, "POST", + "/api/v1/admin/customers/"+teamID.String()+"/impersonate", nil) + tok := extractToken(t, resp) + + // Hit a GET behind RequireAuth + RequireWritable. The chain must let + // us through and the probe handler must see read_only=true. + guarded := impersonateGuardedApp() + req := httptest.NewRequest(http.MethodGet, "/probe", nil) + req.Header.Set("Authorization", "Bearer "+tok) + got, err := guarded.Test(req, 5000) + require.NoError(t, err) + defer got.Body.Close() + assert.Equal(t, http.StatusOK, got.StatusCode) + + var body map[string]any + require.NoError(t, json.NewDecoder(got.Body).Decode(&body)) + assert.Equal(t, true, body["read_only"], + "GET handler must see read_only=true on the impersonated session") + assert.Equal(t, adminCallerEmail, body["impersonated_by"], + "GET handler must see the admin email from the impersonation token") +} + +// TestImpersonate_TokenCannotPOST — the minted token's read_only flag +// MUST cause RequireWritable to 403 every POST/PUT/PATCH/DELETE. This is +// the headline regression test for the "view-as-customer" invariant. +// +// Also asserts the response carries the canonical agent_action string so +// the U3 contract holds end-to-end (mint → middleware → response body). +func TestImpersonate_TokenCannotPOST(t *testing.T) { + db, cleanup := adminAppNeedsDB(t) + defer cleanup() + t.Setenv("ADMIN_EMAILS", adminCallerEmail) + app := adminAppWithImpersonate(t, db, adminCallerEmail) + + teamID, _ := adminSeedTeam(t, db, "pro") + _, resp := adminDoJSON(t, app, "POST", + "/api/v1/admin/customers/"+teamID.String()+"/impersonate", nil) + tok := extractToken(t, resp) + + guarded := impersonateGuardedApp() + req := httptest.NewRequest(http.MethodPost, "/mutate", nil) + req.Header.Set("Authorization", "Bearer "+tok) + got, err := guarded.Test(req, 5000) + require.NoError(t, err) + defer got.Body.Close() + assert.Equal(t, http.StatusForbidden, got.StatusCode, + "POST under impersonated session must 403 via RequireWritable") + + var body map[string]any + require.NoError(t, json.NewDecoder(got.Body).Decode(&body)) + assert.Equal(t, "read_only_session", body["error"], + "error code must be the distinct read_only_session keyword") + aa, _ := body["agent_action"].(string) + assert.Contains(t, aa, "read-only impersonated session", + "agent_action must name the specific rejection reason") + assert.Contains(t, aa, "https://instanode.dev/app", + "agent_action must contain a full https URL") +} + +// TestImpersonate_RealSessionPOST_StillWorks — regression: a normal +// (non-impersonated) session must still be able to POST after this +// middleware lands. The gate is a no-op for tokens without read_only=true, +// and this test pins that invariant. +func TestImpersonate_RealSessionPOST_StillWorks(t *testing.T) { + tok := testhelpers.MustSignSessionJWT(t, uuid.NewString(), uuid.NewString(), "real@example.com") + + guarded := impersonateGuardedApp() + req := httptest.NewRequest(http.MethodPost, "/mutate", nil) + req.Header.Set("Authorization", "Bearer "+tok) + got, err := guarded.Test(req, 5000) + require.NoError(t, err) + defer got.Body.Close() + assert.Equal(t, http.StatusOK, got.StatusCode, + "a real (non-impersonated) session must still be allowed to POST — RequireWritable must be a no-op for read_only=false tokens") +} + +// TestImpersonate_TokenExpires_RejectedByAuth — an expired impersonation +// token must be rejected by RequireAuth (401), NOT silently accepted as +// read-only. Mints a token via the real handler, hand-rewrites its exp to +// the past, and asserts RequireAuth's 401 path fires. +// +// We don't sleep 10 minutes — instead we mint a token with a manually +// crafted exp claim via the test's local jwt-signing helper, signed with +// the same secret, and verify it's rejected. This is a defensive check +// because the impersonation TTL is short by design and a regression that +// neutered the exp claim would be invisible at normal request rates. +func TestImpersonate_TokenExpires_RejectedByAuth(t *testing.T) { + // Build a JWT identical in shape to what AdminImpersonateHandler + // emits, but with exp = 1 hour in the past. RequireAuth must reject. + type impersonateClaims struct { + UserID string `json:"uid"` + TeamID string `json:"tid"` + Email string `json:"email"` + ReadOnly bool `json:"read_only"` + ImpersonatedBy string `json:"impersonated_by"` + jwt.RegisteredClaims + } + expired := time.Now().Add(-1 * time.Hour) + claims := impersonateClaims{ + UserID: uuid.NewString(), + TeamID: uuid.NewString(), + Email: "target@example.com", + ReadOnly: true, + ImpersonatedBy: adminCallerEmail, + RegisteredClaims: jwt.RegisteredClaims{ + ID: uuid.NewString(), + IssuedAt: jwt.NewNumericDate(expired.Add(-10 * time.Minute)), + ExpiresAt: jwt.NewNumericDate(expired), + }, + } + tok := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + signed, err := tok.SignedString([]byte(testhelpers.TestJWTSecret)) + require.NoError(t, err) + + guarded := impersonateGuardedApp() + req := httptest.NewRequest(http.MethodGet, "/probe", nil) + req.Header.Set("Authorization", "Bearer "+signed) + got, err := guarded.Test(req, 5000) + require.NoError(t, err) + defer got.Body.Close() + assert.Equal(t, http.StatusUnauthorized, got.StatusCode, + "expired impersonation token must be rejected by RequireAuth (401), NOT silently accepted as read-only") +} diff --git a/internal/handlers/agent_action.go b/internal/handlers/agent_action.go index c49fc290..23eb9244 100644 --- a/internal/handlers/agent_action.go +++ b/internal/handlers/agent_action.go @@ -304,3 +304,16 @@ func newAgentActionAdminPromoIssued(teamID, code string) string { code, teamID, ) } + +// AgentActionReadOnlySession is returned on every 403 emitted by the +// RequireWritable middleware when a JWT carries `read_only:true` — i.e. the +// admin minted an impersonation token to view-as-customer and the agent +// (or the dashboard the admin is steering) attempted a mutation. Names the +// specific rejection reason ("read-only impersonated session"), the exact +// next action ("switch back to your real account"), and a full +// https://instanode.dev/app URL. The U3 contract test exercises it. +// +// Read-only is irrevocable for the lifetime of the impersonation token — +// there is no "downgrade to writable" path. The remedy is to use the +// admin's own session token, which never carries this flag. +const AgentActionReadOnlySession = "Tell the user this is a read-only impersonated session. Mutations are disabled. Switch back to your real account at https://instanode.dev/app to make changes." diff --git a/internal/handlers/agent_action_contract_test.go b/internal/handlers/agent_action_contract_test.go index 4e1b9767..37d6de72 100644 --- a/internal/handlers/agent_action_contract_test.go +++ b/internal/handlers/agent_action_contract_test.go @@ -38,10 +38,14 @@ func agentActionContractCases() map[string]string { "AgentActionPromotionInvalid": AgentActionPromotionInvalid, "AgentActionPromotionAlreadyUsed": AgentActionPromotionAlreadyUsed, "AgentActionPromotionExpired": AgentActionPromotionExpired, +<<<<<<< HEAD + "AgentActionReadOnlySession": AgentActionReadOnlySession, +======= "AgentActionNotifyWebhookInvalid": AgentActionNotifyWebhookInvalid, "AgentActionPauseRequiresPro": AgentActionPauseRequiresPro, "AgentActionResourceAlreadyPaused": AgentActionResourceAlreadyPaused, "AgentActionResourceNotPaused": AgentActionResourceNotPaused, +>>>>>>> origin/master // Builders — representative inputs covering tier/env/role/limit // interpolation. @@ -103,6 +107,7 @@ func assertContract(t *testing.T, name, s string) { "Remove", "remove", // family-disabled "Redeploy", "redeploy", "Confirm", "confirm", + "Switch", "switch", // read-only impersonation "check ", "Check ", // bindings cross-team / not-found "use ", "Use ", // bindings not-found "must be ", // bindings invalid-uuid → action is "must be a UUID" diff --git a/internal/handlers/cli_auth.go b/internal/handlers/cli_auth.go index 0ba72007..b71571f7 100644 --- a/internal/handlers/cli_auth.go +++ b/internal/handlers/cli_auth.go @@ -297,6 +297,20 @@ func (h *CLIAuthHandler) GetCurrentUser(c *fiber.Ctx) error { resp["admin_path_prefix"] = h.cfg.AdminPathPrefix } + // Impersonation surfacing — when the caller's JWT carries read_only=true + // (i.e. the session was minted via POST /api/v1/admin/customers/:id/impersonate) + // expose two read-only fields so the dashboard can render the "viewing + // as " banner + grey out mutating UI. We only emit the keys + // when the flag is set; non-impersonated sessions see a clean response + // shape. The wire surface (read_only:bool, impersonated_by:string) + // matches what the RequireWritable middleware reads from the same JWT. + if middleware.IsReadOnly(c) { + resp["read_only"] = true + if by := middleware.GetImpersonatedBy(c); by != "" { + resp["impersonated_by"] = by + } + } + return c.JSON(resp) } diff --git a/internal/middleware/auth.go b/internal/middleware/auth.go index a3202113..1a253538 100644 --- a/internal/middleware/auth.go +++ b/internal/middleware/auth.go @@ -25,6 +25,19 @@ const ( // downstream middleware/handlers can branch on identity without a DB hit — // in particular RequireAdmin reads it to check the ADMIN_EMAILS allowlist. LocalKeyEmail = "auth_email" + // LocalKeyReadOnly is the fiber.Locals key set to true when the JWT + // carries `read_only:true` — i.e. the session was minted via + // POST /api/v1/admin/customers/:team_id/impersonate. Consumed by + // RequireWritable, which 403s any POST/PATCH/PUT/DELETE while the flag + // is set. The flag is irrevocable for the session's lifetime. + LocalKeyReadOnly = "auth_read_only" + // LocalKeyImpersonatedBy is the fiber.Locals key holding the admin email + // that minted an impersonation token (`impersonated_by` JWT claim). + // Empty when the session is a normal (non-impersonated) one. Surfaced + // in logs / audit trails so a future investigation can answer "who + // caused this read?" — and emitted on /auth/me so the dashboard can + // render the "you are viewing as " banner. + LocalKeyImpersonatedBy = "auth_impersonated_by" // audienceMismatchError is the error keyword used when an RFC 8707 // audience check fails. Distinct from the generic "unauthorized" so that @@ -101,6 +114,17 @@ type sessionClaims struct { TeamID string `json:"tid"` Email string `json:"email"` Confirmation *confirmation `json:"cnf,omitempty"` + // ReadOnly + ImpersonatedBy back the read-only "view-as-customer" + // impersonation surface: a platform admin mints a 10-minute JWT scoped + // to a target customer's team via POST /api/v1/admin/customers/:id/impersonate. + // RequireWritable consumes ReadOnly to 403 every POST/PATCH/PUT/DELETE + // the impersonated session attempts; ImpersonatedBy is surfaced on + // /auth/me and emitted in audit/log lines so the admin's identity is + // preserved across the session boundary. Both default to zero values + // for normal (non-impersonated) sessions — JSON omitempty keeps the + // wire shape unchanged for the common path. + ReadOnly bool `json:"read_only,omitempty"` + ImpersonatedBy string `json:"impersonated_by,omitempty"` jwt.RegisteredClaims } @@ -247,6 +271,16 @@ func RequireAuth(cfg *config.Config) fiber.Handler { if claims.Confirmation != nil && claims.Confirmation.JKT != "" { c.Locals(LocalKeyDPoPKeyThumbprint, claims.Confirmation.JKT) } + // Impersonation locals — set unconditionally when the claims carry + // them (omitempty on the wire means the receiver only sees them when + // the issuer set them). RequireWritable reads LocalKeyReadOnly to + // gate mutating routes. + if claims.ReadOnly { + c.Locals(LocalKeyReadOnly, true) + } + if claims.ImpersonatedBy != "" { + c.Locals(LocalKeyImpersonatedBy, claims.ImpersonatedBy) + } return c.Next() } } @@ -290,6 +324,25 @@ func GetDPoPKeyThumbprint(c *fiber.Ctx) string { return "" } +// IsReadOnly reports whether the current request's JWT carried +// `read_only:true` — i.e. it was minted by the admin impersonation flow. +// Centralised so RequireWritable, audit-log emitters, and the /auth/me +// surfacing all agree on the single source of truth (LocalKeyReadOnly). +func IsReadOnly(c *fiber.Ctx) bool { + v, ok := c.Locals(LocalKeyReadOnly).(bool) + return ok && v +} + +// GetImpersonatedBy returns the admin email that minted the current +// impersonation token, or "" when the session is a normal one. Surfaced +// on /auth/me so the dashboard can render the impersonation banner. +func GetImpersonatedBy(c *fiber.Ctx) string { + if v, ok := c.Locals(LocalKeyImpersonatedBy).(string); ok { + return v + } + return "" +} + // OptionalAuth is like RequireAuth but does not return 401 when the header is absent or invalid. // If a valid bearer token is present it populates the same Fiber locals as RequireAuth. // Use on routes where anonymous access is allowed but authenticated users get elevated behaviour. @@ -334,6 +387,18 @@ func OptionalAuth(cfg *config.Config) fiber.Handler { if claims.Confirmation != nil && claims.Confirmation.JKT != "" { c.Locals(LocalKeyDPoPKeyThumbprint, claims.Confirmation.JKT) } + // Mirror the impersonation-locals population done in RequireAuth so + // downstream RequireWritable (when attached to an OptionalAuth route) + // sees the read_only flag and gates mutations. An impersonated session + // presenting an Authorization header on an OptionalAuth route must + // still be blocked from writing — that's exactly the /db/new etc. + // case test #5 in the brief exercises. + if claims.ReadOnly { + c.Locals(LocalKeyReadOnly, true) + } + if claims.ImpersonatedBy != "" { + c.Locals(LocalKeyImpersonatedBy, claims.ImpersonatedBy) + } return c.Next() } } diff --git a/internal/middleware/require_writable.go b/internal/middleware/require_writable.go new file mode 100644 index 00000000..46c974a2 --- /dev/null +++ b/internal/middleware/require_writable.go @@ -0,0 +1,120 @@ +package middleware + +// require_writable.go — gates mutating routes against the read_only JWT +// flag set by the platform-admin impersonation flow. +// +// What it does: +// +// - Reads LocalKeyReadOnly (populated by RequireAuth / OptionalAuth from +// the JWT's `read_only` claim). +// - If the flag is true, returns 403 with the canonical agent_action +// handlers.AgentActionReadOnlySession so an LLM agent steering a +// mutating call from inside an admin's view-as-customer impersonation +// session gets verbatim copy to relay ("this is a read-only +// impersonated session, switch back at https://instanode.dev/app"). +// - Otherwise hands off to the next handler, untouched. +// +// Where it lives in the chain: +// +// The router installs it on the /api/v1 group (after RequireAuth + +// PopulateTeamRole) and on the /deploy group, and inline on every +// top-level POST/PATCH/PUT/DELETE that an impersonated bearer could +// conceivably hit (POST /db/new, /cache/new, /nosql/new, /queue/new, +// /storage/new, /webhook/new, /stacks/*, etc.). The impersonation-mint +// endpoint itself is the only deliberate exception — the admin minting +// the read-only token holds a normal (writable) session, so the gate +// would never fire there, but the spec calls out the exemption +// explicitly so the audit comment in router.go reads cleanly. +// +// Why a middleware (not a per-handler check): +// +// The read_only flag is irrevocable for the session's lifetime — there +// is no "downgrade to writable" path within a single token's validity. +// Centralising the check on the route boundary keeps the policy at the +// one place an auditor needs to grep: the router. Handlers stay free of +// "if read_only return 403" boilerplate, and the U3 contract test +// exercises the one agent_action string the middleware emits. + +import ( + "net/http" + + "github.com/gofiber/fiber/v2" +) + +// readOnlyForbiddenAgentAction mirrors handlers.AgentActionReadOnlySession. +// Duplicated here rather than imported because middleware is depended on by +// handlers (not the other way around); a cross-import would introduce a +// cycle. The handlers package keeps its own copy, and the U3 contract test +// exercises that constant — touching either string without the other is the +// regression we want CI to catch. +const readOnlyForbiddenAgentAction = "Tell the user this is a read-only impersonated session. Mutations are disabled. Switch back to your real account at https://instanode.dev/app to make changes." + +// mutatingMethods is the closed set of HTTP verbs RequireWritable gates. +// GET / HEAD / OPTIONS fall through unconditionally — an impersonated +// session's whole purpose is to *read* the customer's data. Set-membership +// is one switch on the request method; cheaper than a map lookup on the +// hot path. +// +// Listed as constants (not a slice) so reviewers can grep for the exact +// set this gate enforces. A new method (PATCH was already standard, +// CONNECT etc. would be the future) would need a deliberate addition. +const ( + methodPOST = "POST" + methodPUT = "PUT" + methodPATCH = "PATCH" + methodDELETE = "DELETE" +) + +// isMutatingMethod reports whether method is one of the four verbs +// RequireWritable gates. Exposed so future audit/log emitters can ask the +// same question without re-encoding the set. +func isMutatingMethod(method string) bool { + switch method { + case methodPOST, methodPUT, methodPATCH, methodDELETE: + return true + } + return false +} + +// RequireWritable returns a Fiber middleware that rejects mutating +// requests (POST/PUT/PATCH/DELETE) from a read-only (impersonation) +// session with 403 + the canonical agent_action. MUST be installed AFTER +// RequireAuth / OptionalAuth — both of those populate LocalKeyReadOnly +// from the JWT. +// +// GET / HEAD / OPTIONS fall through unconditionally so the impersonated +// admin can still browse the customer's dashboard — view-as-customer is +// exactly what this middleware enables. Non-impersonated sessions also +// fall through with a single bool-check (the hot path). +// +// Response shape on rejection (403): +// +// { +// "ok": false, +// "error": "read_only_session", +// "message": "this session is read-only (admin impersonation) — mutations are disabled", +// "agent_action": "Tell the user this is a read-only impersonated session..." +// } +// +// `read_only_session` is distinct from the generic "forbidden" code so an +// agent inspecting the response can branch on "I need to ask the user to +// switch back" without a substring match on the agent_action prose. +func RequireWritable() fiber.Handler { + return func(c *fiber.Ctx) error { + // Fast path: non-impersonated sessions are the vast majority of + // traffic. One bool-check, then c.Next(). + if !IsReadOnly(c) { + return c.Next() + } + // Impersonated session — let reads through, gate the writes. + if !isMutatingMethod(c.Method()) { + return c.Next() + } + return c.Status(http.StatusForbidden).JSON(fiber.Map{ + "ok": false, + "error": "read_only_session", + "message": "this session is read-only (admin impersonation) — mutations are disabled", + "agent_action": readOnlyForbiddenAgentAction, + }) + } +} diff --git a/internal/middleware/require_writable_test.go b/internal/middleware/require_writable_test.go new file mode 100644 index 00000000..60d0c81b --- /dev/null +++ b/internal/middleware/require_writable_test.go @@ -0,0 +1,213 @@ +package middleware_test + +// require_writable_test.go — unit coverage for the RequireWritable +// middleware. Drives every method × every flag combination through a +// minimal Fiber app so a regression in the gate (e.g. accidentally +// blocking GET on a read-only session, or letting POST through) is +// caught here before it ships. +// +// Why the matrix is exhaustive: +// +// The middleware has exactly four axes — read_only flag (set/unset), +// HTTP method (mutating/non-mutating), method case (POST vs post — +// Fiber normalises, but defensive), and the (impossible-in-practice +// but defensive) case where read_only is set to a non-bool. Each axis +// is one test below. Adding a 5th axis (e.g. method allowlisting) means +// adding a 5th test case here — the matrix shape is the contract. + +import ( + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/gofiber/fiber/v2" + "github.com/golang-jwt/jwt/v4" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "instant.dev/internal/config" + "instant.dev/internal/middleware" + "instant.dev/internal/testhelpers" +) + +// signImpersonationToken mints a JWT carrying read_only=true and +// impersonated_by=. Same wire shape as the real handler's +// AdminImpersonateHandler issues. +func signImpersonationToken(t *testing.T, secret, userID, teamID, adminEmail string) string { + t.Helper() + type impersonateClaims struct { + UserID string `json:"uid"` + TeamID string `json:"tid"` + Email string `json:"email"` + ReadOnly bool `json:"read_only"` + ImpersonatedBy string `json:"impersonated_by"` + jwt.RegisteredClaims + } + claims := impersonateClaims{ + UserID: userID, + TeamID: teamID, + Email: "target@example.com", + ReadOnly: true, + ImpersonatedBy: adminEmail, + RegisteredClaims: jwt.RegisteredClaims{ + ID: uuid.NewString(), + IssuedAt: jwt.NewNumericDate(time.Now()), + ExpiresAt: jwt.NewNumericDate(time.Now().Add(10 * time.Minute)), + }, + } + tok := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + signed, err := tok.SignedString([]byte(secret)) + require.NoError(t, err) + return signed +} + +// newWritableTestApp builds a Fiber app with the auth + RequireWritable +// chain installed and one route per HTTP verb echoing back "ok" so the +// test can assert which verb passed/failed. +func newWritableTestApp() *fiber.App { + cfg := &config.Config{JWTSecret: testhelpers.TestJWTSecret} + app := fiber.New() + app.Use(middleware.OptionalAuth(cfg)) + app.Use(middleware.RequireWritable()) + echo := func(c *fiber.Ctx) error { + return c.JSON(fiber.Map{"ok": true}) + } + app.Get("/route", echo) + app.Post("/route", echo) + app.Put("/route", echo) + app.Patch("/route", echo) + app.Delete("/route", echo) + return app +} + +// TestRequireWritable_NoToken_AllMethodsPass — anonymous (no Authorization +// header at all) callers must NOT trip the gate. RequireWritable only +// fires when read_only is set, and OptionalAuth doesn't set it on +// header-less requests. This is the most important guardrail: the gate +// must be inert for the 99.99% of traffic that isn't impersonated. +func TestRequireWritable_NoToken_AllMethodsPass(t *testing.T) { + app := newWritableTestApp() + for _, m := range []string{"GET", "POST", "PUT", "PATCH", "DELETE"} { + req := httptest.NewRequest(m, "/route", nil) + resp, err := app.Test(req, 1000) + require.NoError(t, err) + resp.Body.Close() + assert.Equal(t, http.StatusOK, resp.StatusCode, + "anonymous %s must pass RequireWritable (read_only flag unset)", m) + } +} + +// TestRequireWritable_NormalToken_AllMethodsPass — a writable (non- +// impersonated) session must NOT trip the gate on any verb. Verifies the +// inverse of the impersonation tests: read_only=false on the JWT means +// the gate is a no-op. +func TestRequireWritable_NormalToken_AllMethodsPass(t *testing.T) { + app := newWritableTestApp() + tok := signSession(t, testhelpers.TestJWTSecret, uuid.NewString(), uuid.NewString(), time.Hour) + for _, m := range []string{"GET", "POST", "PUT", "PATCH", "DELETE"} { + req := httptest.NewRequest(m, "/route", nil) + req.Header.Set("Authorization", "Bearer "+tok) + resp, err := app.Test(req, 1000) + require.NoError(t, err) + resp.Body.Close() + assert.Equal(t, http.StatusOK, resp.StatusCode, + "writable %s must pass RequireWritable", m) + } +} + +// TestRequireWritable_ImpersonatedSession_GETPasses — the entire point of +// view-as-customer is to read. A read-only session MUST be able to GET. +// Regression target: an earlier version of this middleware rejected every +// method including GETs, which broke the very use case it was supposed to +// enable. +func TestRequireWritable_ImpersonatedSession_GETPasses(t *testing.T) { + app := newWritableTestApp() + tok := signImpersonationToken(t, testhelpers.TestJWTSecret, + uuid.NewString(), uuid.NewString(), "founder@instanode.dev") + req := httptest.NewRequest(http.MethodGet, "/route", nil) + req.Header.Set("Authorization", "Bearer "+tok) + resp, err := app.Test(req, 1000) + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusOK, resp.StatusCode, + "read_only session must be allowed to GET — view-as-customer is the whole point") +} + +// TestRequireWritable_ImpersonatedSession_PostBlocked — POST under an +// impersonated session must 403 with the canonical agent_action + +// error code. This is the headline rejection path the audit cares about. +func TestRequireWritable_ImpersonatedSession_PostBlocked(t *testing.T) { + app := newWritableTestApp() + tok := signImpersonationToken(t, testhelpers.TestJWTSecret, + uuid.NewString(), uuid.NewString(), "founder@instanode.dev") + req := httptest.NewRequest(http.MethodPost, "/route", nil) + req.Header.Set("Authorization", "Bearer "+tok) + resp, err := app.Test(req, 1000) + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusForbidden, resp.StatusCode, + "POST under read_only session must 403") + + var body map[string]any + testhelpers.DecodeJSON(t, resp, &body) + assert.Equal(t, false, body["ok"]) + assert.Equal(t, "read_only_session", body["error"], + "error code must be the distinct read_only_session keyword (NOT generic forbidden) so agents can branch") + aa, _ := body["agent_action"].(string) + assert.Contains(t, aa, "read-only impersonated session", + "agent_action must name the specific rejection reason") + assert.Contains(t, aa, "https://instanode.dev/app", + "agent_action must contain a full https URL for the LLM to relay") +} + +// TestRequireWritable_ImpersonatedSession_AllMutatingMethodsBlocked — +// POST/PUT/PATCH/DELETE must all 403 under an impersonated session. +// Belt-and-suspenders for the headline test: each verb individually. +func TestRequireWritable_ImpersonatedSession_AllMutatingMethodsBlocked(t *testing.T) { + app := newWritableTestApp() + tok := signImpersonationToken(t, testhelpers.TestJWTSecret, + uuid.NewString(), uuid.NewString(), "founder@instanode.dev") + for _, m := range []string{"POST", "PUT", "PATCH", "DELETE"} { + req := httptest.NewRequest(m, "/route", nil) + req.Header.Set("Authorization", "Bearer "+tok) + resp, err := app.Test(req, 1000) + require.NoError(t, err) + resp.Body.Close() + assert.Equal(t, http.StatusForbidden, resp.StatusCode, + "%s under read_only session must 403", m) + } +} + +// TestRequireWritable_ImpersonationLocalsPopulated — both LocalKeyReadOnly +// and LocalKeyImpersonatedBy must be reachable from a downstream handler +// via the public accessors (IsReadOnly / GetImpersonatedBy). Guards +// against a regression where the auth middleware stops populating one +// of the two locals (e.g. ImpersonatedBy is dropped during a refactor). +func TestRequireWritable_ImpersonationLocalsPopulated(t *testing.T) { + cfg := &config.Config{JWTSecret: testhelpers.TestJWTSecret} + app := fiber.New() + app.Use(middleware.OptionalAuth(cfg)) + app.Get("/probe", func(c *fiber.Ctx) error { + return c.JSON(fiber.Map{ + "read_only": middleware.IsReadOnly(c), + "impersonated_by": middleware.GetImpersonatedBy(c), + }) + }) + + tok := signImpersonationToken(t, testhelpers.TestJWTSecret, + uuid.NewString(), uuid.NewString(), "founder@instanode.dev") + req := httptest.NewRequest(http.MethodGet, "/probe", nil) + req.Header.Set("Authorization", "Bearer "+tok) + resp, err := app.Test(req, 1000) + require.NoError(t, err) + defer resp.Body.Close() + + var body map[string]any + testhelpers.DecodeJSON(t, resp, &body) + assert.Equal(t, true, body["read_only"], + "IsReadOnly must return true for an impersonation token") + assert.Equal(t, "founder@instanode.dev", body["impersonated_by"], + "GetImpersonatedBy must return the admin email from the JWT") +} diff --git a/internal/models/admin_customer_notes.go b/internal/models/admin_customer_notes.go new file mode 100644 index 00000000..960ae44c --- /dev/null +++ b/internal/models/admin_customer_notes.go @@ -0,0 +1,157 @@ +package models + +// admin_customer_notes.go — free-text notes per team, written by platform +// admins. Surfaces on the admin Customer Detail drawer so the founder can +// jot "called this customer 2024-05-10, they want pro tier with annual +// billing" without leaving the dashboard. +// +// Storage shape: dedicated `admin_customer_notes` table (migration 024). +// Hard delete on DELETE — notes are reversible by re-typing, so the soft- +// delete bookkeeping (an `is_deleted` column, paranoid filtering on every +// read) buys nothing operationally. The author_email column is +// denormalized rather than a FK to users so deleting an admin's user row +// doesn't blow up audit coherence; same pattern as audit_log.actor. + +import ( + "context" + "database/sql" + "errors" + "fmt" + "strings" + "time" + + "github.com/google/uuid" +) + +// AdminCustomerNoteMaxBody bounds the user-supplied body to keep one note +// from monopolising a row. 8KB is enough for paragraph-length context +// ("called this customer 2024-05-10, they want pro tier with annual +// billing…") and well under Postgres TOAST overflow. +const AdminCustomerNoteMaxBody = 8 * 1024 + +// ErrAdminCustomerNoteEmpty is returned by CreateAdminCustomerNote when +// the body is empty/whitespace-only. Validated in the model layer so +// the handler doesn't have to repeat the check. +var ErrAdminCustomerNoteEmpty = errors.New("models.CreateAdminCustomerNote: body must be non-empty") + +// ErrAdminCustomerNoteTooLong is returned when the body exceeds +// AdminCustomerNoteMaxBody bytes. +var ErrAdminCustomerNoteTooLong = errors.New("models.CreateAdminCustomerNote: body exceeds 8KB cap") + +// ErrAdminCustomerNoteNotFound is returned by DeleteAdminCustomerNote +// when the note ID doesn't exist. Distinct sentinel so the handler can +// branch to 404 vs 503 cleanly. +var ErrAdminCustomerNoteNotFound = errors.New("models.DeleteAdminCustomerNote: note not found") + +// AdminCustomerNote mirrors one row of the admin_customer_notes table. +type AdminCustomerNote struct { + ID uuid.UUID + TeamID uuid.UUID + Body string + AuthorEmail string + CreatedAt time.Time +} + +// CreateAdminCustomerNoteParams bundles the inputs for inserting a note. +type CreateAdminCustomerNoteParams struct { + TeamID uuid.UUID + Body string + AuthorEmail string +} + +// CreateAdminCustomerNote inserts one row and returns the populated note. +// Validates body length here (not at the DB layer) so the error is a +// typed sentinel callers can branch on without parsing PG error codes. +func CreateAdminCustomerNote(ctx context.Context, db *sql.DB, p CreateAdminCustomerNoteParams) (*AdminCustomerNote, error) { + body := strings.TrimSpace(p.Body) + if body == "" { + return nil, ErrAdminCustomerNoteEmpty + } + if len(body) > AdminCustomerNoteMaxBody { + return nil, ErrAdminCustomerNoteTooLong + } + + out := &AdminCustomerNote{ + TeamID: p.TeamID, + Body: body, + AuthorEmail: p.AuthorEmail, + } + err := db.QueryRowContext(ctx, ` + INSERT INTO admin_customer_notes (team_id, body, author_email) + VALUES ($1, $2, $3) + RETURNING id, created_at + `, p.TeamID, body, p.AuthorEmail).Scan(&out.ID, &out.CreatedAt) + if err != nil { + return nil, fmt.Errorf("models.CreateAdminCustomerNote: %w", err) + } + return out, nil +} + +// ListAdminCustomerNotes returns every note for a team, newest first. +// Capped at limit rows (clamped to a sensible default + max here so the +// handler doesn't have to repeat the bounds-check). Unlike the audit log +// this isn't paginated — the per-team note volume is expected to stay in +// the dozens. +func ListAdminCustomerNotes(ctx context.Context, db *sql.DB, teamID uuid.UUID, limit int) ([]*AdminCustomerNote, error) { + if limit <= 0 { + limit = adminCustomerNotesDefaultLimit + } + if limit > adminCustomerNotesMaxLimit { + limit = adminCustomerNotesMaxLimit + } + rows, err := db.QueryContext(ctx, ` + SELECT id, team_id, body, author_email, created_at + FROM admin_customer_notes + WHERE team_id = $1 + ORDER BY created_at DESC + LIMIT $2 + `, teamID, limit) + if err != nil { + return nil, fmt.Errorf("models.ListAdminCustomerNotes: %w", err) + } + defer rows.Close() + + out := make([]*AdminCustomerNote, 0) + for rows.Next() { + n := &AdminCustomerNote{} + if err := rows.Scan(&n.ID, &n.TeamID, &n.Body, &n.AuthorEmail, &n.CreatedAt); err != nil { + return nil, fmt.Errorf("models.ListAdminCustomerNotes scan: %w", err) + } + out = append(out, n) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("models.ListAdminCustomerNotes rows: %w", err) + } + return out, nil +} + +// DeleteAdminCustomerNote hard-deletes one note by id. Returns +// ErrAdminCustomerNoteNotFound when no row matched — distinct sentinel so +// the handler can map cleanly to 404. Soft-delete was considered and +// rejected: notes are reversible by re-typing, so the column + +// always-filter overhead buys nothing. +func DeleteAdminCustomerNote(ctx context.Context, db *sql.DB, noteID uuid.UUID) error { + res, err := db.ExecContext(ctx, ` + DELETE FROM admin_customer_notes WHERE id = $1 + `, noteID) + if err != nil { + return fmt.Errorf("models.DeleteAdminCustomerNote: %w", err) + } + n, err := res.RowsAffected() + if err != nil { + return fmt.Errorf("models.DeleteAdminCustomerNote rows_affected: %w", err) + } + if n == 0 { + return ErrAdminCustomerNoteNotFound + } + return nil +} + +// adminCustomerNotesDefaultLimit / adminCustomerNotesMaxLimit cap the +// ListAdminCustomerNotes query. Per-team note volume is expected to stay +// in the dozens; if a team ever has 200+ notes the operator should switch +// to the audit log instead. +const ( + adminCustomerNotesDefaultLimit = 50 + adminCustomerNotesMaxLimit = 200 +) diff --git a/internal/router/router.go b/internal/router/router.go index 9f397b25..14178285 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -216,12 +216,22 @@ func New(cfg *config.Config, db *sql.DB, rdb *redis.Client, geoDbs *middleware.G // Provisioning — Phase 2+ (gated by IsServiceEnabled in each handler) // OptionalAuth is registered per-route rather than via app.Group("/", ...) to avoid // accidentally applying it globally to all routes (Fiber's "/" group prefix matches everything). - app.Post("/db/new", middleware.OptionalAuth(cfg), dbH.NewDB) - app.Post("/cache/new", middleware.OptionalAuth(cfg), cacheH.NewCache) - app.Post("/nosql/new", middleware.OptionalAuth(cfg), nosqlH.NewNoSQL) - app.Post("/queue/new", middleware.OptionalAuth(cfg), queueH.NewQueue) - app.Post("/storage/new", middleware.OptionalAuth(cfg), storageH.NewStorage) - app.Post("/webhook/new", middleware.OptionalAuth(cfg), webhookH.NewWebhook) + // + // RequireWritable runs AFTER OptionalAuth on every mutating provisioning + // endpoint so an impersonated (read-only) session presenting an + // Authorization header is 403'd before the handler runs. Anonymous (no + // header) callers fall through — OptionalAuth never sets the read_only + // local, and RequireWritable is a no-op for unset locals. The same + // invariant covers /webhook/receive/:token: that route never reads + // Authorization headers in practice, but installing the gate keeps + // the policy uniform — see test #5 in PR #024 for the explicit + // "POST /db/new under an impersonated session must 403" assertion. + app.Post("/db/new", middleware.OptionalAuth(cfg), middleware.RequireWritable(), dbH.NewDB) + app.Post("/cache/new", middleware.OptionalAuth(cfg), middleware.RequireWritable(), cacheH.NewCache) + app.Post("/nosql/new", middleware.OptionalAuth(cfg), middleware.RequireWritable(), nosqlH.NewNoSQL) + app.Post("/queue/new", middleware.OptionalAuth(cfg), middleware.RequireWritable(), queueH.NewQueue) + app.Post("/storage/new", middleware.OptionalAuth(cfg), middleware.RequireWritable(), storageH.NewStorage) + app.Post("/webhook/new", middleware.OptionalAuth(cfg), middleware.RequireWritable(), webhookH.NewWebhook) app.Post("/webhook/receive/:token", webhookH.Receive) app.Get("/resources/:token/logs", logsH.ResourceLogs) @@ -230,7 +240,11 @@ func New(cfg *config.Config, db *sql.DB, rdb *redis.Client, geoDbs *middleware.G // env scope arrives as a multipart form field (not JSON or query), so // we provide a custom env-lookup that reads c.FormValue("env") and // falls back to "production" for the policy check. - deployGroup := app.Group("/deploy", middleware.RequireAuth(cfg), middleware.PopulateTeamRole()) + // RequireWritable on the deploy group rejects impersonated sessions + // before any mutating deploy handler runs. GETs (deployGroup.Get) are + // no-ops under the middleware so the impersonated admin can still + // inspect deploy state — which is the entire point of view-as-customer. + deployGroup := app.Group("/deploy", middleware.RequireAuth(cfg), middleware.PopulateTeamRole(), middleware.RequireWritable()) deployGroup.Post("/new", middleware.RequireEnvAccess(middleware.EnvPolicyActionDeploy, middleware.WithEnvLookup(func(c *fiber.Ctx) (string, error) { @@ -251,12 +265,15 @@ func New(cfg *config.Config, db *sql.DB, rdb *redis.Client, geoDbs *middleware.G // Stacks — Phase 6 multi-service. // New/Get/Logs/Delete use OptionalAuth (anonymous stacks supported, same as /db/new etc.). // UpdateEnv/Redeploy require auth (mutations on owned stacks). - app.Post("/stacks/new", middleware.OptionalAuth(cfg), stackH.New) + // RequireWritable rejects impersonated sessions on all mutating + // stack endpoints (POST/PATCH/DELETE) so an admin viewing the + // customer's stack page can't accidentally redeploy / nuke it. + app.Post("/stacks/new", middleware.OptionalAuth(cfg), middleware.RequireWritable(), stackH.New) app.Get("/stacks/:slug", middleware.OptionalAuth(cfg), stackH.Get) app.Get("/stacks/:slug/logs/:svc", middleware.OptionalAuth(cfg), stackH.Logs) - app.Delete("/stacks/:slug", middleware.OptionalAuth(cfg), stackH.Delete) - app.Patch("/stacks/:slug/env", middleware.RequireAuth(cfg), stackH.UpdateEnv) - app.Post("/stacks/:slug/redeploy", middleware.RequireAuth(cfg), stackH.Redeploy) + app.Delete("/stacks/:slug", middleware.OptionalAuth(cfg), middleware.RequireWritable(), stackH.Delete) + app.Patch("/stacks/:slug/env", middleware.RequireAuth(cfg), middleware.RequireWritable(), stackH.UpdateEnv) + app.Post("/stacks/:slug/redeploy", middleware.RequireAuth(cfg), middleware.RequireWritable(), stackH.Redeploy) // OAuth — POST handler serves the existing programmatic / SPA flow. // Google login is intentionally NOT supported; if you need it, register @@ -285,7 +302,11 @@ func New(cfg *config.Config, db *sql.DB, rdb *redis.Client, geoDbs *middleware.G billing := handlers.NewBillingHandler(db, cfg, emailClient) // Legacy alias kept for backward compatibility; canonical path is // /api/v1/billing/checkout (registered under the /api/v1 group below). - app.Post("/billing/checkout", middleware.RequireAuth(cfg), billing.CreateCheckoutAPI) + // RequireWritable rejects impersonated sessions — an admin viewing-as- + // customer must not be able to start a checkout on the customer's + // behalf. The canonical /api/v1 alias is already gated by the api + // group's RequireWritable. + app.Post("/billing/checkout", middleware.RequireAuth(cfg), middleware.RequireWritable(), billing.CreateCheckoutAPI) app.Post("/razorpay/webhook", billing.RazorpayWebhook) // §10.20 cached-aggregation endpoints. Separate handlers from BillingHandler @@ -321,7 +342,18 @@ func New(cfg *config.Config, db *sql.DB, rdb *redis.Client, geoDbs *middleware.G middleware.SetRoleLookupDB(db) // populate auth_team_role on every RequireAuth middleware.SetAPIKeyDB(db) // enable PAT auth path in RequireAuth middleware.SetEnvPolicyDB(db) // RequireEnvAccess reads teams.env_policy - api := app.Group("/api/v1", middleware.RequireAuth(cfg), middleware.PopulateTeamRole()) + // RequireWritable gates every mutating route under /api/v1/* against + // the read_only JWT flag minted by the admin-impersonation endpoint + // (POST /api/v1/admin/customers/:team_id/impersonate). GET/HEAD/OPTIONS + // fall through unconditionally — the impersonated admin's whole reason + // for holding the token is to *read* the customer's dashboard state. + // + // One deliberate exemption: the impersonation-mint endpoint itself + // (registered below inside the admin group). It is called by an admin + // holding a *normal* (writable) session, so the gate would never fire + // there — but the brief calls out the exemption explicitly, and the + // audit-comment in router.go is where reviewers expect to find it. + api := app.Group("/api/v1", middleware.RequireAuth(cfg), middleware.PopulateTeamRole(), middleware.RequireWritable()) // /whoami — identity probe for agents. Returning 401 here is the canonical // "your token is bad"; returning anything else from this endpoint means @@ -483,32 +515,13 @@ func New(cfg *config.Config, db *sql.DB, rdb *redis.Client, geoDbs *middleware.G portal := &razorpaybilling.Portal{DB: db, Cfg: cfg} return portal.CancelImmediately(subID) } - // Defense-in-depth gates 3-5, chained in strict order: - // - // AdminRateLimit — 30 req/min/fingerprint cap. Returns 403 with - // a body byte-for-byte identical to the - // allowlist-miss 403. Runs BEFORE RequireAdmin - // so an attacker who knows the prefix cannot - // bypass the limiter by sending invalid JWTs. - // AdminAuditEmit — after-response middleware. Internally calls - // c.Next() and observes the FINAL status; logs - // EVERY hit on the prefix (success AND 403). - // PathSuffix is the URL with the prefix - // stripped — the secret prefix MUST NOT land - // in audit_log. - // RequireAdmin — ADMIN_EMAILS allowlist check (gate 2). - // - // Audit MUST sit BEFORE RequireAdmin in the chain. RequireAdmin - // returns a 403 directly (no c.Next call) on rejection — any - // middleware sitting AFTER it would never run on the rejection - // path, so the brute-force-visibility property would silently - // break. By sitting BEFORE, the audit middleware's internal - // c.Next() dispatches RequireAdmin → handler and observes the - // final status either way. - // - // AdminRateLimit stays first: it short-circuits on excess with - // its own 403 (which AdminAuditEmit's c.Next observes as 403 + - // IsAdminRateLimited(c)=true → denied_by=rate_limit on the row). + adminNotesH := handlers.NewAdminCustomerNotesHandler(db) + adminImpersonateH := handlers.NewAdminImpersonateHandler(db, cfg) + + // Defense-in-depth gates 3-5: AdminRateLimit → AdminAuditEmit → RequireAdmin. + // Audit MUST sit BEFORE RequireAdmin so brute-force probes still get logged + // on rejection (RequireAdmin returns 403 without c.Next). RateLimit first so + // invalid-JWT spam can't bypass the limiter. See PR #58 for full rationale. adminGroup := api.Group("/"+cfg.AdminPathPrefix, middleware.AdminRateLimit(rdb), middleware.AdminAuditEmit(db, cfg.AdminPathPrefix), @@ -519,18 +532,21 @@ func New(cfg *config.Config, db *sql.DB, rdb *redis.Client, geoDbs *middleware.G adminGroup.Post("/customers/:team_id/tier", adminCustH.ChangeTier) adminGroup.Post("/customers/:team_id/promo", adminCustH.IssuePromo) - // Promo lifecycle audit feed. /audit is uncached (admin needs to see - // "issued at 3 sec ago"); /stats is Redis-cached 5 min (the totals tile - // the dashboard polls). See handlers/admin_promos_audit.go for the - // freshness contract. + // Notes — free-text per-team admin annotations. + adminGroup.Get("/customers/:team_id/notes", adminNotesH.ListNotes) + adminGroup.Post("/customers/:team_id/notes", adminNotesH.CreateNote) + adminGroup.Delete("/notes/:note_id", adminNotesH.DeleteNote) + + // Impersonation — mint a 10-minute read-only JWT for the target team. + // RequireWritable on the /api/v1 group gates mutations on the read_only claim. + adminGroup.Post("/customers/:team_id/impersonate", adminImpersonateH.Impersonate) + + // Promo lifecycle audit feed (PR #59). /audit uncached; /stats Redis-cached 5 min. adminPromosH := handlers.NewAdminPromosAuditHandler(db, rdb) adminGroup.Get("/promos/audit", adminPromosH.Audit) adminGroup.Get("/promos/stats", adminPromosH.Stats) - // GET /api/v1//deploys — append-only deploy-identity log. - // Answers "which binary was serving traffic at $TIME?" — see - // internal/handlers/deploys_audit.go and migration 022 for the - // table shape and self-report contract. + // Deploy-identity append-only log (PR #57). Answers "which binary at $TIME?" deploysAuditH := handlers.NewDeploysAuditHandler(db) adminGroup.Get("/deploys", deploysAuditH.List) } diff --git a/internal/testhelpers/testhelpers.go b/internal/testhelpers/testhelpers.go index bb76cb5d..dba50ee1 100644 --- a/internal/testhelpers/testhelpers.go +++ b/internal/testhelpers/testhelpers.go @@ -280,11 +280,17 @@ func runMigrations(t *testing.T, db *sql.DB) { )`, `CREATE INDEX IF NOT EXISTS idx_admin_promo_codes_code ON admin_promo_codes(code) WHERE used_at IS NULL`, `CREATE INDEX IF NOT EXISTS idx_admin_promo_codes_team ON admin_promo_codes(team_id)`, - // 022_deploys_audit — append-only deploy-identity log. Mirrored here - // so handler tests bringing up a fresh test DB get the table without - // running the SQL migration separately. The unique index backs the - // self-report INSERT's ON CONFLICT clause; the service+time index - // supports the admin endpoint's default sort. + // 024_admin_customer_notes — free-text per-team notes by platform admins. + `CREATE TABLE IF NOT EXISTS admin_customer_notes ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + team_id UUID NOT NULL REFERENCES teams(id) ON DELETE CASCADE, + body TEXT NOT NULL, + author_email TEXT NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() + )`, + `CREATE INDEX IF NOT EXISTS idx_admin_customer_notes_team ON admin_customer_notes(team_id, created_at DESC)`, + // 022_deploys_audit — append-only deploy-identity log. Mirrored so + // handler tests get the table without running migrations separately. `CREATE TABLE IF NOT EXISTS deploys_audit ( id UUID PRIMARY KEY DEFAULT gen_random_uuid(), service TEXT NOT NULL,