BigW Consortium Gitlab

Commit 33d8972b by Douwe Maan

Merge branch 'fix-18717' into 'master'

Ensure that group owner cannot request access to a project of their group ## What does this MR do? It fixes two things: - 91ad995d69e1a0f8991fd896f1d9febc109273fe Ensure that group owner cannot request access to a project of their group - ec3ff061148d556757e7cd486cdc6083d77acf34 Ensure group/project owners can see their members' access_level (see the commit message for details) ## Are there points in the code the reviewer needs to double check? Not really, these are pretty simple fixes. ## Why was this MR needed? Because there was an issue created! ## What are the relevant issue numbers? Fixes #18717. ## Does this MR meet the acceptance criteria? - [x] CHANGELOG is not needed since the bug is only present in a 8.9 RC - [x] Tests - [x] Added for this feature/bug - [x] All builds are passing - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if you do - rebase it please) - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) See merge request !4729
parents b218e82b 7c9571a3
......@@ -6,6 +6,12 @@ module MembersHelper
"#{action}_#{member.type.underscore}".to_sym
end
def default_show_roles(member)
can?(current_user, action_member_permission(:update, member), member) ||
can?(current_user, action_member_permission(:destroy, member), member) ||
can?(current_user, action_member_permission(:admin, member), member.source)
end
def remove_member_message(member, user: nil)
user = current_user if defined?(current_user)
......
- member = source.members.find_by(user_id: current_user.id)
- group_member = source.group.members.find_by(user_id: current_user.id) if source.respond_to?(:group) && source.group
- if member
- if member.request?
= link_to 'Withdraw Access Request', polymorphic_path([:leave, source, :members]),
method: :delete,
data: { confirm: remove_member_message(member) },
- unless group_member
- if member
- if member.request?
= link_to 'Withdraw Access Request', polymorphic_path([:leave, source, :members]),
method: :delete,
data: { confirm: remove_member_message(member) },
class: 'btn access-request-button hidden-xs'
- else
= link_to 'Request Access', polymorphic_path([:request_access, source, :members]),
method: :post,
class: 'btn access-request-button hidden-xs'
- else
= link_to 'Request Access', polymorphic_path([:request_access, source, :members]),
method: :post,
class: 'btn access-request-button hidden-xs'
- default_show_roles = can?(current_user, action_member_permission(:update, member), member) || can?(current_user, action_member_permission(:destroy, member), member)
- show_roles = local_assigns.fetch(:show_roles, default_show_roles)
- show_roles = local_assigns.fetch(:show_roles, default_show_roles(member))
- show_controls = local_assigns.fetch(:show_controls, true)
- user = member.user
......
require 'spec_helper'
feature 'Projects > Members > Group member cannot request access to his group project', feature: true do
let(:user) { create(:user) }
let(:group) { create(:group) }
let(:project) { create(:project, namespace: group) }
background do
end
scenario 'owner does not see the request access button' do
group.add_owner(user)
login_and_visit_project_page(user)
expect(page).not_to have_content 'Request Access'
end
scenario 'master does not see the request access button' do
group.add_master(user)
login_and_visit_project_page(user)
expect(page).not_to have_content 'Request Access'
end
scenario 'developer does not see the request access button' do
group.add_developer(user)
login_and_visit_project_page(user)
expect(page).not_to have_content 'Request Access'
end
scenario 'reporter does not see the request access button' do
group.add_reporter(user)
login_and_visit_project_page(user)
expect(page).not_to have_content 'Request Access'
end
scenario 'guest does not see the request access button' do
group.add_guest(user)
login_and_visit_project_page(user)
expect(page).not_to have_content 'Request Access'
end
def login_and_visit_project_page(user)
login_as(user)
visit namespace_project_path(project.namespace, project)
end
end
require 'spec_helper'
feature 'Projects > Members > Group requester cannot request access to project', feature: true do
let(:user) { create(:user) }
let(:owner) { create(:user) }
let(:group) { create(:group, :public) }
let(:project) { create(:project, :public, namespace: group) }
background do
group.add_owner(owner)
login_as(user)
visit group_path(group)
perform_enqueued_jobs { click_link 'Request Access' }
visit namespace_project_path(project.namespace, project)
end
scenario 'group requester does not see the request access / withdraw access request button' do
expect(page).not_to have_content 'Request Access'
expect(page).not_to have_content 'Withdraw Access Request'
end
end
......@@ -9,6 +9,54 @@ describe MembersHelper do
it { expect(action_member_permission(:admin, group_member)).to eq :admin_group_member }
end
describe '#default_show_roles' do
let(:user) { double }
let(:member) { build(:project_member) }
before do
allow(helper).to receive(:current_user).and_return(user)
allow(helper).to receive(:can?).with(user, :update_project_member, member).and_return(false)
allow(helper).to receive(:can?).with(user, :destroy_project_member, member).and_return(false)
allow(helper).to receive(:can?).with(user, :admin_project_member, member.source).and_return(false)
end
context 'when the current cannot update, destroy or admin the passed member' do
it 'returns false' do
expect(helper.default_show_roles(member)).to be_falsy
end
end
context 'when the current can update the passed member' do
before do
allow(helper).to receive(:can?).with(user, :update_project_member, member).and_return(true)
end
it 'returns true' do
expect(helper.default_show_roles(member)).to be_truthy
end
end
context 'when the current can destroy the passed member' do
before do
allow(helper).to receive(:can?).with(user, :destroy_project_member, member).and_return(true)
end
it 'returns true' do
expect(helper.default_show_roles(member)).to be_truthy
end
end
context 'when the current can admin the passed member source' do
before do
allow(helper).to receive(:can?).with(user, :admin_project_member, member.source).and_return(true)
end
it 'returns true' do
expect(helper.default_show_roles(member)).to be_truthy
end
end
end
describe '#remove_member_message' do
let(:requester) { build(:user) }
let(:project) { create(:project) }
......
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