[go: up one dir, main page]

Page MenuHomePhabricator

[Epic] IP Address Reveal for Privileged Users
Open, Needs TriagePublic

Assigned To
Authored By
Niharika
Dec 14 2022, 8:20 PM
Referenced Files
F36353886: image.png
Jan 20 2023, 2:38 PM
F36352837: image.png
Jan 20 2023, 2:38 PM
F36347039: image.png
Jan 20 2023, 12:52 PM
F35881762: image.png
Dec 20 2022, 9:47 PM
F35881745: image.png
Dec 20 2022, 9:47 PM
F35881742: image.png
Dec 20 2022, 9:47 PM

Description

Motivation

Once IP Masking goes into effect, IP addresses will be hidden from most users. Users with certain privileges will continue to be able to view IP addresses. There are a few different ways this will happen. This ticket lists out all the conditions under which IPs will be revealed and who will be able to reveal the IPs.

Who can see IPs?
  • Admins, Bureaucrats, Oversight, Stewards, Checkusers
    • who opt-in to seeing IP addresses (agree to terms)
    • This needs a new preference (T325451) that Legal will decide the text for.
  • Patrollers who meet the following conditions:
    • Condition 1: Will need to meet some TBD thresholds for account age and minimum edit count (T325451)
    • Condition 2: Will need to be granted the IP-viewer right by community consensus
    • Condition 3: Will need to explicitly opt-in to viewing IP addresses (agree to terms)
Where are IPs exposed?
  • action=history
  • Special:Contributions
  • Special:Log (including all subpages where temp accounts are visible)
  • Special:Watchlist
  • Diff page
  • Special:Block T324602
  • Special:RecentChanges (maybe Growth?)
  • Possibly other similar pages that we discover along the way.

Note: IP Revealing in content and talk pages will be tackled in a separate ticket.

How are IPs revealed?

On all other pages users with access to IPs will be able to reveal all IPs for a given temp account. If they click "Show IP" for a temporary account, it will reveal all instances of that temp account on that page irrespective of the IP address. IPs associated with that temporary username will stay revealed on that page and subsequent pages visited for a period of 24 hours after which they will need to reveal that temp username once more.

image.png (802×1 px, 268 KB)
image.png (932×1 px, 221 KB)
Sample IP Reveal mockup for log, watchlist, historySample IP Reveal mockup for contributions
Do revealed IP addresses persist?

Yes. For admins and checkusers, all temp accounts once revealed will stay revealed even when the user moves across pages. They will stay revealed for 24 hours.
For patrollers temp-account-IP address pairs once revealed will stay revealed even when the user moved across pages. This will stay revealed for 24 hours.

What is logged?

Details in T325658: Log access to IP addresses of temporary accounts
Log 1: Activation/deactivation of access

  • Who activated/deactivated access to IP addresses
  • When they received/revoked the access (timestamp)

Log 2: Log of actions taken

  • Temp username that was revealed
  • Performer for the reveal
  • Timestamp of this action
  • Page

These logs will be retained indefinitely.

Related Objects

StatusSubtypeAssignedTask
Resolvedkostajh
DeclinedNone
In ProgressNiharika
OpenNiharika
Resolved Tchanders
Resolved Tchanders
Resolved Tchanders
ResolvedCyndymediawiksim
InvalidBUG REPORTNone
Resolved Tchanders
ResolvedSTran
ResolvedSTran
ResolvedSTran
Resolved AGueyte
Resolved Tchanders
ResolvedBUG REPORTAmmarpad
Resolved Tchanders
Resolved Tchanders
DeclinedBUG REPORT TThoabala
ResolvedBUG REPORTCyndymediawiksim
ResolvedBUG REPORTCyndymediawiksim
ResolvedBUG REPORT Tchanders
ResolvedBUG REPORTJayCano
Resolved Tchanders
Resolved Tchanders
ResolvedBUG REPORTCyndymediawiksim
DeclinedNone
ResolvedSTran
DuplicateNone
ResolvedSTran
DeclinedSTran
Resolved TThoabala
ResolvedBUG REPORT AGueyte
ResolvedSTran
ResolvedBUG REPORT AGueyte
ResolvedSTran
OpenNone
DuplicateNiharika
DeclinedNone
DuplicateNone
Duplicate Prtksxna
DuplicateNone
OpenNone
Resolved Tchanders
OpenKColeman-WMF
OpenNone
OpenNiharika
OpenBUG REPORTNone
ResolvedBUG REPORTDreamy_Jazz
ResolvedDreamy_Jazz
In ProgressAmdrel
OpenNone
ResolvedSTran
Resolved Tchanders

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

