Merge branch '199716_encrypt_static_objects_external_storage_auth_token' into 'master'

Encrypt plaintext static_objects_external_storage_auth_token fields

See merge request gitlab-org/gitlab!75417

app/models/application_setting.rb

@@ -21,7 +21,7 @@ class ApplicationSetting < ApplicationRecord
 
   add_authentication_token_field :runners_registration_token, encrypted: -> { Feature.enabled?(:application_settings_tokens_optional_encryption) ? :optional : :required }
   add_authentication_token_field :health_check_access_token
-  add_authentication_token_field :static_objects_external_storage_auth_token
+  add_authentication_token_field :static_objects_external_storage_auth_token, encrypted: :optional
 
   belongs_to :self_monitoring_project, class_name: "Project", foreign_key: 'instance_administration_project_id'
   belongs_to :push_rule

app/models/application_setting_implementation.rb

@@ -363,6 +363,14 @@ module ApplicationSettingImplementation
     super(levels&.map { |level| Gitlab::VisibilityLevel.level_value(level) })
   end
 
+  def static_objects_external_storage_auth_token=(token)
+    if token.present?
+      set_static_objects_external_storage_auth_token(token)
+    else
+      self.static_objects_external_storage_auth_token_encrypted = nil
+    end
+  end
+
   def performance_bar_allowed_group
     Group.find_by_id(performance_bar_allowed_group_id)
   end

db/migrate/20211126115449_encrypt_static_objects_external_storage_auth_token.rb

+# frozen_string_literal: true
+
+class EncryptStaticObjectsExternalStorageAuthToken < Gitlab::Database::Migration[1.0]
+  class ApplicationSetting < ActiveRecord::Base
+    self.table_name = 'application_settings'
+
+    scope :encrypted_token_is_null, -> { where(static_objects_external_storage_auth_token_encrypted: nil) }
+    scope :encrypted_token_is_not_null, -> { where.not(static_objects_external_storage_auth_token_encrypted: nil) }
+    scope :plaintext_token_is_not_null, -> { where.not(static_objects_external_storage_auth_token: nil) }
+  end
+
+  def up
+    ApplicationSetting.reset_column_information
+
+    ApplicationSetting.encrypted_token_is_null.plaintext_token_is_not_null.find_each do |application_setting|
+      token_encrypted = Gitlab::CryptoHelper.aes256_gcm_encrypt(application_setting.static_objects_external_storage_auth_token)
+      application_setting.update!(static_objects_external_storage_auth_token_encrypted: token_encrypted)
+    end
+  end
+
+  def down
+    ApplicationSetting.reset_column_information
+
+    ApplicationSetting.encrypted_token_is_not_null.find_each do |application_setting|
+      token = Gitlab::CryptoHelper.aes256_gcm_decrypt(application_setting.static_objects_external_storage_auth_token_encrypted)
+      application_setting.update!(static_objects_external_storage_auth_token: token, static_objects_external_storage_auth_token_encrypted: nil)
+    end
+  end
+end

db/schema_migrations/20211126115449

+2e6e432ecf7b2c885905fd4df6b57fa99b324f56cb0850d9fc792b4a9b363423
\ No newline at end of file

spec/features/admin/admin_settings_spec.rb

@@ -445,6 +445,24 @@ RSpec.describe 'Admin updates settings' do
 
         expect(current_settings.repository_storages_weighted).to eq('default' => 50)
       end
+
+      context 'External storage for repository static objects' do
+        it 'changes Repository external storage settings' do
+          encrypted_token = Gitlab::CryptoHelper.aes256_gcm_encrypt('OldToken')
+          current_settings.update_attribute :static_objects_external_storage_auth_token_encrypted, encrypted_token
+
+          visit repository_admin_application_settings_path
+
+          page.within('.as-repository-static-objects') do
+            fill_in 'application_setting_static_objects_external_storage_url', with: 'http://example.com'
+            fill_in 'application_setting_static_objects_external_storage_auth_token', with: 'Token'
+            click_button 'Save changes'
+          end
+
+          expect(current_settings.static_objects_external_storage_url).to eq('http://example.com')
+          expect(current_settings.static_objects_external_storage_auth_token).to eq('Token')
+        end
+      end
     end
 
     context 'Reporting page' do

spec/migrations/20211126115449_encrypt_static_objects_external_storage_auth_token_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe EncryptStaticObjectsExternalStorageAuthToken, :migration do
+  let(:application_settings) do
+    Class.new(ActiveRecord::Base) do
+      self.table_name = 'application_settings'
+    end
+  end
+
+  context 'when static_objects_external_storage_auth_token is not set' do
+    it 'does nothing' do
+      application_settings.create!
+
+      reversible_migration do |migration|
+        migration.before -> {
+          settings = application_settings.first
+
+          expect(settings.static_objects_external_storage_auth_token).to be_nil
+          expect(settings.static_objects_external_storage_auth_token_encrypted).to be_nil
+        }
+
+        migration.after -> {
+          settings = application_settings.first
+
+          expect(settings.static_objects_external_storage_auth_token).to be_nil
+          expect(settings.static_objects_external_storage_auth_token_encrypted).to be_nil
+        }
+      end
+    end
+  end
+
+  context 'when static_objects_external_storage_auth_token is set' do
+    it 'encrypts static_objects_external_storage_auth_token' do
+      settings = application_settings.create!
+      settings.update_column(:static_objects_external_storage_auth_token, 'Test')
+
+      reversible_migration do |migration|
+        migration.before -> {
+          settings = application_settings.first
+
+          expect(settings.static_objects_external_storage_auth_token).to eq('Test')
+          expect(settings.static_objects_external_storage_auth_token_encrypted).to be_nil
+        }
+        migration.after -> {
+          settings = application_settings.first
+
+          expect(settings.static_objects_external_storage_auth_token).to eq('Test')
+          expect(settings.static_objects_external_storage_auth_token_encrypted).to be_present
+        }
+      end
+    end
+  end
+end

spec/models/application_setting_spec.rb

@@ -1239,4 +1239,30 @@ RSpec.describe ApplicationSetting do
       expect(subject.kroki_formats_excalidraw).to eq(true)
     end
   end
+
+  describe '#static_objects_external_storage_auth_token=' do
+    subject { setting.static_objects_external_storage_auth_token = token }
+
+    let(:token) { 'Test' }
+
+    it 'stores an encrypted version of the token' do
+      subject
+
+      expect(setting[:static_objects_external_storage_auth_token]).to be_nil
+      expect(setting[:static_objects_external_storage_auth_token_encrypted]).to be_present
+      expect(setting.static_objects_external_storage_auth_token).to eq('Test')
+    end
+
+    context 'when token is empty' do
+      let(:token) { '' }
+
+      it 'removes an encrypted version of the token' do
+        subject
+
+        expect(setting[:static_objects_external_storage_auth_token]).to be_nil
+        expect(setting[:static_objects_external_storage_auth_token_encrypted]).to be_nil
+        expect(setting.static_objects_external_storage_auth_token).to be_nil
+      end
+    end
+  end
 end