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 4b5cbc6dd38ee8648d22055ef750e9fbef7f20db..eeaa7c7f254bdbb2cbd19aa8397b1bbb354aa7bd 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 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}', + ), 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,30 @@ export default { + + + + {{ $options.i18n.extendedPratExpiryWebhooksExecuteLabel }} + + + diff --git a/app/assets/javascripts/pages/projects/shared/permissions/constants.js b/app/assets/javascripts/pages/projects/shared/permissions/constants.js index fe287998f4968d7792ea37e5c62be67314eca743..80eb524bb1e913d32f9be0118c1ea5d074dc6cac 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/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 08c6dc3f9a02a3377ad16020feaf58a794fefb43..7d3ac2501e10666d4699c234c24a6997402e9a1a 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -54,6 +54,8 @@ 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&.namespace) push_frontend_feature_flag(:work_item_description_templates, @project&.group) end @@ -460,7 +462,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 +471,13 @@ def project_setting_attributes enforce_auth_checks_on_uploads emails_enabled ] + + if ::Feature.enabled?(:extended_expiry_webhook_execution_setting, @project&.namespace) && + 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 e019d78fe1d04532f39dca547680b751a287d976..e6134d729ed310fe8a3ee61665bb23259c7232c4 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 cbace08c55fd6003e37ff5d22982dd0662e71b3e..0aff3037e4d09cb33762b0b44f37ea93a12a3884 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 @@ -1978,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 interval too. + if hooks_scope == :resource_access_token_hooks && + ::Feature.enabled?(:extended_expiry_webhook_execution_setting, self.namespace) && + 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/services/concerns/integrations/project_test_data.rb b/app/services/concerns/integrations/project_test_data.rb index b029717586b6adf414296c8a03d196fba558aeaf..21c3442427511cd9dcb8c85f44ac7116a9f6ac27 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/app/views/groups/settings/_permissions.html.haml b/app/views/groups/settings/_permissions.html.haml index 56db71a5c3b394a5c7371747e5863d8f42733dd7..87287ae91339e9e227108d181424488614c64c72 100644 --- a/app/views/groups/settings/_permissions.html.haml +++ b/app/views/groups/settings/_permissions.html.haml @@ -39,6 +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 + = 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 c6d48a3a8dfcf91bb085140fdcd01e9a195632d3..d81d866afbbd800afdc8667cda9228ca66fad101 100644 --- a/app/workers/personal_access_tokens/expiring_worker.rb +++ b/app/workers/personal_access_tokens/expiring_worker.rb @@ -129,9 +129,12 @@ 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 - execute_web_hooks(project_bot, expiring_user_token) if 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 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, 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::ResourceAccessToken.build(token, :expiring, resource) + hook_data = Gitlab::DataBuilder::ResourceAccessTokenPayload.build(token, :expiring, resource, data) resource.execute_hooks(hook_data, :resource_access_token_hooks) end @@ -218,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/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 new file mode 100644 index 0000000000000000000000000000000000000000..5d73d12d5808a40a7b69654909622d06c4c5cade --- /dev/null +++ b/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/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 0000000000000000000000000000000000000000..2dcde52a094fb15f7deb269bde3f2508ae321e8f --- /dev/null +++ b/db/migrate/20250108062227_add_extended_grat_expiry_webhook_execute_to_namespace_settings.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class AddExtendedGratExpiryWebhookExecuteToNamespaceSettings < Gitlab::Database::Migration[2.2] + enable_lock_retries! + milestone '17.9' + + 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 0000000000000000000000000000000000000000..f739f87b16df8852ae056bc8c0b02b1617e9261d --- /dev/null +++ b/db/migrate/20250108062256_add_extended_prat_expiry_webhook_execute_to_project_settings.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class AddExtendedPratExpiryWebhookExecuteToProjectSettings < Gitlab::Database::Migration[2.2] + enable_lock_retries! + milestone '17.9' + + 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 0000000000000000000000000000000000000000..f1db3e2b49a7c0f797169e8198e7bf8f809bda6f --- /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 0000000000000000000000000000000000000000..89024924e2d5d45a85b469b8aa2a544db97c1a48 --- /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 6d18836c937ae3db83c8507fac8347e74c8fef85..32f95a3482d7045ce0767659bb91c3a8178edeed 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -16886,6 +16886,7 @@ CREATE TABLE namespace_settings ( lock_resource_access_token_notify_inherited boolean DEFAULT false NOT NULL, pipeline_variables_default_role smallint DEFAULT 2 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)) @@ -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 b47a93537332647fdff5f9db5bd60a0ae63121b5..bd46ac8a4e077454b5e2e1933489ffbc669a81ea 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). +## Add additional webhook triggers for group access token expiration + +> - [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 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 can trigger these webhooks 60 days and 30 days before the token expires. + +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**. +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/doc/user/project/settings/_index.md b/doc/user/project/settings/_index.md index e2a15f69b2f8ec3160619c7769d7ec14bd767a97..7855653f7f4ffe7db89d2b482f2d76dc305e9bbd 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**. + +## Add additional webhook triggers for project access token expiration + +> - [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 availability of this feature is controlled by a feature flag. For more information, see the history. + +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. + +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**. +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/controllers/concerns/ee/groups/params.rb b/ee/app/controllers/concerns/ee/groups/params.rb index 32ede0b7204f3a95b6d8be0597d4644345ad43a7..89fcd7d7b227bd23cb1f9865990cee2787efd2d8 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 13de06a8d0d8fc48d09c299f9dfc108ab73f12ad..16647c1a7dbf947a4da38381ad00769e90ab565a 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 @@ -830,10 +832,20 @@ 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| - hook.async_execute(data, hooks_scope.to_s) - 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. + 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) + end + + execute_async_hooks(group_hooks, hooks_scope, data) end override :git_transfer_in_progress? @@ -1099,6 +1111,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| @@ -1112,8 +1129,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 Group.none 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 d3448eca50ff28757ee7d893b4996dd854163bd6..8a392295d9e97f6716e384d0990f9f52484f4409 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -856,7 +856,16 @@ 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, self.namespace) && + scope == :resource_access_token_hooks && + data[:interval] != :seven_days + triggered.add_hooks(group_webhooks_including_extended_token_expiry) + else + triggered.add_hooks(group_hooks) + end + end triggered end @@ -1519,6 +1528,10 @@ def group_hooks GroupHook.where(group_id: group.self_and_ancestors) end + def group_webhooks_including_extended_token_expiry + 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 new file mode 100644 index 0000000000000000000000000000000000000000..846466963a71528e28f2c61917cffb75e543f15a --- /dev/null +++ b/ee/app/views/groups/settings/_extended_grat_expiry_webhook_execute.html.haml @@ -0,0 +1,12 @@ +- return unless Feature.enabled?(:extended_expiry_webhook_execution_setting, group) +- return unless group.licensed_feature_available?(:group_webhooks) + +%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| + - c.with_label do + = 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/features/projects/issues/user_creates_issue_spec.rb b/ee/spec/features/projects/issues/user_creates_issue_spec.rb index a38e6393ce35fc98589591b3fdb70aebad322cb1..40ca1d5f96c68aa5b0a1e0e0f92ea2fdc92169dd 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 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 829ca367b4ec2d01c97b7419052f2d726d09f312..04fe4170d96bb9729721f9c39ddd028191bf37c5 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 c6d3c33890f0d2e68d5f467992b9574a958876eb..c6ae2dda418212d95c42b1b543c5dd27aa444faf 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 @@ -2943,6 +2947,195 @@ def webhook_headers end end + 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) } + + 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 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 + + 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 both for subgroup and 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, but does execute for 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 @@ -4347,6 +4540,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/project_spec.rb b/ee/spec/models/ee/project_spec.rb index ac2b54f0444d20b56d96e373d27cbc4926ed2367..f5b7d64d9258d8a41e077d1a4e8abdcb0cf421e2 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) @@ -1638,9 +1646,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_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) @@ -1675,6 +1683,98 @@ end end + context 'when resource access token hooks for expiry notification' 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') } @@ -2134,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) @@ -2149,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/ee/spec/requests/groups_controller_spec.rb b/ee/spec/requests/groups_controller_spec.rb index 8cf05bdd762925ee3605d0cd816abe6593b3bd59..b4769698f4792474640a0f48f8b9ed9d8d8ce641 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 bb444dc6b91e6fa418b209282d761ba31e9788c3..3e39654140f2cf6f8e351e450ce273cb6742f4d9 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,61 @@ end end + 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?).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( + s_('GroupSettings|Add additional webhook triggers for group access token expiration') + ) + end + end + + context 'when `group_webhooks` licensed feature is available' do + before do + 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( + s_('GroupSettings|Add additional webhook triggers for group access token expiration') + ) + 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).to render_template('groups/settings/_extended_grat_expiry_webhook_execute') + 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 + 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/ee/spec/workers/personal_access_tokens/expiring_worker_spec.rb b/ee/spec/workers/personal_access_tokens/expiring_worker_spec.rb index 5d851685e9595a1e7765ee3469651a90021d2fb4..7d25c7a14e93f8ef9dde45d5ced6fa7eecf90ac0 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,47 @@ 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(: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::ResourceAccessToken).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 diff --git a/lib/gitlab/data_builder/resource_access_token.rb b/lib/gitlab/data_builder/resource_access_token_payload.rb similarity index 79% rename from lib/gitlab/data_builder/resource_access_token.rb rename to lib/gitlab/data_builder/resource_access_token_payload.rb index eba5156658ef9fcc616ab750b113b3763450bb64..9937253fd2989a7010d40cd65c44c068144b8021 100644 --- a/lib/gitlab/data_builder/resource_access_token.rb +++ b/lib/gitlab/data_builder/resource_access_token_payload.rb @@ -2,10 +2,10 @@ module Gitlab module DataBuilder - module ResourceAccessToken + module ResourceAccessTokenPayload extend self - def build(resource_access_token, event, resource) + def build(resource_access_token, event, resource, data = {}) base_data = { object_kind: 'access_token' } @@ -13,7 +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 + base_data.merge(data) end private diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 2c64ef65576c40999c9dfd5d85bf8a129ec83636..d2836cb5b86b29b6bc14b72ee27061a21b7ea1b8 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 "" @@ -28060,6 +28063,9 @@ msgstr "" msgid "GroupSettings|If enabled, enterprise user accounts will not be able to use personal access tokens." msgstr "" +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." msgstr "" @@ -45036,6 +45042,9 @@ msgstr "" msgid "ProjectSettings|A default branch cannot be chosen for an empty project." msgstr "" +msgid "ProjectSettings|Add additional webhook triggers for project access token expiry." +msgstr "" + msgid "ProjectSettings|Add badges to display information about this project." msgstr "" @@ -45240,6 +45249,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 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." msgstr "" diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 757f85df70ec4659306ea0a67e354d11c8793897..b57687137dc3c5421dd093d3b4d00c37bc2ecad5 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/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js index 9ef2a2c1a5a59989676e2fe8390e48a5fdbbcd82..1f82a7d3c6c948acac1dcce1234c410328ed0d9b 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,10 @@ 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 +824,24 @@ 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/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index d18f086b44084775a47685fbf73ec01bfa24b039..5fc3947cf7cbb9460ac44e3de2950ce6e83a56d1 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_payload_spec.rb similarity index 88% 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 05518cd403fe840d48c4d98e0b5fe713ee6f6859..f84c4e7f23f93910fd7e5d3f4cc8f065c579ab88 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,15 +2,16 @@ 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(: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: :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(: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 f239adfa39269671db33965b8c5bf2dbb17859b6..f7400913509f84cc58355d564f2eb8e8d22881f9 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 } @@ -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) } @@ -6330,19 +6338,24 @@ 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) } + let_it_be_with_reload(:project) { create(:project) } - 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]) @@ -6399,6 +6412,72 @@ def has_external_wiki end end.not_to raise_error # Sidekiq::Worker::EnqueueFromTransactionError end + + context 'when resource access token hooks for expiry notification' do + 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 } + + 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 @@ -6918,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 @@ -6927,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 diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index b3b5a26e40e78b0f086f374bb40d1f753b15d41d..cd571064dcb1a479f33dac88e317ddbfd3c19f0f 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/services/test_hooks/project_service_spec.rb b/spec/services/test_hooks/project_service_spec.rb index 7bc86f3046c9cbf89bcdee5e38336c126b76c6cc..1e06911f8f799cb2dd1613fc5519420e9ccd2b66 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 db623b8b7daa18818101faa66537372df3aebf53..e4fc6204fb6a91cf43692c2e4fb68f4624c896d5 100644 --- a/spec/workers/personal_access_tokens/expiring_worker_spec.rb +++ b/spec/workers/personal_access_tokens/expiring_worker_spec.rb @@ -248,17 +248,17 @@ 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) + 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( project_hook, - {}, + { interval: :seven_days }, 'resource_access_token_hooks', idempotency_key: anything ) { fake_wh_service } @@ -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::ResourceAccessTokenPayload).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) }