@Niharika Thanks for adding the screenshots. I have some more questions below, about how the UI works.

image.png (802×1 px, 268 KB)

  • Is the grey area a button? If so:
    • What does is do for a single-reveal user?
    • What does it do for a bulk-reveal user?
  • Is the temp account username a link? If so:
    • Does it link to Special:Contributions?
  • If yes to both the above, how does a link within the button work?
  • How will this list look to someone without the right to reveal IP addresses?
  • How will this list look once IP addresses have been revealed?

image.png (422×776 px, 79 KB)

  • What does the "Show IPs" button at the top do?

@Niharika Thanks for adding the screenshots. I have some more questions below, about how the UI works.

image.png (802×1 px, 268 KB)

  • Is the grey area a button? If so:
    • What does is do for a single-reveal user?
    • What does it do for a bulk-reveal user?
  • Is the temp account username a link? If so:
    • Does it link to Special:Contributions?
  • If yes to both the above, how does a link within the button work?
  • How will this list look to someone without the right to reveal IP addresses?
  • How will this list look once IP addresses have been revealed?

These are good questions. Prateek was imagining the temp username to have a grey background (not necessarily be a button). For all temp users, there will be a "show IP" button/link next to the temp username that will unmask the IP address. I don't have a mockup for this exactly but it will look like the "show IP" link in the second mockup:

image.png (932×1 px, 221 KB)

And once, clicked, that "show IP" link/button will be replaced by the respective IP address:
image.png (802×1 px, 268 KB)

Bulk reveal will not work outside of Checkuser/Investigate (See task description). For others:

  • Temp user reveal: For admins and checkusers, revealing a temp account will unveil all instances of that temp account on that page irrespective of the IP address.
  • Pair (temp user-IP) reveal: patrollers will only be able to reveal a single "temp account - IP address" pair at a time. In other words, revealing a temp account will unveil all other temp account instances on that page that are from the same IP address.

Someone without the right to reveal IPs will not see the "show IP" links.

image.png (422×776 px, 79 KB)

  • What does the "Show IPs" button at the top do?

This show IPs button exists for Bulk reveal on Checkuser and Special:Investigate. It unmasks all IPs for all temp users on that page.

Rough estimates based on early analysis. We don't expect these to be very accurate.

  • Making the Show IP button work - 8
  • Adding an API endpoint that takes multiple temp user accounts - 8
    • @Niharika to start a convo with Legal about this (checkuser and investigate)
  • action=history
    • Adding the "Show IP" button - 3
  • Special:Contributions
    • Depends on IP Info workflows on that page
    • Possibly a 5
  • Special:Log (including all subpages where temp accounts are visible)
    • Adding the "Show IP" button - 3
  • Special:Watchlist
    • Adding the "Show IP" button - 5
  • Diff page
    • Adding the "Show IP" button - 5
  • Special:Checkuser
    • Adding the "Show IP" button and "Show all IPs" (bulk reveal) - 13
  • Special:Investigate
    • Adding the "Show IP" button and "Show all IPs" (bulk reveal) - 13
  • Special:Block T324602: SpecialBlock: Once a temporary account is selected, below the username field display IP addresses associated with the account
    • Estimate 5
  • Special:RecentChanges (maybe Growth?)
    • Adding the "Show IP" button - 5

Rough estimates based on early analysis. We don't expect these to be very accurate.
....

  • Special:Checkuser
    • Adding the "Show IP" button and "Show all IPs" (bulk reveal) - 13
  • Special:Investigate
    • Adding the "Show IP" button and "Show all IPs" (bulk reveal) - 13

Apologies for the length of this comment, but wanted to explain my reasoning behind each thought and question. Each question is a numbered item. The bullet point explains info behind the question and what the answer would mean.

Some thoughts and questions on adding the show IP buttons to both interfaces:

  1. Will the button apply only to temporary accounts, or to all accounts?
    • If the former, fully registered accounts having their IPs shown by default but not temporary accounts is a bit of an inconsistency.
    • If the latter, this represents a change to both interfaces that is likely to meet opposition and will also probably need consultation.
  2. Logging use of the button in both these interfaces means that those without checkuser-log but access to the IP revealing log can see partial information about checks made in these interfaces.
    • While I think the groups allowed access to the log will already have checkuser-log globally, if one group does not then they can see for checks which had either button pressed:
      1. The performer of the check
      2. A timestamp which is close to when the check was run
      3. The temporary users who showed up in said check
      4. The IPs of the temporary users that were revealed in said check
    • IMO, this represents a leak of info and either users who can see the IP reveal log must also have checkuser-log or log entries associated with checks be hidden unless the user also has checkuser-log.

