Page MenuHomePhabricator

Add assertuser=Whomever to the API, to prevent accidentally performing requests as the wrong user
Closed, ResolvedPublic

Description

Steps to reproduce:

  • Load a MediaWiki page that you can edit
  • Use an API-based tool to edit some part of the page (or a page) - e.g. FileAnnotations
  • Go to another tab on the same wiki, and log out.
  • Optional: Log in to a different account
  • Make another edit by the same API-based tool.

Expected outcome: Warning bells occur to let the user know that they're no longer editing as the user displayed at the top of the screen.

Actual outcome: Literally nothing happens to the user (except maybe "permissiondenied" if the wiki disallows editing by anons), but the edit is saved under the IP of the user, or the second username.

Initial proposed fix: mw.Api#postWithEditToken et al. should check meta=userinfo when they get a token, then check the user ID against the user ID in the page config (mw.config.get( 'wgUserId' )). The only problem I have is, how should we signal a mismatch? Adding it to the API response before it's passed back through the promise seems really nasty. A second parameter to mw.Api deferred resolutions might work. A pagewide event might work.

New proposed fix (see below discussion): API gets a new assertuser=Whomever parameter, in addition to assert=user, so you can signal the API to fail if the request is logged out, but also if the logged-in user is not User:Whomever.

Event Timeline

matmarex subscribed.

The only problem I have is, how should we signal a mismatch? Adding it to the API response before it's passed back through the promise seems really nasty. A second parameter to mw.Api deferred resolutions might work. A pagewide event might work.

getToken() could simply reject the promise it returns.

There doesn't seem to be anything for the action API itself to do here. You can already include assert=user in all your requests to avoid the logged-out issue, and include meta=userinfo in all your action=query requests and verify the user returned is still the user you think it is for the logged-in-as-someone-else issue.

I suppose if someone really wanted they could add assertuser=Example that would work like assert=user and additionally assert you're logged in as User:Example to cover the possibility that someone might somehow log in as a different user between making a query and submitting an edit.

This problem could and should be resolved by maintaining server side sessions.

I'm new to mediawiki so I can't say how exactly the user sessions are
already handled, but we need to bank upon that and make the necessary
changes to the API.

I'm new to mediawiki

Welcome to MediaWiki! I suggest you get acquainted with MediaWiki and try things out so you can make better-informed suggestions in the future. (:

@Anomie TIL about assert=user, and yes, assertuser=Whomever might be a nice addition. The mw.Api constructor could then just be passed that parameter, and the code wouldn't have to worry about it later.

I'll edit the task to reflect (what I think is) the best solution.

MarkTraceur renamed this task from mw.Api might want to warn downstream or the user when the page's wgUserId is different from the user the API thinks is acting to Add assertuser=Whomever to the API, to prevent accidentally performing requests as the wrong user.Oct 4 2016, 3:49 PM
MarkTraceur removed a project: JavaScript.
MarkTraceur updated the task description. (Show Details)

I'd probably have created that as a subtask of this task, instead of trying to turn this task into that. As it stands now, once the assertuser parameter is added this bug will be closed without it necessarily being added to the mw.Api JS module.

Change 314271 had a related patch set uploaded (by Anomie):
API: Add assertuser parameter

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

Change 314271 merged by jenkins-bot:
API: Add assertuser parameter

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

Anomie claimed this task.

Note that no special support for this is needed in the JS mediawiki.api module, you can conveniently add this parameter to all of your requests by constructing the mw.Api object like this:

new mw.Api( { parameters: { assertuser: mw.config.get( 'wgUserName' ) } } )

Maybe it would make sense to use this by default for all mw.Api instances… it could prevent some stupid mistakes, but then it's usually not needed for most queries (only for POST, and GET for data only visible to some users).