9

I'm trying to use a authenticateUser() middleware before loading all of my pages. Instead of including it in each call (as in app.get('/', authenticateUser, function()...)), I tried setting it with app.use(authenticateUser) right before calling app.use(app.router).

This didn't work, however. authenticateUser is basically:

if (req.session.loginFailed) {
  next()
else {
    if (req.session.user_id) {
        ... 
        if (userAuthenticated) {
            next();
        } else {
            req.session.loginFailed = true;
            console.log('setting loginFailed to true');
            res.redirect('/login');
        }
     }
}

And then in app.get('/login') I set req.session.loginFailed to be false;

This should work, but I only want to call it on an app.get() or app.post() etc. for one of my actual pages. I think its getting called lots of times for many different requests (because upon loading one page, 'setting loginFailed to true' is called many times)

Is there a better way to do this? Or should I simply be calling it before every page on my site?

Varun Singh
  • 1,676
  • 3
  • 18
  • 25

1 Answers1

12

You are doing way too many checks out there in my opinion. Only one route should handle user login (check for user & pass, and store the username in the session if succeeded) and you should assign the auth middleware only on the routes that require auth (not all).

I've put up a simplified example so you can understand my point:

The login route

app.post('/login', function (req, res) {
  var variables_set = (req.body.user && req.body.pass);
  if (variables_set && (req.body.user === 'username') && (req.body.pass === 'password')) {
    req.session.username = req.body.user;
  } else {
    res.redirect('/login?failed=true'); 
  }
});

The auth middleware

if (!req.session.username) {
  res.redirect('/login');
} else {
  next();
}

You can see a more complete example in action in Alex Young's Nodepad application: https://github.com/alexyoung/nodepad (tutorials for that app here: http://dailyjs.com/tags.html#lmawa )

alessioalex
  • 62,577
  • 16
  • 155
  • 122
  • 1
    Fair enough, its funny because I was actually working off of the nodepad tutorials. The thing is my entire site requires that you're logged in, so I hacked together a way to not have to manually include it on all the routes, but I'll just include it if thats the proper way. – Varun Singh Jan 04 '12 at 18:48
  • I agree with @alessioalex. If you insist on the approach you've laid out, though, ensure that your `app.use(authenticateUser)` is AFTER the session middleware. – danmactough Jan 04 '12 at 14:03