From 7c14db44ccbf73ecbfd2d0c5829e3fe84233e712 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Mon, 2 Mar 2015 12:13:10 -0500 Subject: [PATCH] UX: improve message when admin login is blocked because of admin ip address whitelisting --- .../discourse/controllers/login.js.es6 | 6 ++++++ app/controllers/session_controller.rb | 11 ++++++++-- .../users/omniauth_callbacks_controller.rb | 4 +++- app/models/screened_ip_address.rb | 2 +- config/locales/client.en.yml | 1 + config/locales/server.en.yml | 3 ++- lib/auth/result.rb | 6 ++++-- spec/models/screened_ip_address_spec.rb | 20 +++++++++---------- 8 files changed, 36 insertions(+), 17 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/login.js.es6 b/app/assets/javascripts/discourse/controllers/login.js.es6 index 776026172..e4c2cd6bc 100644 --- a/app/assets/javascripts/discourse/controllers/login.js.es6 +++ b/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'); diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 7cff29e95..6a1317cb7 100644 --- a/app/controllers/session_controller.rb +++ b/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" diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 8c9bf69c5..2e235935b 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/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 diff --git a/app/models/screened_ip_address.rb b/app/models/screened_ip_address.rb index d25540a85..3b05ce1e9 100644 --- a/app/models/screened_ip_address.rb +++ b/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 diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 4c5a08d02..bdb4f0d77 100644 --- a/config/locales/client.en.yml +++ b/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 {{sentTo}}. 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 {{currentEmail}}. It might take a few minutes for it to arrive; be sure to check your spam folder." google: diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index e037414cd..d5f28d346 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1228,7 +1228,8 @@ en: active: "Your account is activated and ready to use." activate_email: "

You're almost done! We sent an activation mail to %{email}. Please follow the instructions in the email to activate your account.

If it doesn't arrive, check your spam folder, or try to log in again to send another activation mail.

" 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}" diff --git a/lib/auth/result.rb b/lib/auth/result.rb index 505541ea4..6bb016c00 100644 --- a/lib/auth/result.rb +++ b/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 diff --git a/spec/models/screened_ip_address_spec.rb b/spec/models/screened_ip_address_spec.rb index c62021ad5..ef176fb2c 100644 --- a/spec/models/screened_ip_address_spec.rb +++ b/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