Page MenuHomePhabricator
Create Task
Maniphest T341228

Update action API to handle temporary users
Closed, ResolvedPublic1 Estimated Story Points
Actions

Assigned To
Dreamy_Jazz
Authored By
Tchanders
Jul 6 2023, 1:44 PM
Tags
Referenced Files
F42212417: image.png
Feb 28 2024, 1:45 PM
F42212385: image.png
Feb 28 2024, 1:45 PM
F42150110: image.png
Feb 26 2024, 4:22 PM
Subscribers
Aklapper
ARamirez_WMF
daniel
dom_walden
Dreamy_Jazz
JayCano
kostajh
View All 10 Subscribers

Description

The action API distinguishes between anonymous and registered users in a number of ways. We need to decide how to update these, given that temporary users will be considered a separate user type compared to anonymous and registered users (see also the discussion in T337103: Decide a standard approach for classifying temporary, IP and registered users).

  • Some modules flag or filter by anon users. Should there be a separate flag/filter for temporary users? Should we use user experience flags/filters instead? Examples:
    • ApiQueryUserInfo returns anon flag - T358683
    • ApiQueryWatchlist returns anon flag - T358693
    • ApiQueryContributors returns anons count
    • ApiFeedRecentChanges accepts hideanons param - T358249
    • T351636: APIs that return anon flag:
      • ApiQueryImageInfo
      • ApiQueryLogEvents
      • ApiQueryRecentChanges
      • anything that extends ApiQueryRevisionsBase
  • IP address blocks can be made against anonymous users only. How to update this will be worked out in T338836: How should blocks treat temporary users?.
  • ApiBlockInfoTrait returns blockanononly flag
  • ApiQueryBlocks and ApiBlock accept anononly param

These flags were documented as part of T343714: Soft blocks against an IP address should block temporary accounts using that IP address. We're unlikely to rename them, since it would be a large breaking change. If we were to rename them, it would be beyond the scope of this task.

  • T351632: ApiMain accepts assert param to define allowed callers (anon, user, or bot)
  • All action APIs can die with error code notloggedin. We could add an error code for temporary users who must register.

Will be covered in T339874: Show more helpful messages when temporary users can't access features, so no more to do in this task.

  • T350701: Action and REST APIs can specify allowed user types for a parameter, via UserDef::PARAM_ALLOWED_USER_TYPES. We should add a temp user type to this list:
UserDef::PARAM_ALLOWED_USER_TYPES
  'name': User names are allowed.
  'ip': IP ("anon") usernames are allowed.
  'cidr': IP ranges are allowed.
  'interwiki': Interwiki usernames are allowed.
  'id': Allow specifying user IDs, formatted like "#123".

Details

SubjectRepoBranchLines +/-
Update summary message for ApiQueryContributors for temp accountsmediawiki/coremaster+18 -3
Customize query in gerrit

Related Objects
Search...

Event Timeline

