From 939180efa87da64bd3201d5a6a4355dc4ec2c464 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Fri, 2 Mar 2018 10:37:13 +0800
Subject: [PATCH] FIX: Missing 2FA guards when sso is enabled or when local
 login is disabled.

---
 app/controllers/users_controller.rb           |  1 +
 app/models/concerns/second_factor_manager.rb  |  4 ++-
 .../concern/second_factor_manager_spec.rb     | 17 +++++++++++++
 spec/requests/users_controller_spec.rb        | 25 +++++++++++++++++++
 4 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 0633c93ae88..d8c0ab70437 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -952,6 +952,7 @@ class UsersController < ApplicationController
   end
 
   def create_second_factor
+    raise Discourse::NotFound if SiteSetting.enable_sso || !SiteSetting.enable_local_logins
     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!
 
diff --git a/app/models/concerns/second_factor_manager.rb b/app/models/concerns/second_factor_manager.rb
index a9a04cd390c..096fc003057 100644
--- a/app/models/concerns/second_factor_manager.rb
+++ b/app/models/concerns/second_factor_manager.rb
@@ -33,6 +33,8 @@ module SecondFactorManager
   end
 
   def totp_enabled?
-    !!(self&.user_second_factor&.enabled?)
+    !!(self&.user_second_factor&.enabled?) &&
+      !SiteSetting.enable_sso &&
+      SiteSetting.enable_local_logins
   end
 end
diff --git a/spec/components/concern/second_factor_manager_spec.rb b/spec/components/concern/second_factor_manager_spec.rb
index afc969d915e..ccecf68da57 100644
--- a/spec/components/concern/second_factor_manager_spec.rb
+++ b/spec/components/concern/second_factor_manager_spec.rb
@@ -89,5 +89,22 @@ RSpec.describe SecondFactorManager do
         expect(user.totp_enabled?).to eq(true)
       end
     end
+
+    describe 'when SSO is enabled' do
+      it 'should return false' do
+        SiteSetting.sso_url = 'http://someurl.com'
+        SiteSetting.enable_sso = true
+
+        expect(user.totp_enabled?).to eq(false)
+      end
+    end
+
+    describe 'when local login is disabled' do
+      it 'should return false' do
+        SiteSetting.enable_local_logins = false
+
+        expect(user.totp_enabled?).to eq(false)
+      end
+    end
   end
 end
diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb
index cd2628ba3c7..c49a227a9a8 100644
--- a/spec/requests/users_controller_spec.rb
+++ b/spec/requests/users_controller_spec.rb
@@ -431,6 +431,31 @@ RSpec.describe UsersController do
           )
         end
 
+        describe 'when local logins are disabled' do
+          it 'should return the right response' do
+            SiteSetting.enable_local_logins = false
+
+            post "/users/second_factors.json", params: {
+              password: 'somecomplicatedpassword'
+            }
+
+            expect(response.status).to eq(404)
+          end
+        end
+
+        describe 'when SSO is enabled' do
+          it 'should return the right response' do
+            SiteSetting.sso_url = 'http://someurl.com'
+            SiteSetting.enable_sso = true
+
+            post "/users/second_factors.json", params: {
+              password: 'somecomplicatedpassword'
+            }
+
+            expect(response.status).to eq(404)
+          end
+        end
+
         it 'succeeds on correct password' do
           post "/users/second_factors.json", params: {
             password: 'somecomplicatedpassword'