Skip to content
GitLab
Commit 17c25003 authored by alexpott's avatar alexpott
Browse files

Issue #2405943 by fago, mikey_p, dashaforbes: User entity validation misses form validation logic

parent a7c37e03
Hide whitespace changes
Inline Side-by-side
core/lib/Drupal/Core/Field/Plugin/Field/FieldType/LanguageItem.php
View file @ 17c25003
......@@ -2,7 +2,7 @@
/**
* @file
* Contains \Drupal\Core\Entity\Plugin\Field\FieldType\LanguageItem.
* Contains \Drupal\Core\Field\Plugin\Field\FieldType\LanguageItem.
*/
namespace Drupal\Core\Field\Plugin\Field\FieldType;
......@@ -25,10 +25,16 @@
* no_ui = TRUE,
* constraints = {
* "ComplexData" = {
* "value" = {"Length" = {"max" = 12}}
* "value" = {
* "Length" = {"max" = 12},
* "AllowedValues" = {"callback" = "\Drupal\Core\Field\Plugin\Field\FieldType\LanguageItem::getAllowedLanguageCodes" }
* }
* }
* }
* )
*
* @todo Define the AllowedValues constraint via an options provider once
* https://www.drupal.org/node/2329937 is completed.
*/
class LanguageItem extends FieldItemBase {
......@@ -50,6 +56,16 @@ public static function propertyDefinitions(FieldStorageDefinitionInterface $fiel
return $properties;
}
/**
* Defines allowed language codes for the field's AllowedValues constraint.
*
* @return string[]
* The allowed values.
*/
public static function getAllowedLanguageCodes() {
return array_keys(\Drupal::languageManager()->getLanguages(LanguageInterface::STATE_ALL));
}
/**
* {@inheritdoc}
*/
......
core/lib/Drupal/Core/Validation/ConstraintManager.php
View file @ 17c25003
......@@ -68,7 +68,7 @@ public function create($name, $options) {
// Plugins need an array as configuration, so make sure we have one.
// The constraint classes support passing the options as part of the
// 'value' key also.
$options = array('value' => $options);
$options = isset($options) ? array('value' => $options) : array();
}
return $this->createInstance($name, $options);
}
......
core/modules/system/src/Tests/Entity/EntityValidationTest.php
View file @ 17c25003
......@@ -126,8 +126,10 @@ protected function checkValidation($entity_type) {
$langcode_key = $this->entityManager->getDefinition($entity_type)->getKey('langcode');
$test_entity->{$langcode_key}->value = $this->randomString(13);
$violations = $test_entity->validate();
$this->assertEqual($violations->count(), 1, 'Validation failed.');
// This should fail on AllowedValues and Length constraints.
$this->assertEqual($violations->count(), 2, 'Validation failed.');
$this->assertEqual($violations[0]->getMessage(), t('This value is too long. It should have %limit characters or less.', array('%limit' => '12')));
$this->assertEqual($violations[1]->getMessage(), t('The value you selected is not a valid choice.'));
$test_entity = clone $entity;
$test_entity->type->value = NULL;
......
core/modules/user/src/AccountForm.php
View file @ 17c25003
......@@ -88,6 +88,7 @@ public function form(array $form, FormStateInterface $form_state) {
// The mail field is NOT required if account originally had no mail set
// and the user performing the edit has 'administer users' permission.
// This allows users without email address to be edited and deleted.
// Also see \Drupal\user\Plugin\Validation\Constraint\UserMailRequired.
$form['account']['mail'] = array(
'#type' => 'email',
'#title' => $this->t('Email address'),
......@@ -359,7 +360,23 @@ public function buildEntity(array $form, FormStateInterface $form_state) {
if (is_string(key($form_state->getValue('roles')))) {
$form_state->setValue('roles', array_keys(array_filter($form_state->getValue('roles'))));
}
return parent::buildEntity($form, $form_state);
/** @var \Drupal\user\UserInterface $account */
$account = parent::buildEntity($form, $form_state);
// Take care of mapping signature form element values as their structure
// does not directly match the field structure.
$signature = $form_state->getValue('signature');
$account->setSignature($signature['value']);
$account->setSignatureFormat($signature['format']);
// Translate the empty value '' of language selects to an unset field.
foreach (array('preferred_langcode', 'preferred_admin_langcode') as $field_name) {
if ($form_state->getValue($field_name) === '') {
$account->$field_name = NULL;
}
}
return $account;
}
/**
......@@ -368,63 +385,25 @@ public function buildEntity(array $form, FormStateInterface $form_state) {
public function validate(array $form, FormStateInterface $form_state) {
parent::validate($form, $form_state);
$account = $this->entity;
// Validate new or changing username.
if ($form_state->hasValue('name')) {
if ($error = user_validate_name($form_state->getValue('name'))) {
$form_state->setErrorByName('name', $error);
}
// Cast the user ID as an integer. It might have been set to NULL, which
// could lead to unexpected results.
else {
$name_taken = (bool) $this->entityQuery->get('user')
->condition('uid', (int) $account->id(), '<>')
->condition('name', $form_state->getValue('name'))
->range(0, 1)
->count()
->execute();
if ($name_taken) {
$form_state->setErrorByName('name', $this->t('The username %name is already taken.', array('%name' => $form_state->getValue('name'))));
}
}
}
$mail = $form_state->getValue('mail');
if (!empty($mail)) {
$mail_taken = (bool) $this->entityQuery->get('user')
->condition('uid', (int) $account->id(), '<>')
->condition('mail', $mail)
->range(0, 1)
->count()
->execute();
if ($mail_taken) {
// Format error message dependent on whether the user is logged in or not.
if (\Drupal::currentUser()->isAuthenticated()) {
$form_state->setErrorByName('mail', $this->t('The email address %email is already taken.', array('%email' => $mail)));
}
else {
$form_state->setErrorByName('mail', $this->t('The email address %email is already registered. <a href="@password">Have you forgotten your password?</a>', array('%email' => $mail, '@password' => $this->url('user.pass'))));
}
}
}
// Make sure the signature isn't longer than the size of the database field.
// Signatures are disabled by default, so make sure it exists first.
if ($signature = $form_state->getValue('signature')) {
// Move text format for user signature into 'signature_format'.
$form_state->setValue('signature_format', $signature['format']);
// Move text value for user signature into 'signature'.
$form_state->setValue('signature', $signature['value']);
// @todo Make the user signature field use a widget to benefit from
// automatic typed data validation in https://drupal.org/node/2227381.
$field_definitions = $this->entityManager->getFieldDefinitions('user', $this->getEntity()->bundle());
$max_length = $field_definitions['signature']->getSetting('max_length');
if (Unicode::strlen($form_state->getValue('signature')) > $max_length) {
$form_state->setErrorByName('signature', $this->t('The signature is too long: it must be %max characters or less.', array('%max' => $max_length)));
/** @var \Drupal\user\UserInterface $account */
$account = $this->buildEntity($form, $form_state);
// 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',
'mail',
'signature',
'signature_format',
'timezone',
'langcode',
'preferred_langcode',
'preferred_admin_langcode'
);
foreach ($field_names as $field_name) {
$violations = $account->$field_name->validate();
foreach ($violations as $violation) {
$form_state->setErrorByName($field_name, $violation->getMessage());
}
}
}
......
core/modules/user/src/Entity/User.php
View file @ 17c25003
......@@ -12,6 +12,7 @@
use Drupal\Core\Entity\EntityStorageInterface;
use Drupal\Core\Entity\EntityTypeInterface;
use Drupal\Core\Field\BaseFieldDefinition;
use Drupal\Core\Language\LanguageInterface;
use Drupal\user\UserInterface;
/**
......@@ -288,6 +289,14 @@ public function getSignature() {
return $this->get('signature')->value;
}
/**
* {@inheritdoc}
*/
public function setSignature($signature) {
$this->get('signature')->value = $signature;
return $this;
}
/**
* {@inheritdoc}
*/
......@@ -295,6 +304,14 @@ public function getSignatureFormat() {
return $this->get('signature_format')->value;
}
/**
* {@inheritdoc}
*/
public function setSignatureFormat($signature_format) {
$this->get('signature_format')->value = $signature_format;
return $this;
}
/**
* {@inheritdoc}
*/
......@@ -462,12 +479,25 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
$fields['preferred_langcode'] = BaseFieldDefinition::create('language')
->setLabel(t('Preferred language code'))
->setDescription(t("The user's preferred language code for receiving emails and viewing the site."));
->setDescription(t("The user's preferred language code for receiving emails and viewing the site."))
// @todo: Define this via an options provider once
// https://www.drupal.org/node/2329937 is completed.
->addPropertyConstraints('value', array(
'AllowedValues' => array('callback' => __CLASS__ . '::getAllowedConfigurableLanguageCodes'),
));
$fields['preferred_admin_langcode'] = BaseFieldDefinition::create('language')
->setLabel(t('Preferred admin language code'))
->setDescription(t("The user's preferred language code for viewing administration pages."))
->setDefaultValue('');
// @todo: A default value of NULL is ignored, so we have to specify
// an empty field item structure instead. Fix this in
// https://www.drupal.org/node/2318605.
->setDefaultValue(array(0 => array ('value' => NULL)))
// @todo: Define this via an options provider once
// https://www.drupal.org/node/2329937 is completed.
->addPropertyConstraints('value', array(
'AllowedValues' => array('callback' => __CLASS__ . '::getAllowedConfigurableLanguageCodes'),
));
// The name should not vary per language. The username is the visual
// identifier for a user and needs to be consistent in all languages.
......@@ -490,7 +520,8 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
->setLabel(t('Email'))
->setDescription(t('The email of this user.'))
->setDefaultValue('')
->setConstraints(array('UserMailUnique' => array()));
->addConstraint('UserMailUnique')
->addConstraint('UserMailRequired');
// @todo Convert to a text field in https://drupal.org/node/1548204.
$fields['signature'] = BaseFieldDefinition::create('string')
......@@ -499,12 +530,22 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
->setTranslatable(TRUE);
$fields['signature_format'] = BaseFieldDefinition::create('string')
->setLabel(t('Signature format'))
->setDescription(t('The signature format of this user.'));
->setDescription(t('The signature format of this user.'))
// @todo: Define this via an options provider once
// https://www.drupal.org/node/2329937 is completed.
->addPropertyConstraints('value', array(
'AllowedValues' => array('callback' => __CLASS__ . '::getAllowedSignatureFormats'),
));
$fields['timezone'] = BaseFieldDefinition::create('string')
->setLabel(t('Timezone'))
->setDescription(t('The timezone of this user.'))
->setSetting('max_length', 32);
->setSetting('max_length', 32)
// @todo: Define this via an options provider once
// https://www.drupal.org/node/2329937 is completed.
->addPropertyConstraints('value', array(
'AllowedValues' => array('callback' => __CLASS__ . '::getAllowedTimezones'),
));
$fields['status'] = BaseFieldDefinition::create('boolean')
->setLabel(t('User status'))
......@@ -553,4 +594,38 @@ protected function getRoleStorage() {
return \Drupal::entityManager()->getStorage('user_role');
}
/**
* Defines allowed signature formats for the field's AllowedValues constraint.
*
* @return string[]
* The allowed values.
*/
public static function getAllowedSignatureFormats() {
if (\Drupal::moduleHandler()->moduleExists('filter')) {
return array_keys(filter_formats());
}
// If filter.module is disabled, no value may be assigned.
return array();
}
/**
* Defines allowed timezones for the field's AllowedValues constraint.
*
* @return string[]
* The allowed values.
*/
public static function getAllowedTimezones() {
return array_keys(system_time_zones());
}
/**
* Defines allowed configurable language codes for AllowedValues constraints.
*
* @return string[]
* The allowed values.
*/
public static function getAllowedConfigurableLanguageCodes() {
return array_keys(\Drupal::languageManager()->getLanguages(LanguageInterface::STATE_CONFIGURABLE));
}
}
core/modules/user/src/Plugin/Validation/Constraint/UserMailRequired.php 0 → 100644
View file @ 17c25003
<?php
/**
* @file
* Contains \Drupal\user\Plugin\Validation\Constraint\UserMailRequired.
*/
namespace Drupal\user\Plugin\Validation\Constraint;
use Drupal\Component\Utility\String;
use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\ConstraintValidatorInterface;
use Symfony\Component\Validator\ExecutionContextInterface;
/**
* Checks if the user's email address is provided if required.
*
* The user mail field is NOT required if account originally had no mail set
* and the user performing the edit has 'administer users' permission.
* This allows users without email address to be edited and deleted.
*
* @Plugin(
* id = "UserMailRequired",
* label = @Translation("User email required", context = "Validation")
* )
*/
class UserMailRequired extends Constraint implements ConstraintValidatorInterface {
/**
* Violation message. Use the same message as FormValidator.
*
* @var string
*/
public $message = '!name field is required.';
/**
* @var \Symfony\Component\Validator\ExecutionContextInterface
*/
protected $context;
/**
* {@inheritDoc}
*/
public function initialize(ExecutionContextInterface $context) {
$this->context = $context;
}
/**
* {@inheritdoc}
*/
public function validatedBy() {
return get_class($this);
}
/**
* {@inheritdoc}
*/
public function validate($items, Constraint $constraint) {
/** @var \Drupal\Core\Field\FieldItemListInterface $items */
/** @var \Drupal\user\UserInterface $account */
$account = $this->context->getMetadata()->getTypedData()->getEntity();
$existing_value = NULL;
if ($account->id()) {
$account_unchanged = \Drupal::entityManager()
->getStorage('user')
->loadUnchanged($account->id());
$existing_value = $account_unchanged->getEmail();
}
$required = !(!$existing_value && \Drupal::currentUser()->hasPermission('administer users'));
if ($required && (!isset($items) || $items->isEmpty())) {
$this->context->addViolation($this->message, array('!name' => String::placeholder($account->getFieldDefinition('mail')->getLabel())));
}
}
}
core/modules/user/src/Tests/UserEditTest.php
View file @ 17c25003
......@@ -16,12 +16,51 @@
*/
class UserEditTest extends WebTestBase {
/**
* Modules to enable.
*
* @var array
*/
public static $modules = array('filter');
/**
* {@inheritdoc}
*/
protected function setUp() {
parent::setUp();
$this->config('user.settings')->set('signatures', TRUE)->save();
// Prefetch and create text formats.
$this->filtered_html_format = entity_create('filter_format', array(
'format' => 'filtered_html_format',
'name' => 'Filtered HTML',
'weight' => -1,
'filters' => array(
'filter_html' => array(
'module' => 'filter',
'status' => TRUE,
'settings' => array(
'allowed_html' => '<a> <em> <strong>',
),
),
),
));
$this->filtered_html_format->save();
$this->full_html_format = entity_create('filter_format', array(
'format' => 'full_html',
'name' => 'Full HTML',
));
$this->full_html_format->save();
}
/**
* Test user edit page.
*/
function testUserEdit() {
// Test user edit functionality.
$user1 = $this->drupalCreateUser(array('change own username'));
$user1 = $this->drupalCreateUser(array('change own username', $this->full_html_format->getPermissionName(), $this->filtered_html_format->getPermissionName()));
$user2 = $this->drupalCreateUser(array());
$this->drupalLogin($user1);
......@@ -85,6 +124,11 @@ function testUserEdit() {
$config->set('password_strength', FALSE)->save();
$this->drupalPostForm("user/" . $user1->id() . "/edit", $edit, t('Save'));
$this->assertNoRaw(t('Password strength:'), 'The password strength indicator is not displayed.');
// Test user signature
$edit = array('signature[format]' => $this->full_html_format->id(), 'signature[value]' => $this->randomString(256));
$this->drupalPostForm('user/' . $user1->id() . '/edit', $edit, t('Save'));
$this->assertRaw(t("%name: may not be longer than @max characters.", array('%name' => t('Signature'), '@max' => 255)));
}
/**
......
core/modules/user/src/Tests/UserRegistrationTest.php
View file @ 17c25003
......@@ -138,13 +138,13 @@ function testRegistrationEmailDuplicates() {
// Attempt to create a new account using an existing email address.
$this->drupalPostForm('user/register', $edit, t('Create new account'));
$this->assertText(t('The email address @email is already registered.', array('@email' => $duplicate_user->getEmail())), 'Supplying an exact duplicate email address displays an error message');
$this->assertText(t('The email address @email is already taken.', array('@email' => $duplicate_user->getEmail())), 'Supplying an exact duplicate email address displays an error message');
// Attempt to bypass duplicate email registration validation by adding spaces.
$edit['mail'] = ' ' . $duplicate_user->getEmail() . ' ';
$this->drupalPostForm('user/register', $edit, t('Create new account'));
$this->assertText(t('The email address @email is already registered.', array('@email' => $duplicate_user->getEmail())), 'Supplying a duplicate email address with added whitespace displays an error message');
$this->assertText(t('The email address @email is already taken.', array('@email' => $duplicate_user->getEmail())), 'Supplying a duplicate email address with added whitespace displays an error message');
}
function testRegistrationDefaultValues() {
......
core/modules/user/src/Tests/UserValidationTest.php
View file @ 17c25003
......@@ -7,8 +7,10 @@
namespace Drupal\user\Tests;
use Drupal\Component\Utility\String;
use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Field\Plugin\Field\FieldType\EmailItem;
use Drupal\Core\Language\Language;
use Drupal\Core\Render\Element\Email;
use Drupal\simpletest\KernelTestBase;
use Drupal\user\Entity\Role;
......@@ -74,7 +76,10 @@ function testUsernames() {
* Runs entity validation checks.
*/
function testValidation() {
$user = User::create(array('name' => 'test'));
$user = User::create(array(
'name' => 'test',
'mail' => 'test@example.com',
));
$violations = $user->validate();
$this->assertEqual(count($violations), 0, 'No violations when validating a default user.');
......@@ -128,20 +133,43 @@ function testValidation() {
$this->assertEqual($violations[0]->getPropertyPath(), 'mail');
$this->assertEqual($violations[0]->getMessage(), t('The email address %mail is already taken.', array('%mail' => 'existing@example.com')));
$user->set('mail', NULL);
$violations = $user->validate();
$this->assertEqual(count($violations), 1, 'E-mail addresses may not be removed');
$this->assertEqual($violations[0]->getPropertyPath(), 'mail');
$this->assertEqual($violations[0]->getMessage(), t('!name field is required.', array('!name' => String::placeholder($user->getFieldDefinition('mail')->getLabel()))));
$user->set('mail', 'someone@example.com');
$user->set('signature', $this->randomString(256));
$this->assertLengthViolation($user, 'signature', 255);
$user->set('signature', NULL);
$user->set('signature_format', $this->randomString(32));
$this->assertAllowedValuesViolation($user, 'signature_format');
$user->set('signature_format', NULL);
$user->set('timezone', $this->randomString(33));
$this->assertLengthViolation($user, 'timezone', 32);
$this->assertLengthViolation($user, 'timezone', 32, 2, 1);
$user->set('timezone', 'invalid zone');
$this->assertAllowedValuesViolation($user, 'timezone');
$user->set('timezone', NULL);
$user->set('init', 'invalid');
$violations = $user->validate();
$this->assertEqual(count($violations), 1, 'Violation found when init email is invalid');
$this->assertEqual($violations[0]->getPropertyPath(), 'init.0.value');
$this->assertEqual($violations[0]->getMessage(), t('This value is not a valid email address.'));
$user->set('init', NULL);
$user->set('langcode', 'invalid');
$this->assertAllowedValuesViolation($user, 'langcode');
$user->set('langcode', NULL);
// Only configurable langcodes are allowed for preferred languages.
$user->set('preferred_langcode', Language::LANGCODE_NOT_SPECIFIED);
$this->assertAllowedValuesViolation($user, 'preferred_langcode');
$user->set('preferred_langcode', NULL);
$user->set('preferred_admin_langcode', Language::LANGCODE_NOT_SPECIFIED);
$this->assertAllowedValuesViolation($user, 'preferred_admin_langcode');
$user->set('preferred_admin_langcode', NULL);
Role::create(array('id' => 'role1'))->save();
Role::create(array('id' => 'role2'))->save();
......@@ -149,6 +177,7 @@ function testValidation() {
// Test cardinality of user roles.
$user = entity_create('user', array(
'name' => 'role_test',
'mail' => 'test@example.com',
'roles' => array('role1', 'role2'),
));
$violations = $user->validate();
......@@ -170,13 +199,32 @@ function testValidation() {
* The field that violates the maximum length.
* @param int $length
* Number of characters that was exceeded.
* @param int $count
* (optional) The number of expected violations. Defaults to 1.
* @param int $expected_index
* (optional) The index at which to expect the violation. Defaults to 0.
*/
protected function assertLengthViolation(EntityInterface $entity, $field_name, $length) {
protected function assertLengthViolation(EntityInterface $entity, $field_name, $length, $count = 1, $expected_index = 0) {
$violations = $entity->validate();
$this->assertEqual(count($violations), 1, "Violation found when $field_name is too long.");
$this->assertEqual($violations[0]->getPropertyPath(), "$field_name.0.value");
$this->assertEqual(count($violations), $count, "Violation found when $field_name is too long.");
$this->assertEqual($violations[$expected_index]->getPropertyPath(), "$field_name.0.value");
$field_label = $entity->get($field_name)->getFieldDefinition()->getLabel();
$this->assertEqual($violations[0]->getMessage(), t('%name: may not be longer than @max characters.', array('%name' => $field_label, '@max' => $length)));
$this->assertEqual($violations[$expected_index]->getMessage(), t('%name: may not be longer than @max characters.', array('%name' => $field_label, '@max' => $length)));
}
/**
* Verifies that a AllowedValues violation exists for the given field.
*
* @param \Drupal\core\Entity\EntityInterface $entity
* The entity object to validate.
* @param string $field_name
* The name of the field to verify.
*/
protected function assertAllowedValuesViolation(EntityInterface $entity, $field_name) {
$violations = $entity->validate();
$this->assertEqual(count($violations), 1, "Allowed values violation for $field_name found.");
$this->assertEqual($violations[0]->getPropertyPath(), "$field_name.0.value");
$this->assertEqual($violations[0]->getMessage(), t('The value you selected is not a valid choice.'));
}
}
core/modules/user/src/UserInterface.php
View file @ 17c25003
......@@ -96,6 +96,16 @@ public function setEmail($mail);
*/
public function getSignature();