kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Only include the user's ID in the time_spent command's update hash

Browse source 
Previously, this would include the entire User record in the update
hash, which was rendered in the response using `to_json`, erroneously
exposing every attribute of that record, including their (now removed)
private token.

Now we only include the user ID, and perform the lookup on-demand.
This commit is contained in:
Robert Speicher 2017-12-14 13:32:55 -06:00
parent 8d0ad36bcf
commit 3e4b45fc21
11 changed files with 25 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/models/concerns/time_trackable.rb
View file

@ -24,7 +24,7 @@ module TimeTrackable
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def spend_time(options)
@time_spent = options[:duration]
@time_spent_user = options[:user]
@time_spent_user = User.find(options[:user_id])
@spent_at = options[:spent_at]
@original_total_time_spent = nil

4
app/services/quick_actions/interpret_service.rb
View file

@ -405,7 +405,7 @@ module QuickActions
if time_spent
@updates[:spend_time] = {
duration: time_spent,
user: current_user,
user_id: current_user.id,
spent_at: time_spent_date
}
end
@ -428,7 +428,7 @@ module QuickActions
current_user.can?(:"admin_#{issuable.to_ability_name}", project)
end
command :remove_time_spent do
@updates[:spend_time] = { duration: :reset, user: current_user }
@updates[:spend_time] = { duration: :reset, user_id: current_user.id }
end
desc "Append the comment with #{SHRUG}"

4
lib/api/time_tracking_endpoints.rb
View file

@ -85,7 +85,7 @@ module API
update_issuable(spend_time: {
duration: Gitlab::TimeTrackingFormatter.parse(params.delete(:duration)),
user: current_user
user_id: current_user.id
})
end
@ -97,7 +97,7 @@ module API
authorize! update_issuable_key, load_issuable
status :ok
update_issuable(spend_time: { duration: :reset, user: current_user })
update_issuable(spend_time: { duration: :reset, user_id: current_user.id })
end
desc "Show time stats for a project #{issuable_name}"

4
lib/api/v3/time_tracking_endpoints.rb
View file

@ -86,7 +86,7 @@ module API
update_issuable(spend_time: {
duration: Gitlab::TimeTrackingFormatter.parse(params.delete(:duration)),
user: current_user
user_id: current_user.id
})
end
@ -98,7 +98,7 @@ module API
authorize! update_issuable_key, load_issuable
status :ok
update_issuable(spend_time: { duration: :reset, user: current_user })
update_issuable(spend_time: { duration: :reset, user_id: current_user.id })
end
desc "Show time stats for a project #{issuable_name}"

4
spec/features/milestone_spec.rb
View file

@ -82,9 +82,9 @@ feature 'Milestone' do
milestone = create(:milestone, project: project, title: 8.7)
issue1 = create(:issue, project: project, milestone: milestone)
issue2 = create(:issue, project: project, milestone: milestone)
issue1.spend_time(duration: 3600, user: user)
issue1.spend_time(duration: 3600, user_id: user.id)
issue1.save!
issue2.spend_time(duration: 7200, user: user)
issue2.spend_time(duration: 7200, user_id: user.id)
issue2.save!
visit project_milestone_path(project, milestone)

4
spec/models/concerns/issuable_spec.rb
View file

@ -291,7 +291,7 @@ describe Issuable do
context 'total_time_spent is updated' do
before do
issue.spend_time(duration: 2, user: user, spent_at: Time.now)
issue.spend_time(duration: 2, user_id: user.id, spent_at: Time.now)
issue.save
expect(Gitlab::HookData::IssuableBuilder)
.to receive(:new).with(issue).and_return(builder)
@ -485,7 +485,7 @@ describe Issuable do
let(:issue) { create(:issue) }
def spend_time(seconds)
issue.spend_time(duration: seconds, user: user)
issue.spend_time(duration: seconds, user_id: user.id)
issue.save!
end

4
spec/models/concerns/milestoneish_spec.rb
View file

@ -189,9 +189,9 @@ describe Milestone, 'Milestoneish' do
describe '#total_issue_time_spent' do
it 'calculates total issue time spent' do
closed_issue_1.spend_time(duration: 300, user: author)
closed_issue_1.spend_time(duration: 300, user_id: author.id)
closed_issue_1.save!
closed_issue_2.spend_time(duration: 600, user: assignee)
closed_issue_2.spend_time(duration: 600, user_id: assignee.id)
closed_issue_2.save!
expect(milestone.total_issue_time_spent).to eq(900)