Some thoughts on adding the buttons to Special:CheckUser:

  1. If a checkuser was to check a IP address or range, would the buttons be shown or will the IPs of the temporary users be shown by default?
    • This is because a CheckUser running a check using 'Get edits' or 'Get users' on a single IP would then already know the IP address of the temporary user as it will be the IP address that was checked. That makes the reveal button provide no more information to the CheckUser than they already know.
  2. Will pressing either show IP button mean that new pages of results will show IPs for temp accounts not already shown previously?
    • If yes, then paging results would cause new log entries without pressing the "Show IPs" button. It also adds complexity as what was revealed will need to be stored by CheckUser.
    • If no, this adds many more steps to running a check that has a lot of pages.
  3. Will checking a temporary user with the 'Get IP addresses' check type show the buttons or display the IPs by default?
    • Running 'Get IPs' on a temporary user would not provide any results as this check type only shows the IPs used by the account. That means pressing "submit" for a check would show no results until one of these new buttons are pressed.

Some thoughts and questions related to Special:Investigate:

  1. Will pressing the button show IPs of temporary users for the entire check process, or will adding targets not show the IPs for the temporary users associated with just the new targets?
    • If the former, then because the log shows IPs revealed and temporary users the log will have to be updated with new entries when adding new targets that include more temporary users without the user pressing the button
    • If the latter, this adds extra complexity (as the system will need to keep a track of what was revealed and what was not) and also means the checkuser will potentially have to press the button multiple times.

The same thoughts on Special:CheckUser and generally apply to the CheckUser API which is not mentioned here, but also:

  1. Would running a check using the API automatically show IPs for temporary users, or would a separate API call be needed to reveal each temporary user's IP?
    • If the former, this means log entries will be made without directly calling the API to reveal IPs for temporary users
    • If the latter, this should be an okay thing but would add extra API calls.

My position is that the "Show IPs" or "Show all IPs" buttons should not exist in either Special:CheckUser, Special:Investigate or the API (via extra API calls to reveal the IP). This is because temporary accounts should be treated like normal accounts, and normal accounts already have their IPs shown without this extra step. Arguably the security risk of seeing IPs of registered accounts (such as privileged accounts or users who are at increased risk of doxxing attempts) is worse than being able to see IPs of temporary accounts, so if anything it should be harder to see IPs of registered accounts over temporary accounts.

Thanks for your detailed comment, @Dreamy_Jazz. I agree with you. I discussed this with @Tchanders today and we both agree that it makes sense to drop the idea of "Show IPs" buttons in Special:CheckUser and Special:Investigate because that would not add anything of value..

@Bugreporter, anything I can help with (specifically anything related to CheckUser)? If so, if you could add me to the task.

@RHo I have a design question about the (Show IP) buttons:

image.png (127×773 px, 23 KB)

In the screenshots they are styled like links, but they behave as buttons. They don't navigate to another page like links; instead they fetch some data and update the appearance of the page when clicked.

Technically it's a bit awkward to do this. We can make a button using the UI component library, and tell it what to do when a user clicks. But to style this to look like a link would take a lot of custom CSS, which can end up being fragile. For me, whenever we seem to be fighting the component library it raises the question: should we be doing this (i.e. should we be styling a button to look like a link)? Do we have any style guidelines about whether the user should be able to distinguish a link from a button by their appearance?

@RHo I have a design question about the (Show IP) buttons:

image.png (127×773 px, 23 KB)

In the screenshots they are styled like links, but they behave as buttons. They don't navigate to another page like links; instead they fetch some data and update the appearance of the page when clicked.

Technically it's a bit awkward to do this. We can make a button using the UI component library, and tell it what to do when a user clicks. But to style this to look like a link would take a lot of custom CSS, which can end up being fragile. For me, whenever we seem to be fighting the component library it raises the question: should we be doing this (i.e. should we be styling a button to look like a link)? Do we have any style guidelines about whether the user should be able to distinguish a link from a button by their appearance?

Hi @Tchanders - this is something that has come up recently relating to these "buttons styled as links" being in many places on wiki. I know of one task T320798 ongoing to stop adding a visited colour style to these elements, with examples being hide in the table of contents on pages, thank, undo, and the reply action on old Talk pages.