Tchanders created this task.Jul 6 2023, 1:44 PM
JJMC89 added a project: MediaWiki-Action-API.Jul 6 2023, 2:44 PM
Tchanders mentioned this in T326759: Investigate: Which WMF deployed code might be affected by IP Masking?.Sep 25 2023, 4:07 PM
kostajh subscribed.Sep 26 2023, 1:35 PM
Tchanders edited projects, added Trust and Safety Product Sprint; removed Anti-Harassment.Oct 31 2023, 7:18 PM
Tchanders edited projects, added Trust and Safety Product Sprint (Sprint Bodhrán); removed Trust and Safety Product Sprint.Nov 7 2023, 2:15 PM
Tchanders edited projects, added Trust and Safety Product Sprint; removed Trust and Safety Product Sprint (Sprint Bodhrán).Nov 7 2023, 4:08 PM
Tchanders updated the task description. (Show Details)Nov 20 2023, 12:16 PM
Tchanders closed subtask T351632: Separate out temporary users from 'assert' param in ApiMain as Declined.Nov 28 2023, 2:51 PM
Tchanders closed subtask T351636: Add `temp` flag to various APIs as Resolved.Jan 30 2024, 4:51 PM
Tchanders closed subtask T350701: [S] Add a temp user type to UserDef::PARAM_ALLOWED_USER_TYPES as Resolved.Jan 30 2024, 4:55 PM
Tchanders updated the task description. (Show Details)Feb 5 2024, 4:52 PM
Tchanders edited projects, added Trust and Safety Product Sprint (Sprint Piano (Feb 19th - 1st March)); removed Trust and Safety Product Sprint.Feb 20 2024, 8:34 PM
Tchanders moved this task from Priority Backlog to Ready on the Trust and Safety Product Sprint (Sprint Piano (Feb 19th - 1st March)) board.Feb 21 2024, 10:21 AM
Tchanders edited projects, added Trust and Safety Product Sprint; removed Trust and Safety Product Sprint (Sprint Piano (Feb 19th - 1st March)).Feb 21 2024, 2:48 PM
Tchanders edited projects, added Trust and Safety Product Sprint (Sprint Gangan (11th - 22nd March)); removed Trust and Safety Product Sprint.Feb 21 2024, 3:56 PM
Tchanders added a subscriber: Dreamy_Jazz.
Tchanders edited projects, added Trust and Safety Product Sprint (Sprint Piano (Feb 19th - 1st March)); removed Trust and Safety Product Sprint (Sprint Gangan (11th - 22nd March)).Feb 22 2024, 1:39 PM
Dreamy_Jazz added a comment.Feb 22 2024, 4:35 PM

The watchlist / recent changes was updated to treat hideanon as also including temporary accounts in T343322. Therefore for the ApiFeedRecentChanges API all we should need to do is update the help description for the hideanon API parameter, as the API uses the same code that was updated in T343322 based on my local testing.

Dreamy_Jazz updated the task description. (Show Details)Feb 22 2024, 4:58 PM
Dreamy_Jazz closed subtask T358249: Update ApiFeedRecentChanges for temporary accounts as Resolved.Feb 23 2024, 11:16 AM
Dreamy_Jazz updated the task description. (Show Details)Feb 26 2024, 3:40 PM
Dreamy_Jazz claimed this task.EditedFeb 26 2024, 4:22 PM
Dreamy_Jazz moved this task from Ready to Needs review on the Trust and Safety Product Sprint (Sprint Piano (Feb 19th - 1st March)) board.

I propose that the ApiQueryContributors API needs no changes for temporary accounts. The reasons for this are below.

The anoncontributors returns the count of edits made by IPs but not the IPs used. This currently does not include the number of temporary accounts, as they are listed in the contributors array under their username and user ID. The commit which added this API talked about how having a a list of all non-anonymous contributors to the page is useful and that the reason why non-anonymous contributors were instead counted was because it was not realistically possible without further schema changes (e06ad6841901df582990718f885a5a655d8c7748).

As such, using a count over a list of the temporary accounts who have made edits is probably less useful for users of this API. Furthermore, the data the API provides in the contributors array is just the username and user ID (which temporary accounts have) so no data is left out when comparing named users to temporary accounts.

Splitting the contributors list so that temporary accounts have their own list is in my own opinion unnecessary (due to there being no difference in the data returned between named and temporary accounts) and because callers would need to update their code to read from this array too.

Therefore, I feel that the temporary accounts being in the contributors array is the best way forward and as this is the current behaviour, no changes should be needed to the code.

The API currently returns the following data which I think is reasonable:

{
    "continue": {
        "pccontinue": "2|23",
        "continue": "||"
    },
    "query": {
        "pages": [
            {
                "pageid": 2,
                "ns": 1,
                "title": "Talk:Main Page",
                "anoncontributors": 1,
                "contributors": [
                    {
                        "userid": 16,
                        "name": "Dreamyjazz"
                    },
                    {
                        "userid": 24,
                        "name": "*Unregistered 1"
                    },
                    {
                        "userid": 35,
                        "name": "*Unregistered 4"
                    },
                    {
                        "userid": 36,
                        "name": "*Unregistered 5"
                    },
                    {
                        "userid": 37,
                        "name": "*Unregistered 6"
                    },
                    {
                        "userid": 39,
                        "name": "*Unregistered 8"
                    },
                    {
                        "userid": 40,
                        "name": "*Unregistered 7"
                    },
                    {
                        "userid": 41,
                        "name": "*Unregistered 9"
                    },
                    {
                        "userid": 42,
                        "name": "*Unregistered 10"
                    },
                    {
                        "userid": 43,
                        "name": "*Unregistered 11"
                    }
                ]
            }
        ]
    }
}

