Fix milestone promotion authorization

Promoting milestone was missing an authorization check, guest
users were being able to promote project milestones to group milestones.

3 files changed, 32 insertions, 6 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)
diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb
index 94a030d9d57..9666080092b 100644
--- a/app/helpers/milestones_helper.rb
+++ b/app/helpers/milestones_helper.rb
@@ -2,6 +2,7 @@
 
 module MilestonesHelper
   include EntityDateHelper
+  include Gitlab::Utils::StrongMemoize
 
   def milestones_filter_path(opts = {})
     if @project
@@ -243,4 +244,16 @@ module MilestonesHelper
       dashboard_milestone_path(milestone.safe_title, title: milestone.title)
     end
   end
+
+  def can_admin_project_milestones?
+    strong_memoize(:can_admin_project_milestones) do
+      can?(current_user, :admin_milestone, @project)
+    end
+  end
+
+  def can_admin_group_milestones?
+    strong_memoize(:can_admin_group_milestones) do
+      can?(current_user, :admin_milestone, @project.group)
+    end
+  end
 end
diff --git a/app/views/shared/milestones/_milestone.html.haml b/app/views/shared/milestones/_milestone.html.haml
index 3dd2842be4f..ed7fefba56d 100644
--- a/app/views/shared/milestones/_milestone.html.haml
+++ b/app/views/shared/milestones/_milestone.html.haml
@@ -35,8 +35,8 @@
     .col-sm-2
       .milestone-actions.d-flex.justify-content-sm-start.justify-content-md-end
         - if @project
-          - if can?(current_user, :admin_milestone, milestone.project) and milestone.active?
-            - if @project.group
+          - if can_admin_project_milestones? and milestone.active?
+            - if can_admin_group_milestones?
               %button.js-promote-project-milestone-button.btn.btn-blank.btn-sm.btn-grouped.has-tooltip{ title: _('Promote to Group Milestone'),
                 disabled: true,
                 type: 'button',