ajmueller/express-auth-session

Password Expiration

Opened this issue ยท 7 comments

I'd love to help out. Have you thought about password expiration at all? Maybe every 12 months prompting for a new password, etc? If you don't have a timeline I could tackle that. This is something I've been looking into doing on an app I'm working on.

Thanks for this awesome project!

Although that 12 months could easily be a configuration variable now that I'm thinking more on it.

@peterramsing that's a great idea and I love that your mind jumped to environment variables. Some initial thoughts:

  • The variable may be best expressed as a number of days. That will give enough granularity to fulfill most folks' requirements without being too difficult to figure out. Need monthly(ish) password changes? Set it to 30 days. Quarterly? 90. Semi-annually? 180. Annually? 365.
  • We'll need a couple new fields in the user model:
    • passwordChanged - the date when their password was last changed. This will be better to use instead of a passwordExpires date because it will allow us to dynamically calculate the expiration date if the password expiration time changes (which it very well could). This also needs to be optional in case the password expiration time is set to 0 days (in other words, no expiration time).
    • passwordIsExpired - a virtual field to tell us if a user's password has expired. This will simply be a comparison of today's date and passwordChanged + password expiration time.

What do you think? Are there other design details we'd need to consider?

On point 1: I fully agree. Statically setting forward dates can be cumbersome especially for the potential for it to be dynamic. PCI typically says 90 days but I'm thinking that 365 is probably just fine. (or 365.25 if we really wanted ๐Ÿ˜‰)

Potential Pitfalls:

  1. What happens if the config is changed to, say, 1 day by accident. Would that virtual field update when they realized the error and set it to 100 days (as they maybe intended) or would that virtual field update. I'm tempted to say "update" but will have a better grasp when in the code.
  2. While we all want security, should this setting be turned on by default? My gut is saying "leave it off but well documented to encourage the developer to turn it on". It's tough because we want security but it has it's downsides as well. There should at least be an "off" feature, though.

Initial Scope:

  • A feature that is configurable in config.js that would be set to either "no/null/off" or "day range between 30 and 9999 (or it could be infinite?)" that determines when passwords should be reset as a mandatory policy.
  • This feature should be able to be set for different roles so that it could only required for "super users" for example. This would be another configuration in config.js that's name is TBD.
  • This should be dynamic, using "changed" date as the starting point and not a "ending date" so that it can be dynamically changed.
  • The same password should not be used again.

