diff --git a/core/core.services.yml b/core/core.services.yml index 5c42b23..f6fae6e 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -41,6 +41,7 @@ parameters: exposedHeaders: false maxAge: false supportsCredentials: false + password_hash_cost: 10 services: # Simple cache contexts, directly derived from the request context. cache_context.ip: @@ -914,15 +915,20 @@ services: class: Drupal\Core\Path\PathValidator arguments: ['@router', '@router.no_access_checks', '@current_user', '@path_processor_manager'] -# The argument to the hashing service defined in services.yml, to the -# constructor of PhpassHashedPassword is the log2 number of iterations for -# password stretching. -# @todo increase by 1 every Drupal version in order to counteract increases in -# the speed and power of computers available to crack the hashes. The current -# password hashing method was introduced in Drupal 7 with a log2 count of 15. +# The first argument of the hashing service (constructor of PhpPassword) is +# the 'cost' option of password_hash(). In Drupal 8 the 'cost' has the default +# value used by password_hash() which is 10. Future versions of Drupal may +# increase this value in order to counteract increases in the speed and power +# of computers available to crack the hashes. Note that an increase of 1 will +# double the time needed for password hashing. password: - class: Drupal\Core\Password\PhpassHashedPassword + class: Drupal\Core\Password\PhpPassword + arguments: ['%password_hash_cost%', '@drupal7_password'] + lazy: true + drupal7_password: + class: Drupal\Core\Password\Drupal7Password arguments: [16] + lazy: true request_format_route_filter: class: Drupal\Core\Routing\RequestFormatRouteFilter tags: diff --git a/core/lib/Drupal/Core/Password/PhpassHashedPassword.php b/core/lib/Drupal/Core/Password/Drupal7Password.php similarity index 95% rename from core/lib/Drupal/Core/Password/PhpassHashedPassword.php rename to core/lib/Drupal/Core/Password/Drupal7Password.php index 28313d2..4a39f68 100644 --- a/core/lib/Drupal/Core/Password/PhpassHashedPassword.php +++ b/core/lib/Drupal/Core/Password/Drupal7Password.php @@ -8,9 +8,13 @@ * Secure password hashing functions based on the Portable PHP password * hashing framework. * + * This is a custom password hashing mechanism used in Drupal 7. We keep this + * password hasher in order to validate passwords migrated from Drupal 7 that + * are being rehashed on first user login. + * * @see http://www.openwall.com/phpass/ */ -class PhpassHashedPassword implements PasswordInterface { +class Drupal7Password implements PasswordInterface { /** * The minimum allowed log2 number of iterations for password stretching. */ @@ -218,7 +222,7 @@ public function hash($password) { /** * {@inheritdoc} */ - public function check($password, $hash) { + public function verify($password, $hash) { if (substr($hash, 0, 2) == 'U$') { // This may be an updated password from user_update_7000(). Such hashes // have 'U' added as the first character and need an extra md5() (see the @@ -247,8 +251,7 @@ public function check($password, $hash) { return FALSE; } - // Compare using hashEquals() instead of === to mitigate timing attacks. - return $computed_hash && Crypt::hashEquals($stored_hash, $computed_hash); + return ($computed_hash && $stored_hash === $computed_hash); } /** diff --git a/core/lib/Drupal/Core/Password/PasswordInterface.php b/core/lib/Drupal/Core/Password/PasswordInterface.php index b38cc79..c2d14e9 100644 --- a/core/lib/Drupal/Core/Password/PasswordInterface.php +++ b/core/lib/Drupal/Core/Password/PasswordInterface.php @@ -34,7 +34,7 @@ public function hash($password); * @return bool * TRUE if the password is valid, FALSE if not. */ - public function check($password, $hash); + public function verify($password, $hash); /** * Check whether a hashed password needs to be replaced with a new hash. diff --git a/core/lib/Drupal/Core/Password/PhpPassword.php b/core/lib/Drupal/Core/Password/PhpPassword.php new file mode 100644 index 0000000..fa2ef02 --- /dev/null +++ b/core/lib/Drupal/Core/Password/PhpPassword.php @@ -0,0 +1,97 @@ +=5.5.0) password hashing + * functions. + * + * @see http://php.net/manual/en/book.password.php + */ +class PhpPassword implements PasswordInterface { + + /** + * The algorithmic cost that should be used. This is the same 'cost' option as + * is used by the PHP (>= 5.5.0) password_hash() function. + * + * @var int + * + * @see password_hash(). + * @see http://php.net/manual/en/ref.password.php + */ + protected $cost; + + /** + * The Drupal 7 password hashing service. + * + * @var \Drupal\Core\Password\Drupal7Password + */ + protected $drupal7Password; + + /** + * Constructs a new password hashing instance. + * + * @param int $cost + * The algorithmic cost that should be used. + * @param \Drupal\Core\Password\PasswordInterface $drupal7_password + * The Drupal7 password hashing service. + */ + function __construct($cost, PasswordInterface $drupal7_password) { + $this->cost = $cost; + $this->drupal7Password = $drupal7_password; + } + + /** + * {@inheritdoc} + */ + public function hash($password) { + // Prevent DoS attacks by refusing to hash large passwords. + if (strlen($password) > static::PASSWORD_MAX_LENGTH) { + return FALSE; + } + + return password_hash($password, PASSWORD_BCRYPT, $this->getOptions()); + } + + /** + * {@inheritdoc} + */ + public function verify($password, $hash) { + // Try to verify an legacy hash. This may be an Drupal 6 or 7 hash. + if (substr($hash, 0, 4) != '$2y$') { + return $this->drupal7Password->verify($password, $hash); + } + + return password_verify($password, $hash); + } + + /** + * {@inheritdoc} + */ + public function needsRehash($hash) { + // The PHP 5.5 password_needs_rehash() will return TRUE in two cases: + // - The password is either a Drupal 6 MD5 hash rehashed during migration, + // either a Drupal 7 hashed password. In this case the legacy hash will + // not comply the expected password_needs_rehash() format (prefix '$2y$'). + // - The parameters of hashing engine were changed. For example the + // parameter 'password_hash_cost' (the hashing cost) has been increased in + // core.services.yml. + return password_needs_rehash($hash, PASSWORD_BCRYPT, $this->getOptions()); + } + + /** + * Returns password options. + * + * @return array + * Associative array with password options. + */ + protected function getOptions() { + return ['cost' => $this->cost]; + } + +} diff --git a/core/modules/migrate_drupal/tests/fixtures/drupal7.php b/core/modules/migrate_drupal/tests/fixtures/drupal7.php index 98e5638..e2928a3 100644 --- a/core/modules/migrate_drupal/tests/fixtures/drupal7.php +++ b/core/modules/migrate_drupal/tests/fixtures/drupal7.php @@ -34021,7 +34021,7 @@ 'weight' => '0', )) ->values(array( - 'name' => 'PasswordHashingTest', + 'name' => 'Drupal7PasswordTest', 'type' => 'class', 'filename' => 'modules/simpletest/tests/password.test', 'module' => 'simpletest', diff --git a/core/modules/migrate_drupal/tests/src/Kernel/MigrateUserTest.php b/core/modules/migrate_drupal/tests/src/Kernel/MigrateUserTest.php new file mode 100644 index 0000000..6a86b4a --- /dev/null +++ b/core/modules/migrate_drupal/tests/src/Kernel/MigrateUserTest.php @@ -0,0 +1,191 @@ +installEntitySchema('file'); + $this->installSchema('file', ['file_usage']); + + // Create the user profile field and instance. + entity_create('field_storage_config', array( + 'entity_type' => 'user', + 'field_name' => 'user_picture', + 'type' => 'image', + 'translatable' => '0', + ))->save(); + entity_create('field_config', array( + 'label' => 'User Picture', + 'description' => '', + 'field_name' => 'user_picture', + 'entity_type' => 'user', + 'bundle' => 'user', + 'required' => 0, + ))->save(); + + $file = entity_create('file', array( + 'fid' => 2, + 'uid' => 2, + 'filename' => 'image-test.jpg', + 'uri' => "public://image-test.jpg", + 'filemime' => 'image/jpeg', + 'created' => 1, + 'changed' => 1, + 'status' => FILE_STATUS_PERMANENT, + )); + $file->enforceIsNew(); + file_put_contents($file->getFileUri(), file_get_contents('core/modules/simpletest/files/image-1.png')); + $file->save(); + + $file = entity_create('file', array( + 'fid' => 8, + 'uid' => 8, + 'filename' => 'image-test.png', + 'uri' => "public://image-test.png", + 'filemime' => 'image/png', + 'created' => 1, + 'changed' => 1, + 'status' => FILE_STATUS_PERMANENT, + )); + $file->enforceIsNew(); + file_put_contents($file->getFileUri(), file_get_contents('core/modules/simpletest/files/image-2.jpg')); + $file->save(); + + // Load database dumps to provide source data. + $dumps = array( + $this->getDumpDirectory() . '/Filters.php', + $this->getDumpDirectory() . '/FilterFormats.php', + $this->getDumpDirectory() . '/Variable.php', + $this->getDumpDirectory() . '/ProfileFields.php', + $this->getDumpDirectory() . '/Permission.php', + $this->getDumpDirectory() . '/Role.php', + $this->getDumpDirectory() . '/Users.php', + $this->getDumpDirectory() . '/ProfileValues.php', + $this->getDumpDirectory() . '/UsersRoles.php', + $this->getDumpDirectory() . '/EventTimezones.php', + ); + $this->loadDumps($dumps); + + $id_mappings = array( + 'd6_user_role' => array( + array(array(1), array('anonymous user')), + array(array(2), array('authenticated user')), + array(array(3), array('migrate test role 1')), + array(array(4), array('migrate test role 2')), + array(array(5), array('migrate test role 3')), + ), + 'd6_user_picture_entity_display' => array( + array(array(1), array('user', 'user', 'default', 'user_picture')), + ), + 'd6_user_picture_entity_form_display' => array( + array(array(1), array('user', 'user', 'default', 'user_picture')), + ), + 'd6_user_picture_file' => array( + array(array(2), array(2)), + array(array(8), array(8)), + ), + ); + + $this->prepareMigrations($id_mappings); + + // Migrate users. + $migration = entity_load('migration', 'd6_user'); + $executable = new MigrateExecutable($migration, $this); + $executable->import(); + } + + /** + * Tests the Drupal6 user to Drupal 8 migration. + */ + public function testUser() { + $users = Database::getConnection('default', 'migrate') + ->select('users', 'u') + ->fields('u') + ->execute() + ->fetchAll(); + + foreach ($users as $source) { + // Get roles directly from the source. + $rids = Database::getConnection('default', 'migrate') + ->select('users_roles', 'ur') + ->fields('ur', array('rid')) + ->condition('ur.uid', $source->uid) + ->execute() + ->fetchCol(); + $roles = array(RoleInterface::AUTHENTICATED_ID); + $migration_role = entity_load('migration', 'd6_user_role'); + foreach ($rids as $rid) { + $role = $migration_role->getIdMap()->lookupDestinationId(array($rid)); + $roles[] = reset($role); + } + + /** @var \Drupal\user\UserInterface $user */ + $user = User::load($source->uid); + $this->assertIdentical($source->uid, $user->id()); + $this->assertIdentical($source->name, $user->label()); + $this->assertIdentical($source->mail, $user->getEmail()); + $this->assertIdentical($source->created, $user->getCreatedTime()); + $this->assertIdentical($source->access, $user->getLastAccessedTime()); + $this->assertIdentical($source->login, $user->getLastLoginTime()); + $is_blocked = $source->status == 0; + $this->assertIdentical($is_blocked, $user->isBlocked()); + // $user->getPreferredLangcode() might fallback to default language if the + // user preferred language is not configured on the site. We just want to + // test if the value was imported correctly. + $this->assertIdentical($source->language, $user->preferred_langcode->value); + $time_zone = $source->expected_timezone ?: $this->config('system.date')->get('timezone.default'); + $this->assertIdentical($time_zone, $user->getTimeZone()); + $this->assertIdentical($source->init, $user->getInitialEmail()); + $this->assertIdentical($roles, $user->getRoles()); + + // We have one empty picture in the data so don't try load that. + if (!empty($source->picture)) { + // Test the user picture. + $file = File::load($user->user_picture->target_id); + $this->assertIdentical(basename($source->picture), $file->getFilename()); + } + + // Use the API to verify if the password has been salted and re-hashed to + // conform the Drupal >= 7. + $this->assertTrue(\Drupal::service('password')->verify($source->pass_plain, $user->getPassword())); + } + } + +} diff --git a/core/modules/simpletest/src/KernelTestBase.php b/core/modules/simpletest/src/KernelTestBase.php index fbf58a5..cd39245 100644 --- a/core/modules/simpletest/src/KernelTestBase.php +++ b/core/modules/simpletest/src/KernelTestBase.php @@ -364,8 +364,9 @@ public function containerBuild(ContainerBuilder $container) { $definition->clearTag('path_processor_inbound')->clearTag('path_processor_outbound'); } - if ($container->hasDefinition('password')) { - $container->getDefinition('password')->setArguments(array(1)); + if ($container->hasDefinition('password') && $container->hasDefinition('drupal7_password')) { + $container->getDefinition('drupal7_password')->setArguments([1]); + $container->getDefinition('password')->setArguments([4, $container->get('drupal7_password')]); } // Register the stream wrapper manager. diff --git a/core/modules/user/src/Entity/User.php b/core/modules/user/src/Entity/User.php index 062af44..3d6fbd2 100644 --- a/core/modules/user/src/Entity/User.php +++ b/core/modules/user/src/Entity/User.php @@ -395,7 +395,7 @@ public function setExistingPassword($password) { * {@inheritdoc} */ public function checkExistingPassword(UserInterface $account_unchanged) { - return strlen($this->get('pass')->existing) > 0 && \Drupal::service('password')->check(trim($this->get('pass')->existing), $account_unchanged->getPassword()); + return strlen($this->get('pass')->existing) > 0 && \Drupal::service('password')->verify(trim($this->get('pass')->existing), $account_unchanged->getPassword()); } /** diff --git a/core/modules/user/src/Tests/UserLoginTest.php b/core/modules/user/src/Tests/UserLoginTest.php index af00c74..19353b4 100644 --- a/core/modules/user/src/Tests/UserLoginTest.php +++ b/core/modules/user/src/Tests/UserLoginTest.php @@ -4,6 +4,7 @@ use Drupal\simpletest\WebTestBase; use Drupal\user\Entity\User; +use Drupal\user\UserInterface; /** * Ensure that login works as expected. @@ -13,6 +14,20 @@ class UserLoginTest extends WebTestBase { /** + * Drupal password hasher service. + * + * @var \Drupal\Core\Password\PasswordInterface + */ + private $passwordHasher; + + /** + * Drupal 7 password hasher service. + * + * @var \Drupal\Core\Password\PasswordInterface + */ + private $drupal7PasswordHasher; + + /** * Tests login with destination. */ function testLoginCacheTagsAndDestination() { @@ -26,6 +41,9 @@ function testLoginCacheTagsAndDestination() { $edit = array('name' => $user->getUserName(), 'pass' => $user->pass_raw); $this->drupalPostForm(NULL, $edit, t('Log in')); $this->assertUrl('foo', [], 'Redirected to the correct URL'); + + $this->passwordHasher = $this->container->get('password'); + $this->drupal7PasswordHasher = $this->container->get('drupal7_password'); } /** @@ -106,30 +124,23 @@ function testPerUserLoginFloodControl() { } /** - * Test that user password is re-hashed upon login after changing $count_log2. + * Test that user password is re-hashed upon login after changing the cost. */ function testPasswordRehashOnLogin() { - // Determine default log2 for phpass hashing algorithm - $default_count_log2 = 16; - - // Retrieve instance of password hashing algorithm - $password_hasher = $this->container->get('password'); - // Create a new user and authenticate. $account = $this->drupalCreateUser(array()); $password = $account->pass_raw; $this->drupalLogin($account); $this->drupalLogout(); - // Load the stored user. The password hash should reflect $default_count_log2. + // Load the stored user. The password hash should reflect $default_cost. $user_storage = $this->container->get('entity.manager')->getStorage('user'); $account = User::load($account->id()); - $this->assertIdentical($password_hasher->getCountLog2($account->getPassword()), $default_count_log2); + $this->assertTrue($this->passwordHasher->verify($password, $account->getPassword())); - // Change the required number of iterations by loading a test-module - // containing the necessary container builder code and then verify that the - // users password gets rehashed during the login. - $overridden_count_log2 = 19; - \Drupal::service('module_installer')->install(array('user_custom_phpass_params_test')); + // Change the required cost by loading a test-module containing the + // necessary container builder code and then verify that the users password + // gets rehashed during the login. + \Drupal::service('module_installer')->install(array('user_custom_pass_hash_params_test')); $this->resetAll(); $account->pass_raw = $password; @@ -137,11 +148,51 @@ function testPasswordRehashOnLogin() { // Load the stored user, which should have a different password hash now. $user_storage->resetCache(array($account->id())); $account = $user_storage->load($account->id()); - $this->assertIdentical($password_hasher->getCountLog2($account->getPassword()), $overridden_count_log2); - $this->assertTrue($password_hasher->check($password, $account->getPassword())); + $this->assertTrue($this->passwordHasher->verify($password, $account->getPassword())); } /** + * Test MD5 (Drupal 6) passwords rehashing. + */ + public function testDrupal6MigratedPasswordRehashing() { + /** @var \Drupal\user\UserInterface $account */ + $account = $this->drupalCreateUser(); + $plain = $account->pass_raw; + + // We pretend that the user was migrated from Drupal 6. + $md5_pass = md5($plain); + $migrated_pass = 'U' . $this->passwordHasher->hash($md5_pass); + $this->storeHashedPassword($account, $migrated_pass); + + // User first login after migration. + $this->drupalLogin($account); + $this->drupalLogout(); + + // After logging in the user password has been rehashed and is valid. + $this->assertTrue($this->passwordHasher->verify($plain, $account->getPassword())); +} + + /** + * Test Drupal 7 passwords rehashing. + */ + public function testDrupal7MigratedPasswordRehashing() { + /** @var \Drupal\user\UserInterface $account */ + $account = $this->drupalCreateUser(); + $plain = $account->pass_raw; + + // We pretend that the user was migrated from Drupal 7. + $d7_hash = $this->drupal7PasswordHasher->hash($plain); + $this->storeHashedPassword($account, $d7_hash); + + // User first login after migration. + $this->drupalLogin($account); + $this->drupalLogout(); + + // After logging in the user password has been rehashed and is valid. + $this->assertTrue($this->passwordHasher->verify($plain, $account->getPassword())); +} + + /** * Make an unsuccessful login attempt. * * @param \Drupal\user\Entity\User $account @@ -175,4 +226,22 @@ function assertFailedLogin($account, $flood_trigger = NULL) { } } + /** + * Updates the hashed user password bypassing the API. + * + * We want to set an already hashed password. + * + * @param \Drupal\user\UserInterface $account + * The user account. + * @param string $hashed_password + * An already hashed password. + */ + protected function storeHashedPassword(UserInterface $account, $hashed_password) { + $account->setPassword($hashed_password); + db_update('users_field_data') + ->fields(['pass' => $hashed_password]) + ->condition('uid', $account->id()) + ->execute(); + } + } diff --git a/core/modules/user/src/UserAuth.php b/core/modules/user/src/UserAuth.php index 9fbcf09..30da858 100644 --- a/core/modules/user/src/UserAuth.php +++ b/core/modules/user/src/UserAuth.php @@ -47,7 +47,7 @@ public function authenticate($username, $password) { $account_search = $this->entityManager->getStorage('user')->loadByProperties(array('name' => $username)); if ($account = reset($account_search)) { - if ($this->passwordChecker->check($password, $account->getPassword())) { + if ($this->passwordChecker->verify($password, $account->getPassword())) { // Successful authentication. $uid = $account->id(); diff --git a/core/modules/user/tests/modules/user_custom_pass_hash_params_test/user_custom_pass_hash_params_test.info.yml b/core/modules/user/tests/modules/user_custom_pass_hash_params_test/user_custom_pass_hash_params_test.info.yml new file mode 100644 index 0000000..aca50c4 --- /dev/null +++ b/core/modules/user/tests/modules/user_custom_pass_hash_params_test/user_custom_pass_hash_params_test.info.yml @@ -0,0 +1,6 @@ +name: 'User custom password hash params test' +type: module +description: 'Support module for testing custom hashing password algorithm parameters.' +package: Testing +version: VERSION +core: 8.x diff --git a/core/modules/user/tests/modules/user_custom_pass_hash_params_test/user_custom_pass_hash_params_test.services.yml b/core/modules/user/tests/modules/user_custom_pass_hash_params_test/user_custom_pass_hash_params_test.services.yml new file mode 100644 index 0000000..56c1281 --- /dev/null +++ b/core/modules/user/tests/modules/user_custom_pass_hash_params_test/user_custom_pass_hash_params_test.services.yml @@ -0,0 +1,7 @@ +services: + password: + class: Drupal\Core\Password\PhpPassword + arguments: [11, '@drupal7_password'] + drupal7_password: + class: Drupal\Core\Password\Drupal7Password + arguments: [16] diff --git a/core/modules/user/tests/modules/user_custom_phpass_params_test/user_custom_phpass_params_test.info.yml b/core/modules/user/tests/modules/user_custom_phpass_params_test/user_custom_phpass_params_test.info.yml deleted file mode 100644 index 68b8ff9..0000000 --- a/core/modules/user/tests/modules/user_custom_phpass_params_test/user_custom_phpass_params_test.info.yml +++ /dev/null @@ -1,6 +0,0 @@ -name: 'User custom phpass params test' -type: module -description: 'Support module for testing custom phpass password algorithm parameters.' -package: Testing -version: VERSION -core: 8.x diff --git a/core/modules/user/tests/modules/user_custom_phpass_params_test/user_custom_phpass_params_test.services.yml b/core/modules/user/tests/modules/user_custom_phpass_params_test/user_custom_phpass_params_test.services.yml deleted file mode 100644 index 8950bfe..0000000 --- a/core/modules/user/tests/modules/user_custom_phpass_params_test/user_custom_phpass_params_test.services.yml +++ /dev/null @@ -1,4 +0,0 @@ -services: - password: - class: Drupal\Core\Password\PhpassHashedPassword - arguments: [19] diff --git a/core/modules/user/tests/src/Unit/UserAuthTest.php b/core/modules/user/tests/src/Unit/UserAuthTest.php index 1b96d8f..6d28101 100644 --- a/core/modules/user/tests/src/Unit/UserAuthTest.php +++ b/core/modules/user/tests/src/Unit/UserAuthTest.php @@ -129,7 +129,7 @@ public function testAuthenticateWithIncorrectPassword() { ->will($this->returnValue(array($this->testUser))); $this->passwordService->expects($this->once()) - ->method('check') + ->method('verify') ->with($this->password, $this->testUser->getPassword()) ->will($this->returnValue(FALSE)); @@ -152,7 +152,7 @@ public function testAuthenticateWithCorrectPassword() { ->will($this->returnValue(array($this->testUser))); $this->passwordService->expects($this->once()) - ->method('check') + ->method('verify') ->with($this->password, $this->testUser->getPassword()) ->will($this->returnValue(TRUE)); @@ -179,7 +179,7 @@ public function testAuthenticateWithZeroPassword() { ->will($this->returnValue(array($this->testUser))); $this->passwordService->expects($this->once()) - ->method('check') + ->method('verify') ->with(0, 0) ->will($this->returnValue(TRUE)); @@ -207,7 +207,7 @@ public function testAuthenticateWithCorrectPasswordAndNewPasswordHash() { ->will($this->returnValue(array($this->testUser))); $this->passwordService->expects($this->once()) - ->method('check') + ->method('verify') ->with($this->password, $this->testUser->getPassword()) ->will($this->returnValue(TRUE)); $this->passwordService->expects($this->once()) diff --git a/core/tests/Drupal/KernelTests/KernelTestBase.php b/core/tests/Drupal/KernelTests/KernelTestBase.php index 4da96ba..3b72863 100644 --- a/core/tests/Drupal/KernelTests/KernelTestBase.php +++ b/core/tests/Drupal/KernelTests/KernelTestBase.php @@ -624,9 +624,11 @@ public function register(ContainerBuilder $container) { ->clearTag('path_processor_outbound'); } - if ($container->hasDefinition('password')) { + if ($container->hasDefinition('password') && $container->hasDefinition('drupal7_password')) { + $container->getDefinition('drupal7_password') + ->setArguments([1]); $container->getDefinition('password') - ->setArguments(array(1)); + ->setArguments([4, $container->get('drupal7_password')]); } TestServiceProvider::addRouteProvider($container); } diff --git a/core/tests/Drupal/Tests/Core/Password/PasswordHashingTest.php b/core/tests/Drupal/Tests/Core/Password/Drupal7PasswordTest.php similarity index 72% rename from core/tests/Drupal/Tests/Core/Password/PasswordHashingTest.php rename to core/tests/Drupal/Tests/Core/Password/Drupal7PasswordTest.php index 5c378ed..6faf929 100644 --- a/core/tests/Drupal/Tests/Core/Password/PasswordHashingTest.php +++ b/core/tests/Drupal/Tests/Core/Password/Drupal7PasswordTest.php @@ -2,22 +2,22 @@ /** * @file - * Contains \Drupal\Tests\Core\Password\PasswordHashingTest. + * Contains Drupal\system\Tests\System\Drupal7PasswordTest. */ namespace Drupal\Tests\Core\Password; -use Drupal\Core\Password\PhpassHashedPassword; +use Drupal\Core\Password\Drupal7Password; use Drupal\Core\Password\PasswordInterface; use Drupal\Tests\UnitTestCase; /** * Unit tests for password hashing API. * - * @coversDefaultClass \Drupal\Core\Password\PhpassHashedPassword + * @coversDefaultClass \Drupal\Core\Password\Drupal7Password * @group System */ -class PasswordHashingTest extends UnitTestCase { +class Drupal7PasswordTest extends UnitTestCase { /** * The user for testing. @@ -50,7 +50,7 @@ class PasswordHashingTest extends UnitTestCase { /** * The password hasher under test. * - * @var \Drupal\Core\Password\PhpassHashedPassword + * @var \Drupal\Core\Password\Drupal7Password */ protected $passwordHasher; @@ -60,7 +60,7 @@ class PasswordHashingTest extends UnitTestCase { protected function setUp() { parent::setUp(); $this->password = $this->randomMachineName(); - $this->passwordHasher = new PhpassHashedPassword(1); + $this->passwordHasher = new Drupal7Password(1); $this->hashedPassword = $this->passwordHasher->hash($this->password); $this->md5HashedPassword = 'U' . $this->passwordHasher->hash(md5($this->password)); } @@ -71,12 +71,11 @@ protected function setUp() { * @covers ::enforceLog2Boundaries */ public function testWithinBounds() { - $hasher = new FakePhpassHashedPassword(); - $this->assertEquals(PhpassHashedPassword::MIN_HASH_COUNT, $hasher->enforceLog2Boundaries(1), "Min hash count enforced"); - $this->assertEquals(PhpassHashedPassword::MAX_HASH_COUNT, $hasher->enforceLog2Boundaries(100), "Max hash count enforced"); + $hasher = new FakeDrupal7Password(); + $this->assertEquals(Drupal7Password::MIN_HASH_COUNT, $hasher->enforceLog2Boundaries(1), 'Min hash count enforced'); + $this->assertEquals(Drupal7Password::MAX_HASH_COUNT, $hasher->enforceLog2Boundaries(100), 'Max hash count enforced'); } - /** * Test a password needs update. * @@ -92,14 +91,14 @@ public function testPasswordNeedsUpdate() { * * @covers ::hash * @covers ::getCountLog2 - * @covers ::check + * @covers ::verify * @covers ::needsRehash */ public function testPasswordHashing() { - $this->assertSame($this->passwordHasher->getCountLog2($this->hashedPassword), PhpassHashedPassword::MIN_HASH_COUNT, 'Hashed password has the minimum number of log2 iterations.'); + $this->assertSame($this->passwordHasher->getCountLog2($this->hashedPassword), Drupal7Password::MIN_HASH_COUNT); $this->assertNotEquals($this->hashedPassword, $this->md5HashedPassword, 'Password hashes not the same.'); - $this->assertTrue($this->passwordHasher->check($this->password, $this->md5HashedPassword), 'Password check succeeds.'); - $this->assertTrue($this->passwordHasher->check($this->password, $this->hashedPassword), 'Password check succeeds.'); + $this->assertTrue($this->passwordHasher->verify($this->password, $this->md5HashedPassword), 'Password check succeeds.'); + $this->assertTrue($this->passwordHasher->verify($this->password, $this->hashedPassword), 'Password check succeeds.'); // Since the log2 setting hasn't changed and the user has a valid password, // userNeedsNewHash() should return FALSE. $this->assertFalse($this->passwordHasher->needsRehash($this->hashedPassword), 'Does not need a new hash.'); @@ -110,22 +109,22 @@ public function testPasswordHashing() { * * @covers ::hash * @covers ::getCountLog2 - * @covers ::check + * @covers ::verify * @covers ::needsRehash */ public function testPasswordRehashing() { // Increment the log2 iteration to MIN + 1. - $password_hasher = new PhpassHashedPassword(PhpassHashedPassword::MIN_HASH_COUNT + 1); + $password_hasher = new Drupal7Password(Drupal7Password::MIN_HASH_COUNT + 1); $this->assertTrue($password_hasher->needsRehash($this->hashedPassword), 'Needs a new hash after incrementing the log2 count.'); // Re-hash the password. $rehashed_password = $password_hasher->hash($this->password); - $this->assertSame($password_hasher->getCountLog2($rehashed_password), PhpassHashedPassword::MIN_HASH_COUNT + 1, 'Re-hashed password has the correct number of log2 iterations.'); + $this->assertSame($password_hasher->getCountLog2($rehashed_password), Drupal7Password::MIN_HASH_COUNT + 1); $this->assertNotEquals($rehashed_password, $this->hashedPassword, 'Password hash changed again.'); // Now the hash should be OK. $this->assertFalse($password_hasher->needsRehash($rehashed_password), 'Re-hashed password does not need a new hash.'); - $this->assertTrue($password_hasher->check($this->password, $rehashed_password), 'Password check succeeds with re-hashed password.'); - $this->assertTrue($this->passwordHasher->check($this->password, $rehashed_password), 'Password check succeeds with re-hashed password with original hasher.'); + $this->assertTrue($password_hasher->verify($this->password, $rehashed_password), 'Password check succeeds with re-hashed password.'); + $this->assertTrue($this->passwordHasher->verify($this->password, $rehashed_password), 'Password check succeeds with re-hashed password with original hasher.'); } /** @@ -175,15 +174,13 @@ public function providerLongPasswords() { /** * A fake class for tests. */ -class FakePhpassHashedPassword extends PhpassHashedPassword { +class FakeDrupal7Password extends Drupal7Password { function __construct() { // Noop. } - /** - * Exposes this method as public for tests. - */ + // Expose this method as public for tests. public function enforceLog2Boundaries($count_log2) { return parent::enforceLog2Boundaries($count_log2); } diff --git a/core/tests/Drupal/Tests/Core/Password/PhpPhpPasswordTest.php b/core/tests/Drupal/Tests/Core/Password/PhpPhpPasswordTest.php new file mode 100644 index 0000000..7fb679c --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Password/PhpPhpPasswordTest.php @@ -0,0 +1,162 @@ +passwordHasher = new PhpPassword(4, $d7_hashing_service); + $this->password = $this->randomMachineName(); + $this->md5Hash = md5($this->password); + $this->d7Hash = $d7_hashing_service->hash($this->password); + } + + /** + * Test a password needs update. + * + * @covers ::needsRehash + */ + public function testPasswordNeedsUpdate() { + // The md5 password should be flagged as needing an rehashing. + $this->assertTrue($this->passwordHasher->needsRehash($this->md5Hash)); + // The Drupal 7 password should be flagged as needing an rehashing. + $this->assertTrue($this->passwordHasher->needsRehash($this->d7Hash)); + } + + /** + * Test password hashing. + * + * @covers ::hash + * @covers ::verify + * @covers ::needsRehash + */ + public function testPasswordHashing() { + $this->hashedPassword = $this->passwordHasher->hash($this->password); + + $this->assertNotEquals($this->hashedPassword, $this->md5Hash); + $this->assertNotEquals($this->hashedPassword, $this->d7Hash); + + $this->assertTrue($this->passwordHasher->verify($this->password, $this->hashedPassword)); + $this->assertFalse($this->passwordHasher->needsRehash($this->hashedPassword)); + } + + /** + * Tests password rehashing. + * + * @covers ::hash + * @covers ::check + * @covers ::needsRehash + */ + public function testPasswordRehashing() { + // Increment the cost from 4 to 5. + $this->passwordHasher = new PhpPassword(5, new Drupal7Password(1)); + $this->assertTrue($this->passwordHasher->needsRehash($this->hashedPassword)); + + // Re-hash the password. + $rehashed_password = $this->passwordHasher->hash($this->password); + $this->assertNotEquals($rehashed_password, $this->hashedPassword); + + // Now the hash should be OK. + $this->assertFalse($this->passwordHasher->needsRehash($rehashed_password)); + $this->assertTrue($this->passwordHasher->verify($this->password, $rehashed_password)); + } + + /** + * Verifies that passwords longer than 512 bytes are not hashed. + * + * @covers ::hash + * + * @dataProvider providerLongPasswords + */ + public function testLongPassword($password, $allowed) { + + $hashed_password = $this->passwordHasher->hash($password); + + if ($allowed) { + $this->assertNotFalse($hashed_password); + } + else { + $this->assertFalse($hashed_password); + } + } + + /** + * Provides the test matrix for testLongPassword(). + */ + public function providerLongPasswords() { + // '512 byte long password is allowed.' + $passwords['allowed'] = array(str_repeat('x', PasswordInterface::PASSWORD_MAX_LENGTH), TRUE); + // 513 byte long password is not allowed. + $passwords['too_long'] = array(str_repeat('x', PasswordInterface::PASSWORD_MAX_LENGTH + 1), FALSE); + + // Check a string of 3-byte UTF-8 characters, 510 byte long password is + // allowed. + $len = floor(PasswordInterface::PASSWORD_MAX_LENGTH / 3); + $diff = PasswordInterface::PASSWORD_MAX_LENGTH % 3; + $passwords['utf8'] = array(str_repeat('€', $len), TRUE); + // 512 byte long password is allowed. + $passwords['ut8_extended'] = array($passwords['utf8'][0] . str_repeat('x', $diff), TRUE); + + // Check a string of 3-byte UTF-8 characters, 513 byte long password is + // allowed. + $passwords['utf8_too_long'] = array(str_repeat('€', $len + 1), FALSE); + return $passwords; + } + +}