Merge pull request openfoodfoundation#7562 from coopdevs/fix-failed-s…

…ca-auth

Fix failed SCA authentication on subs. order payment not reflected in OFN

app/controllers/spree/orders_controller.rb

@@ -26,8 +26,14 @@ class OrdersController < ::BaseController
 
     def show
       @order = Spree::Order.find_by!(number: params[:id])
-      ProcessPaymentIntent.new(params["payment_intent"], @order).call!
-      @order.reload
+
+      if params.key?("payment_intent")
+        result = ProcessPaymentIntent.new(params["payment_intent"], @order).call!
+        unless result.ok?
+          flash[:error] = "#{I18n.t("payment_could_not_process")}. #{result.error}"
+        end
+        @order.reload
+      end
     end
 
     def empty

app/models/spree/payment.rb

@@ -144,6 +144,10 @@ def adjustment_label
       I18n.t('payment_method_fee')
     end
 
+    def mark_as_processed
+      update_attribute(:cvv_response_message, nil)
+    end
+
     private
 
     # Don't charge fees for invalid or failed payments.

app/services/process_payment_intent.rb

@@ -3,39 +3,72 @@
 # When directing a customer to Stripe to authorize the payment, we specify a
 # redirect_url that Stripe should return them to. When checking out, it's
 # /checkout; for admin payments and subscription payemnts it's the order url.
+#
 # This class confirms that the payment intent matches what's in our database,
 # marks the payment as complete, and removes the cvv_response_message field,
 # which we use to indicate that authorization is required. It also completes the
 # Order, if appropriate.
 
 class ProcessPaymentIntent
-  def initialize(payment_intent, order)
+  class Result
+    attr_reader :error
+
+    def initialize(ok:, error: "")
+      @ok = ok
+      @error = error
+    end
+
+    def ok?
+      @ok
+    end
+  end
+
+  def initialize(payment_intent, order, last_payment = nil)
     @payment_intent = payment_intent
     @order = order
-    @last_payment = OrderPaymentFinder.new(order).last_payment
+    @last_payment = last_payment.presence || OrderPaymentFinder.new(order).last_payment
   end
 
   def call!
-    return unless valid?
+    validate_intent!
+    return Result.new(ok: false) unless valid?
+
+    OrderWorkflow.new(order).next
+
+    if last_payment.can_complete?
+      last_payment.complete!
+      last_payment.mark_as_processed
 
-    last_payment.update_attribute(:cvv_response_message, nil)
-    OrderWorkflow.new(@order).next
-    last_payment.complete! if last_payment.can_complete?
+      Result.new(ok: true)
+    else
+      Result.new(ok: false, error: I18n.t("payment_could_not_complete"))
+    end
+
+  rescue Stripe::StripeError => e
+    Result.new(ok: false, error: e.message)
   end
 
   private
 
   attr_reader :order, :payment_intent, :last_payment
 
   def valid?
-    order.present? && valid_intent_string? && matches_last_payment?
+    order.present? && matches_last_payment?
   end
 
-  def valid_intent_string?
-    payment_intent&.starts_with?("pi_")
+  def validate_intent!
+    Stripe::PaymentIntentValidator.new.call(payment_intent, stripe_account_id)
   end
 
   def matches_last_payment?
     last_payment&.state == "pending" && last_payment&.response_code == payment_intent
   end
+
+  def stripe_account_id
+    StripeAccount.find_by(enterprise_id: preferred_enterprise_id).stripe_user_id
+  end
+
+  def preferred_enterprise_id
+    last_payment.payment_method.preferred_enterprise_id
+  end
 end

config/locales/en.yml

@@ -349,6 +349,8 @@ en:
   height: "Height"
   width: "Width"
   depth: "Depth"
+  payment_could_not_process: "The payment could not be processed"
+  payment_could_not_complete: "The payment could not be completed"
 
   actions:
     create_and_add_another: "Create and Add Another"

spec/controllers/spree/orders_controller_spec.rb

@@ -83,8 +83,10 @@
   describe "confirming a payment intent" do
     let(:customer) { create(:customer) }
     let(:order) { create(:order, customer: customer, distributor: customer.enterprise) }
