Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#44736 closed defect (bug) (fixed)

Comment cookies are completely disabled when "Show comments cookies opt-in checkbox" is not enabled

Reported by: pputzer's profile pputzer Owned by: pento's profile pento
Milestone: 5.1 Priority: normal
Severity: normal Version: 4.9.8
Component: Comments Keywords: has-patch
Focuses: privacy Cc:

Description

#44373 introduced a setting to disable the comment cookies opt-in checkbox introduced in 4.9.6. Unfortunately, the patch neglected to set $cookies_consent to true in that case in wp-comment-post.php. This means that when the setting (in options-discussion.php) is disabled, no cookies will be set.

To fix the issue, the line

$cookies_consent = ( isset( $_POST['wp-comment-cookies-consent'] ) );

should be changed to

$cookies_consent = ( isset( $_POST['wp-comment-cookies-consent'] ) || ! get_option( 'show_comments_cookies_opt_in' ) );

Attachments (2)

#44736.patch (478 bytes) - added by dhavalkasvala 5 years ago.
44736.diff (511 bytes) - added by desrosj 5 years ago.

Download all attachments as: .zip

Change History (41)

#1 @johnbillion
6 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.9.9

#2 @pputzer
6 years ago

  • Focuses privacy added

This ticket was mentioned in Slack in #core-privacy by pepe. View the logs.


6 years ago

#4 follow-up: @dhavalkasvala
5 years ago

  • Keywords has-patch added; needs-patch removed
Last edited 5 years ago by dhavalkasvala (previous) (diff)

#5 @dhavalkasvala
5 years ago

  • Keywords needs-patch added; has-patch removed

#6 in reply to: ↑ 4 ; follow-up: @pputzer
5 years ago

Replying to dhavalkasvala: Is there a reason you didn't use the line as suggested in the ticket description? I think it makes more sense semantically (since cookie consent is implied if the checkboxes are not displayed).

#7 in reply to: ↑ 6 ; follow-up: @dhavalkasvala
5 years ago

replying to @pputzer
i just checked for opt-in checkbox disable then cookies will be not set because there is a condition for comment_consent is not set.

Last edited 5 years ago by dhavalkasvala (previous) (diff)

#8 in reply to: ↑ 7 ; follow-up: @pputzer
5 years ago

Replying to dhavalkasvala:

replying to @pputzer
i just checked for opt-in checkbox disable then cookies will be not set because there is a condition for comment_consent is not set.

I'm not sure I understand this answer. Obviously, my original suggested solution also checks for the status of the option, but in the place where the $cookie_consent variable is set (because disabling the checkbox implies consent for cookies).

#9 in reply to: ↑ 8 ; follow-up: @dhavalkasvala
5 years ago

Replying to pputzer:
I think you say like cookies consent checkbox is not appear with status right?

I'm not sure I understand this answer. Obviously, my original suggested solution also checks for the status of the option, but in the place where the $cookie_consent variable is set (because disabling the checkbox implies consent for cookies).

#10 in reply to: ↑ 9 @pputzer
5 years ago

Replying to dhavalkasvala:

I think you say like cookies consent checkbox is not appear with status right?

I'm saying that semantically the option check should be moved to the definition of $cookie_consent:

$cookies_consent = ( isset( $_POST['wp-comment-cookies-consent'] ) || ! get_option( 'show_comments_cookies_opt_in' ) );

#11 follow-up: @dhavalkasvala
5 years ago

when opt-in disable still there is checkbox for cookies_consent is appear with status right in comment form.

#12 in reply to: ↑ 11 ; follow-up: @dhavalkasvala
5 years ago

Replying to @pputzer :

when opt-in disable still there is checkbox for cookies_consent is appear with status right in comment form you want that.

#13 in reply to: ↑ 12 @pputzer
5 years ago

Replying to dhavalkasvala:

Replying to @pputzer :

when opt-in disable still there is checkbox for cookies_consent is appear with status right in comment form you want that.

I am sorry, but I'm still uncertain what you mean by that.

#14 @pento
5 years ago

  • Milestone changed from 4.9.9 to 5.0.1

#15 follow-up: @ThemeZee
5 years ago

This should not be changed. Cookies should never be set without the consent of the user, because then it violates the GDPR. That was the reason to introduce the checkbox in the first place.

There should be two options:

1) Disable comment cookies completely

2) Enable the checkbox, which asks the user for consent to save comment cookies

Right now the current implementation does that. Changing it might lead to website owners violating the GDPR without knowing it, when they deactivate the checkbox.

Last edited 5 years ago by ThemeZee (previous) (diff)

#16 in reply to: ↑ 15 ; follow-up: @pputzer
5 years ago

Replying to ThemeZee:

There should be two options:

1) Disable comment cookies completely

2) Enable the checkbox, which asks the user for consent to save comment cookies

