Last modified: 2014-08-26 19:43:03 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 45199 - API token handling needs to be less special-cased and hard-coded
API token handling needs to be less special-cased and hard-coded
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
API (Other open bugs)
unspecified
All All
: Low enhancement (vote)
: ---
Assigned To: Brad Jorsch
:
Depends on:
Blocks: api-2.0 35993 36078
  Show dependency treegraph
 
Reported: 2013-02-20 17:06 UTC by Yuri Astrakhan
Modified: 2014-08-26 19:43 UTC (History)
7 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Yuri Astrakhan 2013-02-20 17:06:54 UTC
If 'token' is a magic parameter that ApiMain understands, ApiBase should add it to  getAllowedParams() and getParamDescription()
Comment 1 Brad Jorsch 2013-02-20 19:29:46 UTC
We really need to overhaul how tokens work in general. It should be simpler for a module to add a token requirement, and all tokens should be gettable from action=tokens.

Brainstorming:

* needsToken() returning true should be all that's needed for a module to add token protection. Errors, help, and paraminfo should be added automatically based on that.

Right now, it seems a module has to return true for needsToken() and non-false for getTokenSalt() and include a 'token' parameter in getAllowedParams() and either add itself somewhere in the middle of ApiTokens or hook ApiTokensGetTokenTypes. And maybe also add itself to ApiQueryInfo or hook APIQueryInfoTokens, and maybe ApiQueryRevisions or hook APIQueryRevisionsTokens, and maybe ApiQueryUsers or hook APIQueryUsersTokens, and maybe ApiQueryRecentChanges or hook APIQueryRecentChangesTokens. And some modules may just be doing it on their own.

* Each module should have a method getToken() to get the token. Default would just return $wgUser->getEditToken( $this->getTokenSalt() ), at least for now.

Right now, it's using static functions all over the place to get the tokens. Which is nice in that it means classes don't have to be instantiated, but makes it hard to handle the next bullet.

* Some modules need query parameters for salt. Somehow the module should be able to return the names of the parameters (as returned by getAllowedParams() and getParamDescription()) that the module needs to determine the token. Or maybe just add a flag to the data returned by getAllowedParams() to indicate "needed for tokens".

Right now, not really handled at all. It looks like the rollback token you can only get through prop=revisions, and the userrights token you can only get through list=users, and if there's anything in an extension it probably either implements its own "gettoken"-style parameter (see bug 35993) or has its own query module which is the only way to get its token (like rollback or userrights).

* action=tokens should take module names, like action=help now does. "type" would be deprecated but kept for backwards compatibility. Its help would probably have to instantiate every API module to be able to call needsToken() on it and to check whether it needs extra parameters, unfortunately.

This also solves the problem where you don't necessarily know which "type" is needed for any particular module, although that could also be solved with better parameter documentation.

* ApiTokens should provide a method that can be called by ApiQueryInfo, ApiQueryRevisions, etc to handle their special token-getting parameters. There would have to be some way to pass data through to getToken() so it could use the right page/revision/user/etc for each result instead of trying to grab it from the query string.

The annoying part is making sure we don't break any of the ways a module does tokens now.
Comment 2 Brion Vibber 2013-02-20 22:15:19 UTC
Under ideal circumstances I'd recommend this:
* drop all uses of token salt -- use the same token for all things in the session
* return the token in the login response along with the session key
* have a single method for fetching the token (if using saved login cookies, for instance)

This should help simplify things. :)
Comment 3 Tyler Romeo 2013-02-21 06:12:26 UTC
(In reply to comment #2)
> Under ideal circumstances I'd recommend this:
> * drop all uses of token salt -- use the same token for all things in the
> session
> * return the token in the login response along with the session key
> * have a single method for fetching the token (if using saved login cookies,
> for instance)
> 
> This should help simplify things. :)

I'd agree with all except removing the salt. I'd prefer that the editing token not also work for creating accounts and deleting articles, but unfortunately I don't have too much of a reason because the reasons for giving per-request tokens in the browser interface don't apply to the API.
Comment 4 Brad Jorsch 2014-08-26 19:31:22 UTC
Gerrit change #153110

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links