From 4a8a2f80211e6a474edb7e6920113b59579dc763 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 25 Feb 2015 17:06:57 +0100 Subject: [PATCH 1/2] Ignore LDAP SSH key prefix and invalid keys. --- lib/gitlab/ldap/person.rb | 4 +++- spec/lib/gitlab/ldap/access_spec.rb | 8 +++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/gitlab/ldap/person.rb b/lib/gitlab/ldap/person.rb index 2b02b4f0cd..cd6712d886 100644 --- a/lib/gitlab/ldap/person.rb +++ b/lib/gitlab/ldap/person.rb @@ -48,7 +48,9 @@ module Gitlab def ssh_keys if config.sync_ssh_keys? && entry.respond_to?(config.sync_ssh_keys) - entry[config.sync_ssh_keys.to_sym] + entry[config.sync_ssh_keys.to_sym]. + map { |key| key[/(ssh|ecdsa)-[^ ]+ [^\s]+/] }. + compact else [] end diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index 6696ba96c6..b21c4fccac 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -73,10 +73,10 @@ describe Gitlab::LDAP::Access do end describe :update_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) { 'sshpublickey' } + 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' } let(:entry) { - Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{ssh_key_attribute_name}: #{ssh_key}") } + Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{ssh_key_attribute_name}: SSHKey:#{ssh_key}\n#{ssh_key_attribute_name}: KerberosKey:bogus") } before do Gitlab::LDAP::Config.any_instance.stub(sync_ssh_keys: ssh_key_attribute_name) @@ -84,14 +84,12 @@ describe Gitlab::LDAP::Access do 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#{ssh_key_attribute_name}: #{ssh_key}") Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry, 'ldapmain') } expect{ access.update_ssh_keys }.to change(user.keys, :count).from(0).to(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#{ssh_key_attribute_name}: #{ssh_key}") Gitlab::LDAP::Adapter.any_instance.stub(:user) { Gitlab::LDAP::Person.new(entry, 'ldapmain') } access.update_ssh_keys From 99ffb0939ccdc47454316dbf9c9125fe4c2246fe Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sat, 28 Feb 2015 17:18:04 +0100 Subject: [PATCH 2/2] Add tests for LDAP person SSH keys. --- spec/lib/gitlab/ldap/person_spec.rb | 74 +++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 spec/lib/gitlab/ldap/person_spec.rb diff --git a/spec/lib/gitlab/ldap/person_spec.rb b/spec/lib/gitlab/ldap/person_spec.rb new file mode 100644 index 0000000000..2c35b4a30e --- /dev/null +++ b/spec/lib/gitlab/ldap/person_spec.rb @@ -0,0 +1,74 @@ +require "spec_helper" + +describe Gitlab::LDAP::Person do + + 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' } + let(:entry) { + Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com\n#{keys}") } + + subject { Gitlab::LDAP::Person.new(entry, 'ldapmain') } + + before do + Gitlab::LDAP::Config.any_instance.stub(sync_ssh_keys: ssh_key_attribute_name) + end + + context "when the SSH key is literal" do + + let(:keys) { "#{ssh_key_attribute_name}: #{ssh_key}" } + + it "includes the SSH key" do + expect(subject.ssh_keys).to include(ssh_key) + end + end + + context "when the SSH key is prefixed" do + + let(:keys) { "#{ssh_key_attribute_name}: SSHKey:#{ssh_key}" } + + it "includes the SSH key" do + expect(subject.ssh_keys).to include(ssh_key) + end + end + + context "when the SSH key is suffixed" do + + let(:keys) { "#{ssh_key_attribute_name}: #{ssh_key} (SSH key)" } + + it "includes the SSH key" do + expect(subject.ssh_keys).to include(ssh_key) + end + end + + context "when the SSH key is followed by a newline" do + + let(:keys) { "#{ssh_key_attribute_name}: #{ssh_key}\n" } + + it "includes the SSH key" do + expect(subject.ssh_keys).to include(ssh_key) + end + end + + context "when the key is not an SSH key" do + + let(:keys) { "#{ssh_key_attribute_name}: KerberosKey:bogus" } + + it "is empty" do + expect(subject.ssh_keys).to be_empty + end + end + + context "when there are multiple keys" do + + let(:keys) { "#{ssh_key_attribute_name}: #{ssh_key}\n#{ssh_key_attribute_name}: KerberosKey:bogus\n#{ssh_key_attribute_name}: ssh-rsa keykeykey" } + + it "includes both SSH keys" do + expect(subject.ssh_keys).to include(ssh_key) + expect(subject.ssh_keys).to include("ssh-rsa keykeykey") + expect(subject.ssh_keys).not_to include("KerberosKey:bogus") + end + end + end +end