UX: improve message when admin login is blocked because of admin ip address whitelisting

app/assets/javascripts/discourse/controllers/login.js.es6

@@ -164,6 +164,12 @@ export default DiscourseController.extend(ModalFunctionality, {
       this.set('authenticate', null);
       return;
     }
+    if (options.admin_not_allowed_from_ip_address) {
+      this.send('showLogin');
+      this.flash(I18n.t('login.admin_not_allowed_from_ip_address'), 'success');
+      this.set('authenticate', null);
+      return;
+    }
     if (options.not_allowed_from_ip_address) {
       this.send('showLogin');
       this.flash(I18n.t('login.not_allowed_from_ip_address'), 'success');

app/controllers/session_controller.rb

@@ -147,11 +147,14 @@ class SessionController < ApplicationController
       return
     end
 
-    if ScreenedIpAddress.block_login?(user, request.remote_ip) ||
-       ScreenedIpAddress.should_block?(request.remote_ip)
+    if ScreenedIpAddress.should_block?(request.remote_ip)
       return not_allowed_from_ip_address(user)
     end
 
+    if ScreenedIpAddress.block_admin_login?(user, request.remote_ip)
+      return admin_not_allowed_from_ip_address(user)
+    end
+
     (user.active && user.email_confirmed?) ? login(user) : not_activated(user)
   end
 
@@ -229,6 +232,10 @@ class SessionController < ApplicationController
     render json: {error: I18n.t("login.not_allowed_from_ip_address", username: user.username)}
   end
 
+  def admin_not_allowed_from_ip_address(user)
+    render json: {error: I18n.t("login.admin_not_allowed_from_ip_address", username: user.username)}
+  end
+
   def failed_to_login(user)
     message = user.suspend_reason ? "login.suspended_with_reason" : "login.suspended"
 

app/controllers/users/omniauth_callbacks_controller.rb

@@ -85,8 +85,10 @@ class Users::OmniauthCallbacksController < ApplicationController
       user.toggle(:active).save
     end
 
-    if ScreenedIpAddress.block_login?(user, request.remote_ip)
+    if ScreenedIpAddress.should_block?(request.remote_ip)
       @data.not_allowed_from_ip_address = true
+    elsif ScreenedIpAddress.block_admin_login?(user, request.remote_ip)
+      @data.admin_not_allowed_from_ip_address = true
     elsif Guardian.new(user).can_access_forum? && user.active # log on any account that is active with forum access
       log_on_user(user)
       Invite.invalidate_for_email(user.email) # invite link can't be used to log in anymore

app/models/screened_ip_address.rb

@@ -74,7 +74,7 @@ class ScreenedIpAddress < ActiveRecord::Base
     found
   end
 
-  def self.block_login?(user, ip_address)
+  def self.block_admin_login?(user, ip_address)
     return false if user.nil?
     return false if !user.admin?
     return false if ScreenedIpAddress.where(action_type: actions[:allow_admin]).count == 0

config/locales/client.en.yml

@@ -640,6 +640,7 @@ en:
       requires_invite: "Sorry, access to this forum is by invite only."
       not_activated: "You can't log in yet. We previously sent an activation email to you at <b>{{sentTo}}</b>. Please follow the instructions in that email to activate your account."
       not_allowed_from_ip_address: "You can't login from that IP address."
+      admin_not_allowed_from_ip_address: "You can't log in as admin from that IP address."
       resend_activation_email: "Click here to send the activation email again."
       sent_activation_email_again: "We sent another activation email to you at <b>{{currentEmail}}</b>. It might take a few minutes for it to arrive; be sure to check your spam folder."
       google:

config/locales/server.en.yml

@@ -1228,7 +1228,8 @@ en:
     active: "Your account is activated and ready to use."
     activate_email: "<p>You're almost done! We sent an activation mail to <b>%{email}</b>. Please follow the instructions in the email to activate your account.</p><p>If it doesn't arrive, check your spam folder, or try to log in again to send another activation mail.</p>"
     not_activated: "You can't log in yet. We sent an activation email to you. Please follow the instructions in the email to activate your account."
-    not_allowed_from_ip_address: "You can't login as %{username} from that IP address."
+    not_allowed_from_ip_address: "You can't log in as %{username} from that IP address."
+    admin_not_allowed_from_ip_address: "You can't log in as admin from that IP address."
     suspended: "You can't log in until %{date}."
     suspended_with_reason: "You can't log in until %{date}. The reason you were suspended: %{reason}"
     errors: "%{errors}"

lib/auth/result.rb

@@ -2,7 +2,8 @@ class Auth::Result
   attr_accessor :user, :name, :username, :email, :user,
                 :email_valid, :extra_data, :awaiting_activation,
                 :awaiting_approval, :authenticated, :authenticator_name,
-                :requires_invite, :not_allowed_from_ip_address
+                :requires_invite, :not_allowed_from_ip_address,
+                :admin_not_allowed_from_ip_address
 
   def session_data
     {
@@ -30,7 +31,8 @@ class Auth::Result
           authenticated: !!authenticated,
           awaiting_activation: !!awaiting_activation,
           awaiting_approval: !!awaiting_approval,
-          not_allowed_from_ip_address: !!not_allowed_from_ip_address
+          not_allowed_from_ip_address: !!not_allowed_from_ip_address,
+          admin_not_allowed_from_ip_address: !!admin_not_allowed_from_ip_address
         }
       end
     else

spec/models/screened_ip_address_spec.rb

@@ -238,22 +238,22 @@ describe ScreenedIpAddress do
     end
   end
 
-  describe '#block_login?' do
+  describe '#block_admin_login?' do
     context 'no allow_admin records exist' do
       it "returns false when user is nil" do
-        expect(described_class.block_login?(nil, '123.12.12.12')).to eq(false)
+        expect(described_class.block_admin_login?(nil, '123.12.12.12')).to eq(false)
       end
 
       it "returns false for non-admin user" do
-        expect(described_class.block_login?(Fabricate.build(:user), '123.12.12.12')).to eq(false)
+        expect(described_class.block_admin_login?(Fabricate.build(:user), '123.12.12.12')).to eq(false)
       end
 
       it "returns false for admin user" do
-        expect(described_class.block_login?(Fabricate.build(:admin), '123.12.12.12')).to eq(false)
+        expect(described_class.block_admin_login?(Fabricate.build(:admin), '123.12.12.12')).to eq(false)
       end
 
       it "returns false for admin user and ip_address arg is nil" do
-        expect(described_class.block_login?(Fabricate.build(:admin), nil)).to eq(false)
+        expect(described_class.block_admin_login?(Fabricate.build(:admin), nil)).to eq(false)
       end
     end
 
@@ -264,23 +264,23 @@ describe ScreenedIpAddress do
       end
 
       it "returns false when user is nil" do
-        expect(described_class.block_login?(nil, @permitted_ip_address)).to eq(false)
+        expect(described_class.block_admin_login?(nil, @permitted_ip_address)).to eq(false)
       end
 
       it "returns false for an admin user at the allowed ip address" do
-        expect(described_class.block_login?(Fabricate.build(:admin), @permitted_ip_address)).to eq(false)
+        expect(described_class.block_admin_login?(Fabricate.build(:admin), @permitted_ip_address)).to eq(false)
       end
 
       it "returns true for an admin user at another ip address" do
-        expect(described_class.block_login?(Fabricate.build(:admin), '123.12.12.12')).to eq(true)
+        expect(described_class.block_admin_login?(Fabricate.build(:admin), '123.12.12.12')).to eq(true)
       end
 
       it "returns false for regular user at allowed ip address" do
-        expect(described_class.block_login?(Fabricate.build(:user), @permitted_ip_address)).to eq(false)
+        expect(described_class.block_admin_login?(Fabricate.build(:user), @permitted_ip_address)).to eq(false)
       end
 
       it "returns false for regular user at another ip address" do
-        expect(described_class.block_login?(Fabricate.build(:user), '123.12.12.12')).to eq(false)
+        expect(described_class.block_admin_login?(Fabricate.build(:user), '123.12.12.12')).to eq(false)
       end
     end
   end