Port to Drupal 8

If you or anyone can make an initial port, I would be glad to create a new branch and commit it.

7.x-2.x should probably be used as a basis or at least a reference, and not so much 7.x-1.x, since 7.x-2.x adds several important features that 7.x-1.x was lacking, and seems to have an improved design.

I am not sure what approach is typically taken for porting modules to new major versions of Drupal. We could duplicate the existing 7.x-2.x code into an initial 8.x-1.x branch, but someone would probably still need to make an initial, focused effort to develop something functional. So that may be no better than waiting for an initial patch before branching.

For what it's worth, my present interest is in eliminating bugs in the existing releases, and I am not planning any significant effort on a D8 port soon. I could be wrong but I do not think any of the other maintainers are either.

I suppose we should at least create an empty 8.x-1.x branch so we can file this issue under 8.x-1.x-dev.

We can definitely start a 8.x-1.x branch and get development started. Because the plugin structure in D8 is quite new (and 7.x-2.x was never really "finished"), we might as well start fresh in a true D8-style with a 8.x-3.x branch.

I can start working on this.

I have ported over the majority of the functionality to D8.

Please see: https://github.com/nerdstein/password_policy

I would love to have someone review this code.

This is built on D8's plugin system. As such, I have created two modules which implement the plugins.

Password Length: (submodule of the password_policy repo above)

Zxcvbn: https://github.com/nerdstein/password_policy_zxcvbn

The only constraint that is not plugin-based is Password Reset. This is baked into the password_policy repo, since it does not actually try to enforce specific password characteristics.

This is great work. I am happy to try it out. As a test use case, I will try to implement the password policy of an enterprise in which I have deployed Drupal. I think it is representative of a typical password policy.

• Expiration: 180 days
• Minimum length: 8 characters
• Character composition: at least 2 letters and at least 2 numbers
• Password matches username: disallowed
• History: cannot match previous 10 passwords

Of course I wouldn't expect that this initial port can do all of this, but I will try to get as far as I can.

---

OK, I didn't get very far. Installed Drupal 8 beta 4, password_policy, password_policy_length, password_policy_zxcvbn. When following the link "Add a new policy for this constraint" for any constraint, a PHP error occurs:
PHP Fatal error: Call to undefined function Drupal\\password_policy_zxcvbn\\Form\\current_path() in /var/www/drupal8/modules/password_policy_zxcvbn/src/Form/ZxcvbnPolicyForm.php on line 28

Do you know what might be causing this? I have not used Drupal 8 on this system in a few months and it is possible something is misconfigured or I am doing something dumb.

I'll get that fixed shortly. I just wrote all of the tests and they passed. I'll see what is going on.

Please download the latest version and try again. current_path was deprecated between beta-2 and beta-4

Thanks, it's working now.
---
One thing I am finding initially confusing is this version changes the meaning of "policy". Here, a policy is a configured constraint. In 6.x and 7.x, a policy is a set of configured constraints. The 6.x/7.x usage is more conventional, I think. You might ask someone "What is your organization's password policy?" and they would probably respond with a list of restrictions on passwords like I listed in #7. However, it would seem strange (to me at least) to ask "What are all your organization's policies for passwords?" Wikipedia also defines password policy in the set sense: "A password policy is a set of rules..."

It might be clearer, IMHO, to avoid overloading the term "policy", and instead have the links say "Define a new constraint" or something. "constraint" currently is more like a constraint class or template, and the "policy" you create is the actual constraint on the password.

Anyway, this is mainly a semantics issue, not a functional one. (Although the lack of a collection-of-constraints concept as in 6.x/7.x may have some functional implications.)
---
Having constraints as modules is great. If I wanted to restrict passwords to those with >=2 letters and >=2 digits, I should create new constraint modules for this? The Zxcvbn module is really cool and probably more powerful than using dumb constraints like # of letters and # of digits, but in my usage it's often important to comply with the exact organizational password policy, so I need the dumb constraints.

Thanks a lot for developing this port, by the way. This is something we would simply not be able to upgrade our sites to D8 without.

I don't know if you are ready to commit it to the drupal.org repository yet? If so, it should be 8.x-3.x, right, since it is a rewrite versus 1.x and 2.x in D6 and D7? Probably this is something that a real maintainer like erikwebb or deekayen would know better how to do. I have just been committing bug fixes.

In re: policies vs constraints. I agree with what you said. I struggled with this because I thought I could actually simplify the design of this some and still achieve near functional equivalence. I wanted to leverage Drupal's RBAC permission system, which applies other system-wide constraints. If you have specific recommendations for this to be more intuitive - please share them and I'll implement it. The same with any obvious functional gaps, please.

--

In re: modules. You are correct that each different type of restriction is it's own module. I'm happy to build out additional modules, but I want to ensure the foundation is sound so I don't create a ton of technical debt to maintain. I would love feedback from others, including erikwebb, anavarre, and deekayen. And, I am ready to move this into d.o whenever others believe it is ready. If others would like me to maintain this over time, I would need to gain some level of co-maintainership.

I tend to think Zxcvbn is a very clean way to apply a generic password policy. It's just not implementing specific policies, but a generalization of many policies that yield a strength-based score for a password. It's rather slick.

--

Here is what I will work on next:
1. Specific character counts (numbers, letters including upper and lower, special characters). I'll likely include this as a submodule, because of how common this is.
2. Password history. I'll create a third contrib module.

I'll await specific feedback on the terminology/user experience and functional gaps.

I wanted to leverage Drupal's RBAC permission system, which applies other system-wide constraints. If you have specific recommendations for this to be more intuitive - please share them

Suppose a policy were instead a set of configured constraints with a name. Example:

Policy "normal":
Character length: 6
Expiration: 365 days
Zxcvbn score: 2

Policy "strict":
Character length: 12
Expiration: 180 days
Zxcvbn score: 4

Permissions:
Apply password policy "normal" ("anonymous user" and "authenticated user" roles checked)
Apply password policy "strict" ("administrator" role checked)

This would be another possible way to integrate with the Drupal RBAC system. However, it seems like it would be a significant design change.

It would be a big design change but if there is enough agreement on that then I'm willing to adopt it. I think the concept of the policy that you raise is just an additional wrapper around constraints. I am not convinced its completely necessary. In fact, it seems to require additional steps.

I'm not trying to be difficult here but I would like to understand the benefits. On the surface, my approach seems cleaner.

It would be a big design change but if there is enough agreement on that then I'm willing to adopt it. I think the concept of the policy that you raise is just an additional wrapper around constraints. I am not convinced its completely necessary. In fact, it seems to require additional steps.

I'm not trying to be difficult here but I would like to understand the benefits.

What does that mean, practically? Only changing the verbiage would be okay, or are there architectural changes at play, too? Any idea how the lack of a collection-of-constraints concept as in 6.x/7.x may have some functional implications?

Good questions. Perhaps there are two distinct issues to be considered:
1. The change in meaning of the term "policy" from what is used in prior Password Policy versions and from what is conventionally meant by a "password policy".
2. The lack of any collection-of-constraints concept.

#1 could possibly be addressed by just a change in verbiage. Adding a collection-of-constraints concept to address #2 would be a design change.

I can't think of any major drawbacks to not having collections of constraints (#2). Having them could increase ease of use in some ways. For instance, say you are setting password requirements for role "foo". In nerdstein's branch, you would configure each of the constraints, and then go to the permissions page and individually set them to apply to role "foo", picking them out of a potentially long list of other permissions. In 1.x, you need only configure the constraints within a policy, then set the policy once to apply to role "foo".

#1 is the main concern to me. I am wondering if a possibility would be to just drop the new, conflicting usage of "policy", and speak only of constraints.

In any case, if there is no need for groupings of constraints, then it is just a terminological issue and it is perhaps something not important to resolve up front as it could probably be changed without much rework later.

Company policies are what's best until something better is found. I'm not saying zxcvbn is the holy grail (especially since the issue queue is lacking) but it seems calculating password entropy is better than any company policy. See also https://www.drupal.org/node/2231941 FYI.

Hope it was clear that I am not suggesting replacing the Zxcvbn constraint with the typical password constraints. It seems like a great feature. I am just suggesting that the module have typical constraints built-in too. Using Drupal in some organizations may require complying with the organizations' password policies, which can be like "password must be X characters long" and "password must have Y digits".

