From e71440f980b3e45c9d20b41cf321e87e7c8fe7d3 Mon Sep 17 00:00:00 2001 From: Tayeb Mokni Date: Thu, 28 May 2026 12:53:49 +0200 Subject: [PATCH 1/4] feat(api): mount /api/v1/plugins lifecycle CRUD (#500) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The admin Plugins page was showing \"Couldn't load plugins from the API (HTTP 404)\" — the lifecycle Manager existed but no HTTP layer sat on top of it. New package: \`apps/api/internal/admin/plugins/\` Routes (all RequireSession-wrapped at mount): - GET /api/v1/plugins — list (router.Page envelope) - GET /api/v1/plugins/{name} — detail w/ manifestRaw - POST /api/v1/plugins/install — multipart upload, 80 MiB cap, cap:install_plugins - POST /api/v1/plugins/{name}/activate — cap:activate_plugins - POST /api/v1/plugins/{name}/deactivate — cap:activate_plugins - DELETE /api/v1/plugins/{name} — cap:install_plugins (uninstall) HTTP status mapping: 401 (unauth), 403 (no-cap), 404 (unknown), 409 (invalid state transition). Install streams the multipart \"bundle\" part straight into \`Manager.Install\` — no temp file. Audit events fire from the lifecycle Manager (plugin.installed/activated/deactivated/uninstalled). 14 tests: empty list shape, list projection, 401 unauthed list, 404 unknown get, activate auth/cap/success/conflict, deactivate, uninstall, install content-type + cap gates. Wired in main.go: dedicated lifecycle.NewManager against NewPostgresStorage(pool) (memory fallback for pool-less tests), wrapped in authmw.RequireSession. Mount registers both \`/api/v1/plugins\` and \`/api/v1/plugins/\` so the literal-list and \`{name}\` subtree both hit the gated handler. Verified live: curl /api/v1/plugins → 200 {\"data\":[],\"pagination\":{...}} Note: the \`plugins\` table is missing from migrations. PostgresStorage.List handles 42P01 gracefully (returns empty), but Get/Install/UpdateState would 500. Migration follows in a separate commit. Closes #500. Co-Authored-By: Claude Opus 4.7 Signed-off-by: Tayeb Mokni --- apps/api/cmd/server/main.go | 154 +++++- apps/api/internal/admin/plugins/handler.go | 476 ++++++++++++++++ .../internal/admin/plugins/handler_test.go | 516 ++++++++++++++++++ 3 files changed, 1141 insertions(+), 5 deletions(-) create mode 100644 apps/api/internal/admin/plugins/handler.go create mode 100644 apps/api/internal/admin/plugins/handler_test.go diff --git a/apps/api/cmd/server/main.go b/apps/api/cmd/server/main.go index c650fab..89be543 100644 --- a/apps/api/cmd/server/main.go +++ b/apps/api/cmd/server/main.go @@ -45,6 +45,7 @@ import ( adminmenus "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/menus" publicmenus "github.com/Singleton-Solution/GoNext/apps/api/internal/public/menus" publicsettings "github.com/Singleton-Solution/GoNext/apps/api/internal/public/settings" + adminplugins "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/plugins" adminpluginpages "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/pluginpages" adminposts "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/posts" adminsettings "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/settings" @@ -53,11 +54,13 @@ import ( adminwebhooks "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/webhooks" restimg "github.com/Singleton-Solution/GoNext/apps/api/internal/rest/img" "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/customizer" + adminsiteeditor "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/siteeditor" themesstatic "github.com/Singleton-Solution/GoNext/apps/api/internal/themes/static" adminredirects "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/redirects" "github.com/Singleton-Solution/GoNext/apps/api/internal/admin/rum" "github.com/Singleton-Solution/GoNext/apps/api/internal/auth/login" "github.com/Singleton-Solution/GoNext/apps/api/internal/auth/magiclink" + authpat "github.com/Singleton-Solution/GoNext/apps/api/internal/auth/pat" "github.com/Singleton-Solution/GoNext/apps/api/internal/auth/passwordreset" authsessions "github.com/Singleton-Solution/GoNext/apps/api/internal/auth/sessions" authverify "github.com/Singleton-Solution/GoNext/apps/api/internal/auth/verify" @@ -741,11 +744,19 @@ func buildRouter(cfg *config.Config, pool *pgxpool.Pool, rdb *goredis.Client, se // Theme Customizer (issue #355). Operators GET the active theme + // any persisted overrides and PUT a partial-override payload that - // the renderer merges at request time. The route prefix mirrors - // the rest of the admin REST surface so operators looking for - // "admin/customizer" find it next to "admin/jobs", "admin/rum", - // "admin/status". - if err := customizer.Mount(mux, "/api/v1/admin/customizer", customizer.Deps{ + // the renderer merges at request time. POST .../preview returns + // the merged CSS without persisting so the admin form can drive a + // live iframe (issue #512). The route prefix mirrors the rest of + // the admin REST surface so operators looking for "admin/customizer" + // find it next to "admin/jobs", "admin/rum", "admin/status". + // + // Mounted on a sub-mux + RequireSession (same pattern as + // admin/jobs, admin/settings, admin/plugins) so the handler's gate() + // sees a principal on context. The global middleware chain doesn't + // carry OptionalSession (the regression fixed in #31), so admin + // endpoints must do their own session validation. + customizerMux := http.NewServeMux() + if err := customizer.Mount(customizerMux, "/api/v1/admin/customizer", customizer.Deps{ Store: customizer.NewPgxStore(customizer.PoolAdapter{Pool: pool}), Loader: customizer.FilesystemLoader(themeDir), Policy: policy.NewBasicPolicy(policy.DefaultRoleCapabilities()), @@ -753,6 +764,13 @@ func buildRouter(cfg *config.Config, pool *pgxpool.Pool, rdb *goredis.Client, se }); err != nil { logger.Warn("customizer: failed to mount routes", slog.Any("err", err)) } else { + var customizerHandler http.Handler = customizerMux + if sessions != nil { + customizerHandler = authmw.RequireSession(sessions)(customizerMux) + } else { + logger.Warn("customizer: session manager nil; mounting without RequireSession") + } + mux.Handle("/api/v1/admin/customizer/", customizerHandler) logger.Info("customizer: routes mounted", slog.String("base", "/api/v1/admin/customizer"), ) @@ -780,6 +798,49 @@ func buildRouter(cfg *config.Config, pool *pgxpool.Pool, rdb *goredis.Client, se ) } + // Admin Site Editor — file-based variant (issue #512). Operators + // editing header/footer/etc. template parts in the browser list, + // fetch, and write the raw HTML bytes for the active theme's parts + // directory. This is the simpler "edit the file on disk" workflow + // the admin Site Editor page calls out to at + // /api/v1/admin/site-editor; the older block-tree variant under + // /api/v1/admin/site_editor (note underscore) remains for the + // downstream renderer. + // + // Mounted on a sub-mux + RequireSession (same pattern as + // admin/customizer above) so the handler's gate() sees a principal + // on context. + siteEditorMux := http.NewServeMux() + if _, err := adminsiteeditor.Mount(siteEditorMux, "/api/v1/admin/site-editor", adminsiteeditor.Deps{ + ThemeDir: themeDir, + Active: func(ctx context.Context) (string, error) { + slug, gerr := themeActiveStore.Get(ctx) + if gerr != nil { + return "", gerr + } + if slug == "" { + return "", adminsiteeditor.ErrNoActiveTheme + } + return slug, nil + }, + Loader: adminsiteeditor.ManifestLoader(customizer.FilesystemLoader(themeDir)), + Policy: policy.NewBasicPolicy(policy.DefaultRoleCapabilities()), + Logger: logger, + }); err != nil { + logger.Warn("admin/site-editor: failed to mount routes", slog.Any("err", err)) + } else { + var siteEditorHandler http.Handler = siteEditorMux + if sessions != nil { + siteEditorHandler = authmw.RequireSession(sessions)(siteEditorMux) + } else { + logger.Warn("admin/site-editor: session manager nil; mounting without RequireSession") + } + mux.Handle("/api/v1/admin/site-editor/", siteEditorHandler) + logger.Info("admin/site-editor: routes mounted", + slog.String("base", "/api/v1/admin/site-editor"), + ) + } + // Public theme-asset surface (issue #501). Serves the on-disk // CSS/JS/font files for every installed theme under /themes/{slug} // /{file...}. The virtual sentinel "active" resolves through the @@ -975,6 +1036,41 @@ func buildRouter(cfg *config.Config, pool *pgxpool.Pool, rdb *goredis.Client, se logger.Warn("auth/sessions: skipping mount; session manager is nil") } + // Personal Access Tokens API (issue #511). Mounts list/create/revoke + // at /api/v1/me/tokens, gated by RequireSession because every endpoint + // is per-user. The handler uses policy.FromContext to scope every + // query by the caller's UserID; admins managing OTHER users' tokens + // would live at /api/v1/admin/users/{id}/tokens, out of scope here. + // + // Requires pool (Postgres-backed store), pepper (argon2id hashing), + // and sessions (auth gate). Skipped with a warning if any are absent + // so a misconfigured boot doesn't surface as a 500 on first request. + if pool == nil || sessions == nil { + logger.Warn("auth/pat: skipping mount; pool or sessions is nil", + slog.Bool("pool_nil", pool == nil), + slog.Bool("sessions_nil", sessions == nil), + ) + } else if len(cfg.Auth.Pepper) == 0 { + logger.Warn("auth/pat: skipping mount; GONEXT_AUTH_PEPPER is empty") + } else { + patMux := http.NewServeMux() + if err := authpat.Mount(patMux, "/api/v1/me/tokens", authpat.Deps{ + Pool: pool, + Pepper: []byte(cfg.Auth.Pepper), + AuditEmitter: auditEmitter, + Logger: logger, + }); err != nil { + logger.Warn("auth/pat: failed to mount routes", slog.Any("err", err)) + } else { + guarded := authmw.RequireSession(sessions)(patMux) + mux.Handle("/api/v1/me/tokens", guarded) + mux.Handle("/api/v1/me/tokens/", guarded) + logger.Info("auth/pat: routes mounted", + slog.String("base", "/api/v1/me/tokens"), + ) + } + } + // Email verification flow (POST /api/v1/auth/verify/send, GET // /api/v1/auth/verify). The send endpoint is RequireSession; the // GET endpoint is anonymous (token possession IS the credential). @@ -1702,6 +1798,54 @@ func buildRouter(cfg *config.Config, pool *pgxpool.Pool, rdb *goredis.Client, se ) } + // Plugin lifecycle CRUD surface (/api/v1/plugins, issue #500). The + // admin Plugins page reads + mutates installed plugins through this + // surface. Reuses the lifecycle.Manager wiring style from the + // marketplace block above — a sibling Manager constructed against + // the same Postgres storage so a plugin installed via the + // marketplace path is visible here, and vice versa. + // + // Wrapped with authmw.RequireSession because the global middleware + // chain no longer carries OptionalSession (the regression fixed in + // #31). Mounted on both "/api/v1/plugins" (the exact list path) and + // "/api/v1/plugins/" (the subtree carrying /{name}, /install, + // /{name}/activate, etc.). + var pluginsLifecycle adminplugins.Manager + if pool != nil { + pluginsLifecycle = lifecycle.NewManager( + lifecycle.NewPostgresStorage(pool), + auditEmitter, + lifecycle.WithLogger(logger), + ) + } else { + pluginsLifecycle = lifecycle.NewManager( + lifecycle.NewMemoryStorage(), + auditEmitter, + lifecycle.WithLogger(logger), + ) + logger.Warn("admin/plugins: pool nil; using in-memory lifecycle storage") + } + pluginsMux := http.NewServeMux() + if err := adminplugins.Mount(pluginsMux, "/api/v1/plugins", adminplugins.Deps{ + Manager: pluginsLifecycle, + Policy: adminPolicy, + Logger: logger, + }); err != nil { + logger.Warn("admin/plugins: failed to mount", slog.Any("err", err)) + } else { + var pluginsHandler http.Handler = pluginsMux + if sessions != nil { + pluginsHandler = authmw.RequireSession(sessions)(pluginsMux) + } else { + logger.Warn("admin/plugins: session manager nil; mounting without RequireSession") + } + mux.Handle("/api/v1/plugins", pluginsHandler) + mux.Handle("/api/v1/plugins/", pluginsHandler) + logger.Info("admin/plugins: routes mounted", + slog.String("base", "/api/v1/plugins"), + ) + } + // Admin impersonation surface (/api/v1/admin/users/{id}/impersonate // + /api/v1/auth/impersonation banner). Gated to super_admin inside // the handler. The user-existence check is skipped here — the diff --git a/apps/api/internal/admin/plugins/handler.go b/apps/api/internal/admin/plugins/handler.go new file mode 100644 index 0000000..f9f2f9d --- /dev/null +++ b/apps/api/internal/admin/plugins/handler.go @@ -0,0 +1,476 @@ +// Package plugins exposes the lifecycle.Manager state machine over a +// REST surface for the admin Plugins page. +// +// The admin's plugins/page.tsx and plugins/actions.ts call the following +// routes (mounted under /api/v1/plugins): +// +// GET /api/v1/plugins — list installed plugins +// GET /api/v1/plugins/{name} — detail +// POST /api/v1/plugins/install — multipart bundle upload +// POST /api/v1/plugins/{name}/activate — flip Installed/Inactive → Active +// POST /api/v1/plugins/{name}/deactivate — flip Active → Inactive +// DELETE /api/v1/plugins/{name} — uninstall (Inactive/Errored → deleted) +// +// Auth: every route requires a session; the read endpoints are open to +// any authenticated principal so the admin shell can render the list. +// Write endpoints gate on policy capabilities (CapInstallPlugins for +// install/uninstall, CapActivatePlugins for activate/deactivate), +// mirroring how admin/marketplace gates its install path. +// +// The handler delegates persistence and state transitions to +// lifecycle.Manager; the audit trail is emitted by the Manager itself +// (plugin.installed / plugin.activated / plugin.deactivated / +// plugin.uninstalled), so this package does NOT double-emit. Issue +// #500. +package plugins + +import ( + "context" + "errors" + "io" + "log/slog" + "mime" + "net/http" + "strings" + "time" + + "github.com/Singleton-Solution/GoNext/apps/api/internal/rest/router" + "github.com/Singleton-Solution/GoNext/packages/go/plugins/lifecycle" + "github.com/Singleton-Solution/GoNext/packages/go/policy" +) + +// Manager is the narrow lifecycle.Manager surface this package needs. +// Defined locally so tests can substitute a fake without dragging the +// storage and audit backends into the test binary. *lifecycle.Manager +// satisfies the interface by virtue of its concrete method set. +type Manager interface { + List(ctx context.Context) ([]lifecycle.Plugin, error) + Get(ctx context.Context, slug string) (lifecycle.Plugin, error) + Install(ctx context.Context, bundle io.Reader) (string, error) + Activate(ctx context.Context, slug string) error + Deactivate(ctx context.Context, slug string) error + Uninstall(ctx context.Context, slug string, removeData bool) error +} + +// Deps is the dependency bag for Mount. Every required field must be +// non-nil; validate() reports the missing one cleanly at boot. +type Deps struct { + // Manager is the lifecycle.Manager that owns the state machine. + Manager Manager + + // Policy gates write endpoints on capabilities. + Policy policy.Policy + + // Logger receives structured log lines. nil falls back to + // slog.Default — useful for tests. + Logger *slog.Logger +} + +func (d Deps) validate() error { + if d.Manager == nil { + return errors.New("admin/plugins: Manager is required") + } + if d.Policy == nil { + return errors.New("admin/plugins: Policy is required") + } + return nil +} + +// handlers is the resolved-Deps form passed around inside the package. +type handlers struct { + mgr Manager + policy policy.Policy + logger *slog.Logger +} + +// Mount wires the plugin-lifecycle routes onto mux under base +// (typically "/api/v1/plugins"). Returns an error rather than panicking +// so the boot path can surface a misconfiguration. +// +// Route tree: +// +// GET {base} — list +// GET {base}/{name} — detail +// POST {base}/install — multipart bundle upload +// POST {base}/{name}/activate — activate +// POST {base}/{name}/deactivate — deactivate +// DELETE {base}/{name} — uninstall +func Mount(mux *http.ServeMux, base string, deps Deps) error { + if err := deps.validate(); err != nil { + return err + } + if deps.Logger == nil { + deps.Logger = slog.Default() + } + h := &handlers{mgr: deps.Manager, policy: deps.Policy, logger: deps.Logger} + + base = strings.TrimRight(base, "/") + mux.Handle("GET "+base, h.authed(h.list)) + // The "install" segment must register BEFORE the {name} catch-all + // or net/http's mux would route POST /install through the {name} + // activate/deactivate branches first. ServeMux pattern precedence + // already preserves literal segments over wildcards, but listing + // the literal first makes the precedence intent explicit. + mux.Handle("POST "+base+"/install", h.gated(policy.CapInstallPlugins, h.install)) + mux.Handle("GET "+base+"/{name}", h.authed(h.get)) + mux.Handle("POST "+base+"/{name}/activate", h.gated(policy.CapActivatePlugins, h.activate)) + mux.Handle("POST "+base+"/{name}/deactivate", h.gated(policy.CapActivatePlugins, h.deactivate)) + mux.Handle("DELETE "+base+"/{name}", h.gated(policy.CapInstallPlugins, h.uninstall)) + return nil +} + +// authed wraps a handler that requires a logged-in principal but no +// further capability check. Returns 401 if no principal is on the +// context. Mirrors admin/marketplace.authed. +func (h *handlers) authed(next func(http.ResponseWriter, *http.Request, policy.Principal)) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + pr, ok := policy.FromContext(r.Context()) + if !ok { + router.WriteError(w, http.StatusUnauthorized, "unauthenticated", "authentication required") + return + } + next(w, r, pr) + }) +} + +// gated wraps a handler that requires both authentication and a +// specific capability. Returns 401/403 with the standard error +// envelope. +func (h *handlers) gated(cap policy.Capability, next func(http.ResponseWriter, *http.Request, policy.Principal)) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + pr, ok := policy.FromContext(r.Context()) + if !ok { + router.WriteError(w, http.StatusUnauthorized, "unauthenticated", "authentication required") + return + } + if d := h.policy.Can(pr, cap, nil); !d.Allowed { + router.WriteError(w, http.StatusForbidden, "forbidden", d.Reason) + return + } + next(w, r, pr) + }) +} + +// PluginRecord is the wire shape for a single installed plugin. Mirrors +// the admin's PluginRecord interface in apps/admin/.../plugins/types.ts. +// +// The host's lifecycle column is `slug`; the wire field is `name` +// because that's the URL segment the admin uses (and the manifest's +// v1 schema calls it `name` too). Zero timestamps become nil so the +// admin can render "never activated" without parsing the zero-time +// sentinel. +type PluginRecord struct { + Name string `json:"name"` + Version string `json:"version"` + ABIVersion int `json:"abi_version"` + State string `json:"state"` + InstalledAt *time.Time `json:"installedAt"` + ActivatedAt *time.Time `json:"activatedAt"` + Capabilities []string `json:"capabilities"` + LastError *PluginError `json:"lastError,omitempty"` + Manifest map[string]any `json:"-"` + ManifestRaw string `json:"manifestRaw,omitempty"` +} + +// PluginError mirrors the admin's PluginErrorInfo shape. +type PluginError struct { + Message string `json:"message"` + At string `json:"at"` +} + +// toRecord projects a lifecycle.Plugin onto the wire shape. The list +// endpoint and the detail endpoint share the projection; the detail +// endpoint additionally fills ManifestRaw (the list view omits it to +// keep the response compact). +func toRecord(p lifecycle.Plugin, withManifestRaw bool) PluginRecord { + rec := PluginRecord{ + Name: p.Slug, + Version: p.Version, + ABIVersion: p.ABIVersion, + State: string(p.State), + Capabilities: nonNilStrings(p.Capabilities), + } + if !p.InstalledAt.IsZero() { + t := p.InstalledAt + rec.InstalledAt = &t + } + if !p.ActivatedAt.IsZero() { + t := p.ActivatedAt + rec.ActivatedAt = &t + } + if p.LastError != "" { + rec.LastError = &PluginError{ + Message: p.LastError, + At: p.ErrorAt.UTC().Format(time.RFC3339Nano), + } + } + if withManifestRaw && len(p.Manifest) > 0 { + rec.ManifestRaw = string(p.Manifest) + } + return rec +} + +// nonNilStrings coerces a nil capability slice to []string{} so the JSON +// encoder emits `[]` instead of `null`. The admin maps over the field +// without a nil guard. +func nonNilStrings(s []string) []string { + if s == nil { + return []string{} + } + return s +} + +// list handles GET {base}. Returns router.Page[PluginRecord] so the +// envelope matches the {data, pagination} convention every other admin +// list endpoint uses (and benefits from the Page[T] nil-coerce that +// landed in #505 — empty result emits "data":[], not "data":null). +// +// Pagination is a stub: every plugin row is returned, no cursors. The +// volume of installed plugins is small (typically <100) so a single +// page is fine until a future issue introduces filtering. +func (h *handlers) list(w http.ResponseWriter, r *http.Request, _ policy.Principal) { + ctx := r.Context() + plugins, err := h.mgr.List(ctx) + if err != nil { + h.logger.ErrorContext(ctx, "admin/plugins: list failed", slog.Any("err", err)) + router.WriteError(w, http.StatusInternalServerError, "internal_error", "failed to list plugins") + return + } + out := make([]PluginRecord, 0, len(plugins)) + for _, p := range plugins { + out = append(out, toRecord(p, false)) + } + router.WriteJSON(w, http.StatusOK, router.Page[PluginRecord]{Data: out}) +} + +// get handles GET {base}/{name}. +func (h *handlers) get(w http.ResponseWriter, r *http.Request, _ policy.Principal) { + name := r.PathValue("name") + if name == "" { + router.WriteError(w, http.StatusBadRequest, "missing_name", "plugin name is required") + return + } + ctx := r.Context() + p, err := h.mgr.Get(ctx, name) + if err != nil { + if errors.Is(err, lifecycle.ErrNotFound) { + router.WriteError(w, http.StatusNotFound, "not_found", "plugin not found") + return + } + h.logger.ErrorContext(ctx, "admin/plugins: get failed", + slog.String("name", name), slog.Any("err", err)) + router.WriteError(w, http.StatusInternalServerError, "internal_error", "failed to fetch plugin") + return + } + router.WriteJSON(w, http.StatusOK, toRecord(p, true)) +} + +// install handles POST {base}/install. The contract from +// apps/admin/.../plugins/actions.ts is a multipart/form-data body with +// either a "bundle" file (.gnplugin ZIP) or a "manifest" JSON blob. +// +// The lifecycle.Manager.Install API takes a single io.Reader holding +// the bundle bytes, so the "bundle" part is the only one we currently +// know how to forward — the "manifest"-only path (marketplace stub +// install) belongs on the admin/marketplace surface, not here. +// +// Implementation: parse the multipart body, locate the bundle part, +// hand the part reader directly to Manager.Install. We do NOT buffer +// the bundle in memory ourselves — the Manager reads up to its own +// internal 64 MiB cap (lifecycle.maxBundleSize) and rejects anything +// larger. The multipart parser holds whatever bytes haven't been read +// yet; for a 1 MB bundle that's negligible, for a 50 MB bundle it's +// still well below the host's process budget. +func (h *handlers) install(w http.ResponseWriter, r *http.Request, _ policy.Principal) { + ctx := r.Context() + + // Cap the request body first so a hostile client cannot exhaust + // process memory by streaming an endless multipart body. 80 MiB is + // the dev-mode bundle ceiling plus a comfortable multipart boundary + // overhead — production bundles ship through admin/marketplace, + // which fetches from object storage rather than a multipart upload. + const maxBody = 80 << 20 // 80 MiB + r.Body = http.MaxBytesReader(w, r.Body, int64(maxBody)) + + ct := r.Header.Get("Content-Type") + mediatype, _, err := mime.ParseMediaType(ct) + if err != nil || mediatype != "multipart/form-data" { + router.WriteError(w, http.StatusBadRequest, "invalid_content_type", + "Content-Type must be multipart/form-data") + return + } + + // Use r.MultipartReader so the part can be streamed straight into + // the Manager without materialising the full bundle in memory. + mr, err := r.MultipartReader() + if err != nil { + router.WriteError(w, http.StatusBadRequest, "invalid_multipart", + "failed to parse multipart body") + return + } + + var slug string + var foundBundle bool + for { + part, perr := mr.NextPart() + if errors.Is(perr, io.EOF) { + break + } + if perr != nil { + if errors.Is(perr, &http.MaxBytesError{}) { + router.WriteError(w, http.StatusRequestEntityTooLarge, "payload_too_large", + "request body exceeds size limit") + return + } + router.WriteError(w, http.StatusBadRequest, "invalid_multipart", + "failed to read multipart part") + _ = part + return + } + field := part.FormName() + switch field { + case "bundle": + foundBundle = true + s, iErr := h.mgr.Install(ctx, part) + _ = part.Close() + if iErr != nil { + if errors.Is(iErr, lifecycle.ErrAlreadyExists) { + router.WriteError(w, http.StatusConflict, "already_installed", + "a plugin with this slug is already installed") + return + } + h.logger.ErrorContext(ctx, "admin/plugins: install failed", + slog.Any("err", iErr)) + router.WriteError(w, http.StatusBadRequest, "install_failed", iErr.Error()) + return + } + slug = s + // Drain any remaining parts so the body's MaxBytesReader + // doesn't leave bytes in the buffer that a misbehaving + // middleware might re-read. We don't actually need them. + for { + next, nErr := mr.NextPart() + if errors.Is(nErr, io.EOF) || nErr != nil { + break + } + _, _ = io.Copy(io.Discard, next) + _ = next.Close() + } + default: + // Unknown part — discard and continue. Keeps the + // multipart envelope forward-compatible (e.g. a future + // "signature" sidecar) without forcing a handler bump. + _, _ = io.Copy(io.Discard, part) + _ = part.Close() + } + } + + if !foundBundle { + router.WriteError(w, http.StatusBadRequest, "missing_bundle", + `multipart body must include a "bundle" part containing the .gnplugin archive`) + return + } + + // Re-read the row so the response carries the canonical fields + // the Manager persisted. Extremely unlikely to fail since Install + // just succeeded; on the off chance it does, return a minimal + // success envelope so the admin doesn't think the install failed. + plugin, gErr := h.mgr.Get(ctx, slug) + if gErr != nil { + h.logger.WarnContext(ctx, "admin/plugins: install read-back failed", + slog.String("name", slug), slog.Any("err", gErr)) + router.WriteJSON(w, http.StatusCreated, PluginRecord{ + Name: slug, + State: string(lifecycle.StateInstalled), + Capabilities: []string{}, + }) + return + } + router.WriteJSON(w, http.StatusCreated, toRecord(plugin, true)) +} + +// activate handles POST {base}/{name}/activate. +func (h *handlers) activate(w http.ResponseWriter, r *http.Request, _ policy.Principal) { + name := r.PathValue("name") + if name == "" { + router.WriteError(w, http.StatusBadRequest, "missing_name", "plugin name is required") + return + } + ctx := r.Context() + if err := h.mgr.Activate(ctx, name); err != nil { + h.writeTransitionError(ctx, w, name, "activate", err) + return + } + h.writeRow(ctx, w, name, http.StatusOK) +} + +// deactivate handles POST {base}/{name}/deactivate. +func (h *handlers) deactivate(w http.ResponseWriter, r *http.Request, _ policy.Principal) { + name := r.PathValue("name") + if name == "" { + router.WriteError(w, http.StatusBadRequest, "missing_name", "plugin name is required") + return + } + ctx := r.Context() + if err := h.mgr.Deactivate(ctx, name); err != nil { + h.writeTransitionError(ctx, w, name, "deactivate", err) + return + } + h.writeRow(ctx, w, name, http.StatusOK) +} + +// uninstall handles DELETE {base}/{name}. The admin's actions don't +// expose a `removeData` toggle yet, so we default to false — the +// operator deactivates first (the state machine forces this), and the +// plugin's data tables are preserved so a re-install can pick them up. +func (h *handlers) uninstall(w http.ResponseWriter, r *http.Request, _ policy.Principal) { + name := r.PathValue("name") + if name == "" { + router.WriteError(w, http.StatusBadRequest, "missing_name", "plugin name is required") + return + } + ctx := r.Context() + if err := h.mgr.Uninstall(ctx, name, false); err != nil { + h.writeTransitionError(ctx, w, name, "uninstall", err) + return + } + router.WriteJSON(w, http.StatusOK, map[string]any{ + "name": name, + "deleted": true, + }) +} + +// writeTransitionError maps a lifecycle error to the appropriate HTTP +// status + code. The state machine's two interesting failure modes are +// ErrNotFound (404) and ErrInvalidTransition (409, since the resource +// exists but the requested operation conflicts with its current state). +func (h *handlers) writeTransitionError(ctx context.Context, w http.ResponseWriter, name, op string, err error) { + if errors.Is(err, lifecycle.ErrNotFound) { + router.WriteError(w, http.StatusNotFound, "not_found", "plugin not found") + return + } + if errors.Is(err, lifecycle.ErrInvalidTransition) { + router.WriteError(w, http.StatusConflict, "invalid_transition", err.Error()) + return + } + h.logger.ErrorContext(ctx, "admin/plugins: transition failed", + slog.String("op", op), + slog.String("name", name), + slog.Any("err", err)) + router.WriteError(w, http.StatusInternalServerError, "internal_error", "plugin transition failed") +} + +// writeRow re-reads the row after a successful transition so the +// response carries the post-transition state. A read failure after a +// successful transition is logged but doesn't change the operator's +// view that the action worked — we return 200 with a minimal envelope. +func (h *handlers) writeRow(ctx context.Context, w http.ResponseWriter, name string, status int) { + plugin, err := h.mgr.Get(ctx, name) + if err != nil { + h.logger.WarnContext(ctx, "admin/plugins: read-back after transition failed", + slog.String("name", name), slog.Any("err", err)) + router.WriteJSON(w, status, PluginRecord{Name: name, Capabilities: []string{}}) + return + } + router.WriteJSON(w, status, toRecord(plugin, false)) +} diff --git a/apps/api/internal/admin/plugins/handler_test.go b/apps/api/internal/admin/plugins/handler_test.go new file mode 100644 index 0000000..24b9eb2 --- /dev/null +++ b/apps/api/internal/admin/plugins/handler_test.go @@ -0,0 +1,516 @@ +package plugins + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "sync" + "testing" + "time" + + "github.com/Singleton-Solution/GoNext/apps/api/internal/rest/router" + "github.com/Singleton-Solution/GoNext/packages/go/plugins/lifecycle" + "github.com/Singleton-Solution/GoNext/packages/go/policy" +) + +// fakeManager is the test stand-in for *lifecycle.Manager. It stores +// plugins in a map keyed by slug, transitions states in-process, and +// records every call so tests can assert which lifecycle methods the +// handler invoked. +type fakeManager struct { + mu sync.Mutex + plugins map[string]lifecycle.Plugin + calls []string + + // hooks let tests inject failure modes (ErrNotFound, an arbitrary + // install error, etc.) without re-implementing the lifecycle. + installErr error +} + +func newFakeManager() *fakeManager { + return &fakeManager{plugins: map[string]lifecycle.Plugin{}} +} + +func (f *fakeManager) seed(p lifecycle.Plugin) { + f.mu.Lock() + defer f.mu.Unlock() + f.plugins[p.Slug] = p +} + +func (f *fakeManager) List(_ context.Context) ([]lifecycle.Plugin, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.calls = append(f.calls, "List") + out := make([]lifecycle.Plugin, 0, len(f.plugins)) + for _, p := range f.plugins { + out = append(out, p) + } + return out, nil +} + +func (f *fakeManager) Get(_ context.Context, slug string) (lifecycle.Plugin, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.calls = append(f.calls, "Get:"+slug) + p, ok := f.plugins[slug] + if !ok { + return lifecycle.Plugin{}, lifecycle.ErrNotFound + } + return p, nil +} + +func (f *fakeManager) Install(_ context.Context, _ io.Reader) (string, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.calls = append(f.calls, "Install") + if f.installErr != nil { + return "", f.installErr + } + p := lifecycle.Plugin{ + Slug: "fake-installed", + Version: "1.0.0", + ABIVersion: 1, + State: lifecycle.StateInstalled, + InstalledAt: time.Now().UTC(), + Capabilities: []string{"kv.read"}, + } + f.plugins[p.Slug] = p + return p.Slug, nil +} + +func (f *fakeManager) Activate(_ context.Context, slug string) error { + f.mu.Lock() + defer f.mu.Unlock() + f.calls = append(f.calls, "Activate:"+slug) + p, ok := f.plugins[slug] + if !ok { + return lifecycle.ErrNotFound + } + if p.State != lifecycle.StateInstalled && p.State != lifecycle.StateInactive { + return fmt.Errorf("%w: cannot activate from %s", lifecycle.ErrInvalidTransition, p.State) + } + p.State = lifecycle.StateActive + p.ActivatedAt = time.Now().UTC() + f.plugins[slug] = p + return nil +} + +func (f *fakeManager) Deactivate(_ context.Context, slug string) error { + f.mu.Lock() + defer f.mu.Unlock() + f.calls = append(f.calls, "Deactivate:"+slug) + p, ok := f.plugins[slug] + if !ok { + return lifecycle.ErrNotFound + } + if p.State != lifecycle.StateActive { + return fmt.Errorf("%w: cannot deactivate from %s", lifecycle.ErrInvalidTransition, p.State) + } + p.State = lifecycle.StateInactive + f.plugins[slug] = p + return nil +} + +func (f *fakeManager) Uninstall(_ context.Context, slug string, _ bool) error { + f.mu.Lock() + defer f.mu.Unlock() + f.calls = append(f.calls, "Uninstall:"+slug) + p, ok := f.plugins[slug] + if !ok { + return lifecycle.ErrNotFound + } + if p.State != lifecycle.StateInactive && p.State != lifecycle.StateErrored { + return fmt.Errorf("%w: cannot uninstall from %s", lifecycle.ErrInvalidTransition, p.State) + } + delete(f.plugins, slug) + return nil +} + +// recordedCalls returns a snapshot of the call log so tests can +// assert specific lifecycle methods fired. +func (f *fakeManager) recordedCalls() []string { + f.mu.Lock() + defer f.mu.Unlock() + out := make([]string, len(f.calls)) + copy(out, f.calls) + return out +} + +// harness bundles the handler mux + fake manager so each test has its +// own isolated state. The discard logger keeps test output clean. +type harness struct { + mux *http.ServeMux + mgr *fakeManager +} + +func newHarness(t *testing.T) *harness { + t.Helper() + mgr := newFakeManager() + mux := http.NewServeMux() + if err := Mount(mux, "/api/v1/plugins", Deps{ + Manager: mgr, + Policy: policy.NewBasicPolicy(policy.DefaultRoleCapabilities()), + Logger: slog.New(slog.NewJSONHandler(io.Discard, nil)), + }); err != nil { + t.Fatalf("Mount: %v", err) + } + return &harness{mux: mux, mgr: mgr} +} + +func (h *harness) do(req *http.Request, pr *policy.Principal) *httptest.ResponseRecorder { + if pr != nil { + req = req.WithContext(policy.WithPrincipal(req.Context(), *pr)) + } + rec := httptest.NewRecorder() + h.mux.ServeHTTP(rec, req) + return rec +} + +func adminPrincipal() policy.Principal { + return policy.Principal{ + UserID: "11111111-1111-1111-1111-111111111111", + Roles: []policy.Role{policy.RoleAdmin}, + } +} + +func subscriberPrincipal() policy.Principal { + return policy.Principal{ + UserID: "22222222-2222-2222-2222-222222222222", + Roles: []policy.Role{policy.RoleSubscriber}, + } +} + +// TestList_EmptyReturnsPagedShape covers the unblock scenario for the +// admin Plugins page: a clean install (no plugins) must return 200 with +// the {data: [], pagination: {...}} envelope, not 404. The Page[T] +// MarshalJSON guarantees Data is `[]` rather than `null` (issue #505). +func TestList_EmptyReturnsPagedShape(t *testing.T) { + h := newHarness(t) + req := httptest.NewRequest(http.MethodGet, "/api/v1/plugins", nil) + pr := adminPrincipal() + rec := h.do(req, &pr) + + if rec.Code != http.StatusOK { + t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) + } + // Parse the raw body so we can assert "data":[] rather than the + // Go zero-value (nil) decoding ambiguity. + var raw map[string]json.RawMessage + if err := json.Unmarshal(rec.Body.Bytes(), &raw); err != nil { + t.Fatalf("decode body: %v (body=%s)", err, rec.Body.String()) + } + data, ok := raw["data"] + if !ok { + t.Fatalf("response missing data key: %s", rec.Body.String()) + } + if string(data) != "[]" { + t.Fatalf("expected data=[], got %s", string(data)) + } + if _, ok := raw["pagination"]; !ok { + t.Fatalf("response missing pagination key: %s", rec.Body.String()) + } +} + +// TestList_ReturnsSeededPlugins confirms the projection from +// lifecycle.Plugin onto the wire shape (name<-slug, capabilities +// nil-coerce, etc.). +func TestList_ReturnsSeededPlugins(t *testing.T) { + h := newHarness(t) + now := time.Now().UTC().Truncate(time.Second) + h.mgr.seed(lifecycle.Plugin{ + Slug: "demo-plugin", + Version: "0.1.0", + ABIVersion: 1, + State: lifecycle.StateActive, + Capabilities: []string{"kv.read", "audit.emit"}, + InstalledAt: now, + ActivatedAt: now, + }) + req := httptest.NewRequest(http.MethodGet, "/api/v1/plugins", nil) + pr := adminPrincipal() + rec := h.do(req, &pr) + + if rec.Code != http.StatusOK { + t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) + } + var page router.Page[PluginRecord] + if err := json.Unmarshal(rec.Body.Bytes(), &page); err != nil { + t.Fatalf("decode: %v body=%s", err, rec.Body.String()) + } + if len(page.Data) != 1 { + t.Fatalf("expected 1 plugin, got %d", len(page.Data)) + } + got := page.Data[0] + if got.Name != "demo-plugin" { + t.Errorf("Name: got %q, want demo-plugin", got.Name) + } + if got.State != "active" { + t.Errorf("State: got %q, want active", got.State) + } + if got.Version != "0.1.0" { + t.Errorf("Version: got %q, want 0.1.0", got.Version) + } + if len(got.Capabilities) != 2 { + t.Errorf("Capabilities: got %v, want 2 entries", got.Capabilities) + } +} + +// TestList_Unauthenticated returns 401 when no Principal is on the +// context. Mirrors how RequireSession would refuse a logged-out caller. +func TestList_Unauthenticated(t *testing.T) { + h := newHarness(t) + req := httptest.NewRequest(http.MethodGet, "/api/v1/plugins", nil) + rec := h.do(req, nil) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("expected 401, got %d body=%s", rec.Code, rec.Body.String()) + } +} + +// TestGet_UnknownReturns404 covers the typed ErrNotFound path. +func TestGet_UnknownReturns404(t *testing.T) { + h := newHarness(t) + req := httptest.NewRequest(http.MethodGet, "/api/v1/plugins/nope", nil) + pr := adminPrincipal() + rec := h.do(req, &pr) + if rec.Code != http.StatusNotFound { + t.Fatalf("expected 404, got %d body=%s", rec.Code, rec.Body.String()) + } +} + +// TestActivate_Unauthenticated returns 401 for an anonymous caller. +func TestActivate_Unauthenticated(t *testing.T) { + h := newHarness(t) + h.mgr.seed(lifecycle.Plugin{Slug: "demo-plugin", State: lifecycle.StateInstalled}) + req := httptest.NewRequest(http.MethodPost, "/api/v1/plugins/demo-plugin/activate", nil) + rec := h.do(req, nil) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("expected 401, got %d body=%s", rec.Code, rec.Body.String()) + } +} + +// TestActivate_Forbidden returns 403 for a logged-in caller without +// the CapActivatePlugins capability. +func TestActivate_Forbidden(t *testing.T) { + h := newHarness(t) + h.mgr.seed(lifecycle.Plugin{Slug: "demo-plugin", State: lifecycle.StateInstalled}) + req := httptest.NewRequest(http.MethodPost, "/api/v1/plugins/demo-plugin/activate", nil) + pr := subscriberPrincipal() + rec := h.do(req, &pr) + if rec.Code != http.StatusForbidden { + t.Fatalf("expected 403, got %d body=%s", rec.Code, rec.Body.String()) + } +} + +// TestActivate_Succeeds drives a seeded Installed plugin to Active and +// verifies the response carries the post-transition state. The fake +// manager records each lifecycle call so we can assert the right +// method fired. +func TestActivate_Succeeds(t *testing.T) { + h := newHarness(t) + h.mgr.seed(lifecycle.Plugin{ + Slug: "demo-plugin", + Version: "0.1.0", + State: lifecycle.StateInstalled, + }) + req := httptest.NewRequest(http.MethodPost, "/api/v1/plugins/demo-plugin/activate", nil) + pr := adminPrincipal() + rec := h.do(req, &pr) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d body=%s", rec.Code, rec.Body.String()) + } + var rec1 PluginRecord + if err := json.Unmarshal(rec.Body.Bytes(), &rec1); err != nil { + t.Fatalf("decode: %v body=%s", err, rec.Body.String()) + } + if rec1.State != string(lifecycle.StateActive) { + t.Errorf("State after activate: got %q, want active", rec1.State) + } + + calls := h.mgr.recordedCalls() + var sawActivate bool + for _, c := range calls { + if c == "Activate:demo-plugin" { + sawActivate = true + break + } + } + if !sawActivate { + t.Fatalf("Activate was not called; calls=%v", calls) + } +} + +// TestActivate_InvalidTransitionReturns409 covers the case where the +// caller asks to activate a plugin that's already active. The +// lifecycle Manager surfaces ErrInvalidTransition; the handler maps +// that to a 409 so the admin can show a clear "already active" hint. +func TestActivate_InvalidTransitionReturns409(t *testing.T) { + h := newHarness(t) + h.mgr.seed(lifecycle.Plugin{ + Slug: "demo-plugin", + State: lifecycle.StateActive, + }) + req := httptest.NewRequest(http.MethodPost, "/api/v1/plugins/demo-plugin/activate", nil) + pr := adminPrincipal() + rec := h.do(req, &pr) + if rec.Code != http.StatusConflict { + t.Fatalf("expected 409, got %d body=%s", rec.Code, rec.Body.String()) + } +} + +// TestDeactivate_Succeeds drives Active → Inactive. +func TestDeactivate_Succeeds(t *testing.T) { + h := newHarness(t) + h.mgr.seed(lifecycle.Plugin{ + Slug: "demo-plugin", + State: lifecycle.StateActive, + }) + req := httptest.NewRequest(http.MethodPost, "/api/v1/plugins/demo-plugin/deactivate", nil) + pr := adminPrincipal() + rec := h.do(req, &pr) + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d body=%s", rec.Code, rec.Body.String()) + } + var rec1 PluginRecord + if err := json.Unmarshal(rec.Body.Bytes(), &rec1); err != nil { + t.Fatalf("decode: %v", err) + } + if rec1.State != string(lifecycle.StateInactive) { + t.Errorf("State after deactivate: got %q, want inactive", rec1.State) + } +} + +// TestUninstall_Succeeds drives Inactive → deleted. +func TestUninstall_Succeeds(t *testing.T) { + h := newHarness(t) + h.mgr.seed(lifecycle.Plugin{ + Slug: "demo-plugin", + State: lifecycle.StateInactive, + }) + req := httptest.NewRequest(http.MethodDelete, "/api/v1/plugins/demo-plugin", nil) + pr := adminPrincipal() + rec := h.do(req, &pr) + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d body=%s", rec.Code, rec.Body.String()) + } + // Subsequent GET should now 404. + getReq := httptest.NewRequest(http.MethodGet, "/api/v1/plugins/demo-plugin", nil) + getRec := h.do(getReq, &pr) + if getRec.Code != http.StatusNotFound { + t.Fatalf("post-uninstall GET expected 404, got %d", getRec.Code) + } +} + +// TestUninstall_Forbidden covers the cap gate: a subscriber cannot +// remove a plugin. +func TestUninstall_Forbidden(t *testing.T) { + h := newHarness(t) + h.mgr.seed(lifecycle.Plugin{Slug: "demo-plugin", State: lifecycle.StateInactive}) + req := httptest.NewRequest(http.MethodDelete, "/api/v1/plugins/demo-plugin", nil) + pr := subscriberPrincipal() + rec := h.do(req, &pr) + if rec.Code != http.StatusForbidden { + t.Fatalf("expected 403, got %d body=%s", rec.Code, rec.Body.String()) + } +} + +// TestInstall_RejectsNonMultipart confirms the content-type gate. A +// JSON body or empty body should produce 400 rather than reaching the +// lifecycle Manager. +func TestInstall_RejectsNonMultipart(t *testing.T) { + h := newHarness(t) + req := httptest.NewRequest(http.MethodPost, "/api/v1/plugins/install", nil) + req.Header.Set("Content-Type", "application/json") + pr := adminPrincipal() + rec := h.do(req, &pr) + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d body=%s", rec.Code, rec.Body.String()) + } +} + +// TestInstall_Forbidden covers the cap gate on the install endpoint. +func TestInstall_Forbidden(t *testing.T) { + h := newHarness(t) + req := httptest.NewRequest(http.MethodPost, "/api/v1/plugins/install", nil) + req.Header.Set("Content-Type", "multipart/form-data; boundary=zzz") + pr := subscriberPrincipal() + rec := h.do(req, &pr) + if rec.Code != http.StatusForbidden { + t.Fatalf("expected 403, got %d body=%s", rec.Code, rec.Body.String()) + } +} + +// Verifies the manager's error propagation: the install handler +// surfaces an unwrapped lifecycle error as a 400 with the message in +// the detail field so the admin UI can render it. +func TestInstall_ManagerErrorReturns400(t *testing.T) { + h := newHarness(t) + h.mgr.installErr = errors.New("manifest version is required") + + // Build a minimal multipart body with a bundle part. + body, contentType := makeMultipart(t, map[string][]byte{"bundle": []byte("not-a-zip")}) + + req := httptest.NewRequest(http.MethodPost, "/api/v1/plugins/install", body) + req.Header.Set("Content-Type", contentType) + pr := adminPrincipal() + rec := h.do(req, &pr) + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d body=%s", rec.Code, rec.Body.String()) + } +} + +// makeMultipart builds a tiny multipart/form-data body for the install +// tests. Each entry becomes one part with FormName = key, body = value. +func makeMultipart(t *testing.T, parts map[string][]byte) (io.Reader, string) { + t.Helper() + pr, pw := io.Pipe() + mw := newMultipartWriter(pw) + go func() { + defer func() { _ = pw.Close() }() + for name, body := range parts { + if err := mw.writePart(name, body); err != nil { + _ = pw.CloseWithError(err) + return + } + } + _ = mw.close() + }() + return pr, "multipart/form-data; boundary=" + mw.boundary +} + +// Tiny in-test multipart writer. We don't pull in mime/multipart's +// FormDataWriter because we want a stable boundary string for the +// boundary check above and because the standard helper writes a +// trailing CRLF the test doesn't need. +type multipartWriter struct { + w io.Writer + boundary string +} + +func newMultipartWriter(w io.Writer) *multipartWriter { + return &multipartWriter{w: w, boundary: "test-boundary-zzz"} +} + +func (m *multipartWriter) writePart(name string, body []byte) error { + header := "--" + m.boundary + "\r\n" + + `Content-Disposition: form-data; name="` + name + `"; filename="` + name + `.bin"` + "\r\n" + + "Content-Type: application/octet-stream\r\n\r\n" + if _, err := io.WriteString(m.w, header); err != nil { + return err + } + if _, err := m.w.Write(body); err != nil { + return err + } + _, err := io.WriteString(m.w, "\r\n") + return err +} + +func (m *multipartWriter) close() error { + _, err := io.WriteString(m.w, "--"+m.boundary+"--\r\n") + return err +} From 75e9ac2dc6b0ce6b344f2387c1c15160b3bf1956 Mon Sep 17 00:00:00 2001 From: Tayeb Mokni Date: Thu, 28 May 2026 12:53:59 +0200 Subject: [PATCH 2/4] =?UTF-8?q?feat(auth):=20mount=20/api/v1/me/tokens=20?= =?UTF-8?q?=E2=80=94=20Personal=20Access=20Tokens=20(#511)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Admin Settings → Tokens page was calling \`/api/v1/me/tokens\` and getting 404. PAT issuance is table-stakes for any CI / automation integration against a remote GoNext install. New package: \`apps/api/internal/auth/pat/\` Routes (RequireSession-wrapped): - GET /api/v1/me/tokens — list (no secrets, only prefix) - POST /api/v1/me/tokens — create + return raw secret ONCE - DELETE /api/v1/me/tokens/{id} — revoke Token format: \`gn_pat_<32-byte-base64>\` (24 raw bytes from crypto/ rand, base64.RawURLEncoding). Hashed via \`packages/go/auth/password. Hash\` with the configured \`GONEXT_AUTH_PEPPER\`. Prefix is first 8 chars of the secret tail for display. Owner isolation: every query scoped by caller's UserID from policy.FromContext. \"Wrong owner\" and \"not found\" both return 404 — no existence oracle for cross-user enumeration. Migration: \`000026_personal_access_tokens.up.sql\` already shipped. Audit: emits \`auth.pat.created\` / \`auth.pat.revoked\`. 7 tests against testcontainers Postgres: empty list, create+raw- token-once, revoke removes row, revoke other-user returns 404, 401 anonymous, empty-name rejection, audit emission. Verified live: full create/list/revoke round-trip works against the running stack. Note: an older \`apps/api/internal/admin/tokens\` package exists with similar surface but different token format. It's currently unmounted; choice of which to keep + repointing admin/users/{id}/tokens at one of them is a follow-up. Closes #511. Co-Authored-By: Claude Opus 4.7 Signed-off-by: Tayeb Mokni --- apps/api/internal/auth/pat/handler.go | 358 +++++++++++++++ apps/api/internal/auth/pat/handler_test.go | 488 +++++++++++++++++++++ apps/api/internal/auth/pat/store.go | 355 +++++++++++++++ 3 files changed, 1201 insertions(+) create mode 100644 apps/api/internal/auth/pat/handler.go create mode 100644 apps/api/internal/auth/pat/handler_test.go create mode 100644 apps/api/internal/auth/pat/store.go diff --git a/apps/api/internal/auth/pat/handler.go b/apps/api/internal/auth/pat/handler.go new file mode 100644 index 0000000..cd5c478 --- /dev/null +++ b/apps/api/internal/auth/pat/handler.go @@ -0,0 +1,358 @@ +package pat + +import ( + "context" + "encoding/json" + "errors" + "log/slog" + "net/http" + "strings" + "time" + + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/Singleton-Solution/GoNext/apps/api/internal/rest/router" + "github.com/Singleton-Solution/GoNext/packages/go/audit" + "github.com/Singleton-Solution/GoNext/packages/go/policy" +) + +// AuditEmitter is the slice of audit.Emitter we depend on. The real +// emitter satisfies it; tests pass a no-op fake. Accepting an interface +// here lets the audit chain change shape (e.g. wrap with tracing) +// without touching this package. +type AuditEmitter interface { + Emit(ctx context.Context, eventType string, opts ...audit.EmitOption) error +} + +// Deps is the dependency bag for Mount. Every field's zero value is +// either invalid or has a sensible default; Mount documents which. +type Deps struct { + // Pool is the Postgres connection pool the store talks to. Required. + Pool *pgxpool.Pool + + // Pepper is the secret HMAC'd into the password-hash input via + // packages/go/auth/password. Required. May be empty in tests but + // production wiring should always pass cfg.Auth.Pepper bytes. + Pepper []byte + + // AuditEmitter receives auth.pat.* events (created / revoked). + // Optional — nil disables audit emission. Production wiring should + // always pass the live emitter. + AuditEmitter AuditEmitter + + // Logger receives structured log lines. nil falls back to + // slog.Default — fine for tests, but production wiring should + // always pass a service logger. + Logger *slog.Logger +} + +// Audit event types emitted by this package. Exported so admin tooling +// and integration tests can compare without re-typing the magic string. +const ( + EventTokenCreated = "auth.pat.created" + EventTokenRevoked = "auth.pat.revoked" +) + +// handlers is the resolved-Deps form passed around inside the package. +// Constructed by Mount; never used standalone. +type handlers struct { + store *Store + audit AuditEmitter + logger *slog.Logger + now func() time.Time +} + +// Mount wires the /me/tokens routes onto mux under base (canonically +// "/api/v1/me/tokens"). Returns an error rather than panicking if +// Deps is malformed so the caller's boot path can warn-and-continue. +// +// The route tree: +// +// GET {base} — list current user's active tokens +// POST {base} — issue; response carries plaintext ONCE +// DELETE {base}/{id} — revoke (idempotent; second call is 204) +// +// Every route requires the user to be authenticated (a Principal on +// the request context, populated upstream by RequireSession or the +// bearer middleware). Mount itself does NOT add a session wrapper — +// the caller's main.go is the right place to compose that, mirroring +// how /api/v1/auth/sessions wires itself. +func Mount(mux *http.ServeMux, base string, deps Deps) error { + if mux == nil { + return errors.New("pat: mux is required") + } + if deps.Pool == nil { + return errors.New("pat: Pool is required") + } + if len(deps.Pepper) == 0 { + return errors.New("pat: Pepper is required") + } + logger := deps.Logger + if logger == nil { + logger = slog.Default() + } + h := &handlers{ + store: NewStore(deps.Pool, deps.Pepper), + audit: deps.AuditEmitter, + logger: logger, + now: func() time.Time { return time.Now().UTC() }, + } + base = strings.TrimRight(base, "/") + mux.Handle("GET "+base, h.gate(h.list)) + mux.Handle("POST "+base, h.gate(h.create)) + mux.Handle("DELETE "+base+"/{id}", h.gate(h.revoke)) + return nil +} + +// gate ensures a Principal is on the context. It does NOT do a +// capability check — every authenticated user may manage their own +// tokens. Cross-user surfaces (admin managing OTHER users' tokens) +// live at /api/v1/admin/users/{id}/tokens and gate with policy.Require. +func (h *handlers) gate(next func(http.ResponseWriter, *http.Request, policy.Principal)) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + pr, ok := policy.FromContext(r.Context()) + if !ok || pr.UserID == "" { + router.WriteError(w, http.StatusUnauthorized, "unauthenticated", "authentication required") + return + } + next(w, r, pr) + }) +} + +// TokenView is the on-wire shape returned by list and revoke. It is +// deliberately separate from Token (the SQL scan target) so the hash +// never appears in JSON even if a future refactor adds a Hash field +// to Token. Keep this in lock-step with apps/admin/.../tokens/types.ts. +type TokenView struct { + ID string `json:"id"` + Name string `json:"name"` + Prefix string `json:"prefix"` + Scopes []string `json:"scopes"` + CreatedAt time.Time `json:"created_at"` + LastUsedAt *time.Time `json:"last_used_at,omitempty"` + ExpiresAt *time.Time `json:"expires_at,omitempty"` +} + +// IssuedTokenView extends TokenView with the plaintext bearer the +// operator must save. The Plaintext field is the only place the +// operator will ever see the full token; subsequent List calls only +// show Prefix. The SaveNow flag is a UI hint that the plaintext is +// non-recoverable — admin UIs SHOULD render an unmissable warning. +type IssuedTokenView struct { + TokenView + Token string `json:"token"` + SaveNow bool `json:"save_now"` +} + +func toView(t Token) TokenView { + scopes := t.Scopes + if scopes == nil { + // Keep the JSON encoding stable: a token with zero scopes + // serialises as "[]", never "null". The admin UI ranges over + // the field unconditionally. + scopes = []string{} + } + return TokenView{ + ID: t.ID, + Name: t.Name, + Prefix: t.Prefix, + Scopes: append([]string{}, scopes...), + CreatedAt: t.CreatedAt, + LastUsedAt: t.LastUsedAt, + ExpiresAt: t.ExpiresAt, + } +} + +// list handles GET /me/tokens. Returns the active tokens for the +// caller, never another user's. The store enforces user_id scoping +// at the SQL level so a bug here cannot widen the audience. +func (h *handlers) list(w http.ResponseWriter, r *http.Request, pr policy.Principal) { + rows, err := h.store.List(r.Context(), pr.UserID) + if err != nil { + h.logger.ErrorContext(r.Context(), "auth/pat: list failed", + slog.String("user_id", pr.UserID), + slog.Any("err", err), + ) + router.WriteError(w, http.StatusInternalServerError, "internal_error", "failed to list tokens") + return + } + out := make([]TokenView, 0, len(rows)) + for _, t := range rows { + out = append(out, toView(t)) + } + router.WriteJSON(w, http.StatusOK, map[string]any{"data": out}) +} + +// CreateRequest is the JSON body for POST /me/tokens. +type CreateRequest struct { + Name string `json:"name"` + Scopes []string `json:"scopes"` + // ExpiresIn is the duration before the token expires, expressed + // as one of: "30d", "90d", "1y", "never". The presets match the + // admin UI radio group. Empty string means "never". + ExpiresIn string `json:"expires_in"` +} + +// presetExpiry maps the preset string to a duration. Unknown presets +// return ok=false; the handler maps that to a 400. +// +// Keep the preset list in lock-step with apps/admin/.../tokens/types.ts +// (ExpiresPreset). Adding a new preset requires both ends. +func presetExpiry(s string, now time.Time) (*time.Time, bool) { + switch strings.TrimSpace(s) { + case "", "never": + return nil, true + case "30d": + t := now.Add(30 * 24 * time.Hour) + return &t, true + case "90d": + t := now.Add(90 * 24 * time.Hour) + return &t, true + case "1y": + t := now.Add(365 * 24 * time.Hour) + return &t, true + default: + return nil, false + } +} + +// create handles POST /me/tokens. On success returns 201 with the +// IssuedTokenView — the ONLY place the operator will ever see the +// plaintext. +func (h *handlers) create(w http.ResponseWriter, r *http.Request, pr policy.Principal) { + var req CreateRequest + dec := json.NewDecoder(r.Body) + // Strict decode: an extra field is a client bug we want to catch + // at integration time, not silently swallow. + dec.DisallowUnknownFields() + if err := dec.Decode(&req); err != nil { + router.WriteError(w, http.StatusBadRequest, "invalid_body", "request body is not valid JSON") + return + } + if strings.TrimSpace(req.Name) == "" { + router.WriteError(w, http.StatusBadRequest, "invalid_name", "name is required") + return + } + expiresAt, ok := presetExpiry(req.ExpiresIn, h.now()) + if !ok { + router.WriteError(w, http.StatusBadRequest, "invalid_expires_in", "expires_in must be one of: never, 30d, 90d, 1y") + return + } + // Dedup scopes — a multi-select UI can submit duplicates after a + // double-click. The store also strips empties; we dedup here so + // the response shape matches what the caller asked for. + req.Scopes = dedup(req.Scopes) + + created, err := h.store.Create(r.Context(), CreateInput{ + UserID: pr.UserID, + Name: req.Name, + Scopes: req.Scopes, + ExpiresAt: expiresAt, + }) + switch { + case err == nil: + // fall through + case errors.Is(err, ErrInvalidName): + router.WriteError(w, http.StatusBadRequest, "invalid_name", "name is required") + return + case errors.Is(err, ErrInvalidUserID): + router.WriteError(w, http.StatusUnauthorized, "unauthenticated", "authentication required") + return + default: + h.logger.ErrorContext(r.Context(), "auth/pat: create failed", + slog.String("user_id", pr.UserID), + slog.Any("err", err), + ) + router.WriteError(w, http.StatusInternalServerError, "internal_error", "failed to issue token") + return + } + + h.emitAudit(r, pr.UserID, EventTokenCreated, created.Token.ID, map[string]any{ + "name": created.Token.Name, + "prefix": created.Token.Prefix, + "scopes": created.Token.Scopes, + }) + + view := IssuedTokenView{ + TokenView: toView(created.Token), + Token: created.Plaintext, + SaveNow: true, + } + router.WriteJSON(w, http.StatusCreated, view) +} + +// revoke handles DELETE /me/tokens/{id}. ErrNotFound from the store +// covers both "no such id" and "id belongs to another user" — we map +// both to 404 to avoid the existence oracle a 403 would surface. +func (h *handlers) revoke(w http.ResponseWriter, r *http.Request, pr policy.Principal) { + id := r.PathValue("id") + if id == "" { + // Structurally unreachable under ServeMux's "/{id}" pattern, + // but the off-route call path used by tests can hit it. + router.WriteError(w, http.StatusNotFound, "not_found", "token not found") + return + } + err := h.store.Revoke(r.Context(), pr.UserID, id) + switch { + case err == nil: + h.emitAudit(r, pr.UserID, EventTokenRevoked, id, nil) + w.WriteHeader(http.StatusNoContent) + case errors.Is(err, ErrNotFound): + router.WriteError(w, http.StatusNotFound, "not_found", "token not found") + default: + h.logger.ErrorContext(r.Context(), "auth/pat: revoke failed", + slog.String("user_id", pr.UserID), + slog.String("token_id", id), + slog.Any("err", err), + ) + router.WriteError(w, http.StatusInternalServerError, "internal_error", "failed to revoke token") + } +} + +// emitAudit records an auth.pat.* event. Errors are non-fatal — the +// state change has already landed in the DB by the time we get here, +// and rolling back because the audit log failed would be a worse +// outcome (operator stuck without a token, or unable to revoke a +// credential they think is dead). +func (h *handlers) emitAudit(r *http.Request, userID, eventType, tokenID string, meta map[string]any) { + if h.audit == nil { + return + } + opts := []audit.EmitOption{ + audit.WithActorOverride(userID), + audit.WithTarget("personal_access_token", tokenID), + audit.WithSeverity(audit.SeverityInfo), + } + if meta != nil { + opts = append(opts, audit.WithMetadata(meta)) + } + if err := h.audit.Emit(r.Context(), eventType, opts...); err != nil { + h.logger.WarnContext(r.Context(), "auth/pat: audit emit failed", + slog.String("event", eventType), + slog.String("user_id", userID), + slog.String("token_id", tokenID), + slog.Any("err", err), + ) + } +} + +// dedup trims and removes duplicate strings, preserving first-seen +// order. Used on the scopes slice; a multi-select UI can submit dupes +// after a double-click. Trimming + skipping empties means " read " and +// "read" collapse to the single canonical "read". +func dedup(in []string) []string { + seen := make(map[string]struct{}, len(in)) + out := make([]string, 0, len(in)) + for _, s := range in { + s = strings.TrimSpace(s) + if s == "" { + continue + } + if _, dup := seen[s]; dup { + continue + } + seen[s] = struct{}{} + out = append(out, s) + } + return out +} diff --git a/apps/api/internal/auth/pat/handler_test.go b/apps/api/internal/auth/pat/handler_test.go new file mode 100644 index 0000000..04835c2 --- /dev/null +++ b/apps/api/internal/auth/pat/handler_test.go @@ -0,0 +1,488 @@ +package pat + +import ( + "bytes" + "context" + "database/sql" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "runtime" + "sort" + "strings" + "testing" + "time" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgxpool" + _ "github.com/jackc/pgx/v5/stdlib" + + "github.com/Singleton-Solution/GoNext/packages/go/audit" + "github.com/Singleton-Solution/GoNext/packages/go/policy" + "github.com/Singleton-Solution/GoNext/packages/go/testutil/containers" +) + +// testPepper is the fixed pepper used across the tests in this file. +// We use a stable value so test failures aren't masked by per-run salt +// drift; the per-call random salt inside argon2 is what actually keeps +// the resulting hashes distinct. +var testPepper = []byte("test-pepper-do-not-use-in-production") + +// mustPostgresWithPATSchema spins up a Postgres container, applies +// migrations 000001 through 000026 (the PAT migration), and returns a +// pgxpool plus an auto-cleanup hook. Tests skip cleanly with -short. +// +// We stop at 000026 deliberately: the PAT handlers only need users +// (000002), citext (000001), and the PAT table itself. Bringing in +// later migrations would only slow the container's startup. If a +// future PAT change introduces a join against a newer table, bump the +// upper-bound filter here. +func mustPostgresWithPATSchema(t *testing.T) *pgxpool.Pool { + t.Helper() + if testing.Short() { + t.Skip("integration test: skip with -short") + } + dsn := containers.Postgres(t) + if dsn == "" { + t.Skip("docker not available") + } + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + + pool, err := pgxpool.New(ctx, dsn) + if err != nil { + t.Fatalf("pgxpool.New: %v", err) + } + t.Cleanup(pool.Close) + + mustApplyPATMigrations(t, dsn) + return pool +} + +// mustApplyPATMigrations applies migrations 000001..000026 against the +// supplied DSN. We open via database/sql + the pgx stdlib so each +// migration runs as a single statement-block — matching the importer +// helper used by every other DB-touching test in apps/api. +func mustApplyPATMigrations(t *testing.T, dsn string) { + t.Helper() + root := repoRoot(t) + dir := filepath.Join(root, "migrations") + matches, err := filepath.Glob(filepath.Join(dir, "*.up.sql")) + if err != nil { + t.Fatalf("glob migrations: %v", err) + } + // Filter to 000001..000026. Anything later than that brings in + // tables we don't touch and only slows the test. + keep := matches[:0] + for _, m := range matches { + base := filepath.Base(m) + if len(base) < 6 { + continue + } + if base[:6] > "000026" { + continue + } + if base[:6] < "000001" { + continue + } + keep = append(keep, m) + } + matches = keep + if len(matches) == 0 { + t.Fatalf("no migrations found in %s", dir) + } + sort.Strings(matches) + db, err := sql.Open("pgx", dsn) + if err != nil { + t.Fatalf("sql.Open: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second) + defer cancel() + for _, m := range matches { + body, err := os.ReadFile(m) + if err != nil { + t.Fatalf("read %s: %v", m, err) + } + if _, err := db.ExecContext(ctx, string(body)); err != nil { + t.Fatalf("apply %s: %v", filepath.Base(m), err) + } + } +} + +// repoRoot walks up until it finds the directory containing go.work. +func repoRoot(t *testing.T) string { + t.Helper() + _, file, _, ok := runtime.Caller(0) + if !ok { + t.Fatal("runtime.Caller failed") + } + dir := filepath.Dir(file) + for i := 0; i < 12; i++ { + if _, err := os.Stat(filepath.Join(dir, "go.work")); err == nil { + return dir + } + dir = filepath.Dir(dir) + } + t.Fatal("could not locate repo root (go.work)") + return "" +} + +// seedUser inserts a users row and returns its UUID. The PAT user_id +// column has a FK to users.id, so every test that exercises Create +// needs at least one real user. +func seedUser(t *testing.T, pool *pgxpool.Pool, email, handle string) uuid.UUID { + t.Helper() + var id uuid.UUID + err := pool.QueryRow(context.Background(), + `INSERT INTO users (email, handle, display_name) + VALUES ($1::citext, $2::citext, $3) RETURNING id`, + email, handle, handle, + ).Scan(&id) + if err != nil { + t.Fatalf("seed user: %v", err) + } + return id +} + +// withPrincipal wraps a handler with a middleware that injects a fixed +// Principal. Production wires session/PAT auth here; tests just want +// a Principal on the context so the gate passes. +func withPrincipal(p policy.Principal, h http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + r = r.WithContext(policy.WithPrincipal(r.Context(), p)) + h.ServeHTTP(w, r) + }) +} + +// newServer wires Mount + a principal-injecting middleware. Returns +// the server URL, the audit store (so tests can assert on emitted +// events), and a cleanup hook. +func newServer(t *testing.T, pool *pgxpool.Pool, userID string) (string, *audit.MemoryStore, func()) { + t.Helper() + auditStore := audit.NewMemoryStore() + emitter := audit.NewEmitter(auditStore) + mux := http.NewServeMux() + err := Mount(mux, "/api/v1/me/tokens", Deps{ + Pool: pool, + Pepper: testPepper, + AuditEmitter: emitter, + }) + if err != nil { + t.Fatalf("Mount: %v", err) + } + pr := policy.Principal{UserID: userID} + srv := httptest.NewServer(withPrincipal(pr, mux)) + return srv.URL, auditStore, srv.Close +} + +func decodeJSON(t *testing.T, r *http.Response, v any) { + t.Helper() + body, err := io.ReadAll(r.Body) + if err != nil { + t.Fatalf("read body: %v", err) + } + if err := json.Unmarshal(body, v); err != nil { + t.Fatalf("decode: %v\nbody: %s", err, body) + } +} + +// TestList_NoTokens_EmptyData covers the empty-state path the admin +// settings page sees on a fresh account: a 200 with {"data":[]}, never +// a 500 from a nil slice or a 404 from the mount. +func TestList_NoTokens_EmptyData(t *testing.T) { + pool := mustPostgresWithPATSchema(t) + user := seedUser(t, pool, "alice@example.com", "alice") + url, _, cleanup := newServer(t, pool, user.String()) + defer cleanup() + + res, err := http.Get(url + "/api/v1/me/tokens") + if err != nil { + t.Fatalf("GET: %v", err) + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + raw, _ := io.ReadAll(res.Body) + t.Fatalf("status %d body=%s", res.StatusCode, raw) + } + var out struct { + Data []TokenView `json:"data"` + } + decodeJSON(t, res, &out) + if len(out.Data) != 0 { + t.Fatalf("expected empty data, got %v", out.Data) + } +} + +// TestCreate_ReturnsRawTokenOnce is the most important test in this +// file: the create response carries the plaintext, but subsequent +// GETs only show the prefix. A regression here means a leaked DB dump +// becomes a leaked credential. +func TestCreate_ReturnsRawTokenOnce(t *testing.T) { + pool := mustPostgresWithPATSchema(t) + user := seedUser(t, pool, "bob@example.com", "bob") + url, _, cleanup := newServer(t, pool, user.String()) + defer cleanup() + + body, _ := json.Marshal(CreateRequest{ + Name: "ci-token", + Scopes: []string{"read"}, + ExpiresIn: "never", + }) + res, err := http.Post(url+"/api/v1/me/tokens", "application/json", bytes.NewReader(body)) + if err != nil { + t.Fatalf("POST: %v", err) + } + defer res.Body.Close() + if res.StatusCode != http.StatusCreated { + raw, _ := io.ReadAll(res.Body) + t.Fatalf("status %d body=%s", res.StatusCode, raw) + } + var issued IssuedTokenView + decodeJSON(t, res, &issued) + if !strings.HasPrefix(issued.Token, Namespace) { + t.Fatalf("plaintext missing namespace: %q", issued.Token) + } + if len(issued.Token) != MinTokenLen { + t.Fatalf("plaintext wrong length: %d want %d (%q)", len(issued.Token), MinTokenLen, issued.Token) + } + if !issued.SaveNow { + t.Fatal("save_now must be true on the create response") + } + if issued.Prefix == "" || len(issued.Prefix) != PrefixLen { + t.Fatalf("prefix wrong shape: %q", issued.Prefix) + } + // The prefix on the response must match the first PrefixLen chars + // of the secret tail — that's what the operator will use to + // recognise the token in the list later. + wantPrefix := issued.Token[len(Namespace) : len(Namespace)+PrefixLen] + if issued.Prefix != wantPrefix { + t.Fatalf("prefix %q does not match token tail %q", issued.Prefix, wantPrefix) + } + + // A subsequent list MUST NOT include the plaintext. The list + // response shape is {"data":[TokenView]} and TokenView has no + // Token field; we double-check by string-searching the raw body. + listRes, err := http.Get(url + "/api/v1/me/tokens") + if err != nil { + t.Fatalf("GET: %v", err) + } + defer listRes.Body.Close() + raw, _ := io.ReadAll(listRes.Body) + if strings.Contains(string(raw), issued.Token) { + t.Fatalf("list response leaked plaintext: %s", raw) + } + low := strings.ToLower(string(raw)) + if strings.Contains(low, "\"token\":") { + t.Fatalf("list response leaked token field: %s", raw) + } + if strings.Contains(low, "\"hash\"") { + t.Fatalf("list response leaked hash field: %s", raw) + } + // The row IS visible — just without the plaintext. + var listOut struct { + Data []TokenView `json:"data"` + } + if err := json.Unmarshal(raw, &listOut); err != nil { + t.Fatalf("decode list: %v", err) + } + if len(listOut.Data) != 1 { + t.Fatalf("expected 1 token in list, got %d", len(listOut.Data)) + } + if listOut.Data[0].Prefix != issued.Prefix { + t.Fatalf("list row prefix mismatch: got %q want %q", listOut.Data[0].Prefix, issued.Prefix) + } +} + +// TestRevoke_RemovesFromList covers the happy revoke path: DELETE +// returns 204, the list call afterwards omits the row. The store +// soft-deletes (sets revoked_at) but the active-only filter on List +// hides the row. +func TestRevoke_RemovesFromList(t *testing.T) { + pool := mustPostgresWithPATSchema(t) + user := seedUser(t, pool, "carol@example.com", "carol") + url, _, cleanup := newServer(t, pool, user.String()) + defer cleanup() + + // Create the token via the HTTP path so we have the canonical id. + body, _ := json.Marshal(CreateRequest{Name: "to-revoke", Scopes: []string{"read"}}) + res, err := http.Post(url+"/api/v1/me/tokens", "application/json", bytes.NewReader(body)) + if err != nil { + t.Fatalf("POST: %v", err) + } + var issued IssuedTokenView + decodeJSON(t, res, &issued) + res.Body.Close() + + req, _ := http.NewRequest("DELETE", url+"/api/v1/me/tokens/"+issued.ID, nil) + delRes, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("DELETE: %v", err) + } + defer delRes.Body.Close() + if delRes.StatusCode != http.StatusNoContent { + raw, _ := io.ReadAll(delRes.Body) + t.Fatalf("status %d want 204 body=%s", delRes.StatusCode, raw) + } + + listRes, err := http.Get(url + "/api/v1/me/tokens") + if err != nil { + t.Fatalf("GET after revoke: %v", err) + } + defer listRes.Body.Close() + var listOut struct { + Data []TokenView `json:"data"` + } + decodeJSON(t, listRes, &listOut) + if len(listOut.Data) != 0 { + t.Fatalf("expected empty list after revoke, got %d rows", len(listOut.Data)) + } +} + +// TestRevoke_OtherUsersToken_Returns404 is the existence-oracle guard. +// A caller MUST NOT be able to learn whether an id belongs to another +// user via 403-vs-404; the store collapses both to ErrNotFound. +func TestRevoke_OtherUsersToken_Returns404(t *testing.T) { + pool := mustPostgresWithPATSchema(t) + alice := seedUser(t, pool, "alice2@example.com", "alice2") + bobID := seedUser(t, pool, "bob2@example.com", "bob2") + + // Alice issues a token (we manipulate the store directly to avoid + // having to spin up a second server for her). + store := NewStore(pool, testPepper) + created, err := store.Create(context.Background(), CreateInput{ + UserID: alice.String(), + Name: "alices-token", + Scopes: []string{"read"}, + }) + if err != nil { + t.Fatalf("seed Alice's token: %v", err) + } + + // Bob (the principal) tries to revoke Alice's token. + url, _, cleanup := newServer(t, pool, bobID.String()) + defer cleanup() + req, _ := http.NewRequest("DELETE", url+"/api/v1/me/tokens/"+created.Token.ID, nil) + res, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("DELETE: %v", err) + } + defer res.Body.Close() + if res.StatusCode != http.StatusNotFound { + raw, _ := io.ReadAll(res.Body) + t.Fatalf("status %d want 404 body=%s", res.StatusCode, raw) + } + + // Alice's token must still be active — Bob's failed attempt should + // not have flipped a flag. + rows, err := store.List(context.Background(), alice.String()) + if err != nil { + t.Fatalf("List Alice: %v", err) + } + if len(rows) != 1 { + t.Fatalf("expected Alice's token to survive; got %d rows", len(rows)) + } +} + +// TestAuthRequired_AllEndpoints — no Principal on the context means +// 401 for every endpoint. The session middleware would normally make +// this unreachable, but we want a defense-in-depth gate at the +// handler too. +func TestAuthRequired_AllEndpoints(t *testing.T) { + pool := mustPostgresWithPATSchema(t) + + mux := http.NewServeMux() + if err := Mount(mux, "/api/v1/me/tokens", Deps{ + Pool: pool, + Pepper: testPepper, + }); err != nil { + t.Fatalf("Mount: %v", err) + } + // No principal-injecting middleware on this server — every call + // must hit the gate's 401 branch. + srv := httptest.NewServer(mux) + defer srv.Close() + + cases := []struct { + method string + path string + }{ + {"GET", "/api/v1/me/tokens"}, + {"POST", "/api/v1/me/tokens"}, + {"DELETE", "/api/v1/me/tokens/00000000-0000-7000-8000-000000000000"}, + } + for _, tc := range cases { + t.Run(tc.method+" "+tc.path, func(t *testing.T) { + req, _ := http.NewRequest(tc.method, srv.URL+tc.path, bytes.NewReader([]byte("{}"))) + req.Header.Set("Content-Type", "application/json") + res, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("do: %v", err) + } + defer res.Body.Close() + if res.StatusCode != http.StatusUnauthorized { + raw, _ := io.ReadAll(res.Body) + t.Fatalf("status %d want 401 body=%s", res.StatusCode, raw) + } + }) + } +} + +// TestCreate_RejectsEmptyName covers the input-validation path: an +// empty name is a 400 before we burn an argon2 hash. +func TestCreate_RejectsEmptyName(t *testing.T) { + pool := mustPostgresWithPATSchema(t) + user := seedUser(t, pool, "dave@example.com", "dave") + url, _, cleanup := newServer(t, pool, user.String()) + defer cleanup() + + body, _ := json.Marshal(CreateRequest{Name: " ", Scopes: []string{"read"}}) + res, err := http.Post(url+"/api/v1/me/tokens", "application/json", bytes.NewReader(body)) + if err != nil { + t.Fatalf("POST: %v", err) + } + defer res.Body.Close() + if res.StatusCode != http.StatusBadRequest { + raw, _ := io.ReadAll(res.Body) + t.Fatalf("status %d want 400 body=%s", res.StatusCode, raw) + } +} + +// TestCreate_EmitsAuditEvent — the audit emitter receives an +// auth.pat.created event with the issued token id as the target. +func TestCreate_EmitsAuditEvent(t *testing.T) { + pool := mustPostgresWithPATSchema(t) + user := seedUser(t, pool, "eve@example.com", "eve") + url, auditStore, cleanup := newServer(t, pool, user.String()) + defer cleanup() + + body, _ := json.Marshal(CreateRequest{Name: "audit-test", Scopes: []string{"read"}}) + res, err := http.Post(url+"/api/v1/me/tokens", "application/json", bytes.NewReader(body)) + if err != nil { + t.Fatalf("POST: %v", err) + } + defer res.Body.Close() + var issued IssuedTokenView + decodeJSON(t, res, &issued) + + events, err := auditStore.List(context.Background(), audit.Filter{}) + if err != nil { + t.Fatalf("audit List: %v", err) + } + if len(events) == 0 { + t.Fatal("expected at least one audit event, got none") + } + found := false + for _, e := range events { + if e.EventType == EventTokenCreated && e.ResourceID == issued.ID { + found = true + break + } + } + if !found { + t.Fatalf("expected %s event for token %s; got %d events", EventTokenCreated, issued.ID, len(events)) + } +} diff --git a/apps/api/internal/auth/pat/store.go b/apps/api/internal/auth/pat/store.go new file mode 100644 index 0000000..052c156 --- /dev/null +++ b/apps/api/internal/auth/pat/store.go @@ -0,0 +1,355 @@ +// Package pat implements the per-user Personal Access Token (PAT) +// issuance HTTP surface — list, create, revoke — mounted at +// /api/v1/me/tokens by the API server's main.go. +// +// The package is intentionally separate from packages/go/auth/pat, +// which is the lower-level credential primitive (the bearer-token +// middleware that resolves "Authorization: Bearer gn_pat_..." into a +// Principal). Splitting issuance from authentication keeps the admin +// settings page from importing the middleware and inverting the +// dependency graph: handlers depend on store, store depends on +// password+pool, and the bearer middleware is wholly separate. +// +// Wire format +// +// A token is the literal "gn_pat_" followed by 32 URL-safe base64 +// characters (24 raw bytes from crypto/rand). The "gn_pat_" namespace +// lets secret-scanners pattern-match a leaked credential without false +// positives against arbitrary base64. The plaintext is only ever +// returned by the create handler, exactly once. The database stores +// only the argon2id PHC encoding of the full plaintext, peppered with +// GONEXT_AUTH_PEPPER — identical posture to user passwords. +// +// Storage +// +// Rows land in personal_access_tokens (migration 000026). The list +// view exposes id, name, prefix, scopes, created_at, last_used_at, +// expires_at — the hash and plaintext never leave the package. The +// prefix column is the first 8 chars of the random tail (post the +// gn_pat_ namespace), surfaced as "gn_pat_AbCdEfGh…" by the admin UI. +package pat + +import ( + "context" + "crypto/rand" + "encoding/base64" + "errors" + "fmt" + "strings" + "time" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/Singleton-Solution/GoNext/packages/go/auth/password" +) + +// Namespace is the mandatory prefix on every PAT plaintext. The "gn_pat_" +// literal lets log scrubbers, GitHub's secret scanner, and gitleaks +// pattern-match a leaked credential without false positives against +// arbitrary base64. Changing this is a breaking change for every +// downstream scanner — it is a constant for a reason. +const Namespace = "gn_pat_" + +// secretBytes is the entropy in the random tail. 24 bytes → 32 chars +// under base64.RawURLEncoding, matching the width OWASP recommends for +// long-lived bearer tokens. The total token width is then +// +// len("gn_pat_") + 32 = 39 characters +// +// which fits comfortably in a single Authorization header and inside +// the 64-character ceiling most password managers honour. +const secretBytes = 24 + +// secretLen is the encoded length of the random tail. +const secretLen = 32 + +// PrefixLen is the on-disk length of the prefix column. The first 8 +// characters of the random tail (post-namespace) are what the list view +// renders as "gn_pat_AbCdEfGh…". Exposed as a const so the handler +// doesn't hard-code the slice bound. +const PrefixLen = 8 + +// MinTokenLen is the minimum length a string must have to even be +// considered a candidate token. Used to fail fast on obvious garbage +// before paying the argon2 cost. Equal to len(Namespace) + secretLen. +const MinTokenLen = len(Namespace) + secretLen + +// Errors returned by the store. Callers compare with errors.Is. +var ( + // ErrNotFound is returned by Revoke when no (id, user_id) row + // matches. Distinct from "already revoked" (which is a silent + // idempotent no-op) so the handler can map to 404. + ErrNotFound = errors.New("pat: not found") + + // ErrInvalidName surfaces a caller-side validation failure. The + // store re-checks the table's CHECK (length(btrim(name)) > 0) + // because relying on the DB constraint to bubble up a 23xxx + // SQLSTATE through pgx is brittle. + ErrInvalidName = errors.New("pat: name must be non-empty") + + // ErrInvalidUserID is the zero-UUID / empty user-id guard. Belt + // and braces: the handler already gates on policy.FromContext but + // a future caller path could regress. + ErrInvalidUserID = errors.New("pat: user_id is required") +) + +// Token is the in-memory representation of a personal_access_tokens +// row. It mirrors the table columns 1:1 with the hash deliberately +// omitted — the store never exposes the hash bytes across the package +// boundary. The handler turns this into the on-wire TokenView; this +// type keeps the SQL scan target out of the handler so the wire shape +// can evolve without touching the SQL. +type Token struct { + ID string + UserID string + Name string + Prefix string + Scopes []string + CreatedAt time.Time + LastUsedAt *time.Time + ExpiresAt *time.Time + RevokedAt *time.Time +} + +// Store is the Postgres-backed persistence layer for the +// personal_access_tokens table. It is the only thing in the package +// that knows the table's column shape; the handler is wire-shape only. +// +// Zero value is unusable. Construct with NewStore. +type Store struct { + pool *pgxpool.Pool + pepper []byte +} + +// NewStore wraps a pgxpool.Pool and a pepper. Both are required — a +// nil pool or empty pepper is a wiring bug we surface at boot rather +// than silently degrading to an in-memory or unsalted hash. The pool's +// lifecycle stays with the caller; the store never calls Close on it. +func NewStore(pool *pgxpool.Pool, pepper []byte) *Store { + return &Store{pool: pool, pepper: pepper} +} + +// CreateInput is the validated arguments to Create. Splitting the +// validation from the SQL means the handler can map specific errors +// (invalid name, invalid scopes) to 400 without sniffing pg error +// codes. +type CreateInput struct { + UserID string + Name string + Scopes []string + ExpiresAt *time.Time +} + +// Created is the return value of Create. It carries the persisted +// Token row and — exactly once — the plaintext bearer the operator +// must save. The plaintext is intentionally not stashed on the Token +// struct itself; a Token returned by List or any other read path will +// never have a plaintext. +type Created struct { + Token Token + Plaintext string +} + +// Create mints a fresh PAT for input.UserID, hashes it with argon2id +// + the configured pepper, persists the row, and returns the plaintext +// once. Subsequent reads of the same row see only the prefix. +// +// Errors: +// - ErrInvalidUserID, ErrInvalidName for caller-side validation. +// - Any pgx error wrapped with "pat: create: ..." for SQL failures. +// - Any crypto/rand error wrapped with "pat: random: ..." (vanishingly +// unlikely; the kernel CSPRNG essentially never fails). +// +// Create is safe for concurrent use; the underlying pgxpool handles +// the connection multiplexing. +func (s *Store) Create(ctx context.Context, in CreateInput) (Created, error) { + if strings.TrimSpace(in.UserID) == "" { + return Created{}, ErrInvalidUserID + } + name := strings.TrimSpace(in.Name) + if name == "" { + return Created{}, ErrInvalidName + } + + plaintext, secret, err := newPlaintext() + if err != nil { + return Created{}, fmt.Errorf("pat: random: %w", err) + } + + encoded, err := password.Hash(plaintext, s.pepper) + if err != nil { + return Created{}, fmt.Errorf("pat: hash: %w", err) + } + + // Copy scopes so a later mutation of the caller's slice can't + // affect the persisted row. Costs nothing. + scopes := make([]string, 0, len(in.Scopes)) + for _, sc := range in.Scopes { + sc = strings.TrimSpace(sc) + if sc == "" { + continue + } + scopes = append(scopes, sc) + } + + prefix := secret[:PrefixLen] + now := time.Now().UTC() + + const q = ` +INSERT INTO personal_access_tokens ( + user_id, name, prefix, hash, scopes, created_at, expires_at +) VALUES ($1, $2, $3, $4, $5, $6, $7) +RETURNING id, created_at +` + var row Token + row.UserID = in.UserID + row.Name = name + row.Prefix = prefix + row.Scopes = scopes + row.ExpiresAt = in.ExpiresAt + if err := s.pool.QueryRow(ctx, q, + in.UserID, name, prefix, []byte(encoded), scopes, now, in.ExpiresAt, + ).Scan(&row.ID, &row.CreatedAt); err != nil { + return Created{}, fmt.Errorf("pat: create: %w", err) + } + return Created{Token: row, Plaintext: plaintext}, nil +} + +// List returns the active (revoked_at IS NULL, not expired) tokens +// for userID, newest first. The hash column is deliberately not +// selected so the bytes never cross the store boundary. +// +// Returns an empty slice (not a nil slice) on success with no rows so +// the handler can range over the result without a nil check and the +// JSON envelope is "[]" instead of "null". +func (s *Store) List(ctx context.Context, userID string) ([]Token, error) { + if strings.TrimSpace(userID) == "" { + return nil, ErrInvalidUserID + } + const q = ` +SELECT id, user_id, name, prefix, scopes, + created_at, last_used_at, expires_at, revoked_at +FROM personal_access_tokens +WHERE user_id = $1 + AND revoked_at IS NULL + AND (expires_at IS NULL OR expires_at > now()) +ORDER BY created_at DESC +` + rows, err := s.pool.Query(ctx, q, userID) + if err != nil { + return nil, fmt.Errorf("pat: list: %w", err) + } + defer rows.Close() + out := make([]Token, 0) + for rows.Next() { + var t Token + if err := rows.Scan( + &t.ID, &t.UserID, &t.Name, &t.Prefix, &t.Scopes, + &t.CreatedAt, &t.LastUsedAt, &t.ExpiresAt, &t.RevokedAt, + ); err != nil { + return nil, fmt.Errorf("pat: list scan: %w", err) + } + out = append(out, t) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("pat: list iter: %w", err) + } + return out, nil +} + +// Revoke marks the row identified by (userID, id) as revoked. The +// userID is the authorisation gate — a caller cannot revoke another +// user's token by guessing the id. The store collapses both "wrong +// owner" and "no such id" into ErrNotFound so the handler can return +// a uniform 404 without an enumeration oracle (a 403 would leak that +// the id IS valid, just belongs to someone else). +// +// Idempotent: a second Revoke against an already-revoked row is a +// no-op and returns nil. The handler maps that to the same 204 as a +// first-time revoke; CI scripts that retry on transient errors won't +// see a 404 they have to special-case. +func (s *Store) Revoke(ctx context.Context, userID, id string) error { + if strings.TrimSpace(userID) == "" { + return ErrInvalidUserID + } + if strings.TrimSpace(id) == "" { + return ErrNotFound + } + const updateQ = ` +UPDATE personal_access_tokens +SET revoked_at = now() +WHERE id = $1 + AND user_id = $2 + AND revoked_at IS NULL +` + ct, err := s.pool.Exec(ctx, updateQ, id, userID) + if err != nil { + return fmt.Errorf("pat: revoke: %w", err) + } + if ct.RowsAffected() == 1 { + return nil + } + // Zero rows affected: either "already revoked" (fine, return nil) + // or "row does not exist for this (id, user_id)" (ErrNotFound). + // Disambiguate with a cheap EXISTS probe. Note we don't probe by + // id-only — that would let a caller learn the id belongs to another + // user via a side channel. + const existsQ = `SELECT EXISTS(SELECT 1 FROM personal_access_tokens WHERE id = $1 AND user_id = $2)` + var found bool + if err := s.pool.QueryRow(ctx, existsQ, id, userID).Scan(&found); err != nil { + // pgx surfaces "invalid input syntax for type uuid" as a + // regular query error when the id isn't a UUID. We collapse + // that to ErrNotFound — a non-UUID can never match an existing + // row, and surfacing the DB error would be a fingerprint. + if errors.Is(err, pgx.ErrNoRows) { + return ErrNotFound + } + if isInvalidUUID(err) { + return ErrNotFound + } + return fmt.Errorf("pat: revoke exists: %w", err) + } + if !found { + return ErrNotFound + } + return nil +} + +// isInvalidUUID matches pgx's "invalid input syntax for type uuid" +// error so we can collapse it to ErrNotFound. Done via substring match +// because pgx wraps the underlying pgconn.PgError without exporting a +// sentinel for this case. The string is stable in libpq and unlikely +// to change. +func isInvalidUUID(err error) bool { + if err == nil { + return false + } + msg := err.Error() + return strings.Contains(msg, "invalid input syntax for type uuid") || + strings.Contains(msg, "invalid input syntax for uuid") +} + +// newPlaintext mints a fresh "gn_pat_<32-char-base64>" string from +// crypto/rand. Returns the full plaintext and the secret portion (the +// 32 chars after the namespace) so callers can slice the prefix +// without re-parsing. +// +// Uses base64.RawURLEncoding so the secret is safe to drop into a +// URL or a header without any escaping. The alphabet is the URL-safe +// 64-char set (A-Z, a-z, 0-9, '-', '_'); no padding because the +// length is fixed and known. +func newPlaintext() (plaintext, secret string, err error) { + buf := make([]byte, secretBytes) + if _, err := rand.Read(buf); err != nil { + return "", "", err + } + secret = base64.RawURLEncoding.EncodeToString(buf) + // RawURLEncoding of 24 bytes is always 32 chars, but we belt-and- + // brace the length in case a future stdlib change introduces a + // trailing newline or similar. + if len(secret) != secretLen { + return "", "", fmt.Errorf("pat: unexpected encoded length %d, want %d", len(secret), secretLen) + } + return Namespace + secret, secret, nil +} From 5fe90aaaea41f387b06c6b9a8123a6753236a895 Mon Sep 17 00:00:00 2001 From: Tayeb Mokni Date: Thu, 28 May 2026 12:54:11 +0200 Subject: [PATCH 3/4] feat(api): customizer preview + site-editor parts endpoints (#512) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two endpoints the admin Customizer + Site Editor pages were rendering against — both 404'd previously. ### POST /api/v1/admin/customizer/preview Extends existing internal/admin/customizer/handler.go. Body shape permissive — accepts both \`{settings: {...}}\` (direct override) and \`{overrides: {...}}\` (envelope) since the admin client posts both. Returns \`{themeSlug, cssCustomProperties}\` after merging override onto the active theme manifest. Empty body returns base CSS. Permissive JSON decode (no DisallowUnknownFields) since preview is keystroke-grained; strict validation lives on PUT. Gated by theme.customize. 6 new test cases (base CSS, override applied, envelope variant, forbidden, unauth, invalid JSON). ### GET/PUT /api/v1/admin/site-editor/parts(/{name}) New package: internal/admin/siteeditor/. File-based — reads/writes \`themes/{active}/parts/{name}.html\` for theme template parts the operator edits in-browser. Part shape: {name, area: header|footer|general, title, content}. Security: validates name against active theme.json templateParts declaration (undeclared name → 404 — no arbitrary file writes allowed). Path-traversal guard rejects /, \\, .., control chars, uppercase → 400. Atomic write via temp file + rename. Gated by \`manage_themes\` OR \`theme.edit_parts\` (either grants). 17 tests covering list/get/put, undeclared name 404, traversal flavors 400, unauth 401, forbidden 403, missing-file empty content, MkdirAll for fresh installs. ### main.go wiring Both endpoints now go through sub-mux + RequireSession wrap (was missing on the existing customizer mount — that's why /api/v1/admin/customizer/active was previously returning 401). Verified live: GET /api/v1/admin/site-editor/parts → 200 + header + footer POST /api/v1/admin/customizer/preview {} → 200 base CSS POST /api/v1/admin/customizer/preview {paper:#112233} → 200 w/ color flipped Closes #512. Co-Authored-By: Claude Opus 4.7 Signed-off-by: Tayeb Mokni --- apps/api/internal/admin/customizer/handler.go | 125 ++++- .../internal/admin/customizer/handler_test.go | 116 ++++ apps/api/internal/admin/siteeditor/handler.go | 524 ++++++++++++++++++ .../internal/admin/siteeditor/handler_test.go | 465 ++++++++++++++++ 4 files changed, 1227 insertions(+), 3 deletions(-) create mode 100644 apps/api/internal/admin/siteeditor/handler.go create mode 100644 apps/api/internal/admin/siteeditor/handler_test.go diff --git a/apps/api/internal/admin/customizer/handler.go b/apps/api/internal/admin/customizer/handler.go index 1ddc2da..9d9a1db 100644 --- a/apps/api/internal/admin/customizer/handler.go +++ b/apps/api/internal/admin/customizer/handler.go @@ -1,6 +1,7 @@ package customizer import ( + "bytes" "context" "encoding/json" "errors" @@ -77,9 +78,10 @@ type handlers struct { // // The route tree: // -// GET {base}/active — theme + current overrides -// PUT {base}/active — validate + save overrides -// DELETE {base}/active — clear overrides (Reset) +// GET {base}/active — theme + current overrides +// PUT {base}/active — validate + save overrides +// DELETE {base}/active — clear overrides (Reset) +// POST {base}/preview — preview merged CSS without persisting // // All routes are gated by the theme.customize capability. func Mount(mux *http.ServeMux, base string, deps Deps) error { @@ -100,6 +102,7 @@ func Mount(mux *http.ServeMux, base string, deps Deps) error { mux.Handle("GET "+base+"/active", h.gate(h.getActive)) mux.Handle("PUT "+base+"/active", h.gate(h.putActive)) mux.Handle("DELETE "+base+"/active", h.gate(h.deleteActive)) + mux.Handle("POST "+base+"/preview", h.gate(h.postPreview)) return nil } @@ -278,6 +281,122 @@ func (h *handlers) deleteActive(w http.ResponseWriter, r *http.Request, pr polic w.WriteHeader(http.StatusNoContent) } +// PreviewRequest is the POST /preview body. The admin React form posts +// the in-flight override state straight through; we accept either the +// override object directly OR a wrapper {"overrides": } +// because two generations of the admin form sit in production today and +// re-shipping the API while the UI still uses the old wrapper would +// strand operators on a 400. +type PreviewRequest struct { + Overrides json.RawMessage `json:"overrides,omitempty"` +} + +// PreviewResponse carries the rendered CSS the iframe applies. The +// shape stays narrow on purpose — every additional field is something +// the renderer would have to learn to read. +type PreviewResponse struct { + // ThemeSlug echoes the slug the preview was computed for. The admin + // surface uses it to verify the preview matches the theme the form + // thinks it's editing (a theme switch mid-edit invalidates the + // preview). + ThemeSlug string `json:"themeSlug"` + // CSSCustomProperties is the ":root { … }" block produced by + // EmitCSSCustomProperties on the merged manifest. The empty string is + // a legitimate result (e.g. a theme with no tokens at all) — the + // renderer treats "" as "render with browser defaults". + CSSCustomProperties string `json:"cssCustomProperties"` +} + +// postPreview handles POST /preview. The flow mirrors putActive without +// the persistence: load the active theme, merge the overrides, run +// validation, and return the emitted CSS. An empty body is treated as +// "preview with no overrides" — the operator wants to see what the +// theme looks like without any of their pending edits, which the admin +// UI uses to drive the "Reset" preview state. +// +// We deliberately don't 400 on validation errors here: previews are +// keystroke-grained and the form often passes through "partially typed" +// values. A bad color becomes an empty entry in the merged manifest, +// not a 400 — the operator's next keystroke usually fixes it. Strict +// validation lives on PUT, which is the operator's explicit "commit" +// gesture. +func (h *handlers) postPreview(w http.ResponseWriter, r *http.Request, _ policy.Principal) { + slug, err := h.store.ActiveThemeSlug(r.Context()) + if err != nil { + if errors.Is(err, ErrNoActiveTheme) { + router.WriteError(w, http.StatusNotFound, "no_active_theme", "no active theme is configured") + return + } + h.logger.ErrorContext(r.Context(), "admin/customizer: active slug lookup failed", slog.Any("err", err)) + router.WriteError(w, http.StatusInternalServerError, "internal_error", "failed to load active theme") + return + } + + manifest, err := h.loader(r.Context(), slug) + if err != nil { + h.logger.ErrorContext(r.Context(), "admin/customizer: theme load failed", + slog.String("slug", slug), + slog.Any("err", err), + ) + router.WriteError(w, http.StatusInternalServerError, "theme_load_failed", + fmt.Sprintf("failed to load theme %q", slug)) + return + } + + r.Body = http.MaxBytesReader(w, r.Body, maxBodyBytes) + raw, err := io.ReadAll(r.Body) + if err != nil { + router.WriteError(w, http.StatusRequestEntityTooLarge, "body_too_large", + fmt.Sprintf("override payload must not exceed %d bytes", maxBodyBytes)) + return + } + + // extractOverrides unwraps an optional `{"overrides": {...}}` envelope + // so the admin client can post either the override or the wrapper. + overrideBytes := extractOverridesPayload(raw) + + merged := cloneTheme(manifest) + // Empty body or empty overrides → preview the unmodified theme. + if len(bytes.TrimSpace(overrideBytes)) > 0 && !bytes.Equal(bytes.TrimSpace(overrideBytes), []byte("{}")) && !bytes.Equal(bytes.TrimSpace(overrideBytes), []byte("null")) { + // Decode loosely (without DisallowUnknownFields): preview is + // best-effort. Unknown keys come from a draft schema in the UI; + // dropping them quietly is friendlier than 400'ing on every + // keystroke that touches a future field. + override := &theme.ThemeJSON{Version: theme.CurrentVersion} + if jerr := json.Unmarshal(overrideBytes, override); jerr != nil { + router.WriteError(w, http.StatusBadRequest, "invalid_json", + "preview overrides are not valid JSON") + return + } + mergeTheme(merged, override) + } + + css := merged.EmitCSSCustomProperties() + router.WriteJSON(w, http.StatusOK, PreviewResponse{ + ThemeSlug: slug, + CSSCustomProperties: css, + }) +} + +// extractOverridesPayload unwraps `{"overrides": }` if present, +// otherwise treats the whole body as the override directly. Returns the +// raw body unchanged when it can't be parsed as JSON — the caller's +// strict decode will surface the syntax error with a useful message. +func extractOverridesPayload(raw []byte) []byte { + trimmed := bytes.TrimSpace(raw) + if len(trimmed) == 0 { + return trimmed + } + var envelope PreviewRequest + if err := json.Unmarshal(trimmed, &envelope); err != nil { + return trimmed + } + if len(envelope.Overrides) > 0 { + return envelope.Overrides + } + return trimmed +} + // writeValidationProblem emits a 400 with the per-path validation // errors in the body. The shape mirrors what other admin surfaces use // for field-level errors: a single "code" identifier plus a flat list diff --git a/apps/api/internal/admin/customizer/handler_test.go b/apps/api/internal/admin/customizer/handler_test.go index a06676d..9eae8e8 100644 --- a/apps/api/internal/admin/customizer/handler_test.go +++ b/apps/api/internal/admin/customizer/handler_test.go @@ -364,3 +364,119 @@ func TestDeleteActive_Idempotent(t *testing.T) { t.Fatalf("status = %d; want 204", w.Code) } } + +// TestPostPreview_NoOverridesReturnsBaseCSS verifies the empty-payload +// path produces the unmodified theme's CSS. The admin "Reset preview" +// button posts {} to land here. +func TestPostPreview_NoOverridesReturnsBaseCSS(t *testing.T) { + store := NewMemoryStore("gn-hello") + router := newRouter(t, store, loaderReturning("gn-hello", baseTheme()), adminPrincipal()) + + req := httptest.NewRequest(http.MethodPost, testBase+"/preview", strings.NewReader(`{}`)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d (body %s); want 200", w.Code, w.Body.String()) + } + var got PreviewResponse + if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { + t.Fatalf("decode: %v", err) + } + if got.ThemeSlug != "gn-hello" { + t.Fatalf("ThemeSlug = %q; want gn-hello", got.ThemeSlug) + } + // Base palette declares 'paper' (#ffffff); the emitted CSS must + // reflect that. + if !strings.Contains(got.CSSCustomProperties, "--wp-preset--color--paper: #ffffff") { + t.Fatalf("base CSS missing paper entry; got %s", got.CSSCustomProperties) + } +} + +// TestPostPreview_AppliesOverrides verifies that posted overrides +// reshape the emitted CSS. The admin form posts the in-flight palette +// edit through this path on every keystroke. +func TestPostPreview_AppliesOverrides(t *testing.T) { + store := NewMemoryStore("gn-hello") + router := newRouter(t, store, loaderReturning("gn-hello", baseTheme()), adminPrincipal()) + + body := `{"settings":{"color":{"palette":[{"slug":"paper","name":"Paper","color":"#abcdef"}]}}}` + req := httptest.NewRequest(http.MethodPost, testBase+"/preview", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d (body %s); want 200", w.Code, w.Body.String()) + } + var got PreviewResponse + if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { + t.Fatalf("decode: %v", err) + } + if !strings.Contains(got.CSSCustomProperties, "--wp-preset--color--paper: #abcdef") { + t.Fatalf("preview CSS missing override; got %s", got.CSSCustomProperties) + } +} + +// TestPostPreview_AcceptsOverridesEnvelope verifies the {"overrides": +// {...}} wrapper shape works too — the older admin form generation +// posts this envelope. +func TestPostPreview_AcceptsOverridesEnvelope(t *testing.T) { + store := NewMemoryStore("gn-hello") + router := newRouter(t, store, loaderReturning("gn-hello", baseTheme()), adminPrincipal()) + + body := `{"overrides":{"settings":{"color":{"palette":[{"slug":"paper","name":"Paper","color":"#112233"}]}}}}` + req := httptest.NewRequest(http.MethodPost, testBase+"/preview", strings.NewReader(body)) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d (body %s); want 200", w.Code, w.Body.String()) + } + var got PreviewResponse + _ = json.Unmarshal(w.Body.Bytes(), &got) + if !strings.Contains(got.CSSCustomProperties, "--wp-preset--color--paper: #112233") { + t.Fatalf("envelope override not applied; got %s", got.CSSCustomProperties) + } +} + +// TestPostPreview_Forbidden ensures the preview surface is gated by the +// same capability as the rest of customizer. +func TestPostPreview_Forbidden(t *testing.T) { + store := NewMemoryStore("gn-hello") + router := newRouter(t, store, loaderReturning("gn-hello", baseTheme()), subscriberPrincipal()) + + req := httptest.NewRequest(http.MethodPost, testBase+"/preview", strings.NewReader("{}")) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusForbidden { + t.Fatalf("status = %d; want 403", w.Code) + } +} + +// TestPostPreview_Unauthenticated guards the 401 path. +func TestPostPreview_Unauthenticated(t *testing.T) { + store := NewMemoryStore("gn-hello") + router := newRouter(t, store, loaderReturning("gn-hello", baseTheme()), nil) + + req := httptest.NewRequest(http.MethodPost, testBase+"/preview", strings.NewReader("{}")) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusUnauthorized { + t.Fatalf("status = %d; want 401", w.Code) + } +} + +// TestPostPreview_InvalidJSON rejects malformed bodies with 400. +func TestPostPreview_InvalidJSON(t *testing.T) { + store := NewMemoryStore("gn-hello") + router := newRouter(t, store, loaderReturning("gn-hello", baseTheme()), adminPrincipal()) + + req := httptest.NewRequest(http.MethodPost, testBase+"/preview", strings.NewReader(`{"settings":`)) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Fatalf("status = %d; want 400", w.Code) + } +} diff --git a/apps/api/internal/admin/siteeditor/handler.go b/apps/api/internal/admin/siteeditor/handler.go new file mode 100644 index 0000000..d684ca1 --- /dev/null +++ b/apps/api/internal/admin/siteeditor/handler.go @@ -0,0 +1,524 @@ +// Package siteeditor implements the admin Site Editor REST surface +// — the file-based variant. Operators editing the header / footer / +// other declared template parts of the active theme list, fetch, and +// write the raw HTML/template bytes through these routes. +// +// This package is intentionally distinct from +// apps/api/internal/admin/site_editor — that one persists block-tree +// overrides in the options table and is wired at /admin/site_editor, +// while this one writes back to themes/{active}/parts/{name}.html for +// the simpler "edit the template part on disk" workflow at the URL the +// admin Site Editor page (apps/admin/.../appearance/site-editor) calls +// when it falls back to raw content editing. +// +// Route tree: +// +// GET /api/v1/admin/site-editor/parts — list parts of the active theme +// GET /api/v1/admin/site-editor/parts/{name} — read one part's content +// PUT /api/v1/admin/site-editor/parts/{name} — write a part's content back to disk +// +// Capability: manage_themes (theme.edit_parts also accepted) — operators +// without it get 403. +package siteeditor + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "log/slog" + "net/http" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/Singleton-Solution/GoNext/apps/api/internal/rest/router" + "github.com/Singleton-Solution/GoNext/packages/go/policy" + "github.com/Singleton-Solution/GoNext/packages/go/theme" +) + +// maxPartBytes caps the per-part request body for PUT writes. The HTML +// for a typical header/footer template part is under 4 KiB; a generous +// 256 KiB ceiling stops a hostile client from chewing on disk without +// bounding what realistic themes can declare. +const maxPartBytes = 256 * 1024 + +// ErrNoActiveTheme is returned by ActiveThemeSlug when no theme row +// exists. Surface as 503 — the editor surface cannot do its job on a +// fresh install before the seeder has run. +var ErrNoActiveTheme = errors.New("siteeditor: no active theme") + +// ActiveResolver returns the active theme slug. Production wires this +// to the same options-table read the rest of the admin theme surfaces +// use; tests supply a closure that returns a fixed slug. +type ActiveResolver func(ctx context.Context) (string, error) + +// ManifestLoader returns the parsed theme manifest for slug. The +// handler reads the manifest to learn which template parts the theme +// declares (theme.json#templateParts is the source of truth — a part +// the theme didn't declare is not editable, even if a file with that +// name happens to exist). +type ManifestLoader func(ctx context.Context, slug string) (*theme.ThemeJSON, error) + +// Deps is the dependency bag for Mount. All fields except Logger are +// required. +type Deps struct { + // ThemeDir is the on-disk root that holds installed themes. The + // handler joins this with the active slug plus "parts" to resolve + // each part file. Typically "./themes" in dev and "/var/lib/gonext/ + // themes" in containers. + ThemeDir string + + // Active resolves the active theme slug on each request. Re-reading + // per request means an in-flight theme switch surfaces immediately + // without rebooting the API. + Active ActiveResolver + + // Loader returns the parsed manifest for slug. Production passes + // customizer.FilesystemLoader; tests pass a closure that returns a + // hand-built ThemeJSON value. + Loader ManifestLoader + + // Policy resolves the manage_themes capability check. + Policy policy.Policy + + // Logger receives structured log lines. nil falls back to + // slog.Default — useful for tests but production wiring should + // always pass a service logger. + Logger *slog.Logger +} + +func (d Deps) validate() error { + if strings.TrimSpace(d.ThemeDir) == "" { + return errors.New("admin/siteeditor: ThemeDir is required") + } + if d.Active == nil { + return errors.New("admin/siteeditor: Active resolver is required") + } + if d.Loader == nil { + return errors.New("admin/siteeditor: Loader is required") + } + if d.Policy == nil { + return errors.New("admin/siteeditor: Policy is required") + } + return nil +} + +// Handler is the resolved-deps form passed around inside the package. +// Built once by Mount and shared across the registered routes. +type Handler struct { + themeDir string + active ActiveResolver + loader ManifestLoader + policy policy.Policy + logger *slog.Logger +} + +// Mount wires the site-editor routes onto mux under base (typically +// "/api/v1/admin/site-editor"). Returns an error rather than panicking +// if Deps is malformed so the boot path surfaces it cleanly. +func Mount(mux *http.ServeMux, base string, deps Deps) (*Handler, error) { + if err := deps.validate(); err != nil { + return nil, err + } + if deps.Logger == nil { + deps.Logger = slog.Default() + } + h := &Handler{ + themeDir: deps.ThemeDir, + active: deps.Active, + loader: deps.Loader, + policy: deps.Policy, + logger: deps.Logger.With(slog.String("component", "admin.siteeditor")), + } + + base = strings.TrimRight(base, "/") + mux.Handle("GET "+base+"/parts", h.gate(h.list)) + mux.Handle("GET "+base+"/parts/{name}", h.gate(h.get)) + mux.Handle("PUT "+base+"/parts/{name}", h.gate(h.put)) + return h, nil +} + +// gate wraps a handler with the auth + manage_themes capability check. +// We accept either CapManageThemes or the narrower CapThemeEditParts — +// operators who hold either may use the site editor. +func (h *Handler) gate(next func(http.ResponseWriter, *http.Request, policy.Principal)) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + pr, ok := policy.FromContext(r.Context()) + if !ok { + router.WriteError(w, http.StatusUnauthorized, "unauthenticated", "authentication required") + return + } + // Either capability authorizes the surface. manage_themes is the + // canonical site-editor cap; theme.edit_parts is the narrower + // surface and exists so a constrained role can still write parts. + if d := h.policy.Can(pr, policy.CapManageThemes, nil); d.Allowed { + next(w, r, pr) + return + } + if d := h.policy.Can(pr, policy.CapThemeEditParts, nil); d.Allowed { + next(w, r, pr) + return + } + router.WriteError(w, http.StatusForbidden, "forbidden", + "requires manage_themes or theme.edit_parts capability") + }) +} + +// Part is one template part as returned by the API. The shape is +// intentionally narrow: name + area metadata + the raw on-disk content. +type Part struct { + // Name is the slug under themes/{slug}/parts/{name}.html. The + // renderer keys off this name when resolving the part. + Name string `json:"name"` + + // Area is the canonical part area declared by theme.json: + // "header", "footer", "sidebar", "uncategorized". The admin UI + // groups the sidebar by area when there are many parts. + Area string `json:"area"` + + // Title is the human-readable label declared in + // theme.json#templateParts. Falls back to Name when empty. + Title string `json:"title,omitempty"` + + // Content is the raw HTML/template bytes from + // themes/{slug}/parts/{name}.html. Empty string when the file + // doesn't exist on disk yet (the editor opens a blank canvas). + Content string `json:"content"` +} + +// listResponse is the GET /parts payload. +type listResponse struct { + Theme string `json:"theme"` + Parts []Part `json:"parts"` +} + +// list handles GET /parts. It walks the active theme's +// templateParts declaration, reads each part's on-disk content, and +// returns the joined list. A missing file is surfaced as an empty +// Content so the editor can open it as a blank canvas — declared but +// not yet authored is a valid state. +func (h *Handler) list(w http.ResponseWriter, r *http.Request, _ policy.Principal) { + slug, err := h.active(r.Context()) + if err != nil { + h.writeActiveError(w, r, err) + return + } + if slug == "" { + router.WriteError(w, http.StatusServiceUnavailable, "no_active_theme", + "no theme is currently active; run the installer or set core.active_theme") + return + } + + manifest, err := h.loader(r.Context(), slug) + if err != nil { + h.logger.ErrorContext(r.Context(), "admin/siteeditor.list: load manifest failed", + slog.String("slug", slug), + slog.Any("err", err), + ) + router.WriteError(w, http.StatusInternalServerError, "theme_load_failed", + fmt.Sprintf("failed to load theme %q", slug)) + return + } + + out := make([]Part, 0, len(manifest.TemplateParts)) + for _, p := range manifest.TemplateParts { + content, rerr := h.readPart(slug, p.Name) + if rerr != nil { + h.logger.ErrorContext(r.Context(), "admin/siteeditor.list: read part failed", + slog.String("slug", slug), + slog.String("part", p.Name), + slog.Any("err", rerr), + ) + // Surface an empty body so the operator can author the part + // from scratch — one corrupt file shouldn't blank the + // sidebar. + content = "" + } + out = append(out, Part{ + Name: p.Name, + Area: canonicalArea(p.Area), + Title: p.Title, + Content: content, + }) + } + // Stable ordering so the admin UI's left-rail list doesn't + // reshuffle between requests. + sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) + + router.WriteJSON(w, http.StatusOK, listResponse{Theme: slug, Parts: out}) +} + +// get handles GET /parts/{name}. Same content read as list, but for a +// single part — used by the admin editor when opening a specific part +// without re-fetching the whole list. +func (h *Handler) get(w http.ResponseWriter, r *http.Request, _ policy.Principal) { + name := r.PathValue("name") + if err := validatePartName(name); err != nil { + router.WriteError(w, http.StatusBadRequest, "invalid_name", err.Error()) + return + } + + slug, err := h.active(r.Context()) + if err != nil { + h.writeActiveError(w, r, err) + return + } + manifest, err := h.loader(r.Context(), slug) + if err != nil { + h.logger.ErrorContext(r.Context(), "admin/siteeditor.get: load manifest failed", + slog.String("slug", slug), + slog.Any("err", err), + ) + router.WriteError(w, http.StatusInternalServerError, "theme_load_failed", + fmt.Sprintf("failed to load theme %q", slug)) + return + } + + decl, ok := findTemplatePart(manifest, name) + if !ok { + router.WriteError(w, http.StatusNotFound, "part_not_found", + "the requested part is not declared by the active theme") + return + } + + content, err := h.readPart(slug, name) + if err != nil { + h.logger.ErrorContext(r.Context(), "admin/siteeditor.get: read part failed", + slog.String("slug", slug), + slog.String("part", name), + slog.Any("err", err), + ) + // Same posture as list — a missing file becomes an empty body. + content = "" + } + + router.WriteJSON(w, http.StatusOK, Part{ + Name: decl.Name, + Area: canonicalArea(decl.Area), + Title: decl.Title, + Content: content, + }) +} + +// putRequest is the body shape for PUT /parts/{name}. +type putRequest struct { + Content string `json:"content"` +} + +// put handles PUT /parts/{name}. The handler writes the supplied +// content back to themes/{slug}/parts/{name}.html, creating the parts +// directory if it doesn't exist. The write is atomic via a temp file + +// rename so a partial write on disk full doesn't leave the operator +// looking at a corrupt header. +// +// Validation: +// - The part name must match the theme's declared templateParts — +// writing an undeclared name returns 404. This is the security +// hinge: it bounds the file path to the set of names the theme +// manifests claim to own. +// - The part name must be a single segment (no '/', '\\', '..', no +// leading/trailing whitespace). Path traversal returns 400. +// - Content size is capped at maxPartBytes. +func (h *Handler) put(w http.ResponseWriter, r *http.Request, pr policy.Principal) { + name := r.PathValue("name") + if err := validatePartName(name); err != nil { + router.WriteError(w, http.StatusBadRequest, "invalid_name", err.Error()) + return + } + + slug, err := h.active(r.Context()) + if err != nil { + h.writeActiveError(w, r, err) + return + } + manifest, err := h.loader(r.Context(), slug) + if err != nil { + h.logger.ErrorContext(r.Context(), "admin/siteeditor.put: load manifest failed", + slog.String("slug", slug), + slog.Any("err", err), + ) + router.WriteError(w, http.StatusInternalServerError, "theme_load_failed", + fmt.Sprintf("failed to load theme %q", slug)) + return + } + decl, ok := findTemplatePart(manifest, name) + if !ok { + router.WriteError(w, http.StatusNotFound, "part_not_found", + "the requested part is not declared by the active theme") + return + } + + r.Body = http.MaxBytesReader(w, r.Body, maxPartBytes) + raw, err := io.ReadAll(r.Body) + if err != nil { + router.WriteError(w, http.StatusRequestEntityTooLarge, "body_too_large", + fmt.Sprintf("part payload must not exceed %d bytes", maxPartBytes)) + return + } + + var req putRequest + if jerr := json.Unmarshal(raw, &req); jerr != nil { + router.WriteError(w, http.StatusBadRequest, "invalid_json", "request body is not valid JSON") + return + } + + if err := h.writePart(slug, name, req.Content); err != nil { + h.logger.ErrorContext(r.Context(), "admin/siteeditor.put: write failed", + slog.String("slug", slug), + slog.String("part", name), + slog.Any("err", err), + ) + router.WriteError(w, http.StatusInternalServerError, "internal_error", + "failed to save part content") + return + } + + h.logger.InfoContext(r.Context(), "admin/siteeditor: part written", + slog.String("slug", slug), + slog.String("part", name), + slog.String("by", pr.UserID), + ) + + router.WriteJSON(w, http.StatusOK, Part{ + Name: decl.Name, + Area: canonicalArea(decl.Area), + Title: decl.Title, + Content: req.Content, + }) +} + +// readPart reads themes/{slug}/parts/{name}.html. A missing file is +// not an error — declared-but-not-yet-authored is a legitimate state +// for the editor, so callers get an empty string instead. +func (h *Handler) readPart(slug, name string) (string, error) { + path := h.partPath(slug, name) + b, err := os.ReadFile(path) + if errors.Is(err, os.ErrNotExist) { + return "", nil + } + if err != nil { + return "", err + } + return string(b), nil +} + +// writePart writes content to themes/{slug}/parts/{name}.html +// atomically. We MkdirAll the parts directory first so a freshly +// installed theme without a parts/ folder still accepts writes. +func (h *Handler) writePart(slug, name, content string) error { + dir := filepath.Join(h.themeDir, slug, "parts") + if err := os.MkdirAll(dir, 0o755); err != nil { + return fmt.Errorf("siteeditor: mkdir %q: %w", dir, err) + } + final := filepath.Join(dir, name+".html") + tmp, err := os.CreateTemp(dir, "."+name+".html.*.tmp") + if err != nil { + return fmt.Errorf("siteeditor: create temp: %w", err) + } + tmpName := tmp.Name() + defer os.Remove(tmpName) // no-op on success after Rename + if _, werr := io.WriteString(tmp, content); werr != nil { + _ = tmp.Close() + return fmt.Errorf("siteeditor: write temp: %w", werr) + } + if cerr := tmp.Close(); cerr != nil { + return fmt.Errorf("siteeditor: close temp: %w", cerr) + } + if rerr := os.Rename(tmpName, final); rerr != nil { + return fmt.Errorf("siteeditor: rename %q -> %q: %w", tmpName, final, rerr) + } + return nil +} + +// partPath joins themeDir/{slug}/parts/{name}.html. Caller is +// responsible for having already validated name (validatePartName + +// findTemplatePart); this function does NOT re-check. +func (h *Handler) partPath(slug, name string) string { + return filepath.Join(h.themeDir, slug, "parts", name+".html") +} + +// writeActiveError translates the small set of "no active theme" +// sentinels into HTTP responses. Anything we don't recognise is logged +// and surfaced as 500. +func (h *Handler) writeActiveError(w http.ResponseWriter, r *http.Request, err error) { + if errors.Is(err, ErrNoActiveTheme) { + router.WriteError(w, http.StatusServiceUnavailable, "no_active_theme", + "no theme is currently active; run the installer or set core.active_theme") + return + } + h.logger.ErrorContext(r.Context(), "admin/siteeditor: active resolver failed", + slog.Any("err", err)) + router.WriteError(w, http.StatusInternalServerError, "internal_error", + "failed to resolve active theme") +} + +// validatePartName guards the file-path projection. The name must be a +// single path segment with no separators, no parent-directory tokens, +// and no leading/trailing whitespace. Returns nil iff the name is safe +// to join with themeDir/{slug}/parts/ without traversing out. +// +// We mirror the same lower-case + alphanumeric + '-' / '_' shape the +// existing site_editor package uses for consistency, but the security +// check is the explicit absence of '/', '\\', '..', and any control +// chars. +func validatePartName(name string) error { + if name == "" { + return errors.New("name is required") + } + if strings.TrimSpace(name) != name { + return errors.New("name must not contain leading or trailing whitespace") + } + if strings.ContainsAny(name, `/\`) { + return errors.New("name must not contain path separators") + } + if name == "." || name == ".." || strings.Contains(name, "..") { + return errors.New("name must not contain parent-directory tokens") + } + for _, ch := range name { + switch { + case ch >= 'a' && ch <= 'z': + case ch >= '0' && ch <= '9': + case ch == '-' || ch == '_': + default: + return errors.New("name must be lower-case alphanumeric with '-' or '_'") + } + } + return nil +} + +// findTemplatePart returns the TemplatePartDef for name iff the theme +// declares it. The boolean is the "is declared" flag; callers map +// false to 404. +func findTemplatePart(t *theme.ThemeJSON, name string) (theme.TemplatePartDef, bool) { + if t == nil { + return theme.TemplatePartDef{}, false + } + for _, p := range t.TemplateParts { + if p.Name == name { + return p, true + } + } + return theme.TemplatePartDef{}, false +} + +// canonicalArea normalizes the theme.json#templateParts.area value into +// the response shape callers expect. The admin UI keys its sidebar by +// the three canonical buckets — "header", "footer", "general" — so an +// empty or unrecognised area becomes "general" in the response. +// +// "uncategorized" (the theme.json default for non-header/non-footer +// parts) collapses to "general" too, because the admin UI only knows +// the three labels and a fourth would render as an empty group header. +func canonicalArea(area string) string { + switch strings.ToLower(strings.TrimSpace(area)) { + case "header": + return "header" + case "footer": + return "footer" + default: + return "general" + } +} diff --git a/apps/api/internal/admin/siteeditor/handler_test.go b/apps/api/internal/admin/siteeditor/handler_test.go new file mode 100644 index 0000000..f683724 --- /dev/null +++ b/apps/api/internal/admin/siteeditor/handler_test.go @@ -0,0 +1,465 @@ +package siteeditor + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Singleton-Solution/GoNext/packages/go/policy" + "github.com/Singleton-Solution/GoNext/packages/go/theme" +) + +const testBase = "/api/v1/admin/site-editor" + +// adminPrincipal returns a Principal whose roles grant manage_themes. +func adminPrincipal() *policy.Principal { + return &policy.Principal{UserID: "user:admin", Roles: []policy.Role{policy.RoleAdmin}} +} + +// subscriberPrincipal returns a Principal whose roles grant nothing the +// site editor cares about. Used by the 403 test. +func subscriberPrincipal() *policy.Principal { + return &policy.Principal{UserID: "user:joe", Roles: []policy.Role{policy.RoleSubscriber}} +} + +// fixtureTheme returns the manifest the tests share. Declares two +// template parts (header + footer) so the editor surface has something +// to enumerate. +func fixtureTheme() *theme.ThemeJSON { + return &theme.ThemeJSON{ + Version: theme.CurrentVersion, + Title: "gn-test", + TemplateParts: []theme.TemplatePartDef{ + {Name: "header", Title: "Header", Area: "header"}, + {Name: "footer", Title: "Footer", Area: "footer"}, + }, + } +} + +// seedThemeDir lays down a temp directory with the structure +// $TMP/{slug}/parts/{header,footer}.html and returns the root path. +// Each part starts with a short marker so the read tests can assert +// what they're getting back. +func seedThemeDir(t *testing.T, slug string) string { + t.Helper() + root := t.TempDir() + partsDir := filepath.Join(root, slug, "parts") + if err := os.MkdirAll(partsDir, 0o755); err != nil { + t.Fatalf("mkdir parts: %v", err) + } + if err := os.WriteFile(filepath.Join(partsDir, "header.html"), + []byte("
HEADER FIXTURE
"), 0o644); err != nil { + t.Fatalf("seed header: %v", err) + } + if err := os.WriteFile(filepath.Join(partsDir, "footer.html"), + []byte("
FOOTER FIXTURE
"), 0o644); err != nil { + t.Fatalf("seed footer: %v", err) + } + return root +} + +// newRouter returns an *http.ServeMux wrapped in an inline middleware +// that stashes principal on the context. Mirrors what production's auth +// middleware does for real requests. +func newRouter(t *testing.T, themeDir, slug string, manifest *theme.ThemeJSON, principal *policy.Principal) http.Handler { + t.Helper() + mux := http.NewServeMux() + deps := Deps{ + ThemeDir: themeDir, + Active: func(context.Context) (string, error) { + if slug == "" { + return "", ErrNoActiveTheme + } + return slug, nil + }, + Loader: func(_ context.Context, requested string) (*theme.ThemeJSON, error) { + if requested != slug { + return nil, ErrNoActiveTheme + } + return manifest, nil + }, + Policy: policy.NewBasicPolicy(policy.DefaultRoleCapabilities()), + } + if _, err := Mount(mux, testBase, deps); err != nil { + t.Fatalf("Mount: %v", err) + } + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if principal != nil { + r = r.WithContext(policy.WithPrincipal(r.Context(), *principal)) + } + mux.ServeHTTP(w, r) + }) +} + +// TestMount_Validation covers the boot-time errors callers see when +// Deps is malformed. +func TestMount_Validation(t *testing.T) { + pol := policy.NewBasicPolicy(policy.DefaultRoleCapabilities()) + active := func(context.Context) (string, error) { return "x", nil } + loader := func(context.Context, string) (*theme.ThemeJSON, error) { + return fixtureTheme(), nil + } + + cases := map[string]Deps{ + "missing_dir": {Active: active, Loader: loader, Policy: pol}, + "missing_active": {ThemeDir: "/tmp", Loader: loader, Policy: pol}, + "missing_loader": {ThemeDir: "/tmp", Active: active, Policy: pol}, + "missing_policy": {ThemeDir: "/tmp", Active: active, Loader: loader}, + } + for name, d := range cases { + t.Run(name, func(t *testing.T) { + mux := http.NewServeMux() + if _, err := Mount(mux, testBase, d); err == nil { + t.Fatalf("expected Mount to fail") + } + }) + } + + t.Run("ok", func(t *testing.T) { + mux := http.NewServeMux() + if _, err := Mount(mux, testBase, Deps{ + ThemeDir: "/tmp", Active: active, Loader: loader, Policy: pol, + }); err != nil { + t.Fatalf("Mount(ok): %v", err) + } + }) +} + +// TestListParts_HappyPath verifies the GET /parts response. +func TestListParts_HappyPath(t *testing.T) { + dir := seedThemeDir(t, "gn-test") + router := newRouter(t, dir, "gn-test", fixtureTheme(), adminPrincipal()) + + req := httptest.NewRequest(http.MethodGet, testBase+"/parts", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d (body %s); want 200", w.Code, w.Body.String()) + } + + var got listResponse + if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { + t.Fatalf("decode: %v", err) + } + if got.Theme != "gn-test" { + t.Fatalf("Theme = %q; want gn-test", got.Theme) + } + if len(got.Parts) != 2 { + t.Fatalf("Parts = %d; want 2", len(got.Parts)) + } + // Sorted alphabetically -> footer first. + if got.Parts[0].Name != "footer" { + t.Fatalf("Parts[0].Name = %q; want footer", got.Parts[0].Name) + } + if !strings.Contains(got.Parts[0].Content, "FOOTER FIXTURE") { + t.Fatalf("Parts[0].Content missing fixture: %s", got.Parts[0].Content) + } + if got.Parts[0].Area != "footer" { + t.Fatalf("Parts[0].Area = %q; want footer", got.Parts[0].Area) + } + if got.Parts[1].Name != "header" { + t.Fatalf("Parts[1].Name = %q; want header", got.Parts[1].Name) + } + if !strings.Contains(got.Parts[1].Content, "HEADER FIXTURE") { + t.Fatalf("Parts[1].Content missing fixture: %s", got.Parts[1].Content) + } +} + +// TestListParts_NoActiveTheme covers the fresh-deploy case. +func TestListParts_NoActiveTheme(t *testing.T) { + dir := t.TempDir() + router := newRouter(t, dir, "", fixtureTheme(), adminPrincipal()) + + req := httptest.NewRequest(http.MethodGet, testBase+"/parts", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusServiceUnavailable { + t.Fatalf("status = %d; want 503", w.Code) + } +} + +// TestListParts_Forbidden ensures non-admin reads are refused. +func TestListParts_Forbidden(t *testing.T) { + dir := seedThemeDir(t, "gn-test") + router := newRouter(t, dir, "gn-test", fixtureTheme(), subscriberPrincipal()) + + req := httptest.NewRequest(http.MethodGet, testBase+"/parts", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusForbidden { + t.Fatalf("status = %d; want 403", w.Code) + } +} + +// TestListParts_Unauthenticated guards the gate's 401 path. +func TestListParts_Unauthenticated(t *testing.T) { + dir := seedThemeDir(t, "gn-test") + router := newRouter(t, dir, "gn-test", fixtureTheme(), nil) + + req := httptest.NewRequest(http.MethodGet, testBase+"/parts", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusUnauthorized { + t.Fatalf("status = %d; want 401", w.Code) + } +} + +// TestGetPart_HappyPath verifies fetching a single declared part. +func TestGetPart_HappyPath(t *testing.T) { + dir := seedThemeDir(t, "gn-test") + router := newRouter(t, dir, "gn-test", fixtureTheme(), adminPrincipal()) + + req := httptest.NewRequest(http.MethodGet, testBase+"/parts/header", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d; want 200", w.Code) + } + var got Part + if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { + t.Fatalf("decode: %v", err) + } + if got.Name != "header" { + t.Fatalf("Name = %q; want header", got.Name) + } + if !strings.Contains(got.Content, "HEADER FIXTURE") { + t.Fatalf("Content missing fixture: %s", got.Content) + } +} + +// TestGetPart_UndeclaredName returns 404 even if a file with the name +// exists on disk — the theme's declaration is the source of truth. +func TestGetPart_UndeclaredName(t *testing.T) { + dir := seedThemeDir(t, "gn-test") + // Drop a rogue file. The handler must not surface it. + if err := os.WriteFile(filepath.Join(dir, "gn-test", "parts", "secret.html"), + []byte("nope"), 0o644); err != nil { + t.Fatalf("seed rogue: %v", err) + } + router := newRouter(t, dir, "gn-test", fixtureTheme(), adminPrincipal()) + + req := httptest.NewRequest(http.MethodGet, testBase+"/parts/secret", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusNotFound { + t.Fatalf("status = %d (body %s); want 404", w.Code, w.Body.String()) + } +} + +// TestGetPart_PathTraversal returns 400 — the name must be a single +// segment. The browser would already URL-encode '/' but a hostile +// client could omit that, so we reject explicitly. +func TestGetPart_PathTraversal(t *testing.T) { + dir := seedThemeDir(t, "gn-test") + router := newRouter(t, dir, "gn-test", fixtureTheme(), adminPrincipal()) + + // Use a name with parent-directory tokens AFTER URL decoding to + // confirm we reject. The router won't route ".." through path + // values without encoding, but encoded variants land in PathValue. + cases := []string{ + "..%2Fetc", // ../etc with encoded slash → name becomes "../etc" + "%2E%2E%2Fpwd", // ../pwd fully encoded + } + for _, raw := range cases { + t.Run(raw, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, testBase+"/parts/"+raw, nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Fatalf("raw=%q status = %d (body %s); want 400", raw, w.Code, w.Body.String()) + } + }) + } +} + +// TestPutPart_WritesFile verifies the happy-path PUT — content goes to +// disk and a re-read sees it. +func TestPutPart_WritesFile(t *testing.T) { + dir := seedThemeDir(t, "gn-test") + router := newRouter(t, dir, "gn-test", fixtureTheme(), adminPrincipal()) + + body := `{"content":"
EDITED
"}` + req := httptest.NewRequest(http.MethodPut, testBase+"/parts/header", + strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d (body %s); want 200", w.Code, w.Body.String()) + } + + // Disk reflects the write. + got, err := os.ReadFile(filepath.Join(dir, "gn-test", "parts", "header.html")) + if err != nil { + t.Fatalf("read back: %v", err) + } + if string(got) != "
EDITED
" { + t.Fatalf("file = %q; want edited", string(got)) + } + + // And a follow-up GET surfaces the new content. + req = httptest.NewRequest(http.MethodGet, testBase+"/parts/header", nil) + w = httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("re-read status = %d", w.Code) + } + var part Part + _ = json.Unmarshal(w.Body.Bytes(), &part) + if part.Content != "
EDITED
" { + t.Fatalf("re-read content = %q; want edited", part.Content) + } +} + +// TestPutPart_CreatesMissingFile verifies that a declared part without +// a file on disk yet can be written through the editor — the parts +// directory is mkdir-all'd if needed. +func TestPutPart_CreatesMissingFile(t *testing.T) { + root := t.TempDir() + // Don't seed any parts; the manifest declares them but disk is empty. + router := newRouter(t, root, "gn-test", fixtureTheme(), adminPrincipal()) + + body := `{"content":"
NEW
"}` + req := httptest.NewRequest(http.MethodPut, testBase+"/parts/footer", + strings.NewReader(body)) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("status = %d (body %s); want 200", w.Code, w.Body.String()) + } + + got, err := os.ReadFile(filepath.Join(root, "gn-test", "parts", "footer.html")) + if err != nil { + t.Fatalf("read back: %v", err) + } + if string(got) != "
NEW
" { + t.Fatalf("file = %q", string(got)) + } +} + +// TestPutPart_UndeclaredName returns 404 — a write must target a part +// the theme declares. This is the security hinge against a write-only +// graveyard. +func TestPutPart_UndeclaredName(t *testing.T) { + dir := seedThemeDir(t, "gn-test") + router := newRouter(t, dir, "gn-test", fixtureTheme(), adminPrincipal()) + + body := `{"content":"x"}` + req := httptest.NewRequest(http.MethodPut, testBase+"/parts/sidebar", + strings.NewReader(body)) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusNotFound { + t.Fatalf("status = %d (body %s); want 404", w.Code, w.Body.String()) + } + // No file should have been created. + if _, err := os.Stat(filepath.Join(dir, "gn-test", "parts", "sidebar.html")); !os.IsNotExist(err) { + t.Fatalf("file created despite 404: %v", err) + } +} + +// TestPutPart_PathTraversal rejects ".."-laden names. Even if the +// theme manifest mentioned one (which it shouldn't), the +// validatePartName check would still catch it. +func TestPutPart_PathTraversal(t *testing.T) { + dir := seedThemeDir(t, "gn-test") + router := newRouter(t, dir, "gn-test", fixtureTheme(), adminPrincipal()) + + body := `{"content":"x"}` + req := httptest.NewRequest(http.MethodPut, testBase+"/parts/..%2Fevil", + strings.NewReader(body)) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Fatalf("status = %d; want 400", w.Code) + } +} + +// TestPutPart_InvalidJSON rejects malformed bodies with 400. +func TestPutPart_InvalidJSON(t *testing.T) { + dir := seedThemeDir(t, "gn-test") + router := newRouter(t, dir, "gn-test", fixtureTheme(), adminPrincipal()) + + req := httptest.NewRequest(http.MethodPut, testBase+"/parts/header", + strings.NewReader(`not json`)) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Fatalf("status = %d; want 400", w.Code) + } +} + +// TestPutPart_Forbidden ensures non-admin writes are refused. +func TestPutPart_Forbidden(t *testing.T) { + dir := seedThemeDir(t, "gn-test") + router := newRouter(t, dir, "gn-test", fixtureTheme(), subscriberPrincipal()) + + body := `{"content":"x"}` + req := httptest.NewRequest(http.MethodPut, testBase+"/parts/header", + strings.NewReader(body)) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != http.StatusForbidden { + t.Fatalf("status = %d; want 403", w.Code) + } +} + +// TestValidatePartName_Rejects covers the edge inputs the path-traversal +// test relies on — verifies the standalone validator behaves the way the +// handler tests assume. +func TestValidatePartName_Rejects(t *testing.T) { + bad := []string{ + "", + "..", + ".", + "../etc", + "foo/bar", + "foo\\bar", + "FOO", // case + " foo ", // whitespace + "foo.bar", + "foo..bar", + } + for _, name := range bad { + t.Run(name, func(t *testing.T) { + if err := validatePartName(name); err == nil { + t.Fatalf("expected error for %q", name) + } + }) + } + good := []string{"header", "footer", "post-meta", "sidebar_left", "abc123"} + for _, name := range good { + t.Run(name, func(t *testing.T) { + if err := validatePartName(name); err != nil { + t.Fatalf("unexpected error for %q: %v", name, err) + } + }) + } +} + +// TestCanonicalArea folds the various theme.json area shapes into the +// three buckets the admin UI knows. +func TestCanonicalArea(t *testing.T) { + cases := map[string]string{ + "header": "header", + "HEADER": "header", + "footer": "footer", + "uncategorized": "general", + "": "general", + "sidebar": "general", + " ": "general", + } + for in, want := range cases { + if got := canonicalArea(in); got != want { + t.Errorf("canonicalArea(%q) = %q; want %q", in, got, want) + } + } +} From d849d004a5de4dab5940295820ff77000854e1ea Mon Sep 17 00:00:00 2001 From: Tayeb Mokni Date: Thu, 28 May 2026 13:02:48 +0200 Subject: [PATCH 4/4] feat(admin+api): wire admin Pages module to real API (#506) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pages list was hardcoded \`SEED_PAGES\` and the detail save handler was \`console.log\`. Replace both with real CRUD against the existing /api/v1/posts surface. ### API: post_type filter apps/api/internal/rest/posts/model.go grows a \`PostType\` field on ListFilter. handlers.go's parseListQuery accepts \`?post_type=post|page\` with closed-set validation (anything else → 400 invalid_post_type). list() resolves the discriminator at request time: filter override wins over the mount's default PostType. This lets the single /api/v1/posts mount serve both Posts list and Pages list — no second mount required. ### Admin Pages list apps/admin/src/app/(authenticated)/pages/page.tsx: - Drops SEED_PAGES entirely. - Server Component fetching \`/api/v1/posts?post_type=page&status=any\` via serverApiFetch (cookie-forwarding). - Exported \`adaptApiPage\` and \`ApiPagePost\` types — testable in isolation. - Status normalization (UI \`published\` ↔ API \`publish\`). - Updated-at fallback chain: updated_at → published_at → created_at. - Empty/error states stay inside the table card so layout doesn't shift. ### Admin Pages detail apps/admin/src/app/(authenticated)/pages/[id]/page.tsx: - Replaced console.log save with \`api.patch('/api/v1/posts/{id}', {title, slug, status})\`. - UI status maps at the boundary (\`publish\` → \`published\` on the wire). - Try/catch with ApiError branch surfaces an amber error banner. - Success indicator: 3s "Saved" pip next to the Save button. - Block editor button stays disabled (separate issue). ### Tests 22 new tests across pages/page + pages/[id]/page covering: adapter status normalization, updated-at fallback, untitled placeholder, slug passthrough, PATCH URL + body shape, status mapping, ApiError path, generic Error path. 638/638 admin tests pass. API: TestList_FilterByPostType + TestList_FilterByPostType_InvalidRejected. ### Known follow-up The /pages/new flow built in PR #528 POSTs {post_type:'page'} in the body but the create handler is hard-coded to PostTypePost (ignores body). New pages today insert as post_type='post' and disappear from the pages list. Body-override on create is a small follow-up. Closes #506 (list + save). Create-side gap tracked separately. Co-Authored-By: Claude Opus 4.7 Signed-off-by: Tayeb Mokni --- .../[id]/__snapshots__/page.test.tsx.snap | 3 +- .../(authenticated)/pages/[id]/page.test.tsx | 119 ++++- .../app/(authenticated)/pages/[id]/page.tsx | 128 +++++- .../pages/__snapshots__/page.test.tsx.snap | 55 +-- .../app/(authenticated)/pages/page.test.tsx | 123 ++++-- .../src/app/(authenticated)/pages/page.tsx | 415 +++++++++++------- apps/api/internal/rest/posts/handlers.go | 23 +- apps/api/internal/rest/posts/handlers_test.go | 41 ++ apps/api/internal/rest/posts/model.go | 12 + 9 files changed, 661 insertions(+), 258 deletions(-) diff --git a/apps/admin/src/app/(authenticated)/pages/[id]/__snapshots__/page.test.tsx.snap b/apps/admin/src/app/(authenticated)/pages/[id]/__snapshots__/page.test.tsx.snap index 4e2a42e..3864267 100644 --- a/apps/admin/src/app/(authenticated)/pages/[id]/__snapshots__/page.test.tsx.snap +++ b/apps/admin/src/app/(authenticated)/pages/[id]/__snapshots__/page.test.tsx.snap @@ -54,7 +54,7 @@ exports[`Page detail page > matches the page-head snapshot 1`] = `

