From f2978599b483cfefe91d69ab29874fffc7da7c47 Mon Sep 17 00:00:00 2001 From: oleghasjanov Date: Mon, 3 Feb 2025 13:59:03 +0200 Subject: [PATCH] feat: add admin contact ident type validation - Add new setting for allowed admin contact ident types - Add validation for admin contact ident types on domain create/update - Add UI controls for managing allowed ident types - Add tests for new validation rules - Update domain model to respect new settings The changes allow configuring which identification types (private person, organization, birthday) are allowed for administrative contacts. This is enforced when creating new domains or adding new admin contacts. --- app/controllers/admin/settings_controller.rb | 19 +++- app/models/domain.rb | 20 +++- app/models/epp/domain.rb | 26 +++++- app/models/setting_entry.rb | 13 ++- app/views/admin/settings/_setting_row.haml | 28 +++++- config/locales/en.yml | 1 + db/seeds.rb | 13 +++ test/fixtures/setting_entries.yml | 24 +++++ .../epp/domain/create/base_test.rb | 78 ++++++++++++++++ .../epp/domain/update/base_test.rb | 92 +++++++++++++++++++ test/models/domain/domain_version_test.rb | 1 + test/models/domain_test.rb | 42 +++++++++ 12 files changed, 346 insertions(+), 11 deletions(-) diff --git a/app/controllers/admin/settings_controller.rb b/app/controllers/admin/settings_controller.rb index 8a5c275c5..1ebf2946d 100644 --- a/app/controllers/admin/settings_controller.rb +++ b/app/controllers/admin/settings_controller.rb @@ -27,12 +27,25 @@ module Admin def casted_settings settings = {} - + params[:settings].each do |k, v| - settings[k] = { value: v } + setting = SettingEntry.find(k) + value = if setting.format == 'array' + processed_hash = available_options.each_with_object({}) do |option, hash| + hash[option] = (v[option] == "true") + end + processed_hash.to_json + else + v + end + settings[k] = { value: value } end - + settings end + + def available_options + %w[birthday priv org] + end end end diff --git a/app/models/domain.rb b/app/models/domain.rb index 1d1a13083..389d361ba 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -186,6 +186,8 @@ class Domain < ApplicationRecord object_count: admin_contacts_validation_rules(for_org: false), unless: :require_admin_contacts? + validate :validate_admin_contacts_ident_type, on: :create + validates :tech_domain_contacts, object_count: tech_contacts_validation_rules(for_org: true), if: :require_tech_contacts? @@ -857,10 +859,10 @@ class Domain < ApplicationRecord end def require_admin_contacts? - return true if registrant.org? + return true if registrant.org? && Setting.admin_contacts_required_for_org return false unless registrant.priv? - underage_registrant? + underage_registrant? && Setting.admin_contacts_required_for_minors end def require_tech_contacts? @@ -916,4 +918,18 @@ class Domain < ApplicationRecord Date.parse("#{birth_year}-#{month}-#{day}") end + + def validate_admin_contacts_ident_type + allowed_types = Setting.admin_contacts_allowed_ident_type + return if allowed_types.blank? + + admin_contacts.each do |contact| + next if allowed_types[contact.ident_type] == true + + errors.add(:admin_contacts, I18n.t( + 'activerecord.errors.models.domain.admin_contact_invalid_ident_type', + ident_type: contact.ident_type + )) + end + end end diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb index ab5d0c16e..f56947c07 100644 --- a/app/models/epp/domain.rb +++ b/app/models/epp/domain.rb @@ -36,6 +36,23 @@ class Epp::Domain < Domain ok = false end end + + # Validate admin contacts ident type only for new domains or new admin contacts + allowed_types = Setting.admin_contacts_allowed_ident_type + if allowed_types.present? + active_admins.each do |admin_contact| + next if !new_record? && admin_contact.persisted? && !admin_contact.changed? + + contact = admin_contact.contact + unless allowed_types[contact.ident_type] == true + add_epp_error('2306', 'contact', contact.code, + I18n.t('activerecord.errors.models.domain.admin_contact_invalid_ident_type', + ident_type: contact.ident_type)) + ok = false + end + end + end + ok end @@ -95,7 +112,8 @@ class Epp::Domain < Domain [:base, :key_data_not_allowed], [:period, :not_a_number], [:period, :not_an_integer], - [:registrant, :cannot_be_missing] + [:registrant, :cannot_be_missing], + [:admin_contacts, :invalid_ident_type] ], '2308' => [ [:base, :domain_name_blocked, { value: { obj: 'name', val: name_dirty } }], @@ -414,15 +432,15 @@ class Epp::Domain < Domain end def require_admin_contacts? - return true if registrant.org? + return true if registrant.org? && Setting.admin_contacts_required_for_org return false unless registrant.priv? - underage_registrant? + underage_registrant? && Setting.admin_contacts_required_for_minors end def tech_contacts_validation_rules(for_org:) { - min: 0, # Технический контакт опционален для всех + min: 0, max: -> { Setting.tech_contacts_max_count } } end diff --git a/app/models/setting_entry.rb b/app/models/setting_entry.rb index 966831881..739ec71fb 100644 --- a/app/models/setting_entry.rb +++ b/app/models/setting_entry.rb @@ -87,6 +87,17 @@ class SettingEntry < ApplicationRecord end def array_format - JSON.parse(value).to_a + begin + if value.is_a?(String) + JSON.parse(value) + elsif value.is_a?(Hash) + value + else + { 'birthday' => true, 'priv' => true, 'org' => true } + end + rescue JSON::ParserError => e + puts "JSON Parse error: #{e.message}" + { 'birthday' => true, 'priv' => true, 'org' => true } + end end end diff --git a/app/views/admin/settings/_setting_row.haml b/app/views/admin/settings/_setting_row.haml index 44078f32d..720ab6199 100644 --- a/app/views/admin/settings/_setting_row.haml +++ b/app/views/admin/settings/_setting_row.haml @@ -1,6 +1,32 @@ %tr{class: (@errors && @errors.has_key?(setting.code) && "danger")} %td.col-md-6= setting.code.humanize - - if [TrueClass, FalseClass].include?(setting.retrieve.class) + - if setting.format == 'array' + %td.col-md-6 + - available_options = ['birthday', 'priv', 'org'] + - begin + - raw_value = setting.retrieve + - current_values = if raw_value.is_a?(Hash) + - raw_value + - elsif raw_value.is_a?(Array) && raw_value.first.is_a?(Array) + - Hash[raw_value] + - elsif raw_value.is_a?(Array) + - available_options.each_with_object({}) { |opt, hash| hash[opt] = raw_value.include?(opt) } + - else + - begin + - parsed = JSON.parse(raw_value.to_s) + - parsed.is_a?(Hash) ? parsed : available_options.each_with_object({}) { |opt, hash| hash[opt] = true } + - rescue => e + - available_options.each_with_object({}) { |opt, hash| hash[opt] = true } + .row + - available_options.each do |option| + .col-md-4 + .checkbox + %label + = check_box_tag "settings[#{setting.id}][#{option}]", "true", current_values[option], + id: "setting_#{setting.id}_#{option}", + data: { value: current_values[option] } + = option.humanize + - elsif [TrueClass, FalseClass].include?(setting.retrieve.class) %td.col-md-6 = hidden_field_tag("[settings][#{setting.id}]", '', id: nil) = check_box_tag("[settings][#{setting.id}]", true, setting.retrieve) diff --git a/config/locales/en.yml b/config/locales/en.yml index 2f9ba4b15..4083747fe 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -81,6 +81,7 @@ en: domain: <<: *epp_domain_ar_attributes + admin_contact_invalid_ident_type: "Administrative contact with identification type '%{ident_type}' is not allowed" nameserver: attributes: diff --git a/db/seeds.rb b/db/seeds.rb index 4cb36e738..73e8e6f4e 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -7,6 +7,19 @@ ActiveRecord::Base.transaction do SettingEntry.create(code: 'directo_sales_agent', value: 'HELEN', format: 'string', group: 'billing') SettingEntry.create(code: 'admin_contacts_min_count', value: '1', format: 'integer', group: 'domain_validation') SettingEntry.create(code: 'admin_contacts_max_count', value: '10', format: 'integer', group: 'domain_validation') + SettingEntry.create(code: 'admin_contacts_required_for_org', value: 'true', format: 'boolean', group: 'domain_validation') + SettingEntry.create(code: 'admin_contacts_required_for_minors', value: 'true', format: 'boolean', group: 'domain_validation') + SettingEntry.create( + code: 'admin_contacts_allowed_ident_type', + value: { + 'birthday' => true, + 'priv' => true, + 'org' => true + }.to_json, + format: 'array', + group: 'domain_validation' + ) + SettingEntry.create(code: 'tech_contacts_min_count', value: '1', format: 'integer', group: 'domain_validation') SettingEntry.create(code: 'tech_contacts_max_count', value: '10', format: 'integer', group: 'domain_validation') SettingEntry.create(code: 'orphans_contacts_in_months', value: '6', format: 'integer', group: 'domain_validation') diff --git a/test/fixtures/setting_entries.yml b/test/fixtures/setting_entries.yml index c780f6f2a..2df870258 100644 --- a/test/fixtures/setting_entries.yml +++ b/test/fixtures/setting_entries.yml @@ -469,3 +469,27 @@ ip_whitelist_max_count: format: integer created_at: <%= Time.zone.parse('2010-07-05') %> updated_at: <%= Time.zone.parse('2010-07-05') %> + +admin_contacts_required_for_org: + code: admin_contacts_required_for_org + value: 'true' + group: domain_validation + format: boolean + created_at: <%= Time.zone.parse('2010-07-05') %> + updated_at: <%= Time.zone.parse('2010-07-05') %> + +admin_contacts_required_for_minors: + code: admin_contacts_required_for_minors + value: 'true' + group: domain_validation + format: boolean + created_at: <%= Time.zone.parse('2010-07-05') %> + updated_at: <%= Time.zone.parse('2010-07-05') %> + +admin_contacts_allowed_ident_type: + code: admin_contacts_allowed_ident_type + value: '{"birthday":true,"priv":true,"org":false}' + group: domain_validation + format: array + created_at: <%= Time.zone.parse('2010-07-05') %> + updated_at: <%= Time.zone.parse('2010-07-05') %> diff --git a/test/integration/epp/domain/create/base_test.rb b/test/integration/epp/domain/create/base_test.rb index 12eb5bb03..dbb2566ba 100644 --- a/test/integration/epp/domain/create/base_test.rb +++ b/test/integration/epp/domain/create/base_test.rb @@ -597,6 +597,8 @@ class EppDomainCreateBaseTest < EppTestCase end def test_registers_new_domain_with_required_attributes + Setting.admin_contacts_allowed_ident_type = { 'org' => true, 'priv' => true, 'birthday' => true } + now = Time.zone.parse('2010-07-05') travel_to now name = "new.#{dns_zones(:one).origin}" @@ -644,6 +646,8 @@ class EppDomainCreateBaseTest < EppTestCase default_registration_period = 1.year + 1.day assert_equal now + default_registration_period, domain.expire_time + + Setting.admin_contacts_allowed_ident_type = { 'org' => false, 'priv' => true, 'birthday' => true } end def test_registers_domain_without_legaldoc_if_optout @@ -1108,4 +1112,78 @@ class EppDomainCreateBaseTest < EppTestCase ENV["shunter_default_threshold"] = '10000' ENV["shunter_enabled"] = 'false' end + + def test_does_not_register_domain_with_invalid_admin_contact_ident_type + name = "new.#{dns_zones(:one).origin}" + contact = contacts(:john) + registrant = contact.becomes(Registrant) + admin_contact = contacts(:william) + admin_contact.update!(ident_type: 'org') + + request_xml = <<-XML + + + + + + #{name} + #{registrant.code} + #{admin_contact.code} + + + + + #{'test' * 2000} + + + + + XML + + assert_no_difference 'Domain.count' do + post epp_create_path, params: { frame: request_xml }, + headers: { 'HTTP_COOKIE' => 'session=api_bestnames' } + end + + response_xml = Nokogiri::XML(response.body) + assert_correct_against_schema response_xml + assert_epp_response :parameter_value_policy_error + end + + def test_registers_domain_with_valid_admin_contact_ident_type + name = "new.#{dns_zones(:one).origin}" + contact = contacts(:john) + registrant = contact.becomes(Registrant) + admin_contact = contacts(:william) + admin_contact.update!(ident_type: 'priv') + + request_xml = <<-XML + + + + + + #{name} + #{registrant.code} + #{admin_contact.code} + + + + + #{'test' * 2000} + + + + + XML + + assert_difference 'Domain.count' do + post epp_create_path, params: { frame: request_xml }, + headers: { 'HTTP_COOKIE' => 'session=api_bestnames' } + end + + response_xml = Nokogiri::XML(response.body) + assert_correct_against_schema response_xml + assert_epp_response :completed_successfully + end end diff --git a/test/integration/epp/domain/update/base_test.rb b/test/integration/epp/domain/update/base_test.rb index 4f11b1145..0c678afea 100644 --- a/test/integration/epp/domain/update/base_test.rb +++ b/test/integration/epp/domain/update/base_test.rb @@ -969,6 +969,98 @@ class EppDomainUpdateBaseTest < EppTestCase ENV["shunter_enabled"] = 'false' end + def test_does_not_update_domain_with_invalid_admin_contact_ident_type + admin_contact = contacts(:william) + admin_contact.update!(ident_type: 'org') + + request_xml = <<-XML + + + + + + shop.test + + #{admin_contact.code} + + + + + + XML + + post epp_update_path, params: { frame: request_xml }, + headers: { 'HTTP_COOKIE' => 'session=api_bestnames' } + + response_xml = Nokogiri::XML(response.body) + assert_correct_against_schema response_xml + assert_epp_response :parameter_value_policy_error + end + + def test_updates_domain_with_valid_admin_contact_ident_type + admin_contact = contacts(:william) + admin_contact.update!(ident_type: 'priv') + + request_xml = <<-XML + + + + + + shop.test + + #{admin_contact.code} + + + + + + XML + + post epp_update_path, params: { frame: request_xml }, + headers: { 'HTTP_COOKIE' => 'session=api_bestnames' } + + response_xml = Nokogiri::XML(response.body) + assert_correct_against_schema response_xml + assert_epp_response :completed_successfully + end + + def test_skips_admin_contact_ident_type_validation_for_existing_contacts + admin_contact = contacts(:william) + admin_contact.update!(ident_type: 'org') + @domain.admin_contacts << admin_contact + @domain.save! + + # Change allowed types after domain is created + Setting.admin_contacts_allowed_ident_type = { 'birthday' => true, 'priv' => true, 'org' => false } + + # Try to update domain with some other changes + request_xml = <<-XML + + + + + + shop.test + + + new_pw + + + + + + + XML + + post epp_update_path, params: { frame: request_xml }, + headers: { 'HTTP_COOKIE' => 'session=api_bestnames' } + + response_xml = Nokogiri::XML(response.body) + assert_correct_against_schema response_xml + assert_epp_response :completed_successfully + end + private def assert_verification_and_notification_emails diff --git a/test/models/domain/domain_version_test.rb b/test/models/domain/domain_version_test.rb index 4b937e274..7cfdae8d9 100644 --- a/test/models/domain/domain_version_test.rb +++ b/test/models/domain/domain_version_test.rb @@ -14,6 +14,7 @@ class DomainVersionTest < ActiveSupport::TestCase end def test_assigns_creator_to_paper_trail_whodunnit + Setting.admin_contacts_allowed_ident_type = { 'org' => true, 'priv' => true, 'birthday' => true } duplicate_domain = prepare_duplicate_domain PaperTrail.request.whodunnit = @user.id_role_username diff --git a/test/models/domain_test.rb b/test/models/domain_test.rb index d3efe684f..af25c535c 100644 --- a/test/models/domain_test.rb +++ b/test/models/domain_test.rb @@ -9,6 +9,9 @@ class DomainTest < ActiveSupport::TestCase @original_max_admin_contact_count = Setting.admin_contacts_max_count @original_min_tech_contact_count = Setting.tech_contacts_min_count @original_max_tech_contact_count = Setting.tech_contacts_max_count + @original_admin_contacts_required_for_org = Setting.admin_contacts_required_for_org + @original_admin_contacts_required_for_minors = Setting.admin_contacts_required_for_minors + @original_admin_contacts_allowed_ident_type = Setting.admin_contacts_allowed_ident_type end teardown do @@ -17,6 +20,9 @@ class DomainTest < ActiveSupport::TestCase Setting.admin_contacts_max_count = @original_max_admin_contact_count Setting.tech_contacts_min_count = @original_min_tech_contact_count Setting.tech_contacts_max_count = @original_max_tech_contact_count + Setting.admin_contacts_required_for_org = @original_admin_contacts_required_for_org + Setting.admin_contacts_required_for_minors = @original_admin_contacts_required_for_minors + Setting.admin_contacts_allowed_ident_type = @original_admin_contacts_allowed_ident_type end def test_valid_domain_is_valid @@ -615,6 +621,42 @@ class DomainTest < ActiveSupport::TestCase assert domain.valid? end + def test_validates_admin_contact_required_for_org_based_on_setting + domain = valid_domain + domain.registrant.update!(ident_type: 'org') + domain.reload + + # When setting is true + Setting.admin_contacts_required_for_org = true + domain.admin_domain_contacts.clear + assert domain.invalid? + assert_includes domain.errors.full_messages, + 'Admin domain contacts Admin contacts count must be between 1-10' + + # When setting is false + Setting.admin_contacts_required_for_org = false + domain.admin_domain_contacts.clear + assert domain.valid? + end + + def test_validates_admin_contact_required_for_minors_based_on_setting + domain = valid_domain + domain.registrant.update!(ident_type: 'birthday', ident: '2010-07-05') + domain.reload + + # When setting is true + Setting.admin_contacts_required_for_minors = true + domain.admin_domain_contacts.clear + assert domain.invalid? + assert_includes domain.errors.full_messages, + 'Admin domain contacts Admin contacts count must be between 1-10' + + # When setting is false + Setting.admin_contacts_required_for_minors = false + domain.admin_domain_contacts.clear + assert domain.valid? + end + private def valid_domain