From 8ba84a8bced0ceda917352ad3a6b4f3fc5cab687 Mon Sep 17 00:00:00 2001 From: smriti Date: Wed, 8 Jan 2025 17:56:01 +0530 Subject: [PATCH 01/13] Changes for executing webhook for 30D and 60D notification email --- .../groups/settings/_permissions.html.haml | 2 + .../personal_access_tokens/expiring_worker.rb | 9 +- ...y_webhook_execute_to_namespace_settings.rb | 13 ++ ...iry_webhook_execute_to_project_settings.rb | 9 + db/schema_migrations/20250108062227 | 1 + db/schema_migrations/20250108062256 | 1 + db/structure.sql | 8 +- doc/user/group/manage.md | 17 ++ .../controllers/concerns/ee/groups/params.rb | 6 + ee/app/models/ee/group.rb | 7 + ee/app/models/ee/namespace.rb | 6 + ...nded_grat_expiry_webhook_execute.html.haml | 10 ++ ...ended_expiry_webhook_execution_setting.yml | 9 + ee/spec/models/ee/group_spec.rb | 166 ++++++++++++++++++ .../data_builder/resource_access_token.rb | 3 +- locale/gitlab.pot | 9 + .../expiring_worker_spec.rb | 23 +++ 17 files changed, 292 insertions(+), 7 deletions(-) create mode 100644 db/migrate/20250108062227_add_extended_grat_expiry_webhook_execute_to_namespace_settings.rb create mode 100644 db/migrate/20250108062256_add_extended_prat_expiry_webhook_execute_to_project_settings.rb create mode 100644 db/schema_migrations/20250108062227 create mode 100644 db/schema_migrations/20250108062256 create mode 100644 ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml create mode 100644 ee/config/feature_flags/gitlab_com_derisk/extended_expiry_webhook_execution_setting.yml diff --git a/app/views/groups/settings/_permissions.html.haml b/app/views/groups/settings/_permissions.html.haml index 56db71a5c3b394..3c3a3b0119770c 100644 --- a/app/views/groups/settings/_permissions.html.haml +++ b/app/views/groups/settings/_permissions.html.haml @@ -39,6 +39,8 @@ = render_if_exists 'groups/settings/prevent_forking', f: f, group: @group = render_if_exists 'groups/settings/service_access_tokens_expiration_enforced', f: f, group: @group = render_if_exists 'groups/settings/enforce_ssh_certificates', f: f, group: @group + - if Feature.enabled?(:extended_expiry_webhook_execution_setting, @group) + = render_if_exists 'groups/settings/extended_grat_expiry_webhook_execute', f: f, group: @group = render 'groups/settings/two_factor_auth', f: f, group: @group = render_if_exists 'groups/personal_access_token_expiration_policy', f: f, group: @group = render 'groups/settings/membership', f: f, group: @group diff --git a/app/workers/personal_access_tokens/expiring_worker.rb b/app/workers/personal_access_tokens/expiring_worker.rb index c6d48a3a8dfcf9..30d174b7a17249 100644 --- a/app/workers/personal_access_tokens/expiring_worker.rb +++ b/app/workers/personal_access_tokens/expiring_worker.rb @@ -131,7 +131,10 @@ def process_project_bot_tokens(interval = :seven_days) # webhooks do not include information about when the token expires, so # only trigger on seven_days interval to avoid changing existing behavior - execute_web_hooks(project_bot, expiring_user_token) if interval == :seven_days + if Feature.enabled?(:extended_expiry_webhook_execution_setting, + project_bot.bot_namespace) || interval == :seven_days + execute_web_hooks(project_bot, expiring_user_token, interval) + end interval_days = PersonalAccessToken.notification_interval(interval) deliver_bot_notifications(project_bot, expiring_user_token.name, days_to_expire: interval_days) @@ -204,13 +207,13 @@ def log_exception(ex, user) ) end - def execute_web_hooks(bot_user, token) + def execute_web_hooks(bot_user, token, interval) resource = bot_user.resource_bot_resource return unless resource return if resource.is_a?(Project) && !resource.has_active_hooks?(:resource_access_token_hooks) - hook_data = Gitlab::DataBuilder::ResourceAccessToken.build(token, :expiring, resource) + hook_data = Gitlab::DataBuilder::ResourceAccessToken.build(token, :expiring, resource, interval) resource.execute_hooks(hook_data, :resource_access_token_hooks) end diff --git a/db/migrate/20250108062227_add_extended_grat_expiry_webhook_execute_to_namespace_settings.rb b/db/migrate/20250108062227_add_extended_grat_expiry_webhook_execute_to_namespace_settings.rb new file mode 100644 index 00000000000000..15e19643af5c7a --- /dev/null +++ b/db/migrate/20250108062227_add_extended_grat_expiry_webhook_execute_to_namespace_settings.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddExtendedGratExpiryWebhookExecuteToNamespaceSettings < Gitlab::Database::Migration[2.2] + enable_lock_retries! + milestone '17.8' + + def change + add_column :namespace_settings, :extended_grat_expiry_webhooks_execute, :boolean, default: false, null: false + end +end diff --git a/db/migrate/20250108062256_add_extended_prat_expiry_webhook_execute_to_project_settings.rb b/db/migrate/20250108062256_add_extended_prat_expiry_webhook_execute_to_project_settings.rb new file mode 100644 index 00000000000000..2573abea5cb6c6 --- /dev/null +++ b/db/migrate/20250108062256_add_extended_prat_expiry_webhook_execute_to_project_settings.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddExtendedPratExpiryWebhookExecuteToProjectSettings < Gitlab::Database::Migration[2.2] + milestone '17.8' + + def change + add_column :project_settings, :extended_prat_expiry_webhooks_execute, :boolean, default: false, null: false + end +end diff --git a/db/schema_migrations/20250108062227 b/db/schema_migrations/20250108062227 new file mode 100644 index 00000000000000..f1db3e2b49a7c0 --- /dev/null +++ b/db/schema_migrations/20250108062227 @@ -0,0 +1 @@ +d221b3a52a8446515e264adc177c912a39deb8633e57de13addbc778f965ae19 \ No newline at end of file diff --git a/db/schema_migrations/20250108062256 b/db/schema_migrations/20250108062256 new file mode 100644 index 00000000000000..89024924e2d5d4 --- /dev/null +++ b/db/schema_migrations/20250108062256 @@ -0,0 +1 @@ +1a5a6cff5bd53bff910fd918c1580ce455b55a4f9ba5c16e071bb70ee1cde6cd \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 6d18836c937ae3..6bbe233bd19656 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -12748,8 +12748,7 @@ CREATE TABLE deploy_tokens ( creator_id bigint, read_virtual_registry boolean DEFAULT false NOT NULL, project_id bigint, - group_id bigint, - write_virtual_registry boolean DEFAULT false NOT NULL + group_id bigint ); CREATE SEQUENCE deploy_tokens_id_seq @@ -16885,6 +16884,7 @@ CREATE TABLE namespace_settings ( resource_access_token_notify_inherited boolean, lock_resource_access_token_notify_inherited boolean DEFAULT false NOT NULL, pipeline_variables_default_role smallint DEFAULT 2 NOT NULL, + extended_grat_expiry_webhooks_execute boolean DEFAULT false NOT NULL, force_pages_access_control boolean DEFAULT false NOT NULL, CONSTRAINT check_0ba93c78c7 CHECK ((char_length(default_branch_name) <= 255)), CONSTRAINT namespace_settings_unique_project_download_limit_alertlist_size CHECK ((cardinality(unique_project_download_limit_alertlist) <= 100)), @@ -18528,7 +18528,8 @@ CREATE TABLE personal_access_tokens ( thirty_days_notification_sent_at timestamp with time zone, sixty_days_notification_sent_at timestamp with time zone, description text, - CONSTRAINT check_6d2ddc9355 CHECK ((char_length(description) <= 255)) + CONSTRAINT check_6d2ddc9355 CHECK ((char_length(description) <= 255)), + CONSTRAINT check_aa95773861 CHECK ((char_length(advanced_scopes) <= 4096)) ); CREATE SEQUENCE personal_access_tokens_id_seq @@ -19926,6 +19927,7 @@ CREATE TABLE project_settings ( spp_repository_pipeline_access boolean, max_number_of_vulnerabilities integer, pages_primary_domain text, + extended_prat_expiry_webhooks_execute boolean DEFAULT false NOT NULL, CONSTRAINT check_1a30456322 CHECK ((char_length(pages_unique_domain) <= 63)), CONSTRAINT check_3a03e7557a CHECK ((char_length(previous_default_branch) <= 4096)), CONSTRAINT check_3ca5cbffe6 CHECK ((char_length(issue_branch_template) <= 255)), diff --git a/doc/user/group/manage.md b/doc/user/group/manage.md index b47a9353733264..1ce3a910ee4b16 100644 --- a/doc/user/group/manage.md +++ b/doc/user/group/manage.md @@ -200,6 +200,23 @@ For more information, see: - For groups, the [group access tokens documentation](settings/group_access_tokens.md#group-access-token-expiry-emails). - For projects, the [project access tokens documentation](../project/settings/project_access_tokens.md#project-access-token-expiry-emails). +## Extended personal access tokens expiry webhook execution + +> - Webhook execution on the events of expiry notification emails being sent at 30 days and 60 days interval [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/463016) in GitLab 17.9 [with a flag](../../administration/feature_flags.md) named `pat_expiry_inherited_members_notification`. Disabled by default. + +FLAG: +The webhook execution on extended expiry intervals for group access tokens is controlled by a feature flag. For more information, see the history. + +By default [Project and group access tokens webhook](../project/integrations/webhook_events.md#project-and-group-access-token-events) are executed whenever [Access token expiry email](../group/settings/group_access_tokens.md#group-access-token-expiry-emails) is sent when its 7 days remaining for the token to expire. With this setting enabled webhook will execute even if email is sent for 30 days and 60 days interval along with 7 days interval. + +You can enable execution of webhooks for 30 days and 60 days intervals too: + +1. On the left sidebar, select **Search or go to** and find your group. +1. Select **Settings > General**. +1. Expand the **Permissions and group features** section. +1. Select the **Extended Group Access Tokens Expiry Webhook execution** checkbox. +1. Select **Save changes**. + ## Disable group mentions You can prevent users from being added to a conversation and getting notified when diff --git a/ee/app/controllers/concerns/ee/groups/params.rb b/ee/app/controllers/concerns/ee/groups/params.rb index 32ede0b7204f3a..89fcd7d7b227bd 100644 --- a/ee/app/controllers/concerns/ee/groups/params.rb +++ b/ee/app/controllers/concerns/ee/groups/params.rb @@ -79,6 +79,12 @@ def group_params_ee if current_group&.can_manage_extensions_marketplace_for_enterprise_users? params_ee << :enterprise_users_extensions_marketplace_enabled end + + if ::Feature.enabled?(:extended_expiry_webhook_execution_setting, current_group) && + can?(current_user, :admin_group, current_group) && + current_group.licensed_feature_available?(:group_webhooks) + params_ee << :extended_grat_expiry_webhooks_execute + end end end # rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/AbcSize diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index 13de06a8d0d8fc..a50e1687fe0e50 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -831,7 +831,14 @@ def execute_hooks(data, hooks_scope) return unless feature_available?(:group_webhooks) self_and_ancestor_hooks = GroupHook.where(group_id: self_and_ancestors) + self_and_ancestor_hooks.hooks_for(hooks_scope).each do |hook| + if ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self) && hooks_scope == :resource_access_token_hooks + extended_expiry_interval = [:thirty_days, :sixty_days] + + next if extended_expiry_interval.include?(data[:interval]) && !hook.group.extended_grat_expiry_webhooks_execute? + end + hook.async_execute(data, hooks_scope.to_s) end end diff --git a/ee/app/models/ee/namespace.rb b/ee/app/models/ee/namespace.rb index 9db1f706478f21..bdac7c2cfe9a16 100644 --- a/ee/app/models/ee/namespace.rb +++ b/ee/app/models/ee/namespace.rb @@ -212,6 +212,8 @@ module Namespace :early_access_program_participant, :enforce_ssh_certificates, :enforce_ssh_certificates=, + :extended_grat_expiry_webhooks_execute, + :extended_grat_expiry_webhooks_execute=, to: :namespace_settings, allow_nil: true ) @@ -619,6 +621,10 @@ def enforce_ssh_certificates? root? && namespace_settings&.enforce_ssh_certificates? end + def extended_grat_expiry_webhooks_execute? + licensed_feature_available?(:group_webhooks) && namespace_settings&.extended_grat_expiry_webhooks_execute? + end + def ssh_certificates_available? root? && licensed_feature_available?(:ssh_certificates) end diff --git a/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml b/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml new file mode 100644 index 00000000000000..9863f154cc486a --- /dev/null +++ b/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml @@ -0,0 +1,10 @@ +- if group.licensed_feature_available?(:group_webhooks) + %h5= s_('GroupSettings|Extended Group Access Tokens Expiry Webhook execution') + + .form-group.gl-mb-3 + = f.gitlab_ui_checkbox_component :extended_grat_expiry_webhooks_execute, checkbox_options: { disabled: !can?(current_user, :admin_group, group), checked: group.extended_grat_expiry_webhooks_execute? } do |c| + - c.with_label do + = s_('GroupSettings|Extended Group Access Tokens Expiry Webhook execution') + - c.with_help_text do + = s_("GroupSettings|If enabled, Group Acces Tokens Expiry Webhooks will be executed on 30 days and 60 days notification delivery as well along with 7 days notification delivery.") + = link_to s_('GroupSettings|Expiry Notifications for Group access tokens'), help_page_path('user/group/manage.md', anchor: 'extended-personal-access-tokens-expiry-webhook-execution'), class: 'gl-font-sm', target: :_blank diff --git a/ee/config/feature_flags/gitlab_com_derisk/extended_expiry_webhook_execution_setting.yml b/ee/config/feature_flags/gitlab_com_derisk/extended_expiry_webhook_execution_setting.yml new file mode 100644 index 00000000000000..5d73d12d5808a4 --- /dev/null +++ b/ee/config/feature_flags/gitlab_com_derisk/extended_expiry_webhook_execution_setting.yml @@ -0,0 +1,9 @@ +--- +name: extended_expiry_webhook_execution_setting +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/499732 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/178266 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/513684 +milestone: '17.9' +group: group::authentication +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb index c6d3c33890f0d2..bf96db8617eb38 100644 --- a/ee/spec/models/ee/group_spec.rb +++ b/ee/spec/models/ee/group_spec.rb @@ -2943,6 +2943,172 @@ def webhook_headers end end + context 'when resource access token hooks' do + let(:group) { create(:group) } + let(:group_hook) { create(:group_hook, group: group, resource_access_token_events: true) } + + before do + stub_licensed_features(group_webhooks: true) + end + + context 'when interval is seven days' do + let(:data) { { interval: :seven_days } } + + it 'executes webhook' do + expect(WebHookService).to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + + group.execute_hooks(data, :resource_access_token_hooks) + end + end + + context 'when feature flag is disabled' do + let(:data) { { interval: :thirty_days } } + + before do + stub_feature_flags(extended_expiry_webhook_execution_setting: false) + end + + it 'executes webhook even when setting is disabled' do + group.namespace_settings.update!(extended_grat_expiry_webhooks_execute: false) + + expect(WebHookService).to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + + group.execute_hooks(data, :resource_access_token_hooks) + end + end + + context 'when setting extended_grat_expiry_webhooks_execute is disabled' do + before do + group.namespace_settings.update!(extended_grat_expiry_webhooks_execute: false) + end + + context 'when interval is thirty days' do + let(:data) { { interval: :thirty_days } } + + it 'does not execute the hook' do + expect(WebHookService).not_to receive(:new) + + group.execute_hooks(data, :resource_access_token_hooks) + end + end + + context 'when interval is sixty days' do + let(:data) { { interval: :sixty_days } } + + it 'does not execute the hook' do + expect(WebHookService).not_to receive(:new) + + group.execute_hooks(data, :resource_access_token_hooks) + end + end + end + + context 'when setting extended_grat_expiry_webhooks_execute is enabled' do + before do + group.namespace_settings.update!(extended_grat_expiry_webhooks_execute: true) + end + + context 'when interval is thirty days' do + let(:data) { { interval: :thirty_days } } + + it 'executes webhook' do + expect(WebHookService).to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + + group.execute_hooks(data, :resource_access_token_hooks) + end + end + + context 'when interval is sixty days' do + let(:data) { { interval: :sixty_days } } + + it 'executes webhook' do + expect(WebHookService).to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + + group.execute_hooks(data, :resource_access_token_hooks) + end + end + end + + context 'when group has subgroup with same webhook configured' do + let(:subgroup) { create(:group, parent: group) } + let(:subgroup_hook) { create(:group_hook, group: subgroup, resource_access_token_events: true) } + let(:data) { { interval: :thirty_days } } + + context 'when setting extended_grat_expiry_webhooks_execute is disabled for parent group' do + before do + group.namespace_settings.update!(extended_grat_expiry_webhooks_execute: false) + end + + context 'when subgroup setting is enabled' do + before do + subgroup.namespace_settings.update!(extended_grat_expiry_webhooks_execute: true) + end + + it 'executes webhook for subgroup and not parent group' do + expect(WebHookService).to receive(:new) + .with(subgroup_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + expect(WebHookService).not_to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + + subgroup.execute_hooks(data, :resource_access_token_hooks) + end + end + + context 'when subgroup setting is disabled' do + before do + subgroup.namespace_settings.update!(extended_grat_expiry_webhooks_execute: false) + end + + it 'does not execute webhook for subgroup and not parent group' do + expect(WebHookService).not_to receive(:new) + + subgroup.execute_hooks(data, :resource_access_token_hooks) + end + end + end + + context 'when setting extended_grat_expiry_webhooks_execute is enabled for parent group' do + before do + group.namespace_settings.update!(extended_grat_expiry_webhooks_execute: true) + end + + context 'when subgroup setting is enabled' do + before do + subgroup.namespace_settings.update!(extended_grat_expiry_webhooks_execute: true) + end + + it 'executes webhook for subgroup and not parent group' do + expect(WebHookService).to receive(:new) + .with(subgroup_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + expect(WebHookService).to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + + subgroup.execute_hooks(data, :resource_access_token_hooks) + end + end + + context 'when subgroup setting is disabled' do + before do + subgroup.namespace_settings.update!(extended_grat_expiry_webhooks_execute: false) + end + + it 'does not execute webhook for subgroup and not parent group' do + expect(WebHookService).not_to receive(:new) + .with(subgroup_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + expect(WebHookService).to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + + subgroup.execute_hooks(data, :resource_access_token_hooks) + end + end + end + end + end + describe '#self_or_ancestor_marked_for_deletion' do context 'delayed deletion feature is not available' do before do diff --git a/lib/gitlab/data_builder/resource_access_token.rb b/lib/gitlab/data_builder/resource_access_token.rb index eba5156658ef9f..68e2e5909961f2 100644 --- a/lib/gitlab/data_builder/resource_access_token.rb +++ b/lib/gitlab/data_builder/resource_access_token.rb @@ -5,7 +5,7 @@ module DataBuilder module ResourceAccessToken extend self - def build(resource_access_token, event, resource) + def build(resource_access_token, event, resource, interval = :seven_days) base_data = { object_kind: 'access_token' } @@ -13,6 +13,7 @@ def build(resource_access_token, event, resource) base_data[resource.model_name.param_key.to_sym] = resource.hook_attrs base_data[:object_attributes] = resource_access_token.hook_attrs base_data[:event_name] = event_data(event) + base_data[:interval] = interval base_data end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 2c64ef65576c40..cd8ac49c87bd80 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -28033,9 +28033,15 @@ msgstr "" msgid "GroupSettings|Experiment" msgstr "" +msgid "GroupSettings|Expiry Notifications for Group access tokens" +msgstr "" + msgid "GroupSettings|Export group" msgstr "" +msgid "GroupSettings|Extended Group Access Tokens Expiry Webhook execution" +msgstr "" + msgid "GroupSettings|Failed to save changes." msgstr "" @@ -28057,6 +28063,9 @@ msgstr "" msgid "GroupSettings|How do I manage group SSH certificates?" msgstr "" +msgid "GroupSettings|If enabled, Group Acces Tokens Expiry Webhooks will be executed on 30 days and 60 days notification delivery as well along with 7 days notification delivery." +msgstr "" + msgid "GroupSettings|If enabled, enterprise user accounts will not be able to use personal access tokens." msgstr "" diff --git a/spec/workers/personal_access_tokens/expiring_worker_spec.rb b/spec/workers/personal_access_tokens/expiring_worker_spec.rb index db623b8b7daa18..b55798bd4d77a9 100644 --- a/spec/workers/personal_access_tokens/expiring_worker_spec.rb +++ b/spec/workers/personal_access_tokens/expiring_worker_spec.rb @@ -266,6 +266,29 @@ worker.perform end + context 'when feature flag extended_expiry_webhook_execution_setting is disabled' do + before do + stub_feature_flags(extended_expiry_webhook_execution_setting: false) + end + + it "does not call execute_web_hooks for interval 30 days" do + expiring_token.update!(expires_at: 30.days.from_now) + project_hook = create(:project_hook, project: project, resource_access_token_events: true) + + expect(Gitlab::DataBuilder::ResourceAccessToken).not_to receive(:build) + expect(WebHookService) + .not_to receive(:new) + .with( + project_hook, + {}, + 'resource_access_token_hooks', + idempotency_key: anything + ) { fake_wh_service } + + worker.perform + end + end + context 'with multiple batches of tokens' do let_it_be(:expiring_tokens) { create_list(:resource_access_token, 4, expires_at: 6.days.from_now) } -- GitLab From c22e65973e93dff761f5543b3c9f62d1c116fd0c Mon Sep 17 00:00:00 2001 From: smriti Date: Wed, 22 Jan 2025 13:45:37 +0530 Subject: [PATCH 02/13] Changes for project settings Specs finalized --- .../permissions/components/settings_panel.vue | 39 ++++- app/controllers/projects_controller.rb | 10 +- app/helpers/projects_helper.rb | 1 + app/models/project.rb | 5 + app/models/projects/triggered_hooks.rb | 8 + .../personal_access_tokens/expiring_worker.rb | 2 +- ...ended_expiry_webhook_execution_setting.yml | 0 ...y_webhook_execute_to_namespace_settings.rb | 5 +- ...iry_webhook_execute_to_project_settings.rb | 3 +- doc/user/project/settings/_index.md | 17 ++ ee/app/models/ee/group.rb | 9 +- ...nded_grat_expiry_webhook_execute.html.haml | 19 +- ee/spec/models/ee/namespace_spec.rb | 44 +++++ ee/spec/requests/groups_controller_spec.rb | 37 ++++ .../settings/_permissions.html.haml_spec.rb | 46 +++++ locale/gitlab.pot | 6 + spec/controllers/projects_controller_spec.rb | 26 ++- spec/helpers/projects_helper_spec.rb | 1 + .../resource_access_token_spec.rb | 4 +- spec/models/project_spec.rb | 8 + spec/models/projects/triggered_hooks_spec.rb | 164 +++++++++++++++++- spec/requests/api/project_attributes.yml | 1 + .../expiring_worker_spec.rb | 4 +- 23 files changed, 427 insertions(+), 32 deletions(-) rename {ee/config => config}/feature_flags/gitlab_com_derisk/extended_expiry_webhook_execution_setting.yml (100%) diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue index 4b5cbc6dd38ee8..fcef380b41b26b 100644 --- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue +++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue @@ -11,6 +11,7 @@ import { import SecretManagerSettings from 'ee_component/pages/projects/shared/permissions/components/secret_manager_settings.vue'; import ConfirmDanger from '~/vue_shared/components/confirm_danger/confirm_danger.vue'; import settingsMixin from 'ee_else_ce/pages/projects/shared/permissions/mixins/settings_pannel_mixin'; +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { __, s__ } from '~/locale'; import { VISIBILITY_LEVEL_PRIVATE_INTEGER, @@ -30,6 +31,7 @@ import { duoHelpPath, amazonQHelpPath, pipelineExecutionPoliciesHelpPath, + extendedPratExpiryWebhooksExecuteHelpPath, } from '../constants'; import { toggleHiddenClassBySelector } from '../external'; import ProjectFeatureSetting from './project_feature_setting.vue'; @@ -95,6 +97,12 @@ export default { ), confirmButtonText: __('Save changes'), emailsLabel: s__('ProjectSettings|Email notifications'), + extendedPratExpiryWebhooksExecuteLabel: s__( + 'ProjectSettings|Add additional webhook triggers for group access token expiry.', + ), + extendedPratExpiryWebhooksExecuteHelpText: s__( + 'ProjectSettings|If enabled, project access tokens expiry webhooks execute 60, 30, and 7 days before the token expires. If disabled, these webhooks only execute 7 days before the token expires. %{linkStart}Learn more%{linkEnd}', + ), showDiffPreviewLabel: s__('ProjectSettings|Include diff previews'), showDiffPreviewHelpText: s__( 'ProjectSettings|Emails are not encrypted. Concerned administrators may want to disable diff previews.', @@ -113,6 +121,7 @@ export default { modelExperimentsHelpPath, modelRegistryHelpPath, pipelineExecutionPoliciesHelpPath, + extendedPratExpiryWebhooksExecuteHelpPath, components: { CiCatalogSettings, ProjectFeatureSetting, @@ -132,7 +141,7 @@ export default { 'jh_component/pages/projects/shared/permissions/components/other_project_settings.vue' ), }, - mixins: [settingsMixin], + mixins: [settingsMixin, glFeatureFlagMixin()], inject: ['cascadingSettingsData'], props: { requestCveAvailable: { @@ -346,6 +355,7 @@ export default { enforceAuthChecksOnUploads: true, emailsEnabled: true, showDiffPreviewInEmail: true, + extendedPratExpiryWebhooksExecute: false, cveIdRequestEnabled: true, duoFeaturesEnabled: false, sppRepositoryPipelineAccess: false, @@ -1115,6 +1125,31 @@ export default { + + + + {{ $options.i18n.extendedPratExpiryWebhooksExecuteLabel }} + + + @@ -1163,4 +1198,4 @@ export default { - + \ No newline at end of file diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 08c6dc3f9a02a3..1b69f0808cb750 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -54,6 +54,7 @@ class ProjectsController < Projects::ApplicationController push_force_frontend_feature_flag(:work_items, @project&.work_items_feature_flag_enabled?) push_force_frontend_feature_flag(:work_items_beta, @project&.work_items_beta_feature_flag_enabled?) push_force_frontend_feature_flag(:work_items_alpha, @project&.work_items_alpha_feature_flag_enabled?) + push_frontend_feature_flag(:extended_expiry_webhook_execution_setting, @project&.group) push_frontend_feature_flag(:work_item_description_templates, @project&.group) end @@ -460,7 +461,7 @@ def project_feature_attributes end def project_setting_attributes - %i[ + attributes = %i[ show_default_award_emojis show_diff_preview_in_email squash_option @@ -469,6 +470,13 @@ def project_setting_attributes enforce_auth_checks_on_uploads emails_enabled ] + + if ::Feature.enabled?(:extended_expiry_webhook_execution_setting, project) && + can?(current_user, :admin_project, project) + attributes << :extended_prat_expiry_webhooks_execute + end + + attributes end def project_params_attributes diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index e019d78fe1d045..e6134d729ed310 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -949,6 +949,7 @@ def project_permissions_settings(project) containerRegistryEnabled: !!project.container_registry_enabled, lfsEnabled: !!project.lfs_enabled, emailsEnabled: project.emails_enabled?, + extendedPratExpiryWebhooksExecute: project.extended_prat_expiry_webhooks_execute?, showDiffPreviewInEmail: project.show_diff_preview_in_email?, monitorAccessLevel: feature.monitor_access_level, showDefaultAwardEmojis: project.show_default_award_emojis?, diff --git a/app/models/project.rb b/app/models/project.rb index cbace08c55fd60..b46159d27b84a6 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -580,6 +580,7 @@ def self.integration_association_name(name) delegate :mr_default_target_self, :mr_default_target_self= delegate :previous_default_branch, :previous_default_branch= delegate :squash_option, :squash_option= + delegate :extended_prat_expiry_webhooks_execute, :extended_prat_expiry_webhooks_execute= with_options allow_nil: true do delegate :merge_commit_template, :merge_commit_template= @@ -1235,6 +1236,10 @@ def warn_about_potentially_unwanted_characters? !!project_setting&.warn_about_potentially_unwanted_characters? end + def extended_prat_expiry_webhooks_execute? + !!project_setting&.extended_prat_expiry_webhooks_execute? + end + def no_import? !!import_state&.no_import? end diff --git a/app/models/projects/triggered_hooks.rb b/app/models/projects/triggered_hooks.rb index e3aa3d106b7300..77fa75c21ea043 100644 --- a/app/models/projects/triggered_hooks.rb +++ b/app/models/projects/triggered_hooks.rb @@ -17,6 +17,14 @@ def execute # Assumes that the relations implement TriggerableHooks @relations.each do |hooks| hooks.hooks_for(@scope).select_active(@scope, @data).each do |hook| + if @scope == :resource_access_token_hooks && @data[:interval] != :seven_days + if hook.is_a?(GroupHook) && ::Feature.enabled?(:extended_expiry_webhook_execution_setting, hook.group) + next unless hook.group.extended_grat_expiry_webhooks_execute? + elsif ::Feature.enabled?(:extended_expiry_webhook_execution_setting, hook.project) + next unless hook.project.extended_prat_expiry_webhooks_execute? + end + end + hook.async_execute(@data, @scope.to_s) end end diff --git a/app/workers/personal_access_tokens/expiring_worker.rb b/app/workers/personal_access_tokens/expiring_worker.rb index 30d174b7a17249..3aedf48aee3315 100644 --- a/app/workers/personal_access_tokens/expiring_worker.rb +++ b/app/workers/personal_access_tokens/expiring_worker.rb @@ -132,7 +132,7 @@ def process_project_bot_tokens(interval = :seven_days) # webhooks do not include information about when the token expires, so # only trigger on seven_days interval to avoid changing existing behavior if Feature.enabled?(:extended_expiry_webhook_execution_setting, - project_bot.bot_namespace) || interval == :seven_days + project_bot.resource_bot_resource) || interval == :seven_days execute_web_hooks(project_bot, expiring_user_token, interval) end diff --git a/ee/config/feature_flags/gitlab_com_derisk/extended_expiry_webhook_execution_setting.yml b/config/feature_flags/gitlab_com_derisk/extended_expiry_webhook_execution_setting.yml similarity index 100% rename from ee/config/feature_flags/gitlab_com_derisk/extended_expiry_webhook_execution_setting.yml rename to config/feature_flags/gitlab_com_derisk/extended_expiry_webhook_execution_setting.yml diff --git a/db/migrate/20250108062227_add_extended_grat_expiry_webhook_execute_to_namespace_settings.rb b/db/migrate/20250108062227_add_extended_grat_expiry_webhook_execute_to_namespace_settings.rb index 15e19643af5c7a..2dcde52a094fb1 100644 --- a/db/migrate/20250108062227_add_extended_grat_expiry_webhook_execute_to_namespace_settings.rb +++ b/db/migrate/20250108062227_add_extended_grat_expiry_webhook_execute_to_namespace_settings.rb @@ -1,11 +1,8 @@ # frozen_string_literal: true -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - class AddExtendedGratExpiryWebhookExecuteToNamespaceSettings < Gitlab::Database::Migration[2.2] enable_lock_retries! - milestone '17.8' + milestone '17.9' def change add_column :namespace_settings, :extended_grat_expiry_webhooks_execute, :boolean, default: false, null: false diff --git a/db/migrate/20250108062256_add_extended_prat_expiry_webhook_execute_to_project_settings.rb b/db/migrate/20250108062256_add_extended_prat_expiry_webhook_execute_to_project_settings.rb index 2573abea5cb6c6..f739f87b16df88 100644 --- a/db/migrate/20250108062256_add_extended_prat_expiry_webhook_execute_to_project_settings.rb +++ b/db/migrate/20250108062256_add_extended_prat_expiry_webhook_execute_to_project_settings.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true class AddExtendedPratExpiryWebhookExecuteToProjectSettings < Gitlab::Database::Migration[2.2] - milestone '17.8' + enable_lock_retries! + milestone '17.9' def change add_column :project_settings, :extended_prat_expiry_webhooks_execute, :boolean, default: false, null: false diff --git a/doc/user/project/settings/_index.md b/doc/user/project/settings/_index.md index e2a15f69b2f8ec..114ceaaaf440ef 100644 --- a/doc/user/project/settings/_index.md +++ b/doc/user/project/settings/_index.md @@ -151,3 +151,20 @@ To set this default: 1. Select **Settings > Merge requests**. 1. Select **Enable "Delete source branch" option by default**. 1. Select **Save changes**. + +## Extended project access tokens expiry webhook execution + +> - Webhook execution on the events of expiry notification emails being sent at 30 days and 60 days interval [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/463016) in GitLab 17.9 [with a flag](../../../administration/feature_flags.md) named `extended_expiry_webhook_execution_setting`. Disabled by default. + +FLAG: +The webhook execution on extended expiry intervals for project access tokens is controlled by a feature flag. For more information, see the history. + +By default [Project and group access tokens webhook](../integrations/webhook_events.md#project-and-group-access-token-events) are executed whenever [Access token expiry email](project_access_tokens.md#project-access-token-expiry-emails) is sent when its 7 days remaining for the token to expire. With this setting enabled webhook will execute even if email is sent for 30 days and 60 days interval along with 7 days interval. + +You can enable execution of webhooks for 30 days and 60 days intervals too: + +1. On the left sidebar, select **Search or go to** and find your project. +1. Select **Settings > General**. +1. Expand the **Visibility, project features, permissions** section. +1. Select the **Extended Group Access Tokens Expiry Webhook execution** checkbox. +1. Select **Save changes**. diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index a50e1687fe0e50..c83e5a62c24280 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -831,12 +831,11 @@ def execute_hooks(data, hooks_scope) return unless feature_available?(:group_webhooks) self_and_ancestor_hooks = GroupHook.where(group_id: self_and_ancestors) - self_and_ancestor_hooks.hooks_for(hooks_scope).each do |hook| - if ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self) && hooks_scope == :resource_access_token_hooks - extended_expiry_interval = [:thirty_days, :sixty_days] - - next if extended_expiry_interval.include?(data[:interval]) && !hook.group.extended_grat_expiry_webhooks_execute? + if ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self) && + hooks_scope == :resource_access_token_hooks && + (data[:interval] != :seven_days && !hook.group.extended_grat_expiry_webhooks_execute?) + next end hook.async_execute(data, hooks_scope.to_s) diff --git a/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml b/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml index 9863f154cc486a..925950d5e9a150 100644 --- a/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml +++ b/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml @@ -1,10 +1,11 @@ -- if group.licensed_feature_available?(:group_webhooks) - %h5= s_('GroupSettings|Extended Group Access Tokens Expiry Webhook execution') +- return unless group.licensed_feature_available?(:group_webhooks) - .form-group.gl-mb-3 - = f.gitlab_ui_checkbox_component :extended_grat_expiry_webhooks_execute, checkbox_options: { disabled: !can?(current_user, :admin_group, group), checked: group.extended_grat_expiry_webhooks_execute? } do |c| - - c.with_label do - = s_('GroupSettings|Extended Group Access Tokens Expiry Webhook execution') - - c.with_help_text do - = s_("GroupSettings|If enabled, Group Acces Tokens Expiry Webhooks will be executed on 30 days and 60 days notification delivery as well along with 7 days notification delivery.") - = link_to s_('GroupSettings|Expiry Notifications for Group access tokens'), help_page_path('user/group/manage.md', anchor: 'extended-personal-access-tokens-expiry-webhook-execution'), class: 'gl-font-sm', target: :_blank +%h5= s_('GroupSettings|Extended Group Access Tokens Expiry Webhook execution') + +.form-group.gl-mb-3 + = f.gitlab_ui_checkbox_component :extended_grat_expiry_webhooks_execute, checkbox_options: { disabled: !can?(current_user, :admin_group, group), checked: group.extended_grat_expiry_webhooks_execute? } do |c| + - c.with_label do + = s_('GroupSettings|Extended Group Access Tokens Expiry Webhook execution') + - c.with_help_text do + = s_("GroupSettings|If enabled, Group Acces Tokens Expiry Webhooks will be executed on 30 days and 60 days notification delivery as well along with 7 days notification delivery.") + = link_to s_('GroupSettings|Expiry Notifications for Group access tokens'), help_page_path('user/group/manage.md', anchor: 'extended-personal-access-tokens-expiry-webhook-execution'), class: 'gl-font-sm', target: :_blank diff --git a/ee/spec/models/ee/namespace_spec.rb b/ee/spec/models/ee/namespace_spec.rb index 0fe874042462e8..a1718e0ca3a1cf 100644 --- a/ee/spec/models/ee/namespace_spec.rb +++ b/ee/spec/models/ee/namespace_spec.rb @@ -2195,6 +2195,50 @@ end end + describe '#extended_grat_expiry_webhooks_execute?' do + let(:namespace) { create(:group) } + + context 'when extended_grat_expiry_webhooks_execute = true' do + before do + namespace.extended_grat_expiry_webhooks_execute = true + end + + context 'when licensed feature is available' do + before do + stub_licensed_features(group_webhooks: true) + end + + it 'delegates the field to namespace settings and return true' do + expect(namespace.namespace_settings).to receive(:extended_grat_expiry_webhooks_execute?).and_call_original + + expect(namespace.extended_grat_expiry_webhooks_execute?).to be_truthy + end + end + + it 'returns false if licensed feature is not available' do + expect(namespace.extended_grat_expiry_webhooks_execute?).to be_truthy + end + end + + context 'when extended_grat_expiry_webhooks_execute=false' do + before do + namespace.extended_grat_expiry_webhooks_execute = false + end + + context 'when licensed feature is available' do + before do + stub_licensed_features(group_webhooks: true) + end + + it 'delegates the field to namespace settings and returns false' do + expect(namespace.namespace_settings).to receive(:extended_grat_expiry_webhooks_execute?).and_call_original + + expect(namespace.extended_grat_expiry_webhooks_execute?).to be_falsey + end + end + end + end + describe '#ssh_certificates_available?' do let(:namespace) { create(:group) } diff --git a/ee/spec/requests/groups_controller_spec.rb b/ee/spec/requests/groups_controller_spec.rb index 8cf05bdd762925..b4769698f47924 100644 --- a/ee/spec/requests/groups_controller_spec.rb +++ b/ee/spec/requests/groups_controller_spec.rb @@ -363,6 +363,43 @@ end end + context 'when settings extended_grat_expiry_webhooks_execute' do + let(:params) { { group: { extended_grat_expiry_webhooks_execute: true } } } + + context 'when licensed feature is not available' do + before do + stub_licensed_features(group_webhooks: false) + end + + it 'does not change the column' do + expect { subject }.not_to change { group.reload.extended_grat_expiry_webhooks_execute? } + expect(response).to have_gitlab_http_status(:found) + end + end + + context 'when licensed feature is available' do + before do + stub_licensed_features(group_webhooks: true) + end + + it 'successfully changes the column' do + expect { subject }.to change { group.reload.extended_grat_expiry_webhooks_execute? } + expect(response).to have_gitlab_http_status(:found) + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(extended_expiry_webhook_execution_setting: false) + end + + it 'does not change the column' do + expect { subject }.not_to change { group.reload.extended_grat_expiry_webhooks_execute? } + expect(response).to have_gitlab_http_status(:found) + end + end + end + end + context 'settings enterprise_users_extensions_marketplace_enabled' do let(:params) { { group: { enterprise_users_extensions_marketplace_enabled: true } } } diff --git a/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb b/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb index bb444dc6b91e6f..93bb38e5dcfd11 100644 --- a/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb +++ b/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb @@ -80,6 +80,52 @@ end end + context 'for extended token expiry webhook execution setting' do + let_it_be(:checkbox_label) { s_('GroupSettings|Extended Group Access Tokens Expiry Webhook execution') } + + context 'when licensed feature is not available' do + before do + allow(group).to receive(:licensed_feature_available?).and_return(false) + allow(group).to receive(:licensed_feature_available?).with(:group_webhooks).and_return(false) + end + + it 'renders nothing', :aggregate_failures do + render + + expect(rendered).to render_template('groups/settings/_extended_grat_expiry_webhook_execute') + expect(rendered).not_to have_content('Extended Group Access Tokens Expiry Webhook execution') + end + end + + context 'when licensed feature is available' do + before do + allow(group).to receive(:licensed_feature_available?).and_return(false) + allow(group).to receive(:licensed_feature_available?).with(:group_webhooks).and_return(true) + end + + it 'renders checkbox', :aggregate_failures do + render + + expect(rendered).to render_template('groups/settings/_extended_grat_expiry_webhook_execute') + expect(rendered).to have_content('Extended Group Access Tokens Expiry Webhook execution') + expect(rendered).to have_unchecked_field(checkbox_label, type: 'checkbox') + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(extended_expiry_webhook_execution_setting: false) + end + + it 'renders nothing', :aggregate_failures do + render + + expect(rendered).not_to render_template('groups/settings/_extended_grat_expiry_webhook_execute') + expect(rendered).not_to have_content('Extended Group Access Tokens Expiry Webhook execution') + end + end + end + end + context 'for extensions marketplace settings' do let_it_be(:section_title) { _('Web IDE and workspaces') } let_it_be(:checkbox_label) { s_('GroupSettings|Enable extension marketplace') } diff --git a/locale/gitlab.pot b/locale/gitlab.pot index cd8ac49c87bd80..4630881ef16816 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -45198,6 +45198,9 @@ msgstr "" msgid "ProjectSettings|Existing merge requests and protected branches are not affected." msgstr "" +msgid "ProjectSettings|Extended Project Access Tokens Expiry Webhook execution" +msgstr "" + msgid "ProjectSettings|Failed to protect the tag" msgstr "" @@ -45249,6 +45252,9 @@ msgstr "" msgid "ProjectSettings|If GitLab manages your cluster, then GitLab retains your analytics data for 1 year. %{link_start}Learn more about data retention policy%{link_end}." msgstr "" +msgid "ProjectSettings|If enabled, Project Acces Tokens Expiry Webhooks will be executed on 30 days and 60 days notification delivery as well along with 7 days notification delivery." +msgstr "" + msgid "ProjectSettings|If merge trains are enabled, merging is only possible if the branch can be rebased without conflicts." msgstr "" diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 757f85df70ec46..b57687137dc3c5 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -939,7 +939,8 @@ def update_project(**parameters) project_setting_attributes: { show_default_award_emojis: boolean_value, enforce_auth_checks_on_uploads: boolean_value, - emails_enabled: boolean_value + emails_enabled: boolean_value, + extended_prat_expiry_webhooks_execute: boolean_value } } } @@ -950,6 +951,29 @@ def update_project(**parameters) expect(project.enforce_auth_checks_on_uploads?).to eq(result) expect(project.emails_enabled?).to eq(result) expect(project.emails_disabled?).to eq(!result) + expect(project.extended_prat_expiry_webhooks_execute?).to eq(result) + end + + context 'when extended_expiry_webhook_execution_setting feature flag is false' do + before do + stub_feature_flags(extended_expiry_webhook_execution_setting: false) + end + + it "does not update extended_expiry_webhook_execution_setting" do + put :update, params: { + namespace_id: project.namespace, + id: project.path, + project: { + project_setting_attributes: { + extended_prat_expiry_webhooks_execute: boolean_value + } + } + } + + project.reload + + expect(project.extended_prat_expiry_webhooks_execute?).to be false + end end end end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index d18f086b440847..5fc3947cf7cbb9 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -995,6 +995,7 @@ def license_name containerRegistryEnabled: !!project.container_registry_enabled, lfsEnabled: !!project.lfs_enabled, emailsEnabled: project.emails_enabled?, + extendedPratExpiryWebhooksExecute: project.extended_prat_expiry_webhooks_execute?, showDefaultAwardEmojis: project.show_default_award_emojis?, securityAndComplianceAccessLevel: project.security_and_compliance_access_level, containerRegistryAccessLevel: project.project_feature.container_registry_access_level, diff --git a/spec/lib/gitlab/data_builder/resource_access_token_spec.rb b/spec/lib/gitlab/data_builder/resource_access_token_spec.rb index 05518cd403fe84..1b1d2000e07f87 100644 --- a/spec/lib/gitlab/data_builder/resource_access_token_spec.rb +++ b/spec/lib/gitlab/data_builder/resource_access_token_spec.rb @@ -5,12 +5,14 @@ RSpec.describe Gitlab::DataBuilder::ResourceAccessToken, feature_category: :system_access do let_it_be_with_reload(:user) { create(:user, :project_bot) } let(:event) { :expiring } + let(:interval) { :thirty_days } let(:personal_access_token) { create(:personal_access_token, user: user) } - let(:data) { described_class.build(personal_access_token, event, resource) } + let(:data) { described_class.build(personal_access_token, event, resource, interval) } shared_examples 'includes standard data' do specify do expect(data[:object_attributes]).to eq(personal_access_token.hook_attrs) + expect(data[:interval]).to eq(interval) expect(data[:object_kind]).to eq('access_token') end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index f239adfa392696..fed975391b1059 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -4380,6 +4380,14 @@ def has_external_wiki end end + describe '#extended_prat_expiry_webhooks_execute?' do + let(:project) { build(:project, extended_prat_expiry_webhooks_execute: true) } + + it "is the value of extended_prat_expiry_webhooks_execute" do + expect(project.extended_prat_expiry_webhooks_execute?).to eq(true) + end + end + describe '#lfs_enabled?' do let(:namespace) { create(:namespace) } let(:project) { build(:project, namespace: namespace) } diff --git a/spec/models/projects/triggered_hooks_spec.rb b/spec/models/projects/triggered_hooks_spec.rb index 51cf211beb0ec9..5bcd6737c1cd4a 100644 --- a/spec/models/projects/triggered_hooks_spec.rb +++ b/spec/models/projects/triggered_hooks_spec.rb @@ -12,9 +12,10 @@ let(:wh_service) { instance_double(::WebHookService, async_execute: true) } let(:data) { { some: 'data', as: 'json' } } - def run_hooks(scope, data) + def run_hooks(scope, data, group = nil) hooks = described_class.new(scope, data) hooks.add_hooks(ProjectHook.all) + hooks.add_hooks(GroupHook.where(group_id: group.self_and_ancestors)) if group hooks.execute end @@ -42,12 +43,165 @@ def run_hooks(scope, data) end context 'with access token hooks' do - let_it_be(:resource_access_token_hook) { create(:project_hook, project: project, resource_access_token_events: true) } + let(:project) { create(:project) } + let(:project_hook) { create(:project_hook, project: project, resource_access_token_events: true) } - it 'executes hook' do - expect_hook_execution(resource_access_token_hook, data, 'resource_access_token_hooks') + context 'when interval is seven days' do + let(:data) { { interval: :seven_days } } + + it 'executes webhook' do + expect_hook_execution(project_hook, data, 'resource_access_token_hooks') + + run_hooks(:resource_access_token_hooks, data) + end + end + + context 'when feature flag is disabled' do + let(:data) { { interval: :thirty_days } } + + before do + stub_feature_flags(extended_expiry_webhook_execution_setting: false) + end + + it 'executes webhook even when setting is disabled' do + project.update!(extended_prat_expiry_webhooks_execute: false) + + expect_hook_execution(project_hook, data, 'resource_access_token_hooks') + + run_hooks(:resource_access_token_hooks, data) + end + end + + context 'when setting extended_prat_expiry_webhooks_execute is disabled' do + before do + project.update!(extended_prat_expiry_webhooks_execute: false) + end + + context 'when interval is thirty days' do + let(:data) { { interval: :thirty_days } } + + it 'does not execute the hook' do + expect(WebHookService).not_to receive(:new) + + run_hooks(:resource_access_token_hooks, data) + end + end + + context 'when interval is sixty days' do + let(:data) { { interval: :sixty_days } } + + it 'does not execute the hook' do + expect(WebHookService).not_to receive(:new) + + run_hooks(:resource_access_token_hooks, data) + end + end + end + + context 'when setting extended_prat_expiry_webhooks_execute is enabled' do + before do + project.update!(extended_prat_expiry_webhooks_execute: true) + end + + context 'when interval is thirty days' do + let(:data) { { interval: :thirty_days } } + + it 'executes webhook' do + expect_hook_execution(project_hook, data, 'resource_access_token_hooks') + + run_hooks(:resource_access_token_hooks, data) + end + end + + context 'when interval is sixty days' do + let(:data) { { interval: :sixty_days } } + + it 'executes webhook' do + expect_hook_execution(project_hook, data, 'resource_access_token_hooks') + + run_hooks(:resource_access_token_hooks, data) + end + end + end - run_hooks(:resource_access_token_hooks, data) + context 'when project belongs to a group with same webhook configured' do + let(:group) { create(:group) } + let(:group_hook) { create(:group_hook, group: group, resource_access_token_events: true) } + let(:data) { { interval: :thirty_days } } + + before do + stub_licensed_features(group_webhooks: true) + project.update!(namespace_id: group.id) + end + + context 'when setting extended_grat_expiry_webhooks_execute is disabled for parent group' do + before do + group.update!(extended_grat_expiry_webhooks_execute: false) + end + + context 'when project setting is enabled' do + before do + project.update!(extended_prat_expiry_webhooks_execute: true) + end + + it 'executes webhook for project and not parent group' do + expect(WebHookService).not_to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + expect(WebHookService).to receive(:new) + .with(project_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + + run_hooks(:resource_access_token_hooks, data, group) + end + end + + context 'when project setting is disabled' do + before do + project.update!(extended_prat_expiry_webhooks_execute: false) + end + + it 'does not execute webhook for subgroup and not parent group' do + expect(WebHookService).not_to receive(:new) + + run_hooks(:resource_access_token_hooks, data, group) + end + end + end + + context 'when setting extended_grat_expiry_webhooks_execute is enabled for parent group' do + before do + group.update!(extended_grat_expiry_webhooks_execute: true) + end + + context 'when project setting is enabled' do + before do + project.update!(extended_prat_expiry_webhooks_execute: true) + end + + it 'executes webhook for project and parent group' do + expect(WebHookService).to receive(:new) + .with(project_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + expect(WebHookService).to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + + run_hooks(:resource_access_token_hooks, data, group) + end + end + + context 'when project setting is disabled' do + before do + project.update!(extended_prat_expiry_webhooks_execute: false) + end + + it 'does not execute webhook for project but executes for parent group' do + expect(WebHookService).not_to receive(:new) + .with(project_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + expect(WebHookService).to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + + run_hooks(:resource_access_token_hooks, data, group) + end + end + end end end diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index b3b5a26e40e78b..cd571064dcb1a4 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -193,6 +193,7 @@ project_setting: - observability_alerts_enabled - spp_repository_pipeline_access - max_number_of_vulnerabilities + - extended_prat_expiry_webhooks_execute build_service_desk_setting: # service_desk_setting unexposed_attributes: diff --git a/spec/workers/personal_access_tokens/expiring_worker_spec.rb b/spec/workers/personal_access_tokens/expiring_worker_spec.rb index b55798bd4d77a9..d35a7413e7b274 100644 --- a/spec/workers/personal_access_tokens/expiring_worker_spec.rb +++ b/spec/workers/personal_access_tokens/expiring_worker_spec.rb @@ -248,7 +248,7 @@ let_it_be(:expiring_token) { create(:personal_access_token, user: project_bot, expires_at: 5.days.from_now) } it 'executes access token webhook' do - hook_data = {} + hook_data = { interval: :seven_days } project_hook = create(:project_hook, project: project, resource_access_token_events: true) expect(Gitlab::DataBuilder::ResourceAccessToken).to receive(:build).and_return(hook_data) @@ -258,7 +258,7 @@ .to receive(:new) .with( project_hook, - {}, + { interval: :seven_days }, 'resource_access_token_hooks', idempotency_key: anything ) { fake_wh_service } -- GitLab From 7a3d8e0a020862007de18ee741d89422cc347a27 Mon Sep 17 00:00:00 2001 From: Smriti Garg Date: Mon, 27 Jan 2025 11:56:43 +0000 Subject: [PATCH 03/13] Documentation suggestions applied Pipeline failure fixed Moved Project hook filtering to model --- app/controllers/projects_controller.rb | 1 + app/models/project.rb | 14 +- app/models/projects/triggered_hooks.rb | 11 +- .../personal_access_tokens/expiring_worker.rb | 4 +- db/structure.sql | 6 +- doc/user/group/manage.md | 8 +- doc/user/project/settings/_index.md | 8 +- ee/app/models/ee/group.rb | 34 +++- ee/app/models/ee/project.rb | 18 +- ...nded_grat_expiry_webhook_execute.html.haml | 5 +- .../namespace_setting_changes_auditor_spec.rb | 2 +- ee/spec/models/ee/group_spec.rb | 2 +- ee/spec/models/ee/project_spec.rb | 90 +++++++++- locale/gitlab.pot | 10 +- .../components/settings_panel_spec.js | 25 +++ spec/models/project_spec.rb | 84 ++++++++- spec/models/projects/triggered_hooks_spec.rb | 163 ------------------ 17 files changed, 270 insertions(+), 215 deletions(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 1b69f0808cb750..df23b4ac0a1a50 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -54,6 +54,7 @@ class ProjectsController < Projects::ApplicationController push_force_frontend_feature_flag(:work_items, @project&.work_items_feature_flag_enabled?) push_force_frontend_feature_flag(:work_items_beta, @project&.work_items_beta_feature_flag_enabled?) push_force_frontend_feature_flag(:work_items_alpha, @project&.work_items_alpha_feature_flag_enabled?) + # FF to enable setting to allow webhook execution on 30D and 60D notification delivery too push_frontend_feature_flag(:extended_expiry_webhook_execution_setting, @project&.group) push_frontend_feature_flag(:work_item_description_templates, @project&.group) end diff --git a/app/models/project.rb b/app/models/project.rb index b46159d27b84a6..8f27500c25cabf 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1983,7 +1983,19 @@ def execute_hooks(data, hooks_scope = :push_hooks) def triggered_hooks(hooks_scope, data) triggered = ::Projects::TriggeredHooks.new(hooks_scope, data) - triggered.add_hooks(hooks) + + # By default the webhook resource_access_token_hooks will execute for + # seven_days interval but we have a setting to allow webhook execution + # for thirty_days and sixty_days_plus interval too. + if hooks_scope == :resource_access_token_hooks && + ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self) && + data[:interval] != :seven_days && + !self.extended_prat_expiry_webhooks_execute? + + triggered + else + triggered.add_hooks(hooks) + end end def execute_integrations(data, hooks_scope = :push_hooks, skip_ci: false) diff --git a/app/models/projects/triggered_hooks.rb b/app/models/projects/triggered_hooks.rb index 77fa75c21ea043..ca5f9052ebcddf 100644 --- a/app/models/projects/triggered_hooks.rb +++ b/app/models/projects/triggered_hooks.rb @@ -15,16 +15,9 @@ def add_hooks(relation) def execute # Assumes that the relations implement TriggerableHooks - @relations.each do |hooks| - hooks.hooks_for(@scope).select_active(@scope, @data).each do |hook| - if @scope == :resource_access_token_hooks && @data[:interval] != :seven_days - if hook.is_a?(GroupHook) && ::Feature.enabled?(:extended_expiry_webhook_execution_setting, hook.group) - next unless hook.group.extended_grat_expiry_webhooks_execute? - elsif ::Feature.enabled?(:extended_expiry_webhook_execution_setting, hook.project) - next unless hook.project.extended_prat_expiry_webhooks_execute? - end - end + @relations.each do |hooks| + hooks&.hooks_for(@scope)&.select_active(@scope, @data)&.each do |hook| hook.async_execute(@data, @scope.to_s) end end diff --git a/app/workers/personal_access_tokens/expiring_worker.rb b/app/workers/personal_access_tokens/expiring_worker.rb index 3aedf48aee3315..5ca48a793f1b61 100644 --- a/app/workers/personal_access_tokens/expiring_worker.rb +++ b/app/workers/personal_access_tokens/expiring_worker.rb @@ -129,8 +129,8 @@ def process_project_bot_tokens(interval = :seven_days) # project bot does not have more than 1 token expiring_user_token = project_bot.personal_access_tokens.first - # webhooks do not include information about when the token expires, so - # only trigger on seven_days interval to avoid changing existing behavior + # If Feature flag is not enabled webhooks will only execute if + # interval is seven_days if Feature.enabled?(:extended_expiry_webhook_execution_setting, project_bot.resource_bot_resource) || interval == :seven_days execute_web_hooks(project_bot, expiring_user_token, interval) diff --git a/db/structure.sql b/db/structure.sql index 6bbe233bd19656..de02b8cd598af6 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -12748,7 +12748,8 @@ CREATE TABLE deploy_tokens ( creator_id bigint, read_virtual_registry boolean DEFAULT false NOT NULL, project_id bigint, - group_id bigint + group_id bigint, + write_virtual_registry boolean DEFAULT false NOT NULL ); CREATE SEQUENCE deploy_tokens_id_seq @@ -18528,8 +18529,7 @@ CREATE TABLE personal_access_tokens ( thirty_days_notification_sent_at timestamp with time zone, sixty_days_notification_sent_at timestamp with time zone, description text, - CONSTRAINT check_6d2ddc9355 CHECK ((char_length(description) <= 255)), - CONSTRAINT check_aa95773861 CHECK ((char_length(advanced_scopes) <= 4096)) + CONSTRAINT check_6d2ddc9355 CHECK ((char_length(description) <= 255)) ); CREATE SEQUENCE personal_access_tokens_id_seq diff --git a/doc/user/group/manage.md b/doc/user/group/manage.md index 1ce3a910ee4b16..3e39bba186191b 100644 --- a/doc/user/group/manage.md +++ b/doc/user/group/manage.md @@ -200,14 +200,14 @@ For more information, see: - For groups, the [group access tokens documentation](settings/group_access_tokens.md#group-access-token-expiry-emails). - For projects, the [project access tokens documentation](../project/settings/project_access_tokens.md#project-access-token-expiry-emails). -## Extended personal access tokens expiry webhook execution +## Add additional webhook triggers for group access token expiration -> - Webhook execution on the events of expiry notification emails being sent at 30 days and 60 days interval [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/463016) in GitLab 17.9 [with a flag](../../administration/feature_flags.md) named `pat_expiry_inherited_members_notification`. Disabled by default. +> - [Added](https://gitlab.com/gitlab-org/gitlab/-/issues/463016) 60 day and 30 days triggers to project and group access tokens webhooks in GitLab 17.9 [with a flag](../../administration/feature_flags.md) named `pat_expiry_inherited_members_notification`. Disabled by default. FLAG: -The webhook execution on extended expiry intervals for group access tokens is controlled by a feature flag. For more information, see the history. +The availability of this feature is controlled by a feature flag. For more information, see the history. -By default [Project and group access tokens webhook](../project/integrations/webhook_events.md#project-and-group-access-token-events) are executed whenever [Access token expiry email](../group/settings/group_access_tokens.md#group-access-token-expiry-emails) is sent when its 7 days remaining for the token to expire. With this setting enabled webhook will execute even if email is sent for 30 days and 60 days interval along with 7 days interval. +GitLab sends multiple [expiry emails](../group/settings/group_access_tokens.md#group-access-token-expiry-emails) and triggers a related [webhook](../project/integrations/webhook_events.md#project-and-group-access-token-events) before a group token expires. By default, GitLab only triggers these webhooks 7 days before the token expires. When this feature is enabled, GitLab also triggers these webhooks 60 days and 30 days before the token expires. You can enable execution of webhooks for 30 days and 60 days intervals too: diff --git a/doc/user/project/settings/_index.md b/doc/user/project/settings/_index.md index 114ceaaaf440ef..f11294e45c1b47 100644 --- a/doc/user/project/settings/_index.md +++ b/doc/user/project/settings/_index.md @@ -152,14 +152,14 @@ To set this default: 1. Select **Enable "Delete source branch" option by default**. 1. Select **Save changes**. -## Extended project access tokens expiry webhook execution +## Add additional webhook triggers for project access token expiration -> - Webhook execution on the events of expiry notification emails being sent at 30 days and 60 days interval [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/463016) in GitLab 17.9 [with a flag](../../../administration/feature_flags.md) named `extended_expiry_webhook_execution_setting`. Disabled by default. +> - [Added](https://gitlab.com/gitlab-org/gitlab/-/issues/463016) 60 day and 30 days triggers to project and group access tokens webhooks in GitLab 17.9 [with a flag](../../../administration/feature_flags.md) named `pat_expiry_inherited_members_notification`. Disabled by default. FLAG: -The webhook execution on extended expiry intervals for project access tokens is controlled by a feature flag. For more information, see the history. +The availability of this feature is controlled by a feature flag. For more information, see the history. -By default [Project and group access tokens webhook](../integrations/webhook_events.md#project-and-group-access-token-events) are executed whenever [Access token expiry email](project_access_tokens.md#project-access-token-expiry-emails) is sent when its 7 days remaining for the token to expire. With this setting enabled webhook will execute even if email is sent for 30 days and 60 days interval along with 7 days interval. +GitLab sends multiple [expiry emails](project_access_tokens.md#project-access-token-expiry-emails) and triggers a related [webhook](../integrations/webhook_events.md#project-and-group-access-token-events) before a project token expires. By default, GitLab only triggers these webhooks 7 days before the token expires. When this feature is enabled, GitLab also triggers these webhooks 60 days and 30 days before the token expires. You can enable execution of webhooks for 30 days and 60 days intervals too: diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index c83e5a62c24280..b0f9eb7c12e9f7 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -830,16 +830,18 @@ def execute_hooks(data, hooks_scope) return unless feature_available?(:group_webhooks) - self_and_ancestor_hooks = GroupHook.where(group_id: self_and_ancestors) - self_and_ancestor_hooks.hooks_for(hooks_scope).each do |hook| - if ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self) && - hooks_scope == :resource_access_token_hooks && - (data[:interval] != :seven_days && !hook.group.extended_grat_expiry_webhooks_execute?) - next - end + # By default the webhook resource_access_token_hooks will execute for + # seven_days interval but we have a setting to allow webhook execution + # for thirty_days and sixty_days_plus interval too. + group_hooks = if ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self) && + hooks_scope == :resource_access_token_hooks && + (data[:interval] != :seven_days) + GroupHook.where(group_id: groups_for_extended_webhook_execution_on_token_expiry) + else + GroupHook.where(group_id: self_and_ancestors) + end - hook.async_execute(data, hooks_scope.to_s) - end + execute_async_hooks(group_hooks, hooks_scope, data) end override :git_transfer_in_progress? @@ -1118,8 +1120,22 @@ def enable_auto_assign_gitlab_duo_pro_seats? namespace_settings.enable_auto_assign_gitlab_duo_pro_seats? if namespace_settings end + def groups_for_extended_webhook_execution_on_token_expiry + return unless ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self) + + self_and_ancestors + .joins(:namespace_settings) + .where(namespace_settings: { extended_grat_expiry_webhooks_execute: true }) + end + private + def execute_async_hooks(group_hooks, hooks_scope, data) + group_hooks.hooks_for(hooks_scope).each do |hook| + hook.async_execute(data, hooks_scope.to_s) + end + end + def active_project_tokens_of_root_ancestor root_ancestor_and_descendants_project_bots = ::User .joins(projects: :group) diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index d3448eca50ff28..7cf2d4dda45cf3 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -856,7 +856,17 @@ def execute_external_compliance_hooks(data) override :triggered_hooks def triggered_hooks(scope, data) triggered = super - triggered.add_hooks(group_hooks) if group && feature_available?(:group_webhooks) + + if group && feature_available?(:group_webhooks) + + if ::Feature.enabled?(:extended_expiry_webhook_execution_setting, group) && + scope == :resource_access_token_hooks && + data[:interval] != :seven_days + triggered.add_hooks(group_hooks_for_groups_with_extended_execution) + else + triggered.add_hooks(group_hooks) + end + end triggered end @@ -1519,6 +1529,12 @@ def group_hooks GroupHook.where(group_id: group.self_and_ancestors) end + def group_hooks_for_groups_with_extended_execution + return unless ::Feature.enabled?(:extended_expiry_webhook_execution_setting, group) + + GroupHook.where(group_id: group.groups_for_extended_webhook_execution_on_token_expiry) + end + def set_override_pull_mirror_available self.pull_mirror_available_overridden = read_attribute(:mirror) true diff --git a/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml b/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml index 925950d5e9a150..eaf0fbfea5a157 100644 --- a/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml +++ b/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml @@ -1,3 +1,4 @@ +- return unless Feature.enabled?(:extended_expiry_webhook_execution_setting, @group) - return unless group.licensed_feature_available?(:group_webhooks) %h5= s_('GroupSettings|Extended Group Access Tokens Expiry Webhook execution') @@ -7,5 +8,5 @@ - c.with_label do = s_('GroupSettings|Extended Group Access Tokens Expiry Webhook execution') - c.with_help_text do - = s_("GroupSettings|If enabled, Group Acces Tokens Expiry Webhooks will be executed on 30 days and 60 days notification delivery as well along with 7 days notification delivery.") - = link_to s_('GroupSettings|Expiry Notifications for Group access tokens'), help_page_path('user/group/manage.md', anchor: 'extended-personal-access-tokens-expiry-webhook-execution'), class: 'gl-font-sm', target: :_blank + = s_("GroupSettings|If enabled, group access tokens expiry webhooks execute 60, 30, and 7 days before the token expires. If disabled, these webhooks only execute 7 days before the token expires.") + = link_to s_('GroupSettings|Add additional webhook triggers for group access token expiration'), help_page_path('user/group/manage.md', anchor: 'add-additional-webhook-triggers-for-group-access-token-expiration'), class: 'gl-font-sm', target: :_blank diff --git a/ee/spec/lib/namespaces/namespace_setting_changes_auditor_spec.rb b/ee/spec/lib/namespaces/namespace_setting_changes_auditor_spec.rb index 829ca367b4ec2d..04fe4170d96bb9 100644 --- a/ee/spec/lib/namespaces/namespace_setting_changes_auditor_spec.rb +++ b/ee/spec/lib/namespaces/namespace_setting_changes_auditor_spec.rb @@ -123,7 +123,7 @@ lock_duo_features_enabled allow_merge_without_pipeline only_allow_merge_if_pipeline_succeeds lock_spp_repository_pipeline_access spp_repository_pipeline_access archived resource_access_token_notify_inherited lock_resource_access_token_notify_inherited - pipeline_variables_default_role force_pages_access_control] + pipeline_variables_default_role extended_grat_expiry_webhooks_execute force_pages_access_control] columns_to_audit = Namespaces::NamespaceSettingChangesAuditor::EVENT_NAME_PER_COLUMN.keys.map(&:to_s) diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb index bf96db8617eb38..42bfb3fa18fbdf 100644 --- a/ee/spec/models/ee/group_spec.rb +++ b/ee/spec/models/ee/group_spec.rb @@ -2969,7 +2969,7 @@ def webhook_headers stub_feature_flags(extended_expiry_webhook_execution_setting: false) end - it 'executes webhook even when setting is disabled' do + it 'executes webhook since no setting is there to control the same' do group.namespace_settings.update!(extended_grat_expiry_webhooks_execute: false) expect(WebHookService).to receive(:new) diff --git a/ee/spec/models/ee/project_spec.rb b/ee/spec/models/ee/project_spec.rb index ac2b54f0444d20..ee5cf058c94453 100644 --- a/ee/spec/models/ee/project_spec.rb +++ b/ee/spec/models/ee/project_spec.rb @@ -1638,9 +1638,9 @@ describe "#execute_hooks" do context "group hooks" do - let(:group) { create(:group) } - let(:project) { create(:project, namespace: group) } - let(:group_hook) { create(:group_hook, group: group, push_events: true) } + let!(:group) { create(:group) } + let!(:project) { create(:project, namespace: group) } + let!(:group_hook) { create(:group_hook, group: group, resource_access_token_events: true) } it 'does not execute the hook when the feature is disabled' do stub_licensed_features(group_webhooks: false) @@ -1675,6 +1675,90 @@ end end + context 'when resource access token hooks' do + let(:wh_service) { double(async_execute: true) } + + context 'when interval is seven days' do + let(:data) { { interval: :seven_days } } + + it 'executes webhook' do + expect(WebHookService).to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_return(wh_service) + + project.execute_hooks(data, :resource_access_token_hooks) + end + end + + context 'when feature flag is disabled' do + let(:data) { { interval: :thirty_days } } + + before do + stub_feature_flags(extended_expiry_webhook_execution_setting: false) + end + + it 'adds webhook to the execution list since no setting is there' do + expect(WebHookService).to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_return(wh_service) + + project.execute_hooks(data, :resource_access_token_hooks) + end + end + + context 'when setting extended_grat_expiry_webhooks_execute is disabled' do + before do + group.namespace_settings.update!(extended_grat_expiry_webhooks_execute: false) + end + + context 'when interval is thirty days' do + let(:data) { { interval: :thirty_days } } + + it 'does not execute the hook' do + expect(WebHookService).not_to receive(:new) + + project.execute_hooks(data, :resource_access_token_hooks) + end + end + + context 'when interval is sixty days' do + let(:data) { { interval: :sixty_days } } + + it 'does not execute the hook' do + expect(WebHookService).not_to receive(:new) + + project.execute_hooks(data, :resource_access_token_hooks) + end + end + end + + context 'when setting extended_grat_expiry_webhooks_execute is enabled' do + before do + group.namespace_settings.update!(extended_grat_expiry_webhooks_execute: true) + end + + context 'when interval is thirty days' do + let(:data) { { interval: :thirty_days } } + + it 'executes webhook' do + expect(WebHookService).to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_return(wh_service) + + project.execute_hooks(data, :resource_access_token_hooks) + end + end + + context 'when interval is sixty days' do + let(:data) { { interval: :sixty_days } } + + it 'executes webhook' do + expect(WebHookService).to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_return(wh_service) + + project.execute_hooks(data, :resource_access_token_hooks) + end + end + end + end + context 'when the hook defines a branch filter for push events' do let(:wh_service) { double(async_execute: true) } let(:selective_hook) { create(:group_hook, group: group, push_events: true, push_events_branch_filter: 'on-this-branch-only') } diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4630881ef16816..317917fc7816d4 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -27925,6 +27925,9 @@ msgstr "" msgid "GroupSelect|Select a group" msgstr "" +msgid "GroupSettings|Add additional webhook triggers for group access token expiration" +msgstr "" + msgid "GroupSettings|After the instance reaches the user cap, any user who is added or requests access must be approved by an administrator. Leave empty for an unlimited user cap. If you change the user cap to unlimited, you must re-enable %{project_sharing_docs_link_start}project sharing%{link_end} and %{group_sharing_docs_link_start}group sharing%{link_end}." msgstr "" @@ -28033,9 +28036,6 @@ msgstr "" msgid "GroupSettings|Experiment" msgstr "" -msgid "GroupSettings|Expiry Notifications for Group access tokens" -msgstr "" - msgid "GroupSettings|Export group" msgstr "" @@ -28063,10 +28063,10 @@ msgstr "" msgid "GroupSettings|How do I manage group SSH certificates?" msgstr "" -msgid "GroupSettings|If enabled, Group Acces Tokens Expiry Webhooks will be executed on 30 days and 60 days notification delivery as well along with 7 days notification delivery." +msgid "GroupSettings|If enabled, enterprise user accounts will not be able to use personal access tokens." msgstr "" -msgid "GroupSettings|If enabled, enterprise user accounts will not be able to use personal access tokens." +msgid "GroupSettings|If enabled, group access tokens expiry webhooks execute 60, 30, and 7 days before the token expires. If disabled, these webhooks only execute 7 days before the token expires." msgstr "" msgid "GroupSettings|If enabled, individual user accounts will be able to use only issued SSH certificates for Git access. It doesn't apply to service accounts, deploy keys, and other types of internal accounts." diff --git a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js index 9ef2a2c1a5a599..352a7b79a4b41a 100644 --- a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js +++ b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js @@ -44,6 +44,7 @@ const defaultProps = { packagesEnabled: true, showDefaultAwardEmojis: true, warnAboutPotentiallyUnwantedCharacters: true, + extendedPratExpiryWebhooksExecute: false }, isGitlabCom: true, canAddCatalogResource: false, @@ -151,6 +152,8 @@ describe('Settings Panel', () => { wrapper.find( 'input[name="project[project_setting_attributes][warn_about_potentially_unwanted_characters]"]', ); + const findExtendedPratExpiryWebhooksExecute = () => + wrapper.find('input[name="project[project_setting_attributes][extended_prat_expiry_webhooks_execute]"]'); const findConfirmDangerButton = () => wrapper.findComponent(ConfirmDanger); const findEnvironmentsSettings = () => wrapper.findComponent({ ref: 'environments-settings' }); const findFeatureFlagsSettings = () => wrapper.findComponent({ ref: 'feature-flags-settings' }); @@ -819,6 +822,28 @@ describe('Settings Panel', () => { }); }); + describe('Setting to allow webhook execution for extended intervals', () => { + it('should have a "Extended Project Access Tokens Expiry Webhook execution" input when feature is enabled', () => { + wrapper = mountComponent( + { + glFeatures: { extendedExpiryWebhookExecutionSetting: true }, + } + ); + + expect(findExtendedPratExpiryWebhooksExecute().exists()).toBe(true); + }); + + it('should not have a "Extended Project Access Tokens Expiry Webhook execution" input when feature is disabled', () => { + wrapper = mountComponent( + { + glFeatures: { extendedExpiryWebhookExecutionSetting: false }, + } + ); + + expect(findExtendedPratExpiryWebhooksExecute().exists()).toBe(false); + }); + }); + describe('Analytics', () => { it('should show the analytics toggle', () => { wrapper = mountComponent(); diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index fed975391b1059..452cf5626359bf 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -6338,19 +6338,23 @@ def has_external_wiki describe '#execute_hooks' do let(:data) { { ref: 'refs/heads/master', data: 'data' } } + let(:hook_scope) { :push_hooks } + let(:hook) { create(:project_hook, merge_requests_events: false, push_events: true) } - it 'executes active projects hooks with the specified scope' do - hook = create(:project_hook, merge_requests_events: false, push_events: true) - expect(ProjectHook).to receive(:select_active) - .with(:push_hooks, data) + shared_examples 'webhook is added to execution list' do + it 'executes webhook succesfully' do + expect(ProjectHook).to receive(:select_active) + .with(hook_scope, data) .and_return([hook]) - project = create(:project, hooks: [hook]) - expect_any_instance_of(ProjectHook).to receive(:async_execute).once + expect_any_instance_of(ProjectHook).to receive(:async_execute).once - project.execute_hooks(data, :push_hooks) + project.execute_hooks(data, hook_scope) + end end + it_behaves_like 'webhook is added to execution list' + it 'does not execute project hooks that dont match the specified scope' do hook = create(:project_hook, merge_requests_events: true, push_events: false) project = create(:project, hooks: [hook]) @@ -6407,6 +6411,72 @@ def has_external_wiki end end.not_to raise_error # Sidekiq::Worker::EnqueueFromTransactionError end + + context 'when its resource access tokens' do + let!(:project) { create(:project) } + let!(:hook) { create(:project_hook, project: project, resource_access_token_events: true) } + let!(:hook_scope) { :resource_access_token_hooks } + + context 'when interval is seven days' do + let(:data) { { interval: :seven_days } } + + it_behaves_like 'webhook is added to execution list' + end + + context 'when feature flag is disabled' do + let(:data) { { interval: :thirty_days } } + + before do + stub_feature_flags(extended_expiry_webhook_execution_setting: false) + end + + it_behaves_like 'webhook is added to execution list' + end + + context 'when setting extended_prat_expiry_webhooks_execute is disabled' do + before do + project.update!(extended_prat_expiry_webhooks_execute: false) + end + + context 'when interval is thirty days' do + let(:data) { { interval: :thirty_days } } + + it 'does not execute the hook' do + expect_any_instance_of(ProjectHook).not_to receive(:async_execute).once + + project.execute_hooks(data, :resource_access_token_hooks) + end + end + + context 'when interval is sixty days' do + let(:data) { { interval: :sixty_days } } + + it 'does not execute the hook' do + expect_any_instance_of(ProjectHook).not_to receive(:async_execute).once + + project.execute_hooks(data, :resource_access_token_hooks) + end + end + end + + context 'when setting extended_prat_expiry_webhooks_execute is enabled' do + before do + project.update!(extended_prat_expiry_webhooks_execute: true) + end + + context 'when interval is thirty days' do + let(:data) { { interval: :thirty_days } } + + it_behaves_like 'webhook is added to execution list' + end + + context 'when interval is sixty days' do + let(:data) { { interval: :sixty_days } } + + it_behaves_like 'webhook is added to execution list' + end + end + end end describe '#execute_integrations' do diff --git a/spec/models/projects/triggered_hooks_spec.rb b/spec/models/projects/triggered_hooks_spec.rb index 5bcd6737c1cd4a..9062ae0a1868ee 100644 --- a/spec/models/projects/triggered_hooks_spec.rb +++ b/spec/models/projects/triggered_hooks_spec.rb @@ -42,169 +42,6 @@ def run_hooks(scope, data, group = nil) run_hooks(:push_hooks, data) end - context 'with access token hooks' do - let(:project) { create(:project) } - let(:project_hook) { create(:project_hook, project: project, resource_access_token_events: true) } - - context 'when interval is seven days' do - let(:data) { { interval: :seven_days } } - - it 'executes webhook' do - expect_hook_execution(project_hook, data, 'resource_access_token_hooks') - - run_hooks(:resource_access_token_hooks, data) - end - end - - context 'when feature flag is disabled' do - let(:data) { { interval: :thirty_days } } - - before do - stub_feature_flags(extended_expiry_webhook_execution_setting: false) - end - - it 'executes webhook even when setting is disabled' do - project.update!(extended_prat_expiry_webhooks_execute: false) - - expect_hook_execution(project_hook, data, 'resource_access_token_hooks') - - run_hooks(:resource_access_token_hooks, data) - end - end - - context 'when setting extended_prat_expiry_webhooks_execute is disabled' do - before do - project.update!(extended_prat_expiry_webhooks_execute: false) - end - - context 'when interval is thirty days' do - let(:data) { { interval: :thirty_days } } - - it 'does not execute the hook' do - expect(WebHookService).not_to receive(:new) - - run_hooks(:resource_access_token_hooks, data) - end - end - - context 'when interval is sixty days' do - let(:data) { { interval: :sixty_days } } - - it 'does not execute the hook' do - expect(WebHookService).not_to receive(:new) - - run_hooks(:resource_access_token_hooks, data) - end - end - end - - context 'when setting extended_prat_expiry_webhooks_execute is enabled' do - before do - project.update!(extended_prat_expiry_webhooks_execute: true) - end - - context 'when interval is thirty days' do - let(:data) { { interval: :thirty_days } } - - it 'executes webhook' do - expect_hook_execution(project_hook, data, 'resource_access_token_hooks') - - run_hooks(:resource_access_token_hooks, data) - end - end - - context 'when interval is sixty days' do - let(:data) { { interval: :sixty_days } } - - it 'executes webhook' do - expect_hook_execution(project_hook, data, 'resource_access_token_hooks') - - run_hooks(:resource_access_token_hooks, data) - end - end - end - - context 'when project belongs to a group with same webhook configured' do - let(:group) { create(:group) } - let(:group_hook) { create(:group_hook, group: group, resource_access_token_events: true) } - let(:data) { { interval: :thirty_days } } - - before do - stub_licensed_features(group_webhooks: true) - project.update!(namespace_id: group.id) - end - - context 'when setting extended_grat_expiry_webhooks_execute is disabled for parent group' do - before do - group.update!(extended_grat_expiry_webhooks_execute: false) - end - - context 'when project setting is enabled' do - before do - project.update!(extended_prat_expiry_webhooks_execute: true) - end - - it 'executes webhook for project and not parent group' do - expect(WebHookService).not_to receive(:new) - .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original - expect(WebHookService).to receive(:new) - .with(project_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original - - run_hooks(:resource_access_token_hooks, data, group) - end - end - - context 'when project setting is disabled' do - before do - project.update!(extended_prat_expiry_webhooks_execute: false) - end - - it 'does not execute webhook for subgroup and not parent group' do - expect(WebHookService).not_to receive(:new) - - run_hooks(:resource_access_token_hooks, data, group) - end - end - end - - context 'when setting extended_grat_expiry_webhooks_execute is enabled for parent group' do - before do - group.update!(extended_grat_expiry_webhooks_execute: true) - end - - context 'when project setting is enabled' do - before do - project.update!(extended_prat_expiry_webhooks_execute: true) - end - - it 'executes webhook for project and parent group' do - expect(WebHookService).to receive(:new) - .with(project_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original - expect(WebHookService).to receive(:new) - .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original - - run_hooks(:resource_access_token_hooks, data, group) - end - end - - context 'when project setting is disabled' do - before do - project.update!(extended_prat_expiry_webhooks_execute: false) - end - - it 'does not execute webhook for project but executes for parent group' do - expect(WebHookService).not_to receive(:new) - .with(project_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original - expect(WebHookService).to receive(:new) - .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original - - run_hooks(:resource_access_token_hooks, data, group) - end - end - end - end - end - context 'with emoji hooks' do let_it_be(:emoji_hook) { create(:project_hook, project: project, emoji_events: true) } -- GitLab From d8e3b9982181d981012c739b1f7a707b8eac2775 Mon Sep 17 00:00:00 2001 From: smriti Date: Tue, 28 Jan 2025 22:50:02 +0530 Subject: [PATCH 04/13] Missing frontend spec added --- .../permissions/components/settings_panel.vue | 7 ++ app/controllers/projects_controller.rb | 2 +- app/models/project.rb | 2 +- app/models/projects/triggered_hooks.rb | 2 +- .../groups/settings/_permissions.html.haml | 3 +- .../personal_access_tokens/expiring_worker.rb | 2 +- doc/user/group/manage.md | 4 +- doc/user/project/settings/_index.md | 2 +- ee/app/models/ee/group.rb | 11 ++- ee/app/models/ee/namespace.rb | 6 -- ee/app/models/ee/project.rb | 5 +- ...nded_grat_expiry_webhook_execute.html.haml | 2 +- ee/spec/models/ee/group_spec.rb | 79 ++++++++++++++++--- ee/spec/models/ee/namespace_spec.rb | 44 ----------- ee/spec/models/ee/project_spec.rb | 2 +- .../settings/_permissions.html.haml_spec.rb | 9 ++- .../expiring_worker_spec.rb | 2 +- ...en.rb => resource_access_token_payload.rb} | 2 +- locale/gitlab.pot | 8 +- .../components/settings_panel_spec.js | 22 +++--- ... => resource_access_token_payload_spec.rb} | 2 +- spec/models/project_spec.rb | 5 +- spec/models/projects/triggered_hooks_spec.rb | 13 ++- .../test_hooks/project_service_spec.rb | 2 +- .../expiring_worker_spec.rb | 4 +- 25 files changed, 135 insertions(+), 107 deletions(-) rename lib/gitlab/data_builder/{resource_access_token.rb => resource_access_token_payload.rb} (94%) rename spec/lib/gitlab/data_builder/{resource_access_token_spec.rb => resource_access_token_payload_spec.rb} (93%) diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue index fcef380b41b26b..b1f2354e325acd 100644 --- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue +++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue @@ -98,10 +98,17 @@ export default { confirmButtonText: __('Save changes'), emailsLabel: s__('ProjectSettings|Email notifications'), extendedPratExpiryWebhooksExecuteLabel: s__( +<<<<<<< HEAD 'ProjectSettings|Add additional webhook triggers for group access token expiry.', ), extendedPratExpiryWebhooksExecuteHelpText: s__( 'ProjectSettings|If enabled, project access tokens expiry webhooks execute 60, 30, and 7 days before the token expires. If disabled, these webhooks only execute 7 days before the token expires. %{linkStart}Learn more%{linkEnd}', +======= + 'ProjectSettings|Add additional webhook triggers for group access token expiry', + ), + extendedPratExpiryWebhooksExecuteHelpText: s__( + 'ProjectSettings|If enabled, project access tokens expiry webhooks execute 60, 30, and 7 days before the token expires. If disabled, these webhooks only execute 7 days before the token expires.', +>>>>>>> ae3a352ac998 (Missing frontend spec added) ), showDiffPreviewLabel: s__('ProjectSettings|Include diff previews'), showDiffPreviewHelpText: s__( diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index df23b4ac0a1a50..ed5d88c92b97eb 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -472,7 +472,7 @@ def project_setting_attributes emails_enabled ] - if ::Feature.enabled?(:extended_expiry_webhook_execution_setting, project) && + if ::Feature.enabled?(:extended_expiry_webhook_execution_setting, @project&.namespace) && can?(current_user, :admin_project, project) attributes << :extended_prat_expiry_webhooks_execute end diff --git a/app/models/project.rb b/app/models/project.rb index 8f27500c25cabf..c1b5b66c540db8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1988,7 +1988,7 @@ def triggered_hooks(hooks_scope, data) # seven_days interval but we have a setting to allow webhook execution # for thirty_days and sixty_days_plus interval too. if hooks_scope == :resource_access_token_hooks && - ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self) && + ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self.namespace) && data[:interval] != :seven_days && !self.extended_prat_expiry_webhooks_execute? diff --git a/app/models/projects/triggered_hooks.rb b/app/models/projects/triggered_hooks.rb index ca5f9052ebcddf..9fb38a95b05d2c 100644 --- a/app/models/projects/triggered_hooks.rb +++ b/app/models/projects/triggered_hooks.rb @@ -17,7 +17,7 @@ def execute # Assumes that the relations implement TriggerableHooks @relations.each do |hooks| - hooks&.hooks_for(@scope)&.select_active(@scope, @data)&.each do |hook| + hooks.hooks_for(@scope).select_active(@scope, @data).each do |hook| hook.async_execute(@data, @scope.to_s) end end diff --git a/app/views/groups/settings/_permissions.html.haml b/app/views/groups/settings/_permissions.html.haml index 3c3a3b0119770c..87287ae91339e9 100644 --- a/app/views/groups/settings/_permissions.html.haml +++ b/app/views/groups/settings/_permissions.html.haml @@ -39,8 +39,7 @@ = render_if_exists 'groups/settings/prevent_forking', f: f, group: @group = render_if_exists 'groups/settings/service_access_tokens_expiration_enforced', f: f, group: @group = render_if_exists 'groups/settings/enforce_ssh_certificates', f: f, group: @group - - if Feature.enabled?(:extended_expiry_webhook_execution_setting, @group) - = render_if_exists 'groups/settings/extended_grat_expiry_webhook_execute', f: f, group: @group + = render_if_exists 'groups/settings/extended_grat_expiry_webhook_execute', f: f, group: @group = render 'groups/settings/two_factor_auth', f: f, group: @group = render_if_exists 'groups/personal_access_token_expiration_policy', f: f, group: @group = render 'groups/settings/membership', f: f, group: @group diff --git a/app/workers/personal_access_tokens/expiring_worker.rb b/app/workers/personal_access_tokens/expiring_worker.rb index 5ca48a793f1b61..da2a559011e5d2 100644 --- a/app/workers/personal_access_tokens/expiring_worker.rb +++ b/app/workers/personal_access_tokens/expiring_worker.rb @@ -213,7 +213,7 @@ def execute_web_hooks(bot_user, token, interval) return unless resource return if resource.is_a?(Project) && !resource.has_active_hooks?(:resource_access_token_hooks) - hook_data = Gitlab::DataBuilder::ResourceAccessToken.build(token, :expiring, resource, interval) + hook_data = Gitlab::DataBuilder::ResourceAccessTokenPayload.build(token, :expiring, resource, interval) resource.execute_hooks(hook_data, :resource_access_token_hooks) end diff --git a/doc/user/group/manage.md b/doc/user/group/manage.md index 3e39bba186191b..bd46ac8a4e0774 100644 --- a/doc/user/group/manage.md +++ b/doc/user/group/manage.md @@ -207,9 +207,9 @@ For more information, see: FLAG: The availability of this feature is controlled by a feature flag. For more information, see the history. -GitLab sends multiple [expiry emails](../group/settings/group_access_tokens.md#group-access-token-expiry-emails) and triggers a related [webhook](../project/integrations/webhook_events.md#project-and-group-access-token-events) before a group token expires. By default, GitLab only triggers these webhooks 7 days before the token expires. When this feature is enabled, GitLab also triggers these webhooks 60 days and 30 days before the token expires. +GitLab sends multiple [expiry emails](../group/settings/group_access_tokens.md#group-access-token-expiry-emails) and triggers a related [webhook](../project/integrations/webhook_events.md#project-and-group-access-token-events) before a group token expires. By default, GitLab only triggers these webhooks 7 days before the token expires. When this feature is enabled, GitLab also can trigger these webhooks 60 days and 30 days before the token expires. -You can enable execution of webhooks for 30 days and 60 days intervals too: +To enable additional triggers for these webhooks: 1. On the left sidebar, select **Search or go to** and find your group. 1. Select **Settings > General**. diff --git a/doc/user/project/settings/_index.md b/doc/user/project/settings/_index.md index f11294e45c1b47..7855653f7f4ffe 100644 --- a/doc/user/project/settings/_index.md +++ b/doc/user/project/settings/_index.md @@ -161,7 +161,7 @@ The availability of this feature is controlled by a feature flag. For more infor GitLab sends multiple [expiry emails](project_access_tokens.md#project-access-token-expiry-emails) and triggers a related [webhook](../integrations/webhook_events.md#project-and-group-access-token-events) before a project token expires. By default, GitLab only triggers these webhooks 7 days before the token expires. When this feature is enabled, GitLab also triggers these webhooks 60 days and 30 days before the token expires. -You can enable execution of webhooks for 30 days and 60 days intervals too: +To enable additional triggers for these webhooks: 1. On the left sidebar, select **Search or go to** and find your project. 1. Select **Settings > General**. diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index b0f9eb7c12e9f7..007a1e40ea1ca3 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -116,6 +116,8 @@ module Group delegate :wiki_access_level, :wiki_access_level=, to: :group_feature, allow_nil: true delegate :enable_auto_assign_gitlab_duo_pro_seats, :enable_auto_assign_gitlab_duo_pro_seats=, :enable_auto_assign_gitlab_duo_pro_seats_human_readable, :enable_auto_assign_gitlab_duo_pro_seats_human_readable=, to: :namespace_settings, allow_nil: true + delegate :extended_grat_expiry_webhooks_execute, :extended_grat_expiry_webhooks_execute=, to: :namespace_settings + # Use +checked_file_template_project+ instead, which implements important # visibility checks private :file_template_project @@ -1107,6 +1109,11 @@ def disable_personal_access_tokens? namespace_settings.disable_personal_access_tokens? end + def extended_grat_expiry_webhooks_execute? + licensed_feature_available?(:group_webhooks) && + namespace_settings&.extended_grat_expiry_webhooks_execute? + end + def active_compliance_frameworks? # test default framework first since it is most likely to have projects assigned [[default_compliance_framework] + compliance_management_frameworks].flatten.compact.uniq.any? do |framework| @@ -1124,8 +1131,8 @@ def groups_for_extended_webhook_execution_on_token_expiry return unless ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self) self_and_ancestors - .joins(:namespace_settings) - .where(namespace_settings: { extended_grat_expiry_webhooks_execute: true }) + .joins(:namespace_settings) + .where(namespace_settings: { extended_grat_expiry_webhooks_execute: true }) end private diff --git a/ee/app/models/ee/namespace.rb b/ee/app/models/ee/namespace.rb index bdac7c2cfe9a16..9db1f706478f21 100644 --- a/ee/app/models/ee/namespace.rb +++ b/ee/app/models/ee/namespace.rb @@ -212,8 +212,6 @@ module Namespace :early_access_program_participant, :enforce_ssh_certificates, :enforce_ssh_certificates=, - :extended_grat_expiry_webhooks_execute, - :extended_grat_expiry_webhooks_execute=, to: :namespace_settings, allow_nil: true ) @@ -621,10 +619,6 @@ def enforce_ssh_certificates? root? && namespace_settings&.enforce_ssh_certificates? end - def extended_grat_expiry_webhooks_execute? - licensed_feature_available?(:group_webhooks) && namespace_settings&.extended_grat_expiry_webhooks_execute? - end - def ssh_certificates_available? root? && licensed_feature_available?(:ssh_certificates) end diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index 7cf2d4dda45cf3..cecdb4d871d06a 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -858,8 +858,7 @@ def triggered_hooks(scope, data) triggered = super if group && feature_available?(:group_webhooks) - - if ::Feature.enabled?(:extended_expiry_webhook_execution_setting, group) && + if ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self.namespace) && scope == :resource_access_token_hooks && data[:interval] != :seven_days triggered.add_hooks(group_hooks_for_groups_with_extended_execution) @@ -1530,7 +1529,7 @@ def group_hooks end def group_hooks_for_groups_with_extended_execution - return unless ::Feature.enabled?(:extended_expiry_webhook_execution_setting, group) + return unless ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self.namespace) GroupHook.where(group_id: group.groups_for_extended_webhook_execution_on_token_expiry) end diff --git a/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml b/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml index eaf0fbfea5a157..a2f8fa0615f769 100644 --- a/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml +++ b/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml @@ -6,7 +6,7 @@ .form-group.gl-mb-3 = f.gitlab_ui_checkbox_component :extended_grat_expiry_webhooks_execute, checkbox_options: { disabled: !can?(current_user, :admin_group, group), checked: group.extended_grat_expiry_webhooks_execute? } do |c| - c.with_label do - = s_('GroupSettings|Extended Group Access Tokens Expiry Webhook execution') + = s_('GroupSettings|Add additional webhook triggers for group access token expiration') - c.with_help_text do = s_("GroupSettings|If enabled, group access tokens expiry webhooks execute 60, 30, and 7 days before the token expires. If disabled, these webhooks only execute 7 days before the token expires.") = link_to s_('GroupSettings|Add additional webhook triggers for group access token expiration'), help_page_path('user/group/manage.md', anchor: 'add-additional-webhook-triggers-for-group-access-token-expiration'), class: 'gl-font-sm', target: :_blank diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb index 42bfb3fa18fbdf..7cadef7f30dfc4 100644 --- a/ee/spec/models/ee/group_spec.rb +++ b/ee/spec/models/ee/group_spec.rb @@ -2943,7 +2943,7 @@ def webhook_headers end end - context 'when resource access token hooks' do + context 'when resource access token hooks for expiry notification' do let(:group) { create(:group) } let(:group_hook) { create(:group_hook, group: group, resource_access_token_events: true) } @@ -2956,7 +2956,8 @@ def webhook_headers it 'executes webhook' do expect(WebHookService).to receive(:new) - .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) + .and_call_original group.execute_hooks(data, :resource_access_token_hooks) end @@ -2973,7 +2974,8 @@ def webhook_headers group.namespace_settings.update!(extended_grat_expiry_webhooks_execute: false) expect(WebHookService).to receive(:new) - .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) + .and_call_original group.execute_hooks(data, :resource_access_token_hooks) end @@ -3015,7 +3017,8 @@ def webhook_headers it 'executes webhook' do expect(WebHookService).to receive(:new) - .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) + .and_call_original group.execute_hooks(data, :resource_access_token_hooks) end @@ -3026,7 +3029,8 @@ def webhook_headers it 'executes webhook' do expect(WebHookService).to receive(:new) - .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) + .and_call_original group.execute_hooks(data, :resource_access_token_hooks) end @@ -3050,9 +3054,12 @@ def webhook_headers it 'executes webhook for subgroup and not parent group' do expect(WebHookService).to receive(:new) - .with(subgroup_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + .with(subgroup_hook, data, 'resource_access_token_hooks', idempotency_key: anything) + .and_call_original + expect(WebHookService).not_to receive(:new) - .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) + .and_call_original subgroup.execute_hooks(data, :resource_access_token_hooks) end @@ -3083,9 +3090,12 @@ def webhook_headers it 'executes webhook for subgroup and not parent group' do expect(WebHookService).to receive(:new) - .with(subgroup_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + .with(subgroup_hook, data, 'resource_access_token_hooks', idempotency_key: anything) + .and_call_original + expect(WebHookService).to receive(:new) - .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) + .and_call_original subgroup.execute_hooks(data, :resource_access_token_hooks) end @@ -3098,9 +3108,12 @@ def webhook_headers it 'does not execute webhook for subgroup and not parent group' do expect(WebHookService).not_to receive(:new) - .with(subgroup_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + .with(subgroup_hook, data, 'resource_access_token_hooks', idempotency_key: anything) + .and_call_original + expect(WebHookService).to receive(:new) - .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_call_original + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) + .and_call_original subgroup.execute_hooks(data, :resource_access_token_hooks) end @@ -4513,6 +4526,50 @@ def webhook_headers end end + describe '#extended_grat_expiry_webhooks_execute?' do + let(:group) { create(:group) } + + context 'when extended_grat_expiry_webhooks_execute = true' do + before do + group.extended_grat_expiry_webhooks_execute = true + end + + context 'when licensed feature is available' do + before do + stub_licensed_features(group_webhooks: true) + end + + it 'delegates the field to namespace settings and return true' do + expect(group.namespace_settings).to receive(:extended_grat_expiry_webhooks_execute?).and_call_original + + expect(group.extended_grat_expiry_webhooks_execute?).to be_truthy + end + end + + it 'returns false if licensed feature is not available' do + expect(group.extended_grat_expiry_webhooks_execute?).to be_truthy + end + end + + context 'when extended_grat_expiry_webhooks_execute=false' do + before do + group.extended_grat_expiry_webhooks_execute = false + end + + context 'when licensed feature is available' do + before do + stub_licensed_features(group_webhooks: true) + end + + it 'delegates the field to namespace settings and returns false' do + expect(group.namespace_settings).to receive(:extended_grat_expiry_webhooks_execute?).and_call_original + + expect(group.extended_grat_expiry_webhooks_execute?).to be_falsey + end + end + end + end + describe '#scim_identities' do let(:group) { create(:group) } let(:instance_scim_identity) { create(:scim_identity, group: group) } diff --git a/ee/spec/models/ee/namespace_spec.rb b/ee/spec/models/ee/namespace_spec.rb index a1718e0ca3a1cf..0fe874042462e8 100644 --- a/ee/spec/models/ee/namespace_spec.rb +++ b/ee/spec/models/ee/namespace_spec.rb @@ -2195,50 +2195,6 @@ end end - describe '#extended_grat_expiry_webhooks_execute?' do - let(:namespace) { create(:group) } - - context 'when extended_grat_expiry_webhooks_execute = true' do - before do - namespace.extended_grat_expiry_webhooks_execute = true - end - - context 'when licensed feature is available' do - before do - stub_licensed_features(group_webhooks: true) - end - - it 'delegates the field to namespace settings and return true' do - expect(namespace.namespace_settings).to receive(:extended_grat_expiry_webhooks_execute?).and_call_original - - expect(namespace.extended_grat_expiry_webhooks_execute?).to be_truthy - end - end - - it 'returns false if licensed feature is not available' do - expect(namespace.extended_grat_expiry_webhooks_execute?).to be_truthy - end - end - - context 'when extended_grat_expiry_webhooks_execute=false' do - before do - namespace.extended_grat_expiry_webhooks_execute = false - end - - context 'when licensed feature is available' do - before do - stub_licensed_features(group_webhooks: true) - end - - it 'delegates the field to namespace settings and returns false' do - expect(namespace.namespace_settings).to receive(:extended_grat_expiry_webhooks_execute?).and_call_original - - expect(namespace.extended_grat_expiry_webhooks_execute?).to be_falsey - end - end - end - end - describe '#ssh_certificates_available?' do let(:namespace) { create(:group) } diff --git a/ee/spec/models/ee/project_spec.rb b/ee/spec/models/ee/project_spec.rb index ee5cf058c94453..7a7331128355cf 100644 --- a/ee/spec/models/ee/project_spec.rb +++ b/ee/spec/models/ee/project_spec.rb @@ -1675,7 +1675,7 @@ end end - context 'when resource access token hooks' do + context 'when resource access token hooks for expiry notification' do let(:wh_service) { double(async_execute: true) } context 'when interval is seven days' do diff --git a/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb b/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb index 93bb38e5dcfd11..8971769dee88ec 100644 --- a/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb +++ b/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb @@ -83,9 +83,9 @@ context 'for extended token expiry webhook execution setting' do let_it_be(:checkbox_label) { s_('GroupSettings|Extended Group Access Tokens Expiry Webhook execution') } - context 'when licensed feature is not available' do + context 'when `group_webhooks` licensed feature is not available' do before do - allow(group).to receive(:licensed_feature_available?).and_return(false) + allow(group).to receive(:licensed_feature_available?).and_return(true) allow(group).to receive(:licensed_feature_available?).with(:group_webhooks).and_return(false) end @@ -97,9 +97,9 @@ end end - context 'when licensed feature is available' do + context 'when `group_webhooks` licensed feature is available' do before do - allow(group).to receive(:licensed_feature_available?).and_return(false) + allow(group).to receive(:licensed_feature_available?).and_return(true) allow(group).to receive(:licensed_feature_available?).with(:group_webhooks).and_return(true) end @@ -121,6 +121,7 @@ expect(rendered).not_to render_template('groups/settings/_extended_grat_expiry_webhook_execute') expect(rendered).not_to have_content('Extended Group Access Tokens Expiry Webhook execution') + expect(rendered).not_to have_unchecked_field(checkbox_label, type: 'checkbox') end end end diff --git a/ee/spec/workers/personal_access_tokens/expiring_worker_spec.rb b/ee/spec/workers/personal_access_tokens/expiring_worker_spec.rb index 5d851685e9595a..2a348f9364eca3 100644 --- a/ee/spec/workers/personal_access_tokens/expiring_worker_spec.rb +++ b/ee/spec/workers/personal_access_tokens/expiring_worker_spec.rb @@ -20,7 +20,7 @@ it 'executes access token webhook' do stub_licensed_features(group_webhooks: true) - expect(Gitlab::DataBuilder::ResourceAccessToken).to receive(:build).and_return(hook_data) + expect(Gitlab::DataBuilder::ResourceAccessTokenPayload).to receive(:build).and_return(hook_data) expect(fake_wh_service).to receive(:async_execute).once expect(WebHookService) diff --git a/lib/gitlab/data_builder/resource_access_token.rb b/lib/gitlab/data_builder/resource_access_token_payload.rb similarity index 94% rename from lib/gitlab/data_builder/resource_access_token.rb rename to lib/gitlab/data_builder/resource_access_token_payload.rb index 68e2e5909961f2..85fb349dded766 100644 --- a/lib/gitlab/data_builder/resource_access_token.rb +++ b/lib/gitlab/data_builder/resource_access_token_payload.rb @@ -2,7 +2,7 @@ module Gitlab module DataBuilder - module ResourceAccessToken + module ResourceAccessTokenPayload extend self def build(resource_access_token, event, resource, interval = :seven_days) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 317917fc7816d4..fcac4882481d72 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -45045,6 +45045,9 @@ msgstr "" msgid "ProjectSettings|A default branch cannot be chosen for an empty project." msgstr "" +msgid "ProjectSettings|Add additional webhook triggers for group access token expiry" +msgstr "" + msgid "ProjectSettings|Add badges to display information about this project." msgstr "" @@ -45198,9 +45201,6 @@ msgstr "" msgid "ProjectSettings|Existing merge requests and protected branches are not affected." msgstr "" -msgid "ProjectSettings|Extended Project Access Tokens Expiry Webhook execution" -msgstr "" - msgid "ProjectSettings|Failed to protect the tag" msgstr "" @@ -45252,7 +45252,7 @@ msgstr "" msgid "ProjectSettings|If GitLab manages your cluster, then GitLab retains your analytics data for 1 year. %{link_start}Learn more about data retention policy%{link_end}." msgstr "" -msgid "ProjectSettings|If enabled, Project Acces Tokens Expiry Webhooks will be executed on 30 days and 60 days notification delivery as well along with 7 days notification delivery." +msgid "ProjectSettings|If enabled, project access tokens expiry webhooks execute 60, 30, and 7 days before the token expires. If disabled, these webhooks only execute 7 days before the token expires." msgstr "" msgid "ProjectSettings|If merge trains are enabled, merging is only possible if the branch can be rebased without conflicts." diff --git a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js index 352a7b79a4b41a..1f82a7d3c6c948 100644 --- a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js +++ b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js @@ -44,7 +44,7 @@ const defaultProps = { packagesEnabled: true, showDefaultAwardEmojis: true, warnAboutPotentiallyUnwantedCharacters: true, - extendedPratExpiryWebhooksExecute: false + extendedPratExpiryWebhooksExecute: false, }, isGitlabCom: true, canAddCatalogResource: false, @@ -153,7 +153,9 @@ describe('Settings Panel', () => { 'input[name="project[project_setting_attributes][warn_about_potentially_unwanted_characters]"]', ); const findExtendedPratExpiryWebhooksExecute = () => - wrapper.find('input[name="project[project_setting_attributes][extended_prat_expiry_webhooks_execute]"]'); + wrapper.find( + 'input[name="project[project_setting_attributes][extended_prat_expiry_webhooks_execute]"]', + ); const findConfirmDangerButton = () => wrapper.findComponent(ConfirmDanger); const findEnvironmentsSettings = () => wrapper.findComponent({ ref: 'environments-settings' }); const findFeatureFlagsSettings = () => wrapper.findComponent({ ref: 'feature-flags-settings' }); @@ -824,21 +826,17 @@ describe('Settings Panel', () => { describe('Setting to allow webhook execution for extended intervals', () => { it('should have a "Extended Project Access Tokens Expiry Webhook execution" input when feature is enabled', () => { - wrapper = mountComponent( - { - glFeatures: { extendedExpiryWebhookExecutionSetting: true }, - } - ); + wrapper = mountComponent({ + glFeatures: { extendedExpiryWebhookExecutionSetting: true }, + }); expect(findExtendedPratExpiryWebhooksExecute().exists()).toBe(true); }); it('should not have a "Extended Project Access Tokens Expiry Webhook execution" input when feature is disabled', () => { - wrapper = mountComponent( - { - glFeatures: { extendedExpiryWebhookExecutionSetting: false }, - } - ); + wrapper = mountComponent({ + glFeatures: { extendedExpiryWebhookExecutionSetting: false }, + }); expect(findExtendedPratExpiryWebhooksExecute().exists()).toBe(false); }); diff --git a/spec/lib/gitlab/data_builder/resource_access_token_spec.rb b/spec/lib/gitlab/data_builder/resource_access_token_payload_spec.rb similarity index 93% rename from spec/lib/gitlab/data_builder/resource_access_token_spec.rb rename to spec/lib/gitlab/data_builder/resource_access_token_payload_spec.rb index 1b1d2000e07f87..c78027520a35fe 100644 --- a/spec/lib/gitlab/data_builder/resource_access_token_spec.rb +++ b/spec/lib/gitlab/data_builder/resource_access_token_payload_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::DataBuilder::ResourceAccessToken, feature_category: :system_access do +RSpec.describe Gitlab::DataBuilder::ResourceAccessTokenPayload, feature_category: :system_access do let_it_be_with_reload(:user) { create(:user, :project_bot) } let(:event) { :expiring } let(:interval) { :thirty_days } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 452cf5626359bf..754c8a8b1a3041 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2286,7 +2286,7 @@ def has_external_wiki end describe '#builds_enabled' do - let(:project) { create(:project) } + let_it_be_with_reload(:project) { create(:project) } subject { project.builds_enabled } @@ -6340,6 +6340,7 @@ def has_external_wiki let(:data) { { ref: 'refs/heads/master', data: 'data' } } let(:hook_scope) { :push_hooks } let(:hook) { create(:project_hook, merge_requests_events: false, push_events: true) } + let(:project) { create(:project) } shared_examples 'webhook is added to execution list' do it 'executes webhook succesfully' do @@ -6412,7 +6413,7 @@ def has_external_wiki end.not_to raise_error # Sidekiq::Worker::EnqueueFromTransactionError end - context 'when its resource access tokens' do + context 'when resource access token hooks for expiry notification' do let!(:project) { create(:project) } let!(:hook) { create(:project_hook, project: project, resource_access_token_events: true) } let!(:hook_scope) { :resource_access_token_hooks } diff --git a/spec/models/projects/triggered_hooks_spec.rb b/spec/models/projects/triggered_hooks_spec.rb index 9062ae0a1868ee..51cf211beb0ec9 100644 --- a/spec/models/projects/triggered_hooks_spec.rb +++ b/spec/models/projects/triggered_hooks_spec.rb @@ -12,10 +12,9 @@ let(:wh_service) { instance_double(::WebHookService, async_execute: true) } let(:data) { { some: 'data', as: 'json' } } - def run_hooks(scope, data, group = nil) + def run_hooks(scope, data) hooks = described_class.new(scope, data) hooks.add_hooks(ProjectHook.all) - hooks.add_hooks(GroupHook.where(group_id: group.self_and_ancestors)) if group hooks.execute end @@ -42,6 +41,16 @@ def run_hooks(scope, data, group = nil) run_hooks(:push_hooks, data) end + context 'with access token hooks' do + let_it_be(:resource_access_token_hook) { create(:project_hook, project: project, resource_access_token_events: true) } + + it 'executes hook' do + expect_hook_execution(resource_access_token_hook, data, 'resource_access_token_hooks') + + run_hooks(:resource_access_token_hooks, data) + end + end + context 'with emoji hooks' do let_it_be(:emoji_hook) { create(:project_hook, project: project, emoji_events: true) } diff --git a/spec/services/test_hooks/project_service_spec.rb b/spec/services/test_hooks/project_service_spec.rb index 7bc86f3046c9cb..1e06911f8f799c 100644 --- a/spec/services/test_hooks/project_service_spec.rb +++ b/spec/services/test_hooks/project_service_spec.rb @@ -237,7 +237,7 @@ let(:trigger_key) { :resource_access_token_hooks } it 'executes hook' do - allow(Gitlab::DataBuilder::ResourceAccessToken).to receive(:build).and_return(sample_data) + allow(Gitlab::DataBuilder::ResourceAccessTokenPayload).to receive(:build).and_return(sample_data) expect(hook).to receive(:execute).with(sample_data, trigger_key, force: true).and_return(success_result) expect(service.execute).to include(success_result) diff --git a/spec/workers/personal_access_tokens/expiring_worker_spec.rb b/spec/workers/personal_access_tokens/expiring_worker_spec.rb index d35a7413e7b274..e4fc6204fb6a91 100644 --- a/spec/workers/personal_access_tokens/expiring_worker_spec.rb +++ b/spec/workers/personal_access_tokens/expiring_worker_spec.rb @@ -251,7 +251,7 @@ hook_data = { interval: :seven_days } project_hook = create(:project_hook, project: project, resource_access_token_events: true) - expect(Gitlab::DataBuilder::ResourceAccessToken).to receive(:build).and_return(hook_data) + expect(Gitlab::DataBuilder::ResourceAccessTokenPayload).to receive(:build).and_return(hook_data) expect(fake_wh_service).to receive(:async_execute).once expect(WebHookService) @@ -275,7 +275,7 @@ expiring_token.update!(expires_at: 30.days.from_now) project_hook = create(:project_hook, project: project, resource_access_token_events: true) - expect(Gitlab::DataBuilder::ResourceAccessToken).not_to receive(:build) + expect(Gitlab::DataBuilder::ResourceAccessTokenPayload).not_to receive(:build) expect(WebHookService) .not_to receive(:new) .with( -- GitLab From e970c1083103685cff72efac01e403679c775ffb Mon Sep 17 00:00:00 2001 From: smriti Date: Thu, 30 Jan 2025 15:13:25 +0530 Subject: [PATCH 05/13] Webhook execution on personal access token expiry Related webhook should also be executed on other expiry intervals like 30 days and 60 days. We are introducting two settings one at group level and other project level to control the webhook execution additionally on other intervals. Changelog: added MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/178266 Changes in class name missing in test data --- .../permissions/components/settings_panel.vue | 215 +++++++++--------- .../projects/shared/permissions/constants.js | 5 + app/models/projects/triggered_hooks.rb | 1 - .../integrations/project_test_data.rb | 2 +- ...nded_grat_expiry_webhook_execute.html.haml | 2 +- .../settings/_permissions.html.haml_spec.rb | 10 +- .../expiring_worker_spec.rb | 2 +- locale/gitlab.pot | 4 +- 8 files changed, 120 insertions(+), 121 deletions(-) diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue index b1f2354e325acd..3632197dc89f7a 100644 --- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue +++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue @@ -98,17 +98,10 @@ export default { confirmButtonText: __('Save changes'), emailsLabel: s__('ProjectSettings|Email notifications'), extendedPratExpiryWebhooksExecuteLabel: s__( -<<<<<<< HEAD - 'ProjectSettings|Add additional webhook triggers for group access token expiry.', - ), - extendedPratExpiryWebhooksExecuteHelpText: s__( - 'ProjectSettings|If enabled, project access tokens expiry webhooks execute 60, 30, and 7 days before the token expires. If disabled, these webhooks only execute 7 days before the token expires. %{linkStart}Learn more%{linkEnd}', -======= 'ProjectSettings|Add additional webhook triggers for group access token expiry', ), extendedPratExpiryWebhooksExecuteHelpText: s__( 'ProjectSettings|If enabled, project access tokens expiry webhooks execute 60, 30, and 7 days before the token expires. If disabled, these webhooks only execute 7 days before the token expires.', ->>>>>>> ae3a352ac998 (Missing frontend spec added) ), showDiffPreviewLabel: s__('ProjectSettings|Include diff previews'), showDiffPreviewHelpText: s__( @@ -1069,73 +1062,70 @@ export default { /> - - - - - - {{ $options.i18n.showDiffPreviewLabel }} - - - - - + + - + + - - {{ $options.i18n.pucWarningLabel }} - + + {{ $options.i18n.showDiffPreviewLabel }} + - + + + + {{ s__('ProjectSettings|Show default emoji reactions') }} + + + + + + + {{ $options.i18n.pucWarningLabel }} + + + + - - - - - - - - - - \ No newline at end of file + + + + + + + + + {{ $options.i18n.confirmButtonText }} + + + diff --git a/app/assets/javascripts/pages/projects/shared/permissions/constants.js b/app/assets/javascripts/pages/projects/shared/permissions/constants.js index fe287998f4968d..c2121623073f7c 100644 --- a/app/assets/javascripts/pages/projects/shared/permissions/constants.js +++ b/app/assets/javascripts/pages/projects/shared/permissions/constants.js @@ -51,6 +51,11 @@ export const modelExperimentsHelpPath = helpPagePath( export const modelRegistryHelpPath = helpPagePath('user/project/ml/model_registry/_index.md'); +export const extendedPratExpiryWebhooksExecuteHelpPath = helpPagePath( + 'user/project/settings/index.md', + { anchor: 'add-additional-webhook-triggers-for-project-access-token-expiration' }, +); + export const duoHelpPath = helpPagePath('user/ai_features'); export const amazonQHelpPath = helpPagePath('user/duo_amazon_q/index.md'); diff --git a/app/models/projects/triggered_hooks.rb b/app/models/projects/triggered_hooks.rb index 9fb38a95b05d2c..e3aa3d106b7300 100644 --- a/app/models/projects/triggered_hooks.rb +++ b/app/models/projects/triggered_hooks.rb @@ -15,7 +15,6 @@ def add_hooks(relation) def execute # Assumes that the relations implement TriggerableHooks - @relations.each do |hooks| hooks.hooks_for(@scope).select_active(@scope, @data).each do |hook| hook.async_execute(@data, @scope.to_s) diff --git a/app/services/concerns/integrations/project_test_data.rb b/app/services/concerns/integrations/project_test_data.rb index b029717586b6ad..21c3442427511c 100644 --- a/app/services/concerns/integrations/project_test_data.rb +++ b/app/services/concerns/integrations/project_test_data.rb @@ -107,7 +107,7 @@ def access_tokens_events_data expires_at: 2.days.from_now ) - Gitlab::DataBuilder::ResourceAccessToken.build(resource_access_token, :expiring, project) + Gitlab::DataBuilder::ResourceAccessTokenPayload.build(resource_access_token, :expiring, project) end def vulnerability_events_data diff --git a/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml b/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml index a2f8fa0615f769..9f4073c4129358 100644 --- a/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml +++ b/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml @@ -1,4 +1,4 @@ -- return unless Feature.enabled?(:extended_expiry_webhook_execution_setting, @group) +- return unless Feature.enabled?(:extended_expiry_webhook_execution_setting, group) - return unless group.licensed_feature_available?(:group_webhooks) %h5= s_('GroupSettings|Extended Group Access Tokens Expiry Webhook execution') diff --git a/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb b/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb index 8971769dee88ec..fce3089d940695 100644 --- a/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb +++ b/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb @@ -81,7 +81,7 @@ end context 'for extended token expiry webhook execution setting' do - let_it_be(:checkbox_label) { s_('GroupSettings|Extended Group Access Tokens Expiry Webhook execution') } + let_it_be(:checkbox_label) { s_('GroupSettings|Add additional webhook triggers for group access token expiration') } context 'when `group_webhooks` licensed feature is not available' do before do @@ -93,7 +93,7 @@ render expect(rendered).to render_template('groups/settings/_extended_grat_expiry_webhook_execute') - expect(rendered).not_to have_content('Extended Group Access Tokens Expiry Webhook execution') + expect(rendered).not_to have_content('Add additional webhook triggers for group access token expiration') end end @@ -107,7 +107,7 @@ render expect(rendered).to render_template('groups/settings/_extended_grat_expiry_webhook_execute') - expect(rendered).to have_content('Extended Group Access Tokens Expiry Webhook execution') + expect(rendered).to have_content('Add additional webhook triggers for group access token expiration') expect(rendered).to have_unchecked_field(checkbox_label, type: 'checkbox') end @@ -119,8 +119,8 @@ it 'renders nothing', :aggregate_failures do render - expect(rendered).not_to render_template('groups/settings/_extended_grat_expiry_webhook_execute') - expect(rendered).not_to have_content('Extended Group Access Tokens Expiry Webhook execution') + expect(rendered).to render_template('groups/settings/_extended_grat_expiry_webhook_execute') + expect(rendered).not_to have_content('Add additional webhook triggers for group access token expiration') expect(rendered).not_to have_unchecked_field(checkbox_label, type: 'checkbox') end end diff --git a/ee/spec/workers/personal_access_tokens/expiring_worker_spec.rb b/ee/spec/workers/personal_access_tokens/expiring_worker_spec.rb index 2a348f9364eca3..bb4e50bee732b0 100644 --- a/ee/spec/workers/personal_access_tokens/expiring_worker_spec.rb +++ b/ee/spec/workers/personal_access_tokens/expiring_worker_spec.rb @@ -11,7 +11,7 @@ let_it_be(:group) { create(:group) } let_it_be(:expiring_token) { create(:personal_access_token, user: project_bot, expires_at: 5.days.from_now) } let_it_be(:group_hook) { create(:group_hook, group: group, resource_access_token_events: true) } - let_it_be(:hook_data) { Gitlab::DataBuilder::ResourceAccessToken.build(expiring_token, :expiring, group) } + let_it_be(:hook_data) { Gitlab::DataBuilder::ResourceAccessTokenPayload.build(expiring_token, :expiring, group) } let(:fake_wh_service) { double } before_all do diff --git a/locale/gitlab.pot b/locale/gitlab.pot index fcac4882481d72..480cd0b8d767c3 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -45045,7 +45045,7 @@ msgstr "" msgid "ProjectSettings|A default branch cannot be chosen for an empty project." msgstr "" -msgid "ProjectSettings|Add additional webhook triggers for group access token expiry" +msgid "ProjectSettings|Add additional webhook triggers for group access token expiry." msgstr "" msgid "ProjectSettings|Add badges to display information about this project." @@ -45252,7 +45252,7 @@ msgstr "" msgid "ProjectSettings|If GitLab manages your cluster, then GitLab retains your analytics data for 1 year. %{link_start}Learn more about data retention policy%{link_end}." msgstr "" -msgid "ProjectSettings|If enabled, project access tokens expiry webhooks execute 60, 30, and 7 days before the token expires. If disabled, these webhooks only execute 7 days before the token expires." +msgid "ProjectSettings|If enabled, project access tokens expiry webhooks execute 60, 30, and 7 days before the token expires. If disabled, these webhooks only execute 7 days before the token expires. %{linkStart}Learn more%{linkEnd}" msgstr "" msgid "ProjectSettings|If merge trains are enabled, merging is only possible if the branch can be rebased without conflicts." -- GitLab From 2f1d0f9c9af91a75034a6f5110b166c56ca3cfe7 Mon Sep 17 00:00:00 2001 From: smriti Date: Thu, 30 Jan 2025 21:54:00 +0530 Subject: [PATCH 06/13] Review comments applied Review comments applied --- .../_extended_grat_expiry_webhook_execute.html.haml | 2 +- .../views/groups/settings/_permissions.html.haml_spec.rb | 6 ++++-- locale/gitlab.pot | 3 --- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml b/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml index 9f4073c4129358..846466963a7152 100644 --- a/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml +++ b/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml @@ -1,7 +1,7 @@ - return unless Feature.enabled?(:extended_expiry_webhook_execution_setting, group) - return unless group.licensed_feature_available?(:group_webhooks) -%h5= s_('GroupSettings|Extended Group Access Tokens Expiry Webhook execution') +%h5= s_('GroupSettings|Add additional webhook triggers for group access token expiration') .form-group.gl-mb-3 = f.gitlab_ui_checkbox_component :extended_grat_expiry_webhooks_execute, checkbox_options: { disabled: !can?(current_user, :admin_group, group), checked: group.extended_grat_expiry_webhooks_execute? } do |c| diff --git a/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb b/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb index fce3089d940695..7845d684130865 100644 --- a/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb +++ b/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb @@ -83,9 +83,12 @@ context 'for extended token expiry webhook execution setting' do let_it_be(:checkbox_label) { s_('GroupSettings|Add additional webhook triggers for group access token expiration') } + before do + allow(group).to receive(:licensed_feature_available?).and_return(true) + end + context 'when `group_webhooks` licensed feature is not available' do before do - allow(group).to receive(:licensed_feature_available?).and_return(true) allow(group).to receive(:licensed_feature_available?).with(:group_webhooks).and_return(false) end @@ -99,7 +102,6 @@ context 'when `group_webhooks` licensed feature is available' do before do - allow(group).to receive(:licensed_feature_available?).and_return(true) allow(group).to receive(:licensed_feature_available?).with(:group_webhooks).and_return(true) end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 480cd0b8d767c3..16e12b38b72430 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -28039,9 +28039,6 @@ msgstr "" msgid "GroupSettings|Export group" msgstr "" -msgid "GroupSettings|Extended Group Access Tokens Expiry Webhook execution" -msgstr "" - msgid "GroupSettings|Failed to save changes." msgstr "" -- GitLab From b54bc2afbcca7e6bb5f8b62e582c3244196973db Mon Sep 17 00:00:00 2001 From: smriti Date: Mon, 3 Feb 2025 17:33:43 +0530 Subject: [PATCH 07/13] Review comments addressed --- app/controllers/projects_controller.rb | 2 +- app/models/project.rb | 2 +- .../personal_access_tokens/expiring_worker.rb | 16 ++++++++++++---- ee/app/models/ee/project.rb | 4 ++-- ee/spec/models/ee/group_spec.rb | 2 +- ee/spec/models/ee/project_spec.rb | 6 +++--- .../settings/_permissions.html.haml_spec.rb | 12 +++++++++--- .../resource_access_token_payload.rb | 5 ++--- .../resource_access_token_payload_spec.rb | 5 ++--- spec/models/project_spec.rb | 4 ++-- 10 files changed, 35 insertions(+), 23 deletions(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index ed5d88c92b97eb..7d3ac2501e1066 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -55,7 +55,7 @@ class ProjectsController < Projects::ApplicationController push_force_frontend_feature_flag(:work_items_beta, @project&.work_items_beta_feature_flag_enabled?) push_force_frontend_feature_flag(:work_items_alpha, @project&.work_items_alpha_feature_flag_enabled?) # FF to enable setting to allow webhook execution on 30D and 60D notification delivery too - push_frontend_feature_flag(:extended_expiry_webhook_execution_setting, @project&.group) + push_frontend_feature_flag(:extended_expiry_webhook_execution_setting, @project&.namespace) push_frontend_feature_flag(:work_item_description_templates, @project&.group) end diff --git a/app/models/project.rb b/app/models/project.rb index c1b5b66c540db8..0aff3037e4d09c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1986,7 +1986,7 @@ def triggered_hooks(hooks_scope, data) # By default the webhook resource_access_token_hooks will execute for # seven_days interval but we have a setting to allow webhook execution - # for thirty_days and sixty_days_plus interval too. + # for thirty_days and sixty_days interval too. if hooks_scope == :resource_access_token_hooks && ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self.namespace) && data[:interval] != :seven_days && diff --git a/app/workers/personal_access_tokens/expiring_worker.rb b/app/workers/personal_access_tokens/expiring_worker.rb index da2a559011e5d2..7d6702c21a537b 100644 --- a/app/workers/personal_access_tokens/expiring_worker.rb +++ b/app/workers/personal_access_tokens/expiring_worker.rb @@ -132,8 +132,8 @@ def process_project_bot_tokens(interval = :seven_days) # If Feature flag is not enabled webhooks will only execute if # interval is seven_days if Feature.enabled?(:extended_expiry_webhook_execution_setting, - project_bot.resource_bot_resource) || interval == :seven_days - execute_web_hooks(project_bot, expiring_user_token, interval) + bot_resource_namepace(project_bot.resource_bot_resource)) || interval == :seven_days + execute_web_hooks(project_bot, expiring_user_token, { interval: interval }) end interval_days = PersonalAccessToken.notification_interval(interval) @@ -207,13 +207,13 @@ def log_exception(ex, user) ) end - def execute_web_hooks(bot_user, token, interval) + def execute_web_hooks(bot_user, token, data = {}) resource = bot_user.resource_bot_resource return unless resource return if resource.is_a?(Project) && !resource.has_active_hooks?(:resource_access_token_hooks) - hook_data = Gitlab::DataBuilder::ResourceAccessTokenPayload.build(token, :expiring, resource, interval) + hook_data = Gitlab::DataBuilder::ResourceAccessTokenPayload.build(token, :expiring, resource, data) resource.execute_hooks(hook_data, :resource_access_token_hooks) end @@ -221,5 +221,13 @@ def notification_service NotificationService.new end strong_memoize_attr :notification_service + + def bot_resource_namepace(resource) + if resource.is_a?(Project) + resource.namespace + else + resource + end + end end end diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index cecdb4d871d06a..9038f8ebadaf20 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -861,7 +861,7 @@ def triggered_hooks(scope, data) if ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self.namespace) && scope == :resource_access_token_hooks && data[:interval] != :seven_days - triggered.add_hooks(group_hooks_for_groups_with_extended_execution) + triggered.add_hooks(group_webhooks_including_extended_token_expiry) else triggered.add_hooks(group_hooks) end @@ -1528,7 +1528,7 @@ def group_hooks GroupHook.where(group_id: group.self_and_ancestors) end - def group_hooks_for_groups_with_extended_execution + def group_webhooks_including_extended_token_expiry return unless ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self.namespace) GroupHook.where(group_id: group.groups_for_extended_webhook_execution_on_token_expiry) diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb index 7cadef7f30dfc4..ff77af2977b18d 100644 --- a/ee/spec/models/ee/group_spec.rb +++ b/ee/spec/models/ee/group_spec.rb @@ -2970,7 +2970,7 @@ def webhook_headers stub_feature_flags(extended_expiry_webhook_execution_setting: false) end - it 'executes webhook since no setting is there to control the same' do + it 'executes the webhook since the setting does not apply' do group.namespace_settings.update!(extended_grat_expiry_webhooks_execute: false) expect(WebHookService).to receive(:new) diff --git a/ee/spec/models/ee/project_spec.rb b/ee/spec/models/ee/project_spec.rb index 7a7331128355cf..96924ce51419ff 100644 --- a/ee/spec/models/ee/project_spec.rb +++ b/ee/spec/models/ee/project_spec.rb @@ -1638,9 +1638,9 @@ describe "#execute_hooks" do context "group hooks" do - let!(:group) { create(:group) } - let!(:project) { create(:project, namespace: group) } - let!(:group_hook) { create(:group_hook, group: group, resource_access_token_events: true) } + let_it_be_with_reload(:group) { create(:group) } + let_it_be_with_reload(:project) { create(:project, namespace: group) } + let_it_be_with_reload(:group_hook) { create(:group_hook, group: group, resource_access_token_events: true) } it 'does not execute the hook when the feature is disabled' do stub_licensed_features(group_webhooks: false) diff --git a/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb b/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb index 7845d684130865..3e39654140f2cf 100644 --- a/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb +++ b/ee/spec/views/groups/settings/_permissions.html.haml_spec.rb @@ -96,7 +96,9 @@ render expect(rendered).to render_template('groups/settings/_extended_grat_expiry_webhook_execute') - expect(rendered).not_to have_content('Add additional webhook triggers for group access token expiration') + expect(rendered).not_to have_content( + s_('GroupSettings|Add additional webhook triggers for group access token expiration') + ) end end @@ -109,7 +111,9 @@ render expect(rendered).to render_template('groups/settings/_extended_grat_expiry_webhook_execute') - expect(rendered).to have_content('Add additional webhook triggers for group access token expiration') + expect(rendered).to have_content( + s_('GroupSettings|Add additional webhook triggers for group access token expiration') + ) expect(rendered).to have_unchecked_field(checkbox_label, type: 'checkbox') end @@ -122,7 +126,9 @@ render expect(rendered).to render_template('groups/settings/_extended_grat_expiry_webhook_execute') - expect(rendered).not_to have_content('Add additional webhook triggers for group access token expiration') + expect(rendered).not_to have_content( + s_('GroupSettings|Add additional webhook triggers for group access token expiration') + ) expect(rendered).not_to have_unchecked_field(checkbox_label, type: 'checkbox') end end diff --git a/lib/gitlab/data_builder/resource_access_token_payload.rb b/lib/gitlab/data_builder/resource_access_token_payload.rb index 85fb349dded766..9937253fd2989a 100644 --- a/lib/gitlab/data_builder/resource_access_token_payload.rb +++ b/lib/gitlab/data_builder/resource_access_token_payload.rb @@ -5,7 +5,7 @@ module DataBuilder module ResourceAccessTokenPayload extend self - def build(resource_access_token, event, resource, interval = :seven_days) + def build(resource_access_token, event, resource, data = {}) base_data = { object_kind: 'access_token' } @@ -13,8 +13,7 @@ def build(resource_access_token, event, resource, interval = :seven_days) base_data[resource.model_name.param_key.to_sym] = resource.hook_attrs base_data[:object_attributes] = resource_access_token.hook_attrs base_data[:event_name] = event_data(event) - base_data[:interval] = interval - base_data + base_data.merge(data) end private diff --git a/spec/lib/gitlab/data_builder/resource_access_token_payload_spec.rb b/spec/lib/gitlab/data_builder/resource_access_token_payload_spec.rb index c78027520a35fe..f84c4e7f23f939 100644 --- a/spec/lib/gitlab/data_builder/resource_access_token_payload_spec.rb +++ b/spec/lib/gitlab/data_builder/resource_access_token_payload_spec.rb @@ -5,14 +5,13 @@ RSpec.describe Gitlab::DataBuilder::ResourceAccessTokenPayload, feature_category: :system_access do let_it_be_with_reload(:user) { create(:user, :project_bot) } let(:event) { :expiring } - let(:interval) { :thirty_days } let(:personal_access_token) { create(:personal_access_token, user: user) } - let(:data) { described_class.build(personal_access_token, event, resource, interval) } + let(:data) { described_class.build(personal_access_token, event, resource, { interval: :thirty_days }) } shared_examples 'includes standard data' do specify do expect(data[:object_attributes]).to eq(personal_access_token.hook_attrs) - expect(data[:interval]).to eq(interval) + expect(data[:interval]).to eq(:thirty_days) expect(data[:object_kind]).to eq('access_token') end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 754c8a8b1a3041..583d5b11ac3814 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -6340,7 +6340,7 @@ def has_external_wiki let(:data) { { ref: 'refs/heads/master', data: 'data' } } let(:hook_scope) { :push_hooks } let(:hook) { create(:project_hook, merge_requests_events: false, push_events: true) } - let(:project) { create(:project) } + let_it_be_with_reload(:project) { create(:project) } shared_examples 'webhook is added to execution list' do it 'executes webhook succesfully' do @@ -6414,7 +6414,7 @@ def has_external_wiki end context 'when resource access token hooks for expiry notification' do - let!(:project) { create(:project) } + let_it_be_with_reload(:project) { create(:project) } let!(:hook) { create(:project_hook, project: project, resource_access_token_events: true) } let!(:hook_scope) { :resource_access_token_hooks } -- GitLab From 4894fa0317f907f4fd58c145b36818eddbb41afb Mon Sep 17 00:00:00 2001 From: smriti Date: Tue, 4 Feb 2025 11:37:11 +0530 Subject: [PATCH 08/13] Changes for expiring worker spec --- .../expiring_worker_spec.rb | 52 +++++++++++++------ 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/ee/spec/workers/personal_access_tokens/expiring_worker_spec.rb b/ee/spec/workers/personal_access_tokens/expiring_worker_spec.rb index bb4e50bee732b0..768e07204ac23f 100644 --- a/ee/spec/workers/personal_access_tokens/expiring_worker_spec.rb +++ b/ee/spec/workers/personal_access_tokens/expiring_worker_spec.rb @@ -11,28 +11,48 @@ let_it_be(:group) { create(:group) } let_it_be(:expiring_token) { create(:personal_access_token, user: project_bot, expires_at: 5.days.from_now) } let_it_be(:group_hook) { create(:group_hook, group: group, resource_access_token_events: true) } - let_it_be(:hook_data) { Gitlab::DataBuilder::ResourceAccessTokenPayload.build(expiring_token, :expiring, group) } + let_it_be(:interval_data) { { interval: :seven_days } } + let_it_be(:hook_data) do + Gitlab::DataBuilder::ResourceAccessTokenPayload.build(expiring_token, :expiring, group, interval_data) + end + let(:fake_wh_service) { double } before_all do group.add_developer(project_bot) end - it 'executes access token webhook' do - stub_licensed_features(group_webhooks: true) - expect(Gitlab::DataBuilder::ResourceAccessTokenPayload).to receive(:build).and_return(hook_data) - expect(fake_wh_service).to receive(:async_execute).once - - expect(WebHookService) - .to receive(:new) - .with( - group_hook, - hook_data, - 'resource_access_token_hooks', - idempotency_key: anything - ) { fake_wh_service } - - worker.perform + context 'when token payload is passed with data' do + before do + stub_licensed_features(group_webhooks: true) + end + + it 'executes access token webhook' do + expect(Gitlab::DataBuilder::ResourceAccessTokenPayload).to receive(:build).and_return(hook_data) + expect(fake_wh_service).to receive(:async_execute).once + + expect(WebHookService) + .to receive(:new) + .with( + group_hook, + hook_data, + 'resource_access_token_hooks', + idempotency_key: anything + ) { fake_wh_service } + + worker.perform + end + + it 'does not execute access token webhook if interval is not passed' do + hook_data = Gitlab::DataBuilder::ResourceAccessTokenPayload.build(expiring_token, :expiring, group) + + expect(Gitlab::DataBuilder::ResourceAccessTokenPayload).to receive(:build).and_return(hook_data) + + expect(WebHookService) + .not_to receive(:new) + + worker.perform + end end end end -- GitLab From 5149104c2b1f3dffb06c7c9f7108c3d11a973cd8 Mon Sep 17 00:00:00 2001 From: smriti Date: Wed, 5 Feb 2025 14:45:15 +0530 Subject: [PATCH 09/13] Correct method chain formatting --- .../personal_access_tokens/expiring_worker.rb | 8 +-- ee/spec/models/ee/group_spec.rb | 58 ++++++++++++------- ee/spec/models/ee/project_spec.rb | 48 ++++++++++----- spec/models/project_spec.rb | 12 +++- 4 files changed, 84 insertions(+), 42 deletions(-) diff --git a/app/workers/personal_access_tokens/expiring_worker.rb b/app/workers/personal_access_tokens/expiring_worker.rb index 7d6702c21a537b..d81d866afbbd80 100644 --- a/app/workers/personal_access_tokens/expiring_worker.rb +++ b/app/workers/personal_access_tokens/expiring_worker.rb @@ -129,10 +129,10 @@ def process_project_bot_tokens(interval = :seven_days) # project bot does not have more than 1 token expiring_user_token = project_bot.personal_access_tokens.first - # If Feature flag is not enabled webhooks will only execute if - # interval is seven_days - if Feature.enabled?(:extended_expiry_webhook_execution_setting, - bot_resource_namepace(project_bot.resource_bot_resource)) || interval == :seven_days + # If feature flag is not enabled webhooks will only execute if interval is seven_days + resource_namespace = bot_resource_namepace(project_bot.resource_bot_resource) + if Feature.enabled?(:extended_expiry_webhook_execution_setting, resource_namespace) || + interval == :seven_days execute_web_hooks(project_bot, expiring_user_token, { interval: interval }) end diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb index ff77af2977b18d..69f67dd2f79f15 100644 --- a/ee/spec/models/ee/group_spec.rb +++ b/ee/spec/models/ee/group_spec.rb @@ -2783,10 +2783,14 @@ shared_examples 'enabled group hooks' do context 'execution' do it 'executes the hook for self and ancestor groups by default' do - expect(WebHookService).to receive(:new) - .with(group_hook, data, 'member_hooks', idempotency_key: anything).and_call_original - expect(WebHookService).to receive(:new) - .with(parent_group_hook, data, 'member_hooks', idempotency_key: anything).and_call_original + expect(WebHookService) + .to receive(:new) + .with(group_hook, data, 'member_hooks', idempotency_key: anything) + .and_call_original + expect(WebHookService) + .to receive(:new) + .with(parent_group_hook, data, 'member_hooks', idempotency_key: anything) + .and_call_original group.execute_hooks(data, :member_hooks) end @@ -2955,9 +2959,10 @@ def webhook_headers let(:data) { { interval: :seven_days } } it 'executes webhook' do - expect(WebHookService).to receive(:new) - .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) - .and_call_original + expect(WebHookService) + .to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) + .and_call_original group.execute_hooks(data, :resource_access_token_hooks) end @@ -2973,9 +2978,10 @@ def webhook_headers it 'executes the webhook since the setting does not apply' do group.namespace_settings.update!(extended_grat_expiry_webhooks_execute: false) - expect(WebHookService).to receive(:new) - .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) - .and_call_original + expect(WebHookService) + .to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) + .and_call_original group.execute_hooks(data, :resource_access_token_hooks) end @@ -3016,9 +3022,10 @@ def webhook_headers let(:data) { { interval: :thirty_days } } it 'executes webhook' do - expect(WebHookService).to receive(:new) - .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) - .and_call_original + expect(WebHookService) + .to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) + .and_call_original group.execute_hooks(data, :resource_access_token_hooks) end @@ -3028,9 +3035,10 @@ def webhook_headers let(:data) { { interval: :sixty_days } } it 'executes webhook' do - expect(WebHookService).to receive(:new) - .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) - .and_call_original + expect(WebHookService) + .to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) + .and_call_original group.execute_hooks(data, :resource_access_token_hooks) end @@ -3053,11 +3061,13 @@ def webhook_headers end it 'executes webhook for subgroup and not parent group' do - expect(WebHookService).to receive(:new) + expect(WebHookService) + .to receive(:new) .with(subgroup_hook, data, 'resource_access_token_hooks', idempotency_key: anything) .and_call_original - expect(WebHookService).not_to receive(:new) + expect(WebHookService) + .not_to receive(:new) .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) .and_call_original @@ -3089,11 +3099,13 @@ def webhook_headers end it 'executes webhook for subgroup and not parent group' do - expect(WebHookService).to receive(:new) + expect(WebHookService) + .to receive(:new) .with(subgroup_hook, data, 'resource_access_token_hooks', idempotency_key: anything) .and_call_original - expect(WebHookService).to receive(:new) + expect(WebHookService) + .to receive(:new) .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) .and_call_original @@ -3107,11 +3119,13 @@ def webhook_headers end it 'does not execute webhook for subgroup and not parent group' do - expect(WebHookService).not_to receive(:new) + expect(WebHookService) + .not_to receive(:new) .with(subgroup_hook, data, 'resource_access_token_hooks', idempotency_key: anything) .and_call_original - expect(WebHookService).to receive(:new) + expect(WebHookService) + .to receive(:new) .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) .and_call_original diff --git a/ee/spec/models/ee/project_spec.rb b/ee/spec/models/ee/project_spec.rb index 96924ce51419ff..f5b7d64d9258d8 100644 --- a/ee/spec/models/ee/project_spec.rb +++ b/ee/spec/models/ee/project_spec.rb @@ -1391,7 +1391,9 @@ end it 'returns true when the resolver returns true' do - expect(ComplianceManagement::MergeRequestApprovalSettings::Resolver).to receive(:new).with(root_ancestor, project: project) + expect(ComplianceManagement::MergeRequestApprovalSettings::Resolver) + .to receive(:new) + .with(root_ancestor, project: project) allow_next_instance_of(ComplianceManagement::MergeRequestApprovalSettings::Resolver) do |resolver| # internally still maps to require_password_to_approve so mock that call @@ -1407,7 +1409,9 @@ end it 'returns false when the resolver returns false' do - expect(ComplianceManagement::MergeRequestApprovalSettings::Resolver).to receive(:new).with(root_ancestor, project: project) + expect(ComplianceManagement::MergeRequestApprovalSettings::Resolver) + .to receive(:new) + .with(root_ancestor, project: project) allow_next_instance_of(ComplianceManagement::MergeRequestApprovalSettings::Resolver) do |resolver| # internally still maps to require_password_to_approve so mock that call @@ -1432,7 +1436,9 @@ end it 'returns true when the resolver returns true' do - expect(ComplianceManagement::MergeRequestApprovalSettings::Resolver).to receive(:new).with(root_ancestor, project: project) + expect(ComplianceManagement::MergeRequestApprovalSettings::Resolver) + .to receive(:new) + .with(root_ancestor, project: project) allow_next_instance_of(ComplianceManagement::MergeRequestApprovalSettings::Resolver) do |resolver| allow(resolver).to receive(:require_password_to_approve) @@ -1447,7 +1453,9 @@ end it 'returns false when the resolver returns false' do - expect(ComplianceManagement::MergeRequestApprovalSettings::Resolver).to receive(:new).with(root_ancestor, project: project) + expect(ComplianceManagement::MergeRequestApprovalSettings::Resolver) + .to receive(:new) + .with(root_ancestor, project: project) allow_next_instance_of(ComplianceManagement::MergeRequestApprovalSettings::Resolver) do |resolver| allow(resolver).to receive(:require_password_to_approve) @@ -1682,8 +1690,10 @@ let(:data) { { interval: :seven_days } } it 'executes webhook' do - expect(WebHookService).to receive(:new) - .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_return(wh_service) + expect(WebHookService) + .to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) + .and_return(wh_service) project.execute_hooks(data, :resource_access_token_hooks) end @@ -1697,8 +1707,10 @@ end it 'adds webhook to the execution list since no setting is there' do - expect(WebHookService).to receive(:new) - .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_return(wh_service) + expect(WebHookService) + .to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) + .and_return(wh_service) project.execute_hooks(data, :resource_access_token_hooks) end @@ -1739,8 +1751,10 @@ let(:data) { { interval: :thirty_days } } it 'executes webhook' do - expect(WebHookService).to receive(:new) - .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_return(wh_service) + expect(WebHookService) + .to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) + .and_return(wh_service) project.execute_hooks(data, :resource_access_token_hooks) end @@ -1750,8 +1764,10 @@ let(:data) { { interval: :sixty_days } } it 'executes webhook' do - expect(WebHookService).to receive(:new) - .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything).and_return(wh_service) + expect(WebHookService) + .to receive(:new) + .with(group_hook, data, 'resource_access_token_hooks', idempotency_key: anything) + .and_return(wh_service) project.execute_hooks(data, :resource_access_token_hooks) end @@ -2218,7 +2234,9 @@ end it 'returns false when the resolver returns true' do - expect(ComplianceManagement::MergeRequestApprovalSettings::Resolver).to receive(:new).with(root_ancestor, project: project) + expect(ComplianceManagement::MergeRequestApprovalSettings::Resolver) + .to receive(:new) + .with(root_ancestor, project: project) allow_next_instance_of(ComplianceManagement::MergeRequestApprovalSettings::Resolver) do |resolver| allow(resolver).to receive(:retain_approvals_on_push) @@ -2233,7 +2251,9 @@ end it 'returns true when the resolver returns false' do - expect(ComplianceManagement::MergeRequestApprovalSettings::Resolver).to receive(:new).with(root_ancestor, project: project) + expect(ComplianceManagement::MergeRequestApprovalSettings::Resolver) + .to receive(:new) + .with(root_ancestor, project: project) allow_next_instance_of(ComplianceManagement::MergeRequestApprovalSettings::Resolver) do |resolver| allow(resolver).to receive(:retain_approvals_on_push) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 583d5b11ac3814..f7400913509f84 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -6997,7 +6997,11 @@ def has_external_wiki subject(:pages_url) { project.pages_url(pages_url_config) } it "lets URL builder handle the URL resolution" do - expect(::Gitlab::Pages::UrlBuilder).to receive(:new).with(project, pages_url_config).and_return(url_builder) + expect(::Gitlab::Pages::UrlBuilder) + .to receive(:new) + .with(project, pages_url_config) + .and_return(url_builder) + expect(url_builder).to receive(:pages_url).and_return('http://namespace1.example.com/project-1/foo') expect(pages_url).to eq('http://namespace1.example.com/project-1/foo') end @@ -7006,7 +7010,11 @@ def has_external_wiki let(:pages_url_config) { { path_prefix: 'foo' } } it "lets URL builder handle the URL resolution" do - expect(::Gitlab::Pages::UrlBuilder).to receive(:new).with(project, pages_url_config).and_return(url_builder) + expect(::Gitlab::Pages::UrlBuilder) + .to receive(:new) + .with(project, pages_url_config) + .and_return(url_builder) + expect(url_builder).to receive(:pages_url).and_return('http://namespace1.example.com/project-1/foo') expect(pages_url).to eq('http://namespace1.example.com/project-1/foo') end -- GitLab From 53c57180a2a9e67cf574c40d779d266d604d03dc Mon Sep 17 00:00:00 2001 From: smriti Date: Thu, 6 Feb 2025 21:54:57 +0530 Subject: [PATCH 10/13] Increasing threshold to avoid flaky test --- ee/spec/features/projects/issues/user_creates_issue_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/features/projects/issues/user_creates_issue_spec.rb b/ee/spec/features/projects/issues/user_creates_issue_spec.rb index a38e6393ce35fc..40ca1d5f96c68a 100644 --- a/ee/spec/features/projects/issues/user_creates_issue_spec.rb +++ b/ee/spec/features/projects/issues/user_creates_issue_spec.rb @@ -110,7 +110,7 @@ it 'creates an issue with an epic' do # TODO: remove threshold after epic-work item sync # issue: https://gitlab.com/gitlab-org/gitlab/-/issues/438295 - allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(140) + allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(145) click_button 'Select epic' click_on epic.title -- GitLab From 2d19402ccc252ce16baa7c918f22c212a9f36869 Mon Sep 17 00:00:00 2001 From: smriti Date: Sat, 8 Feb 2025 20:31:00 +0530 Subject: [PATCH 11/13] Review comments addressed Rebase issues resolved Rebase issues resolved Rebase issues resolved Rebase issues resolved --- .../permissions/components/settings_panel.vue | 233 +++++++++--------- .../projects/shared/permissions/constants.js | 2 +- ee/app/models/ee/group.rb | 10 +- ee/app/models/ee/project.rb | 2 - ee/spec/models/ee/group_spec.rb | 4 +- .../expiring_worker_spec.rb | 3 +- 6 files changed, 125 insertions(+), 129 deletions(-) diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue index 3632197dc89f7a..372aea721badd9 100644 --- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue +++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue @@ -98,10 +98,10 @@ export default { confirmButtonText: __('Save changes'), emailsLabel: s__('ProjectSettings|Email notifications'), extendedPratExpiryWebhooksExecuteLabel: s__( - 'ProjectSettings|Add additional webhook triggers for group access token expiry', + 'ProjectSettings|Add additional webhook triggers for group access token expiry.', ), extendedPratExpiryWebhooksExecuteHelpText: s__( - 'ProjectSettings|If enabled, project access tokens expiry webhooks execute 60, 30, and 7 days before the token expires. If disabled, these webhooks only execute 7 days before the token expires.', + 'ProjectSettings|If enabled, project access tokens expiry webhooks execute 60, 30, and 7 days before the token expires. If disabled, these webhooks only execute 7 days before the token expires. %{linkStart}Learn more%{linkEnd}', ), showDiffPreviewLabel: s__('ProjectSettings|Include diff previews'), showDiffPreviewHelpText: s__( @@ -1062,142 +1062,139 @@ export default { /> - - - + + - - {{ $options.i18n.showDiffPreviewLabel }} - + + {{ $options.i18n.pucWarningLabel }} + - - - - - {{ s__('ProjectSettings|Show default emoji reactions') }} - - - - - - - {{ $options.i18n.pucWarningLabel }} - - - - - - - {{ $options.i18n.extendedPratExpiryWebhooksExecuteLabel }} - - - - - - - - - - - - - {{ $options.i18n.confirmButtonText }} - - + + + + + + + + + + + diff --git a/app/assets/javascripts/pages/projects/shared/permissions/constants.js b/app/assets/javascripts/pages/projects/shared/permissions/constants.js index c2121623073f7c..80eb524bb1e913 100644 --- a/app/assets/javascripts/pages/projects/shared/permissions/constants.js +++ b/app/assets/javascripts/pages/projects/shared/permissions/constants.js @@ -52,7 +52,7 @@ export const modelExperimentsHelpPath = helpPagePath( export const modelRegistryHelpPath = helpPagePath('user/project/ml/model_registry/_index.md'); export const extendedPratExpiryWebhooksExecuteHelpPath = helpPagePath( - 'user/project/settings/index.md', + 'user/project/settings/_index.md', { anchor: 'add-additional-webhook-triggers-for-project-access-token-expiration' }, ); diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index 007a1e40ea1ca3..16647c1a7dbf94 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -835,9 +835,11 @@ def execute_hooks(data, hooks_scope) # By default the webhook resource_access_token_hooks will execute for # seven_days interval but we have a setting to allow webhook execution # for thirty_days and sixty_days_plus interval too. - group_hooks = if ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self) && - hooks_scope == :resource_access_token_hooks && - (data[:interval] != :seven_days) + is_extended_expiry_webhook = hooks_scope == :resource_access_token_hooks && + data[:interval] != :seven_days && + ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self) + + group_hooks = if is_extended_expiry_webhook GroupHook.where(group_id: groups_for_extended_webhook_execution_on_token_expiry) else GroupHook.where(group_id: self_and_ancestors) @@ -1128,7 +1130,7 @@ def enable_auto_assign_gitlab_duo_pro_seats? end def groups_for_extended_webhook_execution_on_token_expiry - return unless ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self) + return Group.none unless ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self) self_and_ancestors .joins(:namespace_settings) diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index 9038f8ebadaf20..8a392295d9e97f 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -1529,8 +1529,6 @@ def group_hooks end def group_webhooks_including_extended_token_expiry - return unless ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self.namespace) - GroupHook.where(group_id: group.groups_for_extended_webhook_execution_on_token_expiry) end diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb index 69f67dd2f79f15..c6ae2dda418212 100644 --- a/ee/spec/models/ee/group_spec.rb +++ b/ee/spec/models/ee/group_spec.rb @@ -3098,7 +3098,7 @@ def webhook_headers subgroup.namespace_settings.update!(extended_grat_expiry_webhooks_execute: true) end - it 'executes webhook for subgroup and not parent group' do + it 'executes webhook both for subgroup and parent group' do expect(WebHookService) .to receive(:new) .with(subgroup_hook, data, 'resource_access_token_hooks', idempotency_key: anything) @@ -3118,7 +3118,7 @@ def webhook_headers subgroup.namespace_settings.update!(extended_grat_expiry_webhooks_execute: false) end - it 'does not execute webhook for subgroup and not parent group' do + it 'does not execute webhook for subgroup, but does execute for parent group' do expect(WebHookService) .not_to receive(:new) .with(subgroup_hook, data, 'resource_access_token_hooks', idempotency_key: anything) diff --git a/ee/spec/workers/personal_access_tokens/expiring_worker_spec.rb b/ee/spec/workers/personal_access_tokens/expiring_worker_spec.rb index 768e07204ac23f..7d25c7a14e93f8 100644 --- a/ee/spec/workers/personal_access_tokens/expiring_worker_spec.rb +++ b/ee/spec/workers/personal_access_tokens/expiring_worker_spec.rb @@ -48,8 +48,7 @@ expect(Gitlab::DataBuilder::ResourceAccessTokenPayload).to receive(:build).and_return(hook_data) - expect(WebHookService) - .not_to receive(:new) + expect(WebHookService).not_to receive(:new) worker.perform end -- GitLab From 2c9e502b7108ed965470ca4967564c04575f2a81 Mon Sep 17 00:00:00 2001 From: smriti Date: Tue, 11 Feb 2025 09:42:53 +0530 Subject: [PATCH 12/13] Pipeline failures resolved --- db/structure.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/structure.sql b/db/structure.sql index de02b8cd598af6..32f95a3482d704 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -16885,8 +16885,8 @@ CREATE TABLE namespace_settings ( resource_access_token_notify_inherited boolean, lock_resource_access_token_notify_inherited boolean DEFAULT false NOT NULL, pipeline_variables_default_role smallint DEFAULT 2 NOT NULL, - extended_grat_expiry_webhooks_execute boolean DEFAULT false NOT NULL, force_pages_access_control boolean DEFAULT false NOT NULL, + extended_grat_expiry_webhooks_execute boolean DEFAULT false NOT NULL, CONSTRAINT check_0ba93c78c7 CHECK ((char_length(default_branch_name) <= 255)), CONSTRAINT namespace_settings_unique_project_download_limit_alertlist_size CHECK ((cardinality(unique_project_download_limit_alertlist) <= 100)), CONSTRAINT namespace_settings_unique_project_download_limit_allowlist_size CHECK ((cardinality(unique_project_download_limit_allowlist) <= 100)) -- GitLab From bf8040d9aea79d30a1e6dbdf726bb6e2394f1e44 Mon Sep 17 00:00:00 2001 From: smriti Date: Tue, 11 Feb 2025 18:26:21 +0530 Subject: [PATCH 13/13] Corrected the text in view Corrected the text in view --- .../projects/shared/permissions/components/settings_panel.vue | 2 +- locale/gitlab.pot | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue index 372aea721badd9..eeaa7c7f254bdb 100644 --- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue +++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue @@ -98,7 +98,7 @@ export default { confirmButtonText: __('Save changes'), emailsLabel: s__('ProjectSettings|Email notifications'), extendedPratExpiryWebhooksExecuteLabel: s__( - 'ProjectSettings|Add additional webhook triggers for group access token expiry.', + 'ProjectSettings|Add additional webhook triggers for project access token expiry.', ), extendedPratExpiryWebhooksExecuteHelpText: s__( 'ProjectSettings|If enabled, project access tokens expiry webhooks execute 60, 30, and 7 days before the token expires. If disabled, these webhooks only execute 7 days before the token expires. %{linkStart}Learn more%{linkEnd}', diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 16e12b38b72430..d2836cb5b86b29 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -45042,7 +45042,7 @@ msgstr "" msgid "ProjectSettings|A default branch cannot be chosen for an empty project." msgstr "" -msgid "ProjectSettings|Add additional webhook triggers for group access token expiry." +msgid "ProjectSettings|Add additional webhook triggers for project access token expiry." msgstr "" msgid "ProjectSettings|Add badges to display information about this project." -- GitLab