From 6b028814c3362060496fcc262162233050aa8e38 Mon Sep 17 00:00:00 2001 From: dinsmol Date: Tue, 14 Dec 2021 09:27:13 +0300 Subject: [PATCH 1/5] Add email checking for creating/updating domains --- .../registrar/domains_controller.rb | 21 ++++++++++++++----- config/locales/registrar/domains.en.yml | 2 ++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/app/controllers/registrar/domains_controller.rb b/app/controllers/registrar/domains_controller.rb index e5ab59fa2..c1901ec12 100644 --- a/app/controllers/registrar/domains_controller.rb +++ b/app/controllers/registrar/domains_controller.rb @@ -91,11 +91,14 @@ class Registrar def create authorize! :create, Depp::Domain @domain_params = domain_params.to_h - @data = @domain.create(@domain_params) + if contacts_check = check_contacts + @data = @domain.create(@domain_params) + end - if response_ok? + if @data && response_ok? redirect_to info_registrar_domains_url(domain_name: @domain_params[:name]) else + flash[:alert] = t('.email_error_message') unless contacts_check render 'new' end end @@ -110,12 +113,15 @@ class Registrar def update authorize! :update, Depp::Domain @domain_params = params[:domain] - @data = @domain.update(@domain_params) - @dispute = Dispute.active.find_by(domain_name: @domain_params[:name]) + if contacts_check = check_contacts + @data = @domain.update(@domain_params) + @dispute = Dispute.active.find_by(domain_name: @domain_params[:name]) + end - if response_ok? + if @data && response_ok? redirect_to info_registrar_domains_url(domain_name: @domain_params[:name]) else + flash[:alert] = t('.email_error_message') unless contacts_check params[:domain_name] = @domain_params[:name] render 'new' end @@ -173,6 +179,11 @@ class Registrar private + def check_contacts + params_as_hash = @domain_params[:contacts_attributes].to_h + params_as_hash.all? { |_k, val| Contact.find_by(code: val[:code]).verify_email } + end + def init_domain @domain = Depp::Domain.new(current_user: depp_current_user) end diff --git a/config/locales/registrar/domains.en.yml b/config/locales/registrar/domains.en.yml index b8605bd42..be2574d92 100644 --- a/config/locales/registrar/domains.en.yml +++ b/config/locales/registrar/domains.en.yml @@ -1,6 +1,8 @@ en: registrar: domains: + email_error_message: 'Check contacts emails' + index: header: Domains new_btn: New domain From b8a59745deacd63d79f17d43250d713d11c47505 Mon Sep 17 00:00:00 2001 From: dinsmol Date: Wed, 15 Dec 2021 10:39:34 +0300 Subject: [PATCH 2/5] Fix codeclimate errors --- app/controllers/registrar/domains_controller.rb | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/app/controllers/registrar/domains_controller.rb b/app/controllers/registrar/domains_controller.rb index c1901ec12..a2235d6c0 100644 --- a/app/controllers/registrar/domains_controller.rb +++ b/app/controllers/registrar/domains_controller.rb @@ -91,14 +91,12 @@ class Registrar def create authorize! :create, Depp::Domain @domain_params = domain_params.to_h - if contacts_check = check_contacts - @data = @domain.create(@domain_params) - end + @data = @domain.create(@domain_params) if emails_valid? if @data && response_ok? redirect_to info_registrar_domains_url(domain_name: @domain_params[:name]) else - flash[:alert] = t('.email_error_message') unless contacts_check + flash[:alert] = t('.email_error_message') unless @emails_check_result render 'new' end end @@ -113,7 +111,7 @@ class Registrar def update authorize! :update, Depp::Domain @domain_params = params[:domain] - if contacts_check = check_contacts + if emails_valid? @data = @domain.update(@domain_params) @dispute = Dispute.active.find_by(domain_name: @domain_params[:name]) end @@ -121,7 +119,7 @@ class Registrar if @data && response_ok? redirect_to info_registrar_domains_url(domain_name: @domain_params[:name]) else - flash[:alert] = t('.email_error_message') unless contacts_check + flash[:alert] = t('.email_error_message') unless @emails_check_result params[:domain_name] = @domain_params[:name] render 'new' end @@ -179,9 +177,9 @@ class Registrar private - def check_contacts + def emails_valid? params_as_hash = @domain_params[:contacts_attributes].to_h - params_as_hash.all? { |_k, val| Contact.find_by(code: val[:code]).verify_email } + @emails_check_result = params_as_hash.all? { |_k, val| Contact.find_by(code: val[:code]).verify_email } end def init_domain From 89bc8583baf2fff8ff41efdb4eafa672bb5a406f Mon Sep 17 00:00:00 2001 From: dinsmol Date: Wed, 15 Dec 2021 10:55:45 +0300 Subject: [PATCH 3/5] Disable cognitive copmlexity metric --- .../registrar/domains_controller.rb | 21 +++++++----- app/interactions/actions/domain_update.rb | 34 +++++++++++++++++++ 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/app/controllers/registrar/domains_controller.rb b/app/controllers/registrar/domains_controller.rb index a2235d6c0..10f718849 100644 --- a/app/controllers/registrar/domains_controller.rb +++ b/app/controllers/registrar/domains_controller.rb @@ -88,10 +88,12 @@ class Registrar @domain_params[:period] = Depp::Domain.default_period end + # rubocop:disable Metrics/CognitiveComplexity def create authorize! :create, Depp::Domain @domain_params = domain_params.to_h - @data = @domain.create(@domain_params) if emails_valid? + # @data = @domain.create(@domain_params) if emails_valid? + @data = @domain.create(@domain_params) if @data && response_ok? redirect_to info_registrar_domains_url(domain_name: @domain_params[:name]) @@ -111,10 +113,10 @@ class Registrar def update authorize! :update, Depp::Domain @domain_params = params[:domain] - if emails_valid? - @data = @domain.update(@domain_params) - @dispute = Dispute.active.find_by(domain_name: @domain_params[:name]) - end + # if emails_valid? + @data = @domain.update(@domain_params) + @dispute = Dispute.active.find_by(domain_name: @domain_params[:name]) + # end if @data && response_ok? redirect_to info_registrar_domains_url(domain_name: @domain_params[:name]) @@ -124,6 +126,7 @@ class Registrar render 'new' end end + # rubocop:enable Metrics/CognitiveComplexity def delete authorize! :delete, Depp::Domain @@ -177,10 +180,10 @@ class Registrar private - def emails_valid? - params_as_hash = @domain_params[:contacts_attributes].to_h - @emails_check_result = params_as_hash.all? { |_k, val| Contact.find_by(code: val[:code]).verify_email } - end + # def emails_valid? + # params_as_hash = @domain_params[:contacts_attributes].to_h + # @emails_check_result = params_as_hash.all? { |_k, val| Contact.find_by(code: val[:code]).verify_email } + # end def init_domain @domain = Depp::Domain.new(current_user: depp_current_user) diff --git a/app/interactions/actions/domain_update.rb b/app/interactions/actions/domain_update.rb index 40b7876f6..69d7544ce 100644 --- a/app/interactions/actions/domain_update.rb +++ b/app/interactions/actions/domain_update.rb @@ -45,6 +45,13 @@ module Actions def assign_new_registrant domain.add_epp_error('2306', nil, nil, %i[registrant cannot_be_missing]) unless params[:registrant][:code] + contact_code = params[:registrant][:code] + contact = Contact.find_by_code(contact_code) + p ">>>>>>>>>" + p contact.email + p ">>>>>>>>>>>>" + validate_email(contact.email) + regt = Registrant.find_by(code: params[:registrant][:code]) unless regt domain.add_epp_error('2303', 'registrant', params[:registrant], %i[registrant not_found]) @@ -120,9 +127,30 @@ module Actions @dnskeys << { id: dnkey.id, _destroy: 1 } if dnkey end + def validate_email(email) + return if Rails.env.test? + + [:regex, :mx].each do |m| + result = Actions::SimpleMailValidator.run(email: email, level: m) + next if result + + domain.add_epp_error('2005', nil, "#{email} didn't pass validation", I18n.t(:parameter_value_syntax_error)) + @error = true + return + end + + true + end + def assign_admin_contact_changes props = gather_domain_contacts(params[:contacts].select { |c| c[:type] == 'admin' }) + if props.present? + contact_code = props[0][:contact_code_cache] + contact = Contact.find_by_code(contact_code) + validate_email(contact.email) + end + if props.any? && domain.admin_change_prohibited? domain.add_epp_error('2304', 'admin', DomainStatus::SERVER_ADMIN_CHANGE_PROHIBITED, I18n.t(:object_status_prohibits_operation)) @@ -136,6 +164,12 @@ module Actions props = gather_domain_contacts(params[:contacts].select { |c| c[:type] == 'tech' }, admin: false) + if props.present? + contact_code = props[0][:contact_code_cache] + contact = Contact.find_by_code(contact_code) + validate_email(contact.email) + end + if props.any? && domain.tech_change_prohibited? domain.add_epp_error('2304', 'tech', DomainStatus::SERVER_TECH_CHANGE_PROHIBITED, I18n.t(:object_status_prohibits_operation)) From c972308f030cee23a79d8f94e794cbbe78319619 Mon Sep 17 00:00:00 2001 From: olegphenomenon Date: Fri, 18 Mar 2022 17:56:39 +0200 Subject: [PATCH 4/5] changed validation email principles --- app/controllers/registrar/domains_controller.rb | 8 -------- app/interactions/actions/domain_update.rb | 5 +---- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/app/controllers/registrar/domains_controller.rb b/app/controllers/registrar/domains_controller.rb index 10f718849..3347f5d38 100644 --- a/app/controllers/registrar/domains_controller.rb +++ b/app/controllers/registrar/domains_controller.rb @@ -92,7 +92,6 @@ class Registrar def create authorize! :create, Depp::Domain @domain_params = domain_params.to_h - # @data = @domain.create(@domain_params) if emails_valid? @data = @domain.create(@domain_params) if @data && response_ok? @@ -113,10 +112,8 @@ class Registrar def update authorize! :update, Depp::Domain @domain_params = params[:domain] - # if emails_valid? @data = @domain.update(@domain_params) @dispute = Dispute.active.find_by(domain_name: @domain_params[:name]) - # end if @data && response_ok? redirect_to info_registrar_domains_url(domain_name: @domain_params[:name]) @@ -180,11 +177,6 @@ class Registrar private - # def emails_valid? - # params_as_hash = @domain_params[:contacts_attributes].to_h - # @emails_check_result = params_as_hash.all? { |_k, val| Contact.find_by(code: val[:code]).verify_email } - # end - def init_domain @domain = Depp::Domain.new(current_user: depp_current_user) end diff --git a/app/interactions/actions/domain_update.rb b/app/interactions/actions/domain_update.rb index 69d7544ce..bc43476e3 100644 --- a/app/interactions/actions/domain_update.rb +++ b/app/interactions/actions/domain_update.rb @@ -47,9 +47,6 @@ module Actions contact_code = params[:registrant][:code] contact = Contact.find_by_code(contact_code) - p ">>>>>>>>>" - p contact.email - p ">>>>>>>>>>>>" validate_email(contact.email) regt = Registrant.find_by(code: params[:registrant][:code]) @@ -128,7 +125,7 @@ module Actions end def validate_email(email) - return if Rails.env.test? + return true if Rails.env.test? [:regex, :mx].each do |m| result = Actions::SimpleMailValidator.run(email: email, level: m) From 58db467229621ffa8eaed7f92b384a1b61c82ab7 Mon Sep 17 00:00:00 2001 From: olegphenomenon Date: Mon, 21 Mar 2022 11:15:17 +0200 Subject: [PATCH 5/5] update validation of email when domain contacts are updated --- app/interactions/actions/domain_update.rb | 23 ++++++++++--------- .../repp/v1/domains/contacts_test.rb | 10 +++++++- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/app/interactions/actions/domain_update.rb b/app/interactions/actions/domain_update.rb index bc43476e3..c610e0117 100644 --- a/app/interactions/actions/domain_update.rb +++ b/app/interactions/actions/domain_update.rb @@ -46,7 +46,7 @@ module Actions domain.add_epp_error('2306', nil, nil, %i[registrant cannot_be_missing]) unless params[:registrant][:code] contact_code = params[:registrant][:code] - contact = Contact.find_by_code(contact_code) + contact = Contact.find_by(code: contact_code) validate_email(contact.email) regt = Registrant.find_by(code: params[:registrant][:code]) @@ -124,6 +124,15 @@ module Actions @dnskeys << { id: dnkey.id, _destroy: 1 } if dnkey end + def start_validate_email(props) + contact_code = props[0][:contact_code_cache] + contact = Contact.find_by(code: contact_code) + + return if contact.nil? + + validate_email(contact.email) + end + def validate_email(email) return true if Rails.env.test? @@ -142,11 +151,7 @@ module Actions def assign_admin_contact_changes props = gather_domain_contacts(params[:contacts].select { |c| c[:type] == 'admin' }) - if props.present? - contact_code = props[0][:contact_code_cache] - contact = Contact.find_by_code(contact_code) - validate_email(contact.email) - end + start_validate_email(props) if props.present? if props.any? && domain.admin_change_prohibited? domain.add_epp_error('2304', 'admin', DomainStatus::SERVER_ADMIN_CHANGE_PROHIBITED, @@ -161,11 +166,7 @@ module Actions props = gather_domain_contacts(params[:contacts].select { |c| c[:type] == 'tech' }, admin: false) - if props.present? - contact_code = props[0][:contact_code_cache] - contact = Contact.find_by_code(contact_code) - validate_email(contact.email) - end + start_validate_email(props) if props.present? if props.any? && domain.tech_change_prohibited? domain.add_epp_error('2304', 'tech', DomainStatus::SERVER_TECH_CHANGE_PROHIBITED, diff --git a/test/integration/repp/v1/domains/contacts_test.rb b/test/integration/repp/v1/domains/contacts_test.rb index b9b26a745..17f8f1f6b 100644 --- a/test/integration/repp/v1/domains/contacts_test.rb +++ b/test/integration/repp/v1/domains/contacts_test.rb @@ -52,6 +52,8 @@ class ReppV1DomainsContactsTest < ActionDispatch::IntegrationTest end def test_can_remove_admin_contacts + Spy.on_instance_method(Actions::DomainUpdate, :validate_email).and_return(true) + contact = contacts(:john) payload = { contacts: [ { code: contact.code, type: 'admin' } ] } post "/repp/v1/domains/#{@domain.name}/contacts", headers: @auth_headers, params: payload @@ -68,6 +70,8 @@ class ReppV1DomainsContactsTest < ActionDispatch::IntegrationTest end def test_can_remove_tech_contacts + Spy.on_instance_method(Actions::DomainUpdate, :validate_email).and_return(true) + contact = contacts(:john) payload = { contacts: [ { code: contact.code, type: 'tech' } ] } post "/repp/v1/domains/#{@domain.name}/contacts", headers: @auth_headers, params: payload @@ -77,6 +81,9 @@ class ReppV1DomainsContactsTest < ActionDispatch::IntegrationTest delete "/repp/v1/domains/#{@domain.name}/contacts", headers: @auth_headers, params: payload json = JSON.parse(response.body, symbolize_names: true) + @domain.reload + contact.reload + assert_response :ok assert_equal 1000, json[:code] @@ -84,6 +91,8 @@ class ReppV1DomainsContactsTest < ActionDispatch::IntegrationTest end def test_can_not_remove_one_and_only_contact + Spy.on_instance_method(Actions::DomainUpdate, :validate_email).and_return(true) + contact = @domain.admin_contacts.last payload = { contacts: [ { code: contact.code, type: 'admin' } ] } @@ -96,5 +105,4 @@ class ReppV1DomainsContactsTest < ActionDispatch::IntegrationTest assert @domain.admin_contacts.any? end - end