Commit 379c2cbc authored by Sean McGivern's avatar Sean McGivern

Store all secret keys in secrets.yml

Move the last secret from .secret to config/secrets.yml, and delete
.secret if it exists.
parent 405379bb
...@@ -23,6 +23,7 @@ v 8.11.0 (unreleased) ...@@ -23,6 +23,7 @@ v 8.11.0 (unreleased)
- Retrieve rendered HTML from cache in one request - Retrieve rendered HTML from cache in one request
- Fix renaming repository when name contains invalid chararacters under project settings - Fix renaming repository when name contains invalid chararacters under project settings
- Optimize checking if a user has read access to a list of issues !5370 - Optimize checking if a user has read access to a list of issues !5370
- Store all DB secrets in secrets.yml, under descriptive names !5274
- Nokogiri's various parsing methods are now instrumented - Nokogiri's various parsing methods are now instrumented
- Add simple identifier to public SSH keys (muteor) - Add simple identifier to public SSH keys (muteor)
- Add a way to send an email and create an issue based on private personal token. Find the email address from issues page. !3363 - Add a way to send an email and create an issue based on private personal token. Find the email address from issues page. !3363
...@@ -133,7 +134,6 @@ v 8.10.0 ...@@ -133,7 +134,6 @@ v 8.10.0
- Add API "deploy_keys" for admins to get all deploy keys - Add API "deploy_keys" for admins to get all deploy keys
- Allow to pull code with deploy key from public projects - Allow to pull code with deploy key from public projects
- Use limit parameter rather than hardcoded value in `ldap:check` rake task (Mike Ricketts) - Use limit parameter rather than hardcoded value in `ldap:check` rake task (Mike Ricketts)
- Store OTP secret key in secrets.yml with other DB encryption keys
- Add Sidekiq queue duration to transaction metrics. - Add Sidekiq queue duration to transaction metrics.
- Add a new column `artifacts_size` to table `ci_builds`. !4964 - Add a new column `artifacts_size` to table `ci_builds`. !4964
- Let Workhorse serve format-patch diffs - Let Workhorse serve format-patch diffs
......
...@@ -14,35 +14,21 @@ def create_tokens ...@@ -14,35 +14,21 @@ def create_tokens
secret_file = Rails.root.join('.secret') secret_file = Rails.root.join('.secret')
file_key = File.read(secret_file).chomp if File.exist?(secret_file) file_key = File.read(secret_file).chomp if File.exist?(secret_file)
env_key = ENV['SECRET_KEY_BASE'] env_key = ENV['SECRET_KEY_BASE']
secret_key_base = env_key.present? ? env_key : file_key
if secret_key_base.blank?
secret_key_base = generate_new_secure_token
File.write(secret_file, secret_key_base)
end
Rails.application.config.secret_key_base = secret_key_base
otp_key_base = Rails.application.secrets.otp_key_base
db_key_base = Rails.application.secrets.db_key_base
yaml_additions = {} yaml_additions = {}
if otp_key_base.blank? defaults = {
warn_missing_secret('otp_key_base') secret_key_base: env_key || file_key || generate_new_secure_token,
otp_key_base: env_key || file_key || generate_new_secure_token,
otp_key_base ||= env_key || file_key || generate_new_secure_token db_key_base: generate_new_secure_token
yaml_additions['otp_key_base'] = otp_key_base }
end
Rails.application.secrets.otp_key_base = otp_key_base defaults.stringify_keys.each do |key, default|
if Rails.application.secrets[key].blank?
warn_missing_secret(key)
if db_key_base.blank? yaml_additions[key] = Rails.application.secrets[key] = default
warn_missing_secret('db_key_base') end
yaml_additions['db_key_base'] = db_key_base = generate_new_secure_token
end end
Rails.application.secrets.db_key_base = db_key_base
unless yaml_additions.empty? unless yaml_additions.empty?
secrets_yml = Rails.root.join('config/secrets.yml') secrets_yml = Rails.root.join('config/secrets.yml')
...@@ -54,6 +40,12 @@ def create_tokens ...@@ -54,6 +40,12 @@ def create_tokens
File.write(secrets_yml, YAML.dump(all_secrets), mode: 'w', perm: 0600) File.write(secrets_yml, YAML.dump(all_secrets), mode: 'w', perm: 0600)
end end
begin
File.delete(secret_file) if file_key
rescue => e
warn "Error deleting useless .secret file: #{e}"
end
end end
create_tokens create_tokens
...@@ -2,37 +2,36 @@ require 'spec_helper' ...@@ -2,37 +2,36 @@ require 'spec_helper'
require_relative '../../config/initializers/secret_token' require_relative '../../config/initializers/secret_token'
describe 'create_tokens', lib: true do describe 'create_tokens', lib: true do
let(:config) { ActiveSupport::OrderedOptions.new }
let(:secrets) { ActiveSupport::OrderedOptions.new } let(:secrets) { ActiveSupport::OrderedOptions.new }
before do before do
allow(ENV).to receive(:[]).and_call_original allow(ENV).to receive(:[]).and_call_original
allow(File).to receive(:write) allow(File).to receive(:write)
allow(Rails).to receive_message_chain(:application, :config).and_return(config) allow(File).to receive(:delete)
allow(Rails).to receive_message_chain(:application, :secrets).and_return(secrets) allow(Rails).to receive_message_chain(:application, :secrets).and_return(secrets)
allow(Rails).to receive_message_chain(:root, :join) { |string| string } allow(Rails).to receive_message_chain(:root, :join) { |string| string }
end end
context 'setting otp_key_base' do context 'setting secret_key_base and otp_key_base' do
context 'when none of the secrets exist' do context 'when none of the secrets exist' do
before do before do
allow(ENV).to receive(:[]).with('SECRET_KEY_BASE').and_return(nil) allow(ENV).to receive(:[]).with('SECRET_KEY_BASE').and_return(nil)
allow(File).to receive(:exist?).with('.secret').and_return(false) allow(File).to receive(:exist?).with('.secret').and_return(false)
allow(File).to receive(:exist?).with('config/secrets.yml').and_return(false) allow(File).to receive(:exist?).with('config/secrets.yml').and_return(false)
allow(File).to receive(:write)
allow(self).to receive(:warn_missing_secret) allow(self).to receive(:warn_missing_secret)
end end
it 'generates different secrets for secret_key_base, otp_key_base, and db_key_base' do it 'generates different secrets for secret_key_base, otp_key_base, and db_key_base' do
create_tokens create_tokens
keys = [config.secret_key_base, secrets.otp_key_base, secrets.db_key_base] keys = secrets.values_at(:secret_key_base, :otp_key_base, :db_key_base)
expect(keys.uniq).to eq(keys) expect(keys.uniq).to eq(keys)
expect(keys.map(&:length)).to all(eq(128)) expect(keys.map(&:length)).to all(eq(128))
end end
it 'warns about the secrets to add to secrets.yml' do it 'warns about the secrets to add to secrets.yml' do
expect(self).to receive(:warn_missing_secret).with('secret_key_base')
expect(self).to receive(:warn_missing_secret).with('otp_key_base') expect(self).to receive(:warn_missing_secret).with('otp_key_base')
expect(self).to receive(:warn_missing_secret).with('db_key_base') expect(self).to receive(:warn_missing_secret).with('db_key_base')
...@@ -41,25 +40,20 @@ describe 'create_tokens', lib: true do ...@@ -41,25 +40,20 @@ describe 'create_tokens', lib: true do
it 'writes the secrets to secrets.yml' do it 'writes the secrets to secrets.yml' do
expect(File).to receive(:write).with('config/secrets.yml', any_args) do |filename, contents, options| expect(File).to receive(:write).with('config/secrets.yml', any_args) do |filename, contents, options|
new_secrets_yml = YAML.load(contents) new_secrets = YAML.load(contents)[Rails.env]
expect(new_secrets_yml['test']['otp_key_base']).to eq(secrets.otp_key_base) expect(new_secrets['secret_key_base']).to eq(secrets.secret_key_base)
expect(new_secrets_yml['test']['db_key_base']).to eq(secrets.db_key_base) expect(new_secrets['otp_key_base']).to eq(secrets.otp_key_base)
expect(new_secrets['db_key_base']).to eq(secrets.db_key_base)
end end
create_tokens create_tokens
end end
it 'writes the secret_key_base to .secret' do it 'does not write a .secret file' do
secret_key_base = nil expect(File).not_to receive(:write).with('.secret')
expect(File).to receive(:write).with('.secret', any_args) do |filename, contents|
secret_key_base = contents
end
create_tokens create_tokens
expect(secret_key_base).to eq(config.secret_key_base)
end end
end end
...@@ -72,8 +66,11 @@ describe 'create_tokens', lib: true do ...@@ -72,8 +66,11 @@ describe 'create_tokens', lib: true do
allow(File).to receive(:read).with('.secret').and_return('file_key') allow(File).to receive(:read).with('.secret').and_return('file_key')
end end
context 'when the otp_key_base secret exists' do context 'when secret_key_base and otp_key_base exist' do
before { secrets.otp_key_base = 'otp_key_base' } before do
secrets.secret_key_base = 'secret_key_base'
secrets.otp_key_base = 'otp_key_base'
end
it 'does not write any files' do it 'does not write any files' do
expect(File).not_to receive(:write) expect(File).not_to receive(:write)
...@@ -81,22 +78,22 @@ describe 'create_tokens', lib: true do ...@@ -81,22 +78,22 @@ describe 'create_tokens', lib: true do
create_tokens create_tokens
end end
it 'does not generate any new keys' do it 'sets the the keys to the values from secrets.yml' do
expect(SecureRandom).not_to receive(:hex)
create_tokens
end
it 'sets the the keys to the values from the environment and secrets.yml' do
create_tokens create_tokens
expect(config.secret_key_base).to eq('env_key') expect(secrets.secret_key_base).to eq('secret_key_base')
expect(secrets.otp_key_base).to eq('otp_key_base') expect(secrets.otp_key_base).to eq('otp_key_base')
expect(secrets.db_key_base).to eq('db_key_base') expect(secrets.db_key_base).to eq('db_key_base')
end end
it 'deletes the .secret file' do
expect(File).to receive(:delete).with('.secret')
create_tokens
end
end end
context 'when the otp_key_base secret does not exist' do context 'when secret_key_base and otp_key_base do not exist' do
before do before do
allow(File).to receive(:exist?).with('config/secrets.yml').and_return(true) allow(File).to receive(:exist?).with('config/secrets.yml').and_return(true)
allow(YAML).to receive(:load_file).with('config/secrets.yml').and_return('test' => secrets.to_h.stringify_keys) allow(YAML).to receive(:load_file).with('config/secrets.yml').and_return('test' => secrets.to_h.stringify_keys)
...@@ -104,12 +101,12 @@ describe 'create_tokens', lib: true do ...@@ -104,12 +101,12 @@ describe 'create_tokens', lib: true do
end end
it 'uses the env secret' do it 'uses the env secret' do
expect(SecureRandom).not_to receive(:hex)
expect(File).to receive(:write) do |filename, contents, options| expect(File).to receive(:write) do |filename, contents, options|
new_secrets_yml = YAML.load(contents) new_secrets = YAML.load(contents)[Rails.env]
expect(new_secrets_yml['test']['otp_key_base']).to eq('env_key') expect(new_secrets['secret_key_base']).to eq('env_key')
expect(new_secrets_yml['test']['db_key_base']).to eq('db_key_base') expect(new_secrets['otp_key_base']).to eq('env_key')
expect(new_secrets['db_key_base']).to eq('db_key_base')
end end
create_tokens create_tokens
...@@ -120,15 +117,21 @@ describe 'create_tokens', lib: true do ...@@ -120,15 +117,21 @@ describe 'create_tokens', lib: true do
it 'keeps the other secrets as they were' do it 'keeps the other secrets as they were' do
create_tokens create_tokens
expect(config.secret_key_base).to eq('env_key')
expect(secrets.db_key_base).to eq('db_key_base') expect(secrets.db_key_base).to eq('db_key_base')
end end
it 'warns about the missing secret' do it 'warns about the missing secrets' do
expect(self).to receive(:warn_missing_secret).with('secret_key_base')
expect(self).to receive(:warn_missing_secret).with('otp_key_base') expect(self).to receive(:warn_missing_secret).with('otp_key_base')
create_tokens create_tokens
end end
it 'deletes the .secret file' do
expect(File).to receive(:delete).with('.secret')
create_tokens
end
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment