r56133 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r56132‎ | r56133 | r56134 >
Date:13:44, 10 September 2009
Author:catrope
Status:ok (Comments)
Tags:todo 
Comment:
API: Make it possible to fetch userrights tokens for interwiki users; make UserrightsPage::fetchUser() static
Modified paths:
  • /trunk/phase3/includes/api/ApiQueryUsers.php (modified) (history)
  • /trunk/phase3/includes/api/ApiUserrights.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUserrights.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/api/ApiUserrights.php
@@ -45,8 +45,7 @@
4646 if(is_null($params['token']))
4747 $this->dieUsageMsg(array('missingparam', 'token'));
4848
49 - $form = new UserrightsPage;
50 - $user = $form->fetchUser($params['user']);
 49+ $user = UserrightsPage::fetchUser($params['user']);
5150 if($user instanceof WikiErrorMsg)
5251 $this->dieUsageMsg(array_merge(
5352 (array)$user->getMessageKey(),
Index: trunk/phase3/includes/api/ApiQueryUsers.php
@@ -170,9 +170,26 @@
171171 }
172172 // Second pass: add result data to $retval
173173 foreach($goodNames as $u) {
174 - if(!isset($data[$u]))
175 - $data[$u] = array('name' => $u, 'missing' => '');
176 - else {
 174+ if(!isset($data[$u])) {
 175+ $data[$u] = array('name' => $u);
 176+ $iwUser = UserrightsPage::fetchUser($u);
 177+ if($iwUser instanceof UserRightsProxy) {
 178+ $data[$u]['interwiki'] = '';
 179+ if(!is_null($params['token']))
 180+ {
 181+ $tokenFunctions = $this->getTokenFunctions();
 182+ foreach($params['token'] as $t)
 183+ {
 184+ $val = call_user_func($tokenFunctions[$t], $iwUser);
 185+ if($val === false)
 186+ $this->setWarning("Action '$t' is not allowed for the current user");
 187+ else
 188+ $data[$u][$t . 'token'] = $val;
 189+ }
 190+ }
 191+ } else
 192+ $data[$u]['missing'] = '';
 193+ } else {
177194 if(isset($this->prop['groups']) && isset($data[$u]['groups']))
178195 $this->getResult()->setIndexedTagName($data[$u]['groups'], 'g');
179196 }
Index: trunk/phase3/includes/specials/SpecialUserrights.php
@@ -272,7 +272,7 @@
273273 * Side effects: error output for invalid access
274274 * @return mixed User, UserRightsProxy, or WikiErrorMsg
275275 */
276 - function fetchUser( $username ) {
 276+ public static function fetchUser( $username ) {
277277 global $wgUser, $wgUserrightsInterwikiDelimiter;
278278
279279 $parts = explode( $wgUserrightsInterwikiDelimiter, $username );

Follow-up revisions

RevisionCommit summaryAuthorDate
r56447r56133 made fetchUser staticmrzman21:37, 16 September 2009
r56448use fetchUser statically per r56133mrzman21:38, 16 September 2009
r56515Revert r56450 (encompassing r56447 and r56448) and r56133 on wmf-deployment. ...werdna14:30, 17 September 2009

Comments

#Comment by Brion VIBBER (talk | contribs)   19:30, 10 September 2009

What the heck's all this token stuff for? It seems wayyyyyy excessive and it's unclear what they're for or why.

#Comment by Catrope (talk | contribs)   20:05, 10 September 2009

They're userrights tokens for interwiki users. It's literally copied from like 20 lines above (which I'm not proud of, but at least shows that I didn't pull the code out of my ass).

#Comment by Brion VIBBER (talk | contribs)   20:16, 10 September 2009

I'll leave it marked as a todo then; we really ought to need only one token to validate the session, rather than a big map.

#Comment by Catrope (talk | contribs)   21:07, 10 September 2009

Userrights tokens are salted with the user name of the 'victim', rollback tokens with the target title and clearwatchlist tokens with a constant string. All of this was present in core before my time (i.e. before the summer of 2007); if you hate that situation and want it to change, that's fair enough, but it doesn't really have much to do with any particular token-related revision more than 30k revs after the system was put in place.

Status & tagging log