Authentication using express-session

Question

I'm making an online shopping store or e-commerce and I use an express session for many things.

Example: Shopping Cart, Authentication user*

I store the data chart list in the database based on the sessionID so when a user visits the website, the user does not need to log in for add to cart.

As well as login, to check the user has logged in or not, I have checked the sessionID that is in the user and checked it on the database whether or not the sessionID is in the database.

When logging in with Google or Facebook, I use the passport for authentication and when I am successful I store the sessionID to the database.

I am using React SPA, Express.js and MySQL.

My database

Table Session

Table Session Browser

Table User

Table User Session

Table Cart

Is what I'm doing correct? And what about the passport? Because as far as I know the passport uses the Headers Authorization token.

Example add to cart:

export const addToCart = (req,res) =>{
        let ip_address = req.headers['x-forwarded-for'] || req.connection.remoteAddress;
    let ua = UAparser(req.headers['user-agent']);
    if (ip_address.substr(0, 7) == "::ffff:") {
        ip_address = ip_address.substr(7)
    }
    let querySelectSession = `
    SELECT ss.id,ss.ip_address from session as ss
    LEFT JOIN session_browser as sb on ss.id = sb.session_id
    LEFT JOIN session_os as so on ss.id = sb.session_id
    LEFT JOIN session_engine as se on ss.id = se.session_id
    LEFT JOIN session_device as sd on ss.id = se.session_id
    where ss.ip_address = '${ip_address}' and ss.id = '${req.sessionID}' and sb.name = '${ua.browser.name}' and sb.version = '${ua.browser.version}' and so.name = '${ua.os.name}' and so.version = '${ua.os.version}' and se.name = '${ua.engine.name}' and se.version = '${ua.engine.version}' 
    ${ua.device.model ? ` and sd.model='${ua.device.model}'` : ''} 
    ${ua.device.type ? ` and sd.type='${ua.device.type}'` : ''} 
    ${ua.device.vendor ? ` and sd.vendor='${ua.device.vendor}'` : ''}
    group by ss.id,ss.ip_address`;

    let queryCheckCart = `select crt.id from carts as crt where crt.session_id = '${req.sessionID}'`;
    let queryCheckCartList = `select ci.product_id,ci.product_variant_id,ci.product_attribute_id from cart_items as ci where cart_id = (select crt.id from carts as crt where crt.session_id = '${req.sessionID}') and ci.product_id = ${req.body.product_id} and ci.product_variant_id = ${req.body.product_variant_id} and ci.product_attribute_id = ${req.body.product_attribute_id}`;


    let queryInsertSession = `INSERT INTO session (id,ip_address) values ('${req.sessionID}','${ip_address}')`;
    let queryAddToCart = `INSERT INTO carts (session_id,active) values ('${req.sessionID}',1)`;
    let queryAddCartList = `INSERT INTO cart_items (product_id,product_variant_id,product_attribute_id,cart_id,quantity) SELECT ${req.body.product_id},${req.body.product_variant_id},${req.body.product_attribute_id},(SELECT crt.id  from carts as crt where crt.session_id = '${req.sessionID}'),1 where (select pa.stock from product_attribute as pa where pa.id = ${req.body.product_attribute_id}) >= 1 `;

    let queryInsertAll = `${queryAddToCart}; ${queryAddCartList};`;

    let queryUpdateCartList = `UPDATE cart_items as ci set ci.quantity = ci.quantity+1 where 
    ci.cart_id = (select crt.id from carts as crt where crt.session_id = '${req.sessionID}') and ci.product_id = ${req.body.product_id} and ci.product_variant_id = ${req.body.product_variant_id} and ci.product_attribute_id = ${req.body.product_attribute_id} and (select pa.stock from product_attribute as pa where pa.id = ${req.body.product_attribute_id}) >= ci.quantity+1 `;

    let queryFindCartList =`select 
    ci.id as cart_items_id,
    p.name as product_name,
    p.slug as product_slug,
    p.description,
    p.regular_price,
    c.name as category_name,
    c.slug,
    pv.type,
    pd.discount_percentage,
    pd.discount_value,
i.link,i.caption,i.alt,pa.size,pa.stock,crt.active as cart_status,ci.quantity from products as p 
left join product_category as pc on p.id = pc.product_id 
left join categories as c on pc.category_id = c.id 
left join product_variant as pv on p.id = pv.product_id
left join product_discount as pd on pd.id = 
(SELECT pd1.id from product_discount as pd1 where p.id = pd1.id and now() between pd1.valid_from and pd1.valid_until)
left join product_image as pi on pi.id = (SELECT pi1.id from product_image as pi1 where pi1.product_id = p.id order by pi1.product_id asc limit 1)
left join images as i on pi.image_id = i.id 
left join product_attribute as pa on p.id = pa.product_id and pv.id = pa.product_variant_id
left join cart_items as ci on pv.id = ci.product_variant_id and p.id = ci.product_id and pa.id = ci.product_attribute_id
left join carts as crt on ci.cart_id = (SELECT crt1.id from carts as crt1 where crt1.session_id = '${req.sessionID}' 
) where crt.session_id = '${req.sessionID}' and ci.quantity <= pa.stock
`;


    let queryChecking = `${querySelectSession};${queryCheckCart}; ${queryCheckCartList}; ${queryFindCartList};`;


    db.query(queryChecking,(error,result)=>{
        if(error) return res.status(400).json(error);
        if(result[0].length > 0 && result[1].length > 0 && result[2].length === 0 && result[3].length < 15){
            db.query(queryAddCartList,(error,result)=>{
                if (error) return res.status(400).json(error);
                if (result) {
                    db.query(queryFindCartList, (error, result) => {
                        if (error) return res.status(400).json(error);
                        if (result.length > 0) {
                            let payload = {
                                session_id: req.sessionID,
                                ip_address: ip_address
                            }
                            let dataToken = jwt.sign(payload, keys.jwt.secretOrPrivateKey, { expiresIn: keys.jwt.expiresIn });
                            res.cookie("hammerstout_ss", dataToken, { sameSite: true });
                            let token_cart = {
                                result
                            };

                            let notification = {
                                error: false,
                                message: "ADDED TO YOUR CART.",
                                notification: true
                            }
                            let token_c = jwt.sign(token_cart, keys.jwt.secretOrPrivateKey, { expiresIn: keys.jwt.expiresIn });
                            return res.status(200).json({ cart_list: result, status: 'OK', notification: notification, token_c});
                        }
                    })

                }
            })
        }
        else if (result[0].length > 0 && result[1].length === 0 && result[2].length === 0 && result[3].length < 15){
            db.query(queryInsertAll,(error,result)=>{
                if (error) return res.status(400).json(error);

                if (result[0].affectedRows > 0 && result[1].affectedRows > 0){
                            db.query(queryFindCartList, (error, result) => {
                                if (error) return res.status(400).json(error);
                                if (result.length > 0) {
                                    let payload = {
                                        session_id: req.sessionID,
                                        ip_address: ip_address
                                    }
                                    let dataToken = jwt.sign(payload, keys.jwt.secretOrPrivateKey, { expiresIn: keys.jwt.expiresIn });
                                    res.cookie("hammerstout_ss", dataToken, { sameSite: true });
                                    let token_cart = {
                                        result
                                    };
                                    let notification = {
                                        error: false,
                                        message: "ADDED TO YOUR CART.",
                                        notification: true
                                    }
                                    let token_c = jwt.sign(token_cart, keys.jwt.secretOrPrivateKey, { expiresIn: keys.jwt.expiresIn });
                                    return res.status(200).json({ cart_list: result, status: 'OK', notification: notification, token_c});
                                }
                            })   
                        }
                else if (result[0].affectedRows === 0){
                    let notification = {
                        error: true,
                        message: "ERROR CART",
                        notification: true
                    }
                    return res.status(400).json({ notification: notification });
                        }
                else if (result[1].affectedRows === 0){
                    let notification = {
                        error: true,
                        message: "IS OUT OF STOCK !",
                        notification: true
                    }
                    return res.status(400).json({  notification: notification});
                        }
            })
        }
        else if (result[0].length > 0 && result[1].length > 0 && result[2].length > 0 ){
            db.query(queryUpdateCartList,(error,result)=>{
                if (error) return res.status(400).json(error);

                if (result.affectedRows > 0){
                    db.query(queryFindCartList, (error, result) => {
                        if (error) return res.status(400).json(error);
                        if (result.length > 0) {
                            let payload = {
                                session_id: req.sessionID,
                                ip_address: ip_address
                            }
                            let dataToken = jwt.sign(payload, keys.jwt.secretOrPrivateKey, { expiresIn: keys.jwt.expiresIn });
                            res.cookie("hammerstout_ss", dataToken, { sameSite: true });
                            let token_cart = {
                                result
                            };
                            let notification = {
                                error: false,
                                message: "ADDED TO YOUR CART.",
                                notification: true
                            }
                            let token_c = jwt.sign(token_cart, keys.jwt.secretOrPrivateKey, { expiresIn: keys.jwt.expiresIn });
                            return res.status(200).json({ cart_list: result, status: 'OK', notification: notification, token_c});
                        }
                    }) 
                }
                else if (result.affectedRows === 0) {
                    let notification = {
                        error: true,
                        message: "IS OUT OF STOCK !",
                        notification: true
                    }
                    return res.status(400).json({ notification: notification });
                }
            })
        } else if (result[0].length > 0 && result[1].length > 0 && result[3].length >= 15 ){
            let notification = {
                error: true,
                message: "Already the maximum limit",
                notification: true
            }
            return res.status(400).json({ notification: notification });
        }
        else{
            return res.status(400).json(result);
        }
    })

}  

