fix: admin portal redirect loop — serve index.html directly (#96)

cjimti · web-flow · commit 5cb64cc59fd6 · 2026-02-13T19:30:31.000-08:00
Go's http.FileServer treats requests for /index.html as a directory
index and returns 301 Location: ./ — which resolves back to /admin/,
creating an infinite redirect loop. The SPA fallback was routing
through FileServer after setting r.URL.Path = "/index.html", triggering
this behavior on every request.

Pre-read index.html into memory at startup and serve it directly with
w.Write() for the SPA fallback path. Requests for /index.html are also
routed through the direct-serve path to avoid the same redirect.
Static assets (JS, CSS) still use http.FileServer for proper
Content-Type and caching headers.

Extract newSPAHandler(fs.FS) so tests use a synthetic filesystem and
achieve full coverage regardless of whether the frontend was built.
diff --git a/internal/adminui/embed.go b/internal/adminui/embed.go
@@ -35,30 +35,42 @@ func Handler() http.Handler {
 		// Should never happen with a valid embed directive.
 		return http.NotFoundHandler()
 	}
-	fileServer := http.FileServer(http.FS(sub))
+	return newSPAHandler(sub)
+}
+
+// newSPAHandler builds an SPA handler from the given filesystem. Exported via
+// Handler() for production use; called directly in tests with a synthetic FS.
+func newSPAHandler(root fs.FS) http.Handler {
+	fileServer := http.FileServer(http.FS(root))
+
+	// Pre-read index.html so the SPA fallback can serve it directly.
+	// Routing through http.FileServer with path="/index.html" causes a
+	// 301 redirect loop: FileServer treats index.html as a directory
+	// index and redirects to "./" which resolves back to the same URL.
+	indexHTML, _ := fs.ReadFile(root, "index.html")
 
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// Clean the path: strip leading slash for fs lookup
 		path := strings.TrimPrefix(r.URL.Path, "/")
 
-		// If the path points to an actual file, serve it directly
-		if path != "" {
-			if f, err := sub.Open(path); err == nil {
+		// If the path points to an actual file (other than index.html),
+		// serve it via FileServer for proper Content-Type and caching.
+		// index.html is excluded because FileServer redirects it to "./"
+		// which causes a redirect loop when mounted under a prefix.
+		if path != "" && path != "index.html" {
+			if f, err := root.Open(path); err == nil {
 				_ = f.Close()
 				fileServer.ServeHTTP(w, r)
 				return
 			}
 		}
 
-		// SPA fallback: serve index.html for unmatched routes
-		// If index.html doesn't exist (empty dist), return 404
-		f, err := sub.Open("index.html")
-		if err != nil {
+		// SPA fallback: serve index.html directly for unmatched routes.
+		if indexHTML == nil {
 			http.NotFound(w, r)
 			return
 		}
-		_ = f.Close()
-		r.URL.Path = "/index.html"
-		fileServer.ServeHTTP(w, r)
+		w.Header().Set("Content-Type", "text/html; charset=utf-8")
+		_, _ = w.Write(indexHTML)
 	})
 }
diff --git a/internal/adminui/embed_test.go b/internal/adminui/embed_test.go
@@ -4,15 +4,21 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
+	"testing/fstest"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
-// These tests handle two valid states:
-//   - Clean checkout / CI / make verify: dist has only .gitkeep → Available() = false
-//   - After make frontend-build: dist has built assets → Available() = true
-//
-// make verify runs embed-clean first, so the CI path is always exercised.
+// testFS builds a synthetic in-memory filesystem for testing the SPA handler
+// independent of whether the real frontend was built.
+func testFS() fstest.MapFS {
+	return fstest.MapFS{
+		"index.html":       {Data: []byte("<html>SPA</html>")},
+		"assets/app.js":    {Data: []byte("console.log('app')")},
+		"assets/style.css": {Data: []byte("body{}")},
+	}
+}
 
 func TestAvailable(t *testing.T) {
 	// Available() returns whether built frontend assets are embedded.
@@ -29,33 +35,82 @@ func TestHandler_ReturnsHandler(t *testing.T) {
 	assert.NotNil(t, h)
 }
 
-func TestHandler_Root(t *testing.T) {
-	h := Handler()
+func TestSPAHandler_Root_ServesIndexHTML(t *testing.T) {
+	h := newSPAHandler(testFS())
 
 	req := httptest.NewRequest(http.MethodGet, "/", http.NoBody)
 	rec := httptest.NewRecorder()
 	h.ServeHTTP(rec, req)
 
-	if Available() {
-		// SPA fallback rewrites to /index.html; http.FileServer redirects
-		// /index.html back to ./ (Go hides the default index file).
-		assert.Equal(t, http.StatusMovedPermanently, rec.Code)
-	} else {
-		// No index.html in dist — SPA fallback returns 404.
-		assert.Equal(t, http.StatusNotFound, rec.Code)
-	}
+	assert.Equal(t, http.StatusOK, rec.Code)
+	assert.Contains(t, rec.Header().Get("Content-Type"), "text/html")
+	assert.Contains(t, rec.Body.String(), "<html>SPA</html>")
 }
 
-func TestHandler_SPAFallback(t *testing.T) {
-	h := Handler()
+func TestSPAHandler_SPAFallback_ServesIndexHTML(t *testing.T) {
+	h := newSPAHandler(testFS())
 
 	req := httptest.NewRequest(http.MethodGet, "/dashboard", http.NoBody)
 	rec := httptest.NewRecorder()
 	h.ServeHTTP(rec, req)
 
-	if Available() {
-		assert.Equal(t, http.StatusMovedPermanently, rec.Code)
-	} else {
-		assert.Equal(t, http.StatusNotFound, rec.Code)
-	}
+	assert.Equal(t, http.StatusOK, rec.Code)
+	assert.Contains(t, rec.Header().Get("Content-Type"), "text/html")
+	assert.Contains(t, rec.Body.String(), "<html>SPA</html>")
+}
+
+func TestSPAHandler_StaticAsset_ServedByFileServer(t *testing.T) {
+	h := newSPAHandler(testFS())
+
+	req := httptest.NewRequest(http.MethodGet, "/assets/app.js", http.NoBody)
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, req)
+
+	assert.Equal(t, http.StatusOK, rec.Code)
+	assert.Contains(t, rec.Body.String(), "console.log")
+}
+
+func TestSPAHandler_CSSAsset(t *testing.T) {
+	h := newSPAHandler(testFS())
+
+	req := httptest.NewRequest(http.MethodGet, "/assets/style.css", http.NoBody)
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, req)
+
+	assert.Equal(t, http.StatusOK, rec.Code)
+	assert.Contains(t, rec.Body.String(), "body{}")
+}
+
+func TestSPAHandler_NoIndexHTML_Returns404(t *testing.T) {
+	emptyFS := fstest.MapFS{}
+	h := newSPAHandler(emptyFS)
+
+	req := httptest.NewRequest(http.MethodGet, "/", http.NoBody)
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, req)
+
+	assert.Equal(t, http.StatusNotFound, rec.Code)
+}
+
+func TestSPAHandler_NoRedirectLoop(t *testing.T) {
+	h := newSPAHandler(testFS())
+
+	// The exact bug: /index.html must NOT produce a 301
+	req := httptest.NewRequest(http.MethodGet, "/index.html", http.NoBody)
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, req)
+
+	// /index.html is a real file, so FileServer serves it (200), not redirect
+	require.Equal(t, http.StatusOK, rec.Code, "index.html must not redirect")
+}
+
+func TestSPAHandler_NestedUnknownRoute_Fallback(t *testing.T) {
+	h := newSPAHandler(testFS())
+
+	req := httptest.NewRequest(http.MethodGet, "/settings/profile", http.NoBody)
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, req)
+
+	assert.Equal(t, http.StatusOK, rec.Code)
+	assert.Contains(t, rec.Body.String(), "<html>SPA</html>")
 }
