Commit a70346fc 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 f71103df

app/controllers/projects/issues_controller.rb

+1 −1
Original line number Original line Diff line number Diff line
@@ -126,7 +126,7 @@ def update 
      end

      end





      format.json do

      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

    end

    end

app/controllers/projects/merge_requests_controller.rb

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

                       @merge_request.target_project, @merge_request])

        end

        end

        format.json do

        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

      end

      end

    else

    else

spec/controllers/projects/issues_controller_spec.rb

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

  end





  describe 'PUT #update' do

  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

    context 'when moving issue to another private project' do

      let(:another_project) { create(:empty_project, :private) }

      let(:another_project) { create(:empty_project, :private) }

spec/controllers/projects/merge_requests_controller_spec.rb

+18 −0
Original line number Original line Diff line number Diff line
@@ -197,6 +197,24 @@ def get_merge_requests(page = nil) 
  end

  end





  describe 'PUT update' do

  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

    context 'there is no source project' do

      let(:project)       { create(:project) }

      let(:project)       { create(:project) }

      let(:fork_project)  { create(:forked_project_with_submodules) }

      let(:fork_project)  { create(:forked_project_with_submodules) }