search

ajmueller / express-auth-session

A starter Express app using Passport for authentication and an ACL for authorization.
https://express-auth-session.herokuapp.com/
22 stars 11 forks source link

Password Expiration #1

Open peterramsing opened 8 years ago

peterramsing commented 8 years ago

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!

peterramsing commented 8 years ago

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

ajmueller commented 8 years ago

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

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

peterramsing commented 8 years ago

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:

Additional Scope for Question:

ajmueller commented 8 years ago

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:

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 😉

peterramsing commented 8 years ago

Scope Proposal:

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

ajmueller commented 8 years ago

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

peterramsing commented 8 years ago

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