Updated: Comment #28

Problem/Motivation

The minimum password length is currently 1 character. This not good from a security point of view.

Conversation goes on to say minimum password length should be Configurable and discusses where the settings should be kept.

Proposed resolution

Propose to have minimum password length as a per site configurable value.

Remaining tasks

1) Add 'Minimum Password Length' setting to some form. Probably the Configuration > People > Accounts form.
2) Change JS behaviour to implement this setting.
3) Check minimum length when the entity is created, not on user form validation or on the password element, as we want to cover situations when a user is created via the form or API but we don't want to force this when the password element is utilized - although it would be nice to have as a configurable property for the element.
4) Backport to D7 if possible.

User interface changes

New setting on the Configure > People > Accounts form which specified 'Minimum Password Length' (user.settings.yml - core/modules/user/config/user.settings.yml)

Original report by Tsalop

When user types new password - password can be only 1 letter long.
So I suggest a check that the password is at least 4 characters long....

Files: 
CommentFileSizeAuthor
#92 require_a-1824800-92.patch28.44 KBsubhojit777
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,948 pass(es), 54 fail(s), and 28 exception(s). View
#85 1824800-require-pass.patch26.6 KBRavindraSingh
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,179 pass(es), 86 fail(s), and 27 exception(s). View
#80 require_a-1824800-80.patch26.43 KBsubhojit777
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,142 pass(es), 86 fail(s), and 27 exception(s). View
#76 require_a-1824800-76.patch10.94 KBsubhojit777
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,819 pass(es), 69 fail(s), and 28 exception(s). View
#71 require_a-1824800-71.patch11.34 KBsubhojit777
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 89,089 pass(es), 67 fail(s), and 28 exception(s). View
#69 require_a-1824800-69.patch10.43 KBsubhojit777
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch require_a-1824800-69.patch. Unable to apply patch. See the log in the details link for more information. View
#63 password-length-1824800.63.patch10.69 KBlarowlan
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 88,005 pass(es). View
#63 interdiff.txt4.67 KBlarowlan
#62 Screenshot 2015-03-09 10.57.36.png49.11 KBlarowlan
#61 password_min_length-1824800-61.patch8.86 KBleex
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,167 pass(es), 143 fail(s), and 37 exception(s). View
#54 password_min_length-1824800-54.patch7.41 KBadci_contributor
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,845 pass(es). View

Comments

Tsalop’s picture

FileSize
3.62 KB
FAILED: [[SimpleTest]]: [MySQL] 39,395 pass(es), 121 fail(s), and 56 exception(s). View

Okay... Here is the patch that adds the check for password length.
This patch also contains this change.

Tsalop’s picture

Project: Administration Views » Drupal core
Version: 7.x-1.x-dev » 7.x-dev
Component: User interface » user system
Assigned: Tsalop » Unassigned
superspring’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, password_length-check_password_length-1824800-1.patch, failed testing.

earnie’s picture

Version: 7.x-dev » 8.x-dev
Issue tags: +needs backport to D7

All new features go to the developing version. We would need to make this configurable, IMO.

Rob C’s picture

Totally agree with earnie, make this an option. Would be in line with other related developments too, like: #432962: Add option to disable password strength checking and #111317: Allow users to login using either their username OR their e-mail address and (etc, etc). More flexibility++.

scottalan’s picture

FileSize
1.55 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch user-password_length-1824800-7.patch. Unable to apply patch. See the log in the details link for more information. View

I'm posting here as to not further clutter the issue queue with issues related to 'Password length'. I wanted to see if it's possible allow modules (such as logintoboggan) to more easily alter the length of the password. I'm uploading a patch that allows the alteration of a Drupal behaviors setting to set the number of characters.

This way a module could implement hook_element_info_alter() and add a callback to the $type['password_confirm']['#process'] array and override the 'numCharacters' and the 'tooShort' elements of the 'password'.

e.g.,

function MODULE_form_process_password_confirm($element, &$form_state) {
  foreach ($element['#attached']['js'] as &$attached) {
    if (is_array($attached) && !empty($attached['data']['password'])) {
      $length = variable_get('lMODULE_minimum_password_length', 0);
      if ($length) {
        $attached['data']['password']['numCharacters'] = $length;
        $attached['data']['password']['tooShort'] = t('Your password must be at least @num characters.', array('@num' => $length));
      }
    }
  }
  return $element;
}
Crisz’s picture

Issue summary: View changes
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 7: user-password_length-1824800-7.patch, failed testing.

mandar.harkare’s picture

Status: Needs work » Needs review
FileSize
2.34 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed. View

Status: Needs review » Needs work

The last submitted patch, 10: drupal-password-min-length-1824800-10.patch, failed testing.

mandar.harkare’s picture

Status: Needs work » Needs review
FileSize
2.36 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed. View

Added the missed parameter to form_error.

Status: Needs review » Needs work

The last submitted patch, 12: drupal-password-min-length-1824800-12.patch, failed testing.

The last submitted patch, 12: drupal-password-min-length-1824800-12.patch, failed testing.

mandar.harkare’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 12: drupal-password-min-length-1824800-12.patch, failed testing.

mandar.harkare’s picture

Can anyone tell me why this patch is failing ?

mandar.harkare’s picture

Status: Needs work » Needs review
FileSize
3.08 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch drupal-password-min-length-1824800-18.patch. Unable to apply patch. See the log in the details link for more information. View

Status: Needs review » Needs work

The last submitted patch, 18: drupal-password-min-length-1824800-18.patch, failed testing.

mandar.harkare’s picture

Status: Needs work » Needs review
FileSize
2.66 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed. View

Sorry for the previous patch.

Status: Needs review » Needs work

The last submitted patch, 20: drupal-password-min-length-1824800-20.patch, failed testing.

Crisz’s picture

FileSize
706 bytes
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,368 pass(es), 177 fail(s), and 86 exception(s). View

I tested this patch while installing drupal 8-dev and also after installation with a MySQL database.

Although I feel that a minimal password length is extremely important for security reasons, I'm not sure whether the way it is now is the expected one. So, the reason why the other patches failed and why this one can possibly fail, could be simply the design of the tests and not the patches themselves.

Anyhow, the way it is now does not allow the user to configure the minimal length. Therefore, even if this patch passes, the discussion whether this should be configurable or not should continue.

Crisz’s picture

Status: Needs work » Needs review
Crisz’s picture

FileSize
714 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,656 pass(es). View

This version of the patch should improve the number of comparisons when one of the fields is empty.

The last submitted patch, 22: password_min_length-1824800-22.patch, failed testing.

droplet’s picture

Status: Needs review » Needs work
Issue tags: -needs backport to D7 +JavaScript

I think it needed to be optional, so we can set it to 8 chars or even more. Also, missing JS patches.

