summaryrefslogtreecommitdiff
path: root/player-server/internal/api/server_test.go
diff options
context:
space:
mode:
authorPaul Buetow <paul@buetow.org>2026-05-19 19:22:26 +0300
committerPaul Buetow <paul@buetow.org>2026-05-19 19:22:26 +0300
commit4215db6718d4ea662084b647398536aa51934ac6 (patch)
treeff0b659f172682e4f87ce558d4a66bd3e3e38424 /player-server/internal/api/server_test.go
parent35aa611038c97a2ce27510e3898318afca37cac0 (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.go90
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")
+ }
+}