diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 2e0e611f8011feec824bd84d5dd096c9d1dd908e..edc17a741e0622254798daf325f2144835bbb247 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -192,7 +192,11 @@ def transfer # rubocop: enable CodeReuse/ActiveRecord def export - export_service = Groups::ImportExport::ExportService.new(group: @group, user: current_user) + export_service = Groups::ImportExport::ExportService.new( + group: @group, + user: current_user, + exported_by_admin: current_user.can_admin_all_resources? + ) if export_service.async_execute redirect_to edit_group_path(@group), notice: _('Group export started. A download link will be sent by email and made available on this page.') diff --git a/app/models/project.rb b/app/models/project.rb index bb2bb9a1854d30d4c11a98ac5f4ac51ccc3a8890..27b45932d83267a94eb57fba5bc4e47575be4c0d 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2343,6 +2343,8 @@ def pipeline_status def add_export_job(current_user:, after_export_strategy: nil, params: {}) check_project_export_limit! + params[:exported_by_admin] = current_user.can_admin_all_resources? + job_id = if Feature.enabled?(:parallel_project_export, current_user) Projects::ImportExport::CreateRelationExportsWorker .perform_async(current_user.id, self.id, after_export_strategy, params) diff --git a/app/models/project_export_job.rb b/app/models/project_export_job.rb index 65f40732bf83f7ccf55bf0ca6f8cd9fb78a2b3db..18f40255932cc99cdca71762853ab337d2985987 100644 --- a/app/models/project_export_job.rb +++ b/app/models/project_export_job.rb @@ -2,10 +2,12 @@ class ProjectExportJob < ApplicationRecord include EachBatch + include AfterCommitQueue EXPIRES_IN = 7.days belongs_to :project + belongs_to :user has_many :relation_exports, class_name: 'Projects::ImportExport::RelationExport' validates :project, :jid, :status, presence: true @@ -37,5 +39,28 @@ class ProjectExportJob < ApplicationRecord state :started, value: STATUS[:started] state :finished, value: STATUS[:finished] state :failed, value: STATUS[:failed] + + after_transition any => :finished do |export_job| + export_job.run_after_commit_or_now do + audit_project_exported + end + end + end + + private + + def audit_project_exported + return if Feature.disabled?(:export_audit_events, user) + return if exported_by_admin? && Gitlab::CurrentSettings.silent_admin_exports_enabled? + + audit_context = { + name: 'project_export_created', + author: user, + scope: project, + target: project, + message: 'Profile file export was created' + } + + ::Gitlab::Audit::Auditor.audit(audit_context) end end diff --git a/app/services/groups/import_export/export_service.rb b/app/services/groups/import_export/export_service.rb index 2d88283661c4b845116371dd42718b3b5b5d2985..e495e803e82858c76405227e2ffa767536c8cd9a 100644 --- a/app/services/groups/import_export/export_service.rb +++ b/app/services/groups/import_export/export_service.rb @@ -3,16 +3,21 @@ module Groups module ImportExport class ExportService - def initialize(group:, user:, params: {}) + def initialize(group:, user:, exported_by_admin:, params: {}) @group = group @current_user = user + @exported_by_admin = exported_by_admin @params = params @shared = @params[:shared] || Gitlab::ImportExport::Shared.new(@group) @logger = Gitlab::Export::Logger.build end def async_execute - GroupExportWorker.perform_async(current_user.id, group.id, params) + GroupExportWorker.perform_async( + current_user.id, + group.id, + params.merge(exported_by_admin: @exported_by_admin) + ) end def execute @@ -27,7 +32,7 @@ def execute private - attr_reader :group, :current_user, :params + attr_reader :group, :current_user, :exported_by_admin, :params attr_accessor :shared def validate_user_permissions @@ -50,6 +55,7 @@ def save! # it removes the tmp dir. This means that if we want to add new savers # in EE the data won't be available. if save_exporters && file_saver.save + audit_export notify_success else notify_error! @@ -105,6 +111,21 @@ def log_info(message) ) end + def audit_export + return if Feature.disabled?(:export_audit_events, current_user) + return if exported_by_admin && Gitlab::CurrentSettings.silent_admin_exports_enabled? + + audit_context = { + name: 'group_export_created', + author: current_user, + scope: group, + target: group, + message: 'Group file export was created' + } + + Gitlab::Audit::Auditor.audit(audit_context) + end + def notify_success log_info('Group Export succeeded') diff --git a/app/views/admin/application_settings/_import_and_export.html.haml b/app/views/admin/application_settings/_import_and_export.html.haml index e1c2d727342151268c329c1e44af8f993556369a..c64b3c0b54e411880b39cb90ab38e09c9155093d 100644 --- a/app/views/admin/application_settings/_import_and_export.html.haml +++ b/app/views/admin/application_settings/_import_and_export.html.haml @@ -17,6 +17,12 @@ .form-group{ data: { testid: 'bulk-import' } } = f.label :bulk_import, s_('AdminSettings|Allow migrating GitLab groups and projects by direct transfer'), class: 'gl-font-weight-bold' = f.gitlab_ui_checkbox_component :bulk_import_enabled, s_('AdminSettings|Enabled') + - if Feature.enabled?(:export_audit_events, current_user) + .form-group{ data: { testid: 'silent-admin-exports' } } + - tag_pair_silent_admin_exports = tag_pair(link_to('', help_page_path('administration/settings/import_and_export_settings', anchor: 'enable-silent-admin-exports'), target: '_blank', rel: 'noopener noreferrer'), :silent_admin_exports_link_start, :silent_admin_exports_link_end) + - silent_admin_exports_label_text = safe_format(s_('AdminSettings|Silent exports by admins %{silent_admin_exports_link_start}%{icon}%{silent_admin_exports_link_end}'), tag_pair_silent_admin_exports, icon: sprite_icon('question-o')) + = f.label :silent_admin_exports_enabled, silent_admin_exports_label_text, class: 'gl-font-weight-bold' + = f.gitlab_ui_checkbox_component :silent_admin_exports_enabled, s_('AdminSettings|Enabled') .form-group = f.label :max_export_size, _('Maximum export size (MiB)'), class: 'label-light' = f.number_field :max_export_size, class: 'form-control gl-form-input', title: _('Maximum size of export files.'), data: { toggle: 'tooltip', container: 'body' } diff --git a/app/workers/group_export_worker.rb b/app/workers/group_export_worker.rb index f6f9a69fb173fcd54eb8fe0be556e9336ef13e30..7e1ecb9d9bde53ad7f624000a4f4aee09e68594f 100644 --- a/app/workers/group_export_worker.rb +++ b/app/workers/group_export_worker.rb @@ -14,6 +14,13 @@ def perform(current_user_id, group_id, params = {}) current_user = User.find(current_user_id) group = Group.find(group_id) - ::Groups::ImportExport::ExportService.new(group: group, user: current_user, params: params).execute + params.symbolize_keys! + + ::Groups::ImportExport::ExportService.new( + group: group, + user: current_user, + exported_by_admin: params.delete(:exported_by_admin), + params: params + ).execute end end diff --git a/app/workers/project_export_worker.rb b/app/workers/project_export_worker.rb index 1b969844884f76cb561c44e6ac375991edf556c2..0c3586e262677d75ea6051298c8a8e3c2e26ab59 100644 --- a/app/workers/project_export_worker.rb +++ b/app/workers/project_export_worker.rb @@ -18,7 +18,10 @@ def perform(current_user_id, project_id, after_export_strategy = {}, params = {} params.symbolize_keys! project = Project.find(project_id) - export_job = project.export_jobs.safe_find_or_create_by(jid: self.jid) + export_job = project.export_jobs.safe_find_or_create_by(jid: self.jid) do |job| + job.user = current_user + job.exported_by_admin = !!params[:exported_by_admin] + end after_export = build!(after_export_strategy) export_job&.start diff --git a/app/workers/projects/import_export/create_relation_exports_worker.rb b/app/workers/projects/import_export/create_relation_exports_worker.rb index ac5fd343a6ce9d07269bc35d74ae6605a70fe015..407e5003c241d4f4b5ab8d3a651de0df83b7272e 100644 --- a/app/workers/projects/import_export/create_relation_exports_worker.rb +++ b/app/workers/projects/import_export/create_relation_exports_worker.rb @@ -22,7 +22,12 @@ def perform(user_id, project_id, after_export_strategy = {}, params = {}) project = Project.find_by_id(project_id) return unless project - project_export_job = project.export_jobs.find_or_create_by!(jid: jid) + params.symbolize_keys! + + project_export_job = project.export_jobs.find_or_create_by!(jid: jid) do |export_job| + export_job.user_id = user_id + export_job.exported_by_admin = !!params[:exported_by_admin] + end return if project_export_job.started? relation_exports = RelationExport.relation_names_list.map do |relation_name| diff --git a/config/audit_events/types/group_export_created.yml b/config/audit_events/types/group_export_created.yml new file mode 100644 index 0000000000000000000000000000000000000000..1159ac5fdd7c21f14e960c1f3b3dad6d4712821a --- /dev/null +++ b/config/audit_events/types/group_export_created.yml @@ -0,0 +1,9 @@ +name: group_export_created +description: Triggered when a group file export is created +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/294168 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TODO +feature_category: importers +milestone: '17.0' +saved_to_database: true +scope: [Group] +streamed: true diff --git a/config/audit_events/types/project_export_created.yml b/config/audit_events/types/project_export_created.yml new file mode 100644 index 0000000000000000000000000000000000000000..f79ef9252b2bac11d10ee7c2700a1435a02ab9c2 --- /dev/null +++ b/config/audit_events/types/project_export_created.yml @@ -0,0 +1,9 @@ +name: project_export_created +description: Triggered when a project file export is created +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/294168 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TODO +feature_category: importers +milestone: '17.0' +saved_to_database: true +scope: [Project] +streamed: true diff --git a/config/feature_flags/wip/export_audit_events.yml b/config/feature_flags/wip/export_audit_events.yml new file mode 100644 index 0000000000000000000000000000000000000000..a236a691d16d90a45cf1f50bfebc8cee1abcba51 --- /dev/null +++ b/config/feature_flags/wip/export_audit_events.yml @@ -0,0 +1,9 @@ +--- +name: export_audit_events +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/294168 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/151278 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/458597 +milestone: '17.0' +group: group::import and integrate +type: wip +default_enabled: false diff --git a/db/migrate/20240426044935_add_user_id_to_project_import_job.rb b/db/migrate/20240426044935_add_user_id_to_project_import_job.rb new file mode 100644 index 0000000000000000000000000000000000000000..47903060911907d04f9f1f798c67cbdbadf99e59 --- /dev/null +++ b/db/migrate/20240426044935_add_user_id_to_project_import_job.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class AddUserIdToProjectImportJob < Gitlab::Database::Migration[2.2] + milestone '17.0' + + disable_ddl_transaction! + + def up + with_lock_retries do + add_column :project_export_jobs, :user_id, :bigint, null: true + end + + add_concurrent_foreign_key :project_export_jobs, :users, column: :user_id, on_delete: :nullify + end + + def down + with_lock_retries do + remove_foreign_key_if_exists :project_export_jobs, column: :user_id + end + + with_lock_retries do + remove_column :project_export_jobs, :user_id + end + end +end diff --git a/db/migrate/20240428214617_add_user_was_admin_to_project_export_job.rb b/db/migrate/20240428214617_add_user_was_admin_to_project_export_job.rb new file mode 100644 index 0000000000000000000000000000000000000000..e9049238c86f5ecbfa925b82d4113d8a3403de10 --- /dev/null +++ b/db/migrate/20240428214617_add_user_was_admin_to_project_export_job.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddUserWasAdminToProjectExportJob < Gitlab::Database::Migration[2.2] + milestone '17.0' + + def up + add_column :project_export_jobs, :exported_by_admin, :boolean, default: false + end + + def down + remove_column :project_export_jobs, :exported_by_admin + end +end diff --git a/db/post_migrate/20240429022431_add_index_to_user_id_project_import_job.rb b/db/post_migrate/20240429022431_add_index_to_user_id_project_import_job.rb new file mode 100644 index 0000000000000000000000000000000000000000..66fbba182efa9526d7d3e81078eac193986c939c --- /dev/null +++ b/db/post_migrate/20240429022431_add_index_to_user_id_project_import_job.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddIndexToUserIdProjectImportJob < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + + milestone '17.0' + + INDEX_NAME = :index_project_export_jobs_on_user_id + + def up + add_concurrent_index :project_export_jobs, :user_id, name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :project_export_jobs, INDEX_NAME + end +end diff --git a/db/schema_migrations/20240426044935 b/db/schema_migrations/20240426044935 new file mode 100644 index 0000000000000000000000000000000000000000..abf9046822294a7a11a0d1be63329e94e9895e9e --- /dev/null +++ b/db/schema_migrations/20240426044935 @@ -0,0 +1 @@ +2816cebd6241617e67b06864134d5f5eb6e5f6832328c3de7febba51856175b9 \ No newline at end of file diff --git a/db/schema_migrations/20240428214617 b/db/schema_migrations/20240428214617 new file mode 100644 index 0000000000000000000000000000000000000000..72443a98c99d972caf7252eadbbb84d53ab8b817 --- /dev/null +++ b/db/schema_migrations/20240428214617 @@ -0,0 +1 @@ +db636595480e6e30be8f4c2ba3743429cb85fe82ef690320211af15219f48de0 \ No newline at end of file diff --git a/db/schema_migrations/20240429022431 b/db/schema_migrations/20240429022431 new file mode 100644 index 0000000000000000000000000000000000000000..bddfbc3a3af63193a330426496642d7c6e18c8e6 --- /dev/null +++ b/db/schema_migrations/20240429022431 @@ -0,0 +1 @@ +a68722503a1a916f8fbc276cba4239f2b62e5451b2b43514b1876b65603495eb \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 64f611b5e2992848db0a674fde10c45ea4261f55..989dd8f4aafbc43415252ea3807bc54b19f20073 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14414,7 +14414,9 @@ CREATE TABLE project_export_jobs ( created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, status smallint DEFAULT 0 NOT NULL, - jid character varying(100) NOT NULL + jid character varying(100) NOT NULL, + user_id bigint, + exported_by_admin boolean DEFAULT false ); CREATE SEQUENCE project_export_jobs_id_seq @@ -26912,6 +26914,8 @@ CREATE INDEX index_project_export_jobs_on_status ON project_export_jobs USING bt CREATE INDEX index_project_export_jobs_on_updated_at_and_id ON project_export_jobs USING btree (updated_at, id); +CREATE INDEX index_project_export_jobs_on_user_id ON project_export_jobs USING btree (user_id); + CREATE INDEX index_project_feature_usages_on_project_id ON project_feature_usages USING btree (project_id); CREATE UNIQUE INDEX index_project_features_on_project_id ON project_features USING btree (project_id); @@ -30372,6 +30376,9 @@ ALTER TABLE ONLY deploy_keys_projects ALTER TABLE ONLY packages_tags ADD CONSTRAINT fk_5a230894f6 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY project_export_jobs + ADD CONSTRAINT fk_5ab0242530 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE SET NULL; + ALTER TABLE ONLY dependency_list_exports ADD CONSTRAINT fk_5b3d11e1ef FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE SET NULL; diff --git a/doc/administration/audit_event_types.md b/doc/administration/audit_event_types.md index 33637155f675c1809f83ed850b1f1948c438719d..ebe3e7d61c654ad9e3f8ab0d8de2852e8ff684ab 100644 --- a/doc/administration/audit_event_types.md +++ b/doc/administration/audit_event_types.md @@ -332,6 +332,13 @@ Audit event types belong to the following product categories. | [`squash_commit_template_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107533) | Event triggered on updating the merge request squash commit template for a project| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.8](https://gitlab.com/gitlab-org/gitlab/-/issues/369314) | Project | | [`squash_option_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/84624) | Triggered when squash option setting has been changed.| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.0](https://gitlab.com/gitlab-org/gitlab/-/issues/301124) | Project | +### Importers + +| Name | Description | Saved to database | Streamed | Introduced in | Scope | +|:------------|:------------|:------------------|:---------|:--------------|:--------------| +| [`group_export_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TODO) | Triggered when a group file export is created| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.0](https://gitlab.com/gitlab-org/gitlab/-/issues/294168) | Group | +| [`project_export_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TODO) | Triggered when a project file export is created| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.0](https://gitlab.com/gitlab-org/gitlab/-/issues/294168) | Project | + ### Incident management | Name | Description | Saved to database | Streamed | Introduced in | Scope | diff --git a/doc/administration/settings/import_and_export_settings.md b/doc/administration/settings/import_and_export_settings.md index d42a6b328f043daf48f8a51f6f3cf744d248e398..221bfb843bdf45c65efce960f8fafa1c92f7ede1 100644 --- a/doc/administration/settings/import_and_export_settings.md +++ b/doc/administration/settings/import_and_export_settings.md @@ -66,6 +66,24 @@ The same setting [is available](../../api/settings.md#list-of-settings-that-can-be-accessed-via-api-calls) in the API as the `bulk_import_enabled` attribute. +## Enable silent admin exports + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/151278) in GitLab 17.0 [with a flag](../../administration/feature_flags.md) named `export_audit_events`. Disabled by default. + +FLAG: +The availability of this feature is controlled by a feature flag. For more information, see the history. + +Enable silent admin exports to prevent [audit events](../audit_event_reports.md) when +instance administrators trigger a [project or group file export](../../user/project/settings/import_export.md). +Exports from non-administrators still generate audit events. + +To enable silent admin project and group file exports: + +1. On the left sidebar, at the bottom, select **Admin Area**. +1. Select **Settings > General**, then expand **Import and export settings**. +1. Scroll to **Silent exports by admins**. +1. Select the **Enabled** checkbox. + ## Max export size > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/86124) in GitLab 15.0. diff --git a/doc/api/settings.md b/doc/api/settings.md index 5cf87a2ebbba4cdccc51390b174643c36d65b748..e6dd95291d638830794000bdbc515b8900d28e7c 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -597,7 +597,7 @@ listed in the descriptions of the relevant settings. | `sidekiq_job_limiter_limit_bytes` | integer | no | The threshold in bytes at which Sidekiq jobs are rejected. Default: 0 bytes (doesn't reject any job). | | `signin_enabled` | string | no | (Deprecated: Use `password_authentication_enabled_for_web` instead) Flag indicating if password authentication is enabled for the web interface. | | `signup_enabled` | boolean | no | Enable registration. Default is `true`. | -| `silent_admin_exports_enabled` | boolean | no | Enable silent exports for administrators. Default is `false`. | +| `silent_admin_exports_enabled` | boolean | no | Enable [Silent admin exports](../administration/settings/import_and_export_settings.md#enable-silent-admin-exports). Default is `false`. | | `silent_mode_enabled` | boolean | no | Enable [Silent mode](../administration/silent_mode/index.md). Default is `false`. | | `slack_app_enabled` | boolean | no | (**If enabled, requires:** `slack_app_id`, `slack_app_secret`, `slack_app_signing_secret`, and `slack_app_verification_token`) Enable the GitLab for Slack app. | | `slack_app_id` | string | required by: `slack_app_enabled` | The client ID of the GitLab for Slack app. | diff --git a/ee/spec/services/ee/groups/import_export/export_service_spec.rb b/ee/spec/services/ee/groups/import_export/export_service_spec.rb index 89d6081dd193a86ceb54f930bf19634ff38c9c2b..564564084423e538df3bbca9146ecd31c06f9589 100644 --- a/ee/spec/services/ee/groups/import_export/export_service_spec.rb +++ b/ee/spec/services/ee/groups/import_export/export_service_spec.rb @@ -19,7 +19,7 @@ let(:shared) { Gitlab::ImportExport::Shared.new(group) } let(:archive_path) { shared.archive_path } - subject(:export_service) { described_class.new(group: group, user: user, params: { shared: shared }) } + subject(:export_service) { described_class.new(group: group, user: user, exported_by_admin: false, params: { shared: shared }) } after do FileUtils.rm_rf(archive_path) diff --git a/lib/api/group_export.rb b/lib/api/group_export.rb index 819cc4652f6c1bd037e719a42859dbeb4838adac..2ad592740cf2fe8d6d1d48c1c2322d9a036b7d37 100644 --- a/lib/api/group_export.rb +++ b/lib/api/group_export.rb @@ -55,7 +55,11 @@ class GroupExport < ::API::Base post ':id/export' do check_rate_limit! :group_export, scope: current_user - export_service = ::Groups::ImportExport::ExportService.new(group: user_group, user: current_user) + export_service = ::Groups::ImportExport::ExportService.new( + group: user_group, + user: current_user, + exported_by_admin: current_user.can_admin_all_resources? + ) if export_service.async_execute accepted! diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8dc14d2afdd790a55e83db8e68139d4c9a5bb676..8563c32e02ce1465f4d6c1517f667490c3a28230 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3929,6 +3929,9 @@ msgstr "" msgid "AdminSettings|Show a redirect page that warns you about user-generated content in GitLab Pages." msgstr "" +msgid "AdminSettings|Silent exports by admins %{silent_admin_exports_link_start}%{icon}%{silent_admin_exports_link_end}" +msgstr "" + msgid "AdminSettings|Size and domain settings for Pages static sites." msgstr "" diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index fd1e6d7c66af3055faf6f69682de59dd9bf31688..7f9bf700e0c68bf602559201008a2a1e9a42ced3 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -1187,12 +1187,6 @@ def group_moved_message(redirect_route, group) end describe 'POST #export' do - let(:admin) { create(:admin) } - - before do - enable_admin_mode!(admin) - end - context 'when the user does not have permission to export the group' do before do sign_in(guest) @@ -1205,13 +1199,13 @@ def group_moved_message(redirect_route, group) end end - context 'when supplied valid params' do + context 'when the user has permission to export the group' do before do - sign_in(admin) + sign_in(user) end it 'triggers the export job' do - expect(GroupExportWorker).to receive(:perform_async).with(admin.id, group.id, {}) + expect(GroupExportWorker).to receive(:perform_async).with(user.id, group.id, { exported_by_admin: false }) post :export, params: { id: group.to_param } end @@ -1223,9 +1217,21 @@ def group_moved_message(redirect_route, group) end end + context 'when user is admin' do + before do + sign_in(admin_with_admin_mode) + end + + it 'triggers the export job, and passes `exported_by_admin` correctly in the `params` hash' do + expect(GroupExportWorker).to receive(:perform_async).with(admin_with_admin_mode.id, group.id, { exported_by_admin: true }) + + post :export, params: { id: group.to_param } + end + end + context 'when the endpoint receives requests above the rate limit' do before do - sign_in(admin) + sign_in(user) allow_next_instance_of(Gitlab::ApplicationRateLimiter::BaseStrategy) do |strategy| allow(strategy) diff --git a/spec/factories/project_export_jobs.rb b/spec/factories/project_export_jobs.rb index bf8cfd863ec0771e5eadf2c1ddea7eac205647c2..296c5cad5a9f122be0a1acd8e19516764e6e2b72 100644 --- a/spec/factories/project_export_jobs.rb +++ b/spec/factories/project_export_jobs.rb @@ -20,5 +20,7 @@ trait :failed do status { ProjectExportJob::STATUS[:failed] } end + + exported_by_admin { user&.can_admin_all_resources? } end end diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index ad08dcb638667355f2bfd4deb49fbfcfb840bf34..0823909cfc78f08f99886d1aabc8aedc662e3358 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -63,6 +63,10 @@ end it 'change Visibility and Access Controls' do + expect(current_settings.project_export_enabled).to be(true) + expect(current_settings.bulk_import_enabled).to be(false) + expect(current_settings.silent_admin_exports_enabled).to be(false) + within_testid('admin-import-export-settings') do within_testid('project-export') do uncheck 'Enabled' @@ -72,14 +76,30 @@ check 'Enabled' end + within_testid('silent-admin-exports') do + check 'Enabled' + end + click_button 'Save changes' end - expect(current_settings.project_export_enabled).to be_falsey + expect(current_settings.project_export_enabled).to be(false) expect(current_settings.bulk_import_enabled).to be(true) + expect(current_settings.silent_admin_exports_enabled).to be(true) expect(page).to have_content 'Application settings saved successfully' end + context 'when `export_audit_events` flag is disabled' do + before do + stub_feature_flags(export_audit_events: false) + visit general_admin_application_settings_path + end + + it 'does not display the silent admin exports setting' do + expect(page).not_to have_selector('[data-testid="silent-admin-exports"]') + end + end + it 'change Keys settings' do within_testid('admin-visibility-access-settings') do select 'Are forbidden', from: 'RSA SSH keys' diff --git a/spec/models/project_export_job_spec.rb b/spec/models/project_export_job_spec.rb index 79fe4af22880dc2713b97b38f4a3a737d184910f..a06cf995d69ab4ce7948756408fa928db97496fe 100644 --- a/spec/models/project_export_job_spec.rb +++ b/spec/models/project_export_job_spec.rb @@ -5,6 +5,7 @@ RSpec.describe ProjectExportJob, feature_category: :importers, type: :model do describe 'associations' do it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:user) } it { is_expected.to have_many(:relation_exports) } end @@ -105,4 +106,73 @@ end end end + + describe '#finish' do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + + let(:export_job) { build(:project_export_job, :started, user: user, project: project) } + + let(:expected_audit) do + { + name: 'project_export_created', + author: user, + scope: project, + target: project, + message: 'Profile file export was created' + } + end + + subject(:finish) { export_job.finish } + + it 'creates an audit event' do + expect(Gitlab::Audit::Auditor).to receive(:audit).with(expected_audit) + + finish + end + + context 'when the flag is disabled' do + before do + stub_feature_flags(export_audit_events: false) + end + + it 'does not create an audit event' do + expect(Gitlab::Audit::Auditor).not_to receive(:audit) + + finish + end + end + + context 'when user is nil' do + let_it_be(:user) { nil } + + it 'creates an audit event' do + expect(Gitlab::Audit::Auditor).to receive(:audit).with(expected_audit) + + finish + end + end + + context 'when user was admin', :enable_admin_mode do + let_it_be(:user) { create(:admin) } + + it 'creates an audit event' do + expect(Gitlab::Audit::Auditor).to receive(:audit).with(expected_audit) + + finish + end + + context 'when silent exports enabled' do + before do + stub_application_setting(silent_admin_exports_enabled: true) + end + + it 'does not create an audit event' do + expect(Gitlab::Audit::Auditor).not_to receive(:audit) + + finish + end + end + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index c79d13312a81a7835f10c701052f8e684a500471..1aa0792739c3e5b12cd415a2855497b3390ecd74 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -7822,16 +7822,28 @@ def has_external_wiki describe '#add_export_job' do let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project) } + let_it_be_with_reload(:project) { create(:project) } context 'when parallel_project_export feature flag is enabled' do it 'enqueues CreateionProjectExportWorker' do expect(Projects::ImportExport::CreateRelationExportsWorker) .to receive(:perform_async) - .with(user.id, project.id, nil, {}) + .with(user.id, project.id, nil, { exported_by_admin: false }) project.add_export_job(current_user: user) end + + context 'when user is admin', :enable_admin_mode do + let_it_be(:user) { create(:admin) } + + it 'passes `exported_by_admin` correctly in the `params` hash' do + expect(Projects::ImportExport::CreateRelationExportsWorker) + .to receive(:perform_async) + .with(user.id, project.id, nil, { exported_by_admin: true }) + + project.add_export_job(current_user: user) + end + end end context 'when parallel_project_export feature flag is disabled' do @@ -7839,18 +7851,28 @@ def has_external_wiki stub_feature_flags(parallel_project_export: false) end - it 'enquques ProjectExportWorker' do - expect(ProjectExportWorker).to receive(:perform_async).with(user.id, project.id, nil, {}) + it 'enqueues ProjectExportWorker' do + expect(ProjectExportWorker).to receive(:perform_async).with(user.id, project.id, nil, { exported_by_admin: false }) project.add_export_job(current_user: user) end + context 'when user is admin', :enable_admin_mode do + let_it_be(:user) { create(:admin) } + + it 'passes `exported_by_admin` correctly in the `params` hash' do + expect(ProjectExportWorker).to receive(:perform_async).with(user.id, project.id, nil, { exported_by_admin: true }) + + project.add_export_job(current_user: user) + end + end + context 'when project storage_size does not exceed the application setting max_export_size' do it 'starts project export worker' do stub_application_setting(max_export_size: 1) allow(project.statistics).to receive(:storage_size).and_return(0.megabytes) - expect(ProjectExportWorker).to receive(:perform_async).with(user.id, project.id, nil, {}) + expect(ProjectExportWorker).to receive(:perform_async).with(user.id, project.id, nil, { exported_by_admin: false }) project.add_export_job(current_user: user) end @@ -7861,7 +7883,7 @@ def has_external_wiki stub_application_setting(max_export_size: 1) allow(project.statistics).to receive(:storage_size).and_return(2.megabytes) - expect(ProjectExportWorker).not_to receive(:perform_async).with(user.id, project.id, nil, {}) + expect(ProjectExportWorker).not_to receive(:perform_async) expect { project.add_export_job(current_user: user) }.to raise_error(Project::ExportLimitExceeded) end end @@ -7869,7 +7891,7 @@ def has_external_wiki context 'when application setting max_export_size is not set' do it 'starts project export worker' do allow(project.statistics).to receive(:storage_size).and_return(2.megabytes) - expect(ProjectExportWorker).to receive(:perform_async).with(user.id, project.id, nil, {}) + expect(ProjectExportWorker).to receive(:perform_async).with(user.id, project.id, nil, { exported_by_admin: false }) project.add_export_job(current_user: user) end diff --git a/spec/requests/api/group_export_spec.rb b/spec/requests/api/group_export_spec.rb index 1034fe845c61ee03c8212e7063adea7af3f39054..7348af2d318eafd67766d1ef052bc8669fd4c23c 100644 --- a/spec/requests/api/group_export_spec.rb +++ b/spec/requests/api/group_export_spec.rb @@ -119,6 +119,34 @@ expect(response).to have_gitlab_http_status(:accepted) end + + it 'calls the service correctly' do + expect_next_instance_of(Groups::ImportExport::ExportService, + group: group, + user: user, + exported_by_admin: false + ) do |service| + expect(service).to receive(:async_execute).and_return(true) + end + + post api(path, user) + end + + context 'when user is an admin', :enable_admin_mode do + let_it_be(:user) { create(:admin) } + + it 'calls the service correctly' do + expect_next_instance_of(Groups::ImportExport::ExportService, + group: group, + user: user, + exported_by_admin: true + ) do |service| + expect(service).to receive(:async_execute).and_return(true) + end + + post api(path, user) + end + end end context 'when the export cannot be started' do diff --git a/spec/services/groups/import_export/export_service_spec.rb b/spec/services/groups/import_export/export_service_spec.rb index c44c2e3b911b9b487ee63becdc115e2844a179a6..b4db9a77c3b7f6af8a7fd312fd5687511de57299 100644 --- a/spec/services/groups/import_export/export_service_spec.rb +++ b/spec/services/groups/import_export/export_service_spec.rb @@ -3,15 +3,19 @@ require 'spec_helper' RSpec.describe Groups::ImportExport::ExportService, feature_category: :importers do - describe '#async_execute' do - let(:user) { create(:user) } - let(:group) { create(:group) } + let_it_be(:user) { create(:user) } + let_it_be_with_refind(:group) { create(:group) } + + let(:exported_by_admin) { false } + describe '#async_execute' do context 'when the job can be successfully scheduled' do - let(:export_service) { described_class.new(group: group, user: user) } + let(:export_service) { described_class.new(group: group, user: user, exported_by_admin: exported_by_admin) } it 'enqueues an export job' do - allow(GroupExportWorker).to receive(:perform_async).with(user.id, group.id, {}) + expect(GroupExportWorker) + .to receive(:perform_async) + .with(user.id, group.id, { exported_by_admin: exported_by_admin }) export_service.async_execute end @@ -19,10 +23,20 @@ it 'returns truthy' do expect(export_service.async_execute).to be_present end + + context 'when the user was an admin' do + let(:exported_by_admin) { true } + + it 'passes `exported_by_admin` correctly in the `params` hash' do + expect(GroupExportWorker).to receive(:perform_async).with(user.id, group.id, { exported_by_admin: true }) + + export_service.async_execute + end + end end context 'when the job cannot be scheduled' do - let(:export_service) { described_class.new(group: group, user: user) } + let(:export_service) { described_class.new(group: group, user: user, exported_by_admin: exported_by_admin) } before do allow(GroupExportWorker).to receive(:perform_async).and_return(nil) @@ -35,13 +49,18 @@ end describe '#execute' do - let!(:user) { create(:user) } - let(:group) { create(:group) } let(:shared) { Gitlab::ImportExport::Shared.new(group) } let(:archive_path) { shared.archive_path } - let(:service) { described_class.new(group: group, user: user, params: { shared: shared }) } + let(:service) do + described_class.new( + group: group, + user: user, + exported_by_admin: exported_by_admin, + params: { shared: shared } + ) + end - before do + before_all do group.add_owner(user) end @@ -79,6 +98,58 @@ service.execute end + it 'creates an audit event' do + expect(Gitlab::Audit::Auditor).to receive(:audit).with( + { + name: 'group_export_created', + author: user, + scope: group, + target: group, + message: 'Group file export was created' + } + ) + + service.execute + end + + context 'when the flag is disabled' do + before do + stub_feature_flags(export_audit_events: false) + end + + it 'does not create an audit event' do + expect(Gitlab::Audit::Auditor).not_to receive(:audit) + + service.execute + end + end + + context 'when the user was an admin' do + let(:exported_by_admin) { true } + + it 'logs group exported audit event' do + expect(Gitlab::Audit::Auditor).to receive(:audit).with(hash_including(name: 'group_export_created')) + + service.execute + end + + context 'when silent exports are enabled' do + before do + stub_application_setting(silent_admin_exports_enabled: true) + end + + it 'does not create an audit event' do + expect(Gitlab::Audit::Auditor).not_to receive(:audit) + + expect { service.execute }.not_to change { AuditEvent.count } + end + + it 'does not create any Todos' do + expect { service.execute }.not_to change { Todo.count } + end + end + end + context 'when saver succeeds' do it 'saves the group in the file system' do service.execute @@ -90,8 +161,16 @@ end context 'when user does not have admin_group permission' do - let!(:another_user) { create(:user) } - let(:service) { described_class.new(group: group, user: another_user, params: { shared: shared }) } + let_it_be(:another_user) { create(:user) } + + let(:service) do + described_class.new( + group: group, + user: another_user, + exported_by_admin: exported_by_admin, + params: { shared: shared } + ) + end let(:expected_message) do "User with ID: %s does not have required permissions for Group: %s with ID: %s" % @@ -173,7 +252,7 @@ end context 'when there is an existing export file' do - subject(:export_service) { described_class.new(group: group, user: user) } + subject(:export_service) { described_class.new(group: group, user: user, exported_by_admin: exported_by_admin) } let(:import_export_upload) do create( diff --git a/spec/workers/group_export_worker_spec.rb b/spec/workers/group_export_worker_spec.rb index 54f9c38a506f056fbadf159c30e77125d81398ca..a4e5d45216c21134d4ed98571604393b5a0f8784 100644 --- a/spec/workers/group_export_worker_spec.rb +++ b/spec/workers/group_export_worker_spec.rb @@ -10,10 +10,14 @@ describe '#perform' do context 'when it succeeds' do - it 'calls the ExportService' do - expect_any_instance_of(::Groups::ImportExport::ExportService).to receive(:execute) + it 'calls the ExportService correctly' do + expected_params = { group: group, user: user, exported_by_admin: false, params: {} } - subject.perform(user.id, group.id, {}) + expect_next_instance_of(::Groups::ImportExport::ExportService, expected_params) do |service| + expect(service).to receive(:execute) + end + + subject.perform(user.id, group.id, { exported_by_admin: false }) end end diff --git a/spec/workers/project_export_worker_spec.rb b/spec/workers/project_export_worker_spec.rb index eaf1536da63d942cb5d52afeb1154fc47e64a2a9..4f053b4ccf579c0eb5d786e9b7fc69860dffaf6c 100644 --- a/spec/workers/project_export_worker_spec.rb +++ b/spec/workers/project_export_worker_spec.rb @@ -6,16 +6,19 @@ it_behaves_like 'export worker' context 'exporters duration measuring' do - let(:user) { create(:user) } - let(:project) { create(:project) } - let(:worker) { described_class.new } + let_it_be_with_reload(:project) { create(:project) } + let_it_be(:user) { create(:user, owner_of: project) } - subject { worker.perform(user.id, project.id) } + let(:jid) { SecureRandom.hex(8) } + let(:params) { {} } + let(:worker) { described_class.new } before do - project.add_owner(user) + allow(worker).to receive(:jid).and_return(jid) end + subject(:perform) { worker.perform(user.id, project.id, {}, params) } + it 'logs exporters execution duration' do expect(worker).to receive(:log_extra_metadata_on_done).with(:version_saver_duration_s, anything) expect(worker).to receive(:log_extra_metadata_on_done).with(:avatar_saver_duration_s, anything) @@ -27,7 +30,35 @@ expect(worker).to receive(:log_extra_metadata_on_done).with(:snippets_repo_saver_duration_s, anything) expect(worker).to receive(:log_extra_metadata_on_done).with(:design_repo_saver_duration_s, anything) - subject + perform + end + + it 'creates a ProjectExportJob in the correct state' do + expect { perform }.to change { ProjectExportJob.count }.by(1) + + expect(project.export_jobs).to contain_exactly( + have_attributes( + user: user, + exported_by_admin: false, + jid: jid, + status: 2 + ) + ) + end + + context 'when user was an admin' do + let(:params) { { exported_by_admin: true } } + + it 'creates a ProjectExportJob in correct state' do + perform + + expect(project.export_jobs).to contain_exactly( + have_attributes( + user: user, + exported_by_admin: true + ) + ) + end end end end diff --git a/spec/workers/projects/import_export/create_relation_exports_worker_spec.rb b/spec/workers/projects/import_export/create_relation_exports_worker_spec.rb index f3696c69f315911f09c5830e419c6e374a1800eb..852d35a4b00220d37eba2d8de70d1c1db8ddbe9a 100644 --- a/spec/workers/projects/import_export/create_relation_exports_worker_spec.rb +++ b/spec/workers/projects/import_export/create_relation_exports_worker_spec.rb @@ -3,20 +3,24 @@ require 'spec_helper' RSpec.describe Projects::ImportExport::CreateRelationExportsWorker, feature_category: :importers do + let_it_be_with_reload(:project) { create(:project) } let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project) } let(:after_export_strategy) { {} } - let(:job_args) { [user.id, project.id, after_export_strategy] } + let(:params) { {} } + let(:jid) { SecureRandom.hex(8) } + let(:job_args) { [user.id, project.id, after_export_strategy, params] } before do allow_next_instance_of(described_class) do |job| - allow(job).to receive(:jid) { SecureRandom.hex(8) } + allow(job).to receive(:jid).and_return(jid) end end it_behaves_like 'an idempotent worker' + subject(:perform) { described_class.new.perform(user.id, project.id, after_export_strategy, params) } + context 'when job is re-enqueued after an interuption and same JID is used' do before do allow_next_instance_of(described_class) do |job| @@ -29,13 +33,12 @@ it 'does not start the export process twice' do project.export_jobs.create!(jid: 1234, status_event: :start) - expect { described_class.new.perform(user.id, project.id, after_export_strategy) } - .to change { Projects::ImportExport::WaitRelationExportsWorker.jobs.size }.by(0) + expect { perform }.not_to change { Projects::ImportExport::WaitRelationExportsWorker.jobs.size } end end it 'creates a export_job and sets the status to `started`' do - described_class.new.perform(user.id, project.id, after_export_strategy) + perform export_job = project.export_jobs.last expect(export_job.started?).to eq(true) @@ -44,8 +47,7 @@ it 'creates relation export records and enqueues a worker for each relation to be exported' do allow(Projects::ImportExport::RelationExport).to receive(:relation_names_list).and_return(%w[relation_1 relation_2]) - expect { described_class.new.perform(user.id, project.id, after_export_strategy) } - .to change { Projects::ImportExport::RelationExportWorker.jobs.size }.by(2) + expect { perform }.to change { Projects::ImportExport::RelationExportWorker.jobs.size }.by(2) relation_exports = project.export_jobs.last.relation_exports expect(relation_exports.collect(&:relation)).to match_array(%w[relation_1 relation_2]) @@ -54,7 +56,7 @@ it 'enqueues a WaitRelationExportsWorker' do allow(Projects::ImportExport::WaitRelationExportsWorker).to receive(:perform_in) - described_class.new.perform(user.id, project.id, after_export_strategy) + perform export_job = project.export_jobs.last expect(Projects::ImportExport::WaitRelationExportsWorker).to have_received(:perform_in).with( @@ -64,4 +66,32 @@ after_export_strategy ) end + + it 'creates a ProjectExportJob in the correct state' do + expect { perform }.to change { ProjectExportJob.count }.by(1) + + expect(project.export_jobs).to contain_exactly( + have_attributes( + user: user, + exported_by_admin: false, + jid: jid, + status: 1 + ) + ) + end + + context 'when user was an admin' do + let(:params) { { exported_by_admin: true } } + + it 'creates a ProjectExportJob in correct state' do + perform + + expect(project.export_jobs).to contain_exactly( + have_attributes( + user: user, + exported_by_admin: true + ) + ) + end + end end