CODEWITHSUNDEEP
node.jsexpresscallback
snowfinch27
snowfinch27

Reputation: 149

Make callback block the thread

I'm using ExpressJS middleware to check user ip in database and stop responding user if there were more than 7 failed logins for the last hour, I use check for '/' before database connection for not spamming database if everything ok. But it turns out that while middleware is accessing the database and doing checks in callback, code in first else runs. Here is my middleware:

// check for failed logins from this ip in db
// if allowed number exceeded - stop responding
app.use(function (req, res, next) {
if(req._parsedUrl.pathname === '/') {
    MongoClient.connect(databaseUri || 'mongodb://localhost:27017/dev', function (err, db) {
        assert.equal(err, null);
        var failedLogins = db.collection('FailedLogins');
        failedLogins.find({ip: req._remoteAddress}).toArray(function (err, results) {
            assert.equal(err, null);
            console.log('db check');
            // if ip is in FailedLogins collection
            if (results.length) {
                // if there are more than 7 logins and they haven't expired
                if (results[0].failedLoginsNum >= 7 && parseInt(results[0].expiration) >= parseInt(Date.now())) {
                    res.end();
                } else {
                    next();
                }
            } else {
                next();
            }
        });
    });
} else {
    console.log('next');
    next();
}
});

And this is console output:

db check
GET / 200 20.117 ms - -
next
GET /favicon.ico 200 207.559 ms - 1353

Upvotes: 0

Views: 53

Answers (1)

Alexander Suvorov
Alexander Suvorov

Reputation: 159

This is expected behaviour, because second request received earlier than db connection is established and find query is done.

First of all you dont want to create new connection on every request this is a bad practice.

Consider next code:

dbProvider.js

'use strict';

const MongoClient = require('mongodb').MongoClient;
const CONNECTION_PATH = 'mongodb://localhost:27017/dev';

module.exports.init = cb => {
  MongoClient.connect(CONNECTION_PATH, (err, db) => {
    module.exports.db = db;
    cb(err, db);
  });
};

index.js

'use strict';

const express = require('express');
const app = express();

const dbProvider = require('./dbProvider');
const PORT = 8888;

dbProvider.init((err, db) => {
  if(err) {
    console.error(err);
    process.exit(1);
  }

  app.listen(PORT, () => console.log(`Listening on port ${PORT}`));
});

In index.js we wait until connection is established and only then listening for HTTP requests. Next step is login, so if you want to limit login attempts I propose to use simple middleware.

index.js modified

'use strict';

const express = require('express');
const app = express();

const dbProvider = require('./dbProvider');
const PORT = 8888;

dbProvider.init((err, db) => {
  if(err) {
    console.error(err);
    process.exit(1);
  }

  app.listen(PORT, () => console.log(`Listening on port ${PORT}`));
});

// middlewares

function checkAuth(req, res, next) {
  // here check if userId in cookie and match to db record or check token
  if (req.authenticated()) {
    return next();
  } else {
    return res.send(401); // Respond "Unauthorized"
  }
}

function checkLoginAttempts(req, res, next) {
  let failedLogins = dbProvider.db.collection('FailedLogins');
  failedLogins.find({ip: req._remoteAddress}).toArray(function (err, results) {
    if(err) {
      console.error(err);
      return res.status(500).end();
    }
    console.log('db check');

    if(!results.length) return next();

    // if ip is in FailedLogins collection
    // if there are more than 7 logins and they haven't expired
    if (results[0].failedLoginsNum >= 7 && parseInt(results[0].expiration) >= parseInt(Date.now())) {
      res.status(401).send('The maximum number of login attempts has been reached. Please try again in 1 hour.');
    } else {
      next();
    }
  });
}

// routes

app.use('/yourLoginEndPoint', checkLoginAttempts, (req, res) => {
  // login process, set userId as cookie value or create token
});

app.use('/anyOtherEndPoint', checkAuth, (req, res) => {
  // here you sure that user is authenticated
});

If any questions let me know

Upvotes: 1

Related Questions