PasswordFactory::newFromPlaintext( null ) needs to work

Various code passes null around to mean "an invalid password". It
shouldn't all have to test for null and specially handle that.

This also fixes a codepath where User::$mNewpassword could get set to an
empty string rather than a password object, which would cause problems
later when anything else tries to use it.

Bug: 71421
Change-Id: Ib5f94b52c07e7dba89328b98fb43c86db95ee09f

diff --git a/includes/User.php b/includes/User.php
index 9897013..e25bc1f 100644 (file)
--- a/includes/User.php
+++ b/includes/User.php
@@ -2351,11 +2351,7 @@ class User implements IDBAccessObject {
                $this->setToken();
 
                $passwordFactory = self::getPasswordFactory();
-               if ( $str === null ) {
-                       $this->mPassword = $passwordFactory->newFromCiphertext( null );
-               } else {
-                       $this->mPassword = $passwordFactory->newFromPlaintext( $str );
-               }
+               $this->mPassword = $passwordFactory->newFromPlaintext( $str );
 
                $this->mNewpassword = $passwordFactory->newFromCiphertext( null );
                $this->mNewpassTime = null;
@@ -2400,14 +2396,11 @@ class User implements IDBAccessObject {
        public function setNewpassword( $str, $throttle = true ) {
                $this->loadPasswords();
 
+               $this->mNewpassword = self::getPasswordFactory()->newFromPlaintext( $str );
                if ( $str === null ) {
-                       $this->mNewpassword = '';
                        $this->mNewpassTime = null;
-               } else {
-                       $this->mNewpassword = self::getPasswordFactory()->newFromPlaintext( $str );
-                       if ( $throttle ) {
-                               $this->mNewpassTime = wfTimestampNow();
-                       }
+               } elseif ( $throttle ) {
+                       $this->mNewpassTime = wfTimestampNow();
                }
        }
 

diff --git a/includes/password/PasswordFactory.php b/includes/password/PasswordFactory.php
index 3b4ebb1..48d6866 100644 (file)
--- a/includes/password/PasswordFactory.php
+++ b/includes/password/PasswordFactory.php
@@ -141,11 +141,15 @@ final class PasswordFactory {
         * If no existing object is given, make a new default object. If one is given, clone that
         * object. Then pass the plaintext to Password::crypt().
         *
-        * @param string $password Plaintext password
+        * @param string|null $password Plaintext password, or null for an invalid password
         * @param Password|null $existing Optional existing hash to get options from
         * @return Password
         */
        public function newFromPlaintext( $password, Password $existing = null ) {
+               if ( $password === null ) {
+                       return new InvalidPassword( $this, array( 'type' => '' ), null );
+               }
+
                if ( $existing === null ) {
                        $config = $this->types[$this->default];
                        $obj = new $config['class']( $this, $config );

diff --git a/tests/phpunit/includes/PasswordTest.php b/tests/phpunit/includes/PasswordTest.php
index ceb794b..5ad8aca 100644 (file)
--- a/tests/phpunit/includes/PasswordTest.php
+++ b/tests/phpunit/includes/PasswordTest.php
@@ -30,4 +30,10 @@ class PasswordTest extends MediaWikiTestCase {
 
                $this->assertFalse( $invalid1->equals( $invalid2 ) );
        }
+
+       public function testInvalidPlaintext() {
+               $invalid = User::getPasswordFactory()->newFromPlaintext( null );
+
+               $this->assertInstanceOf( 'InvalidPassword', $invalid );
+       }
 }