Pastor Bones
Pastor Bones

Reputation: 7361

Express authentication - cannot send headers after sent

I have created a function that restricts route access by verifying that a stored session user/pass matches what is in the database

var checkAuth = function(req, res, next){
  if(typeof(req.session.user) === 'undefined') {
    req.session.user = { name: '', pass: '', loggedIn: false }
  }
  $R.user.validateLogin(req.session.user, function(err){
    if(err) res.redirect('/login')
    else {
      req.session.user.loggedIn = true
      next()
    }
  })
}

app.get('/restricted', checkAuth, function(req, response){
  response.render('index')
})

It seems to work fine as it will redirect to the /login page if a person is not athenticated, but immediately after redirecting the app shuts down with the error

Error: Can't set headers after they are sent.

I have traced the error down to the res.redirect('/login') but can't figure out how to remedy my error.

EDIT: My login route handler

app.get('/login', function(req, response){
  $R.page.addStyles(['forms','user/user'])
  response.render('user/login')
})
app.post('/login', function(req, response){
  $R.user.validateLogin(req.body, function(err, res){
    if(err) response.end(JSON.stringify({error: err.message}))
    else {
      req.session.user = req.body
      response.end(JSON.stringify({ok: true}))
    }
  })
})

Upvotes: 0

Views: 2109

Answers (1)

hross
hross

Reputation: 3671

Your problem is that the function:

$R.user.validateLogin(req.session.user, function(err){

is aysnchronous. The checkAuth function should return a true/false immediately, or redirect. The current flow of your login is like this:

  1. app.get('/restricted') fires
  2. checkAuth fires
  3. $R.user.validateLogin fires (asynchronously)
  4. At this point, checkAuth returns control to app.get('/restricted')
  5. response.render('index') executes
  6. Code inside of $R.user.validateLogin executes, calling the redirect.

The problem is you don't control whether 5 or 6 executes first. Ultimately, both will execute because you aren't stopping #5 from happening.

To fix this, your checkAuth function needs to return and/or redirect without using a callback inside (or executing a callback synchronously). Since you are already validating user logins in your 'login' route, you should be able to check the user session and return or do the redirect synchronously, like this:

var checkAuth = function(req, res, next){
  if(typeof(req.session.user) === 'undefined') {
    req.session.user = { name: '', pass: '', loggedIn: false }
  }

  if (!req.session.user.loggedIn) {
    // req.session.user.loggedIn = true should be set in the 'login' route, in $R.user.validateLogin
    res.redirect('/login');
  } else {
    // if we already have a req.session.user and they are logged in, keep going
    next();
  }
}

Apologies for any syntax errors, I didn't test the above code.

Upvotes: 1

Related Questions