Refactor IP registrar restriction

#600

app/api/repp/api.rb

@@ -15,7 +15,7 @@ module Repp
     before do
       webclient_request = ENV['webclient_ips'].split(',').map(&:strip).include?(request.ip)
       unless webclient_request
-        error! I18n.t('ip_is_not_whitelisted'), 401 unless @current_user.registrar.api_ip_white?(request.ip)
+        error! I18n.t('api.authorization.ip_not_allowed', ip: request.ip), 401 unless @current_user.registrar.api_ip_white?(request.ip)
       end
 
       if @current_user.cannot?(:view, :repp)

app/controllers/registrar/base_controller.rb

@@ -1,40 +1,37 @@
 class Registrar
   class BaseController < ApplicationController
-    before_action :authenticate_user!, :check_ip
-
     include Registrar::ApplicationHelper
 
+    before_action :authenticate_user!
+    before_action :check_ip_restriction
     helper_method :depp_controller?
-
-    def depp_controller?
-      false
-    end
-
-    def check_ip
-      return unless current_user
-      unless current_user.is_a? ApiUser
-        sign_out(current_user)
-        return
-      end
-
-      registrar_ip_whitelisted = current_user.registrar.registrar_ip_white?(request.ip)
-
-      return if registrar_ip_whitelisted
-      flash[:alert] = t('ip_is_not_whitelisted')
-      sign_out(current_user)
-      redirect_to registrar_login_path and return
-    end
-
     helper_method :head_title_sufix
 
-    def head_title_sufix
-      t(:registrar_head_title_sufix)
-    end
-
     protected
 
     def current_ability
       @current_ability ||= Ability.new(current_user, request.remote_ip)
     end
+
+    private
+
+    def check_ip_restriction
+      ip_restriction = Authorization::RestrictedIP.new(request.ip)
+      allowed = ip_restriction.can_access_registrar_area?(current_user.registrar)
+
+      unless allowed
+        flash[:alert] = t('registrar.authorization.ip_not_allowed', ip: request.ip)
+        sign_out current_user
+        redirect_to registrar_login_url
+      end
+    end
+
+    def depp_controller?
+      false
+    end
+
+    def head_title_sufix
+      t(:registrar_head_title_sufix)
+    end
   end
 end

app/controllers/registrar/sessions_controller.rb

@@ -1,13 +1,8 @@
 class Registrar
   class SessionsController < Devise::SessionsController
+    before_action :check_ip_restriction
     helper_method :depp_controller?
 
-    def depp_controller?
-      false
-    end
-
-    before_action :check_ip
-
     def login
       @depp_user = Depp::User.new
     end
@@ -157,16 +152,24 @@ class Registrar
     # rubocop: enable Metrics/CyclomaticComplexity
     # rubocop: enable Metrics/MethodLength
 
+    private
+
+    def depp_controller?
+      false
+    end
+
     def find_user_by_idc(idc)
       return User.new unless idc
       ApiUser.find_by(identity_code: idc) || User.new
     end
 
-    private
+    def check_ip_restriction
+      ip_restriction = Authorization::RestrictedIP.new(request.ip)
+      allowed = ip_restriction.can_access_registrar_area_sign_in_page?
 
-    def check_ip
+      unless allowed
-      return if WhiteIp.registrar_ip_white?(request.ip)
+        render text: t('registrar.authorization.ip_not_allowed', ip: request.ip), status: :forbidden
-      render :denied, :layout => false, status: :forbidden, :locals => { :ip => request.ip } and return
+      end
     end
   end
 end

app/models/authorization/restricted_ip.rb

@@ -0,0 +1,25 @@
+module Authorization
+  class RestrictedIP
+    def initialize(ip)
+      @ip = ip
+    end
+
+    def self.enabled?
+      Setting.registrar_ip_whitelist_enabled
+    end
+
+    def can_access_registrar_area?(registrar)
+      return true unless self.class.enabled?
+      registrar.white_ips.registrar_area.include_ip?(ip)
+    end
+
+    def can_access_registrar_area_sign_in_page?
+      return true unless self.class.enabled?
+      WhiteIp.registrar_area.include_ip?(ip)
+    end
+
+    private
+
+    attr_reader :ip
+  end
+end

