diff --git a/app/controllers/admin/white_ips_controller.rb b/app/controllers/admin/white_ips_controller.rb index 8554774d3..a621a8386 100644 --- a/app/controllers/admin/white_ips_controller.rb +++ b/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 diff --git a/app/controllers/repp/v1/white_ips_controller.rb b/app/controllers/repp/v1/white_ips_controller.rb index 4cb8d8987..93b9ef155 100644 --- a/app/controllers/repp/v1/white_ips_controller.rb +++ b/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 diff --git a/app/models/white_ip.rb b/app/models/white_ip.rb index fb638c631..24fb6b592 100644 --- a/app/models/white_ip.rb +++ b/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 - errors.add(:base, I18n.t(:ip_must_be_one)) + 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 diff --git a/test/integration/repp/v1/white_ips/create_test.rb b/test/integration/repp/v1/white_ips/create_test.rb index 2b152d41c..7cda9affa 100644 --- a/test/integration/repp/v1/white_ips/create_test.rb +++ b/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'], }, } diff --git a/test/integration/repp/v1/white_ips/update_test.rb b/test/integration/repp/v1/white_ips/update_test.rb index 03b34ef01..b04d357be 100644 --- a/test/integration/repp/v1/white_ips/update_test.rb +++ b/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', }, }