Merge branch '1552-kerberos-ldap-auto-link' into 'master'

Automatically link Kerberos users and LDAP people

Closes #1552

See merge request !2405

changelogs/unreleased-ee/1552-kerberos-ldap-auto-link.yml

+---
+title: Automatically link kerberos users to LDAP people
+merge_request: 2405
+author:

doc/integration/kerberos.md

@@ -105,6 +105,24 @@ user associated with the email, which is built from the Kerberos username and
 realm. User accounts will be created automatically when authentication was
 successful.
 
+## Linking Kerberos and LDAP accounts together
+
+If your users log in with Kerberos, but you also have [LDAP integration](../administration/auth/ldap.md)
+enabled, then your users will be automatically linked to their LDAP accounts on
+first login. For this to work, some prerequisites must be met:
+
+The Kerberos username must match the LDAP user's UID. You can choose which LDAP
+attribute is used as the UID in GitLab's [LDAP configuration](../administration/auth/ldap.md#configuration)
+but for Active Directory, this should be `sAMAccountName`.
+
+The Kerberos realm must match the domain part of the LDAP user's Distinguished
+Name. For instance, if the Kerberos realm is `AD.EXAMPLE.COM`, then the LDAP
+user's Distinguished Name should end in `dc=ad,dc=example,dc=com`.
+
+Taken together, these rules mean that linking will only work if your users'
+Kerberos usernames are of the form `foo@AD.EXAMPLE.COM` and their
+LDAP Distinguished Names look like `sAMAccountName=foo,dc=ad,dc=example,dc=com`.
+
 ## HTTP Git access
 
 A linked Kerberos account enables you to `git pull` and `git push` using your

lib/ee/gitlab/ldap/person.rb

+require 'net/ldap/dn'
+
 module EE
   module Gitlab
     module LDAP
@@ -15,6 +17,31 @@ module EE
 
             nil
           end
+
+          def find_by_kerberos_principal(principal, adapter)
+            uid, domain = principal.split('@', 2)
+            return nil unless uid && domain
+
+            # In multi-forest setups, there may be several users with matching
+            # uids but differing DNs, so skip adapters configured to connect to
+            # non-matching domains
+            return unless domain.casecmp(domain_from_dn(adapter.config.base)) == 0
+
+            find_by_uid(uid, adapter)
+          end
+
+          # Extracts the rightmost unbroken set of domain components from an
+          # LDAP DN and constructs a domain name from them
+          def domain_from_dn(dn)
+            dn_components = []
+            Net::LDAP::DN.new(dn).each_pair { |name, value| dn_components << { name: name, value: value } }
+            dn_components
+              .reverse
+              .take_while { |rdn| rdn[:name].casecmp('DC').zero? } # Domain Component
+              .map { |rdn| rdn[:value] }
+              .reverse
+              .join('.')
+          end
         end
 
         def ssh_keys
@@ -27,23 +54,12 @@ module EE
           end
         end
 
+        # We assume that the Kerberos username matches the configured uid
+        # attribute in LDAP. For Active Directory, this is `sAMAccountName`
         def kerberos_principal
-          # The following is only meaningful for Active Directory
-          return unless entry.respond_to?(:sAMAccountName)
-          entry[:sAMAccountName].first + '@' + windows_domain_name.upcase
-        end
+          return nil unless uid
 
-        def windows_domain_name
-          # The following is only meaningful for Active Directory
-          require 'net/ldap/dn'
-          dn_components = []
-          Net::LDAP::DN.new(dn).each_pair { |name, value| dn_components << { name: name, value: value } }
-          dn_components
-            .reverse
-            .take_while { |rdn| rdn[:name].casecmp('DC').zero? } # Domain Component
-          .map { |rdn| rdn[:value] }
-            .reverse
-            .join('.')
+          uid + '@' + self.class.domain_from_dn(dn).upcase
         end
 
         def memberof

lib/ee/gitlab/o_auth/auth_hash.rb

+module EE
+  module Gitlab
+    module OAuth
+      module AuthHash
+        def kerberos_default_realm
+          ::Gitlab::Kerberos::Authentication.kerberos_default_realm
+        end
+
+        def uid
+          return @ee_uid if defined?(@ee_uid)
+
+          ee_uid = super
+
+          # For Kerberos, usernames `principal` and `principal@DEFAULT.REALM`
+          # are equivalent and may be used indifferently, but omniauth_kerberos
+          # does not normalize them as of version 0.3.0, so add the default
+          # realm ourselves if appropriate
+          if provider == 'kerberos' && ee_uid.present?
+            ee_uid += "@#{kerberos_default_realm}" unless ee_uid.include?('@')
+          end
+
+          @ee_uid = ee_uid
+        end
+      end
+    end
+  end
+end

lib/ee/gitlab/o_auth/user.rb

+module EE
+  module Gitlab
+    module OAuth
+      module User
+        protected
+
+        def find_ldap_person(auth_hash, adapter)
+          if auth_hash.provider == 'kerberos'
+            ::Gitlab::LDAP::Person.find_by_kerberos_principal(auth_hash.uid, adapter)
+          else
+            super
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/o_auth/auth_hash.rb

@@ -3,26 +3,15 @@
 module Gitlab
   module OAuth
     class AuthHash
+      prepend ::EE::Gitlab::OAuth::AuthHash
+
       attr_reader :auth_hash
       def initialize(auth_hash)
         @auth_hash = auth_hash
       end
 
-      def kerberos_default_realm
-        Gitlab::Kerberos::Authentication.kerberos_default_realm
-      end
-
-      def normalized_uid
-        return auth_hash.uid.to_s unless provider == 'kerberos'
-        # For Kerberos, usernames `principal` and `principal@DEFAULT.REALM` are equivalent and
-        # may be used indifferently, but omniauth_kerberos does not normalize them as of version 0.3.0.
-        # Normalize here the uid to always have the canonical Kerberos principal name with realm.
-        return auth_hash.uid if auth_hash.uid.include?("@")
-        auth_hash.uid + "@" + kerberos_default_realm
-      end
-
       def uid
-        @uid ||= Gitlab::Utils.force_utf8(normalized_uid)
+        @uid ||= Gitlab::Utils.force_utf8(auth_hash.uid.to_s)
       end
 
       def provider

lib/gitlab/o_auth/user.rb

@@ -8,6 +8,8 @@ module Gitlab
     SignupDisabledError = Class.new(StandardError)
 
     class User
+      prepend ::EE::Gitlab::OAuth::User
+
       attr_accessor :auth_hash, :gl_user
 
       def initialize(auth_hash)
@@ -101,14 +103,18 @@ module Gitlab
         # Look 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)
-          # The `uid` might actually be a DN. Try it next.
-          @ldap_person ||= Gitlab::LDAP::Person.find_by_dn(auth_hash.uid, adapter)
+          @ldap_person = find_ldap_person(auth_hash, adapter)
           break if @ldap_person
         end
         @ldap_person
       end
 
+      def find_ldap_person(auth_hash, adapter)
+        by_uid = Gitlab::LDAP::Person.find_by_uid(auth_hash.uid, adapter)
+        # The `uid` might actually be a DN. Try it next.
+        by_uid || Gitlab::LDAP::Person.find_by_dn(auth_hash.uid, adapter)
+      end
+
       def ldap_config
         Gitlab::LDAP::Config.new(ldap_person.provider) if ldap_person
       end

spec/lib/ee/gitlab/ldap/person_spec.rb

@@ -29,32 +29,56 @@ describe Gitlab::LDAP::Person do
     end
   end
 
