adds spec for LDAPKeys (+4 squashed commits)

Squashed commits:
[812406c] fixes bug from last refactoring
[53e9508] hides "delete" button if a key is a LDAPKey
[bafc66b] refactores update_ssh_keys in own method
[7895858] makes name of ssh public key configurable

app/views/profiles/keys/_key.html.haml

@@ -6,4 +6,5 @@
   %span.cgray
     added #{time_ago_with_tooltip(key.created_at)}
 
-  = link_to 'Remove', profile_key_path(key), data: { confirm: 'Are you sure?'}, method: :delete, class: "btn btn-small btn-remove delete-key pull-right"
+  - unless key.is_a? LDAPKey
+    = link_to 'Remove', profile_key_path(key), data: { confirm: 'Are you sure?'}, method: :delete, class: "btn btn-small btn-remove delete-key pull-right"

app/views/profiles/keys/show.html.haml

@@ -19,4 +19,5 @@
       = @key.key
 
 .pull-right
-  = link_to 'Remove', profile_key_path(@key), data: {confirm: 'Are you sure?'}, method: :delete, class: "btn btn-remove delete-key"
+  - unless @key.is_a? LDAPKey
+    = link_to 'Remove', profile_key_path(@key), data: {confirm: 'Are you sure?'}, method: :delete, class: "btn btn-remove delete-key"

config/gitlab.yml.example

@@ -168,7 +168,11 @@ production: &base
     #
     admin_group: ''
 
-    # Allow synchronising SSH public keys with LDAP
+    # Name of attribute which holds a ssh public key of the user object.
+    # If false or nil, SSH key syncronisation will be disabled.
+    #
+    #   Ex. sshpublickey
+    #
     sync_ssh_keys: false
 
   ## OmniAuth settings

doc/integration/ldap.md

@@ -82,3 +82,24 @@ For installations from source, add the following setting in the 'ldap' section o
 ```yaml
     admin_group: 'Gitlab administrators'
 ```
+
+## Synchronising user SSH keys with LDAP
+
+It is possible to configure GitLab Enterprise Edition (7.1 and newer) so that users have their SSH public keys synchronised with an attribute in their LDAP object.
+Existing SSH public keys that are manually manged in GitLab are not affected by this feature.
+
+### Enabling the key synchronisation feature
+
+Below we assume that you have LDAP users with an attribute  'sshpublickey' containing the users ssh public key.
+
+For omnibus-gitlab, add the following to `/etc/gitlab/gitlab.rb` and run `gitlab-ctl reconfigure`.
+
+```ruby
+gitlab_rails['ldap_sync_ssh_keys'] = 'sshpublickey'
+```
+
+For installations from source, add the following setting in the 'ldap' section of gitlab.yml, and run `service gitlab reload` afterwards.
+
+```yaml
+    sync_ssh_keys: 'sshpublickey'
+```
\ No newline at end of file

lib/gitlab/ldap/access.rb

@@ -28,28 +28,7 @@ module Gitlab
         ldap_user = Gitlab::LDAP::Person.find_by_dn(user.extern_uid)
 
         if Gitlab.config.ldap['sync_ssh_keys']
-          if ldap_user.entry.respond_to?(:sshpublickey)
-            sshkeys = ldap_user.entry.sshpublickey
-          else
-            sshkeys = []
-          end
-          sshkeys.each do |key|
-            k = user.keys.find_by_key(key)
-            if k && !k.is_a?(LDAPKey)
-              k.destroy
-              k = nil
-            end
-            unless k
-              k = LDAPKey.new(title: "LDAP Key", key: key)
-              k.save
-              user.keys << k
-            end
-          end
-          user.keys.all.each do |k|
-            if k.is_a?(LDAPKey) && !sshkeys.include?(k.key)
-              k.destroy
-            end
-          end
+          update_ssh_keys(user)
         end
 
         # Skip updating group permissions
@@ -78,6 +57,30 @@ module Gitlab
         end
       end
 