Right now the current implementation does that. Changing it might lead to website owners violating the GDPR without knowing it, when they deactivate the checkbox.

While I'm all for privacy by default, I disagree that the current implementation is tenable. The wording of the checkbox is completely misleading and there is the additional issue of backwards compatibility. You are right that @mirkoschubert requested exactly this functionality, but the discussion in the ticket veered in a different direction and the commit results don't match with that.

#17 in reply to: ↑ 16 ; follow-up: @mirkoschubert
5 years ago

I think, it needs a little bit of clarification... I completely agree with @ThemeZee on this one (what a surprise), because there are two ways that this would be GDPR compliant:

  1. The comment cookies should never be set in the first place.
  2. If comment cookies are being used, there should be a user consent - cf. Article 6(1)(a) GDPR. That's the checkbox for.

If the website owner only can choose to set the checkbox or not and the cookie would be set anyway, then the possibility of a violation of the GDPR is very high. Not everyone knows the GDPR as much as we do.

In addition, lawyers here in Germany tend to advice, that the user consent should be logged with date, time and IP for proofing purposes. :D

I'm very strict when it comes to privacy by default - and so is the European Union. So as a website owner I want to store as little personal information about the user as possible. To me, setting a cookie for storing information, only that users don't have to type their information twice is pretty useless.

You may be right, the wording of the new setting could be misleading. Maybe it should be clarified that the cookie will not be set if there's no checkbox. But the outcome should definitely be the same.

Last edited 5 years ago by mirkoschubert (previous) (diff)

#18 in reply to: ↑ 17 ; follow-up: @pputzer
5 years ago

Replying to mirkoschubert:

  1. If comment cookies are used, there should be a user consent - cf. Article 6(1)(a) GDPR. That's the checkbox for.

It could be argued that they are technically necessary for a part of the comment functionality and therefore covered under 6(1)(b). I'm not sure I agree with this argument, but I'd like to highlight that "consent" might not be the only reasonable lawful basis for data processing in this case.

#19 in reply to: ↑ 18 @mirkoschubert
5 years ago

Replying to pputzer:

Replying to mirkoschubert:

  1. If comment cookies are used, there should be a user consent - cf. Article 6(1)(a) GDPR. That's the checkbox for.

It could be argued that they are technically necessary for a part of the comment functionality and therefore covered under 6(1)(b). I'm not sure I agree with this argument, but I'd like to highlight that "consent" might not be the only reasonable lawful basis for data processing in this case.

Yes, you are right, there are more than one reasons for justification under Art. 6(1) GDPR. But 6(1)(b) requires a contract to be involved somehow. And a comment has nothing to do with any contract. ;) In addition, a technical necessity isn't given in this case. The commenter can obviously live without saving his data for the next time. It's convenient, not necessary. And if he (and the website owner) want this kind of behaviour, he can register as a subscriber in WordPress.

#20 @pento
5 years ago

  • Milestone changed from 5.0.1 to 5.0.2

#21 @pento
5 years ago

  • Milestone changed from 5.0.2 to 5.0.3

#22 @audrasjb
5 years ago

  • Milestone changed from 5.0.3 to 5.1

5.0.3 is going to be released in a couple of weeks. We are currently sorting the remaining tickets in the milestone. It doesn't appear that ticket can be handled in the next couple of weeks (still needs some discussion/validation between core-privacy focused experts). Let's address it in 5.1 which is coming in February. Feel free to change/ask to change the milestone if you think the issue can be quickly resolved.

This ticket was mentioned in Slack in #core-privacy by desrosj. View the logs.


5 years ago

@desrosj
5 years ago

#24 follow-up: @desrosj
5 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

#44736.patch does not apply correctly to trunk. @dhavalkasvala can you refresh it so it can be tested?

@pputzer can you confirm that 44736.diff is the change you are suggesting?

#25 in reply to: ↑ 24 @pputzer
5 years ago

Replying to desrosj:

#44736.patch does not apply correctly to trunk. @dhavalkasvala can you refresh it so it can be tested?

@pputzer can you confirm that 44736.diff is the change you are suggesting?

It is indeed the change I've been suggesting and it works as intended (I don't have a Trunk installation at the moment, but I applied it to my 5.0.3 installation).

Last edited 5 years ago by pputzer (previous) (diff)

#26 follow-up: @dhavalkasvala
5 years ago

@pputzer I'm just check with your suggestion with this code

<?php
$cookies_consent = ( isset( $_POST['wp-comment-cookies-consent'] ) || ! get_option( 'show_comments_cookies_opt_in' ) );

still cookies set for like name,email,website.

So find solution complete disable cookies solution pls can tell me for that is it right change or not?

Last edited 5 years ago by dhavalkasvala (previous) (diff)

