Merge branch 'security-issue_51301' into 'master'

[master] Resolve: Promoting a milestone is missing an authorization check

See merge request gitlab/gitlabhq!2598

1 files changed, 17 insertions, 4 deletions

diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb
index 20998c97730..8e68014a30d 100644
--- a/app/controllers/projects/milestones_controller.rb
+++ b/app/controllers/projects/milestones_controller.rb
@@ -11,7 +11,10 @@ class Projects::MilestonesController < Projects::ApplicationController
   before_action :authorize_read_milestone!
 
   # Allow admin milestone
-  before_action :authorize_admin_milestone!, except: [:index, :show, :merge_requests, :participants, :labels, :promote]
+  before_action :authorize_admin_milestone!, except: [:index, :show, :merge_requests, :participants, :labels]
+
+  # Allow to promote milestone
+  before_action :authorize_promote_milestone!, only: :promote
 
   respond_to :html
 
@@ -78,7 +81,7 @@ class Projects::MilestonesController < Projects::ApplicationController
 
   def promote
     promoted_milestone = Milestones::PromoteService.new(project, current_user).execute(milestone)
-    flash[:notice] = flash_notice_for(promoted_milestone, project.group)
+    flash[:notice] = flash_notice_for(promoted_milestone, project_group)
 
     respond_to do |format|
       format.html do
@@ -109,6 +112,12 @@ class Projects::MilestonesController < Projects::ApplicationController
 
   protected
 
+  def project_group
+    strong_memoize(:project_group) do
+      project.group
+    end
+  end
+
   def milestones
     strong_memoize(:milestones) do
       MilestonesFinder.new(search_params).execute
@@ -125,13 +134,17 @@ class Projects::MilestonesController < Projects::ApplicationController
     return render_404 unless can?(current_user, :admin_milestone, @project)
   end
 
+  def authorize_promote_milestone!
+    return render_404 unless can?(current_user, :admin_milestone, project_group)
+  end
+
   def milestone_params
     params.require(:milestone).permit(:title, :description, :start_date, :due_date, :state_event)
   end
 
   def search_params
-    if request.format.json? && @project.group && can?(current_user, :read_group, @project.group)
-      groups = @project.group.self_and_ancestors_ids
+    if request.format.json? && project_group && can?(current_user, :read_group, project_group)
+      groups = project_group.self_and_ancestors_ids
     end
 
     params.permit(:state).merge(project_ids: @project.id, group_ids: groups)