Refactor + tests

app/controllers/epp/sessions_controller.rb

@@ -18,7 +18,7 @@ class Epp::SessionsController < EppController
       @api_user = ApiUser.find_by(login_params)
     end
 
-    if @api_user.try(:active) && cert_valid
+    if @api_user.try(:active) && cert_valid && ip_white?
       if parsed_frame.css('newPW').first
         unless @api_user.update(password: parsed_frame.css('newPW').first.text)
           response.headers['X-EPP-Returncode'] = '2200'
@@ -33,6 +33,17 @@ class Epp::SessionsController < EppController
       render_epp_response('login_fail')
     end
   end
+
+  def ip_white?
+    if @api_user
+      unless @api_user.registrar.epp_ip_white?(request.ip)
+        @msg = t('ip_is_not_whitelisted')
+        return false
+      end
+    end
+    true
+  end
+
   # rubocop: enable Metrics/PerceivedComplexity
   # rubocop: enable Metrics/CyclomaticComplexity
 

app/controllers/epp_controller.rb

@@ -80,11 +80,7 @@ class EppController < ApplicationController
   end
 
   # VALIDATION
-  # rubocop: disable Metrics/PerceivedComplexity
-  # rubocop: disable Metrics/CyclomaticComplexity
   def validate_request
-    handle_errors and return unless ip_white?
-
     validation_method = "validate_#{params[:action]}"
     return unless respond_to?(validation_method, true)
     send(validation_method)
@@ -97,22 +93,6 @@ class EppController < ApplicationController
 
     handle_errors and return if epp_errors.any?
   end
-  # rubocop: enable Metrics/PerceivedComplexity
-  # rubocop: enable Metrics/CyclomaticComplexity
-
-  def ip_white?
-    if current_user
-      unless current_user.registrar.epp_ip_white?(request.ip)
-        epp_errors << {
-          msg: t('ip_is_not_whitelisted'),
-          code: '2201'
-        }
-        return false
-      end
-    end
-
-    true
-  end
 
   # let's follow grape's validations: https://github.com/intridea/grape/#parameter-validation-and-coercion
 

app/views/epp/sessions/login_fail.xml.builder

@@ -1,7 +1,7 @@
 xml.epp_head do
   xml.response do
     xml.result('code' => '2501') do
-      xml.msg('Authentication error; server closing connection')
+      xml.msg(@msg || 'Authentication error; server closing connection')
     end
   end
 

spec/epp/session_spec.rb

@@ -39,6 +39,19 @@ describe 'EPP Session', epp: true do
       response[:result_code].should == '2501'
     end
 
+    it 'does not log in with ip that is not whitelisted' do
+      @registrar = Fabricate(:registrar,
+        { name: 'registrar123', reg_no: '1234', white_ips: [Fabricate(:white_ip_repp), Fabricate(:white_ip_registrar)] }
+      )
+      Fabricate(:api_user, username: 'invalid-ip-user', registrar: @registrar)
+
+      inactive = @epp_xml.session.login(clID: { value: 'invalid-ip-user' }, pw: { value: 'ghyt9e4fu' })
+      response = epp_plain_request(inactive, :xml)
+
+      response[:msg].should == 'IP is not whitelisted'
+      response[:result_code].should == '2501'
+    end
+
     it 'prohibits further actions unless logged in' do
       response = epp_plain_request(@epp_xml.domain.create, :xml)
       response[:msg].should == 'You need to login first.'

spec/fabricators/registrar_fabricator.rb

@@ -10,7 +10,7 @@ Fabricator(:registrar) do
   code { sequence(:code) { |i| "REGISTRAR#{i}" } }
   reference_no { sequence(:reference_no) { |i| "RF#{i}" } }
   accounts(count: 1)
-  white_ips { [Fabricate(:white_ip_repp, ipv4: '127.0.0.1'), Fabricate(:white_ip, ipv4: '127.0.0.1')] }
+  white_ips { [Fabricate(:white_ip)] }
 end
 
 Fabricator(:registrar_with_no_account_activities, from: :registrar) do

spec/fabricators/white_ip_fabricator.rb

@@ -1,8 +1,16 @@
 Fabricator(:white_ip) do
   ipv4 '127.0.0.1'
-  interface WhiteIp::EPP
+  interface WhiteIp::GLOBAL
 end
 
 Fabricator(:white_ip_repp, from: :white_ip) do
   interface WhiteIp::REPP
 end
+
+Fabricator(:white_ip_epp, from: :white_ip) do
+  interface WhiteIp::EPP
+end
+
+Fabricator(:white_ip_registrar, from: :white_ip) do
+  interface WhiteIp::REGISTRAR
+end

spec/requests/v1/account_spec.rb

@@ -2,7 +2,7 @@ require 'rails_helper'
 
 describe Repp::AccountV1 do
   it 'should fail without whitelisted IP' do
-    @registrar1 = Fabricate(:registrar, white_ips: [Fabricate(:white_ip)])
+    @registrar1 = Fabricate(:registrar, white_ips: [Fabricate(:white_ip_epp), Fabricate(:white_ip_registrar)])
     @api_user = Fabricate(:api_user, registrar: @registrar1)
 
     get_with_auth '/repp/v1/accounts/balance', {}, @api_user