Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion auth/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func (auth *Authenticator) GetSession(sessionID string) (*LoginSession, error) {
return &session, nil
}

func (auth *Authenticator) MakeSessionCookie(session *LoginSession, secureCookie bool, httpOnly bool) *http.Cookie {
func (auth *Authenticator) MakeSessionCookie(session *LoginSession, secureCookie bool, httpOnly bool, sameSite http.SameSite) *http.Cookie {
if session == nil {
return nil
}
Expand All @@ -145,6 +145,8 @@ func (auth *Authenticator) MakeSessionCookie(session *LoginSession, secureCookie
Expires: session.Expiration,
Secure: secureCookie,
HttpOnly: httpOnly,
// as of go 1.25, http.SameSiteDefaultMode will omit SameSite attribute from the cookie
SameSite: sameSite,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to go verify that http.Cookie doesn't emit the SameSite attribute when SameSiteDefaultMode is specified, to ensure we won't see any functional change for users not using CORS or setting the flag.
It's possible that's worth mentioning the default handling in a function description comment, to make that clear. (on the fence about the necessity of that, take it or leave it)

}
}

Expand Down
18 changes: 9 additions & 9 deletions auth/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,14 @@ func TestMakeSessionCookie(t *testing.T) {
Ttl: 24 * time.Hour,
}

cookie := auth.MakeSessionCookie(mockSession, false, false)
cookie := auth.MakeSessionCookie(mockSession, false, false, http.SameSiteDefaultMode)
assert.Equal(t, DefaultCookieName, cookie.Name)
assert.Equal(t, sessionID, cookie.Value)
assert.NotEmpty(t, cookie.Expires)

// Cookies should not be created with uninitialized session
mockSession = nil
cookie = auth.MakeSessionCookie(mockSession, false, false)
cookie = auth.MakeSessionCookie(mockSession, false, false, http.SameSiteDefaultMode)
assert.Empty(t, cookie)
}

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

unsecuredCookie := auth.MakeSessionCookie(mockSession, false, false)
unsecuredCookie := auth.MakeSessionCookie(mockSession, false, false, http.SameSiteDefaultMode)
assert.False(t, unsecuredCookie.Secure)

securedCookie := auth.MakeSessionCookie(mockSession, true, false)
securedCookie := auth.MakeSessionCookie(mockSession, true, false, http.SameSiteDefaultMode)
assert.True(t, securedCookie.Secure)

httpOnlyFalseCookie := auth.MakeSessionCookie(mockSession, false, false)
httpOnlyFalseCookie := auth.MakeSessionCookie(mockSession, false, false, http.SameSiteDefaultMode)
assert.False(t, httpOnlyFalseCookie.HttpOnly)

httpOnlyCookie := auth.MakeSessionCookie(mockSession, false, true)
httpOnlyCookie := auth.MakeSessionCookie(mockSession, false, true, http.SameSiteDefaultMode)
assert.True(t, httpOnlyCookie.HttpOnly)
}

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

request, err := http.NewRequest(http.MethodGet, "", nil)
require.NoError(t, err)
request.AddCookie(auth.MakeSessionCookie(session, true, true))
request.AddCookie(auth.MakeSessionCookie(session, true, true, http.SameSiteDefaultMode))

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

request, err := http.NewRequest(http.MethodGet, "", nil)
require.NoError(t, err)
request.AddCookie(auth.MakeSessionCookie(session, true, true))
request.AddCookie(auth.MakeSessionCookie(session, true, true, http.SameSiteDefaultMode))

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

request, err := http.NewRequest(http.MethodGet, "", nil)
require.NoError(t, err)
request.AddCookie(auth.MakeSessionCookie(session, true, true))
request.AddCookie(auth.MakeSessionCookie(session, true, true, http.SameSiteDefaultMode))
recorder := httptest.NewRecorder()

_, err = auth.AuthenticateCookie(request, recorder)
Expand Down
23 changes: 23 additions & 0 deletions db/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ type DatabaseContext struct {
CachedCCVStartingCas *base.VBucketCAS // If set, the cached value of the CCV starting CAS value to avoid repeated lookups
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)
numVBuckets uint16 // Number of vbuckets in the bucket
SameSiteCookieMode http.SameSite
}

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

