Include Session Tokens as personal information in data exports and erasure

[lakenh]

#44161 raised some concerns about if we missed any personal data when the personal information export was released. Upon further investigation, the core-privacy team found multiple places in the user_meta table that still contains information that we should include in exports.

The currently known ones are the following:

• Session Tokens: Contains IP address and user agent
• Community Events: Contains IP address

*Community Events data will be handled via #43921

The scope of this ticket isn't about removing/anonymizing this information, instead just including it within the current user export and erasure tools.

[desrosj]

One thing to be careful with here is the erasing the IP and user agent information used for sessions would cause an issue with sessions becoming invalidated. This would need to either be communicated to the user or, instead of erasing in the current process, a second step could be provided for erasing the sessions.

[garrett-eclipse]

One question I had about the use of IP in the Community Events is would an anonymized IP be sufficient to geolocate an area to surface community events from? If so we could avoid needing to include it in export and erasure by anonymizing it prior to storing in the usermeta table which would make it no longer PII. Just a thought.

With Session Tokens I don't believe they would be as secure if the IP was anonymized and might not work at all so wouldn't go down that route with them.

[garrett-eclipse]

As #43921 already exists with an existing patch we'll continue work for the Community Events Location information through that ticket. As such I've updated this ticket to change it's focus to be specific to Session Tokens.

And to answer my question from previous garrett-eclipse:

One question I had about the use of IP in the Community Events is would an anonymized IP be sufficient to geolocate an area to surface community events from? If so we could avoid needing to include it in export and erasure by anonymizing it prior to storing in the usermeta table which would make it no longer PII. Just a thought.

*This was answered on the other ticket indicating that the IP address is already partially anonymized as was indicated on this ticket comment;
https://core.trac.wordpress.org/ticket/40794#comment:22

[nickylimjj]

Added a patch to export session tokens data. $token-props defined based on ticket but would require investigating into where in codebase it is defined. Did a cursory check but can't seem to find it in the __constructor for abstract base class WP_Session_Tokens nor class WP_User_Meta_Session_Tokens.

[nickylimjj]

Replying to nickylimjj:

Added a patch to export session tokens data. $token-props defined based on ticket but would require investigating into where in codebase it is defined. Did a cursory check but can't seem to find it in the __constructor for abstract base class WP_Session_Tokens nor class WP_User_Meta_Session_Tokens.

To any helpful debugger, the attachment is an example of the key-values for the session_tokens metadata if anyone is familiar where value's type is defined.

[rconde]

Thanks @nickylimjj it would be great to have this feature included in a next version of WP, as IP addresses fall as private data under the GDPR.

Your patch works great so at least it's something to be fully compliant.

[garrett-eclipse]

Updated patch to isolate the Session Tokens to their own exporter group and introduce unit test coverage.

[garrett-eclipse]

Session Tokens export group

[garrett-eclipse]

Thanks @nickylimjj for getting this started, your patch worked nicely but I found the more tokens we get the more convoluted the value of that row becomes, as well the User profile information we latched in to is for the profile. I felt we should isolate the tokens to their own section so have refreshed your patch in 45889.3.diff to go that direction and provide a unit test for coverage.

Note: This follows the recent patch I placed on #43921

I also included in the export the expiration and last login date as that information can assist users attempting to use the data to vet if any malicious sessions exist.

Give it a test please and hopefully we can address this in 5.4.

[xkon]

Do we really need an extra group just for session data? We're adding a filter (core values are protected) at #47509 to have an easier way to include metadata that are simple & fast getters in the User meta section. Since sessions are also within usermeta and they should be in the same table IMHO.

On a further note since you can have multiple sessions per user, the table view from 45889.3.diff​ isn't optimal from a "reading" perspective, see session_tokens.png​ as there's no clear view of where 1 sessions data start and end.

I would prefer them under the existing "User meta data" (it's getting renamed on the ticket mentioned earlier from Users profile data to Users meta data) identified as Session Data group 1, Session data group 2 so on so forth, or with any other incremental name to again have an easier identification while being read by someone. We can explode the actual info with <br/> or something like that to separate the individual data per session but essentially keep them within the same <tr> of the table if that makes sense.

[garrett-eclipse]

Refreshed patch to separate individual session tokens within the group.

[garrett-eclipse]

Updated Session Tokens grouping

[garrett-eclipse]

Thanks for testing @xkon and the feedback.

The screenshot you provided was exactly why going with a grouping I feel will be easier to read and digest. When it was in a single table row inside the User Meta even with one token it's convoluted, as soon as you have 2+ it's impossible to distinguish them. I took that into account for 45889.4.diff so each token inside of the group will be slightly separated similar to comments. See screenshot.

Along with the TOC coming in #46894 by using a grouping we make it easier to find and digest this information. And currently I'm seeing the User grouping is labelled specifically for profile data which Session Tokens aren't apart of.

Let me know what you think.

[xkon]

Aha! Now it actually makes total sense @garrett-eclipse . My main concern will always be on how to make it as easy as possible to end-users who are reading unknown things already. After our Privacy bug-scrubs as well yes we're definitely on the same side on moving this forward as it is on its own section :-).

Thanks for the fast follow-up patch! I'll give it another run asap.

[xkon]

Re-tested the final patch and works as expected. Thanks for the refresh & breaking the groups up @garrett-eclipse ! Marking this for commit.

[SergeyBiryukov]

In 47237:

Privacy: Include session tokens in Personal Data Export.

Session tokens contain an IP address and user agent.

Props garrett-eclipse, nickylimjj, lakenh, xkon, rconde.
Fixes #45889.

[garrett-eclipse]

Dev Note: ​https://make.wordpress.org/core/2020/03/02/privacy-updates-in-5-4/