From 5ba39fb4062ff6489cff1635c65e85c1251b3ef7 Mon Sep 17 00:00:00 2001 From: Martin Lensment Date: Tue, 25 Aug 2015 16:33:47 +0300 Subject: [PATCH] Validate origin domains #2849 --- app/controllers/epp/domains_controller.rb | 1 - app/validators/domain_name_validator.rb | 13 ++++++--- spec/epp/contact_spec.rb | 6 ++++ spec/epp/domain_spec.rb | 7 +++++ spec/epp/keyrelay_spec.rb | 1 + spec/features/admin/blocked_domain_spec.rb | 1 + spec/features/admin/domain_spec.rb | 2 ++ spec/features/admin/reserved_domain_spec.rb | 1 + .../registrant/domain_delete_confirm_spec.rb | 4 +++ .../registrant/domain_update_confirm_spec.rb | 4 +++ spec/features/registrar/domain_spec.rb | 6 +++- spec/features/sessions_spec.rb | 1 + spec/mailers/contact_mailer_spec.rb | 5 ++-- spec/mailers/domain_mailer_spec.rb | 28 +++++++++++-------- spec/models/contact_spec.rb | 2 ++ spec/models/dnskey_spec.rb | 4 +++ spec/models/domain_spec.rb | 17 ++++++++--- spec/models/domain_transfer_spec.rb | 4 +++ spec/models/keyrelay_spec.rb | 4 +++ spec/models/nameserver_spec.rb | 4 +++ spec/models/registrant_verification_spec.rb | 3 ++ spec/models/whois_record_spec.rb | 4 +++ spec/requests/v1/domain_spec.rb | 1 + 23 files changed, 99 insertions(+), 24 deletions(-) diff --git a/app/controllers/epp/domains_controller.rb b/app/controllers/epp/domains_controller.rb index 5d942882b..d3111ddfa 100644 --- a/app/controllers/epp/domains_controller.rb +++ b/app/controllers/epp/domains_controller.rb @@ -29,7 +29,6 @@ class Epp::DomainsController < EppController handle_errors(@domain) and return if @domain.errors.any? handle_errors and return unless balance_ok?('create') - ActiveRecord::Base.transaction do if @domain.save # TODO: Maybe use validate: false here because we have already validated the domain? current_user.registrar.debit!({ diff --git a/app/validators/domain_name_validator.rb b/app/validators/domain_name_validator.rb index c8ed2813c..e39437f2b 100644 --- a/app/validators/domain_name_validator.rb +++ b/app/validators/domain_name_validator.rb @@ -9,22 +9,27 @@ class DomainNameValidator < ActiveModel::EachValidator class << self def validate_format(value) - return true if value == 'ee' return true unless value value = value.mb_chars.downcase.strip - general_domains = /(.pri.ee|.com.ee|.fie.ee|.med.ee|.ee)/ + origins = ZonefileSetting.pluck(:origin) + # if someone tries to register an origin domain, let this validation pass + # the error will be catched in blocked domains validator + return true if origins.include?(value) + + general_domains = /(#{origins.join('|')})/ + # general_domains = /(.pri.ee|.com.ee|.fie.ee|.med.ee|.ee)/ # it's punycode if value[2] == '-' && value[3] == '-' - regexp = /\Axn--[a-zA-Z0-9-]{0,59}#{general_domains}\z/ + regexp = /\Axn--[a-zA-Z0-9-]{0,59}\.#{general_domains}\z/ return false unless value =~ regexp value = SimpleIDN.to_unicode(value).mb_chars.downcase.strip end # rubocop: disable Metrics/LineLength unicode_chars = /\u00E4\u00F5\u00F6\u00FC\u0161\u017E/ # äõöüšž - regexp = /\A[a-zA-Z0-9#{unicode_chars.source}][a-zA-Z0-9#{unicode_chars.source}-]{0,61}[a-zA-Z0-9#{unicode_chars.source}]#{general_domains.source}\z/ + regexp = /\A[a-zA-Z0-9#{unicode_chars.source}][a-zA-Z0-9#{unicode_chars.source}-]{0,61}[a-zA-Z0-9#{unicode_chars.source}]\.#{general_domains.source}\z/ # rubocop: enable Metrics/LineLength # rubocop: disable Style/DoubleNegation !!(value =~ regexp) diff --git a/spec/epp/contact_spec.rb b/spec/epp/contact_spec.rb index 36926e66c..4a8f35bb6 100644 --- a/spec/epp/contact_spec.rb +++ b/spec/epp/contact_spec.rb @@ -3,6 +3,12 @@ require 'rails_helper' describe 'EPP Contact', epp: true do before :all do @xsd = Nokogiri::XML::Schema(File.read('lib/schemas/contact-eis-1.0.xsd')) + Fabricate(:zonefile_setting, origin: 'ee') + Fabricate(:zonefile_setting, origin: 'pri.ee') + Fabricate(:zonefile_setting, origin: 'med.ee') + Fabricate(:zonefile_setting, origin: 'fie.ee') + Fabricate(:zonefile_setting, origin: 'com.ee') + @registrar1 = Fabricate(:registrar1) @registrar2 = Fabricate(:registrar2) @epp_xml = EppXml::Contact.new(cl_trid: 'ABC-12345') diff --git a/spec/epp/domain_spec.rb b/spec/epp/domain_spec.rb index 6df222aa5..2b4026126 100644 --- a/spec/epp/domain_spec.rb +++ b/spec/epp/domain_spec.rb @@ -4,6 +4,13 @@ describe 'EPP Domain', epp: true do before(:all) do @xsd = Nokogiri::XML::Schema(File.read('lib/schemas/domain-eis-1.0.xsd')) @epp_xml = EppXml.new(cl_trid: 'ABC-12345') + + Fabricate(:zonefile_setting, origin: 'ee') + Fabricate(:zonefile_setting, origin: 'pri.ee') + Fabricate(:zonefile_setting, origin: 'med.ee') + Fabricate(:zonefile_setting, origin: 'fie.ee') + Fabricate(:zonefile_setting, origin: 'com.ee') + @registrar1 = Fabricate(:registrar1, code: 'REGDOMAIN1') @registrar1.credit!({ sum: 10000 }) @registrar2 = Fabricate(:registrar2, code: 'REGDOMAIN2') diff --git a/spec/epp/keyrelay_spec.rb b/spec/epp/keyrelay_spec.rb index 17823e9c9..d42d96f4b 100644 --- a/spec/epp/keyrelay_spec.rb +++ b/spec/epp/keyrelay_spec.rb @@ -2,6 +2,7 @@ require 'rails_helper' describe 'EPP Keyrelay', epp: true do before(:all) do + Fabricate(:zonefile_setting, origin: 'ee') @registrar1 = Fabricate(:registrar1) @registrar2 = Fabricate(:registrar2) @domain = Fabricate(:domain, registrar: @registrar2) diff --git a/spec/features/admin/blocked_domain_spec.rb b/spec/features/admin/blocked_domain_spec.rb index 5f2535484..432556d64 100644 --- a/spec/features/admin/blocked_domain_spec.rb +++ b/spec/features/admin/blocked_domain_spec.rb @@ -2,6 +2,7 @@ require 'rails_helper' feature 'BlockedDomain', type: :feature do before :all do + Fabricate(:zonefile_setting, origin: 'ee') @user = Fabricate(:admin_user) end diff --git a/spec/features/admin/domain_spec.rb b/spec/features/admin/domain_spec.rb index 101ba5e8c..fc9548996 100644 --- a/spec/features/admin/domain_spec.rb +++ b/spec/features/admin/domain_spec.rb @@ -2,6 +2,8 @@ require 'rails_helper' feature 'Domain', type: :feature do before :all do + Fabricate(:zonefile_setting, origin: 'ee') + Fabricate(:zonefile_setting, origin: 'pri.ee') @user = Fabricate(:admin_user) end diff --git a/spec/features/admin/reserved_domain_spec.rb b/spec/features/admin/reserved_domain_spec.rb index 9c780285f..276907c6b 100644 --- a/spec/features/admin/reserved_domain_spec.rb +++ b/spec/features/admin/reserved_domain_spec.rb @@ -2,6 +2,7 @@ require 'rails_helper' feature 'ReservedDomain', type: :feature do before :all do + Fabricate(:zonefile_setting, origin: 'ee') @user = Fabricate(:admin_user) end diff --git a/spec/features/registrant/domain_delete_confirm_spec.rb b/spec/features/registrant/domain_delete_confirm_spec.rb index f51374540..db5b0237d 100644 --- a/spec/features/registrant/domain_delete_confirm_spec.rb +++ b/spec/features/registrant/domain_delete_confirm_spec.rb @@ -1,6 +1,10 @@ require 'rails_helper' feature 'DomainDeleteConfirm', type: :feature do + before :all do + Fabricate(:zonefile_setting, origin: 'ee') + end + context 'as unknown user with domain without token' do before :all do @domain = Fabricate(:domain) diff --git a/spec/features/registrant/domain_update_confirm_spec.rb b/spec/features/registrant/domain_update_confirm_spec.rb index 0af47e43b..b9703165e 100644 --- a/spec/features/registrant/domain_update_confirm_spec.rb +++ b/spec/features/registrant/domain_update_confirm_spec.rb @@ -1,6 +1,10 @@ require 'rails_helper' feature 'DomainUpdateConfirm', type: :feature do + before :all do + Fabricate(:zonefile_setting, origin: 'ee') + end + context 'as unknown user with domain without update token' do before :all do @domain = Fabricate(:domain) diff --git a/spec/features/registrar/domain_spec.rb b/spec/features/registrar/domain_spec.rb index f9eb76c76..3eecd5d10 100644 --- a/spec/features/registrar/domain_spec.rb +++ b/spec/features/registrar/domain_spec.rb @@ -2,6 +2,8 @@ require 'rails_helper' feature 'Domains', type: :feature do before :all do + Fabricate(:zonefile_setting, origin: 'ee') + Fabricate(:zonefile_setting, origin: 'pri.ee') @user = Fabricate(:api_user) end @@ -58,7 +60,9 @@ feature 'Domains', type: :feature do it 'should search domains' do # having shared state across tests is really annoying sometimes... - click_link "#{@user} (#{@user.roles.first}) - #{@user.registrar}" + within('.dropdown-menu') do + click_link "#{@user} (#{@user.roles.first}) - #{@user.registrar}" + end Fabricate(:domain, name: 'abcde.ee', registrar: @user.registrar) Fabricate(:domain, name: 'abcdee.ee', registrar: @user.registrar) diff --git a/spec/features/sessions_spec.rb b/spec/features/sessions_spec.rb index 1419b2f2c..8eba74afc 100644 --- a/spec/features/sessions_spec.rb +++ b/spec/features/sessions_spec.rb @@ -2,6 +2,7 @@ require 'rails_helper' feature 'Sessions', type: :feature do before :all do + Fabricate(:zonefile_setting, origin: 'ee') @user = Fabricate(:ee_user) @registrar1 = Fabricate(:registrar1) @registrar2 = Fabricate(:registrar2) diff --git a/spec/mailers/contact_mailer_spec.rb b/spec/mailers/contact_mailer_spec.rb index ce4adabef..60699a2f9 100644 --- a/spec/mailers/contact_mailer_spec.rb +++ b/spec/mailers/contact_mailer_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' describe ContactMailer do describe 'email changed notification when delivery turned off' do - before :all do + before :all do @contact = Fabricate(:contact, email: 'test@example.ee') @mail = ContactMailer.email_updated('test@example.com', @contact) end @@ -25,7 +25,8 @@ describe ContactMailer do end describe 'email changed notification' do - before :all do + before :all do + Fabricate(:zonefile_setting, origin: 'ee') @domain = Fabricate(:domain) @contact = @domain.registrant @contact.reload # until figured out why registrant_domains not loaded diff --git a/spec/mailers/domain_mailer_spec.rb b/spec/mailers/domain_mailer_spec.rb index ec9fd4672..71ed183ce 100644 --- a/spec/mailers/domain_mailer_spec.rb +++ b/spec/mailers/domain_mailer_spec.rb @@ -1,8 +1,12 @@ require 'rails_helper' describe DomainMailer do + before :all do + Fabricate(:zonefile_setting, origin: 'ee') + end + describe 'pending update request for an old registrant when delivery turned off' do - before :all do + before :all do @registrant = Fabricate(:registrant, email: 'test@example.com') @domain = Fabricate(:domain, registrant: @registrant) @mail = DomainMailer.pending_update_request_for_old_registrant(@domain) @@ -26,7 +30,7 @@ describe DomainMailer do end describe 'pending update request for an old registrant' do - before :all do + before :all do @registrant = Fabricate(:registrant, email: 'test@example.com') @new_registrant = Fabricate(:registrant, email: 'test@example.org') @domain = Fabricate(:domain, registrant: @registrant) @@ -59,7 +63,7 @@ describe DomainMailer do end describe 'pending upadte notification for a new registrant' do - before :all do + before :all do @registrant = Fabricate(:registrant, email: 'old@example.com') @new_registrant = Fabricate(:registrant, email: 'new@example.org') @domain = Fabricate(:domain, registrant: @registrant) @@ -88,7 +92,7 @@ describe DomainMailer do end describe 'pending update notification for a new registrant' do - before :all do + before :all do @registrant = Fabricate(:registrant, email: 'old@example.com') @new_registrant = Fabricate(:registrant, email: 'new@example.org') @domain = Fabricate(:domain, registrant: @registrant) @@ -117,7 +121,7 @@ describe DomainMailer do end describe 'pending update rejected notification for a new registrant' do - before :all do + before :all do @registrant = Fabricate(:registrant, email: 'old@example.com') @new_registrant = Fabricate(:registrant, email: 'new@example.org') @domain = Fabricate(:domain, registrant: @registrant) @@ -145,7 +149,7 @@ describe DomainMailer do end describe 'registrant updated notification for a new registrant' do - before :all do + before :all do @registrant = Fabricate(:registrant, email: 'test@example.com') @domain = Fabricate(:domain, registrant: @registrant) @domain.deliver_emails = true @@ -170,7 +174,7 @@ describe DomainMailer do end describe 'registrant updated notification for a old registrant' do - before :all do + before :all do @registrant = Fabricate(:registrant, email: 'test@example.com') @domain = Fabricate(:domain, registrant: @registrant) @domain.deliver_emails = true @@ -195,7 +199,7 @@ describe DomainMailer do end describe 'domain pending delete notification when delivery turned off' do - before :all do + before :all do @registrant = Fabricate(:registrant, email: 'test@example.com') @domain = Fabricate(:domain, registrant: @registrant) @mail = DomainMailer.pending_deleted(@domain) @@ -219,7 +223,7 @@ describe DomainMailer do end describe 'email pending delete notification' do - before :all do + before :all do @registrant = Fabricate(:registrant, email: 'test@example.com') @domain = Fabricate(:domain, name: 'delete-pending.ee', registrant: @registrant) @domain.deliver_emails = true @@ -250,7 +254,7 @@ describe DomainMailer do end describe 'pending delete rejected notification' do - before :all do + before :all do @registrant = Fabricate(:registrant, email: 'test@example.com') @domain = Fabricate(:domain, name: 'delete-pending-rejected.ee', registrant: @registrant) @domain.deliver_emails = true @@ -277,7 +281,7 @@ describe DomainMailer do end describe 'pending delete expired notification' do - before :all do + before :all do @registrant = Fabricate(:registrant, email: 'test@example.com') @domain = Fabricate(:domain, name: 'pending-delete-expired.ee', registrant: @registrant) @domain.deliver_emails = true @@ -304,7 +308,7 @@ describe DomainMailer do end describe 'pending delete rejected notification' do - before :all do + before :all do @registrant = Fabricate(:registrant, email: 'test@example.com') @domain = Fabricate(:domain, name: 'delete-confirmed.ee', registrant: @registrant) @domain.deliver_emails = true diff --git a/spec/models/contact_spec.rb b/spec/models/contact_spec.rb index 303a5d415..4407fdac2 100644 --- a/spec/models/contact_spec.rb +++ b/spec/models/contact_spec.rb @@ -2,6 +2,7 @@ require 'rails_helper' describe Contact do before :all do + Fabricate(:zonefile_setting, origin: 'ee') @api_user = Fabricate(:api_user) end @@ -383,6 +384,7 @@ end describe Contact, '.destroy_orphans' do before do + Fabricate(:zonefile_setting, origin: 'ee') @contact_1 = Fabricate(:contact, code: 'asd12') @contact_2 = Fabricate(:contact, code: 'asd13') end diff --git a/spec/models/dnskey_spec.rb b/spec/models/dnskey_spec.rb index a7ddfece4..8d1185cbd 100644 --- a/spec/models/dnskey_spec.rb +++ b/spec/models/dnskey_spec.rb @@ -1,6 +1,10 @@ require 'rails_helper' describe Dnskey do + before :all do + Fabricate(:zonefile_setting, origin: 'ee') + end + it { should belong_to(:domain) } context 'with invalid attribute' do diff --git a/spec/models/domain_spec.rb b/spec/models/domain_spec.rb index e76529f27..015a407e0 100644 --- a/spec/models/domain_spec.rb +++ b/spec/models/domain_spec.rb @@ -1,6 +1,14 @@ require 'rails_helper' describe Domain do + before :all do + Fabricate(:zonefile_setting, origin: 'ee') + Fabricate(:zonefile_setting, origin: 'pri.ee') + Fabricate(:zonefile_setting, origin: 'med.ee') + Fabricate(:zonefile_setting, origin: 'fie.ee') + Fabricate(:zonefile_setting, origin: 'com.ee') + end + it { should belong_to(:registrar) } it { should have_many(:nameservers) } it { should belong_to(:registrant) } @@ -573,16 +581,17 @@ describe Domain do end it 'should not create zone origin domain' do - zs = Fabricate(:zonefile_setting) d = Fabricate.build(:domain, name: 'ee') d.save.should == false d.errors.full_messages.should match_array([ "Data management policy violation: Domain name is blocked [name]" ]) - zs.destroy - - d.save.should == true + d = Fabricate.build(:domain, name: 'bla') + d.save.should == false + d.errors.full_messages.should match_array([ + "Domain name Domain name is invalid" + ]) end # d = Domain.new diff --git a/spec/models/domain_transfer_spec.rb b/spec/models/domain_transfer_spec.rb index 72892b68d..169b9f24e 100644 --- a/spec/models/domain_transfer_spec.rb +++ b/spec/models/domain_transfer_spec.rb @@ -1,6 +1,10 @@ require 'rails_helper' describe DomainTransfer do + before :all do + Fabricate(:zonefile_setting, origin: 'ee') + end + it { should belong_to(:domain) } context 'with invalid attribute' do diff --git a/spec/models/keyrelay_spec.rb b/spec/models/keyrelay_spec.rb index 5cd5ff9ef..372c63332 100644 --- a/spec/models/keyrelay_spec.rb +++ b/spec/models/keyrelay_spec.rb @@ -1,6 +1,10 @@ require 'rails_helper' describe Keyrelay do + before :all do + Fabricate(:zonefile_setting, origin: 'ee') + end + it { should belong_to(:domain) } it { should belong_to(:requester) } it { should belong_to(:accepter) } diff --git a/spec/models/nameserver_spec.rb b/spec/models/nameserver_spec.rb index 6d2b539e2..64487722b 100644 --- a/spec/models/nameserver_spec.rb +++ b/spec/models/nameserver_spec.rb @@ -1,6 +1,10 @@ require 'rails_helper' describe Nameserver do + before :all do + Fabricate(:zonefile_setting, origin: 'ee') + end + it { should belong_to(:domain) } context 'with invalid attribute' do diff --git a/spec/models/registrant_verification_spec.rb b/spec/models/registrant_verification_spec.rb index 97d165923..c9278cd4a 100644 --- a/spec/models/registrant_verification_spec.rb +++ b/spec/models/registrant_verification_spec.rb @@ -1,6 +1,9 @@ require 'rails_helper' describe RegistrantVerification do + before :all do + Fabricate(:zonefile_setting, origin: 'ee') + end context 'with invalid attribute' do before :all do @registrant_verification = RegistrantVerification.new diff --git a/spec/models/whois_record_spec.rb b/spec/models/whois_record_spec.rb index bf0d5b9ed..95c4c2082 100644 --- a/spec/models/whois_record_spec.rb +++ b/spec/models/whois_record_spec.rb @@ -1,6 +1,10 @@ require 'rails_helper' describe WhoisRecord do + before :all do + Fabricate(:zonefile_setting, origin: 'ee') + end + context 'with invalid attribute' do before :all do @whois_record = WhoisRecord.new diff --git a/spec/requests/v1/domain_spec.rb b/spec/requests/v1/domain_spec.rb index 47b37ea76..e79f2e71b 100644 --- a/spec/requests/v1/domain_spec.rb +++ b/spec/requests/v1/domain_spec.rb @@ -2,6 +2,7 @@ require 'rails_helper' describe Repp::DomainV1 do before :all do + Fabricate(:zonefile_setting, origin: 'ee') @registrar1 = Fabricate(:registrar1) @api_user = Fabricate(:gitlab_api_user, registrar: @registrar1) Fabricate.times(2, :domain, registrar: @api_user.registrar)