From c58b4fb2e913a8ab1f320310b5eb006b70e3cbe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Thu, 7 Jan 2021 13:59:12 +0200 Subject: [PATCH] Hash: select entries by keys --- .codeclimate.yml | 3 + app/controllers/repp/v1/domains_controller.rb | 5 + .../process_update_confirmed.rb | 10 +- app/models/actions/domain_create.rb | 126 +++++++++++------- app/models/actions/domain_update.rb | 118 ++++++++-------- app/models/nameserver.rb | 2 +- config/initializers/monkey_patches.rb | 1 + lib/core_monkey_patches/hash.rb | 5 + lib/deserializers/xml/dnssec.rb | 21 +-- lib/deserializers/xml/domain.rb | 16 ++- lib/deserializers/xml/domain_update.rb | 35 ++--- 11 files changed, 201 insertions(+), 141 deletions(-) create mode 100644 lib/core_monkey_patches/hash.rb diff --git a/.codeclimate.yml b/.codeclimate.yml index d079d891f..1b9e4ee26 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -30,6 +30,9 @@ checks: method-lines: config: threshold: 40 + method-count: + config: + threshold: 25 exclude_patterns: - "app/models/version/" - "bin/" diff --git a/app/controllers/repp/v1/domains_controller.rb b/app/controllers/repp/v1/domains_controller.rb index f69f33dd1..283d2bf7b 100644 --- a/app/controllers/repp/v1/domains_controller.rb +++ b/app/controllers/repp/v1/domains_controller.rb @@ -81,6 +81,11 @@ module Repp h = {} h[transfer_info_params[:id].match?(/\A[0-9]+\z/) ? :id : :name] = transfer_info_params[:id] @domain = Domain.find_by!(h) + + validate_registrar_authorization + end + + def validate_registrar_authorization return if @domain.registrar == current_user.registrar return if @domain.transfer_code.eql?(request.headers['Auth-Code']) diff --git a/app/interactions/domains/update_confirm/process_update_confirmed.rb b/app/interactions/domains/update_confirm/process_update_confirmed.rb index 9088ce71f..291253651 100644 --- a/app/interactions/domains/update_confirm/process_update_confirmed.rb +++ b/app/interactions/domains/update_confirm/process_update_confirmed.rb @@ -22,13 +22,17 @@ module Domains def update_domain frame_json = domain.pending_json['frame'] - user = ApiUser.find(domain.pending_json['current_user_id']) frame = frame_json ? frame_json.with_indifferent_access : {} + assign_domain_update_meta + + Actions::DomainUpdate.new(domain, frame, true).call + end + + def assign_domain_update_meta + user = ApiUser.find(domain.pending_json['current_user_id']) domain.upid = user.registrar.id if user.registrar domain.up_date = Time.zone.now - - Actions::DomainUpdate.new(domain, frame, true).call end end end diff --git a/app/models/actions/domain_create.rb b/app/models/actions/domain_create.rb index 7362be088..ced9c2327 100644 --- a/app/models/actions/domain_create.rb +++ b/app/models/actions/domain_create.rb @@ -1,5 +1,5 @@ module Actions - class DomainCreate + class DomainCreate # rubocop:disable Metrics/ClassLength attr_reader :domain, :params def initialize(domain, params) @@ -14,8 +14,7 @@ module Actions assign_registrant assign_nameservers - assign_admin_contacts - assign_tech_contacts + assign_domain_contacts domain.attach_default_contacts assign_expiry_time maybe_attach_legal_doc @@ -27,21 +26,25 @@ module Actions def validate_domain_integrity return unless Domain.release_to_auction - dn = DNS::DomainName.new(SimpleIDN.to_unicode(params[:name].strip.downcase)) + dn = DNS::DomainName.new(domain.name) if dn.at_auction? domain.add_epp_error('2306', nil, nil, 'Parameter value policy error: domain is at auction') elsif dn.awaiting_payment? domain.add_epp_error('2003', nil, nil, 'Required parameter missing; reserved>pw element' \ ' required for reserved domains') elsif dn.pending_registration? - if params[:reserved_pw].blank? - domain.add_epp_error('2003', nil, nil, 'Required parameter missing; reserved>pw ' \ - 'element is required') - else - unless dn.available_with_code?(params[:reserved_pw]) - domain.add_epp_error('2202', nil, nil, 'Invalid authorization information; invalid ' \ - 'reserved>pw value') - end + validate_reserved_password(dn) + end + end + + def validate_reserved_password(domain_name) + if params[:reserved_pw].blank? + domain.add_epp_error('2003', nil, nil, 'Required parameter missing; reserved>pw ' \ + 'element is required') + else + unless domain_name.available_with_code?(params[:reserved_pw]) + domain.add_epp_error('2202', nil, nil, 'Invalid authorization information; invalid ' \ + 'reserved>pw value') end end end @@ -63,79 +66,95 @@ module Actions def assign_domain_attributes domain.name = params[:name].strip.downcase domain.registrar = Registrar.find(params[:registrar_id]) - domain.period = params[:period] - domain.period_unit = params[:period_unit] + assign_domain_period + assign_domain_auth_codes + domain.dnskeys_attributes = params[:dnskeys_attributes] + end + + def assign_domain_auth_codes domain.transfer_code = params[:transfer_code] if params[:transfer_code].present? domain.reserved_pw = params[:reserved_pw] if params[:reserved_pw].present? - domain.dnskeys_attributes = params[:dnskeys_attributes] + end + + def assign_domain_period + domain.period = params[:period] + domain.period_unit = params[:period_unit] end def assign_nameservers domain.nameservers_attributes = params[:nameservers_attributes] end - def assign_admin_contacts - attrs = [] - params[:admin_domain_contacts_attributes].each do |c| - contact = Contact.find_by(code: c) - domain.add_epp_error('2303', 'contact', c, %i[domain_contacts not_found]) if contact.blank? - attrs << { contact_id: contact.id, contact_code_cache: contact.code } if contact + def assign_contact(contact_code, admin: true) + contact = Contact.find_by(code: contact_code) + arr = admin ? @admin_contacts : @tech_contacts + if contact + arr << { contact_id: contact.id, contact_code_cache: contact.code } + else + domain.add_epp_error('2303', 'contact', contact_code, %i[domain_contacts not_found]) end - - domain.admin_domain_contacts_attributes = attrs end - def assign_tech_contacts - attrs = [] - params[:tech_domain_contacts_attributes].each do |c| - contact = Contact.find_by(code: c) - domain.add_epp_error('2303', 'contact', c, %i[domain_contacts not_found]) if contact.blank? - attrs << { contact_id: contact.id, contact_code_cache: contact.code } if contact - end + def assign_domain_contacts + @admin_contacts = [] + @tech_contacts = [] + params[:admin_domain_contacts_attributes].each { |c| assign_contact(c) } + params[:tech_domain_contacts_attributes].each { |c| assign_contact(c, admin: false) } - domain.tech_domain_contacts_attributes = attrs + domain.admin_domain_contacts_attributes = @admin_contacts + domain.tech_domain_contacts_attributes = @tech_contacts end def assign_expiry_time - period = domain.period.to_i + period = Integer(domain.period) plural_period_unit_name = (domain.period_unit == 'm' ? 'months' : 'years').to_sym exp = (Time.zone.now.advance(plural_period_unit_name => period) + 1.day).beginning_of_day domain.expire_time = exp end - def debit_registrar - @domain_pricelist ||= domain.pricelist('create', domain.period.try(:to_i), domain.period_unit) - if @domain_pricelist.try(:price) && domain.registrar.balance < @domain_pricelist.price.amount - domain.add_epp_error(2104, nil, nil, I18n.t('billing_failure_credit_balance_low')) - return - elsif !@domain_pricelist.try(:price) + def action_billable? + unless domain_pricelist&.price domain.add_epp_error(2104, nil, nil, I18n.t(:active_price_missing_for_this_operation)) - return + return false end - domain.registrar.debit!(sum: @domain_pricelist.price.amount, price: @domain_pricelist, + if domain.registrar.balance < domain_pricelist.price.amount + domain.add_epp_error(2104, nil, nil, I18n.t('billing_failure_credit_balance_low')) + return false + end + + true + end + + def debit_registrar + return unless action_billable? + + domain.registrar.debit!(sum: domain_pricelist.price.amount, price: domain_pricelist, description: "#{I18n.t('create')} #{domain.name}", activity_type: AccountActivity::CREATE) end - def process_auction_and_disputes - dn = DNS::DomainName.new(SimpleIDN.to_unicode(params[:name])) - Dispute.close_by_domain(domain.name) - return unless Domain.release_to_auction && dn.pending_registration? + def domain_pricelist + @domain_pricelist ||= domain.pricelist('create', domain.period.try(:to_i), domain.period_unit) - auction = Auction.find_by(domain: domain.name, status: Auction.statuses[:payment_received]) - auction.domain_registered! + @domain_pricelist end def maybe_attach_legal_doc Actions::BaseAction.attach_legal_doc_to_new(domain, params[:legal_document], domain: true) end + def process_auction_and_disputes + dn = DNS::DomainName.new(domain.name) + Dispute.close_by_domain(domain.name) + return unless Domain.release_to_auction && dn.pending_registration? + + Auction.find_by(domain: domain.name, + status: Auction.statuses[:payment_received])&.domain_registered! + end + def commit - unless domain.valid? - domain.errors.delete(:name_dirty) if domain.errors[:puny_label].any? - return false if domain.errors.any? - end + return false if validation_process_errored? debit_registrar return false if domain.errors.any? @@ -143,5 +162,12 @@ module Actions process_auction_and_disputes domain.save end + + def validation_process_errored? + return if domain.valid? + + domain.errors.delete(:name_dirty) if domain.errors[:puny_label].any? + domain.errors.any? + end end end diff --git a/app/models/actions/domain_update.rb b/app/models/actions/domain_update.rb index 0d05bb853..176a9831b 100644 --- a/app/models/actions/domain_update.rb +++ b/app/models/actions/domain_update.rb @@ -1,28 +1,30 @@ module Actions - class DomainUpdate + class DomainUpdate # rubocop:disable Metrics/ClassLength attr_reader :domain, :params, :bypass_verify def initialize(domain, params, bypass_verify) @domain = domain @params = params @bypass_verify = bypass_verify + @changes_registrant = false end def call - @changes_registrant = false - validate_domain_integrity assign_new_registrant if params[:registrant] - assign_nameserver_modifications if params[:nameservers] - assign_admin_contact_changes if params[:contacts] - assign_tech_contact_changes if params[:contacts] - assign_requested_statuses if params[:statuses] - assign_dnssec_modifications if params[:dns_keys] + assign_relational_modifications + assign_requested_statuses maybe_attach_legal_doc commit end + def assign_relational_modifications + assign_nameserver_modifications if params[:nameservers] + assign_dnssec_modifications if params[:dns_keys] + (assign_admin_contact_changes && assign_tech_contact_changes) if params[:contacts] + end + def validate_domain_integrity domain.auth_info = params[:auth_info] if params[:auth_info] @@ -67,7 +69,7 @@ module Actions end def validate_ns_integrity(ns_attr) - ns = domain.nameservers.find_by_hash_params(ns_attr.except(:action)).first + ns = domain.nameservers.from_hash_params(ns_attr.except(:action)).first if ns @nameservers << { id: ns.id, _destroy: 1 } else @@ -78,16 +80,16 @@ module Actions def assign_dnssec_modifications @dnskeys = [] - params[:dns_key].each do |key| + params[:dns_keys].each do |key| case key[:action] when 'add' - validate_dnskey_integrity(key) && dnskeys << key.except(:action) + validate_dnskey_integrity(key) && @dnskeys << key.except(:action) when 'rem' assign_removable_dnskey(key) end end - domain.dnskeys_attributes = dnskeys + domain.dnskeys_attributes = @dnskeys end def validate_dnskey_integrity(key) @@ -136,7 +138,7 @@ module Actions contacts.each do |c| contact = contact_for_action(action: c[:action], method: admin ? 'admin' : 'tech', code: c[:code]) - entry = assign_contact(contact, add: c[:action] == 'add', admin: admin) + entry = assign_contact(contact, add: c[:action] == 'add', admin: admin, code: c[:code]) props << entry if entry.is_a?(Hash) end @@ -150,11 +152,11 @@ module Actions domain.tech_domain_contacts.find_by(contact_code_cache: code) end - def assign_contact(obj, add: false, admin: true) + def assign_contact(obj, add: false, admin: true, code:) if obj.blank? - domain.add_epp_error('2303', 'contact', obj[:code], %i[domain_contacts not_found]) + domain.add_epp_error('2303', 'contact', code, %i[domain_contacts not_found]) elsif obj.org? && admin - domain.add_epp_error('2306', 'contact', obj[:code], + domain.add_epp_error('2306', 'contact', code, %i[domain_contacts admin_contact_can_be_only_private_person]) else add ? { contact_id: obj.id, contact_code_cache: obj.code } : { id: obj.id, _destroy: 1 } @@ -162,70 +164,82 @@ module Actions end def assign_requested_statuses - rem = [] - add = [] + return unless params[:statuses] - params[:statuses].each do |s| - status, action = s.slice(:status, :action) - if permitted_status?(status, action) - action == 'add' ? add << status : rem << action - end - end + @rem = [] + @add = [] + @failed = false - domain.statuses = domain.statuses - rem + add unless domain.errors.any? + params[:statuses].each { |s| verify_status_eligiblity(s) } + domain.statuses = (domain.statuses - @rem + @add) unless @failed + end + + def verify_status_eligiblity(status_entry) + status, action = status_entry.select_keys(:status, :action) + return unless permitted_status?(status, action) + + action == 'add' ? @add << status : @rem << status end def permitted_status?(status, action) if DomainStatus::CLIENT_STATUSES.include?(status) && (domain.statuses.include?(status) || action == 'add') - - true - else - domain.add_epp_error('2303', 'status', s[:status], %i[statuses not_found]) - false + return true end + + domain.add_epp_error('2303', 'status', status, %i[statuses not_found]) + @failed = true + false end def verify_registrant_change? - return false unless @changes_registrant - return false if params[:registrant][:verified] == true + return if !@changes_registrant || params[:registrant][:verified] == true return true unless domain.disputed? + return validate_dispute_case if params[:reserved_pw] - if params[:reserved_pw] - dispute = Dispute.active.find_by(domain_name: domain.name, password: params[:reserved_pw]) - if dispute - Dispute.close_by_domain(domain.name) - return false - else - domain.add_epp_error('2202', nil, nil, - 'Invalid authorization information; invalid reserved>pw value') - end - else - domain.add_epp_error('2304', nil, nil, 'Required parameter missing; reservedpw element ' \ - 'required for dispute domains') - end + domain.add_epp_error('2304', nil, nil, 'Required parameter missing; reservedpw element ' \ + 'required for dispute domains') true end + def validate_dispute_case + dispute = Dispute.active.find_by(domain_name: domain.name, password: params[:reserved_pw]) + if dispute + Dispute.close_by_domain(domain.name) + false + else + domain.add_epp_error('2202', nil, nil, + 'Invalid authorization information; invalid reserved>pw value') + true + end + end + def maybe_attach_legal_doc Actions::BaseAction.maybe_attach_legal_doc(domain, params[:legal_document]) end - def commit - return false if domain.errors[:epp_errors].any? - return false unless domain.valid? - + def ask_registrant_verification if verify_registrant_change? && !bypass_verify && - Setting.request_confirmation_on_registrant_change_enabled && !bypass_verify + Setting.request_confirmation_on_registrant_change_enabled domain.registrant_verification_asked!(params, params[:registrar_id]) end + end - return false if domain.errors[:epp_errors].any? - return false unless domain.valid? + def commit + return false if any_errors? + + ask_registrant_verification + return false if any_errors? domain.save end + + def any_errors? + return true if domain.errors[:epp_errors].any? || domain.invalid? + + false + end end end diff --git a/app/models/nameserver.rb b/app/models/nameserver.rb index 3e4051165..85e64ce41 100644 --- a/app/models/nameserver.rb +++ b/app/models/nameserver.rb @@ -63,7 +63,7 @@ class Nameserver < ApplicationRecord end class << self - def find_by_hash_params params + def from_hash_params params params = params.with_indifferent_access rel = all rel = rel.where(hostname: params[:hostname]) diff --git a/config/initializers/monkey_patches.rb b/config/initializers/monkey_patches.rb index d342fb6b0..80109ecdd 100644 --- a/config/initializers/monkey_patches.rb +++ b/config/initializers/monkey_patches.rb @@ -1,4 +1,5 @@ require 'core_monkey_patches/array' +require 'core_monkey_patches/hash' require 'gem_monkey_patches/builder' require 'gem_monkey_patches/i18n' require 'gem_monkey_patches/paper_trail' diff --git a/lib/core_monkey_patches/hash.rb b/lib/core_monkey_patches/hash.rb new file mode 100644 index 000000000..debd1da7f --- /dev/null +++ b/lib/core_monkey_patches/hash.rb @@ -0,0 +1,5 @@ +class Hash + def select_keys(*args) + select { |k, _| args.include?(k) }.map { |_k, v| v } + end +end diff --git a/lib/deserializers/xml/dnssec.rb b/lib/deserializers/xml/dnssec.rb index a9c348ea6..89e7934a8 100644 --- a/lib/deserializers/xml/dnssec.rb +++ b/lib/deserializers/xml/dnssec.rb @@ -47,14 +47,16 @@ module Deserializers # schema validation prevents both in the same parent node if frame.css('dsData').present? - frame.css('dsData').each do |ds_data| - @ds_data << Deserializers::Xml::DnssecKey.new(ds_data, true).call - end - else - frame.css('keyData').each do |key| - @key_data << Deserializers::Xml::DnssecKey.new(key, false).call - end + frame.css('dsData').each { |k| @ds_data << key_from_params(k, dsa: true) } end + + return if frame.css('keyData').blank? + + frame.css('keyData').each { |k| @key_data << key_from_params(k, dsa: false) } + end + + def key_from_params(obj, dsa: false) + Deserializers::Xml::DnssecKey.new(obj, dsa).call end def call @@ -67,9 +69,8 @@ module Deserializers 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 + data = ds_data.present? ? ds_filter(dns_keys) : kd_filter(dns_keys) + data.each { |inf_data| inf_data.blank? ? nil : mark(inf_data) } end private diff --git a/lib/deserializers/xml/domain.rb b/lib/deserializers/xml/domain.rb index 7b8ba3097..c663d4e7e 100644 --- a/lib/deserializers/xml/domain.rb +++ b/lib/deserializers/xml/domain.rb @@ -1,8 +1,7 @@ module Deserializers module Xml class Domain - attr_reader :frame - attr_reader :registrar + attr_reader :frame, :registrar def initialize(frame, registrar) @frame = frame @@ -15,15 +14,24 @@ module Deserializers registrar_id: registrar, registrant_id: if_present('registrant'), reserved_pw: if_present('reserved > pw'), - period: frame.css('period').text.present? ? Integer(frame.css('period').text) : 1, - period_unit: frame.css('period').first ? frame.css('period').first[:unit] : 'y', } + attributes.merge!(assign_period_attributes) + pw = frame.css('authInfo > pw').text attributes[:transfer_code] = pw if pw.present? attributes.compact end + def assign_period_attributes + period = frame.css('period') + + { + period: period.text.present? ? Integer(period.text) : 1, + period_unit: period.first ? period.first[:unit] : 'y', + } + end + def if_present(css_path) return if frame.css(css_path).blank? diff --git a/lib/deserializers/xml/domain_update.rb b/lib/deserializers/xml/domain_update.rb index b48f1e4aa..62b528bd7 100644 --- a/lib/deserializers/xml/domain_update.rb +++ b/lib/deserializers/xml/domain_update.rb @@ -42,20 +42,18 @@ module Deserializers end def nameservers - nameservers = [] - frame.css('add > ns > hostAttr').each do |ns| - nsrv = Deserializers::Xml::Nameserver.new(ns).call - nsrv[:action] = 'add' - nameservers << nsrv - end + @nameservers = [] - frame.css('rem > ns > hostAttr').each do |ns| - nsrv = Deserializers::Xml::Nameserver.new(ns).call - nsrv[:action] = 'rem' - nameservers << nsrv - end + frame.css('add > ns > hostAttr').each { |ns| assign_ns(ns) } + frame.css('rem > ns > hostAttr').each { |ns| assign_ns(ns, add: false) } - nameservers.presence + @nameservers.presence + end + + def assign_ns(nameserver, add: true) + nsrv = Deserializers::Xml::Nameserver.new(nameserver).call + nsrv[:action] = add ? 'add' : 'rem' + @nameservers << nsrv end def dns_keys @@ -72,17 +70,12 @@ module Deserializers def statuses return if frame.css('status').blank? - statuses = [] + s = [] - frame.css('add > status').each do |e| - statuses << { status: e.attr('s').to_s, action: 'add' } - end + frame.css('add > status').each { |e| s << { status: e.attr('s'), action: 'add' } } + frame.css('rem > status').each { |e| s << { status: e.attr('s'), action: 'rem' } } - frame.css('rem > status').each do |e| - statuses << { status: e.attr('s').to_s, action: 'rem' } - end - - statuses + s end def legal_document