(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

diff --git a/includes/User.php b/includes/User.php
index ab3fd20..dbe9a64 100644 (file)
--- 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 0209fb1..8c49c17 100644 (file)
--- 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 3a73ebd..7817b96 100644 (file)
--- 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;
        }