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

PROD-1389 for TCF, any consent pref that is not defined on cookie should be assumed opt-out #4430

Merged
merged 4 commits into from
Nov 14, 2023

Conversation

eastandwestwind
Copy link
Contributor

@eastandwestwind eastandwestwind commented Nov 14, 2023

Closes https://ethyca.atlassian.net/browse/PROD-1389

Description Of Changes

For TCF, if we have a previously saved fides_consent cookie or a fides_string override, any consent pref that is not defined on cookie.tcf_consent should be assumed opt-out, instead of using the defaults on the experience.

Code Changes

  • Updates default consent for tcf_entities on experience to be opt-out if we have any previously saved consent prefs on either cookie.tcf_consent or a fides_string override (which actually gets put on the cookie too).
  • Update e2e tests

Steps to Confirm

Pre-Merge Checklist

Copy link

vercel bot commented Nov 14, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
fides-plus-nightly ⬜️ Ignored (Inspect) Visit Preview Nov 14, 2023 7:21pm

: // If experience contains a tcf entity not defined by tcfEntities, this means either:
// A) Most commonly, user has opted out, and opt-outs are not tracked by TC string. It's safe to assume this case.
// B) There is a new tcf entity that requires consent. In this case we would just resurface the banner
ConsentMechanism.OPT_OUT;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we only want this for the fides string override case. in other words, I think this change would break this flow:

  • No fides string override
  • User opts out of all purposes
  • A new legint purpose is added
  • On load, the new legint purpose would be opted out, even though in this case, because we have the cookie key consent, we actually know the user has not opted out.

I think this could be fixed with...

const defaultPreference = cookie.fides_string ? ConsentMechanism.OPT_OUT : item.default_preference;
const preference = Object.hasOwn(cookieConsent, item.id) 
                 ? transformConsentToFidesUserPreference(
                      Boolean(cookieConsent[item.id]),
                      ConsentMechanism.OPT_IN
                    )
                 : defaultPreference;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah this makes more sense, thanks @allisonking !

Copy link

cypress bot commented Nov 14, 2023

Passing run #5246 ↗︎

0 4 0 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

Merge 59b5e00 into 36fc5e7...
Project: fides Commit: 2dd7d6f59f ℹ️
Status: Passed Duration: 00:33 💡
Started: Nov 14, 2023 7:29 PM Ended: Nov 14, 2023 7:29 PM

Review all test suite changes for PR #4430 ↗︎

Copy link
Contributor

@allisonking allisonking left a comment

Choose a reason for hiding this comment

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

nice job turning this around so quickly! 💯

@eastandwestwind eastandwestwind merged commit 7dbd31b into main Nov 14, 2023
13 checks passed
@eastandwestwind eastandwestwind deleted the PROD-1389 branch November 14, 2023 19:41
eastandwestwind added a commit that referenced this pull request Nov 14, 2023
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.

2 participants