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

6 files changed, 77 insertions, 3 deletions

diff --git a/app/policies/issuable_policy.rb b/app/policies/issuable_policy.rb
index 6d8b575102e..ecb2797d1d9 100644
--- a/app/policies/issuable_policy.rb
+++ b/app/policies/issuable_policy.rb
@@ -11,7 +11,7 @@ class IssuablePolicy < BasePolicy
     @user && @subject.assignee_or_author?(@user)
   end
 
-  rule { assignee_or_author }.policy do
+  rule { can?(:guest_access) & assignee_or_author }.policy do
     enable :read_issue
     enable :update_issue
     enable :reopen_issue
diff --git a/changelogs/unreleased/security-53543-user-keeps-access-to-mr-issue-when-removed-from-team.yml b/changelogs/unreleased/security-53543-user-keeps-access-to-mr-issue-when-removed-from-team.yml
new file mode 100644
index 00000000000..ab12ba539c1
--- /dev/null
+++ b/changelogs/unreleased/security-53543-user-keeps-access-to-mr-issue-when-removed-from-team.yml
@@ -0,0 +1,5 @@
+---
+title: Issuable no longer is visible to users when project can't be viewed
+merge_request:
+author:
+type: security
diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb
index 81748681528..a64720f1876 100644
--- a/spec/models/event_spec.rb
+++ b/spec/models/event_spec.rb
@@ -243,6 +243,20 @@ describe Event do
           expect(event.visible_to_user?(admin)).to eq true
         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
 
     context 'merge request diff note event' do
@@ -265,8 +279,8 @@ describe Event do
 
         it do
           expect(event.visible_to_user?(non_member)).to eq false
-          expect(event.visible_to_user?(author)).to eq true
-          expect(event.visible_to_user?(assignee)).to eq true
+          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 false
           expect(event.visible_to_user?(admin)).to eq true
diff --git a/spec/policies/issuable_policy_spec.rb b/spec/policies/issuable_policy_spec.rb
index d1bf98995e7..db3df760472 100644
--- a/spec/policies/issuable_policy_spec.rb
+++ b/spec/policies/issuable_policy_spec.rb
@@ -7,6 +7,33 @@ describe IssuablePolicy, models: true do
   let(:policies) { described_class.new(user, issue) }
 
   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
       let(:issue) { create(:issue, project: project, discussion_locked: true) }
 
diff --git a/spec/services/issuable/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb
index 53c85f73cde..c53627d262b 100644
--- a/spec/services/issuable/bulk_update_service_spec.rb
+++ b/spec/services/issuable/bulk_update_service_spec.rb
@@ -28,6 +28,33 @@ describe Issuable::BulkUpdateService do
       expect(project.issues.opened).to be_empty
       expect(project.issues.closed).not_to be_empty
     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
 
   describe 'reopen issues' do
diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb
index 1746721b0d0..c5b844a8058 100644
--- a/spec/services/todo_service_spec.rb
+++ b/spec/services/todo_service_spec.rb
@@ -19,6 +19,7 @@ describe TodoService do
   before do
     project.add_guest(guest)
     project.add_developer(author)
+    project.add_developer(assignee)
     project.add_developer(member)
     project.add_developer(john_doe)
     project.add_developer(skipped)