From d145f09cd675fa46a6cc20fac8304f02d2d14656 Mon Sep 17 00:00:00 2001
From: Marin Jankovski <marin@gitlab.com>
Date: Mon, 30 Jun 2014 11:38:03 +0200
Subject: [PATCH] Correct authorization for group milestones.

---
 .../groups/milestones_controller.rb           |  6 ++++
 app/models/group_milestone.rb                 | 35 -------------------
 app/views/groups/milestones/index.html.haml   | 13 +++----
 app/views/groups/milestones/show.html.haml    |  9 ++---
 4 files changed, 18 insertions(+), 45 deletions(-)

diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb
index c4b87308e8b..281b2f0c90a 100644
--- a/app/controllers/groups/milestones_controller.rb
+++ b/app/controllers/groups/milestones_controller.rb
@@ -1,6 +1,8 @@
 class Groups::MilestonesController < ApplicationController
   layout 'group'
 
+  before_filter :authorize_group_milestone!, only: :update
+
   def index
     project_milestones = Milestone.where(project_id: group.projects)
     @group_milestones = Milestones::GroupService.new(project_milestones).execute
@@ -47,4 +49,8 @@ class Groups::MilestonesController < ApplicationController
   def status(state)
     @group_milestones.map{ |milestone| next if milestone.state != state; milestone }.compact
   end
+
+  def authorize_group_milestone!
+    return render_404 unless can?(current_user, :manage_group, group)
+  end
 end
diff --git a/app/models/group_milestone.rb b/app/models/group_milestone.rb
index 45222f5329e..8296a0aa71b 100644
--- a/app/models/group_milestone.rb
+++ b/app/models/group_milestone.rb
@@ -76,39 +76,4 @@ class GroupMilestone
   def participants
     milestones.map{ |milestone| milestone.participants.uniq }.reject(&:empty?).flatten
   end
-
-  def filter_by(filter, entity)
-    if entity
-      milestones = self.milestones.sort_by(&:project_id)
-      entities = {}
-      milestones.each do |project_milestone|
-        next unless project_milestone.send(entity).any?
-        project_name = project_milestone.project.name
-        entities_by_state = state_filter(filter, project_milestone.send(entity))
-        entities.store(project_name, entities_by_state)
-      end
-      entities
-    else
-      {}
-    end
-  end
-
-  def state_filter(filter, entities)
-    if entities.present?
-      sorted_entities = entities.sort_by(&:position)
-      entities_by_state =  case filter
-                           when 'active'; sorted_entities.group_by(&:state)['opened']
-                           when 'closed'; sorted_entities.group_by(&:state)['closed']
-                           else sorted_entities
-                           end
-      if entities_by_state.blank?
-        []
-      else
-        entities_by_state
-      end
-    else
-      []
-    end
-  end
-
 end
diff --git a/app/views/groups/milestones/index.html.haml b/app/views/groups/milestones/index.html.haml
index e3dd1ae5ae0..671f81ba06f 100644
--- a/app/views/groups/milestones/index.html.haml
+++ b/app/views/groups/milestones/index.html.haml
@@ -24,18 +24,19 @@
           - @group_milestones.each do |milestone|
             %li{class: "milestone milestone-#{milestone.closed? ? 'closed' : 'open'}", id: dom_id(milestone.milestones.first) }
               .pull-right
-                - if milestone.closed?
-                  = link_to 'Reopen Milestone', group_milestone_path(@group, milestone.safe_title, milestone: {state_event: :activate }), method: :put, class: "btn btn-small btn-grouped"
-                - else
-                  = link_to 'Close Milestone', group_milestone_path(@group, milestone.safe_title, milestone: {state_event: :close }), method: :put, class: "btn btn-small btn-remove"
+                - if can?(current_user, :manage_group, @group)
+                  - if milestone.closed?
+                    = link_to 'Reopen Milestone', group_milestone_path(@group, milestone.safe_title, milestone: {state_event: :activate }), method: :put, class: "btn btn-small btn-grouped"
+                  - else
+                    = link_to 'Close Milestone', group_milestone_path(@group, milestone.safe_title, milestone: {state_event: :close }), method: :put, class: "btn btn-small btn-remove"
               %h4
                 = link_to_gfm truncate(milestone.title, length: 100), group_milestone_path(@group, milestone.safe_title)
               %div
                 %div
-                  = link_to group_milestone_path(@group, milestone.safe_title) do
+                  = link_to group_milestone_path(@group, milestone.safe_title, anchor: 'tab-issues') do
                     = pluralize milestone.issue_count, 'Issue'
                   &nbsp;
-                  = link_to group_milestone_path(@group, milestone.safe_title) do
+                  = link_to group_milestone_path(@group, milestone.safe_title, anchor: 'tab-merge-requests') do
                     = pluralize milestone.merge_requests_count, 'Merge Request'
                   &nbsp;
                   %span.light #{milestone.percent_complete}% complete
diff --git a/app/views/groups/milestones/show.html.haml b/app/views/groups/milestones/show.html.haml
index d7bf6ae3c5c..7e66318e968 100644
--- a/app/views/groups/milestones/show.html.haml
+++ b/app/views/groups/milestones/show.html.haml
@@ -1,10 +1,11 @@
 %h3.page-title
   Milestone #{@group_milestone.title}
   .pull-right
-    - if @group_milestone.active?
-      = link_to 'Close Milestone', group_milestone_path(@group, @group_milestone.safe_title, milestone: {state_event: :close }), method: :put, class: "btn btn-small btn-remove"
-    - else
-      = link_to 'Reopen Milestone', group_milestone_path(@group, @group_milestone.safe_title, milestone: {state_event: :activate }), method: :put, class: "btn btn-small btn-grouped"
+    - if can?(current_user, :manage_group, @group)
+      - if @group_milestone.active?
+        = link_to 'Close Milestone', group_milestone_path(@group, @group_milestone.safe_title, milestone: {state_event: :close }), method: :put, class: "btn btn-small btn-remove"
+      - else
+        = link_to 'Reopen Milestone', group_milestone_path(@group, @group_milestone.safe_title, milestone: {state_event: :activate }), method: :put, class: "btn btn-small btn-grouped"
 
 - if (@group_milestone.total_items_count == @group_milestone.closed_items_count) && @group_milestone.active?
   .alert.alert-success
-- 
GitLab