From 385fa22056c51a42a0eb99fd08a8876009c2c52d Mon Sep 17 00:00:00 2001 From: boojack Date: Sun, 14 Jun 2026 23:20:34 +0800 Subject: [PATCH] fix(cors): open API to any origin for token auth, keep cookies same-origin Reflect any Origin so token-authenticated clients (Access Token V2 / PAT) can call the API cross-origin, but emit Access-Control-Allow-Credentials only for trusted origins (same host / configured InstanceURL). This keeps the SameSite=Lax refresh cookie unreadable by untrusted (incl. same-site subdomain) origins. Origin: null is not reflected. Note for operators: cross-origin token access is now open by default; if you front memos with a caching proxy, ensure it honors `Vary: Origin`. --- server/cors.go | 24 ++++++++++++++++++++---- server/cors_test.go | 39 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/server/cors.go b/server/cors.go index 3eb3db7b..1418d238 100644 --- a/server/cors.go +++ b/server/cors.go @@ -12,13 +12,29 @@ import ( func newCORSMiddleware(profile *profile.Profile) echo.MiddlewareFunc { return middleware.CORSWithConfig(middleware.CORSConfig{ + // The API is open to any origin so that token-authenticated clients + // (Access Token V2 / PAT in the Authorization header) can call it from + // anywhere. Credentials — i.e. the SameSite=Lax refresh-token cookie — are + // granted only to trusted origins (same host or the configured InstanceURL). + // + // AllowCredentials stays false here on purpose: the per-origin + // Access-Control-Allow-Credentials header is set inside the func below. + // Do NOT switch this to AllowCredentials:true — emitting that header for + // every reflected origin would let a malicious same-site subdomain read the + // cookie-authenticated /auth/refresh response and steal an access token. + AllowCredentials: false, UnsafeAllowOriginFunc: func(c *echo.Context, origin string) (string, bool, error) { - if isAllowedCORSOrigin(profile, c.Request().Host, origin) { - return origin, true, nil + // Never reflect the opaque "null" origin (sandboxed iframes, file://). + if origin == "null" { + return "", false, nil } - return "", false, nil + // Trusted origins additionally get credentialed (cookie) access. + if isAllowedCORSOrigin(profile, c.Request().Host, origin) { + c.Response().Header().Set(echo.HeaderAccessControlAllowCredentials, "true") + } + // Reflect every origin; only trusted ones carry the credentials header. + return origin, true, nil }, - AllowCredentials: true, }) } diff --git a/server/cors_test.go b/server/cors_test.go index fe9fd9ae..7a6c2023 100644 --- a/server/cors_test.go +++ b/server/cors_test.go @@ -3,6 +3,7 @@ package server import ( "net/http" "net/http/httptest" + "strings" "testing" "github.com/labstack/echo/v5" @@ -85,7 +86,7 @@ func TestCORSMiddleware(t *testing.T) { return c.NoContent(http.StatusOK) }) - t.Run("allows instance URL origin on preflight", func(t *testing.T) { + t.Run("trusted origin gets credentialed access", func(t *testing.T) { req := httptest.NewRequest(http.MethodOptions, "/api/v1/test", nil) req.Header.Set("Origin", "https://memos.example") req.Header.Set("Access-Control-Request-Method", http.MethodPost) @@ -104,7 +105,7 @@ func TestCORSMiddleware(t *testing.T) { } }) - t.Run("omits CORS headers for unknown origin preflight", func(t *testing.T) { + t.Run("arbitrary origin is reflected without credentials", func(t *testing.T) { req := httptest.NewRequest(http.MethodOptions, "/api/v1/test", nil) req.Header.Set("Origin", "https://evil.example") req.Header.Set("Access-Control-Request-Method", http.MethodPost) @@ -115,8 +116,40 @@ func TestCORSMiddleware(t *testing.T) { if rec.Code != http.StatusNoContent { t.Fatalf("expected status %d, got %d", http.StatusNoContent, rec.Code) } + // The API is open to any origin (token auth), so the origin is reflected... + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "https://evil.example" { + t.Fatalf("expected origin to be reflected, got %q", got) + } + // ...but an untrusted origin must NOT be granted credentialed (cookie) access. + if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "" { + t.Fatalf("expected no Access-Control-Allow-Credentials for untrusted origin, got %q", got) + } + }) + + t.Run("arbitrary origin may send Authorization header", func(t *testing.T) { + req := httptest.NewRequest(http.MethodOptions, "/api/v1/test", nil) + req.Header.Set("Origin", "https://app.third-party.example") + req.Header.Set("Access-Control-Request-Method", http.MethodPost) + req.Header.Set("Access-Control-Request-Headers", "Authorization") + rec := httptest.NewRecorder() + + e.ServeHTTP(rec, req) + + if got := rec.Header().Get("Access-Control-Allow-Headers"); !strings.Contains(strings.ToLower(got), "authorization") { + t.Fatalf("expected Authorization to be allowed for a cross-origin token client, got %q", got) + } + }) + + t.Run("null origin is not reflected", func(t *testing.T) { + req := httptest.NewRequest(http.MethodOptions, "/api/v1/test", nil) + req.Header.Set("Origin", "null") + req.Header.Set("Access-Control-Request-Method", http.MethodPost) + rec := httptest.NewRecorder() + + e.ServeHTTP(rec, req) + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { - t.Fatalf("expected no Access-Control-Allow-Origin, got %q", got) + t.Fatalf("expected null origin not to be reflected, got %q", got) } }) }