From 8b527bb15370551f2049980e7758b5c77bc77e72 Mon Sep 17 00:00:00 2001 From: Priit Tark Date: Tue, 28 Jul 2015 23:40:41 +0300 Subject: [PATCH 1/6] Refactor pendings #2773 --- app/models/domain.rb | 62 ++++++++++++++++++++++++++++++++------ app/models/epp/domain.rb | 4 ++- spec/models/domain_spec.rb | 54 +++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 11 deletions(-) diff --git a/app/models/domain.rb b/app/models/domain.rb index 58ea14186..16353a18d 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -353,10 +353,6 @@ class Domain < ActiveRecord::Base save end - def pending_update? - statuses.include?(DomainStatus::PENDING_UPDATE) - end - def pending_update! return true if pending_update? self.epp_pending_update = true # for epp @@ -378,7 +374,7 @@ class Domain < ActiveRecord::Base self.pending_json = pending_json_cache self.registrant_verification_token = token self.registrant_verification_asked_at = asked_at - self.statuses = [DomainStatus::PENDING_UPDATE] + self.statuses = DomainStatus::PENDING_UPDATE pending_json[:domain] = changes_cache pending_json[:new_registrant_id] = new_registrant_id pending_json[:new_registrant_email] = new_registrant_email @@ -422,16 +418,12 @@ class Domain < ActiveRecord::Base self.registrant_verification_token = SecureRandom.hex(42) end - def pending_delete? - statuses.include?(DomainStatus::PENDING_DELETE) - end - def pending_delete! return true if pending_delete? self.epp_pending_delete = true # for epp return true unless registrant_verification_asked? - self.statuses = [DomainStatus::PENDING_DELETE] + self.statuses = DomainStatus::PENDING_DELETE save(validate: false) # should check if this did succeed DomainMailer.pending_deleted(self).deliver_now @@ -570,6 +562,56 @@ class Domain < ActiveRecord::Base save(validate: false) end + def pending_update? + statuses.include?(DomainStatus::PENDING_UPDATE) + end + + # TODO: Review the list and disallow epp calls + def pending_update_prohibited? + (statuses & [ + DomainStatus::CLIENT_UPDATE_PROHIBITED, + DomainStatus::SERVER_UPDATE_PROHIBITED, + DomainStatus::PENDING_CREATE, + DomainStatus::PENDING_UPDATE, + DomainStatus::PENDING_DELETE, + DomainStatus::PENDING_RENEW, + DomainStatus::PENDING_TRANSFER + ]).present? + end + + def set_pending_update + if pending_update_prohibited? + logger.info "DOMAIN STATUS UPDATE ISSUE ##{id}: PENDING_UPDATE not allowed to set. [#{statuses}]" + return nil + end + statuses << DomainStatus::PENDING_UPDATE + end + + def pending_delete? + statuses.include?(DomainStatus::PENDING_DELETE) + end + + # TODO: Review the list and disallow epp calls + def pending_delete_prohibited? + (statuses & [ + DomainStatus::CLIENT_DELETE_PROHIBITED, + DomainStatus::SERVER_DELETE_PROHIBITED, + DomainStatus::PENDING_CREATE, + DomainStatus::PENDING_UPDATE, + DomainStatus::PENDING_DELETE, + DomainStatus::PENDING_RENEW, + DomainStatus::PENDING_TRANSFER + ]).present? + end + + def set_pending_delete + if pending_delete_prohibited? + logger.info "DOMAIN STATUS UPDATE ISSUE ##{id}: PENDING_DELETE not allowed to set. [#{statuses}]" + return nil + end + statuses << DomainStatus::PENDING_DELETE + end + def manage_automatic_statuses # domain_statuses.create(value: DomainStatus::DELETE_CANDIDATE) if delete_candidateable? if statuses.empty? && valid? diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb index 2cd3458a4..c1ff9ae8d 100644 --- a/app/models/epp/domain.rb +++ b/app/models/epp/domain.rb @@ -411,7 +411,9 @@ class Epp::Domain < Domain # at[:statuses] += at_add[:domain_statuses_attributes] - if verify && frame.css('registrant').present? && frame.css('registrant').attr('verified').to_s.downcase != 'yes' + if verify && + frame.css('registrant').present? && + frame.css('registrant').attr('verified').to_s.downcase != 'yes' registrant_verification_asked!(frame.to_s, current_user.id) end self.deliver_emails = true # turn on email delivery for epp diff --git a/spec/models/domain_spec.rb b/spec/models/domain_spec.rb index dcb344c93..e39300e74 100644 --- a/spec/models/domain_spec.rb +++ b/spec/models/domain_spec.rb @@ -37,6 +37,22 @@ describe Domain do it 'should not be registrant update confirm ready' do @domain.registrant_update_confirmable?('123').should == false end + + it 'should not have pending update' do + @domain.pending_update?.should == false + end + + it 'should allow pending update' do + @domain.pending_update_prohibited?.should == false + end + + it 'should not have pending delete' do + @domain.pending_delete?.should == false + end + + it 'should allow pending delete' do + @domain.pending_delete_prohibited?.should == false + end end context 'with valid attributes' do @@ -328,6 +344,44 @@ describe Domain do domain.pricelist('renew').price.amount.should == 6.1 end + it 'should set pending update' do + @domain.statuses = DomainStatus::OK # restore + @domain.pending_update?.should == false + + @domain.set_pending_update + @domain.pending_update?.should == true + @domain.statuses = DomainStatus::OK # restore + end + + it 'should not set pending update' do + @domain.statuses = DomainStatus::OK # restore + @domain.statuses << DomainStatus::CLIENT_UPDATE_PROHIBITED + + @domain.set_pending_update.should == nil # not updated + @domain.pending_update?.should == false + @domain.statuses = DomainStatus::OK # restore + end + + it 'should set pending delete' do + @domain.statuses = DomainStatus::OK # restore + @domain.pending_delete?.should == false + + @domain.set_pending_delete.should == ['pendingDelete'] + @domain.pending_delete?.should == true + @domain.statuses = DomainStatus::OK # restore + end + + it 'should not set pending delele' do + @domain.statuses = DomainStatus::OK # restore + @domain.pending_delete?.should == false + @domain.statuses << DomainStatus::CLIENT_DELETE_PROHIBITED + + @domain.set_pending_delete.should == nil + + @domain.pending_delete?.should == false + @domain.statuses = DomainStatus::OK # restore + end + context 'about registrant update confirm' do before :all do @domain.registrant_verification_token = 123 From 446576f5b04e41b4e39319554d7694df169da06f Mon Sep 17 00:00:00 2001 From: Priit Tark Date: Tue, 28 Jul 2015 23:42:14 +0300 Subject: [PATCH 2/6] Registrant pending/delete don't overwrite old statuses #2557 --- app/models/domain.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/domain.rb b/app/models/domain.rb index 16353a18d..dc6187eaa 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -374,7 +374,7 @@ class Domain < ActiveRecord::Base self.pending_json = pending_json_cache self.registrant_verification_token = token self.registrant_verification_asked_at = asked_at - self.statuses = DomainStatus::PENDING_UPDATE + set_pending_update pending_json[:domain] = changes_cache pending_json[:new_registrant_id] = new_registrant_id pending_json[:new_registrant_email] = new_registrant_email @@ -423,7 +423,7 @@ class Domain < ActiveRecord::Base self.epp_pending_delete = true # for epp return true unless registrant_verification_asked? - self.statuses = DomainStatus::PENDING_DELETE + set_pending_delete save(validate: false) # should check if this did succeed DomainMailer.pending_deleted(self).deliver_now From a809329421ecd2280e42016b70a60649302a0907 Mon Sep 17 00:00:00 2001 From: Priit Tark Date: Wed, 29 Jul 2015 00:01:40 +0300 Subject: [PATCH 3/6] Turn on pending delete/change settings #2785 --- app/controllers/admin/settings_controller.rb | 4 +++- app/models/epp/domain.rb | 7 +++++-- app/views/admin/settings/index.haml | 2 ++ config/initializers/initial_settings.rb | 2 ++ 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/app/controllers/admin/settings_controller.rb b/app/controllers/admin/settings_controller.rb index 7cdcf323b..615ee63ca 100644 --- a/app/controllers/admin/settings_controller.rb +++ b/app/controllers/admin/settings_controller.rb @@ -68,7 +68,9 @@ class Admin::SettingsController < AdminController :key_data_allowed, :client_side_status_editing_enabled, :registrar_ip_whitelist_enabled, - :api_ip_whitelist_enabled + :api_ip_whitelist_enabled, + :request_confrimation_on_registrant_change_enabled, + :request_confirmation_on_domain_deletion_enabled ] params[:settings].each do |k, v| diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb index c1ff9ae8d..a11e6152b 100644 --- a/app/models/epp/domain.rb +++ b/app/models/epp/domain.rb @@ -411,7 +411,7 @@ class Epp::Domain < Domain # at[:statuses] += at_add[:domain_statuses_attributes] - if verify && + if verify && Setting.request_confrimation_on_registrant_change_enabled && frame.css('registrant').present? && frame.css('registrant').attr('verified').to_s.downcase != 'yes' registrant_verification_asked!(frame.to_s, current_user.id) @@ -457,7 +457,10 @@ class Epp::Domain < Domain def epp_destroy(frame, user_id, verify = true) return false unless valid? - if verify && frame.css('delete').attr('verified').to_s.downcase != 'yes' + if verify && + Setting.request_confirmation_on_domain_deletion_enabled && + frame.css('delete').attr('verified').to_s.downcase != 'yes' + registrant_verification_asked!(frame.to_s, user_id) self.deliver_emails = true # turn on email delivery for epp pending_delete! diff --git a/app/views/admin/settings/index.haml b/app/views/admin/settings/index.haml index 7b727146d..2c46c1f2e 100644 --- a/app/views/admin/settings/index.haml +++ b/app/views/admin/settings/index.haml @@ -52,6 +52,8 @@ = render 'setting_row', var: :client_side_status_editing_enabled = render 'setting_row', var: :api_ip_whitelist_enabled = render 'setting_row', var: :registrar_ip_whitelist_enabled + = render 'setting_row', var: :request_confrimation_on_registrant_change_enabled + = render 'setting_row', var: :request_confirmation_on_domain_deletion_enabled .panel.panel-default .panel-heading.clearfix diff --git a/config/initializers/initial_settings.rb b/config/initializers/initial_settings.rb index 9fbea8343..c2d867c26 100644 --- a/config/initializers/initial_settings.rb +++ b/config/initializers/initial_settings.rb @@ -22,6 +22,8 @@ if con.present? && con.table_exists?('settings') Setting.save_default(:ns_max_count, 11) Setting.save_default(:transfer_wait_time, 0) + Setting.save_default(:request_confrimation_on_registrant_change_enabled, true) + Setting.save_default(:request_confirmation_on_domain_deletion_enabled, true) Setting.save_default(:client_side_status_editing_enabled, false) From c5a8a6909924d15d4fe81293112e96eef216304a Mon Sep 17 00:00:00 2001 From: Priit Tark Date: Wed, 29 Jul 2015 11:53:17 +0300 Subject: [PATCH 4/6] Added unified update_prohibited method for epp #2786 --- app/models/domain.rb | 4 ++++ app/models/epp/domain.rb | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/domain.rb b/app/models/domain.rb index dc6187eaa..23228a20e 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -566,6 +566,10 @@ class Domain < ActiveRecord::Base statuses.include?(DomainStatus::PENDING_UPDATE) end + def update_prohibited? + pending_update_prohibited? && pending_delete_prohibited? + end + # TODO: Review the list and disallow epp calls def pending_update_prohibited? (statuses & [ diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb index a11e6152b..e89c62dcb 100644 --- a/app/models/epp/domain.rb +++ b/app/models/epp/domain.rb @@ -4,7 +4,7 @@ class Epp::Domain < Domain before_validation :manage_permissions def manage_permissions - return unless pending_update? || pending_delete? + return unless update_prohibited? add_epp_error('2304', nil, nil, I18n.t(:object_status_prohibits_operation)) false end From a784eb47692b72be210d924fc831b3eae7edd44b Mon Sep 17 00:00:00 2001 From: Priit Tark Date: Fri, 31 Jul 2015 10:51:23 +0300 Subject: [PATCH 5/6] Email content update --- .../pending_update_notification_for_new_registrant.html.erb | 6 ++---- .../pending_update_notification_for_new_registrant.text.erb | 2 -- .../pending_update_request_for_old_registrant.html.erb | 6 ++++-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/app/views/mailers/domain_mailer/pending_update_notification_for_new_registrant.html.erb b/app/views/mailers/domain_mailer/pending_update_notification_for_new_registrant.html.erb index 27bec4401..df31c9139 100644 --- a/app/views/mailers/domain_mailer/pending_update_notification_for_new_registrant.html.erb +++ b/app/views/mailers/domain_mailer/pending_update_notification_for_new_registrant.html.erb @@ -17,9 +17,7 @@ Riik: <%= @domain.registrant_country %>

