Merge pull request from GHSA-8639-qx56-r428

Fix CSRF vulnerability that allowed (un)finalizing adjustments
solidusio · Jun 1, 2022 · de796a2 · de796a2
2 parents 8dbb160 + bfb7bef
commit de796a2
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 5 deletions.
diff --git a/backend/app/views/spree/admin/adjustments/_adjustments_table.html.erb b/backend/app/views/spree/admin/adjustments/_adjustments_table.html.erb
@@ -13,12 +13,12 @@
     <tr data-hook="adjustment_buttons">
       <td class="align-right" colspan="2" style="width: 50%">
         <% if can? :update, Spree::Adjustment %>
-          <%= button_to t('spree.unfinalize_all_adjustments'), adjustments_unfinalize_admin_order_path(@order), method: :get %>
+          <%= button_to t('spree.unfinalize_all_adjustments'), adjustments_unfinalize_admin_order_path(@order), method: :put %>
         <% end %>
       </td>
       <td colspan="2" style="width: 50%">
         <% if can? :update, Spree::Adjustment %>
-          <%= button_to t('spree.finalize_all_adjustments'), adjustments_finalize_admin_order_path(@order), method: :get %>
+          <%= button_to t('spree.finalize_all_adjustments'), adjustments_finalize_admin_order_path(@order), method: :put %>
         <% end %>
       </td>
       <td class='actions'>&nbsp;</td>

diff --git a/backend/config/routes.rb b/backend/config/routes.rb
@@ -77,8 +77,8 @@
         get :confirm
         put :complete
         post :resend
-        get "/adjustments/unfinalize", to: "orders#unfinalize_adjustments"
-        get "/adjustments/finalize", to: "orders#finalize_adjustments"
+        put "/adjustments/unfinalize", to: "orders#unfinalize_adjustments"
+        put "/adjustments/finalize", to: "orders#finalize_adjustments"
         put :approve
         put :cancel
         put :resume
@@ -91,7 +91,7 @@
         end
       end
 
-      resources :adjustments
+      resources :adjustments, except: [:show]
       resources :return_authorizations do
         member do
           put :fire

diff --git a/backend/spec/features/admin/orders/adjustments_spec.rb b/backend/spec/features/admin/orders/adjustments_spec.rb
@@ -111,6 +111,44 @@
       end
     end
 
+    context "admin bulk editing adjustments" do
+      it "allows finalizing all the adjustments" do
+        order.all_adjustments.each(&:unfinalize!)
+
+        click_button "Finalize All Adjustments"
+
+        expect(order.reload.adjustments.all?(&:finalized?)).to be(true)
+      end
+
+      it "allows unfinalizing all the adjustments" do
+        order.all_adjustments.each(&:finalize!)
+
+        click_button "Unfinalize All Adjustments"
+
+        expect(order.reload.adjustments.any?(&:finalized?)).to be(false)
+      end
+
+      it "can't finalize via a GET request" do
+        order.all_adjustments.each(&:unfinalize!)
+
+        expect {
+          visit "/admin/orders/#{order.number}/adjustments/finalize"
+        }.to raise_error(ActionController::RoutingError)
+
+        expect(order.reload.adjustments.any?(&:finalized?)).to be(false)
+      end
+
+      it "can't unfinalize via a GET request" do
+        order.all_adjustments.each(&:finalize!)
+
+        expect {
+          visit "/admin/orders/#{order.number}/adjustments/unfinalize"
+        }.to raise_error(ActionController::RoutingError)
+
+        expect(order.reload.adjustments.all?(&:finalized?)).to be(true)
+      end
+    end
+
     context "deleting an adjustment" do
       context 'when the adjustment is finalized' do
         let!(:adjustment) { super().tap(&:finalize!) }