Commit aaae14c0 authored by Arturo Herrero's avatar Arturo Herrero Committed by Alessio Caiazza

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
parent 9183bf94
...@@ -364,30 +364,6 @@ class ApplicationSetting < ApplicationRecord ...@@ -364,30 +364,6 @@ class ApplicationSetting < ApplicationRecord
Gitlab::ThreadMemoryCache.cache_backend Gitlab::ThreadMemoryCache.cache_backend
end 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 def recaptcha_or_login_protection_enabled
recaptcha_enabled || login_recaptcha_protection_enabled recaptcha_enabled || login_recaptcha_protection_enabled
end end
......
...@@ -70,12 +70,6 @@ class EncryptPlaintextAttributesOnApplicationSettings < ActiveRecord::Migration[ ...@@ -70,12 +70,6 @@ class EncryptPlaintextAttributesOnApplicationSettings < ActiveRecord::Migration[
end end
) )
application_setting.save(validate: false) 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
end end
......
# 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
...@@ -180,11 +180,8 @@ ActiveRecord::Schema.define(version: 2019_11_24_150431) do ...@@ -180,11 +180,8 @@ ActiveRecord::Schema.define(version: 2019_11_24_150431) do
t.integer "metrics_timeout", default: 10 t.integer "metrics_timeout", default: 10
t.integer "metrics_method_call_threshold", default: 10 t.integer "metrics_method_call_threshold", default: 10
t.boolean "recaptcha_enabled", default: false t.boolean "recaptcha_enabled", default: false
t.string "recaptcha_site_key"
t.string "recaptcha_private_key"
t.integer "metrics_port", default: 8089 t.integer "metrics_port", default: 8089
t.boolean "akismet_enabled", default: false t.boolean "akismet_enabled", default: false
t.string "akismet_api_key"
t.integer "metrics_sample_interval", default: 15 t.integer "metrics_sample_interval", default: 15
t.boolean "email_author_in_body", default: false t.boolean "email_author_in_body", default: false
t.integer "default_group_visibility" t.integer "default_group_visibility"
...@@ -231,7 +228,6 @@ ActiveRecord::Schema.define(version: 2019_11_24_150431) do ...@@ -231,7 +228,6 @@ ActiveRecord::Schema.define(version: 2019_11_24_150431) do
t.boolean "elasticsearch_aws", default: false, null: false t.boolean "elasticsearch_aws", default: false, null: false
t.string "elasticsearch_aws_region", default: "us-east-1" t.string "elasticsearch_aws_region", default: "us-east-1"
t.string "elasticsearch_aws_access_key" t.string "elasticsearch_aws_access_key"
t.string "elasticsearch_aws_secret_access_key"
t.integer "geo_status_timeout", default: 10 t.integer "geo_status_timeout", default: 10
t.string "uuid" t.string "uuid"
t.decimal "polling_interval_multiplier", default: "1.0", null: false t.decimal "polling_interval_multiplier", default: "1.0", null: false
...@@ -247,8 +243,6 @@ ActiveRecord::Schema.define(version: 2019_11_24_150431) do ...@@ -247,8 +243,6 @@ ActiveRecord::Schema.define(version: 2019_11_24_150431) do
t.string "help_page_support_url" t.string "help_page_support_url"
t.boolean "slack_app_enabled", default: false t.boolean "slack_app_enabled", default: false
t.string "slack_app_id" t.string "slack_app_id"
t.string "slack_app_secret"
t.string "slack_app_verification_token"
t.integer "performance_bar_allowed_group_id" t.integer "performance_bar_allowed_group_id"
t.boolean "allow_group_owners_to_manage_ldap", default: true, null: false t.boolean "allow_group_owners_to_manage_ldap", default: true, null: false
t.boolean "hashed_storage_enabled", default: true, null: false t.boolean "hashed_storage_enabled", default: true, null: false
......
...@@ -18,7 +18,7 @@ describe EncryptPlaintextAttributesOnApplicationSettings, :migration do ...@@ -18,7 +18,7 @@ describe EncryptPlaintextAttributesOnApplicationSettings, :migration do
].freeze ].freeze
describe '#up' do 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 = application_settings.create
application_setting.update_columns( application_setting.update_columns(
PLAINTEXT_ATTRIBUTES.each_with_object({}) do |plaintext_attribute, attributes| PLAINTEXT_ATTRIBUTES.each_with_object({}) do |plaintext_attribute, attributes|
...@@ -30,7 +30,7 @@ describe EncryptPlaintextAttributesOnApplicationSettings, :migration do ...@@ -30,7 +30,7 @@ describe EncryptPlaintextAttributesOnApplicationSettings, :migration do
application_setting.reload application_setting.reload
PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute| 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}"]).not_to be_nil
expect(application_setting["encrypted_#{plaintext_attribute}_iv"]).not_to be_nil expect(application_setting["encrypted_#{plaintext_attribute}_iv"]).not_to be_nil
end end
......
...@@ -15,50 +15,6 @@ describe ApplicationSetting do ...@@ -15,50 +15,6 @@ describe ApplicationSetting do
it { expect(setting.uuid).to be_present } it { expect(setting.uuid).to be_present }
it { expect(setting).to have_db_column(:auto_devops_enabled) } 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 describe 'validations' do
let(:http) { 'http://example.com' } let(:http) { 'http://example.com' }
let(:https) { 'https://example.com' } let(:https) { 'https://example.com' }
......
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