I think the problem is inherent to choosing a password in first place whereas people should be using LastPass, 1Password, KeePassX or whatever other tool to generate long, complex passwords, but also - and more importantly - get a high level of entropy.

Yep, I agree. I don't think people should be choosing passwords either and would like to encourage the use of password managers. Constraints that could result in users choosing passwords still have worth though:
- The passwords produced can be sufficiently resistant to online attacks, even if not resistant to offline attacks (e.g. GPU crackers). Having password hashes exfiltrated somehow and attacked offline is a different threat than someone guessing passwords via a login form.
- It may not be practical for sites to effectively force their users to use a password manager. Depending on the site, it may be a lot to demand that your users change their entire way of handling passwords just to access your site.

My view is this module ought to be progressive and promote good password practices like you described, but also:
- Empower administrators to improve password security of their sites in whatever way makes sense in their settings. (This may sometimes be the typical "have this many X" constraints.)
- Support compliance with common organizational password policies, which may otherwise be a barrier to using Drupal at all in organizations.* (There may even be regulatory password requirements in some settings, not sure. HIPAA in the US?)

* Not sure whether I am overgeneralizing from my own experience here. I have a Drupal deployment where it was essential to comply with the organizational password policy to use Drupal at all. I would be interested to know if others have this use case.

If others would like me to maintain this over time, I would need to gain some level of co-maintainership.

deekayen has added you.

I think this dialog as been incredibly helpful for me. I was really busy this week, so sorry for being late to the game.

I am actually leaning toward the collection of constraints idea because this fits the more conventional notion of a password policy- which is a set of constraints. The permissions page would be shorter and conceptially seems like a better fit.

If we were to implement this, now would be the time to do it. I'd like to focus on making this change before adding additional types of constraints.

Once this is complete, I'll try to get the repo ready to port over to an 8.x-3.0-alpha release.

Does this suit both of you? If so, I have what I need to proceed and I hope to work on this soon.

Re #22:

Sounds good to me.

