diff options
author | Nick Thomas <nick@gitlab.com> | 2018-12-04 16:48:30 +0000 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2018-12-04 16:48:30 +0000 |
commit | 80163b972a7bbbd203bb86e1d1474a592979e6d7 (patch) | |
tree | 43d11c56f1020cb0b7b9f20547edea280ae8af26 | |
parent | 42a7d3b7fe22e193bab1edca0165ceafe1d6b7bd (diff) | |
parent | a1bd34e9c04c79488dc20ad0af08b0c94bffe675 (diff) | |
download | gitlab-ce-80163b972a7bbbd203bb86e1d1474a592979e6d7.tar.gz |
Merge branch 'fix/gb/encrypt-runners-tokens' into 'master'
Encrypt runners tokens
Closes #51232 and #52931
See merge request gitlab-org/gitlab-ce!23412
37 files changed, 989 insertions, 74 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/ci/runner.rb b/app/models/ci/runner.rb index 31330d0682e..260348c97b2 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -8,6 +8,9 @@ module Ci include RedisCacheable include ChronicDurationAttribute include FromUnion + include TokenAuthenticatable + + add_authentication_token_field :token, encrypted: true, migrating: true enum access_level: { not_protected: 0, @@ -39,7 +42,7 @@ module Ci has_one :last_build, ->() { order('id DESC') }, class_name: 'Ci::Build' - before_validation :set_default_values + before_save :ensure_token scope :active, -> { where(active: true) } scope :paused, -> { where(active: false) } @@ -145,10 +148,6 @@ module Ci end end - def set_default_values - self.token = SecureRandom.hex(15) if self.token.blank? - end - def assign_to(project, current_user = nil) if instance_type? self.runner_type = :project_type diff --git a/app/models/concerns/token_authenticatable.rb b/app/models/concerns/token_authenticatable.rb index be930b6197a..f5bb559ceda 100644 --- a/app/models/concerns/token_authenticatable.rb +++ b/app/models/concerns/token_authenticatable.rb @@ -9,24 +9,18 @@ module TokenAuthenticatable private # rubocop:disable Lint/UselessAccessModifier def add_authentication_token_field(token_field, options = {}) - @token_fields = [] unless @token_fields - unique = options.fetch(:unique, true) - - if @token_fields.include?(token_field) + if token_authenticatable_fields.include?(token_field) raise ArgumentError.new("#{token_field} already configured via add_authentication_token_field") end - @token_fields << token_field + token_authenticatable_fields.push(token_field) attr_accessor :cleartext_tokens - strategy = if options[:digest] - TokenAuthenticatableStrategies::Digest.new(self, token_field, options) - else - TokenAuthenticatableStrategies::Insecure.new(self, token_field, options) - end + strategy = TokenAuthenticatableStrategies::Base + .fabricate(self, token_field, options) - if unique + if options.fetch(:unique, true) define_singleton_method("find_by_#{token_field}") do |token| strategy.find_token_authenticatable(token) end @@ -59,5 +53,9 @@ module TokenAuthenticatable token.present? && ActiveSupport::SecurityUtils.variable_size_secure_compare(other_token, token) end end + + def token_authenticatable_fields + @token_authenticatable_fields ||= [] + end end end diff --git a/app/models/concerns/token_authenticatable_strategies/base.rb b/app/models/concerns/token_authenticatable_strategies/base.rb index 413721d3e6c..01fb194281a 100644 --- a/app/models/concerns/token_authenticatable_strategies/base.rb +++ b/app/models/concerns/token_authenticatable_strategies/base.rb @@ -2,6 +2,8 @@ module TokenAuthenticatableStrategies class Base + attr_reader :klass, :token_field, :options + def initialize(klass, token_field, options) @klass = klass @token_field = token_field @@ -22,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 @@ -36,6 +39,36 @@ module TokenAuthenticatableStrategies instance.save! if Gitlab::Database.read_write? end + def fallback? + unless options[:fallback].in?([true, false, nil]) + raise ArgumentError, 'fallback: needs to be a boolean value!' + end + + options[:fallback] == true + end + + def migrating? + unless options[:migrating].in?([true, false, nil]) + raise ArgumentError, 'migrating: needs to be a boolean value!' + end + + options[:migrating] == true + end + + def self.fabricate(model, field, options) + if options[:digest] && options[:encrypted] + raise ArgumentError, 'Incompatible options set!' + end + + if options[:digest] + TokenAuthenticatableStrategies::Digest.new(model, field, options) + elsif options[:encrypted] + TokenAuthenticatableStrategies::Encrypted.new(model, field, options) + else + TokenAuthenticatableStrategies::Insecure.new(model, field, options) + end + end + protected def write_new_token(instance) @@ -65,9 +98,5 @@ module TokenAuthenticatableStrategies def token_set?(instance) raise NotImplementedError end - - def token_field_name - @token_field - end end end diff --git a/app/models/concerns/token_authenticatable_strategies/encrypted.rb b/app/models/concerns/token_authenticatable_strategies/encrypted.rb new file mode 100644 index 00000000000..152491aa6e9 --- /dev/null +++ b/app/models/concerns/token_authenticatable_strategies/encrypted.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +module TokenAuthenticatableStrategies + class Encrypted < Base + def initialize(*) + super + + if migrating? && fallback? + raise ArgumentError, '`fallback` and `migrating` options are not compatible!' + end + end + + def find_token_authenticatable(token, unscoped = false) + return if token.blank? + + if fully_encrypted? + return find_by_encrypted_token(token, unscoped) + end + + if fallback? + find_by_encrypted_token(token, unscoped) || + find_by_plaintext_token(token, unscoped) + elsif migrating? + find_by_plaintext_token(token, unscoped) + else + raise ArgumentError, 'Unknown encryption phase!' + end + end + + def ensure_token(instance) + # TODO, tech debt, because some specs are testing migrations, but are still + # using factory bot to create resources, it might happen that a database + # schema does not have "#{token_name}_encrypted" field yet, however a bunch + # of models call `ensure_#{token_name}` in `before_save`. + # + # In that case we are using insecure strategy, but this should only happen + # in tests, because otherwise `encrypted_field` is going to exist. + # + # Another use case is when we are caching resources / columns, like we do + # in case of ApplicationSetting. + + return super if instance.has_attribute?(encrypted_field) + + if fully_encrypted? + raise ArgumentError, 'Using encrypted strategy when encrypted field is missing!' + else + insecure_strategy.ensure_token(instance) + end + end + + def get_token(instance) + return insecure_strategy.get_token(instance) if migrating? + + encrypted_token = instance.read_attribute(encrypted_field) + token = Gitlab::CryptoHelper.aes256_gcm_decrypt(encrypted_token) + + token || (insecure_strategy.get_token(instance) if fallback?) + end + + def set_token(instance, token) + raise ArgumentError unless token.present? + + instance[encrypted_field] = Gitlab::CryptoHelper.aes256_gcm_encrypt(token) + instance[token_field] = token if migrating? + instance[token_field] = nil if fallback? + token + end + + def fully_encrypted? + !migrating? && !fallback? + end + + protected + + def find_by_plaintext_token(token, unscoped) + insecure_strategy.find_token_authenticatable(token, unscoped) + end + + def find_by_encrypted_token(token, unscoped) + encrypted_value = Gitlab::CryptoHelper.aes256_gcm_encrypt(token) + relation(unscoped).find_by(encrypted_field => encrypted_value) + end + + def insecure_strategy + @insecure_strategy ||= TokenAuthenticatableStrategies::Insecure + .new(klass, token_field, options) + end + + def token_set?(instance) + raw_token = instance.read_attribute(encrypted_field) + + unless fully_encrypted? + raw_token ||= insecure_strategy.get_token(instance) + end + + raw_token.present? + end + + def encrypted_field + @encrypted_field ||= "#{@token_field}_encrypted" + end + end +end diff --git a/app/models/group.rb b/app/models/group.rb index adb9169cfcd..02ddc8762af 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -55,7 +55,7 @@ class Group < Namespace validates :two_factor_grace_period, presence: true, numericality: { greater_than_or_equal_to: 0 } - add_authentication_token_field :runners_token + add_authentication_token_field :runners_token, encrypted: true, migrating: true after_create :post_create_hook after_destroy :post_destroy_hook diff --git a/app/models/project.rb b/app/models/project.rb index ade20cc8948..1c5c34111b0 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -85,7 +85,7 @@ class Project < ActiveRecord::Base default_value_for :snippets_enabled, gitlab_config_features.snippets default_value_for :only_allow_merge_if_all_discussions_are_resolved, false - add_authentication_token_field :runners_token + add_authentication_token_field :runners_token, encrypted: true, migrating: true before_validation :mark_remote_mirrors_for_removal, if: -> { RemoteMirror.table_exists? } diff --git a/changelogs/unreleased/fix-gb-encrypt-runners-tokens.yml b/changelogs/unreleased/fix-gb-encrypt-runners-tokens.yml new file mode 100644 index 00000000000..4ce4f96c1dd --- /dev/null +++ b/changelogs/unreleased/fix-gb-encrypt-runners-tokens.yml @@ -0,0 +1,5 @@ +--- +title: Encrypt runners tokens +merge_request: 23412 +author: +type: security diff --git a/config/settings.rb b/config/settings.rb index 3f3481bb65d..1b94df785a7 100644 --- a/config/settings.rb +++ b/config/settings.rb @@ -95,6 +95,14 @@ class Settings < Settingslogic Gitlab::Application.secrets.db_key_base[0..31] end + def attr_encrypted_db_key_base_32 + Gitlab::Utils.ensure_utf8_size(attr_encrypted_db_key_base, bytes: 32.bytes) + end + + def attr_encrypted_db_key_base_12 + Gitlab::Utils.ensure_utf8_size(attr_encrypted_db_key_base, bytes: 12.bytes) + end + # This should be used for :per_attribute_salt_and_iv mode. There is no # need to truncate the key because the encryptor will use the salt to # generate a hash of the password: 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/migrate/20181116141415_add_encrypted_runners_token_to_namespaces.rb b/db/migrate/20181116141415_add_encrypted_runners_token_to_namespaces.rb new file mode 100644 index 00000000000..b92b1b50218 --- /dev/null +++ b/db/migrate/20181116141415_add_encrypted_runners_token_to_namespaces.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddEncryptedRunnersTokenToNamespaces < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :namespaces, :runners_token_encrypted, :string + end +end diff --git a/db/migrate/20181116141504_add_encrypted_runners_token_to_projects.rb b/db/migrate/20181116141504_add_encrypted_runners_token_to_projects.rb new file mode 100644 index 00000000000..53e475bd180 --- /dev/null +++ b/db/migrate/20181116141504_add_encrypted_runners_token_to_projects.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddEncryptedRunnersTokenToProjects < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :projects, :runners_token_encrypted, :string + end +end diff --git a/db/migrate/20181120151656_add_token_encrypted_to_ci_runners.rb b/db/migrate/20181120151656_add_token_encrypted_to_ci_runners.rb new file mode 100644 index 00000000000..40db6b399ab --- /dev/null +++ b/db/migrate/20181120151656_add_token_encrypted_to_ci_runners.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddTokenEncryptedToCiRunners < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :ci_runners, :token_encrypted, :string + end +end diff --git a/db/post_migrate/20181121111200_schedule_runners_token_encryption.rb b/db/post_migrate/20181121111200_schedule_runners_token_encryption.rb new file mode 100644 index 00000000000..753e052f7a7 --- /dev/null +++ b/db/post_migrate/20181121111200_schedule_runners_token_encryption.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +class ScheduleRunnersTokenEncryption < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + BATCH_SIZE = 10000 + RANGE_SIZE = 2000 + MIGRATION = 'EncryptRunnersTokens' + + MODELS = [ + ::Gitlab::BackgroundMigration::Models::EncryptColumns::Settings, + ::Gitlab::BackgroundMigration::Models::EncryptColumns::Namespace, + ::Gitlab::BackgroundMigration::Models::EncryptColumns::Project, + ::Gitlab::BackgroundMigration::Models::EncryptColumns::Runner + ].freeze + + disable_ddl_transaction! + + def up + MODELS.each do |model| + model.each_batch(of: BATCH_SIZE) do |relation, index| + delay = index * 4.minutes + + relation.each_batch(of: RANGE_SIZE) do |relation| + range = relation.pluck('MIN(id)', 'MAX(id)').first + args = [model.name.demodulize.downcase, *range] + + BackgroundMigrationWorker.perform_in(delay, MIGRATION, args) + end + end + end + end + + def down + # no-op + end +end diff --git a/db/schema.rb b/db/schema.rb index 995619bdc69..d048be08d77 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -166,6 +166,7 @@ ActiveRecord::Schema.define(version: 20181126153547) 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" t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id", using: :btree end @@ -520,6 +521,7 @@ ActiveRecord::Schema.define(version: 20181126153547) do t.string "ip_address" t.integer "maximum_timeout" t.integer "runner_type", limit: 2, null: false + t.string "token_encrypted" t.index ["contacted_at"], name: "index_ci_runners_on_contacted_at", using: :btree t.index ["is_shared"], name: "index_ci_runners_on_is_shared", using: :btree t.index ["locked"], name: "index_ci_runners_on_locked", using: :btree @@ -1335,6 +1337,7 @@ ActiveRecord::Schema.define(version: 20181126153547) do t.integer "two_factor_grace_period", default: 48, null: false t.integer "cached_markdown_version" t.string "runners_token" + t.string "runners_token_encrypted" t.index ["created_at"], name: "index_namespaces_on_created_at", using: :btree t.index ["name", "parent_id"], name: "index_namespaces_on_name_and_parent_id", unique: true, using: :btree t.index ["name"], name: "index_namespaces_on_name_trigram", using: :gin, opclasses: {"name"=>"gin_trgm_ops"} @@ -1675,6 +1678,7 @@ ActiveRecord::Schema.define(version: 20181126153547) do t.boolean "pages_https_only", default: true t.boolean "remote_mirror_available_overridden" t.bigint "pool_repository_id" + t.string "runners_token_encrypted" t.index ["ci_id"], name: "index_projects_on_ci_id", using: :btree t.index ["created_at"], name: "index_projects_on_created_at", using: :btree t.index ["creator_id"], name: "index_projects_on_creator_id", using: :btree diff --git a/lib/api/runner.rb b/lib/api/runner.rb index 2f15f3a7d76..c60d25b88cb 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -19,7 +19,6 @@ module API optional :tag_list, type: Array[String], desc: %q(List of Runner's tags) optional :maximum_timeout, type: Integer, desc: 'Maximum timeout set when this Runner will handle the job' end - # rubocop: disable CodeReuse/ActiveRecord post '/' do attributes = attributes_for_keys([:description, :active, :locked, :run_untagged, :tag_list, :maximum_timeout]) .merge(get_runner_details_from_request) @@ -28,10 +27,10 @@ module API if runner_registration_token_valid? # Create shared runner. Requires admin access attributes.merge(runner_type: :instance_type) - elsif project = Project.find_by(runners_token: params[:token]) + elsif project = Project.find_by_runners_token(params[:token]) # Create a specific runner for the project attributes.merge(runner_type: :project_type, projects: [project]) - elsif group = Group.find_by(runners_token: params[:token]) + elsif group = Group.find_by_runners_token(params[:token]) # Create a specific runner for the group attributes.merge(runner_type: :group_type, groups: [group]) else @@ -46,7 +45,6 @@ module API render_validation_error!(runner) end end - # rubocop: enable CodeReuse/ActiveRecord desc 'Deletes a registered Runner' do http_codes [[204, 'Runner was deleted'], [403, 'Forbidden']] diff --git a/lib/gitlab/background_migration/encrypt_columns.rb b/lib/gitlab/background_migration/encrypt_columns.rb index bd5f12276ab..b9ad8267e37 100644 --- a/lib/gitlab/background_migration/encrypt_columns.rb +++ b/lib/gitlab/background_migration/encrypt_columns.rb @@ -5,15 +5,17 @@ module Gitlab # EncryptColumn migrates data from an unencrypted column - `foo`, say - to # an encrypted column - `encrypted_foo`, say. # + # To avoid depending on a particular version of the model in app/, add a + # model to `lib/gitlab/background_migration/models/encrypt_columns` and use + # it in the migration that enqueues the jobs, so code can be shared. + # # For this background migration to work, the table that is migrated _has_ to # have an `id` column as the primary key. Additionally, the encrypted column # should be managed by attr_encrypted, and map to an attribute with the same # name as the unencrypted column (i.e., the unencrypted column should be - # shadowed). + # shadowed), unless you want to define specific methods / accessors in the + # temporary model in `/models/encrypt_columns/your_model.rb`. # - # To avoid depending on a particular version of the model in app/, add a - # model to `lib/gitlab/background_migration/models/encrypt_columns` and use - # it in the migration that enqueues the jobs, so code can be shared. class EncryptColumns def perform(model, attributes, from, to) model = model.constantize if model.is_a?(String) @@ -36,6 +38,10 @@ module Gitlab end end + def clear_migrated_values? + true + end + private # Build a hash of { attribute => encrypted column name } @@ -72,7 +78,10 @@ module Gitlab if instance.changed? instance.save! - instance.update_columns(to_clear) + + if clear_migrated_values? + instance.update_columns(to_clear) + end end end diff --git a/lib/gitlab/background_migration/encrypt_runners_tokens.rb b/lib/gitlab/background_migration/encrypt_runners_tokens.rb new file mode 100644 index 00000000000..91e559a8765 --- /dev/null +++ b/lib/gitlab/background_migration/encrypt_runners_tokens.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # EncryptColumn migrates data from an unencrypted column - `foo`, say - to + # an encrypted column - `encrypted_foo`, say. + # + # We only create a subclass here because we want to isolate this migration + # (migrating unencrypted runner registration tokens to encrypted columns) + # from other `EncryptColumns` migration. This class name is going to be + # serialized and stored in Redis and later picked by Sidekiq, so we need to + # create a separate class name in order to isolate these migration tasks. + # + # We can solve this differently, see tech debt issue: + # + # https://gitlab.com/gitlab-org/gitlab-ce/issues/54328 + # + class EncryptRunnersTokens < EncryptColumns + def perform(model, from, to) + resource = "::Gitlab::BackgroundMigration::Models::EncryptColumns::#{model.to_s.capitalize}" + model = resource.constantize + attributes = model.encrypted_attributes.keys + + super(model, attributes, from, to) + end + + def clear_migrated_values? + false + end + end + end +end diff --git a/lib/gitlab/background_migration/models/encrypt_columns/namespace.rb b/lib/gitlab/background_migration/models/encrypt_columns/namespace.rb new file mode 100644 index 00000000000..41f18979d76 --- /dev/null +++ b/lib/gitlab/background_migration/models/encrypt_columns/namespace.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + module Models + module EncryptColumns + # This model is shared between synchronous and background migrations to + # encrypt the `runners_token` column in `namespaces` table. + # + class Namespace < ActiveRecord::Base + include ::EachBatch + + self.table_name = 'namespaces' + self.inheritance_column = :_type_disabled + + def runners_token=(value) + self.runners_token_encrypted = + ::Gitlab::CryptoHelper.aes256_gcm_encrypt(value) + end + + def self.encrypted_attributes + { runners_token: { attribute: :runners_token_encrypted } } + end + end + end + end + end +end diff --git a/lib/gitlab/background_migration/models/encrypt_columns/project.rb b/lib/gitlab/background_migration/models/encrypt_columns/project.rb new file mode 100644 index 00000000000..bfeae14584d --- /dev/null +++ b/lib/gitlab/background_migration/models/encrypt_columns/project.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + module Models + module EncryptColumns + # This model is shared between synchronous and background migrations to + # encrypt the `runners_token` column in `projects` table. + # + class Project < ActiveRecord::Base + include ::EachBatch + + self.table_name = 'projects' + self.inheritance_column = :_type_disabled + + def runners_token=(value) + self.runners_token_encrypted = + ::Gitlab::CryptoHelper.aes256_gcm_encrypt(value) + end + + def self.encrypted_attributes + { runners_token: { attribute: :runners_token_encrypted } } + end + end + end + end + end +end diff --git a/lib/gitlab/background_migration/models/encrypt_columns/runner.rb b/lib/gitlab/background_migration/models/encrypt_columns/runner.rb new file mode 100644 index 00000000000..14ddce4b147 --- /dev/null +++ b/lib/gitlab/background_migration/models/encrypt_columns/runner.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + module Models + module EncryptColumns + # This model is shared between synchronous and background migrations to + # encrypt the `token` column in `ci_runners` table. + # + class Runner < ActiveRecord::Base + include ::EachBatch + + self.table_name = 'ci_runners' + self.inheritance_column = :_type_disabled + + def token=(value) + self.token_encrypted = + ::Gitlab::CryptoHelper.aes256_gcm_encrypt(value) + end + + def self.encrypted_attributes + { token: { attribute: :token_encrypted } } + end + end + end + end + end +end diff --git a/lib/gitlab/background_migration/models/encrypt_columns/settings.rb b/lib/gitlab/background_migration/models/encrypt_columns/settings.rb new file mode 100644 index 00000000000..08ae35c0671 --- /dev/null +++ b/lib/gitlab/background_migration/models/encrypt_columns/settings.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + module Models + module EncryptColumns + # This model is shared between synchronous and background migrations to + # encrypt the `runners_token` column in `application_settings` table. + # + class Settings < ActiveRecord::Base + include ::EachBatch + include ::CacheableAttributes + + self.table_name = 'application_settings' + self.inheritance_column = :_type_disabled + + after_commit do + ::ApplicationSetting.expire + end + + def runners_registration_token=(value) + self.runners_registration_token_encrypted = + ::Gitlab::CryptoHelper.aes256_gcm_encrypt(value) + end + + def self.encrypted_attributes + { + runners_registration_token: { + attribute: :runners_registration_token_encrypted + } + } + end + end + end + end + end +end diff --git a/lib/gitlab/background_migration/models/encrypt_columns/web_hook.rb b/lib/gitlab/background_migration/models/encrypt_columns/web_hook.rb index bb76eb8ed48..ccd9d4c6d44 100644 --- a/lib/gitlab/background_migration/models/encrypt_columns/web_hook.rb +++ b/lib/gitlab/background_migration/models/encrypt_columns/web_hook.rb @@ -15,12 +15,12 @@ module Gitlab attr_encrypted :token, mode: :per_attribute_iv, algorithm: 'aes-256-gcm', - key: Settings.attr_encrypted_db_key_base_truncated + key: ::Settings.attr_encrypted_db_key_base_truncated attr_encrypted :url, mode: :per_attribute_iv, algorithm: 'aes-256-gcm', - key: Settings.attr_encrypted_db_key_base_truncated + key: ::Settings.attr_encrypted_db_key_base_truncated end end end diff --git a/lib/gitlab/crypto_helper.rb b/lib/gitlab/crypto_helper.rb index 68d0b5d8f8a..87a03d9c58f 100644 --- a/lib/gitlab/crypto_helper.rb +++ b/lib/gitlab/crypto_helper.rb @@ -6,8 +6,8 @@ module Gitlab AES256_GCM_OPTIONS = { algorithm: 'aes-256-gcm', - key: Settings.attr_encrypted_db_key_base_truncated, - iv: Settings.attr_encrypted_db_key_base_truncated[0..11] + key: Settings.attr_encrypted_db_key_base_32, + iv: Settings.attr_encrypted_db_key_base_12 }.freeze def sha256(value) @@ -17,7 +17,7 @@ module Gitlab def aes256_gcm_encrypt(value) encrypted_token = Encryptor.encrypt(AES256_GCM_OPTIONS.merge(value: value)) - Base64.encode64(encrypted_token) + Base64.strict_encode64(encrypted_token) end def aes256_gcm_decrypt(value) diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index 8380e86c128..4f3298812a8 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -100,6 +100,7 @@ excluded_attributes: - :import_source - :mirror - :runners_token + - :runners_token_encrypted - :repository_storage - :repository_read_only - :lfs_enabled @@ -114,6 +115,9 @@ excluded_attributes: - :remote_mirror_available_overridden - :description_html - :repository_languages + namespaces: + - :runners_token + - :runners_token_encrypted project_import_state: - :last_error - :jid @@ -155,6 +159,9 @@ excluded_attributes: - :encrypted_token_iv - :encrypted_url - :encrypted_url_iv + runners: + - :token + - :token_encrypted services: - :template diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index 097c7653754..9cc781ddf8d 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -10,6 +10,7 @@ module Gitlab triggers: 'Ci::Trigger', pipeline_schedules: 'Ci::PipelineSchedule', builds: 'Ci::Build', + runners: 'Ci::Runner', hooks: 'ProjectHook', merge_access_levels: 'ProtectedBranch::MergeAccessLevel', push_access_levels: 'ProtectedBranch::PushAccessLevel', @@ -33,7 +34,7 @@ module Gitlab EXISTING_OBJECT_CHECK = %i[milestone milestones label labels project_label project_labels group_label group_labels project_feature].freeze - TOKEN_RESET_MODELS = %w[Ci::Trigger Ci::Build ProjectHook].freeze + TOKEN_RESET_MODELS = %w[Project Namespace Ci::Trigger Ci::Build Ci::Runner ProjectHook].freeze def self.create(*args) new(*args).create diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb index 9e59137a2c0..e0e8f598ba4 100644 --- a/lib/gitlab/utils.rb +++ b/lib/gitlab/utils.rb @@ -16,6 +16,21 @@ module Gitlab str.force_encoding(Encoding::UTF_8) end + def ensure_utf8_size(str, bytes:) + raise ArgumentError, 'Empty string provided!' if str.empty? + raise ArgumentError, 'Negative string size provided!' if bytes.negative? + + truncated = str.each_char.each_with_object(+'') do |char, object| + if object.bytesize + char.bytesize > bytes + break object + else + object.concat(char) + end + end + + truncated + ('0' * (bytes - truncated.bytesize)) + end + # Append path to host, making sure there's one single / in between def append_path(host, path) "#{host.to_s.sub(%r{\/+$}, '')}/#{path.to_s.sub(%r{^\/+}, '')}" diff --git a/spec/config/settings_spec.rb b/spec/config/settings_spec.rb index 83b2de47741..c89b5f48dc0 100644 --- a/spec/config/settings_spec.rb +++ b/spec/config/settings_spec.rb @@ -6,4 +6,102 @@ describe Settings do expect(described_class.omniauth.enabled).to be true end end + + describe '.attr_encrypted_db_key_base_truncated' do + it 'is a string with maximum 32 bytes size' do + expect(described_class.attr_encrypted_db_key_base_truncated.bytesize) + .to be <= 32 + end + end + + describe '.attr_encrypted_db_key_base_12' do + context 'when db key base secret is less than 12 bytes' do + before do + allow(described_class) + .to receive(:attr_encrypted_db_key_base) + .and_return('a' * 10) + end + + it 'expands db key base secret to 12 bytes' do + expect(described_class.attr_encrypted_db_key_base_12) + .to eq(('a' * 10) + ('0' * 2)) + end + end + + context 'when key has multiple multi-byte UTF chars exceeding 12 bytes' do + before do + allow(described_class) + .to receive(:attr_encrypted_db_key_base) + .and_return('❤' * 18) + end + + it 'does not use more than 32 bytes' do + db_key_base = described_class.attr_encrypted_db_key_base_12 + + expect(db_key_base).to eq('❤' * 4) + expect(db_key_base.bytesize).to eq 12 + end + end + end + + describe '.attr_encrypted_db_key_base_32' do + context 'when db key base secret is less than 32 bytes' do + before do + allow(described_class) + .to receive(:attr_encrypted_db_key_base) + .and_return('a' * 10) + end + + it 'expands db key base secret to 32 bytes' do + expanded_key_base = ('a' * 10) + ('0' * 22) + + expect(expanded_key_base.bytesize).to eq 32 + expect(described_class.attr_encrypted_db_key_base_32) + .to eq expanded_key_base + end + end + + context 'when db key base secret is 32 bytes' do + before do + allow(described_class) + .to receive(:attr_encrypted_db_key_base) + .and_return('a' * 32) + end + + it 'returns original value' do + expect(described_class.attr_encrypted_db_key_base_32) + .to eq 'a' * 32 + end + end + + context 'when db key base contains multi-byte UTF character' do + before do + allow(described_class) + .to receive(:attr_encrypted_db_key_base) + .and_return('❤' * 6) + end + + it 'does not use more than 32 bytes' do + db_key_base = described_class.attr_encrypted_db_key_base_32 + + expect(db_key_base).to eq '❤❤❤❤❤❤' + ('0' * 14) + expect(db_key_base.bytesize).to eq 32 + end + end + + context 'when db key base multi-byte UTF chars exceeding 32 bytes' do + before do + allow(described_class) + .to receive(:attr_encrypted_db_key_base) + .and_return('❤' * 18) + end + + it 'does not use more than 32 bytes' do + db_key_base = described_class.attr_encrypted_db_key_base_32 + + expect(db_key_base).to eq(('❤' * 10) + ('0' * 2)) + expect(db_key_base.bytesize).to eq 32 + end + end + end end diff --git a/spec/lib/gitlab/background_migration/encrypt_runners_tokens_spec.rb b/spec/lib/gitlab/background_migration/encrypt_runners_tokens_spec.rb new file mode 100644 index 00000000000..9d4921968b3 --- /dev/null +++ b/spec/lib/gitlab/background_migration/encrypt_runners_tokens_spec.rb @@ -0,0 +1,77 @@ +require 'spec_helper' + +describe Gitlab::BackgroundMigration::EncryptRunnersTokens, :migration, schema: 20181121111200 do + let(:settings) { table(:application_settings) } + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:runners) { table(:ci_runners) } + + context 'when migrating application settings' do + before do + settings.create!(id: 1, runners_registration_token: 'plain-text-token1') + end + + it 'migrates runners registration tokens' do + migrate!(:settings, 1, 1) + + encrypted_token = settings.first.runners_registration_token_encrypted + decrypted_token = ::Gitlab::CryptoHelper.aes256_gcm_decrypt(encrypted_token) + + expect(decrypted_token).to eq 'plain-text-token1' + expect(settings.first.runners_registration_token).to eq 'plain-text-token1' + end + end + + context 'when migrating namespaces' do + before do + namespaces.create!(id: 11, name: 'gitlab', path: 'gitlab-org', runners_token: 'my-token1') + namespaces.create!(id: 12, name: 'gitlab', path: 'gitlab-org', runners_token: 'my-token2') + namespaces.create!(id: 22, name: 'gitlab', path: 'gitlab-org', runners_token: 'my-token3') + end + + it 'migrates runners registration tokens' do + migrate!(:namespace, 11, 22) + + expect(namespaces.all.reload).to all( + have_attributes(runners_token: be_a(String), runners_token_encrypted: be_a(String)) + ) + end + end + + context 'when migrating projects' do + before do + namespaces.create!(id: 11, name: 'gitlab', path: 'gitlab-org') + projects.create!(id: 111, namespace_id: 11, name: 'gitlab', path: 'gitlab-ce', runners_token: 'my-token1') + projects.create!(id: 114, namespace_id: 11, name: 'gitlab', path: 'gitlab-ce', runners_token: 'my-token2') + projects.create!(id: 116, namespace_id: 11, name: 'gitlab', path: 'gitlab-ce', runners_token: 'my-token3') + end + + it 'migrates runners registration tokens' do + migrate!(:project, 111, 116) + + expect(projects.all.reload).to all( + have_attributes(runners_token: be_a(String), runners_token_encrypted: be_a(String)) + ) + end + end + + context 'when migrating runners' do + before do + runners.create!(id: 201, runner_type: 1, token: 'plain-text-token1') + runners.create!(id: 202, runner_type: 1, token: 'plain-text-token2') + runners.create!(id: 203, runner_type: 1, token: 'plain-text-token3') + end + + it 'migrates runners communication tokens' do + migrate!(:runner, 201, 203) + + expect(runners.all.reload).to all( + have_attributes(token: be_a(String), token_encrypted: be_a(String)) + ) + end + end + + def migrate!(model, from, to) + subject.perform(model, from, to) + end +end diff --git a/spec/lib/gitlab/crypto_helper_spec.rb b/spec/lib/gitlab/crypto_helper_spec.rb new file mode 100644 index 00000000000..05cc6cf15de --- /dev/null +++ b/spec/lib/gitlab/crypto_helper_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe Gitlab::CryptoHelper do + describe '.sha256' do + it 'generates SHA256 digest Base46 encoded' do + digest = described_class.sha256('some-value') + + expect(digest).to match %r{\A[A-Za-z0-9+/=]+\z} + expect(digest).to eq digest.strip + end + end + + describe '.aes256_gcm_encrypt' do + it 'is Base64 encoded string without new line character' do + encrypted = described_class.aes256_gcm_encrypt('some-value') + + expect(encrypted).to match %r{\A[A-Za-z0-9+/=]+\z} + expect(encrypted).not_to include "\n" + end + end + + describe '.aes256_gcm_decrypt' do + let(:encrypted) { described_class.aes256_gcm_encrypt('some-value') } + + it 'correctly decrypts encrypted string' do + decrypted = described_class.aes256_gcm_decrypt(encrypted) + + expect(decrypted).to eq 'some-value' + end + + it 'decrypts a value when it ends with a new line character' do + decrypted = described_class.aes256_gcm_decrypt(encrypted + "\n") + + expect(decrypted).to eq 'some-value' + end + end +end diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb index ad2c9d7f2af..3579ed9a759 100644 --- a/spec/lib/gitlab/utils_spec.rb +++ b/spec/lib/gitlab/utils_spec.rb @@ -127,4 +127,42 @@ describe Gitlab::Utils do end end end + + describe '.ensure_utf8_size' do + context 'string is has less bytes than expected' do + it 'backfills string with null characters' do + transformed = described_class.ensure_utf8_size('a' * 10, bytes: 32) + + expect(transformed.bytesize).to eq 32 + expect(transformed).to eq(('a' * 10) + ('0' * 22)) + end + end + + context 'string size is exactly the one that is expected' do + it 'returns original value' do + transformed = described_class.ensure_utf8_size('a' * 32, bytes: 32) + + expect(transformed).to eq 'a' * 32 + expect(transformed.bytesize).to eq 32 + end + end + + context 'when string contains a few multi-byte UTF characters' do + it 'backfills string with null characters' do + transformed = described_class.ensure_utf8_size('❤' * 6, bytes: 32) + + expect(transformed).to eq '❤❤❤❤❤❤' + ('0' * 14) + expect(transformed.bytesize).to eq 32 + end + end + + context 'when string has multiple multi-byte UTF chars exceeding 32 bytes' do + it 'truncates string to 32 characters and backfills it if needed' do + transformed = described_class.ensure_utf8_size('❤' * 18, bytes: 32) + + expect(transformed).to eq(('❤' * 10) + ('0' * 2)) + expect(transformed.bytesize).to eq 32 + end + end + end end diff --git a/spec/migrations/schedule_runners_token_encryption_spec.rb b/spec/migrations/schedule_runners_token_encryption_spec.rb new file mode 100644 index 00000000000..376d2795277 --- /dev/null +++ b/spec/migrations/schedule_runners_token_encryption_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20181121111200_schedule_runners_token_encryption') + +describe ScheduleRunnersTokenEncryption, :migration do + let(:settings) { table(:application_settings) } + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:runners) { table(:ci_runners) } + + before do + stub_const("#{described_class.name}::BATCH_SIZE", 1) + + settings.create!(id: 1, runners_registration_token: 'plain-text-token1') + namespaces.create!(id: 11, name: 'gitlab', path: 'gitlab-org', runners_token: 'my-token1') + namespaces.create!(id: 12, name: 'gitlab', path: 'gitlab-org', runners_token: 'my-token2') + projects.create!(id: 111, namespace_id: 11, name: 'gitlab', path: 'gitlab-ce', runners_token: 'my-token1') + projects.create!(id: 114, namespace_id: 11, name: 'gitlab', path: 'gitlab-ce', runners_token: 'my-token2') + runners.create!(id: 201, runner_type: 1, token: 'plain-text-token1') + runners.create!(id: 202, runner_type: 1, token: 'plain-text-token2') + end + + it 'schedules runners token encryption migration for multiple resources' do + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(4.minutes, 'settings', 1, 1) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(4.minutes, 'namespace', 11, 11) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(8.minutes, 'namespace', 12, 12) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(4.minutes, 'project', 111, 111) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(8.minutes, 'project', 114, 114) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(4.minutes, 'runner', 201, 201) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(8.minutes, 'runner', 202, 202) + expect(BackgroundMigrationWorker.jobs.size).to eq 7 + end + end + end +end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index d02c3a5765f..4cdcae5f670 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -769,33 +769,15 @@ describe Ci::Build do let(:subject) { build.hide_secrets(data) } context 'hide runners token' do - let(:data) { 'new token data'} + let(:data) { "new #{project.runners_token} data"} - before do - build.project.update(runners_token: 'token') - end - - it { is_expected.to eq('new xxxxx data') } + it { is_expected.to match(/^new x+ data$/) } end context 'hide build token' do - let(:data) { 'new token data'} - - before do - build.update(token: 'token') - end - - it { is_expected.to eq('new xxxxx data') } - end - - context 'hide build token' do - let(:data) { 'new token data'} - - before do - build.update(token: 'token') - end + let(:data) { "new #{build.token} data"} - it { is_expected.to eq('new xxxxx data') } + it { is_expected.to match(/^new x+ data$/) } end end 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/base_spec.rb b/spec/models/concerns/token_authenticatable_strategies/base_spec.rb new file mode 100644 index 00000000000..6605f1f5a5f --- /dev/null +++ b/spec/models/concerns/token_authenticatable_strategies/base_spec.rb @@ -0,0 +1,65 @@ +require 'spec_helper' + +describe TokenAuthenticatableStrategies::Base do + let(:instance) { double(:instance) } + let(:field) { double(:field) } + + describe '.fabricate' do + context 'when digest stragegy is specified' do + it 'fabricates digest strategy object' do + strategy = described_class.fabricate(instance, field, digest: true) + + expect(strategy).to be_a TokenAuthenticatableStrategies::Digest + end + end + + context 'when encrypted strategy is specified' do + it 'fabricates encrypted strategy object' do + strategy = described_class.fabricate(instance, field, encrypted: true) + + expect(strategy).to be_a TokenAuthenticatableStrategies::Encrypted + end + end + + context 'when no strategy is specified' do + it 'fabricates insecure strategy object' do + strategy = described_class.fabricate(instance, field, something: true) + + expect(strategy).to be_a TokenAuthenticatableStrategies::Insecure + end + end + + context 'when incompatible options are provided' do + it 'raises an error' do + expect { described_class.fabricate(instance, field, digest: true, encrypted: true) } + .to raise_error ArgumentError + end + end + end + + describe '#fallback?' do + context 'when fallback is set' do + it 'recognizes fallback setting' do + strategy = described_class.new(instance, field, fallback: true) + + expect(strategy.fallback?).to be true + end + end + + context 'when fallback is not a valid value' do + it 'raises an error' do + strategy = described_class.new(instance, field, fallback: 'something') + + expect { strategy.fallback? }.to raise_error ArgumentError + end + end + + context 'when fallback is not set' do + it 'raises an error' do + strategy = described_class.new(instance, field, {}) + + expect(strategy.fallback?).to eq false + end + end + end +end diff --git a/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb new file mode 100644 index 00000000000..93cab80cb1f --- /dev/null +++ b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb @@ -0,0 +1,156 @@ +require 'spec_helper' + +describe TokenAuthenticatableStrategies::Encrypted do + let(:model) { double(:model) } + let(:instance) { double(:instance) } + + let(:encrypted) do + Gitlab::CryptoHelper.aes256_gcm_encrypt('my-value') + end + + subject do + described_class.new(model, 'some_field', options) + end + + describe '.new' do + context 'when fallback and migration strategies are set' do + let(:options) { { fallback: true, migrating: true } } + + it 'raises an error' do + expect { subject }.to raise_error ArgumentError, /not compatible/ + end + end + end + + describe '#find_token_authenticatable' do + context 'when using fallback strategy' do + let(:options) { { fallback: true } } + + it 'finds the encrypted resource by cleartext' do + allow(model).to receive(:find_by) + .with('some_field_encrypted' => encrypted) + .and_return('encrypted resource') + + expect(subject.find_token_authenticatable('my-value')) + .to eq 'encrypted resource' + end + + it 'uses insecure strategy when encrypted token cannot be found' do + allow(subject.send(:insecure_strategy)) + .to receive(:find_token_authenticatable) + .and_return('plaintext resource') + + allow(model).to receive(:find_by) + .with('some_field_encrypted' => encrypted) + .and_return(nil) + + expect(subject.find_token_authenticatable('my-value')) + .to eq 'plaintext resource' + end + end + + context 'when using migration strategy' do + let(:options) { { migrating: true } } + + it 'finds the cleartext resource by cleartext' do + allow(model).to receive(:find_by) + .with('some_field' => 'my-value') + .and_return('cleartext resource') + + expect(subject.find_token_authenticatable('my-value')) + .to eq 'cleartext resource' + end + + it 'returns nil if resource cannot be found' do + allow(model).to receive(:find_by) + .with('some_field' => 'my-value') + .and_return(nil) + + expect(subject.find_token_authenticatable('my-value')) + .to be_nil + end + end + end + + describe '#get_token' do + context 'when using fallback strategy' do + let(:options) { { fallback: true } } + + it 'returns decrypted token when an encrypted token is present' do + allow(instance).to receive(:read_attribute) + .with('some_field_encrypted') + .and_return(encrypted) + + expect(subject.get_token(instance)).to eq 'my-value' + end + + it 'returns the plaintext token when encrypted token is not present' do + allow(instance).to receive(:read_attribute) + .with('some_field_encrypted') + .and_return(nil) + + allow(instance).to receive(:read_attribute) + .with('some_field') + .and_return('cleartext value') + + expect(subject.get_token(instance)).to eq 'cleartext value' + end + end + + context 'when using migration strategy' do + let(:options) { { migrating: true } } + + it 'returns cleartext token when an encrypted token is present' do + allow(instance).to receive(:read_attribute) + .with('some_field_encrypted') + .and_return(encrypted) + + allow(instance).to receive(:read_attribute) + .with('some_field') + .and_return('my-cleartext-value') + + expect(subject.get_token(instance)).to eq 'my-cleartext-value' + end + + it 'returns the cleartext token when encrypted token is not present' do + allow(instance).to receive(:read_attribute) + .with('some_field_encrypted') + .and_return(nil) + + allow(instance).to receive(:read_attribute) + .with('some_field') + .and_return('cleartext value') + + expect(subject.get_token(instance)).to eq 'cleartext value' + end + end + end + + describe '#set_token' do + context 'when using fallback strategy' do + let(:options) { { fallback: true } } + + it 'writes encrypted token and removes plaintext token and returns it' do + expect(instance).to receive(:[]=) + .with('some_field_encrypted', encrypted) + expect(instance).to receive(:[]=) + .with('some_field', nil) + + expect(subject.set_token(instance, 'my-value')).to eq 'my-value' + end + end + + context 'when using migration strategy' do + let(:options) { { migrating: true } } + + it 'writes encrypted token and writes plaintext token' do + expect(instance).to receive(:[]=) + .with('some_field_encrypted', encrypted) + expect(instance).to receive(:[]=) + .with('some_field', 'my-value') + + expect(subject.set_token(instance, 'my-value')).to eq 'my-value' + end + end + end +end diff --git a/spec/support/shared_examples/ci_trace_shared_examples.rb b/spec/support/shared_examples/ci_trace_shared_examples.rb index 377bd82b67e..c603421d748 100644 --- a/spec/support/shared_examples/ci_trace_shared_examples.rb +++ b/spec/support/shared_examples/ci_trace_shared_examples.rb @@ -180,10 +180,9 @@ shared_examples_for 'common trace features' do end context 'runners token' do - let(:token) { 'my_secret_token' } + let(:token) { build.project.runners_token } before do - build.project.update(runners_token: token) trace.set(token) end @@ -193,10 +192,9 @@ shared_examples_for 'common trace features' do end context 'hides build token' do - let(:token) { 'my_secret_token' } + let(:token) { build.token } before do - build.update(token: token) trace.set(token) end |