diff --git a/Procfile.dev b/Procfile.dev index f010b99c..9afa3631 100644 --- a/Procfile.dev +++ b/Procfile.dev @@ -1,2 +1,3 @@ web: env RUBY_DEBUG_OPEN=true bin/rails-sandbox server watch: bin/rails-sandbox g solidus_stripe:install --force --watch +stripe: stripe listen --forward-to http://localhost:3000/solidus_stripe/test/webhooks --events charge.refunded,payment_intent.succeeded,payment_intent.payment_failed,payment_intent.canceled diff --git a/README.md b/README.md index 27511ef4..95156c0f 100644 --- a/README.md +++ b/README.md @@ -44,7 +44,7 @@ need to append it to a generic webhook endpoint to get the URL for that payment method. For example: ```ruby -SolidusStripe::PaymentMethod.last.webhook_endpoint_slug +SolidusStripe::PaymentMethod.last.slug # "365a8435cd11300e87de864c149516e0" ``` diff --git a/app/controllers/solidus_stripe/intents_controller.rb b/app/controllers/solidus_stripe/intents_controller.rb index 734aca02..02839b46 100644 --- a/app/controllers/solidus_stripe/intents_controller.rb +++ b/app/controllers/solidus_stripe/intents_controller.rb @@ -43,6 +43,8 @@ def after_confirmation def load_payment_method @payment_method = current_order(create_order_if_necessary: true) - .available_payment_methods.find(params[:payment_method_id]) + .available_payment_methods + .merge(SolidusStripe::PaymentMethod.with_slug(params[:slug])) + .first! end end diff --git a/app/models/solidus_stripe/payment_method.rb b/app/models/solidus_stripe/payment_method.rb index fb003568..e91d4359 100644 --- a/app/models/solidus_stripe/payment_method.rb +++ b/app/models/solidus_stripe/payment_method.rb @@ -14,18 +14,11 @@ class PaymentMethod < ::Spree::PaymentMethod validates :available_to_admin, inclusion: { in: [false] } validates :preferred_setup_future_usage, inclusion: { in: ['', 'on_session', 'off_session'] } - has_one :webhook_endpoint, - class_name: 'SolidusStripe::WebhookEndpoint', - inverse_of: :payment_method, - dependent: :destroy + has_one :slug_entry, class_name: 'SolidusStripe::SlugEntry', inverse_of: :payment_method, dependent: :destroy - after_create :assign_webhook_endpoint + after_create :assign_slug - # @!attribute [r] webhook_endpoint_slug - # @return [String] The slug of the webhook endpoint for this payment method. - delegate :slug, - to: :webhook_endpoint, - prefix: true + delegate :slug, to: :slug_entry def partial_name "stripe" @@ -52,6 +45,10 @@ def payment_profiles_supported? false end + def self.with_slug(slug) + where(id: SlugEntry.where(slug: slug).select(:payment_method_id)) + end + # TODO: re-evaluate the need for this and think of ways to always go throught the intent classes. def self.intent_id_for_payment(payment) return unless payment @@ -70,12 +67,12 @@ def stripe_dashboard_url(intent_id) end end - private + def assign_slug + # If there's only one payment method, we can use a default slug. + slug = preferred_test_mode ? 'test' : 'live' if self.class.count == 1 + slug = SecureRandom.hex(16) while SlugEntry.exists?(slug: slug) || slug.nil? - def assign_webhook_endpoint - create_webhook_endpoint!( - slug: WebhookEndpoint.generate_slug - ) + create_slug_entry!(slug: slug) end end end diff --git a/app/models/solidus_stripe/webhook_endpoint.rb b/app/models/solidus_stripe/slug_entry.rb similarity index 56% rename from app/models/solidus_stripe/webhook_endpoint.rb rename to app/models/solidus_stripe/slug_entry.rb index 8026500a..253fc804 100644 --- a/app/models/solidus_stripe/webhook_endpoint.rb +++ b/app/models/solidus_stripe/slug_entry.rb @@ -14,24 +14,7 @@ module SolidusStripe # # We use a slug instead of the payment method ID to be resilient to # database changes and to avoid guessing about valid endpoint URLs. - class WebhookEndpoint < ::Spree::Base - belongs_to :payment_method, - class_name: 'SolidusStripe::PaymentMethod' - - # @api private - def self.generate_slug - SecureRandom.hex(16).then do |slug| - exists?(slug: slug) ? generate_slug : slug - end - end - - # Finds the payment method associated with the given slug. - # - # @param slug [String] - # @raise [ActiveRecord::RecordNotFound] if no payment method is found - # @return [SolidusStripe::PaymentMethod] - def self.payment_method(slug) - find_by!(slug: slug).payment_method - end + class SlugEntry < ::Spree::Base + belongs_to :payment_method, class_name: 'SolidusStripe::PaymentMethod' end end diff --git a/config/routes.rb b/config/routes.rb index 63d9b170..666d0173 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true SolidusStripe::Engine.routes.draw do - scope ':payment_method_id' do - get :after_confirmation, controller: :intents + scope ':slug' do + get :after_confirmation, to: 'intents#after_confirmation' + post :webhooks, to: 'webhooks#create', format: false end - post '/webhooks/:slug', format: false, to: 'webhooks#create' end diff --git a/db/migrate/20230403094916_rename_webhook_endpoint_to_payment_method_slug_entries.rb b/db/migrate/20230403094916_rename_webhook_endpoint_to_payment_method_slug_entries.rb new file mode 100644 index 00000000..d562690d --- /dev/null +++ b/db/migrate/20230403094916_rename_webhook_endpoint_to_payment_method_slug_entries.rb @@ -0,0 +1,5 @@ +class RenameWebhookEndpointToPaymentMethodSlugEntries < ActiveRecord::Migration[7.0] + def change + rename_table :solidus_stripe_webhook_endpoints, :solidus_stripe_slug_entries + end +end diff --git a/lib/generators/solidus_stripe/install/templates/app/views/orders/payment_info/_stripe.html.erb b/lib/generators/solidus_stripe/install/templates/app/views/orders/payment_info/_stripe.html.erb index ca944d7f..9cc7dfcc 100644 --- a/lib/generators/solidus_stripe/install/templates/app/views/orders/payment_info/_stripe.html.erb +++ b/lib/generators/solidus_stripe/install/templates/app/views/orders/payment_info/_stripe.html.erb @@ -13,8 +13,8 @@ data-controller="solidus-stripe-confirm" data-solidus-stripe-confirm-publishable-key-value="<%= payment.payment_method.preferred_publishable_key %>" data-solidus-stripe-confirm-client-secret-value="<%= stripe_intent.client_secret %>" - data-solidus-stripe-confirm-return-url-value="<%= solidus_stripe.after_confirmation_url(payment.payment_method) %>" - data-solidus-stripe-confirm-error-base-url-value="<%= solidus_stripe.after_confirmation_url(payment.payment_method, payment_intent: stripe_intent.id) %>" + data-solidus-stripe-confirm-return-url-value="<%= solidus_stripe.after_confirmation_url(payment.payment_method.slug) %>" + data-solidus-stripe-confirm-error-base-url-value="<%= solidus_stripe.after_confirmation_url(payment.payment_method.slug, payment_intent: stripe_intent.id) %>" data-action="submit@window->solidus-stripe-confirm#confirm" > <% end %> diff --git a/lib/solidus_stripe/testing_support/factories.rb b/lib/solidus_stripe/testing_support/factories.rb index 5e751711..a1184f3b 100644 --- a/lib/solidus_stripe/testing_support/factories.rb +++ b/lib/solidus_stripe/testing_support/factories.rb @@ -71,7 +71,7 @@ stripe_intent_id { "pm_#{SecureRandom.uuid.delete('-')}" } end - factory :stripe_webhook_endpoint, class: 'SolidusStripe::WebhookEndpoint' do + factory :stripe_slug_entry, class: 'SolidusStripe::SlugEntry' do association :payment_method, factory: :stripe_payment_method slug { SecureRandom.hex(16) } end diff --git a/lib/solidus_stripe/webhook/event.rb b/lib/solidus_stripe/webhook/event.rb index e9400251..3da23f31 100644 --- a/lib/solidus_stripe/webhook/event.rb +++ b/lib/solidus_stripe/webhook/event.rb @@ -24,7 +24,7 @@ class Event # @api private class << self def from_request(payload:, signature_header:, slug:, tolerance: default_tolerance) - payment_method = SolidusStripe::WebhookEndpoint.payment_method(slug) + payment_method = SolidusStripe::PaymentMethod.with_slug(slug).first! stripe_event = Stripe::Webhook.construct_event( payload, signature_header, diff --git a/spec/models/solidus_stripe/payment_method_spec.rb b/spec/models/solidus_stripe/payment_method_spec.rb index 7e5a5579..9916013e 100644 --- a/spec/models/solidus_stripe/payment_method_spec.rb +++ b/spec/models/solidus_stripe/payment_method_spec.rb @@ -22,7 +22,7 @@ it 'creates a webhook endpoint' do payment_method = create(:stripe_payment_method) - expect(payment_method.webhook_endpoint).to be_present + expect(payment_method.slug_entry).to be_present end end end @@ -73,4 +73,47 @@ expect(payment_method.stripe_dashboard_url(123)).to eq(nil) end end + + describe '.with_slug' do + it 'selects by slug' do + payment_method = create(:stripe_payment_method) + + expect(described_class.with_slug(payment_method.slug)).to eq([payment_method]) + expect(described_class.with_slug('bad-slug')).to eq([]) + end + end + + describe '.assign_slug' do + it 'generates a "test" slug for the first payment method in test mode' do + payment_method = build(:stripe_payment_method) + + payment_method.save! + + expect(payment_method.slug).to eq('test') + end + + it 'generates a "live" slug for the first payment method in live mode' do + payment_method = build(:stripe_payment_method, preferred_test_mode: false) + + payment_method.save! + + expect(payment_method.slug).to eq('live') + end + + it 'generates a random hex string' do + _existing_payment_method = create(:stripe_payment_method) + payment_method = create(:stripe_payment_method) + + expect(payment_method.slug).to match(/^[0-9a-f]{32}$/) + end + + it 'generates a unique slug' do + slug = SecureRandom.hex(16) + + create(:stripe_slug_entry, slug: slug) + allow(SecureRandom).to receive(:hex).and_return(slug).and_call_original + + expect(create(:stripe_payment_method).slug).not_to eq(slug) + end + end end diff --git a/spec/models/solidus_stripe/webhook_endpoint_spec.rb b/spec/models/solidus_stripe/webhook_endpoint_spec.rb deleted file mode 100644 index 8e3aa368..00000000 --- a/spec/models/solidus_stripe/webhook_endpoint_spec.rb +++ /dev/null @@ -1,32 +0,0 @@ -# frozen_string_literal: true - -require 'solidus_stripe_spec_helper' - -RSpec.describe SolidusStripe::WebhookEndpoint do - describe '.generate_slug' do - it 'generates a random hex string' do - expect(described_class.generate_slug).to match(/^[0-9a-f]{32}$/) - end - - it 'generates a unique slug' do - slug = SecureRandom.hex(16) - create(:stripe_webhook_endpoint, slug: slug) - allow(described_class).to receive(:generate_slug).and_return(slug).and_call_original - - expect(described_class.generate_slug).not_to eq(slug) - end - end - - describe '.payment_method' do - it 'finds the payment method associated with the given slug' do - payment_method = create(:stripe_payment_method) - described_class.create!(payment_method: payment_method, slug: 'foo') - - expect(described_class.payment_method('foo')).to eq(payment_method) - end - - it 'raises an error if no payment method is found' do - expect { described_class.payment_method('invalid') }.to raise_error(ActiveRecord::RecordNotFound) - end - end -end diff --git a/spec/requests/solidus_stripe/intents_controller_spec.rb b/spec/requests/solidus_stripe/intents_controller_spec.rb index 34379f3d..08f7a512 100644 --- a/spec/requests/solidus_stripe/intents_controller_spec.rb +++ b/spec/requests/solidus_stripe/intents_controller_spec.rb @@ -8,7 +8,7 @@ order = create(:order_ready_to_complete) sign_in order.user - get "/solidus_stripe/#{payment_method.id}/after_confirmation" + get "/solidus_stripe/#{payment_method.slug}/after_confirmation" expect(response.status).to eq(422) end @@ -20,7 +20,7 @@ order = create(:order) sign_in order.user - get "/solidus_stripe/#{payment_method.id}/after_confirmation?payment_intent=pi_123" + get "/solidus_stripe/#{payment_method.slug}/after_confirmation?payment_intent=pi_123" expect(response).to redirect_to('/checkout/cart') end diff --git a/spec/support/solidus_stripe/webhook/event_with_context_factory.rb b/spec/support/solidus_stripe/webhook/event_with_context_factory.rb index c6266a18..aa16a307 100644 --- a/spec/support/solidus_stripe/webhook/event_with_context_factory.rb +++ b/spec/support/solidus_stripe/webhook/event_with_context_factory.rb @@ -75,7 +75,7 @@ def signature end def slug - @payment_method.webhook_endpoint_slug + @payment_method.slug end end end diff --git a/spec/support/solidus_stripe/webhook/request_helper.rb b/spec/support/solidus_stripe/webhook/request_helper.rb index 8ee6c674..ab663779 100644 --- a/spec/support/solidus_stripe/webhook/request_helper.rb +++ b/spec/support/solidus_stripe/webhook/request_helper.rb @@ -13,7 +13,7 @@ module RequestHelper # @param slug [String] It allows to override the slug in the payment # method to simulate an invalid request. def webhook_request(context, timestamp: context.timestamp) - post "/solidus_stripe/webhooks/#{context.slug}", + post "/solidus_stripe/#{context.slug}/webhooks", params: context.json, headers: { webhook_signature_header_key => webhook_signature_header(context, timestamp: timestamp) } end