3
\$\begingroup\$

I built a basic authentication system for a node application and I have some security concerns. The username and password the user enters when they log in are stored as plaintext using express-sessions. Then whenever they make a request I call a function to make sure it matches the hashed password stored in the database. My concern is that storing passwords as plaintext using express-sessions is a vulnerability. Anyone familiar with express-sessions let me know if there is something I should do when configuring it to make sure the data is secure (or some alternative if this method for authentication is not viable). Also looking for advice concerning efficiency and obvious bad practices.

Login Router:

router.get('/',(req,res)=>{
    authenticate(req.session.username,req.session.password,(result)=>{
        if (result){//redirects to homepage if logged in
            res.redirect('/')
        }
        else{
            res.render('login')
        }
    })
})
router.post('/',(req,res)=>{
    let db=dbGetter.getDb()
    db.collection('users').findOne({username:req.body.username},async (err,result)=>{
        //queries database for user matching entered username
        if (err) {
            console.error(err);
          } else {
            if (result) {
                if( await bcrypt.compare(req.body.password,result.password)){//compares entered password to db
                    //stores username and password as plain text in session
                    req.session.username=req.body.username
                    req.session.password=req.body.password
                    res.redirect('/')
                }
                else{
                    console.log('wrong password')
                }
            } else {
                console.log("user does not exist");
            }
          }
    })
})

Register Router:

router.get('/', (req,res)=>{
    authenticate(req.session.username,req.session.password,(result)=>{
        if (result){//redirects to homepage if logged in
            res.redirect('/')
        }
        else{
            res.render('register')
        }
    })
})
router.post('/',async(req,res)=>{
    try{

        const hashedPassword=await bcrypt.hash(req.body.password,10)
        const user= new User({
            username:req.body.username,
            password:hashedPassword
        })
        user.save((err)=>{
            if(err){
                console.log('error saving user to db')
            }
            else{
                res.redirect('/login')
            }
        })
    }
    catch(err){
        console.error(err)
        res.redirect('/register')
    }

})

Homepage Router:

router.get('/',(req,res)=>{
    authenticate(req.session.username,req.session.password,(result)=>{
        if (result){
            res.render('index',{username: req.session.username})
        }
        else{//redirects to login page if not logged in
            res.redirect('/login')
        }
    })
})

Authenticate function:

function authenticate(username,password,cb){
    let db=dbGetter.getDb()
    db.collection('users').findOne({username:username},async (err,result)=>{
        if (err) {
            console.error(err);
          } else {
            if (result) {
                if( await bcrypt.compare(password,result.password)){
                    cb (true)
                }
                else{
                    cb(false)
                }
            }else{
                cb(false)
            }

          }
    })
}
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Short answer: yes, don't do it.

Don't put password into session. On one hand, the security of it depends on the used session store. On other hand, you don't need the password in session or anywhere else.

In fact, don't put username and especially email into session either.

Put there the primary key. Only verify password in login process. As long as user id is in session it is logged user and you can get it from db by id.

However, this way after changing password, previously logged in sessions remain logged in. You should deploy a separate mechanism to prevent that. Like putting a separate hash into session and change it along with password.

But this is still quite insecure, because the authentication relies solely on a cookie. You should use a csrf token to prevent xss attacks.

But that still has issues and it would be best to use Authorization header and access tokens instead of cookie based auth.

Security is a very complex topic...

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.