Opened 7 years ago
Closed 6 years ago
#7856 closed task (fixed)
Privacy: Review of cookie behavior
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | 4.0 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Core | Keywords: | early has-patch |
Cc: |
Description
Parent ticket: #7698. Related: #7827.
BP uses cookies in a number of ways. Some of them are critical to BP functionality, while others are for convenience only. Some are for logged-in users only, while others apply to all site visitors. Let's use the description of this ticket as an inventory of what we currently do. Please correct any mistakes I make. Afterward, I'll make suggestions about potential changes that would make it easier for sites to comply with GDPR etc.
===
Activity
Name: bp-activity-oldestpage
Description: bp-legacy only. Applies to all site visitors. Used to store the proper page of results for the next 'Load More' request.
Recommendation: There's no reason to store this in a cookie. It's reset between pageloads. We should remove it.
Name: bp-activity-filter
Description: bp-legacy only. Applies to all site visitors. Used to store the last-selected "Filter by"
Recommendation: I have always found this behavior a little useless. My preference is to remove it altogether. If people like it, we should make it logged-in user only, so that cookie approval can be consolidated.
Name: bp-activity-scope
Description: bp-legacy only. Applies to all site visitors. Used to store the last clicked activity tab - "My Groups", "Mentions", etc, and then switch to it on the next load.
Recommendation: see bp-activity-filter
Name: bp-activity-extras
Description: No idea what this does. Seems to have been added in [2477] just for extensibility.
Recommendation: Is it possible it's being used by a plugin? Since it's Legacy only, I'd say leave it, but make it logged-in only.
Messages
Messages has some cookie code for bp_messages_send_to
, bp_messages_subject
, and bp_messages_content
, but it appears to be unused. Probably no action needed here, and no reference necessary in the default privacy policy.
Core
Name: bp-message
and bp-message-type
Description: Used to store success/failure messages for template_notices
display on next pageload. In practice, it's generally logged-in users only who perform actions that would require this, though third-party plugins might violate this.
Recommendation: Keep.
Groups
Name: bp_new_group_id
, bp_completed_create_steps
Description: Used to store progress in the multi-step group creation process. Logged-in user only.
Recommendation: Keep.
Name: bp-groups-filter
, bp-groups-scope
, bp-groups-extras
Description: See similar activity filters.
Members
Name: bp-members-filter
, bp-members-scope
, bp-members-extras
Description: See similar activity filters.
Attachments (2)
Change History (10)
#2
in reply to:
↑ 1
@
7 years ago
Replying to r-a-y:
Name: bp-activity-oldestpage
Description: bp-legacy only. Applies to all site visitors. Used to store the proper page of results for the next 'Load More' request.
Recommendation: There's no reason to store this in a cookie. It's reset between pageloads. We should remove it.
We can remove the
bp-activity-oldestpage
cookie when the page is loaded, but we cannot outright remove it. See #7896.
Could it be made so it’s only for logged-in users, so that cookie approval can be consolidated?
#3
@
7 years ago
- Keywords early has-patch added
7856.diff is a first pass at refactoring the 'bp-x-scope' etc cookies. The basic idea is this:
- Only set the cookies for logged in users, but this behavior is configurable
- In addition to the cookies, all users will have their "preferences" stored in a JS variable, so that it'll persist for the pageload
- When fetching the "preferences" in order to set up the page or send the AJAX request, first try grabbing from the cookie (if cookie storage is enabled) but otherwise pull from the local JS variable
There could be edge cases not yet covered by this, but I think the general idea is correct. Could I get another set of eyes?
#4
@
6 years ago
7856.2.diff is an update of 7856.diff that removes Object.assign()
(I forgot that it's ES2015+ only). I've reviewed the approach more generally and I think it's a good one, so I'm going to go ahead and commit it. That way, I can apply the same thing to the activity directory filters, and we can have ourselves a pretty significant cookie win for 4.0.
Thanks for taking the time to audit our cookie usage!
As for:
We can remove the
bp-activity-oldestpage
cookie when the page is loaded, but we cannot outright remove it. See #7896.The other recommendations seem fine to me.