Extend specs with verify_mode and tls certificate

app/models/ldap_auth_source.rb

@@ -143,6 +143,22 @@ class LdapAuthSource < AuthSource
     Net::LDAP::Filter.from_rfc2254(filter_string) if filter_string.present?
   end
 
+  def ldap_connection_options
+    {
+      host:,
+      port:,
+      force_no_page: OpenProject::Configuration.ldap_force_no_page,
+      encryption: ldap_encryption
+    }
+  end
+
+  def read_ldap_certificates
+    return if tls_certificate_string.blank?
+
+    # Using load will allow multiple PEM certificates to be passed
+    OpenSSL::X509::Certificate.load(tls_certificate_string)
+  end
+
   private
 
   def strip_ldap_attributes
@@ -156,10 +172,7 @@ class LdapAuthSource < AuthSource
       Rails.logger.info { "SSL connection to LDAP host #{host} is set up to skip certificate verification." }
     end
 
-    options = { host: host,
-                port: port,
-                force_no_page: OpenProject::Configuration.ldap_force_no_page,
-                encryption: ldap_encryption }
+    options = ldap_connection_options
     unless ldap_user.blank? && ldap_password.blank?
       options.merge!(auth: { method: :simple, username: ldap_user,
                              password: ldap_password })
@@ -172,7 +185,7 @@ class LdapAuthSource < AuthSource
 
     {
       method: tls_mode.to_sym,
-      tls_options:,
+      tls_options:
     }
   end
 
@@ -191,13 +204,6 @@ class LdapAuthSource < AuthSource
     }.compact
   end
 
-  def read_ldap_certificates
-    return if tls_certificate_string.blank?
-
-    # Using load will allow multiple PEM certificates to be passed
-    OpenSSL::X509::Certificate.load(tls_certificate_string)
-  end
-
   def tls_verify_mode
     if verify_peer?
       OpenSSL::SSL::VERIFY_PEER
@@ -227,11 +233,12 @@ class LdapAuthSource < AuthSource
     ldap_con.search(base: base_dn,
                     filter: filter,
                     attributes: search_attributes) do |entry|
-      attrs = if onthefly_register?
-                get_user_attributes_from_ldap_entry(entry)
-              else
-                { dn: entry.dn }
-              end
+      attrs =
+        if onthefly_register?
+          get_user_attributes_from_ldap_entry(entry)
+        else
+          { dn: entry.dn }
+        end
 
       Rails.logger.debug { "DN found for #{login}: #{attrs[:dn]}" }
     end

spec/factories/auth_source_factory.rb

@@ -35,6 +35,7 @@ FactoryBot.define do
     host { '127.0.0.1' }
     port { 225 }  # a reserved port, should not be in use
     attr_login { 'uid' }
+    tls_mode { 'plain_ldap' }
   end
 
   factory :dummy_auth_source, class: 'DummyAuthSource' do

spec/fixtures/ldap/snakeoil.pem