#27 in reply to: ↑ 26 @pputzer
5 years ago

Replying to dhavalkasvala:

So find solution complete disable cookies solution pls can tell me for that is it right change or not?

I just re-read your patch #44736.patch and I think it is actually not doing the right thing - so @desrosj's patch 44736.diff of course behaves differently. I'll try to clarify this:

  • Status quo: if show_comments_cookies_opt_in is not set, no cookies are set (because $cookies_consent is false). Existing cookies are deleted.
  • @dhavalkasvala's proposed patch: if show_comments_cookies_opt_in is not set, no cookies are set (because $cookies_consent is false). Existing cookies are deleted in this case. However, if show_comments_cookies_opt_in is set, but no consent was given, existing cookies are not deleted anymore. (IMHO, that's wrong on both points.)
  • My suggestion/@desroj's patch: if show_comments_cookies_opt_in is not set, consent (or necessity) is implied and $cookies_consent is set to true - things work like they did before 4.9.6 introduced the cookies checkbox.
Last edited 5 years ago by pputzer (previous) (diff)

#28 follow-up: @dhavalkasvala
5 years ago

<?php
$cookies_consent = ( isset( $_POST['wp-comment-cookies-consent'] ) || ! get_option( 'show_comments_cookies_opt_in' ) );

@pputzer now i understand because i tested your code again so now your code working fine. Can i need to add patch for it again?

Last edited 5 years ago by dhavalkasvala (previous) (diff)

#29 in reply to: ↑ 28 @pputzer
5 years ago

Replying to dhavalkasvala:

@pputzer now i understand what you what do your code working fine. Can i need to add patch for it again?

@dhavalkasvala I don't think there is a need to port your patch to current trunk anymore.

@desrosj What's your take on this?

#30 @dhavalkasvala
5 years ago

  • Keywords needs-testing removed

#31 @dhavalkasvala
5 years ago

  • Keywords needs-testing added

This ticket was mentioned in Slack in #core-privacy by desrosj. View the logs.


5 years ago

#33 follow-up: @azaozz
5 years ago

  • Keywords close added

#43857 is making good progress and chances are it will be in 5.1. Then comment cookies won't be needed for showing comments that are awaiting moderation. This makes these cookies purely a convenience.

Re-reading the above comments and some related discussions in #core-privacy on Slack, the only way to comprehensively fix this would be to add yet another checkbox to the Settings->Discussion screen. That will cover all possible cases: enable/disable comment cookies, and enable/disable consent for comment cookies. Of course a plugin can do that too.

However having the option to silently enable these cookies may be seen as a step in the wrong direction from user privacy point of view. Also, yet another checkbox?

IMHO the current behavior is inline with both "Privacy by design" and "Decisions, not options".

#34 in reply to: ↑ 33 @pputzer
5 years ago

Replying to azaozz:

#43857 is making good progress and chances are it will be in 5.1. Then comment cookies won't be needed for showing comments that are awaiting moderation. This makes these cookies purely a convenience.

Re-reading the above comments and some related discussions in #core-privacy on Slack, the only way to comprehensively fix this would be to add yet another checkbox to the Settings->Discussion screen. That will cover all possible cases: enable/disable comment cookies, and enable/disable consent for comment cookies. Of course a plugin can do that too.

However having the option to silently enable these cookies may be seen as a step in the wrong direction from user privacy point of view. Also, yet another checkbox?

The point is, 4.9.7 silently dropped the cookies by disabling the option by default. At least, that change needs to be properly documented and the label of the settings checkbox adapted to make it clear that the cookies will only ever be set if you enable the comment form checkbox.

#35 @azaozz
5 years ago

Following the discussion in #43857, lets turn that cookies opt in checkbox "on" by default.

the label of the settings checkbox adapted to make it clear...

Sure. Any suggestions? Currently it is: Show comments cookies opt-in checkbox..

This ticket was mentioned in Slack in #core-privacy by desrosj. View the logs.


5 years ago

#37 @pento
5 years ago

  • Owner set to pento
  • Resolution set to fixed
  • Status changed from new to closed

In 44681:

Comments: Update the message shown when a comment is awaiting moderation.

If the commenter doesn't have cookies set, they won't see the comment preview again. Showing an expanded message will help offset any confusion if they revisit the site later, and their comment is still in moderation, but they can't see it anymore.

Props azaozz, pento.
Fixes #44736.

#38 @pento
5 years ago

  • Keywords needs-testing close removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

#39 @pento
5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 44683:

Comments: Default the show_comments_cookies_opt_in checkbox to enabled.

This also updates the option label, to clarify that it needs to be enabled for comment cookies to work.

Props azaozz, pento, dhavalkasvala, desrosj, pputzer, mirkoschubert, ThemeZee.
Fixes #44736.

Note: See TracTickets for help on using tickets.