Page MenuHomePhabricator

Add deleterevision right to botadmin group on fawiki
Closed, ResolvedPublic

Description

The botadmin group has delete and undelete rights, but not deleterevision right. This was not necessary because Pywikibot did not support it anyway.

In T276726 I have added this capability to pywikibot. We have planned a use case for it on fawiki. Therefore, we need this right to be given to our botadmin group.

Event Timeline

Change 671402 had a related patch set uploaded (by Huji; owner: Huji):
[operations/mediawiki-config@master] Add deleterevision right to botadmin group on fawiki

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

Change 671402 merged by jenkins-bot:
[operations/mediawiki-config@master] Add deleterevision right to botadmin group on fawiki

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

Mentioned in SAL (#wikimedia-operations) [2021-03-15T18:13:57Z] <urbanecm@deploy1002> Synchronized wmf-config/InitialiseSettings.php: a8234a9435a3acf669d44705fbcb19bf4dd5658e: Add deleterevision right to botadmin group on fawiki (T277358) (duration: 00m 59s)

Thanks @Urbanecm for merging the patch. I thought this would resolve our underlying issue but it did not. Somehow, the file information API query returns more values for a sysop than a botadmin. Lets keep this Task open as I will investigate it more.

Thanks @Urbanecm for merging the patch. I thought this would resolve our underlying issue but it did not. Somehow, the file information API query returns more values for a sysop than a botadmin. Lets keep this Task open as I will investigate it more.

I see. If you have an example API query and what you do (not) see,I can help with investigation.

@Urbanecm I was able to get to the bottom of this. What we need is for the botadmin group to have the ability to retrieve the archivename attribute of a file's deleted revisions. Currently, admins (i.e those in sysop group) get it but botadmins do not.

Here is an example API request: https://fa.wikipedia.org/wiki/Special:ApiSandbox#action=query&format=json&prop=imageinfo&titles=File%3AIFAC-IFACnet.png&iiprop=timestamp%7Cuser%7Carchivename&iilimit=10

The target file has two revisions that are revdeled such that their content is not publicly accessible, but their user, upload summary, and timestamp are public.

Here is the output I get using my account as a sysop, please note the presence of archivename key and value in the JSON response:

{
    "batchcomplete": "",
    "query": {
        "normalized": [
            {
                "from": "File:IFAC-IFACnet.png",
                "to": "\u067e\u0631\u0648\u0646\u062f\u0647:IFAC-IFACnet.png"
            }
        ],
        "pages": {
            "1684342": {
                "pageid": 1684342,
                "ns": 6,
                "title": "\u067e\u0631\u0648\u0646\u062f\u0647:IFAC-IFACnet.png",
                "imagerepository": "local",
                "imageinfo": [
                    {
                        "timestamp": "2021-03-23T13:57:26Z",
                        "user": "HujiBot"
                    },
                    {
                        "timestamp": "2021-03-16T03:57:24Z",
                        "user": "HujiBot",
                        "filehidden": "",
                        "archivename": "20210323135723!IFAC-IFACnet.png"
                    },
                    {
                        "timestamp": "2013-02-18T12:27:38Z",
                        "user": "Mohsen ghasemee",
                        "filehidden": "",
                        "archivename": "20210316035722!IFAC-IFACnet.png"
                    }
                ]
            }
        }
    }
}

Here is what a user that is not a sysop but is in the bot and botadmin groups gets, and please notice the absence of filearchive:

{
    "batchcomplete": "",
    "query": {
        "normalized": [
            {
                "from": "File:IFAC-IFACnet.png",
                "to": "\u067e\u0631\u0648\u0646\u062f\u0647:IFAC-IFACnet.png"
            }
        ],
        "pages": {
            "1684342": {
                "pageid": 1684342,
                "ns": 6,
                "title": "\u067e\u0631\u0648\u0646\u062f\u0647:IFAC-IFACnet.png",
                "imagerepository": "local",
                "imageinfo": [
                    {
                        "timestamp": "2021-03-23T13:57:26Z",
                        "user": "HujiBot"
                    },
                    {
                        "timestamp": "2021-03-16T03:57:24Z",
                        "user": "HujiBot",
                        "filehidden": ""
                    },
                    {
                        "timestamp": "2013-02-18T12:27:38Z",
                        "user": "Mohsen ghasemee",
                        "filehidden": ""
                    }
                ]
            }
        }
    }
}

What permission change do we need for the botadmin group to be able to see the archivename values?

Actually, I think I found it. https://phabricator.wikimedia.org/source/mediawiki/browse/master/includes/api/ApiQueryImageInfo.php uses userCanSeeRevDel() but the function currently does not honor deleterevision as a right that would allow user to get the so called "private" bits back. This is wrong; when I log in using that botadmin account and use the web interface, I do see the file history and delete/undelete revisions.

I will make a patch for it shortly.

[...]
What permission change do we need for the botadmin group to be able to see the archivename values?

I just tested it with my own accounts, and granting 'deletedtext' fixed this. You might want to also grant 'deletedhistory', since text without history is rarely useful.

Actually, I think I found it. https://phabricator.wikimedia.org/source/mediawiki/browse/master/includes/api/ApiQueryImageInfo.php uses userCanSeeRevDel() but the function currently does not honor deleterevision as a right that would allow user to get the so called "private" bits back. This is wrong; when I log in using that botadmin account and use the web interface, I do see the file history and delete/undelete revisions.

I will make a patch for it shortly.

I think the function is behaving correctly. deleterevision should be used as a right guarding revision deletion. If a wiki wants a particular group to view deleted stuff, deletedtext or deletedhistory makes much more sense to grant.

Change 674390 had a related patch set uploaded (by Huji; owner: Huji):
[mediawiki/core@master] Users with detelerevision right should see private bits of history

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

[...]
What permission change do we need for the botadmin group to be able to see the archivename values?

I just tested it with my own accounts, and granting 'deletedtext' fixed this. You might want to also grant 'deletedhistory', since text without history is rarely useful.

Actually, I think I found it. https://phabricator.wikimedia.org/source/mediawiki/browse/master/includes/api/ApiQueryImageInfo.php uses userCanSeeRevDel() but the function currently does not honor deleterevision as a right that would allow user to get the so called "private" bits back. This is wrong; when I log in using that botadmin account and use the web interface, I do see the file history and delete/undelete revisions.

I will make a patch for it shortly.

I think the function is behaving correctly. deleterevision should be used as a right guarding revision deletion. If a wiki wants a particular group to view deleted stuff, deletedtext or deletedhistory makes much more sense to grant.

On the web interface, the same user has access to the filearchive information. We should be consistent. I created a patch that would make the API match the web interface's functionality. If you think the API is indeed doing the right thing, could you prepare the opposite patch, i.e. one that would make sure a user with revisiondelete but without deletedhistory cannot see the deleted history and therefore, cannot revdel a file's versions?

[...]
On the web interface, the same user has access to the filearchive information.

Is that true? Here is a screenshot logged-out:

image.png (403×1 px, 59 KB)

This is a screenshot with botadmin hat on:

image.png (497×1 px, 70 KB)

And this is what I see from my personal account (which has sysop-level permissions on fawiki as I'm a steward):

image.png (524×1 px, 69 KB)

The only difference between anonymous screenshot and the botadmin screenshot is that the botadmin one has a "revision " link, and can delete/undelete the file. The "view file version" link from the datetime is grayed out for both anonymous users and botadmins. Or do you mean that the delete link is https://fa.wikipedia.org/w/index.php?title=%D9%BE%D8%B1%D9%88%D9%86%D8%AF%D9%87:IFAC-IFACnet.png&action=delete&oldimage=20210323135723%21IFAC-IFACnet.png, and the archive key (20210323135723!IFAC-IFACnet.png) can be extracted out of it?

We should be consistent. I created a patch that would make the API match the web interface's functionality. If you think the API is indeed doing the right thing, could you prepare the opposite patch, i.e. one that would make sure a user with revisiondelete but without deletedhistory cannot see the deleted history and therefore, cannot revdel a file's versions?

AFAICS, the web interface never shows a field "filearchive key", so it's not really comparable with the API, which displays it explicitly. I'm not super convinced this is the best solution, but also the file archive key is not unexpectable in any way – even the anonymous users can see the date, and the key is basically date + ! + file name, and we even publish it via the replicas:

MariaDB [fawiki_p]> select oi_name, oi_archive_name, oi_deleted from oldimage where oi_name='IFAC-IFACnet.png'\G
*************************** 1. row ***************************
        oi_name: IFAC-IFACnet.png
oi_archive_name: 20210316035722!IFAC-IFACnet.png
     oi_deleted: 1
*************************** 2. row ***************************
        oi_name: IFAC-IFACnet.png
oi_archive_name: 20210323135723!IFAC-IFACnet.png
     oi_deleted: 1
2 rows in set (0.00 sec)

MariaDB [fawiki_p]>

So...I don't intend to be the blocker for this patch, but I'm still not sure that changing this is okay.

Change 674390 merged by jenkins-bot:

[mediawiki/core@master] Users with detelerevision right should see private bits of history

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