Skip to content

default on SameSite attribute unset for Oauth2 cookies#6347

Merged
zirain merged 4 commits intoenvoyproxy:mainfrom
arkodg:revert-default-cookie-samesite
Jun 22, 2025
Merged

default on SameSite attribute unset for Oauth2 cookies#6347
zirain merged 4 commits intoenvoyproxy:mainfrom
arkodg:revert-default-cookie-samesite

Conversation

@arkodg
Copy link
Copy Markdown
Contributor

@arkodg arkodg commented Jun 18, 2025

Revisits #6289 which had set SameSite=Strict. This may cause some issues for specific flows

I had misinterpresented the meaning of Disabled earlier, it means Unset, and is separate from Samesite=None

@arkodg arkodg requested a review from a team as a code owner June 18, 2025 20:32
@arkodg
Copy link
Copy Markdown
Contributor Author

arkodg commented Jun 18, 2025

cc @vibe

@arkodg arkodg requested a review from a team June 18, 2025 20:40
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 18, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.72%. Comparing base (3e14c49) to head (0192e03).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #6347      +/-   ##
==========================================
- Coverage   70.75%   70.72%   -0.03%     
==========================================
  Files         220      220              
  Lines       37103    37089      -14     
==========================================
- Hits        26253    26233      -20     
- Misses       9314     9317       +3     
- Partials     1536     1539       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

zirain
zirain previously approved these changes Jun 18, 2025
View reviewed changes
@arkodg arkodg requested review from a team and zirain June 18, 2025 21:12
@vibe
Copy link
Copy Markdown
Contributor

vibe commented Jun 22, 2025

Totally reasonable to default back to disabled / unset.

Thanks @arkodg

Arko Dasgupta added 4 commits June 22, 2025 09:27
@zirain
default on SameSite attribute unset for Oauth2 cookies
bd4b562 
Revisits envoyproxy#6289
which had set SameSite=Strict. This may cause some issues
for specific flows

I had misinterpresented the meaning of `Disabled` earlier,
it means `Unset`, and is separate from `Samsite=None`

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
@zirain
gen
410a266 
Signed-off-by: Arko Dasgupta <arko@tetrate.io>
@zirain
more gen
02cd071 
Signed-off-by: Arko Dasgupta <arko@tetrate.io>
@zirain
lint
0192e03 
Signed-off-by: Arko Dasgupta <arko@tetrate.io>
@zirain zirain force-pushed the revert-default-cookie-samesite branch from d2cc569 to 0192e03 Compare June 22, 2025 01:27
@shawnh2
Copy link
Copy Markdown
Contributor

shawnh2 commented Jun 22, 2025

/retest

@zirain zirain merged commit fe22706 into envoyproxy:main Jun 22, 2025
45 of 50 checks passed
@BrewTestBot BrewTestBot mentioned this pull request Aug 8, 2025
Merged
arkodg pushed a commit to arkodg/gateway that referenced this pull request Sep 11, 2025
fix: rm Strict SameSite default
fde5931 
* by default it should be unset which implies `Lax`

