Merge pull request #5063 from karlhungus/feature-allow-ldap-update-with-username

Allows username only updates to ldap properties

Merge pull request #5063 from karlhungus/feature-allow-ldap-update-with-username
Allows username only updates to ldap properties
Dmitriy Zaporozhets
2 parents 089f0000 8a8123a3
Showing 3 changed files with 72 additions and 2 deletions Show diff stats
lib/gitlab/ldap/user.rb
spec/lib/gitlab/ldap/ldap_user_auth_spec.rb
spec/models/user_spec.rb
@@ -26,7 +26,7 @@ module Gitlab
             # * When user already has account and need to link his LDAP account.
             # * LDAP uid changed for user with same email and we need to update his uid
             #
-            user = model.find_by_email(email)
+            user = find_user(email)
  
             if user
               user.update_attributes(extern_uid: uid, provider: provider)
@@ -43,6 +43,19 @@ module Gitlab
           user
         end
  
+        def find_user(email)
+          user = model.find_by_email(email)
+
+          # If no user found and allow_username_or_email_login is true
+          # we look for user by extracting part of his email
+          if !user && email && ldap_conf['allow_username_or_email_login']
+            uname = email.partition('@').first
+            user = model.find_by_username(uname)
+          end
+
+          user
+        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
@@ -0,0 +1,57 @@
+require 'spec_helper'
+
+describe Gitlab::LDAP do
+  let(:gl_auth) { Gitlab::LDAP::User }
+
+  before do
+    Gitlab.config.stub(omniauth: {})
+
+    @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 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_or_create(@auth)
+    end
+
+    it "should update credentials by username if missing uid and Gitlab.config.ldap.allow_username_or_email_login is true" do
+      user = double('User')
+      value = Gitlab.config.ldap.allow_username_or_email_login
+      Gitlab.config.ldap['allow_username_or_email_login'] = true
+      User.stub find_by_extern_uid_and_provider: nil
+      User.stub find_by_email: nil
+      User.stub find_by_username: user
+      user.should_receive :update_attributes
+      gl_auth.find_or_create(@auth)
+      Gitlab.config.ldap['allow_username_or_email_login'] = value
+    end
+
+    it "should not update credentials by username if missing uid and Gitlab.config.ldap.allow_username_or_email_login is false" do
+      user = double('User')
+      value = Gitlab.config.ldap.allow_username_or_email_login
+      Gitlab.config.ldap['allow_username_or_email_login'] = false
+      User.stub find_by_extern_uid_and_provider: nil
+      User.stub find_by_email: nil
+      User.stub find_by_username: user
+      user.should_not_receive :update_attributes
+      gl_auth.find_or_create(@auth)
+      Gitlab.config.ldap['allow_username_or_email_login'] = value
+    end
+  end
+end
@@ -233,7 +233,7 @@ describe User do
         it "should apply defaults to user" do
           Gitlab.config.gitlab.default_projects_limit.should_not == 123
           Gitlab.config.gitlab.default_can_create_group.should_not be_true
-          Gitlab.config.gitlab.default_theme.should_not == Gitlab::Theme::MARS
+          Gitlab.config.gitlab.default_theme.should_not == Gitlab::Theme::BASIC
           user.projects_limit.should == 123
           user.can_create_group.should be_true
           user.theme_id.should == Gitlab::Theme::BASIC
...	...	@@ -26,7 +26,7 @@ module Gitlab
26	26	# * When user already has account and need to link his LDAP account.
27	27	# * LDAP uid changed for user with same email and we need to update his uid
28	28	#
29		- user = model.find_by_email(email)
	29	+ user = find_user(email)
30	30
31	31	if user
32	32	user.update_attributes(extern_uid: uid, provider: provider)
...	...	@@ -43,6 +43,19 @@ module Gitlab
43	43	user
44	44	end
45	45
	46	+ def find_user(email)
	47	+ user = model.find_by_email(email)
	48	+
	49	+ # If no user found and allow_username_or_email_login is true
	50	+ # we look for user by extracting part of his email
	51	+ if !user && email && ldap_conf['allow_username_or_email_login']
	52	+ uname = email.partition('@').first
	53	+ user = model.find_by_username(uname)
	54	+ end
	55	+
	56	+ user
	57	+ end
	58	+
46	59	def authenticate(login, password)
47	60	# Check user against LDAP backend if user is not authenticated
48	61	# Only check with valid login and password to prevent anonymous bind results
...	...
...	...	@@ -0,0 +1,57 @@
	1	+require 'spec_helper'
	2	+
	3	+describe Gitlab::LDAP do
	4	+ let(:gl_auth) { Gitlab::LDAP::User }
	5	+
	6	+ before do
	7	+ Gitlab.config.stub(omniauth: {})
	8	+
	9	+ @info = mock(
	10	+ uid: '12djsak321',
	11	+ name: 'John',
	12	+ email: 'john@mail.com'
	13	+ )
	14	+ end
	15	+
	16	+ describe :find_for_ldap_auth do
	17	+ before do
	18	+ @auth = mock(
	19	+ uid: '12djsak321',
	20	+ info: @info,
	21	+ provider: 'ldap'
	22	+ )
	23	+ end
	24	+
	25	+ it "should update credentials by email if missing uid" do
	26	+ user = double('User')
	27	+ User.stub find_by_extern_uid_and_provider: nil
	28	+ User.stub find_by_email: user
	29	+ user.should_receive :update_attributes
	30	+ gl_auth.find_or_create(@auth)
	31	+ end
	32	+
	33	+ it "should update credentials by username if missing uid and Gitlab.config.ldap.allow_username_or_email_login is true" do
	34	+ user = double('User')
	35	+ value = Gitlab.config.ldap.allow_username_or_email_login
	36	+ Gitlab.config.ldap['allow_username_or_email_login'] = true
	37	+ User.stub find_by_extern_uid_and_provider: nil
	38	+ User.stub find_by_email: nil
	39	+ User.stub find_by_username: user
	40	+ user.should_receive :update_attributes
	41	+ gl_auth.find_or_create(@auth)
	42	+ Gitlab.config.ldap['allow_username_or_email_login'] = value
	43	+ end
	44	+
	45	+ it "should not update credentials by username if missing uid and Gitlab.config.ldap.allow_username_or_email_login is false" do
	46	+ user = double('User')
	47	+ value = Gitlab.config.ldap.allow_username_or_email_login
	48	+ Gitlab.config.ldap['allow_username_or_email_login'] = false
	49	+ User.stub find_by_extern_uid_and_provider: nil
	50	+ User.stub find_by_email: nil
	51	+ User.stub find_by_username: user
	52	+ user.should_not_receive :update_attributes
	53	+ gl_auth.find_or_create(@auth)
	54	+ Gitlab.config.ldap['allow_username_or_email_login'] = value
	55	+ end
	56	+ end
	57	+end
...	...
...	...	@@ -233,7 +233,7 @@ describe User do
233	233	it "should apply defaults to user" do
234	234	Gitlab.config.gitlab.default_projects_limit.should_not == 123
235	235	Gitlab.config.gitlab.default_can_create_group.should_not be_true
236		- Gitlab.config.gitlab.default_theme.should_not == Gitlab::Theme::MARS
	236	+ Gitlab.config.gitlab.default_theme.should_not == Gitlab::Theme::BASIC
237	237	user.projects_limit.should == 123
238	238	user.can_create_group.should be_true
239	239	user.theme_id.should == Gitlab::Theme::BASIC
...	...