From a9775ae5722059fe2c661751b0ddacc6d011f404 Mon Sep 17 00:00:00 2001 From: Alexandre Emsenhuber Date: Mon, 21 Jan 2013 12:03:46 +0100 Subject: [PATCH] (bug 44202) Account creation through API no longer leaks IP address of account creator This happens when an anonymous user wants to create an account for himself through the API. This is due to the fact that User::addNewUserLogEntry() was always using $wgUser as performer, but the API does not replace $wgUser by the newly created user object when the peformer is an anonymous user. Changed User::addNewUserLogEntry() to directly take the log action as first parameter, rather than a boolean value saying whether the password was sent by e-mail or not, and force the performer to be the user itself in the log action is "create". This avoids such problems in that case, no matter the value of $wgUser, and it makes this parameter much more readable that the old one. Backward compatibility is maintained. Creating an user and sending its password by e-mail will still log the performer's IP address in the log if this is made by an anonymous user. Finally the second parameter of the AddNewAccount is now correct when creating an account from the API, it was always false previously. Change-Id: I188ecf420b85e9d1dab6fb933ed50d5f58532109 --- includes/User.php | 50 ++++++++++++++++++-------- includes/api/ApiCreateAccount.php | 12 +++++-- includes/specials/SpecialUserlogin.php | 6 ++-- 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/includes/User.php b/includes/User.php index ab3fd203bc..dbe9a64bc9 100644 --- a/includes/User.php +++ b/includes/User.php @@ -4217,38 +4217,60 @@ class User { } /** - * Add a newuser log entry for this user. Before 1.19 the return value was always true. + * Add a newuser log entry for this user. + * Before 1.19 the return value was always true. + * + * @param $action string|bool: account creation type. + * - String, one of the following values: + * - 'create' for an anonymous user creating an account for himself. + * This will force the action's performer to be the created user itself, + * no matter the value of $wgUser + * - 'create2' for a logged in user creating an account for someone else + * - 'byemail' when the created user will receive its password by e-mail + * - Boolean means whether the account was created by e-mail (deprecated): + * - true will be converted to 'byemail' + * - false will be converted to 'create' if this object is the same as + * $wgUser and to 'create2' otherwise * - * @param $byEmail Boolean: account made by email? * @param $reason String: user supplied reason * * @return int|bool True if not $wgNewUserLog; otherwise ID of log item or 0 on failure */ - public function addNewUserLogEntry( $byEmail = false, $reason = '' ) { + public function addNewUserLogEntry( $action = false, $reason = '' ) { global $wgUser, $wgContLang, $wgNewUserLog; if( empty( $wgNewUserLog ) ) { return true; // disabled } - if( $this->getName() == $wgUser->getName() ) { - $action = 'create'; - } else { + if ( $action === true || $action === 'byemail' ) { $action = 'create2'; - if ( $byEmail ) { - if ( $reason === '' ) { - $reason = wfMessage( 'newuserlog-byemail' )->inContentLanguage()->text(); - } else { - $reason = $wgContLang->commaList( array( - $reason, wfMessage( 'newuserlog-byemail' )->inContentLanguage()->text() ) ); - } + if ( $reason === '' ) { + $reason = wfMessage( 'newuserlog-byemail' )->inContentLanguage()->text(); + } else { + $reason = $wgContLang->commaList( array( + $reason, wfMessage( 'newuserlog-byemail' )->inContentLanguage()->text() ) ); + } + } elseif ( $action === false ) { + if ( $this->getName() == $wgUser->getName() ) { + $action = 'create'; + } else { + $action = 'create2'; } } + + if ( $action === 'create' ) { + $performer = $this; + } else { + $performer = $wgUser; + } + $log = new LogPage( 'newusers' ); return (int)$log->addEntry( $action, $this->getUserPage(), $reason, - array( $this->getId() ) + array( $this->getId() ), + $performer ); } diff --git a/includes/api/ApiCreateAccount.php b/includes/api/ApiCreateAccount.php index 0209fb1237..8c49c17c58 100644 --- a/includes/api/ApiCreateAccount.php +++ b/includes/api/ApiCreateAccount.php @@ -89,8 +89,16 @@ class ApiCreateAccount extends ApiBase { // Save settings (including confirmation token) $user->saveSettings(); - wfRunHooks( 'AddNewAccount', array( $user, false ) ); - $user->addNewUserLogEntry( $this->getUser()->isAnon(), $params['reason'] ); + wfRunHooks( 'AddNewAccount', array( $user, $params['mailpassword'] ) ); + + if ( $params['mailpassword'] ) { + $logAction = 'byemail'; + } elseif ( $this->getUser()->isLoggedIn() ) { + $logAction = 'create2'; + } else { + $logAction = 'create'; + } + $user->addNewUserLogEntry( $logAction, (string)$params['reason'] ); // Add username, id, and token to result. $result['username'] = $user->getName(); diff --git a/includes/specials/SpecialUserlogin.php b/includes/specials/SpecialUserlogin.php index 3a73ebda51..7817b960a3 100644 --- a/includes/specials/SpecialUserlogin.php +++ b/includes/specials/SpecialUserlogin.php @@ -212,7 +212,7 @@ class LoginForm extends SpecialPage { $result = $this->mailPasswordInternal( $u, false, 'createaccount-title', 'createaccount-text' ); wfRunHooks( 'AddNewAccount', array( $u, true ) ); - $u->addNewUserLogEntry( true, $this->mReason ); + $u->addNewUserLogEntry( 'byemail', $this->mReason ); $out = $this->getOutput(); $out->setPageTitle( $this->msg( 'accmailtitle' ) ); @@ -274,7 +274,7 @@ class LoginForm extends SpecialPage { // wrong. $this->getContext()->setUser( $u ); wfRunHooks( 'AddNewAccount', array( $u, false ) ); - $u->addNewUserLogEntry(); + $u->addNewUserLogEntry( 'create' ); if( $this->hasSessionCookie() ) { $this->successfulCreation(); } else { @@ -286,7 +286,7 @@ class LoginForm extends SpecialPage { $out->addWikiMsg( 'accountcreatedtext', $u->getName() ); $out->addReturnTo( $this->getTitle() ); wfRunHooks( 'AddNewAccount', array( $u, false ) ); - $u->addNewUserLogEntry( false, $this->mReason ); + $u->addNewUserLogEntry( 'create2', $this->mReason ); } return true; } -- 2.20.1