0

I have this function to findOneAndUpdate, it will find if the username if it exist then set the time field and increase number of other field.

function _lockIfFailedAttemptsReachLimit(username,next) {
    colUser.findOneAndUpdate({ username }, { // increase failed attempt counter
        $set: { failedLoginAt: new Date() },
        $inc: { nFailedLogin: 1 },
    }, { upsert: false, returnOriginal: false }, (err, result) => {
        if (err) { // Undefined.
            next('Undefined error!');
        } else {
            let foundUser = result.value;
            if (_.isNil(foundUser)) { // cannot find username
                //nothing here yet.
            }

            let nFailed = _.get(foundUser, 'nFailedLogin', 0);

            let errMsg = `Invalid username or passsword. You have tried ${nFailed}/${MAX_FAILED_LOGIN_ATTEMPTS} login attempts`;

            if (nFailed < MAX_FAILED_LOGIN_ATTEMPTS) {
                next(errMsg);
            } else {
                _lockAccount(username, (err, result) => {
                    if (!err) {
                        errMsg += `. Your account has been locked for ${WAITING_TIME_AFTER_LOCKED} seconds.`;
                    }
                    next(errMsg);
                });
            }
        }
    });
}

Now I want in the same function if cannot find username, then it will insert username value and do the same process for that username. How could I do that in most effective coding ? I tried:

function _lockIfFailedAttemptsReachLimit(username, req, res, next) {
    colUser.findOneAndUpdate({ username }, { // increase failed attempt counter
        $set: { failedLoginAt: new Date() },
        $inc: { nFailedLogin: 1 },
    }, { upsert: false, returnOriginal: false }, (err, result) => {
        if (err) { // Undefined.
            next('Undefined error!');
        } else {
            let foundUser = result.value;
            if (_.isNil(foundUser)) { // cannot find username
                //I think can try to put the code here ?
                let username = (req.headers['x-forwarded-for'] ||
                    req.connection.remoteAddress ||
                    req.socket.remoteAddress ||
                    req.connection.socket.remoteAddress).split(",")[0];
                // insert username to the table and call the function to continue from the beginning
            }

            let nFailed = _.get(foundUser, 'nFailedLogin', 0);

            let errMsg = `Invalid username or passsword. You have tried ${nFailed}/${MAX_FAILED_LOGIN_ATTEMPTS} login attempts`;

            if (nFailed < MAX_FAILED_LOGIN_ATTEMPTS) {
                next(errMsg);
            } else {
                _lockAccount(username, (err, result) => {
                    if (!err) {
                        errMsg += `. Your account has been locked for ${WAITING_TIME_AFTER_LOCKED} seconds.`;
                    }
                    next(errMsg);
                });
            }
        }
    });
}
user1314404
  • 1,253
  • 3
  • 22
  • 51
  • [How to create item if not exists and return an error if exists](https://stackoverflow.com/a/50012466/2313887) - maybe? I'd certainly consider it a more logical approach. – Neil Lunn May 10 '18 at 03:16
  • thanks ! thats right, I agree too. Do you have any suggestion answer ? – user1314404 May 10 '18 at 03:29
  • I believe the suggestion was to follow the same example as is being used there. "Find or Create" should be a simple function of looking at a "primary key" which is typically "username" in the present situation. It's either "found", in which case you "have your user" and a way to identify it was "found", or "created" in which case there is the logic branch to follow and the explained methods for "identifying" that. Combinations of "username" and "password" should not actually be part of the "create" but rather simply an error on "mismatch" per the "found" response. It's a pretty common pattern – Neil Lunn May 10 '18 at 03:35
  • yes, but quite complex for me to understand... – user1314404 May 10 '18 at 03:37
  • Exactly. Not something you understood in 15 minutes. Actually take the time to go through the content and learn from it. Stack Overflow is not actually for someone submitting "ready to use code" that you can simply copy and paste and go about your business of the next task. The point is to learn, Which is why some of us provide such detailed responses :) – Neil Lunn May 10 '18 at 03:41
  • No wonder why you got so many points in your bag! :) Thanks for directing to the quite similar question though! Any other answer for my question is welcome, I believe it is easier to learn from my direct question then extend it to other issue versus the other way around. – user1314404 May 10 '18 at 03:47

0 Answers0