-1

I am creating a e-commerce website where I store all the data in database as well as session store. For logged-in users, only the login credentials are stored in the session store. So, whenever the website is run for the first time, the data should be fetched from database from the data we get from session store but if i use a middleware for this purpose, it is not efficient as the middleware runs on every request. So, is there any way to solve this problem or is there any better solution to this problem which is more efficient? And also, you may wonder why I don't store data in the session store directly. So, the problem is, when I fetch data from session store, it is not returned in the form of a mongoose model so I have to call the database once. I use mongo store, express, node and ejs.

This is the middleware I use in my index file to fetch data from database in the mongoose model using the id stored in my session store during login.

app.use(async (req, res, next) => {
    try {
        if(req.session.userid) {
            req.user = await user.findById(userid)
        }
    } catch(err) {
        res.redirect('/' + req.oldUrl + '?err=UserNotFound')
    }
    next()
})

app.use(session({
secret: 'secret',
saveUninitialized: false,
resave: false,
cookie: {
    maxAge: 1000 * 60 * 60 * 24 * 365 * 100,
    secure: false
},
store: store
}))

This is my mongoose model

const mongoose = require('mongoose')
const Schema = mongoose.Schema

const userschema = new Schema({
    Name : {
        type : String,
        required : true
    },
    Email : {
        type : String,
        required : true
    },
    Password : {
        type : String,
        required : true
    },
    IsSeller : {
        type : Boolean,
        defualt : false
    }, 
    Shopname : {
        Type : String,
        default : "-"
    },
    Cart : {
        Items : [
            {
                productId : {
                    type : Schema.Types.ObjectId,
                    ref : 'product',
                    required : true
                },
                quantity : {
                    type : Number, 
                    required : true
                }
            }
        ]
    },
    Myproducts : {
        items : [
            {
                productId : {
                    type : Schema.Types.ObjectId,
                    ref : 'product',
                    required : true
                },
                quantity : {
                    type : Number, 
                    required : true
                }
            }
        ]
    },
    Sdate : {
        type : String,
        default : "-"
    }
})

module.exports = mongoose.model('user', userschema)
Someone
  • 9
  • 1
  • 7
  • 1
    Please show some code so we can see what's going wrong here. All relevant code needs to be included, you can of course modify it to make it "anonymous". – anatolhiman Aug 26 '21 at 04:07
  • There is not enough programming/code detail in this question to have any idea how to help you or how to fully understand the question. Questions about code need to show the relevant code. – jfriend00 Aug 26 '21 at 04:17
  • If it's inefficient because the middleware is taking a long time, then you need to somehow optimize it (and we can't help without seeing it), but If it's the general existence of the middleware that's worrying you, you have to validate/verify the incoming requests, so you do need them for most of the cases. – grkmk Aug 26 '21 at 04:44
  • @anatolhiman i have added some code. – Someone Aug 26 '21 at 05:57
  • @jfriend00 code added :D – Someone Aug 26 '21 at 05:57
  • @grkmk added some code :D – Someone Aug 26 '21 at 05:58
  • If you are expecting so much traffic that the amount of `req.user = await user.findById(userid)` calls will add considerable latency to your db, I don't think mongo should be your choice (maybe check https://stackoverflow.com/questions/9702643/mysql-vs-mongodb-1000-reads). Might also be a good idea to load test your app/middleware with your simultaneous traffic estimation, and see whether you should be concerned or not.. – grkmk Aug 26 '21 at 08:49

2 Answers2

0

You can use express-sessions directly in one of your controllers if you don't want to use it as a general middleware. If you set a session cookie when you log in and only check it next time the user logs in, make sure the user is logged out when tab or browser is closed, or after a certain time if you want to keep it more flexible and let users bypass the login form for x amount of time.

// in app.js

const session = require('express-sessions')
const MongoStore = require('connect-mongodb-session')(session)

