Commit ca50492a authored by Abdul Wadood's avatar Abdul Wadood Committed by GitLab Release Tools Bot

Allow inviting only groups with subset of allowed domains to groups

Merge branch 'security-validate-allowed-email-domains-on-group-invite-14-10' into '14-10-stable-ee'

See merge request gitlab-org/security/gitlab!2512

Changelog: security
parent cf7ef970
...@@ -1979,7 +1979,6 @@ Layout/LineLength: ...@@ -1979,7 +1979,6 @@ Layout/LineLength:
- 'ee/spec/features/groups/iterations/user_edits_iteration_spec.rb' - 'ee/spec/features/groups/iterations/user_edits_iteration_spec.rb'
- 'ee/spec/features/groups/iterations/user_views_iteration_cadence_spec.rb' - 'ee/spec/features/groups/iterations/user_views_iteration_cadence_spec.rb'
- 'ee/spec/features/groups/iterations/user_views_iteration_spec.rb' - 'ee/spec/features/groups/iterations/user_views_iteration_spec.rb'
- 'ee/spec/features/groups/members/manage_groups_spec.rb'
- 'ee/spec/features/groups/members/manage_members_spec.rb' - 'ee/spec/features/groups/members/manage_members_spec.rb'
- 'ee/spec/features/groups/members/override_ldap_memberships_spec.rb' - 'ee/spec/features/groups/members/override_ldap_memberships_spec.rb'
- 'ee/spec/features/groups/saml_providers_spec.rb' - 'ee/spec/features/groups/saml_providers_spec.rb'
......
...@@ -41,3 +41,5 @@ class GroupGroupLink < ApplicationRecord ...@@ -41,3 +41,5 @@ class GroupGroupLink < ApplicationRecord
Gitlab::Access.human_access(self.group_access) Gitlab::Access.human_access(self.group_access)
end end
end end
GroupGroupLink.prepend_mod_with('GroupGroupLink')
...@@ -640,6 +640,7 @@ To restrict group access by IP address: ...@@ -640,6 +640,7 @@ To restrict group access by IP address:
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/7297) in GitLab 12.2. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/7297) in GitLab 12.2.
> - Support for specifying multiple email domains [added](https://gitlab.com/gitlab-org/gitlab/-/issues/33143) in GitLab 13.1. > - Support for specifying multiple email domains [added](https://gitlab.com/gitlab-org/gitlab/-/issues/33143) in GitLab 13.1.
> - Support for restricting access to projects in the group [added](https://gitlab.com/gitlab-org/gitlab/-/issues/14004) in GitLab 14.1.2. > - Support for restricting access to projects in the group [added](https://gitlab.com/gitlab-org/gitlab/-/issues/14004) in GitLab 14.1.2.
> - Support for restricting group memberships to groups with a subset of the allowed email domains [added](https://gitlab.com/gitlab-org/gitlab/-/issues/354791) in GitLab 15.0.1
You can prevent users with email addresses in specific domains from being added to a group and its projects. You can prevent users with email addresses in specific domains from being added to a group and its projects.
...@@ -662,6 +663,8 @@ The most popular public email domains cannot be restricted, such as: ...@@ -662,6 +663,8 @@ The most popular public email domains cannot be restricted, such as:
- `hotmail.com`, `hotmail.co.uk`, `hotmail.fr` - `hotmail.com`, `hotmail.co.uk`, `hotmail.fr`
- `msn.com`, `live.com`, `outlook.com` - `msn.com`, `live.com`, `outlook.com`
When you share a group, both the source and target namespaces must allow the domains of the members' email addresses.
## Group file templates **(PREMIUM)** ## Group file templates **(PREMIUM)**
Use group file templates to share a set of templates for common file Use group file templates to share a set of templates for common file
......
# frozen_string_literal: true
module EE
module GroupGroupLink
extend ActiveSupport::Concern
prepended do
scope :in_shared_group, -> (shared_groups) { where(shared_group: shared_groups) }
scope :not_in_shared_with_group, -> (shared_with_groups) { where.not(shared_with_group: shared_with_groups) }
validate :group_with_allowed_email_domains
end
private
def group_with_allowed_email_domains
return unless shared_group && shared_with_group
shared_group_domains = shared_group.root_ancestor_allowed_email_domains.pluck(:domain).to_set
return if shared_group_domains.empty?
shared_with_group_domains = shared_with_group.root_ancestor_allowed_email_domains.pluck(:domain).to_set
if shared_with_group_domains.empty? || !shared_with_group_domains.subset?(shared_group_domains)
errors.add(:group_id, _("Invited group allowed email domains must contain a subset of the allowed"\
" email domains of the root ancestor group. Go to the group's 'Settings &gt; General' page"\
" and check 'Restrict membership by email domain'."))
end
end
end
end
...@@ -6,6 +6,8 @@ module EE ...@@ -6,6 +6,8 @@ module EE
prepended do prepended do
before_destroy :delete_related_access_levels before_destroy :delete_related_access_levels
validate :group_with_allowed_email_domains
end end
def delete_related_access_levels def delete_related_access_levels
...@@ -21,5 +23,21 @@ module EE ...@@ -21,5 +23,21 @@ module EE
# For protected environments # For protected environments
project.protected_environments.revoke_group(group) project.protected_environments.revoke_group(group)
end end
def group_with_allowed_email_domains
return unless shared_from&.group && shared_with_group
root_ancestor_group_domains = shared_from.root_ancestor.allowed_email_domains.pluck(:domain).to_set
return if root_ancestor_group_domains.empty?
shared_with_group_domains = shared_with_group.root_ancestor_allowed_email_domains.pluck(:domain).to_set
if shared_with_group_domains.empty? || !shared_with_group_domains.subset?(root_ancestor_group_domains)
errors.add(:group_id,
_("Invited group allowed email domains must contain a subset of the allowed email domains"\
" of the root ancestor group. Go to the group's 'Settings &gt; General' page"\
" and check 'Restrict membership by email domain'."))
end
end
end end
end end
...@@ -9,22 +9,38 @@ RSpec.describe 'Groups > Members > Manage groups', :js, :saas do ...@@ -9,22 +9,38 @@ RSpec.describe 'Groups > Members > Manage groups', :js, :saas do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:user2) { create(:user) } let_it_be(:user2) { create(:user) }
let_it_be(:group) { create(:group) }
let_it_be(:group_to_add) { create(:group) } let(:group) { create(:group) }
let(:group_to_add) { create(:group) }
let(:premium_plan) { create(:premium_plan) } let(:premium_plan) { create(:premium_plan) }
let(:ultimate_plan) { create(:premium_plan) } let(:ultimate_plan) { create(:ultimate_plan) }
shared_examples 'adds group without validation error' do
it_behaves_like "doesn't trigger an overage modal when adding a group with a given role", 'Maintainer'
end
shared_examples 'inviting group fails with allowed email domain error' do
specify do
group.add_owner(user)
group_to_add.add_owner(user)
visit group_group_members_path(group)
add_group(group_to_add.name, 'Maintainer')
expect(page).to have_content('Invited group allowed email domains must contain a subset of the'\
' allowed email domains of the root ancestor group')
end
end
shared_examples "doesn't trigger an overage modal when adding a group with a given role" do |role| shared_examples "doesn't trigger an overage modal when adding a group with a given role" do |role|
it do specify do
group.add_owner(user) group.add_owner(user)
group_to_add.add_owner(user) group_to_add.add_owner(user)
visit group_group_members_path(group) visit group_group_members_path(group)
add_group(group_to_add.name, role) add_group(group_to_add.name, role)
wait_for_requests
expect(page).not_to have_button 'Continue' expect(page).not_to have_button 'Continue'
page.refresh page.refresh
...@@ -39,7 +55,7 @@ RSpec.describe 'Groups > Members > Manage groups', :js, :saas do ...@@ -39,7 +55,7 @@ RSpec.describe 'Groups > Members > Manage groups', :js, :saas do
end end
shared_examples "triggers an overage modal when adding a group as Reporter" do shared_examples "triggers an overage modal when adding a group as Reporter" do
it do specify do
add_group_with_one_extra_user add_group_with_one_extra_user
click_button 'Continue' click_button 'Continue'
...@@ -98,6 +114,88 @@ RSpec.describe 'Groups > Members > Manage groups', :js, :saas do ...@@ -98,6 +114,88 @@ RSpec.describe 'Groups > Members > Manage groups', :js, :saas do
it_behaves_like "doesn't trigger an overage modal when adding a group with a given role", 'Guest' it_behaves_like "doesn't trigger an overage modal when adding a group with a given role", 'Guest'
end end
describe 'inviting group with restricted email domain' do
shared_examples 'restricted membership by email domain' do
context 'shared group has membership restricted by allowed email domains' do
before do
create(:allowed_email_domain, group: group.root_ancestor, domain: 'gitlab.com')
end
context 'shared with group with a subset of allowed email domains' do
before do
create(:allowed_email_domain, group: group_to_add.root_ancestor, domain: 'gitlab.com')
end
it_behaves_like 'adds group without validation error'
end
context 'shared with group containing domains outside the shared group allowed email domains' do
before do
create(:allowed_email_domain, group: group_to_add.root_ancestor, domain: 'example.com')
end
it_behaves_like 'inviting group fails with allowed email domain error'
end
context 'shared with group does not have membership restricted by allowed domains' do
it_behaves_like 'inviting group fails with allowed email domain error'
end
end
context 'shared group does not have membership restricted by allowed domains' do
context 'shared with group has membership restricted by allowed email domains' do
before do
create(:allowed_email_domain, group: group_to_add.root_ancestor, domain: 'example.com')
end
it_behaves_like 'adds group without validation error'
end
context 'shared with group does not have membership restricted by allowed domains' do
it_behaves_like 'adds group without validation error'
end
end
end
context 'shared group is the root ancestor' do
let(:group) { create(:group) }
let(:group_to_add) { create(:group) }
it_behaves_like 'restricted membership by email domain'
end
context 'shared group is a subgroup' do
let(:parent_group) { create(:group) }
let(:group) { create(:group, parent: parent_group) }
let(:group_to_add) { create(:group) }
before do
parent_group.add_owner(user)
end
it_behaves_like 'restricted membership by email domain'
end
context 'shared with group is a subgroup' do
let(:group) { create(:group) }
let(:group_to_add) { create(:group, parent: create(:group)) }
it_behaves_like 'restricted membership by email domain'
end
context 'shared and shared with group are subgroups' do
let(:parent_group) { create(:group) }
let(:group) { create(:group, parent: parent_group) }
let(:group_to_add) { create(:group, parent: create(:group)) }
before do
parent_group.add_owner(user)
end
it_behaves_like 'restricted membership by email domain'
end
end
def add_group(name, role, expires_at: nil) def add_group(name, role, expires_at: nil)
click_on 'Invite a group' click_on 'Invite a group'
...@@ -107,6 +205,7 @@ RSpec.describe 'Groups > Members > Manage groups', :js, :saas do ...@@ -107,6 +205,7 @@ RSpec.describe 'Groups > Members > Manage groups', :js, :saas do
choose_options(role, expires_at) choose_options(role, expires_at)
click_button 'Invite' click_button 'Invite'
wait_for_requests
end end
def add_group_with_one_extra_user def add_group_with_one_extra_user
...@@ -117,8 +216,7 @@ RSpec.describe 'Groups > Members > Manage groups', :js, :saas do ...@@ -117,8 +216,7 @@ RSpec.describe 'Groups > Members > Manage groups', :js, :saas do
visit group_group_members_path(group) visit group_group_members_path(group)
add_group(group_to_add.name, 'Reporter') add_group(group_to_add.name, 'Reporter')
wait_for_requests expect(page).to have_content("Your subscription includes 1 seat. If you continue, the #{group.name} group will"\
" have 2 seats in use and will be billed for the overage. Learn more.")
expect(page).to have_content("Your subscription includes 1 seat. If you continue, the #{group.name} group will have 2 seats in use and will be billed for the overage. Learn more.")
end end
end end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'Projects > Members > Manage groups' do
include Spec::Support::Helpers::Features::MembersHelpers
include Spec::Support::Helpers::Features::InviteMembersModalHelper
shared_examples 'adds group without validation error' do
specify do
group.add_owner(user)
group_to_add.add_owner(user)
sign_in(user)
role = 'Maintainer'
visit project_project_members_path(project)
add_group(group_to_add.name, role)
page.refresh
click_groups_tab
page.within(first_row) do
expect(page).to have_content(group_to_add.name)
expect(page).to have_content(role)
end
end
end
shared_examples 'inviting group fails with allowed email domain error' do
specify do
group.add_owner(user)
group_to_add.add_owner(user)
sign_in(user)
visit project_project_members_path(project)
add_group(group_to_add.name, 'Maintainer')
error_msg = 'Invited group allowed email domains must contain a subset of the allowed email domains'\
' of the root ancestor group'
expect(page).to have_content(error_msg)
end
end
describe 'inviting group with restricted email domain', :js do
shared_examples 'restricted membership by email domain' do
let(:user) { create(:user) }
let(:project) { create(:project, group: group) }
context 'shared project group has membership restricted by allowed email domains' do
before do
create(:allowed_email_domain, group: group.root_ancestor, domain: 'gitlab.com')
end
context 'shared with group with a subset of allowed email domains' do
before do
create(:allowed_email_domain, group: group_to_add.root_ancestor, domain: 'gitlab.com')
end
it_behaves_like 'adds group without validation error'
end
context 'shared with group containing domains outside the shared group allowed email domains' do
before do
create(:allowed_email_domain, group: group_to_add.root_ancestor, domain: 'example.com')
end
it_behaves_like 'inviting group fails with allowed email domain error'
end
context 'shared with group does not have membership restricted by allowed domains' do
it_behaves_like 'inviting group fails with allowed email domain error'
end
end
context 'shared project group does not have membership restricted by allowed domains' do
context 'shared with group has membership restricted by allowed email domains' do
before do
create(:allowed_email_domain, group: group_to_add.root_ancestor, domain: 'example.com')
end
it_behaves_like 'adds group without validation error'
end
context 'shared with group does not have membership restricted by allowed domains' do
it_behaves_like 'adds group without validation error'
end
end
end
context 'shared project group is the root ancestor' do
let(:group) { create(:group) }
let(:group_to_add) { create(:group) }
it_behaves_like 'restricted membership by email domain'
end
context 'shared project group is a subgroup' do
let(:group) { create(:group, parent: create(:group)) }
let(:group_to_add) { create(:group) }
it_behaves_like 'restricted membership by email domain'
end
context 'shared with group is a subgroup' do
let(:group) { create(:group) }
let(:group_to_add) { create(:group, parent: create(:group)) }
it_behaves_like 'restricted membership by email domain'
end
context 'shared project group and shared with group are subgroups' do
let(:group) { create(:group, parent: create(:group)) }
let(:group_to_add) { create(:group, parent: create(:group)) }
it_behaves_like 'restricted membership by email domain'
end
end
def add_group(name, role, expires_at: nil)
click_on 'Invite a group'
click_on 'Select a group'
wait_for_requests
click_button name
choose_options(role, expires_at)
click_button 'Invite'
wait_for_requests
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe GroupGroupLink do
let_it_be(:group) { create(:group) }
let_it_be(:group_group_link) { create(:group_group_link, shared_group: group, shared_with_group: create(:group)) }
describe 'scopes' do
describe '.in_shared_group' do
it 'provides correct link records' do
create(:group_group_link)
expect(described_class.in_shared_group(group)).to match_array([group_group_link])
end
end
describe '.not_in_shared_with_group' do
it 'provides correct link records' do
not_shared_with_group = create(:group)
create(:group_group_link, shared_with_group: not_shared_with_group)
expect(described_class.not_in_shared_with_group(not_shared_with_group)).to match_array([group_group_link])
end
end
end
describe 'validations' do
describe '#group_with_allowed_email_domains' do
shared_examples 'restricted membership by email domain' do
subject do
build(:group_group_link, shared_group: shared_group, shared_with_group: shared_with_group)
end
context 'shared group has membership restricted by allowed email domains' do
before do
create(:allowed_email_domain, group: shared_group.root_ancestor, domain: 'gitlab.com')
create(:allowed_email_domain, group: shared_group.root_ancestor, domain: 'gitlab.cn')
end
context 'shared with group with a subset of allowed email domains' do
before do
create(:allowed_email_domain, group: shared_with_group.root_ancestor, domain: 'gitlab.com')
end
it { is_expected.to be_valid }
end
context 'shared with group containing domains outside the shared group allowed email domains' do
before do
create(:allowed_email_domain, group: shared_with_group.root_ancestor, domain: 'example.com')
end
it { is_expected.to be_invalid }
end
context 'shared with group does not have membership restricted by allowed domains' do
it { is_expected.to be_invalid }
end
end
context 'shared group does not have membership restricted by allowed domains' do
context 'shared with group has membership restricted by allowed email domains' do
before do
create(:allowed_email_domain, group: shared_with_group.root_ancestor, domain: 'example.com')
end
it { is_expected.to be_valid }
end
context 'shared with group does not have membership restricted by allowed domains' do
it { is_expected.to be_valid }
end
end
end
context 'shared group is the root ancestor' do
let_it_be(:shared_group) { create(:group) }
let_it_be(:shared_with_group) { create(:group) }
it_behaves_like 'restricted membership by email domain'
end
context 'shared group is a subgroup' do
let_it_be(:shared_group) { create(:group, parent: create(:group)) }
let_it_be(:shared_with_group) { create(:group) }
it_behaves_like 'restricted membership by email domain'
end
context 'shared with group is a subgroup' do
let_it_be(:shared_group) { create(:group) }
let_it_be(:shared_with_group) { create(:group, parent: create(:group)) }
it_behaves_like 'restricted membership by email domain'
end
context 'shared and shared with group are subgroups' do
let_it_be(:shared_group) { create(:group, parent: create(:group)) }
let_it_be(:shared_with_group) { create(:group, parent: create(:group)) }
it_behaves_like 'restricted membership by email domain'
end
end
end
end
...@@ -66,4 +66,83 @@ RSpec.describe ProjectGroupLink do ...@@ -66,4 +66,83 @@ RSpec.describe ProjectGroupLink do
end end
end end
end end
describe '#group_with_allowed_email_domains' do
let(:shared_project) { create(:project, group: shared_project_group) }
subject do
build(:project_group_link, project: shared_project, group: shared_with_group)
end
shared_examples 'restricted membership by email domain' do
context 'shared project group has membership restricted by allowed email domains' do
before do
create(:allowed_email_domain, group: shared_project_group.root_ancestor, domain: 'gitlab.com')
create(:allowed_email_domain, group: shared_project_group.root_ancestor, domain: 'gitlab.cn')
end
context 'shared with group with a subset of allowed email domains' do
before do
create(:allowed_email_domain, group: shared_with_group.root_ancestor, domain: 'gitlab.com')
end
it { is_expected.to be_valid }
end
context 'shared with group containing domains outside the shared group allowed email domains' do
before do
create(:allowed_email_domain, group: shared_with_group.root_ancestor, domain: 'example.com')
end
it { is_expected.to be_invalid }
end
context 'shared with group does not have membership restricted by allowed domains' do
it { is_expected.to be_invalid }
end
end
context 'shared project group does not have membership restricted by allowed domains' do
context 'shared with group has membership restricted by allowed email domains' do
before do
create(:allowed_email_domain, group: shared_with_group.root_ancestor, domain: 'example.com')
end
it { is_expected.to be_valid }
end
context 'shared with group does not have membership restricted by allowed domains' do
it { is_expected.to be_valid }
end
end
end
context 'shared project group is the root ancestor' do
let_it_be(:shared_project_group) { create(:group) }
let_it_be(:shared_with_group) { create(:group) }
it_behaves_like 'restricted membership by email domain'
end
context 'shared project group is a subgroup' do
let_it_be(:shared_project_group) { create(:group, parent: create(:group)) }
let_it_be(:shared_with_group) { create(:group) }
it_behaves_like 'restricted membership by email domain'
end
context 'shared with group is a subgroup' do
let_it_be(:shared_project_group) { create(:group) }
let_it_be(:shared_with_group) { create(:group, parent: create(:group)) }
it_behaves_like 'restricted membership by email domain'
end
context 'shared project group and shared with group are subgroups' do
let_it_be(:shared_project_group) { create(:group, parent: create(:group)) }
let_it_be(:shared_with_group) { create(:group, parent: create(:group)) }
it_behaves_like 'restricted membership by email domain'
end
end
end end
...@@ -20839,6 +20839,9 @@ msgstr "" ...@@ -20839,6 +20839,9 @@ msgstr ""
msgid "Invited" msgid "Invited"
msgstr "" msgstr ""
msgid "Invited group allowed email domains must contain a subset of the allowed email domains of the root ancestor group. Go to the group's 'Settings &gt; General' page and check 'Restrict membership by email domain'."
msgstr ""
msgid "Invocations" msgid "Invocations"
msgstr "" msgstr ""
......
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