From feb8d83a26130a535337361ec0e3272e3bb0bcd4 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Fri, 18 Aug 2017 13:24:56 +0300 Subject: [PATCH] Improve Ident validation, extract Ident from Contact #569 --- app/controllers/admin/contacts_controller.rb | 5 + .../registrar/contacts_controller.rb | 5 + app/models/concerns/epp_errors.rb | 31 +++ app/models/contact.rb | 57 +---- app/models/contact/ident.rb | 72 ++++++ app/models/epp/contact.rb | 55 ++-- .../contact/ident/code_validator.rb | 29 +++ app/views/admin/contacts/index.haml | 4 +- app/views/registrar/contacts/index.haml | 4 +- config/locales/contacts.en.yml | 9 - config/locales/idents.yml | 11 + spec/features/admin/contacts/list_spec.rb | 12 + spec/features/registrar/contacts/list_spec.rb | 15 ++ spec/models/contact/ident_spec.rb | 217 ++++++++++++++++ spec/models/contact_spec.rb | 105 +++----- .../requests/epp/contact/create/ident_spec.rb | 237 ++++++++++++++++++ .../requests/epp/contact/update/ident_spec.rb | 136 ++++++++-- 17 files changed, 809 insertions(+), 195 deletions(-) create mode 100644 app/models/contact/ident.rb create mode 100644 app/validators/contact/ident/code_validator.rb create mode 100644 config/locales/idents.yml create mode 100644 spec/features/admin/contacts/list_spec.rb create mode 100644 spec/features/registrar/contacts/list_spec.rb create mode 100644 spec/models/contact/ident_spec.rb create mode 100644 spec/requests/epp/contact/create/ident_spec.rb diff --git a/app/controllers/admin/contacts_controller.rb b/app/controllers/admin/contacts_controller.rb index ec8154e93..104a85a39 100644 --- a/app/controllers/admin/contacts_controller.rb +++ b/app/controllers/admin/contacts_controller.rb @@ -1,6 +1,7 @@ class Admin::ContactsController < AdminController load_and_authorize_resource before_action :set_contact, only: [:show] + helper_method :ident_types def index params[:q] ||= {} @@ -78,4 +79,8 @@ class Admin::ContactsController < AdminController params[:q][:created_at_lteq] = ca_cache end + + def ident_types + Contact::Ident.types + end end diff --git a/app/controllers/registrar/contacts_controller.rb b/app/controllers/registrar/contacts_controller.rb index b927df1a8..4d0de5389 100644 --- a/app/controllers/registrar/contacts_controller.rb +++ b/app/controllers/registrar/contacts_controller.rb @@ -2,6 +2,7 @@ class Registrar class ContactsController < DeppController before_action :init_epp_contact helper_method :address_processing? + helper_method :ident_types def index authorize! :view, Depp::Contact @@ -140,5 +141,9 @@ class Registrar def address_processing? Contact.address_processing? end + + def ident_types + Contact::Ident.types + end end end diff --git a/app/models/concerns/epp_errors.rb b/app/models/concerns/epp_errors.rb index cf3824260..fd064dc09 100644 --- a/app/models/concerns/epp_errors.rb +++ b/app/models/concerns/epp_errors.rb @@ -11,6 +11,12 @@ module EppErrors epp_errors << collect_child_errors(attr) end + if self.class.reflect_on_aggregation(attr) + aggregation = send(attr) + epp_errors << collect_aggregation_errors(aggregation) + next + end + epp_errors << collect_parent_errors(attr, errors) end @@ -46,6 +52,31 @@ module EppErrors epp_errors end + def collect_aggregation_errors(aggregation) + epp_errors = [] + + aggregation.errors.details.each do |attr, error_details| + error_details.each do |error_detail| + aggregation.class.epp_code_map.each do |epp_code, attr_to_error| + epp_code_found = attr_to_error.any? { |i| i == [attr, error_detail[:error]] } + + if epp_code_found + message = aggregation.errors.generate_message(attr, error_detail[:error], error_detail) + message = aggregation.errors.full_message(attr, message) + + if attr != :base + message = "#{aggregation.model_name.human} #{message.camelize(:lower)}" + end + + epp_errors << { code: epp_code, msg: message } + end + end + end + end + + epp_errors + end + def find_epp_code_and_value(msg) epp_code_map.each do |code, values| values.each do |x| diff --git a/app/models/contact.rb b/app/models/contact.rb index 24756d6da..76af74d35 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -19,27 +19,22 @@ class Contact < ActiveRecord::Base accepts_nested_attributes_for :legal_documents - validates :name, :email, :ident, :ident_type, presence: true + validates :name, :email, presence: true validates :street, :city, :zip, :country_code, presence: true, if: 'self.class.address_processing?' validates :phone, presence: true, e164: true, phone: true validates :email, format: /@/ validates :email, email_format: { message: :invalid }, if: proc { |c| c.email_changed? } - validates :ident, - format: { with: /\d{4}-\d{2}-\d{2}/, message: :invalid_birthday_format }, - if: proc { |c| c.ident_type == 'birthday' } - validates :ident_country_code, presence: true, if: proc { |c| %w(org priv).include? c.ident_type }, on: :create + validates :code, uniqueness: { message: :epp_id_taken }, format: { with: /\A[\w\-\:\.\_]*\z/i, message: :invalid }, length: { maximum: 100, message: :too_long_contact_code } + validates_associated :identifier - validate :val_ident_type - validate :val_ident_valid_format? validate :validate_html validate :validate_country_code - validate :validate_ident_country_code after_initialize do self.status_notes = {} if status_notes.nil? @@ -49,8 +44,15 @@ class Contact < ActiveRecord::Base before_validation :to_upcase_country_code before_validation :strip_email before_create :generate_auth_info - before_update :manage_emails + + composed_of :identifier, + class_name: 'Contact::Ident', + constructor: Proc.new { |code, type, country_code| Contact::Ident.new(code: code, + type: type, + country_code: country_code) }, + mapping: [%w[ident code], %w[ident_type type], %w[ident_country_code country_code]] + def manage_emails return nil unless email_changed? return nil unless deliver_emails == true @@ -76,12 +78,6 @@ class Contact < ActiveRecord::Base BIRTHDAY = 'birthday'.freeze PASSPORT = 'passport' - IDENT_TYPES = [ - ORG, # Company registry code (or similar) - PRIV, # National idendtification number - BIRTHDAY # Birthday date - ] - attr_accessor :deliver_emails attr_accessor :domain_transfer # hack but solves problem faster @@ -299,28 +295,6 @@ class Contact < ActiveRecord::Base name || '[no name]' end - def val_ident_type - errors.add(:ident_type, :epp_ident_type_invalid, code: code) if !%w(org priv birthday).include?(ident_type) - end - - def val_ident_valid_format? - case ident_country_code - when 'EE'.freeze - err_msg = "invalid_EE_identity_format#{"_update" if id}".to_sym - case ident_type - when 'priv'.freeze - errors.add(:ident, err_msg) unless Isikukood.new(ident).valid? - when 'org'.freeze - # !%w(1 7 8 9).freeze.include?(ident.first) || - if ident.size != 8 || !(ident =~/\A[0-9]{8}\z/) - errors.add(:ident, err_msg) - end - when BIRTHDAY - errors.add(:ident, err_msg) if id.blank? # only for create action right now. Later for all of them - end - end - end - def validate_html self.class.columns.each do |column| next unless column.type == :string @@ -334,7 +308,6 @@ class Contact < ActiveRecord::Base end end - def org? ident_type == ORG end @@ -344,10 +317,6 @@ class Contact < ActiveRecord::Base !org? end - def birthday? - ident_type == BIRTHDAY - end - def generate_auth_info return if @generate_auth_info_disabled return if auth_info.present? @@ -424,10 +393,6 @@ class Contact < ActiveRecord::Base errors.add(:country_code, :invalid) unless Country.new(country_code) end - def validate_ident_country_code - errors.add(:ident, :invalid_country_code) unless Country.new(ident_country_code) - end - def related_domain_descriptions ActiveSupport::Deprecation.warn('Use #domain_names_with_roles') diff --git a/app/models/contact/ident.rb b/app/models/contact/ident.rb new file mode 100644 index 000000000..5ca44c28d --- /dev/null +++ b/app/models/contact/ident.rb @@ -0,0 +1,72 @@ +class Contact::Ident + include ActiveModel::Model + + attr_accessor :code + attr_accessor :type + attr_accessor :country_code + + validates :code, presence: true, code: true + validates :code, iso8601: { date_only: true }, if: :birthday? + validates :type, presence: true, inclusion: { in: Proc.new { types } } + validates :country_code, presence: true, iso31661_alpha2: true + validate :mismatched + + def self.epp_code_map + { + '2003' => [ + [:code, :blank], + [:type, :blank], + [:country_code, :blank], + ], + '2005' => [ + [:base, :mismatch], + [:code, :invalid_national_id], + [:code, :invalid_reg_no], + [:code, :invalid_iso8601], + [:country_code, :invalid_iso31661_alpha2], + ], + } + end + + def self.types + %w[org priv birthday] + end + + Mismatch = Struct.new(:type, :country) + + def self.mismatches + [ + Mismatch.new('birthday', Country.new('EE')), + ] + end + + def marked_for_destruction? + false + end + + def birthday? + type == 'birthday' + end + + def national_id? + type == 'priv' + end + + def reg_no? + type == 'org' + end + + def country + Country.new(country_code) + end + + private + + # https://github.com/rails/rails/issues/1513 + def validation_context=(value); end + + def mismatched + mismatched = self.class.mismatches.include?(Mismatch.new(type, country)) + errors.add(:base, :mismatch, type: type, country: country) if mismatched + end +end diff --git a/app/models/epp/contact.rb b/app/models/epp/contact.rb index 207f4a81a..1e14fafd3 100644 --- a/app/models/epp/contact.rb +++ b/app/models/epp/contact.rb @@ -109,7 +109,7 @@ class Epp::Contact < Contact end delegate :ident_attr_valid?, to: :class - def epp_code_map # rubocop:disable Metrics/MethodLength + def epp_code_map { '2003' => [ # Required parameter missing [:name, :blank], @@ -124,27 +124,16 @@ class Epp::Contact < Contact [:name, :invalid], [:phone, :invalid], [:email, :invalid], - [:ident, :invalid], - [:ident, :invalid_EE_identity_format], - [:ident, :invalid_EE_identity_format_update], - [:ident, :invalid_birthday_format], - [:ident, :invalid_country_code], [:country_code, :invalid], - [:ident_type, :missing], [:code, :invalid], [:code, :too_long_contact_code] ], '2302' => [ # Object exists [:code, :epp_id_taken] ], - '2304' => [ # Object status prohibits operation - [:ident_type, :epp_ident_type_invalid, { value: { obj: 'code', val: code}, interpolation: {code: code}}] - ], '2305' => [ # Association exists [:domains, :exist] ], - '2306' => [ # Parameter policy error - ] } end @@ -164,36 +153,26 @@ class Epp::Contact < Contact self.deliver_emails = true # turn on email delivery for epp - # Allow ident data update for legacy contacts if frame.css('ident').first - self.ident_updated_at ||= Time.zone.now ident_frame = frame.css('ident').first - if ident_frame && ident_attr_valid?(ident_frame) - org_priv = %w(org priv).freeze - - if ident_frame.text == ident - if ident_type.present? && ident_country_code.present? - at.merge!(ident_type: ident_frame.attr('type')) - at.merge!(ident_country_code: ident_frame.attr('cc')) - elsif ident_country_code.blank? && org_priv.include?(ident_type) && org_priv.include?(ident_frame.attr('type')) - at.merge!(ident_country_code: ident_frame.attr('cc'), ident_type: ident_frame.attr('type')) - elsif ident_type == 'birthday' && !ident[/\A\d{4}-\d{2}-\d{2}\z/] && (Date.parse(ident) rescue false) - at.merge!(ident: ident_frame.text) - at.merge!(ident_country_code: ident_frame.attr('cc')) - elsif ident_type == 'birthday' && ident_country_code.blank? - at.merge!(ident_country_code: ident_frame.attr('cc')) - elsif ident_type.blank? && ident_country_code.blank? - at.merge!(ident_type: ident_frame.attr('type')) - at.merge!(ident_country_code: ident_frame.attr('cc')) - else - deny_ident_update - end - else - deny_ident_update - end - else + # https://github.com/internetee/registry/issues/576 + if identifier.valid? deny_ident_update + else + if ident_frame && ident_attr_valid?(ident_frame) + deny_ident_update if ident_frame.text != ident + + identifier = Ident.new(code: ident, + type: ident_frame.attr('type'), + country_code: ident_frame.attr('cc'), + ) + + identifier.validate + + self.identifier = identifier + self.ident_updated_at ||= Time.zone.now + end end end diff --git a/app/validators/contact/ident/code_validator.rb b/app/validators/contact/ident/code_validator.rb new file mode 100644 index 000000000..25f76f1b1 --- /dev/null +++ b/app/validators/contact/ident/code_validator.rb @@ -0,0 +1,29 @@ +class Contact::Ident::CodeValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + if record.country_code == 'EE' + if record.national_id? + record.errors.add(attribute, + :invalid_national_id, + country: record.country) unless valid_national_id_ee?(value) + end + + if record.reg_no? + validator = ActiveModel::Validations::FormatValidator.new(with: reg_no_ee_format, + attributes: attribute, + message: :invalid_reg_no, + country: record.country) + validator.validate(record) + end + end + end + + private + + def reg_no_ee_format + /\A[0-9]{8}\z/ + end + + def valid_national_id_ee?(ident) + Isikukood.new(ident).valid? + end +end diff --git a/app/views/admin/contacts/index.haml b/app/views/admin/contacts/index.haml index 07cf640ce..30e5529eb 100644 --- a/app/views/admin/contacts/index.haml +++ b/app/views/admin/contacts/index.haml @@ -19,7 +19,7 @@ .col-md-3 .form-group = label_tag t(:ident_type) - = select_tag '[q][ident_type_eq]', options_for_select(Contact::IDENT_TYPES, params[:q][:ident_type_eq]), { include_blank: true, placeholder: t(:choose), class: 'form-control selectize' } + = select_tag '[q][ident_type_eq]', options_for_select(ident_types, params[:q][:ident_type_eq]), { include_blank: true, placeholder: t(:choose), class: 'form-control selectize' } .row .col-md-3 .form-group @@ -75,7 +75,7 @@ .row .col-md-12 .table-responsive - %table.table.table-hover.table-bordered.table-condensed + %table.table.table-hover.table-bordered.table-condensed.contacts %thead %tr %th{class: 'col-xs-2'} diff --git a/app/views/registrar/contacts/index.haml b/app/views/registrar/contacts/index.haml index 3e0e59826..87f16a3a7 100644 --- a/app/views/registrar/contacts/index.haml +++ b/app/views/registrar/contacts/index.haml @@ -21,7 +21,7 @@ .col-md-3 .form-group = label_tag t(:ident_type) - = select_tag '[q][ident_type_eq]', options_for_select(Contact::IDENT_TYPES, params[:q][:ident_type_eq]), { include_blank: true, placeholder: t(:choose), class: 'form-control selectize' } + = select_tag '[q][ident_type_eq]', options_for_select(ident_types, params[:q][:ident_type_eq]), { include_blank: true, placeholder: t(:choose), class: 'form-control selectize' } .row .col-md-3 .form-group @@ -85,7 +85,7 @@ .row .col-md-12 .table-responsive - %table.table.table-hover.table-bordered.table-condensed + %table.table.table-hover.table-bordered.table-condensed.contacts %thead %tr %th{class: 'col-xs-2'} diff --git a/config/locales/contacts.en.yml b/config/locales/contacts.en.yml index c3e183721..0c3c1fe29 100644 --- a/config/locales/contacts.en.yml +++ b/config/locales/contacts.en.yml @@ -20,15 +20,6 @@ en: email: blank: "Required parameter missing - email" invalid: "Email is invalid" - ident: - blank: "Required parameter missing - ident" - invalid_EE_identity_format: "Ident not in valid Estonian identity format." - invalid_EE_identity_format_update: "Ident not in valid Estonian identity format. Please create new contact" - invalid_birthday_format: "Ident not in valid birthday format, should be YYYY-MM-DD" - invalid_country_code: "Ident country code is not valid, should be in ISO_3166-1 alpha 2 format" - ident_type: - ident_type_invalid: 'Ident type is invalid' - epp_ident_type_invalid: 'Object status prohibits operation: ident_type of contact %{code} is invalid' domains: exist: 'Object association prohibits operation' statuses: diff --git a/config/locales/idents.yml b/config/locales/idents.yml new file mode 100644 index 000000000..33b935833 --- /dev/null +++ b/config/locales/idents.yml @@ -0,0 +1,11 @@ +en: + activemodel: + errors: + models: + contact/ident: + attributes: + base: + mismatch: Ident type "%{type}" is invalid for %{country} + code: + invalid_national_id: does not conform to national identification number format of %{country} + invalid_reg_no: does not conform to registration number format of %{country} diff --git a/spec/features/admin/contacts/list_spec.rb b/spec/features/admin/contacts/list_spec.rb new file mode 100644 index 000000000..80312fb17 --- /dev/null +++ b/spec/features/admin/contacts/list_spec.rb @@ -0,0 +1,12 @@ +require 'rails_helper' + +RSpec.feature 'Contact list', settings: false do + background do + sign_in_to_admin_area + end + + it 'is visible' do + visit admin_contacts_path + expect(page).to have_css('.contacts') + end +end diff --git a/spec/features/registrar/contacts/list_spec.rb b/spec/features/registrar/contacts/list_spec.rb new file mode 100644 index 000000000..96f1f2c98 --- /dev/null +++ b/spec/features/registrar/contacts/list_spec.rb @@ -0,0 +1,15 @@ +require 'rails_helper' + +RSpec.feature 'Contact list', settings: false do + given!(:registrar) { create(:registrar) } + given!(:contact) { create(:contact, registrar: registrar) } + + background do + sign_in_to_registrar_area(user: create(:api_user_with_unlimited_balance, registrar: registrar)) + end + + it 'is visible' do + visit registrar_contacts_path + expect(page).to have_css('.contacts') + end +end diff --git a/spec/models/contact/ident_spec.rb b/spec/models/contact/ident_spec.rb new file mode 100644 index 000000000..f18dcc1ca --- /dev/null +++ b/spec/models/contact/ident_spec.rb @@ -0,0 +1,217 @@ +require 'active_model' +require 'lib/validators/iso31661_alpha2' +require 'lib/validators/iso8601' + +RSpec.describe Contact::Ident, db: false do + let(:ident) { described_class.new } + + describe 'country code' do + it_behaves_like 'iso31661_alpha2' do + let(:model) { ident } + let(:attribute) { :country_code } + end + end + + describe 'code validation' do + it 'rejects absent' do + ident.code = nil + ident.validate + expect(ident.errors).to be_added(:code, :blank) + end + + context 'when type is :birthday' do + let(:ident) { described_class.new(type: 'birthday') } + + it_behaves_like 'iso8601' do + let(:model) { ident } + let(:attribute) { :code } + end + end + + context 'when type is not :birthday' do + let(:ident) { described_class.new(type: 'priv') } + + it 'accepts any' do + ident.code = '%123456789%' + ident.validate + expect(ident.errors).to_not include(:code) + end + end + + context 'when country code is EE' do + context 'when type is :priv' do + let(:ident) { described_class.new(country_code: 'EE', type: 'priv') } + + it 'rejects invalid' do + ident.code = 'invalid' + ident.validate + expect(ident.errors).to be_added(:code, :invalid_national_id, country: 'Estonia') + end + + it 'accepts valid' do + ident.code = '47101010033' + ident.validate + expect(ident.errors).to_not be_added(:code, :invalid_national_id, country: 'Estonia') + end + end + + context 'when ident type is :org' do + let(:ident) { described_class.new(country_code: 'EE', type: 'org') } + + it 'rejects invalid' do + ident.code = '1' * 7 + ident.validate + expect(ident.errors).to be_added(:code, :invalid_reg_no, country: 'Estonia') + end + + it 'accepts valid length' do + ident.code = '1' * 8 + ident.validate + expect(ident.errors).to_not be_added(:code, :invalid_reg_no, country: 'Estonia') + end + end + end + + context 'when ident country code is not EE' do + let(:ident) { described_class.new(country_code: 'US') } + + it 'accepts any' do + ident.code = 'test-123456789' + ident.validate + expect(ident.errors).to_not include(:code) + end + end + + it 'translates :invalid_national_id error message' do + expect(ident.errors.generate_message(:code, :invalid_national_id, country: 'Germany')) + .to eq('does not conform to national identification number format of Germany') + end + + it 'translates :invalid_reg_no error message' do + expect(ident.errors.generate_message(:code, :invalid_reg_no, country: 'Germany')) + .to eq('does not conform to registration number format of Germany') + end + end + + describe 'type validation' do + before do + allow(described_class).to receive(:types).and_return(%w(valid)) + end + + it 'rejects absent' do + ident.type = nil + ident.validate + expect(ident.errors).to be_added(:type, :blank) + end + + it 'rejects invalid' do + ident.type = 'invalid' + ident.validate + expect(ident.errors).to be_added(:type, :inclusion) + end + + it 'accepts valid' do + ident.type = 'valid' + ident.validate + expect(ident.errors).to_not be_added(:type, :inclusion) + end + end + + describe 'country code validation' do + it 'rejects absent' do + ident.country_code = nil + ident.validate + expect(ident.errors).to be_added(:country_code, :blank) + end + end + + describe 'mismatch validation' do + let(:ident) { described_class.new(type: 'test', country_code: 'DE') } + + before do + mismatches = [Contact::Ident::Mismatch.new('test', Country.new('DE'))] + allow(described_class).to receive(:mismatches).and_return(mismatches) + end + + it 'rejects mismatched' do + ident.validate + expect(ident.errors).to be_added(:base, :mismatch, type: 'test', country: 'Germany') + end + + it 'accepts matched' do + ident.validate + expect(ident.errors).to_not be_added(:base, :mismatch, type: 'another-test', country: 'Germany') + end + + it 'translates :mismatch error message' do + expect(ident.errors.generate_message(:base, :mismatch, type: 'test', country: 'Germany')) + .to eq('Ident type "test" is invalid for Germany') + end + end + + describe '::types' do + it 'returns types' do + types = %w[ + org + priv + birthday + ] + + expect(described_class.types).to eq(types) + end + end + + describe '::mismatches' do + it 'returns mismatches' do + mismatches = [ + Contact::Ident::Mismatch.new('birthday', Country.new('EE')), + ] + + expect(described_class.mismatches).to eq(mismatches) + end + end + + describe '#birthday?' do + context 'when type is birthday' do + subject(:ident) { described_class.new(type: 'birthday') } + it { is_expected.to be_birthday } + end + + context 'when type is not birthday' do + subject(:ident) { described_class.new(type: 'priv') } + it { is_expected.to_not be_birthday } + end + end + + describe '#national_id?' do + context 'when type is priv' do + subject(:ident) { described_class.new(type: 'priv') } + it { is_expected.to be_national_id } + end + + context 'when type is not' do + subject(:ident) { described_class.new(type: 'org') } + it { is_expected.to_not be_national_id } + end + end + + describe '#reg_no?' do + context 'when type is birthday' do + subject(:ident) { described_class.new(type: 'org') } + it { is_expected.to be_reg_no } + end + + context 'when type is not birthday' do + subject(:ident) { described_class.new(type: 'priv') } + it { is_expected.to_not be_reg_no } + end + end + + describe '#country' do + let(:ident) { described_class.new(country_code: 'US') } + + it 'returns country' do + expect(ident.country).to eq(Country.new('US')) + end + end +end diff --git a/spec/models/contact_spec.rb b/spec/models/contact_spec.rb index c981cab51..bef5b87be 100644 --- a/spec/models/contact_spec.rb +++ b/spec/models/contact_spec.rb @@ -28,45 +28,6 @@ RSpec.describe Contact do @contact.updator.should == nil end - it 'should require country code when org' do - @contact.ident_type = 'org' - @contact.valid? - @contact.errors[:ident_country_code].should == ['is missing'] - end - - it 'should require country code when priv' do - @contact.ident_type = 'priv' - @contact.valid? - @contact.errors[:ident_country_code].should == ['is missing'] - end - - it 'should validate correct country code' do - @contact.ident = 1 - @contact.ident_type = 'org' - @contact.ident_country_code = 'EE' - @contact.valid? - - @contact.errors[:ident_country_code].should == [] - end - - it 'should require valid country code' do - @contact.ident = '123' - @contact.ident_type = 'org' - @contact.ident_country_code = 'INVALID' - @contact.valid? - - expect(@contact.errors).to have_key(:ident) - end - - it 'should convert to alpha2 country code' do - @contact.ident = 1 - @contact.ident_type = 'org' - @contact.ident_country_code = 'ee' - @contact.validate - - @contact.ident_country_code.should == 'EE' - end - it 'should not have any versions' do @contact.versions.should == [] end @@ -119,14 +80,6 @@ RSpec.describe Contact do @contact.domains_present?.should == false end - it 'org should be valid' do - contact = Fabricate.build(:contact, ident_type: 'org', ident: '1' * 8) - - contact.validate - - contact.errors.full_messages.should match_array([]) - end - it 'should not overwrite code' do old_code = @contact.code @contact.code = 'CID:REG1:should-not-overwrite-old-code-12345' @@ -217,31 +170,6 @@ RSpec.describe Contact do end end - context 'as birthday' do - before :example do - @contact.ident_type = 'birthday' - end - - it 'birthday should be valid' do - valid = ['2012-12-11', '1990-02-16'] - valid.each do |date| - @contact.ident = date - @contact.valid? - @contact.errors.full_messages.should match_array([]) - end - end - - it 'birthday should be invalid' do - invalid = ['123' '12/12/2012', 'aaaa', '12/12/12', '02-11-1999'] - invalid.each do |date| - @contact.ident = date - @contact.valid? - @contact.errors.full_messages.should == - ["Ident Ident not in valid birthday format, should be YYYY-MM-DD"] - end - end - end - context 'with callbacks' do before :example do # Ensure callbacks are not taken out from other specs @@ -445,7 +373,7 @@ RSpec.describe Contact do end end - describe 'country code validation' do + describe 'country code validation', db: false do let(:contact) { described_class.new(country_code: 'test') } it 'rejects invalid' do @@ -455,6 +383,28 @@ RSpec.describe Contact do end end + describe 'identifier validation', db: false do + let(:contact) { described_class.new } + + it 'rejects invalid' do + ident = Contact::Ident.new + ident.validate + contact.identifier = ident + contact.validate + + expect(contact.errors).to be_added(:identifier, :invalid) + end + + it 'accepts valid' do + ident = Contact::Ident.new(code: 'test', type: 'priv', country_code: 'US') + ident.validate + contact.identifier = ident + contact.validate + + expect(contact.errors).to_not be_added(:identifier, :invalid) + end + end + describe '#remove_address' do let(:contact) { described_class.new(city: 'test', street: 'test', @@ -561,4 +511,13 @@ RSpec.describe Contact do expect(domain_names).to eq({ 'test.com' => %i[admin_domain_contact].to_set }) end end + + it 'normalizes ident country code', db: false do + contact = described_class.new + + contact.ident_country_code = 'ee' + contact.validate + + expect(contact.ident_country_code).to eq('EE') + end end diff --git a/spec/requests/epp/contact/create/ident_spec.rb b/spec/requests/epp/contact/create/ident_spec.rb new file mode 100644 index 000000000..f800a9102 --- /dev/null +++ b/spec/requests/epp/contact/create/ident_spec.rb @@ -0,0 +1,237 @@ +require 'rails_helper' + +RSpec.describe 'EPP contact:create' do + let(:request) { post '/epp/command/create', frame: request_xml } + + before do + Setting.address_processing = false + sign_in_to_epp_area + end + + context 'when all ident params are valid' do + let(:ident) { Contact.first.identifier } + let(:request_xml) { <<-XML + + + + + + + test + + +1.2 + test@test.com + + + + + test + + + + + XML + } + + it 'creates a contact' do + expect { request }.to change { Contact.count }.from(0).to(1) + end + + it 'saves ident type' do + request + expect(ident.type).to eq('priv') + end + + it 'saves ident country code' do + request + expect(ident.country_code).to eq('US') + end + + specify do + request + expect(epp_response).to have_result(:success) + end + end + + context 'when code is blank' do + let(:request_xml) { <<-XML + + + + + + + test + + +1.2 + test@test.com + + + + + + + + + + XML + } + + it 'does not create a contact' do + expect { request }.to_not change { Contact.count } + end + + specify do + request + expect(epp_response).to have_result(:required_param_missing, + 'Required parameter missing: extension > extdata > ident [ident]') + end + end + + context 'when code is invalid' do + let(:request_xml) { <<-XML + + + + + + + test + + +1.2 + test@test.com + + + + + invalid + + + + + XML + } + + it 'does not create a contact' do + expect { request }.to_not change { Contact.count } + end + + specify do + request + + message = 'Ident code does not conform to national identification number format of Estonia' + expect(epp_response).to have_result(:param_syntax_error, message) + end + end + + context 'when country code is absent' do + let(:request_xml) { <<-XML + + + + + + + test + + +1.2 + test@test.com + + + + + test + + + + + XML + } + + it 'does not create a contact' do + expect { request }.to_not change { Contact.count } + end + + specify do + request + expect(epp_response).to have_result(:required_param_missing, + 'Required ident attribute missing: cc') + end + end + + context 'when country code is blank' do + let(:request_xml) { <<-XML + + + + + + + test + + +1.2 + test@test.com + + + + + test + + + + + XML + } + + it 'does not create a contact' do + expect { request }.to_not change { Contact.count } + end + + specify do + request + expect(epp_response).to have_result(:syntax_error) + end + end + + context 'when mismatches' do + let(:request_xml) { <<-XML + + + + + + + test + + +1.2 + test@test.com + + + + + test + + + + + XML + } + + before do + mismatches = [ + Contact::Ident::Mismatch.new('priv', Country.new('DE')), + ] + allow(Contact::Ident).to receive(:mismatches).and_return(mismatches) + end + + it 'does not create a contact' do + expect { request }.to_not change { Contact.count } + end + + specify do + request + expect(epp_response).to have_result(:param_syntax_error, + 'Ident type "priv" is invalid for Germany') + end + end +end diff --git a/spec/requests/epp/contact/update/ident_spec.rb b/spec/requests/epp/contact/update/ident_spec.rb index 0051b2503..5ecd0ef25 100644 --- a/spec/requests/epp/contact/update/ident_spec.rb +++ b/spec/requests/epp/contact/update/ident_spec.rb @@ -1,6 +1,9 @@ require 'rails_helper' +# https://github.com/internetee/registry/issues/576 + RSpec.describe 'EPP contact:update' do + let(:ident) { contact.identifier } let(:request) { post '/epp/command/update', frame: request_xml } let(:request_xml) { <<-XML @@ -18,7 +21,7 @@ RSpec.describe 'EPP contact:update' do - test + test @@ -30,45 +33,128 @@ RSpec.describe 'EPP contact:update' do sign_in_to_epp_area end - context 'when submitted ident matches current one' do - let!(:contact) { create(:contact, code: 'TEST', ident: 'test', ident_type: 'org', ident_country_code: 'US') } + context 'when contact ident is valid' do + let!(:contact) { create(:contact, code: 'TEST', ident: 'test', ident_type: 'priv', ident_country_code: 'US') } - it 'updates :ident_type' do - request - contact.reload - expect(contact.ident_type).to eq('priv') + it 'does not update code' do + expect { request; contact.reload }.to_not change { ident.code } end - it 'updates :ident_country_code' do - request - contact.reload - expect(contact.ident_country_code).to eq('GB') + it 'does not update type' do + expect { request; contact.reload }.to_not change { ident.type } + end + + it 'does not update country code' do + expect { request; contact.reload }.to_not change { ident.country_code } end specify do request - expect(response).to have_code_of(1000) + + message = 'Data management policy violation:' \ + ' update of ident not allowed, please consider creating new contact object' + expect(epp_response).to have_result(:data_management_policy_violation, message) end end - context 'when submitted ident does not match current one' do - let!(:contact) { create(:contact, code: 'TEST', ident: 'some-ident', ident_type: 'org', ident_country_code: 'US') } + context 'when contact ident is invalid' do + let(:contact) { build(:contact, code: 'TEST', ident: 'test', ident_type: nil, ident_country_code: nil) } - it 'does not update :ident_type' do - request - contact.reload - expect(contact.ident_type).to eq('org') + before do + contact.save(validate: false) end - it 'does not update :ident_country_code' do - request - contact.reload - expect(contact.ident_country_code).to eq('US') + context 'when submitted ident is the same as current one' do + let(:request_xml) { <<-XML + + + + + + TEST + + + test + + + + + + + test + + + + + XML + } + + it 'does not update code' do + expect { request; contact.reload }.to_not change { ident.code } + end + + it 'updates type' do + request + contact.reload + expect(ident.type).to eq('priv') + end + + it 'updates country code' do + request + contact.reload + expect(ident.country_code).to eq('US') + end + + specify do + request + expect(epp_response).to have_result(:success) + end end - specify do - request - expect(response).to have_code_of(2308) + context 'when submitted ident is different from current one' do + let(:request_xml) { <<-XML + + + + + + TEST + + + test + + + + + + + another-test + + + + + XML + } + + it 'does not update code' do + expect { request; contact.reload }.to_not change { ident.code } + end + + it 'does not update type' do + expect { request; contact.reload }.to_not change { ident.type } + end + + it 'does not update country code' do + expect { request; contact.reload }.to_not change { ident.country_code } + end + + specify do + request + + message = 'Data management policy violation:' \ + ' update of ident not allowed, please consider creating new contact object' + expect(epp_response).to have_result(:data_management_policy_violation, message) + end end end end