From 714b8364cbc0c77023aaf34c88bc71ceeb84eb04 Mon Sep 17 00:00:00 2001
From: Matt Farnsworth <matt@perfectline.co>
Date: Fri, 13 Nov 2015 15:01:18 +0200
Subject: [PATCH 01/27] Story #107192666 - only show dsData if allowed, or
 keyData if allowed

---
 app/views/epp/domains/info.xml.builder | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/app/views/epp/domains/info.xml.builder b/app/views/epp/domains/info.xml.builder
index e7962002c..645c731e4 100644
--- a/app/views/epp/domains/info.xml.builder
+++ b/app/views/epp/domains/info.xml.builder
@@ -81,18 +81,17 @@ xml.epp_head do
       end
 
       xml.tag!('secDNS:infData', 'xmlns:secDNS' => 'urn:ietf:params:xml:ns:secDNS-1.1') do
-        # might not have ds in first key? maybe check any? k.ds_digest if requirements change (DS not accepted by EIS)
-        if @domain.dnskeys[0].ds_digest.blank?
-          @domain.dnskeys.sort.each do |key|
-            tag_key_data(xml, key)
+        if Setting.ds_data_allowed
+          (@domain.dnskeys.find_all { |key| key.ds_digest.present? }).sort.each do |key|
+            tag_ds_data(xml, key)
           end
         else
-          @domain.dnskeys.sort.each do |key|
-            tag_ds_data(xml, key)
+          (@domain.dnskeys.find_all { |key| key.ds_digest.blank? }).sort.each do |key|
+            tag_key_data(xml, key)
           end
         end
       end
-    end if @domain.dnskeys.any?
+    end if @domain.dnskeys.any? && (Setting.ds_data_allowed ? @domain.dnskeys.any? { |key| key.ds_digest.present? } :  Setting.key_data_allowed)
 
     render('epp/shared/trID', builder: xml)
   end

From ad3a2a6adb89587e36d5e1dab4e742726244093c Mon Sep 17 00:00:00 2001
From: Matt Farnsworth <matt@perfectline.co>
Date: Fri, 13 Nov 2015 15:23:24 +0200
Subject: [PATCH 02/27] Story #107192666 - ensure registrary/domains/info shows
 only actual data

patch updates previous revision 7d9272c60c90ba27f272a9495829a121ff26ca43
---
 .../registrar/domains/partials/_dnskeys.haml     | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/app/views/registrar/domains/partials/_dnskeys.haml b/app/views/registrar/domains/partials/_dnskeys.haml
index 46dcd0fce..5b8a95ade 100644
--- a/app/views/registrar/domains/partials/_dnskeys.haml
+++ b/app/views/registrar/domains/partials/_dnskeys.haml
@@ -4,23 +4,17 @@
   .panel-body{style: 'word-wrap: break-word;'}
     - @data.css('dsData').each do |x|
       %dl.dl-horizontal
-        - if x.css('keyTag').text.present?
+        - if x.css('digest').text.present?
           %dt= t(:ds_key_tag)
           %dd= x.css('keyTag').text
-
-        - if x.css('alg').first.text.present?
           %dt= t(:ds_algorithm)
           %dd= x.css('alg').first.text
-
-        - if x.css('digestType').text.present?
           %dt= t(:ds_digest_type)
           %dd= x.css('digestType').text
-
-        - if x.css('digest').text.present?
           %dt= t(:ds_digest)
           %dd= x.css('digest').text
-        - @data.css('keyData').each do |x|
-          %dl.dl-horizontal
+          - @data.css('keyData').each do |x|
+            %dl.dl-horizontal
             %dt= t(:flag)
             %dd= x.css('flags').text
 
@@ -32,9 +26,9 @@
 
             %dt= t(:public_key)
             %dd= x.css('pubKey').text
-
-    - @data.css('keyData').each do |x|
+    - if @data.css('dsData').empty?
       %dl.dl-horizontal
+      - @data.css('keyData').each do |x|
         %dt= t(:flag)
         %dd= x.css('flags').text
 

From e7c2444161978665f61a32428aa07cf7598683cf Mon Sep 17 00:00:00 2001
From: Matt Farnsworth <matt@perfectline.co>
Date: Fri, 13 Nov 2015 19:06:12 +0200
Subject: [PATCH 03/27] Story #107192666 - refactor to add stricter inclusion
 rules reformatted block, white space changes, indent for surounding if any?
 pull up find_all? if *_allowed then check if allowed.any?

---
 app/views/epp/domains/info.xml.builder | 63 ++++++++++++++------------
 1 file changed, 34 insertions(+), 29 deletions(-)

diff --git a/app/views/epp/domains/info.xml.builder b/app/views/epp/domains/info.xml.builder
index 645c731e4..17892a3cc 100644
--- a/app/views/epp/domains/info.xml.builder
+++ b/app/views/epp/domains/info.xml.builder
@@ -60,39 +60,44 @@ xml.epp_head do
       end
     end
 
-    xml.extension do
-      def tag_key_data(xml, key)
-        xml.tag!('secDNS:keyData') do
-          xml.tag!('secDNS:flags', key.flags)
-          xml.tag!('secDNS:protocol', key.protocol)
-          xml.tag!('secDNS:alg', key.alg)
-          xml.tag!('secDNS:pubKey', key.public_key)
-        end
-      end
+    if @domain.dnskeys.any?
+      ds_data  = Setting.ds_data_allowed  ? @domain.dnskeys.find_all { |key| key.ds_digest.present? } : []
+      key_data = Setting.key_data_allowed ? @domain.dnskeys.find_all { |key| key.ds_digest.blank? } : []
 
-      def tag_ds_data(xml, key)
-        xml.tag!('secDNS:dsData') do
-          xml.tag!('secDNS:keyTag', key.ds_key_tag)
-          xml.tag!('secDNS:alg', key.ds_alg)
-          xml.tag!('secDNS:digestType', key.ds_digest_type)
-          xml.tag!('secDNS:digest', key.ds_digest)
-          tag_key_data(xml, key) if key.public_key.present?
-        end
-      end
-
-      xml.tag!('secDNS:infData', 'xmlns:secDNS' => 'urn:ietf:params:xml:ns:secDNS-1.1') do
-        if Setting.ds_data_allowed
-          (@domain.dnskeys.find_all { |key| key.ds_digest.present? }).sort.each do |key|
-            tag_ds_data(xml, key)
-          end
-        else
-          (@domain.dnskeys.find_all { |key| key.ds_digest.blank? }).sort.each do |key|
-            tag_key_data(xml, key)
+      # is there any reason to include <extension> without <secDNS:infData>
+      xml.extension do
+        def tag_key_data(xml, key)
+          xml.tag!('secDNS:keyData') do
+            xml.tag!('secDNS:flags', key.flags)
+            xml.tag!('secDNS:protocol', key.protocol)
+            xml.tag!('secDNS:alg', key.alg)
+            xml.tag!('secDNS:pubKey', key.public_key)
           end
         end
-      end
-    end if @domain.dnskeys.any? && (Setting.ds_data_allowed ? @domain.dnskeys.any? { |key| key.ds_digest.present? } :  Setting.key_data_allowed)
 
+        def tag_ds_data(xml, key)
+          xml.tag!('secDNS:dsData') do
+            xml.tag!('secDNS:keyTag', key.ds_key_tag)
+            xml.tag!('secDNS:alg', key.ds_alg)
+            xml.tag!('secDNS:digestType', key.ds_digest_type)
+            xml.tag!('secDNS:digest', key.ds_digest)
+            tag_key_data(xml, key) if key.public_key.present?
+          end
+        end
+
+        xml.tag!('secDNS:infData', 'xmlns:secDNS' => 'urn:ietf:params:xml:ns:secDNS-1.1') do
+          if Setting.ds_data_allowed
+            ds_data.sort.each do |key|
+              tag_ds_data(xml, key)
+            end
+          else
+            key_data.sort.each do |key|
+              tag_key_data(xml, key)
+            end
+          end
+        end
+      end if key_data.present? || ds_data.present?
+    end
     render('epp/shared/trID', builder: xml)
   end
 end

From fd4b2debb2c2c192ec129e1056427b94f314462e Mon Sep 17 00:00:00 2001
From: Matt Farnsworth <matt@perfectline.co>
Date: Fri, 13 Nov 2015 19:30:55 +0200
Subject: [PATCH 04/27] Story #107192666 - refactor dns sec key processing,
 ensure rules are applied and multiple keys ok

---
 app/models/epp/domain.rb | 112 +++++++++++++++++++++++----------------
 1 file changed, 66 insertions(+), 46 deletions(-)

diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb
index f59df6ddc..582ad63f4 100644
--- a/app/models/epp/domain.rb
+++ b/app/models/epp/domain.rb
@@ -314,20 +314,11 @@ class Epp::Domain < Domain
   # rubocop: disable Metrics/PerceivedComplexity
   # rubocop: disable Metrics/CyclomaticComplexity
   def dnskeys_attrs(frame, action)
-    if frame.css('dsData').any? && !Setting.ds_data_allowed
-      errors.add(:base, :ds_data_not_allowed)
-    end
-
-    if frame.xpath('keyData').any? && !Setting.key_data_allowed
-      errors.add(:base, :key_data_not_allowed)
-    end
-
-    res = ds_data_from(frame)
-    dnskeys_list = key_data_from(frame, res)
-
+    return [] if frame.empty?
+    keys = DnsSecKeys.new(self).values frame
     if action == 'rem'
       to_destroy = []
-      dnskeys_list.each do |x|
+      keys.each do |x|
         dk = dnskeys.find_by(public_key: x[:public_key])
 
         unless dk
