Fix removing todos for users without access

3 files changed, 67 insertions, 33 deletions

diff --git a/app/services/todos/destroy/entity_leave_service.rb b/app/services/todos/destroy/entity_leave_service.rb
index b1c4eb95e87..ef6add352e7 100644
--- a/app/services/todos/destroy/entity_leave_service.rb
+++ b/app/services/todos/destroy/entity_leave_service.rb
@@ -2,65 +2,99 @@ module Todos
   module Destroy
     class EntityLeaveService < ::Todos::Destroy::BaseService
       extend ::Gitlab::Utils::Override
+      include Gitlab::Utils::StrongMemoize
 
-      attr_reader :user_id, :entity
+      attr_reader :user, :entity
 
       def initialize(user_id, entity_id, entity_type)
         unless %w(Group Project).include?(entity_type)
           raise ArgumentError.new("#{entity_type} is not an entity user can leave")
         end
 
-        @user_id = user_id
+        @user = User.find_by(id: user_id)
         @entity = entity_type.constantize.find_by(id: entity_id)
       end
 
-      private
+      def execute
+        return unless entity && user
+
+        # if at least reporter, all entities including confidential issues can be accessed
+        return if main_group_reporter?
+
+        remove_confidential_issue_todos
 
-      override :todos
-      def todos
         if entity.private?
-          Todo.where('(project_id IN (?) OR group_id IN (?)) AND user_id = ?', project_ids, group_ids, user_id)
+          remove_project_todos
+          remove_group_todos
         else
-          project_ids.each do |project_id|
-            TodosDestroyer::PrivateFeaturesWorker.perform_async(project_id, user_id)
-          end
+          enqueue_private_features_worker
+        end
+      end
 
-          Todo.where(
-            target_id: confidential_issues.select(:id), target_type: Issue, user_id: user_id
-          )
+      private
+
+      def enqueue_private_features_worker
+        project_ids.each do |project_id|
+          TodosDestroyer::PrivateFeaturesWorker.perform_async(project_id, user.id)
         end
       end
 
+      def remove_confidential_issue_todos
+        Todo.where(
+          target_id: confidential_issues.select(:id), target_type: Issue, user_id: user.id
+        ).delete_all
+      end
+
+      def remove_project_todos
+        Todo.where(project_id: non_authorized_projects, user_id: user.id).delete_all
+      end
+
+      def remove_group_todos
+        Todo.where(group_id: non_authorized_groups, user_id: user.id).delete_all
+      end
+
       override :project_ids
       def project_ids
-        case entity
-        when Project
-          [entity.id]
-        when Namespace
-          Project.select(:id).where(namespace_id: group_ids)
-        end
+        condition = case entity
+                    when Project
+                      { id: entity.id }
+                    when Namespace
+                      { namespace_id: non_member_groups }
+                    end
+
+        Project.where(condition).select(:id)
       end
 
-      def group_ids
-        case entity
-        when Project
-          []
-        when Namespace
-          entity.self_and_descendants.select(:id)
-        end
+      def non_authorized_projects
+        project_ids.where('id NOT IN (?)', user.authorized_projects.select(:id))
       end
 
-      override :todos_to_remove?
-      def todos_to_remove?
-        # if an entity is provided we want to check always at least private features
-        !!entity
+      def non_authorized_groups
+        return [] unless entity.is_a?(Namespace)
+
+        entity.self_and_descendants.select(:id)
+          .where('id NOT IN (?)', GroupsFinder.new(user).execute.select(:id))
+      end
+
+      def non_member_groups
+        entity.self_and_descendants.select(:id)
+          .where('id NOT IN (?)', user.membership_groups.select(:id))
+      end
+
+      def main_group_reporter?
+        return unless entity.is_a?(Namespace)
+
+        entity.member?(User.find(user.id), Gitlab::Access::REPORTER)
       end
 
       def confidential_issues
-        assigned_ids = IssueAssignee.select(:issue_id).where(user_id: user_id)
+        assigned_ids = IssueAssignee.select(:issue_id).where(user_id: user.id)
+        authorized_reporter_projects = user
+          .authorized_projects(Gitlab::Access::REPORTER).select(:id)
 
         Issue.where(project_id: project_ids, confidential: true)
-          .where('author_id != ?', user_id)
+          .where('project_id NOT IN(?)', authorized_reporter_projects)
+          .where('author_id != ?', user.id)
           .where('id NOT IN (?)', assigned_ids)
       end
     end
diff --git a/app/services/todos/destroy/group_private_service.rb b/app/services/todos/destroy/group_private_service.rb
index 1b00bbe91e1..d13fa7a6516 100644
--- a/app/services/todos/destroy/group_private_service.rb
+++ b/app/services/todos/destroy/group_private_service.rb
@@ -18,7 +18,7 @@ module Todos
 
       override :authorized_users
       def authorized_users
-        GroupMember.select(:user_id).where(source: group.id)
+        group.direct_and_indirect_users.select(:id)
       end
 
       override :todos_to_remove?
diff --git a/app/services/todos/destroy/project_private_service.rb b/app/services/todos/destroy/project_private_service.rb
index 171933e7cbc..315a0c33398 100644
--- a/app/services/todos/destroy/project_private_service.rb
+++ b/app/services/todos/destroy/project_private_service.rb
@@ -13,7 +13,7 @@ module Todos
 
       override :todos
       def todos
-        Todo.where(project_id: project_ids)
+        Todo.where(project_id: project.id)
       end
 
       override :project_ids