aravind changes

go-api/internal/httpserver/auth.go

@@ -42,27 +42,30 @@ const sessionCookieName = "krow_session"
 // that never authenticate anything.
 func (s *Server) secureCookies() bool { return s.cfg.AppEnv != "development" }
 
-// sameSite resolves the configured SameSite mode.
+// sessionSameSite reports the SameSite mode the session cookie must carry.
 //
-// Lax remains the default and the recommendation. "none" exists for the one
-// deployment shape that cannot work without it: a frontend on a different
-// registrable domain from the API. In that case Lax withholds the cookie on
-// every cross-site fetch, so the sign-in succeeds, the Set-Cookie arrives, and
-// the next request carries nothing — which reads as a broken session rather
-// than as a cookie policy.
+// Lax is the default and the safer value: it closes the CSRF hole by refusing
+// to travel on cross-site subresource requests. That is exactly right when the
+// page and the API share an origin, which is the supported deployment.
 //
-// An unrecognised value falls back to Lax rather than to None. config.validate
-// rejects those before startup, so this is only a belt-and-braces default in
-// the safe direction.
-func (s *Server) sameSite() http.SameSite {
-	switch s.cfg.HTTP.CookieSameSite {
-	case "none":
+// When the API is configured with a CORS allowlist, the deployment is by
+// definition the other one: a page on some other origin calls this API
+// directly. A Lax cookie is never sent on those requests, so login would
+// succeed once and every request after it would arrive anonymous. None is the
+// only mode a browser will send cross-site, and it requires Secure — which is
+// why an origin allowlist forces Secure on regardless of AppEnv.
+func (s *Server) sessionSameSite() http.SameSite {
+	if len(s.cfg.HTTP.CORSOrigins) > 0 {
 		return http.SameSiteNoneMode
-	case "strict":
-		return http.SameSiteStrictMode
-	default:
-		return http.SameSiteLaxMode
 	}
+	return http.SameSiteLaxMode
+}
+
+// crossSiteCookies reports whether the cookie must be marked Secure because it
+// has to travel cross-site. SameSite=None without Secure is rejected outright
+// by every current browser.
+func (s *Server) crossSiteCookies() bool {
+	return s.sessionSameSite() == http.SameSiteNoneMode
 }
 
 // setSessionCookie writes the raw token to the browser.
@@ -82,15 +85,13 @@ func (s *Server) setSessionCookie(w http.ResponseWriter, token string, lifetime
 		Path:  "/",
 		// HttpOnly: script cannot read it.
 		HttpOnly: true,
-		// Lax by default, and Strict/None available through
-		// HTTP_COOKIE_SAMESITE. Strict would drop the cookie on any cross-site
-		// navigation, so following a link into the app would land on a login
-		// page despite a live session. None sends it on cross-site requests,
-		// which is the CSRF hole Lax exists to close — and is nonetheless the
-		// only workable value when the frontend is on a different registrable
-		// domain. See Server.sameSite.
-		SameSite: s.sameSite(),
-		Secure:   s.secureCookies(),
+		// Lax, not Strict and not None. Strict would drop the cookie on any
+		// cross-site navigation, so following a link into the app would land on
+		// a login page despite a live session. None would require Secure and
+		// would send the cookie on cross-site POSTs, which is the CSRF hole Lax
+		// exists to close.
+		SameSite: s.sessionSameSite(),
+		Secure:   s.secureCookies() || s.crossSiteCookies(),
 		MaxAge:   int(lifetime.Seconds()),
 	})
 }
@@ -107,10 +108,8 @@ func (s *Server) clearSessionCookie(w http.ResponseWriter) {
 		Value:    "",
 		Path:     "/",
 		HttpOnly: true,
-		// Must match the attributes it was set with, SameSite included, or the
-		// browser treats this as a different cookie and leaves the original.
-		SameSite: s.sameSite(),
-		Secure:   s.secureCookies(),
+		SameSite: s.sessionSameSite(),
+		Secure:   s.secureCookies() || s.crossSiteCookies(),
 		MaxAge:   -1,
 	})
 }