Issue: saml users can't change password in Drupal, despite $config->get('allow.set_drupal_pwd') is TRUE.

Expected: saml users should be able to change password without knowing their current Drupal password.

More detail:

In simplesamlphp_auth_form_user_form_alter, If $saml_enabled == TRUE, there is code to set the current_pass field #Access set to false, with a message that they aren't required to know their Drupal password. 

 // If the user has a simplesamlphp_auth authmap record, then don't require
  // them to know their Drupal password. This will allow them to change their
  // e-mail address, and set a Drupal password if they want to
  // (and are allowed).
  $account = $form_state->getFormObject()->getEntity();

  $saml_enabled = $authmap->get($account->id(), 'simplesamlphp_auth');

  if ($saml_enabled) {
    $form['simplesamlphp_auth_user_enable']['#default_value'] = TRUE;
    $form['account']['current_pass']['#access'] = FALSE;

But this doesn't work currently.

There's an error from ProtectedUserFieldConstraint "Your current password is missing or incorrect; it's required to change the password.".

can't reset password

CommentFileSizeAuthor
#14 simplesamlphp_auth-change_password_and_e-mail-2968598-14.patch797 bytespdenooijer
#2 simplesamlphp_auth-change-password-access-2968598-2.patch684 bytesoknate
cant change password.png102.81 KBoknate

Comments

oknate created an issue. See original summary.

oknate’s picture

oknate
Location Greater New York City Area
commented
StatusFileSize
new simplesamlphp_auth-change-password-access-2968598-2.patch684 bytes

There's a constraint now on user entity's "pass" and "mail" fields "ProtectedUserField".

I found some code in Account::buildEntity where you can skip this validation on the user reset form.

    // Skip the protected user field constraint if the user came from the
    // password recovery page.
    $account->_skipProtectedUserFieldConstraint = $form_state->get('user_pass_reset');

We can use this to allow users to skip having to add their existing password. See ProtectedUserFieldConstraintValidator (it skips validation if _skipProtectedUserFieldConstraint set to TRUE).

This should allow users to change their local password or email.

oknate’s picture

oknate
Location Greater New York City Area
commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
commented
Related issues: +#1372576: User cannot change e-mail address because they don't know their password
oknate’s picture

oknate
Location Greater New York City Area
commented
Status: Active » Needs review
mukila’s picture

mukila commented
Status: Needs review » Reviewed & tested by the community

Applied patch #2 and it is working fine.

anas_maw’s picture

anas_maw commented

Patch #2 working fine for me.
Please commit it.

swirt’s picture

swirt
he/him
Primary language English
commented

Confirmed: Patch #2 works for me too.

pdenooijer’s picture

pdenooijer commented

Seems a bit like a hack, but I understand why this is done this way. RTBC +1

pdenooijer’s picture

pdenooijer commented
Status: Reviewed & tested by the community » Needs work

The current patch has security implications, as you don't need the current password any more to change the accounts password. Will create a patch to enable the current password again. This will allow users that have know their old password, to change their own password and e-mail.

pdenooijer’s picture

pdenooijer commented
StatusFileSize
new simplesamlphp_auth-change_password_and_e-mail-2968598-14.patch797 bytes

With this patch someone can reset his password though the mail and then use that current password to update his settings again. There is no need to disable the current password when your allowed to change it.

pdenooijer’s picture

pdenooijer commented
Status: Needs work » Needs review
paul_leclerc’s picture

paul_leclerc commented

Patch #14 works for me.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Fixed

Yes, #14 makes sense, #2 is indeed insecure.

  • Berdir committed 1c8af94 on 8.x-3.x authored by pdenooijer 
    Issue #2968598 by oknate, pdenooijer: User with simplesaml_auth can't...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

anas_maw’s picture

anas_maw commented

I opened a follow-up issue #3112380: Users with simplesaml_auth don't know their passwords after the registration to find a better user experience practice.

anas_maw’s picture

anas_maw commented
Related issues: +#3112380: Users with simplesaml_auth don't know their passwords after the registration

Please check the patch in #3112380-4: Users with simplesaml_auth don't know their passwords after the registration which has a better user experience.