From 4fcc17e6673b271992a9d4a5106f8bd64cfe86b1 Mon Sep 17 00:00:00 2001 From: Izaak Alpert Date: Thu, 12 Sep 2013 16:27:51 -0400 Subject: 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 --- lib/gitlab/ldap/user.rb | 11 ++++- spec/lib/auth_oauth_spec.rb | 98 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 spec/lib/auth_oauth_spec.rb diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb index c8f3a69376a..1606210aafc 100644 --- a/lib/gitlab/ldap/user.rb +++ b/lib/gitlab/ldap/user.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,15 @@ module Gitlab user end + def find_user(email) + if user = model.find_by_email(email) + elsif ldap_conf['allow_username_or_email_login'] + uname = (email.partition('@').first) unless email.nil? + 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 diff --git a/spec/lib/auth_oauth_spec.rb b/spec/lib/auth_oauth_spec.rb new file mode 100644 index 00000000000..c9deb590f67 --- /dev/null +++ b/spec/lib/auth_oauth_spec.rb @@ -0,0 +1,98 @@ +require 'spec_helper' + +describe Gitlab::Auth do + let(:gl_auth) { Gitlab::Auth.new } + + 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 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 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_for_ldap_auth(@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_for_ldap_auth(@auth) + Gitlab.config.ldap['allow_username_or_email_login'] = value + end + + it "should create from auth if user does not 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['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 +end -- cgit v1.2.1 From 75b2ff8f43d52c76d4d4eaac37618295b5046cc6 Mon Sep 17 00:00:00 2001 From: Izaak Alpert Date: Mon, 16 Sep 2013 00:40:21 -0400 Subject: Added ldap tests Change-Id: Ifd77615b3b92d7d8bca92b8875aa8204356cfd85 --- spec/lib/auth_oauth_spec.rb | 98 ----------------------------- spec/lib/gitlab/ldap/ldap_user_auth_spec.rb | 57 +++++++++++++++++ 2 files changed, 57 insertions(+), 98 deletions(-) delete mode 100644 spec/lib/auth_oauth_spec.rb create mode 100644 spec/lib/gitlab/ldap/ldap_user_auth_spec.rb diff --git a/spec/lib/auth_oauth_spec.rb b/spec/lib/auth_oauth_spec.rb deleted file mode 100644 index c9deb590f67..00000000000 --- a/spec/lib/auth_oauth_spec.rb +++ /dev/null @@ -1,98 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Auth do - let(:gl_auth) { Gitlab::Auth.new } - - 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 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 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_for_ldap_auth(@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_for_ldap_auth(@auth) - Gitlab.config.ldap['allow_username_or_email_login'] = value - end - - it "should create from auth if user does not 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['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 -end diff --git a/spec/lib/gitlab/ldap/ldap_user_auth_spec.rb b/spec/lib/gitlab/ldap/ldap_user_auth_spec.rb new file mode 100644 index 00000000000..b1c583c0476 --- /dev/null +++ b/spec/lib/gitlab/ldap/ldap_user_auth_spec.rb @@ -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 -- cgit v1.2.1 From 8a8123a3d4d3a5f991ae599e454b99fd548d47f2 Mon Sep 17 00:00:00 2001 From: Izaak Alpert Date: Sun, 22 Sep 2013 20:25:10 -0400 Subject: Update for readability fixed a test a broke in the configurable theme PR Change-Id: Id894506941bc01ab0d259d48ca7ff9b80bb2c57e --- lib/gitlab/ldap/user.rb | 10 +++++++--- spec/models/user_spec.rb | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb index 1606210aafc..260bacfeeb0 100644 --- a/lib/gitlab/ldap/user.rb +++ b/lib/gitlab/ldap/user.rb @@ -44,11 +44,15 @@ module Gitlab end def find_user(email) - if user = model.find_by_email(email) - elsif ldap_conf['allow_username_or_email_login'] - uname = (email.partition('@').first) unless email.nil? + 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 diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 2b42226ecaf..c879900f8fd 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -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 -- cgit v1.2.1