From 2d6fb778bf0424c91843f62fc2ca35f4e92ab133 Mon Sep 17 00:00:00 2001 From: Pratibha Gupta Date: Sat, 31 May 2025 15:43:17 +0530 Subject: [PATCH 1/4] Add automated email notifications for expiring project deploy tokens Introduces DeployTokens::ExpiringWorker that runs daily to notify project owners when deploy tokens expire within 7, 30, or 60 days. Includes batching, duplicate prevention, and feature flag support. Changelog: added --- app/models/deploy_token.rb | 34 ++ app/workers/all_queues.yml | 10 + app/workers/deploy_tokens/expiring_worker.rb | 117 +++++ config/bounded_contexts.yml | 5 + ...ct_deploy_token_expiring_notifications.yml | 10 + config/initializers/1_settings.rb | 3 + doc/user/profile/notifications.md | 1 + doc/user/project/deploy_tokens/_index.md | 22 + spec/models/deploy_token_spec.rb | 114 +++++ .../deploy_tokens/expiring_worker_spec.rb | 409 ++++++++++++++++++ 10 files changed, 725 insertions(+) create mode 100644 app/workers/deploy_tokens/expiring_worker.rb create mode 100644 config/feature_flags/gitlab_com_derisk/project_deploy_token_expiring_notifications.yml create mode 100644 spec/workers/deploy_tokens/expiring_worker_spec.rb diff --git a/app/models/deploy_token.rb b/app/models/deploy_token.rb index 6082295ed24130..a3825ba34c7f96 100644 --- a/app/models/deploy_token.rb +++ b/app/models/deploy_token.rb @@ -12,6 +12,12 @@ class DeployToken < ApplicationRecord GITLAB_DEPLOY_TOKEN_NAME = 'gitlab-deploy-token' DEPLOY_TOKEN_PREFIX = 'gldt-' + NOTIFICATION_INTERVALS = { + seven_days: 0..7, + thirty_days: 8..30, + sixty_days: 31..60 + }.freeze + add_authentication_token_field :token, encrypted: :required, format_with_prefix: :prefix_for_deploy_token attribute :expires_at, default: -> { Forever.date } @@ -50,6 +56,10 @@ class DeployToken < ApplicationRecord accepts_nested_attributes_for :project_deploy_tokens scope :active, -> { where("revoked = false AND expires_at >= NOW()") } + scope :project_token, -> { where(deploy_token_type: :project_type) } + scope :group_token, -> { where(deploy_token_type: :group_type) } + scope :order_expires_at_asc, -> { order(expires_at: :asc) } + scope :with_project_owners_and_maintainers, -> { includes(projects: :owners_and_maintainers) } def self.gitlab_deploy_token active.find_by(name: GITLAB_DEPLOY_TOKEN_NAME) @@ -61,6 +71,30 @@ def self.prefix_for_deploy_token ::Authn::TokenField::PrefixHelper.prepend_instance_prefix(DEPLOY_TOKEN_PREFIX) end + def self.notification_interval(interval) + NOTIFICATION_INTERVALS.fetch(interval).max + end + + def self.scope_for_notification_interval(interval, min_expires_at: nil, max_expires_at: nil) + interval_range = NOTIFICATION_INTERVALS.fetch(interval).minmax + min_expiry_date, max_expiry_date = interval_range.map { |range| Date.current + range } + min_expiry_date = min_expires_at if min_expires_at + max_expiry_date = max_expires_at if max_expires_at + interval_attr = "#{interval}_notification_sent_at" + + where(revoked: false) + .where(interval_attr => nil) + .where(expires_at: min_expiry_date..max_expiry_date) + end + + def self.ordered_for_keyset_pagination + order(:expires_at, :id) + end + + def self.update_notification_timestamps(token_ids, interval, timestamp = Time.current) + where(id: token_ids).update_all("#{interval}_notification_sent_at" => timestamp) + end + def valid_for_dependency_proxy? group_type? && active? && diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 83dbd9f452ee16..e9cefa5c17cecc 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -646,6 +646,16 @@ :idempotent: false :tags: [] :queue_namespace: :cronjob +- :name: cronjob:deploy_tokens_expiring + :worker_name: DeployTokens::ExpiringWorker + :feature_category: :continuous_delivery + :has_external_dependencies: false + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] + :queue_namespace: :cronjob - :name: cronjob:environments_auto_delete_cron :worker_name: Environments::AutoDeleteCronWorker :feature_category: :continuous_delivery diff --git a/app/workers/deploy_tokens/expiring_worker.rb b/app/workers/deploy_tokens/expiring_worker.rb new file mode 100644 index 00000000000000..d1861b078402a6 --- /dev/null +++ b/app/workers/deploy_tokens/expiring_worker.rb @@ -0,0 +1,117 @@ +# frozen_string_literal: true + +module DeployTokens + class ExpiringWorker + include ApplicationWorker + include Gitlab::Utils::StrongMemoize + idempotent! + + data_consistency :sticky + + include CronjobQueue + + feature_category :continuous_delivery + + MAX_RUNTIME = 3.minutes + REQUEUE_DELAY = 2.minutes + + BATCH_SIZE = 100 + + def perform(*args) + @runtime_limiter = Gitlab::Metrics::RuntimeLimiter.new(MAX_RUNTIME) + + notification_intervals.each do |interval| + process_deploy_tokens(interval) + break if over_time? + end + + self.class.perform_in(REQUEUE_DELAY, *args) if over_time? + end + + private + + attr_reader :runtime_limiter + + delegate :over_time?, to: :runtime_limiter + + def notification_intervals + DeployToken::NOTIFICATION_INTERVALS.keys + end + + def process_deploy_tokens(interval = :seven_days) + scope = DeployToken + .scope_for_notification_interval(interval) + .project_token + .active + .with_project_owners_and_maintainers + .ordered_for_keyset_pagination + + iterator = Gitlab::Pagination::Keyset::Iterator.new(scope: scope) + + iterator.each_batch(of: BATCH_SIZE) do |batch| + tokens_for_update = [] + + batch.each do |deploy_token| + next unless notify_users_of_deploy_token(deploy_token, interval) + + tokens_for_update << deploy_token.id + end + + if tokens_for_update.any? + begin + DeployToken.update_notification_timestamps(tokens_for_update, interval) + rescue ActiveRecord::ActiveRecordError => e + Gitlab::ErrorTracking.track_exception( + e, + message: "Failed to update deploy token notification timestamps", + token_ids: tokens_for_update, + interval: interval + ) + end + end + + break if over_time? + end + end + + def notify_users_of_deploy_token(deploy_token, interval) + notified = false + + deploy_token.projects.each do |project| + next unless Feature.enabled?(:project_deploy_token_expiring_notifications, project) + + users = project.owners_and_maintainers + next if users.empty? + + interval_days = DeployToken.notification_interval(interval) + + users.each do |user| + with_context(user: user) do + notification_service.deploy_token_about_to_expire( + user, + deploy_token.name, + project, + days_to_expire: interval_days + ) + end + rescue StandardError => e + Gitlab::ErrorTracking.track_exception( + e, + deploy_token_id: deploy_token.id, + user_id: user.id, + project_id: project.id + ) + end + + notified = true + end + + notified + end + + def notification_service + NotificationService.new + end + strong_memoize_attr :notification_service + end +end diff --git a/config/bounded_contexts.yml b/config/bounded_contexts.yml index e1183ee23e3751..6e3f3ebd6580e4 100644 --- a/config/bounded_contexts.yml +++ b/config/bounded_contexts.yml @@ -254,6 +254,11 @@ domains: description: feature_categories: - pages + + DeployTokens: + description: Managing deploy tokens + feature_categories: + - continuous_delivery Projects: description: Managing projects as workspaces and their lifecycle. Feature specific behavior must not go here. diff --git a/config/feature_flags/gitlab_com_derisk/project_deploy_token_expiring_notifications.yml b/config/feature_flags/gitlab_com_derisk/project_deploy_token_expiring_notifications.yml new file mode 100644 index 00000000000000..f58450453b30bd --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/project_deploy_token_expiring_notifications.yml @@ -0,0 +1,10 @@ +--- +name: project_deploy_token_expiring_notifications +description: Enable email notifications for project deploy tokens that are about to expire. +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/512197 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/195254 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/555470 +milestone: '18.3' +type: gitlab_com_derisk +group: group::environments +default_enabled: false diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 15f80808a0d7fb..81d3bbf7bb68b8 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -540,6 +540,9 @@ Settings.cron_jobs['personal_access_tokens_expiring_worker'] ||= {} Settings.cron_jobs['personal_access_tokens_expiring_worker']['cron'] ||= '0 1 * * *' Settings.cron_jobs['personal_access_tokens_expiring_worker']['job_class'] = 'PersonalAccessTokens::ExpiringWorker' +Settings.cron_jobs['deploy_tokens_expiring_worker'] ||= {} +Settings.cron_jobs['deploy_tokens_expiring_worker']['cron'] ||= '0 1 * * *' +Settings.cron_jobs['deploy_tokens_expiring_worker']['job_class'] = 'DeployTokens::ExpiringWorker' Settings.cron_jobs['personal_access_tokens_expired_notification_worker'] ||= {} Settings.cron_jobs['personal_access_tokens_expired_notification_worker']['cron'] ||= '0 2 * * *' Settings.cron_jobs['personal_access_tokens_expired_notification_worker']['job_class'] = 'PersonalAccessTokens::ExpiredNotificationWorker' diff --git a/doc/user/profile/notifications.md b/doc/user/profile/notifications.md index b19f179949060f..633423e1f27682 100644 --- a/doc/user/profile/notifications.md +++ b/doc/user/profile/notifications.md @@ -204,6 +204,7 @@ Users are notified of the following events: | Personal access tokens have been created | User | Security email, always sent. | | Personal access tokens have expired | User | Security email, always sent. | | Personal access token has been revoked | User | Security email, always sent. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/98911) in GitLab 15.5. | +| Project deploy tokens expiring soon | Project owners and maintainers | Security email, always sent. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/512197) in GitLab 18.3. | | Personal access token has been rotated | User | Security email, always sent. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/199360) in GitLab 18.3. | | Group access tokens expiring soon | Direct Group Owners | Security email, always sent. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/367705) in GitLab 16.4. | | Project access tokens expiring soon | Direct Project Owners and Maintainers | Security email, always sent. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/367706) in GitLab 16.4. | diff --git a/doc/user/project/deploy_tokens/_index.md b/doc/user/project/deploy_tokens/_index.md index d4aa20bb91e4a3..2f36fd877cc84f 100644 --- a/doc/user/project/deploy_tokens/_index.md +++ b/doc/user/project/deploy_tokens/_index.md @@ -102,6 +102,28 @@ name and token of the group deploy token. When `gitlab-deploy-token` is defined in a group, the `CI_DEPLOY_USER` and `CI_DEPLOY_PASSWORD` CI/CD variables are available only to immediate child projects of the group. +## Deploy token expiration + +{{< history >}} + +- Email notifications for deploy token expiration [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/512197) in GitLab 18.3 [with a flag](../../../administration/feature_flags/_index.md) named `project_deploy_token_expiring_notifications`. Disabled by default. + +{{< /history >}} + +{{< alert type="flag" >}} + +The availability of this feature is controlled by a feature flag. +For more information, see the history. + +{{< /alert >}} + +Deploy tokens expire on the date you define at 00:00 AM UTC. + +GitLab checks every day at 01:00 AM UTC for deploy tokens that are about to expire. +Project owners and maintainers are notified by email 60, 30, and 7 days before these tokens expire. + +These email notifications are sent only once per interval for active (non-revoked) deploy tokens. + ### GitLab deploy token security GitLab deploy tokens are long-lived, making them attractive for attackers. diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb index 3bf73c05b1c419..738d74529d2592 100644 --- a/spec/models/deploy_token_spec.rb +++ b/spec/models/deploy_token_spec.rb @@ -525,4 +525,118 @@ expect(encoded_token).to eq('fake_encrypted_token') end end + + describe '.notification_interval' do + it 'returns the maximum days for seven_days interval' do + expect(described_class.notification_interval(:seven_days)).to eq(7) + end + + it 'returns the maximum days for thirty_days interval' do + expect(described_class.notification_interval(:thirty_days)).to eq(30) + end + + it 'returns the maximum days for sixty_days interval' do + expect(described_class.notification_interval(:sixty_days)).to eq(60) + end + + it 'raises KeyError for invalid interval' do + expect { described_class.notification_interval(:invalid) }.to raise_error(KeyError) + end + end + + describe '.scope_for_notification_interval' do + let_it_be(:project) { create(:project) } + + context 'for seven_days interval' do + let!(:token_in_range) do + create(:deploy_token, :project, projects: [project], expires_at: 5.days.from_now.iso8601) + end + + let!(:token_out_of_range) do + create(:deploy_token, :project, projects: [project], expires_at: 10.days.from_now.iso8601) + end + + let!(:already_notified_token) do + create(:deploy_token, :project, projects: [project], + expires_at: 3.days.from_now.iso8601, + seven_days_notification_sent_at: 1.day.ago) + end + + let!(:revoked_token) do + create(:deploy_token, :project, :revoked, projects: [project], expires_at: 4.days.from_now.iso8601) + end + + it 'returns tokens expiring within the interval that have not been notified' do + result = described_class.scope_for_notification_interval(:seven_days) + + expect(result).to include(token_in_range) + expect(result).not_to include(token_out_of_range) + expect(result).not_to include(already_notified_token) + expect(result).not_to include(revoked_token) + end + end + + context 'with custom date range' do + let!(:token) do + create(:deploy_token, :project, projects: [project], expires_at: 15.days.from_now.iso8601) + end + + it 'respects min_expires_at parameter' do + result = described_class.scope_for_notification_interval(:thirty_days, min_expires_at: 20.days.from_now) + expect(result).not_to include(token) + end + + it 'respects max_expires_at parameter' do + result = described_class.scope_for_notification_interval(:thirty_days, max_expires_at: 10.days.from_now) + expect(result).not_to include(token) + end + end + end + + describe 'scopes' do + describe '.project_token' do + let_it_be(:project) { create(:project) } + let_it_be(:group_deploy_token) { create(:deploy_token, :group) } + + let!(:project_deploy_token) do + create(:deploy_token, :project, projects: [project]) + end + + it 'returns only project type tokens' do + expect(described_class.project_token).to include(project_deploy_token) + expect(described_class.project_token).not_to include(group_deploy_token) + end + end + + describe '.group_token' do + let_it_be(:project) { create(:project) } + let_it_be(:group_deploy_token) { create(:deploy_token, :group) } + + let!(:project_deploy_token) do + create(:deploy_token, :project, projects: [project]) + end + + it 'returns only group type tokens' do + expect(described_class.group_token).to include(group_deploy_token) + expect(described_class.group_token).not_to include(project_deploy_token) + end + end + + describe '.order_expires_at_asc' do + let!(:token_expires_later) { create(:deploy_token, expires_at: 10.days.from_now.iso8601) } + let!(:token_expires_sooner) { create(:deploy_token, expires_at: 5.days.from_now.iso8601) } + + it 'orders tokens by expires_at in ascending order' do + result = described_class.order_expires_at_asc.to_a + expect(result.index(token_expires_sooner)).to be < result.index(token_expires_later) + end + end + + describe '.with_project_owners_and_maintainers' do + it 'includes projects association with owners and maintainers' do + query = described_class.with_project_owners_and_maintainers + expect(query.includes_values).to include(projects: :owners_and_maintainers) + end + end + end end diff --git a/spec/workers/deploy_tokens/expiring_worker_spec.rb b/spec/workers/deploy_tokens/expiring_worker_spec.rb new file mode 100644 index 00000000000000..a3626681c3c6e0 --- /dev/null +++ b/spec/workers/deploy_tokens/expiring_worker_spec.rb @@ -0,0 +1,409 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe DeployTokens::ExpiringWorker, feature_category: :continuous_delivery do + subject(:worker) { described_class.new } + + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be(:maintainer) { create(:user) } + let_it_be(:owner) { create(:user) } + + before_all do + project.add_maintainer(maintainer) + project.add_owner(owner) + end + + describe '#perform' do + subject(:perform) { worker.perform } + + context 'when feature flag is disabled' do + let!(:expiring_token) do + create(:deploy_token, :project_type, projects: [project], expires_at: 15.days.from_now.iso8601) + end + + before do + stub_feature_flags(project_deploy_token_expiring_notifications: false) + end + + it 'does not send any notifications' do + expect(NotificationService).not_to receive(:new) + perform + end + end + + context 'when feature flag is enabled' do + context 'when expiring deploy tokens have already been notified' do + let_it_be(:already_notified_deploy_token_1) do + create( + :deploy_token, + :project_type, + projects: [project], + expires_at: 5.days.from_now.iso8601, + seven_days_notification_sent_at: 2.days.ago.iso8601 + ) + end + + let_it_be(:already_notified_deploy_token_2) do + create( + :deploy_token, + :project_type, + projects: [project], + expires_at: 4.days.from_now.iso8601, + seven_days_notification_sent_at: 3.days.ago.iso8601 + ) + end + + it 'does not send any notifications' do + expect(NotificationService).not_to receive(:new) + perform + end + + it "doesn't update notification sent timestamps of deploy tokens" do + expect { perform }.not_to change { + [ + already_notified_deploy_token_1.reload.seven_days_notification_sent_at, + already_notified_deploy_token_2.reload.seven_days_notification_sent_at + ] + } + end + end + + context 'when tokens expire within 30 days' do + let!(:expiring_token) do + create(:deploy_token, :project_type, projects: [project], expires_at: 15.days.from_now.iso8601) + end + + let!(:expiring_token2) do + create(:deploy_token, :project_type, projects: [project], expires_at: 20.days.from_now.iso8601) + end + + let!(:notified_token) do + create(:deploy_token, :project_type, projects: [project], expires_at: 25.days.from_now.iso8601, + thirty_days_notification_sent_at: Time.current) + end + + let!(:not_expiring_token) do + create(:deploy_token, :project_type, projects: [project], expires_at: 6.months.from_now.iso8601) + end + + let!(:revoked_token) do + create(:deploy_token, :project_type, :revoked, projects: [project], expires_at: 28.days.from_now.iso8601) + end + + it 'uses notification service to send emails to all users for each token' do + expect_next_instance_of(NotificationService) do |notification_service| + project.owners_and_maintainers.each do |user| + expect(notification_service).to receive(:deploy_token_about_to_expire) + .with(user, expiring_token.name, project, hash_including(days_to_expire: 30)) + expect(notification_service).to receive(:deploy_token_about_to_expire) + .with(user, expiring_token2.name, project, hash_including(days_to_expire: 30)) + expect(notification_service).not_to receive(:deploy_token_about_to_expire) + .with(user, not_expiring_token.name, project, hash_including(days_to_expire: 30)) + expect(notification_service).not_to receive(:deploy_token_about_to_expire) + .with(user, notified_token.name, project, hash_including(days_to_expire: 30)) + expect(notification_service).not_to receive(:deploy_token_about_to_expire) + .with(user, revoked_token.name, project, hash_including(days_to_expire: 30)) + end + end + + perform + end + + it 'marks the notification as delivered', :freeze_time do + expect(expiring_token.thirty_days_notification_sent_at).to be_nil + expect(expiring_token2.thirty_days_notification_sent_at).to be_nil + expect(not_expiring_token.thirty_days_notification_sent_at).to be_nil + expect(notified_token.thirty_days_notification_sent_at).to eq(Time.current) + + perform + + expect(expiring_token.reload.thirty_days_notification_sent_at).to eq(Time.current) + expect(expiring_token2.reload.thirty_days_notification_sent_at).to eq(Time.current) + expect(not_expiring_token.reload.thirty_days_notification_sent_at).to be_nil + expect(notified_token.reload.thirty_days_notification_sent_at).to eq(Time.current) + end + end + + context 'when tokens expire within 60 days' do + let!(:expiring_token) do + create(:deploy_token, :project_type, projects: [project], expires_at: 36.days.from_now.iso8601) + end + + let!(:expiring_token2) do + create(:deploy_token, :project_type, projects: [project], expires_at: 47.days.from_now.iso8601) + end + + let!(:notified_token) do + create(:deploy_token, :project_type, projects: [project], expires_at: 55.days.from_now.iso8601, + sixty_days_notification_sent_at: Time.current) + end + + let!(:not_expiring_token) do + create(:deploy_token, :project_type, projects: [project], expires_at: 6.months.from_now.iso8601) + end + + let!(:revoked_token) do + create(:deploy_token, :project_type, :revoked, projects: [project], expires_at: 58.days.from_now.iso8601) + end + + it 'uses notification service to send emails to all users for each token' do + expect_next_instance_of(NotificationService) do |notification_service| + project.owners_and_maintainers.each do |user| + expect(notification_service).to receive(:deploy_token_about_to_expire) + .with(user, expiring_token.name, project, hash_including(days_to_expire: 60)) + expect(notification_service).to receive(:deploy_token_about_to_expire) + .with(user, expiring_token2.name, project, hash_including(days_to_expire: 60)) + expect(notification_service).not_to receive(:deploy_token_about_to_expire) + .with(user, not_expiring_token.name, project, hash_including(days_to_expire: 60)) + expect(notification_service).not_to receive(:deploy_token_about_to_expire) + .with(user, notified_token.name, project, hash_including(days_to_expire: 60)) + expect(notification_service).not_to receive(:deploy_token_about_to_expire) + .with(user, revoked_token.name, project, hash_including(days_to_expire: 60)) + end + end + + perform + end + + it 'marks the notification as delivered', :freeze_time do + expect(expiring_token.sixty_days_notification_sent_at).to be_nil + expect(expiring_token2.sixty_days_notification_sent_at).to be_nil + expect(not_expiring_token.sixty_days_notification_sent_at).to be_nil + expect(notified_token.sixty_days_notification_sent_at).to eq(Time.current) + + perform + + expect(expiring_token.reload.sixty_days_notification_sent_at).to eq(Time.current) + expect(expiring_token2.reload.sixty_days_notification_sent_at).to eq(Time.current) + expect(not_expiring_token.reload.sixty_days_notification_sent_at).to be_nil + expect(notified_token.reload.sixty_days_notification_sent_at).to eq(Time.current) + end + end + + context 'when no deploy tokens are within notification window' do + let_it_be(:deploy_token_1) { create(:deploy_token, :project_type, expires_at: 75.days.from_now.iso8601) } + let_it_be(:deploy_token_2) { create(:deploy_token, :project_type, expires_at: 64.days.from_now.iso8601) } + + it 'does not send any notifications' do + expect(NotificationService).not_to receive(:new) + perform + end + + it "doesn't update notification sent timestamps of deploy tokens" do + expect { perform }.not_to change { + [ + deploy_token_1.reload.seven_days_notification_sent_at, + deploy_token_2.reload.seven_days_notification_sent_at + ] + } + end + end + + context 'when deploy tokens have already expired' do + let_it_be(:expired_deploy_token_1) do + create(:deploy_token, :project_type, projects: [project], expires_at: 5.days.ago.iso8601) + end + + let_it_be(:expired_deploy_token_2) do + create(:deploy_token, :project_type, projects: [project], expires_at: 10.days.ago.iso8601) + end + + it 'does not send any notifications' do + expect(NotificationService).not_to receive(:new) + perform + end + + it "doesn't update notification sent timestamps of deploy tokens" do + expect { perform }.not_to change { + [ + expired_deploy_token_1.reload.seven_days_notification_sent_at, + expired_deploy_token_2.reload.seven_days_notification_sent_at + ] + } + end + end + + context 'when deploy tokens are revoked' do + let_it_be(:revoked_deploy_token_1) do + create( + :deploy_token, + :project_type, :revoked, + expires_at: 5.days.ago.iso8601, + projects: [project] + ) + end + + it 'does not send notifications for revoked tokens' do + expect(NotificationService).not_to receive(:new) + perform + end + end + + context 'when project has no owners or maintainers' do + let_it_be(:project_without_members) { create(:project) } + + let_it_be(:token_without_members) do + create(:deploy_token, :project_type, expires_at: 5.days.from_now.iso8601, projects: [project_without_members]) + end + + before do + project_without_members.project_members.delete_all + end + + it 'does not send notifications' do + notification_service = instance_double(NotificationService) + allow(NotificationService).to receive(:new).and_return(notification_service) + + expect(notification_service).not_to receive(:deploy_token_about_to_expire) + + perform + end + end + + context 'when there are expiring deploy tokens' do + let!(:expiring_seven_day_token) do + create(:deploy_token, :project_type, projects: [project], expires_at: 6.days.from_now.iso8601) + end + + let!(:expiring_thirty_day_token) do + create(:deploy_token, :project_type, projects: [project], expires_at: 15.days.from_now.iso8601) + end + + let!(:expiring_sixty_day_token) do + create(:deploy_token, :project_type, projects: [project], expires_at: 45.days.from_now.iso8601) + end + + let!(:revoked_token) do + create(:deploy_token, :revoked, :project_type, projects: [project], expires_at: 5.days.from_now.iso8601) + end + + let!(:not_expiring_token) do + create(:deploy_token, :project_type, projects: [project], expires_at: 6.months.from_now.iso8601) + end + + let!(:notified_token) do + create( + :deploy_token, + :project_type, + projects: [project], + expires_at: 4.days.from_now.iso8601, + seven_days_notification_sent_at: 2.days.ago.iso8601 + ) + end + + it 'processes all notification intervals' do + expect(worker).to receive(:process_deploy_tokens).with(:seven_days) + expect(worker).to receive(:process_deploy_tokens).with(:thirty_days) + expect(worker).to receive(:process_deploy_tokens).with(:sixty_days) + + perform + end + + it 'uses notification service to send emails to all users for each token' do + expect_next_instance_of(NotificationService) do |notification_service| + project.owners_and_maintainers.each do |user| + expect(notification_service).to receive(:deploy_token_about_to_expire) + .with(user, expiring_seven_day_token.name, project, hash_including(days_to_expire: 7)) + expect(notification_service).to receive(:deploy_token_about_to_expire) + .with(user, expiring_thirty_day_token.name, project, hash_including(days_to_expire: 30)) + expect(notification_service).to receive(:deploy_token_about_to_expire) + .with(user, expiring_sixty_day_token.name, project, hash_including(days_to_expire: 60)) + end + end + + perform + end + + it 'updates notification sent timestamps', :freeze_time do + expect { perform }.to change { + [ + expiring_seven_day_token.reload.seven_days_notification_sent_at, + expiring_thirty_day_token.reload.thirty_days_notification_sent_at, + expiring_sixty_day_token.reload.sixty_days_notification_sent_at + ] + }.from([nil, nil, nil]).to([Time.current, Time.current, Time.current]) + end + + it 'does not send duplicate notifications' do + perform + + notification_service = instance_double(NotificationService) + allow(NotificationService).to receive(:new).and_return(notification_service) + + expect(notification_service).not_to receive(:deploy_token_about_to_expire) + perform + end + + it 'avoids N+1 queries', :use_sql_query_cache do + project1 = create(:project) + user1 = create(:user) + project1.add_maintainer(user1) + create(:deploy_token, :project_type, projects: [project1], expires_at: 5.days.from_now.iso8601) + + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + worker.perform + end + + project2 = create(:project) + user2 = create(:user) + project2.add_maintainer(user2) + create(:deploy_token, :project_type, projects: [project2], expires_at: 5.days.from_now.iso8601) + + expect { worker.perform }.not_to exceed_all_query_limit(control).with_threshold(2) + end + end + + context 'with multiple batches of tokens' do + let_it_be(:expiring_deploy_tokens) do + create_list(:deploy_token, 4, :project_type, expires_at: 6.days.from_now.iso8601, projects: [project]) + end + + before do + stub_const("#{described_class}::BATCH_SIZE", 2) + allow(worker).to receive(:notification_intervals).and_return([:seven_days]) + end + + it 'processes tokens using keyset pagination' do + expect(Gitlab::Pagination::Keyset::Iterator).to receive(:new).and_call_original + + perform + + expiring_deploy_tokens.each do |token| + token.reload + expect(token.seven_days_notification_sent_at).not_to be_nil + end + end + + it 'processes all tokens across multiple batches' do + perform + + expiring_deploy_tokens.each do |token| + token.reload + expect(token.seven_days_notification_sent_at).not_to be_nil + end + end + + context 'when iteration runs over time limit' do + before do + allow(worker).to receive(:over_time?).and_return(false, true) + end + + it 'processes partial batches and requeues the job' do + expect(described_class).to receive(:perform_in).with(described_class::REQUEUE_DELAY) + + worker.perform + + processed_count = expiring_deploy_tokens.count do |token| + token.reload.seven_days_notification_sent_at.present? + end + + expect(processed_count).to be >= 0 + expect(processed_count).to be <= expiring_deploy_tokens.count + end + end + end + end + end +end -- GitLab From 30211c9f5e9ebee003e85fa9a4e10e116f59977f Mon Sep 17 00:00:00 2001 From: Pratibha Gupta Date: Thu, 31 Jul 2025 02:51:04 +0530 Subject: [PATCH 2/4] Add logic change as deploy token belongs to single project --- app/workers/deploy_tokens/expiring_worker.rb | 53 +++++++++----------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/app/workers/deploy_tokens/expiring_worker.rb b/app/workers/deploy_tokens/expiring_worker.rb index d1861b078402a6..70a3c3a2f8cab4 100644 --- a/app/workers/deploy_tokens/expiring_worker.rb +++ b/app/workers/deploy_tokens/expiring_worker.rb @@ -75,38 +75,35 @@ def process_deploy_tokens(interval = :seven_days) end def notify_users_of_deploy_token(deploy_token, interval) - notified = false - - deploy_token.projects.each do |project| - next unless Feature.enabled?(:project_deploy_token_expiring_notifications, project) - - users = project.owners_and_maintainers - next if users.empty? - - interval_days = DeployToken.notification_interval(interval) - - users.each do |user| - with_context(user: user) do - notification_service.deploy_token_about_to_expire( - user, - deploy_token.name, - project, - days_to_expire: interval_days - ) - end - rescue StandardError => e - Gitlab::ErrorTracking.track_exception( - e, - deploy_token_id: deploy_token.id, - user_id: user.id, - project_id: project.id + # every deploy token is mapped to one project, but a project can have multiple deploy tokens + project = deploy_token.projects.first + return false unless project + return false unless Feature.enabled?(:project_deploy_token_expiring_notifications, project) + + users = project.owners_and_maintainers + return false if users.empty? + + interval_days = DeployToken.notification_interval(interval) + + users.each do |user| + with_context(user: user) do + notification_service.deploy_token_about_to_expire( + user, + deploy_token.name, + project, + days_to_expire: interval_days ) end - - notified = true + rescue StandardError => e + Gitlab::ErrorTracking.track_exception( + e, + deploy_token_id: deploy_token.id, + user_id: user.id, + project_id: project.id + ) end - notified + true end def notification_service -- GitLab From 07045b11049e3c61322cd8770f16dacb7e910698 Mon Sep 17 00:00:00 2001 From: Pratibha Gupta Date: Fri, 1 Aug 2025 05:44:30 +0530 Subject: [PATCH 3/4] Add test for notify_users_of_deploy_token --- app/workers/deploy_tokens/expiring_worker.rb | 11 ++- .../deploy_tokens/expiring_worker_spec.rb | 85 +++++++++++++++++++ 2 files changed, 93 insertions(+), 3 deletions(-) diff --git a/app/workers/deploy_tokens/expiring_worker.rb b/app/workers/deploy_tokens/expiring_worker.rb index 70a3c3a2f8cab4..d2f96f494a6b5d 100644 --- a/app/workers/deploy_tokens/expiring_worker.rb +++ b/app/workers/deploy_tokens/expiring_worker.rb @@ -75,6 +75,8 @@ def process_deploy_tokens(interval = :seven_days) end def notify_users_of_deploy_token(deploy_token, interval) + all_succeeded = true + # every deploy token is mapped to one project, but a project can have multiple deploy tokens project = deploy_token.projects.first return false unless project @@ -95,15 +97,18 @@ def notify_users_of_deploy_token(deploy_token, interval) ) end rescue StandardError => e + all_succeeded = false Gitlab::ErrorTracking.track_exception( e, + message: 'Failed to send notification about expiring project deploy tokens', + exception_message: e.message, deploy_token_id: deploy_token.id, - user_id: user.id, - project_id: project.id + project_id: project.id, + user_id: user.id ) end - true + all_succeeded end def notification_service diff --git a/spec/workers/deploy_tokens/expiring_worker_spec.rb b/spec/workers/deploy_tokens/expiring_worker_spec.rb index a3626681c3c6e0..b10cd22d998fbc 100644 --- a/spec/workers/deploy_tokens/expiring_worker_spec.rb +++ b/spec/workers/deploy_tokens/expiring_worker_spec.rb @@ -182,6 +182,91 @@ end end + context 'when notification service fails for a deploy token' do + let_it_be(:user_test) { create(:user) } + + context 'when exception is raised during processing' do + let_it_be(:project_test) { create(:project) } + + before_all do + project_test.project_members.delete_all + project_test.add_maintainer(user_test) + end + + context 'with a single resource deploy token' do + let!(:expiring_token) do + create(:deploy_token, :project_type, projects: [project_test], expires_at: 5.days.from_now.iso8601) + end + + before do + allow_next_instance_of(NotificationService) do |service| + allow(service).to( + receive(:deploy_token_about_to_expire) + .with(an_instance_of(User), expiring_token.name, project_test, hash_including(days_to_expire: 7)) + .and_raise(StandardError.new('boom!')) + ) + end + end + + it 'tracks the exception' do + expect(Gitlab::ErrorTracking).to( + receive(:track_exception) + .with( + an_instance_of(StandardError), + hash_including( + message: 'Failed to send notification about expiring project deploy tokens', + exception_message: 'boom!', + deploy_token_id: expiring_token.id, + project_id: project_test.id + ) + ) + ) + perform + end + + it 'does not update token with failed delivery' do + expect(expiring_token.seven_days_notification_sent_at).to be_nil + perform + expect(expiring_token.reload.seven_days_notification_sent_at).to be_nil + end + end + + context 'with multiple project deploy tokens' do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:failing_token) do + create(:deploy_token, :project_type, projects: [project], expires_at: 5.days.from_now.iso8601) + end + + let_it_be(:successful_token) do + create(:deploy_token, :project_type, projects: [project], expires_at: 5.days.from_now.iso8601) + end + + before_all do + project.project_members.delete_all + project.add_maintainer(user) + + allow_next_instance_of(NotificationService) do |service| + allow(service).to receive(:deploy_token_about_to_expire) + .with(user, failing_token.name, project, hash_including(days_to_expire: 7)) + .and_raise(StandardError.new('boom!')) + + allow(service).to receive(:deploy_token_about_to_expire) + .with(user, successful_token.name, project, hash_including(days_to_expire: 7)) + .and_call_original + end + end + + it 'continues processing other tokens when one fails' do + perform + + expect(failing_token.reload.seven_days_notification_sent_at).to be_nil + expect(successful_token.reload.seven_days_notification_sent_at).not_to be_nil + end + end + end + end + context 'when no deploy tokens are within notification window' do let_it_be(:deploy_token_1) { create(:deploy_token, :project_type, expires_at: 75.days.from_now.iso8601) } let_it_be(:deploy_token_2) { create(:deploy_token, :project_type, expires_at: 64.days.from_now.iso8601) } -- GitLab From c07772e04866e2b3312c73cfab4dbe74aa8e2dcf Mon Sep 17 00:00:00 2001 From: Pratibha Gupta Date: Sat, 2 Aug 2025 02:46:28 +0530 Subject: [PATCH 4/4] Add test for notification update failure --- .../deploy_tokens/expiring_worker_spec.rb | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/spec/workers/deploy_tokens/expiring_worker_spec.rb b/spec/workers/deploy_tokens/expiring_worker_spec.rb index b10cd22d998fbc..503dc85c822c2b 100644 --- a/spec/workers/deploy_tokens/expiring_worker_spec.rb +++ b/spec/workers/deploy_tokens/expiring_worker_spec.rb @@ -70,6 +70,37 @@ end end + context 'when timestamp update fails' do + let!(:expiring_token_1) do + create(:deploy_token, :project_type, projects: [project], expires_at: 25.days.from_now.iso8601) + end + + let!(:expiring_token_2) do + create(:deploy_token, :project_type, projects: [project], expires_at: 28.days.from_now.iso8601) + end + + before do + allow(worker).to receive(:over_time?).and_return(false) + allow(DeployToken).to receive(:update_notification_timestamps) + .with([expiring_token_1.id, expiring_token_2.id], :thirty_days) + .and_raise(ActiveRecord::ActiveRecordError.new('database error')) + end + + it 'tracks the timestamp update exception' do + expect(Gitlab::ErrorTracking).to receive(:track_exception) + .with( + an_instance_of(ActiveRecord::ActiveRecordError), + hash_including( + message: "Failed to update deploy token notification timestamps", + token_ids: match_array([expiring_token_1.id, expiring_token_2.id]), + interval: :thirty_days + ) + ) + + worker.send(:process_deploy_tokens, :thirty_days) + end + end + context 'when tokens expire within 30 days' do let!(:expiring_token) do create(:deploy_token, :project_type, projects: [project], expires_at: 15.days.from_now.iso8601) @@ -245,7 +276,9 @@ before_all do project.project_members.delete_all project.add_maintainer(user) + end + before do allow_next_instance_of(NotificationService) do |service| allow(service).to receive(:deploy_token_about_to_expire) .with(user, failing_token.name, project, hash_including(days_to_expire: 7)) -- GitLab