Skip to content

Commit 01df412

Browse files
torcolvintorcolvin
authored
[4.0.0.2 backport] CBG-4965 create cookie with SameSite=None if CORS enabled (#7844)
1 parent 3e99b8d commit 01df412

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 {
@@ -2588,3 +2591,23 @@ func PurgeDCPCheckpoints(ctx context.Context, database *DatabaseContext, checkpo
25882591
func (db *DatabaseContext) EnableAllowConflicts(tb testing.TB) {
25892592
db.Options.AllowConflicts = base.Ptr(true)
25902593
}
2594+
2595+
// GetSameSiteCookieMode returns the http.SameSite mode based on the unsupported database options. Returns an error if
2596+
// an invalid string is set.
2597+
func (o *UnsupportedOptions) GetSameSiteCookieMode() (http.SameSite, error) {
2598+
if o == nil || o.SameSiteCookie == nil {
2599+
return http.SameSiteDefaultMode, nil
2600+
}
2601+
switch *o.SameSiteCookie {
2602+
case "Lax":
2603+
return http.SameSiteLaxMode, nil
2604+
case "Strict":
2605+
return http.SameSiteStrictMode, nil
2606+
case "None":
2607+
return http.SameSiteNoneMode, nil
2608+
case "Default":
2609+
return http.SameSiteDefaultMode, nil
2610+
default:
2611+
return http.SameSiteDefaultMode, fmt.Errorf("unsupported_options.same_site_cookie option %q is not valid, choices are \"Lax\", \"Strict\", and \"None", *o.SameSiteCookie)
2612+
}
2613+
}

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

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

496+
})
497+
}
470498
})
471499
}
472500
}

‎rest/server_context.go‎

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

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

0 commit comments

Comments
 (0)