PR#1176 "feat: Refactor GDPR stuff into a more readable consent class" accidentally broke my GDPR compliance

[grahamc]

In #1176, has_opted_out_capturing and has_opted_in_capturing changed behavior significantly.

Before that PR, those functions called hasOptedIn and hasOptedOut, which were defined as:

export function hasOptedIn(token: string, options: GDPROptions): boolean {
    return _getStorageValue(token, options) === '1'
}

and:

export function hasOptedOut(token: string, options: Partial<GDPROptions>): boolean {
    if (_hasDoNotTrackFlagOn(options)) {
        return true
    }
    return _getStorageValue(token, options) === '0'
}

but after that PR, those functions were redefined:

    public isOptedOut() {
        return (
            this.consent === ConsentStatus.DENIED ||
            (this.consent === ConsentStatus.PENDING && this.config.opt_out_capturing_by_default)
        )
    }

    public isOptedIn() {
        return !this.isOptedOut()
    }

The difference is subtle, but getStorageValue would return null or undefined if the value wasn't set. That means hasOptedIn and hasOptedOut could both return false. But now, isOptedIn is explicitly the opposite of isOptedOut. If there is no preference, isOptedOut will return false, and isOptedIn will return true. This is not correct, and breaks the behavior of has_opted_in_capturing().

Until earlier this year, the recommended way to implement a cookie banner was by checking !(posthog.has_opted_out_capturing() || posthog.has_opted_in_capturing()). Now, any site that has implemented this mechanism is accidentally violating the GDPR by not displaying the cookie banner, even when the user has not consented.

Further, has_opted_in_capturing() has undoubtedly been used in many places to explicitly turn on additional features and behavior that is predicated on the user having opted in.

My hope is that has_opted_in_capturing() will be corrected to represent the user's actual consent.

[MarconLP]

Hey @grahamc, super sorry for the breaking change. We've decided to have our consent state always be true or false, and you'll be able to control the default state using the opt_out_capturing_by_default config option. To resolve your issue, you'll need to change your cookie banner logic to something like the code below, and has_opted_in_capturing() should continue working correctly. Check out our cookie banner guide for more detailed instructions.

import { useEffect, useState } from "react";

export function cookieConsentGiven() {
  if (!localStorage.getItem('cookie_consent')) {
    return 'undecided';
  }
  return localStorage.getItem('cookie_consent');
}

export default function Banner() {
  const [consentGiven, setConsentGiven] = useState('');

  useEffect(() => {
    // We want this to only run once the client loads
    // or else it causes a hydration error
    setConsentGiven(cookieConsentGiven());
  }, []);

  const handleAcceptCookies = () => {
    localStorage.setItem('cookie_consent', 'yes');
    setConsentGiven('yes');
  };

  const handleDeclineCookies = () => {
    localStorage.setItem('cookie_consent', 'no');
    setConsentGiven('no');
  };

  return (
    <div>
      {consentGiven === 'undecided' && (
        <div>
          <p>
            We use tracking cookies to understand how you use 
            the product and help us improve it.
            Please accept cookies to help us improve.
          </p>
          <button type="button" onClick={handleAcceptCookies}>Accept cookies</button>
          <span> </span>
          <button type="button" onClick={handleDeclineCookies}>Decline cookies</button>
        </div>
      )}
    </div>
  );
}

[grahamc]

Hi @MarconLP thanks for the response. I know and saw that change to the documentation, but to be clear these functions are not doing what they say they do. A function named has_opted_in_capturing should not return true unless the user has in fact opted in. If that isn't the desired behavior of that function anymore, that function should not exist with that name.

This breaking change of these functions, published without release notes and with a PR body that says the author is "fairly sure it works as before and hopefully enough tests to prove it" is giving me pretty low confidence that this was intentional.

To be clear, I am unhappy to have discovered that my previously-GDPR compliant code was no longer, and all of a sudden I'm in violation of the law because I upgraded a point release of a library.

[benjackwhite]

Hey. Chiming in as the author of that original PR. Apologies that this broke things. Part of the changes there were intended to fix other peoples issues surrounding confusing configuration. The PR might sound like we were flippant but I did a lot of customer checking at the time and there was a range of different issues we were trying to overcome, with yours clearly one of the unintended side effects of it. Not trying to excuse, just trying to give the "rock and a hard place" context. Essentially the only change were the exposed top level functions where the underlying posthog functionality stayed the same but with fixes for some incorrect state management. The blind spot is essentially there where I didn't account for the use case you have using the exposed top level methods.

In terms of what to actually do here, I now have the new rock of reverting to the old implementation may break people who are now relying on this working the way it currently does 🤔 I need to do a bit of checking to see how bad reverting it would be as we may now create new issues for a bunch more people...

As an aside - generally I would never recommend powering your cookie banner off of posthog-js state, for the simple reason that we are not a consent management system (and posthog might be blocked, leading to your solution not showing and perhaps violating other tools using cookies etc.) I realise thats not directly the issue just felt it is worth mentioning.

[lucperkins]

@benjackwhite I appreciate your detailed response (I'm a colleague of @grahamc and was thus also bitten by this). We've all been there and it's no fun and I totally get that. But I do need to express that users who depend on the current behavior are, as they say nowadays, holding it wrong, and I don't think that any inconvenience caused by reverting the behavior should be factored into the decision about how to proceed. As for not using posthog-js as a consent management system, like it or not, posthog-js is the canonical way of determining on the client side whether a user has opted in to sending data to PostHog. And so any application that needs to make a consent/no-consent determination needs posthog-js to behave as expected.