From dfcea8ed514c7ef1aea78ce15525e617a10bf6bb Mon Sep 17 00:00:00 2001 From: Alex Lossent Date: Tue, 2 Jun 2015 12:01:29 +0200 Subject: [PATCH 1/2] Add option to automatically link omniauth and LDAP identities Until now, a user needed to first sign in with his LDAP identity and then manually link his/her account with an omniauth identity from their profile. Only when this is done can the user authenticate with the omniauth provider and at the same time benefit from the LDAP integration (HTTPS authentication with LDAP username/password and in EE: LDAP groups, SSH keys etc.). This feature automates the process by looking up a corresponding LDAP person when a user connects with omniauth for the first time and then automatically linking the LDAP and omniauth identities (of course, like the existing allow_single_sign_on setting, this is meant to be used with trusted omniauth providers). The result is identical to a manual account link. Add config initializers for other omniauth settings. --- CHANGELOG | 1 + config/gitlab.yml.example | 3 + config/initializers/1_settings.rb | 3 + lib/gitlab/o_auth/user.rb | 62 ++++++++++- spec/lib/gitlab/o_auth/user_spec.rb | 166 ++++++++++++++++++++++++++-- 5 files changed, 219 insertions(+), 16 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 85fa933fa6b..b0136f3a83a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -41,6 +41,7 @@ v 7.12.0 (unreleased) - Add an option to automatically sign-in with an Omniauth provider - Better performance for web editor (switched from satellites to rugged) - GitLab CI service sends .gitlab-ci.yaml in each push call + - Add option to automatically link omniauth and LDAP identities v 7.11.4 - Fix missing bullets when creating lists diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index c7f22b9388b..787b3ccfc56 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -192,6 +192,9 @@ production: &base allow_single_sign_on: false # Locks down those users until they have been cleared by the admin (default: true). block_auto_created_users: true + # Look up new users in LDAP servers. If a match is found (same uid), automatically + # link the omniauth identity with the LDAP account. (default: false) + auto_link_ldap_user: false ## Auth providers # Uncomment the following lines and fill in the data of the auth provider you want to use diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index c234bd69e9a..1bd14a3a89f 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -88,6 +88,9 @@ end Settings['omniauth'] ||= Settingslogic.new({}) Settings.omniauth['enabled'] = false if Settings.omniauth['enabled'].nil? Settings.omniauth['auto_sign_in_with_provider'] = false if Settings.omniauth['auto_sign_in_with_provider'].nil? +Settings.omniauth['allow_single_sign_on'] = false if Settings.omniauth['allow_single_sign_on'].nil? +Settings.omniauth['block_auto_created_users'] = true if Settings.omniauth['block_auto_created_users'].nil? +Settings.omniauth['auto_link_ldap_user'] = false if Settings.omniauth['auto_link_ldap_user'].nil? Settings.omniauth['providers'] ||= [] diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb index ba5caed6131..040d0ab34ff 100644 --- a/lib/gitlab/o_auth/user.rb +++ b/lib/gitlab/o_auth/user.rb @@ -46,6 +46,10 @@ module Gitlab def gl_user @user ||= find_by_uid_and_provider + if auto_link_ldap_user? + @user ||= find_or_create_ldap_user + end + if signup_enabled? @user ||= build_new_user end @@ -55,8 +59,52 @@ module Gitlab protected + def find_or_create_ldap_user + return unless ldap_person + #If a corresponding person exists with same uid in a LDAP server, + #set up a Gitlab user with dual LDAP and Omniauth identities. + if user = Gitlab::LDAP::User.find_by_uid_and_provider(ldap_person.dn.downcase, ldap_person.provider) + #case when a LDAP user already exists in Gitlab. Add the Omniauth identity to existing account. + user.identities.build(extern_uid: auth_hash.uid, provider: auth_hash.provider) + else + #no account in Gitlab yet: create it and add the LDAP identity + user = build_new_user + user.identities.new(provider: ldap_person.provider, extern_uid: ldap_person.dn) + end + user + end + + def auto_link_ldap_user? + Gitlab.config.omniauth.auto_link_ldap_user + end + + def creating_linked_ldap_user? + auto_link_ldap_user? && ldap_person + end + + def ldap_person + return @ldap_person if defined?(@ldap_person) + + #looks for a corresponding person with same uid in any of the configured LDAP providers + Gitlab::LDAP::Config.providers.each do |provider| + adapter = Gitlab::LDAP::Adapter.new(provider) + @ldap_person = Gitlab::LDAP::Person.find_by_uid(auth_hash.uid, adapter) + break if @ldap_person #exit on first person found + end + @ldap_person #may be nil if we could not find a match + end + + def ldap_config + ldap_person && Gitlab::LDAP::Config.new(ldap_person.provider) + end + def needs_blocking? - new? && block_after_signup? + return false unless new? + if creating_linked_ldap_user? + ldap_config.block_auto_created_users + else + block_after_signup? + end end def signup_enabled? @@ -84,10 +132,16 @@ module Gitlab end def user_attributes - { + # Give preference to LDAP for sensitive information when creating a linked account + username, email = if creating_linked_ldap_user? + [ ldap_person.username, ldap_person.email.first ] + else + [ auth_hash.username, auth_hash.email ] + end + return { name: auth_hash.name, - username: ::Namespace.clean_path(auth_hash.username), - email: auth_hash.email, + username: ::Namespace.clean_path(username), + email: email, password: auth_hash.password, password_confirmation: auth_hash.password, password_automatically_set: true diff --git a/spec/lib/gitlab/o_auth/user_spec.rb b/spec/lib/gitlab/o_auth/user_spec.rb index 44cdd1e4fab..2a982e8b107 100644 --- a/spec/lib/gitlab/o_auth/user_spec.rb +++ b/spec/lib/gitlab/o_auth/user_spec.rb @@ -13,6 +13,7 @@ describe Gitlab::OAuth::User do email: 'john@mail.com' } end + let(:ldap_user) { Gitlab::LDAP::Person.new(Net::LDAP::Entry.new, 'ldapmain') } describe :persisted? do let!(:existing_user) { create(:omniauth_user, extern_uid: 'my-uid', provider: 'my-provider') } @@ -32,31 +33,94 @@ describe Gitlab::OAuth::User do let(:provider) { 'twitter' } describe 'signup' do - context "with allow_single_sign_on enabled" do - before { Gitlab.config.omniauth.stub allow_single_sign_on: true } + shared_examples "to verify compliance with allow_single_sign_on" do + context "with allow_single_sign_on enabled" do + before { Gitlab.config.omniauth.stub allow_single_sign_on: true } - it "creates a user from Omniauth" do - oauth_user.save + it "creates a user from Omniauth" do + oauth_user.save - expect(gl_user).to be_valid - identity = gl_user.identities.first - expect(identity.extern_uid).to eql uid - expect(identity.provider).to eql 'twitter' + expect(gl_user).to be_valid + identity = gl_user.identities.first + expect(identity.extern_uid).to eql uid + expect(identity.provider).to eql 'twitter' + end + end + + context "with allow_single_sign_on disabled (Default)" do + before { Gitlab.config.omniauth.stub allow_single_sign_on: false } + it "throws an error" do + expect{ oauth_user.save }.to raise_error StandardError + end end end - context "with allow_single_sign_on disabled (Default)" do - it "throws an error" do - expect{ oauth_user.save }.to raise_error StandardError + context "with auto_link_ldap_user disabled (default)" do + before { Gitlab.config.omniauth.stub auto_link_ldap_user: false } + include_examples "to verify compliance with allow_single_sign_on" + end + + context "with auto_link_ldap_user enabled" do + before { Gitlab.config.omniauth.stub auto_link_ldap_user: true } + + context "and a corresponding LDAP person" do + before do + ldap_user.stub(:uid) { uid } + ldap_user.stub(:username) { uid } + ldap_user.stub(:email) { ['johndoe@example.com','john2@example.com'] } + ldap_user.stub(:dn) { 'uid=user1,ou=People,dc=example' } + allow(oauth_user).to receive(:ldap_person).and_return(ldap_user) + end + + context "and no account for the LDAP user" do + + it "creates a user with dual LDAP and omniauth identities" do + oauth_user.save + + expect(gl_user).to be_valid + expect(gl_user.username).to eql uid + expect(gl_user.email).to eql 'johndoe@example.com' + expect(gl_user.identities.length).to eql 2 + identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } } + expect(identities_as_hash).to match_array( + [ { provider: 'ldapmain', extern_uid: 'uid=user1,ou=People,dc=example' }, + { provider: 'twitter', extern_uid: uid } + ]) + end + end + + context "and LDAP user has an account already" do + let!(:existing_user) { create(:omniauth_user, email: 'john@example.com', extern_uid: 'uid=user1,ou=People,dc=example', provider: 'ldapmain', username: 'john') } + it "adds the omniauth identity to the LDAP account" do + oauth_user.save + + expect(gl_user).to be_valid + expect(gl_user.username).to eql 'john' + expect(gl_user.email).to eql 'john@example.com' + expect(gl_user.identities.length).to eql 2 + identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } } + expect(identities_as_hash).to match_array( + [ { provider: 'ldapmain', extern_uid: 'uid=user1,ou=People,dc=example' }, + { provider: 'twitter', extern_uid: uid } + ]) + end + end + end + + context "and no corresponding LDAP person" do + before { allow(oauth_user).to receive(:ldap_person).and_return(nil) } + + include_examples "to verify compliance with allow_single_sign_on" end end + end describe 'blocking' do let(:provider) { 'twitter' } before { Gitlab.config.omniauth.stub allow_single_sign_on: true } - context 'signup' do + context 'signup with omniauth only' do context 'dont block on create' do before { Gitlab.config.omniauth.stub block_auto_created_users: false } @@ -78,6 +142,64 @@ describe Gitlab::OAuth::User do end end + context 'signup with linked omniauth and LDAP account' do + before do + Gitlab.config.omniauth.stub auto_link_ldap_user: true + ldap_user.stub(:uid) { uid } + ldap_user.stub(:username) { uid } + ldap_user.stub(:email) { ['johndoe@example.com','john2@example.com'] } + ldap_user.stub(:dn) { 'uid=user1,ou=People,dc=example' } + allow(oauth_user).to receive(:ldap_person).and_return(ldap_user) + end + + context "and no account for the LDAP user" do + context 'dont block on create (LDAP)' do + before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: false } + + it do + oauth_user.save + expect(gl_user).to be_valid + expect(gl_user).not_to be_blocked + end + end + + context 'block on create (LDAP)' do + before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: true } + + it do + oauth_user.save + expect(gl_user).to be_valid + expect(gl_user).to be_blocked + end + end + end + + context 'and LDAP user has an account already' do + let!(:existing_user) { create(:omniauth_user, email: 'john@example.com', extern_uid: 'uid=user1,ou=People,dc=example', provider: 'ldapmain', username: 'john') } + + context 'dont block on create (LDAP)' do + before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: false } + + it do + oauth_user.save + expect(gl_user).to be_valid + expect(gl_user).not_to be_blocked + end + end + + context 'block on create (LDAP)' do + before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: true } + + it do + oauth_user.save + expect(gl_user).to be_valid + expect(gl_user).not_to be_blocked + end + end + end + end + + context 'sign-in' do before do oauth_user.save @@ -103,6 +225,26 @@ describe Gitlab::OAuth::User do expect(gl_user).not_to be_blocked end end + + context 'dont block on create (LDAP)' do + before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: false } + + it do + oauth_user.save + expect(gl_user).to be_valid + expect(gl_user).not_to be_blocked + end + end + + context 'block on create (LDAP)' do + before { Gitlab::LDAP::Config.any_instance.stub block_auto_created_users: true } + + it do + oauth_user.save + expect(gl_user).to be_valid + expect(gl_user).not_to be_blocked + end + end end end end From 45e9150a517970d3782bed642b091ad60a46634f Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 5 Jun 2015 12:32:01 +0200 Subject: [PATCH 2/2] Tweak code. --- lib/gitlab/o_auth/user.rb | 45 +++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb index 040d0ab34ff..c4971b5bcc6 100644 --- a/lib/gitlab/o_auth/user.rb +++ b/lib/gitlab/o_auth/user.rb @@ -61,16 +61,18 @@ module Gitlab def find_or_create_ldap_user return unless ldap_person - #If a corresponding person exists with same uid in a LDAP server, - #set up a Gitlab user with dual LDAP and Omniauth identities. + + # If a corresponding person exists with same uid in a LDAP server, + # set up a Gitlab user with dual LDAP and Omniauth identities. if user = Gitlab::LDAP::User.find_by_uid_and_provider(ldap_person.dn.downcase, ldap_person.provider) - #case when a LDAP user already exists in Gitlab. Add the Omniauth identity to existing account. + # Case when a LDAP user already exists in Gitlab. Add the Omniauth identity to existing account. user.identities.build(extern_uid: auth_hash.uid, provider: auth_hash.provider) else - #no account in Gitlab yet: create it and add the LDAP identity + # No account in Gitlab yet: create it and add the LDAP identity user = build_new_user user.identities.new(provider: ldap_person.provider, extern_uid: ldap_person.dn) end + user end @@ -85,26 +87,20 @@ module Gitlab def ldap_person return @ldap_person if defined?(@ldap_person) - #looks for a corresponding person with same uid in any of the configured LDAP providers - Gitlab::LDAP::Config.providers.each do |provider| + # looks for a corresponding person with same uid in any of the configured LDAP providers + @ldap_person = Gitlab::LDAP::Config.providers.find do |provider| adapter = Gitlab::LDAP::Adapter.new(provider) - @ldap_person = Gitlab::LDAP::Person.find_by_uid(auth_hash.uid, adapter) - break if @ldap_person #exit on first person found + + Gitlab::LDAP::Person.find_by_uid(auth_hash.uid, adapter) end - @ldap_person #may be nil if we could not find a match end def ldap_config - ldap_person && Gitlab::LDAP::Config.new(ldap_person.provider) + Gitlab::LDAP::Config.new(ldap_person.provider) if ldap_person end def needs_blocking? - return false unless new? - if creating_linked_ldap_user? - ldap_config.block_auto_created_users - else - block_after_signup? - end + new? && block_after_signup? end def signup_enabled? @@ -112,7 +108,11 @@ module Gitlab end def block_after_signup? - Gitlab.config.omniauth.block_auto_created_users + if creating_linked_ldap_user? + ldap_config.block_auto_created_users + else + Gitlab.config.omniauth.block_auto_created_users + end end def auth_hash=(auth_hash) @@ -133,12 +133,15 @@ module Gitlab def user_attributes # Give preference to LDAP for sensitive information when creating a linked account - username, email = if creating_linked_ldap_user? - [ ldap_person.username, ldap_person.email.first ] + if creating_linked_ldap_user? + username = ldap_person.username + email = ldap_person.email.first else - [ auth_hash.username, auth_hash.email ] + username = auth_hash.username + email = auth_hash.email end - return { + + { name: auth_hash.name, username: ::Namespace.clean_path(username), email: email,