Merge branch '107192666-ds-key-data-dnssec' into staging

app/controllers/admin/settings_controller.rb

@@ -7,12 +7,18 @@ class Admin::SettingsController < AdminController
   end
 
   def create
-    casted_settings.each do |k, v|
+    @errors = Setting.params_errors(casted_settings)
-      Setting[k] = v
+    if @errors.empty?
-    end
+      casted_settings.each do |k, v|
+        Setting[k] = v
+      end
 
-    flash[:notice] = I18n.t('records_updated')
+      flash[:notice] = I18n.t('records_updated')
-    redirect_to [:admin, :settings]
+      redirect_to [:admin, :settings]
+    else
+      flash[:alert] = @errors.values.uniq.join(", ")
+      render "admin/settings/index"
+    end
   end
 
   def show; end

app/models/epp/domain.rb

@@ -314,80 +314,113 @@ 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
+    keys = []
-      errors.add(:base, :ds_data_not_allowed)
+    return keys if frame.blank?
-    end
+    inf_data = DnsSecKeys.new(frame)
 
-    if frame.xpath('keyData').any? && !Setting.key_data_allowed
+    if  action == 'rem' &&
-      errors.add(:base, :key_data_not_allowed)
+        frame.css('rem > all').first.try(:text) == 'true'
-    end
+      keys = inf_data.mark_destroy_all dnskeys
-
-    res = ds_data_from(frame)
-    dnskeys_list = key_data_from(frame, res)
-
-    if action == 'rem'
-      to_destroy = []
-      dnskeys_list.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 dnskeys_list
+      if Setting.key_data_allowed
+        errors.add(:base, :ds_data_not_allowed) if inf_data.ds_data.present?
+        keys = inf_data.key_data
+      end
+      if Setting.ds_data_allowed
+        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
+    errors.any? ? [] : keys
   end
   # rubocop: enable Metrics/PerceivedComplexity
   # rubocop: enable Metrics/CyclomaticComplexity
 
-  def key_data_from(frame, res)
+  class DnsSecKeys
-    frame.xpath('keyData').each do |x|
+    def initialize(frame)
-      res << {
+      @key_data = []
-        flags: x.css('flags').first.try(:text),
+      @ds_data = []
-        protocol: x.css('protocol').first.try(:text),
+      # schema validation prevents both in the same parent node
-        alg: x.css('alg').first.try(:text),
+      if frame.css('dsData').present?
-        public_key: x.css('pubKey').first.try(:text),
+        ds_data_from frame
-        ds_alg: 3,
+      else
-        ds_digest_type: Setting.ds_algorithm
+        frame.css('keyData').each do |key|
-      }
+          @key_data.append key_data_from(key)
+        end
+      end
     end
 
-    res
+    attr_reader :key_data
-  end
+    attr_reader :ds_data
 
-  def ds_data_from(frame)
+    def mark_destroy_all(dns_keys)
-    res = []
+      # if transition support required mark_destroy dns_keys when has ds/key values otherwise ...
-    frame.css('dsData').each do |x|
+      dns_keys.map { |inf_data| mark inf_data }
-      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
     end
 
-    res
+    def mark_destroy(dns_keys)
-  end
+      (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
+
+    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, elem|
+        result[key] = frame.css(elem).first.try(:text)
+      end
+      result
+    end
+
+    def key_data_from(frame)
+      result = xm_copy frame, KEY_INTERFACE
+      # TODO: can these defaults go where they belong?
+      result.merge({
+                       ds_alg: 3,      # DSA/SHA-1 [DSA] RFC2536
+                       ds_digest_type: Setting.ds_algorithm     # only 1
+                   })
+    end
+
+    def ds_data_from(frame)
+      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_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'
@@ -785,14 +818,14 @@ class Epp::Domain < Domain
   def transferrable?
     (statuses & [
         DomainStatus::PENDING_DELETE_CONFIRMATION,
-      DomainStatus::PENDING_CREATE,
+        DomainStatus::PENDING_CREATE,
-      DomainStatus::PENDING_UPDATE,
+        DomainStatus::PENDING_UPDATE,
-      DomainStatus::PENDING_DELETE,
+        DomainStatus::PENDING_DELETE,
-      DomainStatus::PENDING_RENEW,
+        DomainStatus::PENDING_RENEW,
-      DomainStatus::PENDING_TRANSFER,
+        DomainStatus::PENDING_TRANSFER,
-      DomainStatus::FORCE_DELETE,
+        DomainStatus::FORCE_DELETE,
-      DomainStatus::SERVER_TRANSFER_PROHIBITED,
+        DomainStatus::SERVER_TRANSFER_PROHIBITED,
-      DomainStatus::CLIENT_TRANSFER_PROHIBITED
+        DomainStatus::CLIENT_TRANSFER_PROHIBITED
     ]).empty?
   end
 

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

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

app/views/epp/domains/info.xml.builder

@@ -60,40 +60,44 @@ xml.epp_head do
       end
     end
 
-    xml.extension do
+    if @domain.dnskeys.any?
-      def tag_key_data(xml, key)
+      ds_data  = Setting.ds_data_allowed  ? @domain.dnskeys.find_all { |key| key.ds_digest.present? } : []
-        xml.tag!('secDNS:keyData') do
+      key_data = Setting.key_data_allowed ? @domain.dnskeys.find_all { |key| key.public_key.present? } : []
-          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
 
-      def tag_ds_data(xml, key)
+      # is there any reason to include <extension> without <secDNS:infData>
-        xml.tag!('secDNS:dsData') do
+      xml.extension do
-          xml.tag!('secDNS:keyTag', key.ds_key_tag)
+        def tag_key_data(xml, key)
-          xml.tag!('secDNS:alg', key.ds_alg)
+          xml.tag!('secDNS:keyData') do
-          xml.tag!('secDNS:digestType', key.ds_digest_type)
+            xml.tag!('secDNS:flags', key.flags)
-          xml.tag!('secDNS:digest', key.ds_digest)
+            xml.tag!('secDNS:protocol', key.protocol)
-          tag_key_data(xml, key) if key.public_key.present?
+            xml.tag!('secDNS:alg', key.alg)
-        end
+            xml.tag!('secDNS:pubKey', key.public_key)
-      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)
-          end
-        else
-          @domain.dnskeys.sort.each do |key|
-            tag_ds_data(xml, key)
           end
         end
-      end
-    end if @domain.dnskeys.any?
 
+        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

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|
+          - @data.css('keyData').each do |x|
-          %dl.dl-horizontal
+            %dl.dl-horizontal
             %dt= t(:flag)
             %dd= x.css('flags').text
 
@@ -32,9 +26,9 @@
 
             %dt= t(:public_key)
             %dd= x.css('pubKey').text
-
+    - if @data.css('dsData').empty?
-    - @data.css('keyData').each do |x|
       %dl.dl-horizontal
+      - @data.css('keyData').each do |x|
         %dt= t(:flag)
         %dd= x.css('flags').text
 

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)