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

@@ -8,6 +8,7 @@ module Gitlab
         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 @@ module Gitlab
         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 @@ module Gitlab
         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:

spec/tasks/gitlab/ldap_rake_spec.rb

@@ -77,6 +77,13 @@ RSpec.describe 'gitlab:ldap:secret rake tasks' do
       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 @@ RSpec.describe 'gitlab:ldap:secret rake tasks' do
       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