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

Fix cookies not deleting on opt-out #5338

Merged
merged 31 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
e6183c6
update removeCookiesFromBrowser
jpople Sep 27, 2024
2f68cc8
update cookie deletion
jpople Sep 30, 2024
d94c5fb
update Jest test
jpople Sep 30, 2024
87fbe96
add tests for explicitly configured domains
jpople Sep 30, 2024
7135193
remove console logs
jpople Sep 30, 2024
bdcbbf9
Merge branch 'main' into jpople/prod-2822/cookie-deletion-bug
jpople Sep 30, 2024
65490af
update with env var to determine whether subdomain cookies are deleted
jpople Oct 3, 2024
708c7b6
fides-js changes
jpople Oct 3, 2024
3954aea
Merge branch 'main' into jpople/prod-2822/cookie-deletion-bug
jpople Oct 3, 2024
fe51336
remove debug log
jpople Oct 3, 2024
09bffef
reapply test changes
jpople Oct 3, 2024
6d68bbd
rename 'delete notice cookies on opt out' to 'automatic subdomain coo…
jpople Oct 3, 2024
6c9a51d
revert cookie delete logic
jpople Oct 3, 2024
1e59c17
add comment with link to followup
jpople Oct 3, 2024
e9eb6ac
Merge branch 'main' into jpople/prod-2822/cookie-deletion-bug
jpople Oct 3, 2024
aa28ea2
adjust strategy to use bool flag on experience configs, create migrat…
eastandwestwind Oct 8, 2024
397c22e
format
eastandwestwind Oct 8, 2024
124db46
format
eastandwestwind Oct 8, 2024
8c2d58a
lint, update tests with explicit bool vals
eastandwestwind Oct 8, 2024
867b741
fix test
eastandwestwind Oct 8, 2024
b58c815
Merge branch 'main' of github.com:ethyca/fides into jpople/prod-2822/…
eastandwestwind Oct 8, 2024
e6e8a13
missed table in migration
eastandwestwind Oct 8, 2024
7b2dcbd
Merge branch 'main' of github.com:ethyca/fides into jpople/prod-2822/…
eastandwestwind Oct 8, 2024
4a95e01
fix form field
eastandwestwind Oct 9, 2024
67eba3b
Merge branch 'main' of github.com:ethyca/fides into jpople/prod-2822/…
eastandwestwind Oct 14, 2024
a18d4df
adds tooltip
eastandwestwind Oct 14, 2024
e2e8355
bump downrev
eastandwestwind Oct 14, 2024
6abd382
update migration
eastandwestwind Oct 16, 2024
6a697fb
Merge branch 'main' of github.com:ethyca/fides into jpople/prod-2822/…
eastandwestwind Oct 16, 2024
91c2f73
bump downrev
eastandwestwind Oct 16, 2024
dadabee
rebase
eastandwestwind Oct 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .fides/db_dataset.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1064,6 +1064,8 @@ dataset:
data_categories: [ system.operations ]
- name: auto_detect_language
data_categories: [ system.operations ]
- name: auto_subdomain_cookie_deletion
data_categories: [ system.operations ]
- name: id
data_categories: [system.operations]
- name: name
Expand Down Expand Up @@ -1105,6 +1107,8 @@ dataset:
data_categories: [ system.operations ]
- name: auto_detect_language
data_categories: [ system.operations ]
- name: auto_subdomain_cookie_deletion
data_categories: [ system.operations ]
- name: id
data_categories: [system.operations]
- name: is_default
Expand Down Expand Up @@ -2001,6 +2005,8 @@ dataset:
data_categories: [ system.operations ]
- name: auto_detect_language
data_categories: [ system.operations ]
- name: auto_subdomain_cookie_deletion
data_categories: [ system.operations ]
- name: id
data_categories: [system.operations]
- name: name
Expand Down
1 change: 1 addition & 0 deletions clients/admin-ui/cypress/e2e/privacy-experiences.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ describe("Privacy experiences", () => {
expect(body).to.eql({
allow_language_selection: false,
auto_detect_language: true,
auto_subdomain_cookie_deletion: true,
component: "banner_and_modal",
disabled: true,
dismissable: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"layer1_button_options": "opt_in_opt_out",
"allow_language_selection": false,
"auto_detect_language": true,
"auto_subdomain_cookie_deletion": true,
"regions": ["us_ca", "us_co", "us_ct", "us_ut", "us_va"],
"id": "pri_001",
"created_at": "2024-03-22T20:06:27.119285+00:00",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,12 @@ export const PrivacyExperienceForm = ({
Edit experience text
</Button>
)}
<CustomSwitch
eastandwestwind marked this conversation as resolved.
Show resolved Hide resolved
name="auto_subdomain_cookie_deletion"
id="auto_subdomain_cookie_deletion"
label="Automatically delete subdomain cookies"
variant="stacked"
/>
</PrivacyExperienceConfigColumnLayout>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export const defaultInitialValues: Omit<ExperienceConfigCreate, "component"> = {
regions: [],
translations: defaultTranslations,
auto_detect_language: true,
auto_subdomain_cookie_deletion: true,
};
// utility type to pass as a prop to the translation form
export type TranslationWithLanguageName = ExperienceTranslation &
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export const buildBaseConfig = (
layer1_button_options: Layer1ButtonOption.OPT_IN_OPT_OUT,
allow_language_selection: true,
auto_detect_language: true,
auto_subdomain_cookie_deletion: true,
language: "en",
// in preview mode, we show the first translation in the main window, even when multiple translations are configured
translations: [buildExperienceTranslation(experienceConfig)],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export type ExperienceConfigCreate = {
layer1_button_options?: Layer1ButtonOption | null;
allow_language_selection?: boolean | null;
auto_detect_language?: boolean | null;
auto_subdomain_cookie_deletion?: boolean | null;
regions?: Array<PrivacyNoticeRegion>;
component: ComponentType;
privacy_notice_ids?: Array<string>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export type ExperienceConfigResponse = {
layer1_button_options?: Layer1ButtonOption | null;
allow_language_selection?: boolean | null;
auto_detect_language?: boolean | null;
auto_subdomain_cookie_deletion?: boolean | null;
regions: Array<PrivacyNoticeRegion>;
id: string;
created_at: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export type ExperienceConfigResponseNoNotices = {
layer1_button_options?: Layer1ButtonOption | null;
allow_language_selection?: boolean | null;
auto_detect_language?: boolean | null;
auto_subdomain_cookie_deletion?: boolean | null;
regions: Array<PrivacyNoticeRegion>;
id: string;
created_at: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export type ExperienceConfigUpdate = {
layer1_button_options?: Layer1ButtonOption | null;
allow_language_selection?: boolean | null;
auto_detect_language?: boolean | null;
auto_subdomain_cookie_deletion?: boolean | null;
regions: Array<PrivacyNoticeRegion>;
translations: Array<ExperienceTranslation>;
privacy_notice_ids: Array<string>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { MinimalTCFBannerTranslation } from "./MinimalTCFBannerTranslation"
*/
export type MinimalTCFExperienceConfig = {
auto_detect_language?: boolean | null;
auto_subdomain_cookie_deletion?: boolean | null;
component: ComponentType;
dismissable?: boolean | null;
translations?: Array<MinimalTCFBannerTranslation>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"layer1_button_options": "opt_in_opt_out",
"allow_language_selection": true,
"auto_detect_language": true,
"auto_subdomain_cookie_deletion": true,
"language": "en",
"accept_button_label": "Accept Test",
"acknowledge_button_label": "Acknowledge Test",
Expand Down
75 changes: 60 additions & 15 deletions clients/fides-js/__tests__/lib/cookie.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,35 +395,80 @@ describe("cookies", () => {
afterEach(() => mockRemoveCookie.mockClear());

it.each([
{ cookies: [], expectedAttributes: [] },
{ cookies: [{ name: "_ga123" }], expectedAttributes: [{ path: "/" }] },
{ cookies: [], removeSubdomainCookies: false, expectedAttributes: [] },
{ cookies: [], removeSubdomainCookies: true, expectedAttributes: [] },
{
cookies: [{ name: "_ga123", path: "" }],
expectedAttributes: [{ path: "" }],
cookies: [{ name: "_ga123" }],
removeSubdomainCookies: false,
expectedAttributes: [{ domain: undefined, path: "/" }],
},
{
cookies: [{ name: "_ga123", path: "/subpage" }],
expectedAttributes: [{ path: "/subpage" }],
cookies: [{ name: "_ga123" }],
removeSubdomainCookies: true,
expectedAttributes: [
{ domain: undefined, path: "/" },
{ domain: ".example.co.jp" },
],
},
{
cookies: [{ name: "aax-uid" }, { name: "pixel" }],
removeSubdomainCookies: false,
expectedAttributes: [
{ domain: undefined, path: "/" },
{ domain: undefined, path: "/" },
],
},
{
cookies: [{ name: "aax-uid" }, { name: "pixel" }],
removeSubdomainCookies: true,
expectedAttributes: [
{ domain: undefined, path: "/" },
{ domain: ".example.co.jp" },
],
},
// these cookies won't actually end up being deleted in the browser
// https://ethyca.atlassian.net/browse/PROD-2830
{
cookies: [{ name: "_ga123" }, { name: "shopify" }],
expectedAttributes: [{ path: "/" }, { path: "/" }],
cookies: [{ name: "test-cookie", domain: `"[*]"` }],
removeSubdomainCookies: false,
expectedAttributes: [{ domain: `"[*]"`, path: "/" }],
},
{
cookies: [{ name: "test-cookie", domain: `"[*]"` }],
removeSubdomainCookies: true,
expectedAttributes: [
{ domain: `"[*]"`, path: "/" },
{ domain: ".example.co.jp" },
],
},
])(
"should remove a list of cookies",
({
cookies,
removeSubdomainCookies,
expectedAttributes,
}: {
cookies: CookiesType[];
expectedAttributes: CookieAttributes[];
removeSubdomainCookies?: boolean;
expectedAttributes: Array<CookieAttributes | undefined>;
}) => {
removeCookiesFromBrowser(cookies);
expect(mockRemoveCookie.mock.calls).toHaveLength(cookies.length);
cookies.forEach((cookie, idx) => {
const [name, attributes] = mockRemoveCookie.mock.calls[idx];
expect(name).toEqual(cookie.name);
expect(attributes).toEqual(expectedAttributes[idx]);
removeCookiesFromBrowser(cookies, removeSubdomainCookies);
const expectedLength = removeSubdomainCookies
? cookies.length * 2
: cookies.length;
expect(mockRemoveCookie.mock.calls).toHaveLength(expectedLength);
cookies.forEach((cookie, cookieIdx) => {
const calls = removeSubdomainCookies
? mockRemoveCookie.mock.calls.slice(
cookieIdx * 2,
cookieIdx * 2 + 2,
)
: mockRemoveCookie.mock.calls.slice(cookieIdx, cookieIdx + 1);
calls.forEach((call, i) => {
const [name, attributes] = call;
expect(name).toEqual(cookie.name);
expect(attributes).toEqual(expectedAttributes[i]);
});
});
},
);
Expand Down
6 changes: 5 additions & 1 deletion clients/fides-js/src/lib/consent-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,10 @@ interface ExperienceConfigTranslationMinimal
export interface ExperienceConfigMinimal
extends Pick<
ExperienceConfig,
"component" | "auto_detect_language" | "dismissable"
| "component"
| "auto_detect_language"
| "dismissable"
| "auto_subdomain_cookie_deletion"
> {
translations: ExperienceConfigTranslationMinimal[];
}
Expand Down Expand Up @@ -481,6 +484,7 @@ export type ExperienceConfig = {
allow_language_selection?: boolean;
auto_detect_language?: boolean;
modal_link_label?: string;
auto_subdomain_cookie_deletion?: boolean;

/**
* List of regions that apply to this ExperienceConfig.
Expand Down
10 changes: 9 additions & 1 deletion clients/fides-js/src/lib/cookie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,13 +331,21 @@ export const makeConsentDefaultsLegacy = (

/**
* Given a list of cookies, deletes them from the browser
* Optionally removes subdomain cookies as well
*/
export const removeCookiesFromBrowser = (cookiesToRemove: CookiesType[]) => {
export const removeCookiesFromBrowser = (
cookiesToRemove: CookiesType[],
removeSubdomainCookies: boolean = true,
eastandwestwind marked this conversation as resolved.
Show resolved Hide resolved
) => {
cookiesToRemove.forEach((cookie) => {
cookies.remove(cookie.name, {
path: cookie.path ?? "/",
domain: cookie.domain,
});
if (removeSubdomainCookies) {
const { hostname } = window.location;
cookies.remove(cookie.name, { domain: `.${hostname}` });
}
});
};

Expand Down
5 changes: 4 additions & 1 deletion clients/fides-js/src/lib/preferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,10 @@ export const updateConsentPreferences = async ({
preference.consentPreference === UserConsentPreference.OPT_OUT,
)
.forEach((preference) => {
removeCookiesFromBrowser(preference.notice.cookies);
removeCookiesFromBrowser(
preference.notice.cookies,
experience.experience_config?.auto_subdomain_cookie_deletion,
);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
"layer1_button_options": "opt_in_opt_out",
"allow_language_selection": true,
"auto_detect_language": true,
"auto_subdomain_cookie_deletion": true,
"disabled": false,
"id": "pri_exp-banner-modal-000",
"component": "banner_and_modal",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
"layer1_button_options": "opt_in_opt_out",
"allow_language_selection": true,
"auto_detect_language": true,
"auto_subdomain_cookie_deletion": true,
"disabled": false,
"id": "pri_exp-banner-modal-000",
"component": "banner_and_modal",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
"layer1_button_options": "opt_in_opt_out",
"allow_language_selection": true,
"auto_detect_language": true,
"auto_subdomain_cookie_deletion": true,
"disabled": false,
"id": "pri_exp-gpp-banner-modal-000",
"component": "banner_and_modal",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
"layer1_button_options": "opt_in_opt_out",
"allow_language_selection": true,
"auto_detect_language": true,
"auto_subdomain_cookie_deletion": true,
"disabled": false,
"id": "pri_exp-modal-000",
"component": "modal",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
"layer1_button_options": "opt_in_opt_out",
"allow_language_selection": true,
"auto_detect_language": true,
"auto_subdomain_cookie_deletion": true,
"disabled": false,
"id": "pri_exp-privacy-center-000",
"component": "privacy_center",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
"layer1_button_options": "opt_in_opt_out",
"allow_language_selection": true,
"auto_detect_language": true,
"auto_subdomain_cookie_deletion": true,
"disabled": false,
"id": "pri_exp-privacy-center-000",
"component": "privacy_center",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
"layer1_button_options": "opt_in_opt_out",
"allow_language_selection": true,
"auto_detect_language": true,
"auto_subdomain_cookie_deletion": true,
"disabled": false,
"id": "pri_exp-tcf-overlay-000",
"component": "tcf_overlay",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"experience_config": {
"dismissable": true,
"auto_detect_language": true,
"auto_subdomain_cookie_deletion": true,
"component": "tcf_overlay",
"translations": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
"layer1_button_options": "opt_in_opt_out",
"allow_language_selection": true,
"auto_detect_language": true,
"auto_subdomain_cookie_deletion": true,
"disabled": false,
"id": "pri_exp-banner-modal-000",
"component": "banner_and_modal",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
layer1_button_options: "opt_in_opt_out",
allow_language_selection: true,
auto_detect_language: true,
auto_subdomain_cookie_deletion: true,
language: "en",
accept_button_label: "Accept Test",
acknowledge_button_label: "Acknowledge Test",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export type ExperienceConfigCreate = {
layer1_button_options?: Layer1ButtonOption | null;
allow_language_selection?: boolean | null;
auto_detect_language?: boolean | null;
auto_subdomain_cookie_deletion?: boolean | null;
regions?: Array<PrivacyNoticeRegion>;
component: ComponentType;
privacy_notice_ids?: Array<string>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export type ExperienceConfigResponse = {
layer1_button_options?: Layer1ButtonOption | null;
allow_language_selection?: boolean | null;
auto_detect_language?: boolean | null;
auto_subdomain_cookie_deletion?: boolean | null;
regions: Array<PrivacyNoticeRegion>;
id: string;
created_at: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export type ExperienceConfigResponseNoNotices = {
layer1_button_options?: Layer1ButtonOption | null;
allow_language_selection?: boolean | null;
auto_detect_language?: boolean | null;
auto_subdomain_cookie_deletion?: boolean | null;
regions: Array<PrivacyNoticeRegion>;
id: string;
created_at: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export type ExperienceConfigUpdate = {
layer1_button_options?: Layer1ButtonOption | null;
allow_language_selection?: boolean | null;
auto_detect_language?: boolean | null;
auto_subdomain_cookie_deletion?: boolean | null;
regions: Array<PrivacyNoticeRegion>;
translations: Array<ExperienceTranslation>;
privacy_notice_ids: Array<string>;
Expand Down
Loading
Loading