Answer 1

Thanks for your first submission for review.

---

First and foremost, the thing that really jumps out at me is that this function is doing WAY too many different things. You likely should be considering different Express middleware functions to do things like:

• decorate IP address determination and decoration onto req
• user agent parsing and decoration onto req
• session ID determination, database update, and decoration onto req

...etc.

---

Do you do any validation on req.body at all outside of this function? You are taking a potential dangerous path in there is no input validation here.

---

The code is VERY hard to read. It is not indented well (particularly around SQL queries), has no comments, and has inconsistent use of vertical whitespace to separate logical sections.

---

You have many more nested if-else conditions than are needed. You should always that about inverting conditions to de-nest things, and should use return appropriately to de-nest. Code with this many branches is going to be very fragile to maintain and extremely difficult to test all those code paths.

You have some case where you are doing this well like

db.query(queryChecking,(error,result)=>{
    if(error) return res.status(400).json(error);
    if(result[0].length > 0 && result[1].length > 0 && result[2].length === 0 && result[3].length < 15){

Here you exit early on error condition and let the rest of the code proceed without being in an else condition.

However in most places in code you have this sort of pattern

if (...) {
   ...
   return;
} else if (...) {
   ...
   return;
} else if (...) {
   ...
   return;
}

This should be

if (...) {
   ...
   return;
}
if (...) {
   ...
   return;
}
if (...) {
   ...
   return;
}

There is no need for most of your else conditions if you are making a return in from the previous conditional statement.

---

Many of your let declarations could and should be const when the variables are not intended to ever be reassigned.

---

Besides these code issues noted, I am struggling to understand what you are trying to achieve here.

What does a cart have to do with a session or a login at all?

Why tie something that perhaps should persist across sessions to a given session?

How do you handle updating carts around events that should regenerate session id's like logins/logouts?

How do you handle session expiry?

Why is Session Browser "normalized" into it's own table when it seems there would be 1:1 relationship with session?