(It changed the default behaviours, don't think it can be backported to D7)

Crisz’s picture

Version 7 contains the exact same function but in a different line of the file (line 2880 of the form.inc file), so it can probably be ported to Drupal 7. If configurable, how/where should we include the configuration? In user.settings.yml or similar file or through the user interface? Should 1 or 4 or 6 chars be the default? Does the documentation say anything about this? I think that only after what should be done is decided we should continue.

droplet’s picture

In user.settings.yml is fine.

leex’s picture

Issue summary: View changes

Updated Issue Summary #Drupal8NZ

leex’s picture

Issue summary: View changes
leex’s picture

FileSize
5.13 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch drupal-password-min-length-1824800-31_1.patch. Unable to apply patch. See the log in the details link for more information. View

One sprint later, here is my patch. I have added a setting to the user settings form. If this is good, all we need now are the tests.

I have decided to check password length before password match as we can only display one error per element and in my opinion it's more important for people to know the minimum password length than if the passwords match. Most people already know passwords need to match but they might not be so informed about the minimum length.

#Drupal8NZ

leex’s picture

Status: Needs work » Needs review
FileSize
5.12 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,686 pass(es), 3 fail(s), and 1 exception(s). View

Minor comment edit and push to Needs Review. Tests still need to be done, I am looking into this but have never written a test before.

Status: Needs review » Needs work

The last submitted patch, 32: drupal-password-min-length-1824800-32.patch, failed testing.

leex’s picture

Status: Needs work » Needs review
Issue tags: +Needs tests, +html5
FileSize
6.08 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch drupal-password-min-length-1824800-34.patch. Unable to apply patch. See the log in the details link for more information. View

This should pass the tests. Not sure about how I'm handling error messages as you can't set two on one element so I've made a work around which I'm not 100% happy with but it seems about as elegant as you can get.

Status: Needs review » Needs work

The last submitted patch, 34: drupal-password-min-length-1824800-34.patch, failed testing.

leex’s picture

Status: Needs work » Needs review
FileSize
7.35 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch drupal-password-min-length-1824800-36.patch. Unable to apply patch. See the log in the details link for more information. View

Let's try this again as last patch was against an older version of code base. Also added the required tests and the 'min' property to the form element.

Status: Needs review » Needs work

The last submitted patch, 36: drupal-password-min-length-1824800-36.patch, failed testing.

leex’s picture

Status: Needs work » Needs review
FileSize
7.35 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch drupal-password-min-length-1824800-38.patch. Unable to apply patch. See the log in the details link for more information. View

Well that was a fail, let's try again!

Status: Needs review » Needs work

The last submitted patch, 38: drupal-password-min-length-1824800-38.patch, failed testing.

leex’s picture

Status: Needs work » Needs review
FileSize
7.25 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch drupal-password-min-length-1824800-40.patch. Unable to apply patch. See the log in the details link for more information. View

Another fail and another try. Sorry about the pollution.

drupalviking’s picture

Assigned: Unassigned » drupalviking
drupalviking’s picture

I did test this patch using this method:

I confirmed that this patch was still needed by creating a fresh install at Simplytest.me. That went on without a problem, even if the standard password is five characters (hence the problem still exists). Then I created another user with three character password, and it went through, confirming even further that the problem still exists.

I fired up a Simplytest.me instance and tried to install. The install failed because the standard Simplytest.me password is five characters (proving for some point that the patch works). Changed the admin password to six characters and installed.

For a final test I created another user, tried password with three characters and got a form warning, stating that my password should be six characters. I then went to admin/config/people/accounts and changed the password strength parameter to five characters and tried again to create a user with three. It failed again (as promised). I finally created a five character password that went through. My final test was to test passwords greater than configured, and it went through as well, confirming that the patch works as described.

drupalviking’s picture

Assigned: drupalviking » Unassigned
leex’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

I think this is ready for commit.

The last submitted patch, 31: drupal-password-min-length-1824800-31.patch, failed testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 40: drupal-password-min-length-1824800-40.patch, failed testing.

chanderbhushan’s picture

FileSize
612 bytes
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch password_min_length-1824800-48.patch. Unable to apply patch. See the log in the details link for more information. View

form_error needs to be replaced with $form_state->setError

chanderbhushan’s picture

form_error needs to be replaced with $form_state->setError

chanderbhushan’s picture

Status: Needs work » Needs review
Issue tags: -html5 +html, +#dcdelhi

Status: Needs review » Needs work

The last submitted patch, 48: password_min_length-1824800-48.patch, failed testing.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 48: password_min_length-1824800-48.patch, failed testing.

adci_contributor’s picture

FileSize
7.41 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,845 pass(es). View

Trying to reroll;

adci_contributor’s picture

Status: Needs work » Needs review
leex’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +#Drupal8nz

I have performed manually using simplytest.me the exact procedure @drupalviking followed with the same results, so this patch seems to work fine.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/lib/Drupal/Core/Render/Element/PasswordConfirm.php
@@ -77,9 +77,17 @@ public static function processPasswordConfirm(&$element, FormStateInterface $for
+    $minimum_password_length = \Drupal::config('user.settings')->get('minimum_password_length');

The Password render element should implement ContainerInjectionInterface and have the configuration factory injected.

Also the minimum length should be set as a constraint on the password field in User::baseFieldDefinitions so that users created through the Entity API have this checking.

alexpott’s picture

In #57 I mentioned adding a constraint to the password field in User::baseFieldDefinitions. This is of course wrong because that value is the salted and hashed value. But we do need to consider how this will play out with REST creation or registration of users.

leex’s picture

Issue summary: View changes

Alex you're right, that should be the case and significant progress has been made on this issue at Drupal South 2015 sprint, with most credit to @larowlan for his amazing mentoring.

This is still hitting upon a bug, which appears to be at the theme level but probably isn't. It needs further debugging but is almost there.

The scope of this issue has changed temporarily. It is only going to cover new user creation and user editing is being put aside for the time being. This is because of problems related to this critical blocker: https://www.drupal.org/node/2418119 where the password must be provided for an account edit. Once that issue is resolved, we can implement this feature when an account is edited but in the meantime, this feature for new account creation still adds value.

larowlan’s picture

Leex, can you put up your patch?

leex’s picture

FileSize
8.86 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,167 pass(es), 143 fail(s), and 37 exception(s). View

Yea definitely, sorry. I think the managed_file element + late night got the best of me there.

larowlan’s picture

Issue summary: View changes
FileSize
49.11 KB
+++ b/core/modules/user/src/Plugin/Validation/Constraint/UserPasswordLengthValidator.php
@@ -0,0 +1,57 @@
+  function __construct(ConfigFactoryInterface $config_factory) {
...
+    return new static (null, $container->get('config.factory'));

So this is the only issue - we didn't fix the constructor, we can drop the null. Always the way eh:)

New patch coming, with some more test coverage.

Screenshot showing the constraint kicking in - look ma no #validate

larowlan’s picture

Title: Password length » Require a (configurable) minimum password length for user accounts
Priority: Minor » Normal
Status: Needs work » Needs review
Issue tags: +DrupalSouth, +Entity Field API, +Entity validation
FileSize
4.67 KB
10.69 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 88,005 pass(es). View

Fixes #62, adds some tests, handles when no password is set (mostly in tests) and sorts some coding standards issues.

This might be in the 8.1.x domain now too

larowlan’s picture

Issue tags: +security
leex’s picture

Status: Needs review » Reviewed & tested by the community

Doh, so close aye! Thanks again for all your help. I hope I can push this over the line :)

I have run through the same set of testing drupalviking and I did last time using simplyme. Comment #42

Everything was the same except the initial install which allowed the use of a password less than 6 characters. This also happened when I tried the installation locally. I'm guessing this is because the validation was on the password element before and now it is attach to the user registration as a plugin and so is not attached to the install form.

I also tested trying to change password using the edit form and it works as expected, without the limitation.

Patch looks good.

The last submitted patch, 61: password_min_length-1824800-61.patch, failed testing.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

If you use the interactive installer you can still create a user (and user 1 at that) with a password that breaks the minimum rule. Also I could continue to change the password to be less than the minimum length.

Also should we consider using the html5 minlength attribute - I guess this could be a followup - currently only supported by chrome.

+++ b/core/modules/user/src/AccountForm.php
@@ -390,9 +390,12 @@ public function validate(array $form, FormStateInterface $form_state) {
+
+

Extra space

subhojit777’s picture

Assigned: Unassigned » subhojit777
Issue tags: +SprintWeekend2015
subhojit777’s picture

FileSize
10.43 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch require_a-1824800-69.patch. Unable to apply patch. See the log in the details link for more information. View
2.28 KB

The validation on user edit is working. Will work on rest of the suggestions in #67 later. Not moving to needs review.

leex’s picture

Nice work with putting it on the edit form, it was intentionally left out because if it got committed it could potentially block this blocker https://www.drupal.org/node/2418119 as per #59.

That's fine, it just means we should wait until that is fixed before this gets committed.

subhojit777’s picture

Status: Needs work » Needs review
Issue tags: -SprintWeekend2015 +Needs tests
FileSize
936 bytes
11.34 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 89,089 pass(es), 67 fail(s), and 28 exception(s). View

Lets see what others say about this change. Anyways we need tests.

subhojit777’s picture

Issue tags: -#dcdelhi

Removing drupal camp delhi tags :)

Status: Needs review » Needs work

The last submitted patch, 71: require_a-1824800-71.patch, failed testing.

subhojit777’s picture

Issue tags: +SprintWeekend2015
subhojit777’s picture

subhojit777’s picture

Issue tags: -Needs reroll +india
FileSize
10.94 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,819 pass(es), 69 fail(s), and 28 exception(s). View
subhojit777’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests

The last submitted patch, 69: require_a-1824800-69.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 76: require_a-1824800-76.patch, failed testing.

subhojit777’s picture

Status: Needs work » Needs review
FileSize
26.43 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,142 pass(es), 86 fail(s), and 27 exception(s). View

Status: Needs review » Needs work

The last submitted patch, 80: require_a-1824800-80.patch, failed testing.

subhojit777’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 80: require_a-1824800-80.patch, failed testing.

RavindraSingh’s picture

FileSize
26.6 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,179 pass(es), 86 fail(s), and 27 exception(s). View

@subhojit777, YOu did good job. I think we are missing some testing functionality which is not detecting the test for these patches or we are doing somthing wrong. I have just removed user_password() from the test functions and replaced with $this->randomMachineName(); (It also generates minimum 8 string of length which can be used to test).

RavindraSingh’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 85: 1824800-require-pass.patch, failed testing.

subhojit777’s picture

@RavindraSingh Nice catch and thanks for the patch. Always provide an interdiff when you upload a patch. Helps you to track down the changes.

subhojit777’s picture

Looking into the tests.

RavindraSingh’s picture

Just a small reference. I am able to replicate it at local.

In CreateTest.php
$this->assertEqual($entity->uuid(), $loaded_entity->uuid(), 'UUID of created entity is correct.'); is giving an error.Call to a member function uuid() on null in /Applications/devsites/contrib/d8_may_15/core/modules/rest/src/Tests/CreateTest.php on line 335 Its not a error of this basically but It should be fix in it or it might be in rest componenet issues list.
Error on Revision log and my local:
Call to a member function uuid() on null in /Applications/devsites/contrib/d8_may_15/core/modules/rest/src/Tests/CreateTest.php on line 335

subhojit777’s picture

@RavindraSingh Suprised! Drupal\rest\Tests\CreateTest and CollapsedDrupal\rest\Tests\UpdateTest are running fine in my local. Was looking into rest of the tests. I thought that bot has gone awry.

subhojit777’s picture

Status: Needs work » Needs review
FileSize
28.44 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,948 pass(es), 54 fail(s), and 28 exception(s). View
subhojit777’s picture

Patch in #92 is also a reroll, so no interdiff uploaded.

Status: Needs review » Needs work

The last submitted patch, 92: require_a-1824800-92.patch, failed testing.

subhojit777’s picture

Assigned: subhojit777 » Unassigned

There are some fails, and I dont understand why those tests are failing. I am unassigning this issue so that someone else (with more understanding in User module :) ) may continue.

deepakaryan1988’s picture

Issue tags: -SprintWeekend2015, -india

Removing sprint weekend tag!!
As suggested by @YesCT

pwolanin’s picture

Version: 8.0.x-dev » 8.1.x-dev

Bumping features to 8.1

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.