From 37e7a1fba351d2813bb43625ce97146ade4b059d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Thu, 28 May 2026 14:37:51 +0200 Subject: [PATCH] Fix access scoping of calendar and team planner for deletions https://community.openproject.org/work_packages/75429 --- .../calendar/calendars_controller.rb | 7 ++- .../calendar/spec/requests/calendars_spec.rb | 59 +++++++++++++++++++ .../team_planner/team_planner_controller.rb | 7 ++- .../spec/requests/team_planner_spec.rb | 25 ++++++-- 4 files changed, 87 insertions(+), 11 deletions(-) create mode 100644 modules/calendar/spec/requests/calendars_spec.rb diff --git a/modules/calendar/app/controllers/calendar/calendars_controller.rb b/modules/calendar/app/controllers/calendar/calendars_controller.rb index fe4ce20e867..4604406b898 100644 --- a/modules/calendar/app/controllers/calendar/calendars_controller.rb +++ b/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 diff --git a/modules/calendar/spec/requests/calendars_spec.rb b/modules/calendar/spec/requests/calendars_spec.rb new file mode 100644 index 00000000000..37c04a37568 --- /dev/null +++ b/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 diff --git a/modules/team_planner/app/controllers/team_planner/team_planner_controller.rb b/modules/team_planner/app/controllers/team_planner/team_planner_controller.rb index a0daffd481b..e8a1d508ace 100644 --- a/modules/team_planner/app/controllers/team_planner/team_planner_controller.rb +++ b/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) diff --git a/modules/team_planner/spec/requests/team_planner_spec.rb b/modules/team_planner/spec/requests/team_planner_spec.rb index d8cb015ed20..53a757d2b5c 100644 --- a/modules/team_planner/spec/requests/team_planner_spec.rb +++ b/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