diff --git a/app/controllers/jwks_controller.rb b/app/controllers/jwks_controller.rb
index 2574f6e14d51a1d5c0aba5a53ccc1fbbb285db14..e4e00ecfb79053e36cb77ec005653c9fd4f5e77e 100644
--- a/app/controllers/jwks_controller.rb
+++ b/app/controllers/jwks_controller.rb
@@ -11,7 +11,7 @@ def keys
def payload
[
- Rails.application.secrets.openid_connect_signing_key,
+ Rails.application.credentials.openid_connect_signing_key,
Gitlab::CurrentSettings.ci_jwt_signing_key
].compact.map do |key_data|
OpenSSL::PKey::RSA.new(key_data)
diff --git a/config/initializers/00_deprecations.rb b/config/initializers/00_deprecations.rb
index eeed495a2972a05c9a2651c0ed1b027ebbb948ba..7fcf50e90b93145df8151a5ad3861dd95ecb312c 100644
--- a/config/initializers/00_deprecations.rb
+++ b/config/initializers/00_deprecations.rb
@@ -18,7 +18,6 @@
deprecators.silenced = silenced
ignored_warnings = [
- /`Rails.application.secrets` is deprecated in favor of `Rails.application.credentials`/,
/Your `secret_key_base` is configured in `Rails.application.secrets`, which is deprecated in favor of/,
/Please pass the (coder|class) as a keyword argument/
]
diff --git a/config/initializers/01_secret_token.rb b/config/initializers/01_secret_token.rb
index bb13869c963dd182209de008880182c599bf7bb7..886b6916e5f3b309c44fcb212e3a9ae742e7f8d5 100644
--- a/config/initializers/01_secret_token.rb
+++ b/config/initializers/01_secret_token.rb
@@ -10,43 +10,45 @@
require 'securerandom'
-# Transition material in .secret to the secret_key_base key in config/secrets.yml.
-# Historically, ENV['SECRET_KEY_BASE'] takes precedence over .secret, so we maintain that
-# behavior.
-#
-# It also used to be the case that the key material in ENV['SECRET_KEY_BASE'] or .secret
-# was used to encrypt OTP (two-factor authentication) data so if present, we copy that key
-# material into config/secrets.yml under otp_key_base.
-#
-# Finally, if we have successfully migrated all secrets to config/secrets.yml, delete the
-# .secret file to avoid confusion.
-#
def create_tokens
- secret_file = Rails.root.join('.secret')
- file_secret_key = File.read(secret_file).chomp if File.exist?(secret_file)
- env_secret_key = ENV['SECRET_KEY_BASE']
+ # Inspired by https://github.com/rails/rails/blob/v7.0.8.4/railties/lib/rails/secrets.rb#L25-L36
+ raw_secrets = begin
+ YAML.safe_load(File.read(Rails.root.join('config/secrets.yml')))
+ rescue Errno::ENOENT, Psych::SyntaxError
+ {}
+ end
+ raw_secrets ||= {}
- # Ensure environment variable always overrides secrets.yml.
- Rails.application.secrets.secret_key_base = env_secret_key if env_secret_key.present?
+ secrets = {}
+ secrets.merge!(raw_secrets["shared"].deep_symbolize_keys) if raw_secrets["shared"]
+ secrets.merge!(raw_secrets[Rails.env].deep_symbolize_keys) if raw_secrets[Rails.env]
+
+ # Copy secrets into credentials since Rails.application.secrets is populated from config/secrets.yml
+ # Later, once config/secrets.yml won't be read automatically, we'll need to do it manually, and set
+ secrets.each do |key, value|
+ Rails.application.credentials[key] = value
+ end
+
+ # Historically, ENV['SECRET_KEY_BASE'] takes precedence over secrets.yml, so we maintain that
+ # behavior by ensuring the environment variable always overrides secrets.yml.
+ env_secret_key = ENV['SECRET_KEY_BASE']
+ Rails.application.credentials.secret_key_base = env_secret_key if env_secret_key.present?
defaults = {
- secret_key_base: file_secret_key || generate_new_secure_token,
- otp_key_base: env_secret_key || file_secret_key || generate_new_secure_token,
+ 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
- defaults[:encrypted_settings_key_base] = generate_new_secure_token if ENV['GITLAB_GENERATE_ENCRYPTED_SETTINGS_KEY_BASE']
+ 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) unless missing_secrets.empty?
-
- begin
- File.delete(secret_file) if file_secret_key
- rescue StandardError => e
- warn "Error deleting useless .secret file: #{e}"
- end
end
def generate_new_secure_token
@@ -60,15 +62,15 @@ def generate_new_rsa_private_key
def warn_missing_secret(secret)
return if Rails.env.test?
- warn "Missing Rails.application.secrets.#{secret} for #{Rails.env} environment. The secret will be generated and stored in config/secrets.yml."
+ warn "Missing Rails.application.credentials.#{secret} for #{Rails.env} environment. The secret will be generated and stored in config/secrets.yml."
end
def set_missing_keys(defaults)
defaults.stringify_keys.each_with_object({}) do |(key, default), missing|
- next if Rails.application.secrets[key].present?
+ next if Rails.application.credentials.public_send(key).present?
warn_missing_secret(key)
- missing[key] = Rails.application.secrets[key] = default
+ missing[key] = Rails.application.credentials[key] = default
end
end
@@ -79,26 +81,8 @@ def write_secrets_yml(missing_secrets)
secrets ||= {}
secrets[rails_env] ||= {}
- secrets[rails_env].merge!(missing_secrets) do |key, old, new|
- # Previously, it was possible this was set to the literal contents of an Erb
- # expression that evaluated to an empty value. We don't want to support that
- # specifically, just ensure we don't break things further.
- #
- if old.present?
- warn <<EOM
-Rails.application.secrets.#{key} was blank, but the literal value in config/secrets.yml was:
- #{old}
-
-This probably isn't the expected value for this secret. To keep using a literal Erb string in config/secrets.yml, replace `<%` with `<%%`.
-EOM
-
- exit 1 # rubocop:disable Rails/Exit
- end
-
- new
- end
-
- File.write(secrets_yml, YAML.dump(secrets), mode: 'w', perm: 0600)
+ secrets[rails_env].merge!(missing_secrets)
+ File.write(secrets_yml, YAML.dump(secrets), mode: 'w', perm: 0o600)
end
create_tokens
diff --git a/spec/initializers/secret_token_spec.rb b/spec/initializers/secret_token_spec.rb
index 1520cbb4ac237a05ab0d139342478c53230c336e..67228539db9acd9280b4da880f0c51fa59f32eaa 100644
--- a/spec/initializers/secret_token_spec.rb
+++ b/spec/initializers/secret_token_spec.rb
@@ -11,20 +11,19 @@
let(:rsa_key) { /\A-----BEGIN RSA PRIVATE KEY-----\n.+\n-----END RSA PRIVATE KEY-----\n\Z/m }
before do
- allow(Rails).to receive_message_chain(:application, :secrets).and_return(secrets)
+ allow(Rails).to receive_message_chain(:application, :credentials).and_return(secrets)
allow(Rails).to receive_message_chain(:root, :join) { |string| string }
allow(File).to receive(:write).and_call_original
allow(File).to receive(:write).with(Rails.root.join('config/secrets.yml'))
- allow(File).to receive(:delete).and_call_original
- allow(File).to receive(:delete).with(Rails.root.join('.secret'))
allow(self).to receive(:warn)
allow(self).to receive(:exit)
end
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 rotated_encrypted_settings_key_base]
+ %w[secret_key_base otp_key_base db_key_base openid_connect_signing_key encrypted_settings_key_base
+ rotated_encrypted_settings_key_base]
end
it 'does not allow to add a new secret without a proper handling' do
@@ -50,7 +49,6 @@
context 'when none of the secrets exist' do
before do
stub_env('SECRET_KEY_BASE', nil)
- allow(File).to receive(:exist?).with('.secret').and_return(false)
allow(File).to receive(:exist?).with('config/secrets.yml').and_return(false)
allow(self).to receive(:warn_missing_secret)
end
@@ -95,12 +93,6 @@
create_tokens
end
-
- it 'does not write a .secret file' do
- expect(File).not_to receive(:write).with('.secret')
-
- create_tokens
- end
end
context 'when the other secrets all exist' do
@@ -108,9 +100,6 @@
secrets.db_key_base = 'db_key_base'
secrets.openid_connect_signing_key = 'openid_connect_signing_key'
secrets.encrypted_settings_key_base = 'encrypted_settings_key_base'
-
- allow(File).to receive(:exist?).with('.secret').and_return(true)
- stub_file_read('.secret', content: 'file_key')
end
context 'when secret_key_base exists in the environment and secrets.yml' do
@@ -162,12 +151,6 @@
expect(secrets.openid_connect_signing_key).to eq('openid_connect_signing_key')
expect(secrets.encrypted_settings_key_base).to eq('encrypted_settings_key_base')
end
-
- it 'deletes the .secret file' do
- expect(File).to receive(:delete).with('.secret')
-
- create_tokens
- end
end
context 'when secret_key_base and otp_key_base do not exist' do
@@ -177,21 +160,6 @@
allow(self).to receive(:warn_missing_secret)
end
- it 'uses the file secret' do
- expect(File).to receive(:write) do |filename, contents, options|
- new_secrets = YAML.safe_load(contents)[Rails.env]
-
- expect(new_secrets['secret_key_base']).to eq('file_key')
- expect(new_secrets['otp_key_base']).to eq('file_key')
- expect(new_secrets['db_key_base']).to eq('db_key_base')
- expect(new_secrets['openid_connect_signing_key']).to eq('openid_connect_signing_key')
- end
-
- create_tokens
-
- expect(secrets.otp_key_base).to eq('file_key')
- end
-
it 'keeps the other secrets as they were' do
create_tokens
@@ -204,12 +172,6 @@
create_tokens
end
-
- it 'deletes the .secret file' do
- expect(File).to receive(:delete).with('.secret')
-
- create_tokens
- end
end
context 'when rotated_encrypted_settings_key_base does not exist' do
@@ -253,15 +215,6 @@
create_tokens
end
- it 'warns about the blank value existing in secrets.yml and exits' do
- expect(self).to receive(:warn) do |warning|
- expect(warning).to include('db_key_base')
- expect(warning).to include('<%= an_erb_expression %>')
- end
-
- create_tokens
- end
-
it 'does not update secrets.yml' do
expect(self).to receive(:exit).with(1).and_call_original
expect(File).not_to receive(:write)
diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb
index 1f0f6d44a1d00f9c33b4e17f9301d52349cfdbc1..88a52b250f4b0df7eb9f7bc847c14f0d33a5e0e3 100644
--- a/spec/requests/openid_connect_spec.rb
+++ b/spec/requests/openid_connect_spec.rb
@@ -192,7 +192,8 @@ def request_user_info!
end
it 'does not include any unknown properties' do
- expect(@payload.keys).to eq %w[iss sub aud exp iat auth_time sub_legacy name nickname preferred_username email email_verified website profile picture groups_direct]
+ expect(@payload.keys).to eq %w[iss sub aud exp iat auth_time sub_legacy name nickname preferred_username email
+ email_verified website profile picture groups_direct]
end
it 'does include groups' do