Skip to content

Commit 932f745

Browse files
torcolvintorcolvin
authored
[3.3.0.2 backport] CBG-4964 create cookie with SameSite=None if CORS enabled (#7843)
1 parent 4e01781 commit 932f745

File tree

11 files changed

+226
-61
lines changed

11 files changed

+226
-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
@@ -139,6 +139,7 @@ type DatabaseContext struct {
139139
WasInitializedSynchronously bool // true if the database was initialized synchronously
140140
BroadcastSlowMode atomic.Bool // bool to indicate if a slower ticker value should be used to notify changes feeds of changes
141141
DatabaseStartupError *DatabaseError // Error that occurred during database online processes startup
142+
SameSiteCookieMode http.SameSite
142143
}
143144

144145
type Scope struct {
@@ -251,6 +252,7 @@ type UnsupportedOptions struct {
251252
KVBufferSize int `json:"kv_buffer,omitempty"` // Enables user to set their own KV pool buffer
252253
BlipSendDocsWithChannelRemoval bool `json:"blip_send_docs_with_channel_removal,omitempty"` // Enables sending docs with channel removals using channel filters
253254
RejectWritesWithSkippedSequences bool `json:"reject_writes_with_skipped_sequences,omitempty"` // Reject writes if there are skipped sequences in the database
255+
SameSiteCookie *string `json:"same_site_cookie,omitempty"` // Sets the SameSite attribute on session cookies.
254256
}
255257

256258
type WarningThresholds struct {
@@ -418,6 +420,7 @@ func NewDatabaseContext(ctx context.Context, dbName string, bucket base.Bucket,
418420
CollectionByID: make(map[uint32]*DatabaseCollection),
419421
ServerUUID: serverUUID,
420422
UserFunctionTimeout: defaultUserFunctionTimeout,
423+
SameSiteCookieMode: http.SameSiteDefaultMode,
421424
}
422425

423426
// set up cancellable context based on the background context (context lifecycle for the database
@@ -2441,3 +2444,23 @@ func (db *Database) DataStoreNames() base.ScopeAndCollectionNames {
24412444
}
24422445
return names
24432446
}
2447+
2448+
// GetSameSiteCookieMode returns the http.SameSite mode based on the unsupported database options. Returns an error if
2449+
// an invalid string is set.
2450+
func (o *UnsupportedOptions) GetSameSiteCookieMode() (http.SameSite, error) {
2451+
if o == nil || o.SameSiteCookie == nil {
2452+
return http.SameSiteDefaultMode, nil
2453+
}
2454+
switch *o.SameSiteCookie {
2455+
case "Lax":
2456+
return http.SameSiteLaxMode, nil
2457+
case "Strict":
2458+
return http.SameSiteStrictMode, nil
2459+
case "None":
2460+
return http.SameSiteNoneMode, nil
2461+
case "Default":
2462+
return http.SameSiteDefaultMode, nil
2463+
default:
2464+
return http.SameSiteDefaultMode, fmt.Errorf("unsupported_options.same_site_cookie option %q is not valid, choices are \"Lax\", \"Strict\", and \"None", *o.SameSiteCookie)
2465+
}
2466+
}

‎docs/api/components/schemas.yaml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,6 +1306,15 @@ Database:
13061306
oidc_tls_skip_verify:
13071307
description: Enable self-signed certificates for OIDC testing.
13081308
type: boolean
1309+
same_site_cookie:
1310+
description: |-
1311+
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.
1312+
type: string
1313+
enum:
1314+
- "Default"
1315+
- "Lax"
1316+
- "None"
1317+
- "Strict"
13091318
sgr_tls_skip_verify:
13101319
description: Enable self-signed certificates for SG-replicate testing.
13111320
type: boolean

‎rest/config.go‎

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

1083+
if dbConfig.Unsupported != nil {
1084+
_, err := dbConfig.Unsupported.GetSameSiteCookieMode()
1085+
if err != nil {
1086+
multiError = multiError.Append(err)
1087+
}
1088+
}
1089+
10831090
if validateReplications {
10841091
for name, r := range dbConfig.Replications {
10851092
if name == "" {

‎rest/config_test.go‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3355,3 +3355,62 @@ func TestConfigUserXattrKeyValidation(t *testing.T) {
33553355
})
33563356
}
33573357
}
3358+
3359+
func TestValidateUnsupportedSameSiteCookies(t *testing.T) {
3360+
tests := []struct {
3361+
name string
3362+
unsupportedSettings *db.UnsupportedOptions
3363+
error string
3364+
}{
3365+
{
3366+
name: "no unsupportedSettings present",
3367+
unsupportedSettings: &db.UnsupportedOptions{},
3368+
error: "",
3369+
},
3370+
{
3371+
name: "no samesite, unsupportedSettings present",
3372+
unsupportedSettings: &db.UnsupportedOptions{},
3373+
error: "",
3374+
},
3375+
{
3376+
name: "valid value Lax",
3377+
unsupportedSettings: &db.UnsupportedOptions{SameSiteCookie: base.Ptr("Lax")},
3378+
error: "",
3379+
},
3380+
{
3381+
name: "valid value Strict",
3382+
unsupportedSettings: &db.UnsupportedOptions{SameSiteCookie: base.Ptr("Strict")},
3383+
error: "",
3384+
},
3385+
{
3386+
name: "valid value None",
3387+
unsupportedSettings: &db.UnsupportedOptions{SameSiteCookie: base.Ptr("None")},
3388+
error: "",
3389+
},
3390+
{
3391+
name: "invalid value",
3392+
unsupportedSettings: &db.UnsupportedOptions{SameSiteCookie: base.Ptr("invalid value")},
3393+
error: "unsupported_options.same_site_cookie option",
3394+
},
3395+
}
3396+
3397+
for _, test := range tests {
3398+
t.Run(test.name, func(t *testing.T) {
3399+
dbConfig := DbConfig{
3400+
Name: "db",
3401+
Unsupported: test.unsupportedSettings,
3402+
}
3403+
3404+
validateReplications := false
3405+
validateOIDC := false
3406+
ctx := base.TestCtx(t)
3407+
err := dbConfig.validate(ctx, validateOIDC, validateReplications)
3408+
if test.error != "" {
3409+
require.Error(t, err)
3410+
require.Contains(t, err.Error(), test.error)
3411+
} else {
3412+
require.NoError(t, err)
3413+
}
3414+
})
3415+
}
3416+
}

‎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
@@ -922,6 +922,16 @@ func (sc *ServerContext) _getOrAddDatabaseFromConfig(ctx context.Context, config
922922
} else {
923923
dbcontext.CORS = sc.Config.API.CORS
924924
}
925+
if !dbcontext.CORS.IsEmpty() {
926+
dbcontext.SameSiteCookieMode = http.SameSiteNoneMode
927+
}
928+
if config.Unsupported != nil && config.Unsupported.SameSiteCookie != nil {
929+
var err error
930+
dbcontext.SameSiteCookieMode, err = config.Unsupported.GetSameSiteCookieMode()
931+
if err != nil {
932+
return nil, err
933+
}
934+
}
925935

926936
if config.RevsLimit != nil {
927937
dbcontext.RevsLimit = *config.RevsLimit

0 commit comments

Comments
 (0)