Merge pull request #6534 from andrewpbrett/sca-emails

Send email when SCA authorization is required for admin payment

app/assets/stylesheets/admin/shared/icons.scss

@@ -16,6 +16,7 @@
 }
 
 .icon-email:before    { @extend .icon-envelope:before }
+.icon-resend_authorization_email:before    { @extend .icon-envelope:before }
 .icon-resume:before   { @extend .icon-refresh:before  }
 
 .icon-cancel:before,

app/controllers/base_controller.rb

@@ -1,12 +1,14 @@
 # frozen_string_literal: true
 
 require 'spree/core/controller_helpers/order'
+require 'spree/core/controller_helpers/ssl'
 require 'open_food_network/tag_rule_applicator'
 
 class BaseController < ApplicationController
   layout 'darkswarm'
 
   include Spree::Core::ControllerHelpers::Order
+  include Spree::Core::ControllerHelpers::SSL
 
   include I18nHelper
   include OrderCyclesHelper

app/controllers/payments_controller.rb

@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+class PaymentsController < BaseController
+  ssl_required :redirect_to_authorize
+
+  respond_to :html
+
+  prepend_before_action :require_logged_in, only: :redirect_to_authorize
+
+  def redirect_to_authorize
+    @payment = Spree::Payment.find(params[:id])
+    authorize! :show, @payment.order
+
+    if url = @payment.cvv_response_message
+      redirect_to url
+    else
+      redirect_to order_url(@payment.order)
+    end
+  end
+
+  private
+
+  def require_logged_in
+    return if session[:access_token] || params[:token] || spree_current_user
+
+    flash[:error] = I18n.t("spree.orders.edit.login_to_view_order")
+    redirect_to main_app.root_path(anchor: "login?after_login=#{request.env['PATH_INFO']}")
+  end
+end

app/controllers/payments_controller.rb~aa3478fba... base authorization on the payment's order

@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+class PaymentsController < BaseController
+  ssl_required :redirect_to_authorize
+
+  respond_to :html
+
+  prepend_before_action :require_logged_in, only: :redirect_to_authorize
+
+  def redirect_to_authorize
+    @payment = Spree::Payment.find(params[:id])
+    authorize! :show, @payment.order
+
+    if url = @payment.cvv_response_message
+      redirect_to url
+    else
+      redirect_to order_url(@payment.order)
+    end
+  end
+
+  private
+
+  def require_logged_in
+    return if session[:access_token] || params[:token] || spree_current_user
+
+    flash[:error] = I18n.t("spree.orders.edit.login_to_view_order")
+    redirect_to main_app.root_path(anchor: "login?after_login=#{request.env['PATH_INFO']}")
+  end
+end

app/controllers/spree/admin/payments_controller.rb

@@ -56,7 +56,7 @@ module Spree
 
         # Because we have a transition method also called void, we do this to avoid conflicts.
         event = "void_transaction" if event == "void"
-        if @payment.public_send("#{event}!")
+        if allowed_events.include?(event) && @payment.public_send("#{event}!")
           flash[:success] = t(:payment_updated)
         else
           flash[:error] = t(:cannot_perform_operation)
@@ -153,10 +153,18 @@ module Spree
       def authorize_stripe_sca_payment
         return unless @payment.payment_method.class == Spree::Gateway::StripeSCA
 
-        @payment.authorize!
-        return if @payment.pending? && @payment.cvv_response_message.nil?
+        @payment.authorize!(spree.order_path(@payment.order, only_path: false))
 
-        raise Spree::Core::GatewayError, I18n.t('authorization_failure')
+        raise Spree::Core::GatewayError, I18n.t('authorization_failure') unless @payment.pending?
+
+        return unless @payment.cvv_response_message.present?
+
+        PaymentMailer.authorize_payment(@payment).deliver_later
+        raise Spree::Core::GatewayError, I18n.t('action_required')
+      end
+
+      def allowed_events
+        %w{capture void_transaction credit refund resend_authorization_email}
       end
     end
   end

