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 #244

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

Conversation

cphilleo
Copy link

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.

@coveralls
Copy link

coveralls commented Apr 27, 2019

Pull Request Test Coverage Report for Build 937

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 351 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.05%) to 31.462%

Files with Coverage Reduction New Missed Lines %
dist/oil.bundle.js 351 31.46%
Totals Coverage Status
Change from base Build 936: 0.05%
Covered Lines: 1138
Relevant Lines: 2972

💛 - Coveralls

@cphilleo cphilleo force-pushed the feature/set-cookie-domain branch from 28b1465 to 0ce5b36 Compare May 9, 2019 20:43
@alexgit2k
Copy link

Good suggestion, I would need the same. Can you please resolve the error.

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.

3 participants