Skip to content

Commit 30ee4fe

Browse files
torcolvintorcolvin
authored
CBG-4962 create cookie with SameSite=None if CORS enabled (#7841)
1 parent e412df5 commit 30ee4fe

File tree

10 files changed

+217
-61
lines changed

10 files changed

+217
-61
lines changed

‎auth/session.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ func (auth *Authenticator) GetSession(sessionID string) (*LoginSession, error) {
135135
return &session, nil
136136
}
137137

138-
func (auth *Authenticator) MakeSessionCookie(session *LoginSession, secureCookie bool, httpOnly bool) *http.Cookie {
138+
func (auth *Authenticator) MakeSessionCookie(session *LoginSession, secureCookie bool, httpOnly bool, sameSite http.SameSite) *http.Cookie {
139139
if session == nil {
140140
return nil
141141
}
@@ -145,6 +145,8 @@ func (auth *Authenticator) MakeSessionCookie(session *LoginSession, secureCookie
145145
Expires: session.Expiration,
146146
Secure: secureCookie,
147147
HttpOnly: httpOnly,
148+
// as of go 1.25, http.SameSiteDefaultMode will omit SameSite attribute from the cookie
149+
SameSite: sameSite,
148150
}
149151
}
150152

‎auth/session_test.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -116,14 +116,14 @@ func TestMakeSessionCookie(t *testing.T) {
116116
Ttl: 24 * time.Hour,
117117
}
118118

119-
cookie := auth.MakeSessionCookie(mockSession, false, false)
119+
cookie := auth.MakeSessionCookie(mockSession, false, false, http.SameSiteDefaultMode)
120120
assert.Equal(t, DefaultCookieName, cookie.Name)
121121
assert.Equal(t, sessionID, cookie.Value)
122122
assert.NotEmpty(t, cookie.Expires)
123123

124124
// Cookies should not be created with uninitialized session
125125
mockSession = nil
126-
cookie = auth.MakeSessionCookie(mockSession, false, false)
126+
cookie = auth.MakeSessionCookie(mockSession, false, false, http.SameSiteDefaultMode)
127127
assert.Empty(t, cookie)
128128
}
129129

@@ -143,16 +143,16 @@ func TestMakeSessionCookieProperties(t *testing.T) {
143143
Ttl: 24 * time.Hour,
144144
}
145145

146-
unsecuredCookie := auth.MakeSessionCookie(mockSession, false, false)
146+
unsecuredCookie := auth.MakeSessionCookie(mockSession, false, false, http.SameSiteDefaultMode)
147147
assert.False(t, unsecuredCookie.Secure)
148148

149-
securedCookie := auth.MakeSessionCookie(mockSession, true, false)
149+
securedCookie := auth.MakeSessionCookie(mockSession, true, false, http.SameSiteDefaultMode)
150150
assert.True(t, securedCookie.Secure)
151151

152-
httpOnlyFalseCookie := auth.MakeSessionCookie(mockSession, false, false)
152+
httpOnlyFalseCookie := auth.MakeSessionCookie(mockSession, false, false, http.SameSiteDefaultMode)
153153
assert.False(t, httpOnlyFalseCookie.HttpOnly)
154154

155-
httpOnlyCookie := auth.MakeSessionCookie(mockSession, false, true)
155+
httpOnlyCookie := auth.MakeSessionCookie(mockSession, false, true, http.SameSiteDefaultMode)
156156
assert.True(t, httpOnlyCookie.HttpOnly)
157157
}
158158

