Modified white ip form

app/controllers/admin/white_ips_controller.rb

@@ -2,17 +2,15 @@ module Admin
   class WhiteIpsController < BaseController
     load_and_authorize_resource
 
-    before_action :set_registrar, only: [:new, :show, :edit, :destroy, :update]
+    before_action :set_registrar, only: %i[new show edit destroy update]
 
     def new
       @white_ip = WhiteIp.new(registrar: @registrar)
     end
 
-    def show;
-    end
+    def show; end
 
-    def edit;
-    end
+    def edit; end
 
     def destroy
       if @white_ip.destroy

app/controllers/repp/v1/white_ips_controller.rb

@@ -63,7 +63,7 @@ module Repp
       end
 
       def white_ip_params
-        params.require(:white_ip).permit(:ipv4, :ipv6, interfaces: [])
+        params.require(:white_ip).permit(:address, interfaces: [])
       end
     end
   end

app/models/white_ip.rb

@@ -2,10 +2,14 @@ class WhiteIp < ApplicationRecord
   include Versions
   belongs_to :registrar
 
+  attr_accessor :address
+
+  validate :validate_address_format
+  validates :ipv4, uniqueness: { scope: :registrar_id }, if: :ipv4?
+  validates :ipv6, uniqueness: { scope: :registrar_id }, if: :ipv6?
+  validate :validate_only_one_ip
   validate :valid_ipv4?
   validate :valid_ipv6?
-  validate :validate_ipv4_and_ipv6
-  validate :validate_only_one_ip
   validate :validate_max_ip_count
   before_save :normalize_blank_values
 
@@ -13,16 +17,35 @@ class WhiteIp < ApplicationRecord
     %i[ipv4 ipv6].each { |c| self[c].present? || self[c] = nil }
   end
 
-  def validate_ipv4_and_ipv6
-    return if ipv4.present? || ipv6.present?
+  def validate_address_format
+    return if address.blank?
 
-    errors.add(:base, I18n.t(:ipv4_or_ipv6_must_be_present))
+    ip_address = IPAddr.new(address)
+    ip_version = determine_ip_version(ip_address)
+
+    assign_ip_attributes(ip_version)
+  rescue IPAddr::InvalidAddressError
+    errors.add(:address, :invalid)
   end
 
   def validate_only_one_ip
-    return unless ipv4.present? && ipv6.present?
-
+    if ipv4.present? && ipv6.present?
       errors.add(:base, I18n.t(:ip_must_be_one))
+    elsif ipv4.blank? && ipv6.blank?
+      errors.add(:base, I18n.t(:ipv4_or_ipv6_must_be_present))
+    end
+  end
+
+  def validate_max_ip_count
+    return if errors.any?
+
+    total_exist = calculate_total_network_addresses(registrar.white_ips)
+    total_current = calculate_total_network_addresses([self])
+    total = total_exist + total_current
+    limit = Setting.ip_whitelist_max_count
+    return unless total >= limit
+
+    errors.add(:base, I18n.t(:ip_limit_exceeded, total: total, limit: limit))
   end
 
   def valid_ipv4?
@@ -41,31 +64,6 @@ class WhiteIp < ApplicationRecord
     errors.add(:ipv6, :invalid)
   end
 
-  def validate_max_ip_count
-    return if errors.any?
-
-    ip_addresses = registrar.white_ips
-    total = ip_addresses.size + count_network_addresses(ipv4.presence || ipv6)
-    limit = Setting.ip_whitelist_max_count
-    return unless total >= limit
-
-    errors.add(:base, I18n.t(:ip_limit_exceeded, total: total, limit: limit))
-  end
-
-  def count_network_addresses(ip)
-    address = IPAddr.new(ip)
-
-    if address.ipv4?
-      subnet_mask = address.prefix
-      2**(32 - subnet_mask) - 2
-    elsif address.ipv6?
-      subnet_mask = address.prefix
-      2**(128 - subnet_mask) - 2
-    else
-      0
-    end
-  end
-
   API = 'api'.freeze
   REGISTRAR = 'registrar'.freeze
   INTERFACES = [API, REGISTRAR].freeze
@@ -125,4 +123,44 @@ class WhiteIp < ApplicationRecord
       updated_at,
     ]
   end
