Check for password strength at login

+1 on this

I hate the idea that people will get their initial 4 digit pin and never be forced to change it when we migrate to a later 2.x release.

I agree this is a bug. Whether or not it needs to be fixed in the 2.2 series is open to discussion, so I've left the Importance there as Undecided.

I'm working on this currently. Plan is to check password against the password regex on login to TPAC, direct the patron to a "we recommend that you set a new password" interface.

A related bug https://bugs.launchpad.net/evergreen/+bug/1486151 that spawned from discussion on how to fix this. Sharing here so it doesn't get lost in the shuffle.

Branch containing a variant of what we've been using in production for a few years now: user/jeff/lp1013786_tpac_initial_password

Adding pullrequest, for anyone interested enough to accept the challenge. :-)

Based on the discussion at http://irc.evergreen-ils.org/evergreen/2015-12-15#i_219933 , I'm adding a needstest tag to this bug. There was a bit of uncertainty as to whether the guidelines for regression tests from http://wiki.evergreen-ils.org/doku.php?id=dev:contributing:qa applied to new features, but we can have that discussion here if we think they shouldn't be required for this code.

Thanks Jeff!

I have tested this code and consent to signing off on it with my name, Erica Rohlfs and email address, <email address hidden>.

I have tested this code and consent to signing off on it with my name, Christine Burns and email address, <email address hidden>.

Kathy asked me to comment on the use of needstest tag here. An automated regression test would be a good thing to have -- and I suggest that Jeff or somebody else consider a follow-up to move the password strength check via regex to a self-contained routine (say in OpenILS::WWW::EGCatLoader::Util) that would be reachable to a direct Test::More test.

However, given the age of the patch and the user testing that has taken place so far, for this one it would be sufficient to just meet the new requirement that a written test plan be supplied. This could be done as a new commit message or just via a comment in this bug by one of the testers or by Jeff.

I took a look at this branch today because we were reviewing open tickets in our internal ticket system and we have one on this issue from long ago.

I could probably do what Galen suggest in comment #10 to move the check to a self-contained routine in EGCatLoader::Util and add a test.

I wonder if there should be a setting to disable this feature or not? Currently, a default is used if the password regex setting is not set. Maybe, it should only function if the setting is set as a way to disable this functionality. Alternately, YAOUS could be added to turn the checking off.

Some of our staff seem happy that this feature disappeared when we left JSPAC, despite that someone saw fit to open a ticket about it going away.

> I could probably do what Galen suggest in comment #10 to move the check
> to a self-contained routine in EGCatLoader::Util and add a test.

Thanks!

> I wonder if there should be a setting to disable this feature or not?
> Currently, a default is used if the password regex setting is not set.
> Maybe, it should only function if the setting is set as a way to disable
> this functionality. Alternately, YAOUS could be added to turn the
> checking off.

I'm neutral on whether or not it should be possible to disable the
feature (and to be clear, I mean the feature of "nagging upon login if
the password is weak per the regex), but if you write code to do it, I
suggest making a new YAOUS.

Disabling could be a matter of "set the regex to wide open" but then you don't have password rules...but that is what I think is basically happening if you don't tell people to change their weak passwords in the first place.

I am getting a merge conflict OpenILS/WWW/EGCatLoader.pm when merging against master today. I took a look at the details and decided I shouldn't attempt to resolve it. I won't be able to get this setup for bug squashing day.

I'm just adding a note that we already have two signoffs on this code. Once the merge conflict is resolved, it probably should be tested on, but the code was waiting on a couple of things:

- Jason had said he was going to move the check to a self-contained routine as had previously been suggested by Galen. Given that the comment was made a year ago, I'm guessing Jason is no longer planning to do so.

- Galen had asked that somebody (Jeff? Previous tester?) write a test plan in either the bug report or the commit message. I would suggest that it be done by whomever resolves the merge conflicts.

Until that last issue is resolved, I'm going to remove the pullrequet tag. This is a bug/feature that gets a lot of attention on Bug Squashing Day because a lot of people want to see it make its way into the code, but we just need a few small things done to it before it makes its way in.

Hmm.. Yeah. I don't even remember making that comment, now.

I suppose I could take another look during bug squashing week.

I'll assign myself with that in mind.

I never got around to looking at this during bug squashing week. I'm removing myself as assigned because I'm not sure when I will get the chance to look at this.

This one was signed off two years ago, but there was still some discussion going on - anyone want to take a crack and reapplying it to current master and seeing if it works?

Nine years later, I wish I could board a TARDIS to tell younger me not to worry so much about the regression test or test plan on this one. Is there any hope of resurrecting this code?

We have a new feature that is slated to be included in 3.13. https://bugs.launchpad.net/evergreen/+bug/1979570

It may not quite make it in time though. I thought I'd plug it here, because this feature will flesh out a full password policy infrastructure, including an admin interface and permissions to deal with password policies. The branch is here for now:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/blake/password_policy_3_11

Ooh, thanks Blake! I'll try to take a close look at it, preferably before 3:30 p.m. today, to see if it meets the requirement of this bug report. At first glance, it looks pretty nifty.