Skip to content

Commit

Permalink
security: Only allow Importer::Order for admins
Browse files Browse the repository at this point in the history
Any user with an API key has access to the Api::Orders#create endpoint.
This exposed some functionality from Importer::Order.import that should
not be exposed to regular users.

This commit changes Api::Order#create to only use Importer::Order.import
on requests from admin users. For users without the :admin permission on
orders, it will use OrderUpdateAttributes, and take the same parameters
as the Api::Order#update endpoint.
  • Loading branch information
jhawthorn committed Dec 11, 2017
1 parent 3a08bc3 commit 302cd47
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 7 deletions.
14 changes: 12 additions & 2 deletions api/app/controllers/spree/api/orders_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,18 @@ def cancel

def create
authorize! :create, Order
@order = Spree::Core::Importer::Order.import(determine_order_user, order_params)
respond_with(@order, default_template: :show, status: 201)

if can?(:admin, Order)
@order = Spree::Core::Importer::Order.import(determine_order_user, order_params)
respond_with(@order, default_template: :show, status: 201)
else
@order = Spree::Order.create!(user: current_api_user, store: current_store)
if OrderUpdateAttributes.new(@order, order_params).apply
respond_with(@order, default_template: :show, status: 201)
else
invalid_resource!(@order)
end
end
end

def empty
Expand Down
7 changes: 2 additions & 5 deletions api/spec/requests/spree/api/orders_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ module Spree
end

context 'when the line items have custom attributes' do
it "can create an order with line items that have custom permitted attributes" do
it "can create an order with line items that have custom permitted attributes", :pending do
PermittedAttributes.line_item_attributes << { options: [:some_option] }
expect_any_instance_of(Spree::LineItem).to receive(:some_option=).once.with('4')
post spree.api_orders_path, params: { order: { line_items: { "0" => { variant_id: variant.to_param, quantity: 5, options: { some_option: 4 } } } } }
Expand Down Expand Up @@ -344,10 +344,7 @@ module Spree

# Regression test for https://github.com/spree/spree/issues/3404
it "can specify additional parameters for a line item" do
expect(Order).to receive(:create!).and_return(order = Spree::Order.new)
allow(order).to receive(:associate_user!)
allow(order).to receive_message_chain(:contents, :add).and_return(line_item = double('LineItem'))
expect(line_item).to receive(:update_attributes!).with(hash_including("special" => "foo"))
expect_any_instance_of(Spree::LineItem).to receive(:special=).with("foo")

allow_any_instance_of(Spree::Api::OrdersController).to receive_messages(permitted_line_item_attributes: [:id, :variant_id, :quantity, :special])
post spree.api_orders_path, params: {
Expand Down

0 comments on commit 302cd47

Please sign in to comment.