@@ -343,51 +334,80 @@ class Epp::Domain < Domain
 
       return to_destroy
     else
-      return dnskeys_list
+      return keys
     end
   end
   # rubocop: enable Metrics/PerceivedComplexity
   # rubocop: enable Metrics/CyclomaticComplexity
 
-  def key_data_from(frame, res)
-    frame.xpath('keyData').each do |x|
-      res << {
-        flags: x.css('flags').first.try(:text),
-        protocol: x.css('protocol').first.try(:text),
-        alg: x.css('alg').first.try(:text),
-        public_key: x.css('pubKey').first.try(:text),
-        ds_alg: 3,
-        ds_digest_type: Setting.ds_algorithm
-      }
+  class DnsSecKeys
+    def initialize(domain)
+      @domain = domain
     end
 
-    res
-  end
-
-  def ds_data_from(frame)
-    res = []
-    frame.css('dsData').each do |x|
-      data = {
-        ds_key_tag: x.css('keyTag').first.try(:text),
-        ds_alg: x.css('alg').first.try(:text),
-        ds_digest_type: x.css('digestType').first.try(:text),
-        ds_digest: x.css('digest').first.try(:text)
-      }
-
-      kd = x.css('keyData').first
-      data.merge!({
-        flags: kd.css('flags').first.try(:text),
-        protocol: kd.css('protocol').first.try(:text),
-        alg: kd.css('alg').first.try(:text),
-        public_key: kd.css('pubKey').first.try(:text)
-      }) if kd
-
-      res << data
+    def values(frame)
+      if Setting.key_data_allowed
+        @domain.errors.add(:base, :ds_data_not_allowed) if frame.css('dsData').present?
+        return key_data_from frame
+      end
+      if Setting.ds_data_allowed
+        @domain.errors.add(:base, :key_data_not_allowed) if frame.css('keyData').present?
+        return ds_data_from frame
+      end
+      []
+    end
+
+    private
+
+    KEY_INTERFACE = {flags: 'flags', protocol: 'protocol', alg: 'alg', public_key: 'pubKey' }
+    DS_INTERFACE  =
+        { ds_key_tag:     'keyTag',
+          ds_alg:         'alg',
+          ds_digest_type: 'digestType',
+          ds_digest:      'digest'
+        }
+
+    def xm_copy(frame, map)
+      result = {}
+      map.each do |key, value|
+        result[key] = frame.css(value).first.try(:text)
+      end
+      result
+    end
+
+    # frame requires NodeSet with direct children of keyData
+    def key_data(frame)
+      result = xm_copy frame.css('keyData'), KEY_INTERFACE
+      # TODO: can these defaults go where they belong?
+      result.merge({
+                       ds_alg: 3,
+                       ds_digest_type: Setting.ds_algorithm
+                   })
+    end
+
+    # get all top level dsData from NodeSet
+    def ds_data_from(frame)
+      result = []
+      frame.css('dsData').each do |ds_data|
+        key = ds_data.css('keyData')
+        ds = xm_copy ds_data, DS_INTERFACE
+        ds.merge! (key_data key) if key.present?
+        result << ds
+      end
+      result
+    end
+
+    def key_data_from(frame)
+      result = []
+      frame.css('keyData').each do |key|
+        result << key_data(key)
+      end
+      result
     end
 
-    res
   end
 
+
   def domain_statuses_attrs(frame, action)
     status_list = domain_status_list_from(frame)
     if action == 'rem'

From 035111e13022958763762f1cf6e8a624e034a369 Mon Sep 17 00:00:00 2001
From: Vladimir Krylov <vladimir.krylov@perfectline.co>
Date: Mon, 16 Nov 2015 12:52:26 +0200
Subject: [PATCH 05/27] Story#107192666 - validation for settings that DS data
 allowed and Allow key data cannot be both true

---
 app/controllers/admin/settings_controller.rb | 16 +++++++++++-----
 app/models/setting.rb                        | 14 ++++++++++++++
 app/views/admin/settings/_setting_row.haml   |  2 +-
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/app/controllers/admin/settings_controller.rb b/app/controllers/admin/settings_controller.rb
index 6a0c5c033..4014cd7fc 100644
--- a/app/controllers/admin/settings_controller.rb
+++ b/app/controllers/admin/settings_controller.rb
@@ -7,12 +7,18 @@ class Admin::SettingsController < AdminController
   end
 
   def create
-    casted_settings.each do |k, v|
-      Setting[k] = v
-    end
+    @errors = Setting.params_errors(casted_settings)
+    if @errors.empty?
+      casted_settings.each do |k, v|
+        Setting[k] = v
+      end
 
-    flash[:notice] = I18n.t('records_updated')
-    redirect_to [:admin, :settings]
+      flash[:notice] = I18n.t('records_updated')
+      redirect_to [:admin, :settings]
+    else
+      flash[:alert] = @errors.values.uniq.join(", ")
+      render "admin/settings/index"
+    end
   end
 
   def show; end
diff --git a/app/models/setting.rb b/app/models/setting.rb
index 122bfc99a..cb52fd1b7 100644
--- a/app/models/setting.rb
+++ b/app/models/setting.rb
@@ -6,4 +6,18 @@ class Setting < RailsSettings::CachedSettings
     Rails.cache.delete_matched('settings:.*')
     STDOUT << "#{Time.zone.now.utc} - Settings cache cleared\n"
   end
+
+
+  # cannot do instance validation because CachedSetting use save!
+  def self.params_errors(params)
+    errors = {}
+    # DS data allowed and Allow key data cannot be both true
+    if !!params["key_data_allowed"] && params["key_data_allowed"] == params["ds_data_allowed"]
+      msg = "#{I18n.t(:key_data_allowed)} and #{I18n.t(:ds_data_with_key_allowed)} cannot be both true"
+      errors["key_data_allowed"] = msg
+      errors["ds_data_allowed"]  = msg
+    end
+
+    return errors
+  end
 end
diff --git a/app/views/admin/settings/_setting_row.haml b/app/views/admin/settings/_setting_row.haml
index 632effc74..ab0d8d991 100644
--- a/app/views/admin/settings/_setting_row.haml
+++ b/app/views/admin/settings/_setting_row.haml
@@ -1,5 +1,5 @@
 - value = Setting.send(var)
-%tr
+%tr{class: (@errors && @errors.has_key?(var.to_s) && "danger")}
   %td= t(var)
   - if [TrueClass, FalseClass].include?(value.class)
     %td

From 6fcf72a8e73f2a939a403f04780342d842daabec Mon Sep 17 00:00:00 2001
From: Matt Farnsworth <matt@perfectline.co>
Date: Sat, 14 Nov 2015 23:40:45 +0200
Subject: [PATCH 06/27] Story #107192666 - bug fix
 fd4b2debb2c2c192ec129e1056427b94f314462e, refactor, support secDNS:all

---
 app/models/epp/domain.rb | 79 +++++++++++++++++++++++-----------------
 1 file changed, 46 insertions(+), 33 deletions(-)

diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb
index 582ad63f4..45694bb1f 100644
--- a/app/models/epp/domain.rb
+++ b/app/models/epp/domain.rb
@@ -314,8 +314,31 @@ class Epp::Domain < Domain
   # rubocop: disable Metrics/PerceivedComplexity
   # rubocop: disable Metrics/CyclomaticComplexity
   def dnskeys_attrs(frame, action)
-    return [] if frame.empty?
-    keys = DnsSecKeys.new(self).values frame
+    keys = []
+    return keys if frame.blank?
+
+    if frame.xpath('dnsSec:all').present?
+      # support delete all or all of
+    else
+      inf_data = DnsSecKeys.new(frame)
+      if Setting.key_data_allowed
+        if inf_data.ds_data.present?
+          errors.add(:base, :ds_data_not_allowed)
+          return
+        else
+          keys = inf_data.key_data
+        end
+      end
+      if Setting.ds_data_allowed
+        if inf_data.key_data.present?
+          errors.add(:base, :key_data_not_allowed)
+          return
+        else
+          keys = inf_data.ds_data
+        end
+      end
+    end
+
     if action == 'rem'
       to_destroy = []
       keys.each do |x|
@@ -341,20 +364,22 @@ class Epp::Domain < Domain
   # rubocop: enable Metrics/CyclomaticComplexity
 
   class DnsSecKeys
-    def initialize(domain)
-      @domain = domain
+    def initialize(frame)
+      @key_data = []
+      @ds_data = []
+      if frame.css('dsData').present?
+        ds_data_from frame
+      end
+      frame.css('keyData').each do |key|
+        @key_data.append key_data_from(key)
+      end
     end
 
-    def values(frame)
-      if Setting.key_data_allowed
-        @domain.errors.add(:base, :ds_data_not_allowed) if frame.css('dsData').present?
-        return key_data_from frame
-      end
-      if Setting.ds_data_allowed
-        @domain.errors.add(:base, :key_data_not_allowed) if frame.css('keyData').present?
-        return ds_data_from frame
-      end
-      []
+    attr_reader :key_data
+    attr_reader :ds_data
+
+    def error
+      ds_data.present? && key_data.present?
     end
 
     private
@@ -369,15 +394,15 @@ class Epp::Domain < Domain
 
     def xm_copy(frame, map)
       result = {}
-      map.each do |key, value|
-        result[key] = frame.css(value).first.try(:text)
+      map.each do |key, elem|
+        # content validation might happen later in Dnskey, if we get that far; or not. TODO: check handling
+        result[key] = frame.css(elem).first.try(:text)
       end
       result
     end
 
