Commit cdf396f4 authored by DJ Mountney's avatar DJ Mountney 🔴 Committed by Ruben Alexis
Browse files

Merge branch 'render-json-leak' into 'security' 

fix for render json include leaks

See merge request !2074
Conflicts:
	app/controllers/projects/merge_requests_controller.rb
	spec/controllers/projects/issues_controller_spec.rb
parent fe2feaa7

app/controllers/projects/issues_controller.rb

+1 −1
Original line number Diff line number Diff line
@@ -130,7 +130,7 @@ def update 
      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

app/controllers/projects/merge_requests_controller.rb

+1 −1
Original line number Diff line number Diff line
@@ -281,7 +281,7 @@ def update 
                       @merge_request.target_project, @merge_request])

        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

    else

spec/controllers/projects/issues_controller_spec.rb

+23 −0
Original line number Diff line number Diff line
@@ -123,6 +123,29 @@ 
  end



  describe 'PUT #update' do

    before do

      sign_in(user)

      project.team << [user, :developer]

    end



    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(:project, :private) }

spec/controllers/projects/merge_requests_controller_spec.rb

+18 −0
Original line number Diff line number Diff line
@@ -177,6 +177,24 @@ def get_merge_requests(page = nil) 
  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) }