From b81e8f4469c82d48ff2c0d6fe399f5deb5fcc847 Mon Sep 17 00:00:00 2001 From: JATIN Date: Sun, 9 Feb 2025 00:03:44 +0530 Subject: [PATCH 1/2] Asynchronous Handling Issue in API Key Hashing Middleware Previously, next() was being called after hashing each API key, which caused multiple calls and issues. The new solution uses a pendingTasks counter to wait until all async operations are completed. This fix prevents Mongoose errors and premature next() calls, ensuring proper API key hashing. --- server/models/user.js | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/server/models/user.js b/server/models/user.js index d1c5e16bf2..2d6679971b 100644 --- a/server/models/user.js +++ b/server/models/user.js @@ -116,34 +116,45 @@ userSchema.pre('save', function checkPassword(next) { * API keys hash middleware */ userSchema.pre('save', function checkApiKey(next) { - // eslint-disable-line consistent-return const user = this; if (!user.isModified('apiKeys')) { - next(); - return; + return next(); } + let hasNew = false; + let pendingTasks = 0; + let nextCalled = false; + + const done = (err) => { + if (nextCalled) return; + if (err) { + nextCalled = true; + return next(new Error(err)); // ✅ Pass Error object + } + if (--pendingTasks === 0) { + nextCalled = true; + return next(); + } + }; + user.apiKeys.forEach((k) => { if (k.isNew) { hasNew = true; + pendingTasks++; bcrypt.genSalt(10, (err, salt) => { - // eslint-disable-line consistent-return - if (err) { - next(err); - return; - } + if (err) return done(err); bcrypt.hash(k.hashedKey, salt, (innerErr, hash) => { - if (innerErr) { - next(innerErr); - return; - } + if (innerErr) return done(innerErr); k.hashedKey = hash; - next(); + done(); }); }); } }); - if (!hasNew) next(); + + if (!hasNew) { + return next(); + } }); userSchema.virtual('id').get(function idToString() { From dda34f0856a9a41766377e246c76c934601d160d Mon Sep 17 00:00:00 2001 From: JATIN Date: Fri, 21 Feb 2025 14:08:55 +0530 Subject: [PATCH 2/2] fix linting test now these may work on npm run lint and able to get merge --- server/models/user.js | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/server/models/user.js b/server/models/user.js index 2d6679971b..2090cab568 100644 --- a/server/models/user.js +++ b/server/models/user.js @@ -118,7 +118,8 @@ userSchema.pre('save', function checkPassword(next) { userSchema.pre('save', function checkApiKey(next) { const user = this; if (!user.isModified('apiKeys')) { - return next(); + next(); + return; } let hasNew = false; @@ -129,22 +130,28 @@ userSchema.pre('save', function checkApiKey(next) { if (nextCalled) return; if (err) { nextCalled = true; - return next(new Error(err)); // ✅ Pass Error object + next(err); + return; } - if (--pendingTasks === 0) { + pendingTasks -= 1; + if (pendingTasks === 0) { nextCalled = true; - return next(); + next(); } }; user.apiKeys.forEach((k) => { if (k.isNew) { hasNew = true; - pendingTasks++; + pendingTasks += 1; bcrypt.genSalt(10, (err, salt) => { - if (err) return done(err); + if (err) { + done(err); + } bcrypt.hash(k.hashedKey, salt, (innerErr, hash) => { - if (innerErr) return done(innerErr); + if (innerErr) { + done(innerErr); + } k.hashedKey = hash; done(); }); @@ -153,7 +160,7 @@ userSchema.pre('save', function checkApiKey(next) { }); if (!hasNew) { - return next(); + next(); } });