Merge commit from fork

damianlegawiec · damianlegawiec · commit 3e00be64c128 · 2026-02-05T14:31:26.000+01:00
The OrdersController#show action permitted viewing completed guest
orders by order number alone, without requiring the associated order
token.
diff --git a/storefront/app/controllers/spree/order_status_controller.rb b/storefront/app/controllers/spree/order_status_controller.rb
@@ -7,7 +7,7 @@ def new; end
     # validate email/order number and redirect to order page
     # POST /order_status
     def create
-      raise ActiveRecord::RecordNotFound if params[:number].blank?
+      raise ActiveRecord::RecordNotFound if params[:number].blank? || params[:email].blank?
 
       @order = order_finder.new(number: params[:number], email: params[:email], store: current_store).execute.first
 
diff --git a/storefront/app/controllers/spree/orders_controller.rb b/storefront/app/controllers/spree/orders_controller.rb
@@ -10,11 +10,17 @@ class OrdersController < StoreController
 
     before_action :assign_order_with_lock, only: :update
 
+    rescue_from CanCan::AccessDenied do |exception|
+      raise ActiveRecord::RecordNotFound
+    end
+
     # GET /orders/:id
     def show
       @order = complete_order_finder.new(number: params[:id], token: params[:token], store: current_store).execute.first
 
-      raise ActiveRecord::RecordNotFound if @order.blank? || !authorize_access
+      raise ActiveRecord::RecordNotFound if @order.blank?
+
+      authorize! :show, @order, params[:token]
 
       @shipments = @order.shipments.includes(:stock_location, :address, selected_shipping_rate: :shipping_method, inventory_units: :line_item)
     end
@@ -49,9 +55,7 @@ def edit
     private
 
     def authorize_access
-      return true if @order.user_id.nil?
-
-      @order.user == try_spree_current_user
+      authorize! :show, @order, params[:token]
     end
 
     def find_order_by_cookie
@@ -65,7 +69,7 @@ def find_order_by_cookie
     end
 
     def accurate_title
-      if action_name == 'edit' || action_name == 'update'
+      if ['edit', 'update'].include?(action_name)
         Spree.t(:shopping_cart)
       else
         Spree.t(:order_number, number: @order&.number)
diff --git a/storefront/spec/controllers/spree/order_status_controller_spec.rb b/storefront/spec/controllers/spree/order_status_controller_spec.rb
@@ -64,5 +64,13 @@
         }.to raise_error(ActiveRecord::RecordNotFound)
       end
     end
+
+    context 'with blank email' do
+      it 'raises ActiveRecord::RecordNotFound' do
+        expect {
+          post :create, params: { number: order.number, email: '' }
+        }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
   end
 end
diff --git a/storefront/spec/controllers/spree/orders_controller_spec.rb b/storefront/spec/controllers/spree/orders_controller_spec.rb
@@ -105,7 +105,8 @@
   end
 
   describe '#show' do
-    let(:order) { create(:completed_order_with_totals, store: store, user: user) }
+    let(:order) { create(:completed_order_with_totals, store: store) }
+    let(:user) { nil }
 
     it 'renders the show template' do
       get :show, params: { id: order.number, token: order.token }
@@ -120,13 +121,15 @@
       end
     end
 
-    context 'when order belongs to another user' do
-      let(:order) { create(:completed_order_with_totals, store: store, user: create(:user)) }
+    context 'when token is invalid' do
+      it 'raises ActiveRecord::RecordNotFound' do
+        expect { get :show, params: { id: order.number, token: 'invalid' } }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
 
+    context 'when token is missing' do
       it 'raises ActiveRecord::RecordNotFound' do
-        expect do
-          get :show, params: { id: order.number, token: order.token }
-        end.to raise_error(ActiveRecord::RecordNotFound)
+        expect { get :show, params: { id: order.number, token: '' } }.to raise_error(ActiveRecord::RecordNotFound)
       end
     end
   end
