Change #1013544 had a related patch set uploaded (by STran; author: STran):

[mediawiki/extensions/AbuseFilter@master] [DNM] Add user_ip variable

https://gerrit.wikimedia.org/r/1013544

The simplest thing to do here is probably to continue exposing the IP. Right now these functions check the user_name variable but that variable can be anything:

ip_in_range

Returns true if user's IP (first string) matches the specified IP range (second string, can be in CIDR notation, explicit notation like "1.1.1.1-2.2.2.2", or a single IP). Only works for anonymous users. Supports both IPv4 and IPv6 addresses.

ip_in_ranges

Returns true if user's IP (first string) matches any of the specified IP ranges (following strings in logic OR mode, can be in CIDR notation, explicit notation like "1.1.1.1-2.2.2.2", or a single IP). Only works for anonymous users. Supports both IPv4 and IPv6 addresses.

Here's a quick proof of concept https://gerrit.wikimedia.org/r/c/mediawiki/extensions/AbuseFilter/+/1013544 which would allow the filters to continue working as they are:

ip_in_ranges(
    user_ip, // replacing user_name
    "1.2.3.4/16"
)

This should probably be lazy loaded instead and it would be ideal to only expose them to these filters. In the same vein, we should target the filters that use this data point and make them privileged through a mechanism like T234155: Create CheckUser-level abuse filters.

Thinking through the permissions issue that's being discussed on https://gerrit.wikimedia.org/r/c/mediawiki/extensions/AbuseFilter/+/1013544.

Who can work out a temp user's IP address from the user_ip variable?

Things users can work out:

1. Anyone who can see the filter can see the IP it's blocking
2. Anyone who can see the log can see the user who triggered the filter
3. Anyone who can see /log/examine can see the user_ip variable

This means anyone who can see the first two can work out a temp user's IP address. Anyone who can see the third one also can.

Who should be able to see a temp user's IP address?

This is defined in the access policy: https://foundation.wikimedia.org/wiki/Policy:Access_to_temporary_account_IP_addresses#Minimum_requirements_for_access

• Stewards and checkusers (and probably more groups TBC)
• Admins, if they accept an agreement
• Users who have met certain thresholds, if they accept an agreement

Note that access needs to be logged (exact legal requirements TBC, so I guess we can file a separate task and make sure this adheres to the policy in that task).

Can we add user_ip in a way that adheres to the access policy?

I don't think we can just add the variable without violating the policy. (Caveat: I'm not an AbuseFilter expert so I may have missed something!)

Currently anyone can see the above information (which obviously violates the policy).

Could we just ensure that filters that include user_ip are always private? No. In that case, anyone with abusefilter-view-private can see the above information. In practice (by looking at extension.json and mediawiki-config) this includes lots of users who aren't meant to be able to see temp user IPs without accepting an agreement. So even this would violate the access policy.

Could we only expose user_ip in /log/examine to users with abusefilter-privatedetails? Also no. Anyone who can see the filter and the log could still work out a temp user's IP address.

I think this means we do need to implement some extra level of filter privacy in order to introduce the user_ip variable.

How could the extra level of privacy work?

We'd need equivalent something equivalent to CheckUser's checkuser-temporary-account right in AbuseFilter. (Since we actually store the IPs in AbuseFilter, it makes sense to have the right in AbuseFilter too.)

Checkusers and stewards would get the right automatically, and sysops and other groups (tbc) could get the right if they accept the agreement.

It should not be possible to view both 1 and 2 without this right. It should not be possible to view 3 without this right.

I think this is different from what T234155 is asking for, since it would be available to less privileged users (to prevent the burden of filtering by IP address falling solely on checkusers).

I don't think we can just add the variable without violating the policy.

This is true. To clarify, when I said that that this maintained parity I meant that afaik we haven't rolled out temporary accounts yet so the variable doesn't expose anything that isn't already exposed. We would be able to add the variable but then making filters private would be a rollout blocker. I intended to tackle the two independently but don't feel strongly if you'd like me to implement the access restrictions as part of this ticket as well.

I think this is different from what T234155 is asking for, since it would be available to less privileged users (to prevent the burden of filtering by IP address falling solely on checkusers).

Does "it" refer to the right being proposed here? It's my hope we resolve the CheckUser component of that ticket as a freebie if we implemented a generalized way to enforce rights-based viewing of privileges which would then also neatly solve the issues that user_ip existing causes.

We would be able to add the variable but then making filters private would be a rollout blocker.

I think this is the key point here. I'm not sure if the user_ip variable is considered a testwiki blocker, but if we merge the patch as-is, then all of this work would become one.

I'm all for implementing this independently. I think we need the privacy levels first though, if we don't want this to become a blocker.

I think this is different from what T234155 is asking for, since it would be available to less privileged users (to prevent the burden of filtering by IP address falling solely on checkusers).

Does "it" refer to the right being proposed here? It's my hope we resolve the CheckUser component of that ticket as a freebie if we implemented a generalized way to enforce rights-based viewing of privileges which would then also neatly solve the issues that user_ip existing causes.

Yes sorry, the right. This approach sounds great.