From c03818968f40d8bd22f3d3362d9f87961824c8e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andres=20Keskk=C3=BCla?= Date: Tue, 25 Nov 2014 16:29:40 +0200 Subject: [PATCH 1/4] Views and tests for contact disclosure --- app/helpers/epp/contacts_helper.rb | 16 ++- app/models/contact.rb | 2 +- .../epp/contacts/_postal_info.xml.builder | 18 ++- app/views/epp/contacts/info.xml.builder | 19 +-- config/locales/en.yml | 1 + ...ve_address_type_from_contact_disclosure.rb | 14 +++ db/schema.rb | 17 ++- spec/epp/contact_spec.rb | 111 ++++++++++++------ .../contact_disclosure_fabricator.rb | 10 +- 9 files changed, 130 insertions(+), 78 deletions(-) create mode 100644 db/migrate/20141124105221_remove_address_type_from_contact_disclosure.rb diff --git a/app/helpers/epp/contacts_helper.rb b/app/helpers/epp/contacts_helper.rb index 8945e6b65..044dc13a8 100644 --- a/app/helpers/epp/contacts_helper.rb +++ b/app/helpers/epp/contacts_helper.rb @@ -38,6 +38,8 @@ module Epp::ContactsHelper def info_contact handle_errors(@contact) and return unless @contact handle_errors(@contact) and return unless rights? + @disclosure = @contact.disclosure + @owner = owner?(false) render 'epp/contacts/info' end @@ -54,14 +56,10 @@ module Epp::ContactsHelper def validate_contact_create_request @ph = params_hash['epp']['command']['create']['create'] return false unless validate_params - # xml_attrs_present?(@ph, [%w(postalInfo)]) xml_attrs_present?(@ph, [%w(postalInfo name), %w(postalInfo addr city), %w(postalInfo addr cc), %w(ident), %w(voice), %w(email)]) - epp_errors.empty? # unless @ph['postalInfo'].is_a?(Hash) || @ph['postalInfo'].is_a?(Array) - - # (epp_errors << Address.validate_postal_info_types(parsed_frame)).flatten! - # xml_attrs_array_present?(@ph['postalInfo'], [%w(name), %w(addr city), %w(addr cc)]) + epp_errors.empty? end ## UPDATE @@ -118,10 +116,10 @@ module Epp::ContactsHelper contact end - def owner? + def owner?(with_errors = true) return false unless find_contact - # return true if current_epp_user.registrar == find_contact.created_by.try(:registrar) return true if @contact.registrar == current_epp_user.registrar + return false unless with_errors epp_errors << { code: '2201', msg: t('errors.messages.epp_authorization_error') } false end @@ -132,14 +130,14 @@ module Epp::ContactsHelper return true if current_epp_user.try(:registrar) == @contact.try(:registrar) return true if pw && @contact.auth_info_matches(pw) # @contact.try(:auth_info_matches, pw) - epp_errors << { code: '2201', msg: t('errors.messages.epp_authorization_error'), value: { obj: 'pw', val: pw } } + epp_errors << { code: '2200', msg: t('errors.messages.epp_authentication_error') } false end def update_rights? pw = @ph.try(:[], :authInfo).try(:[], :pw) return true if pw && @contact.auth_info_matches(pw) - epp_errors << { code: '2201', msg: t('errors.messages.epp_authorization_error'), value: { obj: 'pw', val: pw } } + epp_errors << { code: '2200', msg: t('errors.messages.epp_authentication_error') } false end diff --git a/app/models/contact.rb b/app/models/contact.rb index c6025156f..82fe0940e 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -5,7 +5,7 @@ class Contact < ActiveRecord::Base include EppErrors has_one :address, dependent: :destroy - has_one :disclosure, class_name: 'ContactDisclosure' + has_one :disclosure, class_name: 'ContactDisclosure', dependent: :destroy has_many :domain_contacts has_many :domains, through: :domain_contacts diff --git a/app/views/epp/contacts/_postal_info.xml.builder b/app/views/epp/contacts/_postal_info.xml.builder index 4310c2f37..2eb511db6 100644 --- a/app/views/epp/contacts/_postal_info.xml.builder +++ b/app/views/epp/contacts/_postal_info.xml.builder @@ -1,15 +1,13 @@ address = @contact.address xml.tag!('contact:postalInfo', type: 'int') do - xml.tag!('contact:name', @contact.name)# if @contact.disclosure.try(:int_name) - xml.tag!('contact:org', @contact.org_name)# if @contact.disclosure.try(:int_org_name) - #if @contact.disclosure.try(:int_addr) - xml.tag!('contact:addr') do - xml.tag!('contact:street', address.street) if address - #xml.tag!('contact:street', address.street2) if address.street2 - #xml.tag!('contact:street', address.street3) if address.street3 - xml.tag!('contact:cc', address.try(:country).try(:iso)) unless address.try(:country).nil? - xml.tag!('contact:city', address.city) if address + xml.tag!('contact:name', @contact.name) if @disclosure.try(:name) || @owner + xml.tag!('contact:org', @contact.org_name) if @disclosure.try(:org_name) || @owner + if @disclosure.try(:addr) || @owner + xml.tag!('contact:addr') do + xml.tag!('contact:street', address.street) if address + xml.tag!('contact:cc', address.try(:country).try(:iso)) unless address.try(:country).nil? + xml.tag!('contact:city', address.city) if address + end end - #end end diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder index 8e92a1cc6..202bfd583 100644 --- a/app/views/epp/contacts/info.xml.builder +++ b/app/views/epp/contacts/info.xml.builder @@ -6,22 +6,23 @@ xml.epp_head do xml.resData do xml.tag!('contact:chkData', 'xmlns:contact' => 'urn:ietf:params:xml:ns:contact-1.0') do - xml << render('/epp/contacts/postal_info') xml.tag!('contact:id', @contact.code) - xml.tag!('contact:voice', @contact.phone) #if @contact.disclosure.try(:phone) - xml.tag!('contact:fax', @contact.fax) #if @contact.disclosure.try(:fax) - xml.tag!('contact:email', @contact.email) #if @contact.disclosure..try(:email) - xml.tag!('contact:clID', @current_epp_user.username) if @current_epp_user - xml.tag!('contact:crID', @contact.cr_id ) if @contact.cr_id + xml << render('/epp/contacts/postal_info') + xml.tag!('contact:voice', @contact.phone) if @disclosure.try(:phone) || @owner + xml.tag!('contact:fax', @contact.fax) if @disclosure.try(:fax) || @owner + xml.tag!('contact:email', @contact.email) if @disclosure.try(:email) || @owner + #xml.tag!('contact:clID', @current_epp_user.username) if @current_epp_user + #xml.tag!('contact:crID', @contact.cr_id ) if @contact.cr_id xml.tag!('contact:crDate', @contact.created_at) xml.tag!('contact:upID', @contact.up_id) if @contact.up_id xml.tag!('contact:upDate', @contact.updated_at) unless @contact.updated_at == @contact.created_at xml.tag!('contact:trDate', '123') if false - xml.tag!('contact:authInfo') do - xml.tag!('contact:pw', @contact.auth_info) # Doc says we have to return this but is it necessary? + if @owner + xml.tag!('contact:authInfo') do + xml.tag!('contact:pw', @contact.auth_info) # Doc says we have to return this but is it necessary? + end end xml.tag!('contact:disclose', '123') if false - end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 4649972c0..28ba5d564 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -233,6 +233,7 @@ en: epp_obj_does_not_exist: 'Object does not exist' epp_command_failed: 'Command failed' epp_authorization_error: 'Authorization error' + epp_authentication_error: 'Authentication error' epp_id_taken: 'Contact id already exists' epp_domain_not_found: 'Domain not found' epp_exp_dates_do_not_match: 'Given and current expire dates do not match' diff --git a/db/migrate/20141124105221_remove_address_type_from_contact_disclosure.rb b/db/migrate/20141124105221_remove_address_type_from_contact_disclosure.rb new file mode 100644 index 000000000..a4e177fe5 --- /dev/null +++ b/db/migrate/20141124105221_remove_address_type_from_contact_disclosure.rb @@ -0,0 +1,14 @@ +class RemoveAddressTypeFromContactDisclosure < ActiveRecord::Migration + def change + remove_column :contact_disclosures, :int_name, :boolean + remove_column :contact_disclosures, :int_org_name, :boolean + remove_column :contact_disclosures, :int_addr, :boolean + remove_column :contact_disclosures, :loc_name, :boolean + remove_column :contact_disclosures, :loc_org_name, :boolean + remove_column :contact_disclosures, :loc_addr, :boolean + + add_column :contact_disclosures, :name, :boolean + add_column :contact_disclosures, :org_name, :boolean + add_column :contact_disclosures, :address, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 295d7b948..ea64b8ffc 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: 20141114130737) do +ActiveRecord::Schema.define(version: 20141124105221) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -41,17 +41,14 @@ ActiveRecord::Schema.define(version: 20141114130737) do create_table "contact_disclosures", force: true do |t| t.integer "contact_id" - t.boolean "int_name", default: false - t.boolean "int_org_name", default: false - t.boolean "int_addr", default: false - t.boolean "loc_name", default: false - t.boolean "loc_org_name", default: false - t.boolean "loc_addr", default: false - t.boolean "phone", default: false - t.boolean "fax", default: false - t.boolean "email", default: false + t.boolean "phone", default: false + t.boolean "fax", default: false + t.boolean "email", default: false t.datetime "created_at" t.datetime "updated_at" + t.boolean "name" + t.boolean "org_name" + t.boolean "address" end create_table "contact_versions", force: true do |t| diff --git a/spec/epp/contact_spec.rb b/spec/epp/contact_spec.rb index 45dc968ab..d35138526 100644 --- a/spec/epp/contact_spec.rb +++ b/spec/epp/contact_spec.rb @@ -25,7 +25,6 @@ describe 'EPP Contact', epp: true do end context 'create command' do - it 'fails if request xml is missing' do xml = EppXml::Contact.create response = epp_request(xml, :xml) @@ -118,8 +117,8 @@ describe 'EPP Contact', epp: true do response = epp_request(update_contact_xml({ id: { value: 'sh8013' } }), :xml, :elkdata) - expect(response[:msg]).to eq('Authorization error') - expect(response[:result_code]).to eq('2201') + expect(response[:msg]).to eq('Authentication error') + expect(response[:result_code]).to eq('2200') end it 'is succesful' do @@ -165,16 +164,21 @@ describe 'EPP Contact', epp: true do expect(response[:results][1][:msg]).to eq('Email is invalid') end - # it 'updates disclosure items', pending: true do - # pending 'Disclosure needs to be remodeled a bit' - # Fabricate(:contact, code: 'sh8013', auth_info: '2fooBAR', registrar: zone, created_by_id: EppUser.first.id, - # disclosure: Fabricate(:contact_disclosure, phone: true, email: true)) - # epp_request('contacts/update.xml') - # - # expect(Contact.last.disclosure.phone).to eq(false) - # expect(Contact.last.disclosure.email).to eq(false) - # expect(Contact.count).to eq(1) - # end + it 'updates disclosure items' do + Fabricate(:contact, code: 'sh8013', auth_info: '2fooBAR', registrar: zone, created_by_id: EppUser.first.id, + disclosure: Fabricate(:contact_disclosure, phone: true, email: true)) + xml = { + id: { value: 'sh8013' }, + authInfo: { pw: { value: '2fooBAR' } } + } + @response = epp_request(update_contact_xml(xml), :xml) + + expect(@response[:results][0][:result_code]).to eq('1000') + + expect(Contact.last.disclosure.phone).to eq(false) + expect(Contact.last.disclosure.email).to eq(false) + expect(Contact.count).to eq(1) + end end context 'delete command' do @@ -252,6 +256,58 @@ describe 'EPP Contact', epp: true do end context 'info command' do + it 'discloses items with wrong password when queried by owner' do + @contact = Fabricate(:contact, registrar: zone, code: 'info-4444', name: 'Johnny Awesome', auth_info: 'asde', + address: Fabricate(:address), disclosure: Fabricate(:contact_disclosure, name: false)) + + xml = EppXml::Contact.info({ id: { value: @contact.code } }) + response = epp_request(xml, :xml, :zone) + contact = response[:parsed].css('resData chkData') + + expect(response[:result_code]).to eq('1000') + expect(response[:msg]).to eq('Command completed successfully') + expect(contact.css('name').first.text).to eq('Johnny Awesome') + end + + it 'returns auth error for non-owner with wrong password' do + @contact = Fabricate(:contact, registrar: elkdata, code: 'info-4444', name: 'Johnny Awesome', auth_info: 'asde', + address: Fabricate(:address), disclosure: Fabricate(:contact_disclosure, name: false)) + + xml = EppXml::Contact.info({ id: { value: @contact.code }, authInfo: { pw: { value: 'asdesde' } } }) + response = epp_request(xml, :xml, :zone) + + expect(response[:result_code]).to eq('2200') + expect(response[:msg]).to eq('Authentication error') + end + + it 'doesn\'t disclose items to non-owner with right password' do + @contact = Fabricate(:contact, registrar: elkdata, code: 'info-4444', + name: 'Johnny Awesome', auth_info: 'password', + address: Fabricate(:address), disclosure: Fabricate(:contact_disclosure, name: false)) + + xml = EppXml::Contact.info({ id: { value: @contact.code }, authInfo: { pw: { value: 'password' } } }) + response = epp_request(xml, :xml, :zone) + contact = response[:parsed].css('resData chkData') + + expect(response[:result_code]).to eq('1000') + expect(response[:msg]).to eq('Command completed successfully') + expect(contact.css('name').first).to eq(nil) + end + + it 'discloses items to owner' do + @contact = Fabricate(:contact, registrar: zone, code: 'info-4444', name: 'Johnny Awesome', + auth_info: 'password', + address: Fabricate(:address), disclosure: Fabricate(:contact_disclosure, name: false)) + + xml = EppXml::Contact.info({ id: { value: @contact.code } }) + response = epp_request(xml, :xml, :zone) + contact = response[:parsed].css('resData chkData') + + expect(response[:result_code]).to eq('1000') + expect(response[:msg]).to eq('Command completed successfully') + expect(contact.css('name').first.text).to eq('Johnny Awesome') + end + it 'fails if request invalid' do response = epp_request(EppXml::Contact.info({ uid: { value: '123123' } }), :xml) @@ -281,11 +337,13 @@ describe 'EPP Contact', epp: true do end - it 'doesn\'t disclose private elements', pending: true do - pending 'Disclosure needs to have some of the details worked out' - Fabricate(:contact, code: 'info-4444', auth_info: '2fooBAR', - disclosure: Fabricate(:contact_disclosure, email: false, phone: false)) - response = epp_request(info_contact_xml(id: { value: 'info-4444' }), :xml) + it 'doesn\'t disclose private elements' do + Fabricate(:contact, code: 'info-4444', auth_info: '2fooBAR', registrar: elkdata, + disclosure: Fabricate(:contact_disclosure, name: true, email: false, phone: false)) + + xml = EppXml::Contact.info({ id: { value: 'info-4444' }, authInfo: { pw: { value: '2fooBAR' } } }) + + response = epp_request(xml, :xml, :zone) contact = response[:parsed].css('resData chkData') expect(response[:result_code]).to eq('1000') @@ -309,22 +367,9 @@ describe 'EPP Contact', epp: true do xml = EppXml::Contact.info(id: { value: @contact.code }, authInfo: { pw: { value: 'qwe321' } }) response = epp_request(xml, :xml, :elkdata) - expect(response[:result_code]).to eq('2201') - expect(response[:msg]).to eq('Authorization error') + expect(response[:result_code]).to eq('2200') + expect(response[:msg]).to eq('Authentication error') end - - it 'doest display unassociated object with correct password' do - @contact = Fabricate(:contact, code: 'info-4444', registrar: zone, name: 'Johnny Awesome') - - xml = EppXml::Contact.info(id: { value: @contact.code }, authInfo: { pw: { value: @contact.auth_info } }) - response = epp_request(xml, :xml, :elkdata) - contact = response[:parsed].css('resData chkData') - - expect(response[:result_code]).to eq('1000') - expect(response[:msg]).to eq('Command completed successfully') - expect(contact.css('name').first.text).to eq('Johnny Awesome') - end - end context 'renew command' do diff --git a/spec/fabricators/contact_disclosure_fabricator.rb b/spec/fabricators/contact_disclosure_fabricator.rb index 784bd5bf8..6daf05186 100644 --- a/spec/fabricators/contact_disclosure_fabricator.rb +++ b/spec/fabricators/contact_disclosure_fabricator.rb @@ -1,10 +1,8 @@ Fabricator(:contact_disclosure) do email true phone true - loc_addr true - loc_name true - loc_org_name true - int_name true - int_org_name true - int_addr true + fax true + address true + name true + org_name true end From 64b0481cb2889e32e3b77ce916b98d3cfdf89317 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andres=20Keskk=C3=BCla?= Date: Tue, 25 Nov 2014 16:34:13 +0200 Subject: [PATCH 2/4] Removed redundant gems --- Gemfile | 4 ---- Gemfile.lock | 5 ----- 2 files changed, 9 deletions(-) diff --git a/Gemfile b/Gemfile index c3724a18a..591b7d90d 100644 --- a/Gemfile +++ b/Gemfile @@ -70,10 +70,6 @@ gem 'therubyracer', platforms: :ruby # for settings gem 'rails-settings-cached', '0.4.1' -# for scp to whois server -gem 'net-ssh' -gem 'net-scp' - # delayed job gem 'delayed_job_active_record', '~> 4.0.2' # to process delayed jobs diff --git a/Gemfile.lock b/Gemfile.lock index 356435bbc..5cffb8f8e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -169,9 +169,6 @@ GEM mini_portile (0.6.0) minitest (5.4.3) multi_json (1.10.1) - net-scp (1.2.1) - net-ssh (>= 2.6.5) - net-ssh (2.9.1) nokogiri (1.6.2.1) mini_portile (= 0.6.0) nprogress-rails (0.1.3.1) @@ -384,8 +381,6 @@ DEPENDENCIES jbuilder (~> 2.0) jquery-rails kaminari (~> 0.16.1) - net-scp - net-ssh nokogiri (~> 1.6.2.1) nprogress-rails (~> 0.1.3.1) paper_trail (~> 3.0.5) From 4e5aecb26de8c6a5c385f283c5467b80202cb4ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andres=20Keskk=C3=BCla?= Date: Wed, 26 Nov 2014 15:37:22 +0200 Subject: [PATCH 3/4] Support for disclosure settings. Tests --- app/models/contact.rb | 5 +++ app/models/contact_disclosure.rb | 36 +++++++++++--------- config/initializers/initial_settings.rb | 6 ++++ spec/epp/contact_spec.rb | 45 +++++++++++++++++++++++++ spec/models/contact_disclosure_spec.rb | 17 +++++++--- spec/models/contact_spec.rb | 22 +++++++++++- spec/support/general.rb | 9 +++++ 7 files changed, 119 insertions(+), 21 deletions(-) create mode 100644 config/initializers/initial_settings.rb diff --git a/app/models/contact.rb b/app/models/contact.rb index 82fe0940e..e11621273 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -37,6 +37,7 @@ class Contact < ActiveRecord::Base after_destroy :domains_snapshot before_create :generate_code before_create :generate_auth_info + after_create :ensure_disclosure # scopes scope :current_registrars, ->(id) { where(registrar_id: id) } @@ -63,6 +64,10 @@ class Contact < ActiveRecord::Base errors.add(:ident, 'bad format') unless code.valid? end + def ensure_disclosure + create_disclosure!(ContactDisclosure.default_values) unless disclosure + end + def domains_snapshot (domains + domains_owned).uniq.each do |domain| next unless domain.is_a?(Domain) diff --git a/app/models/contact_disclosure.rb b/app/models/contact_disclosure.rb index 61bd4794a..744c260c1 100644 --- a/app/models/contact_disclosure.rb +++ b/app/models/contact_disclosure.rb @@ -4,6 +4,18 @@ class ContactDisclosure < ActiveRecord::Base # value is true or false depending on disclosure flag # rules are the contents of disclose element class << self + def default_values + @dc = { + name: Setting.disclosure_name, + org_name: Setting.disclosure_org_name, + phone: Setting.disclosure_phone, + fax: Setting.disclosure_fax, + email: Setting.disclosure_email, + address: Setting.disclosure_address + } + @dc + end + def extract_attributes(parsed_frame) disclosure_hash = {} rules = parsed_frame.css('disclose').first @@ -11,35 +23,27 @@ class ContactDisclosure < ActiveRecord::Base value = rules.attributes['flag'].value disclosure_hash = parse_disclose_xml(rules) - disclosure_hash.each do |k, _v| + disclosure_hash.each do |k, _v| # provides a correct flag to disclosure elements disclosure_hash[k] = value end - disclosure_hash + default_values.merge(disclosure_hash) end private + # Returns list of disclosure elements present. def parse_disclose_xml(rules) - { int_name: parse_element_attributes_for('name', rules.children, 'int'), - int_org_name: parse_element_attributes_for('org_name', rules.children, 'int'), - int_addr: parse_element_attributes_for('addr', rules.children, 'int'), - loc_name: parse_element_attributes_for('name', rules.children, 'loc'), - loc_org_name: parse_element_attributes_for('org_name', rules.children, 'loc'), - loc_addr: parse_element_attributes_for('addr', rules.children, 'loc'), + { name: parse_element_attributes_for('name', rules.children), + org_name: parse_element_attributes_for('org_name', rules.children), + address: parse_element_attributes_for('addr', rules.children), phone: rules.css('voice').present?, email: rules.css('email').present?, fax: rules.css('fax').present? }.delete_if { |_k, v| v.nil? || v == false } end - # el is the element we are looking for - # rules are the contents of disclose element - # value is loc/int depending on what type of el we are looking for - def parse_element_attributes_for(el, rules, value) - rules.css(el).each do |rule| - next unless rule.try(:attributes) || rule.attributes['type'] - return rule.attributes['type'].value.present? if rule.attributes['type'].value == value - end + def parse_element_attributes_for(el, rules) + return true if rules.css(el).present? nil end end diff --git a/config/initializers/initial_settings.rb b/config/initializers/initial_settings.rb new file mode 100644 index 000000000..7b409f9f6 --- /dev/null +++ b/config/initializers/initial_settings.rb @@ -0,0 +1,6 @@ +Setting.disclosure_name = true if Setting.disclosure_name.nil? +Setting.disclosure_org_name = true if Setting.disclosure_org_name.nil? +Setting.disclosure_email = true if Setting.disclosure_email.nil? +Setting.disclosure_phone = false if Setting.disclosure_phone.nil? +Setting.disclosure_fax = false if Setting.disclosure_fax.nil? +Setting.disclosure_address = false if Setting.disclosure_address.nil? diff --git a/spec/epp/contact_spec.rb b/spec/epp/contact_spec.rb index d35138526..66bcf3308 100644 --- a/spec/epp/contact_spec.rb +++ b/spec/epp/contact_spec.rb @@ -22,6 +22,7 @@ describe 'EPP Contact', epp: true do Fabricate(:epp_user, username: 'zone', registrar: zone) Fabricate(:epp_user, username: 'elkdata', registrar: elkdata) create_settings + create_disclosure_settings end context 'create command' do @@ -96,6 +97,50 @@ describe 'EPP Contact', epp: true do # 5 seconds for what-ever weird lag reasons might happen expect(cr_date.text.to_time).to be_within(5).of(Time.now) end + + it 'creates disclosure data' do + xml = { + disclose: { value: { + voice: { value: '' }, + addr: { value: '' }, + name: { value: '' }, + org_name: { value: '' }, + email: { value: '' }, + fax: { value: '' } + }, attrs: { flag: '1' } + } + } + + response = epp_request(create_contact_xml(xml), :xml) + expect(response[:result_code]).to eq('1000') + + expect(Contact.last.disclosure.name).to eq(true) + expect(Contact.last.disclosure.org_name).to eq(true) + expect(Contact.last.disclosure.phone).to eq(true) + expect(Contact.last.disclosure.fax).to eq(true) + expect(Contact.last.disclosure.email).to eq(true) + expect(Contact.last.disclosure.address).to eq(true) + end + + it 'creates disclosure data merging with defaults' do + xml = { + disclose: { value: { + voice: { value: '' }, + addr: { value: '' } + }, attrs: { flag: '1' } + } + } + + response = epp_request(create_contact_xml(xml), :xml) + expect(response[:result_code]).to eq('1000') + + expect(Contact.last.disclosure.name).to eq(true) + expect(Contact.last.disclosure.org_name).to eq(true) + expect(Contact.last.disclosure.phone).to eq(true) + expect(Contact.last.disclosure.fax).to eq(false) + expect(Contact.last.disclosure.email).to eq(true) + expect(Contact.last.disclosure.address).to eq(true) + end end context 'update command' do diff --git a/spec/models/contact_disclosure_spec.rb b/spec/models/contact_disclosure_spec.rb index f468f93e1..fa9ae0e31 100644 --- a/spec/models/contact_disclosure_spec.rb +++ b/spec/models/contact_disclosure_spec.rb @@ -18,10 +18,19 @@ describe '.extract_attributes' do # TODO: remodel create contact xml to support disclosure it 'should return disclosure has if disclosure' do - f = File.open('spec/epp/requests/contacts/create_with_two_addresses.xml') - parsed_frame = Nokogiri::XML(f).remove_namespaces! - f.close + xml = EppXml::Contact.create( + { + disclose: { value: { + voice: { value: '' }, + addr: { value: '' }, + name: { value: '' }, + org_name: { value: '' }, + email: { value: '' }, + fax: { value: '' } + }, attrs: { flag: '0' } + } }) + parsed_frame = Nokogiri::XML(xml).remove_namespaces! result = ContactDisclosure.extract_attributes(parsed_frame) - expect(result).to eq({ phone: '0', email: '0' }) + expect(result).to eq({ phone: '0', email: '0', fax: '0', address: '0', name: '0', org_name: '0' }) end end diff --git a/spec/models/contact_spec.rb b/spec/models/contact_spec.rb index 2d39fe391..6f69b9ba2 100644 --- a/spec/models/contact_spec.rb +++ b/spec/models/contact_spec.rb @@ -1,6 +1,7 @@ require 'rails_helper' describe Contact do + before { create_disclosure_settings } it { should have_one(:address) } context 'with invalid attribute' do @@ -32,11 +33,30 @@ describe Contact do end context 'with valid attributes' do - before(:each) { @contact = Fabricate(:contact) } + before(:each) { @contact = Fabricate(:contact, disclosure: nil) } it 'should return true' do expect(@contact.valid?).to be true end + + it 'should have default disclosure' do + expect(@contact.disclosure.name).to be true + expect(@contact.disclosure.org_name).to be true + expect(@contact.disclosure.email).to be true + expect(@contact.disclosure.phone).to be false + expect(@contact.disclosure.fax).to be false + expect(@contact.disclosure.address).to be false + end + + it 'should have custom disclosure' do + @contact = Fabricate(:contact, disclosure: Fabricate(:contact_disclosure)) + expect(@contact.disclosure.name).to be true + expect(@contact.disclosure.org_name).to be true + expect(@contact.disclosure.email).to be true + expect(@contact.disclosure.phone).to be true + expect(@contact.disclosure.fax).to be true + expect(@contact.disclosure.address).to be true + end end context 'with callbacks' do diff --git a/spec/support/general.rb b/spec/support/general.rb index 99de8a409..03b3bbd03 100644 --- a/spec/support/general.rb +++ b/spec/support/general.rb @@ -12,6 +12,15 @@ module General Setting.transfer_wait_time = 0 end + + def create_disclosure_settings + Setting.disclosure_name = true + Setting.disclosure_org_name = true + Setting.disclosure_email = true + Setting.disclosure_phone = false + Setting.disclosure_fax = false + Setting.disclosure_address = false + end end RSpec.configure do |c| From 4800e0c315ffa1117fa3d801bce1c8ae1c5a2cb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andres=20Keskk=C3=BCla?= Date: Mon, 1 Dec 2014 11:25:47 +0200 Subject: [PATCH 4/4] Updated disclosure --- app/helpers/epp/contacts_helper.rb | 5 +++-- app/models/contact.rb | 2 +- app/models/contact_disclosure.rb | 20 ++++++++++++++++++- .../contacts/_disclosure_policy.xml.builder | 9 +++++++++ .../epp/contacts/_postal_info.xml.builder | 4 ++-- app/views/epp/contacts/info.xml.builder | 2 +- ...7091027_remove_defaults_from_disclosure.rb | 7 +++++++ db/schema.rb | 8 ++++---- spec/epp/contact_spec.rb | 19 ++++++++++-------- spec/models/contact_spec.rb | 14 ++++++------- spec/support/epp_contact_xml_helper.rb | 8 +++++++- 11 files changed, 71 insertions(+), 27 deletions(-) create mode 100644 app/views/epp/contacts/_disclosure_policy.xml.builder create mode 100644 db/migrate/20141127091027_remove_defaults_from_disclosure.rb diff --git a/app/helpers/epp/contacts_helper.rb b/app/helpers/epp/contacts_helper.rb index 044dc13a8..8450441e0 100644 --- a/app/helpers/epp/contacts_helper.rb +++ b/app/helpers/epp/contacts_helper.rb @@ -38,7 +38,8 @@ module Epp::ContactsHelper def info_contact handle_errors(@contact) and return unless @contact handle_errors(@contact) and return unless rights? - @disclosure = @contact.disclosure + @disclosure = ContactDisclosure.default_values.merge(@contact.disclosure.as_hash) + @disclosure_policy = @contact.disclosure.attributes_with_flag @owner = owner?(false) render 'epp/contacts/info' end @@ -145,7 +146,7 @@ module Epp::ContactsHelper case type when :update # TODO: support for rem/add - contact_hash = merge_attribute_hash(@ph[:chg], type) + contact_hash = merge_attribute_hash(@ph[:chg], type).delete_if { |_k, v| v.empty? } else contact_hash = merge_attribute_hash(@ph, type) end diff --git a/app/models/contact.rb b/app/models/contact.rb index e11621273..42bb2e78b 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -65,7 +65,7 @@ class Contact < ActiveRecord::Base end def ensure_disclosure - create_disclosure!(ContactDisclosure.default_values) unless disclosure + create_disclosure! unless disclosure end def domains_snapshot diff --git a/app/models/contact_disclosure.rb b/app/models/contact_disclosure.rb index 744c260c1..0d7554bd1 100644 --- a/app/models/contact_disclosure.rb +++ b/app/models/contact_disclosure.rb @@ -1,6 +1,24 @@ class ContactDisclosure < ActiveRecord::Base belongs_to :contact + def attributes_with_flag + attrs, policy = { name: name, email: email, phone: phone, address: address, org_name: org_name, fax: fax }, {} + policy[0] = attrs.map { |k, v| k if v == false }.compact + policy[1] = attrs.map { |k, v| k if v }.compact + policy + end + + def as_hash + { + name: name, + org_name: org_name, + phone: phone, + fax: fax, + email: email, + address: address + } + end + # value is true or false depending on disclosure flag # rules are the contents of disclose element class << self @@ -26,7 +44,7 @@ class ContactDisclosure < ActiveRecord::Base disclosure_hash.each do |k, _v| # provides a correct flag to disclosure elements disclosure_hash[k] = value end - default_values.merge(disclosure_hash) + disclosure_hash end private diff --git a/app/views/epp/contacts/_disclosure_policy.xml.builder b/app/views/epp/contacts/_disclosure_policy.xml.builder new file mode 100644 index 000000000..f3ba88159 --- /dev/null +++ b/app/views/epp/contacts/_disclosure_policy.xml.builder @@ -0,0 +1,9 @@ +if @disclosure_policy + @disclosure_policy.each do |k,v| + xml.tag!('contact:disclose', 'flag' => k) do + v.each do |attr| + xml.tag!("contact:#{attr}") + end + end + end +end diff --git a/app/views/epp/contacts/_postal_info.xml.builder b/app/views/epp/contacts/_postal_info.xml.builder index 2eb511db6..bec291a93 100644 --- a/app/views/epp/contacts/_postal_info.xml.builder +++ b/app/views/epp/contacts/_postal_info.xml.builder @@ -1,7 +1,7 @@ address = @contact.address xml.tag!('contact:postalInfo', type: 'int') do - xml.tag!('contact:name', @contact.name) if @disclosure.try(:name) || @owner - xml.tag!('contact:org', @contact.org_name) if @disclosure.try(:org_name) || @owner + xml.tag!('contact:name', @contact.name) if @disclosure.try(:[], :name) || @owner + xml.tag!('contact:org', @contact.org_name) if @disclosure.try(:[], :org_name) || @owner if @disclosure.try(:addr) || @owner xml.tag!('contact:addr') do xml.tag!('contact:street', address.street) if address diff --git a/app/views/epp/contacts/info.xml.builder b/app/views/epp/contacts/info.xml.builder index 202bfd583..8af796009 100644 --- a/app/views/epp/contacts/info.xml.builder +++ b/app/views/epp/contacts/info.xml.builder @@ -22,7 +22,7 @@ xml.epp_head do xml.tag!('contact:pw', @contact.auth_info) # Doc says we have to return this but is it necessary? end end - xml.tag!('contact:disclose', '123') if false + xml << render('/epp/contacts/disclosure_policy') end end diff --git a/db/migrate/20141127091027_remove_defaults_from_disclosure.rb b/db/migrate/20141127091027_remove_defaults_from_disclosure.rb new file mode 100644 index 000000000..69243407e --- /dev/null +++ b/db/migrate/20141127091027_remove_defaults_from_disclosure.rb @@ -0,0 +1,7 @@ +class RemoveDefaultsFromDisclosure < ActiveRecord::Migration + def change + change_column :contact_disclosures, :phone, :boolean, :default => nil + change_column :contact_disclosures, :fax, :boolean, :default => nil + change_column :contact_disclosures, :email, :boolean, :default => nil + end +end diff --git a/db/schema.rb b/db/schema.rb index ea64b8ffc..cbc306167 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: 20141124105221) do +ActiveRecord::Schema.define(version: 20141127091027) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -41,9 +41,9 @@ ActiveRecord::Schema.define(version: 20141124105221) do create_table "contact_disclosures", force: true do |t| t.integer "contact_id" - t.boolean "phone", default: false - t.boolean "fax", default: false - t.boolean "email", default: false + t.boolean "phone" + t.boolean "fax" + t.boolean "email" t.datetime "created_at" t.datetime "updated_at" t.boolean "name" diff --git a/spec/epp/contact_spec.rb b/spec/epp/contact_spec.rb index 66bcf3308..ce73acd72 100644 --- a/spec/epp/contact_spec.rb +++ b/spec/epp/contact_spec.rb @@ -134,11 +134,11 @@ describe 'EPP Contact', epp: true do response = epp_request(create_contact_xml(xml), :xml) expect(response[:result_code]).to eq('1000') - expect(Contact.last.disclosure.name).to eq(true) - expect(Contact.last.disclosure.org_name).to eq(true) + expect(Contact.last.disclosure.name).to eq(nil) + expect(Contact.last.disclosure.org_name).to eq(nil) expect(Contact.last.disclosure.phone).to eq(true) - expect(Contact.last.disclosure.fax).to eq(false) - expect(Contact.last.disclosure.email).to eq(true) + expect(Contact.last.disclosure.fax).to eq(nil) + expect(Contact.last.disclosure.email).to eq(nil) expect(Contact.last.disclosure.address).to eq(true) end end @@ -212,6 +212,7 @@ describe 'EPP Contact', epp: true do it 'updates disclosure items' do Fabricate(:contact, code: 'sh8013', auth_info: '2fooBAR', registrar: zone, created_by_id: EppUser.first.id, disclosure: Fabricate(:contact_disclosure, phone: true, email: true)) + xml = { id: { value: 'sh8013' }, authInfo: { pw: { value: '2fooBAR' } } @@ -336,7 +337,7 @@ describe 'EPP Contact', epp: true do expect(response[:result_code]).to eq('1000') expect(response[:msg]).to eq('Command completed successfully') - expect(contact.css('name').first).to eq(nil) + expect(contact.css('chkData postalInfo name').first).to eq(nil) end it 'discloses items to owner' do @@ -393,9 +394,11 @@ describe 'EPP Contact', epp: true do expect(response[:result_code]).to eq('1000') - expect(contact.css('phone').present?).to eq(false) - expect(contact.css('email').present?).to eq(false) - expect(contact.css('name').present?).to be(true) + expect(contact.css('chkData phone')).to eq(contact.css('chkData disclose phone')) + expect(contact.css('chkData phone').count).to eq(1) + expect(contact.css('chkData email')).to eq(contact.css('chkData disclose email')) + expect(contact.css('chkData email').count).to eq(1) + expect(contact.css('postalInfo name').present?).to be(true) end it 'doesn\'t display unassociated object without password' do diff --git a/spec/models/contact_spec.rb b/spec/models/contact_spec.rb index 6f69b9ba2..70b11b2b9 100644 --- a/spec/models/contact_spec.rb +++ b/spec/models/contact_spec.rb @@ -39,13 +39,13 @@ describe Contact do expect(@contact.valid?).to be true end - it 'should have default disclosure' do - expect(@contact.disclosure.name).to be true - expect(@contact.disclosure.org_name).to be true - expect(@contact.disclosure.email).to be true - expect(@contact.disclosure.phone).to be false - expect(@contact.disclosure.fax).to be false - expect(@contact.disclosure.address).to be false + it 'should have empty disclosure' do + expect(@contact.disclosure.name).to be nil + expect(@contact.disclosure.org_name).to be nil + expect(@contact.disclosure.email).to be nil + expect(@contact.disclosure.phone).to be nil + expect(@contact.disclosure.fax).to be nil + expect(@contact.disclosure.address).to be nil end it 'should have custom disclosure' do diff --git a/spec/support/epp_contact_xml_helper.rb b/spec/support/epp_contact_xml_helper.rb index 63f407843..47aaa1548 100644 --- a/spec/support/epp_contact_xml_helper.rb +++ b/spec/support/epp_contact_xml_helper.rb @@ -27,7 +27,13 @@ module EppContactXmlHelper name: { value: 'John Doe Edited' } }, voice: { value: '+372.7654321' }, - email: { value: 'edited@example.example' } + email: { value: 'edited@example.example' }, + disclose: { + value: { + voice: { value: '' }, + email: { value: '' } + }, attrs: { flag: '0' } + } } } xml_params = defaults.deep_merge(xml_params)