app/mailers/payment_mailer.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+class PaymentMailer < Spree::BaseMailer
+  include I18nHelper
+
+  def authorize_payment(payment)
+    @payment = payment
+    subject = I18n.t('spree.payment_mailer.authorize_payment.subject',
+                     distributor: @payment.order.distributor.name)
+    mail(to: payment.order.user.email,
+         from: from_address,
+         subject: subject)
+  end
+end

app/models/spree/credit_card.rb

@@ -79,11 +79,17 @@ module Spree
     end
 
     def actions
-      %w{capture void credit}
+      %w{capture void credit resend_authorization_email}
+    end
+
+    def can_resend_authorization_email?(payment)
+      payment.pending? && payment.cvv_response_message.present?
     end
 
     # Indicates whether its possible to capture the payment
     def can_capture?(payment)
+      return false if payment.cvv_response_message.present?
+
       payment.pending? || payment.checkout?
     end
 

app/models/spree/gateway/stripe_sca.rb

@@ -45,6 +45,11 @@ module Spree
         failed_activemerchant_billing_response(e.message)
       end
 
+      def capture(money, payment_intent_id, gateway_options)
+        options = basic_options(gateway_options)
+        provider.capture(money, payment_intent_id, options)
+      end
+
       # NOTE: the name of this method is determined by Spree::Payment::Processing
       def charge_offline(money, creditcard, gateway_options)
         customer, payment_method =
@@ -110,7 +115,7 @@ module Spree
 
       def options_for_authorize(money, creditcard, gateway_options)
         options = basic_options(gateway_options)
-        options[:return_url] = full_checkout_path
+        options[:return_url] = gateway_options[:return_url] || full_checkout_path
 
         customer_id, payment_method_id =
           Stripe::CreditCardCloner.new(creditcard, stripe_account_id).find_or_clone

app/models/spree/payment.rb

@@ -121,6 +121,12 @@ module Spree
       actions
     end
 
+    def resend_authorization_email!
+      return unless cvv_response_message.present?
+
+      PaymentMailer.authorize_payment(self).deliver_later
+    end
+
     def payment_source
       res = source.is_a?(Payment) ? source.source : source
       res || payment_method

app/models/spree/payment/processing.rb

@@ -5,6 +5,7 @@ module Spree
     module Processing
       def process!
         return unless validate!
+        return if cvv_response_message.present?
 
         if payment_method.auto_capture?
           purchase!
@@ -15,6 +16,7 @@ module Spree
 
       def process_offline!
         return unless validate!
+        return if cvv_response_message.present?
 
         if payment_method.auto_capture?
           charge_offline!
@@ -23,9 +25,9 @@ module Spree
         end
       end
 
-      def authorize!
+      def authorize!(return_url = nil)
         started_processing!
-        gateway_action(source, :authorize, :pend)
+        gateway_action(source, :authorize, :pend, return_url: return_url)
       end
 
       def purchase!
@@ -44,19 +46,7 @@ module Spree
         started_processing!
         protect_from_connection_error do
           check_environment
-
-          response = if payment_method.payment_profiles_supported?
-                       # Gateways supporting payment profiles will need access to credit
-                       # card object because this stores the payment profile information
-                       # so supply the authorization itself as well as the credit card,
-                       # rather than just the authorization code
-                       payment_method.capture(self, source, gateway_options)
-                     else
-                       # Standard ActiveMerchant capture usage
-                       payment_method.capture(money.money.cents,
-                                              response_code,
-                                              gateway_options)
-                     end
+          response = payment_method.capture(money.money.cents, response_code, gateway_options)
 
           handle_response(response, :complete, :failure)
         end
@@ -222,7 +212,7 @@ module Spree
         refund_amount.to_f
       end
 
-      def gateway_action(source, action, success_state)
+      def gateway_action(source, action, success_state, options = {})
         protect_from_connection_error do
           check_environment
 
@@ -230,7 +220,7 @@ module Spree
             action,
             (amount * 100).round,
             source,
-            gateway_options
+            gateway_options.merge(options)
           )
           handle_response(response, success_state, :failure)
         end

app/views/payment_mailer/authorize_payment.html.haml

