diff --git a/app/models/project_setting.rb b/app/models/project_setting.rb index 6cd6eee261652e0c05738861c8e40f3b49a9e1f6..e5179b62fbb937de05d75ec1493419782ac25b57 100644 --- a/app/models/project_setting.rb +++ b/app/models/project_setting.rb @@ -36,6 +36,15 @@ def target_platforms=(val) super(val&.map(&:to_s)&.sort) end + def human_squash_option + case squash_option + when 'never' then 'Do not allow' + when 'always' then 'Require' + when 'default_on' then 'Encourage' + when 'default_off' then 'Allow' + end + end + private def validates_mr_default_target_self diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb index 96002c8668a98ffc901a30d11a5af1fac5898b04..77038d52efe0740710294a6d64a422bb4c1e2d99 100644 --- a/app/models/protected_branch.rb +++ b/app/models/protected_branch.rb @@ -68,6 +68,10 @@ def self.by_name(query) def allow_multiple?(type) type == :push end + + def self.downcase_humanized_name + name.underscore.humanize.downcase + end end ProtectedBranch.prepend_mod_with('ProtectedBranch') diff --git a/ee/app/models/merge_requests/external_status_check.rb b/ee/app/models/merge_requests/external_status_check.rb index 9e6ae021b57c261c2f7fe4c865a4c97cb8023de1..4e37b39f2142edc80af87e594ad0bd9942147169 100644 --- a/ee/app/models/merge_requests/external_status_check.rb +++ b/ee/app/models/merge_requests/external_status_check.rb @@ -4,6 +4,7 @@ module MergeRequests class ExternalStatusCheck < ApplicationRecord self.table_name = 'external_status_checks' + include Auditable include IgnorableColumns ignore_column :external_approval_rule_id, remove_with: '14.3', remove_after: '2021-09-22' @@ -15,8 +16,10 @@ class ExternalStatusCheck < ApplicationRecord end belongs_to :project - has_and_belongs_to_many :protected_branches - + has_and_belongs_to_many :protected_branches, + after_add: :audit_protected_branch_add, after_remove: :audit_protected_branch_remove + after_create_commit :audit_creation + after_destroy_commit :audit_deletion validates :external_url, presence: true, uniqueness: { scope: :project_id }, addressable_url: true validates :name, uniqueness: { scope: :project_id }, presence: true validate :protected_branches_must_belong_to_project @@ -43,8 +46,43 @@ def to_h } end + def audit_protected_branch_add(model) + message = "Added #{model.class.downcase_humanized_name} #{model.name} to #{self.name} status check" + message += " and removed all other branches from status check" if protected_branches.count == 1 + push_audit_event(message) + end + + def audit_creation + message = "Added #{self.name} status check" + message += if protected_branches.empty? + " with all branches" + else + " with protected branch(es) #{self.protected_branches_names}" + end + + push_audit_event(message) + end + + def audit_deletion + push_audit_event("Removed #{self.name} status check") + end + + def audit_protected_branch_remove(model) + message = if protected_branches.empty? + "Added all branches to #{self.name} status check" + else + "Removed #{model.class.downcase_humanized_name} #{model.name} from #{self.name} status check" + end + + push_audit_event(message) + end + private + def protected_branches_names + self.protected_branches.pluck(:name).join(', ') + end + def payload_data(merge_request_hook_data) merge_request_hook_data.merge(external_approval_rule: self.to_h) end diff --git a/ee/app/services/external_status_checks/base_service.rb b/ee/app/services/external_status_checks/base_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..b8a0bfd39990ce88846f7dc8919e793617499dd7 --- /dev/null +++ b/ee/app/services/external_status_checks/base_service.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module ExternalStatusChecks + class BaseService < BaseContainerService + private + + def with_audit_logged(rule, name, &block) + audit_context = { + name: name, + author: current_user, + scope: rule.project, + target: rule + } + + ::Gitlab::Audit::Auditor.audit(audit_context, &block) + end + end +end diff --git a/ee/app/services/external_status_checks/create_service.rb b/ee/app/services/external_status_checks/create_service.rb index 4cd0deaa4ec4294be77f216c49e10420739817f8..caf04c1a3e3bf4db9014a5822933599924b3cd26 100644 --- a/ee/app/services/external_status_checks/create_service.rb +++ b/ee/app/services/external_status_checks/create_service.rb @@ -1,16 +1,16 @@ # frozen_string_literal: true module ExternalStatusChecks - class CreateService < BaseContainerService + class CreateService < BaseService def execute return ServiceResponse.error(message: 'Failed to create rule', payload: { errors: ['Not allowed'] }, http_status: :unauthorized) unless current_user.can?(:admin_project, container) rule = container.external_status_checks.new(name: params[:name], - project: container, - external_url: params[:external_url], - protected_branch_ids: params[:protected_branch_ids]) + project: container, + external_url: params[:external_url], + protected_branch_ids: params[:protected_branch_ids]) - if rule.save + if with_audit_logged(rule, 'create_status_check') { rule.save } ServiceResponse.success(payload: { rule: rule }) else ServiceResponse.error(message: 'Failed to create rule', payload: { errors: rule.errors.full_messages }, http_status: :unprocessable_entity) diff --git a/ee/app/services/external_status_checks/destroy_service.rb b/ee/app/services/external_status_checks/destroy_service.rb index b6bf2ceb69dbc802be01b7dd11c7e1c97e65db6c..aae20c047efef94261f5a09d9b037518c88bf80d 100644 --- a/ee/app/services/external_status_checks/destroy_service.rb +++ b/ee/app/services/external_status_checks/destroy_service.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true module ExternalStatusChecks - class DestroyService < BaseContainerService + class DestroyService < BaseService def execute(rule) return unauthorized_error_response unless current_user.can?(:admin_project, container) - if rule.destroy + if with_audit_logged(rule, 'delete_status_check') { rule.destroy } ServiceResponse.success else ServiceResponse.error(message: 'Failed to destroy rule', diff --git a/ee/app/services/external_status_checks/update_service.rb b/ee/app/services/external_status_checks/update_service.rb index 31fbed3c31dcf44ca79f57b1aa140b23ecb32eeb..b645ef9401f82aca846fc380dc68a83240a2a3d3 100644 --- a/ee/app/services/external_status_checks/update_service.rb +++ b/ee/app/services/external_status_checks/update_service.rb @@ -1,11 +1,12 @@ # frozen_string_literal: true module ExternalStatusChecks - class UpdateService < BaseContainerService + class UpdateService < BaseService def execute return unauthorized_error_response unless current_user.can?(:admin_project, container) - if rule.update(resource_params) + if with_audit_logged(rule, 'update_status_check') { rule.update(resource_params) } + log_audit_event ServiceResponse.success(payload: { rule: rule }) else ServiceResponse.error(message: 'Failed to update rule', @@ -21,7 +22,7 @@ def resource_params end def rule - container.external_status_checks.find(params[:check_id]) + @rule ||= container.external_status_checks.find(params[:check_id]) end def unauthorized_error_response @@ -31,5 +32,9 @@ def unauthorized_error_response http_status: :unauthorized ) end + + def log_audit_event + Audit::ExternalStatusCheckChangesAuditor.new(current_user, rule).execute + end end end diff --git a/ee/lib/audit/external_status_check_changes_auditor.rb b/ee/lib/audit/external_status_check_changes_auditor.rb new file mode 100644 index 0000000000000000000000000000000000000000..a5d00fd42e9bbd19f6e381545365c1945299abc8 --- /dev/null +++ b/ee/lib/audit/external_status_check_changes_auditor.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Audit + class ExternalStatusCheckChangesAuditor < ::EE::Audit::BaseChangesAuditor + def initialize(current_user, external_status_check) + @project = external_status_check.project + + super + end + + def execute + audit_changes(:name, as: 'name', entity: @project, model: model) + audit_changes(:external_url, as: 'external url', entity: @project, model: model) + end + + def attributes_from_auditable_model(column) + { + from: model.previous_changes[column].first, + to: model.previous_changes[column].last + } + end + end +end diff --git a/ee/lib/ee/audit/project_changes_auditor.rb b/ee/lib/ee/audit/project_changes_auditor.rb index 1c72fa7e3919f2078bd9d1a63a2080223a8de50a..e65431089d271190dc70a4081db8057de8cd25ee 100644 --- a/ee/lib/ee/audit/project_changes_auditor.rb +++ b/ee/lib/ee/audit/project_changes_auditor.rb @@ -16,6 +16,7 @@ def execute audit_changes(:reset_approvals_on_push, as: 'require new approvals when new commits are added to an MR', model: model) audit_changes(:disable_overriding_approvers_per_merge_request, as: 'prevent users from modifying MR approval rules in merge requests', model: model) audit_changes(:require_password_to_approve, as: 'require user password for approvals', model: model) + audit_changes(:merge_requests_template, as: 'merge_requests_template', model: model) if should_audit? :merge_requests_template audit_changes(:resolve_outdated_diff_discussions, as: 'resolve_outdated_diff_discussions', model: model) audit_changes(:printing_merge_request_link_enabled, as: 'printing_merge_request_link_enabled', model: model) diff --git a/ee/lib/ee/audit/project_setting_changes_auditor.rb b/ee/lib/ee/audit/project_setting_changes_auditor.rb index 65a06595ba22e7f7a6a3f8e0ed0526ee74aa3716..2e6d34d6620d8725d1ae2bf23d13dee90b544db5 100644 --- a/ee/lib/ee/audit/project_setting_changes_auditor.rb +++ b/ee/lib/ee/audit/project_setting_changes_auditor.rb @@ -11,9 +11,14 @@ def initialize(current_user, project_setting, project) def execute return if model.blank? - audit_changes(:squash_option, as: 'squash_option', entity: @project, model: model) - audit_changes(:allow_merge_on_skipped_pipeline, as: 'allow_merge_on_skipped_pipeline', entity: @project, - model: model) + if should_audit? :allow_merge_on_skipped_pipeline + audit_changes(:allow_merge_on_skipped_pipeline, as: 'allow_merge_on_skipped_pipeline', entity: @project, + model: model) + end + + audit_squash_option + audit_changes(:merge_commit_template, as: 'merge_commit_template', entity: @project, model: model) + audit_changes(:squash_commit_template, as: 'squash_commit_template', entity: @project, model: model) end def attributes_from_auditable_model(column) @@ -23,6 +28,22 @@ def attributes_from_auditable_model(column) target_details: @project.full_path } end + + private + + def audit_squash_option + return unless audit_required? :squash_option + + squash_option_message = _("Changed squash option to %{squash_option}") % + { squash_option: model.human_squash_option } + audit_context = { + author: @current_user, + scope: @project, + target: @project, + message: squash_option_message + } + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end end diff --git a/ee/spec/lib/audit/external_status_check_changes_auditor_spec.rb b/ee/spec/lib/audit/external_status_check_changes_auditor_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..f3521475e7b8c1e561a0a3c1a05caca3bd7acd33 --- /dev/null +++ b/ee/spec/lib/audit/external_status_check_changes_auditor_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Audit::ExternalStatusCheckChangesAuditor do + describe 'auditing external status check changes' do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:external_status_check) do + create(:external_status_check, + name: 'QA', + external_url: 'http://examplev1.com', + project: project) + end + + let_it_be(:external_status_check_changes_auditor) { described_class.new(user, external_status_check) } + + before do + stub_licensed_features(extended_audit_events: true) + end + + let(:subject) { described_class.new(user, project.external_status_checks) } + + context 'when audit change happens' do + it 'creates an event when the name changes' do + external_status_check.update!(name: 'QAv2') + + expect { external_status_check_changes_auditor.execute }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details).to include({ + change: 'name', + from: 'QA', + to: 'QAv2' + }) + end + + it 'creates an event when the external url changes' do + external_status_check.update!(external_url: 'http://examplev2.com') + + expect { external_status_check_changes_auditor.execute }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details).to include({ + change: 'external url', + from: 'http://examplev1.com', + to: 'http://examplev2.com' + }) + end + end + end +end diff --git a/ee/spec/lib/ee/audit/project_changes_auditor_spec.rb b/ee/spec/lib/ee/audit/project_changes_auditor_spec.rb index 32b37188d672f977bdd2e1a6023b4f571cbe106b..920e81f47fd491e5642c1ea9b1dde94663ca6e9b 100644 --- a/ee/spec/lib/ee/audit/project_changes_auditor_spec.rb +++ b/ee/spec/lib/ee/audit/project_changes_auditor_spec.rb @@ -83,6 +83,18 @@ expect(AuditEvent.last.details[:change]).to eq 'packages_enabled' end + it 'creates an event when the merge requests template changes' do + project.update!(merge_requests_template: 'I am a merge request template') + + expect { foo_instance.execute }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details[:change]).to eq 'merge_requests_template' + expect(AuditEvent.last.details).to include({ + change: 'merge_requests_template', + from: nil, + to: 'I am a merge request template' + }) + end + it 'creates an event when the merge requests author approval changes' do project.update!(merge_requests_author_approval: true) diff --git a/ee/spec/lib/ee/audit/project_setting_changes_auditor_spec.rb b/ee/spec/lib/ee/audit/project_setting_changes_auditor_spec.rb index fe6af59faf22aff02a7a339cb3d3c1109dcb9dd2..ac3b63f105ed04775ebb65d42106de82935a7117 100644 --- a/ee/spec/lib/ee/audit/project_setting_changes_auditor_spec.rb +++ b/ee/spec/lib/ee/audit/project_setting_changes_auditor_spec.rb @@ -22,17 +22,21 @@ project.project_setting.update!(squash_option: prev_value) end - next if new_value == prev_value + if new_value != prev_value + it 'creates an audit event' do + project.project_setting.update!(squash_option: new_value) - it 'creates an audit event' do - project.project_setting.update!(squash_option: new_value) - - expect { project_setting_changes_auditor.execute }.to change { AuditEvent.count }.by(1) - expect(AuditEvent.last.details).to include({ - change: 'squash_option', - from: prev_value, - to: new_value - }) + expect { project_setting_changes_auditor.execute }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details).to include( + { + custom_message: "Changed squash option to #{project.project_setting.human_squash_option}" + }) + end + else + it 'does not create audit event' do + project.project_setting.update!(squash_option: new_value) + expect { project_setting_changes_auditor.execute }.to not_change { AuditEvent.count } + end end end end diff --git a/ee/spec/models/approval_project_rule_spec.rb b/ee/spec/models/approval_project_rule_spec.rb index 8c74cfca0850e11e1da5751a6a647376f9756cb1..b776b903d859c99aecbd18d47776b25cf15527cd 100644 --- a/ee/spec/models/approval_project_rule_spec.rb +++ b/ee/spec/models/approval_project_rule_spec.rb @@ -246,65 +246,39 @@ let_it_be(:rule, reload: true) { create(:approval_project_rule, name: 'Vulnerability', users: [user], groups: [group]) } - shared_examples 'auditable' do - context 'when audit event queue is active' do - before do - allow(::Gitlab::Audit::EventQueue).to receive(:active?).and_return(true) - end - - it 'adds message to audit event queue' do - action! - - expect(::Gitlab::Audit::EventQueue.current).to contain_exactly(message) - end - end - - context 'when audit event queue is not active' do - before do - allow(::Gitlab::Audit::EventQueue).to receive(:active?).and_return(false) - end - - it 'does not add message to audit event queue' do - action! - - expect(::Gitlab::Audit::EventQueue.current).to be_empty - end - end - end - describe '#audit_add users after :add' do let(:action!) { rule.update!(users: [user, new_user]) } let(:message) { 'Added User Spiderman to approval group on Vulnerability rule' } - it_behaves_like 'auditable' + it_behaves_like 'audit event queue' end describe '#audit_remove users after :remove' do let(:action!) { rule.update!(users: []) } let(:message) { 'Removed User Batman from approval group on Vulnerability rule' } - it_behaves_like 'auditable' + it_behaves_like 'audit event queue' end describe '#audit_add groups after :add' do let(:action!) { rule.update!(groups: [group, new_group]) } let(:message) { 'Added Group Avengers to approval group on Vulnerability rule' } - it_behaves_like 'auditable' + it_behaves_like 'audit event queue' end describe '#audit_remove groups after :remove' do let(:action!) { rule.update!(groups: []) } let(:message) { 'Removed Group Justice League from approval group on Vulnerability rule' } - it_behaves_like 'auditable' + it_behaves_like 'audit event queue' end describe "#audit_creation after approval rule is created" do let(:action!) { create(:approval_project_rule, approvals_required: 1) } let(:message) {'Added approval rule with number of required approvals of 1'} - it_behaves_like 'auditable' + it_behaves_like 'audit event queue' end describe '#vulnerability_states_for_branch' do diff --git a/ee/spec/models/merge_requests/external_status_check_spec.rb b/ee/spec/models/merge_requests/external_status_check_spec.rb index 0bd4a4e767c9ae09dc93859096c37c29ec4cef32..427baaf8b39247a6521d886b0013c4de1d12617b 100644 --- a/ee/spec/models/merge_requests/external_status_check_spec.rb +++ b/ee/spec/models/merge_requests/external_status_check_spec.rb @@ -187,4 +187,94 @@ end end end + + describe 'callbacks', :request_store do + let_it_be(:project) { create(:project) } + let_it_be(:master_branch) { create(:protected_branch, project: project, name: 'master') } + let_it_be(:main_branch) { create(:protected_branch, project: project, name: 'main') } + let_it_be(:external_status_check, reload: true) do + create(:external_status_check, project: project, name: 'QA', protected_branches: []) + end + + describe '#audit_add branches after :add' do + context 'when branch is added from zero branches' do + let(:action!) { external_status_check.update!(protected_branches: [main_branch]) } + let(:message) { 'Added protected branch main to QA status check and removed all other branches from status check' } + + it_behaves_like 'audit event queue' + end + + context 'when another branch is added' do + before do + external_status_check.update!(protected_branches: [main_branch]) + end + + let(:action!) { external_status_check.update!(protected_branches: [main_branch, master_branch]) } + let(:message) { 'Added protected branch master to QA status check' } + + it_behaves_like 'audit event queue' + end + end + + describe '#audit_remove branches after :remove' do + context 'when all the branches are removed' do + before do + external_status_check.update!(protected_branches: [main_branch]) + end + + let(:action!) { external_status_check.update!(protected_branches: []) } + let(:message) { 'Added all branches to QA status check' } + + it_behaves_like 'audit event queue' + end + + context 'when a branch is removed' do + before do + external_status_check.update!(protected_branches: [main_branch, master_branch]) + end + + let(:action!) { external_status_check.update!(protected_branches: [master_branch]) } + let(:message) { 'Removed protected branch main from QA status check' } + + it_behaves_like 'audit event queue' + end + end + + describe '#audit_creation external status check after :create' do + context 'when protected branches are added' do + let_it_be(:external_status_check) do + described_class.new(name: 'QAv2', + project: project, + external_url: 'https://www.example.com', + protected_branch_ids: [main_branch.id, master_branch.id]) + end + + let(:action!) { external_status_check.save! } + let(:message) { 'Added QAv2 status check with protected branch(es) main, master' } + + it_behaves_like 'audit event queue' + end + + context 'when all branches are added' do + let_it_be(:external_status_check) do + described_class.new(name: 'QAv2', + project: project, + external_url: 'https://www.example.com', + protected_branch_ids: []) + end + + let(:action!) { external_status_check.save! } + let(:message) { 'Added QAv2 status check with all branches' } + + it_behaves_like 'audit event queue' + end + end + + describe '#audit_creation external status check after :create' do + let(:action!) { external_status_check.destroy! } + let(:message) { 'Removed QA status check' } + + it_behaves_like 'audit event queue' + end + end end diff --git a/ee/spec/services/external_status_checks/create_service_spec.rb b/ee/spec/services/external_status_checks/create_service_spec.rb index b65f9f63e23209890cd8e688aedd04336a84116b..fda0308d890d05ca15fe5435c9ffa6f2ffcfeb25 100644 --- a/ee/spec/services/external_status_checks/create_service_spec.rb +++ b/ee/spec/services/external_status_checks/create_service_spec.rb @@ -66,4 +66,33 @@ expect(rule.protected_branches).to contain_exactly(protected_branch) end end + + describe 'audit events' do + context 'when licensed' do + before do + stub_licensed_features(audit_events: true) + end + + context 'when external status check save operation succeeds', :request_store do + it 'logs an audit event' do + expect { subject }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details).to include({ + custom_message: "Added Test status check with protected branch(es) #{protected_branch.name}" + }) + end + end + + context 'when external status check save operation fails' do + before do + allow(::MergeRequests::ExternalStatusCheck).to receive(:save).and_return(false) + end + + it 'does not log any audit event' do + expect { subject }.not_to change { AuditEvent.count } + end + end + end + + it_behaves_like 'does not create audit event when not licensed' + end end diff --git a/ee/spec/services/external_status_checks/destroy_service_spec.rb b/ee/spec/services/external_status_checks/destroy_service_spec.rb index 91dcf02f0f54bd2a49d5caa839c94d33c4ca0f2c..29ca1d1ccfa1b7fd6a471e233bc9fe4e9dd68702 100644 --- a/ee/spec/services/external_status_checks/destroy_service_spec.rb +++ b/ee/spec/services/external_status_checks/destroy_service_spec.rb @@ -4,7 +4,7 @@ RSpec.describe ExternalStatusChecks::DestroyService do let_it_be(:project) { create(:project) } - let_it_be(:rule) { create(:external_status_check, project: project) } + let_it_be(:rule) { create(:external_status_check, name: 'QA', project: project) } let(:current_user) { project.first_owner } @@ -40,4 +40,34 @@ expect(subject.payload[:errors]).to contain_exactly('Not allowed') end end + + describe 'audit events' do + context 'when licensed' do + before do + stub_licensed_features(audit_events: true) + end + + context 'when rule destroy operation succeeds', :request_store do + it 'logs an audit event' do + expect { subject }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details).to include({ + target_type: 'MergeRequests::ExternalStatusCheck', + custom_message: 'Removed QA status check' + }) + end + end + + context 'when rule destroy operation fails' do + before do + allow(::MergeRequests::ExternalStatusCheck).to receive(:destroy).and_return(false) + end + + it 'does not log any audit event' do + expect { subject }.not_to change { AuditEvent.count } + end + end + end + + it_behaves_like 'does not create audit event when not licensed' + end end diff --git a/ee/spec/services/external_status_checks/update_service_spec.rb b/ee/spec/services/external_status_checks/update_service_spec.rb index e3449571e36824d259629046888e987b8e71d356..e4a96bc4d675d715b3748e5deb2cd2ab655db2cd 100644 --- a/ee/spec/services/external_status_checks/update_service_spec.rb +++ b/ee/spec/services/external_status_checks/update_service_spec.rb @@ -48,4 +48,75 @@ expect(subject.payload[:errors]).to contain_exactly('Not allowed') end end + + describe 'audit events' do + context 'when licensed' do + before do + stub_licensed_features(audit_events: true) + end + let_it_be(:master_branch) { create(:protected_branch, project: project, name: 'master') } + let_it_be(:main_branch) {create(:protected_branch, project: project, name: 'main')} + let_it_be(:external_status_check, reload: true) { create(:external_status_check, name: 'QA', project: project, protected_branches: []) } + + context 'when a branch is added', :request_store do + context 'when a new branch is added' do + let_it_be(:params) {{ id: project.id, check_id: external_status_check.id, protected_branch_ids: [main_branch.id] } } + + it 'logs an audit event' do + expect { subject }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details[:custom_message]).to eq "Added protected branch main to QA status check and removed all other branches from status check" + end + end + + context 'when another branch is added' do + before do + external_status_check.update!(protected_branches: [main_branch]) + end + + let_it_be(:params) {{ id: project.id, check_id: external_status_check.id, protected_branch_ids: [main_branch.id, master_branch.id] } } + + it 'logs an audit event' do + expect { subject }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details[:custom_message]).to eq "Added protected branch master to QA status check" + end + end + end + + context 'when a branch is removed', :request_store do + context 'when the only branch is removed' do + before do + external_status_check.update!(protected_branches: [main_branch]) + end + + let_it_be(:params) {{ id: project.id, check_id: external_status_check.id, protected_branch_ids: [] } } + + it 'logs an audit event' do + expect { subject }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details[:custom_message]).to eq "Added all branches to QA status check" + end + end + + context 'when a branch is removed' do + before do + external_status_check.update!(protected_branches: [main_branch, master_branch]) + end + + let_it_be(:params) {{ id: project.id, check_id: external_status_check.id, protected_branch_ids: [main_branch.id] } } + + it 'logs an audit event' do + expect { subject }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details[:custom_message]).to eq "Removed protected branch master from QA status check" + end + end + end + end + + it 'executes ExternalStatusCheckChangesAuditor' do + expect(Audit::ExternalStatusCheckChangesAuditor).to receive(:new).with(current_user, check).and_call_original + + subject + end + end + + it_behaves_like 'does not create audit event when not licensed' end diff --git a/ee/spec/support/shared_contexts/audit_event_not_licensed_shared_context.rb b/ee/spec/support/shared_contexts/audit_event_not_licensed_shared_context.rb new file mode 100644 index 0000000000000000000000000000000000000000..7736fdca910acce9f055cc818f4331d0fbb18c5b --- /dev/null +++ b/ee/spec/support/shared_contexts/audit_event_not_licensed_shared_context.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +RSpec.shared_context 'does not create audit event when not licensed' do + before do + stub_licensed_features( + admin_audit_log: false, + audit_events: false, + extended_audit_events: false + ) + end + + it 'does not log any audit event' do + expect { subject }.not_to change { AuditEvent.count } + end +end diff --git a/ee/spec/support/shared_contexts/audit_event_queue_shared_context.rb b/ee/spec/support/shared_contexts/audit_event_queue_shared_context.rb new file mode 100644 index 0000000000000000000000000000000000000000..4d6ab5205af01fe2bdac9b954787d11ca93ce5dc --- /dev/null +++ b/ee/spec/support/shared_contexts/audit_event_queue_shared_context.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +RSpec.shared_context 'audit event queue' do + context 'when audit event queue is active' do + before do + allow(::Gitlab::Audit::EventQueue).to receive(:active?).and_return(true) + end + + it 'adds message to audit event queue' do + action! + + expect(::Gitlab::Audit::EventQueue.current).to contain_exactly(message) + end + end + + context 'when audit event queue is not active' do + before do + allow(::Gitlab::Audit::EventQueue).to receive(:active?).and_return(false) + end + + it 'does not add message to audit event queue' do + action! + + expect(::Gitlab::Audit::EventQueue.current).to be_empty + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 028e5399373fd3e6d213d477debe530487588577..5160246afb2f46ab37932120fb5b059e67872be4 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -7034,6 +7034,9 @@ msgstr "" msgid "Changed reviewer(s)." msgstr "" +msgid "Changed squash option to %{squash_option}" +msgstr "" + msgid "Changed the title to \"%{title_param}\"." msgstr "" diff --git a/spec/models/project_setting_spec.rb b/spec/models/project_setting_spec.rb index d03eb3c8bfe4f39a20b7f88f8b2a0e51417521a9..867ad843406eb857945e16688f8b5add3d7050d1 100644 --- a/spec/models/project_setting_spec.rb +++ b/spec/models/project_setting_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' RSpec.describe ProjectSetting, type: :model do + using RSpec::Parameterized::TableSyntax it { is_expected.to belong_to(:project) } describe 'validations' do @@ -27,6 +28,23 @@ end end + describe '#human_squash_option' do + where(:squash_option, :human_squash_option) do + 'never' | 'Do not allow' + 'always' | 'Require' + 'default_on' | 'Encourage' + 'default_off' | 'Allow' + end + + with_them do + let(:project_setting) { create(:project_setting, squash_option: ProjectSetting.squash_options[squash_option]) } + + subject { project_setting.human_squash_option } + + it { is_expected.to eq(human_squash_option) } + end + end + def valid_target_platform_combinations target_platforms = described_class::ALLOWED_TARGET_PLATFORMS diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb index f7c723cd1343b4484db712831dd962d7906d7c47..366de809bedb88a69f0a2945593ad9bad42cf74d 100644 --- a/spec/models/protected_branch_spec.rb +++ b/spec/models/protected_branch_spec.rb @@ -324,4 +324,10 @@ .to match_array([branch_id]) end end + + describe '.downcase_humanized_name' do + it 'returns downcase humanized name' do + expect(described_class.downcase_humanized_name).to eq 'protected branch' + end + end end