cors fixed

main.go

@@ -23,6 +23,20 @@ import (
 	"gorm.io/gorm"
 )
 
+// corsSettings is a function so it can be tested.
+//
+// Inline, it could only be checked by starting the server and pointing a real
+// browser at it — which is how the missing Authorization header reached
+// production in the first place.
+func corsSettings() cors.Config {
+	return cors.Config{
+		AllowHeaders:     "Origin,Content-Type,Accept,Content-Length,Accept-Language,Accept-Encoding,Connection,Authorization",
+		AllowOrigins:     "*",
+		AllowCredentials: false,
+		AllowMethods:     "GET,POST,HEAD,PUT,DELETE,PATCH,OPTIONS",
+	}
+}
+
 func main() {
 	// Loads `.env.<APP_ENV>` (default `.env.local`) and `.env`, then checks
 	// every required setting at once. Nothing below runs against a half
@@ -31,12 +45,43 @@ func main() {
 
 	app := fiber.New()
 
-	app.Use(cors.New(cors.Config{
+	// Cross-origin access.
-		AllowHeaders:     "Origin,Content-Type,Accept,Content-Length,Accept-Language,Accept-Encoding,Connection,Access-Control-Allow-Origin",
+	//
-		AllowOrigins:     "*",
+	// The console is served from app.nearledaily.com and calls this host
-		AllowCredentials: true,
+	// directly, so every request it makes is cross-origin and the browser
-		AllowMethods:     "GET,POST,HEAD,PUT,DELETE,PATCH,OPTIONS",
+	// decides whether to allow it from the headers below.
-	}))
+	//
+	// ── Authorization has to be listed ──────────────────────────────────────
+	//
+	// It was not, and adding the session token to the console broke every call
+	// the moment it shipped. A request carrying `Authorization` is no longer a
+	// "simple" request, so the browser stops and asks permission first — and the
+	// answer has to name that header explicitly. It was never needed before
+	// because the console sent nothing but `Accept` and `Content-Type`.
+	//
+	// The failure is worth recognising again: the preflight returns 204 and
+	// looks healthy in a terminal, the server logs nothing, and only the browser
+	// refuses. `curl` cannot reproduce it, because curl does not enforce CORS.
+	//
+	// ── Credentials off, wildcard on ────────────────────────────────────────
+	//
+	// `AllowOrigins: "*"` with `AllowCredentials: true` is not a valid pair: a
+	// browser rejects a credentialed response that carries a wildcard origin.
+	// That combination was here already and was harmless only because nothing
+	// used credentials — it would have become a second, identical-looking bug
+	// the day anything did.
+	//
+	// Credentials means cookies and TLS client certs, and this backend uses
+	// neither: authentication is a Bearer token, which is an ordinary header and
+	// needs no credentialed mode. Nothing in the console, the app or the POS
+	// sets `credentials: 'include'`, so turning it off costs nothing and makes
+	// the pair legal.
+	//
+	// The wildcard itself is worth revisiting — it lets any site on the internet
+	// call this API from a browser, and the tenant guard is what stops that
+	// mattering. Narrowing it to the known console origins is a separate change,
+	// and one that breaks local development if the list is got wrong.
+	app.Use(cors.New(corsSettings()))
 
 	fmt.Println("🌐 Connecting to databases...")
 	db.Connect(cfg)

main_test.go

@@ -0,0 +1,113 @@
+package main
+
+import (
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/gofiber/fiber/v2"
+	"github.com/gofiber/fiber/v2/middleware/cors"
+)
+
+// Cross-origin access, checked the way a browser checks it.
+//
+// These exist because this went wrong in production and nothing caught it.
+// Adding the session token to the console made every request non-simple, so
+// browsers began asking permission first — and the answer did not name the
+// `Authorization` header, so every call was blocked.
+//
+// The reason it reached production is worth keeping in mind while reading
+// these: the preflight returns 204 and looks perfectly healthy from a terminal,
+// the server logs nothing unusual, and `curl` cannot reproduce it because curl
+// does not enforce CORS. The only thing that noticed was a browser.
+
+// preflight asks the question a browser asks before a cross-origin request.
+func preflight(t *testing.T, requestHeaders string) map[string]string {
+	t.Helper()
+
+	app := fiber.New()
+	app.Use(cors.New(corsSettings()))
+	app.Get("/probe", func(c *fiber.Ctx) error { return c.SendStatus(fiber.StatusOK) })
+
+	req := httptest.NewRequest("OPTIONS", "/probe", nil)
+	req.Header.Set("Origin", "https://app.nearledaily.com")
+	req.Header.Set("Access-Control-Request-Method", "GET")
+	if requestHeaders != "" {
+		req.Header.Set("Access-Control-Request-Headers", requestHeaders)
+	}
+
+	resp, err := app.Test(req, -1)
+	if err != nil {
+		t.Fatalf("preflight: %v", err)
+	}
+
+	out := map[string]string{}
+	for _, name := range []string{
+		"Access-Control-Allow-Origin",
+		"Access-Control-Allow-Headers",
+		"Access-Control-Allow-Methods",
+		"Access-Control-Allow-Credentials",
+	} {
+		out[name] = resp.Header.Get(name)
+	}
+	return out
+}
+
+func TestTheBrowserIsAllowedToSendTheSessionToken(t *testing.T) {
+	// The bug itself. Without `Authorization` in this list the console cannot
+	// make a single authenticated call, and the error surfaces only in a
+	// browser console as a CORS failure.
+	headers := preflight(t, "authorization")["Access-Control-Allow-Headers"]
+
+	if !strings.Contains(strings.ToLower(headers), "authorization") {
+		t.Fatalf("the console may not send its session token: %q", headers)
+	}
+}
+
+func TestTheHeadersTheConsoleAlreadySentStillWork(t *testing.T) {
+	// Adding one header must not quietly drop the others.
+	headers := strings.ToLower(preflight(t, "content-type")["Access-Control-Allow-Headers"])
+
+	for _, needed := range []string{"content-type", "accept", "origin"} {
+		if !strings.Contains(headers, needed) {
+			t.Fatalf("%q is no longer allowed: %q", needed, headers)
+		}
+	}
+}
+
+func TestAWildcardOriginIsNotPairedWithCredentials(t *testing.T) {
+	// Not a valid combination: a browser rejects a credentialed response
+	// carrying a wildcard origin. It was here already and was harmless only
+	// because nothing used credentials — it would have become a second bug
+	// that looked exactly like the first, the day anything did.
+	got := preflight(t, "authorization")
+
+	if got["Access-Control-Allow-Origin"] == "*" &&
+		strings.EqualFold(got["Access-Control-Allow-Credentials"], "true") {
+		t.Fatal("wildcard origin with credentials allowed — browsers reject this pair")
+	}
+}
+
+func TestEveryMethodTheConsoleUsesIsAllowed(t *testing.T) {
+	// The console writes with POST, PUT and DELETE. A missing one fails only
+	// on the screens that use it, which is the kind of gap that ships.
+	methods := strings.ToUpper(preflight(t, "authorization")["Access-Control-Allow-Methods"])
+
+	for _, method := range []string{"GET", "POST", "PUT", "DELETE", "OPTIONS"} {
+		if !strings.Contains(methods, method) {
+			t.Fatalf("%s is not allowed cross-origin: %q", method, methods)
+		}
+	}
+}
+
+func TestAResponseHeaderIsNotListedAsAnAllowedRequestHeader(t *testing.T) {
+	// `Access-Control-Allow-Origin` was in the allowed REQUEST headers, which is
+	// a category error: it is something the server sends back, never something a
+	// browser asks to send. Harmless, but it reads as though somebody added
+	// names until the error went away.
+	headers := strings.ToLower(preflight(t, "authorization")["Access-Control-Allow-Headers"])
+
+	if strings.Contains(headers, "access-control-allow-origin") {
+		t.Fatalf("a response header is listed as an allowed request header: %q", headers)
+	}
+}

middleware/webauth.go

@@ -65,21 +65,29 @@ const WebLocalsKey = "webclaims"
 
 // webAuthRequired reports whether a request without a valid token is refused.
 //
-// Defaults to OFF, for the same reason POS enforcement does: the console is in
+// Defaults to ON. It did not always: this shipped defaulting to off, because
-// use by real merchants right now, and its sign-in does not yet hand back a
+// the console was live and its sign-in did not yet hand back a token, so
-// token. Switching enforcement on before the console sends one would lock every
+// enforcing first would have locked every merchant out of a working product.
-// user out of a working product.
 //
-// So the order is: this middleware ships, sign-in starts issuing tokens, the
+// That rollout is finished. Sign-in mints a token, the console sends it on
-// console starts sending them, and `WEB_AUTH_REQUIRED=true` closes the door.
+// every call, and it expires cleanly. Leaving the default off after that point
-// While it is off a token is still VERIFIED when one is sent, and a request
+// was not caution, it was an open door nobody had got round to shutting — and
-// carrying a token for the wrong tenant is still refused — the flag only
+// it was measured wide open: a `getorders` with no credential at all returned a
-// decides what happens to a request carrying none.
+// real merchant's orders to anyone on the internet.
 //
-// This is a temporary state and should be short. An unauthenticated `/web`
+// ── The way out, if this goes wrong ─────────────────────────────────────────
-// surface is the most serious thing in this codebase.
+//
+// `WEB_AUTH_REQUIRED=false` restores the old behaviour, immediately and without
+// a deploy. That is the escape hatch, and it exists because flipping a default
+// that can lock people out should always be reversible by one person in one
+// minute. A token that is SENT is still always verified either way — the flag
+// only decides what happens to a request carrying none.
 func webAuthRequired() bool {
-	return strings.EqualFold(strings.TrimSpace(os.Getenv("WEB_AUTH_REQUIRED")), "true")
+	setting := strings.TrimSpace(os.Getenv("WEB_AUTH_REQUIRED"))
+	if setting == "" {
+		return true
+	}
+	return !strings.EqualFold(setting, "false")
 }
 
 // publicWebPaths are the endpoints that must work before anybody has a token.

middleware/webauth_test.go

@@ -271,3 +271,64 @@ func TestNoTokenIsNotAPlatformAccount(t *testing.T) {
 		t.Fatal("claims were reported present on a request that carried none")
 	}
 }
+
+/* ── The default, after the rollout ────────────────────────────────────── */
+
+func TestEnforcementIsOnByDefault(t *testing.T) {
+	// It shipped defaulting to off so a live console could adopt tokens without
+	// its users being locked out. That finished, and the default was measured
+	// still open: a getorders with no credential returned a real merchant's
+	// orders to anyone.
+	t.Setenv("POS_TOKEN_SECRET", webTestSecret)
+	t.Setenv("WEB_AUTH_REQUIRED", "")
+
+	got := call(t, fakeLocations{}, "", "GET", "/live/api/v1/web/orders/tenant/getorders?tenantid=916", "")
+	if got != fiber.StatusUnauthorized {
+		t.Fatalf("an untokened request was served with no setting present: %d", got)
+	}
+}
+
+func TestEnforcementCanBeTurnedOffWithoutADeploy(t *testing.T) {
+	// The escape hatch. Flipping a default that can lock people out has to be
+	// reversible by one person in one minute.
+	t.Setenv("POS_TOKEN_SECRET", webTestSecret)
+	t.Setenv("WEB_AUTH_REQUIRED", "false")
+
+	got := call(t, fakeLocations{}, "", "GET", "/live/api/v1/web/orders/tenant/getorders?tenantid=916", "")
+	if got != fiber.StatusOK {
+		t.Fatalf("the escape hatch does not work: %d", got)
+	}
+}
+
+func TestOnlyTheWordFalseOpensTheDoor(t *testing.T) {
+	// A typo must fail closed. "no", "0" and "off" all look like they might
+	// disable it, and a deployment that meant to disable it and did not is far
+	// safer than one that meant to enable it and did not.
+	t.Setenv("POS_TOKEN_SECRET", webTestSecret)
+	for _, setting := range []string{"no", "0", "off", "FALSE ", "nope"} {
+		t.Setenv("WEB_AUTH_REQUIRED", setting)
+		got := call(t, fakeLocations{}, "", "GET", "/live/api/v1/web/orders/tenant/getorders?tenantid=916", "")
+		if setting == "FALSE " && got != fiber.StatusOK {
+			t.Fatalf("a trimmed, case-insensitive false was not honoured: %d", got)
+		}
+		if setting != "FALSE " && got != fiber.StatusUnauthorized {
+			t.Fatalf("%q opened the door: %d", setting, got)
+		}
+	}
+}
+
+func TestSignInStillWorksWithTheNewDefault(t *testing.T) {
+	// The test that catches a locked-out deployment. Guarding the login route
+	// means nobody can ever obtain a token.
+	t.Setenv("POS_TOKEN_SECRET", webTestSecret)
+	t.Setenv("WEB_AUTH_REQUIRED", "")
+
+	for _, path := range []string{
+		"/live/api/v1/web/users/applogin",
+		"/live/api/v1/web/tenant/weblogin",
+	} {
+		if got := call(t, fakeLocations{}, "", "POST", path, `{"authname":"a@b.c"}`); got != fiber.StatusOK {
+			t.Fatalf("%s was locked behind a session: %d", path, got)
+		}
+	}
+}