From 85d8b6a85a68536208804791cd2a77fd1363ee0f Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Thu, 15 Oct 2026 22:27:31 +0200 Subject: [PATCH] fix(19.3-06): zero CookieConfig writes SameSite=Lax and Secure (WR-09) - withDefaults gives the zero config Secure and every unset SameSite Lax; a config with any field set keeps its Secure value --- docs/services/authentication.md | 2 ++ modules/bouncer/README.md | 4 ++-- modules/bouncer/cookie.go | 21 ++++++++++++++++----- modules/bouncer/cookie_test.go | 11 ++++++++++- 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/docs/services/authentication.md b/docs/services/authentication.md index 45a7ef1..5c8f2ba 100644 --- a/docs/services/authentication.md +++ b/docs/services/authentication.md @@ -78,6 +78,8 @@ This fragment belongs in `config/auth.yaml`. With the block unset the cookie is | `auth.cookie.secure` | true when `app.url` is https | In the production environment a value that resolves to false is refused, whether `false` is set or inferred from an http or unset `app.url`. | | `auth.cookie.same_site` | `lax` | `lax`, `strict` or `none`; `none` requires Secure. | +A `bouncer.CookieConfig` built by hand instead of through `bouncer.LoadCookieConfig` gets `SameSite=Lax` when `SameSite` is unset, and the zero config writes `token` on `/` with `HttpOnly`, `SameSite=Lax` and `Secure`; a config with any field set keeps its own `Secure` value. + An invalid domain, `same_site: none` without Secure, or a cookie that is not Secure in production makes `bouncer.LoadCookieConfig` return an error naming the key. In production that covers `secure: false` and an unset `auth.cookie.secure` with an http or missing `app.url`; that error names `auth.cookie.secure` and `app.url`, and you fix it by setting `app.url` to the public https URL or `auth.cookie.secure: true`. Return the error from your plugin's boot so the application does not start with a cookie browsers would drop or send in clear. `bouncer.CookieConfig.TokenCookie` builds the cookie for a token and a lifetime. Pass the guard's refresh window (`refresh_ttl`, 20160 minutes by default), not its access `ttl`: the access token inside expires after `ttl`, the frontend's 401 handling then calls the refresh endpoint, and that endpoint needs the cookie to still be there. A cookie that died with the token would force a new sign-in every hour. `bouncer.CookieConfig.ForgetTokenCookie` builds the matching cookie that deletes it; it carries the same name, path and domain, or the browser would keep the original. diff --git a/modules/bouncer/README.md b/modules/bouncer/README.md index 0e36a48..250cab4 100644 --- a/modules/bouncer/README.md +++ b/modules/bouncer/README.md @@ -31,7 +31,7 @@ Use cases: - Context helpers: `bouncer.WithUser` and `bouncer.User` for the principal, `bouncer.WithCredential` and `bouncer.Credential` for the credential behind it (for example an API token record). - Scope limits for personal API tokens: `bouncer.ScopeLimit` on `bouncer.Principal.Scope` (nil means unrestricted, as for a JWT session). A token guard sets it after filtering the token's scopes by the request method; `Full` means the token acts as the owner on every method (`*`) or on GET/HEAD (`admin:read`). - Blacklist stores behind `bouncer.BlacklistStore`: `bouncer.MemoryBlacklist` for tests and `bouncer.PostgresBlacklist` for production, which works on a caller-supplied table with `jti`, `expires_at` and `valid_until` columns and rejects unsafe table names. -- Cookie transport: `bouncer.LoadCookieConfig` reads the `auth.cookie.*` block into a `bouncer.CookieConfig` and refuses an invalid domain, `same_site: none` without Secure and, in the production environment, a Secure that resolves to false (set to `false`, or inferred from an http or unset `app.url`); `bouncer.CookieConfig.TokenCookie` builds the httpOnly token cookie whose Max-Age is the refresh window, and `bouncer.CookieConfig.ForgetTokenCookie` the matching cookie that deletes it. `bouncer.CookieRefreshHandler` and `bouncer.CookieLogoutHandler` are ready-made handlers for a cookie-based refresh and logout; the application supplies the refresh or revoke call and a writer for its own response envelope. +- Cookie transport: `bouncer.LoadCookieConfig` reads the `auth.cookie.*` block into a `bouncer.CookieConfig` and refuses an invalid domain, `same_site: none` without Secure and, in the production environment, a Secure that resolves to false (set to `false`, or inferred from an http or unset `app.url`); `bouncer.CookieConfig.TokenCookie` builds the httpOnly token cookie whose Max-Age is the refresh window, and `bouncer.CookieConfig.ForgetTokenCookie` the matching cookie that deletes it. A hand-built config with `SameSite` unset writes `SameSite=Lax`, and the zero `bouncer.CookieConfig` writes `token` on `/` with `HttpOnly`, `SameSite=Lax` and `Secure`. `bouncer.CookieRefreshHandler` and `bouncer.CookieLogoutHandler` are ready-made handlers for a cookie-based refresh and logout; the application supplies the refresh or revoke call and a writer for its own response envelope. - Cloudflare Turnstile: `bouncer.Turnstile.Verify` checks a token against siteverify with a 5 second timeout, in PHP's open mode (an empty secret passes) or closed mode (an empty secret is `bouncer.TurnstileUnavailable` in production); `bouncer.Turnstile.Middleware` reads the token from the query, a JSON body or a form body, restores the body and writes refusals through a `bouncer.TurnstileWriter`. surf registers it as `turnstile:open` and `turnstile:closed`. - Passwords: `bouncer.HashPassword`, `bouncer.CheckPassword` and `bouncer.NeedsRehash` (bcrypt, cost chosen by the caller). @@ -156,7 +156,7 @@ func RefreshRoute(cfg bouncer.CookieConfigSource, users bouncer.UserProvider, se | `bouncer.BlacklistStore` | Revoked-jti store with a grace window and sweeping. | | `bouncer.NewMemoryBlacklist` | In-process blacklist for tests. | | `bouncer.NewPostgresBlacklist` | Blacklist over a `*sql.DB` and a table name. | -| `bouncer.CookieConfig` | Token cookie attributes: `Name`, `Domain`, `Path`, `Secure`, `SameSite`. | +| `bouncer.CookieConfig` | Token cookie attributes: `Name`, `Domain`, `Path`, `Secure`, `SameSite`. An unset `SameSite` is Lax; the zero config is also `Secure`, while a config with any field set keeps its `Secure` value. | | `bouncer.CookieConfigSource` | The config methods `bouncer.LoadCookieConfig` reads (`Lookup`, `String`, `Environment`); `*compass.Config` satisfies it. | | `bouncer.LoadCookieConfig` | Reads and validates `auth.cookie.*`; every error names the key. | | `bouncer.DefaultCookieName` | `token`, the cookie name when `auth.cookie.name` is unset. | diff --git a/modules/bouncer/cookie.go b/modules/bouncer/cookie.go index a5e1ef2..b5e30cc 100644 --- a/modules/bouncer/cookie.go +++ b/modules/bouncer/cookie.go @@ -29,10 +29,12 @@ type CookieConfig struct { Domain string // Path is the cookie path (auth.cookie.path, default "/"). Path string - // Secure sends the cookie over https only. + // Secure sends the cookie over https only. It has no default except for + // the zero CookieConfig, which writes a Secure cookie; LoadCookieConfig + // derives it from app.url unless auth.cookie.secure is set. Secure bool - // SameSite is http.SameSiteLaxMode unless auth.cookie.same_site says - // otherwise. + // SameSite is http.SameSiteLaxMode when unset (also on a hand-built + // config) and otherwise what auth.cookie.same_site says. SameSite http.SameSite } @@ -263,9 +265,18 @@ func CookieLogoutHandler(opts CookieLogoutOptions) (http.Handler, error) { }), nil } -// withDefaults fills an empty Name and Path, so a zero CookieConfig writes -// the default token cookie on "/". +// withDefaults fills an empty Name and Path and an unset SameSite (Lax). The +// zero CookieConfig also gets Secure, so it writes the default token cookie +// on "/" with HttpOnly, SameSite=Lax and Secure; a config with any field set +// keeps its Secure value (LoadCookieConfig derives it from app.url). +// TokenCookie, ForgetTokenCookie and both handlers go through it. func (c CookieConfig) withDefaults() CookieConfig { + if c == (CookieConfig{}) { + c.Secure = true + } + if c.SameSite == 0 { + c.SameSite = http.SameSiteLaxMode + } if strings.TrimSpace(c.Name) == "" { c.Name = DefaultCookieName } diff --git a/modules/bouncer/cookie_test.go b/modules/bouncer/cookie_test.go index f068ef3..2a42d7b 100644 --- a/modules/bouncer/cookie_test.go +++ b/modules/bouncer/cookie_test.go @@ -147,11 +147,20 @@ func TestTokenCookieAttributes(t *testing.T) { } } - // A zero config writes the default cookie on "/" and host-only. + // A zero config writes the default cookie on "/", host-only, with + // SameSite=Lax and Secure. z := CookieConfig{}.TokenCookie("v", time.Minute) if z.Name != DefaultCookieName || z.Path != "/" || z.Domain != "" || !z.HttpOnly { t.Fatalf("zero config cookie = %+v", z) } + if z.SameSite != http.SameSiteLaxMode || !z.Secure { + t.Fatalf("zero config cookie SameSite %v Secure %t, want Lax and Secure", z.SameSite, z.Secure) + } + // A partial config gets SameSite=Lax and keeps its Secure value. + p := CookieConfig{Name: "sid"}.TokenCookie("v", time.Minute) + if p.SameSite != http.SameSiteLaxMode || p.Secure { + t.Fatalf("partial config cookie SameSite %v Secure %t, want Lax and not Secure", p.SameSite, p.Secure) + } if strings.Contains(z.String(), "Domain=") { t.Fatalf("host-only cookie carries a domain: %s", z.String()) }