diff options
author | Jarka Kadlecová <jarka@gitlab.com> | 2018-08-06 10:42:51 +0200 |
---|---|---|
committer | Jarka Kadlecová <jarka@gitlab.com> | 2018-08-06 10:42:51 +0200 |
commit | 02b077925dedca390be3e8c4c7960d89ea8d4c6e (patch) | |
tree | ccdf867937ac79f0dcf621965ddfc3afdef8a4e6 /spec | |
parent | 4d4b8f8bbedbfadb49e12df2b123b3528cda4c08 (diff) | |
download | gitlab-ce-02b077925dedca390be3e8c4c7960d89ea8d4c6e.tar.gz |
Fix removing todos for users without access
Diffstat (limited to 'spec')
4 files changed, 228 insertions, 46 deletions
diff --git a/spec/services/todos/destroy/confidential_issue_service_spec.rb b/spec/services/todos/destroy/confidential_issue_service_spec.rb index 54d1d7e83f1..3294f7509aa 100644 --- a/spec/services/todos/destroy/confidential_issue_service_spec.rb +++ b/spec/services/todos/destroy/confidential_issue_service_spec.rb @@ -29,12 +29,8 @@ describe Todos::Destroy::ConfidentialIssueService do issue.update!(confidential: true) end - it 'removes issue todos for a user who is not a project member' do + it 'removes issue todos for users who can not access the confidential issue' do expect { subject }.to change { Todo.count }.from(6).to(4) - - expect(user.todos).to match_array([todo_another_non_member]) - expect(author.todos).to match_array([todo_issue_author]) - expect(project_member.todos).to match_array([todo_issue_member]) end end diff --git a/spec/services/todos/destroy/entity_leave_service_spec.rb b/spec/services/todos/destroy/entity_leave_service_spec.rb index c554db5e024..8cb91e7c1b9 100644 --- a/spec/services/todos/destroy/entity_leave_service_spec.rb +++ b/spec/services/todos/destroy/entity_leave_service_spec.rb @@ -5,7 +5,7 @@ describe Todos::Destroy::EntityLeaveService do let(:project) { create(:project, group: group) } let(:user) { create(:user) } let(:user2) { create(:user) } - let(:issue) { create(:issue, project: project) } + let(:issue) { create(:issue, project: project, confidential: true) } let(:mr) { create(:merge_request, source_project: project) } let!(:todo_mr_user) { create(:todo, user: user, target: mr, project: project) } @@ -25,17 +25,20 @@ describe Todos::Destroy::EntityLeaveService do expect(user.todos).to match_array([todo_group_user]) expect(user2.todos).to match_array([todo_issue_user2, todo_group_user2]) end - end - context 'when project is not private' do - before do - group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) - project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + context 'when the user is member of the project' do + before do + project.add_developer(user) + end + + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end end - context 'when a user is not an author of confidential issue' do + context 'when the user is a project guest' do before do - issue.update!(confidential: true) + project.add_guest(user) end it 'removes only confidential issues todos' do @@ -43,24 +46,79 @@ describe Todos::Destroy::EntityLeaveService do end end - context 'when a user is an author of confidential issue' do + context 'when the user is member of a parent group' do before do - issue.update!(author: user, confidential: true) + group.add_developer(user) end - it 'removes only confidential issues todos' do + it 'does not remove any todos' do expect { subject }.not_to change { Todo.count } end end - context 'when a user is an assignee of confidential issue' do + context 'when the user is guest of a parent group' do before do - issue.update!(confidential: true) - issue.assignees << user + project.add_guest(user) end it 'removes only confidential issues todos' do - expect { subject }.not_to change { Todo.count } + expect { subject }.to change { Todo.count }.from(5).to(4) + end + end + end + + context 'when project is not private' do + before do + group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + end + + context 'confidential issues' do + context 'when a user is not an author of confidential issue' do + it 'removes only confidential issues todos' do + expect { subject }.to change { Todo.count }.from(5).to(4) + end + end + + context 'when a user is an author of confidential issue' do + before do + issue.update!(author: user) + end + + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when a user is an assignee of confidential issue' do + before do + issue.assignees << user + end + + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when a user is a project guest' do + before do + project.add_guest(user) + end + + it 'removes only confidential issues todos' do + expect { subject }.to change { Todo.count }.from(5).to(4) + end + end + + context 'when a user is a project guest but group developer' do + before do + project.add_guest(user) + group.add_developer(user) + end + + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end end end @@ -89,37 +147,128 @@ describe Todos::Destroy::EntityLeaveService do expect(user2.todos).to match_array([todo_issue_user2, todo_group_user2]) end + context 'when the user is member of the group' do + before do + group.add_developer(user) + end + + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when the user is member of the group project but not the group' do + before do + project.add_developer(user) + end + + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + context 'with nested groups', :nested_groups do let(:subgroup) { create(:group, :private, parent: group) } + let(:subgroup2) { create(:group, :private, parent: group) } let(:subproject) { create(:project, group: subgroup) } + let(:subproject2) { create(:project, group: subgroup2) } let!(:todo_subproject_user) { create(:todo, user: user, project: subproject) } + let!(:todo_subproject2_user) { create(:todo, user: user, project: subproject2) } let!(:todo_subgroup_user) { create(:todo, user: user, group: subgroup) } + let!(:todo_subgroup2_user) { create(:todo, user: user, group: subgroup2) } let!(:todo_subproject_user2) { create(:todo, user: user2, project: subproject) } let!(:todo_subpgroup_user2) { create(:todo, user: user2, group: subgroup) } - it 'removes todos for the user including subprojects todos' do - expect { subject }.to change { Todo.count }.from(9).to(4) + context 'when the user is not a member of any groups/projects' do + it 'removes todos for the user including subprojects todos' do + expect { subject }.to change { Todo.count }.from(11).to(4) + + expect(user.todos).to be_empty + expect(user2.todos) + .to match_array( + [todo_issue_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2] + ) + end + end + + context 'when the user is member of a parent group' do + before do + parent_group = create(:group) + group.update!(parent: parent_group) + parent_group.add_developer(user) + end + + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when the user is member of a subgroup' do + before do + subgroup.add_developer(user) + end + + it 'does not remove group and subproject todos' do + expect { subject }.to change { Todo.count }.from(11).to(7) + + expect(user.todos).to match_array([todo_group_user, todo_subgroup_user, todo_subproject_user]) + expect(user2.todos) + .to match_array( + [todo_issue_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2] + ) + end + end - expect(user.todos).to be_empty - expect(user2.todos) - .to match_array( - [todo_issue_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2] - ) + context 'when the user is member of a child project' do + before do + subproject.add_developer(user) + end + + it 'does not remove subproject and group todos' do + expect { subject }.to change { Todo.count }.from(11).to(7) + + expect(user.todos).to match_array([todo_subgroup_user, todo_group_user, todo_subproject_user]) + expect(user2.todos) + .to match_array( + [todo_issue_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2] + ) + end end end end context 'when group is not private' do before do - issue.update!(confidential: true) - group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) end - it 'removes only confidential issues todos' do - expect { subject }.to change { Todo.count }.from(5).to(4) + context 'when user is not member' do + it 'removes only confidential issues todos' do + expect { subject }.to change { Todo.count }.from(5).to(4) + end + end + + context 'when user is a project guest' do + before do + project.add_guest(user) + end + + it 'removes only confidential issues todos' do + expect { subject }.to change { Todo.count }.from(5).to(4) + end + end + + context 'when user is a project guest & group developer' do + before do + project.add_guest(user) + group.add_developer(user) + end + + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end end end end diff --git a/spec/services/todos/destroy/group_private_service_spec.rb b/spec/services/todos/destroy/group_private_service_spec.rb index 48e5a5a91eb..c4ee6ebed50 100644 --- a/spec/services/todos/destroy/group_private_service_spec.rb +++ b/spec/services/todos/destroy/group_private_service_spec.rb @@ -1,31 +1,62 @@ require 'spec_helper' describe Todos::Destroy::GroupPrivateService do - let(:group) { create(:group, :public) } - let(:user) { create(:user) } - let(:group_member) { create(:user) } + let(:group) { create(:group, :public) } + let(:project) { create(:project, group: group) } + let(:user) { create(:user) } + let(:group_member) { create(:user) } + let(:project_member) { create(:user) } - let!(:todo_non_member) { create(:todo, user: user, group: group) } - let!(:todo_member) { create(:todo, user: group_member, group: group) } + let!(:todo_non_member) { create(:todo, user: user, group: group) } let!(:todo_another_non_member) { create(:todo, user: user, group: group) } + let!(:todo_group_member) { create(:todo, user: group_member, group: group) } + let!(:todo_project_member) { create(:todo, user: project_member, group: group) } describe '#execute' do before do group.add_developer(group_member) + project.add_developer(project_member) end subject { described_class.new(group.id).execute } - context 'when a project set to private' do + context 'when a group set to private' do before do group.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) end - it 'removes todos for a user who is not a member' do - expect { subject }.to change { Todo.count }.from(3).to(1) + it 'removes todos only for users who are not group users' do + expect { subject }.to change { Todo.count }.from(4).to(2) expect(user.todos).to be_empty - expect(group_member.todos).to match_array([todo_member]) + expect(group_member.todos).to match_array([todo_group_member]) + expect(project_member.todos).to match_array([todo_project_member]) + end + + context 'with nested groups' do + let(:parent_group) { create(:group) } + let(:subgroup) { create(:group, :private, parent: group) } + let(:subproject) { create(:project, group: subgroup) } + + let(:parent_member) { create(:user) } + let(:subgroup_member) { create(:user) } + let(:subgproject_member) { create(:user) } + + let!(:todo_parent_member) { create(:todo, user: parent_member, group: group) } + let!(:todo_subgroup_member) { create(:todo, user: subgroup_member, group: group) } + let!(:todo_subproject_member) { create(:todo, user: subgproject_member, group: group) } + + before do + group.update!(parent: parent_group) + + parent_group.add_developer(parent_member) + subgroup.add_developer(subgroup_member) + subproject.add_developer(subgproject_member) + end + + it 'removes todos only for users who are not group users' do + expect { subject }.to change { Todo.count }.from(7).to(5) + end end end diff --git a/spec/services/todos/destroy/project_private_service_spec.rb b/spec/services/todos/destroy/project_private_service_spec.rb index badf3f913a5..9ebc0231795 100644 --- a/spec/services/todos/destroy/project_private_service_spec.rb +++ b/spec/services/todos/destroy/project_private_service_spec.rb @@ -1,17 +1,22 @@ require 'spec_helper' describe Todos::Destroy::ProjectPrivateService do - let(:project) { create(:project, :public) } + let(:group) { create(:group, :public) } + let(:project) { create(:project, :public, group: group) } let(:user) { create(:user) } let(:project_member) { create(:user) } + let(:group_member) { create(:user) } - let!(:todo_issue_non_member) { create(:todo, user: user, project: project) } - let!(:todo_issue_member) { create(:todo, user: project_member, project: project) } - let!(:todo_another_non_member) { create(:todo, user: user, project: project) } + let!(:todo_non_member) { create(:todo, user: user, project: project) } + let!(:todo2_non_member) { create(:todo, user: user, project: project) } + let!(:todo_member) { create(:todo, user: project_member, project: project) } + let!(:todo_member) { create(:todo, user: project_member, project: project) } + let!(:todo_group_member) { create(:todo, user: group_member, project: project) } describe '#execute' do before do project.add_developer(project_member) + group.add_developer(group_member) end subject { described_class.new(project.id).execute } @@ -22,10 +27,11 @@ describe Todos::Destroy::ProjectPrivateService do end it 'removes issue todos for a user who is not a member' do - expect { subject }.to change { Todo.count }.from(3).to(1) + expect { subject }.to change { Todo.count }.from(4).to(2) expect(user.todos).to be_empty - expect(project_member.todos).to match_array([todo_issue_member]) + expect(project_member.todos).to match_array([todo_member]) + expect(group_member.todos).to match_array([todo_group_member]) end end |