Commit daeb7d83 authored by Alexandru Croitor's avatar Alexandru Croitor Committed by John T Skarbek

Prevent non-owners to set system_note_timestamp

When an issue is created or updated though API for import
purposes we allow providing created_at and updated_at params
these would then be reflected also in system notes. Only admins
and project owners should be able to set these dates.
parent 4d15b636
......@@ -34,7 +34,7 @@ module Issues
private
def filter_params(merge_request)
def filter_params(issue)
super
moved_issue = params.delete(:moved_issue)
......@@ -44,6 +44,8 @@ module Issues
params.delete(:iid) unless current_user.can?(:set_issue_iid, project)
params.delete(:created_at) unless moved_issue || current_user.can?(:set_issue_created_at, project)
params.delete(:updated_at) unless moved_issue || current_user.can?(:set_issue_updated_at, project)
issue.system_note_timestamp = params[:created_at] || params[:updated_at]
end
def create_assignee_note(issue, old_assignees)
......
---
title: Restrict setting system_note_timestamp to owners
merge_request:
author:
type: security
......@@ -249,7 +249,6 @@ module API
authorize! :create_issue, user_project
issue_params = declared_params(include_missing: false)
issue_params[:system_note_timestamp] = params[:created_at]
issue_params = convert_parameters_from_legacy_format(issue_params)
......@@ -293,8 +292,6 @@ module API
issue = user_project.issues.find_by!(iid: params.delete(:issue_iid))
authorize! :update_issue, issue
issue.system_note_timestamp = params[:updated_at]
update_params = declared_params(include_missing: false).merge(request: request, api: true)
update_params = convert_parameters_from_legacy_format(update_params)
......
......@@ -943,6 +943,34 @@ RSpec.describe API::Issues do
it_behaves_like 'issuable update endpoint' do
let(:entity) { issue }
end
describe 'updated_at param' do
let(:fixed_time) { Time.new(2001, 1, 1) }
let(:updated_at) { Time.new(2000, 1, 1) }
before do
travel_to fixed_time
end
it 'allows admins to set the timestamp' do
put api("/projects/#{project.id}/issues/#{issue.iid}", admin), params: { labels: 'label1', updated_at: updated_at }
expect(response).to have_gitlab_http_status(:ok)
expect(Time.parse(json_response['updated_at'])).to be_like_time(updated_at)
expect(ResourceLabelEvent.last.created_at).to be_like_time(updated_at)
end
it 'does not allow other users to set the timestamp' do
reporter = create(:user)
project.add_developer(reporter)
put api("/projects/#{project.id}/issues/#{issue.iid}", reporter), params: { labels: 'label1', updated_at: updated_at }
expect(response).to have_gitlab_http_status(:ok)
expect(Time.parse(json_response['updated_at'])).to be_like_time(fixed_time)
expect(ResourceLabelEvent.last.created_at).to be_like_time(fixed_time)
end
end
end
describe 'DELETE /projects/:id/issues/:issue_iid' do
......
......@@ -330,15 +330,21 @@ RSpec.describe API::Issues do
end
context 'setting created_at' do
let(:fixed_time) { Time.new(2001, 1, 1) }
let(:creation_time) { 2.weeks.ago }
let(:params) { { title: 'new issue', labels: 'label, label2', created_at: creation_time } }
before do
travel_to fixed_time
end
context 'by an admin' do
it 'sets the creation time on the new issue' do
post api("/projects/#{project.id}/issues", admin), params: params
expect(response).to have_gitlab_http_status(:created)
expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time)
expect(ResourceLabelEvent.last.created_at).to be_like_time(creation_time)
end
end
......@@ -348,6 +354,7 @@ RSpec.describe API::Issues do
expect(response).to have_gitlab_http_status(:created)
expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time)
expect(ResourceLabelEvent.last.created_at).to be_like_time(creation_time)
end
end
......@@ -356,19 +363,24 @@ RSpec.describe API::Issues do
group = create(:group)
group_project = create(:project, :public, namespace: group)
group.add_owner(user2)
post api("/projects/#{group_project.id}/issues", user2), params: params
expect(response).to have_gitlab_http_status(:created)
expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time)
expect(ResourceLabelEvent.last.created_at).to be_like_time(creation_time)
end
end
context 'by another user' do
it 'ignores the given creation time' do
project.add_developer(user2)
post api("/projects/#{project.id}/issues", user2), params: params
expect(response).to have_gitlab_http_status(:created)
expect(Time.parse(json_response['created_at'])).not_to be_like_time(creation_time)
expect(Time.parse(json_response['created_at'])).to be_like_time(fixed_time)
expect(ResourceLabelEvent.last.created_at).to be_like_time(fixed_time)
end
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment