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 <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
This commit is contained in:
committed by
Bryan Frimin
parent
cd6c46212a
commit
62d0ab68c4
@@ -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 ──────────────────────────────────────────────────────────
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
)
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
121
pkg/probodconfig/auth_config_test.go
Normal file
121
pkg/probodconfig/auth_config_test.go
Normal file
@@ -0,0 +1,121 @@
|
||||
// Copyright (c) 2026 Probo Inc <hello@probo.com>.
|
||||
//
|
||||
// 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)
|
||||
}
|
||||
Reference in New Issue
Block a user