From 2e9acedc901d948047ab9208c4f4e4a80cc17c00 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sat, 7 Apr 2018 15:18:10 +0300 Subject: [PATCH 01/61] Improve test name #790 --- test/integration/epp/domain/transfer/request_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/epp/domain/transfer/request_test.rb b/test/integration/epp/domain/transfer/request_test.rb index f3ea644fc..996296257 100644 --- a/test/integration/epp/domain/transfer/request_test.rb +++ b/test/integration/epp/domain/transfer/request_test.rb @@ -75,7 +75,7 @@ class EppDomainTransferRequestTest < ActionDispatch::IntegrationTest assert_equal '2304', Nokogiri::XML(response.body).at_css('result')[:code] end - def test_discarded_domain + def test_discarded_domain_cannot_be_transferred @domain.update!(statuses: [DomainStatus::DELETE_CANDIDATE]) post '/epp/command/transfer', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=api_goodnames' } From 00a30fc019a058e5cbaa6db7190b6afdc18359fa Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sat, 7 Apr 2018 16:21:07 +0300 Subject: [PATCH 02/61] Remove unneeded attribute alias #790 --- app/models/concerns/domain/deletable.rb | 4 ---- app/models/domain.rb | 2 +- app/presenters/domain_presenter.rb | 2 +- spec/models/domain_spec.rb | 6 +++--- spec/presenters/domain_presenter_spec.rb | 4 ++-- 5 files changed, 7 insertions(+), 11 deletions(-) diff --git a/app/models/concerns/domain/deletable.rb b/app/models/concerns/domain/deletable.rb index 86c296d88..204d924ba 100644 --- a/app/models/concerns/domain/deletable.rb +++ b/app/models/concerns/domain/deletable.rb @@ -1,10 +1,6 @@ module Concerns::Domain::Deletable extend ActiveSupport::Concern - included do - alias_attribute :delete_time, :delete_at - end - def discard statuses << DomainStatus::DELETE_CANDIDATE save diff --git a/app/models/domain.rb b/app/models/domain.rb index 9eef0f30f..a9fec134e 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -684,7 +684,7 @@ class Domain < ActiveRecord::Base end def self.delete_candidates - where("#{attribute_alias(:delete_time)} < ?", Time.zone.now) + where('delete_at < ?', Time.zone.now) end def self.uses_zone?(zone) diff --git a/app/presenters/domain_presenter.rb b/app/presenters/domain_presenter.rb index aac3c1527..2ecb82922 100644 --- a/app/presenters/domain_presenter.rb +++ b/app/presenters/domain_presenter.rb @@ -30,7 +30,7 @@ class DomainPresenter end def delete_date - view.l(domain.delete_time, format: :date) if domain.delete_time + view.l(domain.delete_at, format: :date) if domain.delete_at end def force_delete_date diff --git a/spec/models/domain_spec.rb b/spec/models/domain_spec.rb index cc1fe52a2..6c09ef7ed 100644 --- a/spec/models/domain_spec.rb +++ b/spec/models/domain_spec.rb @@ -877,9 +877,9 @@ RSpec.describe Domain do create(:zone, origin: 'ee') - create(:domain, id: 1, delete_time: Time.zone.parse('04.07.2010 23:59')) - create(:domain, id: 2, delete_time: Time.zone.parse('05.07.2010 00:00')) - create(:domain, id: 3, delete_time: Time.zone.parse('05.07.2010 00:01')) + create(:domain, id: 1, delete_at: Time.zone.parse('04.07.2010 23:59')) + create(:domain, id: 2, delete_at: Time.zone.parse('05.07.2010 00:00')) + create(:domain, id: 3, delete_at: Time.zone.parse('05.07.2010 00:01')) end it 'returns domains with delete time in the past' do diff --git a/spec/presenters/domain_presenter_spec.rb b/spec/presenters/domain_presenter_spec.rb index 3726960b4..d0cef5e6d 100644 --- a/spec/presenters/domain_presenter_spec.rb +++ b/spec/presenters/domain_presenter_spec.rb @@ -44,7 +44,7 @@ RSpec.describe DomainPresenter do subject(:delete_date) { presenter.delete_date } context 'when present' do - let(:domain) { instance_double(Domain, delete_time: '05.07.2010') } + let(:domain) { instance_double(Domain, delete_at: '05.07.2010') } it 'returns localized date' do expect(view).to receive(:l).with('05.07.2010', format: :date).and_return('delete date') @@ -53,7 +53,7 @@ RSpec.describe DomainPresenter do end context 'when absent' do - let(:domain) { instance_double(Domain, delete_time: nil) } + let(:domain) { instance_double(Domain, delete_at: nil) } specify { expect(delete_date).to be_nil } end From e776d09f9d99e4cc33ca8cd7666646510e0a0e67 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sun, 8 Apr 2018 00:36:00 +0300 Subject: [PATCH 03/61] Improve domain discard - Extract rake task domain:discard - Remove background job when keeping a domain #790 --- app/models/concerns/domain/deletable.rb | 31 +++++++++++++ app/models/domain.rb | 11 ----- app/models/domain_cron.rb | 16 ------- app/models/que_job.rb | 3 ++ config/schedule.rb | 4 ++ lib/tasks/domain.rake | 5 +++ spec/models/domain_spec.rb | 16 ------- .../epp/domain/transfer/request_test.rb | 2 +- test/integration/tasks/discard_domain_test.rb | 43 +++++++++++++++++++ test/models/domain/deletable_test.rb | 19 +++++++- 10 files changed, 104 insertions(+), 46 deletions(-) create mode 100644 app/models/que_job.rb create mode 100644 lib/tasks/domain.rake create mode 100644 test/integration/tasks/discard_domain_test.rb diff --git a/app/models/concerns/domain/deletable.rb b/app/models/concerns/domain/deletable.rb index 204d924ba..6075ef8f7 100644 --- a/app/models/concerns/domain/deletable.rb +++ b/app/models/concerns/domain/deletable.rb @@ -1,12 +1,43 @@ module Concerns::Domain::Deletable extend ActiveSupport::Concern + class_methods do + def discard_domains + domains = where('delete_at < ? AND ? != ALL(statuses) AND ? != ALL(statuses)', + Time.zone.now, + DomainStatus::SERVER_DELETE_PROHIBITED, + DomainStatus::DELETE_CANDIDATE) + + domains.map(&:discard) + end + end + def discard statuses << DomainStatus::DELETE_CANDIDATE + # We don't validate deliberately since nobody is interested in fixing discarded domain + save(validate: false) + delete_later + logger.info "Domain #{name} (ID: #{id}) is scheduled to be deleted" + end + + def keep + statuses.delete(DomainStatus::DELETE_CANDIDATE) save + do_not_delete_later end def discarded? statuses.include?(DomainStatus::DELETE_CANDIDATE) end + + private + + def delete_later + run_at = rand(((24 * 60) - (DateTime.now.hour * 60 + DateTime.now.minute))).minutes.from_now + DomainDeleteJob.enqueue(id, run_at: run_at) + end + + def do_not_delete_later + QueJob.find_by!("args->>0 = '#{id}'").delete + end end diff --git a/app/models/domain.rb b/app/models/domain.rb index a9fec134e..1425fc1e1 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -281,13 +281,6 @@ class Domain < ActiveRecord::Base true end - def delete_candidateable? - return false if delete_at > Time.zone.now - return false if statuses.include?(DomainStatus::DELETE_CANDIDATE) - return false if statuses.include?(DomainStatus::SERVER_DELETE_PROHIBITED) - true - end - def renewable? if Setting.days_to_renew_domain_before_expire != 0 # if you can renew domain at days_to_renew before domain expiration @@ -683,10 +676,6 @@ class Domain < ActiveRecord::Base where("#{attribute_alias(:outzone_time)} < ?", Time.zone.now) end - def self.delete_candidates - where('delete_at < ?', Time.zone.now) - end - def self.uses_zone?(zone) exists?(["name ILIKE ?", "%.#{zone.origin}"]) end diff --git a/app/models/domain_cron.rb b/app/models/domain_cron.rb index dcd99328a..6ca502b00 100644 --- a/app/models/domain_cron.rb +++ b/app/models/domain_cron.rb @@ -104,22 +104,6 @@ class DomainCron c = 0 - domains = Domain.delete_candidates - - domains.each do |domain| - next unless domain.delete_candidateable? - - domain.statuses << DomainStatus::DELETE_CANDIDATE - - # If domain successfully saved, add it to delete schedule - if domain.save(validate: false) - ::PaperTrail.whodunnit = "cron - #{__method__}" - DomainDeleteJob.enqueue(domain.id, run_at: rand(((24*60) - (DateTime.now.hour * 60 + DateTime.now.minute))).minutes.from_now) - STDOUT << "#{Time.zone.now.utc} Domain.destroy_delete_candidates: job added by deleteCandidate status ##{domain.id} (#{domain.name})\n" unless Rails.env.test? - c += 1 - end - end - Domain.where('force_delete_at <= ?', Time.zone.now.end_of_day.utc).each do |x| DomainDeleteJob.enqueue(x.id, run_at: rand(((24*60) - (DateTime.now.hour * 60 + DateTime.now.minute))).minutes.from_now) STDOUT << "#{Time.zone.now.utc} DomainCron.destroy_delete_candidates: job added by force delete time ##{x.id} (#{x.name})\n" unless Rails.env.test? diff --git a/app/models/que_job.rb b/app/models/que_job.rb new file mode 100644 index 000000000..d08e65d10 --- /dev/null +++ b/app/models/que_job.rb @@ -0,0 +1,3 @@ +class QueJob < ActiveRecord::Base # To be able to remove existing jobs + self.primary_key = 'job_id' +end diff --git a/config/schedule.rb b/config/schedule.rb index a136ba5c3..dd5741dd0 100644 --- a/config/schedule.rb +++ b/config/schedule.rb @@ -61,6 +61,10 @@ if @cron_group == 'registry' every :day, at: '19:00pm' do runner 'Directo.send_receipts' end if @environment == 'production' + + every 42.minutes do + rake 'domain:discard' + end end every 10.minutes do diff --git a/lib/tasks/domain.rake b/lib/tasks/domain.rake new file mode 100644 index 000000000..f59a76141 --- /dev/null +++ b/lib/tasks/domain.rake @@ -0,0 +1,5 @@ +namespace :domain do + task discard: :environment do + Domain.discard_domains + end +end diff --git a/spec/models/domain_spec.rb b/spec/models/domain_spec.rb index 6c09ef7ed..4f1d5d6a1 100644 --- a/spec/models/domain_spec.rb +++ b/spec/models/domain_spec.rb @@ -871,22 +871,6 @@ RSpec.describe Domain do end end - describe '::delete_candidates', db: true do - before :example do - travel_to Time.zone.parse('05.07.2010 00:00') - - create(:zone, origin: 'ee') - - create(:domain, id: 1, delete_at: Time.zone.parse('04.07.2010 23:59')) - create(:domain, id: 2, delete_at: Time.zone.parse('05.07.2010 00:00')) - create(:domain, id: 3, delete_at: Time.zone.parse('05.07.2010 00:01')) - end - - it 'returns domains with delete time in the past' do - expect(described_class.delete_candidates.ids).to eq([1]) - end - end - describe '::uses_zone?', db: true do let!(:zone) { create(:zone, origin: 'domain.tld') } diff --git a/test/integration/epp/domain/transfer/request_test.rb b/test/integration/epp/domain/transfer/request_test.rb index 996296257..aef189104 100644 --- a/test/integration/epp/domain/transfer/request_test.rb +++ b/test/integration/epp/domain/transfer/request_test.rb @@ -76,7 +76,7 @@ class EppDomainTransferRequestTest < ActionDispatch::IntegrationTest end def test_discarded_domain_cannot_be_transferred - @domain.update!(statuses: [DomainStatus::DELETE_CANDIDATE]) + @domain.discard post '/epp/command/transfer', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=api_goodnames' } @domain.reload diff --git a/test/integration/tasks/discard_domain_test.rb b/test/integration/tasks/discard_domain_test.rb new file mode 100644 index 000000000..0a341765b --- /dev/null +++ b/test/integration/tasks/discard_domain_test.rb @@ -0,0 +1,43 @@ +require 'test_helper' + +class DiscardDomainTaskTest < ActiveSupport::TestCase + setup do + travel_to Time.zone.parse('2010-07-05 08:00') + @domain = domains(:shop) + end + + def test_discard_domains_with_past_delete_at + @domain.update!(delete_at: Time.zone.parse('2010-07-05 07:59')) + Rake::Task['domain:discard'].execute + @domain.reload + assert @domain.discarded? + end + + def test_ignore_domains_with_delete_at_in_the_future_or_now + @domain.update!(delete_at: Time.zone.parse('2010-07-05 08:00')) + Rake::Task['domain:discard'].execute + @domain.reload + refute @domain.discarded? + end + + def test_ignore_already_discarded_domains + @domain.update!(delete_at: Time.zone.parse('2010-07-05 07:59')) + @domain.discard + + job_count = lambda do + QueJob.where("args->>0 = '#{@domain.id}'", job_class: DomainDeleteJob.name).count + end + + assert_no_difference job_count, 'A domain should not be discarded again' do + Rake::Task['domain:discard'].execute + end + end + + def test_ignore_domains_with_server_delete_prohibited_status + @domain.update!(delete_at: Time.zone.parse('2010-07-05 07:59'), + statuses: [DomainStatus::SERVER_DELETE_PROHIBITED]) + Rake::Task['domain:discard'].execute + @domain.reload + refute @domain.discarded? + end +end diff --git a/test/models/domain/deletable_test.rb b/test/models/domain/deletable_test.rb index dbec7bc7f..5e41968d7 100644 --- a/test/models/domain/deletable_test.rb +++ b/test/models/domain/deletable_test.rb @@ -5,10 +5,25 @@ class DomainDeletableTest < ActiveSupport::TestCase @domain = domains(:shop) end - def test_discard - refute @domain.discarded? + def test_discard_domain @domain.discard @domain.reload + assert QueJob.find_by("args->>0 = '#{@domain.id}'", job_class: DomainDeleteJob.name) assert @domain.discarded? end + + def test_discard_invalid_domain + domain = domains(:invalid) + domain.discard + domain.reload + assert domain.discarded?, 'a domain should be discarded' + end + + def test_keep_domain + @domain.discard + @domain.keep + @domain.reload + assert_nil QueJob.find_by("args->>0 = '#{@domain.id}'", job_class: DomainDeleteJob.name) + refute @domain.discarded? + end end From a932d43dda65cd89c4f0ac5e316795f18133d5fd Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sun, 8 Apr 2018 15:45:01 +0300 Subject: [PATCH 04/61] Improve UI #790 --- app/views/admin/domains/edit.html.erb | 24 +++++++++++++----------- config/locales/admin/domains.en.yml | 1 - 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/app/views/admin/domains/edit.html.erb b/app/views/admin/domains/edit.html.erb index 3f831e86b..6593f431b 100644 --- a/app/views/admin/domains/edit.html.erb +++ b/app/views/admin/domains/edit.html.erb @@ -1,20 +1,22 @@ <% domain = DomainPresenter.new(domain: @domain, view: self) %> -
-
-

