Adds validation to check if user can read project
An issuable should not be available to a user if the project is not visible to that specific user
This commit is contained in:
parent
ffef28ccd6
commit
52feca595a
|
@ -11,7 +11,7 @@ class IssuablePolicy < BasePolicy
|
||||||
@user && @subject.assignee_or_author?(@user)
|
@user && @subject.assignee_or_author?(@user)
|
||||||
end
|
end
|
||||||
|
|
||||||
rule { assignee_or_author }.policy do
|
rule { can?(:guest_access) & assignee_or_author }.policy do
|
||||||
enable :read_issue
|
enable :read_issue
|
||||||
enable :update_issue
|
enable :update_issue
|
||||||
enable :reopen_issue
|
enable :reopen_issue
|
||||||
|
|
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Issuable no longer is visible to users when project can't be viewed
|
||||||
|
merge_request:
|
||||||
|
author:
|
||||||
|
type: security
|
|
@ -243,6 +243,20 @@ describe Event do
|
||||||
expect(event.visible_to_user?(admin)).to eq true
|
expect(event.visible_to_user?(admin)).to eq true
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'private project' do
|
||||||
|
let(:project) { create(:project, :private) }
|
||||||
|
let(:target) { note_on_issue }
|
||||||
|
|
||||||
|
it do
|
||||||
|
expect(event.visible_to_user?(non_member)).to eq false
|
||||||
|
expect(event.visible_to_user?(author)).to eq false
|
||||||
|
expect(event.visible_to_user?(assignee)).to eq false
|
||||||
|
expect(event.visible_to_user?(member)).to eq true
|
||||||
|
expect(event.visible_to_user?(guest)).to eq true
|
||||||
|
expect(event.visible_to_user?(admin)).to eq true
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'merge request diff note event' do
|
context 'merge request diff note event' do
|
||||||
|
@ -265,8 +279,8 @@ describe Event do
|
||||||
|
|
||||||
it do
|
it do
|
||||||
expect(event.visible_to_user?(non_member)).to eq false
|
expect(event.visible_to_user?(non_member)).to eq false
|
||||||
expect(event.visible_to_user?(author)).to eq true
|
expect(event.visible_to_user?(author)).to eq false
|
||||||
expect(event.visible_to_user?(assignee)).to eq true
|
expect(event.visible_to_user?(assignee)).to eq false
|
||||||
expect(event.visible_to_user?(member)).to eq true
|
expect(event.visible_to_user?(member)).to eq true
|
||||||
expect(event.visible_to_user?(guest)).to eq false
|
expect(event.visible_to_user?(guest)).to eq false
|
||||||
expect(event.visible_to_user?(admin)).to eq true
|
expect(event.visible_to_user?(admin)).to eq true
|
||||||
|
|
|
@ -7,6 +7,33 @@ describe IssuablePolicy, models: true do
|
||||||
let(:policies) { described_class.new(user, issue) }
|
let(:policies) { described_class.new(user, issue) }
|
||||||
|
|
||||||
describe '#rules' do
|
describe '#rules' do
|
||||||
|
context 'when user is author of issuable' do
|
||||||
|
let(:merge_request) { create(:merge_request, source_project: project, author: user) }
|
||||||
|
let(:policies) { described_class.new(user, merge_request) }
|
||||||
|
|
||||||
|
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)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when project is private' do
|
||||||
|
let(:project) { create(:project, :private) }
|
||||||
|
|
||||||
|
context 'when user belongs to the projects team' do
|
||||||
|
it 'enables user to read and update issuables' do
|
||||||
|
project.add_maintainer(user)
|
||||||
|
|
||||||
|
expect(policies).to be_allowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'disallows user from reading and updating issuables from that project' do
|
||||||
|
expect(policies).to be_disallowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
context 'when discussion is locked for the issuable' do
|
context 'when discussion is locked for the issuable' do
|
||||||
let(:issue) { create(:issue, project: project, discussion_locked: true) }
|
let(:issue) { create(:issue, project: project, discussion_locked: true) }
|
||||||
|
|
||||||
|
|
|
@ -28,6 +28,33 @@ describe Issuable::BulkUpdateService do
|
||||||
expect(project.issues.opened).to be_empty
|
expect(project.issues.opened).to be_empty
|
||||||
expect(project.issues.closed).not_to be_empty
|
expect(project.issues.closed).not_to be_empty
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'when issue for a different project is created' do
|
||||||
|
let(:private_project) { create(:project, :private) }
|
||||||
|
let(:issue) { create(:issue, project: private_project, author: user) }
|
||||||
|
|
||||||
|
context 'when user has access to the project' do
|
||||||
|
it 'closes all issues passed' do
|
||||||
|
private_project.add_maintainer(user)
|
||||||
|
|
||||||
|
bulk_update(issues + [issue], state_event: 'close')
|
||||||
|
|
||||||
|
expect(project.issues.opened).to be_empty
|
||||||
|
expect(project.issues.closed).not_to be_empty
|
||||||
|
expect(private_project.issues.closed).not_to be_empty
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when user does not have access to project' do
|
||||||
|
it 'only closes all issues that the user has access to' do
|
||||||
|
bulk_update(issues + [issue], state_event: 'close')
|
||||||
|
|
||||||
|
expect(project.issues.opened).to be_empty
|
||||||
|
expect(project.issues.closed).not_to be_empty
|
||||||
|
expect(private_project.issues.closed).to be_empty
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe 'reopen issues' do
|
describe 'reopen issues' do
|
||||||
|
|
|
@ -19,6 +19,7 @@ describe TodoService do
|
||||||
before do
|
before do
|
||||||
project.add_guest(guest)
|
project.add_guest(guest)
|
||||||
project.add_developer(author)
|
project.add_developer(author)
|
||||||
|
project.add_developer(assignee)
|
||||||
project.add_developer(member)
|
project.add_developer(member)
|
||||||
project.add_developer(john_doe)
|
project.add_developer(john_doe)
|
||||||
project.add_developer(skipped)
|
project.add_developer(skipped)
|
||||||
|
|
Loading…
Reference in New Issue