Require a (configurable) minimum password length for user accounts

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++.

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;
}

Added the missed parameter to form_error.

12: drupal-password-min-length-1824800-12.patch queued for re-testing.

Can anyone tell me why this patch is failing ?

Sorry for the previous patch.

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.

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

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)

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.

In user.settings.yml is fine.

Updated Issue Summary #Drupal8NZ

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

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.

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.

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.

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

Another fail and another try. Sorry about the pollution.

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.

I think this is ready for commit.

form_error needs to be replaced with $form_state->setError

form_error needs to be replaced with $form_state->setError

Trying to reroll;

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

+++ 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.

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.

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.

Leex, can you put up your patch?

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

+++ 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

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

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.

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

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

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.

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

Removing drupal camp delhi tags :)

@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 Nice catch and thanks for the patch. Always provide an interdiff when you upload a patch. Helps you to track down the changes.

Looking into the tests.

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

@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.

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

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.

Removing sprint weekend tag!!
As suggested by @YesCT

Bumping features to 8.1

#2563751: Password field errors on user create/edit/login when password is (literally) 0

Hi, currently the reset password form allows to enter a zero length password :-(

#2855328: Password and confirm password should be mandatory fields while setting up password using one time link following by email

@FiNeX: Thanks for pointing us to that one!

---

I think this should be pluggable/extendable. Minimum password length is just a single, very basic minimum necessity for safe passwords. We (or at least contrib) should be enabled to add further requirements, such as "contains at least one digit", "upper case character" etc. without form_alter'ing. Someone an idea how to implement this best?

@Pancho: on contrib side there is the password policy module but it does't looks stable.

Thanks!

This is a major issue. For example, https://www.zivtech.com/blog/how-prevent-your-drupal-site-getting-hacked...

Let’s start by taking a look at our most common weakest link, the user password. While there is a nice password strength suggestion tool in Drupal core, there’s nothing in core to force users to use secure passwords out of the box. [...] At Zivtech, we use the Password Policy contrib module.

Now, password_policy 8.3.x works with constraints, which is great, but it does have to form_alter, manually validate, define its own plugins etc, which is too much to leave to contrib, given this is highly relevant to security and given that by now, it has obviously been too complex to push it to a stable release. This is not good.

I think we need to include a stripped down version of the password_policy module in Core and push it to the point of being stable and reliable and fully covered by our security team's reviews. At least some minimum password length and good support for adding further password constraints (even if not the actual implementation) IMO is a basic security feature which even in the most basic Drupal installation shouldn't be missing. Individual constraints (such as password_policy's several sub-modules plus password_strength and others) may and probably should remain contrib.

A minimum password length of 1, as currently the case, is outright inacceptable, which is why I even tend to consider this a security-related task, possibly against 8.6.x-dev. But that's a decision I'm leaving to the subsystem manager.

Hope it's not considered too early to call for subsystem manager. But before proceeding, at least need a short feedback would be helpful whether to continue investigating the plan layed out here, or whether a quick stop-gap is preferred.

I agree to include a stripped down version of password policy in core with the minimum security constraints (configurable by the site administrator) like:

- minimum password length
- force to contains a mix of letters, symbols and number (at least one per type)
- password different from email / username

The system should be pluggable to allow contrib module to implement more complex rules.

✅ Minimum password length, yes.

✅ Different from eMail and username, absolutely.

❌ The mix of letters, symbols and number, I'm however absolutely opposed to. This is not just annoying, it is outright insecure, as current research has established, and needs to go from the strength meter, too.

Research using datasets from cracked and leaked password storages such as LinkedIn has shown that in real life, users use symbols and numbers in very predictable ways. Suggesting that a password like "My.Password" or "w1r3l3$$." (= just a leet variant of "wireless") or "Drupal8.0" was secure, is misleading and counterproductive. In fact it's still a weak password. Suggesting a password like "w1r3l3$$." was more secure than, say, "deskbottomkneeobstacle" is even more misleading and counterproductive, effectively mis-educating users to use a weak password.

Some literature, mostly on strength meters, but still applying to strength rules:

• https://blogs.dropbox.com/tech/2012/04/zxcvbn-realistic-password-strengt...
• https://madiba.encs.concordia.ca/~x_decarn/papers/password-meters-ndss20...
• https://nakedsecurity.sophos.com/2015/03/02/why-you-cant-trust-password-...
• https://www.ei.ruhr-uni-bochum.de/media/mobsec/veroeffentlichungen/2018/...
• https://www.youtube.com/watch?v=rx-_RTYSz1g
• https://www.password-meter-comparison.org/static/academicComparison.php

First steps we need to do in D8, to allow enforcing some minimum strength while giving decent advice:

1. Add a configurable minimum length constraint, defaulting to 8 characters (which is still weak, but still is the most common minimum length, therefore is what most admins will be expecting as a default)
2. Disallow any exact match or substring of username or email address.
3. Use zxcvbn for our password strength meter: #1497290: Check for common words in password strength indicators

Next steps to reach an overall decent security level might then be:

1. Use zxcvbn's password strength meter bins to actually enforce a particular, configurable strength.
2. Make constraints pluggable so sitebuilders may add additional constraints.
3. Make dictionaries pluggable so:
   • Spanish websites may penalize most common English and Spanish words
   • Canadian websites may penalize most common English and French words
   • Swiss websites may penalize English, German, French and Italian words.
   • Websites may penalize passwords containing passwords matching sitename, domain and/or other words related to the actual website.

   Word lists being available here: https://en.wiktionary.org/wiki/Wiktionary:Frequency_lists

While zxcvbn can be considered an easily deployable industry standard, our approach would still not be perfect at this point, as advanced Markov models are still better. However stronger approaches are way more complex to deploy, so this might remain a desiderate for Drupal 9.

Drupal has been the first in so many areas of computing, but using passwords is now becoming old school. Probably, a bigger picture approacc should be taken, and use something modern for logging in, eg:
https://developers.facebook.com/docs/accountkit/

Or make social auth the default core https://www.drupal.org/docs/8/distributions/open-social/social-login

If someone wants to go back to old passwordy stuff, that could be a site setting :)

A bit of a history on passwords :) https://www.businessinsider.com/inventor-of-the-password-2014-5?r=US&IR=T

