Asynchronous Handling Issue in API Key Hashing Middleware

What is your operating system?

Windows

Actual Behavior

In the user model, asynchronously hashes API keys within a loop during the checkApiKey middleware. However, due to the asynchronous nature of the bcrypt operations, the next() function may be called multiple times before all API keys are hashed. This can lead to unexpected behavior, such as calling next() prematurely or multiple times.

Implementation Now:

userSchema.pre('save', function checkApiKey(next) {
  // eslint-disable-line consistent-return
  const user = this;
  if (!user.isModified('apiKeys')) {
    next();
    return;
  }
  let hasNew = false;
  user.apiKeys.forEach((k) => {
    if (k.isNew) {
      hasNew = true;
      bcrypt.genSalt(10, (err, salt) => {
        // eslint-disable-line consistent-return
        if (err) {
          next(err);
          return;
        }
        bcrypt.hash(k.hashedKey, salt, (innerErr, hash) => {
          if (innerErr) {
            next(innerErr);
            return;
          }
          k.hashedKey = hash;
          next();
        });
      });
    }
  });
  if (!hasNew) next();
});

Expected Behavior

The checkApiKey middleware should correctly hash API keys using bcrypt and call next() only after all API keys have been hashed. This ensures that the middleware behaves as intended, handling each API key properly and advancing to the next middleware in the chain only when all asynchronous operations are complete.

Correct implementation:

userSchema.pre('save', function checkApiKey(next) {
  const user = this;
  if (!user.isModified('apiKeys')) {
    next();
    return;
  }
  let hasNew = false;
  let pendingTasks = 0;

  user.apiKeys.forEach((k) => {
    if (k.isNew) {
      hasNew = true;
      pendingTasks++;
      bcrypt.genSalt(10, (err, salt) => {
        if (err) {
          next(err);
          return;
        }
        bcrypt.hash(k.hashedKey, salt, (innerErr, hash) => {
          if (innerErr) {
            next(innerErr);
            return;
          }
          k.hashedKey = hash;
          pendingTasks--;
          if (pendingTasks === 0) {
            next();
          }
        });
      });
    }
  });

  if (!hasNew) {
    next();
  }
});

Start in server/models/user.js around the checkApiKey pre-save hook at line 118 and trace how bcrypt callbacks advance the save middleware. Exercise a save with multiple new API keys and verify that hashing completes for every key and the middleware callback is invoked exactly once after all asynchronous work finishes.