-    # frame requires NodeSet with direct children of keyData
-    def key_data(frame)
-      result = xm_copy frame.css('keyData'), KEY_INTERFACE
+    def key_data_from(frame)
+      result = xm_copy frame, KEY_INTERFACE
       # TODO: can these defaults go where they belong?
       result.merge({
                        ds_alg: 3,
@@ -385,26 +410,14 @@ class Epp::Domain < Domain
                    })
     end
 
-    # get all top level dsData from NodeSet
     def ds_data_from(frame)
-      result = []
       frame.css('dsData').each do |ds_data|
         key = ds_data.css('keyData')
         ds = xm_copy ds_data, DS_INTERFACE
-        ds.merge! (key_data key) if key.present?
-        result << ds
+        ds.merge! (key_data_from key) if key.present?
+        @ds_data << ds
       end
-      result
     end
-
-    def key_data_from(frame)
-      result = []
-      frame.css('keyData').each do |key|
-        result << key_data(key)
-      end
-      result
-    end
-
   end
 
 

From b0860b278b1a8f465cbe274d2663446be332b925 Mon Sep 17 00:00:00 2001
From: Matt Farnsworth <matt@perfectline.co>
Date: Mon, 16 Nov 2015 12:14:11 +0200
Subject: [PATCH 07/27] Story #107192666 - feature fix, keyData interface
 should include flags=KSK

---
 app/views/epp/domains/info.xml.builder | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/epp/domains/info.xml.builder b/app/views/epp/domains/info.xml.builder
index 17892a3cc..f89fcd168 100644
--- a/app/views/epp/domains/info.xml.builder
+++ b/app/views/epp/domains/info.xml.builder
@@ -62,7 +62,7 @@ xml.epp_head do
 
     if @domain.dnskeys.any?
       ds_data  = Setting.ds_data_allowed  ? @domain.dnskeys.find_all { |key| key.ds_digest.present? } : []
-      key_data = Setting.key_data_allowed ? @domain.dnskeys.find_all { |key| key.ds_digest.blank? } : []
+      key_data = Setting.key_data_allowed ? @domain.dnskeys.find_all { |key| key.public_key.present? } : []
 
       # is there any reason to include <extension> without <secDNS:infData>
       xml.extension do

From d0fbec8f56b417607df2b158b8d150c24d4442fb Mon Sep 17 00:00:00 2001
From: Matt Farnsworth <matt@perfectline.co>
Date: Mon, 16 Nov 2015 13:26:52 +0200
Subject: [PATCH 08/27] Story #107192666 - EIS policy specifies keyData
 interface not dsData. Both true is not allowed

---
 config/initializers/initial_settings.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config/initializers/initial_settings.rb b/config/initializers/initial_settings.rb
index bcee95150..35c3eebb3 100644
--- a/config/initializers/initial_settings.rb
+++ b/config/initializers/initial_settings.rb
@@ -13,7 +13,7 @@ if con.present? && con.table_exists?('settings')
   Setting.save_default(:expire_pending_confirmation, 48)
 
   Setting.save_default(:ds_algorithm, 2)
-  Setting.save_default(:ds_data_allowed, true)
+  Setting.save_default(:ds_data_allowed, false)
   Setting.save_default(:key_data_allowed, true)
 
   Setting.save_default(:dnskeys_min_count, 0)

From 13c3d2092fe225a9e885a59b9b5729a2311b23e5 Mon Sep 17 00:00:00 2001
From: Matt Farnsworth <matt@perfectline.co>
Date: Mon, 16 Nov 2015 17:51:52 +0200
Subject: [PATCH 09/27] Story #107192666 - refactor, add support for remove
 all, bug fix overly visible nodes

---
 app/models/epp/domain.rb | 110 +++++++++++++++++++--------------------
 1 file changed, 55 insertions(+), 55 deletions(-)

diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb
index 45694bb1f..7b55b018d 100644
--- a/app/models/epp/domain.rb
+++ b/app/models/epp/domain.rb
@@ -316,49 +316,26 @@ class Epp::Domain < Domain
   def dnskeys_attrs(frame, action)
     keys = []
     return keys if frame.blank?
+    inf_data = DnsSecKeys.new(frame)
 
-    if frame.xpath('dnsSec:all').present?
-      # support delete all or all of
+    if  action == 'rem' &&
+        frame.css('rem > all').first.try(:text) == 'true'
+      keys = inf_data.mark_destroy_all dnskeys
     else
-      inf_data = DnsSecKeys.new(frame)
       if Setting.key_data_allowed
-        if inf_data.ds_data.present?
-          errors.add(:base, :ds_data_not_allowed)
-          return
-        else
-          keys = inf_data.key_data
-        end
+        errors.add(:base, :ds_data_not_allowed) if inf_data.ds_data.present?
+        keys = inf_data.key_data
       end
       if Setting.ds_data_allowed
-        if inf_data.key_data.present?
-          errors.add(:base, :key_data_not_allowed)
-          return
-        else
-          keys = inf_data.ds_data
-        end
+        errors.add(:base, :key_data_not_allowed) if inf_data.key_data.present?
+        keys = inf_data.ds_data
+      end
+      if action == 'rem'
+        keys = inf_data.mark_destroy(dnskeys)
+        add_epp_error('2303', nil, nil, [:dnskeys, :not_found]) if keys.include? nil
       end
     end
-
-    if action == 'rem'
-      to_destroy = []
-      keys.each do |x|
-        dk = dnskeys.find_by(public_key: x[:public_key])
-
-        unless dk
-          add_epp_error('2303', 'publicKey', x[:public_key], [:dnskeys, :not_found])
-          next
-        end
-
-        to_destroy << {
-          id: dk.id,
-          _destroy: 1
-        }
-      end
-
-      return to_destroy
-    else
-      return keys
-    end
+    errors.any? ? [] : keys
   end
   # rubocop: enable Metrics/PerceivedComplexity
   # rubocop: enable Metrics/CyclomaticComplexity
@@ -367,19 +344,28 @@ class Epp::Domain < Domain
     def initialize(frame)
       @key_data = []
       @ds_data = []
+      # schema validation prevents both in the same parent node
       if frame.css('dsData').present?
         ds_data_from frame
-      end
-      frame.css('keyData').each do |key|
-        @key_data.append key_data_from(key)
+      else
+        frame.css('keyData').each do |key|
+          @key_data.append key_data_from(key)
+        end
       end
     end
 
     attr_reader :key_data
     attr_reader :ds_data
 
-    def error
-      ds_data.present? && key_data.present?
+    def mark_destroy_all(dns_keys)
+      # if transition support required mark_destroy dns_keys when has ds/key values otherwise ...
+      dns_keys.map { |inf_data| mark inf_data }
+    end
+
+    def mark_destroy(dns_keys)
+      (ds_data.present? ? ds_filter(dns_keys) : kd_filter(dns_keys)).map do |inf_data|
+        inf_data.blank? ? nil : mark(inf_data)
+      end
     end
 
     private
@@ -395,7 +381,6 @@ class Epp::Domain < Domain
     def xm_copy(frame, map)
       result = {}
       map.each do |key, elem|
-        # content validation might happen later in Dnskey, if we get that far; or not. TODO: check handling
         result[key] = frame.css(elem).first.try(:text)
       end
       result
@@ -405,8 +390,8 @@ class Epp::Domain < Domain
       result = xm_copy frame, KEY_INTERFACE
       # TODO: can these defaults go where they belong?
       result.merge({
-                       ds_alg: 3,
-                       ds_digest_type: Setting.ds_algorithm
+                       ds_alg: 3,      # DSA/SHA-1 [DSA] RFC2536
+                       ds_digest_type: Setting.ds_algorithm     # only 1
                    })
     end
 
@@ -414,13 +399,28 @@ class Epp::Domain < Domain
       frame.css('dsData').each do |ds_data|
         key = ds_data.css('keyData')
         ds = xm_copy ds_data, DS_INTERFACE
-        ds.merge! (key_data_from key) if key.present?
+        ds.merge(key_data_from key) if key.present?
         @ds_data << ds
       end
     end
+
+    def ds_filter(dns_keys)
+      @ds_data.map do |ds|
+        dns_keys.find_by(ds.slice(*DS_INTERFACE.keys))
+      end
+     end
+
+    def kd_filter(dns_keys)
+      @key_data.map do |key|
+        dns_keys.find_by(key)
+      end
+    end
+
+    def mark(inf_data)
+      { id: inf_data.id, _destroy: 1 }
+    end
   end
-
-
+  
   def domain_statuses_attrs(frame, action)
     status_list = domain_status_list_from(frame)
     if action == 'rem'
@@ -822,14 +822,14 @@ class Epp::Domain < Domain
   def transferrable?
     (statuses & [
         DomainStatus::PENDING_DELETE_CONFIRMATION,
-      DomainStatus::PENDING_CREATE,
-      DomainStatus::PENDING_UPDATE,
-      DomainStatus::PENDING_DELETE,
-      DomainStatus::PENDING_RENEW,
-      DomainStatus::PENDING_TRANSFER,
-      DomainStatus::FORCE_DELETE,
-      DomainStatus::SERVER_TRANSFER_PROHIBITED,
-      DomainStatus::CLIENT_TRANSFER_PROHIBITED
+        DomainStatus::PENDING_CREATE,
+        DomainStatus::PENDING_UPDATE,
+        DomainStatus::PENDING_DELETE,
+        DomainStatus::PENDING_RENEW,
+        DomainStatus::PENDING_TRANSFER,
+        DomainStatus::FORCE_DELETE,
+        DomainStatus::SERVER_TRANSFER_PROHIBITED,
+        DomainStatus::CLIENT_TRANSFER_PROHIBITED
     ]).empty?
   end
 

