diff options
8 files changed, 46 insertions, 15 deletions
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 207ffae873a..4319db42019 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -7,7 +7,7 @@ class ApplicationSetting < ActiveRecord::Base include IgnorableColumn include ChronicDurationAttribute - add_authentication_token_field :runners_registration_token + add_authentication_token_field :runners_registration_token, encrypted: true, fallback: true add_authentication_token_field :health_check_access_token DOMAIN_LIST_SEPARATOR = %r{\s*[,;]\s* # comma or semicolon, optionally surrounded by whitespace diff --git a/app/models/concerns/token_authenticatable.rb b/app/models/concerns/token_authenticatable.rb index 23a43aec677..f881d119a17 100644 --- a/app/models/concerns/token_authenticatable.rb +++ b/app/models/concerns/token_authenticatable.rb @@ -9,7 +9,7 @@ module TokenAuthenticatable private # rubocop:disable Lint/UselessAccessModifier def add_authentication_token_field(token_field, options = {}) - @token_fields = [] unless @token_fields + @token_fields ||= [] unique = options.fetch(:unique, true) if @token_fields.include?(token_field) @@ -22,6 +22,8 @@ module TokenAuthenticatable strategy = if options[:digest] TokenAuthenticatableStrategies::Digest.new(self, token_field, options) + elsif options[:encrypted] + TokenAuthenticatableStrategies::Encrypted.new(self, token_field, options) else TokenAuthenticatableStrategies::Insecure.new(self, token_field, options) end diff --git a/app/models/concerns/token_authenticatable_strategies/base.rb b/app/models/concerns/token_authenticatable_strategies/base.rb index ef1b2487cea..53b09ffd4d9 100644 --- a/app/models/concerns/token_authenticatable_strategies/base.rb +++ b/app/models/concerns/token_authenticatable_strategies/base.rb @@ -24,6 +24,7 @@ module TokenAuthenticatableStrategies def ensure_token(instance) write_new_token(instance) unless token_set?(instance) + get_token(instance) end # Returns a token, but only saves when the database is in read & write mode diff --git a/app/models/concerns/token_authenticatable_strategies/encrypted.rb b/app/models/concerns/token_authenticatable_strategies/encrypted.rb index e2a5a973d4f..e6cf4f4ac58 100644 --- a/app/models/concerns/token_authenticatable_strategies/encrypted.rb +++ b/app/models/concerns/token_authenticatable_strategies/encrypted.rb @@ -28,6 +28,7 @@ module TokenAuthenticatableStrategies raise ArgumentError unless token.present? instance[encrypted_field] = Gitlab::CryptoHelper.aes256_gcm_encrypt(token) + token end protected diff --git a/db/migrate/20181115140140_add_encrypted_runners_token_to_settings.rb b/db/migrate/20181115140140_add_encrypted_runners_token_to_settings.rb new file mode 100644 index 00000000000..36d9ad45b19 --- /dev/null +++ b/db/migrate/20181115140140_add_encrypted_runners_token_to_settings.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddEncryptedRunnersTokenToSettings < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :application_settings, :runners_registration_token_encrypted, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 56137caf1d7..82e9c8f28e0 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20181107054254) do +ActiveRecord::Schema.define(version: 20181115140140) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -167,6 +167,7 @@ ActiveRecord::Schema.define(version: 20181107054254) do t.integer "diff_max_patch_bytes", default: 102400, null: false t.integer "archive_builds_in_seconds" t.string "commit_email_hostname" + t.string "runners_registration_token_encrypted" end create_table "audit_events", force: :cascade do |t| diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index 782687516ae..0cdf430e9ab 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -21,44 +21,59 @@ end describe ApplicationSetting, 'TokenAuthenticatable' do let(:token_field) { :runners_registration_token } + let(:settings) { described_class.new } + it_behaves_like 'TokenAuthenticatable' describe 'generating new token' do context 'token is not generated yet' do describe 'token field accessor' do - subject { described_class.new.send(token_field) } + subject { settings.send(token_field) } + it { is_expected.not_to be_blank } end - describe 'ensured token' do - subject { described_class.new.send("ensure_#{token_field}") } + describe "ensure_runners_registration_token" do + subject { settings.send("ensure_#{token_field}") } it { is_expected.to be_a String } it { is_expected.not_to be_blank } + + it 'does not persist token' do + expect(settings).not_to be_persisted + end end - describe 'ensured! token' do - subject { described_class.new.send("ensure_#{token_field}!") } + describe 'ensure_runners_registration_token!' do + subject { settings.send("ensure_#{token_field}!") } + + it 'persists new token as an encrypted string' do + expect(subject).to eq settings.reload.runners_registration_token + expect(settings.read_attribute('runners_registration_token_encrypted')) + .to eq Gitlab::CryptoHelper.aes256_gcm_encrypt(subject) + expect(settings).to be_persisted + end - it 'persists new token' do - expect(subject).to eq described_class.current[token_field] + it 'does not persist token in a clear text' do + expect(subject).not_to eq settings.reload + .read_attribute('runners_registration_token_encrypted') end end end context 'token is generated' do before do - subject.send("reset_#{token_field}!") + settings.send("reset_#{token_field}!") end it 'persists a new token' do - expect(subject.send(:read_attribute, token_field)).to be_a String + expect(settings.runners_registration_token).to be_a String end end end describe 'setting new token' do - subject { described_class.new.send("set_#{token_field}", '0123456789') } + subject { settings.send("set_#{token_field}", '0123456789') } it { is_expected.to eq '0123456789' } end diff --git a/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb index e09da304cfb..851e04942c6 100644 --- a/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb +++ b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb @@ -60,11 +60,11 @@ describe TokenAuthenticatableStrategies::Encrypted do end describe '#set_token' do - it 'writes encrypted token to a model instance' do + it 'writes encrypted token to a model instance and returns it' do expect(instance).to receive(:[]=) .with('some_field_encrypted', encrypted) - subject.set_token(instance, 'my-value') + expect(subject.set_token(instance, 'my-value')).to eq 'my-value' end end end |