From 7a1065a8516877ba217d172604cd304fd5688563 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 14 Jan 2025 19:04:23 +0300 Subject: [PATCH] [#60568] Remove permission restriction for users to set their own reminders https://community.openproject.org/work_packages/60568 --- app/contracts/reminders/base_contract.rb | 6 +++++- config/initializers/permissions.rb | 14 ++++---------- lib/api/v3/reminders/reminders_api.rb | 2 +- .../contracts/reminders/base_contract_spec.rb | 14 +++++++++++++- spec/features/work_packages/reminders_spec.rb | 19 +++++++++++++++++-- .../api/v3/reminders/reminders_api_spec.rb | 6 +++--- 6 files changed, 43 insertions(+), 18 deletions(-) diff --git a/app/contracts/reminders/base_contract.rb b/app/contracts/reminders/base_contract.rb index 545d72d59f82..6f2767ccf5ca 100644 --- a/app/contracts/reminders/base_contract.rb +++ b/app/contracts/reminders/base_contract.rb @@ -79,9 +79,13 @@ def validate_note_length def validate_manage_reminders_permissions return if errors.added?(:remindable, :not_found) - unless user.allowed_in_project?(:manage_own_reminders, model.remindable.project) + unless can_manage_reminders? errors.add :base, :error_unauthorized end end + + def can_manage_reminders? + user.logged? && user.allowed_in_project?(:view_work_packages, model.remindable.project) + end end end diff --git a/config/initializers/permissions.rb b/config/initializers/permissions.rb index 50555c7d6124..33d7e12cd9b8 100644 --- a/config/initializers/permissions.rb +++ b/config/initializers/permissions.rb @@ -232,14 +232,6 @@ {}, permissible_on: :project_query, require: :loggedin - - map.permission :manage_own_reminders, - { - "work_packages/reminders": %i[modal_body create update destroy] - }, - permissible_on: :project, - contract_actions: { work_package_reminders: %i[modal_body] }, - require: :member end map.project_module :work_package_tracking, order: 90 do |wpt| @@ -253,10 +245,12 @@ "work_packages/activities_tab": %i[index update_streams update_sorting update_filter], "work_packages/menus": %i[show], "work_packages/hover_card": %i[show], - work_package_relations_tab: %i[index] + work_package_relations_tab: %i[index], + "work_packages/reminders": %i[modal_body create update destroy] }, permissible_on: %i[work_package project], - contract_actions: { work_packages: %i[read] } + contract_actions: { work_packages: %i[read], + work_package_reminders: %i[modal_body] } wpt.permission :add_work_packages, { diff --git a/lib/api/v3/reminders/reminders_api.rb b/lib/api/v3/reminders/reminders_api.rb index 9efc09c5edff..359445d93a3f 100644 --- a/lib/api/v3/reminders/reminders_api.rb +++ b/lib/api/v3/reminders/reminders_api.rb @@ -32,7 +32,7 @@ module Reminders class RemindersAPI < ::API::OpenProjectAPI resource :reminders do after_validation do - authorize_in_project(:manage_own_reminders, project: @work_package.project) + authorize_in_project(:view_work_packages, project: @work_package.project) end get do diff --git a/spec/contracts/reminders/base_contract_spec.rb b/spec/contracts/reminders/base_contract_spec.rb index b8e57fe79641..0b5ea8dc87b3 100644 --- a/spec/contracts/reminders/base_contract_spec.rb +++ b/spec/contracts/reminders/base_contract_spec.rb @@ -54,7 +54,7 @@ before do mock_permissions_for(user) do |mock| - mock.allow_in_project(:manage_own_reminders, project: reminder.remindable.project) + mock.allow_in_project(:view_work_packages, project: reminder.remindable.project) end end @@ -68,6 +68,18 @@ end end + describe "anonymous user" do + let(:user) { build_stubbed(:anonymous) } + + before do + mock_permissions_for(user) do |mock| + mock.allow_in_project(:view_work_packages, project: reminder.remindable.project) + end + end + + it_behaves_like "contract is invalid", base: :error_unauthorized + end + describe "validate creator exists" do context "when creator does not exist" do before { allow(User).to receive(:exists?).with(user.id).and_return(false) } diff --git a/spec/features/work_packages/reminders_spec.rb b/spec/features/work_packages/reminders_spec.rb index 07a704165f44..395541656427 100644 --- a/spec/features/work_packages/reminders_spec.rb +++ b/spec/features/work_packages/reminders_spec.rb @@ -36,10 +36,10 @@ let!(:project) { create(:project) } let!(:work_package) { create(:work_package, project:) } let!(:role_that_allows_managing_own_reminders) do - create(:project_role, permissions: %i[view_work_packages manage_own_reminders]) + create(:project_role, permissions: %i[view_work_packages]) end let!(:role_that_does_not_allow_managing_own_reminders) do - create(:project_role, permissions: %i[view_work_packages]) + create(:project_role, permissions: %i[view_project]) end let!(:user_with_permissions) do @@ -382,4 +382,19 @@ work_package_page.expect_no_reminder_button end end + + context "with anonymous user with role that can view work packages" do + let!(:anonymous_user) do + create(:anonymous).tap do + ProjectRole.anonymous.add_permission! :view_work_packages + end + end + + current_user { anonymous_user } + + it "does not render the reminder button when visiting the work package page" do + work_package_page.visit! + work_package_page.expect_no_reminder_button + end + end end diff --git a/spec/requests/api/v3/reminders/reminders_api_spec.rb b/spec/requests/api/v3/reminders/reminders_api_spec.rb index 6160bde64a10..9367d6c52ae4 100644 --- a/spec/requests/api/v3/reminders/reminders_api_spec.rb +++ b/spec/requests/api/v3/reminders/reminders_api_spec.rb @@ -31,8 +31,8 @@ RSpec.describe API::V3::Reminders::RemindersAPI do let!(:project) { create(:project) } - let!(:role_with_permissions) { create(:project_role, permissions: %i[view_work_packages manage_own_reminders]) } - let!(:role_without_permissions) { create(:project_role, permissions: %i[view_work_packages]) } + let!(:role_with_permissions) { create(:project_role, permissions: %i[view_work_packages]) } + let!(:role_without_permissions) { create(:project_role, permissions: %i[view_project]) } let!(:user_with_permissions) do create(:user, member_with_roles: { project => role_with_permissions }) @@ -110,7 +110,7 @@ def reminders current_user { other_user_without_permissions } it "responds with unprocessable entity" do - expect(result["errorIdentifier"]).to eq("urn:openproject-org:api:v3:errors:MissingPermission") + expect(result["errorIdentifier"]).to eq("urn:openproject-org:api:v3:errors:NotFound") end end