Passport local temporary password (change on first login)

Question

For security reasons we create users and send them a temporary generated password. At their first login the user should change it's password before continuing navigating protected pages.

I'm using an express/node website running a passport-local module. Registering, login in users all work. But I'm kind of lost on best practices for users to change their password on the first login.

My idea was to do the following :

/* POST login page. */
router.post('/login', function(req, res, next) {
  passport.authenticate('local', { successRedirect: '/dashboard/users',
    failureRedirect: 'pages/login'}, function(err, user, info) {
    if(err) {
      console.log('')
      return res.render('pages/login', {title: 'Login', error: err.message});
    }

    if(!user) {
      return res.render('pages/login', {title: 'Login', error: info.message});
    }
    return req.logIn(user, function(err) {
      if(err) {
        return res.render('pages/login', {title: 'Login', error: err.message});
      } else if (user.firstLogin) {
        return res.redirect('/change-password'); // <- First login
      } else {
        return res.redirect('/dashboard/users');
      }
    });
  })(req, res, next);
}); 

As you can see I have a simple boolean (tinyInt 0-1) set in my database (defaults to 1). Afterwards I'll set a post method which after a successful change, the boolean would be set to 0.

Is this a correct way ('a' not 'the' :p )? How about the security?

Answer 1

It definitely is a correct way. I'd say it perfectly suits your needs. I personally like how the usage of your database field directly reflects the business logic behind it.

Alternatives, though I'm fan of your solution, could be:

1

Adding a lastLogin date field to your database which defaults to NULL. This would store a datetime stamp whenever the user logs on. You could use it as an implicit check if the user has ever logged on before. I personally prefer having explicit columns for their purposes (like you're doing with your firstLogin column) because the purpose of the column and business logic of the application is very clear from it.

2

Another alternative would be to store when a user has last updated his password i.e. lastPasswordChange defaulting to NULL for new users. Same reasoning as above. Could be useful if you'd want your users to change their passwords every n days.

Security speaking I'd say this'd be solid. As long as your firstLogin field defaults to 1 then there'd be no way a new user could skip the password change upon first login.

However, when a user updates his password, be sure to either update the firstLogin field in the same query or perform both queries inside a transaction. This way you'll always be sure both the password and the firstLogin field are changed. If for some reason either query would fail you'd have a user who already changed their password and is asked/forced to change it again or a user who has a randomly generated password without being asked to change it. Doing it inside the same query would make sure both, or neither are updated at the same time. Doing it inside a transaction leaves you with the option to fail/rollback the transaction when either query fails.

On another note, your code could be written like this (both your way and this is correct, it's just a matter of preference and visual):

/* POST login page. */
router.post('/login', function(req, res, next) {
    passport.authenticate('local', { 
        successRedirect: '/dashboard/users',
        failureRedirect: 'pages/login'
    }, function(err, user, info) {
        if(err) {
            console.log('')
            return res.render('pages/login', {title: 'Login', error: err.message});
        }

        if(!user) {
            return res.render('pages/login', {title: 'Login', error: info.message});
        }
        return req.logIn(user, function(err) {
            if(err) {
                return res.render('pages/login', {title: 'Login', error: err.message});
            } 

            // Using if/else if/else makes no difference since if the first if is executed
            // in both cases nothing else will execute due to if/else or the return.
            // In case the above statement resolves to `false` the effect wills till be the same

            if (user.firstLogin) { 
                return res.redirect('/change-password'); // <- First login
            }
            // The else is not necessary due to the return in the line above.
            return res.redirect('/dashboard/users');
        });
    })(req, res, next);
});

If you'd like a more specific answer to either question, I'd need a more specific question.

Answer 2

It's probably a good thing to move the "check first time login" logic into a separate middleware which executes for all "logged in routes".

With your suggestion above the user can simply navigate away from /change-password after redirection?