diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index cde1e284d2d..86bade49ec9 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -8,12 +8,12 @@ class AutocompleteController < ApplicationController def users @users = AutocompleteUsersFinder.new(params: params, current_user: current_user, project: @project, group: @group).execute - render json: @users, only: [:name, :username, :id], methods: [:avatar_url] + render json: UserSerializer.new.represent(@users) end def user @user = User.find(params[:id]) - render json: @user, only: [:name, :username, :id], methods: [:avatar_url] + render json: UserSerializer.new.represent(@user) end def projects diff --git a/app/helpers/form_helper.rb b/app/helpers/form_helper.rb index b5dece38de1..e26ce6da030 100644 --- a/app/helpers/form_helper.rb +++ b/app/helpers/form_helper.rb @@ -35,7 +35,7 @@ module FormHelper multi_select: true, 'input-meta': 'name', 'always-show-selectbox': true, - current_user_info: current_user.to_json(only: [:id, :name]) + current_user_info: UserSerializer.new.represent(current_user) } } end diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index b4ca0e68e0b..2668cf78afe 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -362,7 +362,7 @@ module IssuablesHelper moveIssueEndpoint: move_namespace_project_issue_path(namespace_id: issuable.project.namespace.to_param, project_id: issuable.project, id: issuable), projectsAutocompleteEndpoint: autocomplete_projects_path(project_id: @project.id), editable: can_edit_issuable, - currentUser: current_user.as_json(only: [:username, :id, :name], methods: :avatar_url), + currentUser: UserSerializer.new.represent(current_user), rootPath: root_path, fullPath: @project.full_path } diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index 2f57660516d..0f9ac958f95 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -139,7 +139,7 @@ module SearchHelper id: "filtered-search-#{type}", placeholder: 'Search or filter results...', data: { - 'username-params' => @users.to_json(only: [:id, :username]) + 'username-params' => UserSerializer.new.represent(@users) }, autocomplete: 'off' } diff --git a/app/models/concerns/blocks_json_serialization.rb b/app/models/concerns/blocks_json_serialization.rb new file mode 100644 index 00000000000..8019e6adc1c --- /dev/null +++ b/app/models/concerns/blocks_json_serialization.rb @@ -0,0 +1,16 @@ +# Overrides `as_json` and `to_json` to raise an exception when called in order +# to prevent accidentally exposing attributes +# +# Not that that would ever happen... but just in case. +module BlocksJsonSerialization + extend ActiveSupport::Concern + + JsonSerializationError = Class.new(StandardError) + + def to_json(*) + raise JsonSerializationError, + "JSON serialization has been disabled on #{self.class.name}" + end + + alias_method :as_json, :to_json +end diff --git a/app/models/concerns/time_trackable.rb b/app/models/concerns/time_trackable.rb index 89fe6527647..5911b56c34c 100644 --- a/app/models/concerns/time_trackable.rb +++ b/app/models/concerns/time_trackable.rb @@ -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 diff --git a/app/models/user.rb b/app/models/user.rb index 51941f43919..b52f17cd6a8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -18,6 +18,7 @@ class User < ActiveRecord::Base include CreatedAtFilterable include IgnorableColumn include BulkMemberAccessLoad + include BlocksJsonSerialization DEFAULT_NOTIFICATION_LEVEL = :participating diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index 06ac86cd5a9..669c1ba0a22 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -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}" diff --git a/app/views/shared/boards/components/_sidebar.html.haml b/app/views/shared/boards/components/_sidebar.html.haml index b3f73e96b81..8e5e32e9f16 100644 --- a/app/views/shared/boards/components/_sidebar.html.haml +++ b/app/views/shared/boards/components/_sidebar.html.haml @@ -1,5 +1,4 @@ -%board-sidebar{ "inline-template" => true, - ":current-user" => "#{current_user ? current_user.to_json(only: [:username, :id, :name], methods: [:avatar_url]) : {}}" } +%board-sidebar{ "inline-template" => true, ":current-user" => (UserSerializer.new.represent(current_user) || {}).to_json } %transition{ name: "boards-sidebar-slide" } %aside.right-sidebar.right-sidebar-expanded.issue-boards-sidebar{ "v-show" => "showSidebar" } .issuable-sidebar diff --git a/lib/api/time_tracking_endpoints.rb b/lib/api/time_tracking_endpoints.rb index df4632346dd..2bb451dea89 100644 --- a/lib/api/time_tracking_endpoints.rb +++ b/lib/api/time_tracking_endpoints.rb @@ -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}" diff --git a/lib/api/v3/time_tracking_endpoints.rb b/lib/api/v3/time_tracking_endpoints.rb index d5b90e435ba..1aad39815f9 100644 --- a/lib/api/v3/time_tracking_endpoints.rb +++ b/lib/api/v3/time_tracking_endpoints.rb @@ -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}" diff --git a/spec/features/milestone_spec.rb b/spec/features/milestone_spec.rb index 27efc32c95b..9f24193a2ac 100644 --- a/spec/features/milestone_spec.rb +++ b/spec/features/milestone_spec.rb @@ -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) diff --git a/spec/models/concerns/blocks_json_serialization_spec.rb b/spec/models/concerns/blocks_json_serialization_spec.rb new file mode 100644 index 00000000000..5906b588d0e --- /dev/null +++ b/spec/models/concerns/blocks_json_serialization_spec.rb @@ -0,0 +1,17 @@ +require 'rails_helper' + +describe BlocksJsonSerialization do + DummyModel = Class.new do + include BlocksJsonSerialization + end + + it 'blocks as_json' do + expect { DummyModel.new.as_json } + .to raise_error(described_class::JsonSerializationError, /DummyModel/) + end + + it 'blocks to_json' do + expect { DummyModel.new.to_json } + .to raise_error(described_class::JsonSerializationError, /DummyModel/) + end +end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 9df26f06a11..4b217df2e8f 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -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 diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb index 9048da0c73d..673c609f534 100644 --- a/spec/models/concerns/milestoneish_spec.rb +++ b/spec/models/concerns/milestoneish_spec.rb @@ -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) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 4687d9dfa00..e58e7588df0 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -12,6 +12,7 @@ describe User do it { is_expected.to include_module(Referable) } it { is_expected.to include_module(Sortable) } it { is_expected.to include_module(TokenAuthenticatable) } + it { is_expected.to include_module(BlocksJsonSerialization) } end describe 'delegations' do diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index c35177f6ebc..eb46480fa54 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -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 diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 47412110b4b..9025589ae0b 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -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 diff --git a/spec/support/api/time_tracking_shared_examples.rb b/spec/support/api/time_tracking_shared_examples.rb index af1083f4bfd..dd3089d22e5 100644 --- a/spec/support/api/time_tracking_shared_examples.rb +++ b/spec/support/api/time_tracking_shared_examples.rb @@ -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) diff --git a/spec/support/api/v3/time_tracking_shared_examples.rb b/spec/support/api/v3/time_tracking_shared_examples.rb index afe0f4cecda..f27a2d06c83 100644 --- a/spec/support/api/v3/time_tracking_shared_examples.rb +++ b/spec/support/api/v3/time_tracking_shared_examples.rb @@ -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)