r103396 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r103395‎ | r103396 | r103397 >
Date:22:24, 16 November 2011
Author:mah
Status:reverted (Comments)
Tags:
Comment:
Applying Michael Newton's patch from Bug 24464 - Execute
LoginAuthenticateAudit hook more often

Tested and it *seems* reasonable to me.
Modified paths:
  • /trunk/phase3/CREDITS (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUserlogin.php (modified) (history)

Diff [purge]

Index: trunk/phase3/CREDITS
@@ -131,6 +131,7 @@
132132 * Michael Dale
133133 * Michael De La Rue
134134 * Michael M.
 135+* Michael Newton
135136 * Michael Walsh
136137 * Mike Horvath
137138 * Mormegil
Index: trunk/phase3/includes/specials/SpecialUserlogin.php
@@ -475,6 +475,7 @@
476476 $this->load();
477477
478478 if ( $this->mUsername == '' ) {
 479+ wfRunHooks( 'LoginAuthenticateAudit', array( $u, $this->mPassword, self::NO_NAME ) );
479480 return self::NO_NAME;
480481 }
481482
@@ -486,20 +487,24 @@
487488 // If the user doesn't have a login token yet, set one.
488489 if ( !self::getLoginToken() ) {
489490 self::setLoginToken();
 491+ wfRunHooks( 'LoginAuthenticateAudit', array( $u, $this->mPassword, self::NEED_TOKEN ) );
490492 return self::NEED_TOKEN;
491493 }
492494 // If the user didn't pass a login token, tell them we need one
493495 if ( !$this->mToken ) {
 496+ wfRunHooks( 'LoginAuthenticateAudit', array( $u, $this->mPassword, self::NEED_TOKEN ) );
494497 return self::NEED_TOKEN;
495498 }
496499
497500 $throttleCount = self::incLoginThrottle( $this->mUsername );
498501 if ( $throttleCount === true ) {
 502+ wfRunHooks( 'LoginAuthenticateAudit', array( $u, $this->mPassword, self::THROTTLED ) );
499503 return self::THROTTLED;
500504 }
501505
502506 // Validate the login token
503507 if ( $this->mToken !== self::getLoginToken() ) {
 508+ wfRunHooks( 'LoginAuthenticateAudit', array( $u, $this->mPassword, self::WRONG_TOKEN ) );
504509 return self::WRONG_TOKEN;
505510 }
506511
@@ -520,6 +525,7 @@
521526 # user choose a different wiki name.
522527 $u = User::newFromName( $this->mUsername );
523528 if( !( $u instanceof User ) || !User::isUsableName( $u->getName() ) ) {
 529+ wfRunHooks( 'LoginAuthenticateAudit', array( $u, $this->mPassword, self::ILLEGAL ) );
524530 return self::ILLEGAL;
525531 }
526532
@@ -527,6 +533,7 @@
528534 if ( 0 == $u->getID() ) {
529535 $status = $this->attemptAutoCreate( $u );
530536 if ( $status !== self::SUCCESS ) {
 537+ wfRunHooks( 'LoginAuthenticateAudit', array( $u, $this->mPassword, $status ) );
531538 return $status;
532539 } else {
533540 $isAutoCreated = true;
@@ -547,6 +554,7 @@
548555 // Give general extensions, such as a captcha, a chance to abort logins
549556 $abort = self::ABORTED;
550557 if( !wfRunHooks( 'AbortLogin', array( $u, $this->mPassword, &$abort, &$this->mAbortLoginErrorMsg ) ) ) {
 558+ wfRunHooks( 'LoginAuthenticateAudit', array( $u, $this->mPassword, $abort ) );
551559 return $abort;
552560 }
553561

Follow-up revisions

RevisionCommit summaryAuthorDate
r103418revert r103396 (breaks unit tests) - In lots of these places $u is undefined ...bawolff23:55, 16 November 2011
r103467Adapt and re-apply Michael Newton's patch from Bug 24464 - Execute...mah15:16, 17 November 2011
r110797Revert r103467, r106446 (bug 24464: calling LoginAuthenticateAudit hook more ...demon22:44, 6 February 2012

Comments

#Comment by GICodeWarrior (talk | contribs)   22:53, 16 November 2011

There is no variable $u for the first 5 wfRunHooks calls in public function authenticateUserData() {. This is breaking the API tests.

Should we back it out or try to fix it?

#Comment by Bawolff (talk | contribs)   23:56, 16 November 2011

reverted in r103418 due to unit test breakage

Status & tagging log