Maybe something else to consider for the design is that people will eventually want the ability to export/import configuration. (I haven't looked--maybe your 3.x branch already has some support for it.) Ctools import/export was a much requested feature for 1.x (never implemented).

This would be extremely complex to do because each "constraint" may have different parameters. The YAML file, in turn, would need to be defined in each constraint's submodule.

It's certainly worth a check, but on the surface, this is by no means easily to throw in. But, it may be the best design decision and worth exploring now, as opposed to later

I don't view it as an essential feature, for what it's worth. I just mentioned it as something that people will want eventually. (There has been a lot of interest, judging from #985758: Implement Features hooks to export and import configurations.) In addition to making it easy to transfer configuration between development and production, it makes it possible to version control the configuration. Those who need the feature could develop it later.

I think this is a huge value-add. If this puts the burden on the module developers, but adds value for the entire community - I say we go for it. We just need to be extremely clear on the conventions that module developers should follow when working within the context.

Let me research ways to alleviate this. There is the obvious concerns around relationships between a policy (sponsored by the password_policy module) and constraints (sponsored by modules that extend the password_policy plugins). We may be able to implement some kind of exportable interface from each module to be able to export from the policy itself.

I apologize for the delays on my end. I am really busy with project work right now. I have not forgotten about this and will circle back as soon as I am able

nerdstein, you might be interested in this D7 issue: #2451159: Password policy doesn't work when updating the user. I am not sure whether D8 would have the same problem. A D7 core change might be needed to fix it, so I thought it might be good to bring up while D8 is still in beta.

To summarize, in some circumstances passwords in D7 get updated by calls to user_save(), but there is no hook that allows Password Policy to see the password before it gets hashed.

Thanks for bringing that to my attention. I am close to wrapping up my project that has pulled my time. I expect to take a look at that soon.

I have the policy concept pretty much wrapped up. My simpletests are busted, but I will get those prepped soon. Feel free to take a look.

I will be tackling the YAML export next.

Just tried out the latest from https://github.com/nerdstein/password_policy.git. The UI looks good. I tried creating a couple constraints then adding a policy. A message says "Password policy foo has been saved", but then I still see "No policies configured" in the Policies fieldset. I do not see a new permission for the policy either. Maybe I am doing something wrong.

I am running Drupal 8.0.0 beta 9 with a minimal installation. password_policy and password_policy_length modules are enabled. Initially when I tried to create a policy I got a PHP error that dpm() was undefined. I think that is from the devel module. So I commented out the call and tried again, to no avail.

Edit: I am not sure from your previous comment whether this was actually supposed to be functional yet.

I have actually had to gut a lot of that to make the entities exportable. I pushed a bunch of WIPs to get code up for some assistance.

This commit is the last stable one: https://github.com/nerdstein/password_policy/commit/a8205708f21c158bcfdb...

At this point, it's best to hold off until I get the config entities in place for policies and constraints. I am still working on it, but will post back soon.

Thanks to EclipseGC and timplunkett, I have exportable entities ready to go with the plugin system. Please note - this is a significant design change but one that has simplified a lot of the module. You can see how significant of a change this was by reviewing the PR: https://github.com/nerdstein/password_policy/pull/20

This implementation now has the following design:

1. A policy with constraints
2. Fully exportable
3. Constraints are plugins

I will work on the SimpleTests, implement additional constraints, and clean up Zxcvbn based on the updated design.

It should be ready to pull down a fresh D8 instance and test once again.

https://github.com/nerdstein/password_policy/

Zxcvbn has been updated with the changes. https://github.com/nerdstein/password_policy_zxcvbn

Just tried to install the latest from GitHub on a fresh Drupal 8.0.0-beta10 site. After enabling Ctools and trying to enable Password Policy:

Unable to install Password Policy, field.field.user.user.field_last_password_reset, field.storage.user.field_last_password_reset have unmet dependencies. 

I have not looked further into it yet. Maybe I am on the wrong commit again.

Looks like I just needed datetime. (Shouldn't Drupal tell you which dependency is needed? I had to look at the .yml file.)

Error after installing:

PHP Fatal error:  Call to a member function getValue() on a non-object in /var/www/drupal-8.0.0-beta10/modules/password_policy/src/EventSubscriber/PasswordPolicyEventSubscriber.php on line 32

Got past the error in #39 by removing get(0) from the indicated line. That's probably wrong. Now I am able to add a policy with a length constraint, and it seems to work. Looks great.

What needs to be done before putting the module on drupal.org? Even if incomplete it will probably get a lot of contributions/feedback.

I am doing some final testing on a last major push. I believe most of those issues are fixed, but I'll verify. I should have this update done tonight

Take a look now, please. I have installed and reinstalled many times. There is a known issue where the password reset fields do not get deleted upon uninstall. I am working through that.

In terms of current state, we have the following constraint types:
1. Password length (sub module) - greater than / less than character count constraints
2. Character type (sub module) - number of numeric, special chars, upper or lower characters
3. Zxcvbn (https://github.com/nerdstein/password_policy) - a scoring algorithm, constraints by score
4. History (https://github.com/nerdstein/password_policy_history) - tracking use of historical passwords. Please note - this one is not functional yet

What should I work on next?

I think we should consider releasing password policy soon for broader feedback. I would also like to release Zxcvbn. History needs to stagger due to a fundamental issue around how D8 is hashing passwords.

In terms of current state, we have the following constraint types:
1. Password length (sub module) - greater than / less than character count constraints
2. Character type (sub module) - number of numeric, special chars, upper or lower characters
3. Zxcvbn (https://github.com/nerdstein/password_policy) - a scoring algorithm, constraints by score
4. History (https://github.com/nerdstein/password_policy_history) - tracking use of historical passwords.

Awesome. Thanks a lot for developing this!

I think we should consider releasing password policy soon for broader feedback.

Sounds good to me. I assume an 8.x-3.x "alpha" release per the release naming conventions? (Or maybe even "unstable"? Not sure which is more applicable.)

The project page should probably be updated. Please feel free to edit it as you see fit. We should probably de-emphasize or move the 7.x-2.x explanation.

I would also like to release Zxcvbn. History needs to stagger due to a fundamental issue around how D8 is hashing passwords.

As separate drupal.org projects or submodules? I don't really have a preference, just curious. On one side, I think it would be nice to have a "smarter" constraint like Zxcvbn built in. On the other, maybe there is an advantage to keeping it separate in case some better Zxcvbn comes about. Then you don't have to worry about maintaining obsolete constraints in the base project, or phasing them out.

What should I work on next?

I can't think of anything that should be release blocking. But if you're looking for a gap to fill, I think one thing people (including me) will want is expiration warning e-mails. (These are the e-mails such as "John Doe, your password will expire in X days. Please log in and change it".) The existing branches have this feature with configurable intervals and tokens support. Of course this could just as well be left to implement by someone who needs it.

I've tried to use judgement to keep common constraint types bundled with the PP module as separate submodules. This helps with performance, especially for unneeded constraint types.

I strongly recommend Zxcvbn, but it is basically a port of the Password Strength module. And, I thought history was a fairly specialized use-case. So, I made a separate module for that as well.

I will post an alpha release under the 8.x-3.x branch. I still want to fix the field association issue when uninstalling the module. I will also evaluate the documentation. What is the best means of collecting feedback or notifying the community of this alpha release?

What is the best means of collecting feedback or notifying the community of this alpha release?

You could post an announcement on the project page. I don't know what the best approach is though. People will surely submit issues.

Just tried installing latest GitHub master on a fresh 8.0.0-beta10 site. After enabling Password Policy and Password Policy Character Type, the following error again appears:

Unable to install Password Policy, field.field.user.user.field_last_password_reset, field.storage.user.field_last_password_reset have unmet dependencies. 

Got past this before by installing Datetime. But I figured out Datetime was missing by looking at .yml files, and I'm guessing an administrator should not have to do that.

Maybe datetime just needs to be listed as a dependency in the .info.yml? I am wondering why a field itself needs a dependency. Is that so the dependency is enforced for other code (outside the module) that might use the field? Not familiar yet with Drupal 8.

1. Dependency of datetime has been added to password policy
2. 8.x-3.x branch created and pushed to d.o. I do not believe have the ability to cut a release. Do I have permissions?

Also, I am not able to update the project homepage. Maybe it's a permissions issue.

8.x-3.x branch created and pushed to d.o.

woohoo

I do not believe have the ability to cut a release. Do I have permissions?

You now have all permissions. (I thought you did already.)

It appears to be working now. Please take a look at the content updates made to the project page. I will get some real documentation added soon.

I de-emphasized and moved the 7.x-2.x paragraph so attention is drawn to the 8.x-3.x announcement.

Closing this issue, feedback will be collected using the standard issue queue process

Using Beta 11 fresh install, I also got the same error as #39 during installation of the module and after removing get(0) as mentioned in #40, I was able to install the module.

However, clicking on 'Add Policy' gives the following errors:

• Notice: Undefined index: value in Drupal\password_policy\EventSubscriber\PasswordPolicyEventSubscriber->checkForUserPasswordExpiration() (line 33 of /var/www/modules/password_policy/src/EventSubscriber/PasswordPolicyEventSubscriber.php).
• Drupal\user\TempStoreException: Couldn't acquire lock to update item <em class="placeholder">> in class="placeholder">user.shared_tempstore.password_policy.password_policy</em> temporary storage. in Drupal\user\SharedTempStore->set() (line 199/var/www/core/modules/user/src/SharedTempStore.php.

Thanks for letting me know. I'll research this and push a fix as soon as I can.

I am going to be pushing my next release soon. @eclipsegc addressed the ctools issue, found here: https://www.drupal.org/node/2543178

It will include the issues noted above fixed (and others I have found with the recent beta release) and a fresh set of automated tests.

I'm targeting early/mid next week.

That's great! Will be happy to test once it's available.

We're close on this. A lot of stability fixes in place that should help tremendously. We're finishing up the automated tests now and making sure it runs clean. I should have another dev release up very soon.

Brief update - we're really struggling to get over the hurdle of the automated tests due to the modal window of the password policy management interface. We have also rolled in some updates for beta 15, as I knew it would be close on the heels of a release here.

Our work continues over in Github: https://github.com/d8-contrib-modules/password_policy and I am going to be reaching out to one of my peers today to get some feedback on the automated tests.

I have created the first alpha D8 release - found here: https://www.drupal.org/node/2581275

Any and all feedback welcomed. It's not fully passing the automated tests yet, but all manual tests are working and I think it's worth getting feedback.