Page MenuHomePhabricator

Can't edit any page via visual editor while not logged into an account or a temporary account
Closed, ResolvedPublic

Description

Steps to reproduce

  • Cleared my cookies so I'm not currently attached to any temporary account.
  • Enter "Test new page" in the search bar
  • Click the 'Test new page' redlink
  • Click the blue button ('Start editing')
  • I get this popup, and can't enter any text:

image.png (492×982 px, 61 KB)

This can be also reproduced by trying to edit an existing page via the visual editor, for example, by going to https://cs.wikipedia.beta.wmflabs.org/w/index.php?title=Tesla_(automobilka)&veaction=edit.

If the first edit is wikitext one (both editing an existing page and creating a new one works), then visual editor starts working again.

Event Timeline

Urbanecm_WMF renamed this task from Can't create page as first edit while not logged into an account or temporary account to Can't edit any page via visual editor while not logged into an account or a temporary account.Jul 26 2023, 1:59 PM
Urbanecm_WMF updated the task description. (Show Details)

Just confirmed that this seems to be a VE issue specifically, I can create pages with the source editor as expected.

Urbanecm_WMF added subscribers: tstarling, Tgr, Urbanecm_WMF.

This is caused by the following two lines in CommonSettings-labs.php:

$wgGroupPermissions['*']['edit'] = false;
$wgGroupPermissions['temp']['edit'] = true;

which were enabled as part of T342034: Growth: enable IP-masking on betalabs cswiki (cc @tstarling @Tgr).

An easy fix would be to remove those two lines (assign edit to * again), but I don't think that's a correct long-term fix. Anyway, tagging this with a VE tag.

So, VE seems to be correctly picking up that the current user doesn't have permissions to edit... but it should also be noticing (or something else in the system should be noticing-and-overriding) that editing would autocreate a temp user, so effectively it has editing permissions? (And this is correctly happening in the source editor, so wherever this override is, VE isn't being affected by it.)

(Beta cluster seems to be falling apart right now, so I can't directly test this.)

$wgGroupPermissions['*']['edit'] = false; prevents anything we forgot to update for IP masking from working. So if we e.g. forget about the LiquidThreads API and someone makes a LiquidThreads edit, it would fail instead of making an anonymous edit and exposing the user's IP. Not sure if that's a pro or a con from a product perspective.

In theory once everything is updated, it won't be possible to make an anonymous edit or logged action anymore, because every endpoint able to do such actions would know to instead generate a temporary user; so at that point the value of $wgGroupPermissions['*']['edit'] wouldn't really have meaning anymore. But there are lots of obscure endpoints that make an edit, and much more so in third-party extensions (and I don't think we have decided whether temporary users is going to be the standard way that eventually all MediaWiki installations have to use, or just an option). So in the short run it seems useful to be able to disallow anon edits. That means the permission handling of all software doing edits will have to be updated; but then all software doing edits will have to be updated anyway.

I guess the outcome is: this is a blocking issue for any sort of IP masking deployment off-labs, but it also sounds like something that you already know about and have on your list-of-things-to-get-done?

I guess the outcome is: this is a blocking issue for any sort of IP masking deployment off-labs, but it also sounds like something that you already know about and have on your list-of-things-to-get-done?

I don't think anyone realized it will happen, although it makes sense in hindsight. Not sure whose list it should be on. Is this something Editing should fix in VE (as well as every other owner of a component that does some kind of API-based editing)? Should there be some sort of core mechanism that (presumably) the MediaWiki team should add (like a PermissionManager flag)? Should we set $wgGroupPermissions['*']['edit'] = true; instead?

If I read the core correctly, the way it's handled on the server side is that permission checks with the lowest rigor (probablyCan etc) automatically switch to a fake temp user object when the action is one listed in $wgAutoCreateTempUser['action'], so effectively the action ends up being checked against permissions in $wgGroupRights['temp']. In PHP code that should work without any changes needed to the code, as long as no definitelyCan or other higher-rigor check is done before temp user creation.

Not sure how it should be handled on the JS side. We could export the set of extra rights the user would gain by becoming a temp user (or maybe add it to the userinfo API) and add an "include temp user rights" flag to mw.user.getRights(). That wouldn't help with this specific issue though, which is that ApiVisualEdit needs to do a full-rigor check. I think the best way to handle that is to copy the fake temp user logic above before calling the permission check.

Change 946971 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/VisualEditor@master] ApiVisualEditor: Check permissions of the temp user if we will create one

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

It turns out that you also can't edit pages using the core action=edit API with this configuration: https://cs.wikipedia.beta.wmflabs.org/wiki/Speciální:API_pískoviště#action=edit&format=json&title=Test&appendtext=asdf&token=%2B\&formatversion=2 gives:

{
    "error": {
        "code": "permissiondenied",
        "info": "The action you have requested is limited to users in one of the groups: [[Wikipedie:Uživatelé|Uživatelé]], [[Wikipedie:Dočasní uživatelé|Dočasní uživatelé]].",
        "docref": "See https://cs.wikipedia.beta.wmflabs.org/w/api.php for API usage. Subscribe to the mediawiki-api-announce mailing list at <https://lists.wikimedia.org/postorius/lists/mediawiki-api-announce.lists.wikimedia.org/> for notice of API deprecations and breaking changes."
    },
    "servedby": "deployment-mediawiki11"
}

Change 946993 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/core@master] ApiEditPage: Check permissions of the temp user if we will create one

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

Change 946993 merged by jenkins-bot:

[mediawiki/core@master] ApiEditPage: Check permissions of the temp user if we will create one

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

Change 946971 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] ApiVisualEditor: Check permissions of the temp user if we will create one

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