Problem/Motivation

Since #3174306: Viewing shipments requires the "administer commerce_shipment" permission, it's possible to view shipments via JSONAPI if the current customer is able to view order referencing the shipment.

I believe we should also allow updates.

The only potential problem is allowing to update fields that are supposedly "protected" such as the shipping service, method, and amount which shouldn't be set directly.

Applying a rate using Commerce API for example is done by patching the order.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jsacksick created an issue. See original summary.

jsacksick’s picture

Title: Allow shipments to be updated when the parent order can be updated » Allow shipments to be updated if the parent order can be updated
Status: Active » Needs review
FileSize
4.01 KB

So the shipment access control handler logic was copied from commerce_product (i.e view access is determined by the parent order), otherwise the "manage" permission is checked.

It's not really logical to me that granting the "manage $bundle commerce_shipment" permission gives you the ability to update/delete any shipment, but doesn't let you view them...

I added field access logic that applies only to JSON API routes, that forbids the update of the following fields:

  1. original_amount
  2. amount
  3. adjustments
  4. shipping_service
  5. shipping_method

Updating the shipping service / shipping method should be done from the order resources when the shipping is used in combination with Commerce API.

jsacksick’s picture

jsacksick’s picture

Better with the actual FieldAccess class.

Status: Needs review » Needs work

The last submitted patch, 4: 3245754-4.patch, failed testing. View results

jsacksick’s picture

Status: Needs work » Needs review
FileSize
8.21 KB

  • jsacksick committed b46e226 on 8.x-2.x
    Issue #3245754 by jsacksick: Allow shipments to be updated if the parent...
jsacksick’s picture

Status: Needs review » Fixed

Committed!

jsacksick’s picture

FileSize
771 bytes

Bummer, made a mistake with the route check... JSON API routes have a flag at the "default" level, not at the requirement level.

  • jsacksick committed b7c7ffa on 8.x-2.x
    Issue #3245754 followup by jsacksick: Fix the FieldAccess logic to...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.