Commit fcecee43 authored by Mario de la Ossa's avatar Mario de la Ossa

Add a BoardPolicy object

DRY up things a little by having a BoardPolicy that knows how to
delegate to Project or Group depending on parent type instead of having
`group#group_board?` calls everywhere
parent 0e362362
...@@ -34,15 +34,11 @@ module BoardsResponses ...@@ -34,15 +34,11 @@ module BoardsResponses
end end
def authorize_read_list def authorize_read_list
ability = board.group_board? ? :read_group : :read_list authorize_action_for!(board, :read_list)
authorize_action_for!(board.parent, ability)
end end
def authorize_read_issue def authorize_read_issue
ability = board.group_board? ? :read_group : :read_issue authorize_action_for!(board, :read_issue)
authorize_action_for!(board.parent, ability)
end end
def authorize_update_issue def authorize_update_issue
...@@ -57,7 +53,7 @@ module BoardsResponses ...@@ -57,7 +53,7 @@ module BoardsResponses
end end
def authorize_admin_list def authorize_admin_list
authorize_action_for!(board.parent, :admin_list) authorize_action_for!(board, :admin_list)
end end
def authorize_action_for!(resource, ability) def authorize_action_for!(resource, ability)
......
# frozen_string_literal: true
class BoardPolicy < BasePolicy
delegate { @subject.parent }
condition(:is_group_board) { @subject.group_board? }
rule { is_group_board ? can?(:read_group) : can?(:read_project) }.enable :read_parent
rule { is_group_board & can?(:read_group) }.policy do
enable :read_milestone
enable :read_issue
end
end
...@@ -5,15 +5,11 @@ module EE ...@@ -5,15 +5,11 @@ module EE
extend ActiveSupport::Concern extend ActiveSupport::Concern
def authorize_read_parent def authorize_read_parent
ability = board.group_board? ? :read_group : :read_project authorize_action_for!(board, :read_parent)
authorize_action_for!(board.parent, ability)
end end
def authorize_read_milestone def authorize_read_milestone
ability = board.group_board? ? :read_group : :read_milestone authorize_action_for!(board, :read_milestone)
authorize_action_for!(board.parent, ability)
end end
end end
end end
...@@ -3,7 +3,7 @@ require 'spec_helper' ...@@ -3,7 +3,7 @@ require 'spec_helper'
describe Boards::IssuesController do describe Boards::IssuesController do
include ExternalAuthorizationServiceHelpers include ExternalAuthorizationServiceHelpers
let(:group) { create(:group) } let(:group) { create(:group, :private) }
let(:project_1) { create(:project, namespace: group) } let(:project_1) { create(:project, namespace: group) }
let(:project_2) { create(:project, namespace: group) } let(:project_2) { create(:project, namespace: group) }
let(:board) { create(:board, group: group) } let(:board) { create(:board, group: group) }
...@@ -79,8 +79,7 @@ describe Boards::IssuesController do ...@@ -79,8 +79,7 @@ describe Boards::IssuesController do
context 'with unauthorized user' do context 'with unauthorized user' do
before do before do
allow(Ability).to receive(:allowed?).and_call_original group.group_member(user).destroy
allow(Ability).to receive(:allowed?).with(user, :read_group, group).and_return(false)
end end
it 'returns a forbidden 403 response' do it 'returns a forbidden 403 response' do
......
require 'spec_helper' require 'spec_helper'
describe Boards::ListsController do describe Boards::ListsController do
let(:group) { create(:group) } let(:group) { create(:group, :private) }
let(:board) { create(:board, group: group) } let(:board) { create(:board, group: group) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:guest) { create(:user) } let(:guest) { create(:user) }
...@@ -32,7 +32,7 @@ describe Boards::ListsController do ...@@ -32,7 +32,7 @@ describe Boards::ListsController do
context 'with unauthorized user' do context 'with unauthorized user' do
before do before do
allow(Ability).to receive(:allowed?).with(user, :read_group, group).and_return(false) group.group_member(user).destroy
end end
it 'returns a forbidden 403 response' do it 'returns a forbidden 403 response' do
......
require 'spec_helper' require 'spec_helper'
describe Boards::MilestonesController do describe Boards::MilestonesController do
let(:project) { create(:project) } let(:project) { create(:project, :private) }
let(:board) { create(:board, project: project) } let(:board) { create(:board, project: project) }
let(:user) { create(:user) } let(:user) { create(:user) }
...@@ -14,15 +14,34 @@ describe Boards::MilestonesController do ...@@ -14,15 +14,34 @@ describe Boards::MilestonesController do
sign_in(user) sign_in(user)
end end
it 'returns a list of all milestones of board parent' do shared_examples 'authorized board milestone listing' do
get :index, params: { board_id: board.to_param }, format: :json it 'returns a list of all milestones of board parent' do
get :index, params: { board_id: board.to_param }, format: :json
expect(response).to have_gitlab_http_status(200)
parsed_response = JSON.parse(response.body)
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 private group board' do
let(:group) { create(:group, :private) }
let(:board) { create(:board, group: group) }
parsed_response = JSON.parse(response.body) before do
create(:milestone, group: group)
group.add_maintainer(user)
end
expect(response).to have_gitlab_http_status(200) it_behaves_like 'authorized board milestone listing'
expect(response.content_type).to eq('application/json') end
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 'authorized board milestone listing'
end end
end end
......
require 'spec_helper' require 'spec_helper'
describe Boards::IssuesController do describe Boards::IssuesController do
let(:project) { create(:project) } let(:project) { create(:project, :private) }
let(:board) { create(:board, project: project) } let(:board) { create(:board, project: project) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:guest) { create(:user) } let(:guest) { create(:user) }
...@@ -127,14 +127,10 @@ describe Boards::IssuesController do ...@@ -127,14 +127,10 @@ describe Boards::IssuesController do
end end
context 'with unauthorized user' do context 'with unauthorized user' do
before do let(:unauth_user) { create(:user) }
allow(Ability).to receive(:allowed?).and_call_original
allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true)
allow(Ability).to receive(:allowed?).with(user, :read_issue, project).and_return(false)
end
it 'returns a forbidden 403 response' do it 'returns a forbidden 403 response' do
list_issues user: user, board: board, list: list2 list_issues user: unauth_user, board: board, list: list2
expect(response).to have_gitlab_http_status(403) expect(response).to have_gitlab_http_status(403)
end end
......
...@@ -31,13 +31,10 @@ describe Boards::ListsController do ...@@ -31,13 +31,10 @@ describe Boards::ListsController do
end end
context 'with unauthorized user' do context 'with unauthorized user' do
before do let(:unauth_user) { create(:user) }
allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true)
allow(Ability).to receive(:allowed?).with(user, :read_list, project).and_return(false)
end
it 'returns a forbidden 403 response' do it 'returns a forbidden 403 response' do
read_board_list user: user, board: board read_board_list user: unauth_user, board: board
expect(response).to have_gitlab_http_status(403) expect(response).to have_gitlab_http_status(403)
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment