Commit e7bf6d07 authored by Yorick Peterse's avatar Yorick Peterse

Merge branch 'security-shared-project-private-group' into 'master'

Sharing a public project with a private group makes the group page publicly accessible

See merge request gitlab/gitlabhq!2896
parents b50ad884 3a321c80
...@@ -53,7 +53,6 @@ class GroupPolicy < BasePolicy ...@@ -53,7 +53,6 @@ class GroupPolicy < BasePolicy
rule { admin }.enable :read_group rule { admin }.enable :read_group
rule { has_projects }.policy do rule { has_projects }.policy do
enable :read_group
enable :read_label enable :read_label
end end
......
---
title: Fixed ability to see private groups by users not belonging to given group
merge_request:
author:
type: security
...@@ -27,7 +27,7 @@ describe 'Private Group access' do ...@@ -27,7 +27,7 @@ describe 'Private Group access' do
it { is_expected.to be_allowed_for(:developer).of(group) } it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for(:reporter).of(group) } it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for(:guest).of(group) } it { is_expected.to be_allowed_for(:guest).of(group) }
it { is_expected.to be_allowed_for(project_guest) } it { is_expected.to be_denied_for(project_guest) }
it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:user) }
it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:external) }
it { is_expected.to be_denied_for(:visitor) } it { is_expected.to be_denied_for(:visitor) }
...@@ -42,7 +42,7 @@ describe 'Private Group access' do ...@@ -42,7 +42,7 @@ describe 'Private Group access' do
it { is_expected.to be_allowed_for(:developer).of(group) } it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for(:reporter).of(group) } it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for(:guest).of(group) } it { is_expected.to be_allowed_for(:guest).of(group) }
it { is_expected.to be_allowed_for(project_guest) } it { is_expected.to be_denied_for(project_guest) }
it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:user) }
it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:external) }
it { is_expected.to be_denied_for(:visitor) } it { is_expected.to be_denied_for(:visitor) }
...@@ -58,7 +58,7 @@ describe 'Private Group access' do ...@@ -58,7 +58,7 @@ describe 'Private Group access' do
it { is_expected.to be_allowed_for(:developer).of(group) } it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for(:reporter).of(group) } it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for(:guest).of(group) } it { is_expected.to be_allowed_for(:guest).of(group) }
it { is_expected.to be_allowed_for(project_guest) } it { is_expected.to be_denied_for(project_guest) }
it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:user) }
it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:external) }
it { is_expected.to be_denied_for(:visitor) } it { is_expected.to be_denied_for(:visitor) }
...@@ -73,7 +73,7 @@ describe 'Private Group access' do ...@@ -73,7 +73,7 @@ describe 'Private Group access' do
it { is_expected.to be_allowed_for(:developer).of(group) } it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for(:reporter).of(group) } it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for(:guest).of(group) } it { is_expected.to be_allowed_for(:guest).of(group) }
it { is_expected.to be_allowed_for(project_guest) } it { is_expected.to be_denied_for(project_guest) }
it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:user) }
it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:external) }
it { is_expected.to be_denied_for(:visitor) } it { is_expected.to be_denied_for(:visitor) }
...@@ -93,4 +93,28 @@ describe 'Private Group access' do ...@@ -93,4 +93,28 @@ describe 'Private Group access' do
it { is_expected.to be_denied_for(:visitor) } it { is_expected.to be_denied_for(:visitor) }
it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:external) }
end end
describe 'GET /groups/:path for shared projects' do
let(:project) { create(:project, :public) }
before do
Projects::GroupLinks::CreateService.new(
project,
create(:user),
link_group_access: ProjectGroupLink::DEVELOPER
).execute(group)
end
subject { group_path(group) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:owner).of(group) }
it { is_expected.to be_allowed_for(:maintainer).of(group) }
it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for(:guest).of(group) }
it { is_expected.to be_denied_for(project_guest) }
it { is_expected.to be_denied_for(:user) }
it { is_expected.to be_denied_for(:external) }
it { is_expected.to be_denied_for(:visitor) }
end
end end
...@@ -74,6 +74,38 @@ describe GroupPolicy do ...@@ -74,6 +74,38 @@ describe GroupPolicy do
end end
end end
context 'with no user and public project' do
let(:project) { create(:project, :public) }
let(:user) { create(:user) }
let(:current_user) { nil }
before do
Projects::GroupLinks::CreateService.new(
project,
user,
link_group_access: ProjectGroupLink::DEVELOPER
).execute(group)
end
it { expect_disallowed(:read_group) }
end
context 'with foreign user and public project' do
let(:project) { create(:project, :public) }
let(:user) { create(:user) }
let(:current_user) { create(:user) }
before do
Projects::GroupLinks::CreateService.new(
project,
user,
link_group_access: ProjectGroupLink::DEVELOPER
).execute(group)
end
it { expect_disallowed(:read_group) }
end
context 'has projects' do context 'has projects' do
let(:current_user) { create(:user) } let(:current_user) { create(:user) }
let(:project) { create(:project, namespace: group) } let(:project) { create(:project, namespace: group) }
...@@ -82,17 +114,13 @@ describe GroupPolicy do ...@@ -82,17 +114,13 @@ describe GroupPolicy do
project.add_developer(current_user) project.add_developer(current_user)
end end
it do it { expect_allowed(:read_label) }
expect_allowed(:read_group, :read_label)
end
context 'in subgroups', :nested_groups do context 'in subgroups', :nested_groups do
let(:subgroup) { create(:group, :private, parent: group) } let(:subgroup) { create(:group, :private, parent: group) }
let(:project) { create(:project, namespace: subgroup) } let(:project) { create(:project, namespace: subgroup) }
it do it { expect_allowed(:read_label) }
expect_allowed(:read_group, :read_label)
end
end end
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