const store = new MongoStore({
  uri: MONGO_URI
  collection: 'sessions'

app.use(session, { 
  secret: 'myL0ngSecretXXX', 
  resave: false, 
  saveUninitialized: false, 
  store 
})

// in your login controller

exports.setSession = (req, res, next) => {
  req.session.isAuthenticated = true //this saves a session cookie client side
  res.redirect('/dashboard')
}

// Router: 

router.post('/login', loginService)


// Login service

const { setSession } = require('./controllers/login')

// other requires ...

exports.loginService = catchAsync(async (req, res, next) => {
  const allowedFields = ['email', 'password']
  if (!checkFieldsStrict(req.body, allowedFields) || (isEmptyRequest(req.body))) {
    return next(new ErrorResponse(`Invalid request.`, 400))
  }
  const { email, password } = req.body
  if (!email || !password) {
    return next(new ErrorResponse(`Please provide your email and password.`, 400))
  }
  const user = await User.findOne({
    where: { email },
  })
  if (!user) {
    // Don't reveal that the user is unknown:
    return next(new ErrorResponse(`Invalid credentials.`, 401))
  }
  const passwordMatch = await comparePasswords(password, user.password)
  if (!passwordMatch) {
    return next(new ErrorResponse(`Invalid credentials.`, 401))
  }
  setSession()
  // maybe send back a token too in addition to the session?
})

anatolhiman
  • 1,762
  • 2
  • 14
  • 23
  • Seems that you didn't understand question well. Let me be more clear. I don't want the users to logout after the tab closes. So, I store the User ID of logged-in users in the session store. When they open the website later, I have to get the details of the user using the User ID stored in the session store. So, if i use a middleware for this purpose, the middleware fetches data from the database using User ID on every request call. So, my question is whether using a middleware for this purpose is efficient or is there a better way to do this? Also, is there any better approach? – Someone Aug 26 '21 at 06:52
  • If you don't use sessions as per their "design", maybe better to use (JWT) tokens instead, saved in a httpOnly cookie? But they require middleware. It's dangerous to save auth status on the frontend in a way that gives access to the API. Cookies and localStorage can be tampered with. Frontend can keep the user "soft logged in" without access to sensitive endpoints (by saving loginStatus in a localStorage/cookie). Some of your endpoints are probably public, so you don't need to lock those. You only protect the endpoints that are private. The way to do that is definitely middleware. – anatolhiman Aug 26 '21 at 07:47
  • You can also consider using an in-memory data store for your sessions if you don't want to add extra load to your database. Redis is perfect for this. – anatolhiman Aug 26 '21 at 07:48
  • Thanks man. i also wanted to ask that in the answer you provided, i didnt understand what is isEmptyRequest and CheckFieldsStrict. are these in built function or you wrote them? and what do they do? and also what is the need for checking whether request object is empty or not? and lastly, if we use catchAsync, how do we handle the error becuase there is no catch block? This would help me out a lot since i am quite new to programming :D – Someone Aug 26 '21 at 13:14
  • The two you mention are functions I wrote. Checking the input you receive from the frontend is necessary, but how thorough you want to be depends on how good feedback to the frontend you want to provide (or to the dev using your API). Fewer checks = more general error messages and errors where nobody knows what went wrong. Checking if a req is empty isn't important, but it's an error so it's okay to provide some kind of error info to whoever made the request. "CheckfieldsStrict" checks that only the allowed fields are included, like only pw and username and nothing more (thus "strict". – anatolhiman Aug 27 '21 at 06:22
  • I also use a "loose" check for optional form fields. Should never happen that frontend sends the wrong data, but if it happens, you will know why the request fails. And it could easily happen if you let developers use your API that they try to do wrong things with it, so make errors scenarios and develop for that with helpful feedback (but beware of security, it's not everything you should detail out in a response, like whether or not a login attempt is wrong because the password is wrong or what. Leave some things a mystery. – anatolhiman Aug 27 '21 at 06:24
0

First off, you can make the middleware a lot more efficient by not rerunning it on every request. Just save the user in the session itself. Presumably, the relationship between userId and user never changes so whenever you first get the userId, just get the user and save it in the session itself too. Then, you're only querying the database once per session.

app.use(async (req, res, next) => {
    try {
        if(req.session.userid && !req.session.user) {
            req.session.user = await user.findById(userid)
        }
    } catch(err) {
        res.redirect('/' + req.oldUrl + '?err=UserNotFound')
        return;
    }
    next()
});

If you absolutely need the data in req.user instead of req.session.user, you can have the middleware set that too:

app.use(async (req, res, next) => {
    try {
        if(req.session.userid && !req.session.user) {
            req.session.user = await user.findById(userid)
        }
    } catch(err) {
        res.redirect('/' + req.oldUrl + '?err=UserNotFound')
        return;
    }
    // populate req.user if user is available
    if (req.session.user) {
        req.user = req.session.user;
    }
    next()
});

If some data within the user object might change over time and you want a fresh piece of that specific information, then you can put a DB request to get the absolute latest user object in those specific routes. That way you are only hitting the database when the user initially signs in and when a route absolutely needs the latest info from the database, rather than on every single request.

Oh, one other thing. After you call res.redirect(), you need to return from the middleware and not call next() because you're done processing that request and don't want further route handling on that request.

jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • If I save my user object in req.session,user only once during the start of the session, it works. But next time when the user opens the website, the req.session,user still exists but the problem here is that req.session,user is not in the form of a mongoose model because session store does not know what mongoose is. So, can something be done here like removing req.session.user on browser closing or checking whether req.session.user is a mongoose model or not? i have no idea whether these actions can be performed or not. – Someone Aug 27 '21 at 04:43