diff options
author | Robert Speicher <rspeicher@gmail.com> | 2018-12-07 03:53:24 +0300 |
---|---|---|
committer | Robert Speicher <rspeicher@gmail.com> | 2018-12-07 03:53:24 +0300 |
commit | 950b9130adcb4d2fa45bd1757c1b3dc50f1df9a4 (patch) | |
tree | a39525f22b03ba9587bcd3efc7d846e0b4406db6 | |
parent | 9a78524f465488b2b5daa3fbdb2ec7c71c9641a6 (diff) |
Revert "Merge branch 'fix/gb/encrypt-ci-build-token' into 'master'"
This reverts commit 1e8f1de034aa9b6a60b640b2b091f60c4d3ba365, reversing
changes made to 62d971129da99936a3cdc04f3740d26f16a0c7a6.
-rw-r--r-- | app/models/ci/build.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/fix-gb-encrypt-ci-build-token.yml | 5 | ||||
-rw-r--r-- | db/migrate/20181129104854_add_token_encrypted_to_ci_builds.rb | 11 | ||||
-rw-r--r-- | db/migrate/20181129104944_add_index_to_ci_builds_token_encrypted.rb | 17 | ||||
-rw-r--r-- | db/schema.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/import_export/import_export.yml | 1 | ||||
-rw-r--r-- | spec/models/ci/build_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/concerns/token_authenticatable_spec.rb | 86 | ||||
-rw-r--r-- | spec/services/ci/retry_build_service_spec.rb | 6 |
9 files changed, 6 insertions, 128 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index d86a6eceb59..d60861dc95f 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -120,7 +120,7 @@ module Ci acts_as_taggable - add_authentication_token_field :token, encrypted: true, fallback: true + add_authentication_token_field :token before_save :update_artifacts_size, if: :artifacts_file_changed? before_save :ensure_token diff --git a/changelogs/unreleased/fix-gb-encrypt-ci-build-token.yml b/changelogs/unreleased/fix-gb-encrypt-ci-build-token.yml deleted file mode 100644 index 04fc88bc3d3..00000000000 --- a/changelogs/unreleased/fix-gb-encrypt-ci-build-token.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Encrypt CI/CD builds authentication tokens -merge_request: 23436 -author: -type: security diff --git a/db/migrate/20181129104854_add_token_encrypted_to_ci_builds.rb b/db/migrate/20181129104854_add_token_encrypted_to_ci_builds.rb deleted file mode 100644 index 11b98203793..00000000000 --- a/db/migrate/20181129104854_add_token_encrypted_to_ci_builds.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -class AddTokenEncryptedToCiBuilds < ActiveRecord::Migration[5.0] - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - - def change - add_column :ci_builds, :token_encrypted, :string - end -end diff --git a/db/migrate/20181129104944_add_index_to_ci_builds_token_encrypted.rb b/db/migrate/20181129104944_add_index_to_ci_builds_token_encrypted.rb deleted file mode 100644 index f90aca008e5..00000000000 --- a/db/migrate/20181129104944_add_index_to_ci_builds_token_encrypted.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -class AddIndexToCiBuildsTokenEncrypted < ActiveRecord::Migration[5.0] - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - - disable_ddl_transaction! - - def up - add_concurrent_index :ci_builds, :token_encrypted, unique: true, where: 'token_encrypted IS NOT NULL' - end - - def down - remove_concurrent_index :ci_builds, :token_encrypted - end -end diff --git a/db/schema.rb b/db/schema.rb index d7124100621..fc73d30fb1f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -345,7 +345,6 @@ ActiveRecord::Schema.define(version: 20181203002526) do t.boolean "protected" t.integer "failure_reason" t.datetime_with_timezone "scheduled_at" - t.string "token_encrypted" t.index ["artifacts_expire_at"], name: "index_ci_builds_on_artifacts_expire_at", where: "(artifacts_file <> ''::text)", using: :btree t.index ["auto_canceled_by_id"], name: "index_ci_builds_on_auto_canceled_by_id", using: :btree t.index ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree @@ -362,7 +361,6 @@ ActiveRecord::Schema.define(version: 20181203002526) do t.index ["stage_id"], name: "index_ci_builds_on_stage_id", using: :btree t.index ["status", "type", "runner_id"], name: "index_ci_builds_on_status_and_type_and_runner_id", using: :btree t.index ["token"], name: "index_ci_builds_on_token", unique: true, using: :btree - t.index ["token_encrypted"], name: "index_ci_builds_on_token_encrypted", unique: true, where: "(token_encrypted IS NOT NULL)", using: :btree t.index ["updated_at"], name: "index_ci_builds_on_updated_at", using: :btree t.index ["user_id"], name: "index_ci_builds_on_user_id", using: :btree end diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index d10d4f2f746..fde8561c16c 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -143,7 +143,6 @@ excluded_attributes: statuses: - :trace - :token - - :token_encrypted - :when - :artifacts_file - :artifacts_metadata diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 89f78f629d4..4cdcae5f670 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1925,7 +1925,7 @@ describe Ci::Build do context 'when token is empty' do before do - build.update_columns(token: nil, token_encrypted: nil) + build.token = nil end it { is_expected.to be_nil} @@ -2141,7 +2141,7 @@ describe Ci::Build do end before do - build.set_token('my-token') + build.token = 'my-token' build.yaml_variables = [] end diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index 55d83bc3a6b..0cdf430e9ab 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -351,89 +351,3 @@ describe PersonalAccessToken, 'TokenAuthenticatable' do end end end - -describe Ci::Build, 'TokenAuthenticatable' do - let(:token_field) { :token } - let(:build) { FactoryBot.build(:ci_build) } - - it_behaves_like 'TokenAuthenticatable' - - describe 'generating new token' do - context 'token is not generated yet' do - describe 'token field accessor' do - it 'makes it possible to access token' do - expect(build.token).to be_nil - - build.save! - - expect(build.token).to be_present - end - end - - describe "ensure_token" do - subject { build.ensure_token } - - it { is_expected.to be_a String } - it { is_expected.not_to be_blank } - - it 'does not persist token' do - expect(build).not_to be_persisted - end - end - - describe 'ensure_token!' do - it 'persists a new token' do - expect(build.ensure_token!).to eq build.reload.token - expect(build).to be_persisted - end - - it 'persists new token as an encrypted string' do - build.ensure_token! - - encrypted = Gitlab::CryptoHelper.aes256_gcm_encrypt(build.token) - - expect(build.read_attribute('token_encrypted')).to eq encrypted - end - - it 'does not persist a token in a clear text' do - build.ensure_token! - - expect(build.read_attribute('token')).to be_nil - end - end - end - - describe '#reset_token!' do - it 'persists a new token' do - build.save! - - build.token.yield_self do |previous_token| - build.reset_token! - - expect(build.token).not_to eq previous_token - expect(build.token).to be_a String - end - end - end - end - - describe 'setting a new token' do - subject { build.set_token('0123456789') } - - it 'returns the token' do - expect(subject).to eq '0123456789' - end - - it 'writes a new encrypted token' do - expect(build.read_attribute('token_encrypted')).to be_nil - expect(subject).to eq '0123456789' - expect(build.read_attribute('token_encrypted')).to be_present - end - - it 'does not write a new cleartext token' do - expect(build.read_attribute('token')).to be_nil - expect(subject).to eq '0123456789' - expect(build.read_attribute('token')).to be_nil - end - end -end diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index 87185891470..e779675744c 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -20,9 +20,9 @@ describe Ci::RetryBuildService do CLONE_ACCESSORS = described_class::CLONE_ACCESSORS REJECT_ACCESSORS = - %i[id status user token token_encrypted coverage trace runner - artifacts_expire_at artifacts_file artifacts_metadata artifacts_size - created_at updated_at started_at finished_at queued_at erased_by + %i[id status user token coverage trace runner artifacts_expire_at + artifacts_file artifacts_metadata artifacts_size created_at + updated_at started_at finished_at queued_at erased_by erased_at auto_canceled_by job_artifacts job_artifacts_archive job_artifacts_metadata job_artifacts_trace job_artifacts_junit job_artifacts_sast job_artifacts_dependency_scanning |