app/models/registrar.rb

@@ -162,9 +162,4 @@ class Registrar < ActiveRecord::Base
     return true unless Setting.api_ip_whitelist_enabled
     white_ips.api.pluck(:ipv4, :ipv6).flatten.include?(ip)
   end
-
-  def registrar_ip_white?(ip)
-    return true unless Setting.registrar_ip_whitelist_enabled
-    white_ips.registrar.pluck(:ipv4, :ipv6).flatten.include?(ip)
-  end
 end

app/models/white_ip.rb

@@ -18,16 +18,15 @@ class WhiteIp < ActiveRecord::Base
   INTERFACES = [API, REGISTRAR]
 
   scope :api, -> { where("interfaces @> ?::varchar[]", "{#{API}}") }
-  scope :registrar, -> { where("interfaces @> ?::varchar[]", "{#{REGISTRAR}}") }
+  scope :registrar_area, -> { where("interfaces @> ?::varchar[]", "{#{REGISTRAR}}") }
 
   def interfaces=(interfaces)
     super(interfaces.reject(&:blank?))
   end
 
   class << self
-    def registrar_ip_white?(ip)
+    def include_ip?(ip)
-      return true unless Setting.registrar_ip_whitelist_enabled
+      where("#{table_name}.ipv4 = '#{ip}' OR #{table_name}.ipv6 = '#{ip}'").any?
-      WhiteIp.where(ipv4: ip).registrar.any?
     end
   end
 end

app/views/registrar/sessions/denied.haml

@@ -1 +0,0 @@
-#{t('access_denied')} from #{ip}

config/locales/api/authorization.en.yml

@@ -0,0 +1,4 @@
+en:
+  api:
+    authorization:
+      ip_not_allowed: Access denied from IP %{ip}

config/locales/en.yml

@@ -797,7 +797,6 @@ en:
   domain_delete_rejected_title: 'Domain deletion rejection has been received successfully'
   domain_delete_rejected_body: 'You have rejected pending domain deletion. You will receive confirmation by email.'
   no_permission: 'No permission'
-  access_denied: 'Access denied'
   common_name: 'Common name'
   md5: 'Md5'
   interface: 'Interface'
@@ -815,7 +814,6 @@ en:
   create_bank_statement: 'Create bank statement'
   create_bank_transaction: 'Create bank transaction'
   create_new_invoice: 'Create new invoice'
-  ip_is_not_whitelisted: 'IP is not whitelisted'
   billing_settings: 'Billing settings'
   registry_settings: 'Registry settings'
   registry_billing_email: 'Billing e-mail'

config/locales/registrar/authorization.en.yml

@@ -0,0 +1,4 @@
+en:
+  registrar:
+    authorization:
+      ip_not_allowed: Access denied from IP %{ip}

doc/controllers_complete.svg

@@ -136,9 +136,6 @@
 <path fill="none" stroke="black" d="M-708,539.5C-708,539.5 -610,539.5 -610,539.5 -604,539.5 -598,533.5 -598,527.5 -598,527.5 -598,428.5 -598,428.5 -598,422.5 -604,416.5 -610,416.5 -610,416.5 -708,416.5 -708,416.5 -714,416.5 -720,422.5 -720,428.5 -720,428.5 -720,527.5 -720,527.5 -720,533.5 -714,539.5 -708,539.5"/>
 <text text-anchor="middle" x="-659" y="431.7" font-family="Times,serif" font-size="14.00">RegistrarController</text>
 <polyline fill="none" stroke="black" points="-720,439.5 -598,439.5 "/>