Some initial ideas about how to add the "show IP" action when it is appearing after the temp account name inline:

  • Proposal 1 (short term): Add square brackets – Given historically the amount of places where these button links are used, we could keep this convention for now but surround it with square brackets to help indicate it is meant to be an action, i.e., [ show IP ]. It's less disruptive and follows what similar actions do, but seems not great to add to list of not-best-practice applications.
  • Proposal 2 (short term fix): Quiet button – Continuing to use OOUI library, DiscussionTools chose to use a quiet neutral button for the 'reply' action. We could do this too, and experiment with using a quiet icon with or without an icon to better distinguish it as an action. The trade off is possible it will visually bit off in certain places where it will be shown, possibly increasing line-spacing. It will also stand out from other actions like undo and thanks in cerrtain places.
Example of Reply quiet button on DT
image.png (106×582 px, 24 KB)
Quick mock of how this could look in edit history
image.png (82×836 px, 19 KB)

My inclination is proposal 2, and flag this as a possible new variant for the Codex folks to to consider for the button component - small buttons shown with inline text.
What do you think?

My inclination is proposal 2, and flag this as a possible new variant for the Codex folks to to consider for the button component - small buttons shown with inline text.
What do you think?

Just noting here that we agreed on option 2 in our meeting (and having discussed with @Niharika too) - thanks @RHo!

My inclination is proposal 2, and flag this as a possible new variant for the Codex folks to to consider for the button component - small buttons shown with inline text.
What do you think?

Just noting here that we agreed on option 2 in our meeting (and having discussed with @Niharika too) - thanks @RHo!

Discussed this with Rita yesterday to confirm. We're good to go with option 2 with a capital "S" on "Show IP".

I have an update on these requirements after talking through this at length with legal, engineering and design.

For context:

Here's the update:

  • We keep the reveal workflows consistent for admins and patrollers. For both groups, revealing an instance of the temporary username will reveal all IPs associated with that username on that page and subsequent pages visited for a period of 24 hours after which they will need to reveal that temp username once more.
  • We will drop the IP addresses from the log and persist the logs forever. If we aren't selectively revealing IPs for patrollers we likely don't need to explicitly track which IPs were revealed. This isn't perfect of course because IPs would be stored for 90 days only and by the time an oversighter comes by we may not have all the same IPs available in the database. However, this should ease oversight because logs will persist forever.

Questions? Concerns? @JJMC89, @Dreamy_Jazz?

Here's the update:

Thanks for this. I have no questions or concerns. Rest of this comment is why I agree with these changes.

  • We keep the reveal workflows consistent for admins and patrollers. For both groups, revealing an instance of the temporary username will reveal all IPs associated with that username on that page and subsequent pages visited for a period of 24 hours after which they will need to reveal that temp username once more.

I like this change. No concerns from what I see. Agree with the points that have been raised regarding the complication between the different levels of access that I've been seeing here and at gerrit. Also this should make the volume of log entries generated be greatly reduced which will help in performing oversight.

  • We will drop the IP addresses from the log and persist the logs forever. If we aren't selectively revealing IPs for patrollers we likely don't need to explicitly track which IPs were revealed. This isn't perfect of course because IPs would be stored for 90 days only and by the time an oversighter comes by we may not have all the same IPs available in the database. However, this should ease oversight because logs will persist forever.

I think any need to see the IP that was revealed is not as important as keeping the logs past 90 days, so I welcome this change. Having the performer of the reveal, the temporary user who was revealed and the timestamp associated with this should be enough to properly oversight the use of the tool.

  • We will drop the IP addresses from the log and persist the logs forever. If we aren't selectively revealing IPs for patrollers we likely don't need to explicitly track which IPs were revealed. This isn't perfect of course because IPs would be stored for 90 days only and by the time an oversighter comes by we may not have all the same IPs available in the database. However, this should ease oversight because logs will persist forever.

I think any need to see the IP that was revealed is not as important as keeping the logs past 90 days, so I welcome this change. Having the performer of the reveal, the temporary user who was revealed and the timestamp associated with this should be enough to properly oversight the use of the tool.

I agree. This is consistent with what is stored in the CU log.

Note in T356294#9504584 I propose the current Wikimedia Access to Temporary Account IP Addresses Policy be amended.

Will this tool restore the ability to get results currently available from Special:Contributions/IP/Mask -- frequently used by admins when considering and managing rangeblocks.

Will this tool restore the ability to get results currently available from Special:Contributions/IP/Mask -- frequently used by admins when considering and managing rangeblocks.

There will be a way of seeing contributions from temporary accounts from a given IP range, but exactly how is under discussion on T356290#9565086.

That is a subtask of the global user contributions work that we're doing - the same idea but for global contributions.