Merge branch 'restrict_attrs_inc_ssh_pubkey' into 'master'

Add ssh key attribute to return attributes

I realized that https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/712 would cause a regression with the SSH public key sync. This merge request fixes that functionality. 

Also, this uses the `Module#prepend` method that @DouweM told me about a while ago. It ends up being a **really** slick way to override a CE method and make future merges really smooth. I like this a lot!

Part of this has to be changed in CE because the `#user_attributes` method didn't exist before. I made that change here, and in CE at <LINK MR HERE>.

@DouweM What do you think about this usage of `prepend`?

cc/ @jacobvosmaer-gitlab

See merge request !736

lib/ee/gitlab/ldap/adapter.rb

 # LDAP connection adapter EE mixin
 #
 # This module is intended to encapsulate EE-specific adapter methods
-# and be included in the `Gitlab::LDAP::Adapter` class.
+# and be **prepended** in the `Gitlab::LDAP::Adapter` class.
 module EE
   module Gitlab
     module LDAP
@@ -52,6 +52,12 @@ module EE
             LDAP::Group.new(entry, self)
           end
         end
+
+        def user_attributes
+          attributes = super
+          attributes << config.sync_ssh_keys if config.sync_ssh_keys
+          attributes
+        end
       end
     end
   end

lib/gitlab/ldap/adapter.rb

@@ -5,7 +5,7 @@
 module Gitlab
   module LDAP
     class Adapter
-      include EE::Gitlab::LDAP::Adapter
+      prepend EE::Gitlab::LDAP::Adapter
 
       attr_reader :provider, :ldap
 
@@ -76,7 +76,7 @@ module Gitlab
       private
 
       def user_options(field, value, limit)
-        options = { attributes: %W(#{config.uid} cn mail dn) }
+        options = { attributes: user_attributes }
         options[:size] = limit if limit
 
         if field.to_sym == :dn
@@ -104,6 +104,10 @@ module Gitlab
           filter
         end
       end
+
+      def user_attributes
+        %W(#{config.uid} cn mail dn)
+      end
     end
   end
 end

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

@@ -7,9 +7,9 @@ describe Gitlab::LDAP::Adapter, lib: true do
     expect(Gitlab::LDAP::Adapter).to include_module(EE::Gitlab::LDAP::Adapter)
   end
 
-  describe '#groups' do
-    let(:adapter) { ldap_adapter('ldapmain') }
+  let(:adapter) { ldap_adapter('ldapmain') }
 
+  describe '#groups' do
     before do
       stub_ldap_config(
         group_base: 'ou=groups,dc=example,dc=com',
@@ -39,4 +39,11 @@ describe Gitlab::LDAP::Adapter, lib: true do
       expect(results.first.member_dns).to match_array(%w(john mary))
     end
   end
+
+  describe '#user_attributes' do
+    it 'appends EE-specific attributes' do
+      stub_ldap_config(uid: 'uid', sync_ssh_keys: 'sshPublicKey')
+      expect(adapter.user_attributes).to match_array(%w(uid dn cn mail sshPublicKey))
+    end
+  end
 end

spec/requests/api/ldap_spec.rb

@@ -2,7 +2,10 @@ require 'spec_helper'
 
 describe API::API do
   include ApiHelpers
+  include LdapHelpers
+
   let(:user) { create(:user) }
+  let(:adapter) { ldap_adapter }
 
   before do
     groups = [
@@ -10,7 +13,8 @@ describe API::API do
       OpenStruct.new(cn: 'students')
     ]
 
-    allow_any_instance_of(Gitlab::LDAP::Adapter).to receive_messages(groups: groups)
+    allow(Gitlab::LDAP::Adapter).to receive(:new).and_return(adapter)
+    allow(adapter).to receive_messages(groups: groups)
   end
 
   describe "GET /ldap/groups" do