Define a proper class in config/initializers/01_secret_token.rb

Signed-off-by: Rémy Coutable <remy@rymai.me>

config/initializers/01_secret_token.rb

@@ -7,95 +7,105 @@
 # prepend modules that require access to secrets (e.g. EE's 0_as_concern.rb).
 #
 # Be sure to restart your server when you modify this file.
-
 require 'securerandom'
 
-def rails_secrets_config_file
-  Rails.root.join('config/secrets.yml')
-end
+class SecretsInitializer
+  def initialize(secrets_file_path:, rails_env:)
+    @secrets_file_path = secrets_file_path
+    @rails_env = rails_env
+  end
 
-def load_secrets_from_file
-  YAML.safe_load_file(rails_secrets_config_file)
-rescue Errno::ENOENT, Psych::SyntaxError
-  {}
-end
+  def secrets_from_file
+    @secrets_from_file ||= begin
+      YAML.safe_load_file(secrets_file_path)
+    rescue Errno::ENOENT
+      {}
+    end
+  end
 
-def set_credentials_from_file_and_env!
-  # Inspired by https://github.com/rails/rails/blob/v7.0.8.4/railties/lib/rails/secrets.rb#L25-L36
-  # Later, once config/secrets.yml won't be read automatically, we'll need to do it manually, so
-  # we anticipate and do it ourselves here.
-  file_secrets = load_secrets_from_file
-  secrets = file_secrets.fetch("shared", {}).deep_symbolize_keys
-    .merge(file_secrets.fetch(Rails.env, {}).deep_symbolize_keys)
-
-  # Copy secrets from config/secrets.yml into Rails.application.credentials
-  # If we support native Rails.application.credentials later
-  # (e.g. config.credentials.yml.enc + config/master.key ), this loop would
-  # become a no-op as long as credentials are migrated to config.credentials.yml.enc.
-  secrets.each do |key, value|
-    next if Rails.application.credentials.public_send(key).present?
-
-    Rails.application.credentials[key] = value
+  def execute!
+    set_credentials_from_file_and_env!
+    set_missing_from_defaults!
   end
 
-  # Historically, ENV['SECRET_KEY_BASE'] takes precedence over config/secrets.yml, so we maintain that
-  # behavior by ensuring the environment variable always overrides the value from config/secrets.yml.
-  env_secret_key = ENV['SECRET_KEY_BASE']
-  Rails.application.credentials.secret_key_base = env_secret_key if env_secret_key.present?
-end
+  private
 
-def set_missing_from_defaults!
-  defaults = {
-    secret_key_base: generate_new_secure_token,
-    otp_key_base: generate_new_secure_token,
-    db_key_base: generate_new_secure_token,
-    openid_connect_signing_key: generate_new_rsa_private_key
-  }
-
-  # encrypted_settings_key_base is optional for now
-  if ENV['GITLAB_GENERATE_ENCRYPTED_SETTINGS_KEY_BASE']
-    defaults[:encrypted_settings_key_base] = generate_new_secure_token
-  end
+  attr_reader :secrets_file_path, :rails_env
 
-  missing_secrets = set_missing_keys(defaults)
-  write_secrets_yml!(missing_secrets) if missing_secrets.any?
-end
+  def set_credentials_from_file_and_env!
+    # Inspired by https://github.com/rails/rails/blob/v7.0.8.4/railties/lib/rails/secrets.rb#L25-L36
+    # Later, once config/secrets.yml won't be read automatically, we'll need to do it manually, so
+    # we anticipate and do it ourselves here.
+    secrets = secrets_from_file.fetch("shared", {}).deep_symbolize_keys
+      .merge(secrets_from_file.fetch(rails_env, {}).deep_symbolize_keys)
 
-def create_tokens
-  set_credentials_from_file_and_env!
-  set_missing_from_defaults!
-end
+    # Copy secrets from config/secrets.yml into Rails.application.credentials
+    # If we support native Rails.application.credentials later
+    # (e.g. config.credentials.yml.enc + config/master.key ), this loop would
+    # become a no-op as long as credentials are migrated to config.credentials.yml.enc.
+    secrets.each do |key, value|
+      next if Rails.application.credentials.public_send(key).present?
 
