From 62d0ab68c48ee58e0101623d474a5a54ea8b3399 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 29 Jul 2026 16:37:28 +0000 Subject: [PATCH] Make auth cookie SameSite configurable Add same-site to auth cookie config with lax as the default, PROBOD_AUTH_COOKIE_SAMESITE bootstrap mapping, and validation that rejects none unless Secure is enabled. Signed-off-by: Cursor Agent Co-authored-by: Bryan FRIMIN --- .env.example | 1 + .../charts/probo/templates/deployment.yaml | 2 + contrib/helm/charts/probo/values.yaml | 2 + pkg/bootstrap/builder.go | 12 ++ pkg/bootstrap/builder_test.go | 29 +++++ pkg/probod/aliases.go | 7 + pkg/probod/probod.go | 55 +++++--- pkg/probodconfig/auth_config.go | 63 ++++++++- pkg/probodconfig/auth_config_test.go | 121 ++++++++++++++++++ 9 files changed, 266 insertions(+), 26 deletions(-) create mode 100644 pkg/probodconfig/auth_config_test.go diff --git a/.env.example b/.env.example index 9e86082b4..624e9cafe 100644 --- a/.env.example +++ b/.env.example @@ -47,6 +47,7 @@ # ── Cookie ──────────────────────────────────────────────────────────── # PROBOD_AUTH_COOKIE_DOMAIN=localhost # PROBOD_AUTH_COOKIE_SECURE=false +# PROBOD_AUTH_COOKIE_SAMESITE=lax # PROBOD_AUTH_COOKIE_DURATION=24 # ── Postgres ────────────────────────────────────────────────────────── diff --git a/contrib/helm/charts/probo/templates/deployment.yaml b/contrib/helm/charts/probo/templates/deployment.yaml index edc800bd3..eb7e7d374 100644 --- a/contrib/helm/charts/probo/templates/deployment.yaml +++ b/contrib/helm/charts/probo/templates/deployment.yaml @@ -135,6 +135,8 @@ spec: key: cookie-secret - name: PROBOD_AUTH_COOKIE_DURATION value: {{ .Values.probo.auth.cookieDuration | quote }} + - name: PROBOD_AUTH_COOKIE_SAMESITE + value: {{ .Values.probo.auth.cookieSameSite | quote }} - name: PROBOD_AUTH_PASSWORD_PEPPER valueFrom: secretKeyRef: diff --git a/contrib/helm/charts/probo/values.yaml b/contrib/helm/charts/probo/values.yaml index f55f26d3a..6279cfcac 100644 --- a/contrib/helm/charts/probo/values.yaml +++ b/contrib/helm/charts/probo/values.yaml @@ -229,6 +229,8 @@ probo: emailConfirmationTokenValidity: 3600 cookieName: "SSID" cookieDomain: "probo.example.com" + # lax, strict, or none (none requires HTTPS / secure cookies) + cookieSameSite: "lax" # REQUIRED: Generate with openssl rand -base64 32 cookieSecret: "" cookieDuration: 24 diff --git a/pkg/bootstrap/builder.go b/pkg/bootstrap/builder.go index 18fe63918..7fa829e4f 100644 --- a/pkg/bootstrap/builder.go +++ b/pkg/bootstrap/builder.go @@ -59,6 +59,13 @@ func (b *Builder) Build() (*probodconfig.FullConfig, error) { pgCACertBundle := b.getPgCACertBundle() + authCookieSameSite, err := probodconfig.ParseCookieSameSite( + b.resolver.getEnvOrDefault("PROBOD_AUTH_COOKIE_SAMESITE", "lax"), + ) + if err != nil { + return nil, fmt.Errorf("cannot parse PROBOD_AUTH_COOKIE_SAMESITE: %w", err) + } + cfg := &probodconfig.FullConfig{ Unit: probodconfig.UnitConfig{ Metrics: probodconfig.MetricsConfig{ @@ -118,6 +125,7 @@ func (b *Builder) Build() (*probodconfig.FullConfig, error) { Secret: b.resolver.getEnv("PROBOD_AUTH_COOKIE_SECRET"), Duration: b.resolver.getEnvIntOrDefault("PROBOD_AUTH_COOKIE_DURATION", 24), Secure: b.resolver.getEnvBoolOrDefault("PROBOD_AUTH_COOKIE_SECURE", true), + SameSite: authCookieSameSite, }, Password: probodconfig.PasswordConfig{ Pepper: b.resolver.getEnv("PROBOD_AUTH_PASSWORD_PEPPER"), @@ -559,6 +567,10 @@ func (b *Builder) Build() (*probodconfig.FullConfig, error) { return nil, b.resolver.Err() } + if err := cfg.Probod.Auth.Cookie.Validate(); err != nil { + return nil, fmt.Errorf("cannot validate auth cookie config: %w", err) + } + return cfg, nil } diff --git a/pkg/bootstrap/builder_test.go b/pkg/bootstrap/builder_test.go index a6c65a102..17cd47cb1 100644 --- a/pkg/bootstrap/builder_test.go +++ b/pkg/bootstrap/builder_test.go @@ -335,6 +335,7 @@ func TestBuilder_Build_CustomValues(t *testing.T) { env["PROBOD_AUTH_EMAIL_CONFIRMATION_TOKEN_VALIDITY"] = "43200" env["PROBOD_AUTH_COOKIE_DOMAIN"] = ".example.com" env["PROBOD_AUTH_COOKIE_DURATION"] = "48" + env["PROBOD_AUTH_COOKIE_SAMESITE"] = "strict" // SAML env["PROBOD_SAML_DOMAIN_VERIFICATION_INTERVAL_SECONDS"] = "120" env["PROBOD_SAML_DOMAIN_VERIFICATION_RESOLVER_ADDR"] = "1.1.1.1:53" @@ -471,6 +472,7 @@ func TestBuilder_Build_CustomValues(t *testing.T) { assert.Equal(t, 43200, cfg.Probod.Auth.EmailConfirmationTokenValidity) assert.Equal(t, ".example.com", cfg.Probod.Auth.Cookie.Domain) assert.Equal(t, 48, cfg.Probod.Auth.Cookie.Duration) + assert.Equal(t, probodconfig.CookieSameSiteStrict, cfg.Probod.Auth.Cookie.SameSite) // SAML assert.Equal(t, 120, cfg.Probod.Auth.SAML.DomainVerificationIntervalSeconds) assert.Equal(t, "1.1.1.1:53", cfg.Probod.Auth.SAML.DomainVerificationResolverAddr) @@ -896,6 +898,33 @@ func TestBuilder_Build_PgCABundleFromFile(t *testing.T) { assert.Equal(t, "ca-bundle-from-file", cfg.Probod.Pg.CACertBundle) } +func TestBuilder_Build_AuthCookieSameSiteInvalid(t *testing.T) { + env := requiredEnv() + env["PROBOD_AUTH_COOKIE_SAMESITE"] = "invalid" + + b := NewBuilder(NewResolver(mockEnv(env))) + b.samlCertificate = "test-cert" + b.samlPrivateKey = "test-key" + + _, err := b.Build() + require.Error(t, err) + assert.Contains(t, err.Error(), "PROBOD_AUTH_COOKIE_SAMESITE") +} + +func TestBuilder_Build_AuthCookieSameSiteNoneRequiresSecure(t *testing.T) { + env := requiredEnv() + env["PROBOD_AUTH_COOKIE_SAMESITE"] = "none" + env["PROBOD_AUTH_COOKIE_SECURE"] = "false" + + b := NewBuilder(NewResolver(mockEnv(env))) + b.samlCertificate = "test-cert" + b.samlPrivateKey = "test-key" + + _, err := b.Build() + require.Error(t, err) + assert.Contains(t, err.Error(), "secure") +} + func TestBuilder_parseOriginsList(t *testing.T) { tests := []struct { name string diff --git a/pkg/probod/aliases.go b/pkg/probod/aliases.go index 14b784d1c..dc5e952d4 100644 --- a/pkg/probod/aliases.go +++ b/pkg/probod/aliases.go @@ -38,6 +38,7 @@ type ( OAuth2ServerConfig = probodconfig.OAuth2ServerConfig OAuth2SigningKeyConfig = probodconfig.OAuth2SigningKeyConfig CookieConfig = probodconfig.CookieConfig + CookieSameSite = probodconfig.CookieSameSite PasswordConfig = probodconfig.PasswordConfig AWSConfig = probodconfig.AWSConfig ConnectorConfig = probodconfig.ConnectorConfig @@ -67,3 +68,9 @@ type ( ITAMConfig = probodconfig.ITAMConfig SlackConfig = probodconfig.SlackConfig ) + +const ( + CookieSameSiteLax = probodconfig.CookieSameSiteLax + CookieSameSiteStrict = probodconfig.CookieSameSiteStrict + CookieSameSiteNone = probodconfig.CookieSameSiteNone +) diff --git a/pkg/probod/probod.go b/pkg/probod/probod.go index 538ff35ca..f838b81ef 100644 --- a/pkg/probod/probod.go +++ b/pkg/probod/probod.go @@ -134,6 +134,7 @@ func New() *Implm { Duration: 24, Domain: "localhost", Secure: true, + SameSite: CookieSameSiteLax, }, DisableSignup: false, InvitationConfirmationTokenValidity: 3600, @@ -280,6 +281,18 @@ func (impl *Implm) Run( return fmt.Errorf("cannot get cookie secret bytes: %w", err) } + if err := impl.cfg.Auth.Cookie.Validate(); err != nil { + rootSpan.RecordError(err) + return fmt.Errorf("cannot validate auth cookie config: %w", err) + } + + authCookieMaxAge := int(time.Duration(impl.cfg.Auth.Cookie.Duration) * time.Hour) + authCookie, err := authSecureCookieConfig(impl.cfg.Auth.Cookie, authCookieMaxAge) + if err != nil { + rootSpan.RecordError(err) + return fmt.Errorf("cannot configure auth cookie: %w", err) + } + awsConfig, err := awsconfig.NewConfig( l, httpclient.DefaultPooledClient( @@ -768,16 +781,7 @@ func (impl *Implm) Run( CustomDomainCname: impl.cfg.CustomDomains.CnameTarget, TokenSecret: impl.cfg.Auth.Cookie.Secret, Logger: l.Named("http.server"), - Cookie: securecookie.Config{ - Name: impl.cfg.Auth.Cookie.Name, - Domain: impl.cfg.Auth.Cookie.Domain, - Path: "/", - MaxAge: int(time.Duration(impl.cfg.Auth.Cookie.Duration) * time.Hour), - Secret: impl.cfg.Auth.Cookie.Secret, - Secure: impl.cfg.Auth.Cookie.Secure, - HTTPOnly: true, - SameSite: http.SameSiteLaxMode, - }, + Cookie: authCookie, }, ) if err != nil { @@ -796,17 +800,8 @@ func (impl *Implm) Run( File: fileManagerService, ESign: esignService, Mailman: mailmanService, - Cookie: securecookie.Config{ - Name: impl.cfg.Auth.Cookie.Name, - Domain: impl.cfg.Auth.Cookie.Domain, - Path: "/", - MaxAge: int(time.Duration(impl.cfg.Auth.Cookie.Duration) * time.Hour), - Secret: impl.cfg.Auth.Cookie.Secret, - Secure: impl.cfg.Auth.Cookie.Secure, - HTTPOnly: true, - SameSite: http.SameSiteLaxMode, - }, - TokenSecret: impl.cfg.Auth.Cookie.Secret, + Cookie: authCookie, + TokenSecret: impl.cfg.Auth.Cookie.Secret, GraphQLLimits: gqlutils.Limits{ ParserTokenLimit: impl.cfg.Api.GraphQL.ParserTokenLimit, ComplexityLimit: impl.cfg.Api.GraphQL.ComplexityLimit, @@ -1604,3 +1599,21 @@ func oauth2ServerOptions(cfg OAuth2ServerConfig) []oauth2.Option { return opts } + +func authSecureCookieConfig(c CookieConfig, maxAgeSeconds int) (securecookie.Config, error) { + sameSite, err := c.HTTPSameSite() + if err != nil { + return securecookie.Config{}, err + } + + return securecookie.Config{ + Name: c.Name, + Domain: c.Domain, + Path: "/", + MaxAge: maxAgeSeconds, + Secret: c.Secret, + Secure: c.Secure, + HTTPOnly: true, + SameSite: sameSite, + }, nil +} diff --git a/pkg/probodconfig/auth_config.go b/pkg/probodconfig/auth_config.go index 344cd7819..a17c9d70d 100644 --- a/pkg/probodconfig/auth_config.go +++ b/pkg/probodconfig/auth_config.go @@ -23,8 +23,34 @@ package probodconfig import ( "encoding/base64" "fmt" + "net/http" + "strings" ) +type CookieSameSite string + +const ( + CookieSameSiteLax CookieSameSite = "lax" + CookieSameSiteStrict CookieSameSite = "strict" + CookieSameSiteNone CookieSameSite = "none" +) + +func ParseCookieSameSite(raw string) (CookieSameSite, error) { + switch strings.ToLower(strings.TrimSpace(raw)) { + case "", string(CookieSameSiteLax): + return CookieSameSiteLax, nil + case string(CookieSameSiteStrict): + return CookieSameSiteStrict, nil + case string(CookieSameSiteNone): + return CookieSameSiteNone, nil + default: + return "", fmt.Errorf( + "invalid same-site value %q: must be lax, strict, or none", + raw, + ) + } +} + type AuthConfig struct { Cookie CookieConfig `json:"cookie"` Password PasswordConfig `json:"password"` @@ -55,11 +81,38 @@ type OAuth2SigningKeyConfig struct { } type CookieConfig struct { - Domain string `json:"domain,omitempty"` - Secret string `json:"secret"` - Duration int `json:"duration"` - Name string `json:"name,omitempty"` - Secure bool `json:"secure"` + Domain string `json:"domain,omitempty"` + Secret string `json:"secret"` + Duration int `json:"duration"` + Name string `json:"name,omitempty"` + Secure bool `json:"secure"` + SameSite CookieSameSite `json:"same-site,omitempty"` +} + +func (c CookieConfig) HTTPSameSite() (http.SameSite, error) { + switch c.SameSite { + case "", CookieSameSiteLax: + return http.SameSiteLaxMode, nil + case CookieSameSiteStrict: + return http.SameSiteStrictMode, nil + case CookieSameSiteNone: + return http.SameSiteNoneMode, nil + default: + return 0, fmt.Errorf("invalid cookie same-site value %q", c.SameSite) + } +} + +func (c CookieConfig) Validate() error { + sameSite, err := c.HTTPSameSite() + if err != nil { + return err + } + + if sameSite == http.SameSiteNoneMode && !c.Secure { + return fmt.Errorf("cookie same-site none requires secure cookies") + } + + return nil } type PasswordConfig struct { diff --git a/pkg/probodconfig/auth_config_test.go b/pkg/probodconfig/auth_config_test.go new file mode 100644 index 000000000..ccb3b53b3 --- /dev/null +++ b/pkg/probodconfig/auth_config_test.go @@ -0,0 +1,121 @@ +// Copyright (c) 2026 Probo Inc . +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package probodconfig_test + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "go.probo.inc/probo/pkg/probodconfig" +) + +func TestParseCookieSameSite(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + want probodconfig.CookieSameSite + wantErr bool + }{ + {name: "empty", input: "", want: probodconfig.CookieSameSiteLax}, + {name: "lax", input: "lax", want: probodconfig.CookieSameSiteLax}, + {name: "Lax", input: "Lax", want: probodconfig.CookieSameSiteLax}, + {name: "strict", input: "strict", want: probodconfig.CookieSameSiteStrict}, + {name: "none", input: "none", want: probodconfig.CookieSameSiteNone}, + {name: "invalid", input: "cross-site", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, err := probodconfig.ParseCookieSameSite(tt.input) + if tt.wantErr { + require.Error(t, err) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestCookieConfig_Validate(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + cookie probodconfig.CookieConfig + wantErr bool + }{ + { + name: "lax without secure", + cookie: probodconfig.CookieConfig{ + SameSite: probodconfig.CookieSameSiteLax, + Secure: false, + }, + }, + { + name: "none requires secure", + cookie: probodconfig.CookieConfig{ + SameSite: probodconfig.CookieSameSiteNone, + Secure: false, + }, + wantErr: true, + }, + { + name: "none with secure", + cookie: probodconfig.CookieConfig{ + SameSite: probodconfig.CookieSameSiteNone, + Secure: true, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := tt.cookie.Validate() + if tt.wantErr { + require.Error(t, err) + return + } + + require.NoError(t, err) + }) + } +} + +func TestCookieConfig_HTTPSameSite(t *testing.T) { + t.Parallel() + + cookie := probodconfig.CookieConfig{SameSite: probodconfig.CookieSameSiteStrict} + + got, err := cookie.HTTPSameSite() + require.NoError(t, err) + assert.Equal(t, http.SameSiteStrictMode, got) +}