From 43f65cc0aacff6274cacc44a8d4229e2934c896d Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Mon, 6 Aug 2018 15:56:17 +0300 Subject: [PATCH 01/37] Add Domain::Lockable concern --- app/models/concerns/domain/lockable.rb | 46 ++++++++++++++++++++ app/models/domain.rb | 1 + test/models/domain/lockable_test.rb | 59 ++++++++++++++++++++++++++ 3 files changed, 106 insertions(+) create mode 100644 app/models/concerns/domain/lockable.rb create mode 100644 test/models/domain/lockable_test.rb diff --git a/app/models/concerns/domain/lockable.rb b/app/models/concerns/domain/lockable.rb new file mode 100644 index 0000000000..68721e595d --- /dev/null +++ b/app/models/concerns/domain/lockable.rb @@ -0,0 +1,46 @@ +module Concerns::Domain::Lockable + extend ActiveSupport::Concern + + def apply_registry_lock + return unless registry_lockable? + return if locked_by_registrant? + + statuses << DomainStatus::SERVER_UPDATE_PROHIBITED + statuses << DomainStatus::SERVER_DELETE_PROHIBITED + statuses << DomainStatus::SERVER_TRANSFER_PROHIBITED + + save + 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? + 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? + + statuses.delete(DomainStatus::SERVER_UPDATE_PROHIBITED) + statuses.delete(DomainStatus::SERVER_DELETE_PROHIBITED) + statuses.delete(DomainStatus::SERVER_TRANSFER_PROHIBITED) + + save + end +end diff --git a/app/models/domain.rb b/app/models/domain.rb index 30ed0b580f..66b1feb47f 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -6,6 +6,7 @@ class Domain < ActiveRecord::Base include Concerns::Domain::ForceDelete include Concerns::Domain::Deletable include Concerns::Domain::Transferable + include Concerns::Domain::Lockable has_paper_trail class_name: "DomainVersion", meta: { children: :children_log } diff --git a/test/models/domain/lockable_test.rb b/test/models/domain/lockable_test.rb new file mode 100644 index 0000000000..5b62df3555 --- /dev/null +++ b/test/models/domain/lockable_test.rb @@ -0,0 +1,59 @@ +require 'test_helper' + +class DomainLockableTest < ActiveSupport::TestCase + def setup + super + + @domain = domains(:shop) + end + + def test_registry_lock_on_lockable_domain + refute(@domain.locked_by_registrant?) + @domain.apply_registry_lock + + assert_equal( + [DomainStatus::SERVER_UPDATE_PROHIBITED, + DomainStatus::SERVER_DELETE_PROHIBITED, + DomainStatus::SERVER_TRANSFER_PROHIBITED], + @domain.statuses + ) + + assert(@domain.locked_by_registrant?) + end + + def test_registry_lock_cannot_be_applied_twice + @domain.apply_registry_lock + refute(@domain.apply_registry_lock) + assert(@domain.locked_by_registrant?) + 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?) + end + + def test_remove_registry_lock_on_locked_domain + @domain.apply_registry_lock + + assert_equal( + [DomainStatus::SERVER_UPDATE_PROHIBITED, + DomainStatus::SERVER_DELETE_PROHIBITED, + DomainStatus::SERVER_TRANSFER_PROHIBITED], + @domain.statuses + ) + + @domain.remove_registry_lock + + assert_equal(["ok"], @domain.statuses) + refute(@domain.locked_by_registrant?) + end + + def test_remove_registry_lock_on_non_locked_domain + refute(@domain.locked_by_registrant?) + refute(@domain.remove_registry_lock) + + assert_equal([], @domain.statuses) + refute(@domain.locked_by_registrant?) + end +end From 4743b1e2a5fca148334a198af2210e69d5dd7251 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Wed, 8 Aug 2018 13:37:40 +0300 Subject: [PATCH 02/37] Add locked_by_registrant_at field to domain --- db/migrate/20180808064402_add_registry_lock_time_column.rb | 7 +++++++ db/structure.sql | 5 ++++- 2 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20180808064402_add_registry_lock_time_column.rb diff --git a/db/migrate/20180808064402_add_registry_lock_time_column.rb b/db/migrate/20180808064402_add_registry_lock_time_column.rb new file mode 100644 index 0000000000..5bcbd2c7e1 --- /dev/null +++ b/db/migrate/20180808064402_add_registry_lock_time_column.rb @@ -0,0 +1,7 @@ +class AddRegistryLockTimeColumn < ActiveRecord::Migration + def change + change_table(:domains) do |t| + t.column :locked_by_registrant_at, :datetime, null: true + end + end +end diff --git a/db/structure.sql b/db/structure.sql index b07f080008..c4b0b9d7af 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -912,7 +912,8 @@ CREATE TABLE public.domains ( statuses_backup character varying[] DEFAULT '{}'::character varying[], upid integer, up_date timestamp without time zone, - uuid uuid DEFAULT public.gen_random_uuid() NOT NULL + uuid uuid DEFAULT public.gen_random_uuid() NOT NULL, + locked_by_registrant_at timestamp without time zone ); @@ -4757,3 +4758,5 @@ INSERT INTO schema_migrations (version) VALUES ('20180613030330'); INSERT INTO schema_migrations (version) VALUES ('20180613045614'); +INSERT INTO schema_migrations (version) VALUES ('20180808064402'); + From dffe865d89f14aa90af58996b0818852de09449f Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Wed, 8 Aug 2018 15:43:29 +0300 Subject: [PATCH 03/37] Add ability for admin to remove registry lock --- .../admin/domains/registry_lock_controller.rb | 22 ++++++++ app/models/concerns/domain/lockable.rb | 41 ++++++++------ app/presenters/domain_presenter.rb | 17 +++++- app/views/admin/domains/edit.html.erb | 20 +++++-- .../admin/domains/partials/_general.html.erb | 3 + config/locales/admin/domains.en.yml | 7 +++ config/routes.rb | 1 + test/models/domain/lockable_test.rb | 15 ++++- .../admin_area/domains/registry_lock_test.rb | 55 +++++++++++++++++++ test/system/admin_area/domains_test.rb | 22 +++++++- 10 files changed, 174 insertions(+), 29 deletions(-) create mode 100644 app/controllers/admin/domains/registry_lock_controller.rb create mode 100644 test/system/admin_area/domains/registry_lock_test.rb diff --git a/app/controllers/admin/domains/registry_lock_controller.rb b/app/controllers/admin/domains/registry_lock_controller.rb new file mode 100644 index 0000000000..83304fb72c --- /dev/null +++ b/app/controllers/admin/domains/registry_lock_controller.rb @@ -0,0 +1,22 @@ +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') + else + redirect_to edit_admin_domain_url(@domain), alert: t('admin.domains.registry_lock_delete.error') + end + end + + private + + def set_domain + @domain = Domain.find(params[:domain_id]) + end + end + end +end diff --git a/app/models/concerns/domain/lockable.rb b/app/models/concerns/domain/lockable.rb index 68721e595d..8292138ede 100644 --- a/app/models/concerns/domain/lockable.rb +++ b/app/models/concerns/domain/lockable.rb @@ -5,26 +5,30 @@ def apply_registry_lock return unless registry_lockable? return if locked_by_registrant? - statuses << DomainStatus::SERVER_UPDATE_PROHIBITED - statuses << DomainStatus::SERVER_DELETE_PROHIBITED - statuses << DomainStatus::SERVER_TRANSFER_PROHIBITED - - save + 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? + 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, @@ -37,10 +41,13 @@ def locked_by_registrant? def remove_registry_lock return unless locked_by_registrant? - statuses.delete(DomainStatus::SERVER_UPDATE_PROHIBITED) - statuses.delete(DomainStatus::SERVER_DELETE_PROHIBITED) - statuses.delete(DomainStatus::SERVER_TRANSFER_PROHIBITED) + 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 + save + end end end diff --git a/app/presenters/domain_presenter.rb b/app/presenters/domain_presenter.rb index aac3c1527a..4a41a06a31 100644 --- a/app/presenters/domain_presenter.rb +++ b/app/presenters/domain_presenter.rb @@ -52,18 +52,29 @@ def nameserver_names def force_delete_toggle_btn if !domain.force_delete_scheduled? view.content_tag(:a, view.t('admin.domains.force_delete_toggle_btn.schedule'), - class: 'btn btn-default', data: { toggle: 'modal', target: '.domain-edit-force-delete-dialog', - } + }, + class: 'dropdown-item' ) else view.link_to(view.t('admin.domains.force_delete_toggle_btn.cancel'), view.admin_domain_force_delete_path(domain), method: :delete, data: { confirm: view.t('admin.domains.force_delete_toggle_btn.cancel_confirm') }, - class: 'btn btn-primary') + class: 'dropdown-item') + 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 end diff --git a/app/views/admin/domains/edit.html.erb b/app/views/admin/domains/edit.html.erb index 3f831e86b0..335d3b574e 100644 --- a/app/views/admin/domains/edit.html.erb +++ b/app/views/admin/domains/edit.html.erb @@ -6,12 +6,22 @@ Edit: <%= domain.name %> -
-