type WarningThresholds struct {
Expand Down Expand Up @@ -433,6 +435,7 @@ func NewDatabaseContext(ctx context.Context, dbName string, bucket base.Bucket,
ServerUUID: serverUUID,
UserFunctionTimeout: defaultUserFunctionTimeout,
CachedCCVStartingCas: &base.VBucketCAS{},
SameSiteCookieMode: http.SameSiteDefaultMode,
}
dbContext.numVBuckets, err = bucket.GetMaxVbno()
if err != nil {
Expand Down Expand Up @@ -2585,3 +2588,23 @@ func PurgeDCPCheckpoints(ctx context.Context, database *DatabaseContext, checkpo
func (db *DatabaseContext) EnableAllowConflicts(tb testing.TB) {
db.Options.AllowConflicts = base.Ptr(true)
}

// GetSameSiteCookieMode returns the http.SameSite mode based on the unsupported database options. Returns an error if
// an invalid string is set.
func (o *UnsupportedOptions) GetSameSiteCookieMode() (http.SameSite, error) {
if o == nil || o.SameSiteCookie == nil {
return http.SameSiteDefaultMode, nil
}
switch *o.SameSiteCookie {
case "Lax":
return http.SameSiteLaxMode, nil
case "Strict":
return http.SameSiteStrictMode, nil
case "None":
return http.SameSiteNoneMode, nil
case "Default":
return http.SameSiteDefaultMode, nil
default:
return http.SameSiteDefaultMode, fmt.Errorf("unsupported_options.same_site_cookie option %q is not valid, choices are \"Lax\", \"Strict\", and \"None", *o.SameSiteCookie)
}
}
9 changes: 9 additions & 0 deletions docs/api/components/schemas.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1698,6 +1698,15 @@ Database:
remote_config_tls_skip_verify:
description: Enable self-signed certificates for external JavaScript load.
type: boolean
same_site_cookie:
description: |-
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.
type: string
enum:
- "Default"
- "Lax"
- "None"
- "Strict"
sgr_tls_skip_verify:
description: Enable self-signed certificates for SG-replicate testing.
type: boolean
Expand Down
7 changes: 7 additions & 0 deletions rest/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -1083,6 +1083,13 @@ func (dbConfig *DbConfig) validateVersion(ctx context.Context, isEnterpriseEditi
}
}

if dbConfig.Unsupported != nil {
_, err := dbConfig.Unsupported.GetSameSiteCookieMode()
if err != nil {
multiError = multiError.Append(err)
}
}

if validateReplications {
for name, r := range dbConfig.Replications {
if name == "" {
Expand Down
59 changes: 59 additions & 0 deletions rest/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3378,3 +3378,62 @@ func TestConfigUserXattrKeyValidation(t *testing.T) {
})
}
}

func TestValidateUnsupportedSameSiteCookies(t *testing.T) {
tests := []struct {
name string
unsupportedSettings *db.UnsupportedOptions
error string
}{
{
name: "no unsupportedSettings present",
unsupportedSettings: &db.UnsupportedOptions{},
error: "",
},
{
name: "no samesite, unsupportedSettings present",
unsupportedSettings: &db.UnsupportedOptions{},
error: "",
},
{
name: "valid value Lax",
unsupportedSettings: &db.UnsupportedOptions{SameSiteCookie: base.Ptr("Lax")},
error: "",
},
{
name: "valid value Strict",
unsupportedSettings: &db.UnsupportedOptions{SameSiteCookie: base.Ptr("Strict")},
error: "",
},
{
name: "valid value None",
unsupportedSettings: &db.UnsupportedOptions{SameSiteCookie: base.Ptr("None")},
error: "",
},
{
name: "invalid value",
unsupportedSettings: &db.UnsupportedOptions{SameSiteCookie: base.Ptr("invalid value")},
error: "unsupported_options.same_site_cookie option",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
dbConfig := DbConfig{
Name: "db",
Unsupported: test.unsupportedSettings,
}

validateReplications := false
validateOIDC := false
ctx := base.TestCtx(t)
err := dbConfig.validate(ctx, validateOIDC, validateReplications)
if test.error != "" {
require.Error(t, err)
require.Contains(t, err.Error(), test.error)
} else {
require.NoError(t, err)
}
})
}
}
128 changes: 78 additions & 50 deletions rest/cors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (

"github.com/couchbase/sync_gateway/auth"
"github.com/couchbase/sync_gateway/base"
"github.com/couchbase/sync_gateway/db"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -409,66 +410,93 @@ func TestCORSLoginOriginPerDatabase(t *testing.T) {
// Override the default (example.com) CORS configuration in the DbConfig for /db:
rt := NewRestTesterPersistentConfigNoDB(t)
defer rt.Close()
dbConfig := rt.NewDbConfig()
dbConfig.CORS = &auth.CORSConfig{
Origin: []string{"http://couchbase.com", "http://staging.couchbase.com"},
LoginOrigin: []string{"http://couchbase.com"},
Headers: []string{},
}
RequireStatus(t, rt.CreateDatabase("dbloginorigin", dbConfig), http.StatusCreated)

const username = "alice"
rt.CreateUser(username, nil)

testCases := []struct {
name string
origin string
responseCode int
responseErrorBody string
name string
unsupportedOptions *db.UnsupportedOptions
sameSite http.SameSite
}{
{
name: "CORS login origin allowed couchbase",
origin: "http://couchbase.com",
responseCode: http.StatusOK,
name: "No unsupported options",
unsupportedOptions: nil,
sameSite: http.SameSiteNoneMode,
},
{
name: "CORS login origin not allowed staging",
origin: "http://staging.couchbase.com",
responseCode: http.StatusBadRequest,
responseErrorBody: "No CORS",
name: "With unsupported options",
unsupportedOptions: &db.UnsupportedOptions{
SameSiteCookie: base.Ptr("Strict"),
},
sameSite: http.SameSiteStrictMode,
},
}
for _, test := range testCases {
rt.Run(test.name, func(t *testing.T) {
reqHeaders := map[string]string{
"Origin": test.origin,
"Authorization": GetBasicAuthHeader(t, username, RestTesterDefaultUserPassword),
}
resp := rt.SendRequestWithHeaders(http.MethodPost, "/{{.db}}/_session", "", reqHeaders)
RequireStatus(t, resp, test.responseCode)
if test.responseErrorBody != "" {
require.Contains(t, resp.Body.String(), test.responseErrorBody)
// the access control headers are returned based on Origin and not LoginOrigin which could be considered a bug
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
} else {
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
for _, dbTestCases := range testCases {
rt.Run(dbTestCases.name, func(t *testing.T) {
// Override the default (example.com) CORS configuration in the DbConfig for /db:
rt := NewRestTesterPersistentConfigNoDB(t)
defer rt.Close()

dbConfig := rt.NewDbConfig()
dbConfig.Unsupported = dbTestCases.unsupportedOptions
dbConfig.CORS = &auth.CORSConfig{
Origin: []string{"http://couchbase.com", "http://staging.couchbase.com"},
LoginOrigin: []string{"http://couchbase.com"},
Headers: []string{},
}
if test.responseCode == http.StatusOK {
cookie, err := http.ParseSetCookie(resp.Header().Get("Set-Cookie"))
require.NoError(t, err)
require.NotEmpty(t, cookie.Path)
reqHeaders["Cookie"] = fmt.Sprintf("%s=%s", cookie.Name, cookie.Value)
}
resp = rt.SendRequestWithHeaders(http.MethodDelete, "/{{.db}}/_session", "", reqHeaders)
RequireStatus(t, resp, test.responseCode)
if test.responseErrorBody != "" {
require.Contains(t, resp.Body.String(), test.responseErrorBody)
// the access control headers are returned based on Origin and not LoginOrigin which could be considered a bug
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
} else {
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
RequireStatus(t, rt.CreateDatabase(SafeDatabaseName(t, dbTestCases.name), dbConfig), http.StatusCreated)
const username = "alice"
rt.CreateUser(username, nil)

testCases := []struct {
name string
origin string
responseCode int
responseErrorBody string
}{
{
name: "CORS login origin allowed couchbase",
origin: "http://couchbase.com",
responseCode: http.StatusOK,
},
{
name: "CORS login origin not allowed staging",
origin: "http://staging.couchbase.com",
responseCode: http.StatusBadRequest,
responseErrorBody: "No CORS",
},
}
for _, test := range testCases {
rt.Run(test.name, func(t *testing.T) {
reqHeaders := map[string]string{
"Origin": test.origin,
"Authorization": GetBasicAuthHeader(t, username, RestTesterDefaultUserPassword),
}
resp := rt.SendRequestWithHeaders(http.MethodPost, "/{{.db}}/_session", "", reqHeaders)
RequireStatus(t, resp, test.responseCode)
if test.responseErrorBody != "" {
require.Contains(t, resp.Body.String(), test.responseErrorBody)
// the access control headers are returned based on Origin and not LoginOrigin which could be considered a bug
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
} else {
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
}
if test.responseCode == http.StatusOK {
cookie, err := http.ParseSetCookie(resp.Header().Get("Set-Cookie"))
require.NoError(t, err)
require.NotEmpty(t, cookie.Path)
require.Equal(t, dbTestCases.sameSite, cookie.SameSite)
reqHeaders["Cookie"] = fmt.Sprintf("%s=%s", cookie.Name, cookie.Value)
}
resp = rt.SendRequestWithHeaders(http.MethodDelete, "/{{.db}}/_session", "", reqHeaders)
RequireStatus(t, resp, test.responseCode)
if test.responseErrorBody != "" {
require.Contains(t, resp.Body.String(), test.responseErrorBody)
// the access control headers are returned based on Origin and not LoginOrigin which could be considered a bug
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
} else {
require.Equal(t, test.origin, resp.Header().Get(accessControlAllowOrigin))
}

})
}
})
}
}
Expand Down
10 changes: 10 additions & 0 deletions rest/server_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,16 @@ func (sc *ServerContext) _getOrAddDatabaseFromConfig(ctx context.Context, config
} else {
dbcontext.CORS = sc.Config.API.CORS
}
if !dbcontext.CORS.IsEmpty() {
dbcontext.SameSiteCookieMode = http.SameSiteNoneMode
}
if config.Unsupported != nil && config.Unsupported.SameSiteCookie != nil {
var err error
dbcontext.SameSiteCookieMode, err = config.Unsupported.GetSameSiteCookieMode()
if err != nil {
return nil, err
}
}

if config.RevsLimit != nil {
dbcontext.RevsLimit = *config.RevsLimit
Expand Down
Loading
Loading