Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement better handling for cookie 'SameSite' and 'secure' settings #1485

Merged
merged 4 commits into from
Sep 6, 2023

Conversation

shauke
Copy link
Collaborator

@shauke shauke commented Aug 18, 2023

PR Type

[x] Feature

What Is the New Behavior?

feat: implement better handling for cookie 'SameSite' and 'secure' settings

  • defaults to 'SameSite=Strict' and 'secure' if not explicitly set to other values
  • sets all cookies with 'SameSite=None' if run within an iframe
  • if run with 'http' (should only be in development environments) 'secure' is not set
  • inspired by https://medium.com/trabe/cookies-and-iframes-f7cca58b3b9e

Does this PR Introduce a Breaking Change?

[x] Yes

The cookies.service has new defaults: SameSite=Strict and secure (see Migrations / 4.1 to 4.2 for more details).

Other Information

AB#88753

@shauke shauke requested a review from SGrueber August 18, 2023 15:55
@shauke shauke self-assigned this Aug 18, 2023
@shauke shauke added this to the 4.2 milestone Aug 18, 2023
@shauke shauke force-pushed the feature/cookie_handling_defaults branch from 5ee0522 to a39545c Compare August 21, 2023 15:13
src/app/core/utils/cookies/cookies.service.ts Outdated Show resolved Hide resolved
src/app/core/utils/cookies/cookies.service.ts Outdated Show resolved Hide resolved
docs/guides/migrations.md Outdated Show resolved Hide resolved
docs/guides/migrations.md Outdated Show resolved Hide resolved
docs/guides/migrations.md Outdated Show resolved Hide resolved
shauke added a commit that referenced this pull request Aug 28, 2023
…ttings (#1485)

* defaults to 'SameSite=Strict' and 'secure' if not explicitly set to other values
* sets all cookies with 'SameSite=None' if run within an iframe
* if run with 'http' (should only be in development environments) 'secure' is not set
* inspired by https://medium.com/trabe/cookies-and-iframes-f7cca58b3b9e

BREAKING CHANGES: The `cookies.service` has new defaults: `SameSite=Strict` and `secure` (see [Migrations / 4.1 to 4.2](https://github.com/intershop/intershop-pwa/blob/develop/docs/guides/migrations.md#41-to-42) for more details).
@shauke shauke force-pushed the feature/cookie_handling_defaults branch from a39545c to ca823a0 Compare August 28, 2023 21:32
@SGrueber SGrueber added enhancement Enhancement to an existing feature refactoring Refactoring of current code labels Aug 31, 2023
shauke added a commit that referenced this pull request Sep 4, 2023
…ttings (#1485)

* defaults to 'SameSite=Strict' and 'secure' if not explicitly set to other values
* sets all cookies with 'SameSite=None' if run within an iframe
* if run with 'http' (should only be in development environments) 'secure' is not set
* inspired by https://medium.com/trabe/cookies-and-iframes-f7cca58b3b9e

BREAKING CHANGES: The `cookies.service` has new defaults: `SameSite=Strict` and `secure` (see [Migrations / 4.1 to 4.2](https://github.com/intershop/intershop-pwa/blob/develop/docs/guides/migrations.md#41-to-42) for more details).
* macOS 11 with Safari 14 seems to not handle 'SameSite=Strict' cookies (e.g. when redirecting to the payment provider page) correctly
* the 'apiToken' cookie that handles the authentication (logged in user) state is no longer present so that the order cannot be fetched and displayed again
* fix would be to not write 'SameSite=Strict' cookies in Safari 14
* the simple browser detection could be replaced if a more sophisticated version is needed
@shauke shauke force-pushed the feature/cookie_handling_defaults branch from 493868a to 240ed99 Compare September 5, 2023 15:58
@shauke shauke requested a review from SGrueber September 5, 2023 16:03
@shauke shauke merged commit 0a2a380 into develop Sep 6, 2023
21 checks passed
@shauke shauke deleted the feature/cookie_handling_defaults branch September 6, 2023 07:35
shauke added a commit that referenced this pull request Sep 6, 2023
…ttings (#1485)

* defaults to 'SameSite=Strict' and 'secure' if not explicitly set to other values
* sets all cookies with 'SameSite=None' if run within an iframe
* if run with 'http' (should only be in development environments) 'secure' is not set
* inspired by https://medium.com/trabe/cookies-and-iframes-f7cca58b3b9e

BREAKING CHANGES: The `cookies.service` has new defaults: `SameSite=Strict` and `secure` (see [Migrations / 4.1 to 4.2](https://github.com/intershop/intershop-pwa/blob/develop/docs/guides/migrations.md#41-to-42) for more details).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to an existing feature refactoring Refactoring of current code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants