[v4.6] Fix guest-token fixation in OrdersController

app/controllers/spree/orders_controller.rb

@@ -6,14 +6,13 @@ class OrdersController < Spree::StoreController
 
     respond_to :html
 
-    before_action :store_guest_token
     before_action :assign_order, only: :update
     # note: do not lock the #edit action because that's where we redirect when we fail to acquire a lock
     around_action :lock_order, only: :update
 
     def show
       @order = Spree::Order.find_by!(number: params[:id])
-      authorize! :show, @order, cookies.signed[:guest_token]
+      authorize! :show, @order, params[:token] || cookies.signed[:guest_token]
     end
 
     def update
@@ -102,10 +101,6 @@ def accurate_title
 
     private
 
-    def store_guest_token
-      cookies.permanent.signed[:guest_token] = params[:token] if params[:token]
-    end
-
     def order_params
       if params[:order]
         params[:order].permit(*permitted_order_attributes)

spec/controllers/spree/orders_controller_ability_spec.rb

@@ -19,34 +19,35 @@ module Spree
 
       before do
         allow(controller).to receive_messages current_order: order
+        cookies.signed[:guest_token] = token
       end
 
       context '#populate' do
         it 'should check if user is authorized for :update' do
           expect(controller).to receive(:authorize!).with(:update, order, token)
-          post :populate, params: { variant_id: variant.id, token: token }
+          post :populate, params: { variant_id: variant.id }
         end
       end
 
       context '#edit' do
         it 'should check if user is authorized for :edit' do
           expect(controller).to receive(:authorize!).with(:edit, order, token)
-          get :edit, params: { token: token }
+          get :edit
         end
       end
 
       context '#update' do
         it 'should check if user is authorized for :update' do
           allow(order).to receive :update
           expect(controller).to receive(:authorize!).with(:update, order, token)
-          post :update, params: { order: { email: "foo@bar.com" }, token: token }
+          post :update, params: { order: { email: "foo@bar.com" } }
         end
       end
 
       context '#empty' do
         it 'should check if user is authorized for :update' do
           expect(controller).to receive(:authorize!).with(:update, order, token)
-          post :empty, params: { token: token }
+          post :empty
         end
       end
 
@@ -65,15 +66,15 @@ module Spree
 
       context '#show' do
         context 'when token parameter present' do
-          it 'always ooverride existing token when passing a new one' do
-            cookies.signed[:guest_token] = "soo wrong"
+          it 'authorizes the request against the supplied token' do
+            expect(controller).to receive(:authorize!).with(:show, kind_of(Spree::Order), order.guest_token)
             get :show, params: { id: 'R123', token: order.guest_token }
-            expect(cookies.signed[:guest_token]).to eq(order.guest_token)
           end
 
-          it 'should store as guest_token in session' do
+          it 'does not adopt the URL token as the guest_token cookie' do
+            cookies.signed[:guest_token] = 'existing-token'
             get :show, params: { id: 'R123', token: order.guest_token }
-            expect(cookies.signed[:guest_token]).to eq(order.guest_token)
+            expect(cookies.signed[:guest_token]).to eq('existing-token')
           end
         end