@@ -0,0 +1,2 @@
+= t('.instructions', distributor: @payment.order.distributor.name, amount: @payment.display_amount)
+= main_app.authorize_payment_url(@payment)

app/views/payment_mailer/authorize_payment.text.haml

@@ -0,0 +1,3 @@
+= t('.instructions', distributor: @payment.order.distributor.name, amount: @payment.display_amount)
+
+= main_app.authorize_payment_url(@payment)

config/locales/en.yml

@@ -2455,6 +2455,8 @@ See the %{link} to find out more about %{sitename}'s features and to start using
   payment_processing_failed: "Payment could not be processed, please check the details you entered"
   payment_method_not_supported: "That payment method is unsupported. Please choose another one."
   payment_updated: "Payment Updated"
+  cannot_perform_operation: "Could not update the payment"
+  action_required: "Action required"
   inventory_settings: "Inventory Settings"
   tag_rules: "Tag Rules"
   shop_preferences: "Shop Preferences"
@@ -3646,6 +3648,10 @@ See the %{link} to find out more about %{sitename}'s features and to start using
         subject: "Reset password instructions"
       confirmation_instructions:
         subject: "Please confirm your OFN account"
+    payment_mailer:
+      authorize_payment:
+        subject: "Please authorize your payment to %{distributor} on OFN"
+        instructions: "Your payment of %{amount} to %{distributor} requires additional authentication. Please visit the following URL to authorize your payment:"
     shipment_mailer:
       shipped_email:
         dear_customer: "Dear Customer,"

config/routes.rb

@@ -29,6 +29,7 @@ Openfoodnetwork::Application.routes.draw do
   patch "/cart", :to => "spree/orders#update", :as => :update_cart
   put "/cart/empty", :to => 'spree/orders#empty', :as => :empty_cart
   get '/orders/:id/token/:token' => 'spree/orders#show', :as => :token_order
+  get '/payments/:id/authorize' => 'payments#redirect_to_authorize', as: "authorize_payment"
 
   resource :cart, controller: "cart" do
     post :populate

lib/stripe/authorize_response_patcher.rb

@@ -24,7 +24,8 @@ module Stripe
                     next_action.present? &&
                     next_action["type"] == "authorize_with_url"
 
-      next_action["authorize_with_url"]["url"]
+      url = next_action["authorize_with_url"]["url"]
+      return url if url.match(%r{https?:\/\/[\S]+}) && url.include?("stripe.com")
     end
 
     # This field is used because the Spree code recognizes and stores it

spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb

@@ -91,6 +91,20 @@ describe Spree::Admin::PaymentsController, type: :controller do
           end
         end
 
+        context "where further action is required" do
+          before do
+            allow_any_instance_of(Spree::Payment).to receive(:authorize!) do |payment|
+              payment.update cvv_response_message: "https://www.stripe.com/authorize"
+              payment.update state: "pending"
+            end
+          end
+          it "redirects to new payment page with flash error" do
+            spree_post :create, payment: params, order_id: order.number
+
+            redirects_to_new_payment_page_with_flash_error(I18n.t('action_required'))
+          end
+        end
+
         context "where both payment.process! and payment.authorize! work" do
           before do
             allow_any_instance_of(Spree::Payment).to receive(:authorize!) do |payment|
@@ -227,5 +241,41 @@ describe Spree::Admin::PaymentsController, type: :controller do
         expect(flash[:success]).to eq(I18n.t(:payment_updated))
       end
     end