From 3bb6150c1f968836823219805b31aaf02a54d670 Mon Sep 17 00:00:00 2001
From: Vladimir Krylov <vladimir.krylov@perfectline.co>
Date: Fri, 13 Nov 2015 15:09:04 +0200
Subject: [PATCH 10/27] Story#107279016 - admin shows all reserved domains line
 by line

---
 .../admin/reserved_domains_controller.rb      | 28 +++++++++++++++----
 app/models/reserved_domain.rb                 | 10 ++++++-
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/app/controllers/admin/reserved_domains_controller.rb b/app/controllers/admin/reserved_domains_controller.rb
index 430fb9d8a..57e4b8ed3 100644
--- a/app/controllers/admin/reserved_domains_controller.rb
+++ b/app/controllers/admin/reserved_domains_controller.rb
@@ -2,9 +2,9 @@ class Admin::ReservedDomainsController < AdminController
   load_and_authorize_resource
 
   def index
-    rd = ReservedDomain.first_or_initialize
-    rd.names = nil if rd.names.blank?
-    @reserved_domains = rd.names.to_yaml.gsub(/---.?\n/, '').gsub(/\.\.\..?\n/, '')
+    names = ReservedDomain.pluck(:names).each_with_object({}){|e_h,h| h.merge!(e_h)}
+    names.names = nil if names.blank?
+    @reserved_domains = names.to_yaml.gsub(/---.?\n/, '').gsub(/\.\.\..?\n/, '')
   end
 
   def create
@@ -20,9 +20,27 @@ class Admin::ReservedDomainsController < AdminController
       render :index and return
     end
 
-    rd = ReservedDomain.first_or_create
+    result = true
+    ReservedDomain.transaction do
+      # removing old ones
+      existing = ReservedDomain.any_of_domains(names.keys).pluck(:id)
+      ReservedDomain.where.not(id: existing).delete_all
 
-    if rd.update(names: names)
+      #updating and adding
+      names.each do |name, psw|
+        rec   = ReservedDomain.by_domain(name).first
+        rec ||= ReservedDomain.new
+        rec.names = {name => psw}
+
+        unless rec.save
+          result = false
+          raise ActiveRecord::Rollback
+        end
+      end
+    end
+
+
+    if result
       flash[:notice] = I18n.t('record_updated')
       redirect_to :back
     else
diff --git a/app/models/reserved_domain.rb b/app/models/reserved_domain.rb
index 6684d6541..936c744bf 100644
--- a/app/models/reserved_domain.rb
+++ b/app/models/reserved_domain.rb
@@ -9,7 +9,15 @@ class ReservedDomain < ActiveRecord::Base
 
   class << self
     def pw_for(domain_name)
-      select("names -> '#{domain_name}' AS pw").first.try(:pw)
+      by_domain(domain_name).select("names -> '#{domain_name}' AS pw").first.try(:pw)
+    end
+
+    def by_domain name
+      where("names ? '#{name}'")
+    end
+
+    def any_of_domains names
+      where("names ?| ARRAY['#{names.join("','")}']")
     end
   end
 end

From 3db92b246ac62541f643d004f89ec96112236d7a Mon Sep 17 00:00:00 2001
From: Stas <stanislav@perfectline.co>
Date: Fri, 13 Nov 2015 15:49:13 +0200
Subject: [PATCH 11/27] 105361584-no access added

---
 app/views/epp/contacts/info.xml.builder | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder
index e890e0fb6..d29e60298 100644
--- a/app/views/epp/contacts/info.xml.builder
+++ b/app/views/epp/contacts/info.xml.builder
@@ -15,6 +15,7 @@ xml.epp_head do
 
         xml.tag!('contact:postalInfo', type: 'int') do
           xml.tag!('contact:name', @contact.name)
+          if can? :view_full_info, @contact, @password
             xml.tag!('contact:org', @contact.org_name) if @contact.org_name.present?
             xml.tag!('contact:addr') do
               xml.tag!('contact:street', @contact.street)
@@ -23,11 +24,27 @@ xml.epp_head do
               xml.tag!('contact:pc', @contact.zip)
               xml.tag!('contact:cc', @contact.country_code)
             end
+          else
+            xml.tag!('contact:org', 'No access')
+            xml.tag!('contact:addr') do
+              xml.tag!('contact:street', 'No access')
+              xml.tag!('contact:city', 'No access')
+              xml.tag!('contact:sp', 'No access')
+              xml.tag!('contact:pc', 'No access')
+              xml.tag!('contact:cc', 'No access')
+            end
+          end
         end
 
+        if can? :view_full_info, @contact, @password
           xml.tag!('contact:voice', @contact.phone)
           xml.tag!('contact:fax', @contact.fax) if @contact.fax.present?
           xml.tag!('contact:email', @contact.email)
+        else
+          xml.tag!('contact:voice', 'No access')
+          xml.tag!('contact:fax', 'No access')
+          xml.tag!('contact:email', 'No access')
+        end
 
         xml.tag!('contact:clID', @contact.registrar.try(:name))
         if @contact.creator.try(:registrar).blank? && Rails.env.test?

From 091ce79dd85e523d666550be612f0bd33d0b1b94 Mon Sep 17 00:00:00 2001
From: Stas <stanislav@perfectline.co>
Date: Fri, 13 Nov 2015 15:57:45 +0200
Subject: [PATCH 12/27] 105361584-no acces for country

---
 app/models/depp/contact.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/depp/contact.rb b/app/models/depp/contact.rb
index 0fa9f777a..b4597867c 100644
--- a/app/models/depp/contact.rb
+++ b/app/models/depp/contact.rb
@@ -280,7 +280,7 @@ module Depp
     end
 
     def country_name
-      Country.new(country_code)
+      Country.new(country_code) || 'No access'
     end
 
     def org?

From be64143db683b607e474f8e0bf52b6f1f25046f8 Mon Sep 17 00:00:00 2001
From: Stas <stanislav@perfectline.co>
Date: Fri, 13 Nov 2015 16:20:45 +0200
Subject: [PATCH 13/27] 105361584-no access for ident and pass

---
 app/views/epp/contacts/info.xml.builder | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder
index d29e60298..24b37a13f 100644
--- a/app/views/epp/contacts/info.xml.builder
+++ b/app/views/epp/contacts/info.xml.builder
@@ -62,6 +62,10 @@ xml.epp_head do
           xml.tag!('contact:authInfo') do
            xml.tag!('contact:pw', @contact.auth_info)
           end
+        else
+          xml.tag!('contact:authInfo') do
+          xml.tag!('contact:pw', 'No access')
+          end
         end
         # xml << render('/epp/contacts/disclosure_policy')
       end
@@ -73,6 +77,12 @@ xml.epp_head do
                    type: @contact.ident_type, cc: @contact.ident_country_code)
         end
       end
+    else
+      xml.tag!('extension') do
+        xml.tag!('eis:extdata', 'xmlns:eis' => 'https://epp.tld.ee/schema/eis-1.0.xsd') do
+          xml.tag!('eis:ident', 'No access')
+        end
+      end
     end
 
     render('epp/shared/trID', builder: xml)

From 71ea9dd961778fc4638da3dc08468bde9895447c Mon Sep 17 00:00:00 2001
From: Stas <stanislav@perfectline.co>
Date: Fri, 13 Nov 2015 16:40:54 +0200
Subject: [PATCH 14/27] 105361584-ident changes

---
 app/helpers/application_helper.rb       | 9 +++++++--
 app/views/epp/contacts/info.xml.builder | 6 ------
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 751e1d998..9646bd5c9 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -23,8 +23,13 @@ module ApplicationHelper
     case ident_type
     when 'birthday'
       "#{ident} [#{ident_type}]"
-    else
-      "#{ident} [#{ident_country_code} #{ident_type}]"
+      else
+        if ident
+          "#{ident} [#{ident_country_code} #{ident_type}]"
+        else
+          "[No access]"
+        end
+
     end
   end
 
diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder
index 24b37a13f..18019208a 100644
--- a/app/views/epp/contacts/info.xml.builder
+++ b/app/views/epp/contacts/info.xml.builder
@@ -77,12 +77,6 @@ xml.epp_head do
                    type: @contact.ident_type, cc: @contact.ident_country_code)
         end
       end
-    else
-      xml.tag!('extension') do
-        xml.tag!('eis:extdata', 'xmlns:eis' => 'https://epp.tld.ee/schema/eis-1.0.xsd') do
-          xml.tag!('eis:ident', 'No access')
-        end
-      end
     end
 
     render('epp/shared/trID', builder: xml)

From 55abacdf6be58b1dc08eafb25b4d8918fd23a5e3 Mon Sep 17 00:00:00 2001
From: Stas <stanislav@perfectline.co>
Date: Fri, 13 Nov 2015 17:14:31 +0200
Subject: [PATCH 15/27] logic fix

---
 app/helpers/application_helper.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 9646bd5c9..098c10d9b 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -24,7 +24,7 @@ module ApplicationHelper
     when 'birthday'
       "#{ident} [#{ident_type}]"
       else
-        if ident
+        if ident.present?
           "#{ident} [#{ident_country_code} #{ident_type}]"
         else
           "[No access]"

From 8cf8d7c4c3c3074c35b6712766b463817243e171 Mon Sep 17 00:00:00 2001
From: Stas <stanislav@perfectline.co>
Date: Fri, 13 Nov 2015 19:48:46 +0200
Subject: [PATCH 16/27] User import changes

---
 lib/tasks/import.rake | 78 ++++++++++++++++++++++++-------------------
 1 file changed, 43 insertions(+), 35 deletions(-)

diff --git a/lib/tasks/import.rake b/lib/tasks/import.rake
index 60574841f..eee769666 100644
--- a/lib/tasks/import.rake
+++ b/lib/tasks/import.rake
@@ -128,61 +128,69 @@ namespace :import do
   desc 'Import users'
   task users: :environment do
     start = Time.zone.now.to_f
-    puts '-----> Importing users...'
+    puts "-----> Importing users and IP's..."
 
+    id_users = []
     users = []
     ips = []
+    temp = []
 
     existing_ids = ApiUser.pluck(:legacy_id)
-
-    count = 0
+    existing_ips = WhiteIp.pluck(:ipv4)
 
     Legacy::Registrar.all.each do |x|
 
-      next if existing_ids.include?(x.id)
-      count += 1
+      x.acl.all.each do |y|
 
-        if x.acl.last.try(:cert) != 'pki'
-          if x.acl.last.try(:cert) != 'idkaart'
-            users << ApiUser.new({
-                username: x.handle.try(:strip),
-                password: x.acl.last.try(:password) ? x.acl.last.try(:password) : ('a'..'z').to_a.shuffle.first(8).join,
-                identity_code: x.handle.try(:strip),
-                registrar_id: Registrar.find_by(legacy_id: x.try(:id)).try(:id),
-                roles: ['epp'],
-                legacy_id: x.try(:id)
-            })
-          elsif x.acl.last.try(:cert) == 'idkaart'
-            users << ApiUser.new({
-               username: x.acl.last.try(:password) ? x.acl.last.try(:password) : x.acl.first.try(:password),
-               password: ('a'..'z').to_a.shuffle.first(8).join,
-               identity_code: x.acl.last.try(:password) ? x.acl.last.try(:password) : x.acl.first.try(:password),
-               registrar_id: Registrar.find_by(legacy_id: x.try(:id)).try(:id),
-               roles: ['billing'],
-               legacy_id: x.try(:id)
-            })
-          end
-        end
+        next if existing_ids.include?(y.id)
 
-      existing_ips = WhiteIp.pluck(:ipv4)
+        if y.try(:cert) != 'pki'
 
-        x.acl.all.each do |y|
-          next if existing_ips.include?(y.ipaddr)
-          if !y.ipaddr.nil? && y.ipaddr != ''
-            ips << WhiteIp.new({
+          if y.try(:cert) == 'idkaart'
+            id_users << ApiUser.new({
+              username: y.try(:password) ? y.try(:password) : y.try(:password),
+              password: ('a'..'z').to_a.shuffle.first(8).join,
+              identity_code: y.try(:password) ? y.try(:password) : y.try(:password),
               registrar_id: Registrar.find_by(legacy_id: x.try(:id)).try(:id),
-              ipv4: y.ipaddr,
-              interfaces: ['api', 'registrar']
-            })
+              roles: ['billing'],
+              legacy_id: y.try(:id)
+              })
+          else
+            temp << ApiUser.new({
+              username: x.handle.try(:strip),
+              password: y.try(:password) ? y.try(:password) : ('a'..'z').to_a.shuffle.first(8).join,
+              registrar_id: Registrar.find_by(legacy_id: x.try(:id)).try(:id),
+              roles: ['epp'],
+              legacy_id: y.try(:id)
+              })
           end
         end
+        temp = temp.reverse!.uniq{|u| u.username }
+      end
+      users = temp
+
+      x.acl.all.each do |y|
+        next if existing_ips.include?(y.ipaddr)
+        if !y.ipaddr.nil? && y.ipaddr != ''
+          ips << WhiteIp.new({
+            registrar_id: Registrar.find_by(legacy_id: x.try(:id)).try(:id),
+            ipv4: y.ipaddr,
+            interfaces: ['api', 'registrar']
+            })
+        end
+      end
     end
 
+    ApiUser.import id_users, validate: false
     ApiUser.import users, validate: false
+
     if ips
       WhiteIp.import ips, validate: false
     end
-    puts "-----> Imported #{count} new users in #{(Time.zone.now.to_f - start).round(2)} seconds"
+
+    puts "-----> Imported #{id_users.count} billing users and #{users.count} epp users"
+    puts "-----> Imported #{ips.count} white IP's in #{(Time.zone.now.to_f - start).round(2)} seconds"
+
   end
 
   desc 'Import contacts'

From e785bce41587280342f708f3ae28998409aadccf Mon Sep 17 00:00:00 2001
From: Vladimir Krylov <vladimir.krylov@perfectline.co>
Date: Mon, 16 Nov 2015 10:59:26 +0200
Subject: [PATCH 17/27] Story#107571572 - banklinks workflow fixes

---
 app/controllers/registrar/payments_controller.rb | 8 ++++----
 app/models/bank_link.rb                          | 4 ++++
 config/locales/en.yml                            | 2 +-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/app/controllers/registrar/payments_controller.rb b/app/controllers/registrar/payments_controller.rb
index 674b8afed..931dffcf2 100644
--- a/app/controllers/registrar/payments_controller.rb
+++ b/app/controllers/registrar/payments_controller.rb
@@ -20,16 +20,16 @@ class Registrar::PaymentsController < RegistrarController
   # both back and IPN
   def back
     @bank_link = BankLink::Response.new(params[:bank], params)
-    if @bank_link.valid?
+    if @bank_link.valid? && @bank_link.ok?
       @bank_link.complete_payment
 
       if @bank_link.invoice.binded?
-        flash[:notice] = t(:pending_applieds)
+        flash[:notice] = t(:pending_applied)
       else
-        flash[:error]  = t(:something_wrong)
+        flash[:alert]  = t(:something_wrong)
       end
     else
-      flash[:error] = t(:something_wrong)
+      flash[:alert] = t(:something_wrong)
     end
     redirect_to registrar_invoice_path(@bank_link.invoice)
   end
diff --git a/app/models/bank_link.rb b/app/models/bank_link.rb
index b28611542..31be3e222 100644
--- a/app/models/bank_link.rb
+++ b/app/models/bank_link.rb
@@ -88,6 +88,10 @@ class BankLink
       !!validate
     end
 
+    def ok?
+      params["VK_SERVICE"] == "1111"
+    end
+
     def complete_payment
       if valid?
         transaction = BankTransaction.find_by(description: params["VK_MSG"])
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 7a562afa6..937096ad6 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -656,7 +656,7 @@ en:
   m_id: 'M-ID'
   pending_removed: Pending was successfully removed.
   pending_applied: Pending was successfully applied.
-  something_wrong: Not success, something went wrong
+  something_wrong: Sorry, something went wrong
   failure: Not success
   not_found: Not found
   no_connection_to_registry: Connection issue to the registry EPP or REPP server! Please try again later.

From b166cec06e0e8ec08da0397f344e0e8e46eab8a6 Mon Sep 17 00:00:00 2001
From: Vladimir Krylov <vladimir.krylov@perfectline.co>
Date: Mon, 16 Nov 2015 17:13:55 +0200
Subject: [PATCH 18/27] Story#105747240 - remove legal documents from domain
 history

---
 app/controllers/epp/domains_controller.rb |  7 -------
 app/models/epp/domain.rb                  | 10 +++++++++-
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/app/controllers/epp/domains_controller.rb b/app/controllers/epp/domains_controller.rb
index 57136be63..8426c8ccb 100644
--- a/app/controllers/epp/domains_controller.rb
+++ b/app/controllers/epp/domains_controller.rb
@@ -56,10 +56,6 @@ class Epp::DomainsController < EppController
     authorize! :update, @domain, @password
     begin
       if @domain.update(params[:parsed_frame], current_user)
-
-        @domain.attach_legal_document(Epp::Domain.parse_legal_document_from_frame(params[:parsed_frame]))
-        @domain.save(validate: false)
-
         if @domain.epp_pending_update.present?
           render_epp_response '/epp/domains/success_pending'
         else
@@ -80,9 +76,6 @@ class Epp::DomainsController < EppController
 
     handle_errors(@domain) and return unless @domain.can_be_deleted?
 
-    @domain.attach_legal_document(Epp::Domain.parse_legal_document_from_frame(params[:parsed_frame]))
-    @domain.save(validate: false)
-
     if @domain.epp_destroy(params[:parsed_frame], current_user.id)
       if @domain.epp_pending_delete.present?
         render_epp_response '/epp/domains/success_pending'
diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb
index 7b55b018d..9be98d6e6 100644
--- a/app/models/epp/domain.rb
+++ b/app/models/epp/domain.rb
@@ -472,6 +472,10 @@ class Epp::Domain < Domain
     at.deep_merge!(attrs_from(frame.css('chg'), current_user, 'chg'))
     at.deep_merge!(attrs_from(frame.css('rem'), current_user, 'rem'))
 
+    if doc = attach_legal_document(Epp::Domain.parse_legal_document_from_frame(frame))
+      frame.css("legalDocument").first.content = doc.path if doc && doc.persisted?
+    end
+
     at_add = attrs_from(frame.css('add'), current_user)
     at[:nameservers_attributes] += at_add[:nameservers_attributes]
 
@@ -536,7 +540,7 @@ class Epp::Domain < Domain
   def attach_legal_document(legal_document_data)
     return unless legal_document_data
 
