diff --git a/app/contracts/concerns/admin_writable_timestamps.rb b/app/contracts/concerns/admin_writable_timestamps.rb new file mode 100644 index 000000000000..8a83ee22eb85 --- /dev/null +++ b/app/contracts/concerns/admin_writable_timestamps.rb @@ -0,0 +1,47 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2024 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. +#++ + +module AdminWritableTimestamps + extend ActiveSupport::Concern + + class_methods do + def allow_writable_timestamps(attributes = %i[created_at updated_at]) + Array(attributes).each do |attr| + attribute attr, + writable: -> { default_attributes_admin_writable? } + end + end + end + + private + + # Adds an error if user is archived or not an admin. + def default_attributes_admin_writable? + user.admin? && Setting.apiv3_write_readonly_attributes? + end +end diff --git a/app/contracts/projects/create_contract.rb b/app/contracts/projects/create_contract.rb index 2db60762324c..78b38a775c73 100644 --- a/app/contracts/projects/create_contract.rb +++ b/app/contracts/projects/create_contract.rb @@ -28,6 +28,11 @@ module Projects class CreateContract < BaseContract + include AdminWritableTimestamps + # Projects update their updated_at timestamp due to awesome_nested_set + # so allowing writing here would be useless. + allow_writable_timestamps :created_at + private def validate_user_allowed_to_manage diff --git a/app/contracts/work_packages/create_contract.rb b/app/contracts/work_packages/create_contract.rb index a8090b4ff267..22bf6421b02d 100644 --- a/app/contracts/work_packages/create_contract.rb +++ b/app/contracts/work_packages/create_contract.rb @@ -30,10 +30,12 @@ module WorkPackages class CreateContract < BaseContract + include AdminWritableTimestamps + allow_writable_timestamps + attribute :author_id, - writable: false do - errors.add :author_id, :invalid if model.author != user - end + writable: -> { default_attributes_admin_writable? } + attribute :status_id, # Overriding permission from WP base contract to ignore change_work_package_status for creation, # because we don't require that permission for writable status during WP creation. diff --git a/app/models/journal.rb b/app/models/journal.rb index c41a6d787f97..5760be8a3059 100644 --- a/app/models/journal.rb +++ b/app/models/journal.rb @@ -63,6 +63,7 @@ class Journal < ApplicationRecord work_package_children_changed_times work_package_related_changed_times working_days_changed + default_attribute_written system_update ].freeze diff --git a/app/services/work_packages/set_attributes_service.rb b/app/services/work_packages/set_attributes_service.rb index aef4758227ba..15c21a1d5dfb 100644 --- a/app/services/work_packages/set_attributes_service.rb +++ b/app/services/work_packages/set_attributes_service.rb @@ -66,6 +66,7 @@ def set_calculated_attributes(attributes) update_project_dependent_attributes reassign_invalid_status_if_type_changed set_templated_description + set_cause_for_readonly_attributes end def derivable_attribute @@ -416,4 +417,12 @@ def parent_due_later_than_start? (due && !start) || ((due && start) && (due > start)) end + + def set_cause_for_readonly_attributes + return unless work_package.changes.keys.intersect?(%w(created_at updated_at author)) + + work_package.journal_cause = { + "type" => "default_attribute_written" + } + end end diff --git a/app/views/admin/settings/api_settings/show.html.erb b/app/views/admin/settings/api_settings/show.html.erb index 046c3dd5cc31..1f3e78ea358b 100644 --- a/app/views/admin/settings/api_settings/show.html.erb +++ b/app/views/admin/settings/api_settings/show.html.erb @@ -39,6 +39,14 @@ See COPYRIGHT and LICENSE files for more details.

<%= t(:setting_apiv3_max_page_instructions_html) %>

+
+ <%= setting_check_box :apiv3_write_readonly_attributes, container_class: '-slim' %> +
+

<%= t(:setting_apiv3_write_readonly_attributes_instructions_html, + api_documentation_link: static_link_to(:api_docs) + ) %>

