Add authorization to issues board related controllers

app/controllers/projects/board_issues_controller.rb

 class Projects::BoardIssuesController < Projects::ApplicationController
   respond_to :json
 
+  before_action :authorize_read_issue!, only: [:index]
+  before_action :authorize_update_issue!, only: [:update]
+
   rescue_from ActiveRecord::RecordNotFound, with: :record_not_found
 
   def index
@@ -27,6 +30,14 @@ class Projects::BoardIssuesController < Projects::ApplicationController
 
   private
 
+  def authorize_read_issue!
+    return render_403 unless can?(current_user, :read_issue, project)
+  end
+
+  def authorize_update_issue!
+    return render_403 unless can?(current_user, :update_issue, project)
+  end
+
   def filter_params
     params.merge(id: params[:list_id])
   end

app/controllers/projects/board_lists_controller.rb

 class Projects::BoardListsController < Projects::ApplicationController
   respond_to :json
 
+  before_action :authorize_admin_list!
+
   rescue_from ActiveRecord::RecordNotFound, with: :record_not_found
 
   def create
@@ -45,6 +47,10 @@ class Projects::BoardListsController < Projects::ApplicationController
 
   private
 
+  def authorize_admin_list!
+    return render_403 unless can?(current_user, :admin_list, project)
+  end
+
   def list_params
     params.require(:list).permit(:label_id)
   end

app/controllers/projects/boards_controller.rb

 class Projects::BoardsController < Projects::ApplicationController
+  before_action :authorize_read_board!, only: [:show]
+
   def show
     board = Boards::CreateService.new(project, current_user).execute
 
@@ -7,4 +9,15 @@ class Projects::BoardsController < Projects::ApplicationController
       format.json { render json: board.lists.as_json(only: [:id, :list_type, :position], methods: [:title], include: { label: { only: [:id, :title, :color] } }) }
     end
   end
+
+  private
+
+  def authorize_read_board!
+    unless can?(current_user, :read_board, project)
+      respond_to do |format|
+        format.html { return access_denied! }
+        format.json { return render_403 }
+      end
+    end
+  end
 end

app/models/ability.rb

@@ -228,6 +228,7 @@ class Ability
         :read_project,
         :read_wiki,
         :read_issue,
+        :read_board,
         :read_label,
         :read_milestone,
         :read_project_snippet,
@@ -249,6 +250,7 @@ class Ability
         :update_issue,
         :admin_issue,
         :admin_label,
+        :admin_list,
         :read_commit_status,
         :read_build,
         :read_container_image,

app/services/boards/issues/move_service.rb

@@ -4,7 +4,6 @@ module Boards
       def execute
         return false unless issue.present?
         return false unless valid_move?
-        return false unless user.can?(:update_issue, issue)
 
         update_service.execute(issue)
         reopen_service.execute(issue) if moving_from.done?

spec/controllers/projects/board_issues_controller_spec.rb

@@ -12,7 +12,6 @@ describe Projects::BoardIssuesController do
 
   before do
     project.team << [user, :master]
-    sign_in(user)
   end
 
   describe 'GET #index' do
@@ -23,7 +22,7 @@ describe Projects::BoardIssuesController do
         create(:labeled_issue, project: project, labels: [development])
         create(:labeled_issue, project: project, labels: [development], assignee: johndoe)
 
-        list_issues list_id: list2
+        list_issues user: user, list_id: list2
 
         parsed_response = JSON.parse(response.body)
 
@@ -34,13 +33,26 @@ describe Projects::BoardIssuesController do
 
     context 'with invalid list id' do
       it 'returns a not found 404 response' do
-        list_issues list_id: 999
+        list_issues user: user, list_id: 999
 
         expect(response).to have_http_status(404)
       end
     end
 
-    def list_issues(list_id:)
+    context 'with unauthorized user' do
+      it 'returns a successful 403 response' do
+        allow(Ability.abilities).to receive(:allowed?).with(user, :read_project, project).and_return(true)
+        allow(Ability.abilities).to receive(:allowed?).with(user, :read_issue, project).and_return(false)
+
+        list_issues user: user, list_id: list2
+
+        expect(response).to have_http_status(403)
+      end
+    end
+
+    def list_issues(user:, list_id:)
+      sign_in(user)
+
       get :index, namespace_id: project.namespace.to_param,
                   project_id: project.to_param,
                   list_id: list_id.to_param
@@ -52,13 +64,13 @@ describe Projects::BoardIssuesController do
 
     context 'with valid params' do
       it 'returns a successful 200 response' do
-        move issue: issue, from: list1.id, to: list2.id
+        move user: user, issue: issue, from: list1.id, to: list2.id
 
         expect(response).to have_http_status(200)
       end
 
       it 'moves issue to the desired list' do
