diff options
| author | Paul Buetow <paul@buetow.org> | 2026-05-19 19:22:26 +0300 |
|---|---|---|
| committer | Paul Buetow <paul@buetow.org> | 2026-05-19 19:22:26 +0300 |
| commit | 4215db6718d4ea662084b647398536aa51934ac6 (patch) | |
| tree | ff0b659f172682e4f87ce558d4a66bd3e3e38424 /player-server/internal/api/server_test.go | |
| parent | 35aa611038c97a2ce27510e3898318afca37cac0 (diff) | |
Replace hardcoded public-path whitelist with route registry
Previously internal/api/middleware.go contained an isBootstrapPublic
function with a hardcoded switch over public paths plus three /css/ /js/
/images/ prefix checks. Whenever a new public route was added in
server.go a developer also had to remember to extend the switch — easy
to miss, and the symptom is a silent 307 to /bootstrap.html.
Public-route metadata now lives on the Middleware itself:
* Middleware gets publicPaths map[string]bool and publicPrefixes []string
* RegisterPublic(path) / RegisterPublicPrefix(prefix) populate the registry
* BootstrapRedirect consults isPublic(path) instead of a hardcoded list
Server.routes() registers each public route through new helpers
(handlePublic / handlePublicFunc / handlePublicPrefix) so the mux pattern
and the bypass set are declared together — there is no separate whitelist
to keep in sync.
Routes migrated: 14 exact (bootstrap/login {/api,/api/v1/auth} variants,
/healthz, /readyz, /login.html, /bootstrap.html, /favicon.{ico,svg},
/logo.{png,svg}, /manifest.json, /sw.js) and 4 prefixes (/css/, /js/,
/images/, /s/ for tokenised share URLs).
TestMiddleware_BootstrapRedirect now seeds the registry explicitly
(middleware-level unit test, no full server). A new TestServer_PublicRouteRegistry
asserts the full set of public paths is registered after NewServer().
Refs: agent task ha.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Diffstat (limited to 'player-server/internal/api/server_test.go')
| -rw-r--r-- | player-server/internal/api/server_test.go | 90 |
1 files changed, 90 insertions, 0 deletions
diff --git a/player-server/internal/api/server_test.go b/player-server/internal/api/server_test.go index 91f793d..89bc6c6 100644 --- a/player-server/internal/api/server_test.go +++ b/player-server/internal/api/server_test.go @@ -3,6 +3,9 @@ package api import ( "log/slog" "testing" + + "codeberg.org/snonux/player/internal" + "codeberg.org/snonux/player/internal/service" ) // TestNewServerWithLogger_ErrorsOnNilConfig verifies that the constructor @@ -20,3 +23,90 @@ func TestNewServerWithLogger_ErrorsOnNilConfig(t *testing.T) { t.Fatalf("expected nil Server on error, got %v", srv) } } + +// TestNewServerWithLogger_ErrorsOnNilMediaStreamer verifies that the +// constructor refuses to build a Server when deps.MediaStreamer is nil. +// serveFileResult used to fall back to a default streamer at request time, +// which silently hid wiring mistakes and violated DIP. Construction now +// fails fast, mirroring the explicit-deps pattern in podcast/auth services. +func TestNewServerWithLogger_ErrorsOnNilMediaStreamer(t *testing.T) { + srv, err := NewServerWithLogger(ServerDeps{ + Config: &internal.Config{}, + MediaStreamer: nil, + }, slog.Default()) + if err == nil { + t.Fatal("expected error for nil MediaStreamer, got nil") + } + if srv != nil { + t.Fatalf("expected nil Server on error, got %v", srv) + } +} + +// TestNewServerWithLogger_SucceedsWithMediaStreamer is a happy-path sanity +// check ensuring the new MediaStreamer validation does not reject valid +// dependency sets. +func TestNewServerWithLogger_SucceedsWithMediaStreamer(t *testing.T) { + srv, err := NewServerWithLogger(ServerDeps{ + Config: &internal.Config{}, + MediaStreamer: service.NewMediaStreamer(nil), + }, slog.Default()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if srv == nil { + t.Fatal("expected non-nil Server") + } +} + +// TestServer_PublicRouteRegistry verifies that the public-route registry on +// the Middleware is populated by Server.routes() — i.e. the new "register +// at declaration time" mechanism actually wires every previously-hardcoded +// public path. If a route is added in server.go without using the +// handlePublic* helpers, this test catches the regression before the +// silent-401/redirect bug bites users in production. +func TestServer_PublicRouteRegistry(t *testing.T) { + srv, err := NewServerWithLogger(ServerDeps{ + Config: &internal.Config{}, + MediaStreamer: service.NewMediaStreamer(nil), + }, slog.Default()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Exact paths that must be public for bootstrap/login/probes to work + // before any user exists. + wantExact := []string{ + "/bootstrap.html", "/api/bootstrap", "/api/v1/auth/bootstrap", + "/login.html", "/api/login", "/api/v1/auth/login", + "/healthz", "/readyz", + "/favicon.svg", "/favicon.ico", "/logo.svg", "/logo.png", + "/manifest.json", "/sw.js", + } + for _, p := range wantExact { + if !srv.mw.isPublic(p) { + t.Errorf("expected %q to be public, but isPublic returned false", p) + } + } + + // Prefixed routes: static asset trees and dynamic share URLs. + wantPrefixed := []string{ + "/css/site.css", + "/js/app.js", + "/images/logo.png", + "/s/abcdef", + "/s/abcdef/stream", + } + for _, p := range wantPrefixed { + if !srv.mw.isPublic(p) { + t.Errorf("expected %q to be public via prefix, but isPublic returned false", p) + } + } + + // Negative: an arbitrary protected path must NOT be public. + if srv.mw.isPublic("/api/media") { + t.Error("/api/media should not be public") + } + if srv.mw.isPublic("/") { + t.Error("/ should not be public") + } +} |