- <%= link_to t('.add_new_status_btn'), '#', class: 'btn btn-primary js-add-status' %> - <%= domain.force_delete_toggle_btn %> +
+
<%= link_to t('.back_btn'), [:admin, @domain], class: 'btn btn-default' %> -

+
+
+ +
    +
  • <%= domain.force_delete_toggle_btn %>
    • +
  • <%= domain.remove_registry_lock_btn %>
    • +
    +
  • <%= link_to t('.add_new_status_btn'), '#', class: 'js-add-status' %>
    • +
+
diff --git a/app/views/admin/domains/partials/_general.html.erb b/app/views/admin/domains/partials/_general.html.erb index 7e09a77565..cf57e64d03 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 87c2c9c6ab..bbe23e9902 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 3ae18a7cdf..c79822f289 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -182,6 +182,7 @@ 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 5b62df3555..2f14f99da4 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 @@ def test_registry_lock_on_lockable_domain ) 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 @@ def test_remove_registry_lock_on_locked_domain 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 @@ def test_remove_registry_lock_on_non_locked_domain 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 0000000000..47b1f8329d --- /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 538de26044..8103f2a8a7 100644 --- a/test/system/admin_area/domains_test.rb +++ b/test/system/admin_area/domains_test.rb @@ -3,11 +3,27 @@ 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 04/37] 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 47b1f8329d..6c741ba60e 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 @@ def test_cannot_remove_registry_lock_from_not_locked_domain 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 05/37] 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 0000000000..c3ec073b6e --- /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 4a41a06a31..5ae622353a 100644 --- a/app/presenters/domain_presenter.rb +++ b/app/presenters/domain_presenter.rb @@ -14,6 +14,11 @@ def name_with_status 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 335d3b574e..19c093f78c 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 1501b35bb9..f712a8b150 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 bbe23e9902..bb157fe9cb 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 c79822f289..63f71a3441 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -23,7 +23,9 @@ 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 0000000000..d05dec3d5c --- /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 6c741ba60e..3fc053f6a3 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 @@ def test_can_remove_registry_lock_from_a_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 + accept_confirm('Are you sure you want to remove registry lock?') do click_link_or_button('Remove registry lock') end From 2a7caaa33e0d78f36a0e46ffebf2f8111359e3fc Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 10 Aug 2018 13:17:20 +0300 Subject: [PATCH 06/37] Update routes --- config/routes.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index 63f71a3441..a2b2ca790b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -23,8 +23,8 @@ 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 07/37] 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 4df0d226c8..06dfd8804a 100644 --- a/app/controllers/api/v1/registrant/base_controller.rb +++ b/app/controllers/api/v1/registrant/base_controller.rb @@ -22,6 +22,15 @@ def bearer_token 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 27b7b61254..49e950155b 100644 --- a/app/controllers/api/v1/registrant/domains_controller.rb +++ b/app/controllers/api/v1/registrant/domains_controller.rb @@ -32,17 +32,6 @@ def show 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 c3ec073b6e..212d8bc213 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 @@ def create 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 @@ def delete 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 d05dec3d5c..89bb80d97e 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 3fc053f6a3..9a3276943d 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 @@ def teardown 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 @@ def test_can_remove_registry_lock_from_a_domain 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 @@ def test_cannot_remove_registry_lock_from_not_locked_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 + 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 8103f2a8a7..fddb1b328f 100644 --- a/test/system/admin_area/domains_test.rb +++ b/test/system/admin_area/domains_test.rb @@ -25,5 +25,6 @@ def test_admin_registry_lock_date 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 08/37] 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 f712a8b150..1501b35bb9 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 09/37] 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 83304fb72c..6895ac711e 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 93141ade6c..d7ec4e6d0b 100644 --- a/app/controllers/admin/mail_templates_controller.rb +++ b/app/controllers/admin/mail_templates_controller.rb @@ -47,7 +47,7 @@ def update 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 8292138ede..ae7e215b78 100644 --- a/app/models/concerns/domain/lockable.rb +++ b/app/models/concerns/domain/lockable.rb @@ -15,15 +15,11 @@ def apply_registry_lock 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 5ae622353a..59568f83c4 100644 --- a/app/presenters/domain_presenter.rb +++ b/app/presenters/domain_presenter.rb @@ -72,15 +72,13 @@ def force_delete_toggle_btn 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 10/37] 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 6895ac711e..cd04c5f2a3 100644 --- a/app/controllers/admin/domains/registry_lock_controller.rb +++ b/app/controllers/admin/domains/registry_lock_controller.rb @@ -5,11 +5,9 @@ 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('.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 ae7e215b78..0000000000 --- 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 0000000000..7a806559e9 --- /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 66b1feb47f..c9023a29c6 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 59568f83c4..824b7fc9cc 100644 --- a/app/presenters/domain_presenter.rb +++ b/app/presenters/domain_presenter.rb @@ -74,10 +74,10 @@ def force_delete_toggle_btn 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 bb157fe9cb..d86926568e 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 2f14f99da4..cadef69d21 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 11/37] 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 7a806559e9..9abb3ce94c 100644 --- a/app/models/concerns/domain/registry_lockable.rb +++ b/app/models/concerns/domain/registry_lockable.rb @@ -12,7 +12,8 @@ def apply_registry_lock 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 @@ 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? + DomainStatus::PENDING_TRANSFER, DomainStatus::FORCE_DELETE]).empty? end def locked_by_registrant? @@ -45,7 +45,7 @@ def remove_registry_lock 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 12/37] 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 9abb3ce94c..4a759296d0 100644 --- a/app/models/concerns/domain/registry_lockable.rb +++ b/app/models/concerns/domain/registry_lockable.rb @@ -27,11 +27,9 @@ def registry_lockable? 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 3ee5291b57f1aed542834b1913ed2f22c8bfde48 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 17 Aug 2018 13:31:53 +0300 Subject: [PATCH 13/37] 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 06dfd8804a..62ca449a5e 100644 --- a/app/controllers/api/v1/registrant/base_controller.rb +++ b/app/controllers/api/v1/registrant/base_controller.rb @@ -6,6 +6,7 @@ module V1 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 @@ def authenticate 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 212d8bc213..b01e6b40df 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 @@ def destroy 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 5e2bad90c6..77bc484aea 100644 --- a/app/models/concerns/versions.rb +++ b/app/models/concerns/versions.rb @@ -34,16 +34,12 @@ def updator 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 89bb80d97e..8ad1d08269 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 @@ def test_can_lock_a_not_locked_domain 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 @@ def test_cannot_unlock_a_not_locked_domain 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 14/37] 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 6b282d6516..e094b1a8ae 100644 --- a/spec/models/domain_spec.rb +++ b/spec/models/domain_spec.rb @@ -298,50 +298,6 @@ @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 0000000000..74844f3afe --- /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 15/37] 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 b01e6b40df..57a0bfd0c0 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 @@ def destroy 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 5f275edb4e..7e24e28271 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 8ad1d08269..1548c5d134 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 @@ def test_cannot_unlock_a_not_locked_domain 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 16/37] 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 1548c5d134..8f86e446c4 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 @@ def test_can_lock_a_not_locked_domain 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 9f7fc47f19e6dd3f3d7959c1ed7e709ee85bfdc3 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Thu, 23 Aug 2018 15:40:52 +0300 Subject: [PATCH 17/37] 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 1f2e4b8dae..61ddd13bb1 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 ddeaee9f34..97f0c88862 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 @@ def test_root_returns_domain_list 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 @@ def test_root_accepts_limit_and_offset_parameters 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 128d15e20b..0764db3aaf 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 @@ def test_root_accepts_limit_and_offset_parameters 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 18/37] 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 62ca449a5e..043d939489 100644 --- a/app/controllers/api/v1/registrant/base_controller.rb +++ b/app/controllers/api/v1/registrant/base_controller.rb @@ -39,7 +39,8 @@ def authenticate 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 57a0bfd0c0..509ee4c232 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 @@ def destroy 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 889f2ca4ca..8d8cfdeb8c 100644 --- a/app/models/registrant_user.rb +++ b/app/models/registrant_user.rb @@ -8,16 +8,30 @@ def ability 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 8f86e446c4..0ff677b1d4 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 0000000000..fc5a32b4cd --- /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 86ab5591a8..5af663dd90 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 19/37] 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 43d082e314..5c5d89e245 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 20/37] 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 0000000000..10639527ac --- /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 c4b0b9d7af..243a6f9000 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 21/37] 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 509ee4c232..2309cac8bd 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 V1 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 @@ def destroy 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 8d8cfdeb8c..c6effe2d48 100644 --- a/app/models/registrant_user.rb +++ b/app/models/registrant_user.rb @@ -16,22 +16,30 @@ def country_code 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 0ff677b1d4..0b7f31aad7 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 @@ def test_returns_404_when_domain_is_not_found 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 5af663dd90..b048299996 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 22/37] 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 c6effe2d48..18b9506f31 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 @@ def country_code 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 @@ def contacts 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 @@ def find_or_create_by_idc_data(idc_data, issuer_organization) 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 917cd21a40..f4d3864ad7 100644 --- a/app/presenters/domain_presenter.rb +++ b/app/presenters/domain_presenter.rb @@ -77,7 +77,7 @@ def remove_registry_lock_btn 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 @@ def cancel_force_delete_btn 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 5d8f193497..0d5e4589e2 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) %>
    • +
    -
    -

    <%= t '.header', domain: domain.name %>

    +
    +

    + <%= t '.header' %> <%= domain.name_with_status %> +

    - -
    - <%= link_to t('.add_new_status_btn'), '#', class: 'btn btn-primary js-add-status' %> - <%= domain.force_delete_toggle_btn %> +
    +
    + +
      +
    • <%= domain.force_delete_toggle_btn %>
      • +
    • <%= domain.remove_registry_lock_btn %>
      • +
      +
    • <%= link_to t('.add_new_status_btn'), '#', class: 'js-add-status' %>
      • +
    +
    diff --git a/config/locales/admin/domains.en.yml b/config/locales/admin/domains.en.yml index 260fbef6b6..10f72a556f 100644 --- a/config/locales/admin/domains.en.yml +++ b/config/locales/admin/domains.en.yml @@ -13,7 +13,7 @@ en: reset_btn: Reset edit: - header: "Edit: %{domain}" + header: "Edit:" add_new_status_btn: Add new status force_delete_dialog: From 58fec27f72935caba8e516b235dd7e12b41bbcf0 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 24 Aug 2018 13:26:31 +0300 Subject: [PATCH 23/37] Update array indentation to conform with Rubocop requirements --- .../api/v1/registrant/registry_locks_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/v1/registrant/registry_locks_controller.rb b/app/controllers/api/v1/registrant/registry_locks_controller.rb index 2309cac8bd..a8c78925bf 100644 --- a/app/controllers/api/v1/registrant/registry_locks_controller.rb +++ b/app/controllers/api/v1/registrant/registry_locks_controller.rb @@ -38,8 +38,8 @@ def authorized_to_manage_locks? return if current_user.administrated_domains.include?(@domain) render json: { errors: [ - { base: ['Only administrative contacts can manage registry locks'] } - ] }, + { base: ['Only administrative contacts can manage registry locks'] } + ] }, status: :unauthorized and return end end From fc2a4eaa9bcda7ead07d29f8ecd5a27c645aec7f Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 24 Aug 2018 13:48:20 +0300 Subject: [PATCH 24/37] Update API documentation for registry locks --- doc/registrant-api.md | 2 +- .../v1/{domain_lock.md => registry_lock.md} | 34 ++++++++++++++++--- 2 files changed, 30 insertions(+), 6 deletions(-) rename doc/registrant-api/v1/{domain_lock.md => registry_lock.md} (87%) diff --git a/doc/registrant-api.md b/doc/registrant-api.md index c6f063a910..b475733f41 100644 --- a/doc/registrant-api.md +++ b/doc/registrant-api.md @@ -7,5 +7,5 @@ Main communication specification through Registrant API: [Authentication](registrant-api/v1/authentication.md) [Domains](registrant-api/v1/domain.md) -[Domain Lock](registrant-api/v1/domain_lock.md) +[Registry Lock](registrant-api/v1/registry_lock.md) [Contacts](registrant-api/v1/contact.md) diff --git a/doc/registrant-api/v1/domain_lock.md b/doc/registrant-api/v1/registry_lock.md similarity index 87% rename from doc/registrant-api/v1/domain_lock.md rename to doc/registrant-api/v1/registry_lock.md index 7e24e28271..b8d4405548 100644 --- a/doc/registrant-api/v1/domain_lock.md +++ b/doc/registrant-api/v1/registry_lock.md @@ -1,4 +1,4 @@ -# Domain locks +# Registry lock ## POST api/v1/registrant/domains/$UUID/registry_lock @@ -59,12 +59,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 locked" } + { "base": "Domain cannot be locked" } ] } @@ -76,11 +76,23 @@ Content-Type: application/json { "errors": [ - { "base": "domain does not exist" } + { "base": "Domain not found" } ] } ``` +``` +HTTP/1.1 401 +Content-Type: application/json + +{ + "errors": [ + { "base": ["Only administrative contacts can manage registry locks"] } + ] +} + +``` + ## DELETE api/v1/registrant/domains/$UUID/registry_lock @@ -156,7 +168,19 @@ Content-Type: application/json { "errors": [ - { "base": "domain does not exist" } + { "base": "Domain not found" } + ] +} + +``` + +``` +HTTP/1.1 401 +Content-Type: application/json + +{ + "errors": [ + { "base": ["Only administrative contacts can manage registry locks"] } ] } From d725960403cbf72c4f1f60020f966b7d5c5dba31 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Mon, 27 Aug 2018 09:12:47 +0300 Subject: [PATCH 25/37] Fix typo in administered_domains method name --- .../api/v1/registrant/registry_locks_controller.rb | 2 +- app/models/registrant_user.rb | 2 +- test/models/registrant_user_test.rb | 4 ++-- 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 a8c78925bf..8274f5754f 100644 --- a/app/controllers/api/v1/registrant/registry_locks_controller.rb +++ b/app/controllers/api/v1/registrant/registry_locks_controller.rb @@ -35,7 +35,7 @@ def set_domain end def authorized_to_manage_locks? - return if current_user.administrated_domains.include?(@domain) + return if current_user.administered_domains.include?(@domain) render json: { errors: [ { base: ['Only administrative contacts can manage registry locks'] } diff --git a/app/models/registrant_user.rb b/app/models/registrant_user.rb index 18b9506f31..8f39e3da13 100644 --- a/app/models/registrant_user.rb +++ b/app/models/registrant_user.rb @@ -28,7 +28,7 @@ def contacts # 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 + def administered_domains domains_where_is_administrative_contact = begin Domain.joins(:domain_contacts) .where(domain_contacts: { contact_id: contacts, type: [AdminDomainContact] }) diff --git a/test/models/registrant_user_test.rb b/test/models/registrant_user_test.rb index b048299996..3490e35c00 100644 --- a/test/models/registrant_user_test.rb +++ b/test/models/registrant_user_test.rb @@ -19,8 +19,8 @@ def test_domains_returns_an_list_of_distinct_domains_associated_with_a_specific_ refute(domain_names.include?('shop.test')) end - def test_administrated_domains_returns_a_list_of_domains - domain_names = @user.administrated_domains.pluck(:name) + def test_administered_domains_returns_a_list_of_domains + domain_names = @user.administered_domains.pluck(:name) assert_equal(3, domain_names.length) # User is a tech contact for the domain. From e47248e5a3e02f648985fa4e4f137b0bec80e6da Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Mon, 27 Aug 2018 11:47:54 +0300 Subject: [PATCH 26/37] Change current_user to current_registrant_user inside the Registrant API --- app/controllers/api/v1/registrant/base_controller.rb | 4 ++-- .../api/v1/registrant/registry_locks_controller.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/v1/registrant/base_controller.rb b/app/controllers/api/v1/registrant/base_controller.rb index 043d939489..4497d68e6a 100644 --- a/app/controllers/api/v1/registrant/base_controller.rb +++ b/app/controllers/api/v1/registrant/base_controller.rb @@ -37,7 +37,7 @@ def authenticate decryptor.decrypt_token if decryptor.valid? - sign_in decryptor.user + sign_in(:registrant_user, decryptor.user) else render json: { errors: [{ base: ['Not authorized'] }] }, status: :unauthorized @@ -47,7 +47,7 @@ def authenticate # 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 + ::PaperTrail.whodunnit = current_registrant_user.id_role_username 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 8274f5754f..27c98cc9f0 100644 --- a/app/controllers/api/v1/registrant/registry_locks_controller.rb +++ b/app/controllers/api/v1/registrant/registry_locks_controller.rb @@ -26,7 +26,7 @@ def destroy private def set_domain - domain_pool = current_user.domains + domain_pool = current_registrant_user.domains @domain = domain_pool.find_by(uuid: params[:domain_uuid]) return if @domain @@ -35,7 +35,7 @@ def set_domain end def authorized_to_manage_locks? - return if current_user.administered_domains.include?(@domain) + return if current_registrant_user.administered_domains.include?(@domain) render json: { errors: [ { base: ['Only administrative contacts can manage registry locks'] } From 4f982d6d33debfb9106edf63f30916c8b55e22f4 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Mon, 27 Aug 2018 12:24:42 +0300 Subject: [PATCH 27/37] Remove fantom migration --- db/structure.sql | 2 -- 1 file changed, 2 deletions(-) diff --git a/db/structure.sql b/db/structure.sql index 1a402a8647..9d9f4d73e7 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -4762,7 +4762,5 @@ INSERT INTO schema_migrations (version) VALUES ('20180613045614'); INSERT INTO schema_migrations (version) VALUES ('20180713154915'); -INSERT INTO schema_migrations (version) VALUES ('20180808064402'); - INSERT INTO schema_migrations (version) VALUES ('20180824092855'); From 5d4ace8d528a084df0f5872a6627ca7b61149e1f Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Mon, 27 Aug 2018 12:33:22 +0300 Subject: [PATCH 28/37] Revert "Remove fantom migration" This reverts commit 4f982d6d33debfb9106edf63f30916c8b55e22f4. --- db/structure.sql | 2 ++ 1 file changed, 2 insertions(+) diff --git a/db/structure.sql b/db/structure.sql index 9d9f4d73e7..1a402a8647 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -4762,5 +4762,7 @@ INSERT INTO schema_migrations (version) VALUES ('20180613045614'); INSERT INTO schema_migrations (version) VALUES ('20180713154915'); +INSERT INTO schema_migrations (version) VALUES ('20180808064402'); + INSERT INTO schema_migrations (version) VALUES ('20180824092855'); From ba01bb3dfcca3fa5acdaf8751134604c5c0564a5 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Mon, 27 Aug 2018 14:34:12 +0300 Subject: [PATCH 29/37] Fix bug in domains not returning those where a contact was only a registrant --- app/models/registrant_user.rb | 24 ++++++++++++------- .../registrant_api_registry_locks_test.rb | 12 ++++++++++ test/models/registrant_user_test.rb | 6 ++--- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/app/models/registrant_user.rb b/app/models/registrant_user.rb index eeb03a3fdd..ddbf2e664e 100644 --- a/app/models/registrant_user.rb +++ b/app/models/registrant_user.rb @@ -17,29 +17,37 @@ def country_code registrant_ident.to_s.split('-').first 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 domains - Domain.uniq - .joins(:contacts) - .where(contacts: { ident_type: 'priv', ident: ident, ident_country_code: country_code }) + domains_where_is_contact = begin + Domain.joins(:domain_contacts) + .where(domain_contacts: { contact_id: contacts }) + end + + domains_where_is_registrant = Domain.where(registrant_id: contacts) + + Domain.from( + "(#{domains_where_is_registrant.to_sql} UNION " \ + "#{domains_where_is_contact.to_sql}) AS domains" + ) 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 administered_domains 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) + domains_where_is_registrant = Domain.where(registrant_id: contacts) Domain.from( - "(#{domains_where_is_registrar.to_sql} UNION " \ + "(#{domains_where_is_registrant.to_sql} UNION " \ "#{domains_where_is_administrative_contact.to_sql}) AS domains" ) end 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 0b7f31aad7..bb50bdc1b4 100644 --- a/test/integration/api/registrant/registrant_api_registry_locks_test.rb +++ b/test/integration/api/registrant/registrant_api_registry_locks_test.rb @@ -109,6 +109,18 @@ def test_technical_contact_cannot_lock_a_domain response_json) end + def test_registrant_can_lock_a_domain + post '/api/v1/registrant/domains/1b3ee442-e8fe-4922-9492-8fcb9dccc69c/registry_lock', + {}, @auth_headers + + assert_equal(200, response.status) + 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)) + end + private def auth_token diff --git a/test/models/registrant_user_test.rb b/test/models/registrant_user_test.rb index 3490e35c00..298d3a0962 100644 --- a/test/models/registrant_user_test.rb +++ b/test/models/registrant_user_test.rb @@ -13,10 +13,10 @@ def teardown 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) + assert_equal(4, domain_names.length) - # User is a registrant, but not a contact for the domain. - refute(domain_names.include?('shop.test')) + # User is a registrant, but not a contact for the domain. Should be included in the list. + assert(domain_names.include?('shop.test')) end def test_administered_domains_returns_a_list_of_domains From b3a3a8f1a2830f1a54c6701b70c9dd3bc4fe6eb5 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 31 Aug 2018 14:26:06 +0300 Subject: [PATCH 30/37] Rename class to avoid any future collisions --- app/presenters/domain_presenter.rb | 3 ++- test/system/admin_area/domains/registry_lock_test.rb | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/presenters/domain_presenter.rb b/app/presenters/domain_presenter.rb index a4b9423a54..445e5b87db 100644 --- a/app/presenters/domain_presenter.rb +++ b/app/presenters/domain_presenter.rb @@ -66,6 +66,7 @@ def force_delete_toggle_btn def remove_registry_lock_btn return unless domain.locked_by_registrant? + view.link_to(view.t('admin.domains.registry_lock.destroy.btn'), view.admin_domain_registry_lock_path(domain), method: :delete, @@ -79,7 +80,7 @@ def keep_btn 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' + class: 'dropdown-item' 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 9a3276943d..ce09c445cd 100644 --- a/test/system/admin_area/domains/registry_lock_test.rb +++ b/test/system/admin_area/domains/registry_lock_test.rb @@ -1,6 +1,6 @@ require 'test_helper' -class RegistryLockTest < JavaScriptApplicationSystemTestCase +class AdminAreaRegistryLockTest < JavaScriptApplicationSystemTestCase def setup super WebMock.allow_net_connect! From bc36275389464ece911425d287698fd3d638e210 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 31 Aug 2018 14:53:32 +0300 Subject: [PATCH 31/37] Remove Changelog from codeclimate analysis It treats the word bug as `# BUG` comment inside the codebase --- .codeclimate.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.codeclimate.yml b/.codeclimate.yml index be8f1df7af..294b9335cf 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -27,3 +27,4 @@ exclude_patterns: - "vendor/" - "test/" - "spec/" + - "CHANGELOG.md" From 63ac18f0f391a31292657a2c95a4ab882c6f8277 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20V=C3=B5hmar?= Date: Mon, 3 Sep 2018 18:02:22 +0300 Subject: [PATCH 32/37] Changelog update 180904 --- CHANGELOG.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44b66ef4ec..f5f39a382b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,22 @@ +04.09.2018 +* New registrant portal API [#902](https://github.com/internetee/registry/issues/902) +* Registry lock in Registrant API [#927](https://github.com/internetee/registry/issues/927) +* Password encryption for EPP [#914](https://github.com/internetee/registry/issues/914) +* Registrar: 0 amount invoices invalidated [#651](https://github.com/internetee/registry/issues/651) +* Ruby upgrade to 2.4 [#938](https://github.com/internetee/registry/issues/938) +* Admin: removig deleteCandidate status removes Que job as well [#790](https://github.com/internetee/registry/issues/790) +* Admin: Cancel force delete no possible with deleteCandidate status set [#791](https://github.com/internetee/registry/issues/791) +* Contact tests added [#930](https://github.com/internetee/registry/issues/930) +* Change test structure [#924](https://github.com/internetee/registry/issues/924) +* Grape gem update to 1.1.0 (CVE-2018-3769) [#934](https://github.com/internetee/registry/pull/934) +* Remove changelog from codeclimate analysis [#961](https://github.com/internetee/registry/issues/961) +* Remove dead code [#925](https://github.com/internetee/registry/issues/925) +* Quote value in fixture [#937](https://github.com/internetee/registry/issues/937) +* Generate CSS class for every action [#939](https://github.com/internetee/registry/issues/939) +* Add TaskTestCase [#941](https://github.com/internetee/registry/issues/941) +* Set NOT NULL constraint for contact.email field [#936](https://github.com/internetee/registry/issues/936) +* Remove duplicate fixture [#946](https://github.com/internetee/registry/issues/946) + 26.07.2018 * Grape (1.0.3), mustermann (1.0.2), multi_json (1.13.1) gem updates [#912](https://github.com/internetee/registry/issues/912) * Capybara (3.3.1), mini_mime (0.1.3), nokogiri (1.8), rack (1.6.0), xpath (3.1) gem updates [#980](https://github.com/internetee/registry/issues/908) From f0473825e4472b11692e06375968d0d8b4d2d048 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Mon, 3 Sep 2018 19:08:14 +0300 Subject: [PATCH 33/37] Make whois JSON test order independent The test relied on nameservers being inserted in specific order. For whatever reason, that order can change, but it does not make the record invalid. [ns1.google.com, ns2.google.com] is semantically equal to [ns2.google.com, ns1.google.com] --- test/models/whois_record_test.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/models/whois_record_test.rb b/test/models/whois_record_test.rb index 438ee9cc4f..e86deafd95 100644 --- a/test/models/whois_record_test.rb +++ b/test/models/whois_record_test.rb @@ -22,7 +22,6 @@ def test_generated_json_has_expected_values registrant_kind: 'priv', email: 'john@inbox.test', expire: '2010-07-05', - nameservers: ['ns1.bestnames.test', 'ns2.bestnames.test'], registrar_address: 'Main Street, New York, New York, 12345', dnssec_keys: [], } @@ -30,6 +29,10 @@ def test_generated_json_has_expected_values expected_partial_hash.each do |key, value| assert_equal(value, @record.generated_json[key]) end + + ['ns1.bestnames.test', 'ns2.bestnames.test'].each do |item| + assert(@record.generated_json[:nameservers].include?(item)) + end end def test_generated_body_has_justified_disclaimer From 840b6da1be52e9be93d6400396a2120bf3d51653 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Thu, 6 Sep 2018 11:54:06 +0300 Subject: [PATCH 34/37] Change callback method to fix 401 reported by a user --- app/controllers/registrant/domain_delete_confirms_controller.rb | 2 +- app/controllers/registrant/domain_update_confirms_controller.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/registrant/domain_delete_confirms_controller.rb b/app/controllers/registrant/domain_delete_confirms_controller.rb index d6e4666c7e..59492fefbe 100644 --- a/app/controllers/registrant/domain_delete_confirms_controller.rb +++ b/app/controllers/registrant/domain_delete_confirms_controller.rb @@ -1,5 +1,5 @@ class Registrant::DomainDeleteConfirmsController < RegistrantController - skip_before_action :authenticate_user!, only: [:show, :update] + skip_before_action :authenticate_registrant_user!, only: [:show, :update] skip_authorization_check only: [:show, :update] def show diff --git a/app/controllers/registrant/domain_update_confirms_controller.rb b/app/controllers/registrant/domain_update_confirms_controller.rb index 413ac43ff0..d0bc1e7a13 100644 --- a/app/controllers/registrant/domain_update_confirms_controller.rb +++ b/app/controllers/registrant/domain_update_confirms_controller.rb @@ -1,5 +1,5 @@ class Registrant::DomainUpdateConfirmsController < RegistrantController - skip_before_action :authenticate_user!, only: %i[show update] + skip_before_action :authenticate_registrant_user!, only: %i[show update] skip_authorization_check only: %i[show update] def show From 087f890a256c075ece1bfd78cc055f1689785df8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20V=C3=B5hmar?= Date: Thu, 6 Sep 2018 12:13:38 +0300 Subject: [PATCH 35/37] Changelog update 180906 --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5f39a382b..a2dd4e76a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +06.09.2018 +* Bug: registrant confirmation does not require authentication any more [#969](https://github.com/internetee/registry/issues/969) +* Whois JSON tests order independent [#965](https://github.com/internetee/registry/issues/965) + 04.09.2018 * New registrant portal API [#902](https://github.com/internetee/registry/issues/902) * Registry lock in Registrant API [#927](https://github.com/internetee/registry/issues/927) From 0fef419799b2aeee805f917b81a6b1804fc8da90 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Wed, 12 Sep 2018 13:55:22 +0300 Subject: [PATCH 36/37] Fix redirection after sign-in Closes #973 --- app/controllers/registrar/sessions_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/registrar/sessions_controller.rb b/app/controllers/registrar/sessions_controller.rb index 8f4db9fdd2..991d5cbed7 100644 --- a/app/controllers/registrar/sessions_controller.rb +++ b/app/controllers/registrar/sessions_controller.rb @@ -115,7 +115,7 @@ def mid_status sign_in(:registrar_user, @user) flash[:notice] = t(:welcome) flash.keep(:notice) - render js: "window.location = '#{registrar_root_url}'" + render js: "window.location = '#{after_sign_in_path_for(@user)}'" when 'NOT_VALID' render json: { message: t(:user_signature_is_invalid) }, status: :bad_request when 'EXPIRED_TRANSACTION' From 6f4fe945855bfb450ce9faeddaa75bf040290d6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20V=C3=B5hmar?= Date: Wed, 12 Sep 2018 14:33:19 +0300 Subject: [PATCH 37/37] Changelog update 180912 --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2dd4e76a9..7ed78ec6f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +12.09.2018 +* Bug: user with billing access only can now login to the portal for Regsitrars [#973](https://github.com/internetee/registry/issues/973) + 06.09.2018 * Bug: registrant confirmation does not require authentication any more [#969](https://github.com/internetee/registry/issues/969) * Whois JSON tests order independent [#965](https://github.com/internetee/registry/issues/965)