+
+    context 'on resend_authorization_email event' do
+      let(:params) { { e: 'resend_authorization_email', order_id: order.number, id: payment.id } }
+      let(:mail_mock) { double(:mailer_mock, deliver_later: true) }
+
+      before do
+        allow(PaymentMailer).to receive(:authorize_payment) { mail_mock }
+        allow(request).to receive(:referer) { 'http://foo.com' }
+        allow(Spree::Payment).to receive(:find).with(payment.id.to_s) { payment }
+        allow(payment).to receive(:cvv_response_message).and_return("https://www.stripe.com/authorize")
+      end
+
+      it "resends the authorization email" do
+        spree_put :fire, params
+
+        expect(flash[:success]).to eq(I18n.t(:payment_updated))
+        expect(PaymentMailer).to have_received(:authorize_payment)
+        expect(mail_mock).to have_received(:deliver_later)
+      end
+    end
+
+    context 'on an unrecognized event' do
+      let(:params) { { e: 'unrecognized_event', order_id: order.number, id: payment.id } }
+
+      before do
+        allow(request).to receive(:referer) { 'http://foo.com' }
+        allow(Spree::Payment).to receive(:find).with(payment.id.to_s) { payment }
+      end
+
+      it 'does not process the event' do
+        spree_put :fire, params
+
+        expect(payment).to_not receive(:unrecognized_event)
+        expect(flash[:error]).to eq(I18n.t(:cannot_perform_operation))
+      end
+    end
   end
 end

spec/features/admin/payments_stripe_spec.rb

@@ -70,7 +70,7 @@ feature '
       context "with a card that fails on registration because it requires(redirects) extra auth" do
         before do
           stub_payment_intents_post_request_with_redirect order: order,
-                                                          redirect_url: "www.dummy.org"
+                                                          redirect_url: "https://www.stripe.com/authorize"
         end
 
         it "fails to add a payment due to card error" do

spec/lib/stripe/authorize_response_patcher_spec.rb

@@ -20,12 +20,12 @@ module Stripe
         let(:params) {
           { "status" => "requires_source_action",
             "next_source_action" => { "type" => "authorize_with_url",
-                                      "authorize_with_url" => { "url" => "test_url" } } }
+                                      "authorize_with_url" => { "url" => "https://www.stripe.com/authorize" } } }
         }
 
         it "patches response.cvv_result.message with the url in the response" do
           new_response = patcher.call!
-          expect(new_response.cvv_result['message']).to eq "test_url"
+          expect(new_response.cvv_result['message']).to eq "https://www.stripe.com/authorize"
         end
       end
     end

spec/models/spree/payment_spec.rb

@@ -83,25 +83,25 @@ describe Spree::Payment do
       context "#process!" do
         it "should purchase if with auto_capture" do
           payment = build_stubbed(:payment, payment_method: gateway)
-          payment.payment_method.should_receive(:auto_capture?).and_return(true)
-          payment.should_receive(:purchase!)
+          expect(payment.payment_method).to receive(:auto_capture?).and_return(true)
+          expect(payment).to receive(:purchase!)
           payment.process!
         end
 
         it "should authorize without auto_capture" do
           payment = build_stubbed(:payment, payment_method: gateway)
-          payment.payment_method.should_receive(:auto_capture?).and_return(false)
-          payment.should_receive(:authorize!)
+          expect(payment.payment_method).to receive(:auto_capture?).and_return(false)
+          expect(payment).to receive(:authorize!)
           payment.process!
         end
 
         it "should make the state 'processing'" do
-          payment.should_receive(:started_processing!)
+          expect(payment).to receive(:started_processing!)
           payment.process!
         end
 
         it "should invalidate if payment method doesnt support source" do
-          payment.payment_method.should_receive(:supports?).with(payment.source).and_return(false)
+          expect(payment.payment_method).to receive(:supports?).with(payment.source).and_return(false)
           expect { payment.process! }.to raise_error(Spree::Core::GatewayError)
           expect(payment.state).to eq('invalid')
         end
@@ -109,17 +109,17 @@ describe Spree::Payment do
 
       context "#authorize" do
         it "should call authorize on the gateway with the payment amount" do
-          payment.payment_method.should_receive(:authorize).with(amount_in_cents,
-                                                                 card,
-                                                                 anything).and_return(success_response)
+          expect(payment.payment_method).to receive(:authorize).with(
+            amount_in_cents, card, anything
+          ).and_return(success_response)
           payment.authorize!
         end
 
         it "should call authorize on the gateway with the currency code" do
           payment.stub currency: 'GBP'