-        move issue: issue, from: list1.id, to: list2.id
+        move user: user, issue: issue, from: list1.id, to: list2.id
 
         expect(issue.reload.labels).to contain_exactly(development)
       end
@@ -66,19 +78,35 @@ describe Projects::BoardIssuesController do
 
     context 'with invalid params' do
       it 'returns a unprocessable entity 422 response for invalid lists' do
-        move issue: issue, from: nil, to: nil
+        move user: user, issue: issue, from: nil, to: nil
 
         expect(response).to have_http_status(422)
       end
 
       it 'returns a not found 404 response for invalid issue id' do
-        move issue: 999, from: list1.id, to: list2.id
+        move user: user, issue: 999, from: list1.id, to: list2.id
 
         expect(response).to have_http_status(404)
       end
     end
 
-    def move(issue:, from:, to:)
+    context 'with unauthorized user' do
+      let(:guest) { create(:user) }
+
+      before do
+        project.team << [guest, :guest]
+      end
+
+      it 'returns a successful 403 response' do
+        move user: guest, issue: issue, from: list1.id, to: list2.id
+
+        expect(response).to have_http_status(403)
+      end
+    end
+
+    def move(user:, issue:, from:, to:)
+      sign_in(user)
+
       patch :update, namespace_id: project.namespace.to_param,
                      project_id: project.to_param,
                      id: issue.to_param,

spec/controllers/projects/board_lists_controller_spec.rb

@@ -4,24 +4,25 @@ describe Projects::BoardListsController do
   let(:project) { create(:project_with_board) }
   let(:board)   { project.board }
   let(:user)    { create(:user) }
+  let(:guest)   { create(:user) }
 
   before do
     project.team << [user, :master]
-    sign_in(user)
+    project.team << [guest, :guest]
   end
 
   describe 'POST #create' do
-    context 'with valid params' do
     let(:label) { create(:label, project: project, name: 'Development') }
 
+    context 'with valid params' do
       it 'returns a successful 200 response' do
-        create_board_list label_id: label.id
+        create_board_list user: user, label_id: label.id
 
         expect(response).to have_http_status(200)
       end
 
       it 'returns the created list' do
-        create_board_list label_id: label.id
+        create_board_list user: user, label_id: label.id
 
         expect(response).to match_response_schema('list')
       end
@@ -29,7 +30,7 @@ describe Projects::BoardListsController do
 
     context 'with invalid params' do
       it 'returns an error' do
-        create_board_list label_id: nil
+        create_board_list user: user, label_id: nil
 
         parsed_response = JSON.parse(response.body)
 
@@ -38,7 +39,19 @@ describe Projects::BoardListsController do
       end
     end
 
-    def create_board_list(label_id:)
+    context 'with unauthorized user' do
+      let(:label) { create(:label, project: project, name: 'Development') }
+
+      it 'returns a successful 403 response' do
+        create_board_list user: guest, label_id: label.id
+
+        expect(response).to have_http_status(403)
+      end
+    end
+
+    def create_board_list(user:, label_id:)
+      sign_in(user)
+
       post :create, namespace_id: project.namespace.to_param,
                     project_id: project.to_param,
                     list: { label_id: label_id },
@@ -52,13 +65,13 @@ describe Projects::BoardListsController do
 
     context 'with valid position' do
       it 'returns a successful 200 response' do
-        move list: planning, position: 1
+        move user: user, list: planning, position: 1
 
         expect(response).to have_http_status(200)
       end
 
       it 'moves the list to the desired position' do
-        move list: planning, position: 1
+        move user: user, list: planning, position: 1
 
         expect(planning.reload.position).to eq 1
       end
@@ -66,7 +79,7 @@ describe Projects::BoardListsController do
 
     context 'with invalid position' do
       it 'returns a unprocessable entity 422 response' do
-        move list: planning, position: 6
+        move user: user, list: planning, position: 6
 
         expect(response).to have_http_status(422)
       end
@@ -74,13 +87,23 @@ describe Projects::BoardListsController do
 
     context 'with invalid list id' do
       it 'returns a not found 404 response' do
-        move list: 999, position: 1
+        move user: user, list: 999, position: 1
 
         expect(response).to have_http_status(404)
       end
     end
 
-    def move(list:, position:)
+    context 'with unauthorized user' do
+      it 'returns a successful 403 response' do
+        move user: guest, list: planning, position: 6
+
+        expect(response).to have_http_status(403)
+      end
+    end
+
+    def move(user:, list:, position:)
+      sign_in(user)
+
       patch :update, namespace_id: project.namespace.to_param,
                      project_id: project.to_param,
                      id: list.to_param,
@@ -90,29 +113,39 @@ describe Projects::BoardListsController do
   end
 
   describe 'DELETE #destroy' do
-    context 'with valid list id' do
     let!(:planning) { create(:list, board: board, position: 0) }
 
+    context 'with valid list id' do
       it 'returns a successful 200 response' do
-        remove_board_list list: planning
+        remove_board_list user: user, list: planning
 
         expect(response).to have_http_status(200)
       end
 
       it 'removes list from board' do
-        expect { remove_board_list list: planning }.to change(board.lists, :size).by(-1)
+        expect { remove_board_list user: user, list: planning }.to change(board.lists, :size).by(-1)
       end
     end
 
     context 'with invalid list id' do
       it 'returns a not found 404 response' do
-        remove_board_list list: 999
+        remove_board_list user: user, list: 999
 
         expect(response).to have_http_status(404)
       end
     end
 
-    def remove_board_list(list:)
+    context 'with unauthorized user' do
+      it 'returns a successful 403 response' do
+        remove_board_list user: guest, list: planning
+
+        expect(response).to have_http_status(403)
+      end
+    end
+
+    def remove_board_list(user:, list:)
+      sign_in(user)
+
       delete :destroy, namespace_id: project.namespace.to_param,
                        project_id: project.to_param,
                        id: list.to_param,
@@ -123,13 +156,13 @@ describe Projects::BoardListsController do
   describe 'POST #generate' do
     context 'when board lists is empty' do
       it 'returns a successful 200 response' do
-        generate_default_board_lists
+        generate_default_board_lists user: user
 
         expect(response).to have_http_status(200)
       end
 
       it 'returns the defaults lists' do
-        generate_default_board_lists
+        generate_default_board_lists user: user
 
         expect(response).to match_response_schema('list', array: true)
       end
@@ -139,13 +172,23 @@ describe Projects::BoardListsController do
       it 'returns a unprocessable entity 422 response' do
         create(:list, board: board)
 
-        generate_default_board_lists
+        generate_default_board_lists user: user
 
         expect(response).to have_http_status(422)
       end
     end
 
-    def generate_default_board_lists
+    context 'with unauthorized user' do
+      it 'returns a successful 403 response' do
+        generate_default_board_lists user: guest
+
+        expect(response).to have_http_status(403)
+      end
+    end
+
+    def generate_default_board_lists(user:)
+      sign_in(user)
+
       post :generate, namespace_id: project.namespace.to_param,
                       project_id: project.to_param,
                       format: :json

spec/controllers/projects/boards_controller_spec.rb

@@ -12,22 +12,33 @@ describe Projects::BoardsController do
   describe 'GET #show' do
     context 'when project does not have a board' do
       it 'creates a new board' do
-        expect { get :show, namespace_id: project.namespace.to_param, project_id: project.to_param }.to change(Board, :count).by(1)
+        expect { read_board }.to change(Board, :count).by(1)
       end
     end
 
     context 'when format is HTML' do
       it 'renders HTML template' do
-        get :show, namespace_id: project.namespace.to_param, project_id: project.to_param
+        read_board
 
         expect(response).to render_template :show
         expect(response.content_type).to eq 'text/html'
       end
+
+      context 'with unauthorized user' do
+        it 'returns a successful 404 response' do
+          allow(Ability.abilities).to receive(:allowed?).with(user, :read_project, project).and_return(true)
+          allow(Ability.abilities).to receive(:allowed?).with(user, :read_board, project).and_return(false)
+
+          read_board
+
+          expect(response).to have_http_status(404)
+        end
+      end
     end
 
     context 'when format is JSON' do
       it 'returns a successful 200 response' do
-        get :show, namespace_id: project.namespace.to_param, project_id: project.to_param, format: :json
+        read_board format: :json
 
         expect(response).to have_http_status(200)
         expect(response.content_type).to eq 'application/json'
@@ -39,13 +50,30 @@ describe Projects::BoardsController do
         create(:list, board: board)
         create(:done_list, board: board)
 
-        get :show, namespace_id: project.namespace.to_param, project_id: project.to_param, format: :json
+        read_board format: :json
 
         parsed_response = JSON.parse(response.body)
 
         expect(response).to match_response_schema('list', array: true)
         expect(parsed_response.length).to eq 3
       end
+
+      context 'with unauthorized user' do
+        it 'returns a successful 403 response' do
+          allow(Ability.abilities).to receive(:allowed?).with(user, :read_project, project).and_return(true)
+          allow(Ability.abilities).to receive(:allowed?).with(user, :read_board, project).and_return(false)
+
+          read_board format: :json
+
+          expect(response).to have_http_status(403)
+        end
+      end
+    end
+
+    def read_board(format: :html)
+      get :show, namespace_id: project.namespace.to_param,
+                 project_id: project.to_param,
+                 format: format
     end
   end
 end