+
+
diff --git a/config/constants/settings/definition.rb b/config/constants/settings/definition.rb index c3ec3526f229..28df4a724e10 100644 --- a/config/constants/settings/definition.rb +++ b/config/constants/settings/definition.rb @@ -62,6 +62,10 @@ class Definition apiv3_max_page_size: { default: 1000 }, + apiv3_write_readonly_attributes: { + description: "Allow overriding readonly attributes (e.g. createdAt, updatedAt, author) during the creation of resources via the REST API", + default: false + }, app_title: { default: "OpenProject" }, diff --git a/config/locales/en.yml b/config/locales/en.yml index 7703ace3f93d..174a2ba29a24 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1646,6 +1646,7 @@ en: changes_retracted: "The changes were retracted." caused_changes: + default_attribute_written: "Read-only attributes written" dates_changed: "Dates changed" system_update: "OpenProject system update:" @@ -2873,6 +2874,16 @@ en: If CORS is enabled, these are the origins that are allowed to access OpenProject API.
Please check the Documentation on the Origin header on how to specify the expected values. + setting_apiv3_write_readonly_attributes: "Write access to read-only attributes" + setting_apiv3_write_readonly_attributes_instructions_html: > + If enabled, the API will allow administrators to write static read-only attributes during creation, + such as createdAt and updatedAt timestamps. +
+ Warning: This setting has a use-case for e.g., importing data, but allows + administrators to impersonate the creation of items as other users. All creation requests are being + logged however with the true author. +
+ For more information on attributes and supported resources, please see the %{api_documentation_link}. setting_apiv3_max_page_size: "Maximum API page size" setting_apiv3_max_page_instructions_html: > Set the maximum page size the API will respond with. diff --git a/docs/api/apiv3/components/schemas/project_model.yml b/docs/api/apiv3/components/schemas/project_model.yml index 166a14575c47..2e766c0c62f2 100644 --- a/docs/api/apiv3/components/schemas/project_model.yml +++ b/docs/api/apiv3/components/schemas/project_model.yml @@ -29,7 +29,7 @@ properties: createdAt: type: string format: date-time - description: Time of creation + description: Time of creation. Can be writable by admins with the `apiv3_write_readonly_attributes` setting enabled. updatedAt: type: string format: date-time diff --git a/docs/api/apiv3/components/schemas/work_package_model.yml b/docs/api/apiv3/components/schemas/work_package_model.yml index 2e8e48a36c4b..0f364c55fb45 100644 --- a/docs/api/apiv3/components/schemas/work_package_model.yml +++ b/docs/api/apiv3/components/schemas/work_package_model.yml @@ -104,12 +104,12 @@ properties: createdAt: type: string format: date-time - description: Time of creation + description: Time of creation. Can be writable by admins with the `apiv3_write_readonly_attributes` setting enabled. readOnly: true updatedAt: type: string format: date-time - description: Time of the most recent change to the work package + description: Time of the most recent change to the work package. Can be writable by admins with the `apiv3_write_readonly_attributes` setting enabled. readOnly: true _links: type: object diff --git a/lib/open_project/journal_formatter/cause.rb b/lib/open_project/journal_formatter/cause.rb index 53eb0d7e32ad..1b438575225f 100644 --- a/lib/open_project/journal_formatter/cause.rb +++ b/lib/open_project/journal_formatter/cause.rb @@ -45,11 +45,16 @@ def render(_key, values, options = { html: true }) private def cause_type_translation(type) + mapped_type = mapped_cause_type(type) + I18n.t("journals.caused_changes.#{mapped_type}", default: mapped_type) + end + + def mapped_cause_type(type) case type - when 'system_update' - I18n.t("journals.caused_changes.system_update") + when /changed_times/, "working_days_changed" + "dates_changed" else - I18n.t("journals.caused_changes.dates_changed") + type end end diff --git a/spec/contracts/projects/create_contract_spec.rb b/spec/contracts/projects/create_contract_spec.rb index c152356c7134..ed6be8f5d485 100644 --- a/spec/contracts/projects/create_contract_spec.rb +++ b/spec/contracts/projects/create_contract_spec.rb @@ -42,6 +42,9 @@ status_explanation: project_status_explanation) end let(:global_permissions) { [:add_project] } + let(:validated_contract) do + contract.tap(&:validate) + end subject(:contract) { described_class.new(project, current_user) } @@ -52,5 +55,47 @@ expect_valid(true) end end + + describe "writing read-only attributes" do + shared_examples "can not write" do |attribute, value| + it "can not write #{attribute}", :aggregate_failures do + expect(contract.writable_attributes).not_to include(attribute.to_s) + + project.send(:"#{attribute}=", value) + expect(validated_contract).not_to be_valid + expect(validated_contract.errors[attribute]).to include "was attempted to be written but is not writable." + end + + context "when enabled for admin", with_settings: { apiv3_write_readonly_attributes: true } do + let(:current_user) { build_stubbed(:admin) } + + it_behaves_like "can not write", :updated_at, 1.day.ago + + it "can write created_at", :aggregate_failures do + expect(contract.writable_attributes).to include('created_at') + + project.created_at = 10.days.ago + expect(validated_contract.errors[attribute]).to be_empty + end + end + + context "when disabled for admin", with_settings: { apiv3_write_readonly_attributes: false } do + let(:current_user) { build_stubbed(:admin) } + + it_behaves_like "can not write", :created_at, 1.day.ago + it_behaves_like "can not write", :updated_at, 1.day.ago + end + + context "when enabled for regular user", with_settings: { apiv3_write_readonly_attributes: true } do + it_behaves_like "can not write", :created_at, 1.day.ago + it_behaves_like "can not write", :updated_at, 1.day.ago + end + + context "when disabled for regular user", with_settings: { apiv3_write_readonly_attributes: false } do + it_behaves_like "can not write", :created_at, 1.day.ago + it_behaves_like "can not write", :updated_at, 1.day.ago + end + end + end end end diff --git a/spec/contracts/work_packages/create_contract_spec.rb b/spec/contracts/work_packages/create_contract_spec.rb index 1c382797245a..810bca17a333 100644 --- a/spec/contracts/work_packages/create_contract_spec.rb +++ b/spec/contracts/work_packages/create_contract_spec.rb @@ -26,8 +26,8 @@ # See COPYRIGHT and LICENSE files for more details. #++ -require 'spec_helper' -require 'contracts/work_packages/shared_base_contract' +require "spec_helper" +require "contracts/work_packages/shared_base_contract" RSpec.describe WorkPackages::CreateContract do let(:work_package) do @@ -47,10 +47,10 @@ subject(:contract) { described_class.new(work_package, user) } - it_behaves_like 'work package contract' + it_behaves_like "work package contract" - describe 'authorization' do - context 'when user allowed in project and project specified' do + describe "authorization" do + context "when user allowed in project and project specified" do before do mock_permissions_for(user) do |mock| mock.allow_in_project :add_work_packages, project: @@ -59,12 +59,12 @@ work_package.project = project end - it 'has no authorization error' do + it "has no authorization error" do expect(validated_contract.errors[:base]).to be_empty end end - context 'when user not allowed in project and project specified' do + context "when user not allowed in project and project specified" do before do mock_permissions_for(user) do |mock| mock.allow_in_project :add_work_packages, project: other_project @@ -73,96 +73,116 @@ work_package.project = project end - it 'is not authorized' do + it "is not authorized" do expect(validated_contract.errors.symbols_for(:base)) .to contain_exactly(:error_unauthorized) end end - context 'when user allowed in a project and no project specified' do + context "when user allowed in a project and no project specified" do before do mock_permissions_for(user) do |mock| mock.allow_in_project :add_work_packages, project: end end - it 'has no authorization error' do + it "has no authorization error" do expect(validated_contract.errors[:base]).to be_empty end end - context 'when user not allowed in any projects and no project specified' do + context "when user not allowed in any projects and no project specified" do before do mock_permissions_for(user, &:forbid_everything) end - it 'is not authorized' do + it "is not authorized" do expect(validated_contract.errors.symbols_for(:base)) .to contain_exactly(:error_unauthorized) end end - context 'when user not allowed in any projects and project specified' do + context "when user not allowed in any projects and project specified" do before do mock_permissions_for(user, &:forbid_everything) work_package.project = project end - it 'is not authorized' do + it "is not authorized" do expect(validated_contract.errors.symbols_for(:base)) .to contain_exactly(:error_unauthorized) end end end - describe 'author_id' do + describe "remaining hours" do before do mock_permissions_for(user) do |mock| mock.allow_in_project :add_work_packages, project: end + work_package.project = project end - context 'if the user is set by the system and the user is the user the contract is evaluated for' do - subject(:contract) { described_class.new(work_package, user) } - - it 'is valid' do - work_package.change_by_system do - work_package.author = user - end + context "when not changed" do + it("is valid") { expect(validated_contract.errors[:remaining_hours]).to be_empty } + end - expect(validated_contract.errors[:author_id]).to be_empty + context "when changed" do + before do + work_package.remaining_hours = 10 end + + it("is valid") { expect(validated_contract.errors[:remaining_hours]).to be_empty } end + end - context 'if the user is different from the user the contract is evaluated for' do - it 'is invalid' do - work_package.author = build_stubbed(:user) + describe "writing read-only attributes" do + shared_examples "can write" do |attribute, value| + it "can write #{attribute}", :aggregate_failures do + expect(contract.writable_attributes).to include(attribute.to_s) - expect(validated_contract.errors.symbols_for(:author_id)) - .to match_array %i[invalid error_readonly] + work_package.send(:"#{attribute}=", value) + expect(validated_contract.errors[attribute]).to be_empty end end - end - describe 'remaining hours' do - before do - mock_permissions_for(user) do |mock| - mock.allow_in_project :add_work_packages, project: + shared_examples "can not write" do |attribute, value| + it "can not write #{attribute}", :aggregate_failures do + expect(contract.writable_attributes).not_to include(attribute.to_s) + + work_package.send(:"#{attribute}=", value) + expect(validated_contract).not_to be_valid + expect(validated_contract.errors[attribute]).to include "was attempted to be written but is not writable." end - work_package.project = project end - context 'when not changed' do - it('is valid') { expect(validated_contract.errors[:remaining_hours]).to be_empty } + context "when enabled for admin", with_settings: { apiv3_write_readonly_attributes: true } do + let(:user) { build_stubbed(:admin) } + + it_behaves_like "can write", :created_at, 1.day.ago + it_behaves_like "can write", :updated_at, 1.day.ago + it_behaves_like "can write", :author_id, 1234 end - context 'when changed' do - before do - work_package.remaining_hours = 10 - end + context "when disabled for admin", with_settings: { apiv3_write_readonly_attributes: false } do + let(:user) { build_stubbed(:admin) } + + it_behaves_like "can not write", :created_at, 1.day.ago + it_behaves_like "can not write", :updated_at, 1.day.ago + it_behaves_like "can not write", :author_id, 1234 + end + + context "when enabled for regular user", with_settings: { apiv3_write_readonly_attributes: true } do + it_behaves_like "can not write", :created_at, 1.day.ago + it_behaves_like "can not write", :updated_at, 1.day.ago + it_behaves_like "can not write", :author_id, 1234 + end - it('is valid') { expect(validated_contract.errors[:remaining_hours]).to be_empty } + context "when disabled for regular user", with_settings: { apiv3_write_readonly_attributes: false } do + it_behaves_like "can not write", :created_at, 1.day.ago + it_behaves_like "can not write", :updated_at, 1.day.ago + it_behaves_like "can not write", :author_id, 1234 end end end diff --git a/spec/services/projects/create_service_integration_spec.rb b/spec/services/projects/create_service_integration_spec.rb new file mode 100644 index 000000000000..d1b13119ed5d --- /dev/null +++ b/spec/services/projects/create_service_integration_spec.rb @@ -0,0 +1,72 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2024 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 Projects::CreateService, "integration", type: :model do + let(:instance) { described_class.new(user:) } + let(:new_project) { service_result.result } + let(:service_result) { instance.call(**attributes) } + + before do + login_as(user) + end + + describe "writing created_at timestamp" do + shared_let(:user) { create(:admin) } + + let(:created_at) { 11.days.ago } + + let(:attributes) do + { + name: "test", + created_at:, + } + end + + context "when enabled", with_settings: { apiv3_write_readonly_attributes: true } do + it "updates the timestamps correctly" do + expect(service_result) + .to be_success + + new_project.reload + expect(new_project.created_at).to be_within(1.second).of created_at + end + end + + context "when disabled", with_settings: { apiv3_write_readonly_attributes: false } do + it "rejects the creation" do + expect(service_result) + .not_to be_success + + expect(new_project.errors.symbols_for(:created_at)) + .to contain_exactly(:error_readonly) + end + end + end +end diff --git a/spec/services/work_packages/create_service_integration_spec.rb b/spec/services/work_packages/create_service_integration_spec.rb index 529903920b38..7c9b1373706c 100644 --- a/spec/services/work_packages/create_service_integration_spec.rb +++ b/spec/services/work_packages/create_service_integration_spec.rb @@ -26,9 +26,9 @@ # See COPYRIGHT and LICENSE files for more details. #++ -require 'spec_helper' +require "spec_helper" -RSpec.describe WorkPackages::CreateService, 'integration', type: :model do +RSpec.describe WorkPackages::CreateService, "integration", type: :model do let(:user) do create(:user, member_with_roles: { project => role }) end @@ -47,7 +47,7 @@ let(:project) { create(:project, types: [type, default_type]) } let(:parent) do create(:work_package, - subject: 'parent', + subject: "parent", project:, type:) end @@ -77,45 +77,45 @@ login_as(user) end - context 'when the only type of the project is a milestone' do + context "when the only type of the project is a milestone" do let(:default_type) do create(:type_milestone) end let(:project) { create(:project, types: [default_type]) } - describe 'call without date attributes' do + describe "call without date attributes" do let(:attributes) do - { subject: 'blubs', project: } + { subject: "blubs", project: } end - it 'creates the default type without errors' do + it "creates the default type without errors" do expect(service_result).to be_success expect(service_result.errors).to be_empty end end - describe 'call with a parent non-milestone with dates' do + describe "call with a parent non-milestone with dates" do let(:parent) do create(:work_package, project:, - start_date: '2024-01-01', - due_date: '2024-01-10', + start_date: "2024-01-01", + due_date: "2024-01-10", type: create(:type)) end let(:attributes) do - { subject: 'blubs', project:, parent: } + { subject: "blubs", project:, parent: } end - it 'creates the default type without errors' do + it "creates the default type without errors" do expect(service_result).to be_success expect(service_result.errors).to be_empty end end end - describe '#call' do + describe "#call" do let(:attributes) do - { subject: 'blubs', + { subject: "blubs", project:, done_ratio: 50, parent:, @@ -123,7 +123,7 @@ due_date: Date.today + 3.days } end - it 'creates the work_package with the provided attributes and sets the user as a watcher' do + it "creates the work_package with the provided attributes and sets the user as a watcher" do # successful expect(service_result) .to be_success @@ -164,7 +164,7 @@ .to contain_exactly(user) end - describe 'setting the attachments' do + describe "setting the attachments" do let!(:other_users_attachment) do create(:attachment, container: nil, author: create(:user)) end @@ -172,7 +172,7 @@ create(:attachment, container: nil, author: user) end - it 'reports on invalid attachments and sets the new if everything is valid' do + it "reports on invalid attachments and sets the new if everything is valid" do result = instance.call(**attributes.merge(attachment_ids: [other_users_attachment.id])) expect(result) @@ -201,12 +201,12 @@ end end - describe 'with a child creation with both dates and work' do + describe "with a child creation with both dates and work" do let(:start_date) { Date.current } let(:due_date) { start_date + 3.days } let(:attributes) do { - subject: 'child', + subject: "child", project:, parent:, estimated_hours: 5, @@ -215,7 +215,7 @@ } end - it 'correctly updates the parent values' do + it "correctly updates the parent values" do expect(service_result) .to be_success @@ -225,5 +225,46 @@ expect(parent.due_date).to eq(due_date) end end + + describe "writing timestamps" do + shared_let(:user) { create(:admin) } + shared_let(:other_user) { create(:user) } + + let(:created_at) { 11.days.ago } + let(:updated_at) { 10.days.ago } + + let(:attributes) do + { + subject: "child", + project:, + author: other_user, + created_at:, + updated_at: + } + end + + context "when enabled", with_settings: { apiv3_write_readonly_attributes: true } do + it "updates the timestamps correctly" do + expect(service_result) + .to be_success + + expect(new_work_package.created_at).to be_within(1.second).of(created_at) + expect(new_work_package.updated_at).to be_within(1.second).of(updated_at) + end + end + + context "when disabled", with_settings: { apiv3_write_readonly_attributes: false } do + it "rejects the creation" do + expect(service_result) + .not_to be_success + + expect(new_work_package.errors.symbols_for(:created_at)) + .to contain_exactly(:error_readonly) + + expect(new_work_package.errors.symbols_for(:updated_at)) + .to contain_exactly(:error_readonly) + end + end + end end end