ExpressJS Middleware - Execution order of Promises not correct-CodePudding

I want to create an Express Middleware to perform a basic check if the user/password pair in the authorization header exists in a JSON file (educational purpose). I added it on a very simple unit converter app.

The problem is that I receive a 403 instead of the resource when the username/password are correct.

I found that when I perform a request, the Promise.then in the middleware is executed before the Promise is fulfilled in my function findUserByCredentials. See an illustration of the problem in the third code snippet below.

index.js

const express = require('express')
const app = express()
const port = process.env.PORT || 3000
const findUserByCredentials = require("./lib/find-user");

app.use(function (req, res, next) {
    if (req.headers) {
        let header = req.headers.authorization || '';
        let [type, payload] = header.split(' ');

        if (type === 'Basic') {
            let credentials = Buffer.from(payload, 'base64').toString('ascii');
            let [username, password] = credentials.split(':');
            findUserByCredentials({username, password}).then(() => {
                console.log("next")
                next();
            }).catch(() => {
                console.log("403")
                res.sendStatus(403);
            });
        }
    } else {
        next();
    }
});

app.get('/inchtocm', (req, res) => {
    const cm = parseFloat(req.query.inches) * 2.54;
    res.send({"unit": "cm", "value": cm});
});

app.listen(port, () => {
    console.log(`Example app listening at http://localhost:${port}`)
})

module.exports = app;

./lib/find-user.js

const bcrypt = require('bcrypt');
const jsonfile = require('../users.json');

let findUserByCredentials = () => (object) => {
    const username = object.username;
    const password = object.password;
    return new Promise((resolve, reject) => {
        jsonfile.forEach(user => {
            if (user.username === username) {
                bcrypt.compare(password, user.password).then((buffer) => {
                    if (buffer) {
                        console.log("resolve")
                        resolve();
                    } else {
                        console.log("reject")
                        reject();
                    }
                });
            }
        });
        reject();
    });
};
module.exports = findUserByCredentials();

Server console after sending a request

Example app listening at http://localhost:3000
403
resolve

How can I force Express to wait for the first Promise to finish before performing the second operation ?

CodePudding user response：

To have a better control over the order of your promises and also have a less nested code, you should use the async/await syntax. You can read more about it here.

What it essentially does is lets you...well, await for an async operation to finish before proceeding. If you use await before something that returns a Promise (like your findUserByCredentials), it will assign to the variable what you resolve your promise with, for example:

  const myPromise = () => {
    return new Promise(resolve => resolve(3));
  }

  const myFunc = async () => {
    const number = await myPromise(); 
    console.log(number); // Output: 3
  }

I would rephrase your findUserByCredentials function like so:

const bcrypt = require('bcrypt');
const jsonfile = require('../users.json');

const findUserByCredentials = async (object) => {
    const username = object.username;
    const password = object.password;
    // This assumes there's only a single unique user with a specific username
    const potentialUser = jsonfile.find(user => user.username === username);
    if (!potentialUser) {
       throw new Error('wrong credentials')
    }
    const passHashCompare = await bcrypt.compare(password, potentialUser.password);
    if (!passHashCompare) {
       throw new Error('wrong credentials')
    }
};
module.exports = findUserByCredentials;

This way it's less nested, more readable and works in the order you need. You can go even further with this principle and make your middleware (the function you're passing to app.use) an async function as well and use the await keyword instead of .then() and .catch()

CodePudding user response：

I would switch the code to use the latest features of the javascript language related to asynchronous code async/await in that way you can have better control of your execution flow.

I will modify your code in the following way:

Firstly for the findUserByCredentials function:

const bcrypt = require('bcrypt');
const jsonfile = require('../users.json');

const findUserByCredentials = async (object) => {
    const username = object.username;
    const password = object.password;
    const user = jsonfile.find(user => user.username == username);
    if (user)
      await bcrypt.compareSync(user.password, password);
    return false;
    
};
module.exports = findUserByCredentials;

Secondly for the index.js

const express = require('express')
const app = express()
const port = process.env.PORT || 3000
const findUserByCredentials = require("./lib/find-user");

app.use(async (req, res, next) => {
    if (req.headers) {
        let header = req.headers.authorization || '';
        let [type, payload] = header.split(' ');

        if (type === 'Basic') {
            const credentials = Buffer.from(payload, 'base64').toString('ascii');
            const [username, password] = credentials.split(':');
            const result = await findUserByCredentials({username, password})
            if(result) {
              return next()
            } 
            return res.sendStatus(403);
        }
        return res.sendStatus(403);
    }
    return next();
});

app.get('/inchtocm', (req, res) => {
    const cm = parseFloat(req.query.inches) * 2.54;
    res.send({"unit": "cm", "value": cm});
});

app.listen(port, () => {
    console.log(`Example app listening at http://localhost:${port}`)
})

module.exports = app;

It's very important learn how to use asynchronous code in Nodejs because it's a single thread and if it's not used correctly you can block it and your performance of your app won't be optimal, also async/await is syntactical sugar of javascript languages which allows to write clean asynchronous code, try to use the latest things of the specification of language because they are created to ease our lives.