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.

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

Change the comment about when the password field is shown on user_account_form() or change the code to match that comment #6079

Open
kiamlaluno opened this issue Apr 25, 2023 · 2 comments

Comments

@kiamlaluno
Copy link
Member

kiamlaluno commented Apr 25, 2023

user_account_form() contains the following code.

// Display password field only for existing users or when user is allowed to
// assign a password during registration.
if (!$register) {
  $form['account']['pass'] = array(
    '#title' => t('New password'),
    '#type' => 'password',
    '#password_toggle' => TRUE,
    '#password_strength' => TRUE,
  );

The comment says that $form['account']['pass'] is shown when either an account is edited (not created) or when users are allowed to set a password during registration. The code only add $form['account']['pass'] when an account is edited (not created).

($register is not set to TRUE when a visitor is registering an account, despite that variable name.)

Either the code is changed to match the comment, or the comment is changed to reflect what the code does.

I think the code should be changed to avoid adding the password field when visitors registering an account will receive an email with a link to set the password, or when administrator users creates an account for somebody else who then will receive that email.

@kiamlaluno
Copy link
Member Author

kiamlaluno commented Apr 25, 2023

user_register_submit() uses the following code, to access the submitted password value.

$user_email_verification = config_get('system.core', 'user_email_verification');
$admin = $form_state['values']['administer_users'];

if (!$user_email_verification || $admin) {
  $pass = $form_state['values']['pass'];
}

The code in user_account_form() should be changed to include those checks too.

// Display password field only for existing users or when user is allowed to
// assign a password during registration.
$user_email_verification = config_get('system.core', 'user_email_verification');
if (($user->uid == 0 && !$user_email_verification) || $admin_users) {
  $form['account']['pass'] = array(
    '#title' => t('New password'),
    '#type' => 'password',
    '#password_toggle' => TRUE,
    '#password_strength' => TRUE,
  );

I used $admin_users which user_account_form() already initialized with $admin_users = user_access('administer users');.

@kiamlaluno
Copy link
Member Author

kiamlaluno commented Apr 25, 2023

Actually, the password field is not necessary when visitors gets this email.

[user:name],

Your account at [site:name] has been activated.

You may now log in by clicking this link or copying and pasting it into your browser:

[user:one-time-login-url]

This link can only be used once to log in and will lead you to a page where you can set your password.

After setting your password, you will be able to log in at [site:login-url] in the future using:

username: [user:name]
password: Your password

--  [site:name] team

In that case, people set their own password after visiting [user:one-time-login-url].

Asking visitors to provide a password, which they need to set again later, could confuse them.

I still have to check what happens when administrator users create an account for somebody else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant