nuwave / lighthouse

A framework for serving GraphQL from Laravel
https://lighthouse-php.com
MIT License
3.37k stars 438 forks source link

Nested mutation delete allows deletion of other relationship #1400

Open TheOddler opened 4 years ago

TheOddler commented 4 years ago

I have a project model/table that has a number of files attached to it. When updating a project I allow the user to delete files from it. I use a nested mutation with a delete: [ID!] field for this.

However, when updating for instance project "A" and I provide id's of files attached to project B, the files are deleted from project B.

I would expect this to give an error, stating that these files are not connected to project A.

Steps to reproduce

  1. Create a nested mutation that allows deleting
  2. Provide id's from a different relationship
  3. See how they are deleted

Lighthouse Version

4.12

spawnia commented 4 years ago

I agree that nested deletes should be limited to existing relations and am open to PR's. Here is the responsible code https://github.com/nuwave/lighthouse/blob/301930b356faa63da681d564cbef41c7d312f2aa/src/Execution/Arguments/NestedOneToMany.php#L32-L36

Off the top of my head, i can not think of an easy solution. Does Eloquent already allow to do what we want through the relationship methods?

lorado commented 4 years ago

Does Eloquent already allow to do what we want through the relationship methods?

I mean yes. We should probably do something like $model->relation()->where('relation_primary_key', $args->id)->delete(). Laravel adds the relation 'restriction' (binding to parent model) automatically.

If the PR comes, please think also about @forceDelete directive ;) It needs also such logic. I will probably also help

budirec commented 4 years ago

Any update on this? This also affecting update on hasMany relation

spawnia commented 4 years ago

Any update on this?

The last update was this comment. Are you not able to see the comment history?

Unless someone decides to put in the work and provide a PR, nothing will happen.

crThiago commented 4 years ago

this also occurs with the update on hasOne ralation

spawnia commented 4 years ago

Lighthouse has no restrictions in place for any kind of relation. Every possible combination of relation and nested operation where ids are passed by the user is affected by this.

aaronbullard commented 3 years ago

This is also a big problem for me. My parent entity is a Business. I authorize a user against a business entity. The Business has many nested relations that can be mutated. Without this relationship check, a user can mutate any nested relation owned by another business.