The API description probably doesn't need a change either because it talks about how the contributors array is for logged-in users and AFAIK temporary accounts would be included in this description. This is shown in the screenshot below:

image.png (186×1 px, 19 KB)

Therefore, given the above and that all other points in the ticket description were done in other tasks, this task can probably be closed as resolved. Moving to Needs review for thoughts on this.

Tchanders added a comment.Feb 26 2024, 4:38 PM

Thanks @Dreamy_Jazz.

Therefore, I feel that the temporary accounts being in the contributors array is the best way forward and as this is the current behaviour, no changes should be needed to the code.

I agree that we'd be taking information away if we hid the temp account contributions behind the anon count.

I suppose we could make a separate the temporary users into a separate list ("anoncontributors", "temporarycontributors", "anoncontributors"), but I'm happy to do nothing unless we hear that there's a need for that.

image.png (186×1 px, 19 KB)

We could add something about temporary accounts, just to be abundantly clear. Although temporary users are technically logged in, some might find the term "logged-in" ambiguous, in which case they might not know without trying whether temp users would be in the user list or the anon user count. Something like "Get the list of logged-in contributors (including temporary users) and the count of anonymous contributors to a page."?

gerritbot added a comment.Feb 26 2024, 5:53 PM

Change 1006556 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/core@master] Update summary message for ApiQueryContributors for temp accounts

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

gerritbot added a project: Patch-For-Review.Feb 26 2024, 5:53 PM
Dreamy_Jazz added a comment.Feb 26 2024, 5:56 PM
In T341228#9577065, @Tchanders wrote:

image.png (186×1 px, 19 KB)

We could add something about temporary accounts, just to be abundantly clear. Although temporary users are technically logged in, some might find the term "logged-in" ambiguous, in which case they might not know without trying whether temp users would be in the user list or the anon user count. Something like "Get the list of logged-in contributors (including temporary users) and the count of anonymous contributors to a page."?

I've updated the summary following your suggestion in the change https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1006556/

kostajh added a comment.Feb 26 2024, 6:03 PM
In T341228#9577065, @Tchanders wrote:

Thanks @Dreamy_Jazz.

I suppose we could make a separate the temporary users into a separate list ("anoncontributors", "temporarycontributors", "anoncontributors"), but I'm happy to do nothing unless we hear that there's a need for that.

That seems like it would be keeping the spirit of the existing anon/registered split of contributions to a particular page. That said, I don't really know who is using it or the motivations for why this anon count / anon/registered user split was introduced ~10 years ago in rMWe06ad6841901: API: Add prop=contributors. So, I agree with @Tchanders that we can leave it until someone asks for a change.

Dreamy_Jazz added a comment.Feb 27 2024, 4:47 PM
In T341228#9577547, @kostajh wrote:
In T341228#9577065, @Tchanders wrote:

Thanks @Dreamy_Jazz.

I suppose we could make a separate the temporary users into a separate list ("anoncontributors", "temporarycontributors", "anoncontributors"), but I'm happy to do nothing unless we hear that there's a need for that.

That seems like it would be keeping the spirit of the existing anon/registered split of contributions to a particular page. That said, I don't really know who is using it or the motivations for why this anon count / anon/registered user split was introduced ~10 years ago in rMWe06ad6841901: API: Add prop=contributors. So, I agree with @Tchanders that we can leave it until someone asks for a change.

Based on the commit summary it seems that it was because it wasn't possible without schema changes.

