diff options
author | DJ Mountney <dj@gitlab.com> | 2017-03-18 04:23:15 +0000 |
---|---|---|
committer | DJ Mountney <david@twkie.net> | 2017-03-17 21:25:55 -0700 |
commit | 219e0cf36d37848a25ad1cc6a663c461732068fe (patch) | |
tree | e15cfab675d0791a704e49b936c87074d43ab177 | |
parent | f501ead612314afb06be8c1b15739a04c9ea73ff (diff) | |
download | gitlab-ce-219e0cf36d37848a25ad1cc6a663c461732068fe.tar.gz |
Merge branch 'render-json-leak' into 'security'
fix for render json include leaks
See merge request !2074
4 files changed, 38 insertions, 2 deletions
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 1151555b8fa..857d907c1a6 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -129,7 +129,7 @@ class Projects::IssuesController < Projects::ApplicationController end format.json do - render json: @issue.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } }, methods: [:task_status, :task_status_short]) + render json: @issue.to_json(include: { milestone: {}, assignee: { only: [:name, :username], methods: [:avatar_url] }, labels: { methods: :text_color } }, methods: [:task_status, :task_status_short]) end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 82f9b6e06db..677a8a1a73a 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -308,7 +308,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController end format.json do - render json: @merge_request.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } }, methods: [:task_status, :task_status_short]) + render json: @merge_request.to_json(include: { milestone: {}, assignee: { only: [:name, :username], methods: [:avatar_url] }, labels: { methods: :text_color } }, methods: [:task_status, :task_status_short]) end end rescue ActiveRecord::StaleObjectError diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 46c758b4654..972005ede3a 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -132,6 +132,24 @@ describe Projects::IssuesController do it_behaves_like 'update invalid issuable', Issue + context 'changing the assignee' do + it 'limits the attributes exposed on the assignee' do + assignee = create(:user) + project.add_developer(assignee) + + put :update, + namespace_id: project.namespace.to_param, + project_id: project, + id: issue.iid, + issue: { assignee_id: assignee.id }, + format: :json + body = JSON.parse(response.body) + + expect(body['assignee'].keys) + .to match_array(%w(name username avatar_url)) + end + end + context 'when moving issue to another private project' do let(:another_project) { create(:empty_project, :private) } diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 250d64f7055..c310d830e81 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -203,6 +203,24 @@ describe Projects::MergeRequestsController do end describe 'PUT update' do + context 'changing the assignee' do + it 'limits the attributes exposed on the assignee' do + assignee = create(:user) + project.add_developer(assignee) + + put :update, + namespace_id: project.namespace.to_param, + project_id: project, + id: merge_request.iid, + merge_request: { assignee_id: assignee.id }, + format: :json + body = JSON.parse(response.body) + + expect(body['assignee'].keys) + .to match_array(%w(name username avatar_url)) + end + end + context 'there is no source project' do let(:project) { create(:project) } let(:fork_project) { create(:forked_project_with_submodules) } |