@@ -0,0 +1,46 @@
+-----BEGIN CERTIFICATE-----
+MIIDczCCAlugAwIBAgIER8Y4EjANBgkqhkiG9w0BAQsFADBqMRAwDgYDVQQGEwdV
+bmtub3duMRAwDgYDVQQIEwdVbmtub3duMRAwDgYDVQQHEwdVbmtub3duMRAwDgYD
+VQQKEwdleGFtcGxlMQwwCgYDVQQLEwNjb20xEjAQBgNVBAMTCWxvY2FsaG9zdDAe
+Fw0yMjExMTUxMzE5MDFaFw0yMzAyMTMxMzE5MDFaMGoxEDAOBgNVBAYTB1Vua25v
+d24xEDAOBgNVBAgTB1Vua25vd24xEDAOBgNVBAcTB1Vua25vd24xEDAOBgNVBAoT
+B2V4YW1wbGUxDDAKBgNVBAsTA2NvbTESMBAGA1UEAxMJbG9jYWxob3N0MIIBIjAN
+BgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAhktWeAGMy0+sxoTvNRmTrFpUZ6qI
+ou+okofrAEULM+VdiHxfJk4Fsy9B1IgY1LvKJ8jA+MAmX4pBq+5KUFY2hIzMM4yR
+RcvXH9DvzzthkUQLtRWnVmgMPeDS0cTFaxnjElrnM+OzRPTENlv0cn89+Fy2DgQp
+Xoc8WXJmCyy94jVtGbbZgpZbIyGVjG7q5Ht4Fz4bFyjMjS0//JoLTbPuiAZ5bdkI
++eNLxHy8BLIbyCmgxdTXI7hpRCbhfbFY5Cgrmr0O2iP/UI8woUQkecUT+zxsJbSb
+eCb5YmBM35Y/DwxS1wE3JEX4XzyxXMpKWeOB/UQ6HAneHzzSDOihbRYUWQIDAQAB
+oyEwHzAdBgNVHQ4EFgQUWOl3qIbcOWgKoyeJKg3DyzFnW+kwDQYJKoZIhvcNAQEL
+BQADggEBAADXn6mIlqtHcGNAzcjGsVZFTsfHV5bR4Ohi6ms8IMJ+OS0vSYKwzSIy
+38U58xBR+gfQrbzZ+FE9XOym/g63QoQcHe/ERr9fI1dxNQKHliWbd1XtzjcAIelu
+J7Xg9C3eHrlhxQ8Cc0/eFb84vSD+0XlWvd0AOwTWF+rZ5+McrLJu35ZiJtDhBS9P
+2ISPbkZ7eCaeBD3OApJjbo+05IqZsi/9BMY5keBmZijYDIUWozpGCNFQ6tYs8BEm
+efJgyJ3dSOFvS90jqNMnPumEpIfP4BN2RDH2ygmmbbPemsA8Hl/OIy+5qG6x/6Zy
+SFMzIkQvcS1ZgJpD44mZhFzpSJUsD5s=
+-----END CERTIFICATE-----
+
+-----BEGIN CERTIFICATE-----
+MIID7zCCAtegAwIBAgIUdGNOIw1SvmiPaeilr42CkCeMeTowDQYJKoZIhvcNAQEL
+BQAwgYYxCzAJBgNVBAYTAkRFMQ8wDQYDVQQIDAZIZXNzZW4xEjAQBgNVBAcMCURh
+cm1zdGFkdDEPMA0GA1UECgwGVGVzdGVyMRQwEgYDVQQLDAtMREFQIHRlc3RlcjEL
+MAkGA1UEAwwCQ0ExHjAcBgkqhkiG9w0BCQEWD2Zvb0BleGFtcGxlLmNvbTAeFw0y
+MDA1MTQxMTUwMThaFw0yMDA2MTMxMTUwMThaMIGGMQswCQYDVQQGEwJERTEPMA0G
+A1UECAwGSGVzc2VuMRIwEAYDVQQHDAlEYXJtc3RhZHQxDzANBgNVBAoMBlRlc3Rl
+cjEUMBIGA1UECwwLTERBUCB0ZXN0ZXIxCzAJBgNVBAMMAkNBMR4wHAYJKoZIhvcN
+AQkBFg9mb29AZXhhbXBsZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK
+AoIBAQDEshEY/CwpzuyuCdG9H8EvlEZystY6hhHXFBh/rCf4KjWPFURTki5Ha1/U
+6/PyjPEkdgmUF8Px7un+4FJNgSJnRs5Hz4b7GsHOF8JaHf6JZQy+0Mvp9WI81jcM
+i98n1oE2WUDSrlXi1qs3okgX63rhCPjrXL3WCdo/Ef2EtW8usSinXZ0Vfd+3lQoh
+9o0lC8UELot0F5/505NvCYmYNTFNsfiOUNvC++qwLl2bNrk/qPYRIHBh3I8lxF36
+ksii/nFijZaQh0mnXIXeewz5eInzJvTfWH47U3ioXD3iJtHc3pkZ1qJD+UELULDO
+8JU05buSJfLpT64goP+/TQpqzWN5AgMBAAGjUzBRMB0GA1UdDgQWBBS9qy2N1L26
+/2JYGhcyIaadI1DTCjAfBgNVHSMEGDAWgBS9qy2N1L26/2JYGhcyIaadI1DTCjAP
+BgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQBE7lFSeWuWhtb9IZRu
+Z9lV10e+XK3uWpPt9/dyOcR5LvGH196jHnak/vV84uDhudWI2rZlm10DsLEE6J0i
+yI/039yd0HouOA5MpxBAWkKVgM3AefXgeuuQOIsU6QShnMikhqurw8Y2WGt9PT1V
+wSZW9g/CpI8hO8XEX/AriePh62gD3XSS8kSa40nIMTiOzY18orORC+5jFBK50P2/
+CH3cdpePXJnvIM6aYPjMemojg1PN5MmJHUdFBvGHuo2e1CAp7KrI4LPIXZVMxI1z
+jjE7Rd9gfKPhQ0nT6WpXEqovvnliTum+teh2pJ5y3a31jD6xp09ST9JYmDUZwNIm
+HT64
+-----END CERTIFICATE-----

spec/models/ldap_auth_source_spec.rb

@@ -30,28 +30,108 @@ require 'spec_helper'
 
 describe LdapAuthSource, type: :model do
   it 'creates' do
