Commit 3c0892d0 authored by alexpott's avatar alexpott

Issue #2418119 by Berdir, jhedstrom, larowlan, martin107, nlisgo, klausi,...

Issue #2418119 by Berdir, jhedstrom, larowlan, martin107, nlisgo, klausi, fago, Gábor Hojtsy: REST user updates bypass tightened user account change validation
parent ed89a08f
......@@ -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);
......
......@@ -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;
}
}
}
}
......@@ -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.
......
......@@ -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'];
}
}
}
......
......@@ -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);
}
}
......@@ -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());
}
}
......
......@@ -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',
......
......@@ -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'))
......
<?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.";
}
<?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()));
}
}
}
}
......@@ -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'));
......
<?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.'));
}
}
......@@ -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);
}
<?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.'));
}
}
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:
......
......@@ -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.
*/
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment