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

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T47199, the corresponding Phabricator task for complete and up-to-date bug report information.
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