-def generate_new_secure_token
-  SecureRandom.hex(64)
-end
+      Rails.application.credentials[key] = value
+    end
 
-def generate_new_rsa_private_key
-  OpenSSL::PKey::RSA.new(2048).to_pem
-end
+    # Historically, ENV['SECRET_KEY_BASE'] takes precedence over config/secrets.yml, so we maintain that
+    # behavior by ensuring the environment variable always overrides the value from config/secrets.yml.
+    env_secret_key = ENV['SECRET_KEY_BASE']
+    Rails.application.credentials.secret_key_base = env_secret_key if env_secret_key.present?
+  end
 
-def warn_missing_secret(secret)
-  return if Rails.env.test?
+  def set_missing_from_defaults!
+    defaults = {
+      secret_key_base: generate_new_secure_token,
+      otp_key_base: generate_new_secure_token,
+      db_key_base: generate_new_secure_token,
+      openid_connect_signing_key: generate_new_rsa_private_key
+    }
+
+    # encrypted_settings_key_base is optional for now
+    if ENV['GITLAB_GENERATE_ENCRYPTED_SETTINGS_KEY_BASE']
+      defaults[:encrypted_settings_key_base] = generate_new_secure_token
+    end
+
+    missing_secrets = set_missing_keys(defaults)
+    write_secrets_yml!(missing_secrets) if missing_secrets.any?
+  end
 
-  warn "Missing Rails.application.credentials.#{secret} for #{Rails.env} environment. " \
-    "The secret will be generated and stored in config/secrets.yml."
-end
+  def generate_new_secure_token
+    SecureRandom.hex(64)
+  end
 
-def set_missing_keys(defaults)
-  defaults.stringify_keys.each_with_object({}) do |(key, default), missing|
-    next if Rails.application.credentials.public_send(key).present?
+  def generate_new_rsa_private_key
+    OpenSSL::PKey::RSA.new(2048).to_pem
+  end
+
+  def warn_missing_secret(secret)
+    return if rails_env.test?
 
-    warn_missing_secret(key)
-    missing[key] = Rails.application.credentials[key] = default
+    warn "Missing Rails.application.credentials.#{secret} for #{rails_env} environment. " \
+      "The secret will be generated and stored in config/secrets.yml."
   end
-end
 
-def write_secrets_yml!(missing_secrets)
-  rails_env = Rails.env.to_s
-  secrets = load_secrets_from_file
-  secrets[rails_env] ||= {}
-  secrets[rails_env].merge!(missing_secrets)
-  File.write(rails_secrets_config_file, YAML.dump(secrets), mode: 'w', perm: 0o600)
+  def set_missing_keys(defaults)
+    defaults.stringify_keys.each_with_object({}) do |(key, default), missing|
+      next if Rails.application.credentials.public_send(key).present?
+
+      warn_missing_secret(key)
+      missing[key] = Rails.application.credentials[key] = default
+    end
+  end
+
+  def write_secrets_yml!(missing_secrets)
+    secrets_from_file[rails_env.to_s] ||= {}
+    secrets_from_file[rails_env.to_s].merge!(missing_secrets)
+
+    File.write(
+      secrets_file_path,
+      YAML.dump(secrets_from_file),
+      mode: 'w', perm: 0o600
+    )
+  end
 end
 
-create_tokens
+SecretsInitializer.new(secrets_file_path: Rails.root.join('config/secrets.yml'), rails_env: Rails.env).execute!

spec/initializers/secret_token_spec.rb

@@ -3,37 +3,27 @@
 require 'spec_helper'
 require_relative '../../config/initializers/01_secret_token'
 
