diff --git a/app/controllers/epp/domains_controller.rb b/app/controllers/epp/domains_controller.rb index 840a80e6e..6050e9d5d 100644 --- a/app/controllers/epp/domains_controller.rb +++ b/app/controllers/epp/domains_controller.rb @@ -34,8 +34,8 @@ class Epp::DomainsController < EppController authorize! :update, @domain, @password if @domain.update(params[:parsed_frame], current_user) - if @domain.pending_update? - render_epp_response '/epp/domains/success_pending' + if @domain.epp_pending_update.present? + render_epp_response '/epp/shared/success_pending' else render_epp_response '/epp/domains/success' end diff --git a/app/controllers/epp_controller.rb b/app/controllers/epp_controller.rb index a43f1bc59..77cd86b87 100644 --- a/app/controllers/epp_controller.rb +++ b/app/controllers/epp_controller.rb @@ -9,6 +9,7 @@ class EppController < ApplicationController rescue_from CanCan::AccessDenied do |_exception| @errors ||= [] + if @errors.blank? @errors = [{ msg: t('errors.messages.epp_authorization_error'), @@ -50,6 +51,7 @@ class EppController < ApplicationController def handle_errors(obj = nil) @errors ||= [] + if obj obj.construct_epp_errors @errors += obj.errors[:epp_errors] diff --git a/app/controllers/registrant/domain_update_confirms_controller.rb b/app/controllers/registrant/domain_update_confirms_controller.rb index 6f2d4b46d..fac3d0181 100644 --- a/app/controllers/registrant/domain_update_confirms_controller.rb +++ b/app/controllers/registrant/domain_update_confirms_controller.rb @@ -4,11 +4,7 @@ class Registrant::DomainUpdateConfirmsController < RegistrantController def show @domain = Domain.find(params[:id]) - - # if @domain.present? && params[:token].present? && @domain.registrant_verification_token == params[:token] - # else - # @domain = nil - # end + @domain = nil unless @domain.registrant_update_confirmable?(params[:token]) end def create diff --git a/app/mailers/domain_mailer.rb b/app/mailers/domain_mailer.rb index 131018672..b5cb8188e 100644 --- a/app/mailers/domain_mailer.rb +++ b/app/mailers/domain_mailer.rb @@ -2,10 +2,17 @@ class DomainMailer < ApplicationMailer def registrant_updated(domain) @domain = domain return if Rails.env.production? ? false : !TEST_EMAILS.include?(@domain.registrant_email) + # turn on delivery on specific request only, thus rake tasks does not deliver anything return if @domain.deliver_emails != true + if @domain.registrant_verification_token.blank? - logger.warn "EMAIL DID NOT DELIVERED: registrant_verification_token is missing for #{@domain.name}" + logger.warn "EMAIL NOT DELIVERED: registrant_verification_token is missing for #{@domain.name}" + return + end + + if @domain.registrant_verification_asked_at.blank? + logger.warn "EMAIL NOT DELIVERED: registrant_verification_asked_at is missing for #{@domain.name}" return end diff --git a/app/models/domain.rb b/app/models/domain.rb index 1aa845ca4..1ab6bc143 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -18,7 +18,6 @@ class Domain < ActiveRecord::Base has_many :contacts, through: :domain_contacts, source: :contact has_many :admin_contacts, through: :admin_domain_contacts, source: :contact has_many :tech_contacts, through: :tech_domain_contacts, source: :contact - has_many :nameservers, dependent: :destroy accepts_nested_attributes_for :nameservers, allow_destroy: true, @@ -59,10 +58,7 @@ class Domain < ActiveRecord::Base before_update :manage_statuses def manage_statuses return unless registrant_id_changed? - if registrant_verification_asked_at.present? - domain_statuses.build(value: DomainStatus::PENDING_UPDATE) - DomainMailer.registrant_updated(self).deliver_now - end + pending_update! if registrant_verification_asked? true end @@ -121,7 +117,7 @@ class Domain < ActiveRecord::Base validate :validate_nameserver_ips - attr_accessor :registrant_typeahead, :update_me, :deliver_emails + attr_accessor :registrant_typeahead, :update_me, :deliver_emails, :epp_pending_update def subordinate_nameservers nameservers.select { |x| x.hostname.end_with?(name) } @@ -177,7 +173,38 @@ class Domain < ActiveRecord::Base #{DomainStatus::PENDING_UPDATE} )).present? end - alias_method :update_pending?, :pending_update? + + def pending_update! + return true if pending_update? + self.epp_pending_update = true # for handling epp errors correctly + + return true unless registrant_verification_asked? + pending_json_cache = all_changes + DomainMailer.registrant_updated(self).deliver_now + + reload # revert back to original + + self.pending_json = pending_json_cache + domain_statuses.create(value: DomainStatus::PENDING_UPDATE) + end + + def registrant_update_confirmable?(token) + return false unless pending_update? + return false if registrant_verification_token.blank? + return false if registrant_verification_asked_at.blank? + return false if token.blank? + return false if registrant_verification_token != token + true + end + + def registrant_verification_asked? + registrant_verification_asked_at.present? && registrant_verification_token.present? + end + + def registrant_verification_asked! + self.registrant_verification_asked_at = Time.zone.now + self.registrant_verification_token = SecureRandom.hex(42) + end ### VALIDATIONS ### @@ -266,6 +293,10 @@ class Domain < ActiveRecord::Base log end + def all_changes + changes + end + def update_whois_record whois_record.blank? ? create_whois_record : whois_record.save end diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb index d6471309f..d7ca57c56 100644 --- a/app/models/epp/domain.rb +++ b/app/models/epp/domain.rb @@ -2,9 +2,9 @@ class Epp::Domain < Domain include EppErrors - before_update :manage_permissions + before_validation :manage_permissions def manage_permissions - return unless update_pending? + return unless pending_update? add_epp_error('2304', nil, nil, I18n.t(:object_status_prohibits_operation)) false end @@ -95,9 +95,7 @@ class Epp::Domain < Domain regt = Registrant.find_by(code: code) if regt at[:registrant_id] = regt.id - delivery_date = frame.css('registrant').attr('verified').to_s.downcase == 'yes' ? nil : Time.zone.now - at[:registrant_verification_asked_at] = delivery_date - at[:registrant_verification_token] = SecureRandom.hex(42) + registrant_verification_asked! if frame.css('registrant').attr('verified').to_s.downcase != 'yes' else add_epp_error('2303', 'registrant', code, [:registrant, :not_found]) end diff --git a/app/models/pending.rb b/app/models/pending.rb new file mode 100644 index 000000000..07ab0bd88 --- /dev/null +++ b/app/models/pending.rb @@ -0,0 +1,3 @@ +class Pending < ActiveRecord::Base + belongs_to :domain +end diff --git a/app/models/registrant_verification.rb b/app/models/registrant_verification.rb new file mode 100644 index 000000000..1b41b5a02 --- /dev/null +++ b/app/models/registrant_verification.rb @@ -0,0 +1,5 @@ +# Used in Registrant portal to collect registrant verifications +# Registrant postgres user can access this table directly. +class RegistrantVerification < ActiveRecord::Base + validates :verification_token, :domain_name, presence: true +end diff --git a/app/views/epp/shared/success_pending.xml.builder b/app/views/epp/shared/success_pending.xml.builder new file mode 100644 index 000000000..3d1783009 --- /dev/null +++ b/app/views/epp/shared/success_pending.xml.builder @@ -0,0 +1,9 @@ +xml.epp_head do + xml.response do + xml.result('code' => '1001') do + xml.msg 'Command completed successfully; action pending' + end + end + + xml << render('/epp/shared/trID') +end diff --git a/app/views/registrant/domain_update_confirms/show.haml b/app/views/registrant/domain_update_confirms/show.haml index ee495fa38..0484fc153 100644 --- a/app/views/registrant/domain_update_confirms/show.haml +++ b/app/views/registrant/domain_update_confirms/show.haml @@ -1,2 +1,4 @@ -- if @domain.blank? - %h1= t(:not_valid_domain_verification) +- if @domain.present? +- else + %h1= t(:not_valid_domain_verification_title).html_safe + %p= t(:not_valid_domain_verification_body).html_safe diff --git a/config/locales/en.yml b/config/locales/en.yml index c739fd53e..023041d06 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -768,4 +768,6 @@ en: domain_registrant_update_subject: "Kinnitustaotlus domeeni %{name} registreerija vahetuseks / Application for approval for registrant chache of %{name}" whois: WHOIS login_failed_check_id_card: 'Log in failed, check ID card' - not_valid_domain_verification: Not valid domain update verification + not_valid_domain_verification_title: Domain verification not available + not_valid_domain_verification_body: This could mean your verification has been expired or done already.

Please contact us if you think something is wrong. + diff --git a/db/migrate/20150515103222_add_pending_requests.rb b/db/migrate/20150515103222_add_pending_requests.rb new file mode 100644 index 000000000..6174acec0 --- /dev/null +++ b/db/migrate/20150515103222_add_pending_requests.rb @@ -0,0 +1,5 @@ +class AddPendingRequests < ActiveRecord::Migration + def change + add_column :domains, :pending_json, :json + end +end diff --git a/db/migrate/20150518084324_add_registrant_verifications.rb b/db/migrate/20150518084324_add_registrant_verifications.rb new file mode 100644 index 000000000..c06bce6cd --- /dev/null +++ b/db/migrate/20150518084324_add_registrant_verifications.rb @@ -0,0 +1,10 @@ +class AddRegistrantVerifications < ActiveRecord::Migration + def change + create_table :registrant_verifications do |t| + t.string :domain_name + t.string :verification_token + t.timestamps + end + add_index :registrant_verifications, :created_at + end +end diff --git a/db/schema.rb b/db/schema.rb index 834737375..841976d41 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150514132606) do +ActiveRecord::Schema.define(version: 20150518084324) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -302,6 +302,7 @@ ActiveRecord::Schema.define(version: 20150514132606) do t.datetime "delete_at" t.datetime "registrant_verification_asked_at" t.string "registrant_verification_token" + t.json "pending_json" end add_index "domains", ["delete_at"], name: "index_domains_on_delete_at", using: :btree @@ -839,6 +840,15 @@ ActiveRecord::Schema.define(version: 20150514132606) do add_index "nameservers", ["domain_id"], name: "index_nameservers_on_domain_id", using: :btree + create_table "registrant_verifications", force: :cascade do |t| + t.string "domain_name" + t.string "verification_token" + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index "registrant_verifications", ["created_at"], name: "index_registrant_verifications_on_created_at", using: :btree + create_table "registrars", force: :cascade do |t| t.string "name" t.string "reg_no" diff --git a/spec/epp/domain_spec.rb b/spec/epp/domain_spec.rb index c1eed789d..d57c2445f 100644 --- a/spec/epp/domain_spec.rb +++ b/spec/epp/domain_spec.rb @@ -1379,7 +1379,7 @@ describe 'EPP Domain', epp: true do d.registrant_code.should == 'FIXED:CITIZEN_1234' d.auth_info.should == existing_pw - d.update_pending?.should == false + d.pending_update?.should == false end it 'updates a domain' do @@ -1406,12 +1406,12 @@ describe 'EPP Domain', epp: true do d = Domain.last - d.registrant_code.should == 'FIXED:CITIZEN_1234' + d.registrant_code.should_not == 'FIXED:CITIZEN_1234' # should not update, because pending d.auth_info.should == existing_pw - d.update_pending?.should == true + d.pending_update?.should == true end - it 'should not allow any update when status update_pending' do + it 'should not allow any update when status pending update' do domain.domain_statuses.create(value: DomainStatus::PENDING_UPDATE) existing_pw = domain.auth_info @@ -1439,7 +1439,7 @@ describe 'EPP Domain', epp: true do d.registrant_code.should_not == 'FIXED:CITIZEN_1234' d.auth_info.should == existing_pw - d.update_pending?.should == true + d.pending_update?.should == true end it 'updates domain and adds objects' do @@ -1567,6 +1567,85 @@ describe 'EPP Domain', epp: true do d.domain_statuses.count.should == 2 end + it 'updates domain with registrant change what triggers action pending' do + xml = domain_update_xml({ + name: { value: domain.name }, + chg: [ + registrant: { value: 'FIXED:CITIZEN_1234' } + ], + add: [ + { + ns: [ + { + hostAttr: [ + { hostName: { value: 'ns1.example.com' } } + ] + }, + { + hostAttr: [ + { hostName: { value: 'ns2.example.com' } } + ] + } + ] + }, + _anonymus: [ + { contact: { value: 'FIXED:PENDINGMAK21', attrs: { type: 'tech' } } }, + { status: { value: 'Payment overdue.', attrs: { s: 'clientHold', lang: 'en' } } }, + { status: { value: '', attrs: { s: 'clientUpdateProhibited' } } } + ] + ] + }, { + add: [ + { keyData: { + flags: { value: '0' }, + protocol: { value: '3' }, + alg: { value: '5' }, + pubKey: { value: '700b97b591ed27ec2590d19f06f88bba700b97b591ed27ec2590d19f' } + } + }, + { + keyData: { + flags: { value: '256' }, + protocol: { value: '3' }, + alg: { value: '254' }, + pubKey: { value: '841936717ae427ace63c28d04918569a841936717ae427ace63c28d0' } + } + } + ] + }, + { + _anonymus: [ + legalDocument: { + value: 'JVBERi0xLjQKJcOkw7zDtsOfCjIgMCBvYmoKPDwvTGVuZ3RoIDMgMCBSL0Zp==', + attrs: { type: 'pdf' } + } + ] + }) + + response = epp_plain_request(xml, :xml) + response[:results][0][:msg].should == 'Contact was not found' + response[:results][0][:result_code].should == '2303' + + Fabricate(:contact, code: 'FIXED:PENDINGMAK21') + + response = epp_plain_request(xml, :xml) + response[:results][0][:msg].should == 'Command completed successfully; action pending' + response[:results][0][:result_code].should == '1001' + + d = Domain.last + + new_ns_count = d.nameservers.where(hostname: ['ns1.example.com', 'ns2.example.com']).count + new_ns_count.should == 0 # aka old value + + new_contact = d.tech_contacts.find_by(code: 'FIXED:PENDINGMAK21') + new_contact.should_not be_truthy # aka should not add new contact + + d.domain_statuses.count.should == 1 + d.domain_statuses.first.value.should == 'pendingUpdate' + + d.dnskeys.count.should == 0 + end + it 'does not allow to edit statuses if policy forbids it' do Setting.client_status_editing_enabled = false diff --git a/spec/fabricators/registrant_verification_fabricator.rb b/spec/fabricators/registrant_verification_fabricator.rb new file mode 100644 index 000000000..067ecf288 --- /dev/null +++ b/spec/fabricators/registrant_verification_fabricator.rb @@ -0,0 +1,4 @@ +Fabricator(:registrant_verification) do + domain_name { sequence(:name) { |i| "domain#{i}.ee" } } + verification_token '123' +end diff --git a/spec/features/registrant/domain_update_confirm_spec.rb b/spec/features/registrant/domain_update_confirm_spec.rb new file mode 100644 index 000000000..5d5a298f7 --- /dev/null +++ b/spec/features/registrant/domain_update_confirm_spec.rb @@ -0,0 +1,44 @@ +require 'rails_helper' + +feature 'DomainUpdateConfirm', type: :feature do + context 'as unknown user with domain without update token' do + before :all do + @domain = Fabricate(:domain) + end + + it 'should see warning info if token is missing request' do + visit "/registrant/domain_update_confirms/#{@domain.id}" + current_path.should == "/registrant/domain_update_confirms/#{@domain.id}" + page.should have_text('Domain verification not available') + end + + it 'should see warning info if token is missing request' do + visit "/registrant/domain_update_confirms/#{@domain.id}" + current_path.should == "/registrant/domain_update_confirms/#{@domain.id}" + page.should have_text('Domain verification not available') + end + end + + context 'as unknown user with domain with update token' do + before :all do + @domain = Fabricate( + :domain, + registrant_verification_token: '123', + registrant_verification_asked_at: Time.zone.now + ) + @domain.domain_statuses.create(value: DomainStatus::PENDING_UPDATE) + end + + it 'should see warning info if token is missing in request' do + visit "/registrant/domain_update_confirms/#{@domain.id}?token=wrong_token" + current_path.should == "/registrant/domain_update_confirms/#{@domain.id}" + page.should have_text('Domain verification not available') + end + + it 'should show domain info and confirm buttons' do + visit "/registrant/domain_update_confirms/#{@domain.id}?token=123" + current_path.should == "/registrant/domain_update_confirms/#{@domain.id}" + page.should_not have_text('Domain verification not available') + end + end +end diff --git a/spec/features/registrant/root_spec.rb b/spec/features/registrant/root_spec.rb new file mode 100644 index 000000000..b2b77d651 --- /dev/null +++ b/spec/features/registrant/root_spec.rb @@ -0,0 +1,18 @@ +require 'rails_helper' + +feature 'Root', type: :feature do + it 'should redirect to registrant login page' do + visit '/registrant/login' + current_path.should == '/registrant/login' + end + + it 'should redirect to registrant login page' do + visit '/registrant' + current_path.should == '/registrant/login' + end + + it 'should redirect to registrant login page' do + visit '/registrant/' + current_path.should == '/registrant/login' + end +end diff --git a/spec/mailers/domain_mailer_spec.rb b/spec/mailers/domain_mailer_spec.rb index bdf0f1e2a..4f0fc35de 100644 --- a/spec/mailers/domain_mailer_spec.rb +++ b/spec/mailers/domain_mailer_spec.rb @@ -32,6 +32,7 @@ describe DomainMailer do @domain = Fabricate(:domain, registrant: @registrant) @domain.deliver_emails = true @domain.registrant_verification_token = '123' + @domain.registrant_verification_asked_at = Time.zone.now @domain.registrant = @new_registrant @mail = DomainMailer.registrant_updated(@domain) end diff --git a/spec/models/domain_spec.rb b/spec/models/domain_spec.rb index fadcb98e0..56d9ee57f 100644 --- a/spec/models/domain_spec.rb +++ b/spec/models/domain_spec.rb @@ -33,6 +33,10 @@ describe Domain do it 'should not have whois body' do @domain.whois_record.should == nil end + + it 'should not be registrant update confirm ready' do + @domain.registrant_update_confirmable?('123').should == false + end end context 'with valid attributes' do @@ -77,6 +81,31 @@ describe Domain do @domain.whois_record.json.present?.should == true end + it 'should not be registrant update confirm ready' do + @domain.registrant_update_confirmable?('123').should == false + end + + context 'about registrant update confirm' do + before :all do + @domain.registrant_verification_token = 123 + @domain.registrant_verification_asked_at = Time.zone.now + @domain.domain_statuses.create(value: DomainStatus::PENDING_UPDATE) + end + + it 'should be registrant update confirm ready' do + @domain.registrant_update_confirmable?('123').should == true + end + + it 'should not be registrant update confirm ready when token does not match' do + @domain.registrant_update_confirmable?('wrong-token').should == false + end + + it 'should not be registrant update confirm ready when no correct status' do + @domain.domain_statuses.delete_all + @domain.registrant_update_confirmable?('123').should == false + end + end + context 'with versioning' do it 'should not have one version' do with_versioning do diff --git a/spec/models/registrant_verification_spec.rb b/spec/models/registrant_verification_spec.rb new file mode 100644 index 000000000..7640956b0 --- /dev/null +++ b/spec/models/registrant_verification_spec.rb @@ -0,0 +1,34 @@ +require 'rails_helper' + +describe RegistrantVerification do + context 'with invalid attribute' do + before :all do + @registrant_verification = RegistrantVerification.new + end + + it 'should not be valid' do + @registrant_verification.valid? + @registrant_verification.errors.full_messages.should match_array([ + "Domain name is missing", + "Verification token is missing" + ]) + end + end + + context 'with valid attributes' do + before :all do + @registrant_verification = Fabricate(:registrant_verification) + end + + it 'should be valid' do + @registrant_verification.valid? + @registrant_verification.errors.full_messages.should match_array([]) + end + + it 'should be valid twice' do + @registrant_verification = Fabricate(:registrant_verification) + @registrant_verification.valid? + @registrant_verification.errors.full_messages.should match_array([]) + end + end +end