Additional Scope for Question:

  • Should this disable an account or simply prompt a reminder to change password?
    • Should this be configurable or left to the developer to handle?
  • If the password date range is set to 30 days and this causes several accounts to prompt/lockout should setting the date range beyond those several accounts allow the accounts to access to be restored or would they still need to reset their accounts.
  • How far back should we store passwords that were used previously? 2? 3? ...or just the most recent password? (I'm leaning towards the most recent password just to the liability of storing things that are sensitive. First rule of protecting data: don't store what you don't need to.

Thanks for such a well thought out response, this is great. I suppose at this point I should specify that one of my goals with this project is to not provide the kitchen sink for authentication and authorization, but merely a baseline off of which to build. The first sentence of the README echoes this sentiment:

Inspired by Hackathon Starter, this project is a more simplified boilerplate application with some basic examples of user authentication with Passport and authorization via an ACL.

That said, this is a feature that could be pretty easily incorporated at a very basic level to showcase how it may work. Then more advanced features for password expiration will be up to the developer to implement given their specific project requirements. Security is such a funny thing to tackle because you're always trying to hit a moving target and the target looks different based on who your project stakeholders are.

Pitfalls:

  1. I believe virtual fields are only "updated" when they're referenced; they're just essentially calculated fields. So in the case you described whereby the config is set to 1 day, only users who authenticate while the config is set incorrectly will be prompted to change their password. Is that your understanding as well?
  2. I would think that, given today's general authentication practices, this would be turned off. I rarely see web applications today that require you to regularly change your password (unless, perhaps, we're talking about enterprise applications; don't even get me started on "security" of enterprise). Your gut is spot on with how I feel as well.

Scope:

Looking at your above scope items, here's an exhaustive list to discuss:

  • Enable/disable of feature
  • Frequency of change required (# of days)
  • Disabling of account
  • Role-specific enable/disable of feature
  • Role-specific frequency
  • Password re-use allowance
  • Length of password re-use history

The items that I've put in bold are what I'd consider to be those included in an MVP of this feature; the item in italics is one that I consider to be borderline MVP. Enable/disable and frequency could be combined into a single config variable by assuming that a frequency value of 0 means that the feature is disabled and passwords never expire. Maybe we can just include those as a single "feature." Can you think of any ways that an explicit enable/disable of the feature would be advantageous over just using 0 for the frequency?

If we're forcing users to change their passwords at a given frequency, it also makes sense to have the option, like you asked about, of completely locking users out of accessing content until they change their password. This feature would need to be implemented as middleware that checks if a user's password has expired. It could be implemented as part of the isAuthenticated middleware since it really is part of being properly authenticated. We'd need to ensure that our example resources that are protected by the ACL also use isAuthenticated as middleware; I'm currently just relying on the ACL to perform that check since all I need to know if a user is authenticated at all. If users access to data isn't disabled, we could just give them a reminder like you said; use a flash message upon login to remind them that their password has expired.

For the password re-use allowance, I put it in italics because it does make some sense to include in an MVP. Why would we ask users to change their passwords if we allow them to continue to use the same password? However, the length of password re-use history that we address is, I think, a feature that isn't 100% necessary. This is very subjective and a big UX question as well. Personally, I don't particularly care about required password changes or, for web and applications, that I can't re-use my last X passwords. I use a password manager and have done so for a few years, so changing passwords is incredibly simple and they're almost always guaranteed to be unique. However, for the Average Joe and Average Jane, changing passwords is a major pain especially when they can't re-use their last X passwords. I vote for the number of previous passwords that can't be reused to be a feature that we leave up to developers to implement.

Finally, role-specific enable/disable and frequency are a great idea. I think it makes a lot of sense to force users with higher levels of system access to change their passwords more frequently, but this is another feature that I think is beyond the scope of this project. Perhaps this feature could be described in an example Gist rather than fully incorporated into the project?

Conclusion:

Thanks again for such a well thought out response. The discussion is great and it's a fantastic exercise for developers to discuss and think through these kinds of features and issues since they're so important to (nearly) every application that we make. Authentication, authorization, and security in general are a bit of a sticky wicket, especially regarding username/email and password combinations. The entire paradigm of using a username or email address and password to authenticate yourself securely should be disappearing in the next several years; it's frankly useless unless you have a password manager and use randomly generated passwords. It will give way to biometric means of authentication and in some ways it already has. Therefore I see the role of this project as providing a baseline, simple project that give developers a solid and secure basis off of which they can build the additional specific features that their project requires.

Remember, you don't have to be the most secure house on the street, just more secure than your neighbors. And on the web, everyone's our neighbor ๐Ÿ˜‰

Scope Proposal:

  • Enable/disable of feature (done with 0 to mean "disabled")
    • Disabled by default
  • Frequency of change required (# of days)
    • Done with the same configuration of enable/disable
  • Disabling of account if their password expired
    • Should be written to allow for warnings instead of disabling if the developer desires
  • Check the new password against the "current" password hash to ensure that the same password isn't used.

If this is written correctly the following could be considered in another PR, described in a blog post or documentation as "you could add these yourself".

  • Role-specific enable/disable of feature
  • Role-specific frequency
  • Warnings instead of a hard disabling of accounts

Looks good! This can be something we discuss a little more tomorrow morning.

Per our discussion today I'm going to start working on this and see what I can get done over the next couple of months. I'll hold off committing/PR'ing while waiting for the .editorconfig thought process you had to flush out (which I'm excited to hear your thoughts on).