diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 5cc0c8f3970a077342ad640d089791bae898710f..1a57d27127176cce41ec721b9fecde22251900a3 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -105,7 +105,7 @@ def deactivate return redirect_back_or_admin_user(notice: _("Error occurred. A blocked user cannot be deactivated")) if user.blocked? return redirect_back_or_admin_user(notice: _("Successfully deactivated")) if user.deactivated? return redirect_back_or_admin_user(notice: _("Internal users cannot be deactivated")) if user.internal? - return redirect_back_or_admin_user(notice: _("The user you are trying to deactivate has been active in the past %{minimum_inactive_days} days and cannot be deactivated") % { minimum_inactive_days: ::User::MINIMUM_INACTIVE_DAYS }) unless user.can_be_deactivated? + return redirect_back_or_admin_user(notice: _("The user you are trying to deactivate has been active in the past %{minimum_inactive_days} days and cannot be deactivated") % { minimum_inactive_days: Gitlab::CurrentSettings.deactivate_dormant_users_period }) unless user.can_be_deactivated? user.deactivate redirect_back_or_admin_user(notice: _("Successfully deactivated")) diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 321a6e9395e1e1e0748d6408e4e8559bc59d1d60..ba8909fa93a7daa260c85ef5db3956136b9f16a5 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -436,7 +436,10 @@ def visible_attributes :project_runner_token_expiration_interval, :pipeline_limit_per_project_user_sha ].tap do |settings| - settings << :deactivate_dormant_users unless Gitlab.com? + next if Gitlab.com? + + settings << :deactivate_dormant_users + settings << :deactivate_dormant_users_period end end diff --git a/app/models/user.rb b/app/models/user.rb index afee2d708441624df39fd592a27ab60458698540..2a766e3e9964adb3da925488544b10689913d600 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -92,7 +92,6 @@ class User < ApplicationRecord include ForcedEmailConfirmation include RequireEmailVerification - MINIMUM_INACTIVE_DAYS = 90 MINIMUM_DAYS_CREATED = 7 # Override Devise::Models::Trackable#update_tracked_fields! @@ -488,7 +487,7 @@ def blocked? scope :order_oldest_sign_in, -> { reorder(arel_table[:current_sign_in_at].asc.nulls_last) } scope :order_recent_last_activity, -> { reorder(arel_table[:last_activity_on].desc.nulls_last, arel_table[:id].asc) } scope :order_oldest_last_activity, -> { reorder(arel_table[:last_activity_on].asc.nulls_first, arel_table[:id].desc) } - scope :dormant, -> { with_state(:active).human_or_service_user.where('last_activity_on <= ?', MINIMUM_INACTIVE_DAYS.day.ago.to_date) } + scope :dormant, -> { with_state(:active).human_or_service_user.where('last_activity_on <= ?', Gitlab::CurrentSettings.deactivate_dormant_users_period.day.ago.to_date) } scope :with_no_activity, -> { with_state(:active).human_or_service_user.where(last_activity_on: nil).where('created_at <= ?', MINIMUM_DAYS_CREATED.day.ago.to_date) } scope :by_provider_and_extern_uid, ->(provider, extern_uid) { joins(:identities).merge(Identity.with_extern_uid(provider, extern_uid)) } scope :by_ids_or_usernames, -> (ids, usernames) { where(username: usernames).or(where(id: ids)) } @@ -2325,7 +2324,7 @@ def groups_with_developer_maintainer_project_access end def no_recent_activity? - last_active_at.to_i <= MINIMUM_INACTIVE_DAYS.days.ago.to_i + last_active_at.to_i <= Gitlab::CurrentSettings.deactivate_dormant_users_period.days.ago.to_i end def update_highest_role? diff --git a/app/views/admin/application_settings/_account_and_limit.html.haml b/app/views/admin/application_settings/_account_and_limit.html.haml index f914de138a9074266220225318c5628f1e80cd50..3e0f1dc0a1ce640ce37dd38a107563d6b167df01 100644 --- a/app/views/admin/application_settings/_account_and_limit.html.haml +++ b/app/views/admin/application_settings/_account_and_limit.html.haml @@ -52,7 +52,13 @@ = f.label :deactivate_dormant_users, _('Dormant users'), class: 'label-bold' - dormant_users_help_link = help_page_path('user/admin_area/moderate_users', anchor: 'automatically-deactivate-dormant-users') - dormant_users_help_link_start = ''.html_safe % { url: dormant_users_help_link } - = f.gitlab_ui_checkbox_component :deactivate_dormant_users, _('Deactivate dormant users after 90 days of inactivity'), help_text: _('Users can reactivate their account by signing in. %{link_start}Learn more%{link_end}').html_safe % { link_start: dormant_users_help_link_start, link_end: ''.html_safe } + = f.gitlab_ui_checkbox_component :deactivate_dormant_users, _('Deactivate dormant users after a period of inactivity'), help_text: _('Users can reactivate their account by signing in. %{link_start}Learn more%{link_end}').html_safe % { link_start: dormant_users_help_link_start, link_end: ''.html_safe } + .form-group + = f.label :deactivate_dormant_users_period, _('Period of inactivity (days)'), class: 'label-light' + = f.number_field :deactivate_dormant_users_period, class: 'form-control gl-form-input', min: '1' + .form-text.text-muted + = _('Period of inactivity before deactivation') + .form-group = f.label :personal_access_token_prefix, _('Personal Access Token prefix'), class: 'label-light' = f.text_field :personal_access_token_prefix, placeholder: _('Maximum 20 characters'), class: 'form-control gl-form-input' diff --git a/app/views/admin/application_settings/general.html.haml b/app/views/admin/application_settings/general.html.haml index 7535980151bb3b079ebf140298209be940dbb7f8..cd63873a8937b705f9530b874b10d2a026986ad1 100644 --- a/app/views/admin/application_settings/general.html.haml +++ b/app/views/admin/application_settings/general.html.haml @@ -13,7 +13,7 @@ .settings-content = render 'visibility_and_access' -%section.settings.as-account-limit.no-animate#js-account-settings{ class: ('expanded' if expanded_by_default?), data: { qa_selector: 'account_and_limit_settings_content' } } +%section.settings.as-account-limit.no-animate#js-account-settings{ class: ('expanded' if expanded_by_default?), data: { qa_selector: 'account_and_limit_settings_content', testid: 'account-limit' } } .settings-header %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only = _('Account and limit') diff --git a/doc/user/admin_area/moderate_users.md b/doc/user/admin_area/moderate_users.md index ab581cd3aa8da41e6b8f908560306bdeb80b7b86..bdb2fb30b06d6b29150d4fc76cd8cd4f87218be5 100644 --- a/doc/user/admin_area/moderate_users.md +++ b/doc/user/admin_area/moderate_users.md @@ -169,12 +169,13 @@ Users can also be deactivated using the [GitLab API](../../api/users.md#deactiva ### Automatically deactivate dormant users -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/320875) in GitLab 14.0. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/320875) in GitLab 14.0. +> - Customizable time period [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/336747) in GitLab 15.4 Administrators can enable automatic deactivation of users who either: - Were created more than a week ago and have not signed in. -- Have no activity in the last 90 days. +- Have no activity for a specified period of time (defaults to 90 days). To do this: @@ -182,6 +183,7 @@ To do this: 1. On the left sidebar, select **Settings > General**. 1. Expand the **Account and limit** section. 1. Under **Dormant users**, check **Deactivate dormant users after 90 days of inactivity**. +1. Under **Period of inactivity (days)**, enter a period of time before deactivation. 1. Select **Save changes**. When this feature is enabled, GitLab runs a job once a day to deactivate the dormant users. diff --git a/lib/api/users.rb b/lib/api/users.rb index c93c0f601a02074a804fe1b2fe6d45da442a0a06..adaafa08aeaeb9f49ea9184a4433f35a24ca3150 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -733,7 +733,7 @@ def reorder_users(users) unless user.can_be_deactivated? forbidden!('A blocked user cannot be deactivated by the API') if user.blocked? forbidden!('An internal user cannot be deactivated by the API') if user.internal? - forbidden!("The user you are trying to deactivate has been active in the past #{::User::MINIMUM_INACTIVE_DAYS} days and cannot be deactivated") + forbidden!("The user you are trying to deactivate has been active in the past #{Gitlab::CurrentSettings.deactivate_dormant_users_period} days and cannot be deactivated") end if user.deactivate diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4b825afa2ec3e3f5d26813220dcdd2a5306cec4f..875dcc6b2c8dce3a2ee263223d747fcbd4c8a625 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -12291,7 +12291,7 @@ msgstr "" msgid "Days to merge" msgstr "" -msgid "Deactivate dormant users after 90 days of inactivity" +msgid "Deactivate dormant users after a period of inactivity" msgstr "" msgid "Dear Administrator," @@ -28459,6 +28459,12 @@ msgstr "" msgid "Period in seconds" msgstr "" +msgid "Period of inactivity (days)" +msgstr "" + +msgid "Period of inactivity before deactivation" +msgstr "" + msgid "Permalink" msgstr "" diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 515ad9daf36ab5450533b8db7e43e3a3ebd3ddac..bd70787e19f4a0e2be3f0a18c207012ca6558d0a 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -270,19 +270,19 @@ let(:user) { create(:user, **activity) } context 'with no recent activity' do - let(:activity) { { last_activity_on: ::User::MINIMUM_INACTIVE_DAYS.next.days.ago } } + let(:activity) { { last_activity_on: Gitlab::CurrentSettings.deactivate_dormant_users_period.next.days.ago } } it_behaves_like 'a request that deactivates the user' end context 'with recent activity' do - let(:activity) { { last_activity_on: ::User::MINIMUM_INACTIVE_DAYS.pred.days.ago } } + let(:activity) { { last_activity_on: Gitlab::CurrentSettings.deactivate_dormant_users_period.pred.days.ago } } it 'does not deactivate the user' do put :deactivate, params: { id: user.username } user.reload expect(user.deactivated?).to be_falsey - expect(flash[:notice]).to eq("The user you are trying to deactivate has been active in the past #{::User::MINIMUM_INACTIVE_DAYS} days and cannot be deactivated") + expect(flash[:notice]).to eq("The user you are trying to deactivate has been active in the past #{Gitlab::CurrentSettings.deactivate_dormant_users_period} days and cannot be deactivated") end end end diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index 8843e13026b71abbefa88b7669b11d106ed67a5e..0f5d9892e668af959f5376eeaff5ea221aaa8055 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -102,7 +102,7 @@ end it 'change Account and Limit Settings' do - page.within('.as-account-limit') do + page.within(find('[data-testid="account-limit"]')) do uncheck 'Gravatar enabled' click_button 'Save changes' end @@ -112,7 +112,7 @@ end it 'change Maximum export size' do - page.within('.as-account-limit') do + page.within(find('[data-testid="account-limit"]')) do fill_in 'Maximum export size (MB)', with: 25 click_button 'Save changes' end @@ -122,7 +122,7 @@ end it 'change Maximum import size' do - page.within('.as-account-limit') do + page.within(find('[data-testid="account-limit"]')) do fill_in 'Maximum import size (MB)', with: 15 click_button 'Save changes' end @@ -150,16 +150,20 @@ it 'does not expose the setting' do expect(page).to have_no_selector('#application_setting_deactivate_dormant_users') end + + it 'does not expose the setting' do + expect(page).to have_no_selector('#application_setting_deactivate_dormant_users_period') + end end context 'when not Gitlab.com' do let(:dot_com?) { false } - it 'change Dormant users' do - expect(page).to have_unchecked_field('Deactivate dormant users after 90 days of inactivity') + it 'changes Dormant users' do + expect(page).to have_unchecked_field('Deactivate dormant users after a period of inactivity') expect(current_settings.deactivate_dormant_users).to be_falsey - page.within('.as-account-limit') do + page.within(find('[data-testid="account-limit"]')) do check 'application_setting_deactivate_dormant_users' click_button 'Save changes' end @@ -169,7 +173,22 @@ page.refresh expect(current_settings.deactivate_dormant_users).to be_truthy - expect(page).to have_checked_field('Deactivate dormant users after 90 days of inactivity') + expect(page).to have_checked_field('Deactivate dormant users after a period of inactivity') + end + + it 'change Dormant users period' do + expect(page).to have_field _('Period of inactivity (days)') + + page.within(find('[data-testid="account-limit"]')) do + fill_in _('application_setting_deactivate_dormant_users_period'), with: '35' + click_button 'Save changes' + end + + expect(page).to have_content "Application settings saved successfully" + + page.refresh + + expect(page).to have_field _('Period of inactivity (days)'), with: '35' end end end diff --git a/spec/helpers/application_settings_helper_spec.rb b/spec/helpers/application_settings_helper_spec.rb index 0304aac18aefcf548298544bedb8947ffedd2011..ba6caf7cf173d36fe7f54dd545a1f4637b15efaf 100644 --- a/spec/helpers/application_settings_helper_spec.rb +++ b/spec/helpers/application_settings_helper_spec.rb @@ -46,6 +46,10 @@ expect(helper.visible_attributes).to include(:deactivate_dormant_users) end + it 'contains :deactivate_dormant_users_period' do + expect(helper.visible_attributes).to include(:deactivate_dormant_users_period) + end + it 'contains rate limit parameters' do expect(helper.visible_attributes).to include(*%i( issues_create_limit notes_create_limit project_export_limit @@ -63,6 +67,10 @@ it 'does not contain :deactivate_dormant_users' do expect(helper.visible_attributes).not_to include(:deactivate_dormant_users) end + + it 'does not contain :deactivate_dormant_users_period' do + expect(helper.visible_attributes).not_to include(:deactivate_dormant_users_period) + end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 801cc586b7620275edc82fb0a7bdf09ed451f51e..d2aa2a63c1b7b0a183505244bb2ab6ea69d0a11a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -3810,8 +3810,8 @@ def login_method(login) describe '#can_be_deactivated?' do let(:activity) { {} } let(:user) { create(:user, name: 'John Smith', **activity) } - let(:day_within_minium_inactive_days_threshold) { User::MINIMUM_INACTIVE_DAYS.pred.days.ago } - let(:day_outside_minium_inactive_days_threshold) { User::MINIMUM_INACTIVE_DAYS.next.days.ago } + let(:day_within_minium_inactive_days_threshold) { Gitlab::CurrentSettings.deactivate_dormant_users_period.pred.days.ago } + let(:day_outside_minium_inactive_days_threshold) { Gitlab::CurrentSettings.deactivate_dormant_users_period.next.days.ago } shared_examples 'not eligible for deactivation' do it 'returns false' do @@ -7193,8 +7193,8 @@ def have_dismissed_callout 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 + not_that_long_ago = (Gitlab::CurrentSettings.deactivate_dormant_users_period - 1).days.ago.to_date + too_long_ago = Gitlab::CurrentSettings.deactivate_dormant_users_period.days.ago.to_date create(:user, :deactivated, last_activity_on: too_long_ago) @@ -7214,8 +7214,8 @@ def have_dismissed_callout describe '.with_no_activity' do it 'returns users with no activity' do freeze_time do - active_not_that_long_ago = (described_class::MINIMUM_INACTIVE_DAYS - 1).days.ago.to_date - active_too_long_ago = described_class::MINIMUM_INACTIVE_DAYS.days.ago.to_date + active_not_that_long_ago = (Gitlab::CurrentSettings.deactivate_dormant_users_period - 1).days.ago.to_date + active_too_long_ago = Gitlab::CurrentSettings.deactivate_dormant_users_period.days.ago.to_date created_recently = (described_class::MINIMUM_DAYS_CREATED - 1).days.ago.to_date created_not_recently = described_class::MINIMUM_DAYS_CREATED.days.ago.to_date diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 26238a87209d5053adaa68dd8d75d9490e0397d3..331d8261dcfa8f4ecb006214f0a00e8b366ff33a 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -3238,7 +3238,7 @@ def update_password(user, admin, password = User.random_password) let(:user) { create(:user, **activity) } context 'with no recent activity' do - let(:activity) { { last_activity_on: ::User::MINIMUM_INACTIVE_DAYS.next.days.ago } } + let(:activity) { { last_activity_on: Gitlab::CurrentSettings.deactivate_dormant_users_period.next.days.ago } } it 'deactivates an active user' do deactivate @@ -3249,13 +3249,13 @@ def update_password(user, admin, password = User.random_password) end context 'with recent activity' do - let(:activity) { { last_activity_on: ::User::MINIMUM_INACTIVE_DAYS.pred.days.ago } } + let(:activity) { { last_activity_on: Gitlab::CurrentSettings.deactivate_dormant_users_period.pred.days.ago } } it 'does not deactivate an active user' do deactivate expect(response).to have_gitlab_http_status(:forbidden) - expect(json_response['message']).to eq("403 Forbidden - The user you are trying to deactivate has been active in the past #{::User::MINIMUM_INACTIVE_DAYS} days and cannot be deactivated") + expect(json_response['message']).to eq("403 Forbidden - The user you are trying to deactivate has been active in the past #{Gitlab::CurrentSettings.deactivate_dormant_users_period} days and cannot be deactivated") expect(user.reload.state).to eq('active') end end diff --git a/spec/workers/users/deactivate_dormant_users_worker_spec.rb b/spec/workers/users/deactivate_dormant_users_worker_spec.rb index 263ca31e0a0ee8f7f1c960c760ac9afce5df3b76..a8318de669bbf97792734cfc459a4a7ddf144cbe 100644 --- a/spec/workers/users/deactivate_dormant_users_worker_spec.rb +++ b/spec/workers/users/deactivate_dormant_users_worker_spec.rb @@ -6,7 +6,7 @@ using RSpec::Parameterized::TableSyntax describe '#perform' do - let_it_be(:dormant) { create(:user, last_activity_on: User::MINIMUM_INACTIVE_DAYS.days.ago.to_date) } + let_it_be(:dormant) { create(:user, last_activity_on: Gitlab::CurrentSettings.deactivate_dormant_users_period.days.ago.to_date) } let_it_be(:inactive) { create(:user, last_activity_on: nil, created_at: User::MINIMUM_DAYS_CREATED.days.ago.to_date) } let_it_be(:inactive_recently_created) { create(:user, last_activity_on: nil, created_at: (User::MINIMUM_DAYS_CREATED - 1).days.ago.to_date) } @@ -14,7 +14,7 @@ it 'does not run for GitLab.com' do expect(Gitlab).to receive(:com?).and_return(true) - expect(Gitlab::CurrentSettings).not_to receive(:current_application_settings) + # Now makes a call to current settings to determine period of dormancy worker.perform @@ -48,7 +48,7 @@ end with_them do it 'deactivates certain user types' do - user = create(:user, user_type: user_type, state: :active, last_activity_on: User::MINIMUM_INACTIVE_DAYS.days.ago.to_date) + user = create(:user, user_type: user_type, state: :active, last_activity_on: Gitlab::CurrentSettings.deactivate_dormant_users_period.days.ago.to_date) worker.perform @@ -57,8 +57,8 @@ end it 'does not deactivate non-active users' do - human_user = create(:user, user_type: :human, state: :blocked, last_activity_on: User::MINIMUM_INACTIVE_DAYS.days.ago.to_date) - service_user = create(:user, user_type: :service_user, state: :blocked, last_activity_on: User::MINIMUM_INACTIVE_DAYS.days.ago.to_date) + human_user = create(:user, user_type: :human, state: :blocked, last_activity_on: Gitlab::CurrentSettings.deactivate_dormant_users_period.days.ago.to_date) + service_user = create(:user, user_type: :service_user, state: :blocked, last_activity_on: Gitlab::CurrentSettings.deactivate_dormant_users_period.days.ago.to_date) worker.perform