-          payment.payment_method.should_receive(:authorize).with(amount_in_cents,
-                                                                 card,
-                                                                 hash_including({ currency: "GBP" })).and_return(success_response)
+          expect(payment.payment_method).to receive(:authorize).with(
+            amount_in_cents, card, hash_including({ currency: "GBP" })
+          ).and_return(success_response)
           payment.authorize!
         end
 
@@ -137,9 +137,9 @@ describe Spree::Payment do
 
         context "if successful" do
           before do
-            payment.payment_method.should_receive(:authorize).with(amount_in_cents,
-                                                                   card,
-                                                                   anything).and_return(success_response)
+            expect(payment.payment_method).to receive(:authorize).with(
+              amount_in_cents, card, anything
+            ).and_return(success_response)
           end
 
           it "should store the response_code, avs_response and cvv_response fields" do
@@ -151,7 +151,7 @@ describe Spree::Payment do
           end
 
           it "should make payment pending" do
-            payment.should_receive(:pend!)
+            expect(payment).to receive(:pend!)
             payment.authorize!
           end
         end
@@ -159,8 +159,8 @@ describe Spree::Payment do
         context "if unsuccessful" do
           it "should mark payment as failed" do
             gateway.stub(:authorize).and_return(failed_response)
-            payment.should_receive(:failure)
-            payment.should_not_receive(:pend)
+            expect(payment).to receive(:failure)
+            expect(payment).to_not receive(:pend)
             expect {
               payment.authorize!
             }.to raise_error(Spree::Core::GatewayError)
@@ -170,7 +170,7 @@ describe Spree::Payment do
 
       context "purchase" do
         it "should call purchase on the gateway with the payment amount" do
-          gateway.should_receive(:purchase).with(amount_in_cents, card, anything).and_return(success_response)
+          expect(gateway).to receive(:purchase).with(amount_in_cents, card, anything).and_return(success_response)
           payment.purchase!
         end
 
@@ -188,9 +188,9 @@ describe Spree::Payment do
 
         context "if successful" do
           before do
-            payment.payment_method.should_receive(:purchase).with(amount_in_cents,
-                                                                  card,
-                                                                  anything).and_return(success_response)
+            expect(payment.payment_method).to receive(:purchase).with(
+              amount_in_cents, card, anything
+            ).and_return(success_response)
           end
 
           it "should store the response_code and avs_response" do
@@ -200,7 +200,7 @@ describe Spree::Payment do
           end
 
           it "should make payment complete" do
-            payment.should_receive(:complete!)
+            expect(payment).to receive(:complete!)
             payment.purchase!
           end
         end
@@ -208,8 +208,8 @@ describe Spree::Payment do
         context "if unsuccessful" do
           it "should make payment failed" do
             gateway.stub(:purchase).and_return(failed_response)
-            payment.should_receive(:failure)
-            payment.should_not_receive(:pend)
+            expect(payment).to receive(:failure)
+            expect(payment).not_to receive(:pend)
             expect { payment.purchase! }.to raise_error(Spree::Core::GatewayError)
           end
         end
@@ -227,11 +227,11 @@ describe Spree::Payment do
 
           context "if successful" do
             before do
-              payment.payment_method.should_receive(:capture).with(payment, card, anything).and_return(success_response)
+              expect(payment.payment_method).to receive(:capture).and_return(success_response)
             end
 
             it "should make payment complete" do
-              payment.should_receive(:complete)
+              expect(payment).to receive(:complete)
               payment.capture!
             end
 
@@ -245,8 +245,8 @@ describe Spree::Payment do
           context "if unsuccessful" do
             it "should not make payment complete" do
               gateway.stub capture: failed_response
-              payment.should_receive(:failure)
-              payment.should_not_receive(:complete)
+              expect(payment).to receive(:failure)
+              expect(payment).to_not receive(:complete)
               expect { payment.capture! }.to raise_error(Spree::Core::GatewayError)
             end
           end
@@ -256,9 +256,9 @@ describe Spree::Payment do
         context "when payment is completed" do
           it "should do nothing" do
             payment = build_stubbed(:payment, state: 'completed')
