Page MenuHomePhabricator

Don't funnel all protected variable access logs to CheckUser temporary account access log
Closed, DuplicatePublic

Description

There is logic in place that currently results in log entries appearing in CheckUser when someone views a private filter with a protected variable. That's because the user_unnamed_ip protected variable is closely tied with temporary accounts access.

But for new protected variables, we don't need to log this type of access in CheckUser. We need to work out a solution for how to handle this.

Event Timeline

Given that this is coming up again, we should considering notating the consensus in a decision record. I looked up our original reasoning for doing this and found it in our check-in notes:

It probably makes sense to have a central log if possible for all temporary account IP address access (for the sake of not wanting to split it too much).

We wanted to consolidate ip view logs to checkuser-temporary-account instead of splitting it across that and abusefilter-protected-vars (the default if CheckUser isn't installed). I don't have a strong conclusion since the problem is that the protected variables have two different logging needs (one agnostic and one tied to our legal reqs) and no immediate solution seems like a clear winner although just returning to the expected default feels reasonable to me:

  • log everything to AF: it'd centralize all the AF logs but force investigations to check across the CU and AF logs. This might be ok? I don't think any other extension is expected to expose IPs like AF does so it would, in theory, only ever be split across those two.
  • log only user_unnamed_ip access to CU (eg. viewed ip info about <user> from abusefilter) and the rest to AF: we'd have to hardcode more wmf-specific logging logic into AF, AF logs are now split between the two
  • as above but instead log all general access to AF (eg. viewed protected vars associated with <user>): we double-log but maybe we could log something wmf-specific to CU and keep all the agnostic AF logs in one place. Writing this out, this makes some sense to me as the ip viewing logs have a different intention (legal) than the general logging we usually support.

Additionally, we debounce on the user - does this have to be reconsidered? eg. is it okay to log "viewed protected vars associated with <user>" regardless of what variable was accessed? Asking because we care different amounts for the two variables being discussed (specific ip vs information around an ip).

Do we need to create a log entry for the variables that are being added by MediaWiki-extensions-IPReputation? If not, then could we go with a fourth option of not logging when accessing anything other than user_unnamed_ip?

STran claimed this task.

This will be resolved by a combination of T387328: Refactor CheckUser ProtectedVarsAccessLogger logging out of AbuseFilter and T387329: Investigate: Confirm our logging expectations for AbuseFilter/CheckUser Temporary Accounts logs when using protected variables

I forgot this task existed, sorry! Closing this out as more context is available in the newer tickets.