diff --git a/app/models/ci/trigger.rb b/app/models/ci/trigger.rb index 297d1cda50752785caaa83e74cfc6a2a2e7e2e93..2a6c8a38f5029b5bfc56a81a95f7053157503eb8 100644 --- a/app/models/ci/trigger.rb +++ b/app/models/ci/trigger.rb @@ -20,25 +20,20 @@ class Trigger < Ci::ApplicationRecord has_many :pipelines, class_name: 'Ci::Pipeline' - validates :token, presence: true, uniqueness: true + validates :token_encrypted, presence: true, uniqueness: true validates :owner, presence: true validates :project, presence: true validate :expires_at_before_instance_max_expiry_date, on: :create - before_validation :set_default_values + before_validation :ensure_token + ignore_column :token, remove_with: '18.7', remove_after: '2025-11-30' ignore_column :encrypted_token, remove_with: '18.4', remove_after: '2025-09-30' ignore_column :encrypted_token_iv, remove_with: '18.4', remove_after: '2025-09-30' - # rubocop:disable Gitlab/TokenWithoutPrefix -- we are doing this ourselves here since ensure_token - # does not work as expected - add_authentication_token_field(:token, - encrypted: -> { - Feature.enabled?(:encrypted_trigger_token_lookup, :instance) ? :required : :migrating - } - ) - # rubocop:enable Gitlab/TokenWithoutPrefix + add_authentication_token_field(:token, encrypted: :required, format_with_prefix: :token_prefix) + scope :with_last_used, -> do ci_pipelines = Ci::Pipeline.arel_table last_used_pipelines = @@ -54,12 +49,8 @@ class Trigger < Ci::ApplicationRecord scope :with_token, ->(tokens) { tokens = Array.wrap(tokens).reject(&:blank?) - if Feature.enabled?(:encrypted_trigger_token_lookup, :instance) - encrypted_tokens = tokens.map { |token| Ci::Trigger.encode(token) } - where(token_encrypted: encrypted_tokens) - else - where(token: tokens) - end + encrypted_tokens = tokens.map { |token| Ci::Trigger.encode(token) } + where(token_encrypted: encrypted_tokens) } scope :ready_for_deletion, -> { @@ -71,15 +62,9 @@ def self.prefix_for_trigger_token end def token=(token_value) - super self.set_token(token_value) end - def set_default_values - self.set_token(self.attributes['token']) if self.attributes['token'].present? - self.set_token("#{self.class.prefix_for_trigger_token}#{SecureRandom.hex(20)}") if self.token.blank? - end - def last_used # The instance should be preloaded by `.with_last_used` for performance reason return attributes['last_used'] if attributes.has_key?('last_used') @@ -101,6 +86,10 @@ def can_access_project? protected + def token_prefix + Ci::Trigger.prefix_for_trigger_token + end + def expires_at_before_instance_max_expiry_date return unless expires_at diff --git a/config/feature_flags/gitlab_com_derisk/encrypted_trigger_token_lookup.yml b/config/feature_flags/gitlab_com_derisk/encrypted_trigger_token_lookup.yml deleted file mode 100644 index 0fd1e6efb45ff2758862d5f1ae54a8bde9ec7321..0000000000000000000000000000000000000000 --- a/config/feature_flags/gitlab_com_derisk/encrypted_trigger_token_lookup.yml +++ /dev/null @@ -1,10 +0,0 @@ ---- -name: encrypted_trigger_token_lookup -description: -feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/work_items/554239 -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/197268 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/558025 -milestone: '18.3' -group: group::pipeline execution -type: gitlab_com_derisk -default_enabled: false diff --git a/db/post_migrate/20251022131103_remove_encrypted_token_from_ci_trigger.rb b/db/post_migrate/20251022131103_remove_encrypted_token_from_ci_trigger.rb new file mode 100644 index 0000000000000000000000000000000000000000..47d29549dc2d28eba89fe47d8969666e00307028 --- /dev/null +++ b/db/post_migrate/20251022131103_remove_encrypted_token_from_ci_trigger.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class RemoveEncryptedTokenFromCiTrigger < Gitlab::Database::Migration[2.3] + milestone '18.4' + disable_ddl_transaction! + + def up + remove_column :ci_triggers, :encrypted_token, if_exists: true + remove_column :ci_triggers, :encrypted_token_iv, if_exists: true + end + + def down + add_column :ci_triggers, :encrypted_token, :binary, if_not_exists: true + add_column :ci_triggers, :encrypted_token_iv, :binary, if_not_exists: true + end +end diff --git a/db/schema_migrations/20251022131103 b/db/schema_migrations/20251022131103 new file mode 100644 index 0000000000000000000000000000000000000000..f3f24ef594add643770bb72f4f7c4aaf127a651a --- /dev/null +++ b/db/schema_migrations/20251022131103 @@ -0,0 +1 @@ +08faada1fa0194099bf292a75257b08463cae76fab4fd897f8a161390f102494 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 64d8af58f2481beba53dfe077aa42827bfa206ed..da030a1368f5ea75866cb95b158da6d8a9bbc7df 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14385,8 +14385,6 @@ CREATE TABLE ci_triggers ( project_id bigint, owner_id bigint NOT NULL, description character varying, - encrypted_token bytea, - encrypted_token_iv bytea, expires_at timestamp with time zone, token_encrypted text, CONSTRAINT check_4905e4c2cb CHECK ((char_length(token_encrypted) <= 255)), diff --git a/spec/models/ci/trigger_spec.rb b/spec/models/ci/trigger_spec.rb index 6e654c81f0c479f6d806fee8fd063ca5cda99676..ae6cddcbe7ce69a1753c81b745cb956d180ffa00 100644 --- a/spec/models/ci/trigger_spec.rb +++ b/spec/models/ci/trigger_spec.rb @@ -101,7 +101,7 @@ end describe '.with_token' do - context 'when ff lookup for encrypted token is enabled' do + context 'when looking for ci trigger by token' do let_it_be(:project) { create(:project) } let_it_be(:trigger_1) { create(:ci_trigger, project: project) } let_it_be(:trigger_2) { create(:ci_trigger, project: project) } @@ -131,41 +131,6 @@ expect(result).to be_empty end end - - context 'when ff lookup for encrypted token is disabled' do - let_it_be(:project) { create(:project) } - let_it_be(:trigger_1) { create(:ci_trigger, project: project) } - let_it_be(:trigger_2) { create(:ci_trigger, project: project) } - let_it_be(:trigger_3) { create(:ci_trigger, project: project) } - - before do - stub_feature_flags(encrypted_trigger_token_lookup: false) - end - - it 'returns the trigger for a valid token' do - result = described_class.with_token(trigger_1.token) - - expect(result).to contain_exactly(trigger_1) - end - - it 'returns the triggers for multiple valid tokens' do - result = described_class.with_token([trigger_1.token, trigger_2.token]) - - expect(result).to contain_exactly(trigger_1, trigger_2) - end - - it 'ignores blank tokens' do - result = described_class.with_token([nil, '', ' ']) - - expect(result).to be_empty - end - - it 'does not return triggers for invalid token' do - result = described_class.with_token('nonexistent-token') - - expect(result).to be_empty - end - end end end