Skip to content

Conversation

@torcolvin
Copy link
Collaborator

[3.3.0.2 backport] CBG-4964 create cookie with SameSite=None if CORS enabled

cherry-pick of 30ee4fe, minimal conflicts in database.go

@torcolvin torcolvin requested a review from adamcfraser October 29, 2025 19:46
Copilot AI review requested due to automatic review settings October 29, 2025 19:46
@github-actions
Copy link

Redocly previews

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR backports CBG-4964 to the 3.3.0.2 release, adding support for setting the SameSite attribute on session cookies. When CORS is enabled, session cookies will automatically use SameSite=None, while allowing explicit configuration through the same_site_cookie unsupported option.

Key changes:

  • Adds automatic SameSite=None for session cookies when CORS is enabled
  • Implements same_site_cookie configuration option in unsupported settings
  • Updates session cookie creation to accept and apply SameSite attribute

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
rest/utilities_testing.go Adds SafeDatabaseName helper function for test database naming
rest/session_test.go Adds test coverage for SameSite cookie behavior with and without CORS
rest/session_api.go Updates makeSessionWithTTL to pass SameSite mode when creating session cookies
rest/server_context.go Sets default SameSite=None when CORS enabled and handles unsupported config override
rest/cors_test.go Extends CORS tests to verify SameSite cookie behavior with configuration options
rest/config_test.go Adds validation tests for same_site_cookie configuration values
rest/config.go Adds validation for SameSite cookie configuration during database config validation
docs/api/components/schemas.yaml Documents the new same_site_cookie API field
db/database.go Adds SameSiteCookieMode field and GetSameSiteCookieMode method
auth/session_test.go Updates test calls to include SameSite parameter
auth/session.go Updates MakeSessionCookie to accept and apply SameSite parameter

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)
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'choices' to include 'None' and added closing quote and parenthesis.

Suggested change
return http.SameSiteDefaultMode, fmt.Errorf("unsupported_options.same_site_cookie option %q is not valid, choices are \"Lax\", \"Strict\", and \"None", *o.SameSiteCookie)
return http.SameSiteDefaultMode, fmt.Errorf("unsupported_options.same_site_cookie option %q is not valid, choices are \"Lax\", \"Strict\", and \"None\"", *o.SameSiteCookie)

Copilot uses AI. Check for mistakes.
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)
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

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

[nitpick] Database name created from test case name should be stored in a variable for better readability and potential reuse in the test.

Suggested change
RequireStatus(t, rt.CreateDatabase(SafeDatabaseName(t, dbTestCases.name), dbConfig), http.StatusCreated)
dbName := SafeDatabaseName(t, dbTestCases.name)
RequireStatus(t, rt.CreateDatabase(dbName, dbConfig), http.StatusCreated)

Copilot uses AI. Check for mistakes.
@adamcfraser adamcfraser merged commit 932f745 into release/3.3.0.2 Oct 29, 2025
49 checks passed
@adamcfraser adamcfraser deleted the CBG-4964 branch October 29, 2025 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants