Merge branch 'ee_LDAPSync_condensed' into 'master'

Synchronise SSH keys with LDAP + specs

Please see MR !7 for reference.

Please see http://feedback.gitlab.com/forums/176466-general/suggestions/5704654-synchronise-ssh-keys-with-ldap for reference.

I added specs as well as fixed some bugs.

This MR is made of two (squashed) commits on purpose, as it is the only fair way to give @jolla credit for his work too.

app/controllers/profiles/keys_controller.rb

@@ -26,7 +26,7 @@ class Profiles::KeysController < ApplicationController
 
   def destroy
     @key = current_user.keys.find(params[:id])
-    @key.destroy
+    @key.destroy unless @key.is_a? LDAPKey
 
     respond_to do |format|
       format.html { redirect_to profile_keys_url }

app/models/ldap_key.rb

+# == Schema Information
+#
+# Table name: keys
+#
+#  id         :integer          not null, primary key
+#  user_id    :integer
+#  created_at :datetime
+#  updated_at :datetime
+#  key        :text
+#  title      :string(255)
+#  identifier :string(255)
+#  type       :string(255)
+#
+
+class LDAPKey < Key
+end

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/index.html.haml

@@ -14,7 +14,7 @@
   .panel-heading
     SSH Keys (#{@keys.count})
   %ul.well-list#keys-table
-    = render @keys
+    = render partial: "key", collection: @keys
     - if @keys.blank?
       %li
         .nothing-here-block There are no SSH keys with access to your account.

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,6 +168,13 @@ production: &base
     #
     admin_group: ''
 
+    # 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
   omniauth:
     # Allow login via Twitter, Google, etc. using OmniAuth providers

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

@@ -24,13 +24,17 @@ module Gitlab
       end
 
       def update_permissions(user)
+        # Get LDAP user entry
+        ldap_user = Gitlab::LDAP::Person.find_by_dn(user.extern_uid)
+
+        if Gitlab.config.ldap['sync_ssh_keys']
+          update_ssh_keys(user)
+        end
+
         # Skip updating group permissions
         # if instance does not use group_base setting
         return true unless Gitlab.config.ldap['group_base'].present?
 
-        # Get LDAP user entry
-        ldap_user = Gitlab::LDAP::Person.find_by_dn(user.extern_uid, adapter)
-
         # Get all GitLab groups with activated LDAP
         groups = ::Group.where('ldap_cn IS NOT NULL')
 
@@ -53,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

lib/gitlab/ldap/person.rb

@@ -46,12 +46,12 @@ module Gitlab
         entry.dn
       end
 
-      private
-
       def entry
         @entry
       end
 
+      private
+
       def adapter
         @adapter ||= Gitlab::LDAP::Adapter.new
       end

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) }