Fix broken admin login fro SSO enabled sites (#8737)

* When we refactored away the admin-login route we introduced a bug where admins could not log into an SSO enabled site, because of a check in the email_login route that disallowed this.
* Allow admin to get around this check.

app/controllers/session_controller.rb

@@ -6,7 +6,7 @@ class SessionController < ApplicationController
     render body: nil, status: 500
   end
 
-  before_action :check_local_login_allowed, only: %i(create forgot_password email_login email_login_info)
+  before_action :check_local_login_allowed, only: %i(create forgot_password)
   before_action :rate_limit_login, only: %i(create email_login)
   before_action :rate_limit_second_factor_totp, only: %i(create email_login)
   skip_before_action :redirect_to_login_if_required
@@ -303,11 +303,9 @@ class SessionController < ApplicationController
   def email_login_info
     token = params[:token]
     matched_token = EmailToken.confirmable(token)
+    user = matched_token&.user
 
-    if !SiteSetting.enable_local_logins_via_email &&
+    check_local_login_allowed(user: user, check_login_via_email: true)
-          !matched_token.user.admin? # admin-login uses this route, so allow them even if disabled
-      raise Discourse::NotFound
-    end
 
     if matched_token
       response = {
@@ -343,13 +341,10 @@ class SessionController < ApplicationController
   def email_login
     token = params[:token]
     matched_token = EmailToken.confirmable(token)
-
-    if !SiteSetting.enable_local_logins_via_email &&
-      !matched_token&.user&.admin? # admin-login uses this route, so allow them even if disabled
-      raise Discourse::NotFound
-    end
-
     user = matched_token&.user
+
+    check_local_login_allowed(user: user, check_login_via_email: true)
+
     if user.present? && !authenticate_second_factor(user)
       return render(json: @second_factor_failure_payload)
     end
@@ -436,8 +431,13 @@ class SessionController < ApplicationController
 
   protected
 
-  def check_local_login_allowed
+  def check_local_login_allowed(user: nil, check_login_via_email: false)
-    if SiteSetting.enable_sso || !SiteSetting.enable_local_logins
+    # admin-login can get around enabled SSO/disabled local logins
+    return if user&.admin?
+
+    if (check_login_via_email && !SiteSetting.enable_local_logins_via_email) ||
+        SiteSetting.enable_sso ||
+        !SiteSetting.enable_local_logins
       raise LocalLoginNotAllowed, "SSO takes over local login or the local login is disallowed."
     end
   end

spec/requests/session_controller_spec.rb

@@ -25,7 +25,23 @@ RSpec.describe SessionController do
 
       it "only works for admins" do
         get "/session/email-login/#{email_token.token}.json"
-        expect(response.status).to eq(404)
+        expect(response.status).to eq(500)
+
+        user.update(admin: true)
+        get "/session/email-login/#{email_token.token}.json"
+        expect(response.status).to eq(200)
+      end
+    end
+
+    context "when SSO enabled" do
+      before do
+        SiteSetting.sso_url = "https://www.example.com/sso"
+        SiteSetting.enable_sso = true
+      end
+
+      it "only works for admins" do
+        get "/session/email-login/#{email_token.token}.json"
+        expect(response.status).to eq(500)
 
         user.update(admin: true)
         get "/session/email-login/#{email_token.token}.json"
@@ -56,7 +72,7 @@ RSpec.describe SessionController do
 
         get "/session/email-login/#{email_token.token}.json"
 
-        expect(response.status).to eq(404)
+        expect(response.status).to eq(500)
       end
 
       it 'fails when local logins is disabled' do
@@ -111,7 +127,7 @@ RSpec.describe SessionController do
 
       it "only works for admins" do
         post "/session/email-login/#{email_token.token}.json"
-        expect(response.status).to eq(404)
+        expect(response.status).to eq(500)
 
         user.update(admin: true)
         post "/session/email-login/#{email_token.token}.json"
@@ -165,7 +181,7 @@ RSpec.describe SessionController do
 
         post "/session/email-login/#{email_token.token}.json"
 
-        expect(response.status).to eq(404)
+        expect(response.status).to eq(500)
         expect(session[:current_user_id]).to eq(nil)
       end
 
@@ -1118,6 +1134,21 @@ RSpec.describe SessionController do
       it_behaves_like "failed to continue local login"
     end
 
+    context 'local login via email is disabled' do
+      before do
+        SiteSetting.enable_local_logins_via_email = false
+      end
+      it 'doesnt matter, logs in correctly' do
+        events = DiscourseEvent.track_events do
+          post "/session.json", params: {
+            login: user.username, password: 'myawesomepassword'
+          }
+        end
+
+        expect(response.status).to eq(200)
+      end
+    end
+
     context 'when email is confirmed' do
       before do
         token = user.email_tokens.find_by(email: user.email)
@@ -1767,6 +1798,28 @@ RSpec.describe SessionController do
         it_behaves_like "failed to continue local login"
       end
 
+      context "local logins are disabled" do
+        before do
+          SiteSetting.enable_local_logins = false
+
+          post "/session.json", params: {
+            login: user.username, password: 'myawesomepassword'
+          }
+        end
+        it_behaves_like "failed to continue local login"
+      end
+
+      context "local logins via email are disabled" do
+        before do
+          SiteSetting.enable_local_logins_via_email = false
+        end
+        it "does not matter, generates a new token for a made up username" do
+          expect do
+            post "/session/forgot_password.json", params: { login: user.username }
+          end.to change(EmailToken, :count)
+        end
+      end
+
       it "generates a new token for a made up username" do
         expect do
           post "/session/forgot_password.json", params: { login: user.username }