Fix access scoping of calendar and team planner for deletions

https://community.openproject.org/work_packages/75429

modules/calendar/app/controllers/calendar/calendars_controller.rb

@@ -159,9 +159,10 @@ module ::Calendar
       # In that case @view remains nil and split_view_base_route handles it.
       return if params[:id].blank?
 
-      @view = Query
-                .visible(current_user)
-                .find(params[:id])
+      query_scope = Query.visible(current_user)
+      query_scope = query_scope.where(project_id: @project.id) if @project
+
+      @view = query_scope.find(params[:id])
     end
   end
 end

modules/calendar/spec/requests/calendars_spec.rb

@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+require "spec_helper"
+
+RSpec.describe "Calendars", :skip_csrf, type: :rails_request do
+  let(:project_a) { create(:project) }
+  let(:project_b) { create(:project) }
+  let(:attacker) do
+    create(:user,
+           member_with_permissions: {
+             project_a => %i[view_work_packages view_calendar manage_calendars],
+             project_b => %i[view_work_packages view_calendar]
+           })
+  end
+  let(:other_user) { create(:user) }
+  let!(:target_query) { create(:query, project: project_b, user: other_user, public: true) }
+
+  before do
+    create(:view_work_packages_calendar, query: target_query)
+    login_as(attacker)
+  end
+
+  describe "DELETE /projects/:project_id/calendars/:id" do
+    it "does not delete calendars from another project" do
+      delete project_calendar_path(project_a, target_query)
+
+      expect(response).to have_http_status(:not_found)
+      expect(Query.exists?(target_query.id)).to be(true)
+    end
+  end
+end

modules/team_planner/app/controllers/team_planner/team_planner_controller.rb

@@ -118,9 +118,10 @@ module ::TeamPlanner
       # In that case @view remains nil and split_view_base_route handles it.
       return if params[:id].blank?
 
-      @view = Query
-        .visible(current_user)
-        .find(params[:id])
+      query_scope = Query.visible(current_user)
+      query_scope = query_scope.where(project_id: @project.id) if @project
+
+      @view = query_scope.find(params[:id])
     end
 
     def visible_plans(project = nil)

modules/team_planner/spec/requests/team_planner_spec.rb

@@ -31,22 +31,37 @@
 require "spec_helper"
 
 RSpec.describe "Team planners", :skip_csrf, type: :rails_request, with_ee: %i[team_planner_view] do
-  let(:project) { create(:project) }
+  let(:project_a) { create(:project) }
+  let(:project_b) { create(:project) }
   let(:user) do
-    create(:user, member_with_permissions: { project => %i[view_work_packages view_team_planner manage_team_planner] })
+    create(:user,
+           member_with_permissions: {
+             project_a => %i[view_work_packages view_team_planner manage_team_planner],
+             project_b => %i[view_work_packages view_team_planner]
+           })
   end
-  let(:query) { create(:query, project:, user:) }
+  let(:query) { create(:query, project: project_a, user:) }
+  let(:other_user) { create(:user) }
+  let!(:other_project_query) { create(:query, project: project_b, user: other_user, public: true) }
 
   before do
     create(:view_team_planner, query:)
+    create(:view_team_planner, query: other_project_query)
     login_as(user)
   end
 
   describe "DELETE /projects/:project_id/team_planners/:id" do
     it "redirects with 303 See Other" do
-      delete project_team_planner_path(project, query)
+      delete project_team_planner_path(project_a, query)
       expect(response).to have_http_status(:see_other)
-      expect(response).to redirect_to(project_team_planners_path(project))
+      expect(response).to redirect_to(project_team_planners_path(project_a))
+    end
+
+    it "does not delete team planners from another project" do
+      delete project_team_planner_path(project_a, other_project_query)
+
+      expect(response).to have_http_status(:not_found)
+      expect(Query.exists?(other_project_query.id)).to be(true)
     end
   end
 end