+    let(:payment_method) { create(:stripe_sca_payment_method) }
     let!(:payment) { create(
       :payment,
+      payment_method: payment_method,
       cvv_response_message: "https://stripe.com/redirect",
       response_code: "pi_123",
       order: order,
@@ -101,8 +103,16 @@
       context "with a valid payment intent" do
         let(:payment_intent) { "pi_123" }
 
+        before do
+          allow_any_instance_of(Stripe::PaymentIntentValidator)
+            .to receive(:call)
+            .with(payment_intent, anything)
+            .and_return(payment_intent)
+        end
+
         it "completes the payment" do
           get :show, params: { id: order.number, payment_intent: payment_intent }
+
           expect(response).to be_success
           payment.reload
           expect(payment.cvv_response_message).to be nil
@@ -112,9 +122,37 @@
 
       context "with an invalid payment intent" do
         let(:payment_intent) { "invalid" }
+        let(:result) { instance_double(ProcessPaymentIntent::Result) }
+
+        before do
+          allow_any_instance_of(Stripe::PaymentIntentValidator)
+            .to receive(:call)
+            .with(payment_intent, anything)
+            .and_return(result)
+        end
+
+        it "does not complete the payment" do
+          get :show, params: { id: order.number, payment_intent: payment_intent }
+
+          expect(response).to be_success
+          payment.reload
+          expect(payment.cvv_response_message).to eq("https://stripe.com/redirect")
+          expect(payment.state).to eq("pending")
+        end
+      end
+
+      context "with an invalid last payment" do
+        let(:payment_intent) { "valid" }
+        let(:finder) { instance_double(OrderPaymentFinder, last_payment: payment) }
+
+        before do
+          allow(payment).to receive(:response_code).and_return("invalid")
+          allow(OrderPaymentFinder).to receive(:new).with(order).and_return(finder)
+        end
 
         it "does not complete the payment" do
           get :show, params: { id: order.number, payment_intent: payment_intent }
+
           expect(response).to be_success
           payment.reload
           expect(payment.cvv_response_message).to eq("https://stripe.com/redirect")

spec/models/spree/payment_spec.rb

@@ -926,4 +926,13 @@
       end
     end
   end
+
+  describe "#mark_as_processed" do
+    let(:payment) { create(:payment, cvv_response_message: "message") }
+
+    it "removes the cvv_response_message" do
+      payment.mark_as_processed
+      expect(payment.cvv_response_message).to eq(nil)
+    end
+  end
 end

spec/services/process_payment_intent_spec.rb

@@ -8,17 +8,36 @@
   describe "processing a payment intent" do
     let(:customer) { create(:customer) }
     let(:order) { create(:order, customer: customer, distributor: customer.enterprise, state: "payment") }
+    let(:payment_method) { create(:stripe_sca_payment_method) }
     let!(:payment) { create(
       :payment,
+      payment_method: payment_method,
       cvv_response_message: "https://stripe.com/redirect",
       response_code: "pi_123",
       order: order,
       state: "pending")
     }
+    let(:validator) { instance_double(Stripe::PaymentIntentValidator) }
+
+    before do
+      allow(Stripe::PaymentIntentValidator).to receive(:new).and_return(validator)
+    end
 
     context "an invalid intent" do
-      let(:invalid_intent) { "invalid" }
-      let(:service) { ProcessPaymentIntent.new(invalid_intent, order) }
+      let(:intent) { "invalid" }
+      let(:service) { ProcessPaymentIntent.new(intent, order) }
+
+      before do
+        allow(validator)
+          .to receive(:call).with(intent, anything).and_raise(Stripe::StripeError, "error message")
+      end
+
+      it "returns the error message" do
+        result = service.call!
+
+        expect(result.ok?).to eq(false)
+        expect(result.error).to eq("error message")
+      end
 
       it "does not complete the payment" do
         service.call!
@@ -27,11 +46,17 @@
     end
 
     context "a valid intent" do
-      let(:valid_intent) { "pi_123" }
-      let(:service) { ProcessPaymentIntent.new(valid_intent, order) }
+      let(:intent) { "pi_123" }
+      let(:service) { ProcessPaymentIntent.new(intent, order) }
 
       before do
         allow(order).to receive(:deliver_order_confirmation_email)
+        allow(validator).to receive(:call).with(intent, anything).and_return(intent) 
+      end
+
+      it "validates the intent" do
+        service.call!
+        expect(validator).to have_received(:call)
       end
 
       it "completes the payment" do
@@ -49,17 +74,47 @@
     end
 
     context "payment is in a failed state" do
-      let(:invalid_intent) { "invalid" }
-      let(:service) { ProcessPaymentIntent.new(invalid_intent, order) }
+      let(:intent) { "valid" }
+      let(:service) { ProcessPaymentIntent.new(intent, order) }
 
       before do
         payment.update_attribute(:state, "failed")
+        allow(validator).to receive(:call).with(intent, anything).and_return(intent)
+      end
+
+      it "does not return any error message" do
+        result = service.call!
+
+        expect(result.ok?).to eq(false)
+        expect(result.error).to eq("")
       end
 
       it "does not complete the payment" do
         service.call!
         expect(payment.reload.state).to eq("failed")
       end
     end
+
+    context "when the payment can't be completed" do
+      let(:intent) { "pi_123" }
+      let(:service) { ProcessPaymentIntent.new(intent, order, payment) }
+
+      before do
+        allow(payment).to receive(:can_complete?).and_return(false)
+        allow(validator).to receive(:call).with(intent, anything).and_return(intent)
+      end
+
+      it "returns a failed result" do
+        result = service.call!
+
+        expect(result.ok?).to eq(false)
+        expect(result.error).to eq(I18n.t("payment_could_not_complete"))
+      end
+
+      it "does not complete the payment" do
+        service.call!
+        expect(payment.reload.state).to eq("pending")
+      end
+    end
   end
 end