Allow replacing nested fields

[smirea]

Due to the use of moredots in modifyObject, it is currently impossible to remove properties of a nested object via the API (talking about this line: https://github.com/florianholzapfel/express-restify-mongoose/blob/master/src/operations.js#L204)

so if you have a schema:

User = mongoose.model('User', new mongoose.Schema({
    name: String,
    info: Object,
})

User.info is a generic object, so one would assume doing a PATCH /User/1 {info: {bmi: 33}} would update the info prop to the given value. But because moredots has no concept of the schema the above query gets converted to:

{ 'info.bmi': 33 }

This is a problem because if you have the following document:

{
    name: 'bob',
    info: {
         bmi: 33,
         weight: 100,
    }
}

There is no way to remove info.weight since posting with {info: {bmi: 22}} will just update the bmi instead of overwriting the entire info

Not sure if this was the intention or not, but it seems to me that it makes more sense to have PATCH be update only at the root level, and not recursive.

The current workaround is to specifically post with {info: {bmi: 22, weight: null}} which will overwrite that property to null which is not really ideal since it requires prior knowledge about the document and the schema

EDIT: Another workaround is if you make a middleware that does the above and sets unwanted values to undefined, since sending the object {info: {bmi: 22, weight: undefined}} does remove weight but you can't json serialize undefined and you still have the issue of having to know about the structure of the data before updating

[Zertz]

This was done to prevent anyone from overwriting private/protected nested fields. Let's say we have,

private: ['info.weight']

Then, someone with public access does,

PATCH /Model/some-id
{
  info: {
    bmi: 33,
    weight: 100
  }
}

With and without moredots, info.weight is filtered out as expected, the difference comes after. Without dots, mongoose.set('info', { bmi: 33 }) overwrites weight even though the field was marked as private. With dots, mongoose.set('info.bmi', 33) will only write that specific field, respecting private and protected.

I agree that the current situation isn't perfect and can lead to surprising behavior. It's a compromise we chose for security reasons, I am certainly open to hearing alternative ideas here!

[smirea]

i had no idea that there is access control on nested fields ... that seems to complicate things.

currently there are 2 ways to interact with the API that seem different but (unfortunately) compile to the same thing, aka: {info: {bmi: 22}} and {'info.bmi': 22} both compile to the later, but semantically they mean different things. If these would be split and treated separately, then if info.weight is private the second would work and the later could fail with a "not enough permissions to edit info" error

If dropping support for nested security fields is an option, that would simplify this discussion a lot. They're not exposed in the docs (as far as I've seen) and they seem like a niche feature whereas this is a much more frequent occurrence.

[Zertz]

So you are suggesting,

• Send 401 Unauthorized when fields you're not allowed to access are present
• Drop support for nested fields and overwrite the entire field

This is very breaking so it's probably not going to happen on PATCH. However, we have had a deprecation notice on updating with PUT for a while now so it would definitely work there.

This means you would,

PUT /Model/some-id
{
  // entire document
  ...,
  info: {
    bmi: 33
  }
}

Assuming you have proper access, this would entirely replace the existing doc with this one.

As a side note, keep in mind that breaking backward compatibility is okay (yay, semver!), but we have to make the changes as small as possible to allow fellow devs to easily update.

[smirea]

pretty much. It definitely makes sense having this functionality on PUT since it's supposed to do a hard replace so - assuming the permissions middleware are not used/relevant in this context - the result of PUT {doc} should deepEqual the document you just sent, per its definition

I'm not sure how common this issue is, but I find PATCH to be more useful (and safer) than PUT so I have PUT disabled entirely on my app for this reason.
I was also arguing for enabling this functionality on PATCH, essentially having the route behave as _.defaults old_data, new_data instead of the current behavior which is _.defaultsDeep old_data, new_data or at least allowing this behavior via a flag

[Zertz]

Yes, it could be behind a flag.

[Zertz]

This is pretty simple actually.

This code, becomes:

const cleanBody = depopulate(req.body)

if (options.updateDeep) {
  cleanBody = moredots(cleanBody)
}

Where updateDeep defaults to true.

The work here is mostly in writing tests and making it clear that it disables nested protected and protected fields.

[OliverFischer]

Which parts of the code are affected and have to be tested? May be we can help.

[Zertz]

Actually, rather than implementing yet another flag, I think we should perhaps be stricter and more "correct" in our use of HTTP verbs.

PATCH

• Partially update a resource, this works now

POST

• Create resources
• Synonym to PATCH, whether we keep this or not is up for debate

PUT

• Currently behavior is identical to PATCH, but it should entirely replace a resource instead

This is where most of the work is, which mostly boils down to:

1. Skip the call to moredots
2. Run the update without $set

I think that's pretty much it, let me know if you want to give it a shot! If you're not sure about the automated tests, send a PR and we'll go from there 👍

[smirea]

what happens to the private nested fields functionality in the case of PUT ?

i.e. if you have the object {foo: 1, bar:2} if you set bar to private, does that mean that nobody without the private permission would be able to PUT on that endpoint? Since if they are they would be able to eliminate bar by just PUT {foo: 3}

[Zertz]

Right, thanks for bringing that up. I see a couple options, but I'm open to hearing your suggestions.

• Leave the nested private fields as is (which kind of defeats the purpose of PUT)
• Deny access as soon as a private field would be overwritten (greatly limits the use cases)

In retrospect, enabling private/protected nested fields was not a great choice. I'm not against taking this away, but it would make upgrading very hard on users relying on this feature.

That said, if you see other options, please pitch in!

[i4got10]

Any progress here?

[Zertz]

Due to some (of my own!) earlier development decisions, it's a simple issue with complicated ramifications 😞

Removing support for private/protected nested fields would make this issue a lot easier to fix. Honestly I think that would be a good thing, but it puts some of our users in a bad spot because their schema design might require it.

[T-vK]

This is pretty simple actually.

This code, becomes:

const cleanBody = depopulate(req.body)

if (options.updateDeep) {
  cleanBody = moredots(cleanBody)
}

Where updateDeep defaults to true.

The work here is mostly in writing tests and making it clear that it disables nested protected and protected fields.

Thank you, this worked for me. Can you elaborate on the nested protected and protected fields topic and what exactly needs to be done in order to get this merged? I'm very interested in having this feature in the official express-restify-mongoose repo.