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

PersistedModel.updateAll() ignores validation #771

Closed
florianheinze opened this issue Nov 23, 2015 · 10 comments · Fixed by #1445
Closed

PersistedModel.updateAll() ignores validation #771

florianheinze opened this issue Nov 23, 2015 · 10 comments · Fixed by #1445

Comments

@florianheinze
Copy link

I have this model:

{
  "name": "Foo",
  "strict": "validate",
  "options": { "validateUpsert": true },
  "properties": {
    "name": {
      "type": "String",
      "min": 5
    }
  }
}

If I create a new entry with a three character name, I get, as expected, a validation error. However if I create a new entry with a longer name and then update that name to three characters, there is no validation error and the change is written to the DB.

As far as I can tell the whole validation is not triggered when updateAll() is used.

Steps to reproduce:
@Amir-61
Copy link
Contributor

Amir-61 commented May 17, 2016

Yes thats seems a bug to me; the validation is not implemented for updateAll/ update

@dmitru
Copy link

dmitru commented Aug 10, 2016

Any updates on the issue?

@Amir-61
Copy link
Contributor

Amir-61 commented Aug 10, 2016

@dmitru IIRC updateAll/update do not have validation; would you like to submit a patch?

@Amir-61 Amir-61 added feature and removed bug labels Aug 10, 2016
@Amir-61 Amir-61 self-assigned this Aug 21, 2016
@ahmed-abdulmoniem
Copy link

This is still not implemented?

Also, updateAttributes the same?

@tvdstaaij
Copy link

Would like to note that this is a very serious issue, this makes it very easy to corrupt instances through the API. These instances will then fail to validate when they are partially updated with a different set of properties than the one(s) that were corrupted. It's even worse with NoSQL backends: I strongly suspect this issue prevents strict mode from being enforced with updateAll, and adding random properties outside of the model spec would prevent instances from ever being instance-updateable again (due to #759).

I would even consider this a security issue since it allows users to sabotage app features for other users.

@vkruoso
Copy link

vkruoso commented Mar 1, 2017

This is really a consistency problem that will actually allow a malicious user to overcome the developer assertions about its data. Is there any update on this?

@Amir-61
Copy link
Contributor

Amir-61 commented Mar 1, 2017

@kjdelisle @strongloop/sq-lb-apex PTAL ☝️

@ssh24
Copy link
Contributor

ssh24 commented Aug 2, 2017

The issue should be fixed on the upcoming version of juggler. Thank you for your patience!

@bajtos
Copy link
Member

bajtos commented Aug 3, 2017

Hello, I think the solution landed via #771 is a breaking change, see #1445 (comment), and we should rework it to preserve backwards compatibility, before the new version of juggler is released.

@bajtos
Copy link
Member

bajtos commented Apr 18, 2019

Essentially, we need to run a partial validation that will validate only properties affected by updateAttribtes or updateAll request. I am afraid this is way too difficult to achieve in the current LB 3.x code base. We are aware of this issue and would like to eventually address it in LoopBack 4.x (or later) - see the discussion in loopbackio/loopback-next#1872.

I am going to lock this issue down, in order to move the discussion to loopbackio/loopback-next#1872.

@loopbackio loopbackio locked as resolved and limited conversation to collaborators Apr 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.