Juhime Teie tähelepanu asjaolule, et omanikuvahetuse protseduur viiaks lõpule vaid juhul, kui domeeni hetkel kehtiv registreerija <%= @old_registrant.name %> omanikuvahetuse tähtaegselt kinnitab.

-Juhul kui <%= @old_registrant.name %> lükkab omanikuvahtuse taotluse tagasi või ei anna kinnitust enne <%= Setting.expire_pending_confirmation %> tundi, omanikuvahetuse protseduur tühistatakse.
-

-Taotlus on aktiivne <%= Setting.expire_pending_confirmation %> tundi ja lükatakse automaatselt tagasi kui te seda enne ise ei kinnita või tagasi lükka. +Juhul kui <%= @old_registrant.name %> lükkab omanikuvahtuse taotluse tagasi või ei anna kinnitust enne <%= Setting.expire_pending_confirmation %> tundi, omanikuvahetuse protseduur tühistatakse.

Küsimuste korral palun võtke ühendust registripidajaga <%= @domain.registrar_name %>, kelle kontaktid leiate http://internet.ee/registripidajad/akrediteeritud-registripidajad

@@ -41,7 +39,7 @@ Business Registry code: <%= @domain.registrant_ident %>
<% end %> Street: <%= @domain.registrant_street %>
City: <%= @domain.registrant_city %>
-Country: <%= @domain.registrant_country %>
+Country: <%= @domain.registrant_country %>

