Skip to content

Commit

Permalink
Allow administrators to override static attributes
Browse files Browse the repository at this point in the history
  • Loading branch information
oliverguenther committed Mar 15, 2024
1 parent 6ac6553 commit ccef658
Show file tree
Hide file tree
Showing 12 changed files with 327 additions and 67 deletions.
47 changes: 47 additions & 0 deletions app/contracts/concerns/admin_writable_timestamps.rb
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions app/contracts/projects/create_contract.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions app/contracts/work_packages/create_contract.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 8 additions & 0 deletions app/views/admin/settings/api_settings/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ See COPYRIGHT and LICENSE files for more details.
<p><%= t(:setting_apiv3_max_page_instructions_html) %></p>
</div>
</div>
<div class="form--field">
<%= setting_check_box :apiv3_write_readonly_attributes, container_class: '-slim' %>
<div class="form--field-instructions">
<p><%= t(:setting_apiv3_write_readonly_attributes_instructions_html,
api_documentation_link: static_link_to(:api_docs)
) %></p>
</div>
</div>
</section>
<section class="form--section">
<fieldset class="form--fieldset">
Expand Down
4 changes: 4 additions & 0 deletions config/constants/settings/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
},
Expand Down
10 changes: 10 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2872,6 +2872,16 @@ en:
If CORS is enabled, these are the origins that are allowed to access OpenProject API.
<br/>
Please check the <a href="%{origin_link}" target="_blank">Documentation on the Origin header</a> 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.
<br/>
<strong>Warning:</strong> 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.
</br>
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.
Expand Down
2 changes: 1 addition & 1 deletion docs/api/apiv3/components/schemas/project_model.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions docs/api/apiv3/components/schemas/work_package_model.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
45 changes: 45 additions & 0 deletions spec/contracts/projects/create_contract_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }

Expand All @@ -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
102 changes: 61 additions & 41 deletions spec/contracts/work_packages/create_contract_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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
Expand All @@ -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
Loading

0 comments on commit ccef658

Please sign in to comment.