On Drupal 8 administrator by default all permissions are ticked, this means password policy by pass for admin is also ticked by default.

Suggestion / Feature request is to move bypass password policy to config

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ilsodas created an issue. See original summary.

badjava’s picture

Title: How do you enforce password policy for admin user? » Enforce password policy for admin (user 1) account
Status: Active » Needs review
FileSize
1.02 KB

With the current code, you can't enforce the password policy for the user 1 account unfortunately. IMHO, this is the most critical account that needs to have a secure password since it bypasses all permission checks (which ironically is what allows it to bypass the password policy as well).

in _password_policy_show_policy():

  if ($account->hasPermission('bypass password policies')) {
    $show_password_policy_status = FALSE;
  }

The user 1 account will always return true on a hasPermission check. I've attached a patch which removes the above block and the bypass password policies as a temporary solution.

I think the full solution is to remove the permission and make the roles selection of a password policy to allow including and excluding of roles. Then the user 1 account (or any other account) could be added to a role that was excluded from a password policy.

Setting to needs review just to test the patch, this issue should be needs work if it doesn't fail.

badjava’s picture

Status: Needs review » Needs work
daggerhart’s picture

Title: Enforce password policy for admin (user 1) account » Remove "bypass password policies" permission

Hi @badjava,

I think it's a great idea to remove the "bypass password policies" permission and your patch looks good to me. But I don't think we want to start tracking "included" and "excluded" roles on a password policy. That seems like a path to some very convoluted logic when dealing with users with multiple roles, and each role having included and excluded password policies.

I've updated the title of this issue to reflect exactly what the patch is trying to accomplish and re-queued your patch for testing. If it passes, this issue is ready to be reviewed imo. If it doesn't pass, I'll re-roll it against the current 8.x-3.x branch.

daggerhart’s picture

Status: Needs work » Needs review

Tests passed. This is ready for review.

daggerhart’s picture

Status: Needs review » Reviewed & tested by the community

Verified that this works.

As User 1, I tested editing a user that has a passworld policy. Before the patch I could change the password to anything regardless of the password policies. After the patch was applied, I was unable to save a new password that did not match the password policy.

Patch is simple and it works, markingRTBC

Rahulmaheshuni’s picture

This patch doesn't work with composer

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: Password_policy_composer_patch-2862906-7.patch, failed testing. View results

Rahulmaheshuni’s picture

I have updated the patch that works with composer style of applying patches.

johnzzon’s picture

The last patch won't work with composer, it has the paths all wrong. The one in #2 is correct however.

Rahulmaheshuni’s picture

Patch in #2 works with composer. My patch in #7 and #10 has the wrong path.

daggerhart’s picture

Status: Needs work » Needs review

Requeued for testing against 8.7 & removing excess patches.

daggerhart’s picture

Status: Needs review » Reviewed & tested by the community

Re-tested and reviewed. Patch in #2 applies cleanly and does the job. The permission is gone from the system and as user-1 I am required to respect the policies that apply to my user.

nerdstein’s picture

Status: Reviewed & tested by the community » Patch (to be ported)

I recognize that it's difficult to remove existing features, but I believe this is the right move for this permission. It confuses the system behavior. I've let this sit long enough to solicit feedback and I think, at this point in time, the right move is to commit the patch in #2. This will make it into the next release then, and we can see what kind of feedback, if any, arises. Thanks to all who participated in the discussion. I am not clear what the patches in 7 and 9 were trying to accomplish, since 2 seems to get the job done.

  • nerdstein committed 8180e87 on 8.x-3.x authored by badjava
    Issue #2862906 by badjava, daggerhart: Remove "bypass password policies...
nerdstein’s picture

Status: Patch (to be ported) » Fixed

Marking as fixed

Status: Fixed » Closed (fixed)

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