diff --git a/app/models/user.rb b/app/models/user.rb index 92d11d231ec67035b18e7d35d2fa7760dca3d5ba..783a3157375641b4aaf1e4dbd58a38e6dde97dbb 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -335,6 +335,8 @@ def update_tracked_fields!(request) end event :deactivate do + # Any additional changes to this event should be also + # reflected in app/workers/users/deactivate_dormant_users_worker.rb transition active: :deactivated end @@ -416,6 +418,8 @@ def blocked? scope :order_recent_last_activity, -> { reorder(Gitlab::Database.nulls_last_order('last_activity_on', 'DESC')) } scope :order_oldest_last_activity, -> { reorder(Gitlab::Database.nulls_first_order('last_activity_on', 'ASC')) } scope :by_id_and_login, ->(id, login) { where(id: id).where('username = LOWER(:login) OR email = LOWER(:login)', login: login) } + scope :dormant, -> { active.where('last_activity_on <= ?', MINIMUM_INACTIVE_DAYS.day.ago.to_date) } + scope :with_no_activity, -> { active.where(last_activity_on: nil) } def preferred_language read_attribute('preferred_language') || diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index fa6ea54e342ec8c845288e37a9047ed8a8ce0ca5..0417d214231056a041deb5854bf07309edbdfd2a 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -515,6 +515,14 @@ :weight: 1 :idempotent: :tags: [] +- :name: cronjob:users_deactivate_dormant_users + :feature_category: :utilization + :has_external_dependencies: + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: + :tags: [] - :name: cronjob:x509_issuer_crl_check :feature_category: :source_code_management :has_external_dependencies: true diff --git a/app/workers/users/deactivate_dormant_users_worker.rb b/app/workers/users/deactivate_dormant_users_worker.rb new file mode 100644 index 0000000000000000000000000000000000000000..f92a12310b654fb390c7e267a4c1920533a7b283 --- /dev/null +++ b/app/workers/users/deactivate_dormant_users_worker.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module Users + class DeactivateDormantUsersWorker # rubocop:disable Scalability/IdempotentWorker + include ApplicationWorker + include CronjobQueue + + feature_category :utilization + + NUMBER_OF_BATCHES = 50 + BATCH_SIZE = 200 + PAUSE_SECONDS = 0.25 + + def perform + return if Gitlab.com? + + return unless ::Gitlab::CurrentSettings.current_application_settings.deactivate_dormant_users + + with_context(caller_id: self.class.name.to_s) do + NUMBER_OF_BATCHES.times do + result = User.connection.execute(update_query) + + break if result.cmd_tuples == 0 + + sleep(PAUSE_SECONDS) + end + end + end + + private + + def update_query + <<~SQL + UPDATE "users" + SET "state" = 'deactivated' + WHERE "users"."id" IN ( + (#{users.dormant.to_sql}) + UNION + (#{users.with_no_activity.to_sql}) + LIMIT #{BATCH_SIZE} + ) + SQL + end + + def users + User.select(:id).limit(BATCH_SIZE) + end + end +end diff --git a/changelogs/unreleased/211754-automate-dormant-users-deactivation.yml b/changelogs/unreleased/211754-automate-dormant-users-deactivation.yml new file mode 100644 index 0000000000000000000000000000000000000000..f8af79e9a09be610c1dbdc33f7cbaf1ec9e494cf --- /dev/null +++ b/changelogs/unreleased/211754-automate-dormant-users-deactivation.yml @@ -0,0 +1,5 @@ +--- +title: Automate deactivation of dormant users for self-managed instances +merge_request: 57778 +author: +type: added diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 99335321f28e0659efb54d227f9ee9dba317d2d4..4beaa2da810d014e797b6e42e755defb0677d51c 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -569,6 +569,9 @@ Settings.cron_jobs['ssh_keys_expiring_soon_notification_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['ssh_keys_expiring_soon_notification_worker']['cron'] ||= '0 1 * * *' Settings.cron_jobs['ssh_keys_expiring_soon_notification_worker']['job_class'] = 'SshKeys::ExpiringSoonNotificationWorker' +Settings.cron_jobs['users_deactivate_dormant_users_worker'] ||= Settingslogic.new({}) +Settings.cron_jobs['users_deactivate_dormant_users_worker']['cron'] ||= '21,42 0-4 * * *' +Settings.cron_jobs['users_deactivate_dormant_users_worker']['job_class'] = 'Users::DeactivateDormantUsersWorker' Gitlab.com do Settings.cron_jobs['batched_background_migrations_worker'] ||= Settingslogic.new({}) diff --git a/db/migrate/20210421021510_add_deactivate_dormant_users_to_application_settings.rb b/db/migrate/20210421021510_add_deactivate_dormant_users_to_application_settings.rb new file mode 100644 index 0000000000000000000000000000000000000000..74d197cd3b874a383bb58f8f163e4ce3d39f7236 --- /dev/null +++ b/db/migrate/20210421021510_add_deactivate_dormant_users_to_application_settings.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddDeactivateDormantUsersToApplicationSettings < ActiveRecord::Migration[6.0] + def change + add_column :application_settings, :deactivate_dormant_users, :boolean, default: false, null: false + end +end diff --git a/db/migrate/20210421022010_add_index_for_dormant_users.rb b/db/migrate/20210421022010_add_index_for_dormant_users.rb new file mode 100644 index 0000000000000000000000000000000000000000..48eff184ca07f31276e65fc505bb2166b244d836 --- /dev/null +++ b/db/migrate/20210421022010_add_index_for_dormant_users.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddIndexForDormantUsers < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + INDEX_NAME = 'index_users_on_id_and_last_activity_on_for_non_internal_active' + + disable_ddl_transaction! + + def up + index_condition = "state = 'active' AND (users.user_type IS NULL OR users.user_type IN (NULL, 6, 4))" + + add_concurrent_index :users, [:id, :last_activity_on], where: index_condition, name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :users, INDEX_NAME + end +end diff --git a/db/schema_migrations/20210421021510 b/db/schema_migrations/20210421021510 new file mode 100644 index 0000000000000000000000000000000000000000..775f083ac63133dd511d9d79f38238561fc7d9ef --- /dev/null +++ b/db/schema_migrations/20210421021510 @@ -0,0 +1 @@ +6a278c90b8c97fc2255528605ee6bf4547e37ac8c4c17979483ed9db562fa021 \ No newline at end of file diff --git a/db/schema_migrations/20210421022010 b/db/schema_migrations/20210421022010 new file mode 100644 index 0000000000000000000000000000000000000000..75abced628df600b1f6c5e30f3933071235d0cb4 --- /dev/null +++ b/db/schema_migrations/20210421022010 @@ -0,0 +1 @@ +454992d01fa140896ff2a9cea66fb855c9e659a5a7969ac9a3cb5a608de36161 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 331684d42465350a8124f939012e296dd38f91e5..230d595710c7569f557ac84768ae7c5dbe68c996 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9470,6 +9470,7 @@ CREATE TABLE application_settings ( throttle_authenticated_packages_api_period_in_seconds integer DEFAULT 15 NOT NULL, throttle_unauthenticated_packages_api_enabled boolean DEFAULT false NOT NULL, throttle_authenticated_packages_api_enabled boolean DEFAULT false NOT NULL, + deactivate_dormant_users boolean DEFAULT false NOT NULL, CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)), CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)), CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)), @@ -24189,6 +24190,8 @@ CREATE INDEX index_users_on_feed_token ON users USING btree (feed_token); CREATE INDEX index_users_on_group_view ON users USING btree (group_view); +CREATE INDEX index_users_on_id_and_last_activity_on_for_non_internal_active ON users USING btree (id, last_activity_on) WHERE (((state)::text = 'active'::text) AND ((user_type IS NULL) OR (user_type = ANY (ARRAY[NULL::integer, 6, 4])))); + CREATE INDEX index_users_on_incoming_email_token ON users USING btree (incoming_email_token); CREATE INDEX index_users_on_managing_group_id ON users USING btree (managing_group_id); diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 393fd53193b358f56c55178c6b3ffe15046a8f6c..f81b04b99638dfe69b59a7d6fdc0445843f235bd 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -5615,4 +5615,47 @@ def access_levels(groups) end end end + + describe '.dormant' do + it 'returns dormant users' do + freeze_time do + not_that_long_ago = (described_class::MINIMUM_INACTIVE_DAYS - 1).days.ago.to_date + too_long_ago = described_class::MINIMUM_INACTIVE_DAYS.days.ago.to_date + + create(:user, :deactivated, last_activity_on: too_long_ago) + + User::INTERNAL_USER_TYPES.map do |user_type| + create(:user, state: :active, user_type: user_type, last_activity_on: too_long_ago) + end + + create(:user, last_activity_on: not_that_long_ago) + + dormant_user = create(:user, last_activity_on: too_long_ago) + + expect(described_class.dormant).to contain_exactly(dormant_user) + end + end + end + + describe '.with_no_activity' do + it 'returns users with no activity' do + freeze_time do + not_that_long_ago = (described_class::MINIMUM_INACTIVE_DAYS - 1).days.ago.to_date + too_long_ago = described_class::MINIMUM_INACTIVE_DAYS.days.ago.to_date + + create(:user, :deactivated, last_activity_on: nil) + + User::INTERNAL_USER_TYPES.map do |user_type| + create(:user, state: :active, user_type: user_type, last_activity_on: nil) + end + + create(:user, last_activity_on: not_that_long_ago) + create(:user, last_activity_on: too_long_ago) + + user_with_no_activity = create(:user, last_activity_on: nil) + + expect(described_class.with_no_activity).to contain_exactly(user_with_no_activity) + end + end + end end diff --git a/spec/workers/users/deactivate_dormant_users_worker_spec.rb b/spec/workers/users/deactivate_dormant_users_worker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..32291a143ee13e1762a7ee9f9ecda572dfae54f1 --- /dev/null +++ b/spec/workers/users/deactivate_dormant_users_worker_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::DeactivateDormantUsersWorker do + describe '#perform' do + subject(:worker) { described_class.new } + + it 'does not run for GitLab.com' do + create(:user, last_activity_on: User::MINIMUM_INACTIVE_DAYS.days.ago.to_date) + create(:user, last_activity_on: nil) + + expect(Gitlab).to receive(:com?).and_return(true) + expect(Gitlab::CurrentSettings).not_to receive(:current_application_settings) + + worker.perform + + expect(User.dormant.count).to eq(1) + expect(User.with_no_activity.count).to eq(1) + end + + context 'when automatic deactivation of dormant users is enabled' do + before do + stub_application_setting(deactivate_dormant_users: true) + end + + it 'deactivates dormant users' do + freeze_time do + stub_const("#{described_class.name}::BATCH_SIZE", 1) + stub_const("#{described_class.name}::PAUSE_SECONDS", 0) + + create(:user, last_activity_on: User::MINIMUM_INACTIVE_DAYS.days.ago.to_date) + create(:user, last_activity_on: nil) + + expect(worker).to receive(:sleep).twice + + worker.perform + + expect(User.dormant.count).to eq(0) + expect(User.with_no_activity.count).to eq(0) + end + end + end + + context 'when automatic deactivation of dormant users is disabled' do + before do + stub_application_setting(deactivate_dormant_users: false) + end + + it 'does nothing' do + create(:user, last_activity_on: User::MINIMUM_INACTIVE_DAYS.days.ago.to_date) + create(:user, last_activity_on: nil) + + worker.perform + + expect(User.dormant.count).to eq(1) + expect(User.with_no_activity.count).to eq(1) + end + end + end +end