@@ -248,7 +248,7 @@ func TestCreateSessionChangePassword(t *testing.T) {
248248

249249
request, err := http.NewRequest(http.MethodGet, "", nil)
250250
require.NoError(t, err)
251-
request.AddCookie(auth.MakeSessionCookie(session, true, true))
251+
request.AddCookie(auth.MakeSessionCookie(session, true, true, http.SameSiteDefaultMode))
252252

253253
recorder := httptest.NewRecorder()
254254
_, err = auth.AuthenticateCookie(request, recorder)
@@ -304,7 +304,7 @@ func TestUserWithoutSessionUUID(t *testing.T) {
304304

305305
request, err := http.NewRequest(http.MethodGet, "", nil)
306306
require.NoError(t, err)
307-
request.AddCookie(auth.MakeSessionCookie(session, true, true))
307+
request.AddCookie(auth.MakeSessionCookie(session, true, true, http.SameSiteDefaultMode))
308308

309309
recorder := httptest.NewRecorder()
310310
_, err = auth.AuthenticateCookie(request, recorder)
@@ -334,7 +334,7 @@ func TestUserDeleteAllSessions(t *testing.T) {
334334

335335
request, err := http.NewRequest(http.MethodGet, "", nil)
336336
require.NoError(t, err)
337-
request.AddCookie(auth.MakeSessionCookie(session, true, true))
337+
request.AddCookie(auth.MakeSessionCookie(session, true, true, http.SameSiteDefaultMode))
338338
recorder := httptest.NewRecorder()
339339

340340
_, err = auth.AuthenticateCookie(request, recorder)

‎db/database.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ type DatabaseContext struct {
164164
CachedCCVStartingCas *base.VBucketCAS // If set, the cached value of the CCV starting CAS value to avoid repeated lookups
165165
CachedCCVEnabled atomic.Bool // If set, the cached value of the CCV Enabled flag (this is not expected to transition from true->false, but could go false->true)
166166
numVBuckets uint16 // Number of vbuckets in the bucket
167+
SameSiteCookieMode http.SameSite
167168
}
168169

169170
type Scope struct {
@@ -278,6 +279,7 @@ type UnsupportedOptions struct {
278279
KVBufferSize int `json:"kv_buffer,omitempty"` // Enables user to set their own KV pool buffer
279280
BlipSendDocsWithChannelRemoval bool `json:"blip_send_docs_with_channel_removal,omitempty"` // Enables sending docs with channel removals using channel filters
280281
RejectWritesWithSkippedSequences bool `json:"reject_writes_with_skipped_sequences,omitempty"` // Reject writes if there are skipped sequences in the database
282+
SameSiteCookie *string `json:"same_site_cookie,omitempty"` // Sets the SameSite attribute on session cookies.
281283
}
282284

283285
type WarningThresholds struct {
@@ -433,6 +435,7 @@ func NewDatabaseContext(ctx context.Context, dbName string, bucket base.Bucket,
433435
ServerUUID: serverUUID,
434436
UserFunctionTimeout: defaultUserFunctionTimeout,
435437
CachedCCVStartingCas: &base.VBucketCAS{},
438+
SameSiteCookieMode: http.SameSiteDefaultMode,
436439
}
437440
dbContext.numVBuckets, err = bucket.GetMaxVbno()
438441
if err != nil {
@@ -2585,3 +2588,23 @@ func PurgeDCPCheckpoints(ctx context.Context, database *DatabaseContext, checkpo
25852588
func (db *DatabaseContext) EnableAllowConflicts(tb testing.TB) {
25862589
db.Options.AllowConflicts = base.Ptr(true)
25872590
}
2591+
2592+
// GetSameSiteCookieMode returns the http.SameSite mode based on the unsupported database options. Returns an error if
2593+
// an invalid string is set.
2594+
func (o *UnsupportedOptions) GetSameSiteCookieMode() (http.SameSite, error) {
2595+
if o == nil || o.SameSiteCookie == nil {
2596+
return http.SameSiteDefaultMode, nil
2597+
}
2598+
switch *o.SameSiteCookie {
2599+
case "Lax":
2600+
return http.SameSiteLaxMode, nil
2601+
case "Strict":
2602+
return http.SameSiteStrictMode, nil
2603+
case "None":
2604+
return http.SameSiteNoneMode, nil
2605+
case "Default":
2606+
return http.SameSiteDefaultMode, nil
2607+
default:
2608+
return http.SameSiteDefaultMode, fmt.Errorf("unsupported_options.same_site_cookie option %q is not valid, choices are \"Lax\", \"Strict\", and \"None", *o.SameSiteCookie)
2609+
}
2610+
}

‎docs/api/components/schemas.yaml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1698,6 +1698,15 @@ Database:
16981698
remote_config_tls_skip_verify:
16991699
description: Enable self-signed certificates for external JavaScript load.
17001700
type: boolean
1701+
same_site_cookie:
1702+
description: |-
1703+
Override the session cookie SameSite behavior. By default, a session cookie will have SameSite:None if CORS is enabled, and will have no SameSite attribute if CORS is not enabled. Setting this property to`Default` will omit the SameSite attribute from the cookie.
1704+
type: string
1705+
enum:
1706+
- "Default"
1707+
- "Lax"
1708+
- "None"
1709+
- "Strict"
17011710
sgr_tls_skip_verify:
17021711
description: Enable self-signed certificates for SG-replicate testing.
17031712
type: boolean

‎rest/config.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,6 +1083,13 @@ func (dbConfig *DbConfig) validateVersion(ctx context.Context, isEnterpriseEditi
10831083
}
10841084
}
10851085

1086+
if dbConfig.Unsupported != nil {
1087+
_, err := dbConfig.Unsupported.GetSameSiteCookieMode()
1088+
if err != nil {
1089+
multiError = multiError.Append(err)
1090+
}
1091+
}
1092+
10861093
if validateReplications {
10871094
for name, r := range dbConfig.Replications {
10881095
if name == "" {

‎rest/config_test.go‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3378,3 +3378,62 @@ func TestConfigUserXattrKeyValidation(t *testing.T) {
33783378
})
33793379
}
33803380
}
3381+
3382+
func TestValidateUnsupportedSameSiteCookies(t *testing.T) {
3383+
tests := []struct {
3384+
name string
3385+
unsupportedSettings *db.UnsupportedOptions
3386+
error string
3387+
}{
3388+
{
3389+
name: "no unsupportedSettings present",
3390+
unsupportedSettings: &db.UnsupportedOptions{},
3391+
error: "",
3392+
},
3393+
{
3394+
name: "no samesite, unsupportedSettings present",
3395+
unsupportedSettings: &db.UnsupportedOptions{},
3396+
error: "",
3397+
},
3398+
{
3399+
name: "valid value Lax",
3400+
unsupportedSettings: &db.UnsupportedOptions{SameSiteCookie: base.Ptr("Lax")},
3401+
error: "",
3402+
},
3403+
{
3404+
name: "valid value Strict",
3405+
unsupportedSettings: &db.UnsupportedOptions{SameSiteCookie: base.Ptr("Strict")},
3406+
error: "",
3407+
},
3408+
{
3409+
name: "valid value None",
3410+
unsupportedSettings: &db.UnsupportedOptions{SameSiteCookie: base.Ptr("None")},
3411+
error: "",
3412+
},
3413+
{
3414+
name: "invalid value",
3415+
unsupportedSettings: &db.UnsupportedOptions{SameSiteCookie: base.Ptr("invalid value")},
3416+
error: "unsupported_options.same_site_cookie option",
3417+
},
3418+
}
3419+
3420+
for _, test := range tests {
3421+
t.Run(test.name, func(t *testing.T) {
3422+
dbConfig := DbConfig{
3423+
Name: "db",
3424+
Unsupported: test.unsupportedSettings,
3425+
}
3426+
3427+
validateReplications := false
3428+
validateOIDC := false
3429+
ctx := base.TestCtx(t)
3430+
err := dbConfig.validate(ctx, validateOIDC, validateReplications)
3431+
if test.error != "" {
3432+
require.Error(t, err)
3433+
require.Contains(t, err.Error(), test.error)
3434+
} else {
3435+
require.NoError(t, err)
3436+
}
3437+
})
3438+
}
3439+
}

‎rest/cors_test.go‎

Lines changed: 78 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717

1818
"github.com/couchbase/sync_gateway/auth"
1919
"github.com/couchbase/sync_gateway/base"
20+
"github.com/couchbase/sync_gateway/db"
2021
"github.com/stretchr/testify/assert"
2122
"github.com/stretchr/testify/require"
2223
)
@@ -409,66 +410,93 @@ func TestCORSLoginOriginPerDatabase(t *testing.T) {
409410
// Override the default (example.com) CORS configuration in the DbConfig for /db:
410411
rt := NewRestTesterPersistentConfigNoDB(t)
411412
defer rt.Close()
412-
dbConfig := rt.NewDbConfig()
413-
dbConfig.CORS = &auth.CORSConfig{
414-
Origin: []string{"http://couchbase.com", "http://staging.couchbase.com"},
415-
LoginOrigin: []string{"http://couchbase.com"},
416-
Headers: []string{},
417-
}
418-
RequireStatus(t, rt.CreateDatabase("dbloginorigin", dbConfig), http.StatusCreated)
419-
420-
const username = "alice"
421-
rt.CreateUser(username, nil)
422-
423413
testCases := []struct {
424-
name string
425-
origin string
426-
responseCode int
427-
responseErrorBody string
414+
name string
415+
unsupportedOptions *db.UnsupportedOptions
416+
sameSite http.SameSite
428417
}{
429418
{
430-
name: "CORS login origin allowed couchbase",
431-
origin: "http://couchbase.com",
432-
responseCode: http.StatusOK,
419+
name: "No unsupported options",
420+
unsupportedOptions: nil,
421+
sameSite: http.SameSiteNoneMode,
433422
},
434423
{
435-
name: "CORS login origin not allowed staging",
436-
origin: "http://staging.couchbase.com",
437-
responseCode: http.StatusBadRequest,
438-
responseErrorBody: "No CORS",
424+
name: "With unsupported options",
425+
unsupportedOptions: &db.UnsupportedOptions{
426+
SameSiteCookie: base.Ptr("Strict"),
427+
},
428+
sameSite: http.SameSiteStrictMode,
439429
},
440430
}
441-
for _, test := range testCases {
442-
rt.Run(test.name, func(t *testing.T) {
443-
reqHeaders := map[string]string{
444-
"Origin": test.origin,
445-
"Authorization": GetBasicAuthHeader(t, username, RestTesterDefaultUserPassword),
446-
}
447-
resp := rt.SendRequestWithHeaders(http.MethodPost, "/{{.db}}/_session", "", reqHeaders)
448-
RequireStatus(t, resp, test.responseCode)
449-
if test.responseErrorBody != "" {
450-
require.Contains(t, resp.Body.String(), test.responseErrorBody)
451-
// the access control headers are returned based on Origin and not LoginOrigin which could be considered a bug
452-
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
453-
} else {
454-
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
431+
for _, dbTestCases := range testCases {
432+
rt.Run(dbTestCases.name, func(t *testing.T) {
433+
// Override the default (example.com) CORS configuration in the DbConfig for /db:
434+
rt := NewRestTesterPersistentConfigNoDB(t)
435+
defer rt.Close()
436+
437+
dbConfig := rt.NewDbConfig()
438+
dbConfig.Unsupported = dbTestCases.unsupportedOptions
439+
dbConfig.CORS = &auth.CORSConfig{
440+
Origin: []string{"http://couchbase.com", "http://staging.couchbase.com"},
441+
LoginOrigin: []string{"http://couchbase.com"},
442+
Headers: []string{},
455443
}
456-
if test.responseCode == http.StatusOK {
457-
cookie, err := http.ParseSetCookie(resp.Header().Get("Set-Cookie"))
458-
require.NoError(t, err)
459-
require.NotEmpty(t, cookie.Path)
460-
reqHeaders["Cookie"] = fmt.Sprintf("%s=%s", cookie.Name, cookie.Value)
461-
}
462-
resp = rt.SendRequestWithHeaders(http.MethodDelete, "/{{.db}}/_session", "", reqHeaders)
463-
RequireStatus(t, resp, test.responseCode)
464-
if test.responseErrorBody != "" {
465-
require.Contains(t, resp.Body.String(), test.responseErrorBody)
466-
// the access control headers are returned based on Origin and not LoginOrigin which could be considered a bug
467-
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
468-
} else {
469-
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
444+
RequireStatus(t, rt.CreateDatabase(SafeDatabaseName(t, dbTestCases.name), dbConfig), http.StatusCreated)
445+
const username = "alice"
446+
rt.CreateUser(username, nil)
447+
448+
testCases := []struct {
449+
name string
450+
origin string
451+
responseCode int
452+
responseErrorBody string
453+
}{
454+
{
455+
name: "CORS login origin allowed couchbase",
456+
origin: "http://couchbase.com",
457+
responseCode: http.StatusOK,
458+
},
459+
{
460+
name: "CORS login origin not allowed staging",
461+
origin: "http://staging.couchbase.com",
462+
responseCode: http.StatusBadRequest,
463+
responseErrorBody: "No CORS",
464+
},
470465
}
466+
for _, test := range testCases {
467+
rt.Run(test.name, func(t *testing.T) {
468+
reqHeaders := map[string]string{
469+
"Origin": test.origin,
470+
"Authorization": GetBasicAuthHeader(t, username, RestTesterDefaultUserPassword),
471+
}
472+
resp := rt.SendRequestWithHeaders(http.MethodPost, "/{{.db}}/_session", "", reqHeaders)
473+
RequireStatus(t, resp, test.responseCode)
474+
if test.responseErrorBody != "" {
475+
require.Contains(t, resp.Body.String(), test.responseErrorBody)
476+
// the access control headers are returned based on Origin and not LoginOrigin which could be considered a bug
477+
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
478+
} else {
479+
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
480+
}
481+
if test.responseCode == http.StatusOK {
482+
cookie, err := http.ParseSetCookie(resp.Header().Get("Set-Cookie"))
483+
require.NoError(t, err)
484+
require.NotEmpty(t, cookie.Path)
485+
require.Equal(t, dbTestCases.sameSite, cookie.SameSite)
486+
reqHeaders["Cookie"] = fmt.Sprintf("%s=%s", cookie.Name, cookie.Value)
487+
}
488+
resp = rt.SendRequestWithHeaders(http.MethodDelete, "/{{.db}}/_session", "", reqHeaders)
489+
RequireStatus(t, resp, test.responseCode)
490+
if test.responseErrorBody != "" {
491+
require.Contains(t, resp.Body.String(), test.responseErrorBody)
492+
// the access control headers are returned based on Origin and not LoginOrigin which could be considered a bug
493+
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
494+
} else {
495+
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
496+
}
471497

498+
})
499+
}
472500
})
473501
}
474502
}

‎rest/server_context.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,16 @@ func (sc *ServerContext) _getOrAddDatabaseFromConfig(ctx context.Context, config
947947
} else {
948948
dbcontext.CORS = sc.Config.API.CORS
949949
}
950+
if !dbcontext.CORS.IsEmpty() {
951+
dbcontext.SameSiteCookieMode = http.SameSiteNoneMode
952+
}
953+
if config.Unsupported != nil && config.Unsupported.SameSiteCookie != nil {
954+
var err error
955+
dbcontext.SameSiteCookieMode, err = config.Unsupported.GetSameSiteCookieMode()
956+
if err != nil {
957+
return nil, err
958+
}
959+
}
950960

951961
if config.RevsLimit != nil {
952962
dbcontext.RevsLimit = *config.RevsLimit

0 commit comments

Comments
 (0)