Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split validation into multiple methods to simplify login and registration form customizations #722

Open
wants to merge 1 commit into
base: 8.x-2.x
Choose a base branch
from
Open

Split validation into multiple methods to simplify login and registration form customizations #722

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
222 changes: 153 additions & 69 deletions modules/checkout/src/Plugin/Commerce/CheckoutPane/Login.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Drupal\Core\Url;
use Drupal\Core\Link;
use Drupal\user\UserAuthInterface;
use Drupal\user\UserInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\RequestStack;

Expand Down Expand Up @@ -195,7 +196,7 @@ public function buildPaneForm(array $pane_form, FormStateInterface $form_state,
'#type' => 'textfield',
'#title' => $this->t('Username'),
'#size' => 60,
'#maxlength' => USERNAME_MAX_LENGTH,
'#maxlength' => UserInterface::USERNAME_MAX_LENGTH,
'#attributes' => [
'autocorrect' => 'none',
'autocapitalize' => 'none',
Expand Down Expand Up @@ -259,7 +260,7 @@ public function buildPaneForm(array $pane_form, FormStateInterface $form_state,
$pane_form['register']['name'] = [
'#type' => 'textfield',
'#title' => $this->t('Username'),
'#maxlength' => USERNAME_MAX_LENGTH,
'#maxlength' => UserInterface::USERNAME_MAX_LENGTH,
'#description' => $this->t("Several special characters are allowed, including space, period (.), hyphen (-), apostrophe ('), underscore (_), and the @ sign."),
'#required' => FALSE,
'#attributes' => [
Expand Down Expand Up @@ -289,87 +290,170 @@ public function buildPaneForm(array $pane_form, FormStateInterface $form_state,
* {@inheritdoc}
*/
public function validatePaneForm(array &$pane_form, FormStateInterface $form_state, array &$complete_form) {
$values = $form_state->getValue($pane_form['#parents']);
$triggering_element = $form_state->getTriggeringElement();

switch ($triggering_element['#op']) {
case 'continue':
return;

case 'login':
$name_element = $pane_form['returning_customer']['name'];
$username = $values['returning_customer']['name'];
$password = trim($values['returning_customer']['password']);
// Generate the "reset password" url.
$query = !empty($username) ? ['name' => $username] : [];
$password_url = Url::fromRoute('user.pass', [], ['query' => $query])->toString();

if (empty($username) || empty($password)) {
$form_state->setError($pane_form['returning_customer'], $this->t('Unrecognized username or password. <a href=":url">Have you forgotten your password?</a>', [':url' => $password_url]));
return;
}
if (user_is_blocked($username)) {
$form_state->setError($name_element, $this->t('The username %name has not been activated or is blocked.', ['%name' => $username]));
return;
}
if (!$this->credentialsCheckFlood->isAllowedHost($this->clientIp)) {
$form_state->setErrorByName($name_element, $this->t('Too many failed login attempts from your IP address. This IP address is temporarily blocked. Try again later or <a href=":url">request a new password</a>.', [':url' => Url::fromRoute('user.pass')]));
$this->credentialsCheckFlood->register($this->clientIp, $username);
return;
}
elseif (!$this->credentialsCheckFlood->isAllowedAccount($this->clientIp, $username)) {
$form_state->setErrorByName($name_element, $this->t('Too many failed login attempts for this account. It is temporarily blocked. Try again later or <a href=":url">request a new password</a>.', [':url' => Url::fromRoute('user.pass')]));
$this->credentialsCheckFlood->register($this->clientIp, $username);
return;
}

$uid = $this->userAuth->authenticate($username, $password);
if (!$uid) {
$this->credentialsCheckFlood->register($this->clientIp, $username);
$form_state->setError($name_element, $this->t('Unrecognized username or password. <a href=":url">Have you forgotten your password?</a>', [':url' => $password_url]));
}
$form_state->set('logged_in_uid', $uid);
$this->validateReturningCustomer($pane_form, $form_state, $complete_form);
break;

case 'register':
$email = $values['register']['mail'];
$username = $values['register']['name'];
$password = trim($values['register']['password']);
if (empty($email)) {
$form_state->setError($pane_form['register']['mail'], $this->t('Email field is required.'));
return;
}
if (empty($username)) {
$form_state->setError($pane_form['register']['name'], $this->t('Username field is required.'));
return;
}
if (empty($password)) {
$form_state->setError($pane_form['register']['password'], $this->t('Password field is required.'));
return;
}
$this->validateRegister($pane_form, $form_state, $complete_form);

/** @var \Drupal\user\UserInterface $account */
$account = $this->entityTypeManager->getStorage('user')->create([
'mail' => $email,
'name' => $username,
'pass' => $password,
'status' => TRUE,
]);
// Validate the entity. This will ensure that the username and email
// are in the right format and not already taken.
$violations = $account->validate();
foreach ($violations->getByFields(['name', 'mail']) as $violation) {
list($field_name) = explode('.', $violation->getPropertyPath(), 2);
$form_state->setError($pane_form['register'][$field_name], $violation->getMessage());
}

if (!$form_state->hasAnyErrors()) {
$account->save();
$form_state->set('logged_in_uid', $account->id());
}
break;
}
}

/**
* Validates a returning customers username and password.
*
* @param array $pane_form
* The pane form, containing the following basic properties:
* - #parents: Identifies the position of the pane form in the overall
* parent form, and identifies the location where the field values are
* placed within $form_state->getValues().
* @param \Drupal\Core\Form\FormStateInterface $form_state
* The form state of the parent form.
* @param array $complete_form
* The complete form structure.
*/
protected function validateReturningCustomer(array &$pane_form, FormStateInterface $form_state, array &$complete_form) {
$values = $form_state->getValue($pane_form['#parents']);
$name_element = $pane_form['returning_customer']['name'];
$username = $values['returning_customer']['name'];
$password = trim($values['returning_customer']['password']);
// Generate the "reset password" url.
$query = !empty($username) ? ['name' => $username] : [];
$password_url = Url::fromRoute('user.pass', [], ['query' => $query])->toString();

if (empty($username) || empty($password)) {
$form_state->setError($pane_form['returning_customer'], $this->t('Unrecognized username or password. <a href=":url">Have you forgotten your password?</a>', [':url' => $password_url]));
return;
}
if (user_is_blocked($username)) {
$form_state->setError($name_element, $this->t('The username %name has not been activated or is blocked.', ['%name' => $username]));
return;
}
if (!$this->credentialsCheckFlood->isAllowedHost($this->clientIp)) {
$form_state->setErrorByName($name_element, $this->t('Too many failed login attempts from your IP address. This IP address is temporarily blocked. Try again later or <a href=":url">request a new password</a>.', [':url' => Url::fromRoute('user.pass')]));
$this->credentialsCheckFlood->register($this->clientIp, $username);
return;
}
elseif (!$this->credentialsCheckFlood->isAllowedAccount($this->clientIp, $username)) {
$form_state->setErrorByName($name_element, $this->t('Too many failed login attempts for this account. It is temporarily blocked. Try again later or <a href=":url">request a new password</a>.', [':url' => Url::fromRoute('user.pass')]));
$this->credentialsCheckFlood->register($this->clientIp, $username);
return;
}

$uid = $this->userAuth->authenticate($username, $password);
if (!$uid) {
$this->credentialsCheckFlood->register($this->clientIp, $username);
$form_state->setError($name_element, $this->t('Unrecognized username or password. <a href=":url">Have you forgotten your password?</a>', [':url' => $password_url]));
}
$form_state->set('logged_in_uid', $uid);
}

/**
* Validates the registration form and creates a new user.
*
* @param array $pane_form
* The pane form, containing the following basic properties:
* - #parents: Identifies the position of the pane form in the overall
* parent form, and identifies the location where the field values are
* placed within $form_state->getValues().
* @param \Drupal\Core\Form\FormStateInterface $form_state
* The form state of the parent form.
* @param array $complete_form
* The complete form structure.
*/
protected function validateRegister(array &$pane_form, FormStateInterface $form_state, array &$complete_form) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's odd that we have a validateRegister() method that does no validation, the entire logic is in the oddly named getAndValidateRegisterUserValues, which is only used in one place. What's the reasoning behind that split?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the names are bad, suggestions welcome. The reason for the split is being able to change the user values, see my example. For that to work there needs to be a way to do something between bulding the values array and creating the entity. Another approach would be an "alter" method between or/and an event.

if ($user_values = $this->getAndValidateRegisterUserValues($pane_form, $form_state)) {

/** @var \Drupal\user\UserInterface $account */
$account = $this->entityTypeManager->getStorage('user')->create($user_values);

// Validate the entity. This will ensure that the username and email
// are in the right format and not already taken.
$violations = $account->validate();
foreach ($violations->getByFields(['name', 'mail']) as $violation) {
list($field_name) = explode('.', $violation->getPropertyPath(), 2);
$form_state->setError($pane_form['register'][$field_name], $violation->getMessage());
}

if (!$form_state->hasAnyErrors()) {
$account->save();
$form_state->set('logged_in_uid', $account->id());
}
}
}

/**
* Validates form values for the register form.
*
* @param array $pane_form
* The pane form, containing the following basic properties:
* - #parents: Identifies the position of the pane form in the overall
* parent form, and identifies the location where the field values are
* placed within $form_state->getValues().
* @param \Drupal\Core\Form\FormStateInterface $form_state
* The form state of the parent form.
*
* @return array|null
* Either an array of values that can be used to create a new user or NULL
* if there are validation errors.
*/
protected function getAndValidateRegisterUserValues(array &$pane_form, FormStateInterface $form_state) {
$values = $form_state->getValue($pane_form['#parents']);

$user_values = [
'status' => TRUE
];

// If the e-mail, username and password fields are visible, ensure they
// have a value and add them to the user values. This allows subclasses and
// form alters to hide certain form elements and then provide alternative
// values, for example to set the username based on the e-mail.
$email_element = $pane_form['register']['mail'];
if (!isset($email_element['#access']) || $email_element['#access']) {
$email = $values['register']['mail'];
if (empty($email)) {
$form_state->setError($email_element, $this->t('Email field is required.'));
return NULL;
}
else {
$user_values['mail'] = $email;
}
}

$name_element = $pane_form['register']['name'];
if (!isset($name_element['#access']) || $name_element['#access']) {
$username = $values['register']['name'];
if (empty($username)) {
$form_state->setError($name_element, $this->t('Username field is required.'));
return NULL;
}
else {
$user_values['name'] = $username;
}
}

$password_element = $pane_form['register']['password'];
if (!isset($password_element['#access']) || $password_element['#access']) {
$password = trim($values['register']['password']);
if (empty($password)) {
$form_state->setError($pane_form['register']['password'], $this->t('Password field is required.'));
return NULL;
}
else {
$user_values['pass'] = $password;
}
}
return $user_values;
}

/**
* {@inheritdoc}
*/
Expand Down