diff --git a/ee/app/services/security/configuration/set_group_secret_push_protection_service.rb b/ee/app/services/security/configuration/set_group_secret_push_protection_service.rb index 9109326e1e2e3a9edcdd38c248d6a1a7c3cf991e..541e5ccea78d4d444feff8424b688fe66ad9dd15 100644 --- a/ee/app/services/security/configuration/set_group_secret_push_protection_service.rb +++ b/ee/app/services/security/configuration/set_group_secret_push_protection_service.rb @@ -2,15 +2,15 @@ module Security module Configuration - class SetGroupSecretPushProtectionService < SetNamespaceSecretPushProtectionService + class SetGroupSecretPushProtectionService < SetSecretPushProtectionBaseService private def projects_scope - Project.for_group_and_its_subgroups(@namespace) + Project.for_group_and_its_subgroups(@subject) end def audit - return unless @namespace.is_a?(Group) + return unless @subject.is_a?(Group) message = build_group_message(fetch_filtered_out_projects) @@ -29,7 +29,7 @@ def fetch_filtered_out_projects end def build_group_message(filtered_out_projects_full_path) - message = "Secret push protection has been enabled for group #{@namespace.name} and all of its inherited \ + message = "Secret push protection has been enabled for group #{@subject.name} and all of its inherited \ groups/projects" unless filtered_out_projects_full_path.empty? diff --git a/ee/app/services/security/configuration/set_project_secret_push_protection_service.rb b/ee/app/services/security/configuration/set_project_secret_push_protection_service.rb index 7782fc4045b67e3068fd49bac740ad4d525591aa..67f35a3f0ba24bfbdc1c1406b8b68078821d2a95 100644 --- a/ee/app/services/security/configuration/set_project_secret_push_protection_service.rb +++ b/ee/app/services/security/configuration/set_project_secret_push_protection_service.rb @@ -2,12 +2,12 @@ module Security module Configuration - class SetProjectSecretPushProtectionService < SetNamespaceSecretPushProtectionService + class SetProjectSecretPushProtectionService < SetSecretPushProtectionBaseService private def projects_scope # convert project object into a relation for unified logic in parent class - Project.id_in(@namespace.id) + Project.id_in(@subject.id) end def audit diff --git a/ee/app/services/security/configuration/set_namespace_secret_push_protection_service.rb b/ee/app/services/security/configuration/set_secret_push_protection_base_service.rb similarity index 87% rename from ee/app/services/security/configuration/set_namespace_secret_push_protection_service.rb rename to ee/app/services/security/configuration/set_secret_push_protection_base_service.rb index 1a397a14c98d65c52c043f33f78c2be19a6108af..c07d105a59709af08cb4d7f86ed4d13bf7e101c3 100644 --- a/ee/app/services/security/configuration/set_namespace_secret_push_protection_service.rb +++ b/ee/app/services/security/configuration/set_secret_push_protection_base_service.rb @@ -2,10 +2,10 @@ module Security module Configuration - class SetNamespaceSecretPushProtectionService + class SetSecretPushProtectionBaseService PROJECTS_BATCH_SIZE = 100 - def initialize(namespace:, enable:, current_user:, excluded_projects_ids: []) - @namespace = namespace + def initialize(subject:, enable:, current_user:, excluded_projects_ids: []) + @subject = subject @enable = enable @current_user = current_user @excluded_projects_ids = excluded_projects_ids @@ -28,7 +28,7 @@ def execute protected def valid_request? - @namespace.present? && @current_user.present? && [true, false].include?(@enable) + @subject.present? && @current_user.present? && [true, false].include?(@enable) end def update_security_setting(projects) @@ -60,8 +60,8 @@ def build_audit_context(name:, message:) { name: name, author: @current_user, - scope: @namespace, - target: @namespace, + scope: @subject, + target: @subject, message: message } end diff --git a/ee/app/services/security/configuration/set_secret_push_protection_service.rb b/ee/app/services/security/configuration/set_secret_push_protection_service.rb index cf72a927515b491e9611ecde76f205dd50a02040..b957f056ce957d3dd580bb509bdd642676697cf0 100644 --- a/ee/app/services/security/configuration/set_secret_push_protection_service.rb +++ b/ee/app/services/security/configuration/set_secret_push_protection_service.rb @@ -9,7 +9,7 @@ def self.execute(current_user:, project:, enable:) ServiceResponse.success( payload: { - enabled: SetProjectSecretPushProtectionService.new(current_user: current_user, namespace: project, + enabled: SetProjectSecretPushProtectionService.new(current_user: current_user, subject: project, enable: enable).execute, errors: [] }) diff --git a/ee/app/workers/security/configuration/set_group_secret_push_protection_worker.rb b/ee/app/workers/security/configuration/set_group_secret_push_protection_worker.rb index 706e2d86568dd5ff5c34b76963d6be53f712b3c1..94dd2960909c2f347c0f7e32c423c2a80ea628b4 100644 --- a/ee/app/workers/security/configuration/set_group_secret_push_protection_worker.rb +++ b/ee/app/workers/security/configuration/set_group_secret_push_protection_worker.rb @@ -17,7 +17,7 @@ def perform(group_id, enable, current_user_id = nil, excluded_projects_ids = []) return unless group && current_user - SetGroupSecretPushProtectionService.new(namespace: group, enable: enable, current_user: current_user, + SetGroupSecretPushProtectionService.new(subject: group, enable: enable, current_user: current_user, excluded_projects_ids: excluded_projects_ids).execute end end diff --git a/ee/spec/services/security/configuration/set_group_secret_push_protection_service_spec.rb b/ee/spec/services/security/configuration/set_group_secret_push_protection_service_spec.rb index 514b828771a58f6d6c1b9f2f58bf1e5a672c5491..661327f11d3577eea1f536a4863c2853cbb88cda 100644 --- a/ee/spec/services/security/configuration/set_group_secret_push_protection_service_spec.rb +++ b/ee/spec/services/security/configuration/set_group_secret_push_protection_service_spec.rb @@ -17,9 +17,9 @@ let(:projects_to_change) { [top_level_group_project, mid_level_group_project, bottom_level_group_project] } - def execute_service(namespace:, enable: true, excluded_projects_ids: [excluded_project.id]) + def execute_service(subject:, enable: true, excluded_projects_ids: [excluded_project.id]) described_class - .new(namespace: namespace, enable: enable, current_user: user, excluded_projects_ids: excluded_projects_ids) + .new(subject: subject, enable: enable, current_user: user, excluded_projects_ids: excluded_projects_ids) .execute end @@ -30,29 +30,29 @@ def execute_service(namespace:, enable: true, excluded_projects_ids: [excluded_p security_setting = project.security_setting boolean_values.each do |enable_value| - expect { execute_service(namespace: top_level_group, enable: enable_value) }.to change { + expect { execute_service(subject: top_level_group, enable: enable_value) }.to change { security_setting.reload.pre_receive_secret_detection_enabled }.from(!enable_value).to(enable_value) - expect { execute_service(namespace: top_level_group, enable: enable_value) } + expect { execute_service(subject: top_level_group, enable: enable_value) } .not_to change { security_setting.reload.pre_receive_secret_detection_enabled } end end end it 'changes updated_at timestamp' do - expect { execute_service(namespace: top_level_group) }.to change { + expect { execute_service(subject: top_level_group) }.to change { mid_level_group_project.reload.security_setting.updated_at } end it 'doesnt change the attribute for projects in excluded list' do security_setting = excluded_project.security_setting - expect { execute_service(namespace: top_level_group) }.not_to change { + expect { execute_service(subject: top_level_group) }.not_to change { security_setting.reload.pre_receive_secret_detection_enabled } - expect { execute_service(namespace: mid_level_group, enable: false) }.not_to change { + expect { execute_service(subject: mid_level_group, enable: false) }.not_to change { security_setting.reload.pre_receive_secret_detection_enabled } end @@ -72,7 +72,7 @@ def execute_service(namespace:, enable: true, excluded_projects_ids: [excluded_p end expect do - described_class.execute(namespace: top_level_group, enable: true, + described_class.execute(subject: top_level_group, enable: true, excluded_projects_ids: [excluded_project.id]) end.to raise_error(StandardError) @@ -85,7 +85,7 @@ def execute_service(namespace:, enable: true, excluded_projects_ids: [excluded_p describe 'auditing' do context 'when no excluded projects ids are provided' do it 'audits using the correct properties' do - expect { execute_service(namespace: top_level_group, excluded_projects_ids: []) } + expect { execute_service(subject: top_level_group, excluded_projects_ids: []) } .to change { AuditEvent.count }.by(1) expect(AuditEvent.last.details[:custom_message]).to eq( "Secret push protection has been enabled for group #{top_level_group.name} and all of its inherited \ @@ -99,7 +99,7 @@ def execute_service(namespace:, enable: true, excluded_projects_ids: [excluded_p context 'when excluded projects ids are provided' do context 'when excluded ids matches projects in that group' do it 'audits using the correct properties' do - expect { execute_service(namespace: top_level_group) }.to change { AuditEvent.count }.by(1) + expect { execute_service(subject: top_level_group) }.to change { AuditEvent.count }.by(1) expect(AuditEvent.last.details[:custom_message]).to eq( "Secret push protection has been enabled for group #{top_level_group.name} and all of its inherited \ groups/projects except for #{excluded_project.full_path}") @@ -112,7 +112,7 @@ def execute_service(namespace:, enable: true, excluded_projects_ids: [excluded_p context 'when excluded ids does not match projects in that group' do it 'audits using the correct properties' do expect do - execute_service(namespace: top_level_group, excluded_projects_ids: [Time.now.to_i]) + execute_service(subject: top_level_group, excluded_projects_ids: [Time.now.to_i]) end.to change { AuditEvent.count }.by(1) expect(AuditEvent.last.details[:custom_message]).to eq( "Secret push protection has been enabled for group #{top_level_group.name} and all of its inherited \ @@ -131,7 +131,7 @@ def execute_service(namespace:, enable: true, excluded_projects_ids: [excluded_p end it 'creates security_setting and sets the value appropriately' do - expect { execute_service(namespace: bottom_level_group) }.to change { + expect { execute_service(subject: bottom_level_group) }.to change { bottom_level_group_project.reload.security_setting }.from(nil).to(be_a(ProjectSecuritySetting)) @@ -146,7 +146,7 @@ def execute_service(namespace:, enable: true, excluded_projects_ids: [excluded_p context 'when arguments are invalid' do it 'does not change the attribute' do - expect { execute_service(namespace: top_level_group, enable: nil) } + expect { execute_service(subject: top_level_group, enable: nil) } .not_to change { top_level_group_project.reload.security_setting.pre_receive_secret_detection_enabled } end end diff --git a/ee/spec/services/security/configuration/set_project_secret_push_protection_service_spec.rb b/ee/spec/services/security/configuration/set_project_secret_push_protection_service_spec.rb index acc92ce7edf1b372ac8a7a9111d6d39c8911e58d..2047f9602bf6319253e06b8e57a22856dedb8d35 100644 --- a/ee/spec/services/security/configuration/set_project_secret_push_protection_service_spec.rb +++ b/ee/spec/services/security/configuration/set_project_secret_push_protection_service_spec.rb @@ -10,43 +10,43 @@ let_it_be_with_reload(:project_2) { create(:project) } let_it_be_with_reload(:excluded_project) { create(:project) } - def execute_service(namespace:, enable: true, excluded_projects_ids: [excluded_project.id]) + def execute_service(subject:, enable: true, excluded_projects_ids: [excluded_project.id]) described_class - .new(namespace: namespace, enable: enable, current_user: user, excluded_projects_ids: excluded_projects_ids) + .new(subject: subject, enable: enable, current_user: user, excluded_projects_ids: excluded_projects_ids) .execute end it 'changes the attribute' do security_setting = project_2.security_setting - expect { execute_service(namespace: project_2) }.to change { + expect { execute_service(subject: project_2) }.to change { security_setting.reload.pre_receive_secret_detection_enabled }.from(false).to(true) - expect { execute_service(namespace: project_2) }.not_to change { + expect { execute_service(subject: project_2) }.not_to change { security_setting.reload.pre_receive_secret_detection_enabled } - expect { execute_service(namespace: project_2, enable: false) }.to change { + expect { execute_service(subject: project_2, enable: false) }.to change { security_setting.reload.pre_receive_secret_detection_enabled }.from(true).to(false) - expect { execute_service(namespace: project_2, enable: false) }.not_to change { + expect { execute_service(subject: project_2, enable: false) }.not_to change { security_setting.reload.pre_receive_secret_detection_enabled } end it 'changes updated_at timestamp' do - expect { execute_service(namespace: project_1) } + expect { execute_service(subject: project_1) } .to change { project_1.reload.security_setting.updated_at } end describe 'auditing' do context 'when no excluded_projects ids are provided' do it 'audits using the correct properties' do - expect { execute_service(namespace: project_2) }.to change { AuditEvent.count }.by(1) + expect { execute_service(subject: project_2) }.to change { AuditEvent.count }.by(1) expect(AuditEvent.last.details[:custom_message]).to eq("Secret push protection has been enabled") - expect { execute_service(namespace: project_2, enable: false) }.to change { + expect { execute_service(subject: project_2, enable: false) }.to change { AuditEvent.count }.by(1) expect(AuditEvent.last.details[:custom_message]).to eq("Secret push protection has been disabled") @@ -56,15 +56,15 @@ def execute_service(namespace:, enable: true, excluded_projects_ids: [excluded_p end it 'doesnt create audit if no change was made' do - expect { execute_service(namespace: project_2) }.to change { AuditEvent.count }.by(1) + expect { execute_service(subject: project_2) }.to change { AuditEvent.count }.by(1) # executing again with the same value should not create audit as there is no change - expect { execute_service(namespace: project_2) }.not_to change { AuditEvent.count } + expect { execute_service(subject: project_2) }.not_to change { AuditEvent.count } end end context 'when excluded_projects ids includes the project id' do it 'doesnt create audit' do - expect { execute_service(namespace: excluded_project) }.not_to change { AuditEvent.count } + expect { execute_service(subject: excluded_project) }.not_to change { AuditEvent.count } end end end @@ -77,7 +77,7 @@ def execute_service(namespace:, enable: true, excluded_projects_ids: [excluded_p end it 'creates security_setting and sets the value appropriately' do - expect { execute_service(namespace: project_without_security_setting) } + expect { execute_service(subject: project_without_security_setting) } .to change { project_without_security_setting.reload.security_setting } .from(nil).to(be_a(ProjectSecuritySetting)) @@ -91,7 +91,7 @@ def execute_service(namespace:, enable: true, excluded_projects_ids: [excluded_p context 'when arguments are invalid' do it 'does not change the attribute' do - expect { execute_service(namespace: project_2, enable: nil) } + expect { execute_service(subject: project_2, enable: nil) } .not_to change { project_2.reload.security_setting.pre_receive_secret_detection_enabled } end end diff --git a/ee/spec/services/security/configuration/set_namespace_secret_push_protection_service_spec.rb b/ee/spec/services/security/configuration/set_secret_push_protection_base_service_spec.rb similarity index 86% rename from ee/spec/services/security/configuration/set_namespace_secret_push_protection_service_spec.rb rename to ee/spec/services/security/configuration/set_secret_push_protection_base_service_spec.rb index 7e89a22d54d1ecdb59de1a6005bd379e9127f3fd..ff32ab495332bfb524c0ec53867dd2ce452f08f6 100644 --- a/ee/spec/services/security/configuration/set_namespace_secret_push_protection_service_spec.rb +++ b/ee/spec/services/security/configuration/set_secret_push_protection_base_service_spec.rb @@ -2,10 +2,10 @@ require 'spec_helper' -RSpec.describe Security::Configuration::SetNamespaceSecretPushProtectionService, feature_category: :secret_detection do +RSpec.describe Security::Configuration::SetSecretPushProtectionBaseService, feature_category: :secret_detection do let_it_be(:user) { create(:user) } let_it_be(:project_1) { create(:project) } - let(:service) { described_class.new(namespace: project_1, enable: true, current_user: user) } + let(:service) { described_class.new(subject: project_1, enable: true, current_user: user) } describe '#execute' do context 'when the call is valid' do diff --git a/ee/spec/workers/security/configuration/set_group_secret_push_protection_worker_spec.rb b/ee/spec/workers/security/configuration/set_group_secret_push_protection_worker_spec.rb index da2b07c38e10bb383c691e3c24b661a2f5d2927b..9e4d104e22f0f2ed68087295730e17a59eb45534 100644 --- a/ee/spec/workers/security/configuration/set_group_secret_push_protection_worker_spec.rb +++ b/ee/spec/workers/security/configuration/set_group_secret_push_protection_worker_spec.rb @@ -9,7 +9,7 @@ let_it_be(:user_id) { user.id } let(:excluded_projects_ids) { [1, 2, 3] } - let(:set_namespace_spp_service) { instance_double(Security::Configuration::SetGroupSecretPushProtectionService) } + let(:set_group_spp_service) { instance_double(Security::Configuration::SetGroupSecretPushProtectionService) } describe '#perform' do subject(:run_worker) do @@ -17,9 +17,9 @@ end before do - allow(set_namespace_spp_service).to receive(:execute) + allow(set_group_spp_service).to receive(:execute) allow(Security::Configuration::SetGroupSecretPushProtectionService) - .to receive(:new).and_return(set_namespace_spp_service) + .to receive(:new).and_return(set_group_spp_service) end context 'when group exists' do @@ -27,9 +27,9 @@ run_worker expect(Security::Configuration::SetGroupSecretPushProtectionService).to have_received(:new).with( - { enable: true, namespace: group, current_user: user, excluded_projects_ids: excluded_projects_ids } + { enable: true, subject: group, current_user: user, excluded_projects_ids: excluded_projects_ids } ) - expect(set_namespace_spp_service).to have_received(:execute) + expect(set_group_spp_service).to have_received(:execute) end end @@ -39,7 +39,7 @@ it 'does not call SetGroupSecretPushProtectionService' do run_worker expect(Security::Configuration::SetGroupSecretPushProtectionService).not_to have_received(:new) - expect(set_namespace_spp_service).not_to have_received(:execute) + expect(set_group_spp_service).not_to have_received(:execute) end end @@ -49,7 +49,7 @@ it 'does not call SetGroupSecretPushProtectionService' do run_worker expect(Security::Configuration::SetGroupSecretPushProtectionService).not_to have_received(:new) - expect(set_namespace_spp_service).not_to have_received(:execute) + expect(set_group_spp_service).not_to have_received(:execute) end end