+      # Update user ssh keys if they changed in LDAP
+      def update_ssh_keys(user)
+        # Get LDAP user entry
+        ldap_user = Gitlab::LDAP::Person.find_by_dn(user.extern_uid)
+
+        if ldap_user.entry.respond_to?(Gitlab.config.ldap['sync_ssh_keys'].to_sym)
+          sshkeys = ldap_user.entry[Gitlab.config.ldap['sync_ssh_keys'].to_sym]
+        else
+          sshkeys = []
+        end
+        sshkeys.each do |key|
+          unless user.keys.find_by_key(key)
+            k = LDAPKey.new(title: "LDAP - #{Gitlab.config.ldap['sync_ssh_keys']}", key: key)
+            user.keys << k if k.save
+          end
+        end
+        user.keys.to_a.each do |k|
+          if k.is_a?(LDAPKey) && !sshkeys.include?(k.key)
+            user.keys.delete(k)
+            k.destroy
+          end
+        end
+      end
+
       # Update user email if it changed in LDAP
       def update_email(user)
         uid = user.extern_uid

spec/lib/gitlab/ldap/ldap_access_spec.rb

@@ -67,6 +67,74 @@ describe Gitlab::LDAP::Access do
     end
   end
 
+  describe :update_ssh_keys do
+    let(:user_ldap) { create(:user, provider: 'ldap', extern_uid: "66049")}
+    let(:ssh_key) { 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCrSQHff6a1rMqBdHFt+FwIbytMZ+hJKN3KLkTtOWtSvNIriGhnTdn4rs+tjD/w+z+revytyWnMDM9dS7J8vQi006B16+hc9Xf82crqRoPRDnBytgAFFQY1G/55ql2zdfsC5yvpDOFzuwIJq5dNGsojS82t6HNmmKPq130fzsenFnj5v1pl3OJvk513oduUyKiZBGTroWTn7H/eOPtu7s9MD7pAdEjqYKFLeaKmyidiLmLqQlCRj3Tl2U9oyFg4PYNc0bL5FZJ/Z6t0Ds3i/a2RanQiKxrvgu3GSnUKMx7WIX373baL4jeM7cprRGiOY/1NcS+1cAjfJ8oaxQF/1dYj' }
+    let(:key_ldap) { LDAPKey.new(title: 'used to be a ldap key', key: ssh_key) }
+
+    before do
+      @old_value = Gitlab.config.ldap['sync_ssh_keys']
+      key_attribute_name = 'sshpublickey'
+      Gitlab.config.ldap['sync_ssh_keys'] = key_attribute_name
+    end
+
+    after do
+      Gitlab.config.ldap['sync_ssh_keys'] = @old_value
+    end
+
+    it "should add a SSH key if it is in LDAP but not in gitlab" do
+      entry = Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{Gitlab.config.ldap['sync_ssh_keys']}: #{ssh_key}")
+      Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
+
+      expect(user_ldap.keys.size).to be(0)
+      access.update_ssh_keys(user_ldap)
+      expect(user_ldap.keys.size).to be(1)
+    end
+
+    it "should add a SSH key and give it a proper name" do
+      entry = Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{Gitlab.config.ldap['sync_ssh_keys']}: #{ssh_key}")
+      Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
+
+      access.update_ssh_keys(user_ldap)
+      expect(user_ldap.keys.last.title).to match(/LDAP/)
+      expect(user_ldap.keys.last.title).to match(/#{Gitlab.config.ldap['sync_ssh_keys']}/)
+    end
+
+    it "should not add a SSH key if it is invalid" do
+      entry = Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{Gitlab.config.ldap['sync_ssh_keys']}: I am not a valid key")
+      Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
+
+      expect(user_ldap.keys.size).to be(0)
+      access.update_ssh_keys(user_ldap)
+      expect(user_ldap.keys.size).to be(0)
+    end
+
+    context 'user has at least one LDAPKey' do
+
+      it "should remove a SSH key if it is no longer in LDAP" do
+        entry = Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{Gitlab.config.ldap['sync_ssh_keys']}:\n")
+        Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
+        key_ldap.save
+        user_ldap.keys << key_ldap
+
+        expect(user_ldap.keys.size).to be(1)
+        access.update_ssh_keys(user_ldap)
+        expect(user_ldap.keys.size).to be(0)
+      end
+
+      it "should remove a SSH key if the ldap attribute was removes" do
+        entry = Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com")
+        Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry) }
+        key_ldap.save
+        user_ldap.keys << key_ldap
+        expect(user_ldap.keys.size).to be(1)
+        access.update_ssh_keys(user_ldap)
+        expect(user_ldap.keys.size).to be(0)
+      end
+    end
+
+  end
+
   describe :allowed? do
     subject { access.allowed?(user) }