Fix infinite loop when SAML was incorrectly configured.fix-authhash-infinite-loop

3 files changed, 31 insertions, 20 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 7c7f448b9be..0e7c23ea389 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -59,6 +59,7 @@ v 7.14.0 (unreleased)
   - Set max-width for README, issue and merge request description for easier read on big screens
   - Update Flowdock integration to support new Flowdock API (Boyan Tabakov)
   - Remove author from files view (Sven Strickroth)
+  - Fix infinite loop when SAML was incorrectly configured.
 
 v 7.13.5
   - Satellites reverted
diff --git a/lib/gitlab/o_auth/auth_hash.rb b/lib/gitlab/o_auth/auth_hash.rb
index 0f16c925900..9b8e783d16c 100644
--- a/lib/gitlab/o_auth/auth_hash.rb
+++ b/lib/gitlab/o_auth/auth_hash.rb
@@ -9,49 +9,63 @@ module Gitlab
       end
 
       def uid
-        Gitlab::Utils.force_utf8(auth_hash.uid.to_s)
+        @uid ||= Gitlab::Utils.force_utf8(auth_hash.uid.to_s)
       end
 
       def provider
-        Gitlab::Utils.force_utf8(auth_hash.provider.to_s)
+        @provider ||= Gitlab::Utils.force_utf8(auth_hash.provider.to_s)
       end
 
       def info
         auth_hash.info
       end
 
-      def name
-        Gitlab::Utils.force_utf8((info.try(:name) || full_name).to_s)
+      def get_info(key)
+        value = info.try(key)
+        Gitlab::Utils.force_utf8(value) if value
+        value
       end
 
-      def full_name
-        Gitlab::Utils.force_utf8("#{info.first_name} #{info.last_name}")
+      def name
+        @name ||= get_info(:name) || "#{get_info(:first_name)} #{get_info(:last_name)}"
       end
 
       def username
-        Gitlab::Utils.force_utf8(
-          (info.try(:nickname) || generate_username).to_s
-        )
+        @username ||= username_and_email[:username].to_s
       end
 
       def email
-        Gitlab::Utils.force_utf8(
-          (info.try(:email) || generate_temporarily_email).downcase
-        )
+        @email ||= username_and_email[:email].to_s
       end
 
       def password
-        devise_friendly_token = Devise.friendly_token[0, 8].downcase
-        @password ||= Gitlab::Utils.force_utf8(devise_friendly_token)
+        @password ||= Gitlab::Utils.force_utf8(Devise.friendly_token[0, 8].downcase)
+      end
+
+      private
+
+      def username_and_email
+        @username_and_email ||= begin
+          username  = get_info(:nickname) || get_info(:username)
+          email     = get_info(:email)
+
+          username ||= generate_username(email)             if email
+          email    ||= generate_temporarily_email(username) if username
+
+          {
+            username: username,
+            email:    email
+          }
+        end
       end
 
       # Get the first part of the email address (before @)
       # In addtion in removes illegal characters
-      def generate_username
+      def generate_username(email)
         email.match(/^[^@]*/)[0].parameterize
       end
 
-      def generate_temporarily_email
+      def generate_temporarily_email(username)
         "temp-email-for-oauth-#{username}@gitlab.localhost"
       end
     end
diff --git a/spec/lib/gitlab/o_auth/auth_hash_spec.rb b/spec/lib/gitlab/o_auth/auth_hash_spec.rb
index 4c0a4a49d2a..e4a6cd954cc 100644
--- a/spec/lib/gitlab/o_auth/auth_hash_spec.rb
+++ b/spec/lib/gitlab/o_auth/auth_hash_spec.rb
@@ -91,10 +91,6 @@ describe Gitlab::OAuth::AuthHash do
       expect(auth_hash.name.encoding).to eql Encoding::UTF_8
     end
 
-    it 'forces utf8 encoding on full_name' do
-      expect(auth_hash.full_name.encoding).to eql Encoding::UTF_8
-    end
-
     it 'forces utf8 encoding on username' do
       expect(auth_hash.username.encoding).to eql Encoding::UTF_8
     end