From b6ce705748c179e638effaf65cbc552660d37bee Mon Sep 17 00:00:00 2001 From: Martin Lensment Date: Thu, 4 Sep 2014 17:23:05 +0300 Subject: [PATCH] Let only owning registrar approve transfer, tests refactor --- app/models/concerns/epp_errors.rb | 4 +- app/models/domain.rb | 8 +++- config/locales/en.yml | 2 + spec/epp/domain_spec.rb | 79 +++++++++++++++++++------------ 4 files changed, 61 insertions(+), 32 deletions(-) diff --git a/app/models/concerns/epp_errors.rb b/app/models/concerns/epp_errors.rb index e6debf67f..c83899769 100644 --- a/app/models/concerns/epp_errors.rb +++ b/app/models/concerns/epp_errors.rb @@ -74,6 +74,8 @@ module EppErrors errors[:epp_errors] ||= [] t = errors.generate_message(*msg) if msg.is_a?(Array) t = msg if msg.is_a?(String) - errors[:epp_errors] << { code: code, msg: t, value: { val: val, obj: obj } } + err = { code: code, msg: t } + err[:value] = { val: val, obj: obj } if val.present? + errors[:epp_errors] << err end end diff --git a/app/models/domain.rb b/app/models/domain.rb index f837ea14d..34759f67b 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -211,8 +211,14 @@ class Domain < ActiveRecord::Base def transfer(params) return false unless authenticate(params[:pw]) + pt = pending_transfer + + if pt && params[:action] == 'approve' + if params[:current_user].registrar != pt.transfer_from + add_epp_error('2304', nil, nil, I18n.t('transfer_can_be_approved_only_by_current_registrar')) + return false + end - if pending_transfer && params[:action] == 'approve' approve_pending_transfer and return true end diff --git a/config/locales/en.yml b/config/locales/en.yml index 23b836474..65b32234d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -132,3 +132,5 @@ en: domains: 'Domains' epp_users: 'EPP Users' registrars: 'Registrars' + + transfer_can_be_approved_only_by_current_registrar: 'Transfer can be approved only by current domain registrar' diff --git a/spec/epp/domain_spec.rb b/spec/epp/domain_spec.rb index c8242c10e..c693d52da 100644 --- a/spec/epp/domain_spec.rb +++ b/spec/epp/domain_spec.rb @@ -37,9 +37,11 @@ describe 'EPP Domain', epp: true do end context 'with two epp users' do + let(:domain) { Domain.first } + before(:each) do Fabricate(:domain_general_setting_group) - Fabricate(:domain, name: 'example.ee', registrar: zone) # belongs to zone + Fabricate(:domain, name: 'example.ee', registrar: zone) end it 'can not see other registrar domains' do @@ -49,12 +51,13 @@ describe 'EPP Domain', epp: true do end it 'transfers a domain' do - pw = Domain.first.auth_info + pw = domain.auth_info xml = domain_transfer_xml(pw: pw) response = epp_request(xml, :xml, :elkdata) - d = Domain.first - dtl = d.domain_transfers.last + domain.reload + dtl = domain.domain_transfers.last + trn_data = response[:parsed].css('trnData') expect(trn_data.css('name').text).to eq('example.ee') expect(trn_data.css('trStatus').text).to eq('serverApproved') @@ -62,69 +65,85 @@ describe 'EPP Domain', epp: true do expect(trn_data.css('reDate').text).to eq(dtl.transfer_requested_at.to_time.utc.to_s) expect(trn_data.css('acID').text).to eq('10577829') expect(trn_data.css('acDate').text).to eq(dtl.transferred_at.to_time.utc.to_s) - expect(trn_data.css('exDate').text).to eq(d.valid_to.to_time.utc.to_s) + expect(trn_data.css('exDate').text).to eq(domain.valid_to.to_time.utc.to_s) - expect(d.registrar).to eq(elkdata) + expect(domain.registrar).to eq(elkdata) s = Setting.find_by(code: 'transfer_wait_time') s.update(value: 1) - pw = Domain.first.auth_info + domain.reload + pw = domain.auth_info xml = domain_transfer_xml(pw: pw) # request with new password response = epp_request(xml, :xml, :zone) trn_data = response[:parsed].css('trnData') - d = Domain.first - dtl = d.domain_transfers.last + domain.reload + dtl = domain.domain_transfers.last - expect(d.domain_transfers.count).to eq(2) + expect(domain.domain_transfers.count).to eq(2) expect(trn_data.css('name').text).to eq('example.ee') expect(trn_data.css('trStatus').text).to eq('pending') expect(trn_data.css('reID').text).to eq('10577829') expect(trn_data.css('reDate').text).to eq(dtl.transfer_requested_at.to_time.utc.to_s) expect(trn_data.css('acID').text).to eq('123') - expect(trn_data.css('exDate').text).to eq(d.valid_to.to_time.utc.to_s) + expect(trn_data.css('exDate').text).to eq(domain.valid_to.to_time.utc.to_s) - expect(d.registrar).to eq(elkdata) + expect(domain.registrar).to eq(elkdata) # should return same data if pending already response = epp_request(xml, :xml, :zone) trn_data = response[:parsed].css('trnData') - expect(d.domain_transfers.count).to eq(2) + expect(domain.domain_transfers.count).to eq(2) expect(trn_data.css('name').text).to eq('example.ee') expect(trn_data.css('trStatus').text).to eq('pending') expect(trn_data.css('reID').text).to eq('10577829') expect(trn_data.css('reDate').text).to eq(dtl.transfer_requested_at.to_time.utc.to_s) expect(trn_data.css('acID').text).to eq('123') - expect(trn_data.css('exDate').text).to eq(d.valid_to.to_time.utc.to_s) + expect(trn_data.css('exDate').text).to eq(domain.valid_to.to_time.utc.to_s) - expect(d.registrar).to eq(elkdata) + expect(domain.registrar).to eq(elkdata) end - it 'approves the transfer request', pending: true do - fail 'Check if domain belongs to approving user' - s = Setting.find_by(code: 'transfer_wait_time') - s.update(value: 1) + it 'prohibits wrong registrar from approving tranfer' do + domain.domain_transfers.create({ + status: DomainTransfer::PENDING, + transfer_requested_at: Time.zone.now, + transfer_to: elkdata, + transfer_from: zone + }) - pw = Domain.first.auth_info - xml = domain_transfer_xml(pw: pw) - - epp_request(xml, :xml) - xml = domain_transfer_xml(op: 'approve', pw: pw) + xml = domain_transfer_xml(pw: domain.auth_info, op: 'approve') response = epp_request(xml, :xml, :elkdata) + expect(response[:result_code]).to eq('2304') + expect(response[:msg]).to eq('Transfer can be approved only by current domain registrar') + end + + it 'approves the transfer request' do + domain.domain_transfers.create({ + status: DomainTransfer::PENDING, + transfer_requested_at: Time.zone.now, + transfer_to: elkdata, + transfer_from: zone + }) + + xml = domain_transfer_xml(pw: domain.auth_info, op: 'approve') + response = epp_request(xml, :xml, :zone) + + domain.reload + dtl = domain.domain_transfers.last + trn_data = response[:parsed].css('trnData') - d = Domain.first - dtl = d.domain_transfers.last expect(trn_data.css('name').text).to eq('example.ee') expect(trn_data.css('trStatus').text).to eq('clientApproved') - expect(trn_data.css('reID').text).to eq('10577829') # TODO See if this is correct + expect(trn_data.css('reID').text).to eq('123') expect(trn_data.css('reDate').text).to eq(dtl.transfer_requested_at.to_time.utc.to_s) - expect(trn_data.css('acID').text).to eq('10577829') # TODO See if this is correct - expect(trn_data.css('exDate').text).to eq(d.valid_to.to_time.utc.to_s) + expect(trn_data.css('acID').text).to eq('10577829') + expect(trn_data.css('exDate').text).to eq(domain.valid_to.to_time.utc.to_s) end it 'does not transfer with invalid pw' do @@ -134,7 +153,7 @@ describe 'EPP Domain', epp: true do end it 'creates new pw after successful transfer' do - pw = Domain.first.auth_info + pw = domain.auth_info xml = domain_transfer_xml(pw: pw) epp_request(xml, :xml) # transfer domain response = epp_request(xml, :xml) # attempt second transfer