-RSpec.describe 'create_tokens' do
-  include StubENV
-
-  let(:allowed_keys) do
-    %w[
-      secret_key_base
-      db_key_base
-      otp_key_base
-      openid_connect_signing_key
-    ]
-  end
-
-  let(:hex_key) { /\h{128}/ }
-  let(:rsa_key) { /\A-----BEGIN RSA PRIVATE KEY-----\n.+\n-----END RSA PRIVATE KEY-----\n\Z/m }
+# rubocop:disable RSpec/FilePath -- The initializer name starts with `01` because we want to run it ASAP
+# rubocop:disable RSpec/FeatureCategory -- This is a shared responsibility
+RSpec.describe SecretsInitializer do
+  let(:rails_env_name) { 'test' }
+  let(:rails_env) { ActiveSupport::EnvironmentInquirer.new(rails_env_name) }
+  let(:fake_secret_file) { Tempfile.new(['fake-secrets', '.yml']) }
+  let(:secrets_hash) { {} }
+  let(:fake_secret_file_content) { secrets_hash.to_yaml }
 
-  around do |example|
-    original_credentials = Rails.application.credentials
-    # ensure we clear any existing `encrypted_settings_key_base` credential
-    allowed_keys.each do |key|
-      Rails.application.credentials.public_send(:"#{key}=", nil)
-    end
-    example.run
-    Rails.application.credentials = original_credentials
+  before do
+    fake_secret_file.write(fake_secret_file_content)
+    fake_secret_file.rewind
   end
 
-  before do
-    allow(Rails).to receive_message_chain(:root, :join) { |string| string }
-    allow(File).to receive(:write).and_call_original
-    allow(File).to receive(:write).with('config/secrets.yml')
+  after do
+    fake_secret_file.close
+    fake_secret_file.unlink
   end
 
