Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Asynchronous Handling Issue in API Key Hashing Middleware fixed #3017

Closed
wants to merge 3 commits into from

Conversation

Swarnendu0123
Copy link
Contributor

@Swarnendu0123 Swarnendu0123 commented Feb 13, 2024

Fixes #3016

Changes:

userSchema.pre('save', async function checkApiKey(next) {
  const user = this;
  if (!user.isModified('apiKeys')) {
    next();
    return;
  }

  try {
    const hashTasks = user.apiKeys
      .filter(k => k.isNew)
      .map(async (k) => {
        const salt = await bcrypt.genSalt(10);
        const hash = await bcrypt.hash(k.hashedKey, salt);
        k.hashedKey = hash;
        // Mongoose will handle isNew flag during save operation
      });

    await Promise.all(hashTasks);

    next(); // Call next if all operations are successful
  } catch (err) {
    next(err); // Pass any error to the next middleware
  }
});

As for the concern about setting the isNew flag to false, it's important to reset this flag after hashing each API key to prevent rehashing it unnecessarily in subsequent save operations. With the provided code, the isNew flag is set to false within the map function after hashing each API key. This ensures that each new API key is hashed only once, even if the save operation is called multiple times.

This implementation maintains the flow of the middleware and ensures that the next() function is called appropriately based on the completion or failure of the asynchronous operations.

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

@raclim
Copy link
Collaborator

raclim commented Jun 25, 2024

Thank you for your work on this! As this PR currently doesn't pass all the tests and the PR queue we currently have, I'm going to close this one for now. I'm sorry that we couldn't get this in, but please feel free to update and reopen this PR with any changes or check out the other issues!

@raclim raclim closed this Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Asynchronous Handling Issue in API Key Hashing Middleware
2 participants