Skip to content

Commit 04c9037

Browse files
committed
Require attribute edit permission for API writes
Issue#editable? is attributes_editable? || notes_addable?, so the API let note-only users rewrite dates and times. Gate PUT/DELETE on attributes_editable? instead, with a regression test that pins the note-only scenario (#16). While here, restructure the update action: the symbol-table loop with send is replaced by an explicit per-side helper (#20).
1 parent 0642992 commit 04c9037

2 files changed

Lines changed: 48 additions & 17 deletions

File tree

app/controllers/issue_datetimes_controller.rb

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,24 +12,10 @@ def show
1212
# A timestamp sets both the core date and the time of day; null clears
1313
# the time of day and keeps the date. Omitted keys are left untouched.
1414
def update
15-
zone = RedmineIssueDatetime.reference_zone
1615
@issue.init_journal(User.current)
1716

18-
[[:starts_at, :start_date, :start_time=], [:ends_at, :due_date, :due_time=]].each do |param, date_attr, time_writer|
19-
next unless params.key?(param)
20-
21-
value = params[param]
22-
if value.present?
23-
timestamp = parse_timestamp(value)
24-
return render_parse_error(param) if timestamp.nil?
25-
26-
local = timestamp.in_time_zone(zone)
27-
@issue.send(:"#{date_attr}=", local.to_date)
28-
@issue.send(time_writer, local.strftime('%H:%M'))
29-
else
30-
@issue.send(time_writer, '')
31-
end
32-
end
17+
return unless apply_time_param(:starts_at, :start_date=, :start_time=)
18+
return unless apply_time_param(:ends_at, :due_date=, :due_time=)
3319

3420
if @issue.save
3521
render json: issue_payload(@issue.reload)
@@ -81,8 +67,11 @@ def find_issue
8167
render_404
8268
end
8369

70+
# attributes_editable?, not editable?: this endpoint changes issue
71+
# attributes (dates and times). editable? is also true for users who may
72+
# only add notes, and those must not be able to change dates here.
8473
def require_edit_permission
85-
return render_403 unless @issue.editable?
74+
return render_403 unless @issue.attributes_editable?
8675
end
8776

8877
def find_project
@@ -91,6 +80,30 @@ def find_project
9180
render_404
9281
end
9382

83+
# Applies one side (start or due) of the request payload to the issue.
84+
# Returns false when the value could not be parsed; the error response
85+
# has been rendered in that case and the caller must stop.
86+
def apply_time_param(param, date_writer, time_writer)
87+
return true unless params.key?(param)
88+
89+
value = params[param]
90+
if value.blank?
91+
@issue.public_send(time_writer, '')
92+
return true
93+
end
94+
95+
timestamp = parse_timestamp(value)
96+
if timestamp.nil?
97+
render_parse_error(param)
98+
return false
99+
end
100+
101+
local = timestamp.in_time_zone(RedmineIssueDatetime.reference_zone)
102+
@issue.public_send(date_writer, local.to_date)
103+
@issue.public_send(time_writer, local.strftime('%H:%M'))
104+
true
105+
end
106+
94107
# Requires an explicit offset (Z or +hh:mm) so API writes are
95108
# unambiguous regardless of the server's local time zone.
96109
def parse_timestamp(value)

test/integration/issue_datetimes_api_test.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,24 @@ def setup
8787
assert_equal [@issue.id], rows.map { |r| r['issue_id'] }
8888
end
8989

90+
# editable? is also true for users who may only add notes; writing dates
91+
# and times is attribute editing and must require attributes_editable?.
92+
test 'a user who can only add notes cannot change times' do
93+
role = Role.find(1)
94+
role.remove_permission!(:edit_issues, :edit_own_issues)
95+
role.add_permission!(:add_notes)
96+
user = User.find(2)
97+
assert @issue.editable?(user), 'setup: the user should still pass editable?'
98+
assert_not @issue.attributes_editable?(user)
99+
100+
put "/issues/#{@issue.id}/datetime.json",
101+
params: {starts_at: '2026-08-03T09:15:00Z'}.to_json,
102+
headers: {'Content-Type' => 'application/json'}.merge(credentials('jsmith'))
103+
104+
assert_response :forbidden
105+
assert_nil @issue.reload.issue_datetime
106+
end
107+
90108
test 'anonymous users cannot write when login is required' do
91109
with_settings login_required: '1' do
92110
put "/issues/#{@issue.id}/datetime.json",

0 commit comments

Comments
 (0)