+  subject(:initializer) { described_class.new(secrets_file_path: fake_secret_file.path, rails_env: rails_env) }
+
   describe 'ensure acknowledged secrets in any installations' do
     let(:acknowledged_secrets) do
       %w[secret_key_base otp_key_base db_key_base openid_connect_signing_key encrypted_settings_key_base
@@ -41,195 +31,237 @@
     end
 
     it 'does not allow to add a new secret without a proper handling' do
-      create_tokens
+      secrets_hash = YAML.safe_load_file(Rails.root.join('config/secrets.yml'))
 
-      secrets_hash = YAML.load_file(Rails.root.join('config/secrets.yml'))
-
-      secrets_hash.each do |environment, secrets|
+      secrets_hash.each_value do |secrets|
         new_secrets = secrets.keys - acknowledged_secrets
 
         expect(new_secrets).to be_empty,
-          <<~EOS
-           CAUTION:
-           It looks like you have just added new secret(s) #{new_secrets.inspect} to the secrets.yml.
-           Please read the development guide for GitLab secrets at doc/development/application_secrets.md before you proceed this change.
-           If you're absolutely sure that the change is safe, please add the new secrets to the 'acknowledged_secrets' in order to silence this warning.
-          EOS
+          <<~WARNING
+          CAUTION:
+          It looks like you have just added new secret(s) #{new_secrets.inspect} to the secrets.yml.
+          Please read the development guide for GitLab secrets at doc/development/application_secrets.md before you proceed this change.
+          If you're absolutely sure that the change is safe, please add the new secrets to the 'acknowledged_secrets' in order to silence this warning.
+          WARNING
       end
     end
   end
 
-  context 'when none of the secrets exist' do
-    before do
-      # ensure we clear any existing `encrypted_settings_key_base` credential
-      allowed_keys.each do |key|
-        Rails.application.credentials.public_send(:"#{key}=", nil)
-      end
+  describe '#secrets_from_file' do
+    context 'when the secrets files is a valid YAML' do
+      let(:secrets_hash) { { 'foo' => 'bar' } }
 
-      allow(self).to receive(:load_secrets_from_file).and_return({})
-      stub_env('SECRET_KEY_BASE', nil)
+      it 'parses and returns the hash' do
+        expect(initializer.secrets_from_file).to eq({ 'foo' => 'bar' })
+      end
     end
 
-    it 'generates different hashes for secret_key_base, otp_key_base, and db_key_base' do
-      create_tokens
+    context 'when the secrets file does not exist' do
+      let(:fake_secret_file_content) { 'foo = bar' }
 
-      keys = Rails.application.credentials.values_at(:secret_key_base, :otp_key_base, :db_key_base)
+      it 'returns an empty hash' do
+        expect(YAML).to receive(:safe_load_file).and_raise(Errno::ENOENT)
 
-      expect(keys.uniq).to eq(keys)
-      expect(keys).to all(match(hex_key))
+        expect(initializer.secrets_from_file).to eq({})
+      end
     end
 
-    it 'generates an RSA key for openid_connect_signing_key' do
-      create_tokens
+    context 'when the secrets file contains invalid YAML' do
+      let(:fake_secret_file_content) { "foo:\n\tbar: baz\n\tbar: foo" }
+
+      it 'raises a Psych::SyntaxError exception' do
+        expect { initializer.secrets_from_file }.to raise_error(Psych::SyntaxError)
+      end
+    end
+  end
 
-      keys = Rails.application.credentials.values_at(:openid_connect_signing_key)
+  describe '#execute!' do
+    include StubENV
 
-      expect(keys.uniq).to eq(keys)
-      expect(keys).to all(match(rsa_key))
+    let(:allowed_keys) do
+      %w[
+        secret_key_base
+        db_key_base
+        otp_key_base
+        openid_connect_signing_key
+      ]
     end
 
-    it 'warns about the secrets to add to secrets.yml' do
+    let(:hex_key) { /\h{128}/ }
+    let(:rsa_key) { /\A-----BEGIN RSA PRIVATE KEY-----\n.+\n-----END RSA PRIVATE KEY-----\n\Z/m }
+
+    around do |example|
+      original_credentials = Rails.application.credentials
+      # Ensure we clear any existing `encrypted_settings_key_base` credential
       allowed_keys.each do |key|
-        expect(self).to receive(:warn_missing_secret).with(key)
+        Rails.application.credentials.public_send(:"#{key}=", nil)
       end
 
-      create_tokens
+      example.run
+
+      Rails.application.credentials = original_credentials
     end
 
-    it 'writes the secrets to secrets.yml' do
-      expect(File).to receive(:write).with('config/secrets.yml', any_args) do |_filename, contents, _options|
-        new_secrets = YAML.safe_load(contents)['test']
+    before do
+      allow(File).to receive(:write).with(fake_secret_file.path, any_args)
+    end
 
-        allowed_keys.each do |key|
-          expect(new_secrets[key]).to eq(Rails.application.credentials.values_at(key.to_sym).first)
-        end
-        expect(new_secrets['encrypted_settings_key_base']).to be_nil # encrypted_settings_key_base is optional
+    context 'when none of the secrets exist' do
+      before do
+        stub_env('SECRET_KEY_BASE', nil)
       end
 
-      create_tokens
-    end
+      it 'generates different hashes for secret_key_base, otp_key_base, and db_key_base' do
+        initializer.execute!
 
-    context 'when GITLAB_GENERATE_ENCRYPTED_SETTINGS_KEY_BASE is set' do
-      let(:allowed_keys) do
-        super() + ['encrypted_settings_key_base']
-      end
+        keys = Rails.application.credentials.values_at(:secret_key_base, :otp_key_base, :db_key_base)
 
-      before do
-        stub_env('GITLAB_GENERATE_ENCRYPTED_SETTINGS_KEY_BASE', '1')
-        allow(self).to receive(:warn_missing_secret)
+        expect(keys.uniq).to eq(keys)
+        expect(keys).to all(match(hex_key))
       end
 
-      it 'writes the encrypted_settings_key_base secret' do
-        expect(self).to receive(:warn_missing_secret).with('encrypted_settings_key_base')
-        expect(File).to receive(:write).with('config/secrets.yml', any_args) do |_filename, contents, _options|
-          new_secrets = YAML.safe_load(contents)['test']
+      it 'generates an RSA key for openid_connect_signing_key' do
+        initializer.execute!
+
+        keys = Rails.application.credentials.values_at(:openid_connect_signing_key)
+
+        expect(keys.uniq).to eq(keys)
+        expect(keys).to all(match(rsa_key))
+      end
 
-          expect(new_secrets['encrypted_settings_key_base']).to eq(Rails.application.credentials.encrypted_settings_key_base)
+      it 'warns about the secrets to add to secrets.yml' do
+        allowed_keys.each do |key|
+          expect(initializer).to receive(:warn_missing_secret).with(key)
         end
 
-        create_tokens
+        initializer.execute!
       end
-    end
-  end
 
-  shared_examples 'credentials are properly set' do
-    it 'sets Rails.application.credentials' do
-      create_tokens
+      it 'writes the secrets to secrets.yml' do
+        expect(File).to receive(:write).with(fake_secret_file.path, any_args) do |_filename, contents, _options|
+          new_secrets = YAML.safe_load(contents)[rails_env_name]
 
-      expect(Rails.application.credentials.values_at(*allowed_keys.map(&:to_sym))).to eq(allowed_keys)
-    end
+          allowed_keys.each do |key|
+            expect(new_secrets[key]).to eq(Rails.application.credentials.values_at(key.to_sym).first)
+          end
+          expect(new_secrets['encrypted_settings_key_base']).to be_nil # encrypted_settings_key_base is optional
+        end
 
-    it 'does not issue warnings' do
-      expect(self).not_to receive(:warn_missing_secret)
+        initializer.execute!
+      end
 
-      create_tokens
-    end
+      context 'when GITLAB_GENERATE_ENCRYPTED_SETTINGS_KEY_BASE is set' do
+        let(:allowed_keys) do
+          super() + ['encrypted_settings_key_base']
+        end
 
-    it 'does not update secrets.yml' do
-      expect(File).not_to receive(:write)
+        before do
+          stub_env('GITLAB_GENERATE_ENCRYPTED_SETTINGS_KEY_BASE', '1')
+          allow(initializer).to receive(:warn_missing_secret)
+        end
 
-      create_tokens
-    end
-  end
+        it 'writes the encrypted_settings_key_base secret' do
+          expect(initializer).to receive(:warn_missing_secret).with('encrypted_settings_key_base')
+          expect(File).to receive(:write).with(fake_secret_file.path, any_args) do |_filename, contents, _options|
+            new_secrets = YAML.safe_load(contents)[rails_env_name]
 
-  context 'when secrets exist in secrets.yml' do
-    let(:credentials) do
-      Hash[allowed_keys.zip(allowed_keys)]
-    end
+            expect(new_secrets['encrypted_settings_key_base']).to eq(Rails.application.credentials.encrypted_settings_key_base)
+          end
 
-    before do
-      # ensure we clear any existing `encrypted_settings_key_base` credential
-      allowed_keys.each do |key|
-        Rails.application.credentials.public_send(:"#{key}=", nil)
+          initializer.execute!
+        end
       end
-
-      allow(self).to receive(:load_secrets_from_file).and_return({
-        'test' => credentials
-      })
     end
 
-    it_behaves_like 'credentials are properly set'
+    shared_examples 'credentials are properly set' do
+      it 'sets Rails.application.credentials' do
+        initializer.execute!
 
-    context 'when secret_key_base also exist in the environment variable' do
-      before do
-        stub_env('SECRET_KEY_BASE', 'env_key')
+        expect(Rails.application.credentials.values_at(*allowed_keys.map(&:to_sym))).to eq(allowed_keys)
       end
 
-      it 'sets Rails.application.credentials.secret_key_base from the environment variable' do
-        create_tokens
+      it 'does not issue warnings' do
+        expect(initializer).not_to receive(:warn_missing_secret)
 
-        expect(Rails.application.credentials.secret_key_base).to eq('env_key')
+        initializer.execute!
       end
-    end
-  end
 
-  context 'when secrets exist in Rails.application.credentials' do
-    before do
-      allowed_keys.each do |key|
-        Rails.application.credentials.public_send(:"#{key}=", key)
+      it 'does not update secrets.yml' do
+        expect(File).not_to receive(:write)
+
+        initializer.execute!
       end
     end
 
-    it_behaves_like 'credentials are properly set'
+    context 'when secrets exist in secrets.yml' do
+      let(:secrets_hash) { { rails_env_name => Hash[allowed_keys.zip(allowed_keys)] } }
 
-    context 'when secret_key_base also exist in the environment variable' do
-      before do
-        stub_env('SECRET_KEY_BASE', 'env_key')
-      end
+      it_behaves_like 'credentials are properly set'
 
-      it 'sets Rails.application.credentials.secret_key_base from the environment variable' do
-        create_tokens
+      context 'when secret_key_base also exist in the environment variable' do
+        before do
+          stub_env('SECRET_KEY_BASE', 'env_key')
+        end
 
-        expect(Rails.application.credentials.secret_key_base).to eq('env_key')
+        it 'sets Rails.application.credentials.secret_key_base from the environment variable' do
+          initializer.execute!
+
+          expect(Rails.application.credentials.secret_key_base).to eq('env_key')
+        end
       end
     end
-  end
 
-  context 'some secrets miss, some are in env, some are in Rails.application.credentials, and some are in secrets.yml' do
-    before do
-      stub_env('SECRET_KEY_BASE', 'env_key')
+    context 'when secrets exist in Rails.application.credentials' do
+      before do
+        allowed_keys.each do |key|
+          Rails.application.credentials.public_send(:"#{key}=", key)
+        end
+      end
+
+      it_behaves_like 'credentials are properly set'
+
+      context 'when secret_key_base also exist in the environment variable' do
+        before do
+          stub_env('SECRET_KEY_BASE', 'env_key')
+        end
 
-      Rails.application.credentials.db_key_base = 'db_key_base'
+        it 'sets Rails.application.credentials.secret_key_base from the environment variable' do
+          initializer.execute!
 
-      allow(self).to receive(:load_secrets_from_file).and_return({
-        'test' => { 'otp_key_base' => 'otp_key_base' }
-      })
+          expect(Rails.application.credentials.secret_key_base).to eq('env_key')
+        end
+      end
     end
 
-    it 'sets Rails.application.credentials properly, issue a warning and writes config.secrets.yml' do
-      expect(self).to receive(:warn_missing_secret).with('openid_connect_signing_key')
-      expect(File).to receive(:write).with('config/secrets.yml', any_args) do |_filename, contents, _options|
-        new_secrets = YAML.safe_load(contents)['test']
+    context 'with some secrets missing, some in ENV, some in Rails.application.credentials, some in secrets.yml' do
+      let(:rails_env_name) { 'foo' }
+      let(:secrets_hash) { { rails_env_name => { 'otp_key_base' => 'otp_key_base' } } }
 
-        expect(new_secrets['otp_key_base']).to eq('otp_key_base')
-        expect(new_secrets['openid_connect_signing_key']).to match(rsa_key)
+      before do
+        stub_env('SECRET_KEY_BASE', 'env_key')
+        Rails.application.credentials.db_key_base = 'db_key_base'
       end
 
-      create_tokens
+      it 'sets Rails.application.credentials properly, issue a warning and writes config.secrets.yml' do
+        expect(File).to receive(:write).with(fake_secret_file.path, any_args) do |_filename, contents, _options|
+          new_secrets = YAML.safe_load(contents)[rails_env_name]
 
-      expect(Rails.application.credentials.secret_key_base).to eq('env_key')
-      expect(Rails.application.credentials.db_key_base).to eq('db_key_base')
-      expect(Rails.application.credentials.otp_key_base).to eq('otp_key_base')
+          expect(new_secrets['otp_key_base']).to eq('otp_key_base')
+          expect(new_secrets['openid_connect_signing_key']).to match(rsa_key)
+        end
+        expect(initializer).to receive(:warn).with(
+          "Missing Rails.application.credentials.openid_connect_signing_key for #{rails_env_name} environment. " \
+            "The secret will be generated and stored in config/secrets.yml."
+        )
+
+        initializer.execute!
+
+        expect(Rails.application.credentials.secret_key_base).to eq('env_key')
+        expect(Rails.application.credentials.db_key_base).to eq('db_key_base')
+        expect(Rails.application.credentials.otp_key_base).to eq('otp_key_base')
+      end
     end
   end
 end
+# rubocop:enable RSpec/FeatureCategory
+# rubocop:enable RSpec/FilePath