+
+  private
+
+  def determine_ip_version(ip_address)
+    return :ipv4 if ip_address.ipv4?
+    return :ipv6 if ip_address.ipv6?
+
+    nil
+  end
+
+  def assign_ip_attributes(ip_version)
+    case ip_version
+    when :ipv4
+      self.ipv4 = address
+      self.ipv6 = nil
+    when :ipv6
+      self.ipv6 = address
+      self.ipv4 = nil
+    else
+      errors.add(:address, :invalid)
+    end
+  end
+
+  def count_network_addresses(ip)
+    address = IPAddr.new(ip)
+
+    if address.ipv4?
+      subnet_mask = address.prefix
+      (2**(32 - subnet_mask) - 2).abs
+    elsif address.ipv6?
+      subnet_mask = address.prefix
+      (2**(128 - subnet_mask) - 2).abs
+    else
+      0
+    end
+  end
+
+  def calculate_total_network_addresses(ips)
+    ips.sum { |ip| count_network_addresses(ip.ipv4.presence || ip.ipv6) }
+  end
 end

test/integration/repp/v1/white_ips/create_test.rb

@@ -15,8 +15,7 @@ class ReppV1WhiteIpsCreateTest < ActionDispatch::IntegrationTest
   def test_creates_new_white_ip
     request_body = {
       white_ip: {
-        ipv4: '127.0.0.1',
-        ipv6: '',
+        address: '127.1.1.1',
         interfaces: ['API'],
       },
     }
@@ -31,14 +30,13 @@ class ReppV1WhiteIpsCreateTest < ActionDispatch::IntegrationTest
     white_ip = WhiteIp.find(json[:data][:ip][:id])
     assert white_ip.present?
 
-    assert_equal(request_body[:white_ip][:ipv4], white_ip.ipv4)
+    assert_equal(request_body[:white_ip][:address], white_ip.ipv4)
   end
 
   def test_validates_ip_max_count
     request_body = {
       white_ip: {
-        ipv4: '',
-        ipv6: '2001:db8::/120',
+        address: '2001:db8::/120',
         interfaces: ['API'],
       },
     }
@@ -50,14 +48,29 @@ class ReppV1WhiteIpsCreateTest < ActionDispatch::IntegrationTest
     assert json[:message].include? 'IP address limit exceeded'
   end
 
+  def test_validates_ip_uniqueness_per_registrar
+    white_ip = white_ips(:one)
+    request_body = {
+      white_ip: {
+        address: white_ip.ipv4,
+        interfaces: ['API'],
+      },
+    }
+
+    post '/repp/v1/white_ips', headers: @auth_headers, params: request_body
+    json = JSON.parse(response.body, symbolize_names: true)
+
+    assert_response :bad_request
+    assert json[:message].include? 'IPv4 has already been taken'
+  end
+
   def test_returns_error_response_if_throttled
     ENV['shunter_default_threshold'] = '1'
     ENV['shunter_enabled'] = 'true'
 
     request_body = {
       white_ip: {
-        ipv4: '127.0.0.1',
-        ipv6: '',
+        address: '127.0.0.1',
         interfaces: ['API'],
       },
     }

test/integration/repp/v1/white_ips/update_test.rb

@@ -16,7 +16,7 @@ class ReppV1ApiWhiteIpsUpdateTest < ActionDispatch::IntegrationTest
   def test_updates_white_ip
     request_body = {
       white_ip: {
-        ipv4: '127.0.0.1',
+        address: '127.0.0.1',
       },
     }
 
@@ -34,7 +34,7 @@ class ReppV1ApiWhiteIpsUpdateTest < ActionDispatch::IntegrationTest
   def test_returns_error_if_ipv4_wrong_format
     request_body = {
       white_ip: {
-        ipv4: 'wrongip',
+        address: 'wrongip',
       },
     }
 
@@ -42,21 +42,7 @@ class ReppV1ApiWhiteIpsUpdateTest < ActionDispatch::IntegrationTest
     json = JSON.parse(response.body, symbolize_names: true)
 
     assert_response :bad_request
-    assert json[:message].include? 'IPv4 is invalid'
-  end
-
-  def test_returns_error_if_both_ips
-    request_body = {
-      white_ip: {
-        ipv6: '2001:0db8:85a3:0000:0000:8a2e:0370:7334',
-      },
-    }
-
-    put "/repp/v1/white_ips/#{@white_ip.id}", headers: @auth_headers, params: request_body
-    json = JSON.parse(response.body, symbolize_names: true)
-
-    assert_response :bad_request
-    assert json[:message].include? 'Please enter only one IP address'
+    assert json[:message].include? 'Address is invalid'
   end
 
   def test_returns_error_response_if_throttled
@@ -65,7 +51,7 @@ class ReppV1ApiWhiteIpsUpdateTest < ActionDispatch::IntegrationTest
 
     request_body = {
       white_ip: {
-        ipv4: '127.0.0.1',
+        address: '127.0.0.1',
       },
     }