From c21c174ab8824ed545ea656e8a521b147bdba19f Mon Sep 17 00:00:00 2001
From: DJ Mountney <dj@gitlab.com>
Date: Tue, 20 Oct 2020 13:19:27 -0700
Subject: [PATCH 1/8] Add encrypted ldap secrets support

Adds a new ldap_secrets path setting for optional ldap secrets file.
And adds a new set of ldap secrets rake commands for showing/editing an
ldap secret

Adds support for loading ldap servers' password and/or user_bn from the
encrypted secrets.

As part of initial rollout we don't want to complain about secrets for
an optional feature.
---
 changelogs/unreleased/ldap-secret-command.yml |  5 ++
 config/initializers/1_settings.rb             |  1 +
 lib/gitlab/auth/ldap/config.rb                | 34 +++++--
 lib/gitlab/encrypted_ldap_command.rb          | 80 +++++++++++++++++
 lib/tasks/gitlab/ldap.rake                    | 18 ++++
 spec/tasks/gitlab/ldap_rake_spec.rb           | 90 +++++++++++++++++++
 6 files changed, 223 insertions(+), 5 deletions(-)
 create mode 100644 changelogs/unreleased/ldap-secret-command.yml
 create mode 100644 lib/gitlab/encrypted_ldap_command.rb

diff --git a/changelogs/unreleased/ldap-secret-command.yml b/changelogs/unreleased/ldap-secret-command.yml
new file mode 100644
index 00000000000000..a1c0617867c41e
--- /dev/null
+++ b/changelogs/unreleased/ldap-secret-command.yml
@@ -0,0 +1,5 @@
+---
+title: Add encrypted ldap secrets support
+merge_request: 45712
+author:
+type: added
diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb
index 2da7b50cbfc8e8..65f3898aa87e53 100644
--- a/config/initializers/1_settings.rb
+++ b/config/initializers/1_settings.rb
@@ -13,6 +13,7 @@
 Settings['ldap'] ||= Settingslogic.new({})
 Settings.ldap['enabled'] = false if Settings.ldap['enabled'].nil?
 Settings.ldap['prevent_ldap_sign_in'] = false if Settings.ldap['prevent_ldap_sign_in'].blank?
+Settings.ldap['secret_file'] = Settings.absolute(Settings.ldap['secret_file'] || File.join(Settings.encrypted_settings['path'], "ldap.yaml.enc"))
 
 Gitlab.ee do
   Settings.ldap['sync_time'] = 3600 if Settings.ldap['sync_time'].nil?
diff --git a/lib/gitlab/auth/ldap/config.rb b/lib/gitlab/auth/ldap/config.rb
index 88cc840c395f16..f5931a1d5ebb87 100644
--- a/lib/gitlab/auth/ldap/config.rb
+++ b/lib/gitlab/auth/ldap/config.rb
@@ -53,6 +53,10 @@ def self.invalid_provider(provider)
           raise InvalidProvider.new("Unknown provider (#{provider}). Available providers: #{providers}")
         end
 
+        def self.encrypted_secrets
+          Settings.encrypted(Gitlab.config.ldap.secret_file)
+        end
+
         def initialize(provider)
           if self.class.valid_provider?(provider)
             @provider = provider
@@ -89,8 +93,8 @@ def omniauth_options
 
           if has_auth?
             opts.merge!(
-              bind_dn: options['bind_dn'],
-              password: options['password']
+              bind_dn: auth_username,
+              password: auth_password
             )
           end
 
@@ -155,7 +159,7 @@ def external_groups
         end
 
         def has_auth?
-          options['password'] || options['bind_dn']
+          auth_password || auth_username
         end
 
         def allow_username_or_email_login
@@ -267,12 +271,32 @@ def auth_options
           {
             auth: {
               method: :simple,
-              username: options['bind_dn'],
-              password: options['password']
+              username: auth_username,
+              password: auth_password
             }
           }
         end
 
+        def secrets
+          @secrets ||= self.class.encrypted_secrets[@provider.delete_prefix('ldap').to_sym]
+        rescue => e
+          Gitlab::AppLogger.error "LDAP encrypted secrets are invalid: #{e.inspect}"
+
+          nil
+        end
+
+        def auth_password
+          return options['password'] if options['password']
+
+          secrets&.fetch(:password, nil)&.chomp
+        end
+
+        def auth_username
+          return options['bind_dn'] if options['bind_dn']
+
+          secrets&.fetch(:bind_dn, nil)&.chomp
+        end
+
         def omniauth_user_filter
           uid_filter = Net::LDAP::Filter.eq(uid, '%{username}')
 
