From 62fc80642dbdb6a3b840a770fde15b84b8495a03 Mon Sep 17 00:00:00 2001
From: Jan-Willem van der Meer <mail@jewilmeer.nl>
Date: Wed, 3 Sep 2014 15:59:50 +0200
Subject: [PATCH 1/9] Refactor Oauth::User class to use instance methods

---
 app/models/user.rb       |   4 --
 lib/gitlab/ldap/user.rb  |   4 ++
 lib/gitlab/oauth/user.rb | 150 +++++++++++++++++++++------------------
 3 files changed, 83 insertions(+), 75 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index f1ff76edd15..15e56a62a68 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -474,10 +474,6 @@ class User < ActiveRecord::Base
     email =~ /\Atemp-email-for-oauth/
   end
 
-  def generate_tmp_oauth_email
-    self.email = "temp-email-for-oauth-#{username}@gitlab.localhost"
-  end
-
   def public_profile?
     authorized_projects.public_only.any?
   end
diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb
index e6aa3890992..f15d723a063 100644
--- a/lib/gitlab/ldap/user.rb
+++ b/lib/gitlab/ldap/user.rb
@@ -68,6 +68,10 @@ module Gitlab
 
         private
 
+        def needs_blocking?
+          false
+        end
+
         def find_by_uid_and_provider
           find_by_uid(uid)
         end
diff --git a/lib/gitlab/oauth/user.rb b/lib/gitlab/oauth/user.rb
index 9670aad2c5d..f30aec36445 100644
--- a/lib/gitlab/oauth/user.rb
+++ b/lib/gitlab/oauth/user.rb
@@ -16,97 +16,105 @@ module Gitlab
 
         def create(auth)
           @auth = auth
-          password = Devise.friendly_token[0, 8].downcase
-          opts = {
-            extern_uid: uid,
-            provider: provider,
-            name: name,
-            username: username,
-            email: email,
-            password: password,
-            password_confirmation: password,
-          }
-
-          user = model.build_user(opts)
-          user.skip_confirmation!
-
-          # Services like twitter and github does not return email via oauth
-          # In this case we generate temporary email and force user to fill it later
-          if user.email.blank?
-            user.generate_tmp_oauth_email
-          elsif provider != "ldap"
-            # Google oauth returns email but dont return nickname
-            # So we use part of email as username for new user
-            # For LDAP, username is already set to the user's
-            # uid/userid/sAMAccountName.
-            email_username = email.match(/^[^@]*/)[0]
-            # Strip apostrophes since they are disallowed as part of username
-            user.username = email_username.gsub("'", "")
-          end
-
-          begin
-            user.save!
-          rescue ActiveRecord::RecordInvalid => e
-            log.info "(OAuth) Email #{e.record.errors[:email]}. Username #{e.record.errors[:username]}"
-            return nil, e.record.errors
-          end
+          user = new(auth).user
 
+          user.save!
           log.info "(OAuth) Creating user #{email} from login with extern_uid => #{uid}"
-
-          if Gitlab.config.omniauth['block_auto_created_users'] && !ldap?
-            user.block
-          end
+          user.block if needs_blocking?
 
           user
+        rescue ActiveRecord::RecordInvalid => e
+          log.info "(OAuth) Email #{e.record.errors[:email]}. Username #{e.record.errors[:username]}"
+          return nil, e.record.errors
         end
 
         private
 
         def find_by_uid_and_provider
-          model.where(provider: provider, extern_uid: uid).last
-        end
-
-        def uid
-          auth.uid.to_s
-        end
-
-        def email
-          return unless auth.info.respond_to?(:email)
-          auth.info.email.downcase unless auth.info.email.nil?
-        end
-
-        def name
-          if auth.info.name.nil?
-            "#{auth.info.first_name} #{auth.info.last_name}".force_encoding('utf-8')
-          else
-            auth.info.name.to_s.force_encoding('utf-8')
-          end
-        end
-
-        def username
-          return unless auth.info.respond_to?(:nickname)
-          auth.info.nickname.to_s.force_encoding("utf-8")
+          ::User.where(provider: provider, extern_uid: uid).last
         end
 
         def provider
           auth.provider
         end
 
-        def log
-          Gitlab::AppLogger
+        def uid
+          auth.uid.to_s
         end
 
-        def model
-          ::User
+        def needs_blocking?
+          Gitlab.config.omniauth['block_auto_created_users']
         end
+      end
 
-        def raise_error(message)
-          raise OmniAuth::Error, "(OAuth) " + message
-        end
+      attr_accessor :auth, :user
 
-        def ldap?
-          provider == 'ldap'
-        end
+      def initialize(auth)
+        self.auth = auth
+        self.user = ::User.new(user_attributes)
+        user.skip_confirmation!
+      end
+
+      def user_attributes
+        {
+          extern_uid: uid,
+          provider: provider,
+          name: name,
+          username: username,
+          email: email,
+          password: password,
+          password_confirmation: password,
+        }
+      end
+
+      def uid
+        auth.uid.to_s
+      end
+
+      def provider
+        auth.provider
+      end
+
+      def info
+        auth.info
+      end
+
+      def name
+        (info.name || full_name).to_s.force_encoding('utf-8')
+      end
+
+      def full_name
+        "#{info.first_name} #{info.last_name}"
+      end
+
+      def username
+        (info.try(:nickname) || generate_username).to_s.force_encoding('utf-8')
+      end
+
+      def email
+        (info.try(:email) || generate_temporarily_email).downcase
+      end
+
+      def password
+        @password ||= Devise.friendly_token[0, 8].downcase
+      end
+
+      def log
+        Gitlab::AppLogger
+      end
+
+      def raise_error(message)
+        raise OmniAuth::Error, "(OAuth) " + message
+      end
+
+      # Get the first part of the email address (before @)
+      # In addtion in removes illegal characters
+      def generate_username
+        email.match(/^[^@]*/)[0].parameterize
+      end
+
+      def generate_temporarily_email
+        "temp-email-for-oauth-#{username}@gitlab.localhost"
       end
     end
   end

From 1bd15fa717e053824e3abbf72e4010f19677fb79 Mon Sep 17 00:00:00 2001
From: Jan-Willem van der Meer <mail@jewilmeer.nl>
Date: Wed, 3 Sep 2014 17:33:03 +0200
Subject: [PATCH 2/9] Use instance methods of LDAP::User as well

Still in need of some proper cleanup
---
 lib/gitlab/ldap/user.rb  | 57 ++++++++++++++++++----------------------
 lib/gitlab/oauth/user.rb | 45 +++++++++++++++++--------------
 2 files changed, 50 insertions(+), 52 deletions(-)

diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb
index f15d723a063..99f23080a84 100644
--- a/lib/gitlab/ldap/user.rb
+++ b/lib/gitlab/ldap/user.rb
@@ -11,36 +11,25 @@ module Gitlab
     class User < Gitlab::OAuth::User
       class << self
         def find_or_create(auth)
-          @auth = auth
+          self.auth = auth
+          find(auth) || create(auth)
+        end
 
-          if uid.blank? || email.blank? || username.blank?
-            raise_error("Account must provide a dn, uid and email address")
+        # overloaded from Gitlab::Oauth::User
+        # TODO: it's messy, needs cleanup, less complexity
+        def create(auth)
+          ldap_user = new(auth)
+          # first try to find the user based on the returned email address
+          user = ldap_user.find_gitlab_user_by_email
+
+          if user
+            user.update_attributes(extern_uid: ldap_user.uid, provider: ldap_user.provider)
+            Gitlab::AppLogger.info("(LDAP) Updating legacy LDAP user #{ldap_user.email} with extern_uid => #{ldap_user.uid}")
+            return user
           end
 
-          user = find(auth)
-
-          unless user
-            # Look for user with same emails
-            #
-            # Possible cases:
-            # * When user already has account and need to link their LDAP account.
-            # * LDAP uid changed for user with same email and we need to update their uid
-            #
-            user = model.find_by(email: email)
-
-            if user
-              user.update_attributes(extern_uid: uid, provider: provider)
-              log.info("(LDAP) Updating legacy LDAP user #{email} with extern_uid => #{uid}")
-            else
-              # Create a new user inside GitLab database
-              # based on LDAP credentials
-              #
-              #
-              user = create(auth)
-            end
-          end
-
-          user
+          # if the user isn't found by an exact email match, use oauth methods
+          ldap_user.save_and_trigger_callbacks
         end
 
         def authenticate(login, password)
@@ -66,11 +55,7 @@ module Gitlab
           find_by_uid(ldap_user.dn) if ldap_user
         end
 
-        private
-
-        def needs_blocking?
-          false
-        end
+        protected
 
         def find_by_uid_and_provider
           find_by_uid(uid)
@@ -93,6 +78,14 @@ module Gitlab
           Gitlab.config.ldap
         end
       end
+
+      def find_gitlab_user_by_email
+        self.class.model.find_by(email: email)
+      end
+
+      def needs_blocking?
+        false
+      end
     end
   end
 end
diff --git a/lib/gitlab/oauth/user.rb b/lib/gitlab/oauth/user.rb
index f30aec36445..8ac040e3365 100644
--- a/lib/gitlab/oauth/user.rb
+++ b/lib/gitlab/oauth/user.rb
@@ -7,31 +7,25 @@ module Gitlab
   module OAuth
     class User
       class << self
-        attr_reader :auth
+        attr_accessor :auth
 
         def find(auth)
-          @auth = auth
+          self.auth = auth
           find_by_uid_and_provider
         end
 
         def create(auth)
-          @auth = auth
-          user = new(auth).user
-
-          user.save!
-          log.info "(OAuth) Creating user #{email} from login with extern_uid => #{uid}"
-          user.block if needs_blocking?
-
-          user
-        rescue ActiveRecord::RecordInvalid => e
-          log.info "(OAuth) Email #{e.record.errors[:email]}. Username #{e.record.errors[:username]}"
-          return nil, e.record.errors
+          user = new(auth)
+          user.save_and_trigger_callbacks
         end
 
-        private
+        def model
+          ::User
+        end
 
+        protected
         def find_by_uid_and_provider
-          ::User.where(provider: provider, extern_uid: uid).last
+          model.where(provider: provider, extern_uid: uid).last
         end
 
         def provider
@@ -41,20 +35,27 @@ module Gitlab
         def uid
           auth.uid.to_s
         end
-
-        def needs_blocking?
-          Gitlab.config.omniauth['block_auto_created_users']
-        end
       end
 
       attr_accessor :auth, :user
 
       def initialize(auth)
         self.auth = auth
-        self.user = ::User.new(user_attributes)
+        self.user = self.class.model.new(user_attributes)
         user.skip_confirmation!
       end
 
+      def save_and_trigger_callbacks
+        user.save!
+        log.info "(OAuth) Creating user #{email} from login with extern_uid => #{uid}"
+        user.block if needs_blocking?
+
+        user
+      rescue ActiveRecord::RecordInvalid => e
+        log.info "(OAuth) Email #{e.record.errors[:email]}. Username #{e.record.errors[:username]}"
+        return nil, e.record.errors
+      end
+
       def user_attributes
         {
           extern_uid: uid,
@@ -116,6 +117,10 @@ module Gitlab
       def generate_temporarily_email
         "temp-email-for-oauth-#{username}@gitlab.localhost"
       end
+
+      def needs_blocking?
+        Gitlab.config.omniauth['block_auto_created_users']
+      end
     end
   end
 end

From 5b86dab03bbd19d9a3e6090d4672d8f1493e6fe5 Mon Sep 17 00:00:00 2001
From: Jan-Willem van der Meer <mail@jewilmeer.nl>
Date: Thu, 4 Sep 2014 12:55:10 +0200
Subject: [PATCH 3/9] Move auth hash to a seperate class

---
 lib/gitlab/ldap/user.rb           | 34 +++++------
 lib/gitlab/oauth/user.rb          | 93 +++++++++----------------------
 spec/lib/gitlab/ldap/user_spec.rb |  8 +--
 3 files changed, 47 insertions(+), 88 deletions(-)

diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb
index 99f23080a84..6d1bec5f54a 100644
--- a/lib/gitlab/ldap/user.rb
+++ b/lib/gitlab/ldap/user.rb
@@ -10,23 +10,27 @@ module Gitlab
   module LDAP
     class User < Gitlab::OAuth::User
       class << self
-        def find_or_create(auth)
-          self.auth = auth
-          find(auth) || create(auth)
+        def find_or_create(auth_hash)
+          self.auth_hash = auth_hash
+          find(auth_hash) || find_and_connect_by_email(auth_hash) || create(auth_hash)
+        end
+
+        def find_and_connect_by_email(auth_hash)
+          self.auth_hash = auth_hash
+          user = model.find_by(email: self.auth_hash.email)
+
+          if user
+            user.update_attributes(extern_uid: auth_hash.uid, provider: auth_hash.provider)
+            Gitlab::AppLogger.info("(LDAP) Updating legacy LDAP user #{self.auth_hash.email} with extern_uid => #{auth_hash.uid}")
+            return user
+          end
         end
 
         # overloaded from Gitlab::Oauth::User
         # TODO: it's messy, needs cleanup, less complexity
-        def create(auth)
-          ldap_user = new(auth)
+        def create(auth_hash)
+          ldap_user = new(auth_hash)
           # first try to find the user based on the returned email address
-          user = ldap_user.find_gitlab_user_by_email
-
-          if user
-            user.update_attributes(extern_uid: ldap_user.uid, provider: ldap_user.provider)
-            Gitlab::AppLogger.info("(LDAP) Updating legacy LDAP user #{ldap_user.email} with extern_uid => #{ldap_user.uid}")
-            return user
-          end
 
           # if the user isn't found by an exact email match, use oauth methods
           ldap_user.save_and_trigger_callbacks
@@ -58,7 +62,7 @@ module Gitlab
         protected
 
         def find_by_uid_and_provider
-          find_by_uid(uid)
+          find_by_uid(auth_hash.uid)
         end
 
         def find_by_uid(uid)
@@ -79,10 +83,6 @@ module Gitlab
         end
       end
 
-      def find_gitlab_user_by_email
-        self.class.model.find_by(email: email)
-      end
-
       def needs_blocking?
         false
       end
diff --git a/lib/gitlab/oauth/user.rb b/lib/gitlab/oauth/user.rb
index 8ac040e3365..b768eda185f 100644
--- a/lib/gitlab/oauth/user.rb
+++ b/lib/gitlab/oauth/user.rb
@@ -7,15 +7,15 @@ module Gitlab
   module OAuth
     class User
       class << self
-        attr_accessor :auth
+        attr_reader :auth_hash
 
-        def find(auth)
-          self.auth = auth
+        def find(auth_hash)
+          self.auth_hash = auth_hash
           find_by_uid_and_provider
         end
 
-        def create(auth)
-          user = new(auth)
+        def create(auth_hash)
+          user = new(auth_hash)
           user.save_and_trigger_callbacks
         end
 
@@ -23,31 +23,32 @@ module Gitlab
           ::User
         end
 
+        def auth_hash=(auth_hash)
+          @auth_hash = AuthHash.new(auth_hash)
+        end
+
         protected
         def find_by_uid_and_provider
-          model.where(provider: provider, extern_uid: uid).last
-        end
-
-        def provider
-          auth.provider
-        end
-
-        def uid
-          auth.uid.to_s
+          model.where(provider: auth_hash.provider, extern_uid: auth_hash.uid).last
         end
       end
 
-      attr_accessor :auth, :user
+      # Instance methods
+      attr_accessor :auth_hash, :user
 
-      def initialize(auth)
-        self.auth = auth
+      def initialize(auth_hash)
+        self.auth_hash = auth_hash
         self.user = self.class.model.new(user_attributes)
         user.skip_confirmation!
       end
 
+      def auth_hash=(auth_hash)
+        @auth_hash = AuthHash.new(auth_hash)
+      end
+
       def save_and_trigger_callbacks
         user.save!
-        log.info "(OAuth) Creating user #{email} from login with extern_uid => #{uid}"
+        log.info "(OAuth) Creating user #{auth_hash.email} from login with extern_uid => #{auth_hash.uid}"
         user.block if needs_blocking?
 
         user
@@ -58,48 +59,16 @@ module Gitlab
 
       def user_attributes
         {
-          extern_uid: uid,
-          provider: provider,
-          name: name,
-          username: username,
-          email: email,
-          password: password,
-          password_confirmation: password,
+          extern_uid: auth_hash.uid,
+          provider: auth_hash.provider,
+          name: auth_hash.name,
+          username: auth_hash.username,
+          email: auth_hash.email,
+          password: auth_hash.password,
+          password_confirmation: auth_hash.password,
         }
       end
 
-      def uid
-        auth.uid.to_s
-      end
-
-      def provider
-        auth.provider
-      end
-
-      def info
-        auth.info
-      end
-
-      def name
-        (info.name || full_name).to_s.force_encoding('utf-8')
-      end
-
-      def full_name
-        "#{info.first_name} #{info.last_name}"
-      end
-
-      def username
-        (info.try(:nickname) || generate_username).to_s.force_encoding('utf-8')
-      end
-
-      def email
-        (info.try(:email) || generate_temporarily_email).downcase
-      end
-
-      def password
-        @password ||= Devise.friendly_token[0, 8].downcase
-      end
-
       def log
         Gitlab::AppLogger
       end
@@ -108,16 +77,6 @@ module Gitlab
         raise OmniAuth::Error, "(OAuth) " + message
       end
 
-      # Get the first part of the email address (before @)
-      # In addtion in removes illegal characters
-      def generate_username
-        email.match(/^[^@]*/)[0].parameterize
-      end
-
-      def generate_temporarily_email
-        "temp-email-for-oauth-#{username}@gitlab.localhost"
-      end
-
       def needs_blocking?
         Gitlab.config.omniauth['block_auto_created_users']
       end
diff --git a/spec/lib/gitlab/ldap/user_spec.rb b/spec/lib/gitlab/ldap/user_spec.rb
index 725338965be..4ddf6b3039f 100644
--- a/spec/lib/gitlab/ldap/user_spec.rb
+++ b/spec/lib/gitlab/ldap/user_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::LDAP::User do
-  let(:gl_auth) { Gitlab::LDAP::User }
+  let(:gl_user) { Gitlab::LDAP::User }
   let(:info) do
     double(
       name: 'John',
@@ -19,12 +19,12 @@ describe Gitlab::LDAP::User do
     it "finds the user if already existing" do
       existing_user = create(:user, extern_uid: 'my-uid', provider: 'ldap')
 
-      expect{ gl_auth.find_or_create(auth) }.to_not change{ User.count }
+      expect{ gl_user.find_or_create(auth) }.to_not change{ User.count }
     end
 
     it "connects to existing non-ldap user if the email matches" do
       existing_user = create(:user, email: 'john@example.com')
-      expect{ gl_auth.find_or_create(auth) }.to_not change{ User.count }
+      expect{ gl_user.find_or_create(auth) }.to_not change{ User.count }
 
       existing_user.reload
       expect(existing_user.extern_uid).to eql 'my-uid'
@@ -32,7 +32,7 @@ describe Gitlab::LDAP::User do
     end
 
     it "creates a new user if not found" do
-      expect{ gl_auth.find_or_create(auth) }.to change{ User.count }.by(1)
+      expect{ gl_user.find_or_create(auth) }.to change{ User.count }.by(1)
     end
   end
 end

From 18f88a1b7627fe08d5f3e299276709101d091d48 Mon Sep 17 00:00:00 2001
From: Jan-Willem van der Meer <mail@jewilmeer.nl>
Date: Thu, 4 Sep 2014 13:00:27 +0200
Subject: [PATCH 4/9] Add new Gitlab::Oauth::AuthHash class

---
 lib/gitlab/oauth/auth_hash.rb | 54 +++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 lib/gitlab/oauth/auth_hash.rb

diff --git a/lib/gitlab/oauth/auth_hash.rb b/lib/gitlab/oauth/auth_hash.rb
new file mode 100644
index 00000000000..0198f61f427
--- /dev/null
+++ b/lib/gitlab/oauth/auth_hash.rb
@@ -0,0 +1,54 @@
+# Class to parse and transform the info provided by omniauth
+#
+module Gitlab
+  module OAuth
+    class AuthHash
+      attr_reader :auth_hash
+      def initialize(auth_hash)
+        @auth_hash = auth_hash
+      end
+
+      def uid
+        auth_hash.uid.to_s
+      end
+
+      def provider
+        auth_hash.provider
+      end
+
+      def info
+        auth_hash.info
+      end
+
+      def name
+        (info.name || full_name).to_s.force_encoding('utf-8')
+      end
+
+      def full_name
+        "#{info.first_name} #{info.last_name}"
+      end
+
+      def username
+        (info.try(:nickname) || generate_username).to_s.force_encoding('utf-8')
+      end
+
+      def email
+        (info.try(:email) || generate_temporarily_email).downcase
+      end
+
+      def password
+        @password ||= Devise.friendly_token[0, 8].downcase
+      end
+
+      # Get the first part of the email address (before @)
+      # In addtion in removes illegal characters
+      def generate_username
+        email.match(/^[^@]*/)[0].parameterize
+      end
+
+      def generate_temporarily_email
+        "temp-email-for-oauth-#{username}@gitlab.localhost"
+      end
+    end
+  end
+end

From f88b6d03306be1ec487a0746512f8d02722da435 Mon Sep 17 00:00:00 2001
From: Jan-Willem van der Meer <mail@jewilmeer.nl>
Date: Mon, 8 Sep 2014 13:11:39 +0200
Subject: [PATCH 5/9] Refactor gitlab auth tests

---
 spec/lib/gitlab/auth_spec.rb | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 073b811c3fb..ef65e109585 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -4,25 +4,27 @@ describe Gitlab::Auth do
   let(:gl_auth) { Gitlab::Auth.new }
 
   describe :find do
-    before do
-      @user = create(
-        :user,
-        username: 'john',
-        password: '88877711',
-        password_confirmation: '88877711'
-      )
+    let!(:user) do
+      create(:user,
+        username: username,
+        password: password,
+        password_confirmation: password)
     end
+    let(:username) { 'john' }
+    let(:password) { 'my-secret' }
 
     it "should find user by valid login/password" do
-      gl_auth.find('john', '88877711').should == @user
+      expect( gl_auth.find(username, password) ).to eql user
     end
 
     it "should not find user with invalid password" do
-      gl_auth.find('john', 'invalid11').should_not == @user
+      password = 'wrong'
+      expect( gl_auth.find(username, password) ).to_not eql user
     end
 
-    it "should not find user with invalid login and password" do
-      gl_auth.find('jon', 'invalid11').should_not == @user
+    it "should not find user with invalid login" do
+      user = 'wrong'
+      expect( gl_auth.find(username, password) ).to_not eql user
     end
   end
 end

From f27830fa4c11548279b5eed68e92b6f352ad4a9f Mon Sep 17 00:00:00 2001
From: Jan-Willem van der Meer <mail@jewilmeer.nl>
Date: Mon, 8 Sep 2014 13:26:25 +0200
Subject: [PATCH 6/9] Ensure Gitlab::LDAP::authentication is tested

---
 spec/lib/gitlab/auth_spec.rb | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index ef65e109585..551fb3fb5f6 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -26,5 +26,22 @@ describe Gitlab::Auth do
       user = 'wrong'
       expect( gl_auth.find(username, password) ).to_not eql user
     end
+
+    context "with ldap enabled" do
+      before { Gitlab.config.ldap['enabled'] = true }
+      after  { Gitlab.config.ldap['enabled'] = false }
+
+      it "tries to autheticate with db before ldap" do
+        expect(Gitlab::LDAP::User).not_to receive(:authenticate)
+
+        gl_auth.find(username, password)
+      end
+
+      it "uses ldap as fallback to for authentication" do
+        expect(Gitlab::LDAP::User).to receive(:authenticate)
+
+        gl_auth.find('ldap_user', 'password')
+      end
+    end
   end
 end

From 11bb67c3c6d4b90629744f8a011121e35968c58b Mon Sep 17 00:00:00 2001
From: Jan-Willem van der Meer <mail@jewilmeer.nl>
Date: Mon, 8 Sep 2014 14:53:59 +0200
Subject: [PATCH 7/9] Test authenticate method for Gitlab::LDAP::User

---
 lib/gitlab/ldap/user.rb           | 27 ++++++++++++++++-----------
 spec/lib/gitlab/ldap/user_spec.rb | 16 ++++++++++++++++
 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb
index 6d1bec5f54a..e0d718d1065 100644
--- a/lib/gitlab/ldap/user.rb
+++ b/lib/gitlab/ldap/user.rb
@@ -41,17 +41,8 @@ module Gitlab
           # Only check with valid login and password to prevent anonymous bind results
           return nil unless ldap_conf.enabled && login.present? && password.present?
 
-          ldap = OmniAuth::LDAP::Adaptor.new(ldap_conf)
-          filter = Net::LDAP::Filter.eq(ldap.uid, login)
-
-          # Apply LDAP user filter if present
-          if ldap_conf['user_filter'].present?
-            user_filter = Net::LDAP::Filter.construct(ldap_conf['user_filter'])
-            filter = Net::LDAP::Filter.join(filter, user_filter)
-          end
-
-          ldap_user = ldap.bind_as(
-            filter: filter,
+          ldap_user = adapter.bind_as(
+            filter: user_filter(login),
             size: 1,
             password: password
           )
@@ -59,6 +50,10 @@ module Gitlab
           find_by_uid(ldap_user.dn) if ldap_user
         end
 
+        def adapter
+          @adapter ||= OmniAuth::LDAP::Adaptor.new(ldap_conf)
+        end
+
         protected
 
         def find_by_uid_and_provider
@@ -81,6 +76,16 @@ module Gitlab
         def ldap_conf
           Gitlab.config.ldap
         end
+
+        def user_filter(login)
+          filter = Net::LDAP::Filter.eq(adapter.uid, login)
+          # Apply LDAP user filter if present
+          if ldap_conf['user_filter'].present?
+            user_filter = Net::LDAP::Filter.construct(ldap_conf['user_filter'])
+            filter = Net::LDAP::Filter.join(filter, user_filter)
+          end
+          filter
+        end
       end
 
       def needs_blocking?
diff --git a/spec/lib/gitlab/ldap/user_spec.rb b/spec/lib/gitlab/ldap/user_spec.rb
index 4ddf6b3039f..d232cb20759 100644
--- a/spec/lib/gitlab/ldap/user_spec.rb
+++ b/spec/lib/gitlab/ldap/user_spec.rb
@@ -35,4 +35,20 @@ describe Gitlab::LDAP::User do
       expect{ gl_user.find_or_create(auth) }.to change{ User.count }.by(1)
     end
   end
+
+  describe "authenticate" do
+    let(:login) { 'john' }
+    let(:password) { 'my-secret' }
+
+    before {
+      Gitlab.config.ldap['enabled'] = true
+      Gitlab.config.ldap['user_filter'] = 'employeeType=developer'
+    }
+    after  { Gitlab.config.ldap['enabled'] = false }
+
+    it "send an authentication request to ldap" do
+      expect( Gitlab::LDAP::User.adapter ).to receive(:bind_as)
+      Gitlab::LDAP::User.authenticate(login, password)
+    end
+  end
 end

From b18d1c2786c2a385d6b797734a1afad7a01ddf35 Mon Sep 17 00:00:00 2001
From: Jan-Willem van der Meer <mail@jewilmeer.nl>
Date: Mon, 8 Sep 2014 15:25:42 +0200
Subject: [PATCH 8/9] Remove duplicated create method

---
 lib/gitlab/ldap/user.rb | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb
index e0d718d1065..25b5a702f9a 100644
--- a/lib/gitlab/ldap/user.rb
+++ b/lib/gitlab/ldap/user.rb
@@ -26,16 +26,6 @@ module Gitlab
           end
         end
 
-        # overloaded from Gitlab::Oauth::User
-        # TODO: it's messy, needs cleanup, less complexity
-        def create(auth_hash)
-          ldap_user = new(auth_hash)
-          # first try to find the user based on the returned email address
-
-          # if the user isn't found by an exact email match, use oauth methods
-          ldap_user.save_and_trigger_callbacks
-        end
-
         def authenticate(login, password)
           # Check user against LDAP backend if user is not authenticated
           # Only check with valid login and password to prevent anonymous bind results

From 2c047642f5afd68faaafa39385d5b2c29de7ea1e Mon Sep 17 00:00:00 2001
From: Jan-Willem van der Meer <mail@jewilmeer.nl>
Date: Thu, 11 Sep 2014 12:59:42 +0200
Subject: [PATCH 9/9] Update Gemfile.lock

---
 Gemfile.lock | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Gemfile.lock b/Gemfile.lock
index e8636fd7ac5..e6d948e9a07 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -645,8 +645,8 @@ DEPENDENCIES
   omniauth (~> 1.1.3)
   omniauth-github
   omniauth-google-oauth2
-  omniauth-twitter
   omniauth-shibboleth
+  omniauth-twitter
   org-ruby
   pg
   poltergeist (~> 1.5.1)