From 69cb08fb37073ab22fa8bf8dcec7bcee2fa3345c Mon Sep 17 00:00:00 2001 From: Ahmed Shahwan Date: Thu, 27 Mar 2025 00:29:51 +0200 Subject: [PATCH 1/4] Remove useless statement --- app/services/payment_providers/moneyhash_service.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/services/payment_providers/moneyhash_service.rb b/app/services/payment_providers/moneyhash_service.rb index 21e253ef61d..56aedf69bc2 100644 --- a/app/services/payment_providers/moneyhash_service.rb +++ b/app/services/payment_providers/moneyhash_service.rb @@ -29,7 +29,6 @@ def create_or_update(**args) ) end - moneyhash_provider.api_key old_code = moneyhash_provider.code moneyhash_provider.api_key = args[:api_key] if args.key?(:api_key) moneyhash_provider.code = args[:code] if args.key?(:code) From 48cc9dc828d088317fccf2b5b2dc8db85ba5c38b Mon Sep 17 00:00:00 2001 From: Ahmed Shahwan Date: Thu, 27 Mar 2025 00:31:50 +0200 Subject: [PATCH 2/4] Remove useless statement --- app/services/dunning_campaigns/update_service.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/services/dunning_campaigns/update_service.rb b/app/services/dunning_campaigns/update_service.rb index c8fed6d73b8..84b568b0b13 100644 --- a/app/services/dunning_campaigns/update_service.rb +++ b/app/services/dunning_campaigns/update_service.rb @@ -37,8 +37,6 @@ def permitted_attributes end def handle_thresholds - thresholds_updated = false - input_threshold_ids = params[:thresholds].map { |t| t[:id] }.compact # Delete thresholds not included in the payload From 2e2ea0c312042b3c027baa761d9388393cdd59a0 Mon Sep 17 00:00:00 2001 From: Ahmed Shahwan Date: Thu, 27 Mar 2025 20:01:45 +0200 Subject: [PATCH 3/4] validate moneyhash's webhook signature - save signature_key during moneyhash connection creation - use saved signature_key to validate incoming signature_key --- app/controllers/webhooks_controller.rb | 2 +- .../payment_providers/moneyhash_provider.rb | 2 +- .../validate_incoming_webhook_service.rb | 27 +++++++++++++---- .../payment_providers/moneyhash_service.rb | 24 ++++++++++++++- .../moneyhash/webhook_signature_response.json | 13 +++++++++ .../validate_incoming_webhook_service_spec.rb | 26 +++++++++++++++++ .../moneyhash_service_spec.rb | 29 +++++++++++++++++++ 7 files changed, 114 insertions(+), 9 deletions(-) create mode 100644 spec/fixtures/moneyhash/webhook_signature_response.json create mode 100644 spec/services/payment_providers/moneyhash/validate_incoming_webhook_service_spec.rb diff --git a/app/controllers/webhooks_controller.rb b/app/controllers/webhooks_controller.rb index 7c9c1c58ac2..2d90c865785 100644 --- a/app/controllers/webhooks_controller.rb +++ b/app/controllers/webhooks_controller.rb @@ -81,7 +81,7 @@ def moneyhash webhook_source: :moneyhash, code: params[:code].presence, payload: JSON.parse(request.body.read), - # signature: request.headers["Moneyhash-Signature"], # TODO: Implement moneyhash signature + signature: request.headers["MoneyHash-Signature"], event_type: params[:type] ) diff --git a/app/models/payment_providers/moneyhash_provider.rb b/app/models/payment_providers/moneyhash_provider.rb index 0c8931415fc..339cf9b3220 100644 --- a/app/models/payment_providers/moneyhash_provider.rb +++ b/app/models/payment_providers/moneyhash_provider.rb @@ -22,7 +22,7 @@ class MoneyhashProvider < BaseProvider validates :api_key, presence: true validates :flow_id, presence: true, length: {maximum: 20} - secrets_accessors :api_key + secrets_accessors :api_key, :signature_key settings_accessors :flow_id def self.api_base_url diff --git a/app/services/payment_providers/moneyhash/validate_incoming_webhook_service.rb b/app/services/payment_providers/moneyhash/validate_incoming_webhook_service.rb index 3e5d66f20cf..ce9896bdba8 100644 --- a/app/services/payment_providers/moneyhash/validate_incoming_webhook_service.rb +++ b/app/services/payment_providers/moneyhash/validate_incoming_webhook_service.rb @@ -1,5 +1,8 @@ # frozen_string_literal: true +require "openssl" +require "base64" + module PaymentProviders module Moneyhash class ValidateIncomingWebhookService < BaseService @@ -12,13 +15,25 @@ def initialize(payload:, signature:, payment_provider:) end def call - # TODO: Implement moneyhash validation + # extract timestamp and v3_signature from signature + timestamp, v3_signature = signature.split(",").each_with_object({}) do |part, hash| + key, value = part.split("=") + hash[key] = value if %w[t v3].include?(key) + end.values_at("t", "v3") - result + # validate signature + secret = webhook_secret + decoded_body = Base64.strict_encode64(payload.to_json) + + to_sign = "#{decoded_body}#{timestamp}" - # TODO: - # rescue Moneyhash::SignatureVerificationError - # result.service_failure!(code: "webhook_error", message: "Invalid signature") + calculated_signature = OpenSSL::HMAC.hexdigest("SHA256", secret, to_sign) + + if calculated_signature != v3_signature + result.service_failure!(code: "webhook_error", message: "Invalid signature") + end + + result end private @@ -26,7 +41,7 @@ def call attr_reader :payload, :signature, :provider def webhook_secret - # TODO: + provider.signature_key end end end diff --git a/app/services/payment_providers/moneyhash_service.rb b/app/services/payment_providers/moneyhash_service.rb index 56aedf69bc2..c44813faaf0 100644 --- a/app/services/payment_providers/moneyhash_service.rb +++ b/app/services/payment_providers/moneyhash_service.rb @@ -20,7 +20,7 @@ def create_or_update(**args) payment_provider_type: "moneyhash" ) - moneyhash_provider = if payment_provider_result.success? + @moneyhash_provider = if payment_provider_result.success? payment_provider_result.payment_provider else PaymentProviders::MoneyhashProvider.new( @@ -35,6 +35,12 @@ def create_or_update(**args) moneyhash_provider.name = args[:name] if args.key?(:name) moneyhash_provider.flow_id = args[:flow_id] if args.key?(:flow_id) + if moneyhash_provider.signature_key.blank? + signature_result = get_signature_key + return signature_result unless signature_result.success? + moneyhash_provider.signature_key = signature_result.signature_key + end + moneyhash_provider.save(validate: false) if payment_provider_code_changed?(moneyhash_provider, old_code, args) @@ -73,8 +79,24 @@ def event_to_payment_status(event_code) end end + attr_reader :moneyhash_provider + private + def get_signature_key + response = LagoHttpClient::Client.new( + "#{::PaymentProviders::MoneyhashProvider.api_base_url}/api/v1/organizations/get-webhook-signature-key/" + ).get( + headers: { + "X-Api-Key" => moneyhash_provider.api_key + } + ) + result.signature_key = response["data"]["webhook_signature_secret"] + result + rescue LagoHttpClient::HttpError => e + result.service_failure!(code: "moneyhash_error", message: "#{e.error_code}: #{e.error_body}") + end + def event_handlers { "intent.processed" => method(:handle_intent_event), diff --git a/spec/fixtures/moneyhash/webhook_signature_response.json b/spec/fixtures/moneyhash/webhook_signature_response.json new file mode 100644 index 00000000000..80fd92215f2 --- /dev/null +++ b/spec/fixtures/moneyhash/webhook_signature_response.json @@ -0,0 +1,13 @@ +{ + "status": { + "code": 200, + "message": "success", + "errors": [] + }, + "data": { + "webhook_signature_secret": "oVZo2bro-a7UBN3k9QFlsSz6cN_Zrdt9KCWk4_5q2lo=" + }, + "count": 1, + "next": null, + "previous": null +} diff --git a/spec/services/payment_providers/moneyhash/validate_incoming_webhook_service_spec.rb b/spec/services/payment_providers/moneyhash/validate_incoming_webhook_service_spec.rb new file mode 100644 index 00000000000..2f5a1102a13 --- /dev/null +++ b/spec/services/payment_providers/moneyhash/validate_incoming_webhook_service_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe PaymentProviders::Moneyhash::ValidateIncomingWebhookService, type: :service do + subject(:result) do + described_class.call(payload:, signature:, payment_provider:) + end + + let(:payload) { "webhook_payload" } + let(:signature) { "t=1743090080,v1=placeholder,v2=placeholder,v3=ca13480c8142f2f2b44822c764909027974e84b3e8c94457a314f129d8d60148" } + let(:payment_provider) { create(:moneyhash_provider, signature_key: "test_signature_key") } + + it "return success when signature is valid" do + result = described_class.call(payload:, signature:, payment_provider:) + expect(result).to be_success + end + + it "returns a service failure when signature is invalid" do + signature = "Moneyhash-Signature: t=1743090080,v1=placeholder,v2=placeholder,v3=invalid_signature" + result = described_class.call(payload:, signature:, payment_provider:) + expect(result).not_to be_success + expect(result.error).to be_a(BaseService::ServiceFailure) + expect(result.error.message).to eq("webhook_error: Invalid signature") + end +end diff --git a/spec/services/payment_providers/moneyhash_service_spec.rb b/spec/services/payment_providers/moneyhash_service_spec.rb index 6e2fde9b7c9..e27eda9d899 100644 --- a/spec/services/payment_providers/moneyhash_service_spec.rb +++ b/spec/services/payment_providers/moneyhash_service_spec.rb @@ -8,6 +8,35 @@ let(:customer) { create(:customer, organization:) } let(:moneyhash_customer) { create(:moneyhash_customer, customer:) } + describe "#create_or_update" do + let(:webhook_signature_response) { JSON.parse(File.read(Rails.root.join("spec/fixtures/moneyhash/webhook_signature_response.json"))) } + + before do + allow_any_instance_of(LagoHttpClient::Client).to receive(:get).and_return(webhook_signature_response) # rubocop:disable RSpec/AnyInstance + end + + it "creates a new moneyhash provider with the webhook signature key" do + result = described_class.new.create_or_update(organization:, code: "test_code", name: "test_name", flow_id: "test_flow_id") + expect(result).to be_success + expect(result.moneyhash_provider).to be_a(PaymentProviders::MoneyhashProvider) + expect(result.moneyhash_provider.signature_key).to eq(webhook_signature_response.dig("data", "webhook_signature_secret")) + expect(result.moneyhash_provider.code).to eq("test_code") + expect(result.moneyhash_provider.name).to eq("test_name") + expect(result.moneyhash_provider.flow_id).to eq("test_flow_id") + end + + it "updates the existing moneyhash provider but leaves the signature key unchanged" do + moneyhash_provider.update!(signature_key: "same_signature_key") + result = described_class.new.create_or_update(organization:, code: moneyhash_provider.code, name: "updated_name", flow_id: "updated_flow_id") + expect(result).to be_success + expect(result.moneyhash_provider).to be_a(PaymentProviders::MoneyhashProvider) + expect(result.moneyhash_provider.signature_key).to eq("same_signature_key") + expect(result.moneyhash_provider.code).to eq(moneyhash_provider.code) + expect(result.moneyhash_provider.name).to eq("updated_name") + expect(result.moneyhash_provider.flow_id).to eq("updated_flow_id") + end + end + # Intent # handle event - intent.processed <- # handle event - intent.time_expired <- From a696ac1cf1c321df1383dd22e57eb1a68aa19aba Mon Sep 17 00:00:00 2001 From: Ahmed Shahwan Date: Fri, 28 Mar 2025 14:28:47 +0200 Subject: [PATCH 4/4] Fix MH webhook controller test --- spec/requests/webhooks_controller_spec.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/spec/requests/webhooks_controller_spec.rb b/spec/requests/webhooks_controller_spec.rb index 5b2dc53ba12..f15ea5ba673 100644 --- a/spec/requests/webhooks_controller_spec.rb +++ b/spec/requests/webhooks_controller_spec.rb @@ -287,7 +287,7 @@ webhook_source: :moneyhash, code:, payload: body, - # signature:, # TODO: + signature: "t=1743090080,v1=placeholder,v2=placeholder,v3=placeholder", event_type: body["type"] ) .and_return(result) @@ -298,7 +298,8 @@ "/webhooks/moneyhash/#{organization_id}?code=#{code}", params: body.to_json, headers: { - "Content-Type" => "application/json" + "Content-Type" => "application/json", + "Moneyhash-Signature" => "t=1743090080,v1=placeholder,v2=placeholder,v3=placeholder" } ) @@ -317,7 +318,8 @@ "/webhooks/moneyhash/#{organization_id}?code=#{code}", params: body.to_json, headers: { - "Content-Type" => "application/json" + "Content-Type" => "application/json", + "Moneyhash-Signature" => "t=1743090080,v1=placeholder,v2=placeholder,v3=placeholder" } )