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

Unique validator incorrecttly flags reference object id in another collection as not unique #131

Open
Podesta opened this issue Oct 9, 2021 · 14 comments

Comments

@Podesta
Copy link

Podesta commented Oct 9, 2021

First of all, I can confirm that this bug is present on "mongoose": "^6.0.7" with "mongoose-unique-validator": "^3.0.0", but not present on "mongoose": "^5.11.13" with "mongoose-unique-validator": "^2.0.3".

As the title says, when marking, for example, the 'username' key as unique in a 'user' Schema. And then when I try to add a reference of this 'user', using his object '_id' in a different schema, for example a 'notes' Schema, it returns the following error, where the value printed is the '_id' that I want to store as reference.

User validation failed: _id: Error, expected `_id` to be unique. Value: `616126ff472e354e3b3fa554`

Curiously enough, it only happens when there is more than one user in the collection. If I have a single user, I can keep adding new notes, without any issues, and the notes correctly store the user _id as a reference, and the users also correctly store a reference with the object _id from the notes.

Here are the two schemas, for reproducing the bug. Bug found while following Fullstackopen course. It might also be easier to reproduce by simply cloning the fullstack repo, and checking out to branch part4-8, and making sure update package.json to use the newest versions of mongoose and mongoose-unique-validator, as stated on the first line.

const mongoose = require('mongoose');

const noteSchema = new mongoose.Schema({
  content: {
    type: String,
    minlength: 5,
    required: true,
  },
  date: {
    type: Date,
    required: true,
  },
  important: Boolean,
  user: {
    type: mongoose.Schema.Types.ObjectId,
    ref: 'User'
  },
});

noteSchema.set('toJSON', {
  transform: (document, returnedObject) => {
    returnedObject.id = returnedObject._id.toString();
    delete returnedObject._id;
    delete returnedObject.__v;
  }
});

module.exports = mongoose.model('Note', noteSchema);
const mongoose = require('mongoose');
const uniqueValidator = require('mongoose-unique-validator');

const userSchema = new mongoose.Schema({
  username: {
    type: String,
    unique: true,
  },
  name: String,
  passwordHash: String,
  notes: [
    {
      type: mongoose.Schema.Types.ObjectId,
      ref: 'Note'
    }
  ],
});

userSchema.plugin(uniqueValidator);

userSchema.set('toJSON', {
  transform: (document, returnedObject) => {
    returnedObject.id = returnedObject._id.toString();
    delete returnedObject._id;
    delete returnedObject.__v;
    delete returnedObject.passwordHash;   // The password Hash should not be revealed
  }
});

const User = mongoose.model('User', userSchema);

module.exports = User;
@viveleroi
Copy link
Collaborator

Thanks for the detailed report, I'll look into it.

Possibly the actual cause of #88

@viveleroi
Copy link
Collaborator

Can you reduce the schemas to a minimal copy, and add the creation logic? I want to reduce this to a single snippet of code that caused the issue, checking out repos is less helpful.

I have attempted to do this myself but so far have not seen the problem.

  1. I used the schemas provided
  2. I created two users, since you said it only happens with more than one
  3. I added a note with a reference to the second users _id
  4. It works fine. mongoose v6.0.10. m.u.v v3.0.0

@Podesta
Copy link
Author

Podesta commented Oct 9, 2021

Thanks! It might be worth noting that the database is on atlas. I'll try to strip everything down as much as I can and post it.

@sam-simpleclick
Copy link

sam-simpleclick commented Oct 11, 2021

I have ran across this issue too, seems to be incorrect conditions when saving a non-new document.
If this plugin is applied to the _id path, during validation for that path conditions will be { _id: { $ne: this._id } } (edit: if there are no other changes) which obviously is going to match every other document in the database and result in validation failing.
The fix might be to not validate _id unless its a new document?

@viveleroi
Copy link
Collaborator

We already have logic in place to avoid validating self, something beyond that is happening. Hopefully #88 can provide insight.

@niftylettuce
Copy link

I just got the same issue here too

@niftylettuce
Copy link

v2.0.4 should be deprecated and unpublished from npm if possible, this causes major auth issues for probably thousands of projects out there

@niftylettuce
Copy link

there should have been a major semver bump for these changes

@viveleroi
Copy link
Collaborator

2.0.4 was deprecated. unpublishing isn't available. 3.0 was pushed instead.

@niftylettuce
Copy link

Just email support@npmjs.com and tell them the severity of this and they should be able to force unpublish it from there side since it's only been a few days

@viveleroi
Copy link
Collaborator

viveleroi commented Oct 19, 2021

Appreciate the advice, I have emailed them.

@chrisziegler
Copy link

chrisziegler commented Oct 20, 2021

@Podesta I'm doing the same course, came here after encountering the same issue. You may very well have figured-out a workaround in the interim, but if not you can satisfy the (course specific) requirements while keeping mongoose v^6.0.7 by adding this to the express errorHandler middleware:

else if (error.name === 'MongoServerError' && error.code === 11000) {
    return response
      .status(500)
      .send({ success: false, message: 'Username already exists!' })
  }

Use a try/catch block in your notes POST route, and make sure catching and passing on the exception with next(exception).

'unplug' mongoose-unique-validator from the User Schema, you shouldn't need the dependency anymore.

Update the Jest tests to reflect the different status of the error .

@whoisstan
Copy link

Same issue, will try to remove the plugin, since it's reporting errors on _id as well where there is no violation. The somewhat inconsistent occurrence leads me to believe that the plugin is actually checking the database with a find and using that is an indication of uniqueness which can cause all kinds of issues and false negatives/positives. No?

Seems that correct save error can be simply handled through mongo errors and they contain the violating field as well.

{  index: 0,
  code: 11000,
  keyPattern: { username: 1 },
  keyValue: { username: '1635256492062abcDEF' }
}

@studstillprime
Copy link

studstillprime commented Oct 29, 2021

Can you reduce the schemas to a minimal copy, and add the creation logic? I want to reduce this to a single snippet of code that caused the issue, checking out repos is less helpful.

I have attempted to do this myself but so far have not seen the problem.

  1. I used the schemas provided
  2. I created two users, since you said it only happens with more than one
  3. I added a note with a reference to the second users _id
  4. It works fine. mongoose v6.0.10. m.u.v v3.0.0

Edit: It turns out the "fix" was having only one user when I recreated the DB. Upon creating another user entry it gave me the same error on update requests, as before.

I encountered this problem as well, and rectified it by dropping the whole DB (not just the offending collection). I'd offer it has something to do with the indexes being corrupted/incorrect, as the issue (Failure to save because of non-unique id while editing an existing entry) first appeared for me when I did the following:
Had MongoDB Compass running and was looking at unrefreshed entries that had been modified in the interim, and then I deleted a few of them from the GUI, but before refreshing, if that makes sense? I think this generated the issue, as my code was very simple and only the one collection (the one with the deletions) was affected by the error, while the otherwise identically coded collections were not. So to try and reproduce, if that isn't clear, maybe:
1. Create a simple schema (mine had a nested object but otherwise was not complex).
2. Create some entries.
3. Inspect the collection on a GUI.
4. Edit the entries outside of the GUI.
5. Delete the entries.
6. Create new entries.
7. Attempt to modify them without using the GUI should trigger the error.

Hope this is helpful, cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants