fix(auth): #168 — CanvasOrBearer middleware for PUT /canvas/viewport only
Closes #168 by the route-split path from #194's review. #167 put PUT /canvas/viewport behind strict AdminAuth, breaking canvas drag/zoom persist because the canvas uses session cookies not bearer tokens. New narrow middleware CanvasOrBearer: - Accepts a valid bearer (same contract as AdminAuth) OR - Accepts a request whose Origin exactly matches CORS_ORIGINS - Lazy-bootstrap fail-open preserved for fresh installs Applied ONLY to PUT /canvas/viewport. The softer check is acceptable there because viewport corruption is cosmetic-only — worst case a user refreshes the page. This middleware must NOT be used on routes that leak prompts (#165), create resources (#164), or write files (#190) — see #194 review for why. The other canvas-facing routes mentioned in #168 (Events tab, Bundle Export/Import) remain behind strict AdminAuth pending a proper session-cookie-accepting AdminAuth (#168 follow-up for Phase H). 6 new tests cover: bootstrap fail-open, no-creds 401, canvas origin match, wrong origin 401, empty origin rejected, localhost default. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
87d330cf15
commit
7a16eb4f70
@ -4,6 +4,8 @@ import (
|
||||
"database/sql"
|
||||
"log"
|
||||
"net/http"
|
||||
"os"
|
||||
"strings"
|
||||
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/wsauth"
|
||||
"github.com/gin-gonic/gin"
|
||||
@ -84,3 +86,82 @@ func AdminAuth(database *sql.DB) gin.HandlerFunc {
|
||||
c.Next()
|
||||
}
|
||||
}
|
||||
|
||||
// CanvasOrBearer is a softer admin-auth variant used ONLY for cosmetic
|
||||
// canvas routes where forging the request has zero security impact (PUT
|
||||
// /canvas/viewport: worst case an attacker resets the shared viewport
|
||||
// position, user refreshes the page, problem solved).
|
||||
//
|
||||
// Accepts either:
|
||||
//
|
||||
// 1. A valid bearer token (same contract as AdminAuth) — covers molecli,
|
||||
// agent-to-platform calls, and anyone using the API directly.
|
||||
// 2. A browser Origin header that matches CORS_ORIGINS (canvas itself).
|
||||
// This is NOT a strict auth boundary — curl can forge Origin — but for
|
||||
// cosmetic-only routes the trade-off is acceptable. Non-cosmetic routes
|
||||
// MUST NOT use this middleware (see #194 review on why it would re-open
|
||||
// #164 CRITICAL if applied to /bundles/import).
|
||||
//
|
||||
// Lazy-bootstrap fail-open preserved: zero-token installs pass everything
|
||||
// through so fresh self-hosted / dev sessions aren't bricked.
|
||||
func CanvasOrBearer(database *sql.DB) gin.HandlerFunc {
|
||||
return func(c *gin.Context) {
|
||||
ctx := c.Request.Context()
|
||||
|
||||
hasLive, err := wsauth.HasAnyLiveTokenGlobal(ctx, database)
|
||||
if err != nil {
|
||||
log.Printf("wsauth: CanvasOrBearer HasAnyLiveTokenGlobal failed: %v — allowing request", err)
|
||||
c.Next()
|
||||
return
|
||||
}
|
||||
if !hasLive {
|
||||
c.Next()
|
||||
return
|
||||
}
|
||||
|
||||
// Path 1: valid bearer.
|
||||
if tok := wsauth.BearerTokenFromHeader(c.GetHeader("Authorization")); tok != "" {
|
||||
if err := wsauth.ValidateAnyToken(ctx, database, tok); err == nil {
|
||||
c.Next()
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
// Path 2: canvas origin match. Read CORS_ORIGINS at request time so
|
||||
// tests can override via t.Setenv. canvasOriginAllowed returns true
|
||||
// iff Origin is non-empty AND exactly matches one of the configured
|
||||
// origins. Empty Origin (same-origin / server-to-server) does NOT
|
||||
// pass this check — those callers must use the bearer path.
|
||||
if canvasOriginAllowed(c.GetHeader("Origin")) {
|
||||
c.Next()
|
||||
return
|
||||
}
|
||||
|
||||
c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "admin auth required"})
|
||||
}
|
||||
}
|
||||
|
||||
// canvasOriginAllowed returns true if origin matches any entry in the
|
||||
// CORS_ORIGINS env var (comma-separated) or the localhost defaults.
|
||||
// Exact-match only; no prefix or wildcard logic — that's handled by the
|
||||
// real CORS middleware upstream. The intent here is "did this request come
|
||||
// from the canvas page the user is already logged into?" — a binary check.
|
||||
func canvasOriginAllowed(origin string) bool {
|
||||
if origin == "" {
|
||||
return false
|
||||
}
|
||||
allowed := []string{"http://localhost:3000", "http://localhost:3001"}
|
||||
if v := os.Getenv("CORS_ORIGINS"); v != "" {
|
||||
for _, o := range strings.Split(v, ",") {
|
||||
if o = strings.TrimSpace(o); o != "" {
|
||||
allowed = append(allowed, o)
|
||||
}
|
||||
}
|
||||
}
|
||||
for _, a := range allowed {
|
||||
if a == origin {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
@ -678,3 +678,126 @@ func TestAdminAuth_Issue120_PatchWorkspace_NoBearer_Returns401(t *testing.T) {
|
||||
t.Errorf("unmet sqlmock expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// ── CanvasOrBearer (#168) ────────────────────────────────────────────────────
|
||||
// Narrow softer variant of AdminAuth used ONLY on PUT /canvas/viewport.
|
||||
// Accepts bearer or a matching Origin header. MUST NOT be used anywhere a
|
||||
// forged request would leak data or create resources.
|
||||
|
||||
func TestCanvasOrBearer_NoTokens_FailOpen(t *testing.T) {
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("sqlmock: %v", err)
|
||||
}
|
||||
defer mockDB.Close()
|
||||
|
||||
mock.ExpectQuery(hasAnyLiveTokenGlobalQuery).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0))
|
||||
|
||||
r := gin.New()
|
||||
r.PUT("/canvas/viewport", CanvasOrBearer(mockDB), func(c *gin.Context) {
|
||||
c.JSON(http.StatusOK, gin.H{"ok": true})
|
||||
})
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req, _ := http.NewRequest(http.MethodPut, "/canvas/viewport", nil)
|
||||
r.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("bootstrap fail-open: got %d, want 200 (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestCanvasOrBearer_TokensExist_NoCreds_Returns401(t *testing.T) {
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("sqlmock: %v", err)
|
||||
}
|
||||
defer mockDB.Close()
|
||||
|
||||
mock.ExpectQuery(hasAnyLiveTokenGlobalQuery).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1))
|
||||
|
||||
r := gin.New()
|
||||
r.PUT("/canvas/viewport", CanvasOrBearer(mockDB), func(c *gin.Context) {
|
||||
c.JSON(http.StatusOK, gin.H{"ok": true})
|
||||
})
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req, _ := http.NewRequest(http.MethodPut, "/canvas/viewport", nil)
|
||||
r.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusUnauthorized {
|
||||
t.Errorf("no creds: got %d, want 401", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCanvasOrBearer_TokensExist_CanvasOrigin_Passes(t *testing.T) {
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("sqlmock: %v", err)
|
||||
}
|
||||
defer mockDB.Close()
|
||||
|
||||
mock.ExpectQuery(hasAnyLiveTokenGlobalQuery).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1))
|
||||
|
||||
t.Setenv("CORS_ORIGINS", "https://acme.moleculesai.app,https://bob.moleculesai.app")
|
||||
|
||||
r := gin.New()
|
||||
r.PUT("/canvas/viewport", CanvasOrBearer(mockDB), func(c *gin.Context) {
|
||||
c.JSON(http.StatusOK, gin.H{"ok": true})
|
||||
})
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req, _ := http.NewRequest(http.MethodPut, "/canvas/viewport", nil)
|
||||
req.Header.Set("Origin", "https://acme.moleculesai.app")
|
||||
r.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("canvas origin: got %d, want 200 (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestCanvasOrBearer_TokensExist_WrongOrigin_Returns401(t *testing.T) {
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("sqlmock: %v", err)
|
||||
}
|
||||
defer mockDB.Close()
|
||||
|
||||
mock.ExpectQuery(hasAnyLiveTokenGlobalQuery).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1))
|
||||
|
||||
t.Setenv("CORS_ORIGINS", "https://acme.moleculesai.app")
|
||||
|
||||
r := gin.New()
|
||||
r.PUT("/canvas/viewport", CanvasOrBearer(mockDB), func(c *gin.Context) {
|
||||
c.JSON(http.StatusOK, gin.H{"ok": true})
|
||||
})
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req, _ := http.NewRequest(http.MethodPut, "/canvas/viewport", nil)
|
||||
req.Header.Set("Origin", "https://evil.example.com")
|
||||
r.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusUnauthorized {
|
||||
t.Errorf("wrong origin: got %d, want 401", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCanvasOriginAllowed_EmptyOriginRejected(t *testing.T) {
|
||||
if canvasOriginAllowed("") {
|
||||
t.Error("empty Origin must not pass")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCanvasOriginAllowed_LocalhostDefault(t *testing.T) {
|
||||
t.Setenv("CORS_ORIGINS", "")
|
||||
if !canvasOriginAllowed("http://localhost:3000") {
|
||||
t.Error("localhost:3000 should be allowed by default")
|
||||
}
|
||||
if canvasOriginAllowed("http://evil.example.com") {
|
||||
t.Error("random origin should not be allowed")
|
||||
}
|
||||
}
|
||||
|
||||
@ -294,16 +294,17 @@ func Setup(hub *ws.Hub, broadcaster *events.Broadcaster, prov *provisioner.Provi
|
||||
th := handlers.NewTerminalHandler(dockerCli)
|
||||
wsAuth.GET("/terminal", th.HandleConnect)
|
||||
|
||||
// Canvas Viewport — #166: PUT gated behind AdminAuth so an anon caller
|
||||
// can't reset the shared viewport state for all users. GET remains open
|
||||
// because the canvas bootstraps without a bearer and needs the initial
|
||||
// viewport for first paint.
|
||||
// Canvas Viewport — #166 + #168: GET stays fully open for bootstrap.
|
||||
// PUT uses CanvasOrBearer (accepts Origin-match OR bearer token) so the
|
||||
// browser canvas can persist drag/zoom state without a bearer, while
|
||||
// bearer-carrying clients (molecli, integration tests) still work.
|
||||
// Viewport corruption is cosmetic-only — worst case a user refreshes
|
||||
// the page — so the softer check is acceptable here. This middleware
|
||||
// MUST NOT be used on routes that leak prompts, create workspaces,
|
||||
// or write files (#164/#165/#190 class).
|
||||
vh := handlers.NewViewportHandler()
|
||||
r.GET("/canvas/viewport", vh.Get)
|
||||
{
|
||||
viewportAdmin := r.Group("", middleware.AdminAuth(db.DB))
|
||||
viewportAdmin.PUT("/canvas/viewport", vh.Save)
|
||||
}
|
||||
r.PUT("/canvas/viewport", middleware.CanvasOrBearer(db.DB), vh.Save)
|
||||
|
||||
// Templates
|
||||
tmplh := handlers.NewTemplatesHandler(configsDir, dockerCli)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user