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

Updating a model's relationships where the policy denies updating the relationship gives 200 OK, but doesn't update the relationship #281

Open
Azeirah opened this issue May 21, 2024 · 5 comments

Comments

@Azeirah
Copy link

Azeirah commented May 21, 2024

I have the following Schema:

class UpsellSchema extends Schema
{
  /**
   * The model the schema corresponds to.
   *
   * @var string
   */
  public static string $model = Upsell::class;

  /**
   * Get the resource fields.
   *
   * @return array
   */
  public function fields(): array
  {
    return [
      ID::make(),
      Str::make("name"),
      Str::make("upsell_variant"),
      BelongsTo::make("salesarea"),
      BelongsToMany::make("products"),
      DateTime::make("createdAt")
        ->sortable()
        ->readOnly(),
      DateTime::make("updatedAt")
        ->sortable()
        ->readOnly(),
    ];
  }

Note the "BelongsToMany" relationship with "products".

My policy for Upsell is as follows (in short, it allows any updates to the model):

class UpsellPolicy
{
  use HandlesAuthorization;

  /**
   * Determine whether the user can view any models.
   */
  public function viewAny(?User $user): bool
  {
    return true;
  }

  /**
   * Determine whether the user can view the model.
   */
  public function view(?User $user, Upsell $upsell): bool
  {
    return true;
  }

  /**
   * Determine whether the user can create models.
   */
  public function create(User $user): bool
  {
    return true;
  }

  /**
   * Determine whether the user can update the model.
   */
  public function update(User $user, Upsell $upsell): bool
  {
    return true;
  }

  /**
   * Determine whether the user can delete the model.
   */
  public function delete(User $user, Upsell $upsell): bool
  {
    return true;
  }

  /**
   * Determine whether the user can restore the model.
   */
  public function restore(User $user, Upsell $upsell): bool
  {
    return true;
  }

  /**
   * Determine whether the user can permanently delete the model.
   */
  public function forceDelete(User $user, Upsell $upsell): bool
  {
    return true;
  }
}

Now when I make the following request, intending to replace an upsell's products relationship

PATCH http://mysite.com/jsonapi/upsells/20

BODY
{
   "data":{
      "type":"upsells",
      "id":"20",
      "attributes":{
         "name":"MyTesty :Daa",
         "upsell_variant":"upsell"
      },
      "relationships":{
         "products":{
            "data":[
               {
                  "type":"products",
                  "id":"135219"
               },
               {
                  "type":"products",
                  "id":"135189"
               },
               {
                  "type":"products",
                  "id":"135191"
               }
            ]
         }
      }
   }
}

I get a 200 OK, but the upsell#20.products relationship is not updated.

Once I added the following function to the UpsellPolicy, it does get updated.

  public function updateProducts(User $user, Upsell $upsell): bool
  {
    return true;
  }

I'm expecting a 401 Unauthorized exception, not a 200 OK.

@Azeirah
Copy link
Author

Azeirah commented May 21, 2024

I believe this is supported by the json-api spec 9.2.3.4

https://jsonapi.org/format/#crud-updating-resource-relationships

A server MUST return 403 Forbidden in response to an unsupported request to update a resource or relationship.

The policy decides whether or not a request is supported or not. If it denies the request, a 403 Forbidden should be sent back to the client.

@asugai
Copy link

asugai commented May 21, 2024

@Azeirah - the "supported" seems to be key here, if it's allowed under some circumstances but not others, it's still supported. Only if it's not supported under any circumstances does it provide a 403.

@Azeirah
Copy link
Author

Azeirah commented May 21, 2024

@Azeirah - the "supported" seems to be key here, if it's allowed under some circumstances but not others, it's still supported. Only if it's not supported under any circumstances does it provide a 403.

Possibly, however paragraph 9 does state the following

A request MUST completely succeed or fail (in a single “transaction”). No partial updates are allowed.

In the example I provided, my request performs a partial update (only the upsells attributes are updated, but not the relations), this clearly goes against spec 9, and possibly goes against the spec 9.2.3.4 depending on how it is interpreted.

@asugai
Copy link

asugai commented May 21, 2024

@Azeirah - given that the documentation for this package lists that you need to add the authorization to modify the relationships, (https://laraveljsonapi.io/docs/3.0/tutorial/06-modifying-resources.html#authorization), it seems like it simply ignores the request's relationship changes unless the authorization is defined.

So the 200 OK response is based on updating the object itself, ignoring the relationship changes unless you add the authorization. At which point it checks if you can update or not.

Try setting the updateProducts policy to return false and see if you get the result you expect.

@lindyhopchris
Copy link
Contributor

Once I added the following function to the UpsellPolicy, it does get updated.

I'm surprised to hear this, I can't see how that has any affect. The updateProducts method on the policy isn't checked for the PATCH http://mysite.com/jsonapi/upsells/20 request.

Can you check your validation rules? We only fill the model with validated data, so if you haven't got a validation rule for the products relationship, it won't be filled.

In relation to this from the spec:

A server MUST return 403 Forbidden in response to an unsupported request to update a resource or relationship.

We are compliant with that, because we authorise the request to update a resource or relationship. (Relationship in this context means the relationship endpoints, which are authorised as well as the resource endpoints.)

Resource authorisation: https://laraveljsonapi.io/docs/3.0/requests/authorization.html#resource-authorization

vs

Relationship authorisation: https://laraveljsonapi.io/docs/3.0/requests/authorization.html#relationship-authorization

With the latter section worth reading, as it explains all of this. And explains that the implementation is resource level authorisation, not field level authorisation.

Hope this helps.

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

3 participants