diff --git a/lib/gitlab/encrypted_ldap_command.rb b/lib/gitlab/encrypted_ldap_command.rb
new file mode 100644
index 00000000000000..e86aa3bf0d6bfb
--- /dev/null
+++ b/lib/gitlab/encrypted_ldap_command.rb
@@ -0,0 +1,80 @@
+# frozen_string_literal: true
+
+# rubocop:disable Rails/Output
+module Gitlab
+  class EncryptedLdapCommand
+    class << self
+      def write(contents)
+        encrypted = Gitlab::Auth::Ldap::Config.encrypted_secrets
+        return unless validate_config(encrypted)
+
+        encrypted.write(contents)
+
+        puts "File encrypted and saved."
+      rescue Interrupt
+        puts "Aborted changing file: nothing saved."
+      rescue Gitlab::EncryptedConfiguration::MissingKeyError
+        puts "Missing encryption key enc_settings_key_base."
+      rescue ActiveSupport::MessageEncryptor::InvalidMessage
+        puts "Couldn't decrypt #{encrypted.content_path}. Perhaps you passed the wrong key?"
+      end
+
+      def edit
+        encrypted = Gitlab::Auth::Ldap::Config.encrypted_secrets
+        return unless validate_config(encrypted)
+
+        editor = ENV['EDITOR'] || 'editor'
+        temp_file = Tempfile.new(File.basename(encrypted.content_path))
+
+        encrypted.change do |contents|
+          contents = encrypted_file_template unless File.exist?(encrypted.content_path)
+          File.write(temp_file.path, contents)
+          system("#{editor} #{temp_file.path}")
+          File.read(temp_file.path)
+        end
+
+        puts "File encrypted and saved."
+      rescue Interrupt
+        puts "Aborted changing file: nothing saved."
+      rescue Gitlab::EncryptedConfiguration::MissingKeyError
+        puts "Missing encryption key enc_settings_key_base."
+      rescue ActiveSupport::MessageEncryptor::InvalidMessage
+        puts "Couldn't decrypt #{encrypted.content_path}. Perhaps you passed the wrong key?"
+      ensure
+        temp_file&.unlink
+      end
+
+      def show
+        encrypted = Gitlab::Auth::Ldap::Config.encrypted_secrets
+
+        puts encrypted.read.presence || "File '#{encrypted.content_path}' does not exist. Use `rake gitlab:ldap:secret:edit` to change that."
+      rescue Gitlab::EncryptedConfiguration::MissingKeyError
+        puts "Missing encryption key enc_settings_key_base."
+      rescue ActiveSupport::MessageEncryptor::InvalidMessage
+        puts "Couldn't decrypt #{encrypted.content_path}. Perhaps you passed the wrong key?"
+      end
+
+      private
+
+      def validate_config(encrypted)
+        dir_path = File.dirname(encrypted.content_path)
+
+        unless File.exist?(dir_path)
+          puts "Directory #{dir_path} does not exist. Create the directory and try again."
+          return false
+        end
+
+        true
+      end
+
+      def encrypted_file_template
+        <<~YAML
+          # main:
+          #   password: '123'
+          #   user_bn: 'gitlab-adm'
+        YAML
+      end
+    end
+  end
+end
+# rubocop:enable Rails/Output
diff --git a/lib/tasks/gitlab/ldap.rake b/lib/tasks/gitlab/ldap.rake
index 0459de27c965ca..4171bd70f1a35b 100644
--- a/lib/tasks/gitlab/ldap.rake
+++ b/lib/tasks/gitlab/ldap.rake
@@ -36,5 +36,23 @@ namespace :gitlab do
         puts "Successfully updated #{plural_updated_count} out of #{plural_id_count} total"
       end
     end
+
+    namespace :secret do
+      desc 'GitLab | LDAP | Secret | Write ldap secrets'
+      task write: [:environment] do
+        content = STDIN.tty? ? STDIN.gets : STDIN.read
+        Gitlab::EncryptedLdapCommand.write(content)
+      end
+
+      desc 'GitLab | LDAP | Secret | Edit ldap secrets'
+      task edit: [:environment] do
+        Gitlab::EncryptedLdapCommand.edit
+      end
+
+      desc 'GitLab | LDAP | Secret | Show ldap secrets'
+      task show: [:environment] do
+        Gitlab::EncryptedLdapCommand.show
+      end
+    end
   end
 end
diff --git a/spec/tasks/gitlab/ldap_rake_spec.rb b/spec/tasks/gitlab/ldap_rake_spec.rb
index 275fcb98dd0e96..f0647c498b6ffb 100644
--- a/spec/tasks/gitlab/ldap_rake_spec.rb
+++ b/spec/tasks/gitlab/ldap_rake_spec.rb
@@ -13,3 +13,93 @@
     run_rake_task('gitlab:ldap:rename_provider', 'ldapmain', 'ldapfoo')
   end
 end
