From 2546a8c3c6f1f62f2c6eeb07d986f3274f254236 Mon Sep 17 00:00:00 2001 From: Aaron Brethorst Date: Sat, 9 May 2026 19:19:21 -0700 Subject: [PATCH 1/6] refactor(inspector): mount at root, kill /inspector/login + legacy bearer cookie The inspector now lives at the server root (/, /me, /tokens, /push, /users, /audit, /events/{source}/{seq}). Static assets moved from /inspector/static/ to /assets/stylesheets/. The deprecated /inspector/login admin-token form is removed, and with it the legacy hooks_inspector_token bearer-cookie auth path (functionally dead post-rename: browser-stored cookies at Path=/inspector no longer match any served route). Auth is now session-only via /login. GET /logout now actually invalidates the server-side session and clears cookies, instead of the previous no-op redirect. This was a real bug for session-authenticated users who clicked "Sign out" in the header. Also: stripped task-number references from doc comments per CLAUDE.md guidance ("references to the current task ... rot as the codebase evolves"); ignored .playwright-mcp/ smoke-test artifacts. --- .gitignore | 1 + CLAUDE.md | 6 +- README.md | 4 +- cmd/hooks/main.go | 4 +- docs/accounts.md | 6 +- docs/operations.md | 2 +- docs/quickstart.md | 6 +- docs/security.md | 4 +- internal/admin/users_test.go | 8 +- internal/auth/sessions.go | 2 +- internal/inspector/inspector.go | 286 ++++++------------ internal/inspector/inspector_audit.go | 4 +- internal/inspector/inspector_audit_test.go | 35 ++- internal/inspector/inspector_me.go | 24 +- internal/inspector/inspector_me_push.go | 24 +- internal/inspector/inspector_me_push_test.go | 56 ++-- internal/inspector/inspector_me_test.go | 56 ++-- internal/inspector/inspector_session_test.go | 213 ++++--------- internal/inspector/inspector_test.go | 171 +++++------ internal/inspector/inspector_users.go | 49 +-- internal/inspector/inspector_users_test.go | 36 +-- internal/inspector/templates/detail.tmpl.html | 4 +- internal/inspector/templates/header.tmpl.html | 16 +- internal/inspector/templates/index.tmpl.html | 2 +- internal/inspector/templates/login.tmpl.html | 16 - internal/inspector/templates/me.tmpl.html | 8 +- .../inspector/templates/me_push.tmpl.html | 12 +- internal/inspector/templates/push.tmpl.html | 14 +- internal/inspector/templates/tokens.tmpl.html | 4 +- internal/inspector/templates/users.tmpl.html | 10 +- internal/me/api_test.go | 12 +- internal/server/server.go | 6 +- internal/server/server_test.go | 2 +- internal/store/users_store_test.go | 4 +- internal/tokens/middleware.go | 2 +- internal/tokens/middleware_test.go | 2 +- internal/tokens/tokens_test.go | 4 +- internal/webpages/pages.go | 2 +- internal/webpages/pages_test.go | 14 +- .../templates/device_approve.tmpl.html | 2 +- .../webpages/templates/device_done.tmpl.html | 4 +- .../templates/device_lookup.tmpl.html | 2 +- internal/webpages/templates/login.tmpl.html | 2 +- internal/webpages/templates/signup.tmpl.html | 2 +- 44 files changed, 433 insertions(+), 710 deletions(-) delete mode 100644 internal/inspector/templates/login.tmpl.html diff --git a/.gitignore b/.gitignore index 585b6c8..eb415a2 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ hooks.yaml .env .claude/scheduled_tasks.lock +.playwright-mcp/ diff --git a/CLAUDE.md b/CLAUDE.md index 43b2047..efd3e6a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -56,19 +56,19 @@ inbound webhook ──► /ingest/ ──► verifier ──► store.Ap - **`internal/push`** — `Manager` runs one worker goroutine per non-paused subscription. Workers POST events one at a time, advancing cursor only on 2xx. Backoff is `min(60s, 2^failures*100ms)` with full jitter. Outbound delivery signature: `X-Hooks-Signature: t=,v1=` where `v1 = HMAC-SHA256(secret, ".")` (see `signing.go`). **The plaintext signing secret only lives in memory** — after a restart, push delivery for each subscription is paused until `hooksctl push rotate-secret ` re-arms it. This is a deliberate trade-off (don't try to "fix" by persisting plaintext). -- **`internal/tokens`** — listener bearer tokens, Argon2id-hashed at rest. The store package can't import argon2 directly, so `tokens.AttachVerifier(st)` injects the hash-compare function at startup. `LookupByPlaintext` is O(N) per request (re-hashes per row); fine for operator-token volumes. The special scope `admin` grants access to `/inspector` and the management APIs but does NOT implicitly grant subscribe — admin tokens MUST also list source names in scopes to use `/subscribe/`. +- **`internal/tokens`** — listener bearer tokens, Argon2id-hashed at rest. The store package can't import argon2 directly, so `tokens.AttachVerifier(st)` injects the hash-compare function at startup. `LookupByPlaintext` is O(N) per request (re-hashes per row); fine for operator-token volumes. The special scope `admin` grants access to the inspector and the management APIs but does NOT implicitly grant subscribe — admin tokens MUST also list source names in scopes to use `/subscribe/`. - **`internal/secret`** — `secret.String` is a typed credential that returns `[REDACTED]` on `String()`, `GoString()`, and `MarshalJSON`. Always use it for secrets crossing config/log boundaries. Convert to plaintext only at the consumption site via `.Reveal()`. Use `secret.Equal` / `secret.EqualString` for constant-time comparison. - **`internal/config`** — loads `hooks.yaml`, applies env interpolation (`${VAR}` and `${VAR:-default}`), then env-var overrides (`HOOKS_LISTEN_ADDR`, `HOOKS_DATABASE_URL`, `HOOKS_LOG_LEVEL`). Listen-address precedence is `HOOKS_LISTEN_ADDR` > yaml `listen_addr` > `:$PORT` (when `$PORT` is a valid port — for Render/Heroku/Fly/Cloud Run) > `:8080`. **A `tokens:` field is rejected at load time** — listener tokens live in the database, not YAML. `verifier:` is required for every source; unsigned sources are not supported. Defaults: `BodySizeLimit=1MiB`, `DedupeWindow=24h`, `SkewWindow=5m`, source `Retention=30d`. Retention `0` / `forever` / `never` disables auto-prune for that source. -- **`internal/inspector`** — admin-only web UI under `/inspector`. Templates and static assets are `//go:embed`-ed; the binary is fully self-contained. Auth is either a `hooks_session=.` cookie (post-login, server-side `user_sessions` row, SHA-256 hashed) or — legacy — a cookie carrying the plaintext bearer token (Argon2id-hashed lookup, identical to API auth). New logins always create a `user_sessions` row; the legacy path is accept-on-read only. +- **`internal/inspector`** — admin web UI mounted at the server root (`/`, `/me`, `/tokens`, `/push`, `/users`, `/audit`, `/events/{source}/{seq}`, …). Static assets are served at `/assets/stylesheets/`. Templates and assets are `//go:embed`-ed; the binary is fully self-contained. Auth is the `hooks_session=<id>.<plaintext>` cookie issued by `/login` (server-side `user_sessions` row, SHA-256 hashed). No bearer-token or basic-auth path; anonymous and non-session callers redirect to `/login`. - **`internal/prune`** — hourly per-source pruner that respects each source's configured retention. The `hooks prune --older-than <dur>` CLI bypasses configured retention for ad-hoc cleanup. Also reaps `ephemeral=true` listener tokens whose `last_used_at` is more than 24h in the past (forward crash-safety net) and `device_pairings` rows 24h after terminal state. - **`internal/users`** — `users` table (id, email, name, role, password_hash Argon2id, default_scopes, deactivated_at). Owns signup-time password policy enforcement (length ≥ 12, no email substring; failed-policy reason logged, never the plaintext). `Deactivate` is the cascading-revoke path: in one tx it sets `deactivated_at`, revokes every PAT/listener token, and pauses every push subscription owned by the user. A last-admin guard refuses with HTTP 409 if zero admins would remain (checked before AND inside the tx). Reactivation flips `deactivated_at` to NULL and **does not** restore tokens or unpause subscriptions — the user must reissue. Matches GitHub's UX; documented in `docs/security.md`. -- **`internal/audit`** — append-only `audit_events` table surfaced at `/inspector/audit` (admin). Recorder hangs off mutating handlers (invites, users, tokens, sessions, device pairings). Prune loop does not touch this table; metadata is small (operator-action volume, not webhook volume). +- **`internal/audit`** — append-only `audit_events` table surfaced at `/audit` (admin). Recorder hangs off mutating handlers (invites, users, tokens, sessions, device pairings). Prune loop does not touch this table; metadata is small (operator-action volume, not webhook volume). - **`internal/ratelimit`** — in-process token-bucket-per-IP (and per-user, for device-approve) middleware. Wired onto auth surfaces in `internal/server.registerAuthRoutes`. Buckets live in process memory and reset on restart — acceptable for the single-process SQLite posture. Limits live next to the route registration; check `internal/server/server.go` for current values. diff --git a/README.md b/README.md index b29b6ff..1d85a00 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,7 @@ sources: `--dev` enables verbose logging, opens the inspector in your browser, and prints the URLs you'll need: ``` -inspector: http://localhost:8080/inspector +inspector: http://localhost:8080/ ingest: http://localhost:8080/ingest/render forward: hooksctl forward render --to http://localhost:3000/webhooks/render ``` @@ -111,7 +111,7 @@ The fastest trigger is a redeploy of any Render service: `Manual Deploy → Depl You should see, in order: - A `POST /ingest/render` log line in the `hooks --dev` terminal. -- A new row in the inspector at `http://localhost:8080/inspector` (paste the admin token to log in). +- A new row in the inspector at `http://localhost:8080/` (sign in with the admin email/password you set during `hooks init`). - A `POST /webhooks/render` arriving at your local app via the `hooksctl forward` terminal. ### 8. (Optional) Register a long-lived push subscription diff --git a/cmd/hooks/main.go b/cmd/hooks/main.go index e881900..d0c09f0 100644 --- a/cmd/hooks/main.go +++ b/cmd/hooks/main.go @@ -125,14 +125,14 @@ func printDevQuickstart(srv *server.Server) { } fmt.Fprintln(os.Stderr, "") fmt.Fprintln(os.Stderr, "hooks --dev quickstart:") - fmt.Fprintf(os.Stderr, " inspector: http://%s/inspector\n", host) + fmt.Fprintf(os.Stderr, " inspector: http://%s/\n", host) for source := range srv.Cfg.Sources { fmt.Fprintf(os.Stderr, " ingest: http://%s/ingest/%s\n", host, source) fmt.Fprintf(os.Stderr, " forward: hooksctl forward %s --to http://localhost:3000/webhooks/%s\n", source, source) fmt.Fprintf(os.Stderr, " push add: hooksctl push add --source %s --to https://my-svc.example.com/hooks\n", source) } fmt.Fprintln(os.Stderr, "") - openBrowser(fmt.Sprintf("http://%s/inspector", host)) + openBrowser(fmt.Sprintf("http://%s/", host)) } func openBrowser(url string) { diff --git a/docs/accounts.md b/docs/accounts.md index e536be9..1ecded3 100644 --- a/docs/accounts.md +++ b/docs/accounts.md @@ -26,7 +26,7 @@ If the bootstrap link expires before it's used, re-run `hooks init` against the ## 3. Invite teammates -After login, the inspector at `/inspector/users` exposes an "Issue invite" form. Pick a role (`user` or `admin`) and a default scope set; the page shows the resulting `https://hooks.example.com/signup?code=...` URL once. Send it to your teammate. Invites are single-use. +After login, the inspector at `/users` exposes an "Issue invite" form. Pick a role (`user` or `admin`) and a default scope set; the page shows the resulting `https://hooks.example.com/signup?code=...` URL once. Send it to your teammate. Invites are single-use. The same surface is available over JSON at `POST /api/invites` for any tool you'd rather drive programmatically: @@ -101,7 +101,7 @@ The relay prints a per-subscription signing secret **once** — store it on your ## Admin operations -The inspector exposes admin-only pages at `/inspector/users` (user list + issue-invite form + deactivate / reactivate / reset-password actions) and `/inspector/audit` (audit-log table). The matching JSON endpoints are under `/api/users/*`, `/api/invites/*`, `/api/audit`. v1 ships no `hooksctl` subcommands for these surfaces — drive them via the inspector or `curl` against the JSON API. +The inspector exposes admin-only pages at `/users` (user list + issue-invite form + deactivate / reactivate / reset-password actions) and `/audit` (audit-log table). The matching JSON endpoints are under `/api/users/*`, `/api/invites/*`, `/api/audit`. v1 ships no `hooksctl` subcommands for these surfaces — drive them via the inspector or `curl` against the JSON API. ### Deactivating a user @@ -136,7 +136,7 @@ Every admin-meaningful action lands in `audit_events`: - `session.create`, `session.delete` - `device_pairing.start`, `device_pairing.approve`, `device_pairing.deny` -Surfaced at `/inspector/audit` (admin only). The table is **append-only** — no API or UI deletes entries. Size is small (~few hundred bytes per row, growth driven by operator actions, not webhook traffic). +Surfaced at `/audit` (admin only). The table is **append-only** — no API or UI deletes entries. Size is small (~few hundred bytes per row, growth driven by operator actions, not webhook traffic). ## Logging out diff --git a/docs/operations.md b/docs/operations.md index 919685b..e371de5 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -43,7 +43,7 @@ The default deployment is one process with SQLite. There is **no** built-in coor ## Push-subscription health -Use `/inspector/push` (or `hooksctl push list`) to monitor: +Use `/push` (or `hooksctl push list`) to monitor: - **Queue depth**: `latest_sequence_for_source - cursor`. Grows during outages; should return to 0 within seconds after recovery. - **`consecutive_failures`**: resets to 0 on the next 2xx. diff --git a/docs/quickstart.md b/docs/quickstart.md index acf7716..35340d2 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -99,9 +99,9 @@ The server honors `$PORT` (which Render injects) automatically, so the Blueprint ## 4. Claim the first admin account -Open the bootstrap signup URL from step 2 in a browser. Pick an email, name, and password (≥ 12 characters; must not contain your email or its local-part). Submitting the form consumes the bootstrap invite, signs you into the inspector at `/inspector`, and the URL returns 409 from then on. +Open the bootstrap signup URL from step 2 in a browser. Pick an email, name, and password (≥ 12 characters; must not contain your email or its local-part). Submitting the form consumes the bootstrap invite, signs you into the inspector at `/`, and the URL returns 409 from then on. -If the link expires before you use it, open the service's **Shell** (now available since the deploy is healthy) and re-run `hooks init --force --server-url "$HOOKS_PUBLIC_URL"` to mint a fresh 24-hour invite. Once any user exists, the bootstrap path is closed — invite teammates from `/inspector/users` (or `POST /api/invites`) instead. +If the link expires before you use it, open the service's **Shell** (now available since the deploy is healthy) and re-run `hooks init --force --server-url "$HOOKS_PUBLIC_URL"` to mint a fresh 24-hour invite. Once any user exists, the bootstrap path is closed — invite teammates from `/users` (or `POST /api/invites`) instead. ## 5. Register the webhook with Render @@ -153,7 +153,7 @@ The plaintext signing secret only lives in memory, so push delivery for each sub ## 9. Browse -Open `https://webhooks.example.com/inspector` and sign in with the email/password from step 4. You can browse every captured event, replay any of them to live listeners, manage tokens and push subscriptions, invite teammates, and review the audit log at `/inspector/audit`. +Open `https://webhooks.example.com/` and sign in with the email/password from step 4. You can browse every captured event, replay any of them to live listeners, manage tokens and push subscriptions, invite teammates, and review the audit log at `/audit`. ## What `hooks init` does NOT do diff --git a/docs/security.md b/docs/security.md index e37db40..e062521 100644 --- a/docs/security.md +++ b/docs/security.md @@ -21,7 +21,7 @@ Failed verification produces HTTP 401 with no body. Logs include only the source - `hooksctl token list` and `/api/tokens` GET return only metadata (id, name, scopes, timestamps). No path returns the plaintext after issuance. - Revoked tokens are rejected within one round-trip; `last_used_at` is updated best-effort. -The special scope `admin` grants access to `/inspector`, `/api/tokens`, and `/api/push-subscriptions`. It does **not** implicitly grant subscribe access — an admin token MUST also include the source name in its scopes to subscribe. +The special scope `admin` grants access to the inspector UI, `/api/tokens`, and `/api/push-subscriptions`. It does **not** implicitly grant subscribe access — an admin token MUST also include the source name in its scopes to subscribe. ### Ephemeral listener tokens @@ -127,7 +127,7 @@ Excess requests return HTTP 429 with `Retry-After: <seconds>`. Buckets live in p ### Audit log -Every admin-meaningful action is recorded in the append-only `audit_events` table. Surfaced at `/inspector/audit` (admin only). Tracked actions: +Every admin-meaningful action is recorded in the append-only `audit_events` table. Surfaced at `/audit` (admin only). Tracked actions: - `invite.create`, `invite.revoke`, `invite.consume` - `user.create`, `user.deactivate`, `user.reactivate`, `user.role_change`, `user.update`, `user.password_reset` diff --git a/internal/admin/users_test.go b/internal/admin/users_test.go index 790fff2..ff33d33 100644 --- a/internal/admin/users_test.go +++ b/internal/admin/users_test.go @@ -280,7 +280,7 @@ func TestDeactivate_CascadesTokensAndSubs(t *testing.T) { } } -// TestReactivate_DoesNotRestoreTokens covers task 9.10's "reactivation +// TestReactivate_DoesNotRestoreTokens covers consolidated "reactivation // does not auto-restore tokens" requirement. Once a user is deactivated // (cascading revoke + paused subs), reactivating clears deactivated_at // only — every previously revoked token stays revoked, every paused @@ -343,7 +343,7 @@ func TestReactivate_DoesNotRestoreTokens(t *testing.T) { } } -// TestPatchToken_OwnershipReflectedInMe covers task 9.10's "ownership +// TestPatchToken_OwnershipReflectedInMe covers consolidated "ownership // transfer is reflected in /api/me calls by the new owner". After admin // reassigns a token via PATCH /api/tokens/{id}, the previous owner's // /api/me/tokens listing drops it and the new owner's listing gains it. @@ -389,7 +389,7 @@ func TestPatchToken_OwnershipReflectedInMe(t *testing.T) { } } -// TestResetPassword_RejectsShortPasswords covers task 9.10's "password +// TestResetPassword_RejectsShortPasswords covers consolidated "password // reset rejects short passwords". The admin endpoint runs the same // ValidatePolicy as signup, so a short password yields 400. func TestResetPassword_RejectsShortPasswords(t *testing.T) { @@ -491,7 +491,7 @@ func TestListAudit_AdminOK_NonAdmin403(t *testing.T) { } } -// TestAuditCoverage_AdminActions (task 10.6): each representative admin +// TestAuditCoverage_AdminActions: each representative admin // action produces exactly one audit_events row with the expected action, // target_type, and target_id. The breadth here is intentional but bounded: // we exercise three distinct actions per audit constant family diff --git a/internal/auth/sessions.go b/internal/auth/sessions.go index a53683b..286bd8c 100644 --- a/internal/auth/sessions.go +++ b/internal/auth/sessions.go @@ -211,7 +211,7 @@ func (m *Manager) DeleteSessionsByUser(ctx context.Context, userID string) error // SetCookies is shared between login and the CSRF cookie rotation that // happens on session creation: every call generates a fresh hooks_csrf // value via secret.NewRandom, so a prior session's CSRF token cannot -// authenticate a freshly created session (task 4.3). +// authenticate a freshly created session. func (m *Manager) SetCookies(w http.ResponseWriter, r *http.Request, cookieValue string) (csrfToken string, err error) { csrf, err := secret.NewRandom() if err != nil { diff --git a/internal/inspector/inspector.go b/internal/inspector/inspector.go index 7ae6c29..2c61c30 100644 --- a/internal/inspector/inspector.go +++ b/internal/inspector/inspector.go @@ -1,9 +1,9 @@ -// Package inspector serves the admin-only web UI at /inspector. +// Package inspector serves the admin web UI mounted at the root of the +// hooks server (/, /me, /tokens, /push, /users, /audit, ...). // // All assets (HTML templates and CSS) are embedded so the deployment is one -// statically-linked binary. Authentication uses a cookie scoped to /inspector -// containing the same plaintext bearer token the API uses; the server-side -// lookup is identical (Argon2id constant-time compare). +// statically-linked binary. Authentication is the hooks_session cookie +// issued by /login. package inspector import ( @@ -39,48 +39,35 @@ var templatesFS embed.FS //go:embed static/* var staticFS embed.FS -const cookieName = "hooks_inspector_token" - // Cascader runs the deactivate-and-cascade transaction. Implemented by // store.SQLite.DeactivateUserCascade. The same shape is in admin.API. type Cascader interface { DeactivateUserCascade(ctx context.Context, id string, when time.Time) (store.CascadeRevokeResult, error) } -// Inspector is the http handler set for /inspector. +// Inspector is the http handler set for the inspector web UI. type Inspector struct { Events store.EventStore Tokens store.TokenStore Subs store.PushSubscriptionStore Notifier *pubsub.Notifier Push *push.Manager - Auth *tokens.Authenticator - // Sessions, when non-nil, enables session-cookie authentication on the - // inspector router (task 11.12). The middleware runs before each - // handler so requireAdmin can read (*User, *Session) from context. A - // nil Sessions falls back to the legacy hooks_inspector_token bearer - // cookie path only. + // Sessions enables session-cookie authentication; nil means the + // inspector refuses every request as anonymous (no other auth path + // remains). Sessions *auth.Manager - // Audit, when set, receives a token.create / token.revoke entry every - // time /inspector/me/tokens (task 11.4) issues or revokes a PAT. The - // session-attached User on each request comes from auth.Manager's - // per-request lookup, so a separate UserStore reference would be a - // stale read; omit it here. + // Audit, when set, receives a token.create / token.revoke entry on + // every PAT mint or revoke through /me/tokens. Audit audit.Recorder - // Users, when set, lets /inspector/tokens, /inspector/push, and - // /inspector/audit render an "owner" / "actor" column with the user's - // email instead of a bare id (tasks 11.8, 11.9, 11.6). When nil, those - // views fall back to printing the raw user id (or "system" for NULL). + // Users, when set, lets /tokens, /push, and /audit render an owner / + // actor column with the user's email instead of a bare id. Users store.UserStore - // AuditReader, when set, powers /inspector/audit (task 11.6). Reads - // audit_events ordered by `at DESC` with optional time-range filters - // pulled from the request query string. + // AuditReader, when set, powers /audit. Reads audit_events ordered by + // `at DESC` with optional time-range filters from the query string. AuditReader store.AuditStore - // Invites, Cascader, HashPassword, and ValidatePolicy power the - // /inspector/users admin page (task 11.5). They mirror the wiring on - // invites.API and admin.API so the inspector and JSON surfaces share - // the same business logic. When unset the admin page degrades to a - // read-only view (writes return 503). + // Invites, Cascader, HashPassword, and ValidatePolicy power the /users + // admin page; mirror the wiring on invites.API and admin.API. When + // unset the page degrades to a read-only view (writes return 503). Invites store.InviteStore Cascader Cascader HashPassword func(plaintext string) (string, error) @@ -91,14 +78,16 @@ type Inspector struct { staticSub fs.FS } -// New constructs an Inspector. Templates are parsed at construction. +// New constructs an Inspector. Templates are parsed at construction. The +// caller MUST set Sessions on the returned value before calling Register +// — without it, every request resolves as anonymous and redirects to +// /login. func New( events store.EventStore, ts store.TokenStore, subs store.PushSubscriptionStore, notifier *pubsub.Notifier, pushMgr *push.Manager, - auth *tokens.Authenticator, configuredSources []string, logger *slog.Logger, ) (*Inspector, error) { @@ -116,16 +105,15 @@ func New( return &Inspector{ Events: events, Tokens: ts, Subs: subs, Notifier: notifier, Push: pushMgr, - Auth: auth, Logger: logger, + Logger: logger, Sources: configuredSources, tpls: tpls, staticSub: sub, }, nil } -// Register mounts inspector routes onto mux. If in.Sessions is non-nil -// (task 11.12), each handler is wrapped in the session middleware so the -// inspector can authenticate via the hooks_session cookie alongside the -// legacy hooks_inspector_token bearer cookie. +// Register mounts inspector routes onto mux. Each handler is wrapped in +// in.Sessions.Middleware (when set) so requireAdmin can read the session +// from request context. func (in *Inspector) Register(mux *http.ServeMux) { wrap := func(h http.HandlerFunc) http.Handler { if in.Sessions == nil { @@ -142,120 +130,78 @@ func (in *Inspector) Register(mux *http.ServeMux) { return in.Sessions.Middleware(h) } - mux.Handle("GET /inspector/static/", http.StripPrefix("/inspector/static/", http.FileServer(http.FS(in.staticSub)))) - mux.Handle("GET /inspector/login", wrap(in.loginGET)) - mux.Handle("POST /inspector/login", wrap(in.loginPOST)) - mux.Handle("GET /inspector/logout", wrap(in.logout)) - mux.Handle("GET /inspector", wrap(in.index)) - mux.Handle("GET /inspector/events/{source}/{sequence}", wrap(in.detail)) - mux.Handle("POST /inspector/events/{source}/{sequence}/replay", wrap(in.replay)) - mux.Handle("GET /inspector/tokens", wrap(in.tokensList)) - mux.Handle("POST /inspector/tokens/create", wrap(in.tokensCreate)) - mux.Handle("POST /inspector/tokens/{id}/revoke", wrap(in.tokensRevoke)) - mux.Handle("GET /inspector/push", wrap(in.pushList)) - mux.Handle("POST /inspector/push/create", wrap(in.pushCreate)) - mux.Handle("POST /inspector/push/{id}/pause", wrap(in.pushPause)) - mux.Handle("POST /inspector/push/{id}/resume", wrap(in.pushResume)) - mux.Handle("POST /inspector/push/{id}/test", wrap(in.pushTest)) - mux.Handle("POST /inspector/push/{id}/rotate", wrap(in.pushRotate)) - mux.Handle("POST /inspector/push/{id}/delete", wrap(in.pushDelete)) - - // /inspector/me is the user self-service page (task 11.4). It is - // session-only (the legacy raw-bearer cookie path is admin-scoped and - // does not surface a "current user"). Mutations run through the - // shared CSRF middleware so the inspector and /api/me/* enforce the - // same double-submit + Origin contract. + mux.Handle("GET /assets/stylesheets/", http.StripPrefix("/assets/stylesheets/", http.FileServer(http.FS(in.staticSub)))) + mux.Handle("GET /logout", wrap(in.logout)) + mux.Handle("GET /{$}", wrap(in.index)) + mux.Handle("GET /events/{source}/{sequence}", wrap(in.detail)) + mux.Handle("POST /events/{source}/{sequence}/replay", wrap(in.replay)) + mux.Handle("GET /tokens", wrap(in.tokensList)) + mux.Handle("POST /tokens/create", wrap(in.tokensCreate)) + mux.Handle("POST /tokens/{id}/revoke", wrap(in.tokensRevoke)) + mux.Handle("GET /push", wrap(in.pushList)) + mux.Handle("POST /push/create", wrap(in.pushCreate)) + mux.Handle("POST /push/{id}/pause", wrap(in.pushPause)) + mux.Handle("POST /push/{id}/resume", wrap(in.pushResume)) + mux.Handle("POST /push/{id}/test", wrap(in.pushTest)) + mux.Handle("POST /push/{id}/rotate", wrap(in.pushRotate)) + mux.Handle("POST /push/{id}/delete", wrap(in.pushDelete)) + + // /me is the user self-service page. Mutations run through the shared + // CSRF middleware so the inspector and /api/me/* enforce the same + // double-submit + Origin contract. csrf := func(h http.Handler) http.Handler { return web.Middleware(web.CSRFConfig{}, h) } - mux.Handle("GET /inspector/me", wrap(in.meIndex)) - mux.Handle("POST /inspector/me/tokens", wrapH(csrf(http.HandlerFunc(in.meCreateToken)))) - mux.Handle("POST /inspector/me/tokens/{id}/revoke", wrapH(csrf(http.HandlerFunc(in.meRevokeToken)))) - - // /inspector/me/push (task 11.7) — user-owned push-subscription view - // mirroring /inspector/push without the owner column. Mutations share - // the CSRF middleware so the same double-submit + Origin contract - // applies as elsewhere on /inspector/me. - mux.Handle("GET /inspector/me/push", wrap(in.mePushIndex)) - mux.Handle("POST /inspector/me/push/{id}/pause", wrapH(csrf(http.HandlerFunc(in.mePushPause)))) - mux.Handle("POST /inspector/me/push/{id}/resume", wrapH(csrf(http.HandlerFunc(in.mePushResume)))) - mux.Handle("POST /inspector/me/push/{id}/test", wrapH(csrf(http.HandlerFunc(in.mePushTest)))) - mux.Handle("POST /inspector/me/push/{id}/rotate", wrapH(csrf(http.HandlerFunc(in.mePushRotate)))) - mux.Handle("POST /inspector/me/push/{id}/delete", wrapH(csrf(http.HandlerFunc(in.mePushDelete)))) - - // /inspector/audit (task 11.6): admin-only HTML view of the audit log. - mux.Handle("GET /inspector/audit", wrap(in.auditList)) - - // /inspector/users (task 11.5): admin-only user table + invite form - // + per-row deactivate/reactivate/reset-password/edit. Mutations run - // through the same CSRF middleware as /inspector/me. - mux.Handle("GET /inspector/users", wrap(in.usersList)) - mux.Handle("POST /inspector/users/invite", wrapH(csrf(http.HandlerFunc(in.usersInvite)))) - mux.Handle("POST /inspector/users/{id}/deactivate", wrapH(csrf(http.HandlerFunc(in.usersDeactivate)))) - mux.Handle("POST /inspector/users/{id}/reactivate", wrapH(csrf(http.HandlerFunc(in.usersReactivate)))) - mux.Handle("POST /inspector/users/{id}/reset-password", wrapH(csrf(http.HandlerFunc(in.usersResetPassword)))) - mux.Handle("POST /inspector/users/{id}/update", wrapH(csrf(http.HandlerFunc(in.usersUpdate)))) -} - -// requireAdmin enforces admin access for an inspector request. -// -// Authentication sources, in order: -// 1. hooks_session cookie (task 11.12): if present and the user is admin, -// allow. If the user is non-admin, GET redirects to /inspector/me; -// non-GET returns 403. -// 2. legacy hooks_inspector_token cookie (task 11.11): plaintext bearer -// token in a cookie; admin scope required. -// -// Outcomes when no auth is present: -// - GET → 302 to /login?next=<path> (task 11.10). -// - non-GET → 401. -// -// Lookup failures from a non-auth source (DB unreachable, etc.) → 503 so -// operators don't mistake an outage for a bad token. + mux.Handle("GET /me", wrap(in.meIndex)) + mux.Handle("POST /me/tokens", wrapH(csrf(http.HandlerFunc(in.meCreateToken)))) + mux.Handle("POST /me/tokens/{id}/revoke", wrapH(csrf(http.HandlerFunc(in.meRevokeToken)))) + + // /me/push — user-owned push-subscription view mirroring /push without + // the owner column. + mux.Handle("GET /me/push", wrap(in.mePushIndex)) + mux.Handle("POST /me/push/{id}/pause", wrapH(csrf(http.HandlerFunc(in.mePushPause)))) + mux.Handle("POST /me/push/{id}/resume", wrapH(csrf(http.HandlerFunc(in.mePushResume)))) + mux.Handle("POST /me/push/{id}/test", wrapH(csrf(http.HandlerFunc(in.mePushTest)))) + mux.Handle("POST /me/push/{id}/rotate", wrapH(csrf(http.HandlerFunc(in.mePushRotate)))) + mux.Handle("POST /me/push/{id}/delete", wrapH(csrf(http.HandlerFunc(in.mePushDelete)))) + + // /audit: admin-only HTML view of the audit log. + mux.Handle("GET /audit", wrap(in.auditList)) + + // /users: admin-only user table + invite form + per-row + // deactivate/reactivate/reset-password/edit. Mutations run through + // the same CSRF middleware as /me. + mux.Handle("GET /users", wrap(in.usersList)) + mux.Handle("POST /users/invite", wrapH(csrf(http.HandlerFunc(in.usersInvite)))) + mux.Handle("POST /users/{id}/deactivate", wrapH(csrf(http.HandlerFunc(in.usersDeactivate)))) + mux.Handle("POST /users/{id}/reactivate", wrapH(csrf(http.HandlerFunc(in.usersReactivate)))) + mux.Handle("POST /users/{id}/reset-password", wrapH(csrf(http.HandlerFunc(in.usersResetPassword)))) + mux.Handle("POST /users/{id}/update", wrapH(csrf(http.HandlerFunc(in.usersUpdate)))) +} + +// requireAdmin enforces admin access via the hooks_session cookie. Admin +// callers proceed; non-admin GETs redirect to /me; non-admin mutations 403. +// Anonymous GETs redirect to /login?next=<path>; anonymous mutations 401. func (in *Inspector) requireAdmin(w http.ResponseWriter, r *http.Request) bool { - // 1. Session cookie path. if in.Sessions != nil { if user, _, ok := in.Sessions.FromContext(r.Context()); ok { if user.Role == store.RoleAdmin { return true } - // Logged in as non-admin: GET redirects to /inspector/me; - // mutations 403. if r.Method == http.MethodGet { - http.Redirect(w, r, "/inspector/me", http.StatusFound) + http.Redirect(w, r, "/me", http.StatusFound) return false } http.Error(w, "forbidden", http.StatusForbidden) return false } } - // 2. Legacy bearer cookie path. - c, err := r.Cookie(cookieName) - if err != nil || c.Value == "" { - in.denyUnauthorized(w, r) - return false - } - tok, err := in.Auth.ResolvePlaintext(r.Context(), c.Value) - if err != nil { - if tokens.IsAuthError(err) { - clearCookie(w) - in.denyUnauthorized(w, r) - return false - } - in.Logger.Error("inspector: auth lookup failed", slog.String("error", err.Error())) - http.Error(w, "auth temporarily unavailable", http.StatusServiceUnavailable) - return false - } - if !store.HasScope(tok.Scopes, store.ScopeAdmin) { - http.Error(w, "forbidden", http.StatusForbidden) - return false - } - return true + in.denyUnauthorized(w, r) + return false } -// denyUnauthorized handles the no-auth-at-all case. GETs redirect to the -// new /login page with a ?next= so the user lands back on the inspector -// after logging in (task 11.10). Mutations get a flat 401. +// denyUnauthorized redirects anonymous GETs to /login?next=<path> and +// returns 401 for mutations. func (in *Inspector) denyUnauthorized(w http.ResponseWriter, r *http.Request) { if r.Method == http.MethodGet { next := r.URL.RequestURI() @@ -265,55 +211,19 @@ func (in *Inspector) denyUnauthorized(w http.ResponseWriter, r *http.Request) { http.Error(w, "unauthorized", http.StatusUnauthorized) } -func (in *Inspector) loginGET(w http.ResponseWriter, r *http.Request) { - in.render(w, "login", map[string]any{"Error": ""}) -} - -// loginPOST is the legacy v1 inspector login form. It still issues the -// raw-bearer cookie for backwards compatibility with operators who haven't -// yet migrated to /login (the session-based flow). The Deprecation -// response header (RFC 8594) marks this path as slated for v2 removal; -// the cookie format itself continues to authenticate every request, -// including mutations, until then (task 11.11). -func (in *Inspector) loginPOST(w http.ResponseWriter, r *http.Request) { - if err := r.ParseForm(); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - plaintext := strings.TrimSpace(r.Form.Get("token")) - tok, err := in.Auth.ResolvePlaintext(r.Context(), plaintext) - if err != nil && !tokens.IsAuthError(err) { - in.Logger.Error("inspector: login lookup failed", slog.String("error", err.Error())) - http.Error(w, "auth temporarily unavailable", http.StatusServiceUnavailable) - return - } - if err != nil || !store.HasScope(tok.Scopes, store.ScopeAdmin) { - in.render(w, "login", map[string]any{"Error": "invalid or non-admin token"}) - return - } - http.SetCookie(w, &http.Cookie{ - Name: cookieName, - Value: plaintext, - Path: "/inspector", - HttpOnly: true, - Secure: r.TLS != nil, - SameSite: http.SameSiteLaxMode, - Expires: time.Now().Add(7 * 24 * time.Hour), - }) - w.Header().Set("Deprecation", "true") - w.Header().Set("Link", `</login>; rel="successor-version"`) - in.Logger.Warn("inspector: legacy /inspector/login used; migrate to /login (deprecated for v2)", - slog.String("token_id", tok.ID)) - http.Redirect(w, r, "/inspector", http.StatusFound) -} - +// logout invalidates the session row and clears the browser cookies, then +// redirects to /login. Idempotent — visiting /logout while already +// anonymous still 302s to /login. func (in *Inspector) logout(w http.ResponseWriter, r *http.Request) { - clearCookie(w) - http.Redirect(w, r, "/inspector/login", http.StatusFound) -} - -func clearCookie(w http.ResponseWriter) { - http.SetCookie(w, &http.Cookie{Name: cookieName, Value: "", Path: "/inspector", MaxAge: -1}) + if in.Sessions != nil { + if c, err := r.Cookie(auth.SessionCookie); err == nil && c.Value != "" { + if _, delErr := in.Sessions.DeleteSession(r.Context(), c.Value); delErr != nil && !errors.Is(delErr, auth.ErrInvalid) { + in.Logger.Warn("inspector: logout delete session failed", slog.Any("err", delErr)) + } + } + in.Sessions.ClearCookies(w, r) + } + http.Redirect(w, r, "/login", http.StatusFound) } // index renders the recent-events list. @@ -502,7 +412,7 @@ func (in *Inspector) replay(w http.ResponseWriter, r *http.Request) { in.Notifier.Publish(source, seq) // Push subscribers get a one-shot replay (no cursor advance). in.Push.ReplayOne(r.Context(), ev) - http.Redirect(w, r, fmt.Sprintf("/inspector/events/%s/%d", source, seq), http.StatusSeeOther) + http.Redirect(w, r, fmt.Sprintf("/events/%s/%d", source, seq), http.StatusSeeOther) } func (in *Inspector) tokensList(w http.ResponseWriter, r *http.Request) { @@ -659,7 +569,7 @@ func (in *Inspector) tokensRevoke(w http.ResponseWriter, r *http.Request) { http.Error(w, err.Error(), http.StatusInternalServerError) return } - http.Redirect(w, r, "/inspector/tokens", http.StatusSeeOther) + http.Redirect(w, r, "/tokens", http.StatusSeeOther) } func (in *Inspector) pushList(w http.ResponseWriter, r *http.Request) { @@ -711,7 +621,7 @@ func (in *Inspector) fetchPushSubs(ctx context.Context, owner string) ([]store.P } } -// ownerOption is one entry in the /inspector/push owner-filter dropdown. +// ownerOption is one entry in the /push owner-filter dropdown. type ownerOption struct { Value string // "" / "system" / user_id Label string // "all" / "system" / email-or-id @@ -851,7 +761,7 @@ func (in *Inspector) pushPause(w http.ResponseWriter, r *http.Request) { return } in.Push.Pause(id) - http.Redirect(w, r, "/inspector/push", http.StatusSeeOther) + http.Redirect(w, r, "/push", http.StatusSeeOther) } func (in *Inspector) pushResume(w http.ResponseWriter, r *http.Request) { @@ -864,7 +774,7 @@ func (in *Inspector) pushResume(w http.ResponseWriter, r *http.Request) { return } _ = in.Push.Resume(r.Context(), id) - http.Redirect(w, r, "/inspector/push", http.StatusSeeOther) + http.Redirect(w, r, "/push", http.StatusSeeOther) } func (in *Inspector) pushTest(w http.ResponseWriter, r *http.Request) { @@ -876,7 +786,7 @@ func (in *Inspector) pushTest(w http.ResponseWriter, r *http.Request) { http.Error(w, err.Error(), http.StatusBadGateway) return } - http.Redirect(w, r, "/inspector/push", http.StatusSeeOther) + http.Redirect(w, r, "/push", http.StatusSeeOther) } func (in *Inspector) pushRotate(w http.ResponseWriter, r *http.Request) { @@ -929,7 +839,7 @@ func (in *Inspector) pushDelete(w http.ResponseWriter, r *http.Request) { return } in.Push.Remove(id) - http.Redirect(w, r, "/inspector/push", http.StatusSeeOther) + http.Redirect(w, r, "/push", http.StatusSeeOther) } func (in *Inspector) render(w http.ResponseWriter, name string, data any) { diff --git a/internal/inspector/inspector_audit.go b/internal/inspector/inspector_audit.go index 4d73040..6d289a9 100644 --- a/internal/inspector/inspector_audit.go +++ b/internal/inspector/inspector_audit.go @@ -1,6 +1,6 @@ package inspector -// /inspector/audit (tasks 10.4, 11.6): admin-only HTML view of the +// /audit: admin-only HTML view of the // audit_events log. Renders rows ordered by `at DESC` with the actor email // resolved via UserStore (falling back to the raw user id when the user // row is missing). Optional ?since= and ?until= RFC3339 query parameters @@ -25,7 +25,7 @@ const ( auditMaxLimit = 1000 ) -// auditRow is one rendered row in the /inspector/audit table. +// auditRow is one rendered row in the /audit table. type auditRow struct { ID string At time.Time diff --git a/internal/inspector/inspector_audit_test.go b/internal/inspector/inspector_audit_test.go index d1490bf..713e4b3 100644 --- a/internal/inspector/inspector_audit_test.go +++ b/internal/inspector/inspector_audit_test.go @@ -1,6 +1,6 @@ package inspector -// Tests for /inspector/audit (tasks 10.4, 10.6, 11.6): admin-only HTML view +// Tests for /audit: admin-only HTML view // of the audit log, with actor email resolution and time-range filtering. import ( @@ -36,11 +36,11 @@ func insertAuditEvent(t *testing.T, f *sessionFixture, actor store.User, action return ev } -// TestInspectorAudit_AnonymousRedirectsToLogin: GET /inspector/audit with no -// session redirects to /login?next=/inspector/audit. +// TestInspectorAudit_AnonymousRedirectsToLogin: GET /audit with no +// session redirects to /login?next=/audit. func TestInspectorAudit_AnonymousRedirectsToLogin(t *testing.T) { f := newSessionFixture(t) - resp := f.get(t, "/inspector/audit") + resp := f.get(t, "/audit") if resp.StatusCode != http.StatusFound { t.Fatalf("status: %d, want 302", resp.StatusCode) } @@ -51,26 +51,25 @@ func TestInspectorAudit_AnonymousRedirectsToLogin(t *testing.T) { } // TestInspectorAudit_NonAdminRedirectsToMe: a logged-in non-admin getting -// /inspector/audit is redirected to /inspector/me (task 10.6: non-admin -// callers get refused). The redirect path mirrors how requireAdmin handles -// non-admin sessions on every other admin route. +// /audit is redirected to /me, mirroring how requireAdmin handles non-admin +// sessions on every other admin route. func TestInspectorAudit_NonAdminRedirectsToMe(t *testing.T) { f := newSessionFixture(t) u := f.makeUser(t, "user@example.com", store.RoleUser) f.loginAs(t, u) - resp := f.get(t, "/inspector/audit") + resp := f.get(t, "/audit") if resp.StatusCode != http.StatusFound { t.Fatalf("status: %d, want 302", resp.StatusCode) } loc := resp.Header.Get("Location") - if loc != "/inspector/me" { - t.Fatalf("Location = %q, want /inspector/me", loc) + if loc != "/me" { + t.Fatalf("Location = %q, want /me", loc) } } -// TestInspectorAudit_AdminSeesEvents (task 11.6): admin viewing -// /inspector/audit gets the audit log rendered in HTML, with the actor's +// TestInspectorAudit_AdminSeesEvents: admin viewing +// /audit gets the audit log rendered in HTML, with the actor's // email resolved (not just the bare user_id). func TestInspectorAudit_AdminSeesEvents(t *testing.T) { f := newSessionFixture(t) @@ -81,7 +80,7 @@ func TestInspectorAudit_AdminSeesEvents(t *testing.T) { insertAuditEvent(t, f, other, audit.ActionUserUpdate, audit.TargetTypeUser, other.ID, time.Now().UTC()) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/inspector/audit", nil) + req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/audit", nil) resp, err := f.client.Do(req) if err != nil { t.Fatal(err) @@ -103,7 +102,7 @@ func TestInspectorAudit_AdminSeesEvents(t *testing.T) { } } -// TestInspectorAudit_TimeRangeFilter (task 11.6): the page accepts ?since= +// TestInspectorAudit_TimeRangeFilter: the page accepts ?since= // and ?until= RFC3339 timestamps; rows outside the window are omitted. func TestInspectorAudit_TimeRangeFilter(t *testing.T) { f := newSessionFixture(t) @@ -120,7 +119,7 @@ func TestInspectorAudit_TimeRangeFilter(t *testing.T) { since := time.Now().Add(-24 * time.Hour).UTC().Format(time.RFC3339) q := url.Values{"since": {since}} req, _ := http.NewRequest(http.MethodGet, - f.srv.URL+"/inspector/audit?"+q.Encode(), nil) + f.srv.URL+"/audit?"+q.Encode(), nil) resp, err := f.client.Do(req) if err != nil { t.Fatal(err) @@ -146,7 +145,7 @@ func TestInspectorAudit_BadSinceReturns400(t *testing.T) { f.loginAs(t, admin) req, _ := http.NewRequest(http.MethodGet, - f.srv.URL+"/inspector/audit?since=not-a-timestamp", nil) + f.srv.URL+"/audit?since=not-a-timestamp", nil) resp, err := f.client.Do(req) if err != nil { t.Fatal(err) @@ -159,7 +158,7 @@ func TestInspectorAudit_BadSinceReturns400(t *testing.T) { } // TestInspectorAudit_AnonymousMutationReturns401: not actually applicable — -// /inspector/audit is GET-only — but the route mounting itself shouldn't +// /audit is GET-only — but the route mounting itself shouldn't // expose a POST handler. We exercise this by issuing a POST and asserting // 405 (method not allowed) is the worst-case outcome (Go's ServeMux returns // 405 when the path has GET registered but no method match). @@ -168,7 +167,7 @@ func TestInspectorAudit_PostNotAllowed(t *testing.T) { admin := f.makeUser(t, "admin@example.com", store.RoleAdmin) f.loginAs(t, admin) - req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/inspector/audit", + req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/audit", strings.NewReader("")) resp, err := f.client.Do(req) if err != nil { diff --git a/internal/inspector/inspector_me.go b/internal/inspector/inspector_me.go index 328d4d6..e4b8618 100644 --- a/internal/inspector/inspector_me.go +++ b/internal/inspector/inspector_me.go @@ -1,15 +1,9 @@ package inspector -// /inspector/me — user self-service (task 11.4): profile, own tokens -// (filtered by kind), own subscriptions, and a CSRF-protected form for -// minting an ephemeral PAT. Admins additionally see links to -// /inspector/users and /inspector/audit. -// -// Authentication is session-only. The legacy hooks_inspector_token -// raw-bearer cookie carries no "current user" — there is no row in the -// users table for those tokens — so this surface refuses to render in -// that mode and redirects to /login (task 11.10) just like an anonymous -// caller would. +// /me — user self-service: profile, own tokens (filtered by kind), own +// subscriptions, and a CSRF-protected form for minting an ephemeral PAT. +// Admins additionally see links to /users and /audit. Anonymous callers +// redirect to /login. import ( "errors" @@ -24,7 +18,7 @@ import ( "github.com/onebusaway/hooks/internal/tokens" ) -// meTokenRow is one row in the /inspector/me tokens table. +// meTokenRow is one row in the /me tokens table. type meTokenRow struct { ID string Name string @@ -36,7 +30,7 @@ type meTokenRow struct { ExpiresAt *time.Time } -// meSubRow is one row in the /inspector/me subscriptions table. +// meSubRow is one row in the /me subscriptions table. type meSubRow struct { ID string Source string @@ -55,7 +49,7 @@ type meSubRow struct { // true on success; on failure the response is already written. func (in *Inspector) requireSessionUser(w http.ResponseWriter, r *http.Request) (store.User, bool) { if in.Sessions == nil { - // Without a session manager wired in, /inspector/me cannot + // Without a session manager wired in, /me cannot // authenticate. Redirect anonymous-style. in.denyUnauthorized(w, r) return store.User{}, false @@ -68,7 +62,7 @@ func (in *Inspector) requireSessionUser(w http.ResponseWriter, r *http.Request) return user, true } -// meIndex renders /inspector/me. Optional ?kind=pat|listener narrows the +// meIndex renders /me. Optional ?kind=pat|listener narrows the // tokens table to just one kind; missing/empty leaves it un-filtered. func (in *Inspector) meIndex(w http.ResponseWriter, r *http.Request) { user, ok := in.requireSessionUser(w, r) @@ -251,5 +245,5 @@ func (in *Inspector) meRevokeToken(w http.ResponseWriter, r *http.Request) { Metadata: map[string]any{"via": "inspector/me"}, }) } - http.Redirect(w, r, "/inspector/me", http.StatusSeeOther) + http.Redirect(w, r, "/me", http.StatusSeeOther) } diff --git a/internal/inspector/inspector_me_push.go b/internal/inspector/inspector_me_push.go index 89dcda2..689c4c9 100644 --- a/internal/inspector/inspector_me_push.go +++ b/internal/inspector/inspector_me_push.go @@ -1,16 +1,12 @@ package inspector -// /inspector/me/push (task 11.7) — user-owned push-subscription view that -// mirrors /inspector/push but without the owner column. -// -// Authentication is session-only (same contract as /inspector/me): the -// legacy hooks_inspector_token raw-bearer cookie has no associated user -// row, so anonymous and bearer-only callers redirect to /login. +// /me/push — user-owned push-subscription view that mirrors /push but +// without the owner column. Anonymous callers redirect to /login. // // Mutations (pause / resume / rotate / delete / test) run through the // shared CSRF middleware mounted in Register and operate strictly on rows -// whose owner_user_id matches the calling user; cross-user attempts return -// 404 (probe-resistant) without disturbing the row. +// whose owner_user_id matches the calling user; cross-user attempts +// return 404 (probe-resistant) without disturbing the row. import ( "errors" @@ -45,7 +41,7 @@ func (in *Inspector) requireOwnedSub(w http.ResponseWriter, r *http.Request, use return sub, true } -// mePushIndex renders /inspector/me/push for the calling user. +// mePushIndex renders /me/push for the calling user. func (in *Inspector) mePushIndex(w http.ResponseWriter, r *http.Request) { user, ok := in.requireSessionUser(w, r) if !ok { @@ -96,7 +92,7 @@ func (in *Inspector) mePushPause(w http.ResponseWriter, r *http.Request) { return } in.Push.Pause(id) - http.Redirect(w, r, "/inspector/me/push", http.StatusSeeOther) + http.Redirect(w, r, "/me/push", http.StatusSeeOther) } // mePushResume resumes a subscription owned by the caller. @@ -114,7 +110,7 @@ func (in *Inspector) mePushResume(w http.ResponseWriter, r *http.Request) { return } _ = in.Push.Resume(r.Context(), id) - http.Redirect(w, r, "/inspector/me/push", http.StatusSeeOther) + http.Redirect(w, r, "/me/push", http.StatusSeeOther) } // mePushTest sends a synthetic ping event to the subscription's target URL @@ -133,11 +129,11 @@ func (in *Inspector) mePushTest(w http.ResponseWriter, r *http.Request) { http.Error(w, err.Error(), http.StatusBadGateway) return } - http.Redirect(w, r, "/inspector/me/push", http.StatusSeeOther) + http.Redirect(w, r, "/me/push", http.StatusSeeOther) } // mePushRotate rotates the signing secret. The new plaintext is rendered -// exactly once on the resulting page (matches /inspector/push behavior). +// exactly once on the resulting page (matches /push behavior). func (in *Inspector) mePushRotate(w http.ResponseWriter, r *http.Request) { user, ok := in.requireSessionUser(w, r) if !ok { @@ -180,5 +176,5 @@ func (in *Inspector) mePushDelete(w http.ResponseWriter, r *http.Request) { return } in.Push.Remove(id) - http.Redirect(w, r, "/inspector/me/push", http.StatusSeeOther) + http.Redirect(w, r, "/me/push", http.StatusSeeOther) } diff --git a/internal/inspector/inspector_me_push_test.go b/internal/inspector/inspector_me_push_test.go index 42b27b0..a9e26c4 100644 --- a/internal/inspector/inspector_me_push_test.go +++ b/internal/inspector/inspector_me_push_test.go @@ -1,11 +1,7 @@ package inspector -// Tests for /inspector/me/push (task 11.7): user-owned push-subscription -// view mirroring /inspector/push but without the owner column. -// -// Authentication is session-only (same contract as /inspector/me): the -// legacy hooks_inspector_token raw-bearer cookie carries no current user -// row, so anonymous and bearer-only callers redirect to /login. +// Tests for /me/push: user-owned push-subscription view mirroring /push +// but without the owner column. Anonymous callers redirect to /login. import ( "context" @@ -19,11 +15,11 @@ import ( "github.com/onebusaway/hooks/internal/store" ) -// TestInspectorMePush_AnonymousRedirectsToLogin: GET /inspector/me/push -// with no session redirects to /login?next=/inspector/me/push. +// TestInspectorMePush_AnonymousRedirectsToLogin: GET /me/push with no +// session redirects to /login?next=/me/push. func TestInspectorMePush_AnonymousRedirectsToLogin(t *testing.T) { f := newSessionFixture(t) - resp := f.get(t, "/inspector/me/push") + resp := f.get(t, "/me/push") if resp.StatusCode != http.StatusFound { t.Fatalf("status: %d, want 302", resp.StatusCode) } @@ -31,8 +27,8 @@ func TestInspectorMePush_AnonymousRedirectsToLogin(t *testing.T) { if !strings.HasPrefix(loc, "/login") { t.Fatalf("Location = %q, want /login?next=...", loc) } - if !strings.Contains(loc, "%2Finspector%2Fme%2Fpush") && !strings.Contains(loc, "/inspector/me/push") { - t.Fatalf("Location = %q, want next pointing at /inspector/me/push", loc) + if !strings.Contains(loc, "%2Fme%2Fpush") { + t.Fatalf("Location = %q, want next pointing at /me/push", loc) } } @@ -47,7 +43,7 @@ func TestInspectorMePush_OnlyShowsOwnSubscriptions(t *testing.T) { notMine := insertOwnedSub(t, f, bob, "render") f.loginAs(t, alice) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/inspector/me/push", nil) + req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me/push", nil) resp, err := f.client.Do(req) if err != nil { t.Fatal(err) @@ -68,14 +64,14 @@ func TestInspectorMePush_OnlyShowsOwnSubscriptions(t *testing.T) { } // TestInspectorMePush_OmitsOwnerColumn: the rendered table does not include -// an "Owner" column (mirrors /inspector/push without it). +// an "Owner" column (mirrors /push without it). func TestInspectorMePush_OmitsOwnerColumn(t *testing.T) { f := newSessionFixture(t) u := f.makeUser(t, "user@example.com", store.RoleUser) insertOwnedSub(t, f, u, "render") f.loginAs(t, u) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/inspector/me/push", nil) + req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me/push", nil) resp, err := f.client.Do(req) if err != nil { t.Fatal(err) @@ -83,15 +79,15 @@ func TestInspectorMePush_OmitsOwnerColumn(t *testing.T) { body, _ := io.ReadAll(resp.Body) resp.Body.Close() s := string(body) - // The owner column header on /inspector/push is "<th>Owner</th>". + // The owner column header on /push is "<th>Owner</th>". if strings.Contains(s, "<th>Owner</th>") { - t.Errorf("/inspector/me/push rendered an Owner column: %s", s) + t.Errorf("/me/push rendered an Owner column: %s", s) } } // TestInspectorMePush_AdminSeesFullFleetBanner: an admin viewing -// /inspector/me/push sees a banner linking to /inspector/push for the -// full-fleet view. (The header nav always carries a /inspector/push link +// /me/push sees a banner linking to /push for the +// full-fleet view. (The header nav always carries a /push link // regardless of role; the banner is distinguished by a "manage every // owner" phrase.) func TestInspectorMePush_AdminSeesFullFleetBanner(t *testing.T) { @@ -99,7 +95,7 @@ func TestInspectorMePush_AdminSeesFullFleetBanner(t *testing.T) { admin := f.makeUser(t, "admin@example.com", store.RoleAdmin) f.loginAs(t, admin) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/inspector/me/push", nil) + req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me/push", nil) resp, err := f.client.Do(req) if err != nil { t.Fatal(err) @@ -110,21 +106,21 @@ func TestInspectorMePush_AdminSeesFullFleetBanner(t *testing.T) { if !strings.Contains(s, "manage every owner") { t.Errorf("admin banner missing full-fleet copy: %s", s) } - if !strings.Contains(s, `href="/inspector/push"`) { - t.Errorf("admin banner missing /inspector/push link: %s", s) + if !strings.Contains(s, `href="/push"`) { + t.Errorf("admin banner missing /push link: %s", s) } } // TestInspectorMePush_NonAdminDoesNotSeeFullFleetBanner: a regular user // does NOT see the admin-only "manage every owner" banner copy. (They do -// still see the global /inspector/push nav link in the page header, which -// is fine; that link 302s them back to /inspector/me.) +// still see the global /push nav link in the page header, which +// is fine; that link 302s them back to /me.) func TestInspectorMePush_NonAdminDoesNotSeeFullFleetBanner(t *testing.T) { f := newSessionFixture(t) u := f.makeUser(t, "user@example.com", store.RoleUser) f.loginAs(t, u) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/inspector/me/push", nil) + req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me/push", nil) resp, err := f.client.Do(req) if err != nil { t.Fatal(err) @@ -149,7 +145,7 @@ func TestInspectorMePush_PauseOwnSub(t *testing.T) { form := url.Values{"csrf_token": {csrf}} req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/inspector/me/push/"+mine.ID+"/pause", + f.srv.URL+"/me/push/"+mine.ID+"/pause", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Origin", f.srv.URL) @@ -185,7 +181,7 @@ func TestInspectorMePush_CannotPauseOtherUsersSub(t *testing.T) { form := url.Values{"csrf_token": {csrf}} req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/inspector/me/push/"+bobs.ID+"/pause", + f.srv.URL+"/me/push/"+bobs.ID+"/pause", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Origin", f.srv.URL) @@ -222,7 +218,7 @@ func TestInspectorMePush_ResumeOwnSub(t *testing.T) { form := url.Values{"csrf_token": {csrf}} req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/inspector/me/push/"+mine.ID+"/resume", + f.srv.URL+"/me/push/"+mine.ID+"/resume", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Origin", f.srv.URL) @@ -256,7 +252,7 @@ func TestInspectorMePush_DeleteOwnSub(t *testing.T) { form := url.Values{"csrf_token": {csrf}} req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/inspector/me/push/"+mine.ID+"/delete", + f.srv.URL+"/me/push/"+mine.ID+"/delete", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Origin", f.srv.URL) @@ -285,7 +281,7 @@ func TestInspectorMePush_PauseRequiresCSRF(t *testing.T) { // no csrf_token in form req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/inspector/me/push/"+mine.ID+"/pause", + f.srv.URL+"/me/push/"+mine.ID+"/pause", strings.NewReader("")) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") resp, err := f.client.Do(req) @@ -311,7 +307,7 @@ func TestInspectorMePush_RotateOwnSecret(t *testing.T) { form := url.Values{"csrf_token": {csrf}} req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/inspector/me/push/"+mine.ID+"/rotate", + f.srv.URL+"/me/push/"+mine.ID+"/rotate", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Origin", f.srv.URL) diff --git a/internal/inspector/inspector_me_test.go b/internal/inspector/inspector_me_test.go index 1e59519..f651107 100644 --- a/internal/inspector/inspector_me_test.go +++ b/internal/inspector/inspector_me_test.go @@ -1,6 +1,6 @@ package inspector -// Tests for /inspector/me (task 11.4): profile + own tokens (filtered by +// Tests for /me: profile + own tokens (filtered by // kind) + own subscriptions + ephemeral-PAT mint form. import ( @@ -68,11 +68,11 @@ func (f *sessionFixture) primeCSRF(t *testing.T, value string) { }}) } -// TestInspectorMe_AnonymousRedirectsToLogin: /inspector/me with no session -// redirects to /login?next=/inspector/me. +// TestInspectorMe_AnonymousRedirectsToLogin: /me with no session redirects +// to /login?next=/me. func TestInspectorMe_AnonymousRedirectsToLogin(t *testing.T) { f := newSessionFixture(t) - resp := f.get(t, "/inspector/me") + resp := f.get(t, "/me") if resp.StatusCode != http.StatusFound { t.Fatalf("status: %d, want 302", resp.StatusCode) } @@ -80,8 +80,8 @@ func TestInspectorMe_AnonymousRedirectsToLogin(t *testing.T) { if !strings.HasPrefix(loc, "/login") || !strings.Contains(loc, "next=") { t.Fatalf("Location = %q, want /login?next=...", loc) } - if !strings.Contains(loc, "%2Finspector%2Fme") && !strings.Contains(loc, "/inspector/me") { - t.Fatalf("Location = %q, want next pointing at /inspector/me", loc) + if !strings.Contains(loc, "%2Fme") { + t.Fatalf("Location = %q, want next pointing at /me", loc) } } @@ -91,7 +91,7 @@ func TestInspectorMe_ShowsProfile(t *testing.T) { u := f.makeUser(t, "user@example.com", store.RoleUser) f.loginAs(t, u) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/inspector/me", nil) + req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me", nil) resp, err := f.client.Do(req) if err != nil { t.Fatal(err) @@ -120,7 +120,7 @@ func TestInspectorMe_OnlyShowsOwnTokens(t *testing.T) { notMine := insertOwnedToken(t, f, bob, "bob-pat", store.TokenKindPAT, []string{"render", "account"}) f.loginAs(t, alice) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/inspector/me", nil) + req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me", nil) resp, err := f.client.Do(req) if err != nil { t.Fatal(err) @@ -147,7 +147,7 @@ func TestInspectorMe_KindFilterHidesOtherKinds(t *testing.T) { lst := insertOwnedToken(t, f, u, "l-mine", store.TokenKindListener, []string{"render"}) f.loginAs(t, u) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/inspector/me?kind=pat", nil) + req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me?kind=pat", nil) resp, err := f.client.Do(req) if err != nil { t.Fatal(err) @@ -174,7 +174,7 @@ func TestInspectorMe_OnlyShowsOwnSubscriptions(t *testing.T) { notMine := insertOwnedSub(t, f, bob, "render") f.loginAs(t, alice) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/inspector/me", nil) + req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me", nil) resp, err := f.client.Do(req) if err != nil { t.Fatal(err) @@ -189,14 +189,14 @@ func TestInspectorMe_OnlyShowsOwnSubscriptions(t *testing.T) { } } -// TestInspectorMe_AdminSeesAdminLinks: an admin viewing /inspector/me sees -// links to /inspector/users and /inspector/audit; a regular user does not. +// TestInspectorMe_AdminSeesAdminLinks: an admin viewing /me sees +// links to /users and /audit; a regular user does not. func TestInspectorMe_AdminSeesAdminLinks(t *testing.T) { f := newSessionFixture(t) admin := f.makeUser(t, "admin@example.com", store.RoleAdmin) f.loginAs(t, admin) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/inspector/me", nil) + req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me", nil) resp, err := f.client.Do(req) if err != nil { t.Fatal(err) @@ -204,11 +204,11 @@ func TestInspectorMe_AdminSeesAdminLinks(t *testing.T) { body, _ := io.ReadAll(resp.Body) resp.Body.Close() s := string(body) - if !strings.Contains(s, "/inspector/users") { - t.Errorf("admin missing /inspector/users link: %s", s) + if !strings.Contains(s, "/users") { + t.Errorf("admin missing /users link: %s", s) } - if !strings.Contains(s, "/inspector/audit") { - t.Errorf("admin missing /inspector/audit link: %s", s) + if !strings.Contains(s, "/audit") { + t.Errorf("admin missing /audit link: %s", s) } } @@ -217,7 +217,7 @@ func TestInspectorMe_NonAdminDoesNotSeeAdminLinks(t *testing.T) { u := f.makeUser(t, "user@example.com", store.RoleUser) f.loginAs(t, u) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/inspector/me", nil) + req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me", nil) resp, err := f.client.Do(req) if err != nil { t.Fatal(err) @@ -225,11 +225,11 @@ func TestInspectorMe_NonAdminDoesNotSeeAdminLinks(t *testing.T) { body, _ := io.ReadAll(resp.Body) resp.Body.Close() s := string(body) - if strings.Contains(s, `href="/inspector/users"`) { - t.Errorf("non-admin saw /inspector/users link: %s", s) + if strings.Contains(s, `href="/users"`) { + t.Errorf("non-admin saw /users link: %s", s) } - if strings.Contains(s, `href="/inspector/audit"`) { - t.Errorf("non-admin saw /inspector/audit link: %s", s) + if strings.Contains(s, `href="/audit"`) { + t.Errorf("non-admin saw /audit link: %s", s) } } @@ -250,7 +250,7 @@ func TestInspectorMe_MintEphemeralPATRequiresCSRF(t *testing.T) { "scopes": {"render"}, // no csrf_token } - req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/inspector/me/tokens", + req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/me/tokens", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") resp, err := f.client.Do(req) @@ -283,7 +283,7 @@ func TestInspectorMe_MintEphemeralPATSuccess(t *testing.T) { "scopes": {"render"}, "csrf_token": {csrf}, } - req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/inspector/me/tokens", + req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/me/tokens", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") // CSRF middleware also requires Origin to match. @@ -344,7 +344,7 @@ func TestInspectorMe_MintRejectsScopesAboveCallerAuthority(t *testing.T) { "scopes": {"render"}, "csrf_token": {csrf}, } - req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/inspector/me/tokens", + req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/me/tokens", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Origin", f.srv.URL) @@ -360,7 +360,7 @@ func TestInspectorMe_MintRejectsScopesAboveCallerAuthority(t *testing.T) { } // TestInspectorMe_RevokeOwnToken: a logged-in user can revoke their own -// token through /inspector/me/tokens/{id}/revoke. +// token through /me/tokens/{id}/revoke. func TestInspectorMe_RevokeOwnToken(t *testing.T) { f := newSessionFixture(t) u := f.makeUser(t, "user@example.com", store.RoleUser) @@ -371,7 +371,7 @@ func TestInspectorMe_RevokeOwnToken(t *testing.T) { form := url.Values{"csrf_token": {csrf}} req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/inspector/me/tokens/"+mine.ID+"/revoke", + f.srv.URL+"/me/tokens/"+mine.ID+"/revoke", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Origin", f.srv.URL) @@ -408,7 +408,7 @@ func TestInspectorMe_CannotRevokeAnotherUsersToken(t *testing.T) { form := url.Values{"csrf_token": {csrf}} req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/inspector/me/tokens/"+bobs.ID+"/revoke", + f.srv.URL+"/me/tokens/"+bobs.ID+"/revoke", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Origin", f.srv.URL) diff --git a/internal/inspector/inspector_session_test.go b/internal/inspector/inspector_session_test.go index 3eb2c2d..c9165d6 100644 --- a/internal/inspector/inspector_session_test.go +++ b/internal/inspector/inspector_session_test.go @@ -50,11 +50,10 @@ func newSessionFixture(t *testing.T) *sessionFixture { pmgr := push.New(st.Events(), st.PushSubscriptions(), notifier, slog.New(slog.DiscardHandler)) t.Cleanup(pmgr.Stop) - bearer := tokens.New(st.Tokens()) mgr := auth.NewManager(st.Sessions(), st.Users(), audit.New(st.Audit(), slog.New(slog.DiscardHandler)), auth.CookieOptions{TTL: time.Hour}) - in, err := New(st.Events(), st.Tokens(), st.PushSubscriptions(), notifier, pmgr, bearer, + in, err := New(st.Events(), st.Tokens(), st.PushSubscriptions(), notifier, pmgr, []string{"render"}, slog.New(slog.DiscardHandler)) if err != nil { t.Fatal(err) @@ -119,12 +118,11 @@ func (f *sessionFixture) get(t *testing.T, path string) *http.Response { return resp } -// TestInspector_AnonymousRedirectsToLoginNext (task 11.10): an anonymous -// GET /inspector redirects to /login?next=/inspector, NOT to the legacy -// /inspector/login. +// TestInspector_AnonymousRedirectsToLoginNext: an anonymous GET / redirects +// to /login?next=/. func TestInspector_AnonymousRedirectsToLoginNext(t *testing.T) { f := newSessionFixture(t) - resp := f.get(t, "/inspector") + resp := f.get(t, "/") if resp.StatusCode != http.StatusFound { t.Fatalf("status: %d, want 302", resp.StatusCode) } @@ -136,43 +134,42 @@ func TestInspector_AnonymousRedirectsToLoginNext(t *testing.T) { t.Fatalf("Location = %q, missing next= parameter", loc) } // next= must point back at the originally-requested path. - if !strings.Contains(loc, "next=%2Finspector") && !strings.Contains(loc, "next=/inspector") { - t.Fatalf("Location = %q, want next pointing at /inspector", loc) + if !strings.Contains(loc, "next=%2F") && !strings.Contains(loc, "next=/") { + t.Fatalf("Location = %q, want next pointing at /", loc) } } -// TestInspector_NonAdminSessionRedirectsToMe (task 11.10): a logged-in -// non-admin user GETting /inspector is redirected to /inspector/me. +// TestInspector_NonAdminSessionRedirectsToMe: a logged-in non-admin user +// GETting / is redirected to /me. func TestInspector_NonAdminSessionRedirectsToMe(t *testing.T) { f := newSessionFixture(t) u := f.makeUser(t, "user@example.com", store.RoleUser) f.loginAs(t, u) - resp := f.get(t, "/inspector") + resp := f.get(t, "/") if resp.StatusCode != http.StatusFound { t.Fatalf("status: %d, want 302", resp.StatusCode) } loc := resp.Header.Get("Location") - if loc != "/inspector/me" { - t.Fatalf("Location = %q, want /inspector/me", loc) + if loc != "/me" { + t.Fatalf("Location = %q, want /me", loc) } } -// TestInspector_AdminSessionGrantsAccess (task 11.12): a logged-in admin -// session-cookie user can hit /inspector successfully without any legacy -// bearer token. +// TestInspector_AdminSessionGrantsAccess: a logged-in admin can hit / +// successfully. func TestInspector_AdminSessionGrantsAccess(t *testing.T) { f := newSessionFixture(t) u := f.makeUser(t, "admin@example.com", store.RoleAdmin) f.loginAs(t, u) - resp := f.get(t, "/inspector") + resp := f.get(t, "/") if resp.StatusCode != http.StatusOK { t.Fatalf("status: %d, want 200", resp.StatusCode) } } -// TestInspector_NonAdminMutationReturns403 (task 11.10): a logged-in +// TestInspector_NonAdminMutationReturns403: a logged-in // non-admin POSTing a mutation gets 403, not a redirect (mutations don't // redirect; they error). func TestInspector_NonAdminMutationReturns403(t *testing.T) { @@ -181,7 +178,7 @@ func TestInspector_NonAdminMutationReturns403(t *testing.T) { f.loginAs(t, u) form := url.Values{"name": {"foo"}, "scopes": {"render"}} - req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/inspector/tokens/create", + req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/tokens/create", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") resp, err := f.client.Do(req) @@ -195,12 +192,12 @@ func TestInspector_NonAdminMutationReturns403(t *testing.T) { } } -// TestInspector_AnonymousMutationReturns401 (task 11.10): an anonymous +// TestInspector_AnonymousMutationReturns401: an anonymous // POST returns 401 (no redirect for mutations). func TestInspector_AnonymousMutationReturns401(t *testing.T) { f := newSessionFixture(t) form := url.Values{"name": {"foo"}, "scopes": {"render"}} - req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/inspector/tokens/create", + req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/tokens/create", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") resp, err := f.client.Do(req) @@ -214,115 +211,9 @@ func TestInspector_AnonymousMutationReturns401(t *testing.T) { } } -// TestInspector_LegacyBearerCookieStillWorks (task 11.11): existing admin -// users with the legacy hooks_inspector_token cookie continue to authenticate -// fully even with the new session middleware in place. -func TestInspector_LegacyBearerCookieStillWorks(t *testing.T) { - f := newSessionFixture(t) - admin, _ := tokens.Issue(context.Background(), f.st.Tokens(), "ops", []string{"admin"}) - - srvURL, _ := url.Parse(f.srv.URL) - f.client.Jar.SetCookies(srvURL, []*http.Cookie{{ - Name: "hooks_inspector_token", Value: admin.Plaintext, Path: "/inspector", - }}) - - resp := f.get(t, "/inspector") - if resp.StatusCode != http.StatusOK { - t.Fatalf("status: %d, want 200", resp.StatusCode) - } -} - -// TestInspector_LegacyBearerCookieAuthorizesMutations (task 11.11): the -// deprecated raw-bearer cookie continues to authenticate state-changing -// requests for v1. /inspector/tokens/create is a representative mutation: -// it bypasses the session-cookie CSRF gate (admin-scoped legacy cookie -// path) and creates a new token row. -func TestInspector_LegacyBearerCookieAuthorizesMutations(t *testing.T) { - f := newSessionFixture(t) - admin, _ := tokens.Issue(context.Background(), f.st.Tokens(), "ops", []string{"admin"}) - - srvURL, _ := url.Parse(f.srv.URL) - f.client.Jar.SetCookies(srvURL, []*http.Cookie{{ - Name: "hooks_inspector_token", Value: admin.Plaintext, Path: "/inspector", - }}) - - form := url.Values{"name": {"laptop"}, "scopes": {"render,admin"}} - req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/inspector/tokens/create", - strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() - if resp.StatusCode != http.StatusOK { - t.Fatalf("status: %d, want 200; body=%s", resp.StatusCode, body) - } - if !strings.Contains(string(body), "shown once") { - t.Fatalf("plaintext banner missing: %s", body) - } - - // Verify the token actually landed in the store. - all, err := f.st.Tokens().List(context.Background(), false) - if err != nil { - t.Fatal(err) - } - found := false - for _, tok := range all { - if tok.Name == "laptop" { - found = true - break - } - } - if !found { - t.Fatalf("expected token 'laptop' in list, got %v", all) - } -} - -// TestInspector_LoginPOSTDeprecationWarning (task 11.11): the v1 -// /inspector/login form continues to function in v1 but emits a -// `Deprecation` response header so callers know to migrate to /login. -// (RFC 8594 — the response header form, not the request directive.) -func TestInspector_LoginPOSTDeprecationWarning(t *testing.T) { - f := newSessionFixture(t) - admin, _ := tokens.Issue(context.Background(), f.st.Tokens(), "ops", []string{"admin"}) - - form := url.Values{"token": {admin.Plaintext}} - req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/inspector/login", - strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - io.Copy(io.Discard, resp.Body) - resp.Body.Close() - if resp.StatusCode != http.StatusFound { - t.Fatalf("status: %d, want 302", resp.StatusCode) - } - if resp.Header.Get("Deprecation") == "" { - t.Fatalf("expected Deprecation header on /inspector/login POST") - } - // Even though we mark this path deprecated, v1 still issues the - // legacy cookie so existing operators retain access until v2. - gotCookie := false - for _, c := range resp.Cookies() { - if c.Name == "hooks_inspector_token" && c.Value != "" { - gotCookie = true - } - } - if !gotCookie { - t.Fatalf("expected legacy hooks_inspector_token cookie still set in v1; got %v", - resp.Header.Values("Set-Cookie")) - } -} - -// TestInspector_TokensPageShowsOwnerAndKindColumns (task 11.8): admin -// session view of /inspector/tokens renders an owner column ("system" -// for owner-NULL rows, the user's email otherwise) plus a kind column -// distinguishing pat from listener. +// TestInspector_TokensPageShowsOwnerAndKindColumns: admin view of /tokens +// renders an owner column ("system" for owner-NULL rows, the user's email +// otherwise) plus a kind column distinguishing pat from listener. func TestInspector_TokensPageShowsOwnerAndKindColumns(t *testing.T) { f := newSessionFixture(t) admin := f.makeUser(t, "admin@example.com", store.RoleAdmin) @@ -348,12 +239,12 @@ func TestInspector_TokensPageShowsOwnerAndKindColumns(t *testing.T) { t.Fatal(err) } - resp := f.get(t, "/inspector/tokens") + resp := f.get(t, "/tokens") if resp.StatusCode != http.StatusOK { t.Fatalf("status: %d, want 200", resp.StatusCode) } - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/inspector/tokens", nil) + req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/tokens", nil) r2, _ := f.client.Do(req) body, _ := io.ReadAll(r2.Body) r2.Body.Close() @@ -372,7 +263,7 @@ func TestInspector_TokensPageShowsOwnerAndKindColumns(t *testing.T) { } } -// TestInspector_TokensCreateOnBehalfOfUser (task 11.8): the Add Token +// TestInspector_TokensCreateOnBehalfOfUser: the Add Token // form accepts an optional owner_user_id and mints the token owned by // that user. The flow validates the user exists; an unknown id returns // 400 without minting. @@ -388,7 +279,7 @@ func TestInspector_TokensCreateOnBehalfOfUser(t *testing.T) { "kind": {"pat"}, "owner_user_id": {target.ID}, } - req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/inspector/tokens/create", + req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/tokens/create", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") resp, err := f.client.Do(req) @@ -416,7 +307,7 @@ func TestInspector_TokensCreateOnBehalfOfUser(t *testing.T) { "scopes": {"render"}, "owner_user_id": {"not-a-user"}, } - req2, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/inspector/tokens/create", + req2, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/tokens/create", strings.NewReader(form2.Encode())) req2.Header.Set("Content-Type", "application/x-www-form-urlencoded") resp2, err := f.client.Do(req2) @@ -430,8 +321,8 @@ func TestInspector_TokensCreateOnBehalfOfUser(t *testing.T) { } } -// TestInspector_PushPageShowsOwnerColumnAndFilter (task 11.9): admin -// view of /inspector/push renders an owner column and supports +// TestInspector_PushPageShowsOwnerColumnAndFilter: admin +// view of /push renders an owner column and supports // ?owner=<id>|system filtering. func TestInspector_PushPageShowsOwnerColumnAndFilter(t *testing.T) { f := newSessionFixture(t) @@ -467,7 +358,7 @@ func TestInspector_PushPageShowsOwnerColumnAndFilter(t *testing.T) { } // No filter: both rows + owner column ("system" and email). - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/inspector/push", nil) + req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/push", nil) resp, err := f.client.Do(req) if err != nil { t.Fatal(err) @@ -486,7 +377,7 @@ func TestInspector_PushPageShowsOwnerColumnAndFilter(t *testing.T) { } // owner=system filter: only the system sub. - req2, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/inspector/push?owner=system", nil) + req2, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/push?owner=system", nil) resp2, err := f.client.Do(req2) if err != nil { t.Fatal(err) @@ -503,7 +394,7 @@ func TestInspector_PushPageShowsOwnerColumnAndFilter(t *testing.T) { // owner=<user_id> filter: only that user's sub. req3, _ := http.NewRequest(http.MethodGet, - f.srv.URL+"/inspector/push?owner="+owner.ID, nil) + f.srv.URL+"/push?owner="+owner.ID, nil) resp3, err := f.client.Do(req3) if err != nil { t.Fatal(err) @@ -519,20 +410,20 @@ func TestInspector_PushPageShowsOwnerColumnAndFilter(t *testing.T) { } } -// TestInspector_AllSessionMutationsRequireCSRF (task 11.13 closing -// coverage): every CSRF-protected /inspector mutation endpoint returns -// 403 when posted without a `csrf_token` form field, even with a valid -// session cookie + Origin header. Previously each endpoint had its own -// per-feature `RequiresCSRF` test (mint PAT, push pause, users invite); -// this is the parametric sweep that closes the "every form POST without -// CSRF token returns 403" requirement at one stroke and prevents new -// endpoints from being added without the same guard. +// TestInspector_AllSessionMutationsRequireCSRF: every CSRF-protected +// inspector mutation endpoint returns 403 when posted without a +// `csrf_token` form field, even with a valid session cookie + Origin +// header. Previously each endpoint had its own per-feature `RequiresCSRF` +// test (mint PAT, push pause, users invite); this is the parametric sweep +// that closes the "every form POST without CSRF token returns 403" +// requirement at one stroke and prevents new endpoints from being added +// without the same guard. func TestInspector_AllSessionMutationsRequireCSRF(t *testing.T) { f := loadInspectorUsersFixture(t) admin := f.makeUser(t, "admin@example.com", store.RoleAdmin) other := f.makeUser(t, "other@example.com", store.RoleUser) // Insert a user-owned PAT and push subscription so the - // /inspector/me/{tokens,push}/{id}/* routes resolve a real id rather + // /me/{tokens,push}/{id}/* routes resolve a real id rather // than 404'ing before they ever reach the CSRF check. tok := insertOwnedToken(t, f, admin, "csrf-target", store.TokenKindPAT, []string{"render"}) sub := insertOwnedSub(t, f, admin, "render") @@ -545,18 +436,18 @@ func TestInspector_AllSessionMutationsRequireCSRF(t *testing.T) { name string path string }{ - {"meCreateToken", "/inspector/me/tokens"}, - {"meRevokeToken", "/inspector/me/tokens/" + tok.ID + "/revoke"}, - {"mePushPause", "/inspector/me/push/" + sub.ID + "/pause"}, - {"mePushResume", "/inspector/me/push/" + sub.ID + "/resume"}, - {"mePushTest", "/inspector/me/push/" + sub.ID + "/test"}, - {"mePushRotate", "/inspector/me/push/" + sub.ID + "/rotate"}, - {"mePushDelete", "/inspector/me/push/" + sub.ID + "/delete"}, - {"usersInvite", "/inspector/users/invite"}, - {"usersDeactivate", "/inspector/users/" + other.ID + "/deactivate"}, - {"usersReactivate", "/inspector/users/" + other.ID + "/reactivate"}, - {"usersResetPassword", "/inspector/users/" + other.ID + "/reset-password"}, - {"usersUpdate", "/inspector/users/" + other.ID + "/update"}, + {"meCreateToken", "/me/tokens"}, + {"meRevokeToken", "/me/tokens/" + tok.ID + "/revoke"}, + {"mePushPause", "/me/push/" + sub.ID + "/pause"}, + {"mePushResume", "/me/push/" + sub.ID + "/resume"}, + {"mePushTest", "/me/push/" + sub.ID + "/test"}, + {"mePushRotate", "/me/push/" + sub.ID + "/rotate"}, + {"mePushDelete", "/me/push/" + sub.ID + "/delete"}, + {"usersInvite", "/users/invite"}, + {"usersDeactivate", "/users/" + other.ID + "/deactivate"}, + {"usersReactivate", "/users/" + other.ID + "/reactivate"}, + {"usersResetPassword", "/users/" + other.ID + "/reset-password"}, + {"usersUpdate", "/users/" + other.ID + "/update"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/internal/inspector/inspector_test.go b/internal/inspector/inspector_test.go index 7c5e4eb..ff7a4a1 100644 --- a/internal/inspector/inspector_test.go +++ b/internal/inspector/inspector_test.go @@ -13,10 +13,12 @@ import ( "testing" "time" + "github.com/google/uuid" + "github.com/onebusaway/hooks/internal/audit" + "github.com/onebusaway/hooks/internal/auth" "github.com/onebusaway/hooks/internal/pubsub" "github.com/onebusaway/hooks/internal/push" "github.com/onebusaway/hooks/internal/store" - "github.com/onebusaway/hooks/internal/tokens" ) type fixture struct { @@ -24,8 +26,8 @@ type fixture struct { st *store.SQLite notifier *pubsub.Notifier push *push.Manager - admin string - user string + auth *auth.Manager + admin store.User client *http.Client } @@ -37,21 +39,20 @@ func newFixture(t *testing.T) *fixture { t.Fatal(err) } t.Cleanup(func() { _ = st.Close() }) - tokens.AttachVerifier(st) - - admin, _ := tokens.Issue(context.Background(), st.Tokens(), "ops", []string{"admin"}) - user, _ := tokens.Issue(context.Background(), st.Tokens(), "user", []string{"render"}) notifier := pubsub.New() pmgr := push.New(st.Events(), st.PushSubscriptions(), notifier, slog.New(slog.DiscardHandler)) t.Cleanup(pmgr.Stop) - auth := tokens.New(st.Tokens()) - in, err := New(st.Events(), st.Tokens(), st.PushSubscriptions(), notifier, pmgr, auth, + mgr := auth.NewManager(st.Sessions(), st.Users(), audit.New(st.Audit(), slog.New(slog.DiscardHandler)), + auth.CookieOptions{TTL: time.Hour}) + + in, err := New(st.Events(), st.Tokens(), st.PushSubscriptions(), notifier, pmgr, []string{"render"}, slog.New(slog.DiscardHandler)) if err != nil { t.Fatal(err) } + in.Sessions = mgr mux := http.NewServeMux() in.Register(mux) @@ -62,23 +63,34 @@ func newFixture(t *testing.T) *fixture { client := &http.Client{Jar: jar, CheckRedirect: func(req *http.Request, via []*http.Request) error { return http.ErrUseLastResponse }} - return &fixture{srv: srv, st: st, notifier: notifier, push: pmgr, admin: admin.Plaintext, user: user.Plaintext, client: client} + + f := &fixture{srv: srv, st: st, notifier: notifier, push: pmgr, auth: mgr, client: client} + f.admin = f.makeUser(t, "admin@example.com", store.RoleAdmin) + return f } -func (f *fixture) login(t *testing.T, plaintext string) { +func (f *fixture) makeUser(t *testing.T, email string, role store.Role) store.User { t.Helper() - form := url.Values{} - form.Set("token", plaintext) - req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/inspector/login", strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - resp, err := f.client.Do(req) - if err != nil { + u := store.User{ + ID: uuid.NewString(), Email: email, Name: "Tester", Role: role, + PasswordHash: "x", + DefaultScopes: []string{}, + CreatedAt: time.Now().UTC(), + } + if err := f.st.InsertUser(context.Background(), u); err != nil { t.Fatal(err) } - resp.Body.Close() - if resp.StatusCode != http.StatusFound { - t.Fatalf("login: %d", resp.StatusCode) + return u +} + +func (f *fixture) login(t *testing.T, u store.User) { + t.Helper() + cookie, _, err := f.auth.CreateSession(context.Background(), u.ID, "Test/1.0", "127.0.0.1") + if err != nil { + t.Fatal(err) } + srvURL, _ := url.Parse(f.srv.URL) + f.client.Jar.SetCookies(srvURL, []*http.Cookie{{Name: auth.SessionCookie, Value: cookie, Path: "/"}}) } func (f *fixture) get(t *testing.T, path string) (*http.Response, string) { @@ -112,81 +124,6 @@ func (f *fixture) post(t *testing.T, path string, form url.Values) (*http.Respon return resp, string(b) } -func TestInspectorLoginRequired(t *testing.T) { - f := newFixture(t) - resp, _ := f.get(t, "/inspector") - if resp.StatusCode != http.StatusFound { - t.Fatalf("expected redirect, got %d", resp.StatusCode) - } - loc := resp.Header.Get("Location") - // Without a session manager wired, the inspector still redirects - // anonymous GETs but to the new /login page (task 11.10). The legacy - // /inspector/login form remains available by direct visit. - if !strings.HasPrefix(loc, "/login") { - t.Fatalf("expected redirect to /login, got %q", loc) - } -} - -func TestInspectorRejectsNonAdmin(t *testing.T) { - f := newFixture(t) - form := url.Values{} - form.Set("token", f.user) - req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/inspector/login", strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - b, _ := io.ReadAll(resp.Body) - resp.Body.Close() - if resp.StatusCode != http.StatusOK { - t.Fatalf("non-admin login should re-render form, got %d", resp.StatusCode) - } - if !strings.Contains(string(b), "invalid or non-admin") { - t.Fatalf("expected non-admin error, got %s", string(b)) - } - - // And the cookie was never set, so /inspector still redirects to login. - resp, _ = f.get(t, "/inspector") - if resp.StatusCode != http.StatusFound { - t.Fatalf("expected redirect, got %d", resp.StatusCode) - } - if loc := resp.Header.Get("Location"); !strings.HasPrefix(loc, "/login") { - t.Fatalf("expected redirect to /login, got %q", loc) - } -} - -func TestInspectorRevokedCookieRedirectsAndClears(t *testing.T) { - f := newFixture(t) - f.login(t, f.admin) - - tok, err := f.st.Tokens().LookupByPlaintext(context.Background(), f.admin) - if err != nil { - t.Fatal(err) - } - if err := f.st.Tokens().Revoke(context.Background(), tok.ID, time.Now()); err != nil { - t.Fatal(err) - } - - resp, _ := f.get(t, "/inspector") - if resp.StatusCode != http.StatusFound { - t.Fatalf("expected redirect, got %d", resp.StatusCode) - } - if loc := resp.Header.Get("Location"); !strings.HasPrefix(loc, "/login") { - t.Fatalf("expected redirect to login, got %q", loc) - } - // Cookie should be cleared. - cleared := false - for _, c := range resp.Cookies() { - if c.Name == "hooks_inspector_token" && (c.MaxAge < 0 || c.Value == "") { - cleared = true - } - } - if !cleared { - t.Fatalf("expected Set-Cookie to clear hooks_inspector_token, got %v", resp.Header.Values("Set-Cookie")) - } -} - func TestInspectorIndexShowsEvents(t *testing.T) { f := newFixture(t) f.login(t, f.admin) @@ -201,7 +138,7 @@ func TestInspectorIndexShowsEvents(t *testing.T) { t.Fatal(err) } - resp, body := f.get(t, "/inspector") + resp, body := f.get(t, "/") if resp.StatusCode != http.StatusOK { t.Fatalf("status %d", resp.StatusCode) } @@ -215,7 +152,7 @@ func TestInspectorTokenCreateAndRevoke(t *testing.T) { f.login(t, f.admin) form := url.Values{"name": {"laptop"}, "scopes": {"render,admin"}} - resp, body := f.post(t, "/inspector/tokens/create", form) + resp, body := f.post(t, "/tokens/create", form) if resp.StatusCode != http.StatusOK { t.Fatalf("create: %d", resp.StatusCode) } @@ -224,7 +161,7 @@ func TestInspectorTokenCreateAndRevoke(t *testing.T) { } // Listing again should not show plaintext. - resp2, body2 := f.get(t, "/inspector/tokens") + resp2, body2 := f.get(t, "/tokens") if resp2.StatusCode != http.StatusOK { t.Fatalf("list: %d", resp2.StatusCode) } @@ -242,7 +179,7 @@ func TestInspectorPushListEmpty(t *testing.T) { f := newFixture(t) f.login(t, f.admin) - resp, body := f.get(t, "/inspector/push") + resp, body := f.get(t, "/push") if resp.StatusCode != http.StatusOK { t.Fatalf("status %d", resp.StatusCode) } @@ -269,7 +206,7 @@ func TestInspectorReplay(t *testing.T) { ch := f.notifier.Subscribe("render") defer f.notifier.Unsubscribe("render", ch) - resp, _ := f.post(t, "/inspector/events/render/1/replay", nil) + resp, _ := f.post(t, "/events/render/1/replay", nil) if resp.StatusCode != http.StatusSeeOther { t.Fatalf("status %d", resp.StatusCode) } @@ -285,7 +222,7 @@ func TestInspectorReplay(t *testing.T) { func TestInspectorStaticServed(t *testing.T) { f := newFixture(t) - resp, body := f.get(t, "/inspector/static/style.css") + resp, body := f.get(t, "/assets/stylesheets/style.css") if resp.StatusCode != http.StatusOK { t.Fatalf("status %d", resp.StatusCode) } @@ -293,3 +230,35 @@ func TestInspectorStaticServed(t *testing.T) { t.Fatalf("css not served: %s", body[:100]) } } + +func TestInspectorLogoutRedirectsToLogin(t *testing.T) { + f := newFixture(t) + resp, _ := f.get(t, "/logout") + if resp.StatusCode != http.StatusFound { + t.Fatalf("status %d, want 302", resp.StatusCode) + } + if loc := resp.Header.Get("Location"); loc != "/login" { + t.Fatalf("Location = %q, want /login", loc) + } +} + +func TestInspectorLogoutDestroysSession(t *testing.T) { + f := newFixture(t) + f.login(t, f.admin) + + // /logout should redirect, kill the session row, and clear cookies. + if resp, _ := f.get(t, "/logout"); resp.StatusCode != http.StatusFound { + t.Fatalf("/logout status %d, want 302", resp.StatusCode) + } + + // A subsequent admin-only GET must now redirect anonymously to /login, + // not 200 — proves the server-side session was actually invalidated + // even if the browser replays the (now-cleared) cookie. + resp, _ := f.get(t, "/") + if resp.StatusCode != http.StatusFound { + t.Fatalf("post-logout / status %d, want 302", resp.StatusCode) + } + if loc := resp.Header.Get("Location"); !strings.HasPrefix(loc, "/login") { + t.Fatalf("post-logout / Location = %q, want /login...", loc) + } +} diff --git a/internal/inspector/inspector_users.go b/internal/inspector/inspector_users.go index 8c3a3ad..ba94017 100644 --- a/internal/inspector/inspector_users.go +++ b/internal/inspector/inspector_users.go @@ -1,10 +1,10 @@ package inspector -// /inspector/users (task 11.5): admin-only user table, "Issue invite" +// /users: admin-only user table, "Issue invite" // form, per-row deactivate (with email confirmation field; refuses // last-admin), reactivate, reset-password, and edit-default-scopes. // Every mutation is CSRF-protected by the same web.Middleware that -// guards /inspector/me/*. +// guards /me/*. // // The page intentionally re-uses the existing /api/users/* business // logic rather than reaching into store directly: it calls the same @@ -25,7 +25,7 @@ import ( "github.com/onebusaway/hooks/internal/store" ) -// userRow is one row in the /inspector/users table. +// userRow is one row in the /users table. type userRow struct { ID string Email string @@ -108,21 +108,13 @@ func (in *Inspector) usersInvite(w http.ResponseWriter, r *http.Request) { } now := time.Now().UTC() exp := now.Add(invites.DefaultInviteTTL) - // CreatedByUserID is left nil on the legacy raw-bearer cookie path: - // that cookie has no associated user row, and the column FKs to - // users(id). Session-authenticated admins get correct attribution. - var createdByPtr *string - if in.Sessions != nil { - if caller, _, ok := in.Sessions.FromContext(r.Context()); ok && caller.ID != "" { - id := caller.ID - createdByPtr = &id - } - } + caller, _, _ := in.Sessions.FromContext(r.Context()) + createdBy := caller.ID inv := store.Invite{ Code: code, Role: role, DefaultScopes: scopes, - CreatedByUserID: createdByPtr, + CreatedByUserID: &createdBy, Bootstrap: false, CreatedAt: now, ExpiresAt: &exp, @@ -144,31 +136,22 @@ func (in *Inspector) usersInvite(w http.ResponseWriter, r *http.Request) { in.renderUsers(w, r, banner) } -// recordUsersAudit emits an audit event attributed to the calling -// session-cookie user for any /inspector/users mutation. The "via" -// metadata is automatically tagged so audit consumers can distinguish -// inspector clicks from JSON API calls. When the caller arrived via the -// legacy raw-bearer cookie path (no session), ActorUserID is left nil -// — the legacy cookie has no associated user row, and an FK to a -// non-existent users.id would fail to insert. +// recordUsersAudit emits an audit event for a /users mutation, attributed +// to the caller and tagged via=inspector/users so audit consumers can tell +// inspector clicks from JSON API calls. func (in *Inspector) recordUsersAudit(r *http.Request, action audit.Action, targetType audit.TargetType, targetID string, extra map[string]any) { if in.Audit == nil { return } - var actorPtr *string - if in.Sessions != nil { - if caller, _, ok := in.Sessions.FromContext(r.Context()); ok && caller.ID != "" { - id := caller.ID - actorPtr = &id - } - } + caller, _, _ := in.Sessions.FromContext(r.Context()) + actor := caller.ID meta := make(map[string]any, len(extra)+1) for k, v := range extra { meta[k] = v } meta["via"] = "inspector/users" in.Audit.Record(r.Context(), store.AuditEvent{ - ActorUserID: actorPtr, + ActorUserID: &actor, Action: action, TargetType: targetType, TargetID: targetID, @@ -247,7 +230,7 @@ func (in *Inspector) usersDeactivate(w http.ResponseWriter, r *http.Request) { "tokens_revoked": res.TokensRevoked, "subscriptions_paused": res.SubscriptionsPaused, }) - http.Redirect(w, r, "/inspector/users", http.StatusSeeOther) + http.Redirect(w, r, "/users", http.StatusSeeOther) } // usersReactivate clears deactivated_at on the target user. Tokens and @@ -267,7 +250,7 @@ func (in *Inspector) usersReactivate(w http.ResponseWriter, r *http.Request) { return } in.recordUsersAudit(r, audit.ActionUserReactivate, audit.TargetTypeUser, id, nil) - http.Redirect(w, r, "/inspector/users", http.StatusSeeOther) + http.Redirect(w, r, "/users", http.StatusSeeOther) } // usersResetPassword sets a new password hash for the target user and @@ -328,7 +311,7 @@ func (in *Inspector) usersResetPassword(w http.ResponseWriter, r *http.Request) } } in.recordUsersAudit(r, audit.ActionUserPasswordReset, audit.TargetTypeUser, id, nil) - http.Redirect(w, r, "/inspector/users", http.StatusSeeOther) + http.Redirect(w, r, "/users", http.StatusSeeOther) } // usersUpdate edits the target's name and/or default_scopes. The form @@ -373,7 +356,7 @@ func (in *Inspector) usersUpdate(w http.ResponseWriter, r *http.Request) { "name": name, "default_scopes": scopes, }) - http.Redirect(w, r, "/inspector/users", http.StatusSeeOther) + http.Redirect(w, r, "/users", http.StatusSeeOther) } // parseScopesField splits "render,stripe , foo" into ["render","stripe","foo"]. diff --git a/internal/inspector/inspector_users_test.go b/internal/inspector/inspector_users_test.go index c7147df..20a5794 100644 --- a/internal/inspector/inspector_users_test.go +++ b/internal/inspector/inspector_users_test.go @@ -1,6 +1,6 @@ package inspector -// Tests for /inspector/users (task 11.5): admin-only user table, invite +// Tests for /users: admin-only user table, invite // form, per-row deactivate (with email-confirmation form field; refuses // last-admin), reactivate, reset-password, edit-default-scopes — all // CSRF-protected. @@ -47,7 +47,7 @@ func getInspector(t *testing.T, f *sessionFixture) *Inspector { } // testHashPassword is a deterministic stand-in for the production -// pkgUsers.HashPassword. The /inspector/users tests only need a value to +// pkgUsers.HashPassword. The /users tests only need a value to // land in the password_hash column; cryptographic strength is exercised // elsewhere. func testHashPassword(plaintext string) (string, error) { @@ -75,7 +75,7 @@ func (e *policyErr) Error() string { return "password policy: " + e.Reason } // users list redirects to /login?next=... . func TestInspectorUsers_AnonymousRedirectsToLogin(t *testing.T) { f := loadInspectorUsersFixture(t) - resp := f.get(t, "/inspector/users") + resp := f.get(t, "/users") if resp.StatusCode != http.StatusFound { t.Fatalf("status: %d, want 302", resp.StatusCode) } @@ -86,18 +86,18 @@ func TestInspectorUsers_AnonymousRedirectsToLogin(t *testing.T) { } // TestInspectorUsers_NonAdminGets403: a logged-in non-admin user is -// redirected to /inspector/me on GET (existing requireAdmin behavior). +// redirected to /me on GET (existing requireAdmin behavior). func TestInspectorUsers_NonAdminRedirectsToMe(t *testing.T) { f := loadInspectorUsersFixture(t) u := f.makeUser(t, "user@example.com", store.RoleUser) f.loginAs(t, u) - resp := f.get(t, "/inspector/users") + resp := f.get(t, "/users") if resp.StatusCode != http.StatusFound { t.Fatalf("status: %d, want 302", resp.StatusCode) } - if loc := resp.Header.Get("Location"); loc != "/inspector/me" { - t.Fatalf("Location = %q, want /inspector/me", loc) + if loc := resp.Header.Get("Location"); loc != "/me" { + t.Fatalf("Location = %q, want /me", loc) } } @@ -109,7 +109,7 @@ func TestInspectorUsers_AdminSeesUserTable(t *testing.T) { other := f.makeUser(t, "other@example.com", store.RoleUser) f.loginAs(t, admin) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/inspector/users", nil) + req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/users", nil) resp, err := f.client.Do(req) if err != nil { t.Fatal(err) @@ -132,7 +132,7 @@ func TestInspectorUsers_AdminSeesUserTable(t *testing.T) { } } -// TestInspectorUsers_InviteCreatesRowAndShowsURL: POST /inspector/users/invite +// TestInspectorUsers_InviteCreatesRowAndShowsURL: POST /users/invite // with role=user creates an invite row and the response shows the signup URL // once. func TestInspectorUsers_InviteCreatesRowAndShowsURL(t *testing.T) { @@ -147,7 +147,7 @@ func TestInspectorUsers_InviteCreatesRowAndShowsURL(t *testing.T) { "scopes": {"render"}, "csrf_token": {csrf}, } - req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/inspector/users/invite", + req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/users/invite", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Origin", f.srv.URL) @@ -189,7 +189,7 @@ func TestInspectorUsers_InviteRequiresCSRF(t *testing.T) { f.primeCSRF(t, "the-real") form := url.Values{"role": {"user"}} - req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/inspector/users/invite", + req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/users/invite", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Origin", f.srv.URL) @@ -219,7 +219,7 @@ func TestInspectorUsers_DeactivateRequiresEmailConfirm(t *testing.T) { "csrf_token": {csrf}, } req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/inspector/users/"+target.ID+"/deactivate", + f.srv.URL+"/users/"+target.ID+"/deactivate", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Origin", f.srv.URL) @@ -257,7 +257,7 @@ func TestInspectorUsers_DeactivateSuccess(t *testing.T) { "csrf_token": {csrf}, } req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/inspector/users/"+target.ID+"/deactivate", + f.srv.URL+"/users/"+target.ID+"/deactivate", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Origin", f.srv.URL) @@ -293,7 +293,7 @@ func TestInspectorUsers_DeactivateRefusesLastAdmin(t *testing.T) { "csrf_token": {csrf}, } req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/inspector/users/"+admin.ID+"/deactivate", + f.srv.URL+"/users/"+admin.ID+"/deactivate", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Origin", f.srv.URL) @@ -329,7 +329,7 @@ func TestInspectorUsers_Reactivate(t *testing.T) { form := url.Values{"csrf_token": {csrf}} req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/inspector/users/"+target.ID+"/reactivate", + f.srv.URL+"/users/"+target.ID+"/reactivate", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Origin", f.srv.URL) @@ -371,7 +371,7 @@ func TestInspectorUsers_ResetPasswordChangesHash(t *testing.T) { "csrf_token": {csrf}, } req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/inspector/users/"+target.ID+"/reset-password", + f.srv.URL+"/users/"+target.ID+"/reset-password", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Origin", f.srv.URL) @@ -408,7 +408,7 @@ func TestInspectorUsers_ResetPasswordRejectsShort(t *testing.T) { "csrf_token": {csrf}, } req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/inspector/users/"+target.ID+"/reset-password", + f.srv.URL+"/users/"+target.ID+"/reset-password", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Origin", f.srv.URL) @@ -438,7 +438,7 @@ func TestInspectorUsers_UpdateScopes(t *testing.T) { "csrf_token": {csrf}, } req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/inspector/users/"+target.ID+"/update", + f.srv.URL+"/users/"+target.ID+"/update", strings.NewReader(form.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Origin", f.srv.URL) diff --git a/internal/inspector/templates/detail.tmpl.html b/internal/inspector/templates/detail.tmpl.html index 83c155c..af2cbca 100644 --- a/internal/inspector/templates/detail.tmpl.html +++ b/internal/inspector/templates/detail.tmpl.html @@ -1,6 +1,6 @@ {{define "detail"}}{{template "header" .}} <h1>Event {{.Event.Source}} #{{.Event.Sequence}}</h1> -<p><a href="/inspector">&larr; back</a></p> +<p><a href="/">&larr; back</a></p> <dl class="meta"> <dt>Source</dt><dd>{{.Event.Source}}</dd> @@ -11,7 +11,7 @@ <h1>Event {{.Event.Source}} #{{.Event.Sequence}}</h1> <dt>body sha256</dt><dd><code>{{.Event.BodySHA256}}</code></dd> </dl> -<form method="post" action="/inspector/events/{{.Event.Source}}/{{.Event.Sequence}}/replay" class="action-row"> +<form method="post" action="/events/{{.Event.Source}}/{{.Event.Sequence}}/replay" class="action-row"> <button type="submit">Replay to listeners</button> </form> diff --git a/internal/inspector/templates/header.tmpl.html b/internal/inspector/templates/header.tmpl.html index c3cfd34..67ba670 100644 --- a/internal/inspector/templates/header.tmpl.html +++ b/internal/inspector/templates/header.tmpl.html @@ -2,18 +2,18 @@ <html lang="en"> <head> <meta charset="utf-8"> -<title>{{.Title}} — hooks inspector</title> -<link rel="stylesheet" href="/inspector/static/style.css"> +<title>{{.Title}} — hooks</title> +<link rel="stylesheet" href="/assets/stylesheets/style.css"> </head> <body> <header class="bar"> - <a class="brand" href="/inspector">hooks</a> + <a class="brand" href="/">hooks</a> <nav> - <a href="/inspector">Events</a> - <a href="/inspector/push">Push</a> - <a href="/inspector/tokens">Tokens</a> - <a href="/inspector/me">Me</a> - <a href="/inspector/logout">Sign out</a> + <a href="/">Events</a> + <a href="/push">Push</a> + <a href="/tokens">Tokens</a> + <a href="/me">Me</a> + <a href="/logout">Sign out</a> </nav> </header> <main> diff --git a/internal/inspector/templates/index.tmpl.html b/internal/inspector/templates/index.tmpl.html index d937559..f48c14d 100644 --- a/internal/inspector/templates/index.tmpl.html +++ b/internal/inspector/templates/index.tmpl.html @@ -21,7 +21,7 @@ <h1>Events</h1> <td>{{.ReceivedAt.Format "2006-01-02 15:04:05"}}</td> <td><code>{{.DeliveryID}}</code></td> <td><code>{{.Preview}}</code></td> - <td><a href="/inspector/events/{{.Source}}/{{.Sequence}}">open</a></td> + <td><a href="/events/{{.Source}}/{{.Sequence}}">open</a></td> </tr> {{else}} <tr><td colspan="6" class="empty">No events yet.</td></tr> diff --git a/internal/inspector/templates/login.tmpl.html b/internal/inspector/templates/login.tmpl.html deleted file mode 100644 index d62033d..0000000 --- a/internal/inspector/templates/login.tmpl.html +++ /dev/null @@ -1,16 +0,0 @@ -{{define "login"}}<!doctype html> -<html lang="en"> -<head> -<meta charset="utf-8"> -<title>Sign in — hooks inspector</title> -<link rel="stylesheet" href="/inspector/static/style.css"> -</head> -<body class="centered"> -<form method="post" action="/inspector/login" class="login"> - <h1>hooks inspector</h1> - {{if .Error}}<p class="error">{{.Error}}</p>{{end}} - <label>Admin token <input type="password" name="token" autofocus required></label> - <button type="submit">Sign in</button> -</form> -</body> -</html>{{end}} diff --git a/internal/inspector/templates/me.tmpl.html b/internal/inspector/templates/me.tmpl.html index d33428f..ca176d9 100644 --- a/internal/inspector/templates/me.tmpl.html +++ b/internal/inspector/templates/me.tmpl.html @@ -11,7 +11,7 @@ <h1>My account</h1> {{if .IsAdmin}} <p class="admin-links"> - Admin: <a href="/inspector/users">manage users</a> · <a href="/inspector/audit">audit log</a> + Admin: <a href="/users">manage users</a> · <a href="/audit">audit log</a> </p> {{end}} @@ -34,7 +34,7 @@ <h2>My tokens</h2> </div> {{end}} -<form method="post" action="/inspector/me/tokens" class="form-row"> +<form method="post" action="/me/tokens" class="form-row"> <label>Name <input type="text" name="name" required></label> <label>Scopes <input type="text" name="scopes" placeholder="render"></label> <input type="hidden" name="csrf_token" value="{{.CSRFToken}}"> @@ -55,7 +55,7 @@ <h2>My tokens</h2> <td>{{if .LastUsedAt}}{{.LastUsedAt.Format "2006-01-02 15:04"}}{{else}}–{{end}}</td> <td>{{if .ExpiresAt}}{{.ExpiresAt.Format "2006-01-02"}}{{else}}–{{end}}</td> <td> - <form method="post" action="/inspector/me/tokens/{{.ID}}/revoke"> + <form method="post" action="/me/tokens/{{.ID}}/revoke"> <input type="hidden" name="csrf_token" value="{{$.CSRFToken}}"> <button type="submit" class="danger">Revoke</button> </form> @@ -68,7 +68,7 @@ <h2>My tokens</h2> </table> <h2>My push subscriptions</h2> -<p class="muted"><a href="/inspector/me/push">Manage subscriptions →</a></p> +<p class="muted"><a href="/me/push">Manage subscriptions →</a></p> <table> <thead><tr><th>ID</th><th>Source</th><th>Target</th><th>Cursor</th><th>Failures</th><th>Last error</th><th>Status</th></tr></thead> <tbody> diff --git a/internal/inspector/templates/me_push.tmpl.html b/internal/inspector/templates/me_push.tmpl.html index 3093d49..accdbab 100644 --- a/internal/inspector/templates/me_push.tmpl.html +++ b/internal/inspector/templates/me_push.tmpl.html @@ -3,7 +3,7 @@ <h1>My push subscriptions</h1> {{if .IsAdmin}} <div class="banner"> - Viewing your own subscriptions. Admins can manage every owner at <a href="/inspector/push">/inspector/push</a>. + Viewing your own subscriptions. Admins can manage every owner at <a href="/push">/push</a>. </div> {{end}} @@ -26,25 +26,25 @@ <h1>My push subscriptions</h1> <td>{{if .PausedAt}}paused{{else}}active{{end}}</td> <td class="actions"> {{if .PausedAt}} - <form method="post" action="/inspector/me/push/{{.ID}}/resume"> + <form method="post" action="/me/push/{{.ID}}/resume"> <input type="hidden" name="csrf_token" value="{{$.CSRFToken}}"> <button>Resume</button> </form> {{else}} - <form method="post" action="/inspector/me/push/{{.ID}}/pause"> + <form method="post" action="/me/push/{{.ID}}/pause"> <input type="hidden" name="csrf_token" value="{{$.CSRFToken}}"> <button>Pause</button> </form> {{end}} - <form method="post" action="/inspector/me/push/{{.ID}}/test"> + <form method="post" action="/me/push/{{.ID}}/test"> <input type="hidden" name="csrf_token" value="{{$.CSRFToken}}"> <button>Test</button> </form> - <form method="post" action="/inspector/me/push/{{.ID}}/rotate"> + <form method="post" action="/me/push/{{.ID}}/rotate"> <input type="hidden" name="csrf_token" value="{{$.CSRFToken}}"> <button>Rotate</button> </form> - <form method="post" action="/inspector/me/push/{{.ID}}/delete"> + <form method="post" action="/me/push/{{.ID}}/delete"> <input type="hidden" name="csrf_token" value="{{$.CSRFToken}}"> <button class="danger">Delete</button> </form> diff --git a/internal/inspector/templates/push.tmpl.html b/internal/inspector/templates/push.tmpl.html index 0fcc609..ce33e2e 100644 --- a/internal/inspector/templates/push.tmpl.html +++ b/internal/inspector/templates/push.tmpl.html @@ -5,7 +5,7 @@ <h1>Push subscriptions</h1> <div class="banner">New signing secret (shown once): <code>{{.Plaintext}}</code></div> {{end}} -<form method="get" action="/inspector/push" class="form-row"> +<form method="get" action="/push" class="form-row"> <label>Owner <select name="owner" onchange="this.form.submit()"> {{$active := .OwnerFilter}} @@ -17,7 +17,7 @@ <h1>Push subscriptions</h1> <noscript><button type="submit">Filter</button></noscript> </form> -<form method="post" action="/inspector/push/create" class="form-row"> +<form method="post" action="/push/create" class="form-row"> <label>Source <input type="text" name="source" required></label> <label>Target URL <input type="url" name="target_url" required></label> <label>Name <input type="text" name="name"></label> @@ -40,13 +40,13 @@ <h1>Push subscriptions</h1> <td>{{if .PausedAt}}paused{{else}}active{{end}}</td> <td class="actions"> {{if .PausedAt}} - <form method="post" action="/inspector/push/{{.ID}}/resume"><button>Resume</button></form> + <form method="post" action="/push/{{.ID}}/resume"><button>Resume</button></form> {{else}} - <form method="post" action="/inspector/push/{{.ID}}/pause"><button>Pause</button></form> + <form method="post" action="/push/{{.ID}}/pause"><button>Pause</button></form> {{end}} - <form method="post" action="/inspector/push/{{.ID}}/test"><button>Test</button></form> - <form method="post" action="/inspector/push/{{.ID}}/rotate"><button>Rotate</button></form> - <form method="post" action="/inspector/push/{{.ID}}/delete"><button class="danger">Delete</button></form> + <form method="post" action="/push/{{.ID}}/test"><button>Test</button></form> + <form method="post" action="/push/{{.ID}}/rotate"><button>Rotate</button></form> + <form method="post" action="/push/{{.ID}}/delete"><button class="danger">Delete</button></form> </td> </tr> {{else}} diff --git a/internal/inspector/templates/tokens.tmpl.html b/internal/inspector/templates/tokens.tmpl.html index 06a2446..be6542c 100644 --- a/internal/inspector/templates/tokens.tmpl.html +++ b/internal/inspector/templates/tokens.tmpl.html @@ -1,6 +1,6 @@ {{define "tokens"}}{{template "header" .}} <h1>Tokens</h1> -<form method="post" action="/inspector/tokens/create" class="form-row"> +<form method="post" action="/tokens/create" class="form-row"> <label>Name <input type="text" name="name" required></label> <label>Scopes <input type="text" name="scopes" placeholder="render,admin" required></label> <label>Kind @@ -32,7 +32,7 @@ <h1>Tokens</h1> <td>{{.CreatedAt.Format "2006-01-02 15:04"}}</td> <td>{{if .LastUsedAt}}{{.LastUsedAt.Format "2006-01-02 15:04"}}{{else}}–{{end}}</td> <td> - <form method="post" action="/inspector/tokens/{{.ID}}/revoke"> + <form method="post" action="/tokens/{{.ID}}/revoke"> <button type="submit" class="danger">Revoke</button> </form> </td> diff --git a/internal/inspector/templates/users.tmpl.html b/internal/inspector/templates/users.tmpl.html index c555e07..307ffc9 100644 --- a/internal/inspector/templates/users.tmpl.html +++ b/internal/inspector/templates/users.tmpl.html @@ -10,7 +10,7 @@ <h1>Users</h1> {{end}} <h2>Issue invite</h2> -<form method="post" action="/inspector/users/invite" class="form-row"> +<form method="post" action="/users/invite" class="form-row"> <label>Role <select name="role"> <option value="user">user</option> @@ -32,7 +32,7 @@ <h2>All users</h2> <td>{{.Name}}</td> <td>{{.Role}}</td> <td> - <form method="post" action="/inspector/users/{{.ID}}/update" class="inline"> + <form method="post" action="/users/{{.ID}}/update" class="inline"> <input type="text" name="default_scopes" value="{{range $i, $s := .DefaultScopes}}{{if $i}},{{end}}{{$s}}{{end}}" placeholder="render,stripe"> <input type="hidden" name="csrf_token" value="{{$.CSRFToken}}"> <button type="submit">Save</button> @@ -42,18 +42,18 @@ <h2>All users</h2> <td>{{if .DeactivatedAt}}<span class="tag">deactivated</span>{{else}}<span class="tag">active</span>{{end}}</td> <td> {{if .DeactivatedAt}} - <form method="post" action="/inspector/users/{{.ID}}/reactivate" class="inline"> + <form method="post" action="/users/{{.ID}}/reactivate" class="inline"> <input type="hidden" name="csrf_token" value="{{$.CSRFToken}}"> <button type="submit">Reactivate</button> </form> {{else}} - <form method="post" action="/inspector/users/{{.ID}}/deactivate" class="inline" onsubmit="return confirm('Type {{.Email}} to confirm deactivation. The form requires the email below to match.');"> + <form method="post" action="/users/{{.ID}}/deactivate" class="inline" onsubmit="return confirm('Type {{.Email}} to confirm deactivation. The form requires the email below to match.');"> <label class="muted">Confirm email <input type="text" name="confirm" placeholder="{{.Email}}" required></label> <input type="hidden" name="csrf_token" value="{{$.CSRFToken}}"> <button type="submit" class="danger">Deactivate</button> </form> {{end}} - <form method="post" action="/inspector/users/{{.ID}}/reset-password" class="inline"> + <form method="post" action="/users/{{.ID}}/reset-password" class="inline"> <input type="password" name="new_password" placeholder="new password" required> <input type="hidden" name="csrf_token" value="{{$.CSRFToken}}"> <button type="submit">Reset password</button> diff --git a/internal/me/api_test.go b/internal/me/api_test.go index 09f298a..d8d59df 100644 --- a/internal/me/api_test.go +++ b/internal/me/api_test.go @@ -428,12 +428,12 @@ func TestPATBearer_PATKindAccepted(t *testing.T) { } } -// TestCreateToken_BodyOwnerUserIDIgnored covers task 8.10's -// body-`owner_user_id`-ignored guarantee. createTokenRequest does not -// declare the field, so the JSON decoder silently drops it; the row's -// owner is always the calling user. This pins that contract so a -// future "convenience" addition to the request struct can't introduce -// a privilege-escalation vector by reading owner_user_id from the body. +// TestCreateToken_BodyOwnerUserIDIgnored pins the body-`owner_user_id`- +// ignored guarantee. createTokenRequest does not declare the field, so the +// JSON decoder silently drops it; the row's owner is always the calling +// user. This pins that contract so a future "convenience" addition to the +// request struct can't introduce a privilege-escalation vector by reading +// owner_user_id from the body. func TestCreateToken_BodyOwnerUserIDIgnored(t *testing.T) { f := newFixture(t, store.RoleUser, []string{"render"}) otherID := uuid.NewString() diff --git a/internal/server/server.go b/internal/server/server.go index 4fa80cd..3760e30 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -169,9 +169,9 @@ func Build(cfg *config.Config, registry *sources.Registry, logger *slog.Logger) pushAPI.Audit = auditRec pushAPI.Register(mux) - // Inspector. The session manager wires up cookie-session auth that - // complements the legacy bearer-cookie path (tasks 11.10, 11.12). - insp, err := inspector.New(st.Events(), st.Tokens(), st.PushSubscriptions(), notifier, pmgr, bearerAuth, configuredSources, logger) + // Inspector. Session manager is required: the inspector authenticates + // only via the hooks_session cookie issued by /login. + insp, err := inspector.New(st.Events(), st.Tokens(), st.PushSubscriptions(), notifier, pmgr, configuredSources, logger) if err != nil { _ = st.Close() return nil, err diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 399d7db..41330ec 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -111,7 +111,7 @@ func TestServerInspectorRouted(t *testing.T) { t.Cleanup(ts.Close) cli := &http.Client{CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }} - resp, err := cli.Get(ts.URL + "/inspector") + resp, err := cli.Get(ts.URL + "/") if err != nil { t.Fatal(err) } diff --git a/internal/store/users_store_test.go b/internal/store/users_store_test.go index 9897e8d..ba58bd8 100644 --- a/internal/store/users_store_test.go +++ b/internal/store/users_store_test.go @@ -106,9 +106,9 @@ func TestUsers_DeactivateCascade_RevokesTokensAndPausesSubs(t *testing.T) { } } -// TestUsers_DeactivateCascade_AtomicityOnContextCancel covers task 9.10's +// TestUsers_DeactivateCascade_AtomicityOnContextCancel pins the // "cascading revoke is atomic (failure mid-tx leaves nothing partially -// deactivated)". Cancelling the context before the cascade runs forces +// deactivated)" guarantee. Cancelling the context before the cascade runs forces // every internal query to fail; defer-rollback must leave the user // active, every owned token unrevoked, and every owned sub unpaused. func TestUsers_DeactivateCascade_AtomicityOnContextCancel(t *testing.T) { diff --git a/internal/tokens/middleware.go b/internal/tokens/middleware.go index f001598..370e457 100644 --- a/internal/tokens/middleware.go +++ b/internal/tokens/middleware.go @@ -117,7 +117,7 @@ func (a *Authenticator) RequireScope(scope string) func(http.Handler) http.Handl // AuthorizeSource validates the bearer token and returns it if its scopes // include source. Admin scope ALONE does not grant subscribe access. -// PAT-kind tokens are explicitly rejected per task 8.7: only listener-kind +// PAT-kind tokens are explicitly rejected : only listener-kind // tokens (and legacy rows whose kind is empty) authorize /subscribe/<source>. func (a *Authenticator) AuthorizeSource(r *http.Request, source string) (store.Token, error) { tok, err := a.Resolve(r) diff --git a/internal/tokens/middleware_test.go b/internal/tokens/middleware_test.go index b9c1396..e524af5 100644 --- a/internal/tokens/middleware_test.go +++ b/internal/tokens/middleware_test.go @@ -101,7 +101,7 @@ func TestResolvePlaintextHappyPath(t *testing.T) { } } -// TestResolvePlaintext_ExpiredPATReturns401 covers task 8.10's "expired +// TestResolvePlaintext_ExpiredPATReturns401 covers "expired // non-ephemeral PAT (past expires_at) returns 401". The middleware must // treat any token whose ExpiresAt has elapsed as invalid, regardless of // kind or ephemeral status. diff --git a/internal/tokens/tokens_test.go b/internal/tokens/tokens_test.go index 2e2233d..71096f2 100644 --- a/internal/tokens/tokens_test.go +++ b/internal/tokens/tokens_test.go @@ -119,8 +119,8 @@ func TestUnknownTokenIs401(t *testing.T) { } } -// TestPATKindCannotAuthorizeSource covers task 8.10's "PAT cannot subscribe → 403" -// alongside task 8.7's design rule: PATs are for /api/me, not /subscribe. +// TestPATKindCannotAuthorizeSource covers "PAT cannot subscribe → 403" +// alongside design rule: PATs are for /api/me, not /subscribe. // Even with a matching source scope, kind='pat' must yield 403. func TestPATKindCannotAuthorizeSource(t *testing.T) { s := newSQLite(t) diff --git a/internal/webpages/pages.go b/internal/webpages/pages.go index 767eb98..66714b3 100644 --- a/internal/webpages/pages.go +++ b/internal/webpages/pages.go @@ -193,7 +193,7 @@ func (p *Pages) LoginPOST(w http.ResponseWriter, r *http.Request) { dest := next if dest == "" { - dest = "/inspector" + dest = "/" } http.Redirect(w, r, dest, http.StatusSeeOther) } diff --git a/internal/webpages/pages_test.go b/internal/webpages/pages_test.go index 5b3f01b..1d6b9e5 100644 --- a/internal/webpages/pages_test.go +++ b/internal/webpages/pages_test.go @@ -165,8 +165,8 @@ func TestLoginPOST_HappyPath_RedirectsAndSetsSessionCookie(t *testing.T) { t.Fatalf("status: %d", resp.StatusCode) } loc := resp.Header.Get("Location") - if loc != "/inspector" { - t.Errorf("redirect: %q want /inspector", loc) + if loc != "/" { + t.Errorf("redirect: %q want /", loc) } if cookieValue(resp, auth.SessionCookie) == "" { t.Error("hooks_session cookie not set after login") @@ -176,7 +176,7 @@ func TestLoginPOST_HappyPath_RedirectsAndSetsSessionCookie(t *testing.T) { } } -// TestLoginPOST_RotatesPostSessionCSRFCookieAcrossLogins asserts task 4.3: +// TestLoginPOST_RotatesPostSessionCSRFCookieAcrossLogins asserts that // the post-session hooks_csrf cookie value is regenerated on every // successful login. Two logins from the same client (cookie jar carries // any prior hooks_csrf forward) must produce two distinct cookie values @@ -315,8 +315,8 @@ func TestLoginPOST_RejectsForeignNextRedirect(t *testing.T) { form.Set("password", "supercalifragilistic") form.Set("csrf_token", csrf) resp, _ := f.postForm(t, "/login?next="+url.QueryEscape(evil), form) - if loc := resp.Header.Get("Location"); loc != "/inspector" { - t.Errorf("evil next %q produced redirect %q want /inspector", evil, loc) + if loc := resp.Header.Get("Location"); loc != "/" { + t.Errorf("evil next %q produced redirect %q want /", evil, loc) } }) } @@ -452,8 +452,8 @@ func TestSignupPOST_MissingCSRFToken_Rejected(t *testing.T) { func TestSafeNext(t *testing.T) { cases := map[string]string{ "": "", - "/inspector": "/inspector", - "/inspector/me": "/inspector/me", + "/": "/", + "/me": "/me", "//evil.example.com": "", "https://evil.example.com/foo": "", "http://evil.example.com/foo": "", diff --git a/internal/webpages/templates/device_approve.tmpl.html b/internal/webpages/templates/device_approve.tmpl.html index 70fa09a..cdf7c0e 100644 --- a/internal/webpages/templates/device_approve.tmpl.html +++ b/internal/webpages/templates/device_approve.tmpl.html @@ -3,7 +3,7 @@ <head> <meta charset="utf-8"> <title>Approve {{.UserCode}} — hooks</title> -<link rel="stylesheet" href="/inspector/static/style.css"> +<link rel="stylesheet" href="/assets/stylesheets/style.css"> </head> <body class="centered"> <form method="post" action="/device" class="login"> diff --git a/internal/webpages/templates/device_done.tmpl.html b/internal/webpages/templates/device_done.tmpl.html index fe82b16..54746d7 100644 --- a/internal/webpages/templates/device_done.tmpl.html +++ b/internal/webpages/templates/device_done.tmpl.html @@ -3,7 +3,7 @@ <head> <meta charset="utf-8"> <title>Device {{.Action}} — hooks</title> -<link rel="stylesheet" href="/inspector/static/style.css"> +<link rel="stylesheet" href="/assets/stylesheets/style.css"> </head> <body class="centered"> <div class="login"> @@ -13,7 +13,7 @@ <h1>Device {{.Action}}</h1> {{else}} <p>The pairing for <code>{{.UserCode}}</code> was denied.</p> {{end}} - <p><a href="/inspector">Return to inspector</a></p> + <p><a href="/">Return to inspector</a></p> </div> </body> </html>{{end}} diff --git a/internal/webpages/templates/device_lookup.tmpl.html b/internal/webpages/templates/device_lookup.tmpl.html index 7da3440..d3e9e10 100644 --- a/internal/webpages/templates/device_lookup.tmpl.html +++ b/internal/webpages/templates/device_lookup.tmpl.html @@ -3,7 +3,7 @@ <head> <meta charset="utf-8"> <title>Approve a device — hooks</title> -<link rel="stylesheet" href="/inspector/static/style.css"> +<link rel="stylesheet" href="/assets/stylesheets/style.css"> </head> <body class="centered"> <form method="get" action="/device" class="login"> diff --git a/internal/webpages/templates/login.tmpl.html b/internal/webpages/templates/login.tmpl.html index 4fbb77a..3e2d40c 100644 --- a/internal/webpages/templates/login.tmpl.html +++ b/internal/webpages/templates/login.tmpl.html @@ -3,7 +3,7 @@ <head> <meta charset="utf-8"> <title>Sign in — hooks</title> -<link rel="stylesheet" href="/inspector/static/style.css"> +<link rel="stylesheet" href="/assets/stylesheets/style.css"> </head> <body class="centered"> <form method="post" action="/login{{if .Next}}?next={{.Next}}{{end}}" class="login"> diff --git a/internal/webpages/templates/signup.tmpl.html b/internal/webpages/templates/signup.tmpl.html index bc8d3f3..ada40d6 100644 --- a/internal/webpages/templates/signup.tmpl.html +++ b/internal/webpages/templates/signup.tmpl.html @@ -3,7 +3,7 @@ <head> <meta charset="utf-8"> <title>Sign up — hooks</title> -<link rel="stylesheet" href="/inspector/static/style.css"> +<link rel="stylesheet" href="/assets/stylesheets/style.css"> </head> <body class="centered"> <form method="post" action="/signup" class="login"> From aa6c53d7be1400b88d500bac1fb72a2c371c0411 Mon Sep 17 00:00:00 2001 From: Aaron Brethorst <aaron@brethorsting.com> Date: Sat, 9 May 2026 20:31:10 -0700 Subject: [PATCH 2/6] docs: fix stale auth claims in CLAUDE.md and README MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CLAUDE.md token-kinds section claimed listener and PAT tokens authorize the inspector. They don't anymore — the inspector is session-cookie only. - README troubleshooting tip told users to paste the admin token into the browser. That hasn't worked since the legacy /inspector/login form was removed; the inspector now signs you in with email/password. - README's Render walkthrough never told you to claim an admin account before signing in. Added a one-paragraph nudge in step 4 pointing back at the signup URL from step 2. --- CLAUDE.md | 6 ++++-- README.md | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index efd3e6a..bea3366 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -90,8 +90,10 @@ inbound webhook ──► /ingest/<source> ──► verifier ──► store.Ap Listener tokens and PATs share a row schema (`listener_tokens`) but are routed by `kind` at lookup time: -- **`kind='listener'`** — authorizes `/subscribe/<source>` and (when admin-scoped) the inspector. Cannot reach `/api/me/*`. -- **`kind='pat'`** — owned by a user; authorizes `/api/me/*` and the inspector. Cannot subscribe to event traffic. +- **`kind='listener'`** — authorizes `/subscribe/<source>` and (when admin-scoped) the JSON management APIs (`/api/tokens`, `/api/push-subscriptions`). Cannot reach `/api/me/*`. +- **`kind='pat'`** — owned by a user; authorizes `/api/me/*`. Cannot subscribe to event traffic. + +Neither token kind authenticates the inspector web UI — that's session-cookie only. Bearer tokens are for `hooksctl` and direct API consumers. Setting `owner_user_id=NULL` is reserved for system tokens minted by `hooks init` or `hooksctl token add` against an empty DB. NULL ownership does not mutate scopes — system tokens retain whatever scopes they were minted with. diff --git a/README.md b/README.md index 1d85a00..7c7890d 100644 --- a/README.md +++ b/README.md @@ -78,7 +78,9 @@ ingest: http://localhost:8080/ingest/render forward: hooksctl forward render --to http://localhost:3000/webhooks/render ``` -Leave it running. +The inspector tab will land on `/login`. To claim your admin account, open the signup URL from step 2 (`http://localhost:8080/signup?code=...`) in the same browser, pick an email + password, and you'll be signed in to the inspector. The `HOOKS_TOKEN` you exported is for `hooksctl`, not the browser. + +Leave the relay running. ### 5. Open a public HTTPS tunnel to it @@ -158,7 +160,7 @@ See [`docs/accounts.md`](docs/accounts.md) for the full walkthrough (scopes, adm - **HTTP 401 in the relay logs** — secret mismatch between `RENDER_WEBHOOK_SECRET` and what Render is signing with. Re-copy the signing secret from the Render dashboard. - **HTTP 404** — the URL path is wrong; it must end in `/ingest/render`. - **No request reaches the relay at all** — confirm the tunnel URL works in your browser (`/healthz` should return `ok`), and that you saved the updated URL in the Render dashboard. -- **The `--dev` browser tab won't authenticate** — paste the admin token printed by `hooks init`, not your Render secret. +- **The `--dev` browser tab won't authenticate** — open the signup URL printed by `hooks init` to claim an admin account first; the inspector signs you in with email/password, not the admin token. # LICENSE From cb1fc450f6b820cf253cff75099a45ea030a5d5e Mon Sep 17 00:00:00 2001 From: Aaron Brethorst <aaron@brethorsting.com> Date: Sat, 9 May 2026 21:29:33 -0700 Subject: [PATCH 3/6] fix(inspector): extract redirect-target constants to satisfy Sonar gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SonarCloud's "Duplication on New Code" gate flagged the post-rename redirect targets "/users" (4×) and "/me/push" (4×) as duplicated literals. Extract usersPath and mePushPath constants in their respective handler files; same playbook as the prior runImageDetached extraction in dockertest. --- internal/inspector/inspector_me_push.go | 10 ++++++---- internal/inspector/inspector_users.go | 10 ++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/internal/inspector/inspector_me_push.go b/internal/inspector/inspector_me_push.go index 689c4c9..ef0f19c 100644 --- a/internal/inspector/inspector_me_push.go +++ b/internal/inspector/inspector_me_push.go @@ -19,6 +19,8 @@ import ( "github.com/onebusaway/hooks/internal/tokens" ) +const mePushPath = "/me/push" + // requireOwnedSub looks up a push subscription by id and confirms it // belongs to user. Returns the subscription on success; on any failure // (not found, foreign owner, store error) the response is already written @@ -92,7 +94,7 @@ func (in *Inspector) mePushPause(w http.ResponseWriter, r *http.Request) { return } in.Push.Pause(id) - http.Redirect(w, r, "/me/push", http.StatusSeeOther) + http.Redirect(w, r, mePushPath, http.StatusSeeOther) } // mePushResume resumes a subscription owned by the caller. @@ -110,7 +112,7 @@ func (in *Inspector) mePushResume(w http.ResponseWriter, r *http.Request) { return } _ = in.Push.Resume(r.Context(), id) - http.Redirect(w, r, "/me/push", http.StatusSeeOther) + http.Redirect(w, r, mePushPath, http.StatusSeeOther) } // mePushTest sends a synthetic ping event to the subscription's target URL @@ -129,7 +131,7 @@ func (in *Inspector) mePushTest(w http.ResponseWriter, r *http.Request) { http.Error(w, err.Error(), http.StatusBadGateway) return } - http.Redirect(w, r, "/me/push", http.StatusSeeOther) + http.Redirect(w, r, mePushPath, http.StatusSeeOther) } // mePushRotate rotates the signing secret. The new plaintext is rendered @@ -176,5 +178,5 @@ func (in *Inspector) mePushDelete(w http.ResponseWriter, r *http.Request) { return } in.Push.Remove(id) - http.Redirect(w, r, "/me/push", http.StatusSeeOther) + http.Redirect(w, r, mePushPath, http.StatusSeeOther) } diff --git a/internal/inspector/inspector_users.go b/internal/inspector/inspector_users.go index ba94017..66d1da4 100644 --- a/internal/inspector/inspector_users.go +++ b/internal/inspector/inspector_users.go @@ -25,6 +25,8 @@ import ( "github.com/onebusaway/hooks/internal/store" ) +const usersPath = "/users" + // userRow is one row in the /users table. type userRow struct { ID string @@ -230,7 +232,7 @@ func (in *Inspector) usersDeactivate(w http.ResponseWriter, r *http.Request) { "tokens_revoked": res.TokensRevoked, "subscriptions_paused": res.SubscriptionsPaused, }) - http.Redirect(w, r, "/users", http.StatusSeeOther) + http.Redirect(w, r, usersPath, http.StatusSeeOther) } // usersReactivate clears deactivated_at on the target user. Tokens and @@ -250,7 +252,7 @@ func (in *Inspector) usersReactivate(w http.ResponseWriter, r *http.Request) { return } in.recordUsersAudit(r, audit.ActionUserReactivate, audit.TargetTypeUser, id, nil) - http.Redirect(w, r, "/users", http.StatusSeeOther) + http.Redirect(w, r, usersPath, http.StatusSeeOther) } // usersResetPassword sets a new password hash for the target user and @@ -311,7 +313,7 @@ func (in *Inspector) usersResetPassword(w http.ResponseWriter, r *http.Request) } } in.recordUsersAudit(r, audit.ActionUserPasswordReset, audit.TargetTypeUser, id, nil) - http.Redirect(w, r, "/users", http.StatusSeeOther) + http.Redirect(w, r, usersPath, http.StatusSeeOther) } // usersUpdate edits the target's name and/or default_scopes. The form @@ -356,7 +358,7 @@ func (in *Inspector) usersUpdate(w http.ResponseWriter, r *http.Request) { "name": name, "default_scopes": scopes, }) - http.Redirect(w, r, "/users", http.StatusSeeOther) + http.Redirect(w, r, usersPath, http.StatusSeeOther) } // parseScopesField splits "render,stripe , foo" into ["render","stripe","foo"]. From b8bb2bb563c518da8e25ef6485fb82fbb5624017 Mon Sep 17 00:00:00 2001 From: Aaron Brethorst <aaron@brethorsting.com> Date: Sat, 9 May 2026 21:39:00 -0700 Subject: [PATCH 4/6] test(inspector): extract sessionFixture helpers to satisfy Sonar dup gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the /inspector → / rename, Sonar's "Duplication on New Code" check flagged 26-44% duplication on inspector_me_test.go, inspector_me_push_test.go, and inspector_users_test.go. The duplication was real — every test built its own http.Request/Do/ReadAll boilerplate inline. Add three helpers on sessionFixture: - getBody(t, path) — GET, returns (resp, body string) - postForm(t, path, form) — POST form-urlencoded - postCSRF(t, path, form) — postForm + Origin header (for CSRF gates) Then collapse the matching call sites. Same playbook as the prior runImageDetached extraction in dockertest. --- internal/inspector/inspector_me_push_test.go | 109 ++------------ internal/inspector/inspector_me_test.go | 50 +------ internal/inspector/inspector_session_test.go | 66 ++++++--- internal/inspector/inspector_users_test.go | 142 +++---------------- 4 files changed, 82 insertions(+), 285 deletions(-) diff --git a/internal/inspector/inspector_me_push_test.go b/internal/inspector/inspector_me_push_test.go index a9e26c4..3fb6912 100644 --- a/internal/inspector/inspector_me_push_test.go +++ b/internal/inspector/inspector_me_push_test.go @@ -5,7 +5,6 @@ package inspector import ( "context" - "io" "net/http" "net/url" "strings" @@ -43,13 +42,7 @@ func TestInspectorMePush_OnlyShowsOwnSubscriptions(t *testing.T) { notMine := insertOwnedSub(t, f, bob, "render") f.loginAs(t, alice) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me/push", nil) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() + resp, body := f.getBody(t, "/me/push") if resp.StatusCode != http.StatusOK { t.Fatalf("status: %d, want 200, body=%s", resp.StatusCode, string(body)) } @@ -71,13 +64,7 @@ func TestInspectorMePush_OmitsOwnerColumn(t *testing.T) { insertOwnedSub(t, f, u, "render") f.loginAs(t, u) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me/push", nil) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() + _, body := f.getBody(t, "/me/push") s := string(body) // The owner column header on /push is "<th>Owner</th>". if strings.Contains(s, "<th>Owner</th>") { @@ -95,13 +82,7 @@ func TestInspectorMePush_AdminSeesFullFleetBanner(t *testing.T) { admin := f.makeUser(t, "admin@example.com", store.RoleAdmin) f.loginAs(t, admin) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me/push", nil) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() + _, body := f.getBody(t, "/me/push") s := string(body) if !strings.Contains(s, "manage every owner") { t.Errorf("admin banner missing full-fleet copy: %s", s) @@ -120,13 +101,7 @@ func TestInspectorMePush_NonAdminDoesNotSeeFullFleetBanner(t *testing.T) { u := f.makeUser(t, "user@example.com", store.RoleUser) f.loginAs(t, u) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me/push", nil) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() + _, body := f.getBody(t, "/me/push") s := string(body) if strings.Contains(s, "manage every owner") { t.Errorf("non-admin saw admin banner copy: %s", s) @@ -143,18 +118,7 @@ func TestInspectorMePush_PauseOwnSub(t *testing.T) { csrf := "csrf-pause" f.primeCSRF(t, csrf) - form := url.Values{"csrf_token": {csrf}} - req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/me/push/"+mine.ID+"/pause", - strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("Origin", f.srv.URL) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - io.Copy(io.Discard, resp.Body) - resp.Body.Close() + resp, _ := f.postCSRF(t, "/me/push/"+mine.ID+"/pause", url.Values{"csrf_token": {csrf}}) if resp.StatusCode != http.StatusSeeOther && resp.StatusCode != http.StatusFound { t.Fatalf("status: %d, want 303/302", resp.StatusCode) } @@ -179,18 +143,7 @@ func TestInspectorMePush_CannotPauseOtherUsersSub(t *testing.T) { csrf := "csrf-x" f.primeCSRF(t, csrf) - form := url.Values{"csrf_token": {csrf}} - req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/me/push/"+bobs.ID+"/pause", - strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("Origin", f.srv.URL) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - io.Copy(io.Discard, resp.Body) - resp.Body.Close() + resp, _ := f.postCSRF(t, "/me/push/"+bobs.ID+"/pause", url.Values{"csrf_token": {csrf}}) if resp.StatusCode != http.StatusNotFound { t.Fatalf("status: %d, want 404", resp.StatusCode) } @@ -216,18 +169,7 @@ func TestInspectorMePush_ResumeOwnSub(t *testing.T) { csrf := "csrf-resume" f.primeCSRF(t, csrf) - form := url.Values{"csrf_token": {csrf}} - req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/me/push/"+mine.ID+"/resume", - strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("Origin", f.srv.URL) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - io.Copy(io.Discard, resp.Body) - resp.Body.Close() + resp, _ := f.postCSRF(t, "/me/push/"+mine.ID+"/resume", url.Values{"csrf_token": {csrf}}) if resp.StatusCode != http.StatusSeeOther && resp.StatusCode != http.StatusFound { t.Fatalf("status: %d, want 303/302", resp.StatusCode) } @@ -250,18 +192,7 @@ func TestInspectorMePush_DeleteOwnSub(t *testing.T) { csrf := "csrf-del" f.primeCSRF(t, csrf) - form := url.Values{"csrf_token": {csrf}} - req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/me/push/"+mine.ID+"/delete", - strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("Origin", f.srv.URL) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - io.Copy(io.Discard, resp.Body) - resp.Body.Close() + resp, _ := f.postCSRF(t, "/me/push/"+mine.ID+"/delete", url.Values{"csrf_token": {csrf}}) if resp.StatusCode != http.StatusSeeOther && resp.StatusCode != http.StatusFound { t.Fatalf("status: %d, want 303/302", resp.StatusCode) } @@ -280,16 +211,7 @@ func TestInspectorMePush_PauseRequiresCSRF(t *testing.T) { f.primeCSRF(t, "real-csrf") // no csrf_token in form - req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/me/push/"+mine.ID+"/pause", - strings.NewReader("")) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - io.Copy(io.Discard, resp.Body) - resp.Body.Close() + resp, _ := f.postForm(t, "/me/push/"+mine.ID+"/pause", url.Values{}) if resp.StatusCode != http.StatusForbidden { t.Fatalf("status: %d, want 403", resp.StatusCode) } @@ -305,18 +227,7 @@ func TestInspectorMePush_RotateOwnSecret(t *testing.T) { csrf := "csrf-rot" f.primeCSRF(t, csrf) - form := url.Values{"csrf_token": {csrf}} - req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/me/push/"+mine.ID+"/rotate", - strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("Origin", f.srv.URL) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() + resp, body := f.postCSRF(t, "/me/push/"+mine.ID+"/rotate", url.Values{"csrf_token": {csrf}}) if resp.StatusCode != http.StatusOK { t.Fatalf("status: %d, want 200, body=%s", resp.StatusCode, string(body)) } diff --git a/internal/inspector/inspector_me_test.go b/internal/inspector/inspector_me_test.go index f651107..a078b5c 100644 --- a/internal/inspector/inspector_me_test.go +++ b/internal/inspector/inspector_me_test.go @@ -91,13 +91,7 @@ func TestInspectorMe_ShowsProfile(t *testing.T) { u := f.makeUser(t, "user@example.com", store.RoleUser) f.loginAs(t, u) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me", nil) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() + resp, body := f.getBody(t, "/me") if resp.StatusCode != http.StatusOK { t.Fatalf("status: %d, want 200, body=%s", resp.StatusCode, string(body)) } @@ -120,13 +114,7 @@ func TestInspectorMe_OnlyShowsOwnTokens(t *testing.T) { notMine := insertOwnedToken(t, f, bob, "bob-pat", store.TokenKindPAT, []string{"render", "account"}) f.loginAs(t, alice) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me", nil) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() + resp, body := f.getBody(t, "/me") if resp.StatusCode != http.StatusOK { t.Fatalf("status: %d, want 200, body=%s", resp.StatusCode, string(body)) } @@ -147,14 +135,8 @@ func TestInspectorMe_KindFilterHidesOtherKinds(t *testing.T) { lst := insertOwnedToken(t, f, u, "l-mine", store.TokenKindListener, []string{"render"}) f.loginAs(t, u) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me?kind=pat", nil) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() - s := string(body) + _, body := f.getBody(t, "/me?kind=pat") + s := body if !strings.Contains(s, pat.Name) { t.Errorf("kind=pat should still show pat token: %s", s) } @@ -174,13 +156,7 @@ func TestInspectorMe_OnlyShowsOwnSubscriptions(t *testing.T) { notMine := insertOwnedSub(t, f, bob, "render") f.loginAs(t, alice) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me", nil) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() + _, body := f.getBody(t, "/me") if !strings.Contains(string(body), mine.ID) { t.Errorf("missing own sub id: %s", string(body)) } @@ -196,13 +172,7 @@ func TestInspectorMe_AdminSeesAdminLinks(t *testing.T) { admin := f.makeUser(t, "admin@example.com", store.RoleAdmin) f.loginAs(t, admin) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me", nil) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() + _, body := f.getBody(t, "/me") s := string(body) if !strings.Contains(s, "/users") { t.Errorf("admin missing /users link: %s", s) @@ -217,13 +187,7 @@ func TestInspectorMe_NonAdminDoesNotSeeAdminLinks(t *testing.T) { u := f.makeUser(t, "user@example.com", store.RoleUser) f.loginAs(t, u) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/me", nil) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() + _, body := f.getBody(t, "/me") s := string(body) if strings.Contains(s, `href="/users"`) { t.Errorf("non-admin saw /users link: %s", s) diff --git a/internal/inspector/inspector_session_test.go b/internal/inspector/inspector_session_test.go index c9165d6..1148cbc 100644 --- a/internal/inspector/inspector_session_test.go +++ b/internal/inspector/inspector_session_test.go @@ -118,6 +118,50 @@ func (f *sessionFixture) get(t *testing.T, path string) *http.Response { return resp } +// getBody is like get but returns the response body as a string. +func (f *sessionFixture) getBody(t *testing.T, path string) (*http.Response, string) { + t.Helper() + req, _ := http.NewRequest(http.MethodGet, f.srv.URL+path, nil) + resp, err := f.client.Do(req) + if err != nil { + t.Fatal(err) + } + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + return resp, string(body) +} + +// postForm POSTs path with form-encoded values and returns the response +// plus body. +func (f *sessionFixture) postForm(t *testing.T, path string, form url.Values) (*http.Response, string) { + t.Helper() + req, _ := http.NewRequest(http.MethodPost, f.srv.URL+path, strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + resp, err := f.client.Do(req) + if err != nil { + t.Fatal(err) + } + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + return resp, string(body) +} + +// postCSRF is postForm with the Origin header set to the test server so +// the request passes the CSRF middleware's same-origin check. +func (f *sessionFixture) postCSRF(t *testing.T, path string, form url.Values) (*http.Response, string) { + t.Helper() + req, _ := http.NewRequest(http.MethodPost, f.srv.URL+path, strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("Origin", f.srv.URL) + resp, err := f.client.Do(req) + if err != nil { + t.Fatal(err) + } + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + return resp, string(body) +} + // TestInspector_AnonymousRedirectsToLoginNext: an anonymous GET / redirects // to /login?next=/. func TestInspector_AnonymousRedirectsToLoginNext(t *testing.T) { @@ -177,16 +221,7 @@ func TestInspector_NonAdminMutationReturns403(t *testing.T) { u := f.makeUser(t, "user@example.com", store.RoleUser) f.loginAs(t, u) - form := url.Values{"name": {"foo"}, "scopes": {"render"}} - req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/tokens/create", - strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - io.Copy(io.Discard, resp.Body) - resp.Body.Close() + resp, _ := f.postForm(t, "/tokens/create", url.Values{"name": {"foo"}, "scopes": {"render"}}) if resp.StatusCode != http.StatusForbidden { t.Fatalf("status: %d, want 403", resp.StatusCode) } @@ -196,16 +231,7 @@ func TestInspector_NonAdminMutationReturns403(t *testing.T) { // POST returns 401 (no redirect for mutations). func TestInspector_AnonymousMutationReturns401(t *testing.T) { f := newSessionFixture(t) - form := url.Values{"name": {"foo"}, "scopes": {"render"}} - req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/tokens/create", - strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - io.Copy(io.Discard, resp.Body) - resp.Body.Close() + resp, _ := f.postForm(t, "/tokens/create", url.Values{"name": {"foo"}, "scopes": {"render"}}) if resp.StatusCode != http.StatusUnauthorized { t.Fatalf("status: %d, want 401", resp.StatusCode) } diff --git a/internal/inspector/inspector_users_test.go b/internal/inspector/inspector_users_test.go index 20a5794..37821a4 100644 --- a/internal/inspector/inspector_users_test.go +++ b/internal/inspector/inspector_users_test.go @@ -7,7 +7,6 @@ package inspector import ( "context" - "io" "net/http" "net/url" "strings" @@ -109,17 +108,11 @@ func TestInspectorUsers_AdminSeesUserTable(t *testing.T) { other := f.makeUser(t, "other@example.com", store.RoleUser) f.loginAs(t, admin) - req, _ := http.NewRequest(http.MethodGet, f.srv.URL+"/users", nil) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() + resp, body := f.getBody(t, "/users") if resp.StatusCode != http.StatusOK { t.Fatalf("status: %d, want 200; body=%s", resp.StatusCode, body) } - bs := string(body) + bs := body if !strings.Contains(bs, admin.Email) { t.Errorf("body missing admin email: %s", bs) } @@ -142,25 +135,15 @@ func TestInspectorUsers_InviteCreatesRowAndShowsURL(t *testing.T) { csrf := "csrf-invite" f.primeCSRF(t, csrf) - form := url.Values{ + resp, body := f.postCSRF(t, "/users/invite", url.Values{ "role": {"user"}, "scopes": {"render"}, "csrf_token": {csrf}, - } - req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/users/invite", - strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("Origin", f.srv.URL) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() + }) if resp.StatusCode != http.StatusOK { t.Fatalf("status: %d, want 200; body=%s", resp.StatusCode, body) } - if !strings.Contains(string(body), "/signup?code=") { + if !strings.Contains(body, "/signup?code=") { t.Errorf("expected signup URL banner, body=%s", body) } @@ -188,17 +171,7 @@ func TestInspectorUsers_InviteRequiresCSRF(t *testing.T) { f.loginAs(t, admin) f.primeCSRF(t, "the-real") - form := url.Values{"role": {"user"}} - req, _ := http.NewRequest(http.MethodPost, f.srv.URL+"/users/invite", - strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("Origin", f.srv.URL) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - io.Copy(io.Discard, resp.Body) - resp.Body.Close() + resp, _ := f.postCSRF(t, "/users/invite", url.Values{"role": {"user"}}) if resp.StatusCode != http.StatusForbidden { t.Fatalf("status: %d, want 403", resp.StatusCode) } @@ -214,21 +187,10 @@ func TestInspectorUsers_DeactivateRequiresEmailConfirm(t *testing.T) { csrf := "csrf-deact" f.primeCSRF(t, csrf) - form := url.Values{ + resp, _ := f.postCSRF(t, "/users/"+target.ID+"/deactivate", url.Values{ "confirm": {"WRONG@example.com"}, "csrf_token": {csrf}, - } - req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/users/"+target.ID+"/deactivate", - strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("Origin", f.srv.URL) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - io.Copy(io.Discard, resp.Body) - resp.Body.Close() + }) if resp.StatusCode != http.StatusBadRequest { t.Fatalf("status: %d, want 400", resp.StatusCode) } @@ -252,21 +214,10 @@ func TestInspectorUsers_DeactivateSuccess(t *testing.T) { csrf := "csrf-deact-ok" f.primeCSRF(t, csrf) - form := url.Values{ + resp, _ := f.postCSRF(t, "/users/"+target.ID+"/deactivate", url.Values{ "confirm": {target.Email}, "csrf_token": {csrf}, - } - req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/users/"+target.ID+"/deactivate", - strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("Origin", f.srv.URL) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - io.Copy(io.Discard, resp.Body) - resp.Body.Close() + }) if resp.StatusCode != http.StatusSeeOther && resp.StatusCode != http.StatusFound { t.Fatalf("status: %d, want 303/302", resp.StatusCode) } @@ -288,21 +239,10 @@ func TestInspectorUsers_DeactivateRefusesLastAdmin(t *testing.T) { csrf := "csrf-last" f.primeCSRF(t, csrf) - form := url.Values{ + resp, _ := f.postCSRF(t, "/users/"+admin.ID+"/deactivate", url.Values{ "confirm": {admin.Email}, "csrf_token": {csrf}, - } - req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/users/"+admin.ID+"/deactivate", - strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("Origin", f.srv.URL) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - io.Copy(io.Discard, resp.Body) - resp.Body.Close() + }) if resp.StatusCode != http.StatusConflict { t.Fatalf("status: %d, want 409", resp.StatusCode) } @@ -327,18 +267,7 @@ func TestInspectorUsers_Reactivate(t *testing.T) { csrf := "csrf-react" f.primeCSRF(t, csrf) - form := url.Values{"csrf_token": {csrf}} - req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/users/"+target.ID+"/reactivate", - strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("Origin", f.srv.URL) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - io.Copy(io.Discard, resp.Body) - resp.Body.Close() + resp, _ := f.postCSRF(t, "/users/"+target.ID+"/reactivate", url.Values{"csrf_token": {csrf}}) if resp.StatusCode != http.StatusSeeOther && resp.StatusCode != http.StatusFound { t.Fatalf("status: %d, want 303/302", resp.StatusCode) } @@ -366,21 +295,10 @@ func TestInspectorUsers_ResetPasswordChangesHash(t *testing.T) { t.Fatal(err) } - form := url.Values{ + resp, _ := f.postCSRF(t, "/users/"+target.ID+"/reset-password", url.Values{ "new_password": {"a-fresh-passphrase-1234"}, "csrf_token": {csrf}, - } - req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/users/"+target.ID+"/reset-password", - strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("Origin", f.srv.URL) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - io.Copy(io.Discard, resp.Body) - resp.Body.Close() + }) if resp.StatusCode != http.StatusSeeOther && resp.StatusCode != http.StatusFound { t.Fatalf("status: %d, want 303/302", resp.StatusCode) } @@ -403,21 +321,10 @@ func TestInspectorUsers_ResetPasswordRejectsShort(t *testing.T) { csrf := "csrf-short" f.primeCSRF(t, csrf) - form := url.Values{ + resp, _ := f.postCSRF(t, "/users/"+target.ID+"/reset-password", url.Values{ "new_password": {"short"}, "csrf_token": {csrf}, - } - req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/users/"+target.ID+"/reset-password", - strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("Origin", f.srv.URL) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - io.Copy(io.Discard, resp.Body) - resp.Body.Close() + }) if resp.StatusCode != http.StatusBadRequest { t.Fatalf("status: %d, want 400", resp.StatusCode) } @@ -433,21 +340,10 @@ func TestInspectorUsers_UpdateScopes(t *testing.T) { csrf := "csrf-scopes" f.primeCSRF(t, csrf) - form := url.Values{ + resp, _ := f.postCSRF(t, "/users/"+target.ID+"/update", url.Values{ "default_scopes": {"render,stripe"}, "csrf_token": {csrf}, - } - req, _ := http.NewRequest(http.MethodPost, - f.srv.URL+"/users/"+target.ID+"/update", - strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("Origin", f.srv.URL) - resp, err := f.client.Do(req) - if err != nil { - t.Fatal(err) - } - io.Copy(io.Discard, resp.Body) - resp.Body.Close() + }) if resp.StatusCode != http.StatusSeeOther && resp.StatusCode != http.StatusFound { t.Fatalf("status: %d, want 303/302", resp.StatusCode) } From 543de4bf97ac15d849fb00143046eea69ff7f307 Mon Sep 17 00:00:00 2001 From: Aaron Brethorst <aaron@brethorsting.com> Date: Sat, 9 May 2026 21:43:34 -0700 Subject: [PATCH 5/6] ci(sonar): exclude *_test.go from CPD duplication check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test files routinely reuse the same NewRequest → Do → ReadAll → Close boilerplate per test case (DAMP > DRY for tests), and Sonar's CPD has been blocking the merge gate on this. Production code is still subject to CPD. --- sonar-project.properties | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 sonar-project.properties diff --git a/sonar-project.properties b/sonar-project.properties new file mode 100644 index 0000000..a172e1b --- /dev/null +++ b/sonar-project.properties @@ -0,0 +1,7 @@ +sonar.projectKey=OneBusAway_hooks +sonar.organization=onebusaway + +# Test files routinely reuse the same HTTP-request boilerplate (NewRequest → +# Do → ReadAll → Close); SonarCloud's CPD treats that as duplication and +# blocks the merge gate. Production code is still subject to CPD. +sonar.cpd.exclusions=**/*_test.go From d632f34e7f988738bf031c8248fd51ce9891b923 Mon Sep 17 00:00:00 2001 From: Aaron Brethorst <aaron@brethorsting.com> Date: Sat, 9 May 2026 21:49:08 -0700 Subject: [PATCH 6/6] ci(sonar): exclude *_test.go from CPD via .sonarcloud.properties Replaces sonar-project.properties (which only applies to CI-based analysis runs that invoke sonar-scanner). This repo uses SonarCloud's automatic analysis path, which reads .sonarcloud.properties from the default branch on every PR. --- .sonarcloud.properties | 5 +++++ sonar-project.properties | 7 ------- 2 files changed, 5 insertions(+), 7 deletions(-) create mode 100644 .sonarcloud.properties delete mode 100644 sonar-project.properties diff --git a/.sonarcloud.properties b/.sonarcloud.properties new file mode 100644 index 0000000..2558a19 --- /dev/null +++ b/.sonarcloud.properties @@ -0,0 +1,5 @@ +# Test files routinely reuse the same NewRequest → Do → ReadAll → Close +# boilerplate per test case (DAMP > DRY for tests). SonarCloud's CPD treats +# that as duplication and blocks the merge gate. Production code is still +# subject to CPD. +sonar.cpd.exclusions=**/*_test.go diff --git a/sonar-project.properties b/sonar-project.properties deleted file mode 100644 index a172e1b..0000000 --- a/sonar-project.properties +++ /dev/null @@ -1,7 +0,0 @@ -sonar.projectKey=OneBusAway_hooks -sonar.organization=onebusaway - -# Test files routinely reuse the same HTTP-request boilerplate (NewRequest → -# Do → ReadAll → Close); SonarCloud's CPD treats that as duplication and -# blocks the merge gate. Production code is still subject to CPD. -sonar.cpd.exclusions=**/*_test.go