DEV: Respond with 403 instead of 500 for disabled local login via email

Previously if local login via email was disabled because of the site setting or because SSO was enabled, we were raising a 500 error. We now raise a 403 error instead; we shouldn't raise 500 errors on purpose, instead keeping that code for unhandled errors. It doesn't make sense in the context of what we are validating either to raise a 500.

app/controllers/session_controller.rb

@@ -1,11 +1,6 @@
 # frozen_string_literal: true
 
 class SessionController < ApplicationController
-  class LocalLoginNotAllowed < StandardError; end
-  rescue_from LocalLoginNotAllowed do
-    render body: nil, status: 500
-  end
-
   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)
@@ -438,7 +433,7 @@ class SessionController < ApplicationController
     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."
+      raise Discourse::InvalidAccess, "SSO takes over local login or the local login is disallowed."
     end
   end
 

spec/requests/session_controller_spec.rb

@@ -11,7 +11,7 @@ RSpec.describe SessionController do
   shared_examples 'failed to continue local login' do
     it 'should return the right response' do
       expect(response).not_to be_successful
-      expect(response.status).to eq(500)
+      expect(response.status).to eq(403)
     end
   end
 
@@ -25,7 +25,7 @@ RSpec.describe SessionController do
 
       it "only works for admins" do
         get "/session/email-login/#{email_token.token}.json"
-        expect(response.status).to eq(500)
+        expect(response.status).to eq(403)
 
         user.update(admin: true)
         get "/session/email-login/#{email_token.token}.json"
@@ -41,7 +41,7 @@ RSpec.describe SessionController do
 
       it "only works for admins" do
         get "/session/email-login/#{email_token.token}.json"
-        expect(response.status).to eq(500)
+        expect(response.status).to eq(403)
 
         user.update(admin: true)
         get "/session/email-login/#{email_token.token}.json"
@@ -72,7 +72,7 @@ RSpec.describe SessionController do
 
         get "/session/email-login/#{email_token.token}.json"
 
-        expect(response.status).to eq(500)
+        expect(response.status).to eq(403)
       end
 
       it 'fails when local logins is disabled' do
@@ -80,7 +80,7 @@ RSpec.describe SessionController do
 
         get "/session/email-login/#{email_token.token}.json"
 
-        expect(response.status).to eq(500)
+        expect(response.status).to eq(403)
       end
 
       context 'user has 2-factor logins' do
@@ -127,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(500)
+        expect(response.status).to eq(403)
 
         user.update(admin: true)
         post "/session/email-login/#{email_token.token}.json"
@@ -181,7 +181,7 @@ RSpec.describe SessionController do
 
         post "/session/email-login/#{email_token.token}.json"
 
-        expect(response.status).to eq(500)
+        expect(response.status).to eq(403)
         expect(session[:current_user_id]).to eq(nil)
       end
 
@@ -190,7 +190,7 @@ RSpec.describe SessionController do
 
         post "/session/email-login/#{email_token.token}.json"
 
-        expect(response.status).to eq(500)
+        expect(response.status).to eq(403)
         expect(session[:current_user_id]).to eq(nil)
       end