Skip to content

Update Operation with Roar JSONAPI representer forces contract to open up id #25

@KonstantinKo

Description

@KonstantinKo

Cross-referencing trailblazer/trailblazer-operation#17, where I originially submitted this issue

Complete Description of Issue

When using trailblazer-operation, roar-jsonapi, and reform in conjunction with an Update call, the id field of a resource is forced to be editable.

That seems like a bad idea to me. I personally would prefer it to ignore the id part of the JSONAPI request.

Admittedly this is also a bit of an inconsistency with the JSONAPI spec. An update as a PATCH /resources/1 MUST include the id field. An inconsistency between the URL id and the given id in the document is not handled. Maybe there is an edge case where you would want to edit the ID. However whether you want to or not, it has to be provided as a parameter.

Steps to reproduce (Code sample)

class MyResource < Struct.new(:id, :name)
  def self.find_by(id:)
     DB[id.to_i]
  end
  def save
    DB[id.to_i] = self
  end
end
DB = {1 => MyResource.new(1, 'one')} # Simulating database

class MyContract < Reform::Form
  property :name
end

class MyRepresenter < Roar::Decorator
  include Roar::JSON::JSONAPI.resource :resources
  attributes do
    property :name
  end
end

class MyOperation < Trailblazer::Operation
  step Model(MyResource, :find_by)
  step Contract::Build(constant: MyContract)
  step Contract::Validate(representer: MyRepresenter)
  step Contract::Persist()
end

MyOperation.({id: 1}, 'document' => '{"data":{"type":"resources","attributes":{"name":"changed"},"id":"9999"}}')
# => NoMethodError: undefined method `id=' for #<MyContract>

# Adding `property :id` to MyContract fixes the NoMethodError,
# but allows editing the ID field which is usually a security vulnerability:
MyOperation.({id: 1}, 'document' => '{"data":{"type":"resources","attributes":{"name":"changed"},"id":"9999"}}')
# => <Result:true ...>
DB
# => {1 => #<struct name="one">, 9999 => #<struct name="changed">}

Expected behavior

As stated, in edge cases this might be the actually desired behavior. In most cases one would not want an ID to be editable. So an opt-in would be preferable to me.

System configuration

Roar version: roar-edcd8efa0181
Roar JSONAPI version: roar-jsonapi-3ca7fa8e0f5a

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions