FIX: If an IP is blocked, don't allow people to login using it

app/controllers/session_controller.rb

@@ -52,14 +52,16 @@ class SessionController < ApplicationController
 
   def sso_login
     unless SiteSetting.enable_sso
-      render nothing: true, status: 404
+      return render(nothing: true, status: 404)
-      return
     end
 
     sso = DiscourseSingleSignOn.parse(request.query_string)
     if !sso.nonce_valid?
-      render text: I18n.t("sso.timeout_expired"), status: 500
+      return render(text: I18n.t("sso.timeout_expired"), status: 500)
-      return
+    end
+
+    if ScreenedIpAddress.should_block?(request.remote_ip)
+      return render(text: I18n.t("sso.unknown_error"), status: 500)
     end
 
     return_path = sso.return_path
@@ -145,7 +147,8 @@ class SessionController < ApplicationController
       return
     end
 
-    if ScreenedIpAddress.block_login?(user, request.remote_ip)
+    if ScreenedIpAddress.block_login?(user, request.remote_ip) ||
+       ScreenedIpAddress.should_block?(request.remote_ip)
       return not_allowed_from_ip_address(user)
     end
 

spec/controllers/session_controller_spec.rb

@@ -67,21 +67,39 @@ describe SessionController do
       expect(logged_on_user.single_sign_on_record.external_username).to eq('sam')
     end
 
-    it 'respects IP restrictions' do
+    def sso_for_ip_specs
       sso = get_sso('/a/')
       sso.external_id = '666' # the number of the beast
       sso.email = 'bob@bob.com'
       sso.name = 'Sam Saffron'
       sso.username = 'sam'
+      sso
+    end
 
+    it 'respects IP restrictions on create' do
       screened_ip = Fabricate(:screened_ip_address)
       ActionDispatch::Request.any_instance.stubs(:remote_ip).returns(screened_ip.ip_address)
+
+      sso = sso_for_ip_specs
       get :sso_login, Rack::Utils.parse_query(sso.payload)
 
       logged_on_user = Discourse.current_user_provider.new(request.env).current_user
       expect(logged_on_user).to eq(nil)
     end
 
+    it 'respects IP restrictions on login' do
+      sso = sso_for_ip_specs
+      user = DiscourseSingleSignOn.parse(sso.payload).lookup_or_create_user(request.remote_ip)
+
+      sso = sso_for_ip_specs
+      screened_ip = Fabricate(:screened_ip_address)
+      ActionDispatch::Request.any_instance.stubs(:remote_ip).returns(screened_ip.ip_address)
+
+      get :sso_login, Rack::Utils.parse_query(sso.payload)
+      logged_on_user = Discourse.current_user_provider.new(request.env).current_user
+      expect(logged_on_user).to be_blank
+    end
+
     it 'respects email restrictions' do
       sso = get_sso('/a/')
       sso.external_id = '666' # the number of the beast
@@ -367,6 +385,19 @@ describe SessionController do
         end
       end
 
+      describe 'with a blocked IP' do
+        before do
+          screened_ip = Fabricate(:screened_ip_address)
+          ActionDispatch::Request.any_instance.stubs(:remote_ip).returns(screened_ip.ip_address)
+          xhr :post, :create, login: "@" + user.username, password: 'myawesomepassword'
+          user.reload
+        end
+
+        it "doesn't log in" do
+          expect(session[:current_user_id]).to be_nil
+        end
+      end
+
       describe 'strips leading @ symbol' do
         before do
           xhr :post, :create, login: "@" + user.username, password: 'myawesomepassword'