8
spec/services/quick_actions/interpret_service_spec.rb
View file

@ -209,7 +209,7 @@ describe QuickActions::InterpretService do
expect(updates).to eq(spend_time: {
duration: 3600,
user: developer,
user_id: developer.id,
spent_at: DateTime.now.to_date
})
end
@ -221,7 +221,7 @@ describe QuickActions::InterpretService do
expect(updates).to eq(spend_time: {
duration: -1800,
user: developer,
user_id: developer.id,
spent_at: DateTime.now.to_date
})
end
@ -233,7 +233,7 @@ describe QuickActions::InterpretService do
expect(updates).to eq(spend_time: {
duration: 1800,
user: developer,
user_id: developer.id,
spent_at: Date.parse(date)
})
end
@ -267,7 +267,7 @@ describe QuickActions::InterpretService do
it 'populates spend_time: :reset if content contains /remove_time_spent' do
_, updates = service.execute(content, issuable)
expect(updates).to eq(spend_time: { duration: :reset, user: developer })
expect(updates).to eq(spend_time: { duration: :reset, user_id: developer.id })
end
end

4
spec/services/system_note_service_spec.rb
View file

@ -927,7 +927,7 @@ describe SystemNoteService do
# We need a custom noteable in order to the shared examples to be green.
let(:noteable) do
mr = create(:merge_request, source_project: project)
mr.spend_time(duration: 360000, user: author)
mr.spend_time(duration: 360000, user_id: author.id)
mr.save!
mr
end
@ -965,7 +965,7 @@ describe SystemNoteService do
end
def spend_time!(seconds)
noteable.spend_time(duration: seconds, user: author)
noteable.spend_time(duration: seconds, user_id: author.id)
noteable.save!
end
end

6
spec/support/api/time_tracking_shared_examples.rb
View file

@ -79,7 +79,7 @@ shared_examples 'time tracking endpoints' do |issuable_name|
context 'when subtracting time' do
it 'subtracts time of the total spent time' do
issuable.update_attributes!(spend_time: { duration: 7200, user: user })
issuable.update_attributes!(spend_time: { duration: 7200, user_id: user.id })
post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/add_spent_time", user),
duration: '-1h'
@ -91,7 +91,7 @@ shared_examples 'time tracking endpoints' do |issuable_name|
context 'when time to subtract is greater than the total spent time' do
it 'does not modify the total time spent' do
issuable.update_attributes!(spend_time: { duration: 7200, user: user })
issuable.update_attributes!(spend_time: { duration: 7200, user_id: user.id })
post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/add_spent_time", user),
duration: '-1w'
@ -119,7 +119,7 @@ shared_examples 'time tracking endpoints' do |issuable_name|
describe "GET /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/time_stats" do
it "returns the time stats for #{issuable_name}" do
issuable.update_attributes!(spend_time: { duration: 1800, user: user },
issuable.update_attributes!(spend_time: { duration: 1800, user_id: user.id },
time_estimate: 3600)
get api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_stats", user)

6
spec/support/api/v3/time_tracking_shared_examples.rb
View file

@ -75,7 +75,7 @@ shared_examples 'V3 time tracking endpoints' do |issuable_name|
context 'when subtracting time' do
it 'subtracts time of the total spent time' do
issuable.update_attributes!(spend_time: { duration: 7200, user: user })
issuable.update_attributes!(spend_time: { duration: 7200, user_id: user.id })
post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", user),
duration: '-1h'
@ -87,7 +87,7 @@ shared_examples 'V3 time tracking endpoints' do |issuable_name|
context 'when time to subtract is greater than the total spent time' do
it 'does not modify the total time spent' do
issuable.update_attributes!(spend_time: { duration: 7200, user: user })
issuable.update_attributes!(spend_time: { duration: 7200, user_id: user.id })
post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", user),
duration: '-1w'
@ -115,7 +115,7 @@ shared_examples 'V3 time tracking endpoints' do |issuable_name|
describe "GET /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/time_stats" do
it "returns the time stats for #{issuable_name}" do
issuable.update_attributes!(spend_time: { duration: 1800, user: user },
issuable.update_attributes!(spend_time: { duration: 1800, user_id: user.id },
time_estimate: 3600)
get v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_stats", user)