-            payment.should_not_receive(:complete)
-            payment.payment_method.should_not_receive(:capture)
-            payment.log_entries.should_not_receive(:create)
+            expect(payment).to_not receive(:complete)
+            expect(payment.payment_method).to_not receive(:capture)
+            expect(payment.log_entries).to_not receive(:create)
             payment.capture!
           end
         end
@@ -273,7 +273,7 @@ describe Spree::Payment do
         context "when profiles are supported" do
           it "should call payment_gateway.void with the payment's response_code" do
             gateway.stub payment_profiles_supported?: true
-            gateway.should_receive(:void).with('123', card, anything).and_return(success_response)
+            expect(gateway).to receive(:void).with('123', card, anything).and_return(success_response)
             payment.void_transaction!
           end
         end
@@ -281,7 +281,7 @@ describe Spree::Payment do
         context "when profiles are not supported" do
           it "should call payment_gateway.void with the payment's response_code" do
             gateway.stub payment_profiles_supported?: false
-            gateway.should_receive(:void).with('123', anything).and_return(success_response)
+            expect(gateway).to receive(:void).with('123', anything).and_return(success_response)
             payment.void_transaction!
           end
         end
@@ -311,7 +311,7 @@ describe Spree::Payment do
         context "if unsuccessful" do
           it "should not void the payment" do
             gateway.stub void: failed_response
-            payment.should_not_receive(:void)
+            expect(payment).to_not receive(:void)
             expect { payment.void_transaction! }.to raise_error(Spree::Core::GatewayError)
           end
         end
@@ -320,7 +320,7 @@ describe Spree::Payment do
         context "if payment is already voided" do
           it "should not void the payment" do
             payment = build_stubbed(:payment, payment_method: gateway, state: 'void')
-            payment.payment_method.should_not_receive(:void)
+            expect(payment.payment_method).to_not receive(:void)
             payment.void_transaction!
           end
         end
@@ -339,7 +339,7 @@ describe Spree::Payment do
           end
 
           it "should call credit on the gateway with the credit amount and response_code" do
-            gateway.should_receive(:credit).with(1000, card, '123', anything).and_return(success_response)
+            expect(gateway).to receive(:credit).with(1000, card, '123', anything).and_return(success_response)
             payment.credit!
           end
         end
@@ -350,7 +350,9 @@ describe Spree::Payment do
           end
 
           it "should call credit on the gateway with the credit amount and response_code" do
-            gateway.should_receive(:credit).with(amount_in_cents, card, '123', anything).and_return(success_response)
+            expect(gateway).to receive(:credit).with(
+              amount_in_cents, card, '123', anything
+            ).and_return(success_response)
             payment.credit!
           end
         end
@@ -361,7 +363,9 @@ describe Spree::Payment do
           end
 
           it "should call credit on the gateway with the original payment amount and response_code" do
-            gateway.should_receive(:credit).with(amount_in_cents.to_f, card, '123', anything).and_return(success_response)
+            expect(gateway).to receive(:credit).with(
+              amount_in_cents.to_f, card, '123', anything
+            ).and_return(success_response)
             payment.credit!
           end
         end
@@ -431,8 +435,8 @@ describe Spree::Payment do
         payment = build_stubbed(:payment)
         payment.state = 'processing'
 
-        payment.should_not_receive(:authorize!)
-        payment.should_not_receive(:purchase!)
+        expect(payment).to_not receive(:authorize!)
+        expect(payment).to_not receive(:purchase!)
         expect(payment.process!).to be_nil
       end
     end
@@ -496,7 +500,7 @@ describe Spree::Payment do
         it "calls credit on the source with the payment and amount" do
           payment.state = 'completed'
           payment.stub(:credit_allowed).and_return(10)
-          payment.should_receive(:credit!).with(10)
+          expect(payment).to receive(:credit!).with(10)
           payment.partial_credit(10)
         end
       end
@@ -519,7 +523,7 @@ describe Spree::Payment do
 
         order = create(:order)
         payment = Spree::Payment.create(amount: 100, order: order, payment_method: gateway)
-        order.should_receive(:update!)
+        expect(order).to receive(:update!)
         payment.save
       end