Merge branch 'fix-missing-saml-error-handling' into 'master'

Add proper nil and error handling to SAML login process

While writing the feature that would allow certain Omniauth providers to be marked as external I noticed that there is a scenario where the `gl_user` method can return `nil` and if this is not properly checked, it will lead to exceptions that will cause 500 errors. It is quite easy to land in this scenario, so I added `nil` checks.

I also noticed that the `saml` method in the `omniauth_callbacks_controller.rb` file lacked a `rescue` for `Gitlab::OAuth::SignupDisabledError`, which can happen if the default configuration from `1_settings.rb` is applied. So I also added this check.

See merge request !3609

1 files changed, 2 insertions, 1 deletions

diff --git a/lib/gitlab/saml/user.rb b/lib/gitlab/saml/user.rb
index c1072452abe..dba4bbfc899 100644
--- a/lib/gitlab/saml/user.rb
+++ b/lib/gitlab/saml/user.rb
@@ -26,7 +26,7 @@ module Gitlab
           @user ||= build_new_user
         end
 
-        if external_users_enabled?
+        if external_users_enabled? && @user
           # Check if there is overlap between the user's groups and the external groups
           # setting then set user as external or internal.
           if (auth_hash.groups & Gitlab::Saml::Config.external_groups).empty?
@@ -48,6 +48,7 @@ module Gitlab
       end
 
       def changed?
+        return true unless gl_user
         gl_user.changed? || gl_user.identities.any?(&:changed?)
       end