-<text text-anchor="start" x="-712" y="454.7" font-family="Times,serif" font-size="14.00">check_ip</text>
-<text text-anchor="start" x="-712" y="469.7" font-family="Times,serif" font-size="14.00">depp_controller?</text>
-<text text-anchor="start" x="-712" y="484.7" font-family="Times,serif" font-size="14.00">head_title_sufix</text>
 <polyline fill="none" stroke="black" points="-720,492.5 -598,492.5 "/>
 <polyline fill="none" stroke="black" points="-720,516.5 -598,516.5 "/>
 <text text-anchor="start" x="-712" y="531.7" font-family="Times,serif" font-size="14.00">_layout</text>
@@ -491,8 +488,6 @@
 <text text-anchor="middle" x="-212" y="-513.8" font-family="Times,serif" font-size="14.00">Registrar::SessionsController</text>
 <polyline fill="none" stroke="black" points="-300,-506 -124,-506 "/>
 <text text-anchor="start" x="-292" y="-490.8" font-family="Times,serif" font-size="14.00">create</text>
-<text text-anchor="start" x="-292" y="-475.8" font-family="Times,serif" font-size="14.00">depp_controller?</text>
-<text text-anchor="start" x="-292" y="-460.8" font-family="Times,serif" font-size="14.00">find_user_by_idc</text>
 <text text-anchor="start" x="-292" y="-445.8" font-family="Times,serif" font-size="14.00">id</text>
 <text text-anchor="start" x="-292" y="-430.8" font-family="Times,serif" font-size="14.00">login</text>
 <text text-anchor="start" x="-292" y="-415.8" font-family="Times,serif" font-size="14.00">login_mid</text>

spec/features/registrar/sessions/new_spec.rb

@@ -0,0 +1,42 @@
+require 'rails_helper'
+
+RSpec.feature 'Registrar area ip restriction', settings: false do
+  context 'when enabled' do
+    background do
+      Setting.registrar_ip_whitelist_enabled = true
+    end
+
+    context 'when ip is allowed' do
+      given!(:white_ip) { create(:white_ip,
+                                 ipv4: '127.0.0.1',
+                                 interfaces: [WhiteIp::REGISTRAR]) }
+
+      it 'does not show error message' do
+        visit registrar_login_path
+        expect(page).to_not have_text(error_message)
+      end
+    end
+
+    context 'when ip is not allowed' do
+      it 'shows error message' do
+        visit registrar_login_path
+        expect(page).to have_text(error_message)
+      end
+    end
+  end
+
+  context 'when disabled' do
+    background do
+      Setting.registrar_ip_whitelist_enabled = false
+    end
+
+    it 'does not show error message' do
+      visit registrar_login_path
+      expect(page).to_not have_text(error_message)
+    end
+  end
+
+  def error_message
+    t('registrar.authorization.ip_not_allowed', ip: '127.0.0.1')
+  end
+end

spec/models/authorization/restricted_ip_spec.rb

@@ -0,0 +1,94 @@
+require 'rails_helper'
+
+RSpec.describe Authorization::RestrictedIP do
+  describe '#enabled?', db: true, settings: false do
+    context 'when "registrar_ip_whitelist_enabled" is true' do
+      before do
+        Setting.registrar_ip_whitelist_enabled = true
+      end
+
+      specify do
+        expect(described_class).to be_enabled
+      end
+    end
+
+    context 'when "registrar_ip_whitelist_enabled" is false' do
+      before do
+        Setting.registrar_ip_whitelist_enabled = false
+      end
+
+      specify do
+        expect(described_class).to_not be_enabled
+      end
+    end
+  end
+
+  describe '#can_access_registrar_area?', db: true do
+    let(:registrar) { create(:registrar) }
+    subject(:allowed) { described_class.new('127.0.0.1').can_access_registrar_area?(registrar) }
+
+    context 'when enabled' do
+      before do
+        allow(described_class).to receive(:enabled?).and_return(true)
+      end
+
+      context 'when ip is whitelisted', db: true do
+        let!(:white_ip) { create(:white_ip, registrar: registrar, ipv4: '127.0.0.1', interfaces: [WhiteIp::REGISTRAR]) }
+
+        specify do
+          expect(allowed).to be true
+        end
+      end
+
+      context 'when ip is not whitelisted' do
+        specify do
+          expect(allowed).to be false
+        end
+      end
+    end
+
+    context 'when disabled' do
+      before do
+        allow(described_class).to receive(:enabled?).and_return(false)
+      end
+
+      specify do
+        expect(allowed).to be true
+      end
+    end
+  end
+
+  describe '#can_access_registrar_area_sign_in_page?' do
+    subject(:allowed) { described_class.new('127.0.0.1').can_access_registrar_area_sign_in_page? }
+
+    context 'when enabled' do
+      before do
+        allow(described_class).to receive(:enabled?).and_return(true)
+      end
+
+      context 'when ip is whitelisted', db: true do
+        let!(:white_ip) { create(:white_ip, ipv4: '127.0.0.1', interfaces: [WhiteIp::REGISTRAR]) }
+
+        specify do
+          expect(allowed).to be true
+        end
+      end
+
+      context 'when ip is not whitelisted' do
+        specify do
+          expect(allowed).to be false
+        end
+      end
+    end
+
+    context 'when disabled' do
+      before do
+        allow(described_class).to receive(:enabled?).and_return(false)
+      end
+
+      specify do
+        expect(allowed).to be true
+      end
+    end
+  end
+end

