diff --git a/app/controllers/epp/contacts_controller.rb b/app/controllers/epp/contacts_controller.rb index 87919da9c..2ee87d24c 100644 --- a/app/controllers/epp/contacts_controller.rb +++ b/app/controllers/epp/contacts_controller.rb @@ -123,12 +123,6 @@ class Epp::ContactsController < EppController def validate_update @prefix = 'update > update >' - if element_count('chg') == 0 && element_count('rem') == 0 && element_count('add') == 0 - epp_errors << { - code: '2003', - msg: I18n.t('errors.messages.required_parameter_missing', key: 'add, rem or chg') - } - end contact_org_disabled fax_disabled status_editing_disabled @@ -148,6 +142,7 @@ class Epp::ContactsController < EppController def contact_org_disabled return true if ENV['contact_org_enabled'] == 'true' return true if params[:parsed_frame].css('postalInfo org').text.blank? + epp_errors << { code: '2306', msg: "#{I18n.t(:contact_org_error)}: postalInfo > org [org]" diff --git a/app/models/domain.rb b/app/models/domain.rb index 2a03277de..55c246987 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -618,7 +618,6 @@ class Domain < ActiveRecord::Base statuses.include?(DomainStatus::FORCE_DELETE) end - # TODO: Review the list and disallow epp calls def pending_update_prohibited? (statuses & [ DomainStatus::CLIENT_UPDATE_PROHIBITED, @@ -643,7 +642,6 @@ class Domain < ActiveRecord::Base statuses.include?(DomainStatus::PENDING_DELETE) && !statuses.include?(DomainStatus::FORCE_DELETE) end - # TODO: Review the list and disallow epp calls def pending_delete_prohibited? (statuses & [ DomainStatus::CLIENT_DELETE_PROHIBITED, diff --git a/app/models/epp/contact.rb b/app/models/epp/contact.rb index fa5be307e..3136beabd 100644 --- a/app/models/epp/contact.rb +++ b/app/models/epp/contact.rb @@ -20,35 +20,21 @@ class Epp::Contact < Contact # rubocop: disable Metrics/PerceivedComplexity # rubocop: disable Metrics/CyclomaticComplexity - # rubocop: disable Metrics/MethodLength # rubocop: disable Metrics/AbcSize - def attrs_from(frame, rem = nil) + def attrs_from(frame) f = frame at = {}.with_indifferent_access - if rem - at[:name] = nil if f.css('postalInfo name').present? - at[:org_name] = nil if f.css('postalInfo org').present? - at[:email] = nil if f.css('email').present? - at[:fax] = nil if f.css('fax').present? - at[:phone] = nil if f.css('voice').present? - at[:city] = nil if f.css('postalInfo addr city').present? - at[:zip] = nil if f.css('postalInfo addr pc').present? - at[:street] = nil if f.css('postalInfo addr street').present? - at[:state] = nil if f.css('postalInfo addr sp').present? - at[:country_code] = nil if f.css('postalInfo addr cc').present? - else - at[:name] = f.css('postalInfo name').text if f.css('postalInfo name').present? - at[:org_name] = f.css('postalInfo org').text if f.css('postalInfo org').present? - at[:email] = f.css('email').text if f.css('email').present? - at[:fax] = f.css('fax').text if f.css('fax').present? - at[:phone] = f.css('voice').text if f.css('voice').present? - at[:city] = f.css('postalInfo addr city').text if f.css('postalInfo addr city').present? - at[:zip] = f.css('postalInfo addr pc').text if f.css('postalInfo addr pc').present? - at[:street] = f.css('postalInfo addr street').text if f.css('postalInfo addr street').present? - at[:state] = f.css('postalInfo addr sp').text if f.css('postalInfo addr sp').present? - at[:country_code] = f.css('postalInfo addr cc').text if f.css('postalInfo addr cc').present? - at[:auth_info] = f.css('authInfo pw').text if f.css('authInfo pw').present? - end + at[:name] = f.css('postalInfo name').text if f.css('postalInfo name').present? + at[:org_name] = f.css('postalInfo org').text if f.css('postalInfo org').present? + at[:email] = f.css('email').text if f.css('email').present? + at[:fax] = f.css('fax').text if f.css('fax').present? + at[:phone] = f.css('voice').text if f.css('voice').present? + at[:city] = f.css('postalInfo addr city').text if f.css('postalInfo addr city').present? + at[:zip] = f.css('postalInfo addr pc').text if f.css('postalInfo addr pc').present? + at[:street] = f.css('postalInfo addr street').text if f.css('postalInfo addr street').present? + at[:state] = f.css('postalInfo addr sp').text if f.css('postalInfo addr sp').present? + at[:country_code] = f.css('postalInfo addr cc').text if f.css('postalInfo addr cc').present? + at[:auth_info] = f.css('authInfo pw').text if f.css('authInfo pw').present? legal_frame = f.css('legalDocument').first if legal_frame.present? @@ -57,7 +43,6 @@ class Epp::Contact < Contact at.merge!(ident_attrs(f.css('ident').first)) at end - # rubocop: enable Metrics/MethodLength # rubocop: enable Metrics/PerceivedComplexity # rubocop: enable Metrics/CyclomaticComplexity # rubocop: enable Metrics/AbcSize @@ -150,12 +135,49 @@ class Epp::Contact < Contact def update_attributes(frame) return super if frame.blank? at = {}.with_indifferent_access - at.deep_merge!(self.class.attrs_from(frame.css('rem'), 'rem')) - at.deep_merge!(self.class.attrs_from(frame.css('add'))) at.deep_merge!(self.class.attrs_from(frame.css('chg'))) + + if Setting.client_status_editing_enabled + at[:statuses] = statuses - statuses_attrs(frame.css('rem'), 'rem') + statuses_attrs(frame.css('add'), 'add') + end + legal_frame = frame.css('legalDocument').first at[:legal_documents_attributes] = self.class.legal_document_attrs(legal_frame) self.deliver_emails = true # turn on email delivery for epp super(at) end + + def statuses_attrs(frame, action) + status_list = status_list_from(frame) + + if action == 'rem' + to_destroy = [] + status_list.each do |status| + if statuses.include?(status) + to_destroy << status + else + add_epp_error('2303', 'status', status, [:contact_statuses, :not_found]) + end + end + + return to_destroy + else + return status_list + end + end + + def status_list_from(frame) + status_list = [] + + frame.css('status').each do |status| + unless Contact::CLIENT_STATUSES.include?(status['s']) + add_epp_error('2303', 'status', status['s'], [:domain_statuses, :not_found]) + next + end + + status_list << status['s'] + end + + status_list + end end diff --git a/spec/epp/contact_spec.rb b/spec/epp/contact_spec.rb index 35ad4558f..36926e66c 100644 --- a/spec/epp/contact_spec.rb +++ b/spec/epp/contact_spec.rb @@ -567,29 +567,6 @@ describe 'EPP Contact', epp: true do Contact.find_by(code: 'FIRST0:SH8013').phone.should == phone # aka not changed end - it 'should honor chg value over add value when both changes same attribute' do - pending 'It should not be possible to add voice (in add)' - xml = @epp_xml.update({ - id: { value: 'FIRST0:SH8013' }, - add: { - voice: { value: '+372.11111111111' } - }, - chg: { - voice: { value: '+372.222222222222' }, - authInfo: { pw: { value: 'password' } } - } - }) - - response = epp_plain_request(xml) - response[:results][0][:msg].should == 'Command completed successfully' - response[:results][0][:result_code].should == '1000' - - contact = Contact.find_by(code: 'FIRST0:SH8013') - contact.phone.should == '+372.222222222222' - - contact.update_attribute(:phone, '+372.7654321') # restore default value - end - it 'should not allow to remove required voice attribute' do contact = Contact.find_by(code: 'FIRST0:SH8013') phone = contact.phone @@ -609,128 +586,12 @@ describe 'EPP Contact', epp: true do contact.phone.should == phone end - # TODO: Update request rem block must be analyzed - it 'should not allow to remove required attribute' do - pending 'It should not be possible to remove or add voice (in add and rem)' - contact = Contact.find_by(code: 'FIRST0:SH8013') - phone = contact.phone - # TODO: Refactor authInfo under chg block - xml = @epp_xml.update({ - id: { value: 'FIRST0:SH8013' }, - authInfo: { pw: { value: 'password' } }, - rem: { - voice: { value: '+372.7654321' } - } - }) - - response = epp_plain_request(xml) - response[:results][0][:msg].should == 'Required parameter missing - phone [phone]' - response[:results][0][:result_code].should == '2003' - - contact = Contact.find_by(code: 'FIRST0:SH8013') - contact.phone.should == phone - end - - it 'should honor add over rem' do - pending 'It should not be possible to remove or add voice (in add and rem)' - # TODO: Refactor authInfo under chg block - xml = @epp_xml.update({ - id: { value: 'FIRST0:SH8013' }, - authInfo: { pw: { value: 'password' } }, - rem: { - voice: { value: 'not important' } - }, - add: { - voice: { value: '+372.3333333' } - } - }) - - response = epp_plain_request(xml) - response[:results][0][:msg].should == 'Command completed successfully' - response[:results][0][:result_code].should == '1000' - - contact = Contact.find_by(code: 'FIRST0:SH8013') - contact.phone.should == '+372.3333333' - - contact.update_attribute(:phone, '+372.7654321') # restore default value - end - - it 'should honor chg over rem' do - pending 'It should not be possible to remove or add voice (in add and rem)' - # TODO: Refactor authInfo under chg block - xml = @epp_xml.update({ - id: { value: 'FIRST0:SH8013' }, - authInfo: { pw: { value: 'password' } }, - rem: { - voice: { value: 'not important' } - }, - chg: { - voice: { value: '+372.44444444' } - } - }) - - response = epp_plain_request(xml) - response[:results][0][:msg].should == 'Command completed successfully' - response[:results][0][:result_code].should == '1000' - - contact = Contact.find_by(code: 'FIRST0:SH8013') - contact.phone.should == '+372.44444444' - - contact.update_attribute(:phone, '+372.7654321') # restore default value - end - - it 'should honor chg over rem and add' do - pending 'It should not be possible to remove or add voice (in add and rem)' - # TODO: Refactor authInfo under chg block - xml = @epp_xml.update({ - id: { value: 'FIRST0:SH8013' }, - authInfo: { pw: { value: 'password' } }, - chg: { - voice: { value: '+372.666666' } - }, - add: { - voice: { value: '+372.555555' } - }, - rem: { - voice: { value: 'not important' } - } - }) - - response = epp_plain_request(xml) - response[:results][0][:msg].should == 'Command completed successfully' - response[:results][0][:result_code].should == '1000' - - contact = Contact.find_by(code: 'FIRST0:SH8013') - contact.phone.should == '+372.666666' - - contact.update_attribute(:phone, '+372.7654321') # restore default value - end - - it 'should not remove password' do - pending 'There should be no possibility to remove pw' - xml = @epp_xml.update({ - id: { value: 'FIRST0:SH8013' }, - authInfo: { pw: { value: 'password' } }, - rem: { - authInfo: { pw: { value: 'password' } } - } - }) - - response = epp_plain_request(xml) - response[:results][0][:msg].should == 'Command completed successfully' - response[:results][0][:result_code].should == '1000' - - contact = Contact.find_by(code: 'FIRST0:SH8013') - contact.auth_info.should == 'password' - end - - it 'should return general policy error when removing org' do - pending 'Test says it should throw error when removing org, it does not do it when removing it with chg block' + it 'should return general policy error when updating org' do xml = @epp_xml.update({ id: { value: 'FIRST0:SH8013' }, chg: { postalInfo: { - org: { value: '' } + org: { value: 'shouldnot' } }, authInfo: { pw: { value: 'password' } } } @@ -742,22 +603,24 @@ describe 'EPP Contact', epp: true do response[:results][0][:result_code].should == '2306' end - it 'should return error when removing street' do - pending 'Test says it tests removing street, but actually street is not removed' - # TODO: Refactor authInfo under chg block + it 'does not allow to edit statuses if policy forbids it' do + Setting.client_status_editing_enabled = false + xml = @epp_xml.update({ id: { value: 'FIRST0:SH8013' }, - authInfo: { pw: { value: 'password' } }, - rem: { - postalInfo: { - name: { value: 'not important' } - } - } + add: [{ + _anonymus: [ + { status: { value: '', attrs: { s: 'clientUpdateProhibited' } } } + ] + }] }) response = epp_plain_request(xml) - response[:results][0][:msg].should == "Required parameter missing - name [name]" - response[:results][0][:result_code].should == '2003' + response[:results][0][:msg].should == "Parameter value policy error. Client-side object status "\ + "management not supported: status [status]" + response[:results][0][:result_code].should == '2306' + + Setting.client_status_editing_enabled = true end end diff --git a/spec/models/domain_spec.rb b/spec/models/domain_spec.rb index a5a48ddb6..6aad6246d 100644 --- a/spec/models/domain_spec.rb +++ b/spec/models/domain_spec.rb @@ -399,6 +399,7 @@ describe Domain do it 'should set pending update' do @domain.statuses = DomainStatus::OK # restore + @domain.save @domain.pending_update?.should == false @domain.set_pending_update @@ -409,6 +410,7 @@ describe Domain do it 'should not set pending update' do @domain.statuses = DomainStatus::OK # restore @domain.statuses << DomainStatus::CLIENT_UPDATE_PROHIBITED + @domain.save @domain.set_pending_update.should == nil # not updated @domain.pending_update?.should == false @@ -417,9 +419,12 @@ describe Domain do it 'should set pending delete' do @domain.statuses = DomainStatus::OK # restore + @domain.save @domain.pending_delete?.should == false - @domain.set_pending_delete.should == ['pendingDelete'] + @domain.set_pending_delete + @domain.save + @domain.statuses.should == ['pendingDelete'] @domain.pending_delete?.should == true @domain.statuses = DomainStatus::OK # restore end