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

Add option to set the domain of the SOI cookie #295

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add option to set the domain of the SOI cookie #295

wants to merge 1 commit into from

Conversation

alexgit2k
Copy link

This is a rebased pull-request of #244 from @cphilleo.

The Problem

Multiple sites on sub-domains would like to share consent, e.g. sub1.example.com, sub2.example.com, sub3.example.com. This can be achieved by using the POI feature and specifying a common domain such as consent.example.com however there is overhead of at least 20K per page load to use POI.

Proposal

Since subdomains can set a shared cookie on the parent domain, it's more efficient to allow the SOI cookie to be set on the parent domain, and then be shared by all subdomains. This would be a form of group consent.

This pull request adds an option to set the domain to be used for the SOI cookie. Setting the cookie to an invalid non-parent domain results in no cookie being set.

@edubz99
Copy link

edubz99 commented Apr 20, 2022

This is solving an issue that we are currently facing, any chance this can be merged in ?

// decoded consent data must not be written to the cookie
delete value.consentData;
Cookie.set(name, value, { expires: expires_in_days });
Cookie.set(name, value, { expires: expires_in_days, domain: domain });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before i merge this, what happens when domain is an empty string? Would it perhaps be better to leave out the domain parameter altogether in that case?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nevermind my write access was revoked

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants