diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c846c2..85560fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,50 @@ All notable changes to Armature are documented here. +## v0.9.1 — Server-Side Sub-Path Routing + +Hardens multi-surface routing so full-page refreshes on sub-paths work +correctly server-side, with early auth optimization and back-button +resilience. + +**Route consolidation** + +- Root (`/s/:slug`) and catch-all (`/s/:slug/*path`) handlers now share + a single `dispatch` function. The root handler injects `path="/"` and + delegates, eliminating the separate code path. + +**Early auth short-circuit** + +- New `aggregateAccess()` function scans all surfaces to determine + whether a package is all-public, all-authenticated, or mixed. +- All-authenticated packages redirect to login before surface matching, + skipping `matchSurface()` + `evaluateAccess()` overhead. +- Mixed-access packages fall through to per-surface checks as before. + +**SDK back-button fix** + +- On boot, `history.replaceState()` seeds the initial history entry + with `__SURFACE_PATH__` and `__SURFACE_PARAMS__`. This fixes + back-button navigation from `sw.navigate()` to the initial + server-rendered view. +- SDK version bumped to `0.9.1`. + +**Tests: 13 new** + +- 5 `aggregateAccess` unit tests (all-public, all-auth, mixed, + default-access, single-public). +- 8 handler integration tests covering root refresh, sub-path static + and param refresh, unauth redirect, public anonymous, mixed-access, + non-matching sub-path, and early auth short-circuit. + +**Modified files:** + +- `server/pages/pages.go` — `aggregateAccess()`, route consolidation, + early auth short-circuit in `RenderExtensionSurface` +- `server/pages/pages_surface_match_test.go` — 5 new unit tests +- `server/pages/pages_handler_test.go` — 8 new integration tests +- `src/js/sw/sdk/index.js` — `history.replaceState()` seed, version bump + ## v0.9.0 — Multi-Surface Packages Packages can now declare multiple surfaces — each with its own path, access diff --git a/ROADMAP.md b/ROADMAP.md index 0af80d7..1d73aa9 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -84,12 +84,12 @@ titles, and layouts. Unified route tree dispatches between surface rendering and ext API calls. `sw.navigate()` for client-side sub-path routing. Design doc: `docs/DESIGN-multi-surface.md`. -**v0.9.1 — Server-Side Sub-Path Routing** +**v0.9.1 — Server-Side Sub-Path Routing** *(completed)* -Full-page refresh on sub-paths (e.g. `/s/my-pkg/monitor`) currently -requires client-side routing. This version restructures the Gin route -tree so the kernel resolves sub-paths server-side, including proper -`OptionalAuth` for packages with mixed public/authenticated surfaces. +Consolidated root and catch-all route handlers into a unified dispatcher. +Added `aggregateAccess()` for early auth short-circuit on all-authenticated +packages. SDK seeds initial history state for back-button resilience. +8 handler integration tests + 5 aggregateAccess unit tests. **v0.9.2 — Starlark Converter Consolidation + Snapshot Cleanup** diff --git a/VERSION b/VERSION index ac39a10..f374f66 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.9.0 +0.9.1 diff --git a/server/pages/pages.go b/server/pages/pages.go index bdc1a1e..deeeeb2 100644 --- a/server/pages/pages.go +++ b/server/pages/pages.go @@ -470,6 +470,16 @@ func (e *Engine) RenderExtensionSurface() gin.HandlerFunc { return } + // ── Early auth short-circuit ───────────────────────────── + // For all-authenticated packages, redirect before surface matching. + if rawSurfaces, ok := sr.Manifest["surfaces"].([]any); ok && len(rawSurfaces) > 0 { + profile := aggregateAccess(rawSurfaces) + if profile == "authenticated" && c.GetString("user_id") == "" { + c.Redirect(http.StatusTemporaryRedirect, e.cfg.BasePath+"/login") + return + } + } + // ── Multi-surface resolution ───────────────────────────── var surfacePath string var surfaceParams map[string]string @@ -611,6 +621,35 @@ func splitPath(p string) []string { return parts } +// aggregateAccess scans surfaces to determine the package's access profile. +// Returns "public" (all public), "authenticated" (all require auth), or "mixed". +func aggregateAccess(surfaces []any) string { + hasPublic := false + hasAuth := false + for _, raw := range surfaces { + s, ok := raw.(map[string]any) + if !ok { + continue + } + access, _ := s["access"].(string) + if access == "" { + access = "authenticated" // default + } + if access == "public" { + hasPublic = true + } else { + hasAuth = true + } + if hasPublic && hasAuth { + return "mixed" + } + } + if hasPublic { + return "public" + } + return "authenticated" +} + // evaluateAccess checks whether the current request meets an access requirement. func evaluateAccess(c *gin.Context, access string) bool { switch { @@ -747,12 +786,14 @@ func (e *Engine) RegisterExtensionRoutes( sGroup := base.Group("/s/:slug") sGroup.Use(optAuth) // populate user context if token present; don't require it - // Root path: /s/:slug (no sub-path) — surface only - sGroup.GET("", surfaceHandler) - - // Sub-paths: /s/:slug/* — dispatch between API and surface - sGroup.Any("/*path", func(c *gin.Context) { + // Dispatcher handles both root and sub-path requests. + // API requests (/s/:slug/api/*) go to extAPIHandler with JWT auth. + // Everything else goes to the surface handler with per-surface access checks. + dispatch := func(c *gin.Context) { subPath := c.Param("path") + if subPath == "" { + subPath = "/" + } // API requests: /s/:slug/api/* if strings.HasPrefix(subPath, "/api/") || subPath == "/api" { @@ -783,7 +824,17 @@ func (e *Engine) RegisterExtensionRoutes( } surfaceHandler(c) + } + + // Root path: /s/:slug (Gin requires a separate registration since + // /*path doesn't match the bare group path) + sGroup.GET("", func(c *gin.Context) { + c.Params = append(c.Params, gin.Param{Key: "path", Value: "/"}) + dispatch(c) }) + + // Sub-paths: /s/:slug/*path — API and surface dispatch + sGroup.Any("/*path", dispatch) } // surfaceHandler returns the appropriate Gin handler for a surface. diff --git a/server/pages/pages_handler_test.go b/server/pages/pages_handler_test.go new file mode 100644 index 0000000..969af19 --- /dev/null +++ b/server/pages/pages_handler_test.go @@ -0,0 +1,293 @@ +package pages + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gin-gonic/gin" + + "armature/config" + "armature/store" +) + +// ── mock PackageStore ──────────────────────────────────────── + +// mockPackageStore implements store.PackageStore with in-memory data. +// Only Get and List are wired; everything else is a no-op stub. +type mockPackageStore struct { + pkgs map[string]*store.PackageRegistration +} + +func newMockPackageStore(pkgs ...*store.PackageRegistration) *mockPackageStore { + m := &mockPackageStore{pkgs: make(map[string]*store.PackageRegistration)} + for _, p := range pkgs { + m.pkgs[p.ID] = p + } + return m +} + +func (m *mockPackageStore) Get(_ context.Context, id string) (*store.PackageRegistration, error) { + if p, ok := m.pkgs[id]; ok { + return p, nil + } + return nil, nil +} + +func (m *mockPackageStore) List(_ context.Context) ([]store.PackageRegistration, error) { + var out []store.PackageRegistration + for _, p := range m.pkgs { + out = append(out, *p) + } + return out, nil +} + +func (m *mockPackageStore) ListEnabled(_ context.Context) ([]string, error) { + var ids []string + for id, p := range m.pkgs { + if p.Enabled { + ids = append(ids, id) + } + } + return ids, nil +} + +// Stubs — not exercised by the pages handler tests. +func (m *mockPackageStore) Seed(context.Context, string, string, string, map[string]any) error { return nil } +func (m *mockPackageStore) SetEnabled(context.Context, string, bool) error { return nil } +func (m *mockPackageStore) SetStatus(context.Context, string, string) error { return nil } +func (m *mockPackageStore) Delete(context.Context, string) error { return nil } +func (m *mockPackageStore) Create(context.Context, *store.PackageRegistration) error { return nil } +func (m *mockPackageStore) Update(context.Context, string, *store.PackageRegistration) error { return nil } +func (m *mockPackageStore) ListByType(context.Context, string) ([]store.PackageRegistration, error) { return nil, nil } +func (m *mockPackageStore) ListEnabledByType(context.Context, string) ([]store.PackageRegistration, error) { return nil, nil } +func (m *mockPackageStore) ListForUser(context.Context, string) ([]store.UserPackage, error) { return nil, nil } +func (m *mockPackageStore) GetUserSettings(context.Context, string, string) (*store.PackageUserSettings, error) { return nil, nil } +func (m *mockPackageStore) SetUserSettings(context.Context, *store.PackageUserSettings) error { return nil } +func (m *mockPackageStore) DeleteUserSettings(context.Context, string, string) error { return nil } +func (m *mockPackageStore) ListVisiblePackages(context.Context, string) ([]store.PackageRegistration, error) { return nil, nil } +func (m *mockPackageStore) SetSchemaVersion(context.Context, string, int) error { return nil } +func (m *mockPackageStore) GetPackageSettings(context.Context, string) (json.RawMessage, error) { return nil, nil } +func (m *mockPackageStore) SetPackageSettings(context.Context, string, json.RawMessage) error { return nil } +func (m *mockPackageStore) GetTeamSettings(context.Context, string, string) (json.RawMessage, error) { return nil, nil } +func (m *mockPackageStore) SetTeamSettings(context.Context, string, string, json.RawMessage) error { return nil } +func (m *mockPackageStore) DeleteTeamSettings(context.Context, string, string) error { return nil } + +// ── test helpers ───────────────────────────────────────────── + +// testEngine creates a minimal Engine with parsed templates and mock stores. +func testEngine(t *testing.T, pkgs ...*store.PackageRegistration) *Engine { + t.Helper() + gin.SetMode(gin.TestMode) + e := &Engine{ + cfg: &config.Config{BasePath: ""}, + stores: store.Stores{Packages: newMockPackageStore(pkgs...)}, + loaders: make(map[string]DataLoaderFunc), + } + e.parseTemplates() + e.registerCoreSurfaces() + return e +} + +// testRouter builds a Gin router with extension routes registered. +func testRouter(t *testing.T, pkgs ...*store.PackageRegistration) *gin.Engine { + t.Helper() + engine := testEngine(t, pkgs...) + r := gin.New() + + // OptionalAuth — set user_id if "X-User-ID" header is present (test shortcut) + optAuth := func(c *gin.Context) { + if uid := c.GetHeader("X-User-ID"); uid != "" { + c.Set("user_id", uid) + c.Set("role", "user") + } + c.Next() + } + + // API auth — for ext API sub-paths (not exercised in surface tests) + apiAuth := func(c *gin.Context) { c.Next() } + + // Ext API handler stub + apiHandler := func(c *gin.Context) { c.JSON(200, gin.H{"ok": true}) } + + engine.RegisterExtensionRoutes(r.Group(""), optAuth, apiAuth, apiHandler) + return r +} + +func doGet(r *gin.Engine, path string, headers ...string) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", path, nil) + for i := 0; i+1 < len(headers); i += 2 { + req.Header.Set(headers[i], headers[i+1]) + } + r.ServeHTTP(w, req) + return w +} + +// ── handler integration tests ─────────────────────────────── + +func TestHandler_RootPathRefresh(t *testing.T) { + pkg := &store.PackageRegistration{ + ID: "test-pkg", Title: "Test", Type: "surface", Source: "extension", + Enabled: true, Status: "active", + Manifest: map[string]any{ + "surfaces": []any{ + map[string]any{"path": "/", "access": "authenticated", "title": "Dashboard"}, + }, + }, + } + r := testRouter(t, pkg) + w := doGet(r, "/s/test-pkg", "X-User-ID", "user-1") + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d", w.Code) + } +} + +func TestHandler_SubPathStaticRefresh(t *testing.T) { + pkg := &store.PackageRegistration{ + ID: "test-pkg", Title: "Test", Type: "surface", Source: "extension", + Enabled: true, Status: "active", + Manifest: map[string]any{ + "surfaces": []any{ + map[string]any{"path": "/", "access": "authenticated", "title": "Dashboard"}, + map[string]any{"path": "/monitor", "access": "authenticated", "title": "Monitor"}, + }, + }, + } + r := testRouter(t, pkg) + w := doGet(r, "/s/test-pkg/monitor", "X-User-ID", "user-1") + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d", w.Code) + } +} + +func TestHandler_SubPathParamRefresh(t *testing.T) { + pkg := &store.PackageRegistration{ + ID: "test-pkg", Title: "Test", Type: "surface", Source: "extension", + Enabled: true, Status: "active", + Manifest: map[string]any{ + "surfaces": []any{ + map[string]any{"path": "/", "access": "authenticated", "title": "Dashboard"}, + map[string]any{"path": "/:id", "access": "authenticated", "title": "Detail"}, + }, + }, + } + r := testRouter(t, pkg) + w := doGet(r, "/s/test-pkg/abc123", "X-User-ID", "user-1") + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d", w.Code) + } + body := w.Body.String() + if !strings.Contains(body, `"id":"abc123"`) { + t.Errorf("expected __SURFACE_PARAMS__ to contain id=abc123") + } +} + +func TestHandler_UnauthRedirect(t *testing.T) { + pkg := &store.PackageRegistration{ + ID: "test-pkg", Title: "Test", Type: "surface", Source: "extension", + Enabled: true, Status: "active", + Manifest: map[string]any{ + "surfaces": []any{ + map[string]any{"path": "/dashboard", "access": "authenticated", "title": "Dashboard"}, + }, + }, + } + r := testRouter(t, pkg) + w := doGet(r, "/s/test-pkg/dashboard") // no auth header + if w.Code != http.StatusTemporaryRedirect { + t.Errorf("expected 307, got %d", w.Code) + } + loc := w.Header().Get("Location") + if !strings.Contains(loc, "/login") { + t.Errorf("expected redirect to /login, got %s", loc) + } +} + +func TestHandler_PublicSurfaceAnonymous(t *testing.T) { + pkg := &store.PackageRegistration{ + ID: "test-pkg", Title: "Test", Type: "surface", Source: "extension", + Enabled: true, Status: "active", + Manifest: map[string]any{ + "surfaces": []any{ + map[string]any{"path": "/submit", "access": "public", "title": "Submit"}, + }, + }, + } + r := testRouter(t, pkg) + w := doGet(r, "/s/test-pkg/submit") // no auth + if w.Code != http.StatusOK { + t.Errorf("expected 200 for public surface, got %d", w.Code) + } +} + +func TestHandler_MixedAccessPackage(t *testing.T) { + pkg := &store.PackageRegistration{ + ID: "test-pkg", Title: "Test", Type: "surface", Source: "extension", + Enabled: true, Status: "active", + Manifest: map[string]any{ + "surfaces": []any{ + map[string]any{"path": "/submit", "access": "public", "title": "Submit"}, + map[string]any{"path": "/dashboard", "access": "authenticated", "title": "Dashboard"}, + }, + }, + } + r := testRouter(t, pkg) + + // Public surface — anonymous OK + w1 := doGet(r, "/s/test-pkg/submit") + if w1.Code != http.StatusOK { + t.Errorf("public surface: expected 200, got %d", w1.Code) + } + + // Authenticated surface — anonymous gets redirect + w2 := doGet(r, "/s/test-pkg/dashboard") + if w2.Code != http.StatusTemporaryRedirect { + t.Errorf("auth surface: expected 307, got %d", w2.Code) + } +} + +func TestHandler_NonMatchingSubPath(t *testing.T) { + pkg := &store.PackageRegistration{ + ID: "test-pkg", Title: "Test", Type: "surface", Source: "extension", + Enabled: true, Status: "active", + Manifest: map[string]any{ + "surfaces": []any{ + map[string]any{"path": "/", "access": "authenticated", "title": "Dashboard"}, + }, + }, + } + r := testRouter(t, pkg) + w := doGet(r, "/s/test-pkg/nonexistent", "X-User-ID", "user-1") + if w.Code != http.StatusNotFound { + t.Errorf("expected 404, got %d", w.Code) + } +} + +func TestHandler_EarlyAuthShortCircuit(t *testing.T) { + // All surfaces require auth → early short-circuit before surface matching + pkg := &store.PackageRegistration{ + ID: "test-pkg", Title: "Test", Type: "surface", Source: "extension", + Enabled: true, Status: "active", + Manifest: map[string]any{ + "surfaces": []any{ + map[string]any{"path": "/", "access": "authenticated", "title": "Dashboard"}, + map[string]any{"path": "/settings", "access": "admin", "title": "Settings"}, + }, + }, + } + r := testRouter(t, pkg) + + // Any path — should redirect before even matching surfaces + w := doGet(r, "/s/test-pkg/anything") // no auth + if w.Code != http.StatusTemporaryRedirect { + t.Errorf("expected 307 (early short-circuit), got %d", w.Code) + } + loc := w.Header().Get("Location") + if !strings.Contains(loc, "/login") { + t.Errorf("expected redirect to /login, got %s", loc) + } +} diff --git a/server/pages/pages_surface_match_test.go b/server/pages/pages_surface_match_test.go index 08e0132..9edd50e 100644 --- a/server/pages/pages_surface_match_test.go +++ b/server/pages/pages_surface_match_test.go @@ -168,6 +168,57 @@ func TestFindNavSurface_NoMatch(t *testing.T) { } } +// ── aggregateAccess tests ────────────────────────────────── + +func TestAggregateAccess_AllPublic(t *testing.T) { + surfaces := []any{ + map[string]any{"path": "/", "access": "public"}, + map[string]any{"path": "/submit", "access": "public"}, + } + if got := aggregateAccess(surfaces); got != "public" { + t.Errorf("expected public, got %s", got) + } +} + +func TestAggregateAccess_AllAuthenticated(t *testing.T) { + surfaces := []any{ + map[string]any{"path": "/", "access": "authenticated"}, + map[string]any{"path": "/admin", "access": "admin"}, + } + if got := aggregateAccess(surfaces); got != "authenticated" { + t.Errorf("expected authenticated, got %s", got) + } +} + +func TestAggregateAccess_Mixed(t *testing.T) { + surfaces := []any{ + map[string]any{"path": "/submit", "access": "public"}, + map[string]any{"path": "/dashboard", "access": "authenticated"}, + } + if got := aggregateAccess(surfaces); got != "mixed" { + t.Errorf("expected mixed, got %s", got) + } +} + +func TestAggregateAccess_DefaultAccess(t *testing.T) { + // No access field → defaults to "authenticated" + surfaces := []any{ + map[string]any{"path": "/"}, + } + if got := aggregateAccess(surfaces); got != "authenticated" { + t.Errorf("expected authenticated (default), got %s", got) + } +} + +func TestAggregateAccess_SinglePublic(t *testing.T) { + surfaces := []any{ + map[string]any{"path": "/", "access": "public"}, + } + if got := aggregateAccess(surfaces); got != "public" { + t.Errorf("expected public, got %s", got) + } +} + // ── evaluateAccess tests ──────────────────────────────────── // Note: evaluateAccess depends on gin.Context which is hard to unit test // without a full Gin setup. These are covered via handler integration tests. diff --git a/src/js/sw/sdk/index.js b/src/js/sw/sdk/index.js index b44f498..aabbd1c 100644 --- a/src/js/sw/sdk/index.js +++ b/src/js/sw/sdk/index.js @@ -224,8 +224,18 @@ export async function boot() { } }); + // Seed initial history state so back-navigation from sw.navigate() works. + // Without this, the initial server-rendered page has no pushState entry, + // so pressing back after sw.navigate() fires popstate with null state. + if (window.__SURFACE_PATH__) { + history.replaceState( + { surfacePath: window.__SURFACE_PATH__, surfaceParams: window.__SURFACE_PARAMS__ || {} }, + '' + ); + } + // Marker for idempotency - sw._sdk = '0.9.0'; + sw._sdk = '0.9.1'; // 8. Expose globally window.sw = sw;