-    legal_documents.build(
+    legal_documents.create(
       document_type: legal_document_data[:type],
       body: legal_document_data[:body]
     )
@@ -545,6 +549,10 @@ class Epp::Domain < Domain
   def epp_destroy(frame, user_id)
     return false unless valid?
 
+    if doc = attach_legal_document(Epp::Domain.parse_legal_document_from_frame(frame))
+      frame.css("legalDocument").first.content = doc.path if doc && doc.persisted?
+    end
+
     if Setting.request_confirmation_on_domain_deletion_enabled &&
        frame.css('delete').children.css('delete').attr('verified').to_s.downcase != 'yes'
 

From 24f90d51197b2bdd670c5e2c8f3585b5427b4a6a Mon Sep 17 00:00:00 2001
From: Matt Farnsworth <matt@perfectline.co>
Date: Fri, 13 Nov 2015 15:01:18 +0200
Subject: [PATCH 19/27] Story #107192666 - only show dsData if allowed, or
 keyData if allowed

---
 app/views/epp/domains/info.xml.builder | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/app/views/epp/domains/info.xml.builder b/app/views/epp/domains/info.xml.builder
index e7962002c..645c731e4 100644
--- a/app/views/epp/domains/info.xml.builder
+++ b/app/views/epp/domains/info.xml.builder
@@ -81,18 +81,17 @@ xml.epp_head do
       end
 
       xml.tag!('secDNS:infData', 'xmlns:secDNS' => 'urn:ietf:params:xml:ns:secDNS-1.1') do
-        # might not have ds in first key? maybe check any? k.ds_digest if requirements change (DS not accepted by EIS)
-        if @domain.dnskeys[0].ds_digest.blank?
-          @domain.dnskeys.sort.each do |key|
-            tag_key_data(xml, key)
+        if Setting.ds_data_allowed
+          (@domain.dnskeys.find_all { |key| key.ds_digest.present? }).sort.each do |key|
+            tag_ds_data(xml, key)
           end
         else
-          @domain.dnskeys.sort.each do |key|
-            tag_ds_data(xml, key)
+          (@domain.dnskeys.find_all { |key| key.ds_digest.blank? }).sort.each do |key|
+            tag_key_data(xml, key)
           end
         end
       end
-    end if @domain.dnskeys.any?
+    end if @domain.dnskeys.any? && (Setting.ds_data_allowed ? @domain.dnskeys.any? { |key| key.ds_digest.present? } :  Setting.key_data_allowed)
 
     render('epp/shared/trID', builder: xml)
   end

From 334179fae956ec671204402a7cf157a2d90b3a56 Mon Sep 17 00:00:00 2001
From: Matt Farnsworth <matt@perfectline.co>
Date: Fri, 13 Nov 2015 15:23:24 +0200
Subject: [PATCH 20/27] Story #107192666 - ensure registrary/domains/info shows
 only actual data

patch updates previous revision 7d9272c60c90ba27f272a9495829a121ff26ca43
---
 .../registrar/domains/partials/_dnskeys.haml     | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/app/views/registrar/domains/partials/_dnskeys.haml b/app/views/registrar/domains/partials/_dnskeys.haml
index 46dcd0fce..5b8a95ade 100644
--- a/app/views/registrar/domains/partials/_dnskeys.haml
+++ b/app/views/registrar/domains/partials/_dnskeys.haml
@@ -4,23 +4,17 @@
   .panel-body{style: 'word-wrap: break-word;'}
     - @data.css('dsData').each do |x|
       %dl.dl-horizontal
-        - if x.css('keyTag').text.present?
+        - if x.css('digest').text.present?
           %dt= t(:ds_key_tag)
           %dd= x.css('keyTag').text
-
-        - if x.css('alg').first.text.present?
           %dt= t(:ds_algorithm)
           %dd= x.css('alg').first.text
-
-        - if x.css('digestType').text.present?
           %dt= t(:ds_digest_type)
           %dd= x.css('digestType').text
-
-        - if x.css('digest').text.present?
           %dt= t(:ds_digest)
           %dd= x.css('digest').text
-        - @data.css('keyData').each do |x|
-          %dl.dl-horizontal
+          - @data.css('keyData').each do |x|
+            %dl.dl-horizontal
             %dt= t(:flag)
             %dd= x.css('flags').text
 
@@ -32,9 +26,9 @@
 
             %dt= t(:public_key)
             %dd= x.css('pubKey').text
-
-    - @data.css('keyData').each do |x|
+    - if @data.css('dsData').empty?
       %dl.dl-horizontal
+      - @data.css('keyData').each do |x|
         %dt= t(:flag)
         %dd= x.css('flags').text
 

From 9ca17e6b4a775b50a0eca6d3e2b6c9c324791ab8 Mon Sep 17 00:00:00 2001
From: Matt Farnsworth <matt@perfectline.co>
Date: Fri, 13 Nov 2015 19:06:12 +0200
Subject: [PATCH 21/27] Story #107192666 - refactor to add stricter inclusion
 rules reformatted block, white space changes, indent for surounding if any?
 pull up find_all? if *_allowed then check if allowed.any?

---
 app/views/epp/domains/info.xml.builder | 63 ++++++++++++++------------
 1 file changed, 34 insertions(+), 29 deletions(-)

diff --git a/app/views/epp/domains/info.xml.builder b/app/views/epp/domains/info.xml.builder
index 645c731e4..17892a3cc 100644
--- a/app/views/epp/domains/info.xml.builder
+++ b/app/views/epp/domains/info.xml.builder
@@ -60,39 +60,44 @@ xml.epp_head do
       end
     end
 
-    xml.extension do
-      def tag_key_data(xml, key)
-        xml.tag!('secDNS:keyData') do
-          xml.tag!('secDNS:flags', key.flags)
-          xml.tag!('secDNS:protocol', key.protocol)
-          xml.tag!('secDNS:alg', key.alg)
-          xml.tag!('secDNS:pubKey', key.public_key)
-        end
-      end
+    if @domain.dnskeys.any?
+      ds_data  = Setting.ds_data_allowed  ? @domain.dnskeys.find_all { |key| key.ds_digest.present? } : []
+      key_data = Setting.key_data_allowed ? @domain.dnskeys.find_all { |key| key.ds_digest.blank? } : []
 
-      def tag_ds_data(xml, key)
-        xml.tag!('secDNS:dsData') do
-          xml.tag!('secDNS:keyTag', key.ds_key_tag)
-          xml.tag!('secDNS:alg', key.ds_alg)
-          xml.tag!('secDNS:digestType', key.ds_digest_type)
-          xml.tag!('secDNS:digest', key.ds_digest)
-          tag_key_data(xml, key) if key.public_key.present?
-        end
-      end
-
-      xml.tag!('secDNS:infData', 'xmlns:secDNS' => 'urn:ietf:params:xml:ns:secDNS-1.1') do
-        if Setting.ds_data_allowed
-          (@domain.dnskeys.find_all { |key| key.ds_digest.present? }).sort.each do |key|
-            tag_ds_data(xml, key)
-          end
-        else
-          (@domain.dnskeys.find_all { |key| key.ds_digest.blank? }).sort.each do |key|
-            tag_key_data(xml, key)
+      # is there any reason to include <extension> without <secDNS:infData>
+      xml.extension do
+        def tag_key_data(xml, key)
+          xml.tag!('secDNS:keyData') do
+            xml.tag!('secDNS:flags', key.flags)
+            xml.tag!('secDNS:protocol', key.protocol)
+            xml.tag!('secDNS:alg', key.alg)
+            xml.tag!('secDNS:pubKey', key.public_key)
           end
         end
-      end
-    end if @domain.dnskeys.any? && (Setting.ds_data_allowed ? @domain.dnskeys.any? { |key| key.ds_digest.present? } :  Setting.key_data_allowed)
 
+        def tag_ds_data(xml, key)
+          xml.tag!('secDNS:dsData') do
+            xml.tag!('secDNS:keyTag', key.ds_key_tag)
+            xml.tag!('secDNS:alg', key.ds_alg)
+            xml.tag!('secDNS:digestType', key.ds_digest_type)
+            xml.tag!('secDNS:digest', key.ds_digest)
+            tag_key_data(xml, key) if key.public_key.present?
+          end
+        end
+
+        xml.tag!('secDNS:infData', 'xmlns:secDNS' => 'urn:ietf:params:xml:ns:secDNS-1.1') do
+          if Setting.ds_data_allowed
+            ds_data.sort.each do |key|
+              tag_ds_data(xml, key)
+            end
+          else
+            key_data.sort.each do |key|
+              tag_key_data(xml, key)
+            end
+          end
+        end
+      end if key_data.present? || ds_data.present?
+    end
     render('epp/shared/trID', builder: xml)
   end
 end

From e01599fb1a8bce42b018be329fde516fb2ca61cd Mon Sep 17 00:00:00 2001
From: Matt Farnsworth <matt@perfectline.co>
Date: Fri, 13 Nov 2015 19:30:55 +0200
Subject: [PATCH 22/27] Story #107192666 - refactor dns sec key processing,
 ensure rules are applied and multiple keys ok

---
 app/models/epp/domain.rb | 112 +++++++++++++++++++++++----------------
 1 file changed, 66 insertions(+), 46 deletions(-)

diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb
index ffd525d9d..2b1a9027b 100644
--- a/app/models/epp/domain.rb
+++ b/app/models/epp/domain.rb
@@ -314,20 +314,11 @@ class Epp::Domain < Domain
   # rubocop: disable Metrics/PerceivedComplexity
   # rubocop: disable Metrics/CyclomaticComplexity
   def dnskeys_attrs(frame, action)
-    if frame.css('dsData').any? && !Setting.ds_data_allowed
-      errors.add(:base, :ds_data_not_allowed)
-    end
-
-    if frame.xpath('keyData').any? && !Setting.key_data_allowed
-      errors.add(:base, :key_data_not_allowed)
-    end
-
-    res = ds_data_from(frame)
-    dnskeys_list = key_data_from(frame, res)
-
+    return [] if frame.empty?
+    keys = DnsSecKeys.new(self).values frame
     if action == 'rem'
       to_destroy = []
-      dnskeys_list.each do |x|
+      keys.each do |x|
         dk = dnskeys.find_by(public_key: x[:public_key])
 
         unless dk
@@ -343,51 +334,80 @@ class Epp::Domain < Domain
 
       return to_destroy
     else
-      return dnskeys_list
+      return keys
     end
   end
   # rubocop: enable Metrics/PerceivedComplexity
   # rubocop: enable Metrics/CyclomaticComplexity
 
-  def key_data_from(frame, res)
-    frame.xpath('keyData').each do |x|
-      res << {
-        flags: x.css('flags').first.try(:text),
-        protocol: x.css('protocol').first.try(:text),
-        alg: x.css('alg').first.try(:text),
-        public_key: x.css('pubKey').first.try(:text),
-        ds_alg: 3,
-        ds_digest_type: Setting.ds_algorithm
-      }
+  class DnsSecKeys
+    def initialize(domain)
+      @domain = domain
     end
 
-    res
-  end
-
-  def ds_data_from(frame)
-    res = []
-    frame.css('dsData').each do |x|
-      data = {
-        ds_key_tag: x.css('keyTag').first.try(:text),
-        ds_alg: x.css('alg').first.try(:text),
-        ds_digest_type: x.css('digestType').first.try(:text),
-        ds_digest: x.css('digest').first.try(:text)
-      }
-
-      kd = x.css('keyData').first
-      data.merge!({
-        flags: kd.css('flags').first.try(:text),
-        protocol: kd.css('protocol').first.try(:text),
-        alg: kd.css('alg').first.try(:text),
-        public_key: kd.css('pubKey').first.try(:text)
-      }) if kd
-
-      res << data
+    def values(frame)
+      if Setting.key_data_allowed
+        @domain.errors.add(:base, :ds_data_not_allowed) if frame.css('dsData').present?
+        return key_data_from frame
+      end
+      if Setting.ds_data_allowed
+        @domain.errors.add(:base, :key_data_not_allowed) if frame.css('keyData').present?
+        return ds_data_from frame
+      end
+      []
+    end
+
+    private
+
+    KEY_INTERFACE = {flags: 'flags', protocol: 'protocol', alg: 'alg', public_key: 'pubKey' }
+    DS_INTERFACE  =
+        { ds_key_tag:     'keyTag',
+          ds_alg:         'alg',
+          ds_digest_type: 'digestType',
+          ds_digest:      'digest'
+        }
+
+    def xm_copy(frame, map)
+      result = {}
+      map.each do |key, value|
+        result[key] = frame.css(value).first.try(:text)
+      end
+      result
+    end
+
+    # frame requires NodeSet with direct children of keyData
+    def key_data(frame)
+      result = xm_copy frame.css('keyData'), KEY_INTERFACE
+      # TODO: can these defaults go where they belong?
+      result.merge({
+                       ds_alg: 3,
+                       ds_digest_type: Setting.ds_algorithm
+                   })
+    end
+
+    # get all top level dsData from NodeSet
+    def ds_data_from(frame)
+      result = []
+      frame.css('dsData').each do |ds_data|
+        key = ds_data.css('keyData')
+        ds = xm_copy ds_data, DS_INTERFACE
+        ds.merge! (key_data key) if key.present?
+        result << ds
+      end
+      result
+    end
+
+    def key_data_from(frame)
+      result = []
+      frame.css('keyData').each do |key|
+        result << key_data(key)
+      end
+      result
     end
 
-    res
   end
 
+
   def domain_statuses_attrs(frame, action)
     status_list = domain_status_list_from(frame)
     if action == 'rem'

From e31087bf60cbcc8ee09108cb2f397a4a625f5cd3 Mon Sep 17 00:00:00 2001
From: Vladimir Krylov <vladimir.krylov@perfectline.co>
Date: Mon, 16 Nov 2015 12:52:26 +0200
Subject: [PATCH 23/27] Story#107192666 - validation for settings that DS data
 allowed and Allow key data cannot be both true

---
 app/controllers/admin/settings_controller.rb | 16 +++++++++++-----
 app/models/setting.rb                        | 14 ++++++++++++++
 app/views/admin/settings/_setting_row.haml   |  2 +-
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/app/controllers/admin/settings_controller.rb b/app/controllers/admin/settings_controller.rb
index 6a0c5c033..4014cd7fc 100644
--- a/app/controllers/admin/settings_controller.rb
+++ b/app/controllers/admin/settings_controller.rb
@@ -7,12 +7,18 @@ class Admin::SettingsController < AdminController
   end
 
   def create
-    casted_settings.each do |k, v|
-      Setting[k] = v
-    end
+    @errors = Setting.params_errors(casted_settings)
+    if @errors.empty?
+      casted_settings.each do |k, v|
+        Setting[k] = v
+      end
 
-    flash[:notice] = I18n.t('records_updated')
-    redirect_to [:admin, :settings]
+      flash[:notice] = I18n.t('records_updated')
+      redirect_to [:admin, :settings]
+    else
+      flash[:alert] = @errors.values.uniq.join(", ")
+      render "admin/settings/index"
+    end
   end
 
   def show; end
diff --git a/app/models/setting.rb b/app/models/setting.rb
index 122bfc99a..cb52fd1b7 100644
--- a/app/models/setting.rb
+++ b/app/models/setting.rb
@@ -6,4 +6,18 @@ class Setting < RailsSettings::CachedSettings
     Rails.cache.delete_matched('settings:.*')
     STDOUT << "#{Time.zone.now.utc} - Settings cache cleared\n"
   end
+
+
+  # cannot do instance validation because CachedSetting use save!
+  def self.params_errors(params)
+    errors = {}
+    # DS data allowed and Allow key data cannot be both true
+    if !!params["key_data_allowed"] && params["key_data_allowed"] == params["ds_data_allowed"]
+      msg = "#{I18n.t(:key_data_allowed)} and #{I18n.t(:ds_data_with_key_allowed)} cannot be both true"
+      errors["key_data_allowed"] = msg
+      errors["ds_data_allowed"]  = msg
+    end
+
+    return errors
+  end
 end
diff --git a/app/views/admin/settings/_setting_row.haml b/app/views/admin/settings/_setting_row.haml
index 632effc74..ab0d8d991 100644
--- a/app/views/admin/settings/_setting_row.haml
+++ b/app/views/admin/settings/_setting_row.haml
@@ -1,5 +1,5 @@
 - value = Setting.send(var)
-%tr
+%tr{class: (@errors && @errors.has_key?(var.to_s) && "danger")}
   %td= t(var)
   - if [TrueClass, FalseClass].include?(value.class)
     %td

From 277b7390654a1b303189f2ce7e563281798f9e53 Mon Sep 17 00:00:00 2001
From: Matt Farnsworth <matt@perfectline.co>
Date: Sat, 14 Nov 2015 23:40:45 +0200
Subject: [PATCH 24/27] Story #107192666 - bug fix
 fd4b2debb2c2c192ec129e1056427b94f314462e, refactor, support secDNS:all

---
 app/models/epp/domain.rb | 79 +++++++++++++++++++++++-----------------
 1 file changed, 46 insertions(+), 33 deletions(-)

diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb
index 2b1a9027b..c90e5faa2 100644
--- a/app/models/epp/domain.rb
+++ b/app/models/epp/domain.rb
@@ -314,8 +314,31 @@ class Epp::Domain < Domain
   # rubocop: disable Metrics/PerceivedComplexity
   # rubocop: disable Metrics/CyclomaticComplexity
   def dnskeys_attrs(frame, action)
-    return [] if frame.empty?
-    keys = DnsSecKeys.new(self).values frame
+    keys = []
+    return keys if frame.blank?
+
+    if frame.xpath('dnsSec:all').present?
+      # support delete all or all of
+    else
+      inf_data = DnsSecKeys.new(frame)
+      if Setting.key_data_allowed
+        if inf_data.ds_data.present?
+          errors.add(:base, :ds_data_not_allowed)
+          return
+        else
+          keys = inf_data.key_data
+        end
+      end
+      if Setting.ds_data_allowed
+        if inf_data.key_data.present?
+          errors.add(:base, :key_data_not_allowed)
+          return
+        else
+          keys = inf_data.ds_data
+        end
+      end
+    end
+
     if action == 'rem'
       to_destroy = []
       keys.each do |x|
@@ -341,20 +364,22 @@ class Epp::Domain < Domain
   # rubocop: enable Metrics/CyclomaticComplexity
 
   class DnsSecKeys
-    def initialize(domain)
-      @domain = domain
+    def initialize(frame)
+      @key_data = []
+      @ds_data = []
+      if frame.css('dsData').present?
+        ds_data_from frame
+      end
+      frame.css('keyData').each do |key|
+        @key_data.append key_data_from(key)
+      end
     end
 
-    def values(frame)
-      if Setting.key_data_allowed
-        @domain.errors.add(:base, :ds_data_not_allowed) if frame.css('dsData').present?
-        return key_data_from frame
-      end
-      if Setting.ds_data_allowed
-        @domain.errors.add(:base, :key_data_not_allowed) if frame.css('keyData').present?
-        return ds_data_from frame
-      end
-      []
+    attr_reader :key_data
+    attr_reader :ds_data
+
+    def error
+      ds_data.present? && key_data.present?
     end
 
     private
@@ -369,15 +394,15 @@ class Epp::Domain < Domain
 
     def xm_copy(frame, map)
       result = {}
-      map.each do |key, value|
-        result[key] = frame.css(value).first.try(:text)
+      map.each do |key, elem|
+        # content validation might happen later in Dnskey, if we get that far; or not. TODO: check handling
+        result[key] = frame.css(elem).first.try(:text)
       end
       result
     end
 
-    # frame requires NodeSet with direct children of keyData
-    def key_data(frame)
-      result = xm_copy frame.css('keyData'), KEY_INTERFACE
+    def key_data_from(frame)
+      result = xm_copy frame, KEY_INTERFACE
       # TODO: can these defaults go where they belong?
       result.merge({
                        ds_alg: 3,
@@ -385,26 +410,14 @@ class Epp::Domain < Domain
                    })
     end
 
-    # get all top level dsData from NodeSet
     def ds_data_from(frame)
-      result = []
       frame.css('dsData').each do |ds_data|
         key = ds_data.css('keyData')
         ds = xm_copy ds_data, DS_INTERFACE
-        ds.merge! (key_data key) if key.present?
-        result << ds
+        ds.merge! (key_data_from key) if key.present?
+        @ds_data << ds
       end
-      result
     end
-
-    def key_data_from(frame)
-      result = []
-      frame.css('keyData').each do |key|
-        result << key_data(key)
-      end
-      result
-    end
-
   end
 
 

From a30c3db96807cfc0bcc4b25c7073725dff05d91f Mon Sep 17 00:00:00 2001
From: Matt Farnsworth <matt@perfectline.co>
Date: Mon, 16 Nov 2015 12:14:11 +0200
Subject: [PATCH 25/27] Story #107192666 - feature fix, keyData interface
 should include flags=KSK

---
 app/views/epp/domains/info.xml.builder | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/epp/domains/info.xml.builder b/app/views/epp/domains/info.xml.builder
index 17892a3cc..f89fcd168 100644
--- a/app/views/epp/domains/info.xml.builder
+++ b/app/views/epp/domains/info.xml.builder
@@ -62,7 +62,7 @@ xml.epp_head do
 
     if @domain.dnskeys.any?
       ds_data  = Setting.ds_data_allowed  ? @domain.dnskeys.find_all { |key| key.ds_digest.present? } : []
-      key_data = Setting.key_data_allowed ? @domain.dnskeys.find_all { |key| key.ds_digest.blank? } : []
+      key_data = Setting.key_data_allowed ? @domain.dnskeys.find_all { |key| key.public_key.present? } : []
 
       # is there any reason to include <extension> without <secDNS:infData>
       xml.extension do

From 4380cfcb3e53d7c0ecf4f6fdd3d4b7162aafb1d3 Mon Sep 17 00:00:00 2001
From: Matt Farnsworth <matt@perfectline.co>
Date: Mon, 16 Nov 2015 13:26:52 +0200
Subject: [PATCH 26/27] Story #107192666 - EIS policy specifies keyData
 interface not dsData. Both true is not allowed

---
 config/initializers/initial_settings.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config/initializers/initial_settings.rb b/config/initializers/initial_settings.rb
index bcee95150..35c3eebb3 100644
--- a/config/initializers/initial_settings.rb
+++ b/config/initializers/initial_settings.rb
@@ -13,7 +13,7 @@ if con.present? && con.table_exists?('settings')
   Setting.save_default(:expire_pending_confirmation, 48)
 
   Setting.save_default(:ds_algorithm, 2)
-  Setting.save_default(:ds_data_allowed, true)
+  Setting.save_default(:ds_data_allowed, false)
   Setting.save_default(:key_data_allowed, true)
 
   Setting.save_default(:dnskeys_min_count, 0)

From db545e3de24fbd183671192756d034c24f360c05 Mon Sep 17 00:00:00 2001
From: Matt Farnsworth <matt@perfectline.co>
Date: Mon, 16 Nov 2015 17:51:52 +0200
Subject: [PATCH 27/27] Story #107192666 - refactor, add support for remove
 all, bug fix overly visible nodes

---
 app/models/epp/domain.rb | 110 +++++++++++++++++++--------------------
 1 file changed, 55 insertions(+), 55 deletions(-)

diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb
index c90e5faa2..9be98d6e6 100644
--- a/app/models/epp/domain.rb
+++ b/app/models/epp/domain.rb
@@ -316,49 +316,26 @@ class Epp::Domain < Domain
   def dnskeys_attrs(frame, action)
     keys = []
     return keys if frame.blank?
+    inf_data = DnsSecKeys.new(frame)
 
-    if frame.xpath('dnsSec:all').present?
-      # support delete all or all of
+    if  action == 'rem' &&
+        frame.css('rem > all').first.try(:text) == 'true'
+      keys = inf_data.mark_destroy_all dnskeys
     else
-      inf_data = DnsSecKeys.new(frame)
       if Setting.key_data_allowed
-        if inf_data.ds_data.present?
-          errors.add(:base, :ds_data_not_allowed)
-          return
-        else
-          keys = inf_data.key_data
-        end
+        errors.add(:base, :ds_data_not_allowed) if inf_data.ds_data.present?
+        keys = inf_data.key_data
       end
       if Setting.ds_data_allowed
-        if inf_data.key_data.present?
-          errors.add(:base, :key_data_not_allowed)
-          return
-        else
-          keys = inf_data.ds_data
-        end
+        errors.add(:base, :key_data_not_allowed) if inf_data.key_data.present?
+        keys = inf_data.ds_data
+      end
+      if action == 'rem'
+        keys = inf_data.mark_destroy(dnskeys)
+        add_epp_error('2303', nil, nil, [:dnskeys, :not_found]) if keys.include? nil
       end
     end
-
-    if action == 'rem'
-      to_destroy = []
-      keys.each do |x|
-        dk = dnskeys.find_by(public_key: x[:public_key])
-
-        unless dk
-          add_epp_error('2303', 'publicKey', x[:public_key], [:dnskeys, :not_found])
-          next
-        end
-
-        to_destroy << {
-          id: dk.id,
-          _destroy: 1
-        }
-      end
-
-      return to_destroy
-    else
-      return keys
-    end
+    errors.any? ? [] : keys
   end
   # rubocop: enable Metrics/PerceivedComplexity
   # rubocop: enable Metrics/CyclomaticComplexity
@@ -367,19 +344,28 @@ class Epp::Domain < Domain
     def initialize(frame)
       @key_data = []
       @ds_data = []
+      # schema validation prevents both in the same parent node
       if frame.css('dsData').present?
         ds_data_from frame
-      end
-      frame.css('keyData').each do |key|
-        @key_data.append key_data_from(key)
+      else
+        frame.css('keyData').each do |key|
+          @key_data.append key_data_from(key)
+        end
       end
     end
 
     attr_reader :key_data
     attr_reader :ds_data
 
-    def error
-      ds_data.present? && key_data.present?
+    def mark_destroy_all(dns_keys)
+      # if transition support required mark_destroy dns_keys when has ds/key values otherwise ...
+      dns_keys.map { |inf_data| mark inf_data }
+    end
+
+    def mark_destroy(dns_keys)
+      (ds_data.present? ? ds_filter(dns_keys) : kd_filter(dns_keys)).map do |inf_data|
+        inf_data.blank? ? nil : mark(inf_data)
+      end
     end
 
     private
@@ -395,7 +381,6 @@ class Epp::Domain < Domain
     def xm_copy(frame, map)
       result = {}
       map.each do |key, elem|
-        # content validation might happen later in Dnskey, if we get that far; or not. TODO: check handling
         result[key] = frame.css(elem).first.try(:text)
       end
       result
@@ -405,8 +390,8 @@ class Epp::Domain < Domain
       result = xm_copy frame, KEY_INTERFACE
       # TODO: can these defaults go where they belong?
       result.merge({
-                       ds_alg: 3,
-                       ds_digest_type: Setting.ds_algorithm
+                       ds_alg: 3,      # DSA/SHA-1 [DSA] RFC2536
+                       ds_digest_type: Setting.ds_algorithm     # only 1
                    })
     end
 
@@ -414,13 +399,28 @@ class Epp::Domain < Domain
       frame.css('dsData').each do |ds_data|
         key = ds_data.css('keyData')
         ds = xm_copy ds_data, DS_INTERFACE
-        ds.merge! (key_data_from key) if key.present?
+        ds.merge(key_data_from key) if key.present?
         @ds_data << ds
       end
     end
+
+    def ds_filter(dns_keys)
+      @ds_data.map do |ds|
+        dns_keys.find_by(ds.slice(*DS_INTERFACE.keys))
+      end
+     end
+
+    def kd_filter(dns_keys)
+      @key_data.map do |key|
+        dns_keys.find_by(key)
+      end
+    end
+
+    def mark(inf_data)
+      { id: inf_data.id, _destroy: 1 }
+    end
   end
-
-
+  
   def domain_statuses_attrs(frame, action)
     status_list = domain_status_list_from(frame)
     if action == 'rem'
@@ -830,14 +830,14 @@ class Epp::Domain < Domain
   def transferrable?
     (statuses & [
         DomainStatus::PENDING_DELETE_CONFIRMATION,
-      DomainStatus::PENDING_CREATE,
-      DomainStatus::PENDING_UPDATE,
-      DomainStatus::PENDING_DELETE,
-      DomainStatus::PENDING_RENEW,
-      DomainStatus::PENDING_TRANSFER,
-      DomainStatus::FORCE_DELETE,
-      DomainStatus::SERVER_TRANSFER_PROHIBITED,
-      DomainStatus::CLIENT_TRANSFER_PROHIBITED
+        DomainStatus::PENDING_CREATE,
+        DomainStatus::PENDING_UPDATE,
+        DomainStatus::PENDING_DELETE,
+        DomainStatus::PENDING_RENEW,
+        DomainStatus::PENDING_TRANSFER,
+        DomainStatus::FORCE_DELETE,
+        DomainStatus::SERVER_TRANSFER_PROHIBITED,
+        DomainStatus::CLIENT_TRANSFER_PROHIBITED
     ]).empty?
   end