Merge pull request #411 from internetee/registry-386

Validate contact phone for zeros

app/models/contact.rb

@@ -22,8 +22,8 @@ class Contact < ActiveRecord::Base
   validates :name, :phone, :email, :ident, :ident_type, :registrar, presence: true
   validates :street, :city, :zip, :country_code, presence: true, if: 'self.class.address_processing?'
 
-  # Phone nr validation is very minimam in order to support legacy requirements
-  validates :phone, format: /\+[0-9]{1,3}\.[0-9]{1,14}?/
+  validates :phone, format: /\+[0-9]{1,3}\.[0-9]{1,14}?/, phone: true
+
   validates :email, format: /@/
   validates :email, email_format: { message: :invalid }, if: proc { |c| c.email_changed? }
   validates :ident,

lib/validators/phone_validator.rb

@@ -0,0 +1,19 @@
+class PhoneValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    return if record.errors[:phone].any?
+
+    splitted_phone = value.split('.')
+    country_code = splitted_phone.first
+    phone_number = splitted_phone.second
+
+    if zeros_only?(country_code) || zeros_only?(phone_number)
+      record.errors.add(attribute, :invalid)
+    end
+  end
+
+  private
+
+  def zeros_only?(value)
+    value.delete('0+').empty?
+  end
+end

spec/models/contact_spec.rb

@@ -28,12 +28,6 @@ RSpec.describe Contact do
       @contact.updator.should == nil
     end
 
-    it 'phone should return false' do
-      @contact.phone = '32341'
-      @contact.valid?
-      @contact.errors[:phone].should == ["Phone nr is invalid"]
-    end
-
     it 'should require country code when org' do
       @contact.ident_type = 'org'
       @contact.valid?
@@ -201,17 +195,17 @@ RSpec.describe Contact do
       it 'should have related domain descriptions hash' do
         contact = @domain.registrant
         contact.reload # somehow it registrant_domains are empty?
-        contact.related_domain_descriptions.should == {"#{@domain.name}" => [:registrant]}
+        contact.related_domain_descriptions.should == { "#{@domain.name}" => [:registrant] }
       end
 
       it 'should have related domain descriptions hash when find directly' do
         contact = @domain.registrant
-        Contact.find(contact.id).related_domain_descriptions.should == {"#{@domain.name}" => [:registrant]}
+        Contact.find(contact.id).related_domain_descriptions.should == { "#{@domain.name}" => [:registrant] }
       end
 
       it 'should have related domain descriptions hash' do
         contact = @domain.contacts.first
-        contact.related_domain_descriptions.should == {"#{@domain.name}" => [:admin]}
+        contact.related_domain_descriptions.should == { "#{@domain.name}" => [:admin] }
       end
 
       it 'should fully validate email syntax for old records' do
@@ -243,7 +237,7 @@ RSpec.describe Contact do
           @contact.ident = date
           @contact.valid?
           @contact.errors.full_messages.should ==
-              ["Ident Ident not in valid birthady format, should be YYYY-MM-DD"]
+            ["Ident Ident not in valid birthady format, should be YYYY-MM-DD"]
         end
       end
     end
@@ -451,6 +445,40 @@ RSpec.describe Contact, db: false do
     end
   end
 
+  describe 'phone validation', db: false do
+    let(:contact) { described_class.new }
+
+    it 'rejects absent' do
+      contact.phone = nil
+      contact.validate
+      expect(contact.errors).to have_key(:phone)
+    end
+
+    it 'rejects invalid format' do
+      contact.phone = '123'
+      contact.validate
+      expect(contact.errors).to have_key(:phone)
+    end
+
+    it 'rejects all zeros in country code' do
+      contact.phone = '+000.1'
+      contact.validate
+      expect(contact.errors).to have_key(:phone)
+    end
+
+    it 'rejects all zeros in phone number' do
+      contact.phone = '+123.0'
+      contact.validate
+      expect(contact.errors).to have_key(:phone)
+    end
+
+    it 'accepts valid' do
+      contact.phone = '+123.4'
+      contact.validate
+      expect(contact.errors).to_not have_key(:phone)
+    end
+  end
+
   describe '#remove_address' do
     let(:contact) { described_class.new(city: 'test',
                                         street: 'test',

spec/requests/epp/contact/create/phone_spec.rb

@@ -0,0 +1,35 @@
+require 'rails_helper'
+require_relative '../shared/phone'
+
+RSpec.describe 'EPP contact:create' do
+  let(:request) { post '/epp/command/create', frame: request_xml }
+  let(:request_xml) { <<-XML
+    <?xml version="1.0" encoding="UTF-8" standalone="no"?>
+    <epp xmlns="https://epp.tld.ee/schema/epp-ee-1.0.xsd">
+      <command>
+        <create>
+          <contact:create xmlns:contact="https://epp.tld.ee/schema/contact-ee-1.1.xsd">
+            <contact:postalInfo>
+              <contact:name>test</contact:name>
+            </contact:postalInfo>
+            <contact:voice>#{phone}</contact:voice>
+            <contact:email>test@test.com</contact:email>
+          </contact:create>
+        </create>
+        <extension>
+          <eis:extdata xmlns:eis="https://epp.tld.ee/schema/eis-1.0.xsd">
+            <eis:ident type="org" cc="US">123456</eis:ident>
+          </eis:extdata>
+        </extension>
+      </command>
+    </epp>
+  XML
+  }
+
+  before do
+    sign_in_to_epp_area
+    allow(Contact).to receive(:address_processing?).and_return(false)
+  end
+
+  include_examples 'EPP contact phone'
+end

spec/requests/epp/contact/shared/phone.rb

@@ -0,0 +1,28 @@
+RSpec.shared_examples 'EPP contact phone' do
+  context 'when phone is valid' do
+    let(:phone) { '+123.4' }
+
+    specify do
+      request
+      expect(response).to have_code_of(1000)
+    end
+  end
+
+  context 'when phone has invalid format' do
+    let(:phone) { '1234' }
+
+    specify do
+      request
+      expect(response).to have_code_of(2005)
+    end
+  end
+
+  context 'when phone has only zeros' do
+    let(:phone) { '+000.0' }
+
+    specify do
+      request
+      expect(response).to have_code_of(2005)
+    end
+  end
+end

spec/requests/epp/contact/update/phone_spec.rb

@@ -0,0 +1,30 @@
+require 'rails_helper'
+require_relative '../shared/phone'
+
+RSpec.describe 'EPP contact:update' do
+  let!(:contact) { create(:contact, code: 'TEST') }
+  let(:request) { post '/epp/command/update', frame: request_xml }
+  let(:request_xml) { <<-XML
+    <?xml version="1.0" encoding="UTF-8" standalone="no"?>
+    <epp xmlns="https://epp.tld.ee/schema/epp-ee-1.0.xsd">
+      <command>
+        <update>
+          <contact:update xmlns:contact="https://epp.tld.ee/schema/contact-ee-1.1.xsd">
+            <contact:id>TEST</contact:id>
+            <contact:chg>
+              <contact:voice>#{phone}</contact:voice>
+            </contact:chg>
+          </contact:update>
+        </update>
+      </command>
+    </epp>
+  XML
+  }
+
+  before do
+    sign_in_to_epp_area
+    allow(Contact).to receive(:address_processing?).and_return(false)
+  end
+
+  include_examples 'EPP contact phone'
+end