From 7cd8f4e5f90f9d85792728f8423306b265f1997d Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Mon, 29 Jan 2018 16:57:14 +0200 Subject: [PATCH 01/34] Add foreign key constraint #661 --- .../20180129143538_add_nameservers_domain_id_fk.rb | 5 +++++ db/structure.sql | 10 ++++++++++ 2 files changed, 15 insertions(+) create mode 100644 db/migrate/20180129143538_add_nameservers_domain_id_fk.rb diff --git a/db/migrate/20180129143538_add_nameservers_domain_id_fk.rb b/db/migrate/20180129143538_add_nameservers_domain_id_fk.rb new file mode 100644 index 000000000..774d4f483 --- /dev/null +++ b/db/migrate/20180129143538_add_nameservers_domain_id_fk.rb @@ -0,0 +1,5 @@ +class AddNameserversDomainIdFk < ActiveRecord::Migration + def change + add_foreign_key :nameservers, :domains, name: 'nameservers_domain_id_fk' + end +end diff --git a/db/structure.sql b/db/structure.sql index d823ae27c..3b5ce16ed 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -4538,6 +4538,14 @@ ALTER TABLE ONLY account_activities ADD CONSTRAINT fk_rails_d2cc3c2fa9 FOREIGN KEY (price_id) REFERENCES prices(id); +-- +-- Name: nameservers_domain_id_fk; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY nameservers + ADD CONSTRAINT nameservers_domain_id_fk FOREIGN KEY (domain_id) REFERENCES domains(id); + + -- -- Name: user_registrar_id_fk; Type: FK CONSTRAINT; Schema: public; Owner: - -- @@ -5060,3 +5068,5 @@ INSERT INTO schema_migrations (version) VALUES ('20171121233843'); INSERT INTO schema_migrations (version) VALUES ('20171123035941'); +INSERT INTO schema_migrations (version) VALUES ('20180129143538'); + From 3dee90143bab333417e7f5df517ff151c94a1ee6 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Mon, 29 Jan 2018 17:46:46 +0200 Subject: [PATCH 02/34] Remove commented code --- app/models/nameserver.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/app/models/nameserver.rb b/app/models/nameserver.rb index fb56f1198..4780a5824 100644 --- a/app/models/nameserver.rb +++ b/app/models/nameserver.rb @@ -2,15 +2,10 @@ class Nameserver < ActiveRecord::Base include Versions # version/nameserver_version.rb include EppErrors - # belongs_to :registrar belongs_to :domain - # scope :owned_by_registrar, -> (registrar) { joins(:domain).where('domains.registrar_id = ?', registrar.id) } - # rubocop: disable Metrics/LineLength validates :hostname, format: { with: /\A(([a-zA-Z0-9]|[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9][a-zA-ZäöüõšžÄÖÜÕŠŽ0-9\-]*[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\z/ } - # validates :ipv4, format: { with: /\A(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\z/, allow_blank: true } - # validates :ipv6, format: { with: /(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]).){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]).){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]))/, allow_blank: true } validate :val_ipv4 validate :val_ipv6 # rubocop: enable Metrics/LineLength @@ -113,8 +108,6 @@ class Nameserver < ActiveRecord::Base params = params.with_indifferent_access rel = all rel = rel.where(hostname: params[:hostname]) - # rel = rel.where(hostname: params[:hostname]) if params[:ipv4] - # ignoring ips rel end From 11e4329c70171c7fe338a89276865a5dfdc95bb6 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Mon, 29 Jan 2018 17:51:40 +0200 Subject: [PATCH 03/34] Remove duplicated translation --- config/locales/en.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 0a4a83c98..509c7c847 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -520,7 +520,6 @@ en: type: 'Type' code: 'Code' nameservers: 'Nameservers' - hostname: 'Hostname' dnskeys: 'DNS Keys' flag: 'Flag' protocol: 'Protocol' From 9dffe962b63a7a0979ee82611a663430932073bd Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Mon, 29 Jan 2018 17:53:53 +0200 Subject: [PATCH 04/34] Remove unused code --- .reek | 4 ---- app/models/nameserver.rb | 31 ------------------------------- spec/models/nameserver_spec.rb | 27 --------------------------- 3 files changed, 62 deletions(-) diff --git a/.reek b/.reek index 060f76785..b04386078 100644 --- a/.reek +++ b/.reek @@ -114,7 +114,6 @@ UncommunicativeVariableName: - Invoice#cancel_overdue_invoices - Legacy::Db - LegalDocument#save_to_filesystem - - Nameserver#replace_hostname_ends - RegistrantUser#find_or_create_by_idc_data - RegistrantUser#find_or_create_by_mid_data - Registrar @@ -327,7 +326,6 @@ DuplicateMethodCall: - Invoice#set_invoice_number - LegalDocument#save_to_filesystem - LegalDocument#self.remove_duplicates - - Nameserver#replace_hostname_ends - Setting#self.params_errors - Setting#self.reload_settings! - Soap::Arireg#associated_businesses @@ -822,7 +820,6 @@ TooManyStatements: - Invoice#set_invoice_number - LegalDocument#save_to_filesystem - LegalDocument#self.remove_duplicates - - Nameserver#replace_hostname_ends - RegistrantUser#find_or_create_by_idc_data - Registrar#generate_iso_11649_reference_no - Soap::Arireg#associated_businesses @@ -1026,7 +1023,6 @@ NestedIterators: - Domain#self.to_csv - Epp::Domain#nameservers_from - LegalDocument#self.remove_duplicates - - Nameserver#replace_hostname_ends - RegistrantPresenter#domain_names_with_roles - UniquenessMultiValidator#validate_each UnusedParameters: diff --git a/app/models/nameserver.rb b/app/models/nameserver.rb index 4780a5824..d91325ae0 100644 --- a/app/models/nameserver.rb +++ b/app/models/nameserver.rb @@ -73,37 +73,6 @@ class Nameserver < ActiveRecord::Base end class << self - def replace_hostname_ends(domains, old_end, new_end) - domains = domains.where('EXISTS( - select 1 from nameservers ns where ns.domain_id = domains.id AND ns.hostname LIKE ? - )', "%#{old_end}") - - count, success_count = 0.0, 0.0 - domains.each do |d| - ns_attrs = { nameservers_attributes: [] } - - d.nameservers.each do |ns| - next unless ns.hostname.end_with?(old_end) - - hn = ns.hostname.chomp(old_end) - ns_attrs[:nameservers_attributes] << { - id: ns.id, - hostname: "#{hn}#{new_end}" - } - end - - success_count += 1 if d.update(ns_attrs) - count += 1 - end - - return 'replaced_none' if count == 0.0 - - prc = success_count / count - - return 'replaced_all' if prc == 1.0 - 'replaced_some' - end - def find_by_hash_params params params = params.with_indifferent_access rel = all diff --git a/spec/models/nameserver_spec.rb b/spec/models/nameserver_spec.rb index b7cd7a6be..6cc48d168 100644 --- a/spec/models/nameserver_spec.rb +++ b/spec/models/nameserver_spec.rb @@ -81,33 +81,6 @@ describe Nameserver do create(:nameserver, hostname: 'ns3.test.ee') ]) end - - it 'should replace hostname ends' do - res = Nameserver.replace_hostname_ends(@api_user.registrar.domains, 'ns.ee', 'test.ee') - res.should == 'replaced_some' - - @api_user.registrar.nameservers.where("hostname LIKE '%test.ee'").count.should == 4 - @domain_1.nameservers.pluck(:hostname).should include('ns1.ns.ee', 'ns2.ns.ee', 'ns2.test.ee') - @domain_2.nameservers.pluck(:hostname).should include('ns1.test.ee', 'ns2.test.ee', 'ns3.test.ee') - - res = Nameserver.replace_hostname_ends(@api_user.registrar.domains, 'test.ee', 'testing.ee') - res.should == 'replaced_all' - - @api_user.registrar.nameservers.where("hostname LIKE '%testing.ee'").count.should == 4 - @domain_1.nameservers.pluck(:hostname).should include('ns1.ns.ee', 'ns2.ns.ee', 'ns2.testing.ee') - @domain_2.nameservers.pluck(:hostname).should include('ns1.testing.ee', 'ns2.testing.ee', 'ns3.testing.ee') - - res = Nameserver.replace_hostname_ends(@api_user.registrar.domains, 'ns.ee', 'test.ee') - res.should == 'replaced_all' - - @api_user.registrar.nameservers.where("hostname LIKE '%test.ee'").count.should == 2 - @domain_1.nameservers.pluck(:hostname).should include('ns1.test.ee', 'ns2.test.ee', 'ns2.testing.ee') - @domain_2.nameservers.pluck(:hostname).should include('ns1.testing.ee', 'ns2.testing.ee', 'ns3.testing.ee') - @domain_3.nameservers.pluck(:hostname).should include('ns1.ns.ee', 'ns2.ns.ee', 'ns3.test.ee') - - res = Nameserver.replace_hostname_ends(@api_user.registrar.domains, 'xcv.ee', 'test.ee') - res.should == 'replaced_none' - end end end end From 871d2c313f02716bed9b4a11aa9588a4163fb2ab Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Mon, 29 Jan 2018 18:40:15 +0200 Subject: [PATCH 05/34] Update REST API documentation [ci skip] #661 --- doc/repp-doc.md | 3 ++- doc/repp/v1/nameservers.md | 50 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 doc/repp/v1/nameservers.md diff --git a/doc/repp-doc.md b/doc/repp-doc.md index 8eab257ab..0ba0ec548 100644 --- a/doc/repp-doc.md +++ b/doc/repp-doc.md @@ -17,4 +17,5 @@ Main communication specification through Restful EPP (REPP): [Contact related functions](repp/v1/contact.md) [Domain related functions](repp/v1/domain.md) -[Account related functions](repp/v1/account.md) +[Account related functions](repp/v1/account.md) +[Nameservers](repp/v1/nameservers.md) diff --git a/doc/repp/v1/nameservers.md b/doc/repp/v1/nameservers.md new file mode 100644 index 000000000..1f47dc312 --- /dev/null +++ b/doc/repp/v1/nameservers.md @@ -0,0 +1,50 @@ +# Nameservers + +## PUT /repp/v1/nameservers +Replaces nameservers + +#### Request +``` +PUT /repp/v1/nameservers +Accept: application/json +Content-Type: application/json +Authorization: Basic dGVzdDp0ZXN0dGVzdA== + +{ + "data":{ + "nameservers":[ + { + "hostname":"ns1.example.com", + "ipv4":"192.0.2.1" + "ipv6":"2001:DB8::1" + }, + { + "hostname":"ns2.example.com", + "ipv4":"192.0.2.1" + "ipv6":"2001:DB8::1" + } + ] + } +} +``` + +#### Response on success +``` +HTTP/1.1 204 +``` + +#### Response on failure +``` +HTTP/1.1 400 +Content-Type: application/json +{ + "errors":[ + { + "title":"ns1.example.com does not exist" + }, + { + "title":"192.0.2.1 is not a valid IPv4 address" + } + ] +} +``` From 00041df311906f17cff6e825008e9c8df31c1fad Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 30 Jan 2018 01:31:48 +0200 Subject: [PATCH 06/34] Require attribute #660 --- app/models/nameserver.rb | 2 +- ..._change_nameservers_domain_id_to_not_null.rb | 5 +++++ db/structure.sql | 6 ++++-- test/fixtures/nameservers.yml | 3 +++ test/models/nameserver_test.rb | 17 +++++++++++++++++ 5 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20180129232054_change_nameservers_domain_id_to_not_null.rb create mode 100644 test/fixtures/nameservers.yml create mode 100644 test/models/nameserver_test.rb diff --git a/app/models/nameserver.rb b/app/models/nameserver.rb index d91325ae0..9d57501f4 100644 --- a/app/models/nameserver.rb +++ b/app/models/nameserver.rb @@ -2,7 +2,7 @@ class Nameserver < ActiveRecord::Base include Versions # version/nameserver_version.rb include EppErrors - belongs_to :domain + belongs_to :domain, required: true # rubocop: disable Metrics/LineLength validates :hostname, format: { with: /\A(([a-zA-Z0-9]|[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9][a-zA-ZäöüõšžÄÖÜÕŠŽ0-9\-]*[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\z/ } diff --git a/db/migrate/20180129232054_change_nameservers_domain_id_to_not_null.rb b/db/migrate/20180129232054_change_nameservers_domain_id_to_not_null.rb new file mode 100644 index 000000000..3f2abb42f --- /dev/null +++ b/db/migrate/20180129232054_change_nameservers_domain_id_to_not_null.rb @@ -0,0 +1,5 @@ +class ChangeNameserversDomainIdToNotNull < ActiveRecord::Migration + def change + change_column_null :nameservers, :domain_id, false + end +end diff --git a/db/structure.sql b/db/structure.sql index ad9464820..435c2cefe 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -2226,7 +2226,7 @@ CREATE TABLE nameservers ( created_at timestamp without time zone, updated_at timestamp without time zone, ipv6 character varying[] DEFAULT '{}'::character varying[], - domain_id integer, + domain_id integer NOT NULL, creator_str character varying, updator_str character varying, legacy_domain_id integer, @@ -3628,7 +3628,7 @@ ALTER TABLE ONLY settings -- --- Name: unique_contact_code; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: +-- Name: unique_contact_code; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: -- ALTER TABLE ONLY contacts @@ -5080,3 +5080,5 @@ INSERT INTO schema_migrations (version) VALUES ('20180126104903'); INSERT INTO schema_migrations (version) VALUES ('20180129143538'); +INSERT INTO schema_migrations (version) VALUES ('20180129232054'); + diff --git a/test/fixtures/nameservers.yml b/test/fixtures/nameservers.yml new file mode 100644 index 000000000..4138be4e4 --- /dev/null +++ b/test/fixtures/nameservers.yml @@ -0,0 +1,3 @@ +ns1: + hostname: ns1.bestnames.test + domain: shop diff --git a/test/models/nameserver_test.rb b/test/models/nameserver_test.rb new file mode 100644 index 000000000..ce396142f --- /dev/null +++ b/test/models/nameserver_test.rb @@ -0,0 +1,17 @@ +require 'test_helper' + +class NameserverTest < ActiveSupport::TestCase + def setup + @nameserver = nameservers(:ns1) + end + + def test_valid + assert @nameserver.valid? + end + + def test_invalid_without_domain + @nameserver.domain = nil + @nameserver.validate + assert @nameserver.invalid? + end +end From 80327d34471adeede62f339964248e7c7a0200c0 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 30 Jan 2018 01:37:31 +0200 Subject: [PATCH 07/34] Require attribute #660 --- app/models/nameserver.rb | 4 +++- ...0180129233223_change_nameservers_hostname_to_not_null.rb | 5 +++++ db/structure.sql | 4 +++- test/models/nameserver_test.rb | 6 ++++++ 4 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20180129233223_change_nameservers_hostname_to_not_null.rb diff --git a/app/models/nameserver.rb b/app/models/nameserver.rb index 9d57501f4..6c990dc8e 100644 --- a/app/models/nameserver.rb +++ b/app/models/nameserver.rb @@ -5,7 +5,7 @@ class Nameserver < ActiveRecord::Base belongs_to :domain, required: true # rubocop: disable Metrics/LineLength - validates :hostname, format: { with: /\A(([a-zA-Z0-9]|[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9][a-zA-ZäöüõšžÄÖÜÕŠŽ0-9\-]*[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\z/ } + validates :hostname, presence: true, format: { with: /\A(([a-zA-Z0-9]|[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9][a-zA-ZäöüõšžÄÖÜÕŠŽ0-9\-]*[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\z/ } validate :val_ipv4 validate :val_ipv6 # rubocop: enable Metrics/LineLength @@ -40,6 +40,8 @@ class Nameserver < ActiveRecord::Base end def check_label_length + return unless hostname + hostname_puny.split('.').each do |label| errors.add(:hostname, :puny_to_long) if label.length > 63 end diff --git a/db/migrate/20180129233223_change_nameservers_hostname_to_not_null.rb b/db/migrate/20180129233223_change_nameservers_hostname_to_not_null.rb new file mode 100644 index 000000000..d665abcab --- /dev/null +++ b/db/migrate/20180129233223_change_nameservers_hostname_to_not_null.rb @@ -0,0 +1,5 @@ +class ChangeNameserversHostnameToNotNull < ActiveRecord::Migration + def change + change_column_null :nameservers, :hostname, false + end +end diff --git a/db/structure.sql b/db/structure.sql index 435c2cefe..52f22c900 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -2221,7 +2221,7 @@ ALTER SEQUENCE messages_id_seq OWNED BY messages.id; CREATE TABLE nameservers ( id integer NOT NULL, - hostname character varying, + hostname character varying NOT NULL, ipv4 character varying[] DEFAULT '{}'::character varying[], created_at timestamp without time zone, updated_at timestamp without time zone, @@ -5082,3 +5082,5 @@ INSERT INTO schema_migrations (version) VALUES ('20180129143538'); INSERT INTO schema_migrations (version) VALUES ('20180129232054'); +INSERT INTO schema_migrations (version) VALUES ('20180129233223'); + diff --git a/test/models/nameserver_test.rb b/test/models/nameserver_test.rb index ce396142f..0d5663eb1 100644 --- a/test/models/nameserver_test.rb +++ b/test/models/nameserver_test.rb @@ -14,4 +14,10 @@ class NameserverTest < ActiveSupport::TestCase @nameserver.validate assert @nameserver.invalid? end + + def test_invalid_without_hostname + @nameserver.hostname = nil + @nameserver.validate + assert @nameserver.invalid? + end end From a8dd176cf6799f80a774ea413a157db2d7b51f6c Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Wed, 31 Jan 2018 22:38:41 +0200 Subject: [PATCH 08/34] Remove useless tests #661 --- spec/models/nameserver_spec.rb | 85 ---------------------------------- 1 file changed, 85 deletions(-) diff --git a/spec/models/nameserver_spec.rb b/spec/models/nameserver_spec.rb index 6cc48d168..d65c6c16d 100644 --- a/spec/models/nameserver_spec.rb +++ b/spec/models/nameserver_spec.rb @@ -1,90 +1,5 @@ require 'rails_helper' -describe Nameserver do - before :example do - Setting.ds_algorithm = 2 - Setting.ds_data_allowed = true - Setting.ds_data_with_key_allowed = true - Setting.key_data_allowed = true - - Setting.dnskeys_min_count = 0 - Setting.dnskeys_max_count = 9 - Setting.ns_min_count = 2 - Setting.ns_max_count = 11 - - Setting.transfer_wait_time = 0 - - Setting.admin_contacts_min_count = 1 - Setting.admin_contacts_max_count = 10 - Setting.tech_contacts_min_count = 0 - Setting.tech_contacts_max_count = 10 - - Setting.client_side_status_editing_enabled = true - - create(:zone, origin: 'ee') - end - - context 'with invalid attribute' do - before :example do - @nameserver = Nameserver.new - end - - it 'should not have any versions' do - @nameserver.versions.should == [] - end - end - - context 'with valid attributes' do - before :example do - @nameserver = create(:nameserver) - end - - it 'should be valid' do - @nameserver.valid? - @nameserver.errors.full_messages.should match_array([]) - end - - it 'should be valid twice' do - @nameserver = create(:nameserver) - @nameserver.valid? - @nameserver.errors.full_messages.should match_array([]) - end - - it 'should have one version' do - with_versioning do - @nameserver.versions.should == [] - @nameserver.hostname = 'hostname.ee' - @nameserver.save - @nameserver.errors.full_messages.should match_array([]) - @nameserver.versions.size.should == 1 - end - end - - context 'with many nameservers' do - before :example do - @api_user = create(:api_user) - @domain_1 = create(:domain, nameservers: [ - create(:nameserver, hostname: 'ns1.ns.ee'), - create(:nameserver, hostname: 'ns2.ns.ee'), - create(:nameserver, hostname: 'ns2.test.ee') - ], registrar: @api_user.registrar) - - @domain_2 = create(:domain, nameservers: [ - create(:nameserver, hostname: 'ns1.ns.ee'), - create(:nameserver, hostname: 'ns2.ns.ee'), - create(:nameserver, hostname: 'ns3.test.ee') - ], registrar: @api_user.registrar) - - @domain_3 = create(:domain, nameservers: [ - create(:nameserver, hostname: 'ns1.ns.ee'), - create(:nameserver, hostname: 'ns2.ns.ee'), - create(:nameserver, hostname: 'ns3.test.ee') - ]) - end - end - end -end - RSpec.describe Nameserver do describe '::hostnames', db: false do before :example do From 3ef38768a56513d88fdc0ead4d73920bff2dbacd Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Wed, 31 Jan 2018 22:48:00 +0200 Subject: [PATCH 09/34] Convert spec to test #661 --- spec/models/nameserver_spec.rb | 13 ------------- test/fixtures/nameservers.yml | 12 ++++++++++++ test/models/nameserver_test.rb | 4 ++++ 3 files changed, 16 insertions(+), 13 deletions(-) delete mode 100644 spec/models/nameserver_spec.rb diff --git a/spec/models/nameserver_spec.rb b/spec/models/nameserver_spec.rb deleted file mode 100644 index d65c6c16d..000000000 --- a/spec/models/nameserver_spec.rb +++ /dev/null @@ -1,13 +0,0 @@ -require 'rails_helper' - -RSpec.describe Nameserver do - describe '::hostnames', db: false do - before :example do - expect(described_class).to receive(:pluck).with(:hostname).and_return('hostnames') - end - - it 'returns names' do - expect(described_class.hostnames).to eq('hostnames') - end - end -end diff --git a/test/fixtures/nameservers.yml b/test/fixtures/nameservers.yml index 4138be4e4..a7d0e066a 100644 --- a/test/fixtures/nameservers.yml +++ b/test/fixtures/nameservers.yml @@ -1,3 +1,15 @@ ns1: hostname: ns1.bestnames.test + ipv4: + - 192.0.2.1 + ipv6: + - 2001:DB8::1 + domain: shop + +ns2: + hostname: ns2.bestnames.test + ipv4: + - 192.0.2.2 + ipv6: + - 2001:DB8::2 domain: shop diff --git a/test/models/nameserver_test.rb b/test/models/nameserver_test.rb index 0d5663eb1..343190b14 100644 --- a/test/models/nameserver_test.rb +++ b/test/models/nameserver_test.rb @@ -20,4 +20,8 @@ class NameserverTest < ActiveSupport::TestCase @nameserver.validate assert @nameserver.invalid? end + + def test_hostnames + assert_equal %w[ns1.bestnames.test ns2.bestnames.test], Nameserver.hostnames + end end From a5eaae562a3719d9af21fb616af942c65fe587d7 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Thu, 1 Feb 2018 01:35:32 +0200 Subject: [PATCH 10/34] Add nameservers REST API #661 --- app/api/repp/api.rb | 1 + app/api/repp/nameservers_v1.rb | 30 +++++++++++++++++++ test/fixtures/nameservers.yml | 4 +-- .../api/nameservers/put_nameservers_test.rb | 29 ++++++++++++++++++ 4 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 app/api/repp/nameservers_v1.rb create mode 100644 test/integration/api/nameservers/put_nameservers_test.rb diff --git a/app/api/repp/api.rb b/app/api/repp/api.rb index aec46c68b..9c12470a0 100644 --- a/app/api/repp/api.rb +++ b/app/api/repp/api.rb @@ -58,5 +58,6 @@ module Repp mount Repp::ContactV1 mount Repp::AccountV1 mount Repp::DomainTransfersV1 + mount Repp::NameserversV1 end end diff --git a/app/api/repp/nameservers_v1.rb b/app/api/repp/nameservers_v1.rb new file mode 100644 index 000000000..10e9930f3 --- /dev/null +++ b/app/api/repp/nameservers_v1.rb @@ -0,0 +1,30 @@ +module Repp + class NameserversV1 < Grape::API + version 'v1', using: :path + + resource :nameservers do + put '/' do + params do + requires :data, type: Hash do + requires :type, type: String, allow_blank: false + requires :id, type: String, allow_blank: false + requires :attributes, type: Hash do + requires :hostname, type: String, allow_blank: false + end + end + end + + current_user.registrar.nameservers.where(hostname: params[:data][:id]).each do |nameserver| + nameserver.hostname = params[:data][:attributes][:hostname] + nameserver.ipv4 = params[:data][:attributes][:ipv4] + nameserver.ipv6 = params[:data][:attributes][:ipv6] + nameserver.save! + end + + status 204 + body false + @response = {} + end + end + end +end diff --git a/test/fixtures/nameservers.yml b/test/fixtures/nameservers.yml index a7d0e066a..870e678ad 100644 --- a/test/fixtures/nameservers.yml +++ b/test/fixtures/nameservers.yml @@ -3,7 +3,7 @@ ns1: ipv4: - 192.0.2.1 ipv6: - - 2001:DB8::1 + - 2001:db8::1 domain: shop ns2: @@ -11,5 +11,5 @@ ns2: ipv4: - 192.0.2.2 ipv6: - - 2001:DB8::2 + - 2001:db8::2 domain: shop diff --git a/test/integration/api/nameservers/put_nameservers_test.rb b/test/integration/api/nameservers/put_nameservers_test.rb new file mode 100644 index 000000000..e21eae1cc --- /dev/null +++ b/test/integration/api/nameservers/put_nameservers_test.rb @@ -0,0 +1,29 @@ +require 'test_helper' + +class APIPutNameserversTest < ActionDispatch::IntegrationTest + def test_changes_nameservers_of_all_domains_of_current_registrar + ns2 = domains(:shop).nameservers.find_by(hostname: 'ns2.bestnames.test') + request_params = { format: :json, data: { type: 'nameservers', id: 'ns2.bestnames.test', + attributes: { hostname: 'ns3.bestnames.test', + ipv4: ['192.0.2.3'], + ipv6: ['2001:DB8::3'] } } } + put '/repp/v1/nameservers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } + ns2.reload + assert_equal 'ns3.bestnames.test', ns2.hostname + assert_equal ['192.0.2.3'], ns2.ipv4 + assert_equal ['2001:DB8::3'], ns2.ipv6 + assert_response 204 + assert_empty response.body + end + + def test_unauthenticated + put '/repp/v1/nameservers' + assert_response 401 + end + + private + + def http_auth_key + ActionController::HttpAuthentication::Basic.encode_credentials('test_bestnames', 'testtest') + end +end From 44e3314b76c0a9d0c19cccbe528b11acf0a1501d Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 20 Feb 2018 04:35:25 +0200 Subject: [PATCH 11/34] Fix invalid factory #661 --- spec/factories/nameserver.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/factories/nameserver.rb b/spec/factories/nameserver.rb index edae45597..ee0a7f987 100644 --- a/spec/factories/nameserver.rb +++ b/spec/factories/nameserver.rb @@ -2,5 +2,6 @@ FactoryBot.define do factory :nameserver do sequence(:hostname) { |n| "ns.test#{n}.ee" } ipv4 '192.168.1.1' + domain end end From b7ff3d1f84d58758abddb44a47e88e6556642487 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Wed, 21 Feb 2018 09:40:40 +0200 Subject: [PATCH 12/34] Improve EPP spec [skip ci] #661 --- doc/epp/domain.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/epp/domain.md b/doc/epp/domain.md index a9c531f7d..0933efe4b 100644 --- a/doc/epp/domain.md +++ b/doc/epp/domain.md @@ -22,7 +22,7 @@ Domain name mapping protocol short version: 0-1 2-11 1 Hostname of the nameserver - 0-2 Required if nameserver is under domain zone. + 0-2 Required if nameserver hostname is under the same domain. Attribute ip="v4 / v6" 1 Contact reference to the registrant 0-n Contact reference. Admin contact is required if registrant is @@ -62,7 +62,7 @@ Domain name mapping protocol short version: 0-1 1 1 Hostname of the nameserver - 0-2 Required if nameserver is under domain zone. + 0-2 Required if nameserver hostname is under the same domain. Attribute ip="v4 / v6" 0-1 Objects to remove 0-n Contact reference. Attribute: type="admin / tech" From ee63cf5a84c8146e503b3a57f3067a678423f7d8 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Wed, 21 Feb 2018 10:03:23 +0200 Subject: [PATCH 13/34] Add test #661 --- .../epp/domain/create/nameservers_test.rb | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 test/integration/epp/domain/create/nameservers_test.rb diff --git a/test/integration/epp/domain/create/nameservers_test.rb b/test/integration/epp/domain/create/nameservers_test.rb new file mode 100644 index 000000000..ddaddf2d1 --- /dev/null +++ b/test/integration/epp/domain/create/nameservers_test.rb @@ -0,0 +1,35 @@ +require 'test_helper' + +class EppDomainCreateNameserversTest < ActionDispatch::IntegrationTest + # Glue record requirement + def test_nameserver_ip_address_is_required_if_hostname_is_under_the_same_domain + request_xml = <<-XML + + + + + + new.test + + + ns1.new.test + + + john-001 + + + + + test + + + + + XML + + assert_no_difference 'Domain.count' do + post '/epp/command/create', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=api_bestnames' } + end + assert_equal '2003', Nokogiri::XML(response.body).at_css('result')[:code] + end +end From b0ef967ff80c3806c260cf1788b0a89e21611a4c Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Thu, 22 Feb 2018 07:35:27 +0200 Subject: [PATCH 14/34] Remove duplicated translation #661 --- config/locales/en.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 8b90fda73..2787dd36c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -489,7 +489,6 @@ en: name: 'Name' type: 'Type' code: 'Code' - nameservers: 'Nameservers' dnskeys: 'DNS Keys' flag: 'Flag' protocol: 'Protocol' From 1b22e17a94135dc6e27f9f7f815a329ab1c8ddee Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Thu, 22 Feb 2018 08:57:20 +0200 Subject: [PATCH 15/34] Add glue record requirement validation #661 --- app/models/nameserver.rb | 12 ++++++++++ config/locales/nameservers.en.yml | 8 +++++++ test/models/nameserver/glue_record_test.rb | 26 ++++++++++++++++++++++ 3 files changed, 46 insertions(+) create mode 100644 config/locales/nameservers.en.yml create mode 100644 test/models/nameserver/glue_record_test.rb diff --git a/app/models/nameserver.rb b/app/models/nameserver.rb index 6c990dc8e..849a026e4 100644 --- a/app/models/nameserver.rb +++ b/app/models/nameserver.rb @@ -8,6 +8,7 @@ class Nameserver < ActiveRecord::Base validates :hostname, presence: true, format: { with: /\A(([a-zA-Z0-9]|[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9][a-zA-ZäöüõšžÄÖÜÕŠŽ0-9\-]*[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\z/ } validate :val_ipv4 validate :val_ipv6 + validate :require_ip, if: :glue_record_required? # rubocop: enable Metrics/LineLength before_validation :normalize_attributes @@ -86,4 +87,15 @@ class Nameserver < ActiveRecord::Base pluck(:hostname) end end + + private + + def require_ip + errors.add(:base, :ip_required) if ipv4.blank? && ipv6.blank? + end + + def glue_record_required? + return unless hostname? && domain + hostname.end_with?(domain.name) + end end diff --git a/config/locales/nameservers.en.yml b/config/locales/nameservers.en.yml new file mode 100644 index 000000000..82a6d045e --- /dev/null +++ b/config/locales/nameservers.en.yml @@ -0,0 +1,8 @@ +en: + activemodel: + errors: + models: + nameserver: + attributes: + base: + ip_required: At least one IPv4 or IPv6 is required diff --git a/test/models/nameserver/glue_record_test.rb b/test/models/nameserver/glue_record_test.rb new file mode 100644 index 000000000..5f162cb2b --- /dev/null +++ b/test/models/nameserver/glue_record_test.rb @@ -0,0 +1,26 @@ +require 'test_helper' + +class NameserverGlueRecordTest < ActiveSupport::TestCase + def setup + @nameserver = nameservers(:ns1) + end + + def test_invalid_without_ip_if_glue_record_is_required + @nameserver.hostname = 'ns1.shop.test' + @nameserver.ipv4 = @nameserver.ipv6 = '' + assert @nameserver.invalid? + assert @nameserver.errors.added?(:base, :ip_required) + end + + def test_valid_with_ip_if_glue_record_is_required + @nameserver.hostname = 'ns1.shop.test' + @nameserver.ipv4 = ['192.0.2.1'] + @nameserver.ipv6 = '' + assert @nameserver.valid? + end + + def test_valid_without_ip_if_glue_record_is_not_required + @nameserver.ipv4 = @nameserver.ipv6 = '' + assert @nameserver.valid? + end +end From ea2beebde359cf251efdd4cc70bd9df44d581be2 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Thu, 22 Feb 2018 08:58:29 +0200 Subject: [PATCH 16/34] Improve test #661 --- test/models/nameserver_test.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/models/nameserver_test.rb b/test/models/nameserver_test.rb index 343190b14..5e12efde3 100644 --- a/test/models/nameserver_test.rb +++ b/test/models/nameserver_test.rb @@ -11,13 +11,11 @@ class NameserverTest < ActiveSupport::TestCase def test_invalid_without_domain @nameserver.domain = nil - @nameserver.validate assert @nameserver.invalid? end def test_invalid_without_hostname - @nameserver.hostname = nil - @nameserver.validate + @nameserver.hostname = '' assert @nameserver.invalid? end From 42e14cd8920bbf713a5e15869ae574cdd749d049 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Thu, 22 Feb 2018 09:14:12 +0200 Subject: [PATCH 17/34] Add tests #661 --- test/models/nameserver_test.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/models/nameserver_test.rb b/test/models/nameserver_test.rb index 5e12efde3..377ff446a 100644 --- a/test/models/nameserver_test.rb +++ b/test/models/nameserver_test.rb @@ -22,4 +22,22 @@ class NameserverTest < ActiveSupport::TestCase def test_hostnames assert_equal %w[ns1.bestnames.test ns2.bestnames.test], Nameserver.hostnames end + + def test_normalizes_hostname + @nameserver.hostname = ' ns1.bestnameS.test.' + @nameserver.validate + assert_equal 'ns1.bestnames.test', @nameserver.hostname + end + + def test_normalizes_ipv4 + @nameserver.ipv4 = [' 192.0.2.1'] + @nameserver.validate + assert_equal ['192.0.2.1'], @nameserver.ipv4 + end + + def test_normalizes_ipv6 + @nameserver.ipv6 = [' 2001:db8::1'] + @nameserver.validate + assert_equal ['2001:DB8::1'], @nameserver.ipv6 + end end From 1fe520261f1cc9120694f680c6ec38a333284d86 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Thu, 22 Feb 2018 09:14:33 +0200 Subject: [PATCH 18/34] Hide methods #661 --- app/models/nameserver.rb | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/app/models/nameserver.rb b/app/models/nameserver.rb index 849a026e4..1880bd091 100644 --- a/app/models/nameserver.rb +++ b/app/models/nameserver.rb @@ -34,25 +34,6 @@ class Nameserver < ActiveRecord::Base } end - def normalize_attributes - self.hostname = hostname.try(:strip).try(:downcase) - self.ipv4 = Array(ipv4).reject(&:blank?).map(&:strip) - self.ipv6 = Array(ipv6).reject(&:blank?).map(&:strip).map(&:upcase) - end - - def check_label_length - return unless hostname - - hostname_puny.split('.').each do |label| - errors.add(:hostname, :puny_to_long) if label.length > 63 - end - end - - def check_puny_symbols - regexp = /(\A|\.)..--/ - errors.add(:hostname, :invalid) if hostname =~ regexp - end - def to_s hostname end @@ -98,4 +79,23 @@ class Nameserver < ActiveRecord::Base return unless hostname? && domain hostname.end_with?(domain.name) end + + def normalize_attributes + self.hostname = hostname.try(:strip).try(:downcase) + self.ipv4 = Array(ipv4).reject(&:blank?).map(&:strip) + self.ipv6 = Array(ipv6).reject(&:blank?).map(&:strip).map(&:upcase) + end + + def check_label_length + return unless hostname + + hostname_puny.split('.').each do |label| + errors.add(:hostname, :puny_to_long) if label.length > 63 + end + end + + def check_puny_symbols + regexp = /(\A|\.)..--/ + errors.add(:hostname, :invalid) if hostname =~ regexp + end end From 69c32546de96f380c357105a01b9654c903470c1 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Thu, 22 Feb 2018 12:44:24 +0200 Subject: [PATCH 19/34] Add name server API #661 --- app/api/repp/nameservers_v1.rb | 25 +++++--- app/models/registrar.rb | 10 +++ doc/repp/v1/nameservers.md | 60 ++++++++++-------- .../integration/api/nameservers/patch_test.rb | 61 +++++++++++++++++++ .../api/nameservers/put_nameservers_test.rb | 29 --------- 5 files changed, 120 insertions(+), 65 deletions(-) create mode 100644 test/integration/api/nameservers/patch_test.rb delete mode 100644 test/integration/api/nameservers/put_nameservers_test.rb diff --git a/app/api/repp/nameservers_v1.rb b/app/api/repp/nameservers_v1.rb index 10e9930f3..fe391b704 100644 --- a/app/api/repp/nameservers_v1.rb +++ b/app/api/repp/nameservers_v1.rb @@ -10,20 +10,27 @@ module Repp requires :id, type: String, allow_blank: false requires :attributes, type: Hash do requires :hostname, type: String, allow_blank: false + requires :ipv4, type: Array + requires :ipv6, type: Array end end end - current_user.registrar.nameservers.where(hostname: params[:data][:id]).each do |nameserver| - nameserver.hostname = params[:data][:attributes][:hostname] - nameserver.ipv4 = params[:data][:attributes][:ipv4] - nameserver.ipv6 = params[:data][:attributes][:ipv6] - nameserver.save! - end + old_nameserver = current_user.registrar.nameservers.find_by(hostname: params[:data][:id]) + error!({ errors: [{ title: "Hostname #{params[:data][:id]} does not exist" }] }, 404) unless old_nameserver - status 204 - body false - @response = {} + new_nameserver = old_nameserver.dup + new_nameserver.hostname = params[:data][:attributes][:hostname] + new_nameserver.ipv4 = params[:data][:attributes][:ipv4] + new_nameserver.ipv6 = params[:data][:attributes][:ipv6] + + error!({ errors: [{ title: 'Invalid params' }] }, 400) unless new_nameserver.valid? + + current_user.registrar.replace_nameserver(old_nameserver, new_nameserver) + + status 200 + @response = { data: { type: 'nameserver', + id: new_nameserver.hostname, attributes: params[:data][:attributes] } } end end end diff --git a/app/models/registrar.rb b/app/models/registrar.rb index 7646b2ceb..299813dbe 100644 --- a/app/models/registrar.rb +++ b/app/models/registrar.rb @@ -167,6 +167,16 @@ class Registrar < ActiveRecord::Base white_ips.api.pluck(:ipv4, :ipv6).flatten.include?(ip) end + def replace_nameserver(old_nameserver, new_nameserver) + transaction do + nameservers.where(hostname: old_nameserver.hostname).find_each do |nameserver| + nameserver.update!(hostname: new_nameserver.hostname, + ipv4: new_nameserver.ipv4, + ipv6: new_nameserver.ipv6) # Audit log is needed, therefore no raw SQL + end + end + end + private def set_defaults diff --git a/doc/repp/v1/nameservers.md b/doc/repp/v1/nameservers.md index 1f47dc312..9c3fc9561 100644 --- a/doc/repp/v1/nameservers.md +++ b/doc/repp/v1/nameservers.md @@ -1,36 +1,42 @@ # Nameservers -## PUT /repp/v1/nameservers -Replaces nameservers +## PATCH /repp/v1/nameservers +Replaces all name servers of current registrar domains. #### Request ``` -PUT /repp/v1/nameservers +PATCH /repp/v1/nameservers Accept: application/json Content-Type: application/json Authorization: Basic dGVzdDp0ZXN0dGVzdA== - { - "data":{ - "nameservers":[ - { - "hostname":"ns1.example.com", - "ipv4":"192.0.2.1" - "ipv6":"2001:DB8::1" - }, - { - "hostname":"ns2.example.com", - "ipv4":"192.0.2.1" - "ipv6":"2001:DB8::1" - } - ] - } + "data":{ + "type": "nameserver", + "id": "ns1.example.com", + "attributes": { + "hostname": "new-ns1.example.com", + "ipv4": ["192.0.2.1", "192.0.2.2"], + "ipv6": ["2001:db8::1", "2001:db8::2"] + }, + } } ``` #### Response on success ``` -HTTP/1.1 204 +HTTP/1.1 200 +Content-Type: application/json +{ + "data":{ + "type": "nameserver", + "id": "new-ns1.example.com", + "attributes": { + "hostname": "new-ns1.example.com", + "ipv4": ["192.0.2.1", "192.0.2.2"], + "ipv6": ["2001:db8::1", "2001:db8::2"] + }, + } +} ``` #### Response on failure @@ -38,13 +44,13 @@ HTTP/1.1 204 HTTP/1.1 400 Content-Type: application/json { - "errors":[ - { - "title":"ns1.example.com does not exist" - }, - { - "title":"192.0.2.1 is not a valid IPv4 address" - } - ] + "errors":[ + { + "title":"ns1.example.com does not exist" + }, + { + "title":"192.0.2.1 is not a valid IPv4 address" + } + ] } ``` diff --git a/test/integration/api/nameservers/patch_test.rb b/test/integration/api/nameservers/patch_test.rb new file mode 100644 index 000000000..1633cc9d1 --- /dev/null +++ b/test/integration/api/nameservers/patch_test.rb @@ -0,0 +1,61 @@ +require 'test_helper' + +class APINameserversPatchTest < ActionDispatch::IntegrationTest + def test_replaces_current_registrar_nameservers + request_params = { format: :json, data: { type: 'nameserver', id: 'ns1.bestnames.test', + attributes: { hostname: 'ns55.bestnames.test', + ipv4: ['192.0.2.55'], + ipv6: ['2001:db8::55'] } } } + put '/repp/v1/nameservers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } + + new_nameserver = registrars(:bestnames).nameservers.find_by(hostname: 'ns55.bestnames.test') + assert_nil registrars(:bestnames).nameservers.find_by(hostname: 'ns1.bestnames.test') + assert_equal ['192.0.2.55'], new_nameserver.ipv4 + assert_equal ['2001:DB8::55'], new_nameserver.ipv6 + assert_response 200 + assert_equal ({ data: { type: 'nameserver', + id: 'ns55.bestnames.test', + attributes: { hostname: 'ns55.bestnames.test', + ipv4: ['192.0.2.55'], + ipv6: ['2001:db8::55'] } } }), + JSON.parse(response.body, symbolize_names: true) + end + + def test_honors_optional_params + request_params = { format: :json, data: { type: 'nameserver', id: 'ns1.bestnames.test', + attributes: { hostname: 'ns55.bestnames.test' } } } + put '/repp/v1/nameservers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } + assert_response 200 + end + + def test_non_existent_nameserver_hostname + request_params = { format: :json, data: { type: 'nameserver', id: 'non-existent.test', + attributes: { hostname: 'any.bestnames.test' } } } + put '/repp/v1/nameservers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } + + assert_response 404 + assert_equal ({ errors: [{ title: 'Hostname non-existent.test does not exist' }] }), + JSON.parse(response.body, symbolize_names: true) + end + + def test_invalid_request_params + request_params = { format: :json, data: { type: 'nameserver', id: 'ns1.bestnames.test', + attributes: { hostname: '' } } } + put '/repp/v1/nameservers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } + + assert_response 400 + assert_equal ({ errors: [{ title: 'Invalid params' }] }), + JSON.parse(response.body, symbolize_names: true) + end + + def test_unauthenticated + put '/repp/v1/nameservers' + assert_response 401 + end + + private + + def http_auth_key + ActionController::HttpAuthentication::Basic.encode_credentials('test_bestnames', 'testtest') + end +end diff --git a/test/integration/api/nameservers/put_nameservers_test.rb b/test/integration/api/nameservers/put_nameservers_test.rb deleted file mode 100644 index e21eae1cc..000000000 --- a/test/integration/api/nameservers/put_nameservers_test.rb +++ /dev/null @@ -1,29 +0,0 @@ -require 'test_helper' - -class APIPutNameserversTest < ActionDispatch::IntegrationTest - def test_changes_nameservers_of_all_domains_of_current_registrar - ns2 = domains(:shop).nameservers.find_by(hostname: 'ns2.bestnames.test') - request_params = { format: :json, data: { type: 'nameservers', id: 'ns2.bestnames.test', - attributes: { hostname: 'ns3.bestnames.test', - ipv4: ['192.0.2.3'], - ipv6: ['2001:DB8::3'] } } } - put '/repp/v1/nameservers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } - ns2.reload - assert_equal 'ns3.bestnames.test', ns2.hostname - assert_equal ['192.0.2.3'], ns2.ipv4 - assert_equal ['2001:DB8::3'], ns2.ipv6 - assert_response 204 - assert_empty response.body - end - - def test_unauthenticated - put '/repp/v1/nameservers' - assert_response 401 - end - - private - - def http_auth_key - ActionController::HttpAuthentication::Basic.encode_credentials('test_bestnames', 'testtest') - end -end From 2db401fd98221b5e8da5760fd20411d9d8825e36 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Mon, 26 Feb 2018 10:30:46 +0200 Subject: [PATCH 20/34] Add registrar nameserver replacement UI #661 --- app/api/repp/nameservers_v1.rb | 2 +- .../registrar_nameservers_controller.rb | 59 +++++++++++++++++++ app/views/registrar/domains/index.html.erb | 6 +- .../registrar_nameservers/_form.html.erb | 50 ++++++++++++++++ .../registrar_nameservers/edit.html.erb | 11 ++++ config/locales/registrar/domains.en.yml | 1 + .../registrar/registrar_nameservers.en.yml | 12 ++++ config/routes.rb | 2 + doc/repp/v1/nameservers.md | 4 +- .../{patch_test.rb => put_test.rb} | 12 ++-- .../registrar/nameserver_replacement_test.rb | 49 +++++++++++++++ 11 files changed, 197 insertions(+), 11 deletions(-) create mode 100644 app/controllers/registrar/registrar_nameservers_controller.rb create mode 100644 app/views/registrar/registrar_nameservers/_form.html.erb create mode 100644 app/views/registrar/registrar_nameservers/edit.html.erb create mode 100644 config/locales/registrar/registrar_nameservers.en.yml rename test/integration/api/nameservers/{patch_test.rb => put_test.rb} (82%) create mode 100644 test/integration/registrar/nameserver_replacement_test.rb diff --git a/app/api/repp/nameservers_v1.rb b/app/api/repp/nameservers_v1.rb index fe391b704..cbc58eece 100644 --- a/app/api/repp/nameservers_v1.rb +++ b/app/api/repp/nameservers_v1.rb @@ -2,7 +2,7 @@ module Repp class NameserversV1 < Grape::API version 'v1', using: :path - resource :nameservers do + resource 'registrar/nameservers' do put '/' do params do requires :data, type: Hash do diff --git a/app/controllers/registrar/registrar_nameservers_controller.rb b/app/controllers/registrar/registrar_nameservers_controller.rb new file mode 100644 index 000000000..b4776f70e --- /dev/null +++ b/app/controllers/registrar/registrar_nameservers_controller.rb @@ -0,0 +1,59 @@ +class Registrar + class RegistrarNameserversController < DeppController + def edit + authorize! :manage, :repp + end + + def update + authorize! :manage, :repp + + ipv4 = params[:ipv4].split("\r\n") + ipv6 = params[:ipv6].split("\r\n") + + uri = URI.parse("#{ENV['repp_url']}registrar/nameservers") + request = Net::HTTP::Post.new(uri, 'Content-Type' => 'application/json') + request.body = { data: { type: 'nameserver', id: params[:old_hostname], + attributes: { hostname: params[:new_hostname], + ipv4: ipv4, + ipv6: ipv6 } } }.to_json + request.basic_auth(current_user.username, current_user.password) + + if Rails.env.test? + response = Net::HTTP.start(uri.hostname, uri.port, + use_ssl: (uri.scheme == 'https'), + verify_mode: OpenSSL::SSL::VERIFY_NONE) do |http| + http.request(request) + end + elsif Rails.env.development? + client_cert = File.read(ENV['cert_path']) + client_key = File.read(ENV['key_path']) + response = Net::HTTP.start(uri.hostname, uri.port, + use_ssl: (uri.scheme == 'https'), + verify_mode: OpenSSL::SSL::VERIFY_NONE, + cert: OpenSSL::X509::Certificate.new(client_cert), + key: OpenSSL::PKey::RSA.new(client_key)) do |http| + http.request(request) + end + else + client_cert = File.read(ENV['cert_path']) + client_key = File.read(ENV['key_path']) + response = Net::HTTP.start(uri.hostname, uri.port, + use_ssl: (uri.scheme == 'https'), + cert: OpenSSL::X509::Certificate.new(client_cert), + key: OpenSSL::PKey::RSA.new(client_key)) do |http| + http.request(request) + end + end + + parsed_response = JSON.parse(response.body, symbolize_names: true) + + if response.code == '200' + flash[:notice] = t '.replaced' + redirect_to registrar_domains_url + else + @api_errors = parsed_response[:errors] + render :edit + end + end + end +end diff --git a/app/views/registrar/domains/index.html.erb b/app/views/registrar/domains/index.html.erb index a62619ca4..319f0d04f 100644 --- a/app/views/registrar/domains/index.html.erb +++ b/app/views/registrar/domains/index.html.erb @@ -1,12 +1,14 @@ @@ -16,7 +16,7 @@
- <%= text_field_tag :new_hostname, nil, required: true, class: 'form-control' %> + <%= text_field_tag :new_hostname, params[:new_hostname], required: true, class: 'form-control' %>
@@ -26,7 +26,7 @@
- <%= text_area_tag :ipv4, nil, class: 'form-control' %> + <%= text_area_tag :ipv4, params[:ipv4], class: 'form-control' %>
@@ -36,7 +36,7 @@
- <%= text_area_tag :ipv6, nil, class: 'form-control' %> + <%= text_area_tag :ipv6, params[:ipv6], class: 'form-control' %>
diff --git a/test/integration/registrar/nameserver_replacement_test.rb b/test/integration/registrar/nameserver_replacement_test.rb index a6c1dcb7b..9523d2bbd 100644 --- a/test/integration/registrar/nameserver_replacement_test.rb +++ b/test/integration/registrar/nameserver_replacement_test.rb @@ -42,8 +42,16 @@ class RegistrarNameserverReplacementTest < ActionDispatch::IntegrationTest visit registrar_domains_url click_link 'Replace nameserver' + fill_in 'Old hostname', with: 'old hostname' + fill_in 'New hostname', with: 'new hostname' + fill_in 'ipv4', with: 'ipv4' + fill_in 'ipv6', with: 'ipv6' click_on 'Replace nameserver' assert_text 'epic fail' + assert_field 'Old hostname', with: 'old hostname' + assert_field 'New hostname', with: 'new hostname' + assert_field 'ipv4', with: 'ipv4' + assert_field 'ipv6', with: 'ipv6' end end From ed1600a787ae25a77f87543252baf75a23d24900 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Mon, 26 Feb 2018 11:35:39 +0200 Subject: [PATCH 23/34] Improve UI #661 --- app/views/registrar/registrar_nameservers/_form.html.erb | 1 + config/locales/registrar/registrar_nameservers.en.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/app/views/registrar/registrar_nameservers/_form.html.erb b/app/views/registrar/registrar_nameservers/_form.html.erb index 7d4935a53..64a3d9074 100644 --- a/app/views/registrar/registrar_nameservers/_form.html.erb +++ b/app/views/registrar/registrar_nameservers/_form.html.erb @@ -37,6 +37,7 @@
<%= text_area_tag :ipv6, params[:ipv6], class: 'form-control' %> + <%= t '.ip_hint' %>
diff --git a/config/locales/registrar/registrar_nameservers.en.yml b/config/locales/registrar/registrar_nameservers.en.yml index 6d783f564..b9e7cb2fa 100644 --- a/config/locales/registrar/registrar_nameservers.en.yml +++ b/config/locales/registrar/registrar_nameservers.en.yml @@ -6,6 +6,7 @@ en: replace_btn: Replace form: + ip_hint: One IP per line replace_btn: Replace nameserver update: From b6a0c04571311ab3ffe9482db5a56c95fc2e4345 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Mon, 26 Feb 2018 13:02:55 +0200 Subject: [PATCH 24/34] Turn spellcheck off #661 --- .../registrar/registrar_nameservers/_form.html.erb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/app/views/registrar/registrar_nameservers/_form.html.erb b/app/views/registrar/registrar_nameservers/_form.html.erb index 64a3d9074..bedb7ca81 100644 --- a/app/views/registrar/registrar_nameservers/_form.html.erb +++ b/app/views/registrar/registrar_nameservers/_form.html.erb @@ -5,7 +5,9 @@
- <%= text_field_tag :old_hostname, params[:old_hostname], autofocus: true, required: true, + <%= text_field_tag :old_hostname, params[:old_hostname], autofocus: true, + required: true, + spellcheck: false, class: 'form-control' %>
@@ -16,7 +18,9 @@
- <%= text_field_tag :new_hostname, params[:new_hostname], required: true, class: 'form-control' %> + <%= text_field_tag :new_hostname, params[:new_hostname], required: true, + spellcheck: false, + class: 'form-control' %>
@@ -26,7 +30,7 @@
- <%= text_area_tag :ipv4, params[:ipv4], class: 'form-control' %> + <%= text_area_tag :ipv4, params[:ipv4], spellcheck: false, class: 'form-control' %>
@@ -36,7 +40,7 @@
- <%= text_area_tag :ipv6, params[:ipv6], class: 'form-control' %> + <%= text_area_tag :ipv6, params[:ipv6], spellcheck: false, class: 'form-control' %> <%= t '.ip_hint' %>
From 1732d0966da4dc295891e2681222cc4a23ccc14e Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Mon, 26 Feb 2018 13:04:02 +0200 Subject: [PATCH 25/34] Remove unneeded param #661 --- app/views/registrar/registrar_nameservers/_form.html.erb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/registrar/registrar_nameservers/_form.html.erb b/app/views/registrar/registrar_nameservers/_form.html.erb index bedb7ca81..cb872a91a 100644 --- a/app/views/registrar/registrar_nameservers/_form.html.erb +++ b/app/views/registrar/registrar_nameservers/_form.html.erb @@ -1,7 +1,7 @@ <%= form_tag registrar_update_registrar_nameserver_path, method: :put, class: 'form-horizontal' do %>
- <%= label_tag :old_hostname, nil %> + <%= label_tag :old_hostname %>
@@ -14,7 +14,7 @@
- <%= label_tag :new_hostname, nil %> + <%= label_tag :new_hostname %>
@@ -26,7 +26,7 @@
- <%= label_tag :ipv4, nil %> + <%= label_tag :ipv4 %>
@@ -36,7 +36,7 @@
- <%= label_tag :ipv6, nil %> + <%= label_tag :ipv6 %>
From d951a90bf81be7f7845773b232fd7be0dd7d1b9a Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Mon, 26 Feb 2018 15:57:14 +0200 Subject: [PATCH 26/34] Improve translation #661 --- config/locales/nameservers.en.yml | 4 ++-- test/models/nameserver/glue_record_test.rb | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/config/locales/nameservers.en.yml b/config/locales/nameservers.en.yml index 82a6d045e..a5d0f1cfc 100644 --- a/config/locales/nameservers.en.yml +++ b/config/locales/nameservers.en.yml @@ -1,8 +1,8 @@ en: - activemodel: + activerecord: errors: models: nameserver: attributes: base: - ip_required: At least one IPv4 or IPv6 is required + ip_required: Either IPv4 or IPv6 is required for glue record generation diff --git a/test/models/nameserver/glue_record_test.rb b/test/models/nameserver/glue_record_test.rb index 5f162cb2b..4a7f55b0c 100644 --- a/test/models/nameserver/glue_record_test.rb +++ b/test/models/nameserver/glue_record_test.rb @@ -9,7 +9,8 @@ class NameserverGlueRecordTest < ActiveSupport::TestCase @nameserver.hostname = 'ns1.shop.test' @nameserver.ipv4 = @nameserver.ipv6 = '' assert @nameserver.invalid? - assert @nameserver.errors.added?(:base, :ip_required) + assert_includes @nameserver.errors.full_messages, 'Either IPv4 or IPv6 is required' \ + ' for glue record generation' end def test_valid_with_ip_if_glue_record_is_required From c53d5542f6e410b934d1b7a408d19a286c8ba1fd Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Mon, 26 Feb 2018 16:09:42 +0200 Subject: [PATCH 27/34] Create new records on registrar nameserver replace #661 --- app/api/repp/nameservers_v1.rb | 29 +++++++----- app/models/registrar.rb | 14 ++++-- test/fixtures/contacts.yml | 11 +++++ test/fixtures/domains.yml | 10 ++++ test/fixtures/nameservers.yml | 8 ++++ test/integration/api/nameservers/put_test.rb | 48 ++++++++++++++++++-- 6 files changed, 99 insertions(+), 21 deletions(-) diff --git a/app/api/repp/nameservers_v1.rb b/app/api/repp/nameservers_v1.rb index cbc58eece..d653adf7f 100644 --- a/app/api/repp/nameservers_v1.rb +++ b/app/api/repp/nameservers_v1.rb @@ -5,10 +5,10 @@ module Repp resource 'registrar/nameservers' do put '/' do params do - requires :data, type: Hash do + requires :data, type: Hash, allow_blank: false do requires :type, type: String, allow_blank: false requires :id, type: String, allow_blank: false - requires :attributes, type: Hash do + requires :attributes, type: Hash, allow_blank: false do requires :hostname, type: String, allow_blank: false requires :ipv4, type: Array requires :ipv6, type: Array @@ -16,21 +16,28 @@ module Repp end end - old_nameserver = current_user.registrar.nameservers.find_by(hostname: params[:data][:id]) - error!({ errors: [{ title: "Hostname #{params[:data][:id]} does not exist" }] }, 404) unless old_nameserver + hostname = params[:data][:id] - new_nameserver = old_nameserver.dup - new_nameserver.hostname = params[:data][:attributes][:hostname] - new_nameserver.ipv4 = params[:data][:attributes][:ipv4] - new_nameserver.ipv6 = params[:data][:attributes][:ipv6] + unless current_user.registrar.nameservers.exists?(hostname: hostname) + error!({ errors: [{ title: "Hostname #{hostname} does not exist" }] }, 404) + end - error!({ errors: [{ title: 'Invalid params' }] }, 400) unless new_nameserver.valid? + new_attributes = { + hostname: params[:data][:attributes][:hostname], + ipv4: params[:data][:attributes][:ipv4], + ipv6: params[:data][:attributes][:ipv6], + } - current_user.registrar.replace_nameserver(old_nameserver, new_nameserver) + begin + current_user.registrar.replace_nameservers(hostname, new_attributes) + rescue ActiveRecord::RecordInvalid => e + error!({ errors: e.record.errors.full_messages.map { |error| { title: error } } }, 400) + end status 200 @response = { data: { type: 'nameserver', - id: new_nameserver.hostname, attributes: params[:data][:attributes] } } + id: params[:data][:attributes][:hostname], + attributes: params[:data][:attributes] } } end end end diff --git a/app/models/registrar.rb b/app/models/registrar.rb index 4bfb191e1..6a10983d2 100644 --- a/app/models/registrar.rb +++ b/app/models/registrar.rb @@ -158,12 +158,16 @@ class Registrar < ActiveRecord::Base white_ips.api.pluck(:ipv4, :ipv6).flatten.include?(ip) end - def replace_nameserver(old_nameserver, new_nameserver) + # Audit log is needed, therefore no raw SQL + def replace_nameservers(hostname, new_attributes) transaction do - nameservers.where(hostname: old_nameserver.hostname).find_each do |nameserver| - nameserver.update!(hostname: new_nameserver.hostname, - ipv4: new_nameserver.ipv4, - ipv6: new_nameserver.ipv6) # Audit log is needed, therefore no raw SQL + nameservers.where(hostname: hostname).find_each do |original_nameserver| + new_nameserver = Nameserver.new + new_nameserver.domain = original_nameserver.domain + new_nameserver.attributes = new_attributes + new_nameserver.save! + + original_nameserver.destroy! end end end diff --git a/test/fixtures/contacts.yml b/test/fixtures/contacts.yml index eaf4401b7..fe11968b9 100644 --- a/test/fixtures/contacts.yml +++ b/test/fixtures/contacts.yml @@ -42,6 +42,17 @@ acme_ltd: code: acme-ltd-001 auth_info: 720b3c +jack: + name: Jack + email: jack@inbox.test + phone: '+555.555' + ident: 1234 + ident_type: org + registrar: goodnames + ident_country_code: US + code: jack-001 + auth_info: e2c440 + invalid: name: any code: any diff --git a/test/fixtures/domains.yml b/test/fixtures/domains.yml index 7c0844d97..b2b2a24b5 100644 --- a/test/fixtures/domains.yml +++ b/test/fixtures/domains.yml @@ -28,6 +28,16 @@ library: period: 1 period_unit: m +metro: + name: metro.test + name_dirty: metro.test + registrar: goodnames + registrant: jack + transfer_code: 1071ad + valid_to: 2010-07-05 + period: 1 + period_unit: m + invalid: name: invalid.test transfer_code: any diff --git a/test/fixtures/nameservers.yml b/test/fixtures/nameservers.yml index 870e678ad..a03dc8cc9 100644 --- a/test/fixtures/nameservers.yml +++ b/test/fixtures/nameservers.yml @@ -13,3 +13,11 @@ ns2: ipv6: - 2001:db8::2 domain: shop + +airport_ns1: + hostname: ns1.bestnames.test + domain: airport + +metro_ns1: + hostname: ns1.bestnames.test + domain: metro diff --git a/test/integration/api/nameservers/put_test.rb b/test/integration/api/nameservers/put_test.rb index d048c29fe..034c885b7 100644 --- a/test/integration/api/nameservers/put_test.rb +++ b/test/integration/api/nameservers/put_test.rb @@ -1,17 +1,55 @@ require 'test_helper' class APINameserversPutTest < ActionDispatch::IntegrationTest - def test_replaces_current_registrar_nameservers + def setup + @registrar = registrars(:bestnames) + end + + def test_deletes_old_nameservers + previous_nameserver_ids = @registrar.nameservers.where(hostname: 'ns1.bestnames.test').ids + request_params = { format: :json, data: { type: 'nameserver', id: 'ns1.bestnames.test', + attributes: { hostname: 'ns55.bestnames.test' } } } + + put '/repp/v1/registrar/nameservers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } + refute @registrar.nameservers(true).where(id: previous_nameserver_ids).exists? + end + + def test_creates_new_nameservers + request_params = { format: :json, data: { type: 'nameserver', id: 'ns1.bestnames.test', + attributes: { hostname: 'ns55.bestnames.test' } } } + put '/repp/v1/registrar/nameservers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } + assert_equal 2, @registrar.nameservers.where(hostname: 'ns55.bestnames.test').size + end + + def test_saves_all_attributes request_params = { format: :json, data: { type: 'nameserver', id: 'ns1.bestnames.test', attributes: { hostname: 'ns55.bestnames.test', ipv4: ['192.0.2.55'], ipv6: ['2001:db8::55'] } } } put '/repp/v1/registrar/nameservers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } - new_nameserver = registrars(:bestnames).nameservers.find_by(hostname: 'ns55.bestnames.test') - assert_nil registrars(:bestnames).nameservers.find_by(hostname: 'ns1.bestnames.test') + new_nameserver = domains(:shop).nameservers.find_by(hostname: 'ns55.bestnames.test') assert_equal ['192.0.2.55'], new_nameserver.ipv4 assert_equal ['2001:DB8::55'], new_nameserver.ipv6 + end + + def test_keeps_other_registrar_nameservers_intact + request_params = { format: :json, data: { type: 'nameserver', id: 'ns1.bestnames.test', + attributes: { hostname: 'ns55.bestnames.test' } } } + + other_registrar_nameserver_ids = registrars(:goodnames).nameservers.ids + assert other_registrar_nameserver_ids.any? + put '/repp/v1/registrar/nameservers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } + assert_equal other_registrar_nameserver_ids, registrars(:goodnames).nameservers(true).ids + end + + def test_returns_new_nameserver_record + request_params = { format: :json, data: { type: 'nameserver', id: 'ns1.bestnames.test', + attributes: { hostname: 'ns55.bestnames.test', + ipv4: ['192.0.2.55'], + ipv6: ['2001:db8::55'] } } } + put '/repp/v1/registrar/nameservers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } + assert_response 200 assert_equal ({ data: { type: 'nameserver', id: 'ns55.bestnames.test', @@ -21,7 +59,7 @@ class APINameserversPutTest < ActionDispatch::IntegrationTest JSON.parse(response.body, symbolize_names: true) end - def test_honors_optional_params + def test_optional_params request_params = { format: :json, data: { type: 'nameserver', id: 'ns1.bestnames.test', attributes: { hostname: 'ns55.bestnames.test' } } } put '/repp/v1/registrar/nameservers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } @@ -44,7 +82,7 @@ class APINameserversPutTest < ActionDispatch::IntegrationTest put '/repp/v1/registrar/nameservers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } assert_response 400 - assert_equal ({ errors: [{ title: 'Invalid params' }] }), + assert_equal ({ errors: [{ title: 'Hostname is missing' }] }), JSON.parse(response.body, symbolize_names: true) end From 4960b4400f92bdbe2b1b106a2fe70185d55b7067 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 27 Feb 2018 08:18:34 +0200 Subject: [PATCH 28/34] Improve tests #661 --- app/models/nameserver.rb | 3 +- test/fixtures/nameservers.yml | 4 +-- test/integration/api/nameservers/put_test.rb | 34 +++++++++----------- test/models/nameserver/glue_record_test.rb | 2 +- test/models/nameserver_test.rb | 18 +++++++++-- 5 files changed, 37 insertions(+), 24 deletions(-) diff --git a/app/models/nameserver.rb b/app/models/nameserver.rb index 1880bd091..11537d3bc 100644 --- a/app/models/nameserver.rb +++ b/app/models/nameserver.rb @@ -5,7 +5,8 @@ class Nameserver < ActiveRecord::Base belongs_to :domain, required: true # rubocop: disable Metrics/LineLength - validates :hostname, presence: true, format: { with: /\A(([a-zA-Z0-9]|[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9][a-zA-ZäöüõšžÄÖÜÕŠŽ0-9\-]*[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\z/ } + validates :hostname, presence: true#, format: { with: /\A(([a-zA-Z0-9]|[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9][a-zA-ZäöüõšžÄÖÜÕŠŽ0-9\-]*[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\z/ } + validates :hostname, format: { with: /\A(([a-zA-Z0-9]|[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9][a-zA-ZäöüõšžÄÖÜÕŠŽ0-9\-]*[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\z/ }, allow_blank: true validate :val_ipv4 validate :val_ipv6 validate :require_ip, if: :glue_record_required? diff --git a/test/fixtures/nameservers.yml b/test/fixtures/nameservers.yml index a03dc8cc9..8c3c021d6 100644 --- a/test/fixtures/nameservers.yml +++ b/test/fixtures/nameservers.yml @@ -1,4 +1,4 @@ -ns1: +shop_ns1: hostname: ns1.bestnames.test ipv4: - 192.0.2.1 @@ -6,7 +6,7 @@ ns1: - 2001:db8::1 domain: shop -ns2: +shop_ns2: hostname: ns2.bestnames.test ipv4: - 192.0.2.2 diff --git a/test/integration/api/nameservers/put_test.rb b/test/integration/api/nameservers/put_test.rb index 034c885b7..416510541 100644 --- a/test/integration/api/nameservers/put_test.rb +++ b/test/integration/api/nameservers/put_test.rb @@ -1,24 +1,14 @@ require 'test_helper' class APINameserversPutTest < ActionDispatch::IntegrationTest - def setup - @registrar = registrars(:bestnames) - end - - def test_deletes_old_nameservers - previous_nameserver_ids = @registrar.nameservers.where(hostname: 'ns1.bestnames.test').ids - request_params = { format: :json, data: { type: 'nameserver', id: 'ns1.bestnames.test', - attributes: { hostname: 'ns55.bestnames.test' } } } - - put '/repp/v1/registrar/nameservers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } - refute @registrar.nameservers(true).where(id: previous_nameserver_ids).exists? - end - - def test_creates_new_nameservers + def test_replaces_registrar_nameservers + old_nameserver_ids = [nameservers(:shop_ns1).id, + nameservers(:airport_ns1).id, + nameservers(:metro_ns1).id] request_params = { format: :json, data: { type: 'nameserver', id: 'ns1.bestnames.test', attributes: { hostname: 'ns55.bestnames.test' } } } put '/repp/v1/registrar/nameservers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } - assert_equal 2, @registrar.nameservers.where(hostname: 'ns55.bestnames.test').size + assert_empty (old_nameserver_ids & registrars(:bestnames).nameservers(true).ids) end def test_saves_all_attributes @@ -33,14 +23,22 @@ class APINameserversPutTest < ActionDispatch::IntegrationTest assert_equal ['2001:DB8::55'], new_nameserver.ipv6 end + def test_keeps_other_nameserver_intact + request_params = { format: :json, data: { type: 'nameserver', id: 'ns1.bestnames.test', + attributes: { hostname: 'ns55.bestnames.test' } } } + + other_nameserver_hash = nameservers(:shop_ns2).attributes + put '/repp/v1/registrar/nameservers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } + assert_equal other_nameserver_hash, nameservers(:shop_ns2).reload.attributes + end + def test_keeps_other_registrar_nameservers_intact request_params = { format: :json, data: { type: 'nameserver', id: 'ns1.bestnames.test', attributes: { hostname: 'ns55.bestnames.test' } } } - other_registrar_nameserver_ids = registrars(:goodnames).nameservers.ids - assert other_registrar_nameserver_ids.any? + nameserver_hash = nameservers(:metro_ns1).attributes put '/repp/v1/registrar/nameservers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key } - assert_equal other_registrar_nameserver_ids, registrars(:goodnames).nameservers(true).ids + assert_equal nameserver_hash, nameservers(:metro_ns1).reload.attributes end def test_returns_new_nameserver_record diff --git a/test/models/nameserver/glue_record_test.rb b/test/models/nameserver/glue_record_test.rb index 4a7f55b0c..7d5e39d8d 100644 --- a/test/models/nameserver/glue_record_test.rb +++ b/test/models/nameserver/glue_record_test.rb @@ -2,7 +2,7 @@ require 'test_helper' class NameserverGlueRecordTest < ActiveSupport::TestCase def setup - @nameserver = nameservers(:ns1) + @nameserver = nameservers(:shop_ns1) end def test_invalid_without_ip_if_glue_record_is_required diff --git a/test/models/nameserver_test.rb b/test/models/nameserver_test.rb index 377ff446a..45c1d0fb2 100644 --- a/test/models/nameserver_test.rb +++ b/test/models/nameserver_test.rb @@ -2,7 +2,7 @@ require 'test_helper' class NameserverTest < ActiveSupport::TestCase def setup - @nameserver = nameservers(:ns1) + @nameserver = nameservers(:shop_ns1) end def test_valid @@ -19,8 +19,22 @@ class NameserverTest < ActiveSupport::TestCase assert @nameserver.invalid? end + def test_hostname_format_validation + @nameserver.hostname = 'foo_bar' + assert @nameserver.invalid? + + @nameserver.hostname = 'foo.bar' + assert @nameserver.valid? + + @nameserver.hostname = 'äöüõšž.ÄÖÜÕŠŽ.umlauts' + assert @nameserver.valid? + end + def test_hostnames - assert_equal %w[ns1.bestnames.test ns2.bestnames.test], Nameserver.hostnames + assert_equal %w[ns1.bestnames.test + ns2.bestnames.test + ns1.bestnames.test + ns1.bestnames.test], Nameserver.hostnames end def test_normalizes_hostname From 827a330eec07be15a9654a36d431907d435fdfed Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 27 Feb 2018 08:28:22 +0200 Subject: [PATCH 29/34] Introduce constant #661 --- app/models/nameserver.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/app/models/nameserver.rb b/app/models/nameserver.rb index 11537d3bc..ec9abb19f 100644 --- a/app/models/nameserver.rb +++ b/app/models/nameserver.rb @@ -2,15 +2,18 @@ class Nameserver < ActiveRecord::Base include Versions # version/nameserver_version.rb include EppErrors + HOSTNAME_REGEXP = /\A(([a-zA-Z0-9]|[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9][a-zA-ZäöüõšžÄÖÜÕŠŽ0-9\-] + *[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9])\.) + *([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-] + *[A-Za-z0-9])\z/x + belongs_to :domain, required: true - # rubocop: disable Metrics/LineLength - validates :hostname, presence: true#, format: { with: /\A(([a-zA-Z0-9]|[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9][a-zA-ZäöüõšžÄÖÜÕŠŽ0-9\-]*[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\z/ } - validates :hostname, format: { with: /\A(([a-zA-Z0-9]|[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9][a-zA-ZäöüõšžÄÖÜÕŠŽ0-9\-]*[a-zA-ZäöüõšžÄÖÜÕŠŽ0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\z/ }, allow_blank: true + validates :hostname, presence: true + validates :hostname, format: { with: HOSTNAME_REGEXP }, allow_blank: true validate :val_ipv4 validate :val_ipv6 validate :require_ip, if: :glue_record_required? - # rubocop: enable Metrics/LineLength before_validation :normalize_attributes before_validation :check_puny_symbols From 114f15d4ea8469527bbb0d119074790c7e011dd2 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 27 Feb 2018 09:58:51 +0200 Subject: [PATCH 30/34] Add association `inverse_of` #661 --- app/models/domain.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/domain.rb b/app/models/domain.rb index 54dfe608e..5cc81723f 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -40,7 +40,7 @@ class Domain < ActiveRecord::Base has_many :contacts, through: :domain_contacts, source: :contact has_many :admin_contacts, through: :admin_domain_contacts, source: :contact has_many :tech_contacts, through: :tech_domain_contacts, source: :contact - has_many :nameservers, dependent: :destroy + has_many :nameservers, dependent: :destroy, inverse_of: :domain accepts_nested_attributes_for :nameservers, allow_destroy: true, reject_if: proc { |attrs| attrs[:hostname].blank? } From 561b28ffb6112844f2f443ce721a523e34a1ab59 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 27 Feb 2018 10:01:34 +0200 Subject: [PATCH 31/34] Fix spec #661 --- spec/models/domain_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/models/domain_spec.rb b/spec/models/domain_spec.rb index 0ccc9f4fa..cc1fe52a2 100644 --- a/spec/models/domain_spec.rb +++ b/spec/models/domain_spec.rb @@ -568,8 +568,8 @@ RSpec.describe Domain do with_versioning do context 'when saved' do before(:each) do - domain = create(:domain) - domain.nameservers << create(:nameserver) + domain = create(:domain, nameservers_attributes: [attributes_for(:nameserver), + attributes_for(:nameserver)]) end it 'creates domain version' do @@ -660,7 +660,7 @@ RSpec.describe Domain do end describe 'nameserver validation', db: true do - let(:domain) { described_class.new } + let(:domain) { described_class.new(name: 'whatever.test') } it 'rejects less than min' do Setting.ns_min_count = 2 From f7c0c886bd9effe88906d8ef18195d533e7e0923 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 27 Feb 2018 10:36:42 +0200 Subject: [PATCH 32/34] Add tests #661 --- test/models/nameserver_test.rb | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/test/models/nameserver_test.rb b/test/models/nameserver_test.rb index 45c1d0fb2..5a552a7ee 100644 --- a/test/models/nameserver_test.rb +++ b/test/models/nameserver_test.rb @@ -20,14 +20,33 @@ class NameserverTest < ActiveSupport::TestCase end def test_hostname_format_validation - @nameserver.hostname = 'foo_bar' - assert @nameserver.invalid? - @nameserver.hostname = 'foo.bar' assert @nameserver.valid? @nameserver.hostname = 'äöüõšž.ÄÖÜÕŠŽ.umlauts' assert @nameserver.valid? + + @nameserver.hostname = 'foo_bar' + assert @nameserver.invalid? + end + + def test_ipv4_format_validation + @nameserver.ipv4 = ['192.0.2.1'] + assert @nameserver.valid? + + @nameserver.ipv4 = ['0.0.0.256'] + assert @nameserver.invalid? + + @nameserver.ipv4 = ['192.168.0.0/24'] + assert @nameserver.invalid? + end + + def test_ipv6_format_validation + @nameserver.ipv6 = ['2001:db8::1'] + assert @nameserver.valid? + + @nameserver.ipv6 = ['3ffe:0b00:0000:0001:0000:0000:000a'] + assert @nameserver.invalid? end def test_hostnames @@ -54,4 +73,9 @@ class NameserverTest < ActiveSupport::TestCase @nameserver.validate assert_equal ['2001:DB8::1'], @nameserver.ipv6 end + + def test_encodes_hostname_to_punycode + @nameserver.hostname = 'ns1.münchen.de' + assert_equal 'ns1.xn--mnchen-3ya.de', @nameserver.hostname_puny + end end From cfec74f9f81f5abcb2f5dcf01c40d48fccf98bb9 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 27 Feb 2018 10:37:36 +0200 Subject: [PATCH 33/34] Hide methods #661 --- app/models/nameserver.rb | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/app/models/nameserver.rb b/app/models/nameserver.rb index ec9abb19f..ca82d5fb6 100644 --- a/app/models/nameserver.rb +++ b/app/models/nameserver.rb @@ -47,19 +47,6 @@ class Nameserver < ActiveRecord::Base self[:hostname_puny] = SimpleIDN.to_ascii(hostname) end - def val_ipv4 - regexp = /\A(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\z/ - ipv4.to_a.each do |ip| - errors.add(:ipv4, :invalid) unless ip =~ regexp - end - end - def val_ipv6 - regexp = /(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]).){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]).){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]))/ - ipv6.to_a.each do |ip| - errors.add(:ipv6, :invalid) unless ip =~ regexp - end - end - class << self def find_by_hash_params params params = params.with_indifferent_access @@ -102,4 +89,18 @@ class Nameserver < ActiveRecord::Base regexp = /(\A|\.)..--/ errors.add(:hostname, :invalid) if hostname =~ regexp end + + def val_ipv4 + regexp = /\A(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\z/ + ipv4.to_a.each do |ip| + errors.add(:ipv4, :invalid) unless ip =~ regexp + end + end + + def val_ipv6 + regexp = /(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]).){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]).){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]))/ + ipv6.to_a.each do |ip| + errors.add(:ipv6, :invalid) unless ip =~ regexp + end + end end From 45064b9392bc839d4f9a778b1f91ec8f376a0b84 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 27 Feb 2018 10:42:48 +0200 Subject: [PATCH 34/34] Introduce constants #661 --- .reek | 2 -- app/models/nameserver.rb | 27 +++++++++++++++++++-------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/.reek b/.reek index f108f4bf3..6cdd58c75 100644 --- a/.reek +++ b/.reek @@ -1136,7 +1136,5 @@ UncommunicativeParameterName: - Dnskey#int_to_hex UncommunicativeMethodName: exclude: - - Nameserver#val_ipv4 - - Nameserver#val_ipv6 - Soap::Arireg#country_code_3 - WhiteIp#validate_ipv4_and_ipv6 diff --git a/app/models/nameserver.rb b/app/models/nameserver.rb index ca82d5fb6..d9a5f2a75 100644 --- a/app/models/nameserver.rb +++ b/app/models/nameserver.rb @@ -7,12 +7,25 @@ class Nameserver < ActiveRecord::Base *([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-] *[A-Za-z0-9])\z/x + IPV4_REGEXP = /\A(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3} + ([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\z/x + + IPV6_REGEXP = /(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:| + ([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}| + ([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}| + ([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}| + ([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})| + :((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}| + ::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]).){3,3}(25[0-5]|(2[0-4]| + 1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]| + 1{0,1}[0-9]){0,1}[0-9]).){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]))/x + belongs_to :domain, required: true validates :hostname, presence: true validates :hostname, format: { with: HOSTNAME_REGEXP }, allow_blank: true - validate :val_ipv4 - validate :val_ipv6 + validate :validate_ipv4_format + validate :validate_ipv6_format validate :require_ip, if: :glue_record_required? before_validation :normalize_attributes @@ -90,17 +103,15 @@ class Nameserver < ActiveRecord::Base errors.add(:hostname, :invalid) if hostname =~ regexp end - def val_ipv4 - regexp = /\A(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\z/ + def validate_ipv4_format ipv4.to_a.each do |ip| - errors.add(:ipv4, :invalid) unless ip =~ regexp + errors.add(:ipv4, :invalid) unless ip =~ IPV4_REGEXP end end - def val_ipv6 - regexp = /(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]).){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]).){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]))/ + def validate_ipv6_format ipv6.to_a.each do |ip| - errors.add(:ipv6, :invalid) unless ip =~ regexp + errors.add(:ipv6, :invalid) unless ip =~ IPV6_REGEXP end end end