Feat v0.7.6 code hygiene (#60)

Co-authored-by: Jeffrey Smith <jasafpro@gmail.com>
Co-committed-by: Jeffrey Smith <jasafpro@gmail.com>

server/middleware/permissions_test.go

@@ -0,0 +1,255 @@
+package middleware
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/gin-gonic/gin"
+
+	"armature/auth"
+	"armature/store"
+)
+
+// ---------------------------------------------------------------------------
+// Mocks
+// ---------------------------------------------------------------------------
+
+// mockRateLimitStore implements store.RateLimitStore for testing.
+type mockRateLimitStore struct {
+	allowed bool
+	err     error
+}
+
+func (m *mockRateLimitStore) Allow(_ context.Context, _ string, _ float64, _ int) (bool, error) {
+	return m.allowed, m.err
+}
+
+func (m *mockRateLimitStore) Cleanup(_ context.Context, _ time.Duration) error { return nil }
+
+// ---------------------------------------------------------------------------
+// Helpers
+// ---------------------------------------------------------------------------
+
+func init() { gin.SetMode(gin.TestMode) }
+
+// newEngine builds a test engine that injects user_id and optional pre-cached
+// permissions before the middleware under test runs. This avoids needing to
+// mock auth.ResolvePermissions (a package-level function).
+func newEngine(userID string, perms map[string]bool, mw gin.HandlerFunc) *gin.Engine {
+	e := gin.New()
+	e.Use(func(c *gin.Context) {
+		if userID != "" {
+			c.Set("user_id", userID)
+		}
+		if perms != nil {
+			c.Set(permCacheKey, perms)
+		}
+		c.Next()
+	})
+	e.Use(mw)
+	e.GET("/test", func(c *gin.Context) {
+		c.JSON(http.StatusOK, gin.H{"ok": true})
+	})
+	return e
+}
+
+func bodyMap(w *httptest.ResponseRecorder) map[string]interface{} {
+	var m map[string]interface{}
+	json.Unmarshal(w.Body.Bytes(), &m)
+	return m
+}
+
+// ---------------------------------------------------------------------------
+// RequirePermission tests
+// ---------------------------------------------------------------------------
+
+func TestRequirePermission_Allowed(t *testing.T) {
+	s := store.Stores{} // not used — perms are pre-cached
+	perms := map[string]bool{"notes.read": true}
+	engine := newEngine("user-1", perms, RequirePermission("notes.read", s))
+
+	w := httptest.NewRecorder()
+	req := httptest.NewRequest("GET", "/test", nil)
+	engine.ServeHTTP(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("expected 200, got %d", w.Code)
+	}
+	body := bodyMap(w)
+	if body["ok"] != true {
+		t.Errorf("expected ok=true, got %v", body)
+	}
+}
+
+func TestRequirePermission_Denied(t *testing.T) {
+	s := store.Stores{}
+	perms := map[string]bool{"notes.read": true} // no "notes.write"
+	engine := newEngine("user-1", perms, RequirePermission("notes.write", s))
+
+	w := httptest.NewRecorder()
+	req := httptest.NewRequest("GET", "/test", nil)
+	engine.ServeHTTP(w, req)
+
+	if w.Code != http.StatusForbidden {
+		t.Errorf("expected 403, got %d", w.Code)
+	}
+	body := bodyMap(w)
+	errMsg, _ := body["error"].(string)
+	if errMsg != "permission required: notes.write" {
+		t.Errorf("unexpected error message: %q", errMsg)
+	}
+}
+
+func TestRequirePermission_CachingAcrossChain(t *testing.T) {
+	// Two RequirePermission middlewares in the same chain should share the
+	// cached permission map — the second must NOT trigger a fresh resolve.
+	s := store.Stores{}
+	perms := map[string]bool{"a.read": true, "b.read": true}
+
+	e := gin.New()
+	e.Use(func(c *gin.Context) {
+		c.Set("user_id", "user-1")
+		c.Set(permCacheKey, perms)
+		c.Next()
+	})
+	e.Use(RequirePermission("a.read", s))
+	e.Use(RequirePermission("b.read", s))
+	e.GET("/test", func(c *gin.Context) {
+		// Verify the cached permissions are still present and correct.
+		resolved := GetResolvedPermissions(c)
+		if resolved == nil {
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "cache missing"})
+			return
+		}
+		if !resolved["a.read"] || !resolved["b.read"] {
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "cache incomplete"})
+			return
+		}
+		c.JSON(http.StatusOK, gin.H{"ok": true})
+	})
+
+	w := httptest.NewRecorder()
+	req := httptest.NewRequest("GET", "/test", nil)
+	e.ServeHTTP(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("expected 200, got %d (body: %s)", w.Code, w.Body.String())
+	}
+}
+
+// ---------------------------------------------------------------------------
+// GetResolvedPermissions tests
+// ---------------------------------------------------------------------------
+
+func TestGetResolvedPermissions_NilWhenNotSet(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	if got := GetResolvedPermissions(c); got != nil {
+		t.Errorf("expected nil, got %v", got)
+	}
+}
+
+func TestGetResolvedPermissions_ReturnsCached(t *testing.T) {
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	want := map[string]bool{"x": true}
+	c.Set(permCacheKey, want)
+
+	got := GetResolvedPermissions(c)
+	if got == nil || !got["x"] {
+		t.Errorf("expected cached perms, got %v", got)
+	}
+}
+
+// ---------------------------------------------------------------------------
+// RequireAdmin tests
+// ---------------------------------------------------------------------------
+
+func TestRequireAdmin_Allowed(t *testing.T) {
+	s := store.Stores{}
+	perms := map[string]bool{auth.PermSurfaceAdminAccess: true}
+	engine := newEngine("admin-1", perms, RequireAdmin(s))
+
+	w := httptest.NewRecorder()
+	req := httptest.NewRequest("GET", "/test", nil)
+	engine.ServeHTTP(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("expected 200, got %d", w.Code)
+	}
+}
+
+func TestRequireAdmin_Denied(t *testing.T) {
+	s := store.Stores{}
+	perms := map[string]bool{"notes.read": true} // no admin perm
+	engine := newEngine("user-1", perms, RequireAdmin(s))
+
+	w := httptest.NewRecorder()
+	req := httptest.NewRequest("GET", "/test", nil)
+	engine.ServeHTTP(w, req)
+
+	if w.Code != http.StatusForbidden {
+		t.Errorf("expected 403, got %d", w.Code)
+	}
+	body := bodyMap(w)
+	errMsg, _ := body["error"].(string)
+	if errMsg != "admin access required" {
+		t.Errorf("unexpected error message: %q", errMsg)
+	}
+}
+
+// ---------------------------------------------------------------------------
+// RateLimiter.Limit tests
+// ---------------------------------------------------------------------------
+
+func TestRateLimiter_Allowed(t *testing.T) {
+	rl := NewRateLimiter(&mockRateLimitStore{allowed: true}, 10, 20)
+	engine := newEngine("", nil, rl.Limit())
+
+	w := httptest.NewRecorder()
+	req := httptest.NewRequest("GET", "/test", nil)
+	engine.ServeHTTP(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("expected 200, got %d", w.Code)
+	}
+}
+
+func TestRateLimiter_Exceeded(t *testing.T) {
+	rl := NewRateLimiter(&mockRateLimitStore{allowed: false}, 10, 20)
+	engine := newEngine("", nil, rl.Limit())
+
+	w := httptest.NewRecorder()
+	req := httptest.NewRequest("GET", "/test", nil)
+	engine.ServeHTTP(w, req)
+
+	if w.Code != http.StatusTooManyRequests {
+		t.Errorf("expected 429, got %d", w.Code)
+	}
+	if ra := w.Header().Get("Retry-After"); ra != "1" {
+		t.Errorf("expected Retry-After=1, got %q", ra)
+	}
+	body := bodyMap(w)
+	errMsg, _ := body["error"].(string)
+	if errMsg != "rate limit exceeded" {
+		t.Errorf("unexpected error message: %q", errMsg)
+	}
+}
+
+func TestRateLimiter_StoreError_FailOpen(t *testing.T) {
+	rl := NewRateLimiter(&mockRateLimitStore{err: errors.New("db down")}, 10, 20)
+	engine := newEngine("", nil, rl.Limit())
+
+	w := httptest.NewRecorder()
+	req := httptest.NewRequest("GET", "/test", nil)
+	engine.ServeHTTP(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("expected 200 (fail-open), got %d", w.Code)
+	}
+}

server/middleware/prometheus.go

@@ -11,7 +11,7 @@ import (
 
 // Prometheus returns a Gin middleware that records HTTP request metrics.
 // Uses c.FullPath() (Gin's registered route pattern, e.g.
-// "/api/v1/channels/:id") to avoid label cardinality explosion from
+// "/api/v1/workflows/:id") to avoid label cardinality explosion from
 // path parameters.
 func Prometheus() gin.HandlerFunc {
 	return func(c *gin.Context) {