From 44928263544e0a9e771bf62dd7a0575cbb8c20d2 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 29 Apr 2026 17:27:47 -0400 Subject: [PATCH] feat(auth): add RequireAdmin middleware for /api/admin/* routes Replaces the old X-API-Token-based RequireAdmin in middleware.go with a context-aware RequireAdmin() that runs after RequireUser, checks user.IsAdmin, and returns 403 {"error":"not_authorized"} for non-admins or 500 {"error":"internal_error"} if RequireUser was bypassed. Updates server.go to mount RequireUser then RequireAdmin on the /api/admin group. Co-Authored-By: Claude Sonnet 4.6 --- internal/auth/admin.go | 45 ++++++++++++++++++ internal/auth/admin_test.go | 94 +++++++++++++++++++++++++++++++++++++ internal/auth/middleware.go | 41 ++-------------- internal/server/server.go | 3 +- 4 files changed, 144 insertions(+), 39 deletions(-) create mode 100644 internal/auth/admin.go create mode 100644 internal/auth/admin_test.go diff --git a/internal/auth/admin.go b/internal/auth/admin.go new file mode 100644 index 00000000..7cd9b75c --- /dev/null +++ b/internal/auth/admin.go @@ -0,0 +1,45 @@ +package auth + +import ( + "encoding/json" + "net/http" +) + +// RequireAdmin is a middleware that MUST run after RequireUser. It reads the +// authenticated user from request context and rejects non-admin callers with +// 403. If no user is in context (RequireUser was bypassed), it returns 500 — +// that is a programmer error in the routing setup, not a client error. +// +// Mount this on /api/admin/* after auth.RequireUser: +// +// r.Route("/api/admin", func(admin chi.Router) { +// admin.Use(auth.RequireUser(pool)) +// admin.Use(auth.RequireAdmin()) +// ... +// }) +func RequireAdmin() func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + user, ok := UserFromContext(r.Context()) + if !ok { + // Programmer error: RequireUser was not mounted before RequireAdmin. + writeAdminErr(w, http.StatusInternalServerError, "internal_error") + return + } + if !user.IsAdmin { + writeAdminErr(w, http.StatusForbidden, "not_authorized") + return + } + next.ServeHTTP(w, r) + }) + } +} + +// writeAdminErr writes a flat JSON error envelope {"error":""} and sets +// Content-Type. Uses a flat envelope (not the nested api.errorBody shape) +// because the spec for /api/admin/* errors defines {"error":""} directly. +func writeAdminErr(w http.ResponseWriter, status int, code string) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(map[string]string{"error": code}) +} diff --git a/internal/auth/admin_test.go b/internal/auth/admin_test.go new file mode 100644 index 00000000..393738b8 --- /dev/null +++ b/internal/auth/admin_test.go @@ -0,0 +1,94 @@ +package auth + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// injectUser returns a copy of r with the given user stored under userCtxKey. +// This mirrors what RequireUser does at runtime. +func injectUser(r *http.Request, u dbq.User) *http.Request { + ctx := context.WithValue(r.Context(), userCtxKey, u) + return r.WithContext(ctx) +} + +func TestRequireAdmin_AdminPasses(t *testing.T) { + called := false + stub := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + w.WriteHeader(http.StatusOK) + }) + + h := RequireAdmin()(stub) + + req := injectUser( + httptest.NewRequest(http.MethodGet, "/api/admin/test", nil), + dbq.User{IsAdmin: true}, + ) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("status = %d, want 200", w.Code) + } + if !called { + t.Error("stub handler was not called for admin user") + } +} + +func TestRequireAdmin_NonAdminReturns403(t *testing.T) { + stub := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatal("stub handler must not be called for non-admin user") + }) + + h := RequireAdmin()(stub) + + req := injectUser( + httptest.NewRequest(http.MethodGet, "/api/admin/test", nil), + dbq.User{IsAdmin: false}, + ) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + + if w.Code != http.StatusForbidden { + t.Errorf("status = %d, want 403", w.Code) + } + body := w.Body.String() + if !strings.Contains(body, "not_authorized") { + t.Errorf("body %q does not contain %q", body, "not_authorized") + } + ct := w.Header().Get("Content-Type") + if !strings.HasPrefix(ct, "application/json") { + t.Errorf("Content-Type = %q, want application/json", ct) + } +} + +func TestRequireAdmin_NoUserContextReturns500(t *testing.T) { + stub := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatal("stub handler must not be called when no user is in context") + }) + + h := RequireAdmin()(stub) + + // Do NOT inject a user — simulate RequireUser being bypassed. + req := httptest.NewRequest(http.MethodGet, "/api/admin/test", nil) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + + if w.Code != http.StatusInternalServerError { + t.Errorf("status = %d, want 500", w.Code) + } + body := w.Body.String() + if !strings.Contains(body, "internal_error") { + t.Errorf("body %q does not contain %q", body, "internal_error") + } + ct := w.Header().Get("Content-Type") + if !strings.HasPrefix(ct, "application/json") { + t.Errorf("Content-Type = %q, want application/json", ct) + } +} diff --git a/internal/auth/middleware.go b/internal/auth/middleware.go index 0f847480..1438c418 100644 --- a/internal/auth/middleware.go +++ b/internal/auth/middleware.go @@ -2,11 +2,6 @@ package auth import ( "context" - "errors" - "net/http" - - "github.com/jackc/pgx/v5" - "github.com/jackc/pgx/v5/pgxpool" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) @@ -15,39 +10,9 @@ type ctxKey int const userCtxKey ctxKey = 1 -// RequireAdmin gates a handler on X-API-Token matching an admin user. This is -// the Minstrel-native token path (`/api/*`); Subsonic-compatible auth under -// `/rest/*` lands with the Subsonic server. -func RequireAdmin(pool *pgxpool.Pool) func(http.Handler) http.Handler { - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - token := r.Header.Get("X-API-Token") - if token == "" { - http.Error(w, "missing X-API-Token", http.StatusUnauthorized) - return - } - q := dbq.New(pool) - user, err := q.GetUserByAPIToken(r.Context(), token) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - http.Error(w, "invalid token", http.StatusUnauthorized) - return - } - http.Error(w, "auth lookup failed", http.StatusInternalServerError) - return - } - if !user.IsAdmin { - http.Error(w, "admin required", http.StatusForbidden) - return - } - ctx := context.WithValue(r.Context(), userCtxKey, user) - next.ServeHTTP(w, r.WithContext(ctx)) - }) - } -} - -// UserFromContext returns the authenticated user when the request passed -// through RequireAdmin (or a future RequireUser middleware). +// UserFromContext returns the authenticated user placed in context by +// RequireUser. Returns false when RequireUser has not run (e.g. in tests that +// bypass the middleware, or programmer-error routing). func UserFromContext(ctx context.Context) (dbq.User, bool) { u, ok := ctx.Value(userCtxKey).(dbq.User) return u, ok diff --git a/internal/server/server.go b/internal/server/server.go index 3e449693..9358e8d2 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -57,7 +57,8 @@ func (s *Server) Router() http.Handler { ) api.Mount(r, s.Pool, s.Logger, writer, s.RecommendationCfg) r.Route("/api/admin", func(admin chi.Router) { - admin.Use(auth.RequireAdmin(s.Pool)) + admin.Use(auth.RequireUser(s.Pool)) + admin.Use(auth.RequireAdmin()) if s.Scanner != nil { admin.Post("/scan", s.handleAdminScan) }