diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index a8d7a05f50985596f5bb878f87d72f0262561f47..bb800929ea901b23bf3c4b38310f3115c26514e7 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -72,6 +72,8 @@ class GroupPolicy < BasePolicy enable :admin_namespace enable :admin_group_member enable :change_visibility_level + + enable :set_note_created_at end rule { can?(:read_nested_project_resources) }.policy do diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 00c58f15013ba244f5c467b8ac8adfaa5f0f86da..fd6cc504a3bd5c41bb3b9d052f0b1983cc780bf3 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -143,6 +143,10 @@ class ProjectPolicy < BasePolicy enable :destroy_merge_request enable :destroy_issue enable :remove_pages + + enable :set_issue_iid + enable :set_issue_created_at + enable :set_note_created_at end rule { can?(:guest_access) }.policy do diff --git a/lib/api/helpers/notes_helpers.rb b/lib/api/helpers/notes_helpers.rb index 7885e3fa1e103a033b33f858633a7964332e0048..7b1f5c2584b9c5f980ccc007620bda7790f4dcda 100644 --- a/lib/api/helpers/notes_helpers.rb +++ b/lib/api/helpers/notes_helpers.rb @@ -92,10 +92,7 @@ module API parent = noteable_parent(noteable) - if opts[:created_at] - opts.delete(:created_at) unless - (current_user.admin? || user_project.owner == current_user || current_user.owned_groups.include?(user_project.owner)) - end + opts.delete(:created_at) unless current_user.can?(:set_note_created_at, policy_object) opts[:updated_at] = opts[:created_at] if opts[:created_at] diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 2d2250acaa2011190cd1a64e24d73c3776624f91..cedfd2fbaa0696cbcb998fd0fe56750303de3e0b 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -172,11 +172,8 @@ module API authorize! :create_issue, user_project - # Setting created_at time or iid only allowed for admins and project owners - unless current_user.admin? || user_project.owner == current_user || current_user.owned_groups.include?(user_project.owner) - params.delete(:created_at) - params.delete(:iid) - end + params.delete(:created_at) unless current_user.can?(:set_issue_created_at, user_project) + params.delete(:iid) unless current_user.can?(:set_issue_iid, user_project) issue_params = declared_params(include_missing: false) diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index 615fea11f26667915a60dadd85214e9a8030a66e..f276535265bede9ecd6e7d1abde210af821e19f7 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -31,6 +31,7 @@ describe GroupPolicy do :admin_namespace, :admin_group_member, :change_visibility_level, + :set_note_created_at, (Gitlab::Database.postgresql? ? :create_subgroup : nil) ].compact end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index dd3fa4e6a518882a39cf6b9de35189525aee5332..b7ec35d6ec51240098f6c12950ffcf378c9be011 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -64,6 +64,7 @@ describe ProjectPolicy do %i[ change_namespace change_visibility_level rename_project remove_project archive_project remove_fork_project destroy_merge_request destroy_issue + set_issue_iid set_issue_created_at set_note_created_at ] end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 28ba00c7293a43aecf94b130c22b62efaed2fa2b..f64815feffa6370c83bfc94fba7aff1b8c9a979c 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -1023,6 +1023,20 @@ describe API::Issues do end end + context 'by a group owner' do + let(:group) { create(:group) } + let(:group_project) { create(:project, :public, namespace: group) } + + it 'sets the internal ID on the new issue' do + group.add_owner(user2) + post api("/projects/#{group_project.id}/issues", user2), + title: 'new issue', iid: 9001 + + expect(response).to have_gitlab_http_status(201) + expect(json_response['iid']).to eq 9001 + end + end + context 'by another user' do it 'ignores the given internal ID' do post api("/projects/#{project.id}/issues", user2), @@ -1154,14 +1168,47 @@ describe API::Issues do end end - context 'when an admin or owner makes the request' do - it 'accepts the creation date to be set' do - creation_time = 2.weeks.ago - post api("/projects/#{project.id}/issues", user), - title: 'new issue', labels: 'label, label2', created_at: creation_time + context 'setting created_at' do + let(:creation_time) { 2.weeks.ago } + let(:params) { { title: 'new issue', labels: 'label, label2', created_at: creation_time } } - expect(response).to have_gitlab_http_status(201) - expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time) + context 'by an admin' do + it 'sets the creation time on the new issue' do + post api("/projects/#{project.id}/issues", admin), params + + expect(response).to have_gitlab_http_status(201) + expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time) + end + end + + context 'by a project owner' do + it 'sets the creation time on the new issue' do + post api("/projects/#{project.id}/issues", user), params + + expect(response).to have_gitlab_http_status(201) + expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time) + end + end + + context 'by a group owner' do + it 'sets the creation time on the new issue' do + group = create(:group) + group_project = create(:project, :public, namespace: group) + group.add_owner(user2) + post api("/projects/#{group_project.id}/issues", user2), params + + expect(response).to have_gitlab_http_status(201) + expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time) + end + end + + context 'by another user' do + it 'ignores the given creation time' do + post api("/projects/#{project.id}/issues", user2), params + + expect(response).to have_gitlab_http_status(201) + expect(Time.parse(json_response['created_at'])).not_to be_like_time(creation_time) + end end end diff --git a/spec/support/shared_examples/requests/api/notes.rb b/spec/support/shared_examples/requests/api/notes.rb index 1b5630212449ee8ad12b86e8ac9e36eb27d2fd04..0e20dfe0725a8217cebd065035f1bde78e423f46 100644 --- a/spec/support/shared_examples/requests/api/notes.rb +++ b/spec/support/shared_examples/requests/api/notes.rb @@ -111,17 +111,79 @@ shared_examples 'noteable API' do |parent_type, noteable_type, id_name| post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user), body: 'hi!' end - context 'when an admin or owner makes the request' do - it 'accepts the creation date to be set' do - creation_time = 2.weeks.ago - post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user), - body: 'hi!', created_at: creation_time + context 'setting created_at' do + let(:creation_time) { 2.weeks.ago } + let(:params) { { body: 'hi!', created_at: creation_time } } + + context 'by an admin' do + it 'sets the creation time on the new note' do + admin = create(:admin) + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", admin), params + + expect(response).to have_gitlab_http_status(201) + expect(json_response['body']).to eq('hi!') + expect(json_response['author']['username']).to eq(admin.username) + expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time) + expect(Time.parse(json_response['updated_at'])).to be_like_time(creation_time) + end + end - expect(response).to have_gitlab_http_status(201) - expect(json_response['body']).to eq('hi!') - expect(json_response['author']['username']).to eq(user.username) - expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time) - expect(Time.parse(json_response['updated_at'])).to be_like_time(creation_time) + if parent_type == 'projects' + context 'by a project owner' do + it 'sets the creation time on the new note' do + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user), params + + expect(response).to have_gitlab_http_status(201) + expect(json_response['body']).to eq('hi!') + expect(json_response['author']['username']).to eq(user.username) + expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time) + expect(Time.parse(json_response['updated_at'])).to be_like_time(creation_time) + end + end + + context 'by a group owner' do + it 'sets the creation time on the new note' do + user2 = create(:user) + group = create(:group) + group.add_owner(user2) + parent.update!(namespace: group) + user2.refresh_authorized_projects + + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user2), params + + expect(response).to have_gitlab_http_status(201) + expect(json_response['body']).to eq('hi!') + expect(json_response['author']['username']).to eq(user2.username) + expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time) + expect(Time.parse(json_response['updated_at'])).to be_like_time(creation_time) + end + end + elsif parent_type == 'groups' + context 'by a group owner' do + it 'sets the creation time on the new note' do + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user), params + + expect(response).to have_gitlab_http_status(201) + expect(json_response['body']).to eq('hi!') + expect(json_response['author']['username']).to eq(user.username) + expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time) + expect(Time.parse(json_response['updated_at'])).to be_like_time(creation_time) + end + end + end + + context 'by another user' do + it 'ignores the given creation time' do + user2 = create(:user) + parent.add_developer(user2) + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user2), params + + expect(response).to have_gitlab_http_status(201) + expect(json_response['body']).to eq('hi!') + expect(json_response['author']['username']).to eq(user2.username) + expect(Time.parse(json_response['created_at'])).not_to be_like_time(creation_time) + expect(Time.parse(json_response['updated_at'])).not_to be_like_time(creation_time) + end end end