Merge branch 'security-authorize-boards' into 'master'

[master] Authorize user when listing board resources

Closes gitlabhq#2738

See merge request gitlab/gitlab-ee!721

app/controllers/concerns/boards_responses.rb

@@ -85,3 +85,5 @@ module BoardsResponses
     end
   end
 end
+
+BoardsResponses.prepend(EE::BoardsResponses)

ee/app/controllers/boards/milestones_controller.rb

@@ -2,6 +2,10 @@
 
 module Boards
   class MilestonesController < Boards::ApplicationController
+    include BoardsResponses
+
+    before_action :authorize_read_milestone, only: [:index]
+
     def index
       milestones_finder = Boards::MilestonesFinder.new(board, current_user)
 

ee/app/controllers/boards/users_controller.rb

@@ -7,6 +7,11 @@ module Boards
     # If board parent is a group it enumerates all members of current group,
     # ancestors, and descendants
     # rubocop: disable CodeReuse/ActiveRecord
+
+    include BoardsResponses
+
+    before_action :authorize_read_parent, only: [:index]
+
     def index
       user_ids = user_finder.execute.select(:user_id)
 

ee/app/controllers/concerns/ee/boards_responses.rb

+module EE
+  module BoardsResponses
+    extend ActiveSupport::Concern
+
+    def authorize_read_parent
+      ability = board.group_board? ? :read_group : :read_project
+
+      authorize_action_for!(board.parent, ability)
+    end
+
+    def authorize_read_milestone
+      ability = board.group_board? ? :read_group : :read_milestone
+
+      authorize_action_for!(board.parent, ability)
+    end
+  end
+end

ee/changelogs/unreleased/security-authorize-boards.yml

+---
+title: Authorize users when listing board users and milestones.
+merge_request:
+author:
+type: security

ee/spec/controllers/boards/milestones_controller_spec.rb

@@ -5,23 +5,50 @@ describe Boards::MilestonesController do
   let(:board) { create(:board, project: project) }
   let(:user)  { create(:user) }
 
-  before do
-    create(:milestone, project: project)
+  describe 'GET index' do
+    context 'with authorized user' do
+      before do
+        create(:milestone, project: project)
 
-    project.add_maintainer(user)
-    sign_in(user)
-  end
+        project.add_maintainer(user)
+        sign_in(user)
+      end
 
-  describe 'GET index' do
-    it 'returns a list of all milestones of board parent' do
-      get :index, board_id: board.to_param, format: :json
+      it 'returns a list of all milestones of board parent' do
+        get :index, board_id: board.to_param, format: :json
+
+        parsed_response = JSON.parse(response.body)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response.content_type).to eq('application/json')
+        expect(parsed_response).to all(match_schema('entities/milestone', dir: 'ee'))
+        expect(parsed_response.size).to eq(1)
+      end
+    end
+
+    context 'with unauthorized user' do
+      before do
+        sign_in(user)
+      end
+
+      shared_examples 'unauthorized board milestone listing' do
+        it 'returns a forbidden 403 response' do
+          get :index, board_id: board.to_param, format: :json
+
+          expect(response).to have_gitlab_http_status(403)
+        end
+      end
+
+      context 'with private group board' do
+        let(:group) { create(:group, :private) }
+        let(:board) { create(:board, group: group) }
 
-      parsed_response = JSON.parse(response.body)
+        it_behaves_like 'unauthorized board milestone listing'
+      end
 
-      expect(response).to have_gitlab_http_status(200)
-      expect(response.content_type).to eq('application/json')
-      expect(parsed_response).to all(match_schema('entities/milestone', dir: 'ee'))
-      expect(parsed_response.size).to eq(1)
+      context 'with private project board' do
+        it_behaves_like 'unauthorized board milestone listing'
+      end
     end
   end
 end

ee/spec/controllers/boards/users_controller_spec.rb

 require 'spec_helper'
 
 describe Boards::UsersController do
-  let(:group) { create(:group) }
+  let(:group) { create(:group, :private) }
   let(:board) { create(:board, group: group) }
   let(:guest) { create(:user) }
   let(:user)  { create(:user) }
 
-  before do
-    group.add_maintainer(user)
-    group.add_guest(guest)
+  describe 'GET index' do
+    context 'with authorized user' do
+      before do
+        group.add_maintainer(user)
+        group.add_guest(guest)
 
-    sign_in(user)
-  end
+        sign_in(user)
+      end
 
-  describe 'GET index' do
-    it 'returns a list of all members of board parent' do
-      get :index, namespace_id: group.to_param,
-                  board_id: board.to_param,
-                  format: :json
+      it 'returns a list of all members of board parent' do
+        get :index, namespace_id: group.to_param,
+                    board_id: board.to_param,
+                    format: :json
+
+        parsed_response = JSON.parse(response.body)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response.content_type).to eq 'application/json'
+        expect(parsed_response).to all(match_schema('entities/user'))
+        expect(parsed_response.length).to eq 2
+      end
+    end
+
+    context 'with unauthorized user' do
+      before do
+        sign_in(user)
+      end
+
+      shared_examples 'unauthorized board user listing' do
+        it 'returns a forbidden 403 response' do
+          get :index, board_id: board.to_param, format: :json
+
+          expect(response).to have_gitlab_http_status(403)
+        end
+      end
+
+      context 'with private group board' do
+        it_behaves_like 'unauthorized board user listing'
+      end
 
-      parsed_response = JSON.parse(response.body)
+      context 'with private project board' do
+        let(:project) { create(:project) }
+        let(:board) { create(:board, project: project) }
 
-      expect(response).to have_gitlab_http_status(200)
-      expect(response.content_type).to eq 'application/json'
-      expect(parsed_response).to all(match_schema('entities/user'))
-      expect(parsed_response.length).to eq 2
+        it_behaves_like 'unauthorized board user listing'
+      end
     end
   end
 end