From dd52464497213191a15f0b7ea11c4d20b77d707e Mon Sep 17 00:00:00 2001 From: Marius Hoch Date: Mon, 20 Oct 2014 01:54:16 +0200 Subject: [PATCH] Fix creating non-parameterized hashes in ParameterizedPassword I noticed MWOldPassword is broken while working on I7024b287a7. When generating new passwords for it, a superfluous : is being added to the serialized hash within the database (and that breaks parsing so that people can't ever log in). As this is not really relevant in the real world (as nobody is hopefully using plain MD5 passwords anymore), this doesn't need any backward compatibility handling for the broken hashes. Change-Id: I753c135a6de39008488bd7462c2bfcda2cbac116 --- includes/password/MWOldPassword.php | 2 +- includes/password/ParameterizedPassword.php | 12 ++++++++---- tests/phpunit/includes/TestUser.php | 4 ++-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/includes/password/MWOldPassword.php b/includes/password/MWOldPassword.php index 0ba407f2f0..afa5cacc78 100644 --- a/includes/password/MWOldPassword.php +++ b/includes/password/MWOldPassword.php @@ -38,7 +38,7 @@ class MWOldPassword extends ParameterizedPassword { public function crypt( $plaintext ) { global $wgPasswordSalt; - if ( $wgPasswordSalt && count( $this->args ) == 1 ) { + if ( $wgPasswordSalt && count( $this->args ) === 1 ) { $this->hash = md5( $this->args[0] . '-' . md5( $plaintext ) ); } else { $this->args = array(); diff --git a/includes/password/ParameterizedPassword.php b/includes/password/ParameterizedPassword.php index 4d6e415526..187f895421 100644 --- a/includes/password/ParameterizedPassword.php +++ b/includes/password/ParameterizedPassword.php @@ -83,10 +83,14 @@ abstract class ParameterizedPassword extends Password { } public function toString() { - return - ':' . $this->config['type'] . ':' . - implode( $this->getDelimiter(), array_merge( $this->params, $this->args ) ) . - $this->getDelimiter() . $this->hash; + $str = ':' . $this->config['type'] . ':'; + + if ( count( $this->params ) || count( $this->args ) ) { + $str .= implode( $this->getDelimiter(), array_merge( $this->params, $this->args ) ); + $str .= $this->getDelimiter(); + } + + return $str . $this->hash; } /** diff --git a/tests/phpunit/includes/TestUser.php b/tests/phpunit/includes/TestUser.php index 39822dc0a7..499353e0d9 100644 --- a/tests/phpunit/includes/TestUser.php +++ b/tests/phpunit/includes/TestUser.php @@ -114,8 +114,8 @@ class TestUser { $passwordFactory = $this->user->getPasswordFactory(); $oldDefaultType = $passwordFactory->getDefaultType(); - // B is salted MD5 (thus fast) ... we don't care about security here, this is test only - $passwordFactory->setDefaultType( 'B' ); // @TODO: Change this to A once that is fixed: https://gerrit.wikimedia.org/r/167523 + // A is unsalted MD5 (thus fast) ... we don't care about security here, this is test only + $passwordFactory->setDefaultType( 'A' ); $newPassword = $passwordFactory->newFromPlaintext( $password , $this->user->getPassword() ); $change = false; -- 2.20.1