Dreamy_Jazz set the point value for this task to 1.Feb 28 2024, 12:12 AM
Dreamy_Jazz moved this task from Needs review to Needs QA on the Trust and Safety Product Sprint (Sprint Piano (Feb 19th - 1st March)) board.Feb 28 2024, 1:45 PM

Testing notes that I used to test this change which may be useful for QA:

  1. Enable temporary accounts on your wiki
  2. Open Special:ApiSandbox
  3. Select 'query' as the action and contributors as the prop
  4. Verify that the description looks like the following:

image.png (502×1 px, 66 KB)

  1. Disable temporary accounts on your wiki
  2. Repeat steps 2 and 3
  3. Verify that the description now looks like the following:

image.png (420×1 px, 47 KB)

Dreamy_Jazz updated the task description. (Show Details)Feb 28 2024, 1:45 PM
Dreamy_Jazz updated the task description. (Show Details)
Dreamy_Jazz updated the task description. (Show Details)Feb 28 2024, 1:57 PM
gerritbot added a comment.Feb 28 2024, 1:57 PM

Change 1006556 merged by jenkins-bot:

[mediawiki/core@master] Update summary message for ApiQueryContributors for temp accounts

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

Maintenance_bot removed a project: Patch-For-Review.Feb 28 2024, 2:31 PM
ReleaseTaggerBot added a project: MW-1.42-notes (1.42.0-wmf.21; 2024-03-05).Feb 28 2024, 5:01 PM
Tchanders updated the task description. (Show Details)Feb 28 2024, 5:10 PM
Tchanders updated the task description. (Show Details)
Tchanders added a comment.Feb 28 2024, 5:15 PM

ApiQueryWatchlist returns recent changes information, including an anon flag. I think this needs to stay as-is, to keep in line with RecentChanges - see T343322: IP Masking: Update Recent changes filters user registration filters for IP Masking. I wondered about documenting this somewhere, but I can't see anywhere obvious.

Here is where the flag is added: https://gerrit.wikimedia.org/g/mediawiki/core/+/277e4ea01e50d64768bfb5acd26f823df4dc58a3/includes/api/ApiQueryWatchlist.php#367

Tchanders added a comment.Feb 28 2024, 5:34 PM
In T341228#9584626, @Tchanders wrote:

ApiQueryWatchlist returns recent changes information, including an anon flag. I think this needs to stay as-is, to keep in line with RecentChanges - see T343322: IP Masking: Update Recent changes filters user registration filters for IP Masking. I wondered about documenting this somewhere, but I can't see anywhere obvious.

Here is where the flag is added: https://gerrit.wikimedia.org/g/mediawiki/core/+/277e4ea01e50d64768bfb5acd26f823df4dc58a3/includes/api/ApiQueryWatchlist.php#367

Actually, it currently returns 'anon': false for a temporary user. I've filed a separate task for this: T358693: Update ApiQueryWatchlist to handle temporary users.

Tchanders updated the task description. (Show Details)Feb 28 2024, 5:35 PM
Dreamy_Jazz closed subtask T358683: Update ApiQueryUserInfo to handle temporary users as Resolved.Mar 1 2024, 1:37 PM
dom_walden moved this task from Needs QA to Done on the Trust and Safety Product Sprint (Sprint Piano (Feb 19th - 1st March)) board.Mar 1 2024, 1:52 PM
dom_walden subscribed.
In T341228#9583535, @Dreamy_Jazz wrote:

Testing notes that I used to test this change which may be useful for QA:

I can confirm.

Test environments:

Dreamy_Jazz updated the task description. (Show Details)Mar 5 2024, 5:09 PM
Dreamy_Jazz closed this task as Resolved.Mar 11 2024, 3:16 PM
Dreamy_Jazz closed subtask T358693: Update ApiQueryWatchlist to handle temporary users as Resolved.
Dreamy_Jazz updated the task description. (Show Details)Mar 12 2024, 1:31 PM
Tchanders mentioned this in T370803: Filter temporary accounts correctly on ApiQueryRecentChanges.Jul 23 2024, 6:51 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits