Refactorn oauth & ldap

app/models/user.rb

@@ -87,62 +87,19 @@ class User < ActiveRecord::Base
   end
 
   def self.create_from_omniauth(auth, ldap = false)
-    provider, uid = auth.provider, auth.uid
-    name = auth.info.name.force_encoding("utf-8")
-    email = auth.info.email.downcase unless auth.info.email.nil?
-
-    ldap_prefix = ldap ? '(LDAP) ' : ''
-    raise OmniAuth::Error, "#{ldap_prefix}#{provider} does not provide an email"\
-                           " address" if auth.info.email.blank?
-
-    logger.info "#{ldap_prefix}Creating user from #{provider} login"\
-                " {uid => #{uid}, name => #{name}, email => #{email}}"
-    password = Devise.friendly_token[0, 8].downcase
-    @user = User.new(
-      extern_uid: uid,
-      provider: provider,
-      name: name,
-      email: email,
-      password: password,
-      password_confirmation: password,
-      projects_limit: Gitlab.config.default_projects_limit,
-    )
-    if Gitlab.config.omniauth.block_auto_created_users && !ldap
-      @user.blocked = true
-    end
-    @user.save!
-    @user
+    gitlab_auth.create_from_omniauth(auth, ldap)
   end
 
   def self.find_or_new_for_omniauth(auth)
-    provider, uid = auth.provider, auth.uid
+    gitlab_auth.find_or_new_for_omniauth(auth)
+  end
 
-    if @user = User.find_by_provider_and_extern_uid(provider, uid)
-      @user
-    else
-      if Gitlab.config.omniauth.allow_single_sign_on
-        @user = User.create_from_omniauth(auth)
-        @user
-      end
-    end
+  def self.find_for_ldap_auth(auth, signed_in_resource = nil)
+    gitlab_auth.find_for_ldap_auth(auth, signed_in_resource)
   end
 
-  def self.find_for_ldap_auth(auth, signed_in_resource=nil)
-    uid = auth.info.uid
-    provider = auth.provider
-    email = auth.info.email.downcase unless auth.info.email.nil?
-    raise OmniAuth::Error, "LDAP accounts must provide an uid and email address" if uid.nil? or email.nil?
-
-    if @user = User.find_by_extern_uid_and_provider(uid, provider)
-      @user
-    # workaround for backward compatibility
-    elsif @user = User.find_by_email(email)
-      logger.info "Updating legacy LDAP user #{email} with extern_uid => #{uid}"
-      @user.update_attributes(:extern_uid => uid, :provider => provider)
-      @user
-    else
-      create_from_omniauth(auth)
-    end
+  def self.gitlab_auth
+    Gitlab::Auth.new
   end
 
   def self.search query

config/gitlab.yml.example

@@ -42,7 +42,16 @@ ldap:
   password: '_the_password_of_the_bind_user'
 
 omniauth:
-  enabled: false
+  # Enable ability for users
+  # to login via twitter, google ..
+  enabled: true
+
+  # IMPORTANT! 
+  # It allows user to login without having user account
+  allow_single_sign_on: false
+  block_auto_created_users: true
+
+  # Auth providers 
   providers:
     # - { name: 'google_oauth2', app_id: 'YOUR APP ID',
     #     app_secret: 'YOUR APP SECRET',
@@ -51,10 +60,6 @@ omniauth:
     #     app_secret: 'YOUR APP SECRET'}
     # - { name: 'github', app_id: 'YOUR APP ID',
     #     app_secret: 'YOUR APP SECRET' }
-  # IMPORTANT! 
-  # It allows user to login without having user account
-  allow_single_sign_on: false
-  block_auto_created_users: true
 
 
 #

lib/gitlab/auth.rb

