From deedc28e7487f692de276cc8d70d0a1527afc8a9 Mon Sep 17 00:00:00 2001 From: Christian Rimondi Date: Fri, 19 Mar 2021 16:00:15 +0100 Subject: Validate payment_source ownership on the subscription --- app/models/solidus_subscriptions/subscription.rb | 11 +++++++++++ config/locales/en.yml | 2 ++ .../controllers/spree/admin/subscriptions_controller_spec.rb | 3 ++- spec/lib/solidus_subscriptions/checkout_spec.rb | 2 +- .../lib/solidus_subscriptions/subscription_generator_spec.rb | 2 +- spec/models/solidus_subscriptions/subscription_spec.rb | 12 +++++++++++- 6 files changed, 28 insertions(+), 4 deletions(-) diff --git a/app/models/solidus_subscriptions/subscription.rb b/app/models/solidus_subscriptions/subscription.rb index df82447..7e98f82 100644 --- a/app/models/solidus_subscriptions/subscription.rb +++ b/app/models/solidus_subscriptions/subscription.rb @@ -28,6 +28,8 @@ module SolidusSubscriptions validates :payment_source, presence: true, if: -> { payment_method&.source_required? } validates :currency, inclusion: { in: ::Money::Currency.all.map(&:iso_code) } + validate :validate_payment_source_ownership + accepts_nested_attributes_for :shipping_address accepts_nested_attributes_for :billing_address accepts_nested_attributes_for :line_items, allow_destroy: true, reject_if: ->(p) { p[:quantity].blank? } @@ -273,6 +275,15 @@ module SolidusSubscriptions private + def validate_payment_source_ownership + return if payment_source.blank? + + if payment_source.respond_to?(:user_id) && + payment_source.user_id != user_id + errors.add(:payment_source, :not_owned_by_user) + end + end + def check_successive_skips_exceeded return unless SolidusSubscriptions.configuration.maximum_successive_skips diff --git a/config/locales/en.yml b/config/locales/en.yml index b921a2c..522a29e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -120,3 +120,5 @@ en: limit. It can no longer be skipped. currency: inclusion: "is not a valid currency code" + payment_source: + not_owned_by_user: "does not belong to the user associated with the subscription" diff --git a/spec/controllers/spree/admin/subscriptions_controller_spec.rb b/spec/controllers/spree/admin/subscriptions_controller_spec.rb index b2a5ec5..d64d756 100644 --- a/spec/controllers/spree/admin/subscriptions_controller_spec.rb +++ b/spec/controllers/spree/admin/subscriptions_controller_spec.rb @@ -91,10 +91,11 @@ RSpec.describe Spree::Admin::SubscriptionsController, type: :request do end it 'updates the subscription payment source if payment method requires source' do - subscription = create :subscription payment = create :credit_card_payment payment_source = payment.source payment_method = payment.payment_method + + subscription = create :subscription, user: payment.order.user subscription_params = { subscription: { payment_method_id: payment_method.id, diff --git a/spec/lib/solidus_subscriptions/checkout_spec.rb b/spec/lib/solidus_subscriptions/checkout_spec.rb index 3dc1a6d..e6c1d51 100644 --- a/spec/lib/solidus_subscriptions/checkout_spec.rb +++ b/spec/lib/solidus_subscriptions/checkout_spec.rb @@ -64,7 +64,7 @@ RSpec.describe SolidusSubscriptions::Checkout, :checkout do it 'calls the payment failed dispatcher' do stub_spree_preferences(auto_capture: true) installment = create(:installment, :actionable).tap do |i| - i.subscription.update!(payment_source: create(:credit_card, number: '4111123412341234')) + i.subscription.update!(payment_source: create(:credit_card, number: '4111123412341234', user: i.subscription.user)) end payment_failed_dispatcher = stub_dispatcher(SolidusSubscriptions::Dispatcher::PaymentFailedDispatcher, installment) diff --git a/spec/lib/solidus_subscriptions/subscription_generator_spec.rb b/spec/lib/solidus_subscriptions/subscription_generator_spec.rb index 72cbfe6..02082e3 100644 --- a/spec/lib/solidus_subscriptions/subscription_generator_spec.rb +++ b/spec/lib/solidus_subscriptions/subscription_generator_spec.rb @@ -32,7 +32,7 @@ RSpec.describe SolidusSubscriptions::SubscriptionGenerator do it 'copies the payment method from the order' do subscription_line_item = build(:subscription_line_item) payment_method = create(:credit_card_payment_method) - payment_source = create(:credit_card, payment_method: payment_method) + payment_source = create(:credit_card, payment_method: payment_method, user: subscription_line_item.order.user) create(:payment, order: subscription_line_item.spree_line_item.order, source: payment_source, diff --git a/spec/models/solidus_subscriptions/subscription_spec.rb b/spec/models/solidus_subscriptions/subscription_spec.rb index c2fd4a4..0d0f047 100644 --- a/spec/models/solidus_subscriptions/subscription_spec.rb +++ b/spec/models/solidus_subscriptions/subscription_spec.rb @@ -14,6 +14,16 @@ RSpec.describe SolidusSubscriptions::Subscription, type: :model do with_message('is not a valid currency code') end + it 'validates payment_source ownership' do + subscription = create(:subscription) + + subscription.update(payment_source: create(:credit_card)) + expect(subscription.errors.messages[:payment_source]).to include('does not belong to the user associated with the subscription') + + subscription.update(payment_source: create(:credit_card, user: subscription.user)) + expect(subscription.errors.messages[:payment_source]).not_to include('does not belong to the user associated with the subscription') + end + describe 'creating a subscription' do it 'tracks the creation' do stub_const('Spree::Event', class_spy(Spree::Event)) @@ -80,7 +90,7 @@ RSpec.describe SolidusSubscriptions::Subscription, type: :model do stub_const('Spree::Event', class_spy(Spree::Event)) subscription = create(:subscription) - subscription.update!(payment_source: create(:credit_card)) + subscription.update!(payment_source: create(:credit_card, user: subscription.user)) expect(Spree::Event).to have_received(:fire).with( 'solidus_subscriptions.subscription_payment_method_changed', -- cgit v1.2.3