I'm aware of that funny story of password history, and clearly, there are more effective approaches such as two-factor-authentification. However, passwords won't be going away in the next five or ten years, and our current approach is not even satisfactory as a default. Please take your time to read some of the literature I provided in my last post.

Chiming in here as the maintainer of Password Policy. It's definitely stable and it's really on me to move that forward more visibly. The module has had many improvements, tests, and more.

This module basically leveraged a totally separate display on the user form due to the same limitations identified here. It would be really great to have one, extensible feature anyone could leverage. This is good for usability, but simplifies much of the code in the password policy module that provides this as an API.

Based on issues like #1497290 and the lack of core being extensible, I recommend leveraging Password Policy's constraint plugin system and leverage the desired set of "default" plugins like the length. One of the default plugins can be the Password Strength module, which can leverage the desired ZXCVBN library. Tests can be moved into core too.

This would deprecate the Password Strength module entirely and strip down Password Policy into a set of extensible set of submodules enabled and disabled as desired.

I'm happy to work on this if there is buy-in here.

Yay, thanks for chiming in!
I agree that before starting work on this we need some buy-in by subsystem maintainers and/or Core committers.

I recommend leveraging Password Policy's constraint plugin system.

We're probably only adding two or three constraints in Core covering one or two recommended approaches for 80% usecases. And Core needs to keep it simple, introducing as few concepts and vehicles and layers as possible.
So while Password Policy's custom plugins (example here) certainly are the most flexible and encapsulated way of adding constraints together with its settings form and everything, a less optimal, yet less custom approach using regular Constraints plus some extra form settings might be more acceptable in core.
Also, the additional layer of password policy entities (= the sets of multiple, individual constraints) IMO needs to remain in contrib in order to reduce complexity.

Again, we need some feedback from Core committers if the outlined approach is welcome!

Oh lá lá https://venturebeat.com/2019/03/04/w3c-approves-webauthn-as-the-web-stan...
how about WebAuthN? :)

@Pancho great analysis and resources, thanks for reviving this issue!

I agree that implementing these security enhancements as a simple set of constraints using the tools we have, without adding a new abstractions for password policies is probably the simplest path forward. I think even keeping this issue as a narrowly scoped "allow a configurable length" would be preferable, then hopefully additional strengthening efforts are easier based on the lessons learned here.

@moshe weitzman is listed as the subsystem maintainer for the user module, so assigning for input on the proposed enhancement and technical approach.

At this point the earlier patches are almost beyond a reroll, so the relevant parts of the code and probably just be lifted into 8.8.

Thanks for paging me.

I think a configurable min length makes sense. In addition, I'm happy to consider improvements which simplify Password Policy module. Once thats simplified, we can discuss moving more of it to core.

Hello all, I created a simple module Min Password for this.