diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-09-28 21:59:41 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-09-28 21:59:41 +0000 |
commit | cc201d1e1be2c8f4de2e2265c2b83bd925f8a260 (patch) | |
tree | 7347a079cde32c08900547d96a7c5ddbc2a50259 /spec | |
parent | 70d9f335be46efecb1328cd2b7da3f3e17516d7d (diff) | |
download | gitlab-ce-cc201d1e1be2c8f4de2e2265c2b83bd925f8a260.tar.gz |
Add latest changes from gitlab-org/security/gitlab@15-4-stable-ee
Diffstat (limited to 'spec')
-rw-r--r-- | spec/policies/issuable_policy_spec.rb | 21 | ||||
-rw-r--r-- | spec/policies/todo_policy_spec.rb | 115 | ||||
-rw-r--r-- | spec/requests/api/todos_spec.rb | 2 | ||||
-rw-r--r-- | spec/services/todos/allowed_target_filter_service_spec.rb | 105 |
4 files changed, 188 insertions, 55 deletions
diff --git a/spec/policies/issuable_policy_spec.rb b/spec/policies/issuable_policy_spec.rb index fd7ec5917d6..c02294571ff 100644 --- a/spec/policies/issuable_policy_spec.rb +++ b/spec/policies/issuable_policy_spec.rb @@ -31,6 +31,10 @@ RSpec.describe IssuablePolicy, models: true do expect(policies).to be_allowed(:resolve_note) end + it 'allows reading confidential notes' do + expect(policies).to be_allowed(:read_confidential_notes) + end + context 'when user is able to read project' do it 'enables user to read and update issuables' do expect(policies).to be_allowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request, :reopen_merge_request) @@ -86,6 +90,15 @@ RSpec.describe IssuablePolicy, models: true do end end + context 'when user is assignee of issuable' do + let(:issue) { create(:issue, project: project, assignees: [user]) } + let(:policies) { described_class.new(user, issue) } + + it 'allows reading confidential notes' do + expect(policies).to be_allowed(:read_confidential_notes) + end + end + context 'when discussion is locked for the issuable' do let(:issue) { create(:issue, project: project, discussion_locked: true) } @@ -138,6 +151,10 @@ RSpec.describe IssuablePolicy, models: true do it 'does not allow timelogs creation' do expect(permissions(guest, issue)).to be_disallowed(:create_timelog) end + + it 'does not allow reading confidential notes' do + expect(permissions(guest, issue)).to be_disallowed(:read_confidential_notes) + end end context 'when user is a guest member of the project and the author of the issuable' do @@ -152,6 +169,10 @@ RSpec.describe IssuablePolicy, models: true do it 'allows timelogs creation' do expect(permissions(reporter, issue)).to be_allowed(:create_timelog) end + + it 'allows reading confidential notes' do + expect(permissions(reporter, issue)).to be_allowed(:read_confidential_notes) + end end context 'when subject is a Merge Request' do diff --git a/spec/policies/todo_policy_spec.rb b/spec/policies/todo_policy_spec.rb index 16435b21666..34ba7bf9276 100644 --- a/spec/policies/todo_policy_spec.rb +++ b/spec/policies/todo_policy_spec.rb @@ -3,53 +3,100 @@ require 'spec_helper' RSpec.describe TodoPolicy do - let_it_be(:author) { create(:user) } - - let_it_be(:user1) { create(:user) } - let_it_be(:user2) { create(:user) } - let_it_be(:user3) { create(:user) } + using RSpec::Parameterized::TableSyntax let_it_be(:project) { create(:project) } let_it_be(:issue) { create(:issue, project: project) } - - let_it_be(:todo1) { create(:todo, author: author, user: user1, issue: issue) } - let_it_be(:todo2) { create(:todo, author: author, user: user2, issue: issue) } - let_it_be(:todo3) { create(:todo, author: author, user: user2) } - let_it_be(:todo4) { create(:todo, author: author, user: user3, issue: issue) } + let_it_be(:author) { create(:user) } def permissions(user, todo) described_class.new(user, todo) end - before_all do - project.add_developer(user1) - project.add_developer(user2) + shared_examples 'grants the expected permissions' do |policy| + it do + if allowed + expect(permissions(user, todo)).to be_allowed(policy) + else + expect(permissions(user, todo)).to be_disallowed(policy) + end + end end describe 'own_todo' do - it 'allows owners to access their own todos if they can read todo target' do - [ - [user1, todo1], - [user2, todo2] - ].each do |user, todo| - expect(permissions(user, todo)).to be_allowed(:read_todo) - end + let_it_be(:user1) { create(:user) } + let_it_be(:user2) { create(:user) } + let_it_be(:user3) { create(:user) } + + let_it_be(:todo1) { create(:todo, author: author, user: user1, issue: issue) } + let_it_be(:todo2) { create(:todo, author: author, user: user2, issue: issue) } + let_it_be(:todo3) { create(:todo, author: author, user: user2) } + let_it_be(:todo4) { create(:todo, author: author, user: user3, issue: issue) } + + where(:user, :todo, :allowed) do + ref(:user1) | ref(:todo1) | true + ref(:user2) | ref(:todo2) | true + ref(:user1) | ref(:todo2) | false + ref(:user1) | ref(:todo3) | false + ref(:user2) | ref(:todo1) | false + ref(:user2) | ref(:todo4) | false + ref(:user3) | ref(:todo1) | false + ref(:user3) | ref(:todo2) | false + ref(:user3) | ref(:todo3) | false + ref(:user3) | ref(:todo4) | false + ref(:user2) | ref(:todo3) | false end - it 'does not allow users to access todos of other users' do - [ - [user1, todo2], - [user1, todo3], - [user2, todo1], - [user2, todo4], - [user3, todo1], - [user3, todo2], - [user3, todo3], - [user2, todo3], - [user3, todo4] - ].each do |user, todo| - expect(permissions(user, todo)).to be_disallowed(:read_todo) - end + before_all do + project.add_developer(user1) + project.add_developer(user2) + end + + with_them do + it_behaves_like 'grants the expected permissions', :read_todo + end + end + + describe 'read_note' do + let_it_be(:non_member) { create(:user) } + let_it_be(:guest) { create(:user) } + let_it_be(:reporter) { create(:user) } + + let_it_be(:note) { create(:note, noteable: issue, project: project) } + let_it_be(:internal) { create(:note, :confidential, noteable: issue, project: project) } + + let_it_be(:no_note_todo1) { create(:todo, author: author, user: reporter, issue: issue) } + let_it_be(:note_todo1) { create(:todo, note: note, author: author, user: reporter, issue: issue) } + let_it_be(:internal_note_todo1) { create(:todo, note: internal, author: author, user: reporter, issue: issue) } + + let_it_be(:no_note_todo2) { create(:todo, author: author, user: guest, issue: issue) } + let_it_be(:note_todo2) { create(:todo, note: note, author: author, user: guest, issue: issue) } + let_it_be(:internal_note_todo2) { create(:todo, note: internal, author: author, user: guest, issue: issue) } + + let_it_be(:no_note_todo3) { create(:todo, author: author, user: non_member, issue: issue) } + let_it_be(:note_todo3) { create(:todo, note: note, author: author, user: non_member, issue: issue) } + let_it_be(:internal_note_todo3) { create(:todo, note: internal, author: author, user: non_member, issue: issue) } + + where(:user, :todo, :allowed) do + ref(:reporter) | ref(:no_note_todo1) | true + ref(:reporter) | ref(:note_todo1) | true + ref(:reporter) | ref(:internal_note_todo1) | true + ref(:guest) | ref(:no_note_todo2) | true + ref(:guest) | ref(:note_todo2) | true + ref(:guest) | ref(:internal_note_todo2) | false + ref(:non_member) | ref(:no_note_todo3) | false + ref(:non_member) | ref(:note_todo3) | false + ref(:non_member) | ref(:internal_note_todo3) | false + end + + before_all do + project.add_guest(guest) + project.add_reporter(reporter) + end + + with_them do + it_behaves_like 'grants the expected permissions', :read_todo + it_behaves_like 'grants the expected permissions', :update_todo end end end diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb index 0944bfb6ba6..d75ca0d4f6f 100644 --- a/spec/requests/api/todos_spec.rb +++ b/spec/requests/api/todos_spec.rb @@ -224,7 +224,7 @@ RSpec.describe API::Todos do project: project_1, target: create(:design, issue: issue), author: create(:user), - note: create(:note, project: project_1, note: "I am note, hear me roar")) + note: create(:note, :confidential, project: project_1, note: "I am note, hear me roar")) end def api_request diff --git a/spec/services/todos/allowed_target_filter_service_spec.rb b/spec/services/todos/allowed_target_filter_service_spec.rb index 707df8e8514..1d2b1b044db 100644 --- a/spec/services/todos/allowed_target_filter_service_spec.rb +++ b/spec/services/todos/allowed_target_filter_service_spec.rb @@ -10,14 +10,23 @@ RSpec.describe Todos::AllowedTargetFilterService do let_it_be(:unauthorized_group) { create(:group, :private) } let_it_be(:unauthorized_project) { create(:project, group: unauthorized_group) } let_it_be(:user) { create(:user) } + let_it_be(:authorized_issue) { create(:issue, project: authorized_project) } let_it_be(:authorized_issue_todo) { create(:todo, project: authorized_project, target: authorized_issue, user: user) } + let_it_be(:authorized_note) { create(:note, noteable: authorized_issue, project: authorized_project) } + let_it_be(:authorized_note_todo) { create(:todo, project: authorized_project, target: authorized_issue, note: authorized_note, user: user) } + let_it_be(:confidential_issue) { create(:issue, :confidential, project: authorized_project) } + let_it_be(:confidential_issue_todo) { create(:todo, project: authorized_project, target: confidential_issue, user: user) } + let_it_be(:confidential_note) { create(:note, :confidential, noteable: confidential_issue, project: authorized_project) } + let_it_be(:confidential_note_todo) { create(:todo, project: authorized_project, target: authorized_issue, note: confidential_note, user: user) } let_it_be(:unauthorized_issue) { create(:issue, project: unauthorized_project) } let_it_be(:unauthorized_issue_todo) { create(:todo, project: unauthorized_project, target: unauthorized_issue, user: user) } let_it_be(:authorized_design) { create(:design, issue: authorized_issue) } let_it_be(:authorized_design_todo) { create(:todo, project: authorized_project, target: authorized_design, user: user) } let_it_be(:unauthorized_design) { create(:design, issue: unauthorized_issue) } let_it_be(:unauthorized_design_todo) { create(:todo, project: unauthorized_project, target: unauthorized_design, user: user) } + let_it_be(:unauthorized_note) { create(:note, noteable: unauthorized_issue, project: unauthorized_project) } + let_it_be(:unauthorized_note_todo) { create(:todo, project: unauthorized_project, target: unauthorized_issue, note: unauthorized_note, user: user) } # Cannot use let_it_be with MRs let(:authorized_mr) { create(:merge_request, source_project: authorized_project) } @@ -25,35 +34,91 @@ RSpec.describe Todos::AllowedTargetFilterService do let(:unauthorized_mr) { create(:merge_request, source_project: unauthorized_project) } let(:unauthorized_mr_todo) { create(:todo, project: unauthorized_project, user: user, target: unauthorized_mr) } - before_all do - authorized_group.add_developer(user) - end - describe '#execute' do + let(:all_todos) { authorized_todos + unauthorized_todos } + subject(:execute_service) { described_class.new(all_todos, user).execute } - let!(:all_todos) { authorized_todos + unauthorized_todos } + shared_examples 'allowed Todos filter' do + before do + enable_design_management + end - let(:authorized_todos) do - [ - authorized_mr_todo, - authorized_issue_todo, - authorized_design_todo - ] + it { is_expected.to match_array(authorized_todos) } end - let(:unauthorized_todos) do - [ - unauthorized_mr_todo, - unauthorized_issue_todo, - unauthorized_design_todo - ] + context 'with reporter user' do + before_all do + authorized_group.add_reporter(user) + end + + it_behaves_like 'allowed Todos filter' do + let(:authorized_todos) do + [ + authorized_mr_todo, + authorized_issue_todo, + confidential_issue_todo, + confidential_note_todo, + authorized_design_todo + ] + end + + let(:unauthorized_todos) do + [ + unauthorized_mr_todo, + unauthorized_issue_todo, + unauthorized_note_todo, + unauthorized_design_todo + ] + end + end end - before do - enable_design_management + context 'with guest user' do + before_all do + authorized_group.add_guest(user) + end + + it_behaves_like 'allowed Todos filter' do + let(:authorized_todos) do + [ + authorized_issue_todo, + authorized_design_todo + ] + end + + let(:unauthorized_todos) do + [ + authorized_mr_todo, + confidential_issue_todo, + confidential_note_todo, + unauthorized_mr_todo, + unauthorized_issue_todo, + unauthorized_note_todo, + unauthorized_design_todo + ] + end + end end - it { is_expected.to match_array(authorized_todos) } + context 'with a non-member user' do + it_behaves_like 'allowed Todos filter' do + let(:authorized_todos) { [] } + + let(:unauthorized_todos) do + [ + authorized_issue_todo, + authorized_design_todo, + authorized_mr_todo, + confidential_issue_todo, + confidential_note_todo, + unauthorized_mr_todo, + unauthorized_issue_todo, + unauthorized_note_todo, + unauthorized_design_todo + ] + end + end + end end end |