spec/models/white_ip_spec.rb

@@ -38,4 +38,32 @@ describe WhiteIp do
       end
     end
   end
+
+  describe '#include_ip?' do
+    context 'when given ip v4 exists' do
+      before do
+        create(:white_ip, ipv4: '127.0.0.1')
+      end
+
+      specify do
+        expect(described_class.include_ip?('127.0.0.1')).to be true
+      end
+    end
+
+    context 'when given ip v6 exists' do
+      before do
+        create(:white_ip, ipv6: '::1')
+      end
+
+      specify do
+        expect(described_class.include_ip?('::1')).to be true
+      end
+    end
+
+    context 'when given ip does not exists', db: false do
+      specify do
+        expect(described_class.include_ip?('127.0.0.1')).to be false
+      end
+    end
+  end
 end

spec/requests/registrar/sessions_spec.rb

@@ -0,0 +1,67 @@
+require 'rails_helper'
+
+RSpec.describe 'Registrar session management', db: false do
+  describe 'GET /registrar/login' do
+    context 'when ip is allowed' do
+      let(:restricted_ip) { instance_double(Authorization::RestrictedIP,
+                                            can_access_registrar_area_sign_in_page?: true) }
+
+      before do
+        allow(Authorization::RestrictedIP).to receive(:new).and_return(restricted_ip)
+      end
+
+      specify do
+        get registrar_login_path
+        expect(response).to be_success
+      end
+    end
+
+    context 'when ip is not allowed' do
+      let(:restricted_ip) { instance_double(Authorization::RestrictedIP,
+                                            can_access_registrar_area_sign_in_page?: false) }
+
+      before do
+        allow(Authorization::RestrictedIP).to receive(:new).and_return(restricted_ip)
+      end
+
+      specify do
+        get registrar_login_path
+        expect(response).to be_forbidden
+      end
+    end
+  end
+
+  describe 'POST /registrar/sessions' do
+    context 'when ip is allowed' do
+      let(:restricted_ip) { instance_double(Authorization::RestrictedIP,
+                                            can_access_registrar_area_sign_in_page?: true) }
+
+      before do
+        allow(Authorization::RestrictedIP).to receive(:new).and_return(restricted_ip)
+      end
+
+      specify do
+        make_request
+        expect(response).to be_success
+      end
+    end
+
+    context 'when ip is not allowed' do
+      let(:restricted_ip) { instance_double(Authorization::RestrictedIP,
+                                            can_access_registrar_area_sign_in_page?: false) }
+
+      before do
+        allow(Authorization::RestrictedIP).to receive(:new).and_return(restricted_ip)
+      end
+
+      specify do
+        make_request
+        expect(response).to be_forbidden
+      end
+    end
+
+    def make_request
+      post registrar_sessions_path, depp_user: { tag: 'test', password: 'test' }
+    end
+  end
+end