Refactor complext dnskeys method

app/models/dnskey.rb

@@ -3,7 +3,7 @@ class Dnskey < ActiveRecord::Base
 
   belongs_to :domain
 
-  validates :alg, :protocol, :flags, :public_key, presence: true
+  validates :alg, :protocol, :flags, :public_key, presence: true, if: :validate_key_data
   validate :validate_algorithm
   validate :validate_protocol
   validate :validate_flags
@@ -36,6 +36,10 @@ class Dnskey < ActiveRecord::Base
     }
   end
 
+  def validate_key_data
+    alg.present? || protocol.present? || flags.present? || public_key.present?
+  end
+
   def validate_algorithm
     return if alg.blank?
     return if ALGORITHMS.include?(alg.to_s)

app/models/epp/epp_domain.rb

@@ -21,7 +21,8 @@ class Epp::EppDomain < Domain
       ],
       '2306' => [ # Parameter policy error
         [:owner_contact, :blank],
-        [:admin_contacts, :out_of_range]
+        [:admin_contacts, :out_of_range],
+        [:base, :ds_data_with_key_not_allowed]
       ],
       '2004' => [ # Parameter value range error
         [:nameservers, :out_of_range,
@@ -187,27 +188,10 @@ class Epp::EppDomain < Domain
 
   def attach_dnskeys(dnssec_data)
     sg = SettingGroup.dnskeys
-    ds_data_allowed = sg.setting(Setting::ALLOW_DS_DATA).value == '0' ? false : true
-    ds_data_with_keys_allowed = sg.setting(Setting::ALLOW_DS_DATA_WITH_KEYS).value == '0' ? false : true
-    key_data_allowed = sg.setting(Setting::ALLOW_KEY_DATA).value == '0' ? false : true
-
-    if dnssec_data[:ds_data].any? && !ds_data_allowed
-      errors.add(:base, :ds_data_not_allowed)
-      return
-    end
+    return false unless validate_dnssec_data(dnssec_data, sg)
 
     dnssec_data[:ds_data].each do |ds_data|
-      if ds_data[:public_key] && !ds_data_with_keys_allowed
-        errors.add(:base, :ds_data_with_keys_not_allowed)
-        next
-      else
-        dnskeys.build(ds_data)
-      end
-    end
-
-    if dnssec_data[:key_data].any? && !key_data_allowed
-      errors.add(:base, :key_data_not_allowed)
-      return
+      dnskeys.build(ds_data)
     end
 
     dnssec_data[:key_data].each do |x|
@@ -217,8 +201,44 @@ class Epp::EppDomain < Domain
         ds_digest_type: sg.setting(Setting::DS_ALGORITHM).value
       }.merge(x))
     end
+  end
 
-    errors.any?
+  def validate_dnssec_data(dnssec_data, sg)
+    ds_data_allowed?(dnssec_data, sg)
+    ds_data_with_keys_allowed?(dnssec_data, sg)
+    key_data_allowed?(dnssec_data, sg)
+
+    errors.empty?
+  end
+
+  def ds_data_allowed?(dnssec_data, sg)
+    ds_data_allowed = sg.setting(Setting::ALLOW_DS_DATA).value == '0' ? false : true
+
+    return if (dnssec_data[:ds_data].any? && ds_data_allowed) || dnssec_data[:ds_data].empty?
+    errors.add(:base, :ds_data_not_allowed)
+  end
+
+  def ds_data_with_keys_allowed?(dnssec_data, sg)
+    ds_data_with_keys_allowed = sg.setting(Setting::ALLOW_DS_DATA_WITH_KEYS).value == '0' ? false : true
+
+    dnssec_data[:ds_data].each do |ds_data|
+      if key_data?(ds_data) && !ds_data_with_keys_allowed
+        errors.add(:base, :ds_data_with_key_not_allowed)
+        return
+      end
+    end
+  end
+
+  def key_data_allowed?(dnssec_data, sg)
+    key_data_allowed = sg.setting(Setting::ALLOW_KEY_DATA).value == '0' ? false : true
+
+    return if (dnssec_data[:key_data].any? && key_data_allowed) || dnssec_data[:key_data].empty?
+    errors.add(:base, :key_data_not_allowed)
+  end
+
+  def key_data?(data)
+    key_data_attrs = [:public_key, :alg, :protocol, :flags]
+    (data.keys & key_data_attrs).any?
   end
 
   def detach_dnskeys(dnssec_data)

config/locales/en.yml

@@ -75,7 +75,7 @@ en:
               domain_status_prohibits_operation: 'Domain status prohibits operation'
               domain_already_belongs_to_the_querying_registrar: 'Domain already belongs to the querying registrar'
               ds_data_not_allowed: 'dsData object is not allowed'
-              ds_data_with_keys_not_allowed: 'dsData object with key data is not allowed'
+              ds_data_with_key_not_allowed: 'dsData object with key data is not allowed'
               key_data_not_allowed: 'keyData object is not allowed'
             name_dirty:
               invalid: 'Domain name is invalid'

spec/epp/domain_spec.rb

@@ -509,7 +509,6 @@ describe 'EPP Domain', epp: true do
       end
 
       it 'creates domain with ds data' do
-        pending true
         xml = domain_create_xml({}, {
           _other: [
             { dsData: {
@@ -553,7 +552,7 @@ describe 'EPP Domain', epp: true do
             }]
           })
 
-        r = epp_request(xml, :xml)
+        epp_request(xml, :xml)
 
         d = Domain.first
         ds = d.dnskeys.first
@@ -567,6 +566,33 @@ describe 'EPP Domain', epp: true do
         expect(ds.public_key).to eq('700b97b591ed27ec2590d19f06f88bba700b97b591ed27ec2590d19f')
       end
 
+      it 'does not create domain with ds data when it is not allowed' do
+        sg = SettingGroup.dnskeys
+        s = sg.setting(Setting::ALLOW_DS_DATA_WITH_KEYS)
+        s.value = 0
+        s.save
+
+        xml = domain_create_xml({}, {
+          _other: [
+            { dsData: {
+                keyTag: { value: '12345' },
+                alg: { value: '3' },
+                digestType: { value: '1' },
+                digest: { value: '49FD46E6C4B45C55D4AC' },
+                keyData: {
+                  flags: { value: '0' },
+                  protocol: { value: '3' },
+                  alg: { value: '5' },
+                  pubKey: { value: '700b97b591ed27ec2590d19f06f88bba700b97b591ed27ec2590d19f' }
+                }
+              }
+            }]
+          })
+
+        response = epp_request(xml, :xml)
+        expect(response[:result_code]).to eq('2306')
+        expect(response[:msg]).to eq('dsData object with key data is not allowed')
+      end
     end
 
     context 'with juridical persion as an owner' do