From e7659f2f4b0301243ff181cb641afac0a87c67fe Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Thu, 15 Oct 2026 17:49:35 +0200 Subject: [PATCH] feat(19.3-02): cookie token refresh across subdomains with hardened credentialed CORS - bouncer: CookieConfig from auth.cookie.* (LoadCookieConfig validates domain, same_site none without Secure, secure false in production), TokenCookie with Max-Age = refresh window, ForgetTokenCookie, CookieRefreshHandler and CookieLogoutHandler with an app-supplied writer - surf: http.cors refuses credentials with a * origin and bad or PHP-delimited patterns at boot, turns wildcard origins into anchored patterns, echoes the exact origin and sends Vary: Origin on every dynamic CORS answer; the non-credentialed * answer is unchanged - smoke tests for the www -> api refresh and the refused wildcard config --- docs/services/authentication.md | 30 +++ docs/services/routing.md | 21 ++ modules/bouncer/README.md | 70 ++++++- modules/bouncer/cookie.go | 280 +++++++++++++++++++++++++++ modules/surf/README.md | 8 +- modules/surf/cors.go | 209 +++++++++++++++----- modules/surf/transport_smoke_test.go | 184 ++++++++++++++++++ 7 files changed, 748 insertions(+), 54 deletions(-) create mode 100644 modules/bouncer/cookie.go create mode 100644 modules/surf/transport_smoke_test.go diff --git a/docs/services/authentication.md b/docs/services/authentication.md index c7fabd1..6f5edde 100644 --- a/docs/services/authentication.md +++ b/docs/services/authentication.md @@ -58,6 +58,36 @@ Revoked token IDs are kept in a `bouncer.BlacklistStore`: Logging out is blacklisting the token's `jti`. Setting a user's `TokensValidAfter` to now revokes all their tokens at once, for example after a password change. +## Cookie transport + +A browser frontend can keep the token in an httpOnly cookie instead of JavaScript-readable storage. The cookie attributes come from one block, `auth.cookie.*`, which `bouncer.LoadCookieConfig` reads into a `bouncer.CookieConfig` at boot: + +```yaml +cookie: + domain: example.com + same_site: lax +``` + +This fragment belongs in `config/auth.yaml`. With the block unset the cookie is named `token`, has `Path=/`, `HttpOnly` and `SameSite=Lax`, no `Domain` (host-only) and `Secure` exactly when `app.url` is https. The keys: + +| Key | Default | Controls | +|-----|---------|----------| +| `auth.cookie.name` | `token` | Cookie name. | +| `auth.cookie.domain` | empty (host-only) | A parent domain shares the cookie with its subdomains. | +| `auth.cookie.path` | `/` | Cookie path. | +| `auth.cookie.secure` | true when `app.url` is https | `false` is refused in the production environment. | +| `auth.cookie.same_site` | `lax` | `lax`, `strict` or `none`; `none` requires Secure. | + +An invalid domain, `same_site: none` without Secure or `secure: false` in production makes `bouncer.LoadCookieConfig` return an error naming the key; return it from your plugin's boot so the application does not start with a cookie browsers would drop. + +`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. + +`bouncer.CookieRefreshHandler` is a ready-made refresh endpoint. It reads the token from the cookie (only from the cookie), calls your refresh func, which wraps `bouncer.Refresh` or `bouncer.RefreshAudienceFor` and so rotates the token and blacklists the old `jti`, sets the rotated cookie and passes a `bouncer.CookieResult` to your writer, which answers in the application's own envelope. A missing cookie (`bouncer.ErrNoTokenCookie`) or a refused refresh reaches the writer with status 401 and leaves the cookie alone. `bouncer.CookieLogoutHandler` revokes the cookie's token through your revoke func, always clears the cookie and answers 200. The [bouncer README](../../modules/bouncer/README.md) has a complete example. + +**Subdomains.** For a frontend on `www.example.com` calling an API on `api.example.com`, set `auth.cookie.domain: example.com`, list `https://www.example.com` in the API's CORS origins with `supports_credentials: true` (see [Routing](routing.md#cors)), and send requests with credentials. A `Domain` cookie reaches every subdomain of `example.com`, so every host under it is inside the trust boundary: an application on another subdomain receives the token too. Keep the default host-only cookie when frontend and API share one host. + +**CSRF.** CORS controls who may read a response, not who may send a request. `SameSite=Lax` treats sibling subdomains as the same site, so a page on any `*.example.com` host can make the browser send the cookie. Mount the refresh and logout handlers on POST routes only, and for routes that act on the cookie prefer a JSON body or a custom header, which a cross-site form cannot send without a CORS preflight. The refresh answer is unreadable to an origin CORS does not allow, and a forced logout is the worst a cross-site POST to the logout route can do. + ## Guards and middleware A guard implements `bouncer.Guard`: it turns a request into a principal or an error. `bouncer.NewJWTGuard` is the frontend guard. It reads the bearer token, then any cookie names you give it, verifies the token, checks the blacklist and the user's cutoff, and loads the user through your `bouncer.UserProvider`. On failure it answers 401 with `{"error":true,"message":...}`. `bouncer.NewBackendJWTGuard` is the same for the admin audience. diff --git a/docs/services/routing.md b/docs/services/routing.md index 7e1f593..f93d889 100644 --- a/docs/services/routing.md +++ b/docs/services/routing.md @@ -224,4 +224,25 @@ cors: This fragment belongs in `config/http.yaml`. List the frontend's exact origin; `*` is for public, credential-free APIs only. +The rules surf applies to the origin answer: + +- `supports_credentials: true` together with `*` in `allowed_origins` fails the start-up. A credentialed answer that trusted any origin would let every site read responses made with the user's cookies; Laravel's php-cors reflects the origin in that case, surf refuses it. +- An allowed origin is echoed exactly in `Access-Control-Allow-Origin`; `*` is sent only for `allowed_origins: ["*"]` without credentials, and that answer is unchanged from earlier releases. +- When the answer depends on the request's `Origin` (with `supports_credentials`, with any pattern, or with more than one listed origin), every response on a CORS path carries `Vary: Origin`, the preflight and a disallowed origin's response included, so a shared cache never serves one origin's answer to another. +- An `allowed_origins` entry with a `*` inside, such as `https://*.example.com`, matches like Laravel's `Str::is`: the `*` stands for any characters and the rest is literal. +- `allowed_origins_patterns` entries are Go regular expressions. One that does not compile fails the start-up, and so does one still wrapped in PHP delimiters, such as `#^https://.*\.example\.com$#`: drop the delimiters and any flags when porting it. + +A frontend on `https://www.example.com` that calls an API on `api.example.com` with the session cookie (see [Cookie transport](authentication.md#cookie-transport)) needs the www origin listed and credentials on: + +```yaml +cors: + paths: ["api/*"] + allowed_origins: ["https://www.example.com"] + allowed_methods: ["*"] + allowed_headers: ["*"] + supports_credentials: true +``` + +The browser sends the request with `credentials: "include"`; the answer carries `Access-Control-Allow-Origin: https://www.example.com`, `Access-Control-Allow-Credentials: true` and `Vary: Origin`. + surf answers every `OPTIONS` request on a CORS path itself, with 204, before any route runs, so a plugin never registers an `OPTIONS` route. A preflight gets the same headers Laravel's `HandleCors` sends: with `allowed_methods: ["*"]` the `Access-Control-Allow-Methods` value is the requested method, upper-cased, and with `allowed_headers: ["*"]` the `Access-Control-Allow-Headers` value is the requested header list. diff --git a/modules/bouncer/README.md b/modules/bouncer/README.md index f796ae6..fee6a99 100644 --- a/modules/bouncer/README.md +++ b/modules/bouncer/README.md @@ -31,6 +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 `secure: false` in the production environment; `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. - Passwords: `bouncer.HashPassword`, `bouncer.CheckPassword` and `bouncer.NeedsRehash` (bcrypt, cost chosen by the caller). ## Usage @@ -79,6 +80,47 @@ func Login(secret string) (string, error) { } ``` +An application that keeps the token in an httpOnly cookie, for a frontend on `www.example.com` calling the API on `api.example.com`, loads the cookie config at boot and mounts the refresh handler on a POST route: + +```go +package acme + +import ( + "context" + "encoding/json" + "net/http" + "time" + + "git.golem15.com/golem15/summercms/modules/bouncer" +) + +// RefreshRoute rotates the cookie's token. cfg is the app config +// (app.Config); users, secret and bl are the guard's; refreshTTL is its +// refresh_ttl. +func RefreshRoute(cfg bouncer.CookieConfigSource, users bouncer.UserProvider, secret string, bl bouncer.BlacklistStore, refreshTTL time.Duration) (http.Handler, error) { + cookie, err := bouncer.LoadCookieConfig(cfg) // a bad auth.cookie block fails boot + if err != nil { + return nil, err + } + return bouncer.CookieRefreshHandler(bouncer.CookieRefreshOptions{ + Cookie: cookie, + RefreshTTL: refreshTTL, + Refresh: func(ctx context.Context, token string) (string, error) { + return bouncer.RefreshAudienceFor(ctx, users, secret, token, bouncer.AudienceUser, refreshTTL, bl, 0, "https://api.example.com/api/auth/refresh") + }, + Write: func(w http.ResponseWriter, r *http.Request, res bouncer.CookieResult) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(res.Status) + if res.Err != nil { + _ = json.NewEncoder(w).Encode(map[string]any{"success": false, "error": "UNAUTHENTICATED"}) + return + } + _ = json.NewEncoder(w).Encode(map[string]any{"success": true}) + }, + }) +} +``` + ## API reference | Identifier | Description | @@ -113,15 +155,41 @@ func Login(secret string) (string, error) { | `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.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. | +| `bouncer.CookieConfig.TokenCookie` | The httpOnly cookie carrying a token, Max-Age the refresh window passed in. | +| `bouncer.CookieConfig.ForgetTokenCookie` | The same cookie with an empty value and a past expiry, which deletes it. | +| `bouncer.CookieRefreshHandler` | Reads the token cookie, calls the application's refresh func, sets the rotated cookie and hands a `bouncer.CookieResult` to the writer. | +| `bouncer.CookieRefreshOptions` | Cookie config, refresh window, refresh func and writer of `bouncer.CookieRefreshHandler`. | +| `bouncer.CookieLogoutHandler` | Revokes the cookie's token through the application's func, clears the cookie and writes 200. | +| `bouncer.CookieLogoutOptions` | Cookie config, revoke func and writer of `bouncer.CookieLogoutHandler`. | +| `bouncer.CookieResult` | What the cookie handlers pass to the writer: status, rotated token and error. | +| `bouncer.ErrNoTokenCookie` | The refresh request carried no token cookie (401). | | `bouncer.HashPassword` | Returns a bcrypt hash at the given cost. | | `bouncer.CheckPassword` | Reports whether a password matches a hash. | | `bouncer.NeedsRehash` | Reports whether a hash was made below the configured cost. | +## Configuration + +`bouncer.LoadCookieConfig` reads these keys; nothing else in bouncer reads config. The per-guard token lifetimes (`ttl`, `refresh_ttl`) stay with the guard's own config, such as the user plugin's `jwt` block or `admin.jwt.*`. + +| Key | Default | Controls | +|-----|---------|----------| +| `auth.cookie.name` | `token` | Cookie name; must be a valid cookie name. | +| `auth.cookie.domain` | empty (host-only) | Cookie domain. Set a parent domain such as `example.com` to share the cookie between `api.example.com` and `www.example.com`; every subdomain then receives it. An invalid domain fails boot. | +| `auth.cookie.path` | `/` | Cookie path; must start with `/`. | +| `auth.cookie.secure` | true when `app.url` is https | Sends the cookie over https only. `false` is refused in the production environment. | +| `auth.cookie.same_site` | `lax` | `lax`, `strict` or `none`; `none` requires Secure. | + +Environment variables follow the compass mapping, for example `SUMMER_AUTH__COOKIE__DOMAIN`. + ## Dependencies - SummerCMS modules: none. - Third-party: `github.com/golang-jwt/jwt/v5`, `golang.org/x/crypto/bcrypt`. -- Standard library: `context`, `crypto/rand`, `database/sql`, `encoding/hex`, `encoding/json`, `errors`, `fmt`, `math`, `net/http`, `reflect`, `regexp`, `strconv`, `strings`, `sync`, `time`. +- Standard library: `context`, `crypto/rand`, `database/sql`, `encoding/hex`, `encoding/json`, `errors`, `fmt`, `math`, `net/http`, `net/url`, `reflect`, `regexp`, `strconv`, `strings`, `sync`, `time`. - Tests additionally use `github.com/testcontainers/testcontainers-go` with its `modules/postgres` package, and `github.com/jackc/pgx/v5/stdlib`. ## Testing diff --git a/modules/bouncer/cookie.go b/modules/bouncer/cookie.go new file mode 100644 index 0000000..662f617 --- /dev/null +++ b/modules/bouncer/cookie.go @@ -0,0 +1,280 @@ +package bouncer + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/url" + "strconv" + "strings" + "time" +) + +// DefaultCookieName is the token cookie name when auth.cookie.name is unset. +const DefaultCookieName = "token" + +// ErrNoTokenCookie is reported by CookieRefreshHandler when the request has +// no token cookie or an empty one. +var ErrNoTokenCookie = errors.New(msgTokenNotProvided) + +// CookieConfig holds the attributes of the httpOnly token cookie. Every guard +// shares one config; the lifetime is passed per call (the guard's +// refresh_ttl), so the cookie outlives the access token it carries. +type CookieConfig struct { + // Name is the cookie name (auth.cookie.name, default "token"). + Name string + // Domain is empty for a host-only cookie (the default); a parent domain + // such as example.com shares the cookie with every subdomain. + Domain string + // Path is the cookie path (auth.cookie.path, default "/"). + Path string + // Secure sends the cookie over https only. + Secure bool + // SameSite is http.SameSiteLaxMode unless auth.cookie.same_site says + // otherwise. + SameSite http.SameSite +} + +// CookieConfigSource is the part of the application config LoadCookieConfig +// reads. *compass.Config satisfies it, so bouncer imports no framework module. +type CookieConfigSource interface { + Lookup(path string) (any, bool) + String(path string) string + Environment() string +} + +// LoadCookieConfig reads the auth.cookie block: +// +// - auth.cookie.name: default DefaultCookieName; must be a valid cookie name. +// - auth.cookie.path: default "/"; must start with "/". +// - auth.cookie.domain: default empty (host-only); a non-empty value must +// be a valid cookie domain, so it is never dropped silently when the +// cookie is written. +// - auth.cookie.same_site: lax (default), strict or none, case-insensitive. +// - auth.cookie.secure: unset means true exactly when app.url is https; an +// explicit false is refused in the production environment. +// +// same_site none without Secure is refused, as browsers drop such cookies. +// Every error names the key. +func LoadCookieConfig(src CookieConfigSource) (CookieConfig, error) { + cfg := CookieConfig{Name: DefaultCookieName, Path: "/", SameSite: http.SameSiteLaxMode} + if src == nil { + return cfg, nil + } + if raw, ok := src.Lookup("auth.cookie.name"); ok && raw != nil { + name := strings.TrimSpace(fmt.Sprint(raw)) + if name == "" || (&http.Cookie{Name: name, Value: "x"}).Valid() != nil { + return cfg, fmt.Errorf("bouncer: auth.cookie.name %q is not a valid cookie name", name) + } + cfg.Name = name + } + if raw, ok := src.Lookup("auth.cookie.path"); ok && raw != nil { + p := strings.TrimSpace(fmt.Sprint(raw)) + if !strings.HasPrefix(p, "/") || (&http.Cookie{Name: "x", Value: "x", Path: p}).Valid() != nil { + return cfg, fmt.Errorf("bouncer: auth.cookie.path %q must be an absolute URL path starting with /", p) + } + cfg.Path = p + } + if raw, ok := src.Lookup("auth.cookie.domain"); ok && raw != nil { + d := strings.TrimSpace(fmt.Sprint(raw)) + if d != "" { + if (&http.Cookie{Name: "x", Value: "x", Domain: d}).Valid() != nil { + return cfg, fmt.Errorf("bouncer: auth.cookie.domain %q is not a valid cookie domain", d) + } + cfg.Domain = d + } + } + if raw, ok := src.Lookup("auth.cookie.same_site"); ok && raw != nil { + switch v := strings.ToLower(strings.TrimSpace(fmt.Sprint(raw))); v { + case "", "lax": + cfg.SameSite = http.SameSiteLaxMode + case "strict": + cfg.SameSite = http.SameSiteStrictMode + case "none": + cfg.SameSite = http.SameSiteNoneMode + default: + return cfg, fmt.Errorf("bouncer: auth.cookie.same_site %q must be lax, strict or none", v) + } + } + secure, explicit, err := cookieSecure(src) + if err != nil { + return cfg, err + } + if explicit && !secure && strings.EqualFold(strings.TrimSpace(src.Environment()), "production") { + return cfg, errors.New("bouncer: auth.cookie.secure: false is not allowed in the production environment") + } + cfg.Secure = secure + if cfg.SameSite == http.SameSiteNoneMode && !cfg.Secure { + return cfg, errors.New("bouncer: auth.cookie.same_site none requires auth.cookie.secure true (browsers drop SameSite=None cookies without Secure)") + } + return cfg, nil +} + +// cookieSecure reads auth.cookie.secure; unset falls back to the app.url +// scheme. explicit reports whether the key was set. +func cookieSecure(src CookieConfigSource) (secure, explicit bool, err error) { + raw, ok := src.Lookup("auth.cookie.secure") + if !ok || raw == nil { + u, perr := url.Parse(strings.TrimSpace(src.String("app.url"))) + return perr == nil && strings.EqualFold(u.Scheme, "https"), false, nil + } + switch v := raw.(type) { + case bool: + return v, true, nil + case string: + b, perr := strconv.ParseBool(strings.TrimSpace(v)) + if perr != nil { + return false, true, fmt.Errorf("bouncer: auth.cookie.secure %q must be true or false", v) + } + return b, true, nil + default: + return false, true, fmt.Errorf("bouncer: auth.cookie.secure must be true or false, got %T", raw) + } +} + +// TokenCookie returns the httpOnly cookie carrying token. Max-Age is +// refreshTTL in whole seconds: the refresh window, never the access ttl, so +// the refresh endpoint still has a cookie to read after the token expired. +func (c CookieConfig) TokenCookie(token string, refreshTTL time.Duration) *http.Cookie { + c = c.withDefaults() + return &http.Cookie{ + Name: c.Name, + Value: token, + Path: c.Path, + Domain: c.Domain, + MaxAge: int(refreshTTL / time.Second), + Expires: time.Now().Add(refreshTTL), + HttpOnly: true, + Secure: c.Secure, + SameSite: c.SameSite, + } +} + +// ForgetTokenCookie returns a cookie with the same name, path, domain and +// flags as TokenCookie, an empty value and an expiry in the past, so the +// browser deletes the token cookie. +func (c CookieConfig) ForgetTokenCookie() *http.Cookie { + c = c.withDefaults() + return &http.Cookie{ + Name: c.Name, + Value: "", + Path: c.Path, + Domain: c.Domain, + MaxAge: -1, + Expires: time.Unix(0, 0).UTC(), + HttpOnly: true, + Secure: c.Secure, + SameSite: c.SameSite, + } +} + +// CookieResult is what the cookie handlers pass to the application's writer. +// Status is the HTTP status to answer with; Token is the rotated token on a +// successful refresh; Err is the refusal (refresh) or the revoke failure +// (logout, which still answers 200 and clears the cookie). +type CookieResult struct { + Status int + Token string + Err error +} + +// CookieRefreshOptions configures CookieRefreshHandler. +type CookieRefreshOptions struct { + // Cookie names the cookie to read and the attributes of the new one. + Cookie CookieConfig + // RefreshTTL is the guard's refresh window (refresh_ttl); it becomes the + // new cookie's Max-Age. + RefreshTTL time.Duration + // Refresh rotates the token, typically by calling Refresh or + // RefreshAudienceFor, which blacklist the old jti. + Refresh func(ctx context.Context, token string) (string, error) + // Write answers the request in the application's response envelope. + Write func(w http.ResponseWriter, r *http.Request, res CookieResult) +} + +// CookieLogoutOptions configures CookieLogoutHandler. +type CookieLogoutOptions struct { + // Cookie names the cookie to read and clear. + Cookie CookieConfig + // Revoke blacklists the token, for example through + // VerifyRefreshableClaimsAudience and BlacklistStore.Add. + Revoke func(ctx context.Context, token string) error + // Write answers the request in the application's response envelope. + Write func(w http.ResponseWriter, r *http.Request, res CookieResult) +} + +// CookieRefreshHandler returns a handler that reads the token cookie, calls +// opts.Refresh and, on success, sets the rotated cookie and writes 200 with +// the new token. A missing or empty cookie writes 401 with ErrNoTokenCookie, +// and a refused refresh writes 401 with the refusal; both leave the cookie +// alone. A nil Refresh or Write, or a non-positive RefreshTTL, is an error. +// Mount it on a POST route. +func CookieRefreshHandler(opts CookieRefreshOptions) (http.Handler, error) { + if opts.Refresh == nil { + return nil, errors.New("bouncer: CookieRefreshOptions.Refresh is nil") + } + if opts.Write == nil { + return nil, errors.New("bouncer: CookieRefreshOptions.Write is nil") + } + if opts.RefreshTTL <= 0 { + return nil, errors.New("bouncer: CookieRefreshOptions.RefreshTTL must be positive") + } + cookie := opts.Cookie.withDefaults() + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + token := readTokenCookie(r, cookie.Name) + if token == "" { + opts.Write(w, r, CookieResult{Status: http.StatusUnauthorized, Err: ErrNoTokenCookie}) + return + } + next, err := opts.Refresh(r.Context(), token) + if err != nil { + opts.Write(w, r, CookieResult{Status: http.StatusUnauthorized, Err: err}) + return + } + http.SetCookie(w, cookie.TokenCookie(next, opts.RefreshTTL)) + opts.Write(w, r, CookieResult{Status: http.StatusOK, Token: next}) + }), nil +} + +// CookieLogoutHandler returns a handler that revokes the token in the cookie +// (when there is one) through opts.Revoke, always clears the cookie and +// writes 200; a revoke failure is passed to Write in CookieResult.Err. A nil +// Revoke or Write is an error. Mount it on a POST route. +func CookieLogoutHandler(opts CookieLogoutOptions) (http.Handler, error) { + if opts.Revoke == nil { + return nil, errors.New("bouncer: CookieLogoutOptions.Revoke is nil") + } + if opts.Write == nil { + return nil, errors.New("bouncer: CookieLogoutOptions.Write is nil") + } + cookie := opts.Cookie.withDefaults() + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var revokeErr error + if token := readTokenCookie(r, cookie.Name); token != "" { + revokeErr = opts.Revoke(r.Context(), token) + } + http.SetCookie(w, cookie.ForgetTokenCookie()) + opts.Write(w, r, CookieResult{Status: http.StatusOK, Err: revokeErr}) + }), nil +} + +// withDefaults fills an empty Name and Path, so a zero CookieConfig writes +// the default token cookie on "/". +func (c CookieConfig) withDefaults() CookieConfig { + if strings.TrimSpace(c.Name) == "" { + c.Name = DefaultCookieName + } + if c.Path == "" { + c.Path = "/" + } + return c +} + +func readTokenCookie(r *http.Request, name string) string { + c, err := r.Cookie(name) + if err != nil { + return "" + } + return strings.TrimSpace(c.Value) +} diff --git a/modules/surf/README.md b/modules/surf/README.md index 85d51f5..49f649b 100644 --- a/modules/surf/README.md +++ b/modules/surf/README.md @@ -31,7 +31,7 @@ Use cases: - Fixed-window rate limiting (`surf.FixedWindowLimiter`): named buckets from plugins that implement `surf.BucketProvider`, or inline limits keyed by the signed-in user, or by client IP for guests. Rejected requests get a 429 with `Retry-After` and `X-RateLimit-*` headers. The in-process `surf.MemoryStore` sits behind the `surf.Store` interface; `surf.BuildRouter` uses a `surf.Store` published on the app instead when there is one, which is how `serve` shares counters across instances through [cooler](../cooler/README.md)'s Redis store (`http.rate_limit.store: redis`). - Client IP resolution for limiter keys (`surf.ClientIP`) that only trusts `X-Forwarded-For` hops when the direct peer is inside a configured trusted proxy range (`surf.TrustedProxies`). - Every non-raw route runs inside JSON panic recovery (an opaque 500 via [wire](../wire/README.md)), gets the request locale from a published `surf.LocaleResolver` when one exists or from the `Accept-Language` header otherwise (see [towel](../towel/README.md)), and a request body cap. Responses are buffered until the handler returns, so a panic never leaves a half-written body. -- Path-scoped CORS configured with the same keys as Laravel's `config/cors.php` (`surf.CORSConfig`), including preflight handling. Every `OPTIONS` request on a CORS path is answered with 204 before routing, with the headers Laravel's `HandleCors` sends: `Cache-Control: no-cache, private` always and, on a preflight (an `Origin` and an `Access-Control-Request-Method`), the requested method (upper-cased) and headers echoed in `Access-Control-Allow-Methods` and `Access-Control-Allow-Headers` when `*` allows any, `Vary` on the request headers and PHP's default `Content-Type: text/html; charset=UTF-8`. +- Path-scoped CORS configured with the same keys as Laravel's `config/cors.php` (`surf.CORSConfig`), including preflight handling. `surf.LoadCORSConfig` refuses credentials with a `*` origin and origin patterns that do not compile; an allowed origin is echoed exactly, with `Vary: Origin` on every response whose answer depends on the request origin. Every `OPTIONS` request on a CORS path is answered with 204 before routing, with the headers Laravel's `HandleCors` sends: `Cache-Control: no-cache, private` always and, on a preflight (an `Origin` and an `Access-Control-Request-Method`), the requested method (upper-cased) and headers echoed in `Access-Control-Allow-Methods` and `Access-Control-Allow-Headers` when `*` allows any, `Vary` on the request headers and PHP's default `Content-Type: text/html; charset=UTF-8`. - Optional `surf.LocaleResolver`: a compiled plugin publishes the interface through backpack; surf looks it up without importing the plugin. When present, it may strip an enabled locale prefix before routing and writes a validated code with `towel.WithLocale`. When absent, hosts keep the raw `Accept-Language` header and `surf.LocaleFromPrincipal` overlay. - `surf.LocaleFromPrincipal` switches the request locale to the signed-in user's preferred locale. - Opt-in uploads route: with `storage.uploads.serve` true, `surf.BuildRouter` mounts `GET /{path...}` (owned by `summercms.surf`) on `attach.StaticHandlerPublic`, so a file is served only when its `system_files` row is public. Every response carries `X-Content-Type-Options: nosniff` and a sandboxing `Content-Security-Policy`, and a file whose type is not a raster image (JPEG, PNG, GIF, WebP, AVIF) is sent with `Content-Disposition: attachment`, so a stored HTML or SVG file never renders on the app origin. The bucket and the database are looked up per request, so `route:list` lists the route without opening either; until `serve` publishes them it answers 404. The route owns the whole prefix: `surf.BuildRouter` fails when a plugin route sits at or under it, and when the prefix is not an absolute URL path (no leading slash, a scheme or host, a query, an empty, `.` or `..` segment, or `{`/`}`). @@ -149,13 +149,13 @@ The generated application `main` wires surf in with `surf.ServeCommand` and `sur | `http.body_limits.upload_bytes` | none, required | Upload body cap in bytes. Must be a whole number of at least 1; it is validated at boot. | | `http.trusted_proxies` | empty | List of CIDRs whose `X-Forwarded-For` header is trusted when resolving the client IP. Malformed entries are skipped. | | `http.cors.paths` | empty | Path globs (`api/*`) that get CORS headers. With no `http.cors` section no CORS headers are sent. | -| `http.cors.allowed_origins` | empty | Allowed origins; `*` allows any. | -| `http.cors.allowed_origins_patterns` | empty | Regular expressions matched against the origin. | +| `http.cors.allowed_origins` | empty | Allowed origins, echoed exactly; `*` allows any (without credentials only). An entry with a `*` inside, such as `https://*.example.com`, matches any characters there, as Laravel's `Str::is` does. | +| `http.cors.allowed_origins_patterns` | empty | Go regular expressions matched against the origin. A pattern that does not compile, or one still wrapped in PHP delimiters (`#^...$#`), fails boot. | | `http.cors.allowed_methods` | empty | Methods sent in `Access-Control-Allow-Methods`; `*` allows any. | | `http.cors.allowed_headers` | empty | Headers sent in `Access-Control-Allow-Headers`; `*` allows any. | | `http.cors.exposed_headers` | empty | Headers sent in `Access-Control-Expose-Headers`. | | `http.cors.max_age` | `0` | Preflight cache time in seconds. | -| `http.cors.supports_credentials` | `false` | Sends `Access-Control-Allow-Credentials: true`. | +| `http.cors.supports_credentials` | `false` | Sends `Access-Control-Allow-Credentials: true`. Combined with `*` in `allowed_origins` it fails boot. With credentials, a pattern or several origins, every response on a CORS path carries `Vary: Origin`, preflights and disallowed origins included. | | `http.rate_limit.store` | `memory` | Where `throttle` counters live: `memory` (per instance) or `redis` (shared by every instance through [cooler](../cooler/README.md), which then needs `redis.url`). Only `serve` installs the Redis store; `route:list` keeps the memory store and never dials Redis. Any other value fails at boot. Environment: `SUMMER_HTTP__RATE_LIMIT__STORE`. | | `http.access_log` | `false` | Logs one `http request` record per request (method, matched pattern, status, duration in milliseconds; never the path or query). Environment: `SUMMER_HTTP__ACCESS_LOG`. | | `storage.uploads.serve` | `false` | Mounts the uploads route `GET /{path...}` (prefix default `/storage/uploads`; it must then be an absolute URL path, or start-up fails) on `attach.StaticHandlerPublic` with `X-Content-Type-Options: nosniff`, a sandboxing CSP and `Content-Disposition: attachment` for anything but a raster image. Prefer a separate origin for uploads in production. Environment: `SUMMER_STORAGE__UPLOADS__SERVE`. | diff --git a/modules/surf/cors.go b/modules/surf/cors.go index 3324017..13d3832 100644 --- a/modules/surf/cors.go +++ b/modules/surf/cors.go @@ -1,6 +1,7 @@ package surf import ( + "fmt" "net/http" "regexp" "strconv" @@ -22,7 +23,9 @@ type CORSConfig struct { } // LoadCORSConfig reads http.cors. Missing section yields a zero config (no -// path matches, so no CORS headers). +// path matches, so no CORS headers). The config is validated: credentials +// with a "*" origin, an origin pattern that does not compile and a pattern +// still wrapped in PHP delimiters (#^...$#) are errors naming the key. func LoadCORSConfig(cfg *compass.Config) (CORSConfig, error) { var out CORSConfig if cfg == nil || !cfg.Has("http.cors") { @@ -31,9 +34,162 @@ func LoadCORSConfig(cfg *compass.Config) (CORSConfig, error) { if err := cfg.LoadSection("http.cors", &out); err != nil { return out, err } + if err := validateCORSConfig("http.cors", out); err != nil { + return out, err + } return out, nil } +// validateCORSConfig checks a CORS block; prefix is its config path +// (http.cors or http.cors_profiles.) for the error messages. +func validateCORSConfig(prefix string, cfg CORSConfig) error { + if cfg.SupportsCredentials && containsStar(cfg.AllowedOrigins) { + return fmt.Errorf("surf: %s.supports_credentials is true with \"*\" in %s.allowed_origins: a credentialed answer would trust every origin; list the exact origins (or use %s.allowed_origins_patterns)", prefix, prefix, prefix) + } + for i, p := range cfg.AllowedOriginsPatterns { + if phpDelimited(p) { + return fmt.Errorf("surf: %s.allowed_origins_patterns[%d] %q is wrapped in PHP delimiters; drop the leading and trailing %q (and any flags): Go patterns take none", prefix, i, p, p[:1]) + } + if _, err := regexp.Compile(p); err != nil { + return fmt.Errorf("surf: %s.allowed_origins_patterns[%d] %q does not compile: %v", prefix, i, p, err) + } + } + return nil +} + +// phpDelimited reports a preg pattern such as #^https://x$# or /^x$/i: the +// first byte is # or /, the same byte closes the pattern and only flag +// letters follow it. An origin pattern never starts with either byte. +func phpDelimited(p string) bool { + if len(p) < 2 || (p[0] != '#' && p[0] != '/') { + return false + } + end := strings.LastIndexByte(p, p[0]) + if end <= 0 { + return false + } + for _, c := range p[end+1:] { + if (c < 'a' || c > 'z') && (c < 'A' || c > 'Z') { + return false + } + } + return true +} + +// wildcardOriginPattern turns an allowed_origins entry containing "*" into an +// anchored pattern, as php-cors convertWildcardToPattern does: the rest is +// quoted and each "*" matches any characters. +func wildcardOriginPattern(origin string) string { + parts := strings.Split(origin, "*") + for i, part := range parts { + parts[i] = regexp.QuoteMeta(part) + } + return "^" + strings.Join(parts, ".*") + "$" +} + +// corsPolicy is the per-request CORS logic shared by the global path-scoped +// wrapper and the cors: middleware. +type corsPolicy struct { + cfg CORSConfig + origins map[string]struct{} + originPats []*regexp.Regexp + allowAnyOrigin bool + allowAnyMethod bool + allowAnyHeader bool + // dynamic is true when the Allow-Origin answer depends on the request + // Origin (credentials, patterns or several origins): every response then + // carries Vary: Origin, so a shared cache never serves one origin's + // answer to another. + dynamic bool + methods string + headers string + exposed string +} + +func newCORSPolicy(cfg CORSConfig) *corsPolicy { + p := &corsPolicy{ + cfg: cfg, + origins: make(map[string]struct{}, len(cfg.AllowedOrigins)), + allowAnyOrigin: containsStar(cfg.AllowedOrigins), + allowAnyMethod: containsStar(cfg.AllowedMethods), + allowAnyHeader: containsStar(cfg.AllowedHeaders), + methods: strings.Join(cfg.AllowedMethods, ", "), + headers: strings.Join(cfg.AllowedHeaders, ", "), + exposed: strings.Join(cfg.ExposedHeaders, ", "), + } + for _, pat := range cfg.AllowedOriginsPatterns { + re, err := regexp.Compile(pat) + if err != nil { + continue // LoadCORSConfig refuses it; a hand-built config skips it + } + p.originPats = append(p.originPats, re) + } + for _, o := range cfg.AllowedOrigins { + switch { + case o == "" || o == "*": + case strings.Contains(o, "*"): + if !p.allowAnyOrigin { + p.originPats = append(p.originPats, regexp.MustCompile(wildcardOriginPattern(o))) + } + default: + p.origins[o] = struct{}{} + } + } + p.dynamic = cfg.SupportsCredentials || len(p.originPats) > 0 || len(p.origins) > 1 + return p +} + +// apply sets the CORS response headers for origin and reports whether the +// origin is allowed. +func (p *corsPolicy) apply(h http.Header, origin string) bool { + allowed, value := corsAllowOrigin(p.allowAnyOrigin, p.origins, p.originPats, origin) + if p.dynamic || (allowed && value != "*") { + addVary(h, "Origin") + } + if !allowed { + return false + } + h.Set("Access-Control-Allow-Origin", value) + if p.allowAnyMethod { + h.Set("Access-Control-Allow-Methods", "*") + } else if p.methods != "" { + h.Set("Access-Control-Allow-Methods", p.methods) + } + if p.allowAnyHeader { + h.Set("Access-Control-Allow-Headers", "*") + } else if p.headers != "" { + h.Set("Access-Control-Allow-Headers", p.headers) + } + if p.exposed != "" { + h.Set("Access-Control-Expose-Headers", p.exposed) + } + if p.cfg.MaxAge > 0 { + h.Set("Access-Control-Max-Age", strconv.Itoa(p.cfg.MaxAge)) + } + if p.cfg.SupportsCredentials { + h.Set("Access-Control-Allow-Credentials", "true") + } + return true +} + +// preflight answers an OPTIONS request from the policy. +func (p *corsPolicy) preflight(w http.ResponseWriter, r *http.Request) { + allowed := p.apply(w.Header(), r.Header.Get("Origin")) + writeOptions(w, r, allowed, p.allowAnyMethod, p.allowAnyHeader) +} + +// addVary adds value to the Vary header unless a Vary line already names it. +func addVary(h http.Header, value string) { + for _, line := range h.Values("Vary") { + for _, v := range strings.Split(line, ",") { + if strings.EqualFold(strings.TrimSpace(v), value) { + return + } + } + } + h.Add("Vary", value) +} + func pathScopedCORS(cfg CORSConfig, next http.Handler) http.Handler { globs := make([]*regexp.Regexp, 0, len(cfg.Paths)) for _, p := range cfg.Paths { @@ -41,62 +197,17 @@ func pathScopedCORS(cfg CORSConfig, next http.Handler) http.Handler { globs = append(globs, re) } } - originPats := make([]*regexp.Regexp, 0, len(cfg.AllowedOriginsPatterns)) - for _, p := range cfg.AllowedOriginsPatterns { - re, err := regexp.Compile(p) - if err != nil { - continue - } - originPats = append(originPats, re) - } - allowAnyOrigin := containsStar(cfg.AllowedOrigins) - allowAnyMethod := containsStar(cfg.AllowedMethods) - allowAnyHeader := containsStar(cfg.AllowedHeaders) - origins := make(map[string]struct{}, len(cfg.AllowedOrigins)) - for _, o := range cfg.AllowedOrigins { - if o != "" && o != "*" { - origins[o] = struct{}{} - } - } - methods := strings.Join(cfg.AllowedMethods, ", ") - headers := strings.Join(cfg.AllowedHeaders, ", ") - exposed := strings.Join(cfg.ExposedHeaders, ", ") + policy := newCORSPolicy(cfg) return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if !pathMatchesCORS(globs, r.URL.Path) { next.ServeHTTP(w, r) return } - origin := r.Header.Get("Origin") - allowed, value := corsAllowOrigin(allowAnyOrigin, origins, originPats, origin) - if allowed { - w.Header().Set("Access-Control-Allow-Origin", value) - if value != "*" { - w.Header().Set("Vary", "Origin") - } - if allowAnyMethod { - w.Header().Set("Access-Control-Allow-Methods", "*") - } else if methods != "" { - w.Header().Set("Access-Control-Allow-Methods", methods) - } - if allowAnyHeader { - w.Header().Set("Access-Control-Allow-Headers", "*") - } else if headers != "" { - w.Header().Set("Access-Control-Allow-Headers", headers) - } - if exposed != "" { - w.Header().Set("Access-Control-Expose-Headers", exposed) - } - if cfg.MaxAge > 0 { - w.Header().Set("Access-Control-Max-Age", strconv.Itoa(cfg.MaxAge)) - } - if cfg.SupportsCredentials { - w.Header().Set("Access-Control-Allow-Credentials", "true") - } - } if r.Method == http.MethodOptions { - writeOptions(w, r, allowed, allowAnyMethod, allowAnyHeader) + policy.preflight(w, r) return } + policy.apply(w.Header(), r.Header.Get("Origin")) next.ServeHTTP(w, r) }) } diff --git a/modules/surf/transport_smoke_test.go b/modules/surf/transport_smoke_test.go new file mode 100644 index 0000000..8991fa7 --- /dev/null +++ b/modules/surf/transport_smoke_test.go @@ -0,0 +1,184 @@ +package surf + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "git.golem15.com/golem15/summercms/modules/backpack" + "git.golem15.com/golem15/summercms/modules/bouncer" + "git.golem15.com/golem15/summercms/modules/compass" + "git.golem15.com/golem15/summercms/modules/pact" + "git.golem15.com/golem15/summercms/modules/party" +) + +// Smoke tests for plan 02 of phase 19.3 (cookie transport, credentialed CORS, +// CORS profiles, Turnstile, api-docs); plan 05 owns full coverage. + +const transportBodyLimits = `body_limits: + default_bytes: 1048576 + upload_bytes: 1048576 +` + +// transportApp builds an app from config files (name -> YAML body) in +// environment env. +func transportApp(t *testing.T, env string, files map[string]string) *backpack.App { + t.Helper() + dir := t.TempDir() + if _, ok := files["app.yaml"]; !ok { + files["app.yaml"] = "name: transport-test\n" + } + for name, body := range files { + if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + cfg, err := compass.Open(compass.Options{Dir: dir, Environ: []string{"SUMMER_ENV=" + env}}) + if err != nil { + t.Fatal(err) + } + return backpack.New(cfg) +} + +// routesPluginFunc is a test plugin whose routes come from a func. +type routesPluginFunc struct { + id string + routes func(pact.Router) +} + +func (p routesPluginFunc) ID() string { return p.id } +func (p routesPluginFunc) Requires() []string { return nil } +func (p routesPluginFunc) Register(*backpack.App) error { return nil } +func (p routesPluginFunc) Boot(*backpack.App) error { return nil } +func (p routesPluginFunc) Routes(r pact.Router) error { + p.routes(r) + return nil +} + +func varyHas(h http.Header, value string) bool { + for _, line := range h.Values("Vary") { + for _, v := range strings.Split(line, ",") { + if strings.EqualFold(strings.TrimSpace(v), value) { + return true + } + } + } + return false +} + +func TestCookieRefreshAcrossSubdomainsSmoke(t *testing.T) { + const secret = "transport-smoke-secret-with-32-bytes-at-least" + const issuer = "https://api.example.com/api/auth/login" + app := transportApp(t, "development", map[string]string{ + "app.yaml": "name: transport-test\nurl: https://api.example.com\n", + "auth.yaml": "cookie:\n domain: example.com\n", + "http.yaml": transportBodyLimits + `cors: + paths: ["api/*"] + allowed_origins: ["https://www.example.com"] + allowed_methods: ["*"] + allowed_headers: ["*"] + supports_credentials: true +`, + }) + cookieCfg, err := bouncer.LoadCookieConfig(app.Config) + if err != nil { + t.Fatal(err) + } + refreshTTL := 20160 * time.Minute + bl := bouncer.NewMemoryBlacklist() + refresh, err := bouncer.CookieRefreshHandler(bouncer.CookieRefreshOptions{ + Cookie: cookieCfg, + RefreshTTL: refreshTTL, + Refresh: func(_ context.Context, token string) (string, error) { + return bouncer.Refresh(secret, token, refreshTTL, bl, 0, issuer) + }, + Write: func(w http.ResponseWriter, _ *http.Request, res bouncer.CookieResult) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(res.Status) + _ = json.NewEncoder(w).Encode(map[string]any{"ok": res.Err == nil}) + }, + }) + if err != nil { + t.Fatal(err) + } + plugin := routesPluginFunc{id: "acme.auth", routes: func(r pact.Router) { + r.Post("/api/auth/refresh", refresh.ServeHTTP) + }} + h, err := Assemble(app, []party.Plugin{plugin}) + if err != nil { + t.Fatal(err) + } + + old, oldJTI, err := bouncer.Mint(secret, "42", issuer, time.Hour) + if err != nil { + t.Fatal(err) + } + req := httptest.NewRequest(http.MethodPost, "https://api.example.com/api/auth/refresh", nil) + req.Header.Set("Origin", "https://www.example.com") + req.AddCookie(&http.Cookie{Name: "token", Value: old}) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("refresh status = %d body %s", rec.Code, rec.Body.String()) + } + setCookie := rec.Header().Get("Set-Cookie") + c, err := http.ParseSetCookie(setCookie) + if err != nil { + t.Fatalf("Set-Cookie %q: %v", setCookie, err) + } + if c.Name != "token" || c.Value == "" || c.Value == old { + t.Fatalf("cookie %s=%q, want a rotated token", c.Name, c.Value) + } + for _, want := range []string{"Domain=example.com", "Path=/", "Max-Age=1209600", "HttpOnly", "Secure", "SameSite=Lax"} { + if !strings.Contains(setCookie, want) { + t.Errorf("Set-Cookie %q lacks %s", setCookie, want) + } + } + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "https://www.example.com" { + t.Errorf("Allow-Origin = %q", got) + } + if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "true" { + t.Errorf("Allow-Credentials = %q", got) + } + if !varyHas(rec.Header(), "Origin") { + t.Errorf("Vary = %q, want Origin", rec.Header().Values("Vary")) + } + if revoked, _ := bl.IsBlacklisted(context.Background(), oldJTI); !revoked { + t.Error("the old jti is not blacklisted") + } + + pre := httptest.NewRequest(http.MethodOptions, "https://api.example.com/api/auth/refresh", nil) + pre.Header.Set("Origin", "https://www.example.com") + pre.Header.Set("Access-Control-Request-Method", "POST") + rec = httptest.NewRecorder() + h.ServeHTTP(rec, pre) + if rec.Code != http.StatusNoContent { + t.Fatalf("preflight status = %d", rec.Code) + } + if !varyHas(rec.Header(), "Origin") { + t.Errorf("preflight Vary = %q, want Origin", rec.Header().Values("Vary")) + } + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "https://www.example.com" { + t.Errorf("preflight Allow-Origin = %q", got) + } +} + +func TestCORSCredentialsWildcardRefusedSmoke(t *testing.T) { + app := transportApp(t, "development", map[string]string{ + "http.yaml": transportBodyLimits + `cors: + paths: ["api/*"] + allowed_origins: ["*"] + supports_credentials: true +`, + }) + _, err := BuildRouter(app, nil) + if err == nil || !strings.Contains(err.Error(), "http.cors") { + t.Fatalf("BuildRouter error = %v, want one naming http.cors", err) + } +}