diff --git a/app/assets/stylesheets/admin/shared/icons.scss b/app/assets/stylesheets/admin/shared/icons.scss index 04a3fb74c0..a2de27ba99 100644 --- a/app/assets/stylesheets/admin/shared/icons.scss +++ b/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, diff --git a/app/controllers/base_controller.rb b/app/controllers/base_controller.rb index 143370f3e9..8ea4718cb5 100644 --- a/app/controllers/base_controller.rb +++ b/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 diff --git a/app/controllers/payments_controller.rb b/app/controllers/payments_controller.rb new file mode 100644 index 0000000000..c0fb7eadcc --- /dev/null +++ b/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 diff --git a/app/controllers/payments_controller.rb~aa3478fba... base authorization on the payment's order b/app/controllers/payments_controller.rb~aa3478fba... base authorization on the payment's order new file mode 100644 index 0000000000..c0fb7eadcc --- /dev/null +++ b/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 diff --git a/app/controllers/spree/admin/payments_controller.rb b/app/controllers/spree/admin/payments_controller.rb index 5115a9d553..af913d50a8 100644 --- a/app/controllers/spree/admin/payments_controller.rb +++ b/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 diff --git a/app/mailers/payment_mailer.rb b/app/mailers/payment_mailer.rb new file mode 100644 index 0000000000..b0c4a4bbff --- /dev/null +++ b/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 diff --git a/app/models/spree/credit_card.rb b/app/models/spree/credit_card.rb index 948dcf829d..2a10e78794 100644 --- a/app/models/spree/credit_card.rb +++ b/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 diff --git a/app/models/spree/gateway/stripe_sca.rb b/app/models/spree/gateway/stripe_sca.rb index a3ccde45b2..351adf019d 100644 --- a/app/models/spree/gateway/stripe_sca.rb +++ b/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 diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index 578b91e3e5..3bd1bcd4a3 100644 --- a/app/models/spree/payment.rb +++ b/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 diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb index 36704362a8..1648b0ce36 100644 --- a/app/models/spree/payment/processing.rb +++ b/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 diff --git a/app/views/payment_mailer/authorize_payment.html.haml b/app/views/payment_mailer/authorize_payment.html.haml new file mode 100644 index 0000000000..438a51c416 --- /dev/null +++ b/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) diff --git a/app/views/payment_mailer/authorize_payment.text.haml b/app/views/payment_mailer/authorize_payment.text.haml new file mode 100644 index 0000000000..956e63ec9f --- /dev/null +++ b/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) diff --git a/config/locales/en.yml b/config/locales/en.yml index 788b068502..0354b9c4d2 100644 --- a/config/locales/en.yml +++ b/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," diff --git a/config/routes.rb b/config/routes.rb index a657f7f234..5236a42c62 100644 --- a/config/routes.rb +++ b/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 diff --git a/lib/stripe/authorize_response_patcher.rb b/lib/stripe/authorize_response_patcher.rb index 7614ff47be..d67b8dbca9 100644 --- a/lib/stripe/authorize_response_patcher.rb +++ b/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 diff --git a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb index 2f327e2de2..1e1437aee2 100644 --- a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb +++ b/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 diff --git a/spec/features/admin/payments_stripe_spec.rb b/spec/features/admin/payments_stripe_spec.rb index 00c110195a..a4467a4f5a 100644 --- a/spec/features/admin/payments_stripe_spec.rb +++ b/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 diff --git a/spec/lib/stripe/authorize_response_patcher_spec.rb b/spec/lib/stripe/authorize_response_patcher_spec.rb index 1572c1ced5..9ecb800ee2 100644 --- a/spec/lib/stripe/authorize_response_patcher_spec.rb +++ b/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 diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 2420c7766d..cbba2b4b9a 100644 --- a/spec/models/spree/payment_spec.rb +++ b/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