diff options
author | Arturo Herrero <arturo.herrero@gmail.com> | 2019-11-22 17:29:53 +0300 |
---|---|---|
committer | Alessio Caiazza <acaiazza@gitlab.com> | 2019-11-26 16:02:55 +0300 |
commit | aaae14c00eb7fff7c8868d6794231f12c7ca2165 (patch) | |
tree | 58908245d1b1bcfc58981a2bcfe2ca11ac01f066 | |
parent | 9183bf943b36f7505f4ec64c2db14dc3f641b617 (diff) |
Encrypt application settings with pre and post deployments
We had concerns about the cached values on Redis with the previous two
releases strategy:
First release (this commit):
- Create new encrypted fields in the database.
- Start populating new encrypted fields, read the encrypted fields or
fallback to the plaintext fields.
- Backfill the data removing the plaintext fields to the encrypted
fields.
Second release:
- Remove the virtual attribute (created in step 2).
- Drop plaintext columns from the database (empty columns after
step 3).
We end up with a better strategy only using migration scripts in one
release:
- Pre-deployment migration: Add columns required for storing encrypted
values.
- Pre-deployment migration: Store the encrypted values in the new
columns.
- Post-deployment migration: Remove the old unencrypted columns
6 files changed, 30 insertions, 82 deletions
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index dae1235fa6b..72605af433f 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -364,30 +364,6 @@ class ApplicationSetting < ApplicationRecord Gitlab::ThreadMemoryCache.cache_backend end - def akismet_api_key - decrypt(:akismet_api_key, self[:encrypted_akismet_api_key]) || self[:akismet_api_key] - end - - def elasticsearch_aws_secret_access_key - decrypt(:elasticsearch_aws_secret_access_key, self[:encrypted_elasticsearch_aws_secret_access_key]) || self[:elasticsearch_aws_secret_access_key] - end - - def recaptcha_private_key - decrypt(:recaptcha_private_key, self[:encrypted_recaptcha_private_key]) || self[:recaptcha_private_key] - end - - def recaptcha_site_key - decrypt(:recaptcha_site_key, self[:encrypted_recaptcha_site_key]) || self[:recaptcha_site_key] - end - - def slack_app_secret - decrypt(:slack_app_secret, self[:encrypted_slack_app_secret]) || self[:slack_app_secret] - end - - def slack_app_verification_token - decrypt(:slack_app_verification_token, self[:encrypted_slack_app_verification_token]) || self[:slack_app_verification_token] - end - def recaptcha_or_login_protection_enabled recaptcha_enabled || login_recaptcha_protection_enabled end diff --git a/db/migrate/20191120115530_encrypt_plaintext_attributes_on_application_settings.rb b/db/migrate/20191120115530_encrypt_plaintext_attributes_on_application_settings.rb index d7abb29fd75..e6b9a40ad4f 100644 --- a/db/migrate/20191120115530_encrypt_plaintext_attributes_on_application_settings.rb +++ b/db/migrate/20191120115530_encrypt_plaintext_attributes_on_application_settings.rb @@ -70,12 +70,6 @@ class EncryptPlaintextAttributesOnApplicationSettings < ActiveRecord::Migration[ end ) application_setting.save(validate: false) - - application_setting.update_columns( - PLAINTEXT_ATTRIBUTES.each_with_object({}) do |plaintext_attribute, attributes| - attributes[plaintext_attribute] = nil - end - ) end end diff --git a/db/post_migrate/20191122135327_remove_plaintext_columns_from_application_settings.rb b/db/post_migrate/20191122135327_remove_plaintext_columns_from_application_settings.rb new file mode 100644 index 00000000000..b5cd58b10a8 --- /dev/null +++ b/db/post_migrate/20191122135327_remove_plaintext_columns_from_application_settings.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class RemovePlaintextColumnsFromApplicationSettings < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + PLAINTEXT_ATTRIBUTES = %w[ + akismet_api_key + elasticsearch_aws_secret_access_key + recaptcha_private_key + recaptcha_site_key + slack_app_secret + slack_app_verification_token + ].freeze + + def up + PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute| + remove_column :application_settings, plaintext_attribute + end + end + + def down + PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute| + add_column :application_settings, plaintext_attribute, :text + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 29d812e250e..ef1831d79a1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -180,11 +180,8 @@ ActiveRecord::Schema.define(version: 2019_11_24_150431) do t.integer "metrics_timeout", default: 10 t.integer "metrics_method_call_threshold", default: 10 t.boolean "recaptcha_enabled", default: false - t.string "recaptcha_site_key" - t.string "recaptcha_private_key" t.integer "metrics_port", default: 8089 t.boolean "akismet_enabled", default: false - t.string "akismet_api_key" t.integer "metrics_sample_interval", default: 15 t.boolean "email_author_in_body", default: false t.integer "default_group_visibility" @@ -231,7 +228,6 @@ ActiveRecord::Schema.define(version: 2019_11_24_150431) do t.boolean "elasticsearch_aws", default: false, null: false t.string "elasticsearch_aws_region", default: "us-east-1" t.string "elasticsearch_aws_access_key" - t.string "elasticsearch_aws_secret_access_key" t.integer "geo_status_timeout", default: 10 t.string "uuid" t.decimal "polling_interval_multiplier", default: "1.0", null: false @@ -247,8 +243,6 @@ ActiveRecord::Schema.define(version: 2019_11_24_150431) do t.string "help_page_support_url" t.boolean "slack_app_enabled", default: false t.string "slack_app_id" - t.string "slack_app_secret" - t.string "slack_app_verification_token" t.integer "performance_bar_allowed_group_id" t.boolean "allow_group_owners_to_manage_ldap", default: true, null: false t.boolean "hashed_storage_enabled", default: true, null: false diff --git a/spec/migrations/encrypt_plaintext_attributes_on_application_settings_spec.rb b/spec/migrations/encrypt_plaintext_attributes_on_application_settings_spec.rb index 6435e43f38c..122da7b3d72 100644 --- a/spec/migrations/encrypt_plaintext_attributes_on_application_settings_spec.rb +++ b/spec/migrations/encrypt_plaintext_attributes_on_application_settings_spec.rb @@ -18,7 +18,7 @@ describe EncryptPlaintextAttributesOnApplicationSettings, :migration do ].freeze describe '#up' do - it 'encrypts token, saves it and removes plaintext token' do + it 'encrypts token and saves it' do application_setting = application_settings.create application_setting.update_columns( PLAINTEXT_ATTRIBUTES.each_with_object({}) do |plaintext_attribute, attributes| @@ -30,7 +30,7 @@ describe EncryptPlaintextAttributesOnApplicationSettings, :migration do application_setting.reload PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute| - expect(application_setting[plaintext_attribute]).to be_nil + expect(application_setting[plaintext_attribute]).not_to be_nil expect(application_setting["encrypted_#{plaintext_attribute}"]).not_to be_nil expect(application_setting["encrypted_#{plaintext_attribute}_iv"]).not_to be_nil end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 7b1ebe586cd..ba3b99f4421 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -15,50 +15,6 @@ describe ApplicationSetting do it { expect(setting.uuid).to be_present } it { expect(setting).to have_db_column(:auto_devops_enabled) } - context "with existing plaintext attributes" do - before do - setting.update_columns( - akismet_api_key: "akismet_api_key", - elasticsearch_aws_secret_access_key: "elasticsearch_aws_secret_access_key", - recaptcha_private_key: "recaptcha_private_key", - recaptcha_site_key: "recaptcha_site_key", - slack_app_secret: "slack_app_secret", - slack_app_verification_token: "slack_app_verification_token" - ) - end - - it "returns the attributes" do - expect(setting.akismet_api_key).to eq("akismet_api_key") - expect(setting.elasticsearch_aws_secret_access_key).to eq("elasticsearch_aws_secret_access_key") - expect(setting.recaptcha_private_key).to eq("recaptcha_private_key") - expect(setting.recaptcha_site_key).to eq("recaptcha_site_key") - expect(setting.slack_app_secret).to eq("slack_app_secret") - expect(setting.slack_app_verification_token).to eq("slack_app_verification_token") - end - end - - context "with encrypted attributes" do - before do - setting.update( - akismet_api_key: "akismet_api_key", - elasticsearch_aws_secret_access_key: "elasticsearch_aws_secret_access_key", - recaptcha_private_key: "recaptcha_private_key", - recaptcha_site_key: "recaptcha_site_key", - slack_app_secret: "slack_app_secret", - slack_app_verification_token: "slack_app_verification_token" - ) - end - - it "returns the attributes" do - expect(setting.akismet_api_key).to eq("akismet_api_key") - expect(setting.elasticsearch_aws_secret_access_key).to eq("elasticsearch_aws_secret_access_key") - expect(setting.recaptcha_private_key).to eq("recaptcha_private_key") - expect(setting.recaptcha_site_key).to eq("recaptcha_site_key") - expect(setting.slack_app_secret).to eq("slack_app_secret") - expect(setting.slack_app_verification_token).to eq("slack_app_verification_token") - end - end - describe 'validations' do let(:http) { 'http://example.com' } let(:https) { 'https://example.com' } |