diff --git a/app/api/repp/api.rb b/app/api/repp/api.rb index 27d0322b0..394edfdad 100644 --- a/app/api/repp/api.rb +++ b/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) diff --git a/app/controllers/registrar/base_controller.rb b/app/controllers/registrar/base_controller.rb index d2b2edd69..0455b2a2e 100644 --- a/app/controllers/registrar/base_controller.rb +++ b/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 diff --git a/app/controllers/registrar/sessions_controller.rb b/app/controllers/registrar/sessions_controller.rb index 6bf31b03a..9f6189c71 100644 --- a/app/controllers/registrar/sessions_controller.rb +++ b/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 - return if WhiteIp.registrar_ip_white?(request.ip) - render :denied, :layout => false, status: :forbidden, :locals => { :ip => request.ip } and return + unless allowed + render text: t('registrar.authorization.ip_not_allowed', ip: request.ip), status: :forbidden + end end end end diff --git a/app/models/authorization/restricted_ip.rb b/app/models/authorization/restricted_ip.rb new file mode 100644 index 000000000..b3c7b7cdb --- /dev/null +++ b/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 diff --git a/app/models/registrar.rb b/app/models/registrar.rb index 734e68898..b906beee3 100644 --- a/app/models/registrar.rb +++ b/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 diff --git a/app/models/white_ip.rb b/app/models/white_ip.rb index 918315004..f11606610 100644 --- a/app/models/white_ip.rb +++ b/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) - return true unless Setting.registrar_ip_whitelist_enabled - WhiteIp.where(ipv4: ip).registrar.any? + def include_ip?(ip) + where("#{table_name}.ipv4 = '#{ip}' OR #{table_name}.ipv6 = '#{ip}'").any? end end end diff --git a/app/views/registrar/sessions/denied.haml b/app/views/registrar/sessions/denied.haml deleted file mode 100644 index a1aea470b..000000000 --- a/app/views/registrar/sessions/denied.haml +++ /dev/null @@ -1 +0,0 @@ -#{t('access_denied')} from #{ip} diff --git a/config/locales/api/authorization.en.yml b/config/locales/api/authorization.en.yml new file mode 100644 index 000000000..53c49d146 --- /dev/null +++ b/config/locales/api/authorization.en.yml @@ -0,0 +1,4 @@ +en: + api: + authorization: + ip_not_allowed: Access denied from IP %{ip} diff --git a/config/locales/en.yml b/config/locales/en.yml index 2627b112a..a9b54c210 100644 --- a/config/locales/en.yml +++ b/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' diff --git a/config/locales/registrar/authorization.en.yml b/config/locales/registrar/authorization.en.yml new file mode 100644 index 000000000..1e4395cb3 --- /dev/null +++ b/config/locales/registrar/authorization.en.yml @@ -0,0 +1,4 @@ +en: + registrar: + authorization: + ip_not_allowed: Access denied from IP %{ip} diff --git a/doc/controllers_complete.svg b/doc/controllers_complete.svg index f48433540..1c9bc94ec 100644 --- a/doc/controllers_complete.svg +++ b/doc/controllers_complete.svg @@ -136,9 +136,6 @@ RegistrarController -check_ip -depp_controller? -head_title_sufix _layout @@ -491,8 +488,6 @@ Registrar::SessionsController create -depp_controller? -find_user_by_idc id login login_mid diff --git a/spec/features/registrar/sessions/new_spec.rb b/spec/features/registrar/sessions/new_spec.rb new file mode 100644 index 000000000..feec6cae1 --- /dev/null +++ b/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 diff --git a/spec/models/authorization/restricted_ip_spec.rb b/spec/models/authorization/restricted_ip_spec.rb new file mode 100644 index 000000000..6fba76657 --- /dev/null +++ b/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 diff --git a/spec/models/white_ip_spec.rb b/spec/models/white_ip_spec.rb index 931431c1e..bb286c0ca 100644 --- a/spec/models/white_ip_spec.rb +++ b/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 diff --git a/spec/requests/registrar/sessions_spec.rb b/spec/requests/registrar/sessions_spec.rb new file mode 100644 index 000000000..674cd8f0c --- /dev/null +++ b/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