From 0f74d03f0d6e1a2e208b228d637da85304ff5965 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 27 Oct 2021 10:13:45 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@14-2-stable-ee --- app/graphql/mutations/issues/set_severity.rb | 2 + app/services/issuable_base_service.rb | 1 + lib/gitlab/import_export/project/import_export.yml | 3 ++ .../import_export/project/relation_factory.rb | 5 ++ spec/graphql/mutations/issues/set_severity_spec.rb | 54 +++++++++++++++++----- .../import_export/project/relation_factory_spec.rb | 29 ++++++++++++ .../import_export/project/tree_restorer_spec.rb | 2 +- .../gitlab/import_export/safe_model_attributes.yml | 1 - spec/services/issues/update_service_spec.rb | 36 +++++++++------ 9 files changed, 106 insertions(+), 27 deletions(-) diff --git a/app/graphql/mutations/issues/set_severity.rb b/app/graphql/mutations/issues/set_severity.rb index 778563ba053..872a0e7b33d 100644 --- a/app/graphql/mutations/issues/set_severity.rb +++ b/app/graphql/mutations/issues/set_severity.rb @@ -8,6 +8,8 @@ module Mutations argument :severity, Types::IssuableSeverityEnum, required: true, description: 'Set the incident severity level.' + authorize :admin_issue + def resolve(project_path:, iid:, severity:) issue = authorized_find!(project_path: project_path, iid: iid) project = issue.project diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 0984238517e..4532d46ddd7 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -139,6 +139,7 @@ class IssuableBaseService < ::BaseProjectService def filter_severity(issuable) severity = params.delete(:severity) return unless severity && issuable.supports_severity? + return unless can_admin_issuable?(issuable) severity = IssuableSeverity::DEFAULT unless IssuableSeverity.severities.key?(severity) return if severity == issuable.severity diff --git a/lib/gitlab/import_export/project/import_export.yml b/lib/gitlab/import_export/project/import_export.yml index eac671a3a05..99088c844ec 100644 --- a/lib/gitlab/import_export/project/import_export.yml +++ b/lib/gitlab/import_export/project/import_export.yml @@ -174,6 +174,7 @@ excluded_attributes: - :services - :exported_protected_branches - :repository_size_limit + - :external_webhook_token namespaces: - :runners_token - :runners_token_encrypted @@ -377,6 +378,8 @@ excluded_attributes: system_note_metadata: - :description_version_id - :note_id + pipeline_schedules: + - :active methods: notes: - :type diff --git a/lib/gitlab/import_export/project/relation_factory.rb b/lib/gitlab/import_export/project/relation_factory.rb index 102fcedd2fc..888a5a10f2c 100644 --- a/lib/gitlab/import_export/project/relation_factory.rb +++ b/lib/gitlab/import_export/project/relation_factory.rb @@ -84,6 +84,7 @@ module Gitlab when :'Ci::Pipeline' then setup_pipeline when *BUILD_MODELS then setup_build when :issues then setup_issue + when :'Ci::PipelineSchedule' then setup_pipeline_schedule end update_project_references @@ -143,6 +144,10 @@ module Gitlab @relation_hash['relative_position'] = compute_relative_position end + def setup_pipeline_schedule + @relation_hash['active'] = false + end + def compute_relative_position return unless max_relative_position diff --git a/spec/graphql/mutations/issues/set_severity_spec.rb b/spec/graphql/mutations/issues/set_severity_spec.rb index 7ce9c7f6621..84736fe7ee6 100644 --- a/spec/graphql/mutations/issues/set_severity_spec.rb +++ b/spec/graphql/mutations/issues/set_severity_spec.rb @@ -3,26 +3,58 @@ require 'spec_helper' RSpec.describe Mutations::Issues::SetSeverity do - let_it_be(:user) { create(:user) } - let_it_be(:issue) { create(:incident) } + let_it_be(:project) { create(:project) } + let_it_be(:guest) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:issue) { create(:incident, project: project) } let(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) } - specify { expect(described_class).to require_graphql_authorizations(:update_issue) } + specify { expect(described_class).to require_graphql_authorizations(:update_issue, :admin_issue) } + + before_all do + project.add_guest(guest) + project.add_reporter(reporter) + end describe '#resolve' do let(:severity) { 'critical' } - let(:mutated_incident) { subject[:issue] } - subject(:resolve) { mutation.resolve(project_path: issue.project.full_path, iid: issue.iid, severity: severity) } + subject(:resolve) do + mutation.resolve( + project_path: issue.project.full_path, + iid: issue.iid, + severity: severity + ) + end - it_behaves_like 'permission level for issue mutation is correctly verified' + context 'as guest' do + let(:user) { guest } - context 'when the user can update the issue' do - before do - issue.project.add_developer(user) + it 'raises an error' do + expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + end + + context 'and also author' do + let!(:issue) { create(:incident, project: project, author: user) } + + it 'raises an error' do + expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + end end + context 'and also assignee' do + let!(:issue) { create(:incident, project: project, assignee_ids: [user.id]) } + + it 'raises an error' do + expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + end + end + end + + context 'as reporter' do + let(:user) { reporter } + context 'when issue type is incident' do context 'when severity has a correct value' do it 'updates severity' do @@ -48,9 +80,9 @@ RSpec.describe Mutations::Issues::SetSeverity do end context 'when issue type is not incident' do - let!(:issue) { create(:issue) } + let!(:issue) { create(:issue, project: project) } - it 'does not updates the issue' do + it 'does not update the issue' do expect { resolve }.not_to change { issue.updated_at } end end diff --git a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb index 88bd71d3d64..49df2313924 100644 --- a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb @@ -267,6 +267,35 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory, :use_clean_rails_ end end + context 'pipeline_schedule' do + let(:relation_sym) { :pipeline_schedules } + let(:relation_hash) do + { + "id": 3, + "created_at": "2016-07-22T08:55:44.161Z", + "updated_at": "2016-07-22T08:55:44.161Z", + "description": "pipeline schedule", + "ref": "main", + "cron": "0 4 * * 0", + "cron_timezone": "UTC", + "active": value, + "project_id": project.id + } + end + + subject { created_object.active } + + [true, false].each do |v| + context "when relation_hash has active set to #{v}" do + let(:value) { v } + + it "the created object is not active" do + expect(created_object.active).to eq(false) + end + end + end + end + # `project_id`, `described_class.USER_REFERENCES`, noteable_id, target_id, and some project IDs are already # re-assigned by described_class. context 'Potentially hazardous foreign keys' do diff --git a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb index 518a9337826..f512f49764d 100644 --- a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb @@ -376,7 +376,7 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do expect(pipeline_schedule.ref).to eq('master') expect(pipeline_schedule.cron).to eq('0 4 * * 0') expect(pipeline_schedule.cron_timezone).to eq('UTC') - expect(pipeline_schedule.active).to eq(true) + expect(pipeline_schedule.active).to eq(false) end end diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index b238a4733e3..e5558866576 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -555,7 +555,6 @@ Project: - merge_requests_rebase_enabled - jobs_cache_index - external_authorization_classification_label -- external_webhook_token - pages_https_only - merge_requests_disable_committers_approval - require_password_to_approve diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 29ac7df88eb..27e4351241e 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Issues::UpdateService, :mailer do let_it_be(:user) { create(:user) } let_it_be(:user2) { create(:user) } let_it_be(:user3) { create(:user) } + let_it_be(:guest) { create(:user) } let_it_be(:group) { create(:group, :public) } let_it_be(:project, reload: true) { create(:project, :repository, group: group) } let_it_be(:label) { create(:label, project: project) } @@ -24,6 +25,7 @@ RSpec.describe Issues::UpdateService, :mailer do project.add_maintainer(user) project.add_developer(user2) project.add_developer(user3) + project.add_guest(guest) end describe 'execute' do @@ -95,9 +97,7 @@ RSpec.describe Issues::UpdateService, :mailer do end context 'user is a guest' do - before do - project.add_guest(user) - end + let(:user) { guest } it 'does not assign the sentry error' do update_issue(opts) @@ -245,11 +245,7 @@ RSpec.describe Issues::UpdateService, :mailer do context 'from issue to restricted issue types' do context 'without sufficient permissions' do - let(:user) { create(:user) } - - before do - project.add_guest(user) - end + let(:user) { guest } it 'does nothing to the labels' do expect { update_issue(issue_type: 'issue') }.not_to change(issue.labels, :count) @@ -394,12 +390,6 @@ RSpec.describe Issues::UpdateService, :mailer do end context 'when current user cannot admin issues in the project' do - let(:guest) { create(:user) } - - before do - project.add_guest(guest) - end - it 'filters out params that cannot be set without the :admin_issue permission' do described_class.new( project: project, current_user: guest, params: opts.merge( @@ -1100,6 +1090,24 @@ RSpec.describe Issues::UpdateService, :mailer do it_behaves_like 'does not change the severity' end + + context 'as guest' do + let(:user) { guest } + + it_behaves_like 'does not change the severity' + + context 'and also author' do + let(:issue) { create(:incident, project: project, author: user) } + + it_behaves_like 'does not change the severity' + end + + context 'and also assignee' do + let(:issue) { create(:incident, project: project, assignee_ids: [user.id]) } + + it_behaves_like 'does not change the severity' + end + end end context 'when severity has been set before' do -- cgit v1.2.3