-  describe '#kerberos_principal' do
-    let(:entry) do
-      ldif = "dn: cn=foo, dc=bar, dc=com\n"
-      ldif += "sAMAccountName: #{sam_account_name}\n" if sam_account_name
-      Net::LDAP::Entry.from_single_ldif_string(ldif)
+  describe '.find_by_kerberos_principal' do
+    let(:adapter) { ldap_adapter }
+    let(:username) { 'foo' }
+    let(:principal) { username + '@' + kerberos_realm }
+    let(:ldap_server) { 'ad.example.com' }
+
+    subject { described_class.find_by_kerberos_principal(principal, adapter) }
+
+    before do
+      stub_ldap_config(uid: 'sAMAccountName', base: 'ou=foo,dc=' + ldap_server.gsub('.', ',dc='))
     end
 
-    subject { described_class.new(entry, 'ldapmain') }
+    context 'LDAP server is not for kerberos realm' do
+      let(:kerberos_realm) { 'kerberos.example.com' }
 
-    context 'when sAMAccountName is not defined (non-AD LDAP server)' do
-      let(:sam_account_name) { nil }
+      it 'returns nil without searching' do
+        expect(adapter).not_to receive(:user)
 
-      it 'returns nil' do
-        expect(subject.kerberos_principal).to be_nil
+        is_expected.to be_nil
       end
     end
 
-    context 'when sAMAccountName is defined (AD server)' do
-      let(:sam_account_name) { 'mylogin' }
+    context 'LDAP server is for kerberos realm' do
+      let(:kerberos_realm) { ldap_server }
+
+      it 'searches by configured uid attribute' do
+        expect(adapter).to receive(:user).with('sAMAccountName', username).and_return(:fake_user)
 
-      it 'returns the principal combining sAMAccountName and DC components of the distinguishedName' do
-        expect(subject.kerberos_principal).to eq('mylogin@BAR.COM')
+        is_expected.to eq(:fake_user)
       end
     end
   end
 
+  describe '#kerberos_principal' do
+    let(:entry) do
+      ldif = "dn: cn=foo, dc=bar, dc=com\nsAMAccountName: myName\n"
+      Net::LDAP::Entry.from_single_ldif_string(ldif)
+    end
+
+    subject { described_class.new(entry, 'ldapmain') }
+
+    before do
+      stub_ldap_config(uid: 'sAMAccountName')
+    end
+
+    it 'returns the principal combining the configured UID and DC components of the distinguishedName' do
+      expect(subject.kerberos_principal).to eq('myName@BAR.COM')
+    end
+  end
+
   describe '#ssh_keys' do
     let(:ssh_key) { 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCrSQHff6a1rMqBdHFt+FwIbytMZ+hJKN3KLkTtOWtSvNIriGhnTdn4rs+tjD/w+z+revytyWnMDM9dS7J8vQi006B16+hc9Xf82crqRoPRDnBytgAFFQY1G/55ql2zdfsC5yvpDOFzuwIJq5dNGsojS82t6HNmmKPq130fzsenFnj5v1pl3OJvk513oduUyKiZBGTroWTn7H/eOPtu7s9MD7pAdEjqYKFLeaKmyidiLmLqQlCRj3Tl2U9oyFg4PYNc0bL5FZJ/Z6t0Ds3i/a2RanQiKxrvgu3GSnUKMx7WIX373baL4jeM7cprRGiOY/1NcS+1cAjfJ8oaxQF/1dYj' }
     let(:ssh_key_attribute_name) { 'altSecurityIdentities' }

spec/lib/ee/gitlab/o_auth/auth_hash_spec.rb

+require 'spec_helper'
+
+describe Gitlab::OAuth::AuthHash, lib: true do
+  let(:auth_hash) do
+    Gitlab::OAuth::AuthHash.new(
+      OmniAuth::AuthHash.new(
+        provider: ascii('kerberos'),
+        uid: ascii(uid),
+        info: { uid: ascii(uid) }
+      )
+    )
+  end
+
+  describe '#uid' do
+    subject { auth_hash.uid }
+
+    context 'contains a kerberos realm' do
+      let(:uid) { 'mylogin@BAR.COM' }
+
+      it 'preserves the canonical uid' do
+        is_expected.to eq('mylogin@BAR.COM')
+      end
+    end
+
+    context 'does not contain a kerberos realm' do
+      let(:uid) { 'mylogin' }
+
+      before do
+        allow(Gitlab::Kerberos::Authentication).to receive(:kerberos_default_realm).and_return('FOO.COM')
+      end
+
+      it 'canonicalizes uid with kerberos realm' do
+        is_expected.to eq('mylogin@FOO.COM')
+      end
+    end
+  end
+
+  def ascii(text)
+    text.force_encoding(Encoding::ASCII_8BIT)
+  end
+end

spec/lib/ee/gitlab/o_auth/user_spec.rb

+require 'spec_helper'
+
+describe Gitlab::OAuth::User, lib: true do
+  include LdapHelpers
+
+  describe 'login through kerberos with linkable LDAP user' do
+    let(:uid)        { 'foo' }
+    let(:provider)   { 'kerberos' }
+    let(:realm)      { 'ad.example.com' }
+    let(:base_dn)    { 'ou=users,dc=ad,dc=example,dc=com' }
+    let(:info_hash)  { { email: uid + '@' + realm, username: uid } }
+    let(:auth_hash)  { OmniAuth::AuthHash.new(uid: uid, provider: provider, info: info_hash) }
+    let(:oauth_user) { Gitlab::OAuth::User.new(auth_hash) }
+    let(:real_email) { 'myname@example.com' }
+
+    before do
+      allow(::Gitlab::Kerberos::Authentication).to receive(:kerberos_default_realm).and_return(realm)
+      allow(Gitlab.config.omniauth).to receive_messages(auto_link_ldap_user: true, allow_single_sign_on: ['kerberos'])
+      stub_ldap_config(base: base_dn)
+
+      ldap_entry = Net::LDAP::Entry.new("uid=#{uid}," + base_dn).tap do |entry|
+        entry['uid'] = uid
+        entry['email'] = real_email
+      end
+
+      stub_ldap_person_find_by_uid(uid, ldap_entry)
+
+      oauth_user.save
+    end
+
+    it 'links the LDAP person to the GitLab user' do
+      gl_user = oauth_user.gl_user
+
+      identities = gl_user.identities.map do |identity|
+        { provider: identity.provider, extern_uid: identity.extern_uid }
+      end
+
+      expect(identities).to contain_exactly(
+        { provider: 'ldapmain', extern_uid: "uid=#{uid},#{base_dn}" },
+        { provider: 'kerberos', extern_uid: uid + '@' + realm }
+      )
+ 
+      expect(gl_user.email).to eq(real_email)
+    end
+  end
+end

spec/lib/gitlab/o_auth/auth_hash_spec.rb

@@ -54,29 +54,6 @@ describe Gitlab::OAuth::AuthHash, lib: true do
     it { expect(auth_hash.password).not_to be_empty }
   end
 
-  context 'with kerberos provider' do
-    let(:provider_ascii) { 'kerberos'.force_encoding(Encoding::ASCII_8BIT) }
-
-    context "and uid contains a kerberos realm" do
-      let(:uid_ascii) { 'mylogin@BAR.COM'.force_encoding(Encoding::ASCII_8BIT) }
-
-      it "preserves the canonical uid" do
-        expect(auth_hash.uid).to eq('mylogin@BAR.COM')
-      end
-    end
-
-    context "and uid does not contain a kerberos realm" do
-      let(:uid_ascii) { 'mylogin'.force_encoding(Encoding::ASCII_8BIT) }
-      before do
-        allow(Gitlab::Kerberos::Authentication).to receive(:kerberos_default_realm).and_return("FOO.COM")
-      end
-
-      it "canonicalizes uid with kerberos realm" do
-        expect(auth_hash.uid).to eq('mylogin@FOO.COM')
-      end
-    end
-  end
-
   context 'email not provided' do
     before do
       info_hash.delete(:email)