diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2019-03-04 18:36:21 +0000 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2019-03-04 18:36:21 +0000 |
commit | 68df57373f1aa5c4dc012bf8a9d6526548d59bc3 (patch) | |
tree | 99505d315542e5ae03b977a3f1bffc31d105ceb1 | |
parent | bcd4e1f55de9482bda8e86cdb638316f1ed88c30 (diff) | |
parent | 97b595d41d10218db1bb9111be2790ad58a9a0f3 (diff) | |
download | gitlab-ce-68df57373f1aa5c4dc012bf8a9d6526548d59bc3.tar.gz |
Merge branch 'security-shared-project-private-group-11-7' into '11-7-stable'
Sharing a public project with a private group makes the group page publicly accessible
See merge request gitlab/gitlabhq!2985
-rw-r--r-- | app/policies/group_policy.rb | 1 | ||||
-rw-r--r-- | changelogs/unreleased/security-shared-project-private-group.yml | 5 | ||||
-rw-r--r-- | spec/features/security/group/private_access_spec.rb | 32 | ||||
-rw-r--r-- | spec/policies/group_policy_spec.rb | 40 |
4 files changed, 67 insertions, 11 deletions
diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index c25766a5af8..55d8da78dc8 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -53,7 +53,6 @@ class GroupPolicy < BasePolicy rule { admin }.enable :read_group rule { has_projects }.policy do - enable :read_group enable :read_label end diff --git a/changelogs/unreleased/security-shared-project-private-group.yml b/changelogs/unreleased/security-shared-project-private-group.yml new file mode 100644 index 00000000000..3b21daa5491 --- /dev/null +++ b/changelogs/unreleased/security-shared-project-private-group.yml @@ -0,0 +1,5 @@ +--- +title: Fixed ability to see private groups by users not belonging to given group +merge_request: +author: +type: security diff --git a/spec/features/security/group/private_access_spec.rb b/spec/features/security/group/private_access_spec.rb index 4705cd12d23..3238e07fe15 100644 --- a/spec/features/security/group/private_access_spec.rb +++ b/spec/features/security/group/private_access_spec.rb @@ -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(:reporter).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(:external) } it { is_expected.to be_denied_for(:visitor) } @@ -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(:reporter).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(:external) } it { is_expected.to be_denied_for(:visitor) } @@ -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(:reporter).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(:external) } it { is_expected.to be_denied_for(:visitor) } @@ -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(:reporter).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(:external) } it { is_expected.to be_denied_for(:visitor) } @@ -93,4 +93,28 @@ describe 'Private Group access' do it { is_expected.to be_denied_for(:visitor) } it { is_expected.to be_denied_for(:external) } 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 diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index be1804c5ce0..4c31ff30fc6 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -74,6 +74,38 @@ describe GroupPolicy do 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 let(:current_user) { create(:user) } let(:project) { create(:project, namespace: group) } @@ -82,17 +114,13 @@ describe GroupPolicy do project.add_developer(current_user) end - it do - expect_allowed(:read_group, :read_label) - end + it { expect_allowed(:read_label) } context 'in subgroups', :nested_groups do let(:subgroup) { create(:group, :private, parent: group) } let(:project) { create(:project, namespace: subgroup) } - it do - expect_allowed(:read_group, :read_label) - end + it { expect_allowed(:read_label) } end end |