+module Gitlab
+  class Auth
+    def find_for_ldap_auth(auth, signed_in_resource = nil)
+      uid = auth.info.uid
+      provider = auth.provider
+      email = auth.info.email.downcase unless auth.info.email.nil?
+      raise OmniAuth::Error, "LDAP accounts must provide an uid and email address" if uid.nil? or email.nil?
+
+      if @user = User.find_by_extern_uid_and_provider(uid, provider)
+        @user
+      elsif @user = User.find_by_email(email)
+        log.info "Updating legacy LDAP user #{email} with extern_uid => #{uid}"
+        @user.update_attributes(:extern_uid => uid, :provider => provider)
+        @user
+      else
+        create_from_omniauth(auth, true)
+      end
+    end
+
+    def create_from_omniauth auth, ldap = false
+      provider = auth.provider
+      uid = auth.info.uid || auth.uid
+      name = auth.info.name.force_encoding("utf-8")
+      email = auth.info.email.downcase unless auth.info.email.nil?
+
+      ldap_prefix = ldap ? '(LDAP) ' : ''
+      raise OmniAuth::Error, "#{ldap_prefix}#{provider} does not provide an email"\
+        " address" if auth.info.email.blank?
+
+      log.info "#{ldap_prefix}Creating user from #{provider} login"\
+        " {uid => #{uid}, name => #{name}, email => #{email}}"
+      password = Devise.friendly_token[0, 8].downcase
+      @user = User.new(
+        extern_uid: uid,
+        provider: provider,
+        name: name,
+        email: email,
+        password: password,
+        password_confirmation: password,
+        projects_limit: Gitlab.config.default_projects_limit,
+      )
+      if Gitlab.config.omniauth.block_auto_created_users && !ldap
+        @user.blocked = true
+      end
+      @user.save!
+      @user
+    end
+
+    def find_or_new_for_omniauth(auth)
+      provider, uid = auth.provider, auth.uid
+
+      if @user = User.find_by_provider_and_extern_uid(provider, uid)
+        @user
+      else
+        if Gitlab.config.omniauth.allow_single_sign_on
+          @user = create_from_omniauth(auth)
+          @user
+        end
+      end
+    end
+
+    def log
+      Gitlab::AppLogger
+    end
+  end
+end

spec/lib/auth_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Auth do
+  let(:gl_auth) { Gitlab::Auth.new }
+
+  before do
+    @info = mock(
+      uid: '12djsak321',
+      name: 'John',
+      email: 'john@mail.com'
+    )
+  end
+
+  describe :find_for_ldap_auth do
+    before do
+      @auth = mock(
+        uid: '12djsak321',
+        info: @info,
+        provider: 'ldap'
+      )
+    end
+
+    it "should find by uid & provider" do
+      User.should_receive :find_by_extern_uid_and_provider
+      gl_auth.find_for_ldap_auth(@auth)
+    end
+
+    it "should update credentials by email if missing uid" do
+      user = double('User')
+      User.stub find_by_extern_uid_and_provider: nil
+      User.stub find_by_email: user
+      user.should_receive :update_attributes
+      gl_auth.find_for_ldap_auth(@auth)
+    end
+
+
+    it "should create from auth if user doesnot exist"do
+      User.stub find_by_extern_uid_and_provider: nil
+      User.stub find_by_email: nil
+      gl_auth.should_receive :create_from_omniauth
+      gl_auth.find_for_ldap_auth(@auth)
+    end
+  end
+
+  describe :find_or_new_for_omniauth do
+    before do
+      @auth = mock(
+        info: @info,
+        provider: 'twitter',
+        uid: '12djsak321',
+      )
+    end
+
+    it "should find user"do
+      User.should_receive :find_by_provider_and_extern_uid
+      gl_auth.should_not_receive :create_from_omniauth
+      gl_auth.find_or_new_for_omniauth(@auth)
+    end
+
+    it "should not create user"do
+      User.stub find_by_provider_and_extern_uid: nil
+      gl_auth.should_not_receive :create_from_omniauth
+      gl_auth.find_or_new_for_omniauth(@auth)
+    end
+
+    it "should create user if single_sing_on"do
+      Gitlab.config.omniauth.stub allow_single_sign_on: true
+      User.stub find_by_provider_and_extern_uid: nil
+      gl_auth.should_receive :create_from_omniauth
+      gl_auth.find_or_new_for_omniauth(@auth)
+    end
+  end
+
+  describe :create_from_omniauth do
+    it "should create user from LDAP" do
+      @auth = mock(info: @info, provider: 'ldap')
+      user = gl_auth.create_from_omniauth(@auth, true)
+
+      user.should be_valid
+      user.extern_uid.should == @info.uid
+      user.provider.should == 'ldap'
+    end
+
+    it "should create user from Omniauth" do
+      @auth = mock(info: @info, provider: 'twitter')
+      user = gl_auth.create_from_omniauth(@auth, false)
+
+      user.should be_valid
+      user.extern_uid.should == @info.uid
+      user.provider.should == 'twitter'
+    end
+  end
+end