Please contact to your registrar <%= @domain.registrar_name %> if you have any questions.

diff --git a/app/views/mailers/domain_mailer/pending_update_notification_for_new_registrant.text.erb b/app/views/mailers/domain_mailer/pending_update_notification_for_new_registrant.text.erb index e4a401977..80265c05f 100644 --- a/app/views/mailers/domain_mailer/pending_update_notification_for_new_registrant.text.erb +++ b/app/views/mailers/domain_mailer/pending_update_notification_for_new_registrant.text.erb @@ -19,8 +19,6 @@ Juhime Teie tähelepanu asjaolule, et omanikuvahetuse protseduur viiaks lõpule Juhul kui <%= @old_registrant.name %> lükkab omanikuvahtuse taotluse tagasi või ei anna kinnitust enne <%= Setting.expire_pending_confirmation %> tundi, omanikuvahetuse protseduur tühistatakse. -Taotlus on aktiivne <%= Setting.expire_pending_confirmation %> tundi ja lükatakse automaatselt tagasi kui te seda enne ise ei kinnita või tagasi lükka. - Küsimuste korral palun võtke ühendust registripidajaga <%= @domain.registrar_name %>, kelle kontaktid leiate http://internet.ee/registripidajad/akrediteeritud-registripidajad Lugupidamisega diff --git a/app/views/mailers/domain_mailer/pending_update_request_for_old_registrant.html.erb b/app/views/mailers/domain_mailer/pending_update_request_for_old_registrant.html.erb index 2689eff68..c62cb5561 100644 --- a/app/views/mailers/domain_mailer/pending_update_request_for_old_registrant.html.erb +++ b/app/views/mailers/domain_mailer/pending_update_request_for_old_registrant.html.erb @@ -13,7 +13,8 @@ Tänav: <%= @domain.registrant_street %>
Linn: <%= @domain.registrant_city %>
Riik: <%= @domain.registrant_country %>