+
+RSpec.describe 'gitlab:ldap:secret rake tasks' do
+  let(:ldap_secret_file) { 'tmp/tests/ldapenc/ldap_secret.yaml.enc' }
+
+  before do
+    Rake.application.rake_require 'tasks/gitlab/ldap'
+    stub_env('EDITOR', 'cat')
+    stub_warn_user_is_not_gitlab
+    FileUtils.mkdir_p('tmp/tests/ldapenc/')
+    allow(Gitlab.config.ldap).to receive(:secret_file).and_return(ldap_secret_file)
+    allow(Gitlab::Application.secrets).to receive(:enc_settings_key_base).and_return(SecureRandom.hex(64))
+  end
+
+  after do
+    FileUtils.rm_rf('tmp/tests/ldapenc/')
+  end
+
+  describe ':show' do
+    it 'displays error when file does not exist' do
+      expect { run_rake_task('gitlab:ldap:secret:show') }.to output(/File .* does not exist. Use `rake gitlab:ldap:secret:edit` to change that./).to_stdout
+    end
+
+    it 'displays error when key does not exist' do
+      Settings.encrypted(ldap_secret_file).write('somevalue')
+      allow(Gitlab::Application.secrets).to receive(:enc_settings_key_base).and_return(nil)
+      expect { run_rake_task('gitlab:ldap:secret:show') }.to output(/Missing encryption key enc_settings_key_base./).to_stdout
+    end
+
+    it 'displays error when key is changed' do
+      Settings.encrypted(ldap_secret_file).write('somevalue')
+      allow(Gitlab::Application.secrets).to receive(:enc_settings_key_base).and_return(SecureRandom.hex(64))
+      expect { run_rake_task('gitlab:ldap:secret:show') }.to output(/Couldn't decrypt .* Perhaps you passed the wrong key?/).to_stdout
+    end
+
+    it 'outputs the unencrypted content when present' do
+      encrypted = Settings.encrypted(ldap_secret_file)
+      encrypted.write('somevalue')
+      expect { run_rake_task('gitlab:ldap:secret:show') }.to output(/somevalue/).to_stdout
+    end
+  end
+
+  describe 'edit' do
+    it 'creates encrypted file' do
+      expect { run_rake_task('gitlab:ldap:secret:edit') }.to output(/File encrypted and saved./).to_stdout
+      expect(File.exist?(ldap_secret_file)).to be true
+      value = Settings.encrypted(ldap_secret_file)
+      expect(value.read).to match(/password: '123'/)
+    end
+
+    it 'displays error when key does not exist' do
+      allow(Gitlab::Application.secrets).to receive(:enc_settings_key_base).and_return(nil)
+      expect { run_rake_task('gitlab:ldap:secret:edit') }.to output(/Missing encryption key enc_settings_key_base./).to_stdout
+    end
+
+    it 'displays error when key is changed' do
+      Settings.encrypted(ldap_secret_file).write('somevalue')
+      allow(Gitlab::Application.secrets).to receive(:enc_settings_key_base).and_return(SecureRandom.hex(64))
+      expect { run_rake_task('gitlab:ldap:secret:edit') }.to output(/Couldn't decrypt .* Perhaps you passed the wrong key?/).to_stdout
+    end
+
+    it 'displays error when write directory does not exist' do
+      FileUtils.rm_rf('tmp/tests/ldapenc/')
+      expect { run_rake_task('gitlab:ldap:secret:edit') }.to output(/Directory .* does not exist./).to_stdout
+    end
+  end
+
+  describe 'write' do
+    before do
+      allow(STDIN).to receive(:tty?).and_return(false)
+      allow(STDIN).to receive(:read).and_return('testvalue')
+    end
+
+    it 'creates encrypted file from stdin' do
+      expect { run_rake_task('gitlab:ldap:secret:write') }.to output(/File encrypted and saved./).to_stdout
+      expect(File.exist?(ldap_secret_file)).to be true
+      value = Settings.encrypted(ldap_secret_file)
+      expect(value.read).to match(/testvalue/)
+    end
+
+    it 'displays error when key does not exist' do
+      allow(Gitlab::Application.secrets).to receive(:enc_settings_key_base).and_return(nil)
+      expect { run_rake_task('gitlab:ldap:secret:write') }.to output(/Missing encryption key enc_settings_key_base./).to_stdout
+    end
+
+    it 'displays error when write directory does not exist' do
+      FileUtils.rm_rf('tmp/tests/ldapenc/')
+      expect { run_rake_task('gitlab:ldap:secret:write') }.to output(/Directory .* does not exist./).to_stdout
+    end
+  end
+end
-- 
GitLab


From 187618c50bb5508d1523ac1f1aa4ec301f30fd84 Mon Sep 17 00:00:00 2001
From: DJ Mountney <dj@gitlab.com>
Date: Fri, 23 Oct 2020 11:05:49 -0700
Subject: [PATCH 2/8] Apply review feedback

-Ensure we creation our tempfile within our final directory
- Add encrypted settings config to gitlab.yml.example
- Update missing key syntax
- And check early for missing key during the edit command
- Ensure spec tests are rails rooted
---
 config/gitlab.yml.example            |  3 +++
 lib/gitlab/encrypted_ldap_command.rb | 14 +++++++-------
 spec/tasks/gitlab/ldap_rake_spec.rb  |  4 ++--
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index 5db0e22ed10228..e90f2a7317c2e0 100644
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@@ -616,6 +616,9 @@ production: &base
     enabled: false
     prevent_ldap_sign_in: false
 
+    # File location to read encrypted secrets from
+    # secret_file: /mnt/gitlab/ldap.yaml.enc # Default: shared/encrypted_settings/ldap.yaml.enc
+
     # This setting controls the number of seconds between LDAP permission checks
     # for each user. After this time has expired for a given user, their next
     # interaction with GitLab (a click in the web UI, a git pull, etc.) will be
diff --git a/lib/gitlab/encrypted_ldap_command.rb b/lib/gitlab/encrypted_ldap_command.rb
index e86aa3bf0d6bfb..682edbf1758f5a 100644
--- a/lib/gitlab/encrypted_ldap_command.rb
+++ b/lib/gitlab/encrypted_ldap_command.rb
@@ -13,8 +13,6 @@ def write(contents)
         puts "File encrypted and saved."
       rescue Interrupt
         puts "Aborted changing file: nothing saved."
-      rescue Gitlab::EncryptedConfiguration::MissingKeyError
-        puts "Missing encryption key enc_settings_key_base."
       rescue ActiveSupport::MessageEncryptor::InvalidMessage
         puts "Couldn't decrypt #{encrypted.content_path}. Perhaps you passed the wrong key?"
       end
@@ -24,7 +22,7 @@ def edit
         return unless validate_config(encrypted)
 
         editor = ENV['EDITOR'] || 'editor'
-        temp_file = Tempfile.new(File.basename(encrypted.content_path))
+        temp_file = Tempfile.new(File.basename(encrypted.content_path), File.dirname(encrypted.content_path))
 
         encrypted.change do |contents|
           contents = encrypted_file_template unless File.exist?(encrypted.content_path)
@@ -36,8 +34,6 @@ def edit
         puts "File encrypted and saved."
       rescue Interrupt
         puts "Aborted changing file: nothing saved."
-      rescue Gitlab::EncryptedConfiguration::MissingKeyError
-        puts "Missing encryption key enc_settings_key_base."
       rescue ActiveSupport::MessageEncryptor::InvalidMessage
         puts "Couldn't decrypt #{encrypted.content_path}. Perhaps you passed the wrong key?"
       ensure
@@ -46,10 +42,9 @@ def edit
 
       def show
         encrypted = Gitlab::Auth::Ldap::Config.encrypted_secrets
+        return unless validate_config(encrypted)
 
         puts encrypted.read.presence || "File '#{encrypted.content_path}' does not exist. Use `rake gitlab:ldap:secret:edit` to change that."
-      rescue Gitlab::EncryptedConfiguration::MissingKeyError
-        puts "Missing encryption key enc_settings_key_base."
       rescue ActiveSupport::MessageEncryptor::InvalidMessage
         puts "Couldn't decrypt #{encrypted.content_path}. Perhaps you passed the wrong key?"
       end
@@ -64,6 +59,11 @@ def validate_config(encrypted)
           return false
         end
 
+        if encrypted.key.nil?
+          puts "Missing encryption key enc_settings_key_base."
+          return false
+        end
+
         true
       end
 
diff --git a/spec/tasks/gitlab/ldap_rake_spec.rb b/spec/tasks/gitlab/ldap_rake_spec.rb
index f0647c498b6ffb..636260e8dab185 100644
--- a/spec/tasks/gitlab/ldap_rake_spec.rb
+++ b/spec/tasks/gitlab/ldap_rake_spec.rb
@@ -27,7 +27,7 @@
   end
 
   after do
-    FileUtils.rm_rf('tmp/tests/ldapenc/')
+    FileUtils.rm_rf(Rails.root.join('tmp/tests/ldapenc'))
   end
 
   describe ':show' do
@@ -74,7 +74,7 @@
     end
 
     it 'displays error when write directory does not exist' do
-      FileUtils.rm_rf('tmp/tests/ldapenc/')
+      FileUtils.rm_rf(Rails.root.join('tmp/tests/ldapenc'))
       expect { run_rake_task('gitlab:ldap:secret:edit') }.to output(/Directory .* does not exist./).to_stdout
     end
   end
-- 
GitLab


From 1025d6ac6d30509db46aab4283e1d6be80f7acff Mon Sep 17 00:00:00 2001
From: DJ Mountney <dj@gitlab.com>
Date: Fri, 23 Oct 2020 14:24:35 -0700
Subject: [PATCH 3/8] Apply review feedback

- Autogenerate the encrypted key but only when requested
- Generate the new key as part of the secret token initializer
- Add additional config check during ldap secrets edit
- Ensure that the provided input is valid yaml, and is a hash
---
 lib/gitlab/encrypted_ldap_command.rb | 13 +++++++++++--
 spec/tasks/gitlab/ldap_rake_spec.rb  | 14 ++++++++++++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/lib/gitlab/encrypted_ldap_command.rb b/lib/gitlab/encrypted_ldap_command.rb
index 682edbf1758f5a..6ade602a975c59 100644
--- a/lib/gitlab/encrypted_ldap_command.rb
+++ b/lib/gitlab/encrypted_ldap_command.rb
@@ -8,6 +8,7 @@ def write(contents)
         encrypted = Gitlab::Auth::Ldap::Config.encrypted_secrets
         return unless validate_config(encrypted)
 
+        validate_contents(contents)
         encrypted.write(contents)
 
         puts "File encrypted and saved."
@@ -27,8 +28,10 @@ def edit
         encrypted.change do |contents|
           contents = encrypted_file_template unless File.exist?(encrypted.content_path)
           File.write(temp_file.path, contents)
-          system("#{editor} #{temp_file.path}")
-          File.read(temp_file.path)
+          system(editor, temp_file.path)
+          changes = File.read(temp_file.path)
+          validate_contents(changes)
+          changes
         end
 
         puts "File encrypted and saved."
@@ -67,6 +70,12 @@ def validate_config(encrypted)
         true
       end
 
+      def validate_contents(contents)
+        config = YAML.safe_load(contents, permitted_classes: [Symbol])
+        puts "WARNING: Content was not a valid LDAP secret yml file." if config.nil? || !config.is_a?(Hash)
+        contents
+      end
+
       def encrypted_file_template
         <<~YAML
           # main:
diff --git a/spec/tasks/gitlab/ldap_rake_spec.rb b/spec/tasks/gitlab/ldap_rake_spec.rb
index 636260e8dab185..d93b4b58092e81 100644
--- a/spec/tasks/gitlab/ldap_rake_spec.rb
+++ b/spec/tasks/gitlab/ldap_rake_spec.rb
@@ -77,6 +77,13 @@
       FileUtils.rm_rf(Rails.root.join('tmp/tests/ldapenc'))
       expect { run_rake_task('gitlab:ldap:secret:edit') }.to output(/Directory .* does not exist./).to_stdout
     end
+
+    it 'shows a warning when content is invalid' do
+      Settings.encrypted(ldap_secret_file).write('somevalue')
+      expect { run_rake_task('gitlab:ldap:secret:edit') }.to output(/WARNING: Content was not a valid LDAP secret yml file/).to_stdout
+      value = Settings.encrypted(ldap_secret_file)
+      expect(value.read).to match(/somevalue/)
+    end
   end
 
   describe 'write' do
@@ -101,5 +108,12 @@
       FileUtils.rm_rf('tmp/tests/ldapenc/')
       expect { run_rake_task('gitlab:ldap:secret:write') }.to output(/Directory .* does not exist./).to_stdout
     end
+
+    it 'shows a warning when content is invalid' do
+      Settings.encrypted(ldap_secret_file).write('somevalue')
+      expect { run_rake_task('gitlab:ldap:secret:edit') }.to output(/WARNING: Content was not a valid LDAP secret yml file/).to_stdout
+      value = Settings.encrypted(ldap_secret_file)
+      expect(value.read).to match(/somevalue/)
+    end
   end
 end
-- 
GitLab


From 8f00408f0140832c29411837326b6d3b1f0f8d09 Mon Sep 17 00:00:00 2001
From: DJ Mountney <dj@gitlab.com>
Date: Mon, 23 Nov 2020 09:23:58 -0800
Subject: [PATCH 4/8] Ensure user provides EDITOR env variable

Rather than falling back to the `editor` binary, which is often only on
debian systems if an alernative editor was installed.
---
 lib/gitlab/encrypted_ldap_command.rb | 11 ++++++++---
 spec/tasks/gitlab/ldap_rake_spec.rb  | 23 ++++++++++++++---------
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/lib/gitlab/encrypted_ldap_command.rb b/lib/gitlab/encrypted_ldap_command.rb
index 6ade602a975c59..06a3780c777106 100644
--- a/lib/gitlab/encrypted_ldap_command.rb
+++ b/lib/gitlab/encrypted_ldap_command.rb
@@ -22,13 +22,18 @@ def edit
         encrypted = Gitlab::Auth::Ldap::Config.encrypted_secrets
         return unless validate_config(encrypted)
 
-        editor = ENV['EDITOR'] || 'editor'
+        if ENV["EDITOR"].to_s.empty?
+          puts 'No $EDITOR specified to open file. Please provide one when running the command:'
+          puts 'gitlab:ldap:secret:edit EDITOR=vim'
+          return
+        end
+
         temp_file = Tempfile.new(File.basename(encrypted.content_path), File.dirname(encrypted.content_path))
 
         encrypted.change do |contents|
           contents = encrypted_file_template unless File.exist?(encrypted.content_path)
           File.write(temp_file.path, contents)
-          system(editor, temp_file.path)
+          system(ENV['EDITOR'], temp_file.path)
           changes = File.read(temp_file.path)
           validate_contents(changes)
           changes
@@ -63,7 +68,7 @@ def validate_config(encrypted)
         end
 
         if encrypted.key.nil?
-          puts "Missing encryption key enc_settings_key_base."
+          puts "Missing encryption key encrypted_settings_key_base."
           return false
         end
 
diff --git a/spec/tasks/gitlab/ldap_rake_spec.rb b/spec/tasks/gitlab/ldap_rake_spec.rb
index d93b4b58092e81..fd18741c5a98ee 100644
--- a/spec/tasks/gitlab/ldap_rake_spec.rb
+++ b/spec/tasks/gitlab/ldap_rake_spec.rb
@@ -23,7 +23,7 @@
     stub_warn_user_is_not_gitlab
     FileUtils.mkdir_p('tmp/tests/ldapenc/')
     allow(Gitlab.config.ldap).to receive(:secret_file).and_return(ldap_secret_file)
-    allow(Gitlab::Application.secrets).to receive(:enc_settings_key_base).and_return(SecureRandom.hex(64))
+    allow(Gitlab::Application.secrets).to receive(:encrypted_settings_key_base).and_return(SecureRandom.hex(64))
   end
 
   after do
@@ -37,13 +37,13 @@
 
     it 'displays error when key does not exist' do
       Settings.encrypted(ldap_secret_file).write('somevalue')
-      allow(Gitlab::Application.secrets).to receive(:enc_settings_key_base).and_return(nil)
-      expect { run_rake_task('gitlab:ldap:secret:show') }.to output(/Missing encryption key enc_settings_key_base./).to_stdout
+      allow(Gitlab::Application.secrets).to receive(:encrypted_settings_key_base).and_return(nil)
+      expect { run_rake_task('gitlab:ldap:secret:show') }.to output(/Missing encryption key encrypted_settings_key_base./).to_stdout
     end
 
     it 'displays error when key is changed' do
       Settings.encrypted(ldap_secret_file).write('somevalue')
-      allow(Gitlab::Application.secrets).to receive(:enc_settings_key_base).and_return(SecureRandom.hex(64))
+      allow(Gitlab::Application.secrets).to receive(:encrypted_settings_key_base).and_return(SecureRandom.hex(64))
       expect { run_rake_task('gitlab:ldap:secret:show') }.to output(/Couldn't decrypt .* Perhaps you passed the wrong key?/).to_stdout
     end
 
@@ -63,13 +63,13 @@
     end
 
     it 'displays error when key does not exist' do
-      allow(Gitlab::Application.secrets).to receive(:enc_settings_key_base).and_return(nil)
-      expect { run_rake_task('gitlab:ldap:secret:edit') }.to output(/Missing encryption key enc_settings_key_base./).to_stdout
+      allow(Gitlab::Application.secrets).to receive(:encrypted_settings_key_base).and_return(nil)
+      expect { run_rake_task('gitlab:ldap:secret:edit') }.to output(/Missing encryption key encrypted_settings_key_base./).to_stdout
     end
 
     it 'displays error when key is changed' do
       Settings.encrypted(ldap_secret_file).write('somevalue')
-      allow(Gitlab::Application.secrets).to receive(:enc_settings_key_base).and_return(SecureRandom.hex(64))
+      allow(Gitlab::Application.secrets).to receive(:encrypted_settings_key_base).and_return(SecureRandom.hex(64))
       expect { run_rake_task('gitlab:ldap:secret:edit') }.to output(/Couldn't decrypt .* Perhaps you passed the wrong key?/).to_stdout
     end
 
@@ -84,6 +84,11 @@
       value = Settings.encrypted(ldap_secret_file)
       expect(value.read).to match(/somevalue/)
     end
+
+    it 'displays error when $EDITOR is not set' do
+      stub_env('EDITOR', nil)
+      expect { run_rake_task('gitlab:ldap:secret:edit') }.to output(/No \$EDITOR specified to open file. Please provide one when running the command/).to_stdout
+    end
   end
 
   describe 'write' do
@@ -100,8 +105,8 @@
     end
 
     it 'displays error when key does not exist' do
-      allow(Gitlab::Application.secrets).to receive(:enc_settings_key_base).and_return(nil)
-      expect { run_rake_task('gitlab:ldap:secret:write') }.to output(/Missing encryption key enc_settings_key_base./).to_stdout
+      allow(Gitlab::Application.secrets).to receive(:encrypted_settings_key_base).and_return(nil)
+      expect { run_rake_task('gitlab:ldap:secret:write') }.to output(/Missing encryption key encrypted_settings_key_base./).to_stdout
     end
 
     it 'displays error when write directory does not exist' do
-- 
GitLab


From 9d6627b72b776325d9222bba176f3f87f86d8b41 Mon Sep 17 00:00:00 2001
From: DJ Mountney <dj@gitlab.com>
Date: Tue, 8 Dec 2020 16:10:36 +0000
Subject: [PATCH 5/8] Apply 1 suggestion(s) to 1 file(s)

---
 lib/gitlab/encrypted_ldap_command.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/gitlab/encrypted_ldap_command.rb b/lib/gitlab/encrypted_ldap_command.rb
index 06a3780c777106..e3917142eedc58 100644
--- a/lib/gitlab/encrypted_ldap_command.rb
+++ b/lib/gitlab/encrypted_ldap_command.rb
@@ -85,7 +85,7 @@ def encrypted_file_template
         <<~YAML
           # main:
           #   password: '123'
-          #   user_bn: 'gitlab-adm'
+          #   user_dn: 'gitlab-adm'
         YAML
       end
     end
-- 
GitLab


From 9a98dba14a2c3a40da5b833e725b538db9304303 Mon Sep 17 00:00:00 2001
From: DJ Mountney <dj@gitlab.com>
Date: Wed, 9 Dec 2020 13:26:49 -0800
Subject: [PATCH 6/8] Update ldap encrypted to use consistent wording

Use gitlab-rake for the rake commands, and always refer to LDAP in
uppercase
---
 lib/gitlab/encrypted_ldap_command.rb | 4 ++--
 lib/tasks/gitlab/ldap.rake           | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/gitlab/encrypted_ldap_command.rb b/lib/gitlab/encrypted_ldap_command.rb
index e3917142eedc58..97c14b00d77e7d 100644
--- a/lib/gitlab/encrypted_ldap_command.rb
+++ b/lib/gitlab/encrypted_ldap_command.rb
@@ -24,7 +24,7 @@ def edit
 
         if ENV["EDITOR"].to_s.empty?
           puts 'No $EDITOR specified to open file. Please provide one when running the command:'
-          puts 'gitlab:ldap:secret:edit EDITOR=vim'
+          puts 'gitlab-rake gitlab:ldap:secret:edit EDITOR=vim'
           return
         end
 
@@ -52,7 +52,7 @@ def show
         encrypted = Gitlab::Auth::Ldap::Config.encrypted_secrets
         return unless validate_config(encrypted)
 
-        puts encrypted.read.presence || "File '#{encrypted.content_path}' does not exist. Use `rake gitlab:ldap:secret:edit` to change that."
+        puts encrypted.read.presence || "File '#{encrypted.content_path}' does not exist. Use `gitlab-rake gitlab:ldap:secret:edit` to change that."
       rescue ActiveSupport::MessageEncryptor::InvalidMessage
         puts "Couldn't decrypt #{encrypted.content_path}. Perhaps you passed the wrong key?"
       end
diff --git a/lib/tasks/gitlab/ldap.rake b/lib/tasks/gitlab/ldap.rake
index 4171bd70f1a35b..fe7920c621f920 100644
--- a/lib/tasks/gitlab/ldap.rake
+++ b/lib/tasks/gitlab/ldap.rake
@@ -38,18 +38,18 @@ namespace :gitlab do
     end
 
     namespace :secret do
-      desc 'GitLab | LDAP | Secret | Write ldap secrets'
+      desc 'GitLab | LDAP | Secret | Write LDAP secrets'
       task write: [:environment] do
         content = STDIN.tty? ? STDIN.gets : STDIN.read
         Gitlab::EncryptedLdapCommand.write(content)
       end
 
-      desc 'GitLab | LDAP | Secret | Edit ldap secrets'
+      desc 'GitLab | LDAP | Secret | Edit LDAP secrets'
       task edit: [:environment] do
         Gitlab::EncryptedLdapCommand.edit
       end
 
-      desc 'GitLab | LDAP | Secret | Show ldap secrets'
+      desc 'GitLab | LDAP | Secret | Show LDAP secrets'
       task show: [:environment] do
         Gitlab::EncryptedLdapCommand.show
       end
-- 
GitLab


From 812b7e118c76cd55e79003636531b1c4ab35b952 Mon Sep 17 00:00:00 2001
From: DJ Mountney <dj@gitlab.com>
Date: Wed, 9 Dec 2020 13:54:31 -0800
Subject: [PATCH 7/8] Add exception handling for psych errors

For validating the contents passed to ldap write
---
 lib/gitlab/encrypted_ldap_command.rb | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/lib/gitlab/encrypted_ldap_command.rb b/lib/gitlab/encrypted_ldap_command.rb
index 97c14b00d77e7d..ad6bd5c1216592 100644
--- a/lib/gitlab/encrypted_ldap_command.rb
+++ b/lib/gitlab/encrypted_ldap_command.rb
@@ -76,8 +76,15 @@ def validate_config(encrypted)
       end
 
       def validate_contents(contents)
-        config = YAML.safe_load(contents, permitted_classes: [Symbol])
-        puts "WARNING: Content was not a valid LDAP secret yml file." if config.nil? || !config.is_a?(Hash)
+        begin
+          config = YAML.safe_load(contents, permitted_classes: [Symbol])
+          error_contents = "Did not include any key-value pairs" unless config.is_a?(Hash)
+        rescue Psych::Exception => e
+          error_contents = e.message
+        end
+
+        puts "WARNING: Content was not a valid LDAP secret yml file. #{error_contents}" if error_contents
+
         contents
       end
 
-- 
GitLab


From 01275f09e9e06728640ad782be91d21dbea47f52 Mon Sep 17 00:00:00 2001
From: DJ Mountney <dj@gitlab.com>
Date: Wed, 9 Dec 2020 14:24:50 -0800
Subject: [PATCH 8/8] Add some messaging when the encrypted contents don't
 change

And update the spec tests for the latest changes
---
 lib/gitlab/encrypted_ldap_command.rb | 5 ++++-
 spec/tasks/gitlab/ldap_rake_spec.rb  | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/gitlab/encrypted_ldap_command.rb b/lib/gitlab/encrypted_ldap_command.rb
index ad6bd5c1216592..cdb3e268b51a5c 100644
--- a/lib/gitlab/encrypted_ldap_command.rb
+++ b/lib/gitlab/encrypted_ldap_command.rb
@@ -22,23 +22,26 @@ def edit
         encrypted = Gitlab::Auth::Ldap::Config.encrypted_secrets
         return unless validate_config(encrypted)
 
-        if ENV["EDITOR"].to_s.empty?
+        if ENV["EDITOR"].blank?
           puts 'No $EDITOR specified to open file. Please provide one when running the command:'
           puts 'gitlab-rake gitlab:ldap:secret:edit EDITOR=vim'
           return
         end
 
         temp_file = Tempfile.new(File.basename(encrypted.content_path), File.dirname(encrypted.content_path))
+        contents_changed = false
 
         encrypted.change do |contents|
           contents = encrypted_file_template unless File.exist?(encrypted.content_path)
           File.write(temp_file.path, contents)
           system(ENV['EDITOR'], temp_file.path)
           changes = File.read(temp_file.path)
+          contents_changed = contents != changes
           validate_contents(changes)
           changes
         end
 
+        puts "Contents were unchanged." unless contents_changed
         puts "File encrypted and saved."
       rescue Interrupt
         puts "Aborted changing file: nothing saved."
diff --git a/spec/tasks/gitlab/ldap_rake_spec.rb b/spec/tasks/gitlab/ldap_rake_spec.rb
index fd18741c5a98ee..5286cd98944f03 100644
--- a/spec/tasks/gitlab/ldap_rake_spec.rb
+++ b/spec/tasks/gitlab/ldap_rake_spec.rb
@@ -32,7 +32,7 @@
 
   describe ':show' do
     it 'displays error when file does not exist' do
-      expect { run_rake_task('gitlab:ldap:secret:show') }.to output(/File .* does not exist. Use `rake gitlab:ldap:secret:edit` to change that./).to_stdout
+      expect { run_rake_task('gitlab:ldap:secret:show') }.to output(/File .* does not exist. Use `gitlab-rake gitlab:ldap:secret:edit` to change that./).to_stdout
     end
 
     it 'displays error when key does not exist' do
-- 
GitLab