diff options
author | DJ Mountney <dj@gitlab.com> | 2017-03-18 07:23:15 +0300 |
---|---|---|
committer | DJ Mountney <david@twkie.net> | 2017-03-21 04:54:17 +0300 |
commit | 7be39a894b27c0c0e4fab52c2f8147f216376538 (patch) | |
tree | 806b9552af5476d8a59d746e5260dade42e4237b | |
parent | 83a0c39808b132e8759d75cc774e0724f56b17ab (diff) |
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 f2fee62ebd6..088f46457b6 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -146,7 +146,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 6ceaf96f78f..98f3122240c 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -141,6 +141,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) } |