From 3c0892d082db2fd872212ff36e1ce1bf1a9bc0ab Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Tue, 31 Mar 2015 23:46:24 +0100 Subject: [PATCH] =?UTF-8?q?Issue=20#2418119=20by=20Berdir,=20jhedstrom,=20?= =?UTF-8?q?larowlan,=20martin107,=20nlisgo,=20klausi,=20fago,=20G=C3=A1bor?= =?UTF-8?q?=20Hojtsy:=20REST=20user=20updates=20bypass=20tightened=20user?= =?UTF-8?q?=20account=20change=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../Drupal/Core/Entity/ContentEntityBase.php | 2 +- .../Plugin/Field/FieldType/PasswordItem.php | 48 ++- .../Plugin/rest/resource/EntityResource.php | 8 +- core/modules/rest/src/Tests/RESTTestBase.php | 10 + core/modules/rest/src/Tests/UpdateTest.php | 75 ++++- .../modules/entity_test/entity_test.module | 2 +- core/modules/user/src/AccountForm.php | 17 +- core/modules/user/src/Entity/User.php | 39 +-- .../ProtectedUserFieldConstraint.php | 29 ++ .../ProtectedUserFieldConstraintValidator.php | 98 ++++++ core/modules/user/src/Tests/UserEditTest.php | 2 +- .../UserValidateCurrentPassCustomFormTest.php | 57 ---- core/modules/user/src/UserInterface.php | 25 ++ .../src/Form/TestCurrentPassword.php | 68 ---- .../user_form_test/user_form_test.routing.yml | 7 - ...tectedUserFieldConstraintValidatorTest.php | 313 ++++++++++++++++++ core/modules/user/user.module | 24 -- 17 files changed, 628 insertions(+), 196 deletions(-) create mode 100644 core/modules/user/src/Plugin/Validation/Constraint/ProtectedUserFieldConstraint.php create mode 100644 core/modules/user/src/Plugin/Validation/Constraint/ProtectedUserFieldConstraintValidator.php delete mode 100644 core/modules/user/src/Tests/UserValidateCurrentPassCustomFormTest.php delete mode 100644 core/modules/user/tests/modules/user_form_test/src/Form/TestCurrentPassword.php create mode 100644 core/modules/user/tests/src/Unit/Plugin/Validation/Constraint/ProtectedUserFieldConstraintValidatorTest.php diff --git a/core/lib/Drupal/Core/Entity/ContentEntityBase.php b/core/lib/Drupal/Core/Entity/ContentEntityBase.php index c26b98b418b7..40da4b84cfbe 100644 --- a/core/lib/Drupal/Core/Entity/ContentEntityBase.php +++ b/core/lib/Drupal/Core/Entity/ContentEntityBase.php @@ -585,7 +585,7 @@ public function onChange($name) { case $this->defaultLangcodeKey: // @todo Use a standard method to make the default_langcode field // read-only. See https://www.drupal.org/node/2443991. - if (isset($this->values[$this->defaultLangcodeKey])) { + if (isset($this->values[$this->defaultLangcodeKey]) && $this->get($this->defaultLangcodeKey)->value != $this->isDefaultTranslation()) { $this->get($this->defaultLangcodeKey)->setValue($this->isDefaultTranslation(), FALSE); $message = SafeMarkup::format('The default translation flag cannot be changed (@langcode).', array('@langcode' => $this->activeLangcode)); throw new \LogicException($message); diff --git a/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/PasswordItem.php b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/PasswordItem.php index 40945c09fa9f..7de771abf5d4 100644 --- a/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/PasswordItem.php +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/PasswordItem.php @@ -7,6 +7,11 @@ namespace Drupal\Core\Field\Plugin\Field\FieldType; +use Drupal\Core\Entity\EntityMalformedException; +use Drupal\Core\Field\FieldStorageDefinitionInterface; +use Drupal\Core\StringTranslation\TranslationWrapper; +use Drupal\Core\TypedData\DataDefinition; + /** * Defines the 'password' entity field type. * @@ -17,4 +22,45 @@ * no_ui = TRUE, * ) */ -class PasswordItem extends StringItem {} +class PasswordItem extends StringItem { + + /** + * {@inheritdoc} + */ + public static function propertyDefinitions(FieldStorageDefinitionInterface $field_definition) { + $properties['value'] = DataDefinition::create('string') + ->setLabel(new TranslationWrapper('The hashed password')) + ->setSetting('case_sensitive', TRUE); + $properties['existing'] = DataDefinition::create('string') + ->setLabel(new TranslationWrapper('Existing password')); + + return $properties; + } + + /** + * {@inheritdoc} + */ + public function preSave() { + parent::preSave(); + + $entity = $this->getEntity(); + + // Update the user password if it has changed. + if ($entity->isNew() || ($this->value && $this->value != $entity->original->{$this->getFieldDefinition()->getName()}->value)) { + // Allow alternate password hashing schemes. + $this->value = \Drupal::service('password')->hash(trim($this->value)); + // Abort if the hashing failed and returned FALSE. + if (!$this->value) { + throw new EntityMalformedException('The entity does not have a password.'); + } + } + + if (!$entity->isNew()) { + // If the password is empty, that means it was not changed, so use the + // original password. + if (empty($this->value)) { + $this->value = $entity->original->{$this->getFieldDefinition()->getName()}->value; + } + } + } +} diff --git a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php index 67dec06c4361..b07f3e68c20d 100644 --- a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php @@ -145,13 +145,11 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity if ($field_name == $langcode_key && $field->isEmpty()) { continue; } - if ($field->isEmpty() && !$original_entity->get($field_name)->access('delete')) { - throw new AccessDeniedHttpException(SafeMarkup::format('Access denied on deleting field @field.', array('@field' => $field_name))); - } - $original_entity->set($field_name, $field->getValue()); - if (!$original_entity->get($field_name)->access('update')) { + + if (!$original_entity->get($field_name)->access('edit')) { throw new AccessDeniedHttpException(SafeMarkup::format('Access denied on updating field @field.', array('@field' => $field_name))); } + $original_entity->set($field_name, $field->getValue()); } // Validate the received data before saving. diff --git a/core/modules/rest/src/Tests/RESTTestBase.php b/core/modules/rest/src/Tests/RESTTestBase.php index 4856bde2a977..e8e10803acd8 100644 --- a/core/modules/rest/src/Tests/RESTTestBase.php +++ b/core/modules/rest/src/Tests/RESTTestBase.php @@ -332,6 +332,16 @@ protected function entityPermissions($entity_type, $operation) { case 'delete': return array('delete any resttest content'); } + + case 'user': + switch ($operation) { + case 'view': + return ['access user profiles']; + + default: + return ['administer users']; + + } } } diff --git a/core/modules/rest/src/Tests/UpdateTest.php b/core/modules/rest/src/Tests/UpdateTest.php index 6046ce9f6611..481e33b5f02e 100644 --- a/core/modules/rest/src/Tests/UpdateTest.php +++ b/core/modules/rest/src/Tests/UpdateTest.php @@ -89,7 +89,7 @@ public function testPatchUpdate() { // Enable access protection for the text field. // @see entity_test_entity_field_access() - $entity->field_test_text->value = 'no delete access value'; + $entity->field_test_text->value = 'no edit access value'; $entity->field_test_text->format = 'plain_text'; $entity->save(); @@ -99,7 +99,7 @@ public function testPatchUpdate() { // Re-load the entity from the database. $entity = entity_load($entity_type, $entity->id(), TRUE); - $this->assertEqual($entity->field_test_text->value, 'no delete access value', 'Text field was not deleted.'); + $this->assertEqual($entity->field_test_text->value, 'no edit access value', 'Text field was not deleted.'); // Try to update an access protected field. $normalized = $serializer->normalize($patch_entity, $this->defaultFormat, $context); @@ -110,9 +110,12 @@ public function testPatchUpdate() { // Re-load the entity from the database. $entity = entity_load($entity_type, $entity->id(), TRUE); - $this->assertEqual($entity->field_test_text->value, 'no delete access value', 'Text field was not updated.'); + $this->assertEqual($entity->field_test_text->value, 'no edit access value', 'Text field was not updated.'); // Try to update the field with a text format this user has no access to. + // First change the original field value so we're allowed to edit it again. + $entity->field_test_text->value = 'test'; + $entity->save(); $patch_entity->set('field_test_text', array( 'value' => 'test', 'format' => 'full_html', @@ -123,7 +126,7 @@ public function testPatchUpdate() { // Re-load the entity from the database. $entity = entity_load($entity_type, $entity->id(), TRUE); - $this->assertEqual($entity->field_test_text->value, 'no delete access value', 'Text field was not updated.'); + $this->assertEqual($entity->field_test_text->format, 'plain_text', 'Text format was not updated.'); // Restore the valid test value. $entity->field_test_text->value = $this->randomString(); @@ -160,4 +163,68 @@ public function testPatchUpdate() { $this->assertResponse(405); } + /** + * Tests several valid and invalid update requests for the 'user' entity type. + */ + public function testUpdateUser() { + $serializer = $this->container->get('serializer'); + $entity_type = 'user'; + // Enables the REST service for 'user' entity type. + $this->enableService('entity:' . $entity_type, 'PATCH'); + $permissions = $this->entityPermissions($entity_type, 'update'); + $permissions[] = 'restful patch entity:' . $entity_type; + $account = $this->drupalCreateUser($permissions); + $account->set('mail', 'old-email@example.com'); + $this->drupalLogin($account); + + // Create an entity and save it to the database. + $account->save(); + $account->set('changed', NULL); + + // Try and set a new email without providing the password. + $account->set('mail', 'new-email@example.com'); + $context = ['account' => $account]; + $normalized = $serializer->normalize($account, $this->defaultFormat, $context); + $serialized = $serializer->serialize($normalized, $this->defaultFormat, $context); + $response = $this->httpRequest($account->urlInfo(), 'PATCH', $serialized, $this->defaultMimeType); + $this->assertResponse(422); + $error = Json::decode($response); + $this->assertEqual($error['error'], "Unprocessable Entity: validation failed.\nmail: Your current password is missing or incorrect; it's required to change the <em class=\"placeholder\">Email</em>.\n"); + + // Try and send the new email with a password. + $normalized['pass'][0]['existing'] = 'wrong'; + $serialized = $serializer->serialize($normalized, $this->defaultFormat, $context); + $response = $this->httpRequest($account->urlInfo(), 'PATCH', $serialized, $this->defaultMimeType); + $this->assertResponse(422); + $error = Json::decode($response); + $this->assertEqual($error['error'], "Unprocessable Entity: validation failed.\nmail: Your current password is missing or incorrect; it's required to change the <em class=\"placeholder\">Email</em>.\n"); + + // Try again with the password. + $normalized['pass'][0]['existing'] = $account->pass_raw; + $serialized = $serializer->serialize($normalized, $this->defaultFormat, $context); + $this->httpRequest($account->urlInfo(), 'PATCH', $serialized, $this->defaultMimeType); + $this->assertResponse(204); + + // Try to change the password without providing the current password. + $new_password = $this->randomString(); + $normalized = $serializer->normalize($account, $this->defaultFormat, $context); + $normalized['pass'][0]['value'] = $new_password; + $serialized = $serializer->serialize($normalized, $this->defaultFormat, $context); + $response = $this->httpRequest($account->urlInfo(), 'PATCH', $serialized, $this->defaultMimeType); + $this->assertResponse(422); + $error = Json::decode($response); + $this->assertEqual($error['error'], "Unprocessable Entity: validation failed.\npass: Your current password is missing or incorrect; it's required to change the <em class=\"placeholder\">Password</em>.\n"); + + // Try again with the password. + $normalized['pass'][0]['existing'] = $account->pass_raw; + $serialized = $serializer->serialize($normalized, $this->defaultFormat, $context); + $this->httpRequest($account->urlInfo(), 'PATCH', $serialized, $this->defaultMimeType); + $this->assertResponse(204); + + // Verify that we can log in with the new password. + $account->pass_raw = $new_password; + $this->drupalLogin($account); + + } + } diff --git a/core/modules/system/tests/modules/entity_test/entity_test.module b/core/modules/system/tests/modules/entity_test/entity_test.module index 64418c7291e5..df80c1a9ddbe 100644 --- a/core/modules/system/tests/modules/entity_test/entity_test.module +++ b/core/modules/system/tests/modules/entity_test/entity_test.module @@ -343,7 +343,7 @@ function entity_test_entity_field_access($operation, FieldDefinitionInterface $f if ($items->value == 'no access value') { return AccessResult::forbidden()->cacheUntilEntityChanges($items->getEntity()); } - elseif ($operation == 'delete' && $items->value == 'no delete access value') { + elseif ($operation == 'edit' && $items->value == 'no edit access value') { return AccessResult::forbidden()->cacheUntilEntityChanges($items->getEntity()); } } diff --git a/core/modules/user/src/AccountForm.php b/core/modules/user/src/AccountForm.php index f61fd0614f37..16ab69f6f0f4 100644 --- a/core/modules/user/src/AccountForm.php +++ b/core/modules/user/src/AccountForm.php @@ -154,11 +154,6 @@ public function form(array $form, FormStateInterface $form_state) { // The user must enter their current password to change to a new one. if ($user->id() == $account->id()) { - $form['account']['current_pass_required_values'] = array( - '#type' => 'value', - '#value' => $protected_values, - ); - $form['account']['current_pass'] = array( '#type' => 'password', '#title' => $this->t('Current password'), @@ -173,7 +168,6 @@ public function form(array $form, FormStateInterface $form_state) { ); $form_state->set('user', $account); - $form['#validate'][] = 'user_validate_current_pass'; } } elseif (!$config->get('verify_mail') || $admin) { @@ -348,6 +342,12 @@ public function buildEntity(array $form, FormStateInterface $form_state) { $account->$field_name = NULL; } } + + // Set existing password if set in the form state. + if ($current_pass = $form_state->getValue('current_pass')) { + $account->setExistingPassword($current_pass); + } + return $account; } @@ -358,11 +358,16 @@ public function validate(array $form, FormStateInterface $form_state) { /** @var \Drupal\user\UserInterface $account */ $account = parent::validate($form, $form_state); + // Skip the protected user field constraint if the user came from the + // password recovery page. + $account->_skipProtectedUserFieldConstraint = $form_state->get('user_pass_reset'); + // Customly trigger validation of manually added fields and add in // violations. This is necessary as entity form displays only invoke entity // validation for fields contained in the display. $field_names = array( 'name', + 'pass', 'mail', 'timezone', 'langcode', diff --git a/core/modules/user/src/Entity/User.php b/core/modules/user/src/Entity/User.php index 1cecfdc64a68..54a268bdbb2a 100644 --- a/core/modules/user/src/Entity/User.php +++ b/core/modules/user/src/Entity/User.php @@ -8,7 +8,6 @@ namespace Drupal\user\Entity; use Drupal\Core\Entity\ContentEntityBase; -use Drupal\Core\Entity\EntityMalformedException; use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Field\BaseFieldDefinition; @@ -90,24 +89,6 @@ public function preSave(EntityStorageInterface $storage) { } } - // Update the user password if it has changed. - if ($this->isNew() || ($this->pass->value && $this->pass->value != $this->original->pass->value)) { - // Allow alternate password hashing schemes. - $this->pass->value = \Drupal::service('password')->hash(trim($this->pass->value)); - // Abort if the hashing failed and returned FALSE. - if (!$this->pass->value) { - throw new EntityMalformedException('The entity does not have a password.'); - } - } - - if (!$this->isNew()) { - // If the password is empty, that means it was not changed, so use the - // original password. - if (empty($this->pass->value)) { - $this->pass->value = $this->original->pass->value; - } - } - // Store account cancellation information. foreach (array('user_cancel_method', 'user_cancel_notify') as $key) { if (isset($this->{$key})) { @@ -429,6 +410,20 @@ public function getChangedTime() { return $this->get('changed')->value; } + /** + * {@inheritdoc} + */ + public function setExistingPassword($password) { + $this->get('pass')->existing = $password; + } + + /** + * {@inheritdoc} + */ + public function checkExistingPassword(UserInterface $account_unchanged) { + return !empty($this->get('pass')->existing) && \Drupal::service('password')->check(trim($this->get('pass')->existing), $account_unchanged); + } + /** * {@inheritdoc} */ @@ -486,14 +481,16 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) { $fields['pass'] = BaseFieldDefinition::create('password') ->setLabel(t('Password')) - ->setDescription(t('The password of this user (hashed).')); + ->setDescription(t('The password of this user (hashed).')) + ->addConstraint('ProtectedUserField'); $fields['mail'] = BaseFieldDefinition::create('email') ->setLabel(t('Email')) ->setDescription(t('The email of this user.')) ->setDefaultValue('') ->addConstraint('UserMailUnique') - ->addConstraint('UserMailRequired'); + ->addConstraint('UserMailRequired') + ->addConstraint('ProtectedUserField'); $fields['timezone'] = BaseFieldDefinition::create('string') ->setLabel(t('Timezone')) diff --git a/core/modules/user/src/Plugin/Validation/Constraint/ProtectedUserFieldConstraint.php b/core/modules/user/src/Plugin/Validation/Constraint/ProtectedUserFieldConstraint.php new file mode 100644 index 000000000000..fd3ca3d94cbd --- /dev/null +++ b/core/modules/user/src/Plugin/Validation/Constraint/ProtectedUserFieldConstraint.php @@ -0,0 +1,29 @@ +<?php + +/** + * @file + * Contains \Drupal\user\Plugin\Validation\Constraint\ProtectedUserFieldConstraint. + */ + +namespace Drupal\user\Plugin\Validation\Constraint; + +use Symfony\Component\Validator\Constraint; + +/** + * Checks if the plain text password is provided for editing a protected field. + * + * @Plugin( + * id = "ProtectedUserField", + * label = @Translation("Password required for protected field change", context = "Validation") + * ) + */ +class ProtectedUserFieldConstraint extends Constraint { + + /** + * Violation message. + * + * @var string + */ + public $message = "Your current password is missing or incorrect; it's required to change the %name."; + +} diff --git a/core/modules/user/src/Plugin/Validation/Constraint/ProtectedUserFieldConstraintValidator.php b/core/modules/user/src/Plugin/Validation/Constraint/ProtectedUserFieldConstraintValidator.php new file mode 100644 index 000000000000..d99fc5d3f842 --- /dev/null +++ b/core/modules/user/src/Plugin/Validation/Constraint/ProtectedUserFieldConstraintValidator.php @@ -0,0 +1,98 @@ +<?php +/** + * @file + * Contains \Drupal\user\Plugin\Validation\Constraint\ProtectedUserFieldConstraintValidator. + */ + +namespace Drupal\user\Plugin\Validation\Constraint; + +use Drupal\Core\DependencyInjection\ContainerInjectionInterface; +use Drupal\Core\Session\AccountProxyInterface; +use Drupal\user\UserStorageInterface; +use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\Validator\Constraint; +use Symfony\Component\Validator\ConstraintValidator; + +/** + * Validates the ProtectedUserFieldConstraint constraint. + */ +class ProtectedUserFieldConstraintValidator extends ConstraintValidator implements ContainerInjectionInterface { + + /** + * User storage handler. + * + * @var \Drupal\user\UserStorageInterface + */ + protected $userStorage; + + /** + * The current user. + * + * @var \Drupal\Core\Session\AccountProxyInterface + */ + protected $currentUser; + + /** + * Constructs the object. + * + * @param \Drupal\user\UserStorageInterface $user_storage + * The user storage handler. + * @param \Drupal\Core\Session\AccountProxyInterface $current_user + * The current user. + */ + public function __construct(UserStorageInterface $user_storage, AccountProxyInterface $current_user) { + $this->userStorage = $user_storage; + $this->currentUser = $current_user; + } + + /** + * {@inheritdoc} + */ + public static function create(ContainerInterface $container) { + return new static( + $container->get('entity.manager')->getStorage('user'), + $container->get('current_user') + ); + } + + /** + * {@inheritdoc} + */ + public function validate($items, Constraint $constraint) { + if (!isset($items)) { + return; + } + /* @var \Drupal\Core\Field\FieldItemListInterface $items */ + $field = $items->getFieldDefinition(); + + /* @var \Drupal\user\UserInterface $account */ + $account = $items->getEntity(); + if (!isset($account) || !empty($account->_skipProtectedUserFieldConstraint)) { + // Looks like we are validating a field not being part of a user, or the + // constraint should be skipped, so do nothing. + return; + } + + // Only validate for existing entities and if this is the current user. + if (!$account->isNew() && $account->id() == $this->currentUser->id()) { + + /* @var \Drupal\user\UserInterface $account_unchanged */ + $account_unchanged = $this->userStorage + ->loadUnchanged($account->id()); + + $changed = FALSE; + + // Special case for the password, it being empty means that the existing + // password should not be changed, ignore empty password fields. + $value = $items->value; + if ($field->getName() != 'pass' || !empty($value)) { + // Compare the values of the field this is being validated on. + $changed = $items->getValue() != $account_unchanged->get($field->getName())->getValue(); + } + if ($changed && (!$account->checkExistingPassword($account_unchanged))) { + $this->context->addViolation($constraint->message, array('%name' => $field->getLabel())); + } + } + } + +} diff --git a/core/modules/user/src/Tests/UserEditTest.php b/core/modules/user/src/Tests/UserEditTest.php index 308fad96d767..46502f408443 100644 --- a/core/modules/user/src/Tests/UserEditTest.php +++ b/core/modules/user/src/Tests/UserEditTest.php @@ -47,7 +47,7 @@ function testUserEdit() { $edit = array(); $edit['mail'] = $this->randomMachineName() . '@new.example.com'; $this->drupalPostForm("user/" . $user1->id() . "/edit", $edit, t('Save')); - $this->assertRaw(t("Your current password is missing or incorrect; it's required to change the %name.", array('%name' => t('Email address')))); + $this->assertRaw(t("Your current password is missing or incorrect; it's required to change the %name.", array('%name' => t('Email')))); $edit['current_pass'] = $user1->pass_raw; $this->drupalPostForm("user/" . $user1->id() . "/edit", $edit, t('Save')); diff --git a/core/modules/user/src/Tests/UserValidateCurrentPassCustomFormTest.php b/core/modules/user/src/Tests/UserValidateCurrentPassCustomFormTest.php deleted file mode 100644 index 025a5ed108c8..000000000000 --- a/core/modules/user/src/Tests/UserValidateCurrentPassCustomFormTest.php +++ /dev/null @@ -1,57 +0,0 @@ -<?php - -/** - * @file - * Definition of Drupal\user\Tests\UserValidateCurrentPassCustomFormTest. - */ - -namespace Drupal\user\Tests; - -use Drupal\simpletest\WebTestBase; - -/** - * Tests user_validate_current_pass on a custom form. - * - * @group user - */ -class UserValidateCurrentPassCustomFormTest extends WebTestBase { - - /** - * Modules to enable. - * - * @var array - */ - public static $modules = array('user_form_test'); - - /** - * User with permission to view content. - */ - protected $accessUser; - - /** - * User permission to administer users. - */ - protected $adminUser; - - protected function setUp() { - parent::setUp(); - - // Create two users - $this->accessUser = $this->drupalCreateUser(array()); - $this->adminUser = $this->drupalCreateUser(array('administer users')); - } - - /** - * Tests that user_validate_current_pass can be reused on a custom form. - */ - function testUserValidateCurrentPassCustomForm() { - $this->drupalLogin($this->adminUser); - - // Submit the custom form with the admin user using the access user's password. - $edit = array(); - $edit['user_form_test_field'] = $this->accessUser->getUsername(); - $edit['current_pass'] = $this->accessUser->pass_raw; - $this->drupalPostForm('user_form_test_current_password/' . $this->accessUser->id(), $edit, t('Test')); - $this->assertText(t('The password has been validated and the form submitted successfully.')); - } -} diff --git a/core/modules/user/src/UserInterface.php b/core/modules/user/src/UserInterface.php index 079777635bb4..17f7b16e7866 100644 --- a/core/modules/user/src/UserInterface.php +++ b/core/modules/user/src/UserInterface.php @@ -164,4 +164,29 @@ public function block(); */ public function getInitialEmail(); + /** + * Sets the existing plain text password. + * + * Required for validation when changing the password, name or email fields. + * + * @param string $password + * The existing plain text password of the user. + * + * @return $this + */ + public function setExistingPassword($password); + + /** + * Checks the existing password if set. + * + * @param \Drupal\user\UserInterface $account_unchanged + * The unchanged user entity to compare against. + * + * @return bool + * TRUE if the correct existing password was provided. + * + * @see UserInterface::setExistingPassword(). + */ + public function checkExistingPassword(UserInterface $account_unchanged); + } diff --git a/core/modules/user/tests/modules/user_form_test/src/Form/TestCurrentPassword.php b/core/modules/user/tests/modules/user_form_test/src/Form/TestCurrentPassword.php deleted file mode 100644 index 3dd75e16f3b0..000000000000 --- a/core/modules/user/tests/modules/user_form_test/src/Form/TestCurrentPassword.php +++ /dev/null @@ -1,68 +0,0 @@ -<?php - -/** - * @file - * Contains \Drupal\user_form_test\Form\TestCurrentPassword. - */ - -namespace Drupal\user_form_test\Form; - -use Drupal\Core\Form\FormBase; -use Drupal\Core\Form\FormStateInterface; -use Drupal\user\UserInterface; - -/** - * Provides a current password validation form. - */ -class TestCurrentPassword extends FormBase { - - /** - * {@inheritdoc} - */ - public function getFormId() { - return 'user_form_test_current_password'; - } - - /** - * {@inheritdoc} - * - * @param \Drupal\user\Entity\UserInterface $user - * The user account. - */ - public function buildForm(array $form, FormStateInterface $form_state, UserInterface $user = NULL) { - $form_state->set('user', $user); - $form['user_form_test_field'] = array( - '#type' => 'textfield', - '#title' => $this->t('Test field'), - '#description' => $this->t('A field that would require a correct password to change.'), - '#required' => TRUE, - ); - - $form['current_pass'] = array( - '#type' => 'password', - '#title' => $this->t('Current password'), - '#size' => 25, - '#description' => $this->t('Enter your current password'), - ); - - $form['current_pass_required_values'] = array( - '#type' => 'value', - '#value' => array('user_form_test_field' => $this->t('Test field')), - ); - - $form['#validate'][] = 'user_validate_current_pass'; - $form['submit'] = array( - '#type' => 'submit', - '#value' => $this->t('Test'), - ); - return $form; - } - - /** - * {@inheritdoc} - */ - public function submitForm(array &$form, FormStateInterface $form_state) { - drupal_set_message($this->t('The password has been validated and the form submitted successfully.')); - } - -} diff --git a/core/modules/user/tests/modules/user_form_test/user_form_test.routing.yml b/core/modules/user/tests/modules/user_form_test/user_form_test.routing.yml index 78f813cd3620..9b7f58a214f0 100644 --- a/core/modules/user/tests/modules/user_form_test/user_form_test.routing.yml +++ b/core/modules/user/tests/modules/user_form_test/user_form_test.routing.yml @@ -1,10 +1,3 @@ -user_form_test.current_password: - path: '/user_form_test_current_password/{user}' - defaults: - _form: '\Drupal\user_form_test\Form\TestCurrentPassword' - requirements: - _permission: 'administer users' - user_form_test.cancel: path: '/user_form_test_cancel/{user}' defaults: diff --git a/core/modules/user/tests/src/Unit/Plugin/Validation/Constraint/ProtectedUserFieldConstraintValidatorTest.php b/core/modules/user/tests/src/Unit/Plugin/Validation/Constraint/ProtectedUserFieldConstraintValidatorTest.php new file mode 100644 index 000000000000..7e443d15b6e4 --- /dev/null +++ b/core/modules/user/tests/src/Unit/Plugin/Validation/Constraint/ProtectedUserFieldConstraintValidatorTest.php @@ -0,0 +1,313 @@ +<?php + +/** + * @file + * Contains \Drupal\Tests\user\Unit\Plugin\Validation\Constraint\ProtectedUserFieldConstraintValidatorTest. + */ + +namespace Drupal\Tests\user\Unit\Plugin\Validation\Constraint; + +use Drupal\user\Plugin\Validation\Constraint\ProtectedUserFieldConstraint; +use Drupal\user\Plugin\Validation\Constraint\ProtectedUserFieldConstraintValidator; +use Symfony\Component\Validator\Tests\Constraints\AbstractConstraintValidatorTest; +use Symfony\Component\Validator\Validation; + +/** + * @coversDefaultClass \Drupal\user\Plugin\Validation\Constraint\ProtectedUserFieldConstraintValidator + * @group user + */ +class ProtectedUserFieldConstraintValidatorTest extends AbstractConstraintValidatorTest { + + /** + * {@inheritdoc} + */ + protected function getApiVersion() { + return Validation::API_VERSION_2_4; + } + + /** + * {@inheritdoc} + */ + protected function createValidator() { + // Setup mocks that don't need to change. + $unchanged_field = $this->getMock('Drupal\Core\Field\FieldItemListInterface'); + $unchanged_field->expects($this->any()) + ->method('getValue') + ->willReturn('unchanged-value'); + + $unchanged_account = $this->getMock('Drupal\user\UserInterface'); + $unchanged_account->expects($this->any()) + ->method('get') + ->willReturn($unchanged_field); + $user_storage = $this->getMock('Drupal\user\UserStorageInterface'); + $user_storage->expects($this->any()) + ->method('loadUnchanged') + ->willReturn($unchanged_account); + $current_user = $this->getMock('Drupal\Core\Session\AccountProxyInterface'); + $current_user->expects($this->any()) + ->method('id') + ->willReturn('current-user'); + return new ProtectedUserFieldConstraintValidator($user_storage, $current_user); + } + + /** + * @covers ::validate + * + * @dataProvider providerTestValidate + */ + public function testValidate($items, $expected_violation, $name = FALSE) { + $constraint = new ProtectedUserFieldConstraint(); + $this->validator->validate($items, $constraint); + if ($expected_violation) { + $this->buildViolation($constraint->message) + ->setParameter('%name', $name) + ->assertRaised(); + } + else { + $this->assertNoViolation(); + } + } + + /** + * Data provider for ::testValidate(). + */ + public function providerTestValidate() { + $cases = []; + + // Case 1: Validation context should not be touched if no items are passed. + $cases[] = [NULL, FALSE]; + + // Case 2: Empty user should be ignored. + $field_definition = $this->getMock('Drupal\Core\Field\FieldDefinitionInterface'); + $items = $this->getMock('Drupal\Core\Field\FieldItemListInterface'); + $items->expects($this->once()) + ->method('getFieldDefinition') + ->willReturn($field_definition); + $items->expects($this->once()) + ->method('getEntity') + ->willReturn(NULL); + $cases[] = [$items, FALSE]; + + // Case 3: Account flagged to skip protected user should be ignored. + $field_definition = $this->getMock('Drupal\Core\Field\FieldDefinitionInterface'); + $account = $this->getMock('Drupal\user\UserInterface'); + $account->_skipProtectedUserFieldConstraint = TRUE; + $items = $this->getMock('Drupal\Core\Field\FieldItemListInterface'); + $items->expects($this->once()) + ->method('getFieldDefinition') + ->willReturn($field_definition); + $items->expects($this->once()) + ->method('getEntity') + ->willReturn($account); + $cases[] = [$items, FALSE]; + + // Case 4: New user should be ignored. + $field_definition = $this->getMock('Drupal\Core\Field\FieldDefinitionInterface'); + $account = $this->getMock('Drupal\user\UserInterface'); + $account->expects($this->once()) + ->method('isNew') + ->willReturn(TRUE); + $items = $this->getMock('Drupal\Core\Field\FieldItemListInterface'); + $items->expects($this->once()) + ->method('getFieldDefinition') + ->willReturn($field_definition); + $items->expects($this->once()) + ->method('getEntity') + ->willReturn($account); + $cases[] = [$items, FALSE]; + + // Case 5: Mismatching user IDs should also be ignored. + $account = $this->getMock('Drupal\user\UserInterface'); + $account->expects($this->once()) + ->method('isNew') + ->willReturn(FALSE); + $account->expects($this->once()) + ->method('id') + ->willReturn('not-current-user'); + $items = $this->getMock('Drupal\Core\Field\FieldItemListInterface'); + $items->expects($this->once()) + ->method('getFieldDefinition') + ->willReturn($field_definition); + $items->expects($this->once()) + ->method('getEntity') + ->willReturn($account); + $cases[] = [$items, FALSE]; + + // Case 6: Non-password fields that have not changed should be ignored. + $field_definition = $this->getMock('Drupal\Core\Field\FieldDefinitionInterface'); + $field_definition->expects($this->exactly(2)) + ->method('getName') + ->willReturn('field_not_password'); + $account = $this->getMock('Drupal\user\UserInterface'); + $account->expects($this->once()) + ->method('isNew') + ->willReturn(FALSE); + $account->expects($this->exactly(2)) + ->method('id') + ->willReturn('current-user'); + $account->expects($this->never()) + ->method('checkExistingPassword'); + $items = $this->getMock('Drupal\Core\Field\FieldItemListInterface'); + $items->expects($this->once()) + ->method('getFieldDefinition') + ->willReturn($field_definition); + $items->expects($this->once()) + ->method('getEntity') + ->willReturn($account); + $items->expects($this->once()) + ->method('getValue') + ->willReturn('unchanged-value'); + $cases[] = [$items, FALSE]; + + // Case 7: Password field with no value set should be ignored. + $field_definition = $this->getMock('Drupal\Core\Field\FieldDefinitionInterface'); + $field_definition->expects($this->once()) + ->method('getName') + ->willReturn('pass'); + $account = $this->getMock('Drupal\user\UserInterface'); + $account->expects($this->once()) + ->method('isNew') + ->willReturn(FALSE); + $account->expects($this->exactly(2)) + ->method('id') + ->willReturn('current-user'); + $account->expects($this->never()) + ->method('checkExistingPassword'); + $items = $this->getMock('Drupal\Core\Field\FieldItemListInterface'); + $items->expects($this->once()) + ->method('getFieldDefinition') + ->willReturn($field_definition); + $items->expects($this->once()) + ->method('getEntity') + ->willReturn($account); + $cases[] = [$items, FALSE]; + + // Case 8: Non-password field changed, but user has passed provided current + // password. + $field_definition = $this->getMock('Drupal\Core\Field\FieldDefinitionInterface'); + $field_definition->expects($this->exactly(2)) + ->method('getName') + ->willReturn('field_not_password'); + $account = $this->getMock('Drupal\user\UserInterface'); + $account->expects($this->once()) + ->method('isNew') + ->willReturn(FALSE); + $account->expects($this->exactly(2)) + ->method('id') + ->willReturn('current-user'); + $account->expects($this->once()) + ->method('checkExistingPassword') + ->willReturn(TRUE); + $items = $this->getMock('Drupal\Core\Field\FieldItemListInterface'); + $items->expects($this->once()) + ->method('getFieldDefinition') + ->willReturn($field_definition); + $items->expects($this->once()) + ->method('getEntity') + ->willReturn($account); + $items->expects($this->once()) + ->method('getValue') + ->willReturn('changed-value'); + $cases[] = [$items, FALSE]; + + // Case 9: Password field changed, current password confirmed. + $field_definition = $this->getMock('Drupal\Core\Field\FieldDefinitionInterface'); + $field_definition->expects($this->exactly(2)) + ->method('getName') + ->willReturn('pass'); + $account = $this->getMock('Drupal\user\UserInterface'); + $account->expects($this->once()) + ->method('isNew') + ->willReturn(FALSE); + $account->expects($this->exactly(2)) + ->method('id') + ->willReturn('current-user'); + $account->expects($this->once()) + ->method('checkExistingPassword') + ->willReturn(TRUE); + $items = $this->getMock('Drupal\Core\Field\FieldItemListInterface'); + $items->expects($this->once()) + ->method('getFieldDefinition') + ->willReturn($field_definition); + $items->expects($this->once()) + ->method('getEntity') + ->willReturn($account); + $items->expects($this->any()) + ->method('getValue') + ->willReturn('changed-value'); + $items->expects($this->once()) + ->method('__get') + ->with('value') + ->willReturn('changed-value'); + $cases[] = [$items, FALSE]; + + // The below calls should result in a violation. + + // Case 10: Password field changed, current password not confirmed. + $field_definition = $this->getMock('Drupal\Core\Field\FieldDefinitionInterface'); + $field_definition->expects($this->exactly(2)) + ->method('getName') + ->willReturn('pass'); + $field_definition->expects($this->any()) + ->method('getLabel') + ->willReturn('Password'); + $account = $this->getMock('Drupal\user\UserInterface'); + $account->expects($this->once()) + ->method('isNew') + ->willReturn(FALSE); + $account->expects($this->exactly(2)) + ->method('id') + ->willReturn('current-user'); + $account->expects($this->once()) + ->method('checkExistingPassword') + ->willReturn(FALSE); + $items = $this->getMock('Drupal\Core\Field\FieldItemListInterface'); + $items->expects($this->once()) + ->method('getFieldDefinition') + ->willReturn($field_definition); + $items->expects($this->once()) + ->method('getEntity') + ->willReturn($account); + $items->expects($this->once()) + ->method('getValue') + ->willReturn('changed-value'); + $items->expects($this->once()) + ->method('__get') + ->with('value') + ->willReturn('changed-value'); + $cases[] = [$items, TRUE, 'Password']; + + // Case 11: Non-password field changed, current password not confirmed. + $field_definition = $this->getMock('Drupal\Core\Field\FieldDefinitionInterface'); + $field_definition->expects($this->exactly(2)) + ->method('getName') + ->willReturn('field_not_password'); + $field_definition->expects($this->any()) + ->method('getLabel') + ->willReturn('Protected field'); + $account = $this->getMock('Drupal\user\UserInterface'); + $account->expects($this->once()) + ->method('isNew') + ->willReturn(FALSE); + $account->expects($this->exactly(2)) + ->method('id') + ->willReturn('current-user'); + $account->expects($this->once()) + ->method('checkExistingPassword') + ->willReturn(FALSE); + $items = $this->getMock('Drupal\Core\Field\FieldItemListInterface'); + $items->expects($this->once()) + ->method('getFieldDefinition') + ->willReturn($field_definition); + $items->expects($this->once()) + ->method('getEntity') + ->willReturn($account); + $items->expects($this->once()) + ->method('getValue') + ->willReturn('changed-value'); + $cases[] = [$items, TRUE, 'Protected field']; + + return $cases; + } + +} diff --git a/core/modules/user/user.module b/core/modules/user/user.module index dd37814fdddb..2497ec07c47c 100644 --- a/core/modules/user/user.module +++ b/core/modules/user/user.module @@ -404,30 +404,6 @@ function user_user_view(array &$build, UserInterface $account, EntityViewDisplay } } -/** - * Form validation handler for the current password on the user account form. - * - * @see AccountForm::form() - */ -function user_validate_current_pass(&$form, FormStateInterface $form_state) { - $account = $form_state->get('user'); - foreach ($form_state->getValue('current_pass_required_values') as $key => $name) { - // This validation only works for required textfields (like mail) or - // form values like password_confirm that have their own validation - // that prevent them from being empty if they are changed. - $current_value = $account->hasField($key) ? $account->get($key)->value : $account->$key; - if ((strlen(trim($form_state->getValue($key))) > 0) && ($form_state->getValue($key) != $current_value)) { - $current_pass_failed = $form_state->isValueEmpty('current_pass') || !\Drupal::service('password')->check($form_state->getValue('current_pass'), $account); - if ($current_pass_failed) { - $form_state->setErrorByName('current_pass', t("Your current password is missing or incorrect; it's required to change the %name.", array('%name' => $name))); - $form_state->setErrorByName($key); - } - // We only need to check the password once. - break; - } - } -} - /** * Implements hook_preprocess_HOOK() for block templates. */ -- GitLab