Page MenuHomePhabricator

gettoken should go die in a fire
Closed, ResolvedPublic


It's been responsible for a few (security) bugs, and I don't really see the point of it (even more so now we have action=tokens).

I know we don't like breaking changes, which is fine, though, we can either deprecate this and say we'll remove it in X, or jfdi on security grounds

See for the latest

Version: unspecified
Severity: enhancement



Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 12:20 AM
bzimport added a project: MediaWiki-API.
bzimport set Reference to bz35993.
bzimport added a subscriber: Unknown Object (MLST).

beau wrote:

I think we should get rid of that. It is very confusing <s>bug</s> feature...

beau wrote:

I have submitted gerrit #6730 for review. This change marks gettoken parameter as deprecated.

The parameter deprecation should be documented in RELEASE-NOTES-1.21 (section 1.20).

Please take this opportunity to fix a delay, per Reedy suggestion ("say we'll remove it in X", you haven't set this X yet).

Done, Gerrit change 40566.

Do we maintain old versions RELEASE-NOTES files?

I know FreeBSD project use RL errata for this purpose,

A bit confused by the tokens implementation - there is a needsToken() which appears useless except to generate some help string, and yet many modules diligently override it.
Also, in ApiMain.php / setupModule() has this line
if ( $salt !== false && !$gettoken ) ...
doesn't php give false when gettoken is ''?
Lastly, the isset( $moduleParams['token'] ) - wouldn't that be always true even if the user didn't pass value (will be equal to null).

merged the HISTORY file - Gerrit change #40566 (even though was a bit reluctant - changing history is not the best way to do it). But its better to have it than not to mention it at all.

Closing as resolved. There is a relevant discussion in bug 45199 about restructuring tokens.