diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 60d30691512..dfa417f880e 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -2,7 +2,12 @@ require_dependency 'rate_limiter' require_dependency 'single_sign_on' class SessionController < ApplicationController + class LocalLoginNotAllowed < StandardError; end + rescue_from LocalLoginNotAllowed do + render nothing: true, status: 500 + end + before_filter :check_local_login_allowed, only: %i(create forgot_password) skip_before_filter :redirect_to_login_if_required skip_before_filter :preload_json, :check_xhr, only: ['sso', 'sso_login', 'become', 'sso_provider', 'destroy'] @@ -176,12 +181,6 @@ class SessionController < ApplicationController end def create - - unless allow_local_auth? - render nothing: true, status: 500 - return - end - RateLimiter.new(nil, "login-hr-#{request.remote_ip}", SiteSetting.max_logins_per_ip_per_hour, 1.hour).performed! RateLimiter.new(nil, "login-min-#{request.remote_ip}", SiteSetting.max_logins_per_ip_per_minute, 1.minute).performed! @@ -234,11 +233,6 @@ class SessionController < ApplicationController def forgot_password params.require(:login) - unless allow_local_auth? - render nothing: true, status: 500 - return - end - RateLimiter.new(nil, "forgot-password-hr-#{request.remote_ip}", 6, 1.hour).performed! RateLimiter.new(nil, "forgot-password-min-#{request.remote_ip}", 3, 1.minute).performed! @@ -281,12 +275,16 @@ class SessionController < ApplicationController end end - private + protected - def allow_local_auth? - !SiteSetting.enable_sso && SiteSetting.enable_local_logins + def check_local_login_allowed + if SiteSetting.enable_sso || !SiteSetting.enable_local_logins + raise LocalLoginNotAllowed, "SSO takes over local login or the local login is disallowed." + end end + private + def login_not_approved_for?(user) SiteSetting.must_approve_users? && !user.approved? && !user.admin? end diff --git a/spec/controllers/session_controller_spec.rb b/spec/controllers/session_controller_spec.rb index 8dc4464e777..44ebb652de5 100644 --- a/spec/controllers/session_controller_spec.rb +++ b/spec/controllers/session_controller_spec.rb @@ -1,6 +1,12 @@ require 'rails_helper' describe SessionController do + shared_examples 'failed to continue local login' do + it 'should return the right response' do + expect(response).not_to be_success + expect(response.status.to_i).to eq 500 + end + end describe 'become' do let!(:user) { Fabricate(:user) } @@ -458,6 +464,22 @@ describe SessionController do let(:user) { Fabricate(:user) } + context 'local login is disabled' do + before do + SiteSetting.enable_local_logins = false + xhr :post, :create, login: user.username, password: 'myawesomepassword' + end + it_behaves_like "failed to continue local login" + end + + context 'SSO is enabled' do + before do + SiteSetting.enable_sso = true + xhr :post, :create, login: user.username, password: 'myawesomepassword' + end + it_behaves_like "failed to continue local login" + end + context 'when email is confirmed' do before do token = user.email_tokens.find_by(email: user.email) @@ -524,14 +546,6 @@ describe SessionController do end end - describe 'local logins disabled' do - it 'fails' do - SiteSetting.enable_local_logins = false - xhr :post, :create, login: user.username, password: 'myawesomepassword' - expect(response.status.to_i).to eq(500) - end - end - describe 'with a blocked IP' do before do screened_ip = Fabricate(:screened_ip_address) @@ -727,10 +741,20 @@ describe SessionController do context 'for an existing username' do let(:user) { Fabricate(:user) } - it "returns a 500 if local logins are disabled" do - SiteSetting.enable_local_logins = false - xhr :post, :forgot_password, login: user.username - expect(response.code.to_i).to eq(500) + context 'local login is disabled' do + before do + SiteSetting.enable_local_logins = false + xhr :post, :forgot_password, login: user.username + end + it_behaves_like "failed to continue local login" + end + + context 'SSO is enabled' do + before do + SiteSetting.enable_sso = true + xhr :post, :create, login: user.username, password: 'myawesomepassword' + end + it_behaves_like "failed to continue local login" end it "generates a new token for a made up username" do