-
Notifications
You must be signed in to change notification settings - Fork 363
fix: Remove Beamer cookies on consent withdraw #3610
fix: Remove Beamer cookies on consent withdraw #3610
Conversation
CLA Assistant Lite All Contributors have signed the CLA. |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
src/logic/cookies/utils/index.ts
Outdated
@@ -37,3 +42,9 @@ export const saveCookie = async <T extends Record<string, any>>( | |||
} | |||
|
|||
export const removeCookie = (key: string, path: string, domain: string): void => Cookies.remove(key, { path, domain }) | |||
|
|||
export const removeCookies = (cookieList: Cookie[]): void => { | |||
// Extracts the main domain, e.g. gnosis-safe.io |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't you get this from location.hostname
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like location.hostname
returns the same value as location.host
. Where is the difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hostname also returns the port, e.g. localhost:3000, whereas host
doesn't. So it's better to use host
here.
{ name: `_BEAMER_LAST_POST_SHOWN_${BEAMER_ID}`, path: '/' }, | ||
{ name: `_BEAMER_DATE_${BEAMER_ID}`, path: '/' }, | ||
{ name: `_BEAMER_FIRST_VISIT_${BEAMER_ID}`, path: '/' }, | ||
{ name: `_BEAMER_USER_ID_${BEAMER_ID}`, path: '/' }, | ||
{ name: `_BEAMER_FILTER_BY_URL_${BEAMER_ID}`, path: '/' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't we establish that the id is generated on Beamer's side? Can't you just recurse through the cookies and remove anything that starts with _BEAMER
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Beamer ID is saved on our side within .env
. I like the idea of simply deleting all cookies with the _BEAMER
prefix but unfortunately it looks like we can't read cross-site cookies and at least on localhost the cookies are saved with getbeamer.com
domain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, but when @francovenica demoed the cookies earlier, it generated a different id the second time round? document.cookie
doesn't return all cookies?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to reproduce it but for me they all have the same product id now and no duplicates. If the cookies are saved with a different domain than the one the user is on, document.cookie
will not return them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright. Let's see how it goes in testing.
E2E Tests Failed Failed tests:
|
src/logic/cookies/utils/index.ts
Outdated
|
||
export const removeCookies = (cookieList: Cookie[]): void => { | ||
// Extracts the main domain, e.g. gnosis-safe.io | ||
const subDomain = location.host.split('.').slice(-2).join('.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically, it's not a subdomain, it's the main domain.
What it solves
Resolves #3608
How this PR fixes it
When withdrawing consent for Community Support & Updates cookies all
_BEAMER
cookies are deletedHow to test it
_BEAMER
cookies being added_BEAMER
cookies not existing anymore