-Taotlus on aktiivne <%= Setting.expire_pending_confirmation %> tundi ja lükatakse automaatselt tagasi kui te seda enne ise ei kinnita või tagasi lükka.
+Taotlus on aktiivne <%= Setting.expire_pending_confirmation %> tundi ja lükatakse automaatselt tagasi kui te seda enne ise ei kinnita või tagasi lükka. +

Muudatuse kinnitamiseks külastage palun allolevat võrgulehekülge, kontrollige uuesti üle muudatuse andmed ning vajutage nuppu kinnitan:
<%= link_to @verification_url, @verification_url %> @@ -38,7 +39,8 @@ Street: <%= @domain.registrant_street %>
City: <%= @domain.registrant_city %>
Country: <%= @domain.registrant_country %>

-The application will remain in pending status for <%= Setting.expire_pending_confirmation %> hrs and will be automaticcally rejected if it is not approved nor rejected before.
+The application will remain in pending status for <%= Setting.expire_pending_confirmation %> hrs and will be automaticcally rejected if it is not approved nor rejected before. +

To confirm the update please visit this website, once again review the data and press approve:
<%= link_to @verification_url, @verification_url %>

From 19f61dd30f285b138e4bc9d01b34316da69352ac Mon Sep 17 00:00:00 2001 From: Priit Tark Date: Fri, 31 Jul 2015 12:51:32 +0300 Subject: [PATCH 6/6] Update automatic statuses for domain pending #2731 --- app/models/domain.rb | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/app/models/domain.rb b/app/models/domain.rb index 23228a20e..48f8c4960 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -62,19 +62,20 @@ class Domain < ActiveRecord::Base before_create :generate_auth_info before_create :set_validity_dates before_create -> { self.reserved = in_reserved_list?; nil } - before_update :manage_statuses - def manage_statuses - return unless registrant_id_changed? - pending_update! if registrant_verification_asked? - true - end before_save :manage_automatic_statuses - before_save :touch_always_version def touch_always_version self.updated_at = Time.zone.now end + + before_update :manage_statuses + def manage_statuses + return unless registrant_id_changed? # rollback has not yet happened + pending_update! if registrant_verification_asked? + true + end + after_save :update_whois_record after_create :update_reserved_domains @@ -379,6 +380,12 @@ class Domain < ActiveRecord::Base pending_json[:new_registrant_id] = new_registrant_id pending_json[:new_registrant_email] = new_registrant_email pending_json[:new_registrant_name] = new_registrant_name + + # This pending_update! method is triggered by before_update + # Note, all before_save callbacks are excecuted before before_update, + # thus automatic statuses has already excectued by this point + # and we need to trigger automatic statuses manually (second time). + manage_automatic_statuses end # rubocop: disable Metrics/CyclomaticComplexity