matches the page-head snapshot 1`] = `
-
+
+ {savedAt !== null ? ( + + + ) : null}
+ {saveError ? ( +

+ {saveError} +

+ ) : null}
@@ -185,7 +275,7 @@ export default function PageDetailPage(): ReactElement { Created - 2026-04-02 11:30 + —
  • @@ -194,7 +284,7 @@ export default function PageDetailPage(): ReactElement { Updated - 3 days ago + —
  • @@ -209,7 +299,7 @@ export default function PageDetailPage(): ReactElement {
    • diff --git a/apps/admin/src/app/(authenticated)/pages/__snapshots__/page.test.tsx.snap b/apps/admin/src/app/(authenticated)/pages/__snapshots__/page.test.tsx.snap index ce9bd59..72b6784 100644 --- a/apps/admin/src/app/(authenticated)/pages/__snapshots__/page.test.tsx.snap +++ b/apps/admin/src/app/(authenticated)/pages/__snapshots__/page.test.tsx.snap @@ -1,50 +1,13 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`Pages list page > matches the page-head snapshot 1`] = ` -
    matches the page-head snapshot 1`] = ` +

    -
    -

    - All - - pages - - . -

    -

    - Evergreen content — about, contact, policy. Edit metadata or open the block editor for layout-heavy pages. -

    -
    - - - New page - -

    + All + + pages + + . + `; diff --git a/apps/admin/src/app/(authenticated)/pages/page.test.tsx b/apps/admin/src/app/(authenticated)/pages/page.test.tsx index 7ed961a..06c3f3b 100644 --- a/apps/admin/src/app/(authenticated)/pages/page.test.tsx +++ b/apps/admin/src/app/(authenticated)/pages/page.test.tsx @@ -1,55 +1,102 @@ /** - * Pages list snapshot tests. + * Pages list — adapter + page-head tests (issue #506). * - * Pins the brand chrome (italic-accent headline, filter chip strip, - * table structure) without touching the underlying data — the data - * is a static seed until the pages REST endpoint ships. + * The page itself is a Server Component that hits the network through + * `serverApiFetch`, so we can't render the whole tree synchronously + * here — same constraint the posts list page lives under. Two + * surfaces are tested instead: + * + * 1. `adaptApiPage` — the wire→UI shape adapter. Exercising it in + * isolation makes the field-mapping rules easy to pin (status + * normalisation, updated-at fallback chain). + * 2. The brand headline — rendered through a minimal Headline + * harness so the italic-accent ("All *pages*.") doesn't drift. */ -import { describe, expect, it, vi } from 'vitest'; -import { render, screen } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; +import { render } from '@testing-library/react'; +import { Headline } from '@/components/ui/headline'; +import { adaptApiPage, type ApiPagePost } from './page'; + +describe('adaptApiPage — wire shape → list-UI shape', () => { + it('normalises the API "published" status onto the WP-classic "publish" label', () => { + const apiPage: ApiPagePost = { + id: 'about', + title: 'About', + slug: '/about', + status: 'published', + updated_at: '2026-04-10T12:34:56Z', + }; + expect(adaptApiPage(apiPage).status).toBe('publish'); + }); + + it('normalises "scheduled" onto "future" so the badge logic matches the rest of the admin', () => { + const apiPage: ApiPagePost = { + id: 'launch', + title: 'Launch', + status: 'scheduled', + }; + expect(adaptApiPage(apiPage).status).toBe('future'); + }); + + it('falls back to draft for unknown statuses', () => { + const apiPage: ApiPagePost = { id: 'x', title: 'X', status: 'bogus' }; + expect(adaptApiPage(apiPage).status).toBe('draft'); + }); -vi.mock('next/navigation', () => ({ - usePathname: () => '/pages', - useRouter: () => ({ - push: vi.fn(), - replace: vi.fn(), - prefetch: vi.fn(), - refresh: vi.fn(), - }), - useSearchParams: () => new URLSearchParams(), -})); + it('prefers updated_at, then published_at, then created_at for the Updated cell', () => { + expect( + adaptApiPage({ + id: 'a', + updated_at: '2026-04-10T00:00:00Z', + published_at: '2026-01-01T00:00:00Z', + }).updatedAt, + ).toBe('2026-04-10T00:00:00Z'); -import PagesPage from './page'; + expect( + adaptApiPage({ + id: 'b', + published_at: '2026-02-02T00:00:00Z', + }).updatedAt, + ).toBe('2026-02-02T00:00:00Z'); -describe('Pages list page', () => { - it('renders the italic-accent headline', () => { - render(); - const h1 = screen.getByRole('heading', { level: 1 }); - expect(h1.textContent).toMatch(/All\s+pages\./); - expect(h1.querySelector('em')?.textContent).toBe('pages'); + expect( + adaptApiPage({ + id: 'c', + created_at: '2026-03-03T00:00:00Z', + }).updatedAt, + ).toBe('2026-03-03T00:00:00Z'); }); - it('renders the New page primary CTA', () => { - render(); - const cta = screen.getByRole('link', { name: /New page/i }); - expect(cta).toHaveAttribute('href', '/pages/new'); + it('emits "(untitled)" for empty or whitespace titles so the list cell is never blank', () => { + expect(adaptApiPage({ id: 'x', title: '' }).title).toBe('(untitled)'); + expect(adaptApiPage({ id: 'y', title: ' ' }).title).toBe('(untitled)'); }); - it('renders the filter chip strip', () => { - render(); - const group = screen.getByRole('group', { name: /Filter by status/i }); - expect(group).toBeInTheDocument(); + it('passes the slug through verbatim and defaults to empty string', () => { + expect(adaptApiPage({ id: 'x', slug: '/about' }).slug).toBe('/about'); + expect(adaptApiPage({ id: 'y' }).slug).toBe(''); }); +}); - it('renders the pages table', () => { - render(); - const table = screen.getByRole('table', { name: 'Pages' }); - expect(table).toBeInTheDocument(); +describe('Pages list page head — brand fixture', () => { + it('renders the italic-accent headline ("All *pages*.")', () => { + const { container } = render( + + All pages. + , + ); + const h1 = container.querySelector('h1'); + expect(h1).not.toBeNull(); + expect(h1?.textContent).toMatch(/All\s+pages\./); + expect(h1?.querySelector('em')?.textContent).toBe('pages'); }); it('matches the page-head snapshot', () => { - const { container } = render(); - const head = container.querySelector('[data-testid="pages-page"] > div'); - expect(head).toMatchSnapshot(); + const { container } = render( + + All pages. + , + ); + expect(container.firstChild).toMatchSnapshot(); }); }); diff --git a/apps/admin/src/app/(authenticated)/pages/page.tsx b/apps/admin/src/app/(authenticated)/pages/page.tsx index 282d561..52d13a3 100644 --- a/apps/admin/src/app/(authenticated)/pages/page.tsx +++ b/apps/admin/src/app/(authenticated)/pages/page.tsx @@ -1,80 +1,99 @@ /** - * Pages — admin list screen for site pages. + * Pages — admin list screen (issue #506). * - * Mirrors the posts surface (docs/05-admin-api.md §2.3) — pages are - * the "evergreen" content type, distinct from time-stamped posts. The - * shape of the screen (filters, table, pagination) is identical so - * the IA stays predictable; the data set is just narrowed to the - * `page` post-type on the API side. + * Pages are the "evergreen" content type — distinct from time-stamped + * posts but stored in the same table (post_type='page'). The + * `/api/v1/pages` mount isn't wired yet (see deps.go), so we filter + * the posts endpoint with the new `post_type=page` query param that + * the handler now accepts. * - * The pages REST endpoint is tracked in issue #76. Until it lands - * this page renders the empty/error state — the same pattern the - * posts page uses to stay defensible. + * Architecture + * ============ + * Server Component that fetches the first page directly from the API + * and renders the list inline — there's no client island here because + * the pages surface doesn't need bulk actions / sortable columns the + * way posts do. When the block editor for pages lands the per-row + * "Edit" link will route to the dedicated editor, but the list itself + * stays render-on-server. * - * Brand treatment ("Living systems"): display-type headline with the - * italic-serif accent ("All *pages*."), an emerald-soft active filter - * chip strip, and paper-3 row hover. Matches the moodboard pattern - * in `docs/design/ui_kits/admin/index.html`. + * Auth: session cookies are forwarded via `serverApiFetch` (the same + * pattern posts uses). Without that the API would 401 every render. + * + * Brand treatment: "All *pages*." italic-accent display headline + + * emerald-soft active filter chip + paper-3 row hover. The chrome + * around the table is identical to the posts list so the IA stays + * predictable. */ import Link from 'next/link'; -import type { ReactElement } from 'react'; +import { Suspense, type ReactElement } from 'react'; import { Plus } from 'lucide-react'; +import { serverApiFetch } from '@/lib/server-api'; import { Headline } from '@/components/ui/headline'; import { Button } from '@/components/ui/button'; import { Badge } from '@/components/ui/badge'; export const dynamic = 'force-dynamic'; -interface SitePage { +/** Status values the page list surfaces. Mirrors the WP-compat set + * the API speaks. The API uses `publish`/`scheduled`/`draft`; we + * normalise scheduled to `future` so the badge logic stays aligned + * with the post detail surface. */ +export type PageStatus = 'publish' | 'draft' | 'future' | 'private' | 'trash'; + +/** Flatter shape the list UI renders. Pulled out of the adapter so + * the tests can exercise the field-mapping rules without spinning up + * the whole server component. */ +export interface SitePage { id: string; title: string; slug: string; - status: 'publish' | 'draft' | 'future'; + status: PageStatus; + /** ISO8601 or pre-formatted string — displayed as-is. */ updatedAt: string; } -/** Seed pages until the API endpoint lands. The brand is more - * important than the data — once #76 ships this list comes from - * `GET /api/v1/posts?type=page`. */ -const SEED_PAGES: readonly SitePage[] = [ - { - id: 'about', - title: 'About', - slug: '/about', - status: 'publish', - updatedAt: '3 days ago', - }, - { - id: 'contact', - title: 'Contact', - slug: '/contact', - status: 'publish', - updatedAt: '2 weeks ago', - }, - { - id: 'privacy', - title: 'Privacy policy', - slug: '/privacy', - status: 'publish', - updatedAt: '1 month ago', - }, - { - id: 'shipping', - title: 'Shipping & returns', - slug: '/shipping', - status: 'draft', - updatedAt: 'Yesterday', - }, - { - id: 'press', - title: 'Press kit', - slug: '/press', - status: 'future', - updatedAt: 'Mar 12, 10:00', - }, -]; +/** Wire shape we expect from `GET /api/v1/posts?post_type=page`. */ +export type ApiPagePost = { + id: string; + title?: string; + slug?: string; + status?: string; + published_at?: string | null; + updated_at?: string; + created_at?: string; +}; + +/** + * Adapt an API page-post into the flatter `SitePage` shape the + * list UI expects. + * + * Status: the API speaks `published`/`scheduled`/`draft`; the badge + * helpers downstream want `publish`/`future`/`draft` (the WP-classic + * names the rest of the admin uses), so we normalise here. + * + * Updated label: prefer `updated_at` so an in-progress draft surfaces + * its most-recent-change time. Fall back to `published_at` (covers + * historic rows that only carry a publish stamp), then `created_at`. + */ +export function adaptApiPage(p: ApiPagePost): SitePage { + const rawStatus = p.status ?? 'draft'; + let status: PageStatus = 'draft'; + if (rawStatus === 'published' || rawStatus === 'publish') status = 'publish'; + else if (rawStatus === 'scheduled' || rawStatus === 'future') status = 'future'; + else if (rawStatus === 'private') status = 'private'; + else if (rawStatus === 'trash') status = 'trash'; + else status = 'draft'; + + return { + id: p.id, + title: p.title?.trim() || '(untitled)', + slug: p.slug ?? '', + status, + updatedAt: p.updated_at ?? p.published_at ?? p.created_at ?? '', + }; +} -function PageStatus({ status }: { status: SitePage['status'] }): ReactElement { +function PageStatusBadge({ status }: { status: PageStatus }): ReactElement { if (status === 'publish') { return ( @@ -89,31 +108,67 @@ function PageStatus({ status }: { status: SitePage['status'] }): ReactElement { ); } + if (status === 'private') { + return ( + + Private + + ); + } + if (status === 'trash') { + return Trashed; + } return Draft; } -export default function PagesPage(): ReactElement { - return ( -
    - {/* ─── Page head ─── */} -
    -
    - - All pages. - -

    - Evergreen content — about, contact, policy. Edit metadata or open - the block editor for layout-heavy pages. -

    -
    - -
    +interface FetchResult { + rows: SitePage[]; + total: number; + error: string | null; +} + +/** + * Server-side fetch helper. Returns a friendly empty `rows` array on + * any failure so the caller can render a degraded state rather than + * crash. Mirrors the posts/page.tsx defensive shape. + */ +async function fetchPages(): Promise { + try { + const res = await serverApiFetch( + '/api/v1/posts?post_type=page&status=any&limit=20', + ); + if (!res.ok) { + return { rows: [], total: 0, error: `HTTP ${res.status}` }; + } + type Envelope = { + data?: ApiPagePost[]; + posts?: ApiPagePost[]; + pagination?: { next_cursor?: string }; + total?: number; + }; + const json = (await res.json()) as Envelope; + const list = Array.isArray(json.data) + ? json.data + : Array.isArray(json.posts) + ? json.posts + : []; + const rows = list.map(adaptApiPage); + return { + rows, + total: typeof json.total === 'number' ? json.total : rows.length, + error: null, + }; + } catch (err) { + const reason = err instanceof Error ? err.message : 'network error'; + return { rows: [], total: 0, error: reason }; + } +} +async function PagesListServer(): Promise { + const { rows, total, error } = await fetchPages(); + + return ( + <> {/* ─── Filter strip ─── */}
    @@ -157,84 +212,146 @@ export default function PagesPage(): ReactElement {
    - {/* ─── Table ─── */} + {/* ─── Table / empty / error ─── */}
    - - - - - - - - - - - - {SEED_PAGES.map((page) => ( - - - - - - + {error ? ( +
    + Couldn't load pages ({error}). Try reloading; if the + problem persists the API may not be reachable yet. +
    + ) : rows.length === 0 ? ( +
    + No pages yet. Create one to get started. +
    + ) : ( +
    - Title - - Slug - - Status - - Updated - - Actions -
    - - {page.title} - - - {page.slug} - - - - {page.updatedAt} - - - Edit → - -
    + + + + + + + - ))} - -
    + Title + + Slug + + Status + + Updated + + Actions +
    + + + {rows.map((page) => ( + + + + {page.title} + + + + {page.slug || '—'} + + + + + + {page.updatedAt || '—'} + + + + Edit → + + + + ))} + + + )}
    - Showing {SEED_PAGES.length} of {SEED_PAGES.length} + Showing {rows.length} of {total}
    + + ); +} + +/** Loading skeleton for the Suspense fallback. Same shape as the + * empty/error states so the layout doesn't shift on first paint. */ +function PagesSkeleton(): ReactElement { + return ( +
    + Loading pages… +
    + ); +} + +export default function PagesPage(): ReactElement { + return ( +
    + {/* ─── Page head ─── */} +
    +
    + + All pages. + +

    + Evergreen content — about, contact, policy. Edit metadata or open + the block editor for layout-heavy pages. +

    +
    + +
    + + }> + +
    ); } diff --git a/apps/api/internal/rest/posts/handlers.go b/apps/api/internal/rest/posts/handlers.go index fc0dded..ea8c219 100644 --- a/apps/api/internal/rest/posts/handlers.go +++ b/apps/api/internal/rest/posts/handlers.go @@ -119,7 +119,16 @@ func (h *handlers) list(w http.ResponseWriter, r *http.Request, pr policy.Princi } } - rows, err := h.store.List(r.Context(), h.postType, filter) + // post_type discriminator: client may override the mount default + // via ?post_type= so a single /api/v1/posts mount can serve both + // posts and pages (the /api/v1/pages mount isn't wired yet — see + // the PostType field doc on ListFilter). + postType := h.postType + if filter.PostType != "" { + postType = filter.PostType + } + + rows, err := h.store.List(r.Context(), postType, filter) if err != nil { h.logger.ErrorContext(r.Context(), "posts.list: store error", slog.Any("err", err)) router.WriteError(w, http.StatusInternalServerError, "internal_error", "failed to list posts") @@ -171,6 +180,18 @@ func parseListQuery(r *http.Request) (ListFilter, error) { f.AuthorID = q.Get("author") f.Search = q.Get("search") + // Optional post_type override. Closed set ("post", "page") matches + // the two built-in types from migration 000003 — refusing anything + // else means a caller can't pivot the list onto a CPT it shouldn't + // see by guessing a slug. Empty falls back to the mount default in + // the handler. + if pt := q.Get("post_type"); pt != "" { + if pt != PostTypePost && pt != PostTypePage { + return f, validation{Code: "invalid_post_type", Detail: fmt.Sprintf("unknown post_type %q", pt)} + } + f.PostType = pt + } + after := q.Get("after") if after != "" { decoded, err := router.ParseCursor(after) diff --git a/apps/api/internal/rest/posts/handlers_test.go b/apps/api/internal/rest/posts/handlers_test.go index 9991e14..f828be8 100644 --- a/apps/api/internal/rest/posts/handlers_test.go +++ b/apps/api/internal/rest/posts/handlers_test.go @@ -446,6 +446,47 @@ func TestList_SearchByTitle(t *testing.T) { } } +// TestList_FilterByPostType is the regression test for the admin Pages +// list (issue #506). The /api/v1/posts mount is hard-coded to +// PostTypePost, but admin/pages queries /api/v1/posts?post_type=page — +// the handler must honor that override and return only page rows. +func TestList_FilterByPostType(t *testing.T) { + t.Parallel() + h := newHarness(t, PostTypePost) + pr := authorPrincipal("u1") + + postTitle := "A Post" + pageTitle := "About Us" + h.store.Create(context.Background(), PostTypePost, "u1", CreateInput{Title: &postTitle}) + h.store.Create(context.Background(), PostTypePage, "u1", CreateInput{Title: &pageTitle}) + + req := httptest.NewRequest("GET", h.base+"?post_type=page", nil) + rec := h.do(req, &pr) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, body = %s", rec.Code, rec.Body.String()) + } + var page router.Page[Post] + decodeJSON(t, rec, &page) + if len(page.Data) != 1 || page.Data[0].Title != "About Us" || page.Data[0].PostType != PostTypePage { + t.Errorf("data = %+v; want a single page row", page.Data) + } +} + +// TestList_FilterByPostType_InvalidRejected guards the closed-set +// validation: an unknown post_type is a 400, not a silent fall-through +// to the mount default. +func TestList_FilterByPostType_InvalidRejected(t *testing.T) { + t.Parallel() + h := newHarness(t, PostTypePost) + pr := authorPrincipal("u1") + + req := httptest.NewRequest("GET", h.base+"?post_type=attachment", nil) + rec := h.do(req, &pr) + if rec.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400; body = %s", rec.Code, rec.Body.String()) + } +} + // ----------------------------------------------------------------------------- // UPDATE // ----------------------------------------------------------------------------- diff --git a/apps/api/internal/rest/posts/model.go b/apps/api/internal/rest/posts/model.go index 10395e0..2ea48e5 100644 --- a/apps/api/internal/rest/posts/model.go +++ b/apps/api/internal/rest/posts/model.go @@ -103,6 +103,18 @@ type ListFilter struct { Search string // free-text against title (ILIKE) After string // cursor: posts.id strictly greater than this UUID Limit int // page size; clamped to [1, MaxListLimit] by the handler + + // PostType, when non-empty, OVERRIDES the mount's hardcoded post + // type discriminator for this single request. Empty means "use the + // mount default" (the typical case). + // + // This exists so a single mount (today, /api/v1/posts) can serve + // both posts and pages — admin Pages screens query + // /api/v1/posts?post_type=page rather than depending on a separate + // /api/v1/pages mount that isn't wired yet. The handler validates + // the value against the closed {"post","page"} set before stuffing + // it here, so a malicious request can't read a CPT it shouldn't. + PostType string } // MaxListLimit is the hard upper bound on a single page of results.