diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-08-30 15:22:09 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-08-30 15:22:09 +0300 |
commit | eba52140851d2fb08665119c0a3997d0612ccb88 (patch) | |
tree | 4bc562fadc518009435642e0bd265c8fb5bdc5a5 /spec | |
parent | 2da7c8579601c14a93d4291b8cf5fa39c6eeabd8 (diff) |
Add latest changes from gitlab-org/security/gitlab@14-2-stable-ee
Diffstat (limited to 'spec')
-rw-r--r-- | spec/helpers/integrations_helper_spec.rb | 15 | ||||
-rw-r--r-- | spec/lib/banzai/filter/references/design_reference_filter_spec.rb | 46 | ||||
-rw-r--r-- | spec/models/design_management/design_spec.rb | 14 | ||||
-rw-r--r-- | spec/models/integrations/datadog_spec.rb | 12 | ||||
-rw-r--r-- | spec/services/groups/destroy_service_spec.rb | 133 |
5 files changed, 136 insertions, 84 deletions
diff --git a/spec/helpers/integrations_helper_spec.rb b/spec/helpers/integrations_helper_spec.rb index 8e652d2f150..3a7d4d12513 100644 --- a/spec/helpers/integrations_helper_spec.rb +++ b/spec/helpers/integrations_helper_spec.rb @@ -98,4 +98,19 @@ RSpec.describe IntegrationsHelper do end end end + + describe '#jira_issue_breadcrumb_link' do + let(:issue_reference) { nil } + + subject { helper.jira_issue_breadcrumb_link(issue_reference) } + + context 'when issue_reference contains HTML' do + let(:issue_reference) { "<script>alert('XSS')</script>" } + + it 'escapes issue reference' do + is_expected.not_to include(issue_reference) + is_expected.to include(html_escape(issue_reference)) + end + end + end end diff --git a/spec/lib/banzai/filter/references/design_reference_filter_spec.rb b/spec/lib/banzai/filter/references/design_reference_filter_spec.rb index cb1f3d520a4..d616aabea45 100644 --- a/spec/lib/banzai/filter/references/design_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/references/design_reference_filter_spec.rb @@ -90,11 +90,8 @@ RSpec.describe Banzai::Filter::References::DesignReferenceFilter do [ ['simple.png'], ['SIMPLE.PNG'], - ['has spaces.png'], ['has-hyphen.jpg'], - ['snake_case.svg'], - ['has "quotes".svg'], - ['has <special> characters [o].svg'] + ['snake_case.svg'] ] end @@ -138,40 +135,25 @@ RSpec.describe Banzai::Filter::References::DesignReferenceFilter do end end - context 'a design with a quoted filename' do - let(:filename) { %q{A "very" good file.png} } - let(:design) { create(:design, :with_versions, issue: issue, filename: filename) } - - it 'links to the design' do - expect(doc.css('a').first.attr('href')) - .to eq url_for_design(design) - end - end - context 'internal reference' do it_behaves_like 'a reference containing an element node' - context 'the reference is valid' do - it_behaves_like 'a good link reference' + it_behaves_like 'a good link reference' - context 'the filename needs to be escaped' do - where(:filename) do - [ - ['with some spaces.png'], - ['with <script>console.log("pwded")<%2Fscript>.png'] - ] - end + context 'the filename contains invalid characters' do + where(:filename) do + [ + ['with some spaces.png'], + ['with <script>console.log("pwded")<%2Fscript>.png'], + ['foo"bar.png'], + ['A "very" good file.png'] + ] + end - with_them do - let(:design) { create(:design, :with_versions, filename: filename, issue: issue) } - let(:link) { doc.css('a').first } + with_them do + let(:design) { create(:design, :with_versions, filename: filename, issue: issue) } - it 'replaces the content with the reference, but keeps the link', :aggregate_failures do - expect(doc.text).to eq(CGI.unescapeHTML("Added #{design.to_reference}")) - expect(link.attr('title')).to eq(design.filename) - expect(link.attr('href')).to eq(design_url) - end - end + it_behaves_like 'a no-op filter' end end diff --git a/spec/models/design_management/design_spec.rb b/spec/models/design_management/design_spec.rb index f2ce5e42eaf..b0601ea3f08 100644 --- a/spec/models/design_management/design_spec.rb +++ b/spec/models/design_management/design_spec.rb @@ -572,6 +572,12 @@ RSpec.describe DesignManagement::Design do expect(described_class.link_reference_pattern).not_to match(url_for_designs(issue)) end + it 'intentionally ignores filenames with any special character' do + design = build(:design, issue: issue, filename: '"invalid') + + expect(described_class.link_reference_pattern).not_to match(url_for_design(design)) + end + where(:ext) do (described_class::SAFE_IMAGE_EXT + described_class::DANGEROUS_IMAGE_EXT).flat_map do |ext| [[ext], [ext.upcase]] @@ -593,14 +599,6 @@ RSpec.describe DesignManagement::Design do ) end - context 'the file needs to be encoded' do - let(:filename) { "my file.#{ext}" } - - it 'extracts the encoded filename' do - expect(captures).to include('url_filename' => 'my%20file.' + ext) - end - end - context 'the file is all upper case' do let(:filename) { "file.#{ext}".upcase } diff --git a/spec/models/integrations/datadog_spec.rb b/spec/models/integrations/datadog_spec.rb index 677bd4c5e48..7049e64c2ce 100644 --- a/spec/models/integrations/datadog_spec.rb +++ b/spec/models/integrations/datadog_spec.rb @@ -127,18 +127,6 @@ RSpec.describe Integrations::Datadog do end end - describe '#api_keys_url' do - subject { instance.api_keys_url } - - it { is_expected.to eq("https://app.#{dd_site}/account/settings#api") } - - context 'with unset datadog_site' do - let(:dd_site) { '' } - - it { is_expected.to eq("https://docs.datadoghq.com/account_management/api-app-keys/") } - end - end - describe '#test' do subject(:result) { saved_instance.test(pipeline_data) } diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb index c794acdf76d..5135be8fff5 100644 --- a/spec/services/groups/destroy_service_spec.rb +++ b/spec/services/groups/destroy_service_spec.rb @@ -152,56 +152,125 @@ RSpec.describe Groups::DestroyService do end context 'for shared groups within different hierarchies' do - let(:shared_with_group) { group } - let!(:shared_group) { create(:group, :private) } - let!(:shared_group_child) { create(:group, :private, parent: shared_group) } - let!(:shared_group_user) { create(:user) } + let(:group1) { create(:group, :private) } + let(:group2) { create(:group, :private) } - let!(:project) { create(:project, group: shared_group) } - let!(:project_child) { create(:project, group: shared_group_child) } + let(:group1_user) { create(:user) } + let(:group2_user) { create(:user) } before do - shared_group.add_user(shared_group_user, Gitlab::Access::OWNER) - - create(:group_group_link, shared_group: shared_group, shared_with_group: shared_with_group) - shared_with_group.refresh_members_authorized_projects + group1.add_user(group1_user, Gitlab::Access::OWNER) + group2.add_user(group2_user, Gitlab::Access::OWNER) end - context 'the shared group is deleted' do - it 'updates project authorization' do - expect(shared_group_user.can?(:read_project, project)).to eq(true) - expect(shared_group_user.can?(:read_project, project_child)).to eq(true) + context 'when a project is shared with a group' do + let!(:group1_project) { create(:project, :private, group: group1) } + + before do + create(:project_group_link, project: group1_project, group: group2) + end + + context 'and the shared group is deleted' do + it 'updates project authorizations so group2 users no longer have access', :aggregate_failures do + expect(group1_user.can?(:read_project, group1_project)).to eq(true) + expect(group2_user.can?(:read_project, group1_project)).to eq(true) - destroy_group(shared_group, shared_group_user, false) + destroy_group(group2, group2_user, false) - expect(shared_group_user.can?(:read_project, project)).to eq(false) - expect(shared_group_user.can?(:read_project, project_child)).to eq(false) + expect(group1_user.can?(:read_project, group1_project)).to eq(true) + expect(group2_user.can?(:read_project, group1_project)).to eq(false) + end + + it 'calls the service to update project authorizations only with necessary user ids' do + expect(UserProjectAccessChangedService) + .to receive(:new).with(array_including(group2_user.id)).and_call_original + + destroy_group(group2, group2_user, false) + end end - it 'does not make use of specific service to update project_authorizations records' do - expect(UserProjectAccessChangedService) - .not_to receive(:new).with(shared_group.user_ids_for_project_authorizations).and_call_original + context 'and the group is shared with another group' do + let(:group3) { create(:group, :private) } + let(:group3_user) { create(:user) } + + before do + group3.add_user(group3_user, Gitlab::Access::OWNER) + + create(:group_group_link, shared_group: group2, shared_with_group: group3) + group3.refresh_members_authorized_projects + end + + it 'updates project authorizations so group2 and group3 users no longer have access', :aggregate_failures do + expect(group1_user.can?(:read_project, group1_project)).to eq(true) + expect(group2_user.can?(:read_project, group1_project)).to eq(true) + expect(group3_user.can?(:read_project, group1_project)).to eq(true) + + destroy_group(group2, group2_user, false) + + expect(group1_user.can?(:read_project, group1_project)).to eq(true) + expect(group2_user.can?(:read_project, group1_project)).to eq(false) + expect(group3_user.can?(:read_project, group1_project)).to eq(false) + end - destroy_group(shared_group, shared_group_user, false) + it 'calls the service to update project authorizations only with necessary user ids' do + expect(UserProjectAccessChangedService) + .to receive(:new).with(array_including(group2_user.id, group3_user.id)).and_call_original + + destroy_group(group2, group2_user, false) + end end end - context 'the shared_with group is deleted' do - it 'updates project authorization' do - expect(user.can?(:read_project, project)).to eq(true) - expect(user.can?(:read_project, project_child)).to eq(true) + context 'when a group is shared with a group' do + let!(:group2_project) { create(:project, :private, group: group2) } - destroy_group(shared_with_group, user, false) + before do + create(:group_group_link, shared_group: group2, shared_with_group: group1) + group1.refresh_members_authorized_projects + end + + context 'and the shared group is deleted' do + it 'updates project authorizations since the project has been deleted with the group', :aggregate_failures do + expect(group1_user.can?(:read_project, group2_project)).to eq(true) + expect(group2_user.can?(:read_project, group2_project)).to eq(true) + + destroy_group(group2, group2_user, false) - expect(user.can?(:read_project, project)).to eq(false) - expect(user.can?(:read_project, project_child)).to eq(false) + expect(group1_user.can?(:read_project, group2_project)).to eq(false) + expect(group2_user.can?(:read_project, group2_project)).to eq(false) + end + + it 'does not call the service to update project authorizations' do + expect(UserProjectAccessChangedService).not_to receive(:new) + + destroy_group(group2, group2_user, false) + end end - it 'makes use of a specific service to update project_authorizations records' do - expect(UserProjectAccessChangedService) - .to receive(:new).with(shared_with_group.user_ids_for_project_authorizations).and_call_original + context 'the shared_with group is deleted' do + let!(:group2_subgroup) { create(:group, :private, parent: group2)} + let!(:group2_subgroup_project) { create(:project, :private, group: group2_subgroup) } - destroy_group(shared_with_group, user, false) + it 'updates project authorizations so users of both groups lose access', :aggregate_failures do + expect(group1_user.can?(:read_project, group2_project)).to eq(true) + expect(group2_user.can?(:read_project, group2_project)).to eq(true) + expect(group1_user.can?(:read_project, group2_subgroup_project)).to eq(true) + expect(group2_user.can?(:read_project, group2_subgroup_project)).to eq(true) + + destroy_group(group1, group1_user, false) + + expect(group1_user.can?(:read_project, group2_project)).to eq(false) + expect(group2_user.can?(:read_project, group2_project)).to eq(true) + expect(group1_user.can?(:read_project, group2_subgroup_project)).to eq(false) + expect(group2_user.can?(:read_project, group2_subgroup_project)).to eq(true) + end + + it 'calls the service to update project authorizations only with necessary user ids' do + expect(UserProjectAccessChangedService) + .to receive(:new).with([group1_user.id]).and_call_original + + destroy_group(group1, group1_user, false) + end end end end |