-    a = LdapAuthSource.new(name: 'My LDAP', host: 'ldap.example.net', port: 389, base_dn: 'dc=example,dc=net',
+    a = described_class.new(name: 'My LDAP', host: 'ldap.example.net', port: 389, base_dn: 'dc=example,dc=net',
                            attr_login: 'sAMAccountName')
     expect(a.save).to be true
   end
 
   it 'strips ldap attributes' do
-    a = LdapAuthSource.new(name: 'My LDAP', host: 'ldap.example.net', port: 389, base_dn: 'dc=example,dc=net', attr_login: 'sAMAccountName',
+    a = described_class.new(name: 'My LDAP', host: 'ldap.example.net', port: 389,
+                            base_dn: 'dc=example,dc=net', attr_login: 'sAMAccountName',
                            attr_firstname: 'givenName ')
     expect(a.save).to be true
     expect(a.reload.attr_firstname).to eq 'givenName'
   end
 
-  describe 'overriding tls_options',
-           with_settings: { ldap_tls_options: { ca_file: '/path/to/ca/file' } } do
-    it 'sets the encryption options for start_tls' do
-      ldap = LdapAuthSource.new tls_mode: :start_tls
-      expect(ldap.send(:ldap_encryption)).to eq(method: :start_tls, tls_options: { 'ca_file' => '/path/to/ca/file' })
+  describe 'verify_peer' do
+    let(:tls_mode) { :start_tls }
+    let(:ldap) { described_class.new tls_mode:, verify_peer: }
+
+    subject { ldap.ldap_connection_options[:encryption] }
+
+    context 'when set' do
+      let(:verify_peer) { true }
+
+      it 'reflects in tls_options' do
+        expect(subject).to have_key :tls_options
+        expect(subject[:tls_options]).to match(hash_including(verify_mode: OpenSSL::SSL::VERIFY_PEER))
+      end
+    end
+
+    context 'when set, but tls_mode differs' do
+      let(:tls_mode) { :plain_ldap }
+      let(:verify_peer) { true }
+
+      it 'does nothing' do
+        expect(subject).to be_nil
+      end
+    end
+
+    context 'when unset' do
+      let(:verify_peer) { false }
+
+      it 'reflects in tls_options' do
+        expect(subject).to have_key :tls_options
+        expect(subject[:tls_options]).to match(hash_including(verify_mode: OpenSSL::SSL::VERIFY_NONE))
+      end
+    end
+  end
+
+  describe 'cert_store' do
+    let(:fixture) { Rails.root.join('spec/fixtures/ldap/snakeoil.pem') }
+    let(:ldap) { build(:ldap_auth_source, tls_mode: :start_tls, tls_certificate_string: File.read(fixture)) }
+    let(:store_double) { instance_double(OpenSSL::X509::Store) }
+
+    subject { ldap.ldap_connection_options.dig(:encryption, :tls_options) }
+
+    it 'adds the certificates to the store' do
+      allow(OpenSSL::X509::Store).to receive(:new).and_return(store_double)
+      allow(store_double).to receive(:set_default_paths)
+      allow(store_double).to receive(:add_cert)
+
+      expect(subject).to have_key :cert_store
+      expect(subject[:cert_store]).to eq store_double
+
+      expect(store_double).to have_received(:add_cert).twice
+    end
+  end
+
+  describe 'tls_certificate_string' do
+    let(:ldap) { build(:ldap_auth_source, tls_certificate_string:) }
+
+    subject { ldap.read_ldap_certificates }
+
+    context 'when single certificate' do
+      let(:fixture) { Rails.root.join('spec/fixtures/ldap/snakeoil.pem') }
+      let(:tls_certificate_string) { File.read(fixture).split(/^$/)[0] }
+
+      it 'is valid' do
+        expect(ldap).to be_valid
+        expect(subject).to be_a Array
+        expect(subject.length).to eq 1
+        expect(subject).to all(be_a(OpenSSL::X509::Certificate))
+      end
+    end
+
+    context 'when multiple certificates' do
+      let(:fixture) { Rails.root.join('spec/fixtures/ldap/snakeoil.pem') }
+      let(:tls_certificate_string) { File.read(fixture) }
+
+      it 'is valid' do
+        expect(ldap).to be_valid
+        expect(subject).to be_a Array
+        expect(subject.length).to eq 2
+        expect(subject).to all(be_a(OpenSSL::X509::Certificate))
+      end
     end
 
-    it 'does nothing for plain_ldap' do
-      ldap = LdapAuthSource.new tls_mode: :plain_ldap
-      expect(ldap.send(:ldap_encryption)).to be_nil
+    context 'when bogus content' do
+      let(:tls_certificate_string) { 'foo' }
+
+      it 'is invalid' do
+        expect(ldap).not_to be_valid
+        expect { subject }.to raise_error(OpenSSL::X509::CertificateError)
+      end
     end
   end
 
@@ -151,6 +231,7 @@ describe LdapAuthSource, type: :model do
     let(:ldap) do
       create :ldap_auth_source,
              port: ParallelHelper.port_for_ldap.to_s,
+             tls_mode: :plain_ldap,
              account: 'uid=admin,ou=system',
              account_password: 'secret',
              base_dn: 'ou=people,dc=example,dc=com',