feathersjs-ecosystem / feathers-mongoose

Easily create a Mongoose Service for Feathersjs.
MIT License
189 stars 96 forks source link

Support for mongoDb 4.2 pipeline updates #370

Open florianbepunkt opened 4 years ago

florianbepunkt commented 4 years ago

Steps to reproduce

Mongodb 4.2 introduced pipeline updates, meaning you can use an aggregation pipeline for updates: https://docs.mongodb.com/manual/tutorial/update-documents-with-aggregation-pipeline/

Mongoose introduced support for this, but as far as I can tell it's not possible with the adapater:

Expected behavior

All of the following commands should perform the same update to a document

await app
  .service("example")
  .patch("someId", [{ $set: { foo: "bar" } }]);

await app
  .service("example")
  .patch("someId", { foo: "bar" });

await app
  .service("example")
  .Model.updateOne({ _id: "someId" }, [
    { $set: { foo: "bar" } }
  ]);

Actual behavior

The first command (pipeline update) is not performing any data change.

Possible solution

I believe this behaviour is because internally the adapter creates a copy of the update (and in doing so assumes it is an object):

// ensure we are working on a copy
data = Object.assign({}, data);

changing this to

data = Array.isArray(data) ? [].concat(data) : Object.assign({}, data);

solves the problem

Security

Pipeline updates are a very powerful feature as they allow updating documents based on other document values, thereby eliminating a lot of roundtrips. I hope, this adapter will support them.

However there are some security ramifications as it is possible to retreive other documents and values in an aggregation pipeline update. So it might be prudent that it is a feature that is explicitly whitelisted or allowed for internal calls only.

Over-simplified example to show what I mean: Let's say you have a document like this

{
_id: 1,
username: "fox_mulder",
password: "scully",
themePreference: "veryDarkMode"
}

You use a hook that prevents the password field from ever being sent to the user. An attacker could use a pipeline update to circumvent this by setting the value of themePreference to the value of the password field.

await app.service.patch(1, [{$set: {themePreference: "$password"}}]);

System configuration

Tell us about the applicable parts of your setup.

Module versions (especially the part that's not working): "feathers-mongoose": "^8.3.0"

NodeJS version: 13.6.0

Operating System: MacOs

Trukl commented 2 years ago

Any news about this enhancement ?