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 12 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
47 changes: 33 additions & 14 deletions clients/fides-js/__tests__/lib/cookie.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -396,34 +396,53 @@ describe("cookies", () => {

it.each([
{ cookies: [], expectedAttributes: [] },
{ cookies: [{ name: "_ga123" }], expectedAttributes: [{ path: "/" }] },
{ cookies: [], removeSubdomainCookies: true, expectedAttributes: [] },
{
cookies: [{ name: "_ga123", path: "" }],
expectedAttributes: [{ path: "" }],
cookies: [{ name: "_ga123" }],
expectedAttributes: [undefined],
},
{
cookies: [{ name: "_ga123", path: "/subpage" }],
expectedAttributes: [{ path: "/subpage" }],
cookies: [{ name: "_ga123" }],
removeSubdomainCookies: true,
expectedAttributes: [undefined, { domain: ".example.co.jp" }],
},
{
cookies: [{ name: "_ga123" }, { name: "shopify" }],
expectedAttributes: [{ path: "/" }, { path: "/" }],
cookies: [{ name: "aax-uid" }, { name: "pixel" }],
expectedAttributes: [undefined, undefined],
},
{
cookies: [{ name: "aax-uid" }, { name: "pixel" }],
removeSubdomainCookies: true,
expectedAttributes: [undefined, { 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
1 change: 1 addition & 0 deletions clients/fides-js/src/fides-tcf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ const _Fides: FidesGlobal = {
base64Cookie: false,
fidesPrimaryColor: null,
fidesClearCookie: false,
automaticSubdomainCookieDeletion: false,
},
fides_meta: {},
identity: {},
Expand Down
1 change: 1 addition & 0 deletions clients/fides-js/src/fides.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ const _Fides: FidesGlobal = {
base64Cookie: false,
fidesPrimaryColor: null,
fidesClearCookie: false,
automaticSubdomainCookieDeletion: false,
},
fides_meta: {},
identity: {},
Expand Down
3 changes: 3 additions & 0 deletions clients/fides-js/src/lib/consent-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@ export interface FidesInitOptions {

// Shows fides.js overlay UI on load deleting the fides_consent cookie as if no preferences have been saved
fidesClearCookie: boolean;

// Whether cookies on subdomains should be deleted when user opts out
automaticSubdomainCookieDeletion: boolean;
}

/**
Expand Down
15 changes: 10 additions & 5 deletions clients/fides-js/src/lib/cookie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,13 +331,18 @@ 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 = false,
) => {
cookiesToRemove.forEach((cookie) => {
cookies.remove(cookie.name, {
path: cookie.path ?? "/",
domain: cookie.domain,
});
const { hostname } = window.location;
cookies.remove(cookie.name);
jpople marked this conversation as resolved.
Show resolved Hide resolved
if (removeSubdomainCookies) {
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,
options.automaticSubdomainCookieDeletion,
);
});
}

Expand Down
3 changes: 3 additions & 0 deletions clients/privacy-center/app/server-environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export type PrivacyCenterClientSettings = Pick<
| "BASE_64_COOKIE"
| "FIDES_PRIMARY_COLOR"
| "FIDES_CLEAR_COOKIE"
| "AUTOMATIC_SUBDOMAIN_COOKIE_DELETION"
>;

export type Styles = string;
Expand Down Expand Up @@ -342,6 +343,8 @@ export const loadPrivacyCenterEnvironment = async ({
BASE_64_COOKIE: settings.BASE_64_COOKIE,
FIDES_PRIMARY_COLOR: settings.FIDES_PRIMARY_COLOR,
FIDES_CLEAR_COOKIE: settings.FIDES_CLEAR_COOKIE,
AUTOMATIC_SUBDOMAIN_COOKIE_DELETION:
settings.AUTOMATIC_SUBDOMAIN_COOKIE_DELETION,
};

// For backwards-compatibility, override FIDES_API_URL with the value from the config file if present
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,5 @@ export interface PrivacyCenterSettings {
BASE_64_COOKIE: boolean; // whether or not to encode cookie as base64 on top of the default JSON string
FIDES_PRIMARY_COLOR: string | null; // (optional) sets fides primary color
FIDES_CLEAR_COOKIE: boolean; // (optional) deletes fides_consent cookie on reload
AUTOMATIC_SUBDOMAIN_COOKIE_DELETION: boolean; // (optional) deletes notice cookies from subdomains in addition to top-level domain on opt-out
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@ const loadEnvironmentVariables = () => {
FIDES_CLEAR_COOKIE: process.env.FIDES_PRIVACY_CENTER__FIDES_CLEAR_COOKIE
? process.env.FIDES_PRIVACY_CENTER__FIDES_CLEAR_COOKIE === "true"
: false,
AUTOMATIC_SUBDOMAIN_COOKIE_DELETION: process.env
.FIDES_PRIVACY_CENTER__AUTOMATIC_SUBDOMAIN_COOKIE_DELETION
? process.env
.FIDES_PRIVACY_CENTER__AUTOMATIC_SUBDOMAIN_COOKIE_DELETION === "true"
: false,
eastandwestwind marked this conversation as resolved.
Show resolved Hide resolved
};
return settings;
};
Expand Down
2 changes: 2 additions & 0 deletions clients/privacy-center/pages/api/fides-js.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,8 @@ export default async function handler(
base64Cookie: environment.settings.BASE_64_COOKIE,
fidesPrimaryColor: environment.settings.FIDES_PRIMARY_COLOR,
fidesClearCookie: environment.settings.FIDES_CLEAR_COOKIE,
automaticSubdomainCookieDeletion:
environment.settings.AUTOMATIC_SUBDOMAIN_COOKIE_DELETION,
},
experience: experience || undefined,
geolocation: geolocation || undefined,
Expand Down
Loading