Relates to envoyproxy#6347

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
@arkodg arkodg deleted the revert-default-cookie-samesite branch September 11, 2025 02:32
@arkodg arkodg mentioned this pull request Sep 11, 2025
Merged
arkodg added a commit that referenced this pull request Sep 11, 2025
@arkodg
fix: rm Strict SameSite default (#6941)
b19dfa1 
* by default it should be unset which implies `Lax`

Relates to #6347

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
zirain pushed a commit to zirain/gateway that referenced this pull request Sep 16, 2025
@arkodg @zirain
fix: rm Strict SameSite default (envoyproxy#6941)
d00435e 
* by default it should be unset which implies `Lax`

Relates to envoyproxy#6347

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Signed-off-by: zirain <zirain2009@gmail.com>
zirain pushed a commit to zirain/gateway that referenced this pull request Sep 16, 2025
@arkodg @zirain
fix: rm Strict SameSite default (envoyproxy#6941)
a71027f 
* by default it should be unset which implies `Lax`

Relates to envoyproxy#6347

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Signed-off-by: zirain <zirain2009@gmail.com>
zirain added a commit that referenced this pull request Sep 16, 2025
@zirain @guydc
@y-rabie @Windfarer @jukie @qicz @TomerJLevy @shahar-h @rudrakhp @arkodg @sudiptob2 @jacobneiltaylor @mathetake
[release/v1.5] cherry pick for v1.5.1 (#6975)
3e5d1aa 
* fix: cluster stat name: lowercase Kind (#6780)

cluster stat name: lowercase Kind

Signed-off-by: Guy Daich <guy.daich@sap.com>
Signed-off-by: zirain <zirain2009@gmail.com>

* fix: envoy service cluster name for zone-aware routing (#6763)

* fix!: fix envoy service cluster name for zone-aware routing

Signed-off-by: y-rabie <youssef.rabie@procore.com>

* extend e2e tests for zone aware routing

Signed-off-by: y-rabie <youssef.rabie@procore.com>

* extend unit tests for zone aware routing

Signed-off-by: y-rabie <youssef.rabie@procore.com>

---------

Signed-off-by: y-rabie <youssef.rabie@procore.com>
Signed-off-by: zirain <zirain2009@gmail.com>

* conformance: update experimental test report (#6782)

* conformance: update experimental test report

Signed-off-by: zirain <zirain2009@gmail.com>

* fix version

Signed-off-by: zirain <zirain2009@gmail.com>

* fix(api): image validation regex, support port in repository (#6819)

fix: match repository in image with port

Signed-off-by: Windfarer <windfarer@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>

* fix: Actually update xdsIR with maxAcceptPerSocketEvent (#6834)

* Actually update xdsIR with maxAcceptPerSocketEvent

Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>

* release note

Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>

* newline lint

Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>

---------

Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
Signed-off-by: zirain <zirain2009@gmail.com>

* bugfix: fix the topologyInjectorDisabled and the local cluster was not defined (#6847)

* bugfix: fix the topologyInjectorDisabled and the local cluster was not defined.

Signed-off-by: qicz <qiczzhu@gmail.com>

* fix ut

Signed-off-by: qicz <qiczzhu@gmail.com>

* add topology-injector-enabled ut

Signed-off-by: qicz <qiczzhu@gmail.com>

* add release note

Signed-off-by: qi <qiczzhu@gmail.com>

---------

Signed-off-by: qicz <qiczzhu@gmail.com>
Signed-off-by: qi <qiczzhu@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>

* fix(logging): correct log formatting to avoid DPANIC in controller-runtime logger (#6846)

* Update filters.go

Signed-off-by: TomerJLevy <TomerJlevy@gmail.com>

* add release notes

Signed-off-by: TomerJLevy <TomerJlevy@gmail.com>

---------

Signed-off-by: TomerJLevy <TomerJlevy@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>

* fix: handle context errors as transient errors (#6850)

* handle context errors as transient errors

Signed-off-by: TomerJLevy <TomerJlevy@gmail.com>

* add test cases

Signed-off-by: TomerJLevy <TomerJlevy@gmail.com>

* no need the new line

Signed-off-by: TomerJLevy <TomerJlevy@gmail.com>

* add release notes

Signed-off-by: TomerJLevy <TomerJlevy@gmail.com>

* Return the error as is

Signed-off-by: TomerJLevy <TomerJlevy@gmail.com>

* revert redundant changes

Signed-off-by: TomerJLevy <TomerJlevy@gmail.com>

* revert unrelated changes

Signed-off-by: TomerJLevy <TomerJlevy@gmail.com>

* revert more changes...

Signed-off-by: TomerJLevy <TomerJlevy@gmail.com>

---------

Signed-off-by: TomerJLevy <TomerJlevy@gmail.com>
Co-authored-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>

* bugfix: the controller cannot read the EnvoyProxy attached gatewayclass only. (#6838)

Signed-off-by: zirain <zirain2009@gmail.com>

* chore: fix CVE (#6903)

Signed-off-by: Shahar Harari <shahar.harari@sap.com>
Signed-off-by: zirain <zirain2009@gmail.com>

* fix: nil pointer dereference in btp configmap indexer (#6921)

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>

* improve targetRef selection for targetSelectors (#6917)

* improve targetRef selection for targetSelectors

* only select refs in the same namespace as the policy

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* fix lint

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

---------

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Signed-off-by: zirain <zirain2009@gmail.com>

* fix: suppress lua validation logs (#6929)

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>

* fix: rm incorrectly set exclusiveMaximum field in CRD (#6926)

* fix: rm incorrectly set exclusiveMaximum field in CRD

* Also fix maximum value to 599 which includes 599 as a valid num

Fixes: #6925

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Signed-off-by: zirain <zirain2009@gmail.com>

* fix: rm Strict SameSite default (#6941)

* by default it should be unset which implies `Lax`

Relates to #6347

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Signed-off-by: zirain <zirain2009@gmail.com>

* Optimize pod cache (#6936)

* Optimize pod cache

Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>

* release note

Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>

* Remove retry

Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>

* cleanup

Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>

---------

Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
Signed-off-by: Isaac <10012479+jukie@users.noreply.github.com>
Co-authored-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>

* reduce DeepCopy in gateway-api layer (#6940)

* reduce deep copy in gateway-api layer

* also fixed the DeepCopy implementation for ControllerResources
which was performing a Shallow Copy resulting it lack of isolation
b/w provider and gateway-api layer

Relates to #6919

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* fix lint

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

---------

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Signed-off-by: zirain <zirain2009@gmail.com>

* fix: validation for grpc routes with extension ref filters (#6949)

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>

* fix: cleanup dangling route status conditions (#6812)

Signed-off-by: y-rabie <youssef.rabie@procore.com>
Signed-off-by: zirain <zirain2009@gmail.com>

* Fix: Add missing patch annotations to Compression struct for proper Merge (#6951)

* fix: merge compression annotation

Signed-off-by: sudipto baral <sudiptobaral.me@gmail.com>

* test: add more compression merge test cases

Signed-off-by: sudipto baral <sudiptobaral.me@gmail.com>

---------

Signed-off-by: sudipto baral <sudiptobaral.me@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>

* fix: update distroless image to resolve glibc CVEs (#6953)

Signed-off-by: Shahar Harari <shahar.harari@sap.com>
Signed-off-by: zirain <zirain2009@gmail.com>

* chore: bump golang to 1.24.7 (#6959)

chore: bump golang

Signed-off-by: zirain <zirain2009@gmail.com>

* fix: Make sure proxy protocol filter is the first listener filter (#6972)

Fixes: #6873

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Co-authored-by: Jacob Neil Taylor <me@jacobtaylor.id.au>
Signed-off-by: zirain <zirain2009@gmail.com>

* release notes

Signed-off-by: zirain <zirain2009@gmail.com>

* Removes reflection from RouteContext to reduce allocations (#6820)

* bench: adds APIToXDS bench & small opt

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>

* no refect

goos: darwin
goarch: arm64
pkg: github.com/envoyproxy/gateway/test/gobench
cpu: Apple M1 Pro
                          │   old.txt    │               new.txt               │
                          │    sec/op    │   sec/op     vs base                │
GatewayAPItoXDS/small-10    881.2µ ±  7%   803.4µ ± 1%   -8.82% (p=0.000 n=10)
GatewayAPItoXDS/medium-10   4.130m ± 26%   2.959m ± 4%  -28.36% (p=0.000 n=10)
GatewayAPItoXDS/large-10     5.375 ±  2%    4.553 ± 1%  -15.28% (p=0.000 n=10)
geomean                     26.94m         22.12m       -17.90%

                          │   old.txt    │               new.txt                │
                          │     B/op     │     B/op      vs base                │
GatewayAPItoXDS/small-10    507.2Ki ± 0%   492.9Ki ± 0%   -2.83% (p=0.000 n=10)
GatewayAPItoXDS/medium-10   2.545Mi ± 7%   1.954Mi ± 2%  -23.21% (p=0.000 n=10)
GatewayAPItoXDS/large-10    2.832Gi ± 0%   2.831Gi ± 0%        ~ (p=0.529 n=10)
geomean                     15.40Mi        13.97Mi        -9.31%

                          │   old.txt   │               new.txt               │
                          │  allocs/op  │  allocs/op   vs base                │
GatewayAPItoXDS/small-10    8.328k ± 0%   8.017k ± 0%   -3.73% (p=0.000 n=10)
GatewayAPItoXDS/medium-10   39.45k ± 6%   29.74k ± 2%  -24.60% (p=0.000 n=10)
GatewayAPItoXDS/large-10    38.75M ± 0%   38.71M ± 0%   -0.11% (p=0.000 n=10)
geomean                     233.5k        209.8k       -10.16%

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>

* removes garbage

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>

* more tests

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>

* more tests

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>

---------

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>

---------

Signed-off-by: Guy Daich <guy.daich@sap.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: y-rabie <youssef.rabie@procore.com>
Signed-off-by: Windfarer <windfarer@gmail.com>
Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
Signed-off-by: qicz <qiczzhu@gmail.com>
Signed-off-by: qi <qiczzhu@gmail.com>
Signed-off-by: TomerJLevy <TomerJlevy@gmail.com>
Signed-off-by: Shahar Harari <shahar.harari@sap.com>
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Signed-off-by: Isaac <10012479+jukie@users.noreply.github.com>
Signed-off-by: sudipto baral <sudiptobaral.me@gmail.com>
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Co-authored-by: Guy Daich <guy.daich@sap.com>
Co-authored-by: Youssef Rabie <youssef.rabie@procore.com>
Co-authored-by: Windfarer <windfarer@gmail.com>
Co-authored-by: Isaac <10012479+jukie@users.noreply.github.com>
Co-authored-by: qi <qiczzhu@gmail.com>
Co-authored-by: TomerJLevy <TomerJlevy@gmail.com>
Co-authored-by: shahar-h <shahar.harari@sap.com>
Co-authored-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com>
Co-authored-by: Sudipto Baral <sudiptobaral.me@gmail.com>
Co-authored-by: Jacob Neil Taylor <me@jacobtaylor.id.au>
Co-authored-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zirain zirain zirain approved these changes

@shawnh2 shawnh2 shawnh2 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@arkodg @vibe @shawnh2 @zirain