Let only owning registrar approve transfer, tests refactor

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

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
 

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'

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