- Edit: <%= domain.name %> -

-
-
-

+ + +

+
-
<%= render 'form' %> <%= render 'force_delete_dialog', domain: @domain, templates: force_delete_templates %> diff --git a/config/locales/admin/domains.en.yml b/config/locales/admin/domains.en.yml index 87c2c9c6a..aa574e8a3 100644 --- a/config/locales/admin/domains.en.yml +++ b/config/locales/admin/domains.en.yml @@ -14,7 +14,6 @@ en: edit: add_new_status_btn: Add new status - back_btn: Back to domain force_delete_dialog: title: Force delete From 4dc77235542dece7e80ece28bd2808fc71430f5a Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sun, 8 Apr 2018 15:53:29 +0300 Subject: [PATCH 05/61] Remove dead code #790 --- app/controllers/admin/domains_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/admin/domains_controller.rb b/app/controllers/admin/domains_controller.rb index 576b5f8e1..3cf4577fc 100644 --- a/app/controllers/admin/domains_controller.rb +++ b/app/controllers/admin/domains_controller.rb @@ -1,7 +1,7 @@ module Admin class DomainsController < BaseController load_and_authorize_resource - before_action :set_domain, only: [:show, :edit, :update, :zonefile] + before_action :set_domain, only: [:show, :edit, :update] helper_method :force_delete_templates # rubocop: disable Metrics/PerceivedComplexity From 9e46222e6bf5b2a3398f03b16b81a42124ebf0c9 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sun, 8 Apr 2018 15:54:42 +0300 Subject: [PATCH 06/61] Remove unused routes #790 --- config/routes.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index 9caeef4a2..883cad6a8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -186,7 +186,7 @@ Rails.application.routes.draw do match 'forward', via: [:post, :get] end - resources :domains do + resources :domains, except: %i[new create destroy] do resources :domain_versions, controller: 'domains', action: 'versions' resources :pending_updates resources :pending_deletes From b4ed8f6c8200fbaa3f5112144a5478c2890e5fbe Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sun, 8 Apr 2018 16:01:00 +0300 Subject: [PATCH 07/61] Remove double load #790 --- app/controllers/admin/domains_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/domains_controller.rb b/app/controllers/admin/domains_controller.rb index 3cf4577fc..f2e78aaf0 100644 --- a/app/controllers/admin/domains_controller.rb +++ b/app/controllers/admin/domains_controller.rb @@ -1,7 +1,7 @@ module Admin class DomainsController < BaseController - load_and_authorize_resource - before_action :set_domain, only: [:show, :edit, :update] + before_action :set_domain, only: %i[show edit update] + authorize_resource helper_method :force_delete_templates # rubocop: disable Metrics/PerceivedComplexity From 90a4e11ce4057ed92c13571d5e495b613bf53155 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sun, 8 Apr 2018 16:07:08 +0300 Subject: [PATCH 08/61] Improve readability #790 --- app/controllers/admin/domains_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/domains_controller.rb b/app/controllers/admin/domains_controller.rb index f2e78aaf0..9237d61af 100644 --- a/app/controllers/admin/domains_controller.rb +++ b/app/controllers/admin/domains_controller.rb @@ -40,7 +40,8 @@ module Admin # rubocop: enable Metrics/AbcSize def show - @domain.valid? + # Validation is needed to warn users + @domain.validate end def edit From cc4a2448442854c782db296815e9483ef6656ae1 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sun, 8 Apr 2018 18:52:32 +0300 Subject: [PATCH 09/61] Enable to keep a domain #790 --- app/controllers/admin/domains_controller.rb | 7 ++++++- app/models/domain_status.rb | 3 ++- app/presenters/domain_presenter.rb | 9 +++++++++ app/views/admin/domains/edit.html.erb | 5 +++-- config/locales/admin/domains.en.yml | 5 +++++ config/routes.rb | 4 ++++ test/integration/admin/domains/details_test.rb | 10 ++++++++++ 7 files changed, 39 insertions(+), 4 deletions(-) diff --git a/app/controllers/admin/domains_controller.rb b/app/controllers/admin/domains_controller.rb index 9237d61af..0718d600a 100644 --- a/app/controllers/admin/domains_controller.rb +++ b/app/controllers/admin/domains_controller.rb @@ -1,6 +1,6 @@ module Admin class DomainsController < BaseController - before_action :set_domain, only: %i[show edit update] + before_action :set_domain, only: %i[show edit update keep] authorize_resource helper_method :force_delete_templates @@ -68,6 +68,11 @@ module Admin @versions = @domain.versions end + def keep + @domain.keep + redirect_to edit_admin_domain_url(@domain), notice: t('.kept') + end + private def set_domain diff --git a/app/models/domain_status.rb b/app/models/domain_status.rb index 4da6a4a3c..3c4e09beb 100644 --- a/app/models/domain_status.rb +++ b/app/models/domain_status.rb @@ -175,7 +175,8 @@ class DomainStatus < ActiveRecord::Base PENDING_RENEW, PENDING_TRANSFER, PENDING_UPDATE, - PENDING_DELETE_CONFIRMATION + PENDING_DELETE_CONFIRMATION, + DELETE_CANDIDATE, ] end end diff --git a/app/presenters/domain_presenter.rb b/app/presenters/domain_presenter.rb index 2ecb82922..11d767203 100644 --- a/app/presenters/domain_presenter.rb +++ b/app/presenters/domain_presenter.rb @@ -67,6 +67,15 @@ class DomainPresenter end end + def keep_btn + return unless domain.discarded? + + view.link_to view.t('admin.domains.edit.keep_btn'), view.keep_admin_domain_path(@domain), + method: :patch, + data: { confirm: view.t('admin.domains.edit.keep_btn_confirm') }, + class: 'btn btn-default' + end + private attr_reader :domain diff --git a/app/views/admin/domains/edit.html.erb b/app/views/admin/domains/edit.html.erb index 6593f431b..24735734c 100644 --- a/app/views/admin/domains/edit.html.erb +++ b/app/views/admin/domains/edit.html.erb @@ -7,12 +7,13 @@
diff --git a/app/views/admin/domains/partials/_general.html.erb b/app/views/admin/domains/partials/_general.html.erb index 7e09a7756..cf57e64d0 100644 --- a/app/views/admin/domains/partials/_general.html.erb +++ b/app/views/admin/domains/partials/_general.html.erb @@ -33,6 +33,9 @@
<%= t('.force_delete_time') %>
<%= l(@domain.force_delete_at) %>
+ +
<%= t('.locked_by_registrant_at') %>
+
<%= l(@domain.locked_by_registrant_at) %>
diff --git a/config/locales/admin/domains.en.yml b/config/locales/admin/domains.en.yml index 87c2c9c6a..bbe23e990 100644 --- a/config/locales/admin/domains.en.yml +++ b/config/locales/admin/domains.en.yml @@ -23,6 +23,12 @@ en: close_btn: Close dialog submit_btn: Force delete domain + registry_lock_delete: + btn: Remove registry lock + confirm: Are you sure you want to remove registry lock that was set by registrant? + success: Registry lock removed + error: Registry lock could not be removed + versions: time: Time registrant: Registrant @@ -34,6 +40,7 @@ en: outzone_time: Outzone time delete_time: Delete time force_delete_time: Force delete time + locked_by_registrant_at: Registry lock time admin_contacts: title: Admin. contacts diff --git a/config/routes.rb b/config/routes.rb index 3ae18a7cd..c79822f28 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -182,6 +182,7 @@ Rails.application.routes.draw do resources :pending_updates resources :pending_deletes resource :force_delete, controller: 'domains/force_delete', only: %i[create destroy] + resource :registry_lock, controller: 'domains/registry_lock', only: :destroy end resources :domain_versions do diff --git a/test/models/domain/lockable_test.rb b/test/models/domain/lockable_test.rb index 5b62df355..2f14f99da 100644 --- a/test/models/domain/lockable_test.rb +++ b/test/models/domain/lockable_test.rb @@ -4,7 +4,7 @@ class DomainLockableTest < ActiveSupport::TestCase def setup super - @domain = domains(:shop) + @domain = domains(:airport) end def test_registry_lock_on_lockable_domain @@ -19,18 +19,21 @@ class DomainLockableTest < ActiveSupport::TestCase ) assert(@domain.locked_by_registrant?) + assert(@domain.locked_by_registrant_at) end def test_registry_lock_cannot_be_applied_twice @domain.apply_registry_lock refute(@domain.apply_registry_lock) assert(@domain.locked_by_registrant?) + assert(@domain.locked_by_registrant_at) end def test_registry_lock_cannot_be_applied_on_pending_statuses @domain.statuses << DomainStatus::PENDING_RENEW refute(@domain.apply_registry_lock) refute(@domain.locked_by_registrant?) + refute(@domain.locked_by_registrant_at) end def test_remove_registry_lock_on_locked_domain @@ -47,6 +50,7 @@ class DomainLockableTest < ActiveSupport::TestCase assert_equal(["ok"], @domain.statuses) refute(@domain.locked_by_registrant?) + refute(@domain.locked_by_registrant_at) end def test_remove_registry_lock_on_non_locked_domain @@ -55,5 +59,14 @@ class DomainLockableTest < ActiveSupport::TestCase assert_equal([], @domain.statuses) refute(@domain.locked_by_registrant?) + refute(@domain.locked_by_registrant_at) + end + + def test_registry_lock_cannot_be_removed_if_statuses_were_set_by_admin + @domain.statuses << DomainStatus::SERVER_UPDATE_PROHIBITED + @domain.statuses << DomainStatus::SERVER_DELETE_PROHIBITED + @domain.statuses << DomainStatus::SERVER_TRANSFER_PROHIBITED + + refute(@domain.remove_registry_lock) end end diff --git a/test/system/admin_area/domains/registry_lock_test.rb b/test/system/admin_area/domains/registry_lock_test.rb new file mode 100644 index 000000000..47b1f8329 --- /dev/null +++ b/test/system/admin_area/domains/registry_lock_test.rb @@ -0,0 +1,55 @@ +require 'test_helper' + +class RegistryLockTest < JavaScriptApplicationSystemTestCase + def setup + super + WebMock.allow_net_connect! + + sign_in users(:admin) + travel_to Time.zone.parse('2010-07-05 00:30:00') + @domain = domains(:airport) + end + + def teardown + travel_back + end + + def test_does_not_have_link_when_domain_is_not_locked + visit edit_admin_domain_path(@domain) + refute(page.has_link?('Remove registry lock')) + end + + def test_can_remove_registry_lock_from_a_domain + @domain.apply_registry_lock + + visit edit_admin_domain_path(@domain) + click_link_or_button('Actions') + assert(page.has_link?('Remove registry lock')) + + accept_confirm('Are you sure you want to remove registry lock that was set by registrant?') do + click_link_or_button('Remove registry lock') + end + + assert_text('Registry lock removed') + + @domain.reload + refute @domain.locked_by_registrant? + end + + def test_cannot_remove_registry_lock_from_not_locked_domain + @domain.apply_registry_lock + visit edit_admin_domain_path(@domain) + @domain.remove_registry_lock + + refute @domain.locked_by_registrant? + + click_link_or_button('Actions') + assert(page.has_link?('Remove registry lock')) + + accept_confirm('Are you sure you want to remove registry lock that was set by registrant?') do + click_link_or_button('Remove registry lock') + end + + assert_text('Registry lock could not be removed') + end +end diff --git a/test/system/admin_area/domains_test.rb b/test/system/admin_area/domains_test.rb index 538de2604..8103f2a8a 100644 --- a/test/system/admin_area/domains_test.rb +++ b/test/system/admin_area/domains_test.rb @@ -3,11 +3,27 @@ require 'test_helper' class AdminDomainsTestTest < ApplicationSystemTestCase setup do sign_in users(:admin) + travel_to Time.zone.parse('2010-07-05 00:30:00') + @domain = domains(:shop) + end + + teardown do + travel_back end def test_shows_details - domain = domains(:shop) - visit admin_domain_path(domain) - assert_field nil, with: domain.transfer_code + visit admin_domain_path(@domain) + assert_field nil, with: @domain.transfer_code + end + + def test_admin_registry_lock_date + visit admin_domain_path(@domain) + refute_text 'Registry lock time 2010-07-05 00:30' + + lockable_domain = domains(:airport) + lockable_domain.apply_registry_lock + + visit admin_domain_path(lockable_domain) + assert_text 'Registry lock time 2010-07-05 00:30' end end From 7ce092dff2626cda61a501ea8da9fed65a0865ba Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Wed, 8 Aug 2018 15:49:37 +0300 Subject: [PATCH 25/61] Add final assertion To make sure that the domain is in the right state after out-of-order execution of the lock removal --- test/system/admin_area/domains/registry_lock_test.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/test/system/admin_area/domains/registry_lock_test.rb b/test/system/admin_area/domains/registry_lock_test.rb index 47b1f8329..6c741ba60 100644 --- a/test/system/admin_area/domains/registry_lock_test.rb +++ b/test/system/admin_area/domains/registry_lock_test.rb @@ -51,5 +51,6 @@ class RegistryLockTest < JavaScriptApplicationSystemTestCase end assert_text('Registry lock could not be removed') + refute @domain.locked_by_registrant? end end From faeeb55bc77d95c83619d9221ba896471400bf0c Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Thu, 9 Aug 2018 13:57:31 +0300 Subject: [PATCH 26/61] Add badges to locked domains --- .../registrant/registry_locks_controller.rb | 37 +++++++++++++++++++ app/presenters/domain_presenter.rb | 5 +++ app/views/admin/domains/edit.html.erb | 6 +-- app/views/admin/domains/show.html.erb | 2 +- config/locales/admin/domains.en.yml | 3 +- config/routes.rb | 4 +- ...egistrant_api_domain_registry_lock_test.rb | 28 ++++++++++++++ .../admin_area/domains/registry_lock_test.rb | 2 +- 8 files changed, 80 insertions(+), 7 deletions(-) create mode 100644 app/controllers/api/v1/registrant/registry_locks_controller.rb create mode 100644 test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb diff --git a/app/controllers/api/v1/registrant/registry_locks_controller.rb b/app/controllers/api/v1/registrant/registry_locks_controller.rb new file mode 100644 index 000000000..c3ec073b6 --- /dev/null +++ b/app/controllers/api/v1/registrant/registry_locks_controller.rb @@ -0,0 +1,37 @@ +module Api + module V1 + module Registrant + class RegistryLocksController < BaseController + before_action :set_domain + + def create + if @domain.apply_registry_lock + render json: @domain + else + render json: { errors: [{ base: 'Domain cannot be locked' }] }, + status: :unprocessable_entity + end + end + + def delete + if @domain.remove_registry_lock + render json: @domain + else + render json: { errors: [{ base: 'Domain cannot be unlocked' }] }, + status: :unprocessable_entity + end + end + + private + + def set_domain + @domain = Domain.find_by(uuid: params[:domain_uuid]) + + return if @domain + render json: { errors: [{ base: ['Domain not found'] }] }, + status: :not_found and return + end + end + end + end +end diff --git a/app/presenters/domain_presenter.rb b/app/presenters/domain_presenter.rb index 4a41a06a3..5ae622353 100644 --- a/app/presenters/domain_presenter.rb +++ b/app/presenters/domain_presenter.rb @@ -14,6 +14,11 @@ class DomainPresenter html += " #{label}" end + if domain.locked_by_registrant? + label = view.content_tag(:span, 'registryLock', class: 'label label-danger') + html += " #{label}" + end + html.html_safe end diff --git a/app/views/admin/domains/edit.html.erb b/app/views/admin/domains/edit.html.erb index 335d3b574..19c093f78 100644 --- a/app/views/admin/domains/edit.html.erb +++ b/app/views/admin/domains/edit.html.erb @@ -1,12 +1,12 @@ <% domain = DomainPresenter.new(domain: @domain, view: self) %>
-
+

