From bcc8e828dc33bb4e792d65410cf8c0ff51132054 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Tue, 27 Sep 2016 16:38:00 -0700 Subject: [PATCH 1/4] Update CC brand detection regex The existing hash was incomplete, missing some brands. It was also missing MasterCard's new (October 2016) BIN range (222100-272099). This hash was originally added in https://github.com/spree/spree/pull/3918 and has not been changed since. I don't know the original source, but I would rather trust the Hash taken from ActiveMerchant. One notable difference is that this will treat maestro and mastercard as separate brands. NB: For most stores, this code will not be run, and instead jquery.payment on the frontend will be responsible for providing the credit card brand. --- core/app/models/spree/credit_card.rb | 22 +++++++++++++++------- core/spec/models/spree/credit_card_spec.rb | 4 ++++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/core/app/models/spree/credit_card.rb b/core/app/models/spree/credit_card.rb index b7d95252767..27331aa1ab1 100644 --- a/core/app/models/spree/credit_card.rb +++ b/core/app/models/spree/credit_card.rb @@ -29,14 +29,22 @@ class CreditCard < Spree::Base # needed for some of the ActiveMerchant gateways (eg. SagePay) alias_attribute :brand, :cc_type + # Taken from ActiveMerchant + # https://github.com/activemerchant/active_merchant/blob/2f2acd4696e8de76057b5ed670b9aa022abc1187/lib/active_merchant/billing/credit_card_methods.rb#L5 CARD_TYPES = { - visa: /^4[0-9]{12}(?:[0-9]{3})?$/, - master: /(^5[1-5][0-9]{14}$)|(^6759[0-9]{2}([0-9]{10})$)|(^6759[0-9]{2}([0-9]{12})$)|(^6759[0-9]{2}([0-9]{13})$)/, - diners_club: /^3(?:0[0-5]|[68][0-9])[0-9]{11}$/, - american_express: /^3[47][0-9]{13}$/, - discover: /^6(?:011|5[0-9]{2})[0-9]{12}$/, - jcb: /^(?:2131|1800|35\d{3})\d{11}$/ - } + 'visa' => /^4\d{12}(\d{3})?(\d{3})?$/, + 'master' => /^(5[1-5]\d{4}|677189|222[1-9]\d{2}|22[3-9]\d{3}|2[3-6]\d{4}|27[01]\d{3}|2720\d{2})\d{10}$/, + 'discover' => /^(6011|65\d{2}|64[4-9]\d)\d{12}|(62\d{14})$/, + 'american_express' => /^3[47]\d{13}$/, + 'diners_club' => /^3(0[0-5]|[68]\d)\d{11}$/, + 'jcb' => /^35(28|29|[3-8]\d)\d{12}$/, + 'switch' => /^6759\d{12}(\d{2,3})?$/, + 'solo' => /^6767\d{12}(\d{2,3})?$/, + 'dankort' => /^5019\d{12}$/, + 'maestro' => /^(5[06-8]|6\d)\d{10,17}$/, + 'forbrugsforeningen' => /^600722\d{10}$/, + 'laser' => /^(6304|6706|6709|6771(?!89))\d{8}(\d{4}|\d{6,7})?$/ + }.freeze def address_attributes=(attributes) self.address = Address.immutable_merge(address, attributes) diff --git a/core/spec/models/spree/credit_card_spec.rb b/core/spec/models/spree/credit_card_spec.rb index a028b736f97..6e3bb9088f5 100644 --- a/core/spec/models/spree/credit_card_spec.rb +++ b/core/spec/models/spree/credit_card_spec.rb @@ -235,6 +235,10 @@ def self.payment_states credit_card.cc_type = '' expect(credit_card.cc_type).to eq('master') + credit_card.number = '2221000000000000' + credit_card.cc_type = '' + expect(credit_card.cc_type).to eq('master') + credit_card.number = '378282246310005' credit_card.cc_type = '' expect(credit_card.cc_type).to eq('american_express') From 9771747c4b362395ee726bd7102267ebba26aaeb Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Tue, 27 Sep 2016 18:03:25 -0700 Subject: [PATCH 2/4] Remove unnecessary CC number cleanup The removal of whitespace is already done (more completely) in number=. --- core/app/models/spree/credit_card.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/app/models/spree/credit_card.rb b/core/app/models/spree/credit_card.rb index 27331aa1ab1..ecfd3160c74 100644 --- a/core/app/models/spree/credit_card.rb +++ b/core/app/models/spree/credit_card.rb @@ -105,8 +105,7 @@ def set_last_digits # @return [String] the credit card type if it can be determined from the # number, otherwise the empty string def try_type_from_number - numbers = number.delete(' ') if number - CARD_TYPES.find{ |type, pattern| return type.to_s if numbers =~ pattern }.to_s + CARD_TYPES.find{ |type, pattern| return type.to_s if number =~ pattern }.to_s end # @return [Boolean] true when a verification value is present From 735a8e52eeb63c44b45063572d55022b33e2c048 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Tue, 27 Sep 2016 18:10:15 -0700 Subject: [PATCH 3/4] Improve readability of try_type_from_number The existing code returned from within a find, and if the find ever succeeded, would return '' because nil (the return of the find) .to_s is ''. This works, but is difficult to read. I prefer being explicit about the return value being an empty string if there isn't an early return. --- core/app/models/spree/credit_card.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/app/models/spree/credit_card.rb b/core/app/models/spree/credit_card.rb index ecfd3160c74..8a996880894 100644 --- a/core/app/models/spree/credit_card.rb +++ b/core/app/models/spree/credit_card.rb @@ -105,7 +105,10 @@ def set_last_digits # @return [String] the credit card type if it can be determined from the # number, otherwise the empty string def try_type_from_number - CARD_TYPES.find{ |type, pattern| return type.to_s if number =~ pattern }.to_s + CARD_TYPES.each do |type, pattern| + return type.to_s if number =~ pattern + end + '' end # @return [Boolean] true when a verification value is present From 001122ef92068a4a18d182a52fada72bcba01f0c Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Thu, 29 Sep 2016 13:44:27 -0700 Subject: [PATCH 4/4] Remove unnecessary to_s from try_type_from_number Our CARD_TYPES hash now uses strings as keys so we no longer need to stringify them. --- core/app/models/spree/credit_card.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/app/models/spree/credit_card.rb b/core/app/models/spree/credit_card.rb index 8a996880894..7b20fabfb4f 100644 --- a/core/app/models/spree/credit_card.rb +++ b/core/app/models/spree/credit_card.rb @@ -106,7 +106,7 @@ def set_last_digits # number, otherwise the empty string def try_type_from_number CARD_TYPES.each do |type, pattern| - return type.to_s if number =~ pattern + return type if number =~ pattern end '' end