PII is leaked across project deletion

[dw]

Describe the bug

• User A creates a project on Jan 11 2017 (pre-migration?)
• User B mails User A to ask for the project name, because it's unused.
• User A agrees, deletes project.
• User B runs "sdist upload" to register the name, becomes project admin.
• User B notices User A's IP addresses visible in project history tab.

Expected behavior

Sensitive information relating to the prior owner should probably be masked somehow, although I'm not sure what an ideal remedy would look like. Revealing user A's login history to user B without any pre-agreement is probably not desirable behaviour

It's worth note that while A and B had been communicating by e-mail in the example above, similar deletions and recreations are possible without A and B being aware of each other, or of B being aware of A, but A not being aware of B.

To Reproduce

See description.

Additional context

See warehouse user dw's recent project registration for an example.

[dw]

One simple fix might be to stop iterating change history rows after the first 'create' is encountered, unless the user has some magic superuser flag set.

[ewdurbin]

That solution seems reasonable. We generally don't want to purge all Journal entries for a Project on deletion given that they may be used for determining what happened and when after the fact.

There is also the case of a less abrupt transfer that I'm not sure how to approach.

• User A decides to transfer ownership of Project to User B
• User A adds User B as Owner for Project
• User B logs in and views Journal entries with User A's IP address.

This exposes similar information, and may not be anticipated either.

[dw]

Depending on how the code is structured, it might be possible to find the timestamp the user was added, and whiteout/omit any rows before that point?

[brainwane]

@nlhkabu could you take a look at this & #4440?

[brainwane]

@woodruffw @nlhkabu heads-up since this has to do with logging and viewing of sensitive events, and y'all will be working on a related feature in #5863.

[nlhkabu]

@brainwane - happy to take a look. Would this (and #4440) be considered within scope of the current contract?

[brainwane]

@nlhkabu I don't think addressing this PII issue is necessarily within the scope of the current contract. But I'm going to defer to @woodruffw, @ewdurbin and @di to figure out whether addressing this, or maybe instead making room architecturally to address it later, is part of #5863 and thus scoped within the current contract.

Am updating #4440 with a comment there. :) Thanks for checking!

[woodruffw]

I could be wrong, but I believe this is now addressed (not directly, but as a result of the IP masking changes).

[miketheman]

I could be wrong, but I believe this is now addressed (not directly, but as a result of the IP masking changes).

I believe this will be correct once we run a backfill to geo-hash all existing event data. Until then, older entries may display an IP address.