- Edit: <%= domain.name %> + Edit: <%= domain.name_with_status %>

-
+
<%= link_to t('.back_btn'), [:admin, @domain], class: 'btn btn-default' %>
diff --git a/app/views/admin/domains/show.html.erb b/app/views/admin/domains/show.html.erb index 1501b35bb..f712a8b15 100644 --- a/app/views/admin/domains/show.html.erb +++ b/app/views/admin/domains/show.html.erb @@ -13,7 +13,7 @@
<%= link_to t('.edit_btn'), edit_admin_domain_path(@domain), class: 'btn btn-primary' %> <%= link_to t('.history_btn'), admin_domain_domain_versions_path(@domain), - class: 'btn btn-primary' %> + class: 'btn btn-primary' %>
diff --git a/config/locales/admin/domains.en.yml b/config/locales/admin/domains.en.yml index bbe23e990..bb157fe9c 100644 --- a/config/locales/admin/domains.en.yml +++ b/config/locales/admin/domains.en.yml @@ -25,7 +25,8 @@ en: registry_lock_delete: btn: Remove registry lock - confirm: Are you sure you want to remove registry lock that was set by registrant? + banner: Domain has a registry lock set by registrant. + confirm: Are you sure you want to remove the registry lock? success: Registry lock removed error: Registry lock could not be removed diff --git a/config/routes.rb b/config/routes.rb index c79822f28..63f71a344 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -23,7 +23,9 @@ Rails.application.routes.draw do namespace :registrant do post 'auth/eid', to: 'auth#eid' - resources :domains, only: [:index] + resources :domains, only: [:index], param: :uuid do + resource :registry_lock, only: [:create, :destroy] + end end end end diff --git a/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb b/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb new file mode 100644 index 000000000..d05dec3d5 --- /dev/null +++ b/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb @@ -0,0 +1,28 @@ +require 'test_helper' +require 'auth_token/auth_token_creator' + +class RegistrantApiDomainRegistryLockTest < ApplicationIntegrationTest + def setup + super + + @user = users(:registrant) + @domain = domains(:airport) + @auth_headers = { 'HTTP_AUTHORIZATION' => auth_token } + end + + def test_can_lock_a_not_locked_domain + assert(@domain.locked_by_registrant?) + end + + def test_cannot_lock_an_already_locked_domain + assert(@domain.locked_by_registrant?) + end + + private + + def auth_token + token_creator = AuthTokenCreator.create_with_defaults(@user) + hash = token_creator.token_in_hash + "Bearer #{hash[:access_token]}" + end +end diff --git a/test/system/admin_area/domains/registry_lock_test.rb b/test/system/admin_area/domains/registry_lock_test.rb index 6c741ba60..3fc053f6a 100644 --- a/test/system/admin_area/domains/registry_lock_test.rb +++ b/test/system/admin_area/domains/registry_lock_test.rb @@ -26,7 +26,7 @@ class RegistryLockTest < JavaScriptApplicationSystemTestCase click_link_or_button('Actions') assert(page.has_link?('Remove registry lock')) - accept_confirm('Are you sure you want to remove registry lock that was set by registrant?') do + accept_confirm('Are you sure you want to remove registry lock?') do click_link_or_button('Remove registry lock') end From 13f2bff8d0d00dbd4a822eb9825fa17ba90bdce7 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Thu, 9 Aug 2018 15:05:28 +0300 Subject: [PATCH 27/61] Use setup callback --- test/models/domain/discardable_test.rb | 4 ++-- test/models/domain/force_delete_test.rb | 2 +- test/system/admin_area/domains_test.rb | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/models/domain/discardable_test.rb b/test/models/domain/discardable_test.rb index dad23b17b..0181a6bb4 100644 --- a/test/models/domain/discardable_test.rb +++ b/test/models/domain/discardable_test.rb @@ -1,13 +1,13 @@ require 'test_helper' class DomainDiscardableTest < ActiveSupport::TestCase - def setup + setup do travel_to Time.zone.parse('2010-07-05 10:30') @domain = domains(:shop) @domain.delete_at = Time.zone.parse('2010-07-05 10:00') end - def teardown + teardown do travel_back end diff --git a/test/models/domain/force_delete_test.rb b/test/models/domain/force_delete_test.rb index 535ad84d8..70db52659 100644 --- a/test/models/domain/force_delete_test.rb +++ b/test/models/domain/force_delete_test.rb @@ -1,7 +1,7 @@ require 'test_helper' class DomainForceDeleteTest < ActiveSupport::TestCase - def setup + setup do @domain = domains(:shop) end diff --git a/test/system/admin_area/domains_test.rb b/test/system/admin_area/domains_test.rb index 83ceac3d5..cd7a8f10a 100644 --- a/test/system/admin_area/domains_test.rb +++ b/test/system/admin_area/domains_test.rb @@ -1,12 +1,12 @@ require 'test_helper' class AdminDomainsTestTest < ApplicationSystemTestCase - def setup + setup do sign_in users(:admin) @domain = domains(:shop) end - def teardown + teardown do travel_back end From 2a7caaa33e0d78f36a0e46ffebf2f8111359e3fc Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 10 Aug 2018 13:17:20 +0300 Subject: [PATCH 28/61] Update routes --- config/routes.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index 63f71a344..a2b2ca790 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -23,8 +23,8 @@ Rails.application.routes.draw do namespace :registrant do post 'auth/eid', to: 'auth#eid' - resources :domains, only: [:index], param: :uuid do - resource :registry_lock, only: [:create, :destroy] + resources :domains, only: %i[index show], param: :uuid do + resource :registry_lock, only: %i[create destroy] end end end From 0a0962e0079113528d062b75bbb9b750e0bf996e Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 10 Aug 2018 14:18:24 +0300 Subject: [PATCH 29/61] Write tests around API for domain lock --- .../api/v1/registrant/base_controller.rb | 9 +++ .../api/v1/registrant/domains_controller.rb | 11 --- .../registrant/registry_locks_controller.rb | 9 +-- ...egistrant_api_domain_registry_lock_test.rb | 71 +++++++++++++++++++ .../admin_area/domains/registry_lock_test.rb | 5 +- test/system/admin_area/domains_test.rb | 1 + 6 files changed, 89 insertions(+), 17 deletions(-) diff --git a/app/controllers/api/v1/registrant/base_controller.rb b/app/controllers/api/v1/registrant/base_controller.rb index 4df0d226c..06dfd8804 100644 --- a/app/controllers/api/v1/registrant/base_controller.rb +++ b/app/controllers/api/v1/registrant/base_controller.rb @@ -22,6 +22,15 @@ module Api header.gsub(pattern, '') if header&.match(pattern) end + def associated_domains(user) + country_code, ident = user.registrant_ident.split('-') + + BusinessRegistryCache.fetch_associated_domains(ident, country_code) + rescue Soap::Arireg::NotAvailableError => error + Rails.logger.fatal("[EXCEPTION] #{error}") + user.domains + end + def authenticate decryptor = AuthTokenDecryptor.create_with_defaults(bearer_token) decryptor.decrypt_token diff --git a/app/controllers/api/v1/registrant/domains_controller.rb b/app/controllers/api/v1/registrant/domains_controller.rb index 27b7b6125..49e950155 100644 --- a/app/controllers/api/v1/registrant/domains_controller.rb +++ b/app/controllers/api/v1/registrant/domains_controller.rb @@ -32,17 +32,6 @@ module Api render json: { errors: [{ base: ['Domain not found'] }] }, status: :not_found end end - - private - - def associated_domains(user) - country_code, ident = user.registrant_ident.split('-') - - BusinessRegistryCache.fetch_associated_domains(ident, country_code) - rescue Soap::Arireg::NotAvailableError => error - Rails.logger.fatal("[EXCEPTION] #{error}") - user.domains - end end end end diff --git a/app/controllers/api/v1/registrant/registry_locks_controller.rb b/app/controllers/api/v1/registrant/registry_locks_controller.rb index c3ec073b6..212d8bc21 100644 --- a/app/controllers/api/v1/registrant/registry_locks_controller.rb +++ b/app/controllers/api/v1/registrant/registry_locks_controller.rb @@ -8,16 +8,16 @@ module Api if @domain.apply_registry_lock render json: @domain else - render json: { errors: [{ base: 'Domain cannot be locked' }] }, + render json: { errors: [{ base: ['Domain cannot be locked'] }] }, status: :unprocessable_entity end end - def delete + def destroy if @domain.remove_registry_lock render json: @domain else - render json: { errors: [{ base: 'Domain cannot be unlocked' }] }, + render json: { errors: [{ base: ['Domain cannot be unlocked'] }] }, status: :unprocessable_entity end end @@ -25,7 +25,8 @@ module Api private def set_domain - @domain = Domain.find_by(uuid: params[:domain_uuid]) + domain_pool = associated_domains(current_user) + @domain = domain_pool.find_by(uuid: params[:domain_uuid]) return if @domain render json: { errors: [{ base: ['Domain not found'] }] }, diff --git a/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb b/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb index d05dec3d5..89bb80d97 100644 --- a/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb +++ b/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb @@ -5,17 +5,88 @@ class RegistrantApiDomainRegistryLockTest < ApplicationIntegrationTest def setup super + @original_registry_time = Setting.days_to_keep_business_registry_cache + Setting.days_to_keep_business_registry_cache = 1 + travel_to Time.zone.parse('2010-07-05') + @user = users(:registrant) @domain = domains(:airport) @auth_headers = { 'HTTP_AUTHORIZATION' => auth_token } end + def teardown + super + + Setting.days_to_keep_business_registry_cache = @original_registry_time + travel_back + end + def test_can_lock_a_not_locked_domain + post '/api/v1/registrant/domains/2df2c1a1-8f6a-490a-81be-8bdf29866880/registry_lock', + {}, @auth_headers + + response_json = JSON.parse(response.body, symbolize_names: true) + + assert(response_json[:statuses].include?(DomainStatus::SERVER_DELETE_PROHIBITED)) + assert(response_json[:statuses].include?(DomainStatus::SERVER_TRANSFER_PROHIBITED)) + assert(response_json[:statuses].include?(DomainStatus::SERVER_UPDATE_PROHIBITED)) + + @domain.reload assert(@domain.locked_by_registrant?) end + def test_cannot_lock_a_domain_in_pending_state + @domain.statuses << DomainStatus::PENDING_UPDATE + @domain.save + + post '/api/v1/registrant/domains/2df2c1a1-8f6a-490a-81be-8bdf29866880/registry_lock', + {}, @auth_headers + + response_json = JSON.parse(response.body, symbolize_names: true) + assert_equal(422, response.status) + assert_equal({ errors: [{ base: ['Domain cannot be locked'] }] }, response_json) + end + def test_cannot_lock_an_already_locked_domain + @domain.apply_registry_lock assert(@domain.locked_by_registrant?) + + post '/api/v1/registrant/domains/2df2c1a1-8f6a-490a-81be-8bdf29866880/registry_lock', + {}, @auth_headers + + response_json = JSON.parse(response.body, symbolize_names: true) + assert_equal(422, response.status) + assert_equal({ errors: [{ base: ['Domain cannot be locked'] }] }, response_json) + end + + def test_can_unlock_a_locked_domain + @domain.apply_registry_lock + + delete '/api/v1/registrant/domains/2df2c1a1-8f6a-490a-81be-8bdf29866880/registry_lock', + {}, @auth_headers + + response_json = JSON.parse(response.body, symbolize_names: true) + assert(response_json[:statuses].include?(DomainStatus::OK)) + @domain.reload + refute(@domain.locked_by_registrant?) + end + + def test_cannot_unlock_a_not_locked_domain + delete '/api/v1/registrant/domains/2df2c1a1-8f6a-490a-81be-8bdf29866880/registry_lock', + {}, @auth_headers + + response_json = JSON.parse(response.body, symbolize_names: true) + assert_equal(422, response.status) + assert_equal({ errors: [{ base: ['Domain cannot be unlocked'] }] }, response_json) + end + + def test_returns_404_when_domain_is_not_found + post '/api/v1/registrant/domains/random-uuid/registry_lock', + {}, @auth_headers + + response_json = JSON.parse(response.body, symbolize_names: true) + assert_equal(404, response.status) + assert_equal({ errors: [{ base: ['Domain not found'] }] }, response_json) end private diff --git a/test/system/admin_area/domains/registry_lock_test.rb b/test/system/admin_area/domains/registry_lock_test.rb index 3fc053f6a..9a3276943 100644 --- a/test/system/admin_area/domains/registry_lock_test.rb +++ b/test/system/admin_area/domains/registry_lock_test.rb @@ -16,6 +16,7 @@ class RegistryLockTest < JavaScriptApplicationSystemTestCase def test_does_not_have_link_when_domain_is_not_locked visit edit_admin_domain_path(@domain) + click_link_or_button('Actions') refute(page.has_link?('Remove registry lock')) end @@ -26,7 +27,7 @@ class RegistryLockTest < JavaScriptApplicationSystemTestCase click_link_or_button('Actions') assert(page.has_link?('Remove registry lock')) - accept_confirm('Are you sure you want to remove registry lock?') do + accept_confirm('Are you sure you want to remove the registry lock?') do click_link_or_button('Remove registry lock') end @@ -46,7 +47,7 @@ class RegistryLockTest < JavaScriptApplicationSystemTestCase click_link_or_button('Actions') assert(page.has_link?('Remove registry lock')) - accept_confirm('Are you sure you want to remove registry lock that was set by registrant?') do + accept_confirm('Are you sure you want to remove the registry lock?') do click_link_or_button('Remove registry lock') end diff --git a/test/system/admin_area/domains_test.rb b/test/system/admin_area/domains_test.rb index 8103f2a8a..fddb1b328 100644 --- a/test/system/admin_area/domains_test.rb +++ b/test/system/admin_area/domains_test.rb @@ -25,5 +25,6 @@ class AdminDomainsTestTest < ApplicationSystemTestCase visit admin_domain_path(lockable_domain) assert_text 'Registry lock time 2010-07-05 00:30' + assert_text 'registryLock' end end From d26f073e1f4c453027bafda768789dd7967a2b36 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 10 Aug 2018 14:22:50 +0300 Subject: [PATCH 30/61] Revert indentation change --- app/views/admin/domains/show.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/admin/domains/show.html.erb b/app/views/admin/domains/show.html.erb index f712a8b15..1501b35bb 100644 --- a/app/views/admin/domains/show.html.erb +++ b/app/views/admin/domains/show.html.erb @@ -13,7 +13,7 @@
<%= link_to t('.edit_btn'), edit_admin_domain_path(@domain), class: 'btn btn-primary' %> <%= link_to t('.history_btn'), admin_domain_domain_versions_path(@domain), - class: 'btn btn-primary' %> + class: 'btn btn-primary' %>
From 9d7dc596525e08ea772d9e1eca11af66f3864b77 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 10 Aug 2018 14:43:06 +0300 Subject: [PATCH 31/61] Fix rubocop issues and a stray typo spotted along the way --- .../admin/domains/registry_lock_controller.rb | 7 ++++--- app/controllers/admin/mail_templates_controller.rb | 2 +- app/models/concerns/domain/lockable.rb | 14 +++++--------- app/presenters/domain_presenter.rb | 14 ++++++-------- 4 files changed, 16 insertions(+), 21 deletions(-) diff --git a/app/controllers/admin/domains/registry_lock_controller.rb b/app/controllers/admin/domains/registry_lock_controller.rb index 83304fb72..6895ac711 100644 --- a/app/controllers/admin/domains/registry_lock_controller.rb +++ b/app/controllers/admin/domains/registry_lock_controller.rb @@ -1,14 +1,15 @@ module Admin module Domains class RegistryLockController < BaseController - def destroy set_domain authorize! :manage, @domain if @domain.remove_registry_lock - redirect_to edit_admin_domain_url(@domain), notice: t('admin.domains.registry_lock_delete.success') + redirect_to edit_admin_domain_url(@domain), + notice: t('admin.domains.registry_lock_delete.success') else - redirect_to edit_admin_domain_url(@domain), alert: t('admin.domains.registry_lock_delete.error') + redirect_to edit_admin_domain_url(@domain), + alert: t('admin.domains.registry_lock_delete.error') end end diff --git a/app/controllers/admin/mail_templates_controller.rb b/app/controllers/admin/mail_templates_controller.rb index 93141ade6..d7ec4e6d0 100644 --- a/app/controllers/admin/mail_templates_controller.rb +++ b/app/controllers/admin/mail_templates_controller.rb @@ -47,7 +47,7 @@ module Admin def destroy @mail_template = MailTemplate.find(params[:id]) if @mail_template.destroy - redirect_to admin_mail_templates_path, notise: t(:deleted) + redirect_to admin_mail_templates_path, notice: t(:deleted) else flash.now[:alert] = I18n.t(:failure) render 'show' diff --git a/app/models/concerns/domain/lockable.rb b/app/models/concerns/domain/lockable.rb index 8292138ed..ae7e215b7 100644 --- a/app/models/concerns/domain/lockable.rb +++ b/app/models/concerns/domain/lockable.rb @@ -15,15 +15,11 @@ module Concerns::Domain::Lockable end def registry_lockable? - (statuses & [ - DomainStatus::PENDING_DELETE_CONFIRMATION, - DomainStatus::PENDING_CREATE, - DomainStatus::PENDING_UPDATE, - DomainStatus::PENDING_DELETE, - DomainStatus::PENDING_RENEW, - DomainStatus::PENDING_TRANSFER, - DomainStatus::FORCE_DELETE, - ]).empty? + (statuses & [DomainStatus::PENDING_DELETE_CONFIRMATION, + DomainStatus::PENDING_CREATE, DomainStatus::PENDING_UPDATE, + DomainStatus::PENDING_DELETE, DomainStatus::PENDING_RENEW, + DomainStatus::PENDING_TRANSFER, DomainStatus::FORCE_DELETE + ]).empty? end def locked_by_registrant? diff --git a/app/presenters/domain_presenter.rb b/app/presenters/domain_presenter.rb index 5ae622353..59568f83c 100644 --- a/app/presenters/domain_presenter.rb +++ b/app/presenters/domain_presenter.rb @@ -72,15 +72,13 @@ class DomainPresenter end end - def remove_registry_lock_btn - if domain.locked_by_registrant? - view.link_to(view.t('admin.domains.registry_lock_delete.btn'), - view.admin_domain_registry_lock_path(domain), - method: :delete, - data: { confirm: view.t('admin.domains.registry_lock_delete.confirm') }, - class: 'dropdown-item') - end + return unless domain.locked_by_registrant? + view.link_to(view.t('admin.domains.registry_lock_delete.btn'), + view.admin_domain_registry_lock_path(domain), + method: :delete, + data: { confirm: view.t('admin.domains.registry_lock_delete.confirm') }, + class: 'dropdown-item') end private From 322d93185625b58cbb28a1df233aa408a4f0b2df Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 10 Aug 2018 15:18:53 +0300 Subject: [PATCH 32/61] Further massage rubocop issues --- .../admin/domains/registry_lock_controller.rb | 6 +-- app/models/concerns/domain/lockable.rb | 49 ----------------- .../concerns/domain/registry_lockable.rb | 53 +++++++++++++++++++ app/models/domain.rb | 2 +- app/presenters/domain_presenter.rb | 4 +- config/locales/admin/domains.en.yml | 12 ++--- ...able_test.rb => registry_lockable_test.rb} | 2 +- 7 files changed, 65 insertions(+), 63 deletions(-) delete mode 100644 app/models/concerns/domain/lockable.rb create mode 100644 app/models/concerns/domain/registry_lockable.rb rename test/models/domain/{lockable_test.rb => registry_lockable_test.rb} (97%) diff --git a/app/controllers/admin/domains/registry_lock_controller.rb b/app/controllers/admin/domains/registry_lock_controller.rb index 6895ac711..cd04c5f2a 100644 --- a/app/controllers/admin/domains/registry_lock_controller.rb +++ b/app/controllers/admin/domains/registry_lock_controller.rb @@ -5,11 +5,9 @@ module Admin set_domain authorize! :manage, @domain if @domain.remove_registry_lock - redirect_to edit_admin_domain_url(@domain), - notice: t('admin.domains.registry_lock_delete.success') + redirect_to edit_admin_domain_url(@domain), notice: t('.success') else - redirect_to edit_admin_domain_url(@domain), - alert: t('admin.domains.registry_lock_delete.error') + redirect_to edit_admin_domain_url(@domain), alert: t('.error') end end diff --git a/app/models/concerns/domain/lockable.rb b/app/models/concerns/domain/lockable.rb deleted file mode 100644 index ae7e215b7..000000000 --- a/app/models/concerns/domain/lockable.rb +++ /dev/null @@ -1,49 +0,0 @@ -module Concerns::Domain::Lockable - extend ActiveSupport::Concern - - def apply_registry_lock - return unless registry_lockable? - return if locked_by_registrant? - - transaction do - statuses << DomainStatus::SERVER_UPDATE_PROHIBITED - statuses << DomainStatus::SERVER_DELETE_PROHIBITED - statuses << DomainStatus::SERVER_TRANSFER_PROHIBITED - self.locked_by_registrant_at = Time.zone.now - save - end - end - - def registry_lockable? - (statuses & [DomainStatus::PENDING_DELETE_CONFIRMATION, - DomainStatus::PENDING_CREATE, DomainStatus::PENDING_UPDATE, - DomainStatus::PENDING_DELETE, DomainStatus::PENDING_RENEW, - DomainStatus::PENDING_TRANSFER, DomainStatus::FORCE_DELETE - ]).empty? - end - - def locked_by_registrant? - return false unless locked_by_registrant_at - - lock_statuses = [ - DomainStatus::SERVER_UPDATE_PROHIBITED, - DomainStatus::SERVER_DELETE_PROHIBITED, - DomainStatus::SERVER_TRANSFER_PROHIBITED, - ] - - (statuses & lock_statuses).count == 3 - end - - def remove_registry_lock - return unless locked_by_registrant? - - transaction do - statuses.delete(DomainStatus::SERVER_UPDATE_PROHIBITED) - statuses.delete(DomainStatus::SERVER_DELETE_PROHIBITED) - statuses.delete(DomainStatus::SERVER_TRANSFER_PROHIBITED) - self.locked_by_registrant_at = nil - - save - end - end -end diff --git a/app/models/concerns/domain/registry_lockable.rb b/app/models/concerns/domain/registry_lockable.rb new file mode 100644 index 000000000..7a806559e --- /dev/null +++ b/app/models/concerns/domain/registry_lockable.rb @@ -0,0 +1,53 @@ +module Concerns + module Domain + module RegistryLockable + extend ActiveSupport::Concern + + def apply_registry_lock + return unless registry_lockable? + return if locked_by_registrant? + + transaction do + statuses << DomainStatus::SERVER_UPDATE_PROHIBITED + statuses << DomainStatus::SERVER_DELETE_PROHIBITED + statuses << DomainStatus::SERVER_TRANSFER_PROHIBITED + self.locked_by_registrant_at = Time.zone.now + save + end + end + + def registry_lockable? + (statuses & [DomainStatus::PENDING_DELETE_CONFIRMATION, + DomainStatus::PENDING_CREATE, DomainStatus::PENDING_UPDATE, + DomainStatus::PENDING_DELETE, DomainStatus::PENDING_RENEW, + DomainStatus::PENDING_TRANSFER, DomainStatus::FORCE_DELETE + ]).empty? + end + + def locked_by_registrant? + return false unless locked_by_registrant_at + + lock_statuses = [ + DomainStatus::SERVER_UPDATE_PROHIBITED, + DomainStatus::SERVER_DELETE_PROHIBITED, + DomainStatus::SERVER_TRANSFER_PROHIBITED, + ] + + (statuses & lock_statuses).count == 3 + end + + def remove_registry_lock + return unless locked_by_registrant? + + transaction do + statuses.delete(DomainStatus::SERVER_UPDATE_PROHIBITED) + statuses.delete(DomainStatus::SERVER_DELETE_PROHIBITED) + statuses.delete(DomainStatus::SERVER_TRANSFER_PROHIBITED) + self.locked_by_registrant_at = nil + + save + end + end + end + end +end diff --git a/app/models/domain.rb b/app/models/domain.rb index 66b1feb47..c9023a29c 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -6,7 +6,7 @@ class Domain < ActiveRecord::Base include Concerns::Domain::ForceDelete include Concerns::Domain::Deletable include Concerns::Domain::Transferable - include Concerns::Domain::Lockable + include Concerns::Domain::RegistryLockable has_paper_trail class_name: "DomainVersion", meta: { children: :children_log } diff --git a/app/presenters/domain_presenter.rb b/app/presenters/domain_presenter.rb index 59568f83c..824b7fc9c 100644 --- a/app/presenters/domain_presenter.rb +++ b/app/presenters/domain_presenter.rb @@ -74,10 +74,10 @@ class DomainPresenter def remove_registry_lock_btn return unless domain.locked_by_registrant? - view.link_to(view.t('admin.domains.registry_lock_delete.btn'), + view.link_to(view.t('admin.domains.registry_lock.destroy.btn'), view.admin_domain_registry_lock_path(domain), method: :delete, - data: { confirm: view.t('admin.domains.registry_lock_delete.confirm') }, + data: { confirm: view.t('admin.domains.registry_lock.destroy.confirm') }, class: 'dropdown-item') end diff --git a/config/locales/admin/domains.en.yml b/config/locales/admin/domains.en.yml index bb157fe9c..d86926568 100644 --- a/config/locales/admin/domains.en.yml +++ b/config/locales/admin/domains.en.yml @@ -23,12 +23,12 @@ en: close_btn: Close dialog submit_btn: Force delete domain - registry_lock_delete: - btn: Remove registry lock - banner: Domain has a registry lock set by registrant. - confirm: Are you sure you want to remove the registry lock? - success: Registry lock removed - error: Registry lock could not be removed + registry_lock: + destroy: + btn: Remove registry lock + confirm: Are you sure you want to remove the registry lock? + success: Registry lock removed + error: Registry lock could not be removed versions: time: Time diff --git a/test/models/domain/lockable_test.rb b/test/models/domain/registry_lockable_test.rb similarity index 97% rename from test/models/domain/lockable_test.rb rename to test/models/domain/registry_lockable_test.rb index 2f14f99da..cadef69d2 100644 --- a/test/models/domain/lockable_test.rb +++ b/test/models/domain/registry_lockable_test.rb @@ -1,6 +1,6 @@ require 'test_helper' -class DomainLockableTest < ActiveSupport::TestCase +class DomainRegistryLockableTest < ActiveSupport::TestCase def setup super From a7a07ac93ae38afcab5cc0b205a31499f9fb3c9f Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 10 Aug 2018 15:20:33 +0300 Subject: [PATCH 33/61] Insert whitespace for consistency, change save for save! --- app/models/concerns/domain/registry_lockable.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models/concerns/domain/registry_lockable.rb b/app/models/concerns/domain/registry_lockable.rb index 7a806559e..9abb3ce94 100644 --- a/app/models/concerns/domain/registry_lockable.rb +++ b/app/models/concerns/domain/registry_lockable.rb @@ -12,7 +12,8 @@ module Concerns statuses << DomainStatus::SERVER_DELETE_PROHIBITED statuses << DomainStatus::SERVER_TRANSFER_PROHIBITED self.locked_by_registrant_at = Time.zone.now - save + + save! end end @@ -20,8 +21,7 @@ module Concerns (statuses & [DomainStatus::PENDING_DELETE_CONFIRMATION, DomainStatus::PENDING_CREATE, DomainStatus::PENDING_UPDATE, DomainStatus::PENDING_DELETE, DomainStatus::PENDING_RENEW, - DomainStatus::PENDING_TRANSFER, DomainStatus::FORCE_DELETE - ]).empty? + DomainStatus::PENDING_TRANSFER, DomainStatus::FORCE_DELETE]).empty? end def locked_by_registrant? @@ -45,7 +45,7 @@ module Concerns statuses.delete(DomainStatus::SERVER_TRANSFER_PROHIBITED) self.locked_by_registrant_at = nil - save + save! end end end From 76ceefcf0b2fe73a2afbefd7af903c4d8e4ab99b Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 10 Aug 2018 15:28:00 +0300 Subject: [PATCH 34/61] Save 2 lines of code, lines of code are expensive --- app/models/concerns/domain/registry_lockable.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/models/concerns/domain/registry_lockable.rb b/app/models/concerns/domain/registry_lockable.rb index 9abb3ce94..4a759296d 100644 --- a/app/models/concerns/domain/registry_lockable.rb +++ b/app/models/concerns/domain/registry_lockable.rb @@ -27,11 +27,9 @@ module Concerns def locked_by_registrant? return false unless locked_by_registrant_at - lock_statuses = [ - DomainStatus::SERVER_UPDATE_PROHIBITED, - DomainStatus::SERVER_DELETE_PROHIBITED, - DomainStatus::SERVER_TRANSFER_PROHIBITED, - ] + lock_statuses = [DomainStatus::SERVER_UPDATE_PROHIBITED, + DomainStatus::SERVER_DELETE_PROHIBITED, + DomainStatus::SERVER_TRANSFER_PROHIBITED] (statuses & lock_statuses).count == 3 end From 6ebb77fb5c6b8659d05cccab91afdda5c6d9e30f Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Thu, 16 Aug 2018 15:43:22 +0300 Subject: [PATCH 35/61] Change `contacts.email` to NOT NULL --- .../20180816123540_change_contacts_email_to_not_null.rb | 5 +++++ db/structure.sql | 4 +++- test/system/admin_area/contact_versions_test.rb | 4 ++-- test/system/admin_area/domain_versions_test.rb | 4 ++-- 4 files changed, 12 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20180816123540_change_contacts_email_to_not_null.rb diff --git a/db/migrate/20180816123540_change_contacts_email_to_not_null.rb b/db/migrate/20180816123540_change_contacts_email_to_not_null.rb new file mode 100644 index 000000000..e6080e38b --- /dev/null +++ b/db/migrate/20180816123540_change_contacts_email_to_not_null.rb @@ -0,0 +1,5 @@ +class ChangeContactsEmailToNotNull < ActiveRecord::Migration + def change + change_column_null :contacts, :email, false + end +end \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index b07f08000..939b00565 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -613,7 +613,7 @@ CREATE TABLE public.contacts ( id integer NOT NULL, code character varying NOT NULL, phone character varying, - email character varying, + email character varying NOT NULL, fax character varying, created_at timestamp without time zone, updated_at timestamp without time zone, @@ -4757,3 +4757,5 @@ INSERT INTO schema_migrations (version) VALUES ('20180613030330'); INSERT INTO schema_migrations (version) VALUES ('20180613045614'); +INSERT INTO schema_migrations (version) VALUES ('20180816123540'); + diff --git a/test/system/admin_area/contact_versions_test.rb b/test/system/admin_area/contact_versions_test.rb index 10d20615a..8130706b6 100644 --- a/test/system/admin_area/contact_versions_test.rb +++ b/test/system/admin_area/contact_versions_test.rb @@ -21,8 +21,8 @@ class ContactVersionsTest < ApplicationSystemTestCase VALUES (75, 'test_registrar', 'test123', 'test@test.com', 'EE', 'TEST123', 'test123', 'en'); - INSERT INTO contacts (id, code, auth_info, registrar_id) - VALUES (75, 'test_code', '8b4d462aa04194ca78840a', 75); + INSERT INTO contacts (id, code, email, auth_info, registrar_id) + VALUES (75, 'test_code', 'test@inbox.test', '8b4d462aa04194ca78840a', 75); INSERT INTO log_contacts (item_type, item_id, event, whodunnit, object, object_changes, created_at, session, children, ident_updated_at, uuid) diff --git a/test/system/admin_area/domain_versions_test.rb b/test/system/admin_area/domain_versions_test.rb index 6c375cefe..dae7592c8 100644 --- a/test/system/admin_area/domain_versions_test.rb +++ b/test/system/admin_area/domain_versions_test.rb @@ -21,8 +21,8 @@ class DomainVersionsTest < ApplicationSystemTestCase VALUES (54, 'test_registrar', 'test123', 'test@test.com', 'EE', 'TEST123', 'test123', 'en'); - INSERT INTO contacts (id, code, auth_info, registrar_id) - VALUES (54, 'test_code', '8b4d462aa04194ca78840a', 54); + INSERT INTO contacts (id, code, email, auth_info, registrar_id) + VALUES (54, 'test_code', 'test@inbox.test', '8b4d462aa04194ca78840a', 54); INSERT INTO domains (id, registrar_id, valid_to, registrant_id, transfer_code) From 3ee5291b57f1aed542834b1913ed2f22c8bfde48 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 17 Aug 2018 13:31:53 +0300 Subject: [PATCH 36/61] Add handling of PaperTrail.whodunnit to Registrant API controllers --- .../api/v1/registrant/base_controller.rb | 7 +++++++ .../v1/registrant/registry_locks_controller.rb | 2 +- app/models/concerns/versions.rb | 16 ++++++---------- .../registrant_api_domain_registry_lock_test.rb | 10 +++++++++- 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/app/controllers/api/v1/registrant/base_controller.rb b/app/controllers/api/v1/registrant/base_controller.rb index 06dfd8804..62ca449a5 100644 --- a/app/controllers/api/v1/registrant/base_controller.rb +++ b/app/controllers/api/v1/registrant/base_controller.rb @@ -6,6 +6,7 @@ module Api module Registrant class BaseController < ActionController::API before_action :authenticate + before_action :set_paper_trail_whodunnit rescue_from(ActionController::ParameterMissing) do |parameter_missing_exception| error = {} @@ -41,6 +42,12 @@ module Api render json: { errors: [{base: ['Not authorized']}] }, status: :unauthorized end end + + # This controller does not inherit from ApplicationController, + # so user_for_paper_trail method is not usable. + def set_paper_trail_whodunnit + ::PaperTrail.whodunnit = current_user.id_role_username + end end end end diff --git a/app/controllers/api/v1/registrant/registry_locks_controller.rb b/app/controllers/api/v1/registrant/registry_locks_controller.rb index 212d8bc21..b01e6b40d 100644 --- a/app/controllers/api/v1/registrant/registry_locks_controller.rb +++ b/app/controllers/api/v1/registrant/registry_locks_controller.rb @@ -17,7 +17,7 @@ module Api if @domain.remove_registry_lock render json: @domain else - render json: { errors: [{ base: ['Domain cannot be unlocked'] }] }, + render json: { errors: [{ base: ['Domain not locked'] }] }, status: :unprocessable_entity end end diff --git a/app/models/concerns/versions.rb b/app/models/concerns/versions.rb index 5e2bad90c..77bc484ae 100644 --- a/app/models/concerns/versions.rb +++ b/app/models/concerns/versions.rb @@ -34,16 +34,12 @@ module Versions end def user_from_id_role_username(str) - user = ApiUser.find_by(id: $1) if str =~ /^(\d+)-(ApiUser:|api-)/ - unless user.present? - user = AdminUser.find_by(id: $1) if str =~ /^(\d+)-AdminUser:/ - unless user.present? - # on import we copied Registrar name, which may eql code - registrar = Registrar.find_by(name: str) - # assume each registrar has only one user - user = registrar.api_users.first if registrar - end - end + registrar = Registrar.find_by(name: str) + user = registrar.api_users.first if registrar + + str_match = str.match(/^(\d+)-(ApiUser:|api-|AdminUser:|RegistrantUser:)/) + user ||= User.find_by(id: str_match[1]) if str_match + user end diff --git a/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb b/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb index 89bb80d97..8ad1d0826 100644 --- a/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb +++ b/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb @@ -35,6 +35,14 @@ class RegistrantApiDomainRegistryLockTest < ApplicationIntegrationTest assert(@domain.locked_by_registrant?) end + def test_locking_a_domain_leaves_paper_trail + post '/api/v1/registrant/domains/2df2c1a1-8f6a-490a-81be-8bdf29866880/registry_lock', + {}, @auth_headers + + @domain.reload + assert_equal(@domain.updator, @user) + end + def test_cannot_lock_a_domain_in_pending_state @domain.statuses << DomainStatus::PENDING_UPDATE @domain.save @@ -77,7 +85,7 @@ class RegistrantApiDomainRegistryLockTest < ApplicationIntegrationTest response_json = JSON.parse(response.body, symbolize_names: true) assert_equal(422, response.status) - assert_equal({ errors: [{ base: ['Domain cannot be unlocked'] }] }, response_json) + assert_equal({ errors: [{ base: ['Domain not locked'] }] }, response_json) end def test_returns_404_when_domain_is_not_found From 0c7c8eb480935d9d870d88ea09d69f2040e70bbb Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 17 Aug 2018 15:50:21 +0300 Subject: [PATCH 37/61] Move domain versioning tests from RSpec to minitest --- spec/models/domain_spec.rb | 44 ------------------- test/models/domain/domain_version_test.rb | 52 +++++++++++++++++++++++ 2 files changed, 52 insertions(+), 44 deletions(-) create mode 100644 test/models/domain/domain_version_test.rb diff --git a/spec/models/domain_spec.rb b/spec/models/domain_spec.rb index 6b282d651..e094b1a8a 100644 --- a/spec/models/domain_spec.rb +++ b/spec/models/domain_spec.rb @@ -298,50 +298,6 @@ RSpec.describe Domain do @domain.registrant_update_confirmable?('123').should == false end end - - context 'with versioning' do - it 'should not have one version' do - with_versioning do - @domain.versions.size.should == 0 - @domain.name = 'new-test-name.ee' - @domain.save - @domain.errors.full_messages.should match_array([]) - @domain.versions.size.should == 1 - end - end - - it 'should return api_creator when created by api user' do - with_versioning do - @user = create(:admin_user) - @api_user = create(:api_user) - @user.id.should == 1 - @api_user.id.should == 2 - ::PaperTrail.whodunnit = '2-ApiUser: testuser' - - @domain = create(:domain) - @domain.creator_str.should == '2-ApiUser: testuser' - - @domain.creator.should == @api_user - @domain.creator.should_not == @user - end - end - - it 'should return api_creator when created by api user' do - with_versioning do - @user = create(:admin_user, id: 1000) - @api_user = create(:api_user, id: 2000) - @user.id.should == 1000 - @api_user.id.should == 2000 - ::PaperTrail.whodunnit = '1000-AdminUser: testuser' - - @domain = create(:domain) - @domain.creator_str.should == '1000-AdminUser: testuser' - - @domain.creator.should == @user - @domain.creator.should_not == @api_user - end - end - end end it 'validates domain name' do diff --git a/test/models/domain/domain_version_test.rb b/test/models/domain/domain_version_test.rb new file mode 100644 index 000000000..74844f3af --- /dev/null +++ b/test/models/domain/domain_version_test.rb @@ -0,0 +1,52 @@ +require 'test_helper' + +class DomainVersionTest < ActiveSupport::TestCase + def setup + super + + @domain = domains(:shop) + @contacts = @domain.contacts + @user = users(:registrant) + end + + def teardown + super + end + + def test_assigns_creator_to_paper_trail_whodunnit + duplicate_domain = prepare_duplicate_domain + + PaperTrail.whodunnit = @user.id_role_username + assert_difference 'duplicate_domain.versions.count', 1 do + duplicate_domain.save! + end + + assert_equal(duplicate_domain.creator, @user) + assert_equal(duplicate_domain.updator, @user) + assert_equal(duplicate_domain.creator_str, @user.id_role_username) + assert_equal(duplicate_domain.updator_str, @user.id_role_username) + end + + def test_assigns_updator_to_paper_trail_whodunnit + PaperTrail.whodunnit = @user.id_role_username + + assert_difference '@domain.versions.count', 1 do + @domain.apply_registry_lock + end + + assert_equal(@domain.updator, @user) + assert_equal(@domain.updator_str, @user.id_role_username) + end + + private + + def prepare_duplicate_domain + duplicate_domain = @domain.dup + duplicate_domain.tech_contacts << @contacts + duplicate_domain.admin_contacts << @contacts + duplicate_domain.name = 'duplicate.test' + duplicate_domain.uuid = nil + + duplicate_domain + end +end From 97617764fbc75473f861e8bc7a649a90465076af Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 17 Aug 2018 16:00:43 +0300 Subject: [PATCH 38/61] Update documentation for registry lock. Also, make rewrite the return error message using better grammar --- .../api/v1/registrant/registry_locks_controller.rb | 2 +- doc/registrant-api/v1/domain_lock.md | 4 ++-- .../registrant/registrant_api_domain_registry_lock_test.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/v1/registrant/registry_locks_controller.rb b/app/controllers/api/v1/registrant/registry_locks_controller.rb index b01e6b40d..57a0bfd0c 100644 --- a/app/controllers/api/v1/registrant/registry_locks_controller.rb +++ b/app/controllers/api/v1/registrant/registry_locks_controller.rb @@ -17,7 +17,7 @@ module Api if @domain.remove_registry_lock render json: @domain else - render json: { errors: [{ base: ['Domain not locked'] }] }, + render json: { errors: [{ base: ['Domain is not locked'] }] }, status: :unprocessable_entity end end diff --git a/doc/registrant-api/v1/domain_lock.md b/doc/registrant-api/v1/domain_lock.md index 5f275edb4..7e24e2827 100644 --- a/doc/registrant-api/v1/domain_lock.md +++ b/doc/registrant-api/v1/domain_lock.md @@ -139,12 +139,12 @@ Content-Type: application/json #### Response for failure ``` -HTTP/1.1 400 +HTTP/1.1 422 Content-Type: application/json { "errors": [ - { "base": "domain cannot be unlocked" } + { "base": "Domain is not locked" } ] } diff --git a/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb b/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb index 8ad1d0826..1548c5d13 100644 --- a/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb +++ b/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb @@ -85,7 +85,7 @@ class RegistrantApiDomainRegistryLockTest < ApplicationIntegrationTest response_json = JSON.parse(response.body, symbolize_names: true) assert_equal(422, response.status) - assert_equal({ errors: [{ base: ['Domain not locked'] }] }, response_json) + assert_equal({ errors: [{ base: ['Domain is not locked'] }] }, response_json) end def test_returns_404_when_domain_is_not_found From ec5e4fe3567c7ecb0f0ed6b729f2b193ddf5a738 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 17 Aug 2018 16:11:39 +0300 Subject: [PATCH 39/61] Add count check to registry lock test --- .../registrant_api_domain_registry_lock_test.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb b/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb index 1548c5d13..8f86e446c 100644 --- a/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb +++ b/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb @@ -35,9 +35,11 @@ class RegistrantApiDomainRegistryLockTest < ApplicationIntegrationTest assert(@domain.locked_by_registrant?) end - def test_locking_a_domain_leaves_paper_trail - post '/api/v1/registrant/domains/2df2c1a1-8f6a-490a-81be-8bdf29866880/registry_lock', - {}, @auth_headers + def test_locking_a_domain_creates_a_version_record + assert_difference '@domain.versions.count', 1 do + post '/api/v1/registrant/domains/2df2c1a1-8f6a-490a-81be-8bdf29866880/registry_lock', + {}, @auth_headers + end @domain.reload assert_equal(@domain.updator, @user) From 8b340cbafa658ccebeb8da11c898802f5529ca2a Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Mon, 20 Aug 2018 22:45:07 +0300 Subject: [PATCH 40/61] Add rake task description --- lib/tasks/domain.rake | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/tasks/domain.rake b/lib/tasks/domain.rake index f59a76141..926175180 100644 --- a/lib/tasks/domain.rake +++ b/lib/tasks/domain.rake @@ -1,4 +1,5 @@ namespace :domain do + desc 'Discard domains' task discard: :environment do Domain.discard_domains end From 31935a933fee4a0b678954ffdfcb90a10a44718e Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 21 Aug 2018 00:39:04 +0300 Subject: [PATCH 41/61] Take NULL into account --- app/models/concerns/domain/discardable.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/concerns/domain/discardable.rb b/app/models/concerns/domain/discardable.rb index a5aa3d2b4..68ba9406a 100644 --- a/app/models/concerns/domain/discardable.rb +++ b/app/models/concerns/domain/discardable.rb @@ -3,7 +3,8 @@ module Concerns::Domain::Discardable class_methods do def discard_domains - domains = where('delete_at < ? AND ? != ALL(statuses) AND ? != ALL(statuses)', + domains = where('delete_at < ? AND ? != ALL(coalesce(statuses, array[]::varchar[])) AND' \ + ' ? != ALL(COALESCE(statuses, array[]::varchar[]))', Time.zone.now, DomainStatus::SERVER_DELETE_PROHIBITED, DomainStatus::DELETE_CANDIDATE) From cd633d0a4b405e0d20e175ffa8030e693b4ea54b Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 21 Aug 2018 00:53:25 +0300 Subject: [PATCH 42/61] Output rake task results --- app/models/concerns/domain/discardable.rb | 5 ++++- lib/tasks/domain.rake | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/models/concerns/domain/discardable.rb b/app/models/concerns/domain/discardable.rb index 68ba9406a..e46492220 100644 --- a/app/models/concerns/domain/discardable.rb +++ b/app/models/concerns/domain/discardable.rb @@ -9,7 +9,10 @@ module Concerns::Domain::Discardable DomainStatus::SERVER_DELETE_PROHIBITED, DomainStatus::DELETE_CANDIDATE) - domains.map(&:discard) + domains.each do |domain| + domain.discard + yield domain if block_given? + end end end diff --git a/lib/tasks/domain.rake b/lib/tasks/domain.rake index 926175180..60075adde 100644 --- a/lib/tasks/domain.rake +++ b/lib/tasks/domain.rake @@ -1,6 +1,8 @@ namespace :domain do desc 'Discard domains' task discard: :environment do - Domain.discard_domains + Domain.discard_domains do |domain| + puts "#{domain} is discarded" + end end -end +end \ No newline at end of file From 99516d87d6eca3e8ad1008f4de0bb0c14b78386e Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 21 Aug 2018 00:53:47 +0300 Subject: [PATCH 43/61] Use TaskTestCase as a parent --- test/integration/tasks/discard_domain_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/tasks/discard_domain_test.rb b/test/integration/tasks/discard_domain_test.rb index 0a341765b..05d7e3083 100644 --- a/test/integration/tasks/discard_domain_test.rb +++ b/test/integration/tasks/discard_domain_test.rb @@ -1,6 +1,6 @@ require 'test_helper' -class DiscardDomainTaskTest < ActiveSupport::TestCase +class DiscardDomainTaskTest < TaskTestCase setup do travel_to Time.zone.parse('2010-07-05 08:00') @domain = domains(:shop) From 5338e59a2e01c5e5d9700d2d1e357dc704976c70 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 21 Aug 2018 12:48:08 +0300 Subject: [PATCH 44/61] Fix tests --- test/models/domain/force_delete_test.rb | 1 + test/system/admin_area/domains/force_delete_test.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/test/models/domain/force_delete_test.rb b/test/models/domain/force_delete_test.rb index 1cc76e2d7..b9df6f24a 100644 --- a/test/models/domain/force_delete_test.rb +++ b/test/models/domain/force_delete_test.rb @@ -74,6 +74,7 @@ class DomainForceDeleteTest < ActiveSupport::TestCase end def test_force_delete_cannot_be_cancelled_when_a_domain_is_discarded + @domain.delete_at = Time.zone.parse('2010-07-05 10:00') @domain.discard @domain.schedule_force_delete assert_raises StandardError do diff --git a/test/system/admin_area/domains/force_delete_test.rb b/test/system/admin_area/domains/force_delete_test.rb index cc8292c48..224bceb24 100644 --- a/test/system/admin_area/domains/force_delete_test.rb +++ b/test/system/admin_area/domains/force_delete_test.rb @@ -56,6 +56,7 @@ class AdminAreaDomainForceDeleteTest < ApplicationSystemTestCase end def test_force_delete_cannot_be_cancelled_when_a_domain_is_discarded + @domain.delete_at = Time.zone.parse('2010-07-05 10:00') @domain.discard @domain.schedule_force_delete From 9f7fc47f19e6dd3f3d7959c1ed7e709ee85bfdc3 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Thu, 23 Aug 2018 15:40:52 +0300 Subject: [PATCH 45/61] Make contact fixtures have unique ident numbers --- test/fixtures/contacts.yml | 10 +++++----- .../api/registrant/registrant_api_contacts_test.rb | 4 ++-- .../api/registrant/registrant_api_domains_test.rb | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/test/fixtures/contacts.yml b/test/fixtures/contacts.yml index 1f2e4b8da..61ddd13bb 100644 --- a/test/fixtures/contacts.yml +++ b/test/fixtures/contacts.yml @@ -15,7 +15,7 @@ william: &william email: william@inbox.test phone: '+555.555' fax: +555.555 - ident: 1234 + ident: 12345 ident_type: priv ident_country_code: US registrar: bestnames @@ -34,7 +34,7 @@ jane: name: Jane email: jane@mail.test phone: '+555.555' - ident: 1234 + ident: 123456 ident_type: priv ident_country_code: US registrar: bestnames @@ -46,7 +46,7 @@ acme_ltd: name: Acme Ltd email: acme@outlook.test phone: '+555.555' - ident: 1234 + ident: 1234567 ident_type: org registrar: bestnames ident_country_code: US @@ -58,7 +58,7 @@ jack: name: Jack email: jack@inbox.test phone: '+555.555' - ident: 1234 + ident: 12345678 ident_type: org registrar: goodnames ident_country_code: US @@ -87,4 +87,4 @@ invalid: email: invalid@invalid.test auth_info: any registrar: bestnames - uuid: bd80c0f9-26ee-49e0-a2cb-2311d931c433 \ No newline at end of file + uuid: bd80c0f9-26ee-49e0-a2cb-2311d931c433 diff --git a/test/integration/api/registrant/registrant_api_contacts_test.rb b/test/integration/api/registrant/registrant_api_contacts_test.rb index ddeaee9f3..97f0c8886 100644 --- a/test/integration/api/registrant/registrant_api_contacts_test.rb +++ b/test/integration/api/registrant/registrant_api_contacts_test.rb @@ -25,7 +25,7 @@ class RegistrantApiContactsTest < ApplicationIntegrationTest assert_equal(200, response.status) json_body = JSON.parse(response.body, symbolize_names: true) - assert_equal(5, json_body.count) + assert_equal(4, json_body.count) array_of_contact_codes = json_body.map { |x| x[:code] } assert(array_of_contact_codes.include?('william-001')) assert(array_of_contact_codes.include?('jane-001')) @@ -39,7 +39,7 @@ class RegistrantApiContactsTest < ApplicationIntegrationTest get '/api/v1/registrant/contacts', {}, @auth_headers response_json = JSON.parse(response.body, symbolize_names: true) - assert_equal(5, response_json.count) + assert_equal(4, response_json.count) end def test_get_contact_details_by_uuid diff --git a/test/integration/api/registrant/registrant_api_domains_test.rb b/test/integration/api/registrant/registrant_api_domains_test.rb index 128d15e20..0764db3aa 100644 --- a/test/integration/api/registrant/registrant_api_domains_test.rb +++ b/test/integration/api/registrant/registrant_api_domains_test.rb @@ -57,7 +57,7 @@ class RegistrantApiDomainsTest < ApplicationIntegrationTest get '/api/v1/registrant/domains', {}, @auth_headers response_json = JSON.parse(response.body, symbolize_names: true) - assert_equal(5, response_json.count) + assert_equal(4, response_json.count) end def test_root_does_not_accept_limit_higher_than_200 From 9623e2fb97a5286da6d9da5b0ce5553153b24ecb Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Thu, 23 Aug 2018 16:00:44 +0300 Subject: [PATCH 46/61] Refactor RegistrantUser class * Extract frequently used country_code and ident methods * Refactor domain query * Add contact query method * Add adminstrated_domains query method. Name will most likely change in the future developments * Change registry locks integration test name --- .../api/v1/registrant/base_controller.rb | 3 +- .../registrant/registry_locks_controller.rb | 2 +- app/models/registrant_user.rb | 28 ++++++--- ... => registrant_api_registry_locks_test.rb} | 2 +- .../registrant_user_creation_test.rb | 56 ++++++++++++++++++ test/models/registrant_user_test.rb | 59 +++++-------------- 6 files changed, 95 insertions(+), 55 deletions(-) rename test/integration/api/registrant/{registrant_api_domain_registry_lock_test.rb => registrant_api_registry_locks_test.rb} (98%) create mode 100644 test/models/registrant_user/registrant_user_creation_test.rb diff --git a/app/controllers/api/v1/registrant/base_controller.rb b/app/controllers/api/v1/registrant/base_controller.rb index 62ca449a5..043d93948 100644 --- a/app/controllers/api/v1/registrant/base_controller.rb +++ b/app/controllers/api/v1/registrant/base_controller.rb @@ -39,7 +39,8 @@ module Api if decryptor.valid? sign_in decryptor.user else - render json: { errors: [{base: ['Not authorized']}] }, status: :unauthorized + render json: { errors: [{ base: ['Not authorized'] }] }, + status: :unauthorized end end diff --git a/app/controllers/api/v1/registrant/registry_locks_controller.rb b/app/controllers/api/v1/registrant/registry_locks_controller.rb index 57a0bfd0c..509ee4c23 100644 --- a/app/controllers/api/v1/registrant/registry_locks_controller.rb +++ b/app/controllers/api/v1/registrant/registry_locks_controller.rb @@ -25,7 +25,7 @@ module Api private def set_domain - domain_pool = associated_domains(current_user) + domain_pool = current_user.administrated_domains @domain = domain_pool.find_by(uuid: params[:domain_uuid]) return if @domain diff --git a/app/models/registrant_user.rb b/app/models/registrant_user.rb index 889f2ca4c..8d8cfdeb8 100644 --- a/app/models/registrant_user.rb +++ b/app/models/registrant_user.rb @@ -8,16 +8,30 @@ class RegistrantUser < User delegate :can?, :cannot?, to: :ability def ident - registrant_ident.to_s.split("-").last + registrant_ident.to_s.split('-').last + end + + def country_code + registrant_ident.to_s.split('-').first end def domains - ident_cc, ident = registrant_ident.to_s.split '-' - Domain.includes(:registrar, :registrant).where(contacts: { - ident_type: 'priv', - ident: ident, #identity_code, - ident_country_code: ident_cc #country_code - }) + Domain.includes(:registrar, :registrant).where( + contacts: { + ident_type: 'priv', + ident: ident, + ident_country_code: country_code + } + ) + end + + def contacts + Contact.where(ident_type: 'priv', ident: ident, ident_country_code: country_code) + end + + def administrated_domains + Domain.joins(:domain_contacts) + .where(domain_contacts: { contact_id: contacts, type: AdminDomainContact }) end def to_s diff --git a/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb b/test/integration/api/registrant/registrant_api_registry_locks_test.rb similarity index 98% rename from test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb rename to test/integration/api/registrant/registrant_api_registry_locks_test.rb index 8f86e446c..0ff677b1d 100644 --- a/test/integration/api/registrant/registrant_api_domain_registry_lock_test.rb +++ b/test/integration/api/registrant/registrant_api_registry_locks_test.rb @@ -1,7 +1,7 @@ require 'test_helper' require 'auth_token/auth_token_creator' -class RegistrantApiDomainRegistryLockTest < ApplicationIntegrationTest +class RegistrantApiRegistryLocksTest < ApplicationIntegrationTest def setup super diff --git a/test/models/registrant_user/registrant_user_creation_test.rb b/test/models/registrant_user/registrant_user_creation_test.rb new file mode 100644 index 000000000..fc5a32b4c --- /dev/null +++ b/test/models/registrant_user/registrant_user_creation_test.rb @@ -0,0 +1,56 @@ +require 'test_helper' + +class RegistrantUserCreationTest < ActiveSupport::TestCase + def test_find_or_create_by_api_data_creates_a_user + user_data = { + ident: '37710100070', + first_name: 'JOHN', + last_name: 'SMITH' + } + + RegistrantUser.find_or_create_by_api_data(user_data) + + user = User.find_by(registrant_ident: 'EE-37710100070') + assert_equal('JOHN SMITH', user.username) + end + + def test_find_or_create_by_api_data_creates_a_user_after_upcasing_input + user_data = { + ident: '37710100070', + first_name: 'John', + last_name: 'Smith' + } + + RegistrantUser.find_or_create_by_api_data(user_data) + + user = User.find_by(registrant_ident: 'EE-37710100070') + assert_equal('JOHN SMITH', user.username) + end + + def test_find_or_create_by_mid_data_creates_a_user + user_data = OpenStruct.new(user_country: 'EE', user_id_code: '37710100070', + user_givenname: 'JOHN', user_surname: 'SMITH') + + RegistrantUser.find_or_create_by_mid_data(user_data) + user = User.find_by(registrant_ident: 'EE-37710100070') + assert_equal('JOHN SMITH', user.username) + end + + def test_find_or_create_by_idc_with_legacy_header_creates_a_user + header = '/C=EE/O=ESTEID/OU=authentication/CN=SMITH,JOHN,37710100070/SN=SMITH/GN=JOHN/serialNumber=37710100070' + + RegistrantUser.find_or_create_by_idc_data(header, RegistrantUser::ACCEPTED_ISSUER) + + user = User.find_by(registrant_ident: 'EE-37710100070') + assert_equal('JOHN SMITH', user.username) + end + + def test_find_or_create_by_idc_with_rfc2253_header_creates_a_user + header = 'serialNumber=37710100070,GN=JOHN,SN=SMITH,CN=SMITH\\,JOHN\\,37710100070,OU=authentication,O=ESTEID,C=EE' + + RegistrantUser.find_or_create_by_idc_data(header, RegistrantUser::ACCEPTED_ISSUER) + + user = User.find_by(registrant_ident: 'EE-37710100070') + assert_equal('JOHN SMITH', user.username) + end +end diff --git a/test/models/registrant_user_test.rb b/test/models/registrant_user_test.rb index 86ab5591a..5af663dd9 100644 --- a/test/models/registrant_user_test.rb +++ b/test/models/registrant_user_test.rb @@ -1,62 +1,31 @@ +require 'test_helper' + class RegistrantUserTest < ActiveSupport::TestCase def setup super + + @user = RegistrantUser.new(registrant_ident: 'US-1234') end def teardown super end - def test_find_or_create_by_api_data_creates_a_user - user_data = { - ident: '37710100070', - first_name: 'JOHN', - last_name: 'SMITH' - } - - RegistrantUser.find_or_create_by_api_data(user_data) - - user = User.find_by(registrant_ident: 'EE-37710100070') - assert_equal('JOHN SMITH', user.username) + def test_domains_returns_an_list_of_domains_associated_with_a_specific_id_code + domain_names = @user.domains.pluck(:name) + assert_equal(3, domain_names.length) end - def test_find_or_create_by_api_data_creates_a_user_after_upcasing_input - user_data = { - ident: '37710100070', - first_name: 'John', - last_name: 'Smith' - } - - RegistrantUser.find_or_create_by_api_data(user_data) - - user = User.find_by(registrant_ident: 'EE-37710100070') - assert_equal('JOHN SMITH', user.username) + def test_administrated_domains_returns_a_list_of_domains_that_is_smaller_than_domains + assert_equal(2, @user.administrated_domains.count) end - def test_find_or_create_by_mid_data_creates_a_user - user_data = OpenStruct.new(user_country: 'EE', user_id_code: '37710100070', - user_givenname: 'JOHN', user_surname: 'SMITH') - - RegistrantUser.find_or_create_by_mid_data(user_data) - user = User.find_by(registrant_ident: 'EE-37710100070') - assert_equal('JOHN SMITH', user.username) + def test_contacts_returns_an_list_of_contacts_associated_with_a_specific_id_code + assert_equal(1, @user.contacts.count) end - def test_find_or_create_by_idc_with_legacy_header_creates_a_user - header = '/C=EE/O=ESTEID/OU=authentication/CN=SMITH,JOHN,37710100070/SN=SMITH/GN=JOHN/serialNumber=37710100070' - - RegistrantUser.find_or_create_by_idc_data(header, RegistrantUser::ACCEPTED_ISSUER) - - user = User.find_by(registrant_ident: 'EE-37710100070') - assert_equal('JOHN SMITH', user.username) - end - - def test_find_or_create_by_idc_with_rfc2253_header_creates_a_user - header = 'serialNumber=37710100070,GN=JOHN,SN=SMITH,CN=SMITH\\,JOHN\\,37710100070,OU=authentication,O=ESTEID,C=EE' - - RegistrantUser.find_or_create_by_idc_data(header, RegistrantUser::ACCEPTED_ISSUER) - - user = User.find_by(registrant_ident: 'EE-37710100070') - assert_equal('JOHN SMITH', user.username) + def test_ident_and_country_code_helper_methods + assert_equal('1234', @user.ident) + assert_equal('US', @user.country_code) end end From b7dfc19b9fd5c52e6cb0f054beefab070d8ce4e9 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 24 Aug 2018 11:57:12 +0300 Subject: [PATCH 47/61] Update domain_contacts fixtures Previously, records were invalid according to our initial settings --- test/fixtures/domain_contacts.yml | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/test/fixtures/domain_contacts.yml b/test/fixtures/domain_contacts.yml index 43d082e31..5c5d89e24 100644 --- a/test/fixtures/domain_contacts.yml +++ b/test/fixtures/domain_contacts.yml @@ -28,11 +28,36 @@ airport_william_tech: contact: william type: TechDomainContact -library_john: +library_acme_admin: + domain: library + contact: acme_ltd + type: AdminDomainContact + +library_john_tech: domain: library contact: john + type: TechDomainContact + +metro_jack_admin: + domain: metro + contact: jack type: AdminDomainContact +metro_jack_tech: + domain: metro + contact: jack + type: TechDomainContact + +hospital_john_admin: + domain: hospital + contact: john + type: AdminDomainContact + +hospital_john_tech: + domain: hospital + contact: john + type: TechDomainContact + invalid_invalid_admin: domain: invalid contact: invalid From 1d53e7bb5b4a48fb843b59f040fb1271442d0394 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 24 Aug 2018 12:53:29 +0300 Subject: [PATCH 48/61] Convert pending_json field to jsonb so it can be compared without typecasting --- ...20180824092855_change_domain_pending_json_to_jsonb.rb | 9 +++++++++ db/structure.sql | 4 +++- 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20180824092855_change_domain_pending_json_to_jsonb.rb diff --git a/db/migrate/20180824092855_change_domain_pending_json_to_jsonb.rb b/db/migrate/20180824092855_change_domain_pending_json_to_jsonb.rb new file mode 100644 index 000000000..10639527a --- /dev/null +++ b/db/migrate/20180824092855_change_domain_pending_json_to_jsonb.rb @@ -0,0 +1,9 @@ +class ChangeDomainPendingJsonToJsonb < ActiveRecord::Migration + def up + change_column :domains, :pending_json, 'jsonb USING CAST(pending_json AS jsonb)' + end + + def down + change_column :domains, :pending_json, 'json USING CAST(pending_json AS json)' + end +end diff --git a/db/structure.sql b/db/structure.sql index c4b0b9d7a..243a6f900 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -904,7 +904,7 @@ CREATE TABLE public.domains ( delete_at timestamp without time zone, registrant_verification_asked_at timestamp without time zone, registrant_verification_token character varying, - pending_json json, + pending_json jsonb, force_delete_at timestamp without time zone, statuses character varying[], reserved boolean DEFAULT false, @@ -4760,3 +4760,5 @@ INSERT INTO schema_migrations (version) VALUES ('20180613045614'); INSERT INTO schema_migrations (version) VALUES ('20180808064402'); +INSERT INTO schema_migrations (version) VALUES ('20180824092855'); + From a64b03d2042f832f839ad43e4c503855199f23c6 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 24 Aug 2018 12:54:05 +0300 Subject: [PATCH 49/61] Make sure that only admin contacts and registrants can lock a domain --- .../registrant/registry_locks_controller.rb | 12 ++++++++- app/models/registrant_user.rb | 26 ++++++++++++------- .../registrant_api_registry_locks_test.rb | 10 +++++++ test/models/registrant_user_test.rb | 15 ++++++++--- 4 files changed, 49 insertions(+), 14 deletions(-) diff --git a/app/controllers/api/v1/registrant/registry_locks_controller.rb b/app/controllers/api/v1/registrant/registry_locks_controller.rb index 509ee4c23..2309cac8b 100644 --- a/app/controllers/api/v1/registrant/registry_locks_controller.rb +++ b/app/controllers/api/v1/registrant/registry_locks_controller.rb @@ -3,6 +3,7 @@ module Api module Registrant class RegistryLocksController < BaseController before_action :set_domain + before_action :authorized_to_manage_locks? def create if @domain.apply_registry_lock @@ -25,13 +26,22 @@ module Api private def set_domain - domain_pool = current_user.administrated_domains + domain_pool = current_user.domains @domain = domain_pool.find_by(uuid: params[:domain_uuid]) return if @domain render json: { errors: [{ base: ['Domain not found'] }] }, status: :not_found and return end + + def authorized_to_manage_locks? + return if current_user.administrated_domains.include?(@domain) + + render json: { errors: [ + { base: ['Only administrative contacts can manage registry locks'] } + ] }, + status: :unauthorized and return + end end end end diff --git a/app/models/registrant_user.rb b/app/models/registrant_user.rb index 8d8cfdeb8..c6effe2d4 100644 --- a/app/models/registrant_user.rb +++ b/app/models/registrant_user.rb @@ -16,22 +16,30 @@ class RegistrantUser < User end def domains - Domain.includes(:registrar, :registrant).where( - contacts: { - ident_type: 'priv', - ident: ident, - ident_country_code: country_code - } - ) + Domain.uniq + .joins(:contacts) + .where(contacts: { ident_type: 'priv', ident: ident, ident_country_code: country_code }) end def contacts Contact.where(ident_type: 'priv', ident: ident, ident_country_code: country_code) end + # In Rails 5, can be replaced with a much simpler `or` query method and the raw SQL parts can be + # removed. + # https://guides.rubyonrails.org/active_record_querying.html#or-conditions def administrated_domains - Domain.joins(:domain_contacts) - .where(domain_contacts: { contact_id: contacts, type: AdminDomainContact }) + domains_where_is_administrative_contact = begin + Domain.joins(:domain_contacts) + .where(domain_contacts: { contact_id: contacts, type: [AdminDomainContact] }) + end + + domains_where_is_registrar = Domain.where(registrant_id: contacts) + + Domain.from( + "(#{domains_where_is_registrar.to_sql} UNION " \ + "#{domains_where_is_administrative_contact.to_sql}) AS domains" + ) end def to_s diff --git a/test/integration/api/registrant/registrant_api_registry_locks_test.rb b/test/integration/api/registrant/registrant_api_registry_locks_test.rb index 0ff677b1d..0b7f31aad 100644 --- a/test/integration/api/registrant/registrant_api_registry_locks_test.rb +++ b/test/integration/api/registrant/registrant_api_registry_locks_test.rb @@ -99,6 +99,16 @@ class RegistrantApiRegistryLocksTest < ApplicationIntegrationTest assert_equal({ errors: [{ base: ['Domain not found'] }] }, response_json) end + def test_technical_contact_cannot_lock_a_domain + post '/api/v1/registrant/domains/647bcc48-8d5e-4a04-8ce5-2a3cd17b6eab/registry_lock', + {}, @auth_headers + + response_json = JSON.parse(response.body, symbolize_names: true) + assert_equal(401, response.status) + assert_equal({ errors: [{ base: ['Only administrative contacts can manage registry locks'] }] }, + response_json) + end + private def auth_token diff --git a/test/models/registrant_user_test.rb b/test/models/registrant_user_test.rb index 5af663dd9..b04829999 100644 --- a/test/models/registrant_user_test.rb +++ b/test/models/registrant_user_test.rb @@ -4,20 +4,27 @@ class RegistrantUserTest < ActiveSupport::TestCase def setup super - @user = RegistrantUser.new(registrant_ident: 'US-1234') + @user = users(:registrant) end def teardown super end - def test_domains_returns_an_list_of_domains_associated_with_a_specific_id_code + def test_domains_returns_an_list_of_distinct_domains_associated_with_a_specific_id_code domain_names = @user.domains.pluck(:name) assert_equal(3, domain_names.length) + + # User is a registrant, but not a contact for the domain. + refute(domain_names.include?('shop.test')) end - def test_administrated_domains_returns_a_list_of_domains_that_is_smaller_than_domains - assert_equal(2, @user.administrated_domains.count) + def test_administrated_domains_returns_a_list_of_domains + domain_names = @user.administrated_domains.pluck(:name) + assert_equal(3, domain_names.length) + + # User is a tech contact for the domain. + refute(domain_names.include?('library.test')) end def test_contacts_returns_an_list_of_contacts_associated_with_a_specific_id_code From c92fc256838a6bd744560490c694e75990a3d3e4 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 24 Aug 2018 13:17:08 +0300 Subject: [PATCH 50/61] Fix tests that failed after merging master Also, fix rubocop issues --- app/models/registrant_user.rb | 12 ++++++------ app/presenters/domain_presenter.rb | 6 +++--- app/views/admin/domains/edit.html.erb | 24 ++++++++++++++++++------ config/locales/admin/domains.en.yml | 2 +- 4 files changed, 28 insertions(+), 16 deletions(-) diff --git a/app/models/registrant_user.rb b/app/models/registrant_user.rb index c6effe2d4..18b9506f3 100644 --- a/app/models/registrant_user.rb +++ b/app/models/registrant_user.rb @@ -1,5 +1,5 @@ class RegistrantUser < User - ACCEPTED_ISSUER = 'AS Sertifitseerimiskeskus' + ACCEPTED_ISSUER = 'AS Sertifitseerimiskeskus'.freeze attr_accessor :idc_data def ability @@ -17,8 +17,8 @@ class RegistrantUser < User def domains Domain.uniq - .joins(:contacts) - .where(contacts: { ident_type: 'priv', ident: ident, ident_country_code: country_code }) + .joins(:contacts) + .where(contacts: { ident_type: 'priv', ident: ident, ident_country_code: country_code }) end def contacts @@ -31,7 +31,7 @@ class RegistrantUser < User def administrated_domains domains_where_is_administrative_contact = begin Domain.joins(:domain_contacts) - .where(domain_contacts: { contact_id: contacts, type: [AdminDomainContact] }) + .where(domain_contacts: { contact_id: contacts, type: [AdminDomainContact] }) end domains_where_is_registrar = Domain.where(registrant_id: contacts) @@ -55,13 +55,13 @@ class RegistrantUser < User user_data = {} # handling here new and old mode - if idc_data.starts_with?("/") + if idc_data.starts_with?('/') user_data[:ident] = idc_data.scan(/serialNumber=(\d+)/).flatten.first user_data[:country_code] = idc_data.scan(/^\/C=(.{2})/).flatten.first user_data[:first_name] = idc_data.scan(%r{/GN=(.+)/serialNumber}).flatten.first user_data[:last_name] = idc_data.scan(%r{/SN=(.+)/GN}).flatten.first else - parse_str = "," + idc_data + parse_str = ',' + idc_data user_data[:ident] = parse_str.scan(/,serialNumber=(\d+)/).flatten.first user_data[:country_code] = parse_str.scan(/,C=(.{2})/).flatten.first user_data[:first_name] = parse_str.scan(/,GN=([^,]+)/).flatten.first diff --git a/app/presenters/domain_presenter.rb b/app/presenters/domain_presenter.rb index 917cd21a4..f4d3864ad 100644 --- a/app/presenters/domain_presenter.rb +++ b/app/presenters/domain_presenter.rb @@ -77,7 +77,7 @@ class DomainPresenter def schedule_force_delete_btn view.content_tag(:a, view.t('admin.domains.force_delete_toggle_btn.schedule'), - class: 'btn btn-default', + class: 'dropdown-item', data: { toggle: 'modal', target: '.domain-edit-force-delete-dialog', @@ -91,14 +91,14 @@ class DomainPresenter data: { confirm: view.t('admin.domains.force_delete_toggle_btn.cancel_confirm'), }, - class: 'btn btn-primary' + class: 'dropdown-item' end def inactive_schedule_force_delete_btn view.content_tag :button, view.t('admin.domains.force_delete_toggle_btn.schedule'), title: view.t('admin.domains.force_delete_toggle_btn.unable_to_schedule'), disabled: true, - class: 'btn btn-default' + class: 'dropdown-item' end attr_reader :domain diff --git a/app/views/admin/domains/edit.html.erb b/app/views/admin/domains/edit.html.erb index 5d8f19349..0d5e4589e 100644 --- a/app/views/admin/domains/edit.html.erb +++ b/app/views/admin/domains/edit.html.erb @@ -5,15 +5,27 @@
  • <%= link_to @domain, admin_domain_path(@domain) %>
  • +