From 1d2429af9b0fd4ef1427c7676a50dae4e2cf0ff9 Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Thu, 7 Apr 2016 16:45:33 -0500
Subject: [PATCH] Add missing proper nil and error handling to SAML login
 process.

---
 .../omniauth_callbacks_controller.rb          | 26 ++++++++++++-------
 lib/gitlab/saml/user.rb                       | 22 ++++++++++------
 2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index d28e96c3f18..df98f56a1cd 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -60,6 +60,8 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
 
       continue_login_process
     end
+  rescue Gitlab::OAuth::SignupDisabledError
+    handle_signup_error
   end
 
   def omniauth_error
@@ -92,16 +94,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
       continue_login_process
     end
   rescue Gitlab::OAuth::SignupDisabledError
-    label = Gitlab::OAuth::Provider.label_for(oauth['provider'])
-    message = "Signing in using your #{label} account without a pre-existing GitLab account is not allowed."
-
-    if current_application_settings.signup_enabled?
-      message << " Create a GitLab account first, and then connect it to your #{label} account."
-    end
-
-    flash[:notice] = message
-
-    redirect_to new_user_session_path
+    handle_signup_error
   end
 
   def handle_service_ticket provider, ticket
@@ -122,6 +115,19 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     end
   end
 
+  def handle_signup_error
+    label = Gitlab::OAuth::Provider.label_for(oauth['provider'])
+    message = "Signing in using your #{label} account without a pre-existing GitLab account is not allowed."
+
+    if current_application_settings.signup_enabled?
+      message << " Create a GitLab account first, and then connect it to your #{label} account."
+    end
+
+    flash[:notice] = message
+
+    redirect_to new_user_session_path
+  end
+
   def oauth
     @oauth ||= request.env['omniauth.auth']
   end
diff --git a/lib/gitlab/saml/user.rb b/lib/gitlab/saml/user.rb
index c1072452abe..dd77216be48 100644
--- a/lib/gitlab/saml/user.rb
+++ b/lib/gitlab/saml/user.rb
@@ -26,13 +26,15 @@ module Gitlab
           @user ||= build_new_user
         end
 
-        if external_users_enabled?
-          # 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?
-            @user.external = false
-          else
-            @user.external = true
+        unless @user.nil?
+          if external_users_enabled?
+            # 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?
+              @user.external = false
+            else
+              @user.external = true
+            end
           end
         end
 
@@ -48,7 +50,11 @@ module Gitlab
       end
 
       def changed?
-        gl_user.changed? || gl_user.identities.any?(&:changed?)
+        if gl_user
+          gl_user.changed? || gl_user.identities.any?(&:changed?)
+        else
+          true
+        end
       end
 
       protected