Commit 4fcc17e6673b271992a9d4a5106f8bd64cfe86b1
1 parent
089f0000
Exists in
master
and in
4 other branches
Allows username only updates to ldap properties
-when logging in if users are allowed to login with just usernames in ldap we will update uid of the user if their uid is out of date Conflicts: spec/lib/auth_spec.rb Change-Id: Ia171b3d5133da86edc18c0d08ecfaf6a174f2574
Showing
2 changed files
with
108 additions
and
1 deletions
Show diff stats
lib/gitlab/ldap/user.rb
| @@ -26,7 +26,7 @@ module Gitlab | @@ -26,7 +26,7 @@ module Gitlab | ||
| 26 | # * When user already has account and need to link his LDAP account. | 26 | # * When user already has account and need to link his LDAP account. |
| 27 | # * LDAP uid changed for user with same email and we need to update his uid | 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 | if user | 31 | if user |
| 32 | user.update_attributes(extern_uid: uid, provider: provider) | 32 | user.update_attributes(extern_uid: uid, provider: provider) |
| @@ -43,6 +43,15 @@ module Gitlab | @@ -43,6 +43,15 @@ module Gitlab | ||
| 43 | user | 43 | user |
| 44 | end | 44 | end |
| 45 | 45 | ||
| 46 | + def find_user(email) | ||
| 47 | + if user = model.find_by_email(email) | ||
| 48 | + elsif ldap_conf['allow_username_or_email_login'] | ||
| 49 | + uname = (email.partition('@').first) unless email.nil? | ||
| 50 | + user = model.find_by_username(uname) | ||
| 51 | + end | ||
| 52 | + user | ||
| 53 | + end | ||
| 54 | + | ||
| 46 | def authenticate(login, password) | 55 | def authenticate(login, password) |
| 47 | # Check user against LDAP backend if user is not authenticated | 56 | # Check user against LDAP backend if user is not authenticated |
| 48 | # Only check with valid login and password to prevent anonymous bind results | 57 | # Only check with valid login and password to prevent anonymous bind results |
| @@ -0,0 +1,98 @@ | @@ -0,0 +1,98 @@ | ||
| 1 | +require 'spec_helper' | ||
| 2 | + | ||
| 3 | +describe Gitlab::Auth do | ||
| 4 | + let(:gl_auth) { Gitlab::Auth.new } | ||
| 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 find by uid & provider" do | ||
| 26 | + User.should_receive :find_by_extern_uid_and_provider | ||
| 27 | + gl_auth.find_for_ldap_auth(@auth) | ||
| 28 | + end | ||
| 29 | + | ||
| 30 | + it "should update credentials by email if missing uid" do | ||
| 31 | + user = double('User') | ||
| 32 | + User.stub find_by_extern_uid_and_provider: nil | ||
| 33 | + User.stub find_by_email: user | ||
| 34 | + user.should_receive :update_attributes | ||
| 35 | + gl_auth.find_for_ldap_auth(@auth) | ||
| 36 | + end | ||
| 37 | + | ||
| 38 | + it "should update credentials by username if missing uid and Gitlab.config.ldap.allow_username_or_email_login is true" do | ||
| 39 | + user = double('User') | ||
| 40 | + value = Gitlab.config.ldap.allow_username_or_email_login | ||
| 41 | + Gitlab.config.ldap['allow_username_or_email_login'] = true | ||
| 42 | + User.stub find_by_extern_uid_and_provider: nil | ||
| 43 | + User.stub find_by_email: nil | ||
| 44 | + User.stub find_by_username: user | ||
| 45 | + user.should_receive :update_attributes | ||
| 46 | + gl_auth.find_for_ldap_auth(@auth) | ||
| 47 | + Gitlab.config.ldap['allow_username_or_email_login'] = value | ||
| 48 | + end | ||
| 49 | + | ||
| 50 | + it "should not update credentials by username if missing uid and Gitlab.config.ldap.allow_username_or_email_login is false" do | ||
| 51 | + user = double('User') | ||
| 52 | + value = Gitlab.config.ldap.allow_username_or_email_login | ||
| 53 | + Gitlab.config.ldap['allow_username_or_email_login'] = false | ||
| 54 | + User.stub find_by_extern_uid_and_provider: nil | ||
| 55 | + User.stub find_by_email: nil | ||
| 56 | + User.stub find_by_username: user | ||
| 57 | + user.should_not_receive :update_attributes | ||
| 58 | + gl_auth.find_for_ldap_auth(@auth) | ||
| 59 | + Gitlab.config.ldap['allow_username_or_email_login'] = value | ||
| 60 | + end | ||
| 61 | + | ||
| 62 | + it "should create from auth if user does not exist"do | ||
| 63 | + User.stub find_by_extern_uid_and_provider: nil | ||
| 64 | + User.stub find_by_email: nil | ||
| 65 | + gl_auth.should_receive :create_from_omniauth | ||
| 66 | + gl_auth.find_for_ldap_auth(@auth) | ||
| 67 | + end | ||
| 68 | + end | ||
| 69 | + | ||
| 70 | + describe :find_or_new_for_omniauth do | ||
| 71 | + before do | ||
| 72 | + @auth = mock( | ||
| 73 | + info: @info, | ||
| 74 | + provider: 'twitter', | ||
| 75 | + uid: '12djsak321', | ||
| 76 | + ) | ||
| 77 | + end | ||
| 78 | + | ||
| 79 | + it "should find user"do | ||
| 80 | + User.should_receive :find_by_provider_and_extern_uid | ||
| 81 | + gl_auth.should_not_receive :create_from_omniauth | ||
| 82 | + gl_auth.find_or_new_for_omniauth(@auth) | ||
| 83 | + end | ||
| 84 | + | ||
| 85 | + it "should not create user"do | ||
| 86 | + User.stub find_by_provider_and_extern_uid: nil | ||
| 87 | + gl_auth.should_not_receive :create_from_omniauth | ||
| 88 | + gl_auth.find_or_new_for_omniauth(@auth) | ||
| 89 | + end | ||
| 90 | + | ||
| 91 | + it "should create user if single_sing_on"do | ||
| 92 | + Gitlab.config.omniauth['allow_single_sign_on'] = true | ||
| 93 | + User.stub find_by_provider_and_extern_uid: nil | ||
| 94 | + gl_auth.should_receive :create_from_omniauth | ||
| 95 | + gl_auth.find_or_new_for_omniauth(@auth) | ||
| 96 | + end | ||
| 97 | + end | ||
| 98 | +end |