Merge pull request #214 from solidusio-contrib/christianr/payment-source-ownership

Validate payment_source ownership on the subscription

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 7055aad..79d400a 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -124,3 +124,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 a2fc8e6..6b2b3eb 100644
--- a/spec/controllers/spree/admin/subscriptions_controller_spec.rb
+++ b/spec/controllers/spree/admin/subscriptions_controller_spec.rb
@@ -93,10 +93,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 25cbcf4..e7c18fa 100644
--- a/spec/lib/solidus_subscriptions/checkout_spec.rb
+++ b/spec/lib/solidus_subscriptions/checkout_spec.rb
@@ -66,7 +66,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 b78e66c..1369988 100644
--- a/spec/lib/solidus_subscriptions/subscription_generator_spec.rb
+++ b/spec/lib/solidus_subscriptions/subscription_generator_spec.rb
@@ -34,7 +34,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 145cc44..6b05cce 100644
--- a/spec/models/solidus_subscriptions/subscription_spec.rb
+++ b/spec/models/solidus_subscriptions/subscription_spec.rb
@@ -16,6 +16,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))
@@ -82,7 +92,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',