Skip to content

Commit

Permalink
feat: implement better handling for cookie 'SameSite' and 'secure' se…
Browse files Browse the repository at this point in the history
…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).
  • Loading branch information
shauke committed Sep 6, 2023
1 parent 469f99d commit 0d28fae
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 8 deletions.
11 changes: 11 additions & 0 deletions docs/guides/migrations.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,17 @@ kb_sync_latest_only

# Migrations

## 4.1 to 4.2

A better handling for cookie `SameSite` and `secure` settings was implemented with new defaults to `SameSite=Strict` and `secure`.
This can still be overridden when calling `cookies.services` `put` method with explicitly set values.
Now the `secure` setting is always set to `true` if in `https` mode, you can prevent this by explicitly setting it to `false`.
If the PWA is run with `http` (should only be in development environments) `secure` is not set.
Additionally if the PWA is run in an iframe all cookies are set with `SameSite=None` (e.g. for punchout or Design View).
Be aware that some browsers no longer accept cookies with `SameSite=None` without `secure`.
Before by default no `SameSite` was set so browsers treated it as `SameSite=Lax`, this needs to be set explicitly now if it is really intended.
For migrating check the calls of the `cookies.service` `put` method whether they need to be adapted.

## 4.0 to 4.1

The Intershop PWA now uses Node.js 18.16.0 LTS with the corresponding npm version 9.5.1 to resolve an issue with Azure Docker deployments (see #1416).
Expand Down
57 changes: 51 additions & 6 deletions src/app/core/utils/cookies/cookies.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,72 @@ describe('Cookies Service', () => {

cookiesService = TestBed.inject(CookiesService);
document = TestBed.inject(DOCUMENT);
window = Object.create(window);
delete window.parent;
window.parent = window;
Object.defineProperty(window, 'location', { value: { protocol: 'https:' }, writable: true });
});

it('should be created', () => {
expect(cookiesService).toBeTruthy();
});

it('should call put of underlying implementation and set cookie defaults', () => {
cookiesService.put('foo', 'bar');
expect(document.cookie).toMatchInlineSnapshot(`"foo=bar;path=/de;SameSite=Strict;secure"`);
});

it('should call get of underlying implementation', () => {
cookiesService.put('foo', 'bar');
expect(document.cookie).toMatchInlineSnapshot(`"foo=bar;path=/de"`);
expect(cookiesService.get('foo')).toMatchInlineSnapshot(`"bar;path=/de"`);
expect(document.cookie).toMatchInlineSnapshot(`"foo=bar;path=/de;SameSite=Strict;secure"`);
expect(cookiesService.get('foo')).toMatchInlineSnapshot(`"bar;path=/de;SameSite=Strict;secure"`);
});

it('should call remove of underlying implementation', () => {
cookiesService.put('foo', 'bar');
expect(document.cookie).toMatchInlineSnapshot(`"foo=bar;path=/de"`);
expect(document.cookie).toMatchInlineSnapshot(`"foo=bar;path=/de;SameSite=Strict;secure"`);
cookiesService.remove('foo');
expect(document.cookie).toMatchInlineSnapshot(`"foo=;path=/de;expires=Thu, 01 Jan 1970 00:00:00 GMT"`);
expect(document.cookie).toMatchInlineSnapshot(
`"foo=;path=/de;expires=Thu, 01 Jan 1970 00:00:00 GMT;SameSite=Strict;secure"`
);
});

it('should not set "secure" if protocoll is "http:"', () => {
Object.defineProperty(window, 'location', { value: { protocol: 'http:' } });
cookiesService.put('foo', 'bar');
expect(document.cookie).toMatchInlineSnapshot(`"foo=bar;path=/de;SameSite=Strict"`);
});

it('should set "SameSite=None" if PWA is run in iframe', () => {
window.parent = Object.create(window);
cookiesService.put('foo', 'bar');
expect(document.cookie).toMatchInlineSnapshot(`"foo=bar;path=/de;SameSite=None;secure"`);
});

it('should call put of underlying implementation', () => {
it('should set cookie defaults', () => {
cookiesService.put('foo', 'bar');
expect(document.cookie).toMatchInlineSnapshot(`"foo=bar;path=/de"`);
expect(document.cookie).toMatchInlineSnapshot(`"foo=bar;path=/de;SameSite=Strict;secure"`);
});

it('should set cookie secure false explicitly', () => {
cookiesService.put('foo', 'bar', { secure: false });
expect(document.cookie).toMatchInlineSnapshot(`"foo=bar;path=/de;SameSite=Strict"`);
});

it('should set cookie "SameSite=None" explicitly', () => {
cookiesService.put('foo', 'bar', { sameSite: 'None' });
expect(document.cookie).toMatchInlineSnapshot(`"foo=bar;path=/de;SameSite=None;secure"`);
});

it('should set cookie "SameSite=Lax" and secure false explicitly', () => {
cookiesService.put('foo', 'bar', { sameSite: 'Lax', secure: false });
expect(document.cookie).toMatchInlineSnapshot(`"foo=bar;path=/de;SameSite=Lax"`);
});

it('should set cookie expire time', () => {
cookiesService.put('foo', 'bar', { expires: new Date(1234567890) });
expect(document.cookie).toMatchInlineSnapshot(
`"foo=bar;path=/de;expires=Thu, 15 Jan 1970 06:56:07 GMT;SameSite=Strict;secure"`
);
});
});
10 changes: 8 additions & 2 deletions src/app/core/utils/cookies/cookies.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,14 @@ export class CookiesService {
str += `;path=${path}`;
str += opts.domain ? `;domain=${opts.domain}` : '';
str += expires ? `;expires=${expires.toUTCString()}` : '';
str += opts.sameSite ? `;SameSite=${opts.sameSite}` : '';
str += opts.secure && location.protocol === 'https:' ? ';secure' : '';

// if in an iframe set cookies always with SameSite=None, otherwise set the given SameSite, default to SameSite=Strict
str += `;SameSite=${window.parent !== window ? 'None' : opts.sameSite || 'Strict'}`;

// if in http mode (should only be in development) or if explicitly set to false do not set the cookie secure, default to secure
// eslint-disable-next-line @typescript-eslint/no-unnecessary-boolean-literal-compare
str += window.location.protocol === 'http:' || opts.secure === false ? '' : ';secure';

const cookiesLength = str.length + 1;
if (cookiesLength > 4096) {
// eslint-disable-next-line no-console
Expand Down

0 comments on commit 0d28fae

Please sign in to comment.