Merge pull request #87 from gocardless/fix-iban-validation

Fix IBAN validation for countries we never explicitly supported

lib/ibandit/constants.rb

@@ -1,14 +1,19 @@
 module Ibandit
   module Constants
-    SUPPORTED_COUNTRY_CODES = %w[AT AU BE BG CY CZ DE DK EE ES FI FR GB GR HR
-                                 HU IE IS IT LT LU LV MC MT NL NO PL PT RO SE
-                                 SI SK SM].freeze
+    CONSTRUCTABLE_IBAN_COUNTRY_CODES = %w[AT BE BG CY CZ DE DK EE ES FI FR GB GR
+                                          HR HU IE IS IT LT LU LV MC MT NL NO PL
+                                          PT RO SE SI SK SM].freeze
 
     PSEUDO_IBAN_COUNTRY_CODES = %w[AU SE].freeze
-    CONSTRUCTABLE_IBAN_COUNTRY_CODES = SUPPORTED_COUNTRY_CODES - %w[AU]
     DECONSTRUCTABLE_IBAN_COUNTRY_CODES =
-      SUPPORTED_COUNTRY_CODES - PSEUDO_IBAN_COUNTRY_CODES
+      CONSTRUCTABLE_IBAN_COUNTRY_CODES - PSEUDO_IBAN_COUNTRY_CODES
 
     PSEUDO_IBAN_CHECK_DIGITS = "ZZ".freeze
+
+    SUPPORTED_COUNTRY_CODES = (
+      CONSTRUCTABLE_IBAN_COUNTRY_CODES +
+      DECONSTRUCTABLE_IBAN_COUNTRY_CODES +
+      PSEUDO_IBAN_COUNTRY_CODES
+    ).uniq
   end
 end

lib/ibandit/iban.rb

@@ -4,20 +4,26 @@ module Ibandit
   class IBAN
     attr_reader :errors, :iban, :country_code, :check_digits, :bank_code,
                 :branch_code, :account_number, :swift_bank_code,
-                :swift_branch_code, :swift_account_number
+                :swift_branch_code, :swift_account_number, :source
 
     def initialize(argument)
       if argument.is_a?(String)
         input = argument.to_s.gsub(/\s+/, "").upcase
 
-        if pseudo_iban?(input) && !PseudoIBANSplitter.new(input).split.nil?
-          local_details = PseudoIBANSplitter.new(input).split
+        pseudo_iban_splitter = PseudoIBANSplitter.new(input)
+        is_pseudo_iban_country = Constants::PSEUDO_IBAN_COUNTRY_CODES.
+          include?(pseudo_iban_splitter.country_code)
+        if pseudo_iban?(input) && is_pseudo_iban_country
+          @source = :pseudo_iban
+          local_details = pseudo_iban_splitter.split
           build_iban_from_local_details(local_details)
         else
+          @source = :iban
           @iban = input
           extract_swift_details_from_iban!
         end
       elsif argument.is_a?(Hash)
+        @source = :local_details
         build_iban_from_local_details(argument)
       else
         raise TypeError, "Must pass an IBAN string or hash of local details"
@@ -73,17 +79,27 @@ def pseudo_iban
     ###############
 
     def valid?
-      return false if country_code.nil?
+      has_iban = !iban.nil?
+      has_pseudo_iban = !pseudo_iban.nil?
 
-      if Constants::CONSTRUCTABLE_IBAN_COUNTRY_CODES.include?(country_code)
-        return false unless valid_iban?
+      if has_pseudo_iban && !has_iban
+        valid_pseudo_iban?
+      else
+        valid_iban?
       end
+    end
 
-      if Constants::PSEUDO_IBAN_COUNTRY_CODES.include?(country_code)
-        return false unless valid_pseudo_iban?
-      end
+    def valid_pseudo_iban_check_digits?
+      return true unless source == :pseudo_iban
+      return true if check_digits == Constants::PSEUDO_IBAN_CHECK_DIGITS
 
-      true
+      @errors[:check_digits] =
+        Ibandit.translate(
+          :invalid_check_digits,
+          expected_check_digits: Constants::PSEUDO_IBAN_CHECK_DIGITS,
+          check_digits: check_digits,
+        )
+      false
     end
 
     def valid_country_code?
@@ -192,6 +208,7 @@ def valid_characters?
 
     def valid_format?
       return unless valid_country_code?
+      return unless structure[:bban_format]
 
       if bban =~ Regexp.new(structure[:bban_format])
         true
@@ -332,6 +349,7 @@ def valid_swedish_local_details?
 
     def valid_pseudo_iban?
       [
+        valid_pseudo_iban_check_digits?,
         valid_country_code?,
         valid_bank_code_length?,
         valid_branch_code_length?,
@@ -378,7 +396,12 @@ def build_iban_from_local_details(details_hash)
       @swift_bank_code      = try_dup(local_details[:swift_bank_code])
 
       @iban                 = IBANAssembler.assemble(swift_details)
-      @check_digits         = @iban.slice(2, 2) unless @iban.nil?
+
+      if source == :pseudo_iban
+        @check_digits       = try_dup(local_details[:check_digits])
+      else
+        @check_digits       = @iban.slice(2, 2) unless @iban.nil?
+      end
     end
 
     def extract_swift_details_from_iban!

lib/ibandit/iban_assembler.rb

@@ -69,7 +69,8 @@ def self.can_assemble?(local_details)
     end
 
     def self.supported_country_code?(local_details)
-      Constants::SUPPORTED_COUNTRY_CODES.include?(local_details[:country_code])
+      Constants::CONSTRUCTABLE_IBAN_COUNTRY_CODES.
+        include?(local_details[:country_code])
     end
 
     def self.valid_arguments?(local_details)

lib/ibandit/pseudo_iban_splitter.rb

@@ -5,36 +5,38 @@ def initialize(pseudo_iban)
     end
 
     def split
-      return unless decomposable?
-
       {
         country_code: country_code,
+        check_digits: check_digits,
         bank_code: bank_code,
         branch_code: branch_code,
         account_number: account_number,
       }
     end
 
-    private
-
     def country_code
       @pseudo_iban.slice(0, 2)
     end
 
+    private
+
     def check_digits
       @pseudo_iban.slice(2, 2)
     end
 
     def bank_code
+      return unless country_code_valid?
       pseudo_iban_part(bank_code_start_index, :pseudo_iban_bank_code_length)
     end
 
     def branch_code
+      return unless country_code_valid?
       pseudo_iban_part(branch_code_start_index,
                        :pseudo_iban_branch_code_length)
     end
 
     def account_number
+      return unless country_code_valid?
       remove_leading_padding(
         @pseudo_iban.slice(account_number_start_index, @pseudo_iban.length),
       )
@@ -59,18 +61,10 @@ def account_number_start_index
       branch_code_start_index + structure.fetch(:pseudo_iban_branch_code_length)
     end
 
-    def decomposable?
-      country_code_valid? && check_digits_valid?
-    end
-
     def country_code_valid?
       Constants::PSEUDO_IBAN_COUNTRY_CODES.include?(country_code)
     end
 
-    def check_digits_valid?
-      check_digits == Constants::PSEUDO_IBAN_CHECK_DIGITS
-    end
-
     def structure
       Ibandit.structures[country_code]
     end