Merge branch '10972-be-allow-restricting-group-members-by-a-domain-whitelist' into 'master'

Allow restricting group members by a domain whitelist

Closes #10972

See merge request gitlab-org/gitlab-ee!14800

app/views/groups/settings/_permissions.html.haml

@@ -18,6 +18,7 @@
           %span.descr.text-muted= share_with_group_lock_help_text(@group)
 
     = render_if_exists 'groups/settings/ip_restriction', f: f, group: @group
+    = render_if_exists 'groups/settings/allowed_email_domain', f: f, group: @group
     = render 'groups/settings/lfs', f: f
     = render 'groups/settings/project_creation_level', f: f, group: @group
     = render 'groups/settings/subgroup_creation_level', f: f, group: @group

changelogs/unreleased/10972-be-allow-restricting-group-members-by-a-domain-whitelist-ce.yml

+---
+title: Add new table to store email domain per group
+merge_request: 31071
+author:
+type: added

db/migrate/20190723153247_create_allowed_email_domains_for_groups.rb

+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class CreateAllowedEmailDomainsForGroups < ActiveRecord::Migration[5.2]
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def change
+    create_table :allowed_email_domains do |t|
+      t.timestamps_with_timezone null: false
+      t.references :group, references: :namespace,
+        column: :group_id,
+        type: :integer,
+        null: false,
+        index: true
+      t.foreign_key :namespaces, column: :group_id, on_delete: :cascade
+      t.string :domain, null: false, limit: 255
+    end
+  end
+end

db/schema.rb

@@ -26,6 +26,14 @@ ActiveRecord::Schema.define(version: 2019_08_15_093949) do
     t.integer "cached_markdown_version"
   end
 
+  create_table "allowed_email_domains", force: :cascade do |t|
+    t.datetime_with_timezone "created_at", null: false
+    t.datetime_with_timezone "updated_at", null: false
+    t.integer "group_id", null: false
+    t.string "domain", limit: 255, null: false
+    t.index ["group_id"], name: "index_allowed_email_domains_on_group_id"
+  end
+
   create_table "analytics_cycle_analytics_group_stages", force: :cascade do |t|
     t.datetime_with_timezone "created_at", null: false
     t.datetime_with_timezone "updated_at", null: false
@@ -3670,6 +3678,7 @@ ActiveRecord::Schema.define(version: 2019_08_15_093949) do
     t.index ["type"], name: "index_web_hooks_on_type"
   end
 
+  add_foreign_key "allowed_email_domains", "namespaces", column: "group_id", on_delete: :cascade
   add_foreign_key "analytics_cycle_analytics_group_stages", "labels", column: "end_event_label_id", on_delete: :cascade
   add_foreign_key "analytics_cycle_analytics_group_stages", "labels", column: "start_event_label_id", on_delete: :cascade
   add_foreign_key "analytics_cycle_analytics_group_stages", "namespaces", column: "group_id", on_delete: :cascade

doc/user/group/index.md

@@ -350,6 +350,38 @@ Restriction currently applies to UI, API access is not restricted.
 To avoid accidental lock-out, admins and group owners are are able to access
 the group regardless of the IP restriction.
 
+#### Allowed domain restriction **(PREMIUM ONLY)**
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/issues/7297) in
+[GitLab Premium](https://about.gitlab.com/pricing/) 12.2.
+
+You can restrict access to groups and their underlying projects by
+allowing only users with email addresses in particular domains to be added to the group.
+
+Add email domains you want to whitelist and users with emails from different 
+domains won't be allowed to be added to this group.
+
+Some domains cannot be restricted. These are the most popular public email domains, such as:
+
+- `gmail.com`
+- `yahoo.com`
+- `hotmail.com`
+- `aol.com`
+- `msn.com`
+- `hotmail.co.uk`
+- `hotmail.fr`
+- `live.com`
+- `outlook.com`
+- `icloud.com`
+
+To enable this feature:
+
+1. Navigate to the group's **Settings > General** page.
+1. Expand the **Permissions, LFS, 2FA** section, and enter domain name into **Restrict membership by email** field.
+1. Click **Save changes**.
+
+This will enable the domain-checking for all new users added to the group from this moment on.
+
 #### Group file templates **(PREMIUM)**
 
 Group file templates allow you to share a set of templates for common file

ee/app/controllers/ee/groups_controller.rb

@@ -7,6 +7,7 @@ module EE
 
     prepended do
       before_action :set_ip_restriction, only: [:edit]
+      before_action :set_allowed_domain, only: [:edit]
     end
 
     override :render_show_html
@@ -33,6 +34,7 @@ module EE
         params_ee << :file_template_project_id if current_group&.feature_available?(:custom_file_templates_for_namespace)
         params_ee << :custom_project_templates_group_id if current_group&.group_project_template_available?
         params_ee << { ip_restriction_attributes: [:id, :range] } if current_group&.feature_available?(:group_ip_restriction)
+        params_ee << { allowed_email_domain_attributes: [:id, :domain] } if current_group&.feature_available?(:group_allowed_email_domains)
       end
     end
 
@@ -64,5 +66,11 @@ module EE
 
       group.build_ip_restriction
     end
+
+    def set_allowed_domain
+      return if group.allowed_email_domain.present?
+
+      group.build_allowed_email_domain
+    end
   end
 end

ee/app/models/allowed_email_domain.rb

+# frozen_string_literal: true
+
+class AllowedEmailDomain < ApplicationRecord
+  RESERVED_DOMAINS = [
+    'gmail.com',
+    'yahoo.com',
+    'hotmail.com',
+    'aol.com',
+    'msn.com',
+    'hotmail.co.uk',
+    'hotmail.fr',
+    'live.com',
+    'outlook.com',
+    'icloud.com'
+  ].freeze
+
+  validates :group_id, presence: true
+  validates :domain, presence: true
+  validate :allow_root_group_only
+  validates :domain, exclusion: { in: RESERVED_DOMAINS,
+    message: _('The domain you entered is not allowed.') }
+  validates :domain, format: { with: /\w*\./,
+    message: _('The domain you entered is misformatted.') }
+
+  belongs_to :group, class_name: 'Group', foreign_key: :group_id
+
+  def allow_root_group_only
+    if group&.parent_id
+      errors.add(:base, _('Allowed email domain restriction only permitted for top-level groups'))
+    end
+  end
+
+  def email_matches_domain?(email)
+    email.end_with?(email_domain)
+  end
+
+  def email_domain
+    @email_domain ||= "@#{domain}"
+  end
+end

ee/app/models/ee/group.rb

@@ -31,6 +31,9 @@ module EE
       has_one :dependency_proxy_setting, class_name: 'DependencyProxy::GroupSetting'
       has_many :dependency_proxy_blobs, class_name: 'DependencyProxy::Blob'
 
+      has_one :allowed_email_domain
+      accepts_nested_attributes_for :allowed_email_domain, allow_destroy: true, reject_if: :all_blank
+
       # We cannot simply set `has_many :audit_events, as: :entity, dependent: :destroy`
       # here since Group inherits from Namespace, the entity_type would be set to `Namespace`.
       has_many :audit_events, -> { where(entity_type: ::Group.name) }, foreign_key: 'entity_id'
@@ -180,6 +183,12 @@ module EE
       root_ancestor.ip_restriction
     end
 
+    def root_ancestor_allowed_email_domain
+      return allowed_email_domain if parent_id.nil?
+
+      root_ancestor.allowed_email_domain
+    end
+
     # Overrides a method defined in `::EE::Namespace`
     override :checked_file_template_project
     def checked_file_template_project(*args, &blk)

ee/app/models/ee/group_member.rb

@@ -8,6 +8,7 @@ module EE
       extend ::Gitlab::Utils::Override
 
       validate :sso_enforcement, if: :group
+      validate :group_domain_limitations, if: :group_has_domain_limitations?
 
       scope :with_ldap_dn, -> { joins(user: :identities).where("identities.provider LIKE ?", 'ldap%') }
       scope :with_identity_provider, ->(provider) do
@@ -22,5 +23,35 @@ module EE
         exists?(group: group, user: user)
       end
     end
+
+    def group_has_domain_limitations?
+      group.feature_available?(:group_allowed_email_domains) && group.root_ancestor_allowed_email_domain.present?
+    end
+
+    def group_domain_limitations
+      user ? validate_users_email : validate_invitation_email
+    end
+
+    def validate_users_email
+      return if group_allowed_email_domain.email_matches_domain?(user.email)
+
+      errors.add(:user, email_no_match_email_domain(user.email))
+    end
+
+    def validate_invitation_email
+      return if group_allowed_email_domain.email_matches_domain?(invite_email)
+
+      errors.add(:invite_email, email_no_match_email_domain(invite_email))
+    end
+
+    private
+
+    def email_no_match_email_domain(email)
+      _("email '%{email}' does not match the allowed domain of '%{email_domain}'" % { email: email, email_domain: group_allowed_email_domain.domain })
+    end
+
+    def group_allowed_email_domain
+      group.root_ancestor_allowed_email_domain
+    end
   end
 end

ee/app/models/license.rb

@@ -89,6 +89,7 @@ class License < ApplicationRecord
     required_ci_templates
     project_aliases
     cycle_analytics_for_groups
+    group_allowed_email_domains
   ]
   EEP_FEATURES.freeze
 

ee/app/services/ee/groups/update_service.rb

@@ -12,7 +12,7 @@ module EE
           return false if group.errors.present?
         end
 
-        handle_ip_restriction_deletion
+        handle_deletion
 
         remove_insight_if_insight_project_absent
 
@@ -79,8 +79,13 @@ module EE
         end
       end
 
+      def handle_deletion
+        handle_ip_restriction_deletion
+        handle_allowed_domain_deletion
+      end
+
       def handle_ip_restriction_deletion
-        return unless ip_restriction_editable?
+        return unless associations_editable?
 
         return unless group.ip_restriction.present?
 
@@ -93,12 +98,26 @@ module EE
         end
       end
 
-      def ip_restriction_editable?
+      def associations_editable?
         return false if group.parent_id.present?
 
         true
       end
 
+      def handle_allowed_domain_deletion
+        return unless associations_editable?
+        return unless group.allowed_email_domain.present?
+        return unless allowed_domain_params
+
+        if allowed_domain_params[:domain]&.blank?
+          allowed_domain_params[:_destroy] = 1
+        end
+      end
+
+      def allowed_domain_params
+        @allowed_domain_params ||= params[:allowed_email_domain_attributes]
+      end
+
       def log_audit_event
         EE::Audit::GroupChangesAuditor.new(current_user, group).execute
       end

ee/app/views/groups/settings/_allowed_email_domain.html.haml

+- return unless group.feature_available?(:group_allowed_email_domains)
+
+- read_only = group.parent_id.present?
+
+%h5= _('Restrict membership by email')
+
+= f.fields_for :allowed_email_domain do |allowed_email_domain_form|
+  .form-group
+    - if read_only
+      = allowed_email_domain_form.text_field :domain, value: group.root_ancestor_allowed_email_domain&.domain, class: 'form-control', disabled: true, placeholder: _('No value set by top-level parent group.')
+      .form-text.text-muted
+        = _('Email domain is not editable in subgroups. Value inherited from top-level parent group.')
+    - else
+      = allowed_email_domain_form.text_field :domain, class: 'form-control', placeholder: _('Enter domain')
+    .form-text.text-muted
+      - read_more_link = link_to(_('Read more'), help_page_path('user/group/index', anchor: 'allowed-domain-restriction-premium-only'))
+      = _('Only users with an email address in this domain can be added to the group.<br>Example: <code>gitlab.com</code>. Some common domains are not allowed. %{read_more_link}.').html_safe % { read_more_link: read_more_link }

ee/changelogs/unreleased/10972-be-allow-restricting-group-members-by-a-domain-whitelist.yml

+---
+title: Allow adding email domain to group to limit users to ones with email in this
+  particular domain
+merge_request: 14800
+author:
+type: added

ee/spec/factories/allowed_email_domains.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :allowed_email_domain, class: AllowedEmailDomain do
+    domain { 'gitlab.com' }
+    group
+  end
+end

ee/spec/models/allowed_email_domain_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe AllowedEmailDomain do
+  describe 'relations' do
+    it { is_expected.to belong_to(:group) }
+  end
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:domain) }
+    it { is_expected.to validate_presence_of(:group_id) }
+
+    describe '#valid domain' do
+      subject { described_class.new(group: create(:group), domain: domain) }
+
+      context 'valid domain' do
+        let(:domain) { 'gitlab.com' }
+
+        it 'succeeds' do
+          expect(subject.valid?).to be_truthy
+        end
+      end
+
+      context 'invalid domain' do
+        let(:domain) { 'gitlab' }
+
+        it 'fails' do
+          expect(subject.valid?).to be_falsey
+          expect(subject.errors[:domain]).to include('The domain you entered is misformatted.')
+        end
+      end
+
+      context 'domain from excluded list' do
+        let(:domain) { 'hotmail.co.uk' }
+
+        it 'fails' do
+          expect(subject.valid?).to be_falsey
+          expect(subject.errors[:domain]).to include('The domain you entered is not allowed.')
+        end
+      end
+    end
+
+    describe '#allow_root_group_only' do
+      subject { described_class.new(group: group, domain: 'gitlab.com' ) }
+
+      context 'top-level group' do
+        let(:group) { create(:group) }
+
+        it 'succeeds' do
+          expect(subject.valid?).to be_truthy
+        end
+      end
+
+      context 'subgroup' do
+        let(:group) { create(:group, :nested) }
+
+        it 'fails' do
+          expect(subject.valid?).to be_falsey
+          expect(subject.errors[:base]).to include('Allowed email domain restriction only permitted for top-level groups')
+        end
+      end
+    end
+  end
+
+  describe '#email_matches_domain?' do
+    subject { described_class.new(group: create(:group), domain: 'gitlab.com') }
+
+    context 'with matching domain' do
+      it 'returns true' do
+        expect(subject.email_matches_domain?('test@gitlab.com')).to eq(true)
+      end
+    end
+
+    context 'with not matching domain' do
+      it 'returns false' do
+        expect(subject.email_matches_domain?('test@gitlab.com.uk')).to eq(false)
+      end
+    end
+  end
+
+  describe '#email_domain' do
+    subject { described_class.new(group: create(:group), domain: 'gitlab.com') }
+
+    it 'returns formatted domain' do
+      expect(subject.email_domain).to eq('@gitlab.com')
+    end
+  end
+end

ee/spec/models/group_member_spec.rb

@@ -5,4 +5,58 @@ describe GroupMember do
   it { is_expected.to include_module(EE::GroupMember) }
 
   it_behaves_like 'member validations'
+
+  describe 'validations' do
+    describe 'group domain limitations' do
+      let(:group) { create(:group) }
+      let(:user) { create(:user, email: 'test@gitlab.com')}
+      let(:user_2) { create(:user, email: 'test@gmail.com')}
+
+      before do
+        create(:allowed_email_domain, group: group)
+      end
+
+      context 'when group has email domain feature switched on' do
+        before do
+          stub_licensed_features(group_allowed_email_domains: true)
+        end
+
+        it 'users email must match allowed domain email' do
+          expect(build(:group_member, group: group, user: user_2)).to be_invalid
+          expect(build(:group_member, group: group, user: user)).to be_valid
+        end
+
+        it 'invited email must match allowed domain email' do
+          expect(build(:group_member, group: group, user: nil, invite_email: 'user@gmail.com')).to be_invalid
+          expect(build(:group_member, group: group, user: nil, invite_email: 'user@gitlab.com')).to be_valid
+        end
+
+        context 'when group is subgroup' do
+          let(:subgroup) { create(:group, parent: group) }
+
+          it 'users email must match allowed domain email' do
+            expect(build(:group_member, group: subgroup, user: user_2)).to be_invalid
+            expect(build(:group_member, group: subgroup, user: user)).to be_valid
+          end
+
+          it 'invited email must match allowed domain email' do
+            expect(build(:group_member, group: subgroup, user: nil, invite_email: 'user@gmail.com')).to be_invalid
+            expect(build(:group_member, group: subgroup, user: nil, invite_email: 'user@gitlab.com')).to be_valid
+          end
+        end
+      end
+
+      context 'when group has email domain feature switched off' do
+        it 'users email must match allowed domain email' do
+          expect(build(:group_member, group: group, user: user_2)).to be_valid
+          expect(build(:group_member, group: group, user: user)).to be_valid
+        end
+
+        it 'invited email must match allowed domain email' do
+          expect(build(:group_member, group: group, invite_email: 'user@gmail.com')).to be_valid
+          expect(build(:group_member, group: group, invite_email: 'user@gitlab.com')).to be_valid
+        end
+      end
+    end
+  end
 end

ee/spec/services/groups/update_service_spec.rb

@@ -180,6 +180,32 @@ describe Groups::UpdateService, '#execute' do
     end
   end
 
+  context 'setting allowed email domain' do
+    let(:group) { create(:group) }
+
+    subject { update_group(group, user, params) }
+
+    before do
+      stub_licensed_features(group_allowed_email_domains: true)
+    end
+
+    context 'when allowed_email_domain already exists' do
+      let!(:allowed_domain) { create(:allowed_email_domain, group: group, domain: 'gitlab.com') }
+
+      context 'empty allowed_email_domain param' do
+        let(:params) { { allowed_email_domain_attributes: { id: allowed_domain.id, domain: '' } } }
+
+        it 'deletes ip restriction' do
+          expect(group.allowed_email_domain.domain).to eql('gitlab.com')
+
+          subject
+
+          expect(group.reload.allowed_email_domain).to be_nil
+        end
+      end
+    end
+  end
+
   context 'updating protected params' do
     let(:attrs) { { shared_runners_minutes_limit: 1000, extra_shared_runners_minutes_limit: 100 } }
 

ee/spec/views/groups/edit.html.haml_spec.rb

@@ -64,4 +64,57 @@ describe 'groups/edit.html.haml' do
       end
     end
   end
+
+  context 'allowed_email_domain' do
+    before do
+      allow(group).to receive(:feature_available?).and_return(false)
+      allow(group).to receive(:feature_available?).with(:group_allowed_email_domains).and_return(true)
+    end
+
+    context 'top-level group' do
+      before do
+        create(:allowed_email_domain, group: group)
+      end
+
+      it 'renders allowed_email_domain setting' do
+        render
+
+        expect(rendered).to render_template('groups/settings/_allowed_email_domain')
+        expect(rendered).to(have_field('group_allowed_email_domain_attributes_domain',
+                                       { disabled: false,
+                                         with: 'gitlab.com' }))
+      end
+    end
+
+    context 'subgroup' do
+      let(:group) { create(:group, :nested) }
+
+      before do
+        create(:allowed_email_domain, group: group.parent)
+        group.build_allowed_email_domain
+      end
+
+      it 'show read-only allowed_email_domain setting of root ancestor' do
+        render
+
+        expect(rendered).to render_template('groups/settings/_allowed_email_domain')
+        expect(rendered).to(have_field('group_allowed_email_domain_attributes_domain',
+                                       { disabled: true,
+                                         with: 'gitlab.com' }))
+      end
+    end
+
+    context 'feature is disabled' do
+      before do
+        stub_licensed_features(group_allowed_email_domains: false)
+      end
+
+      it 'does not show allowed_email_domain setting' do
+        render
+
+        expect(rendered).to render_template('groups/settings/_allowed_email_domain')
+        expect(rendered).not_to have_field('group_allowed_email_domain_attributes_domain')
+      end
+    end
+  end
 end

locale/gitlab.pot

@@ -1240,6 +1240,9 @@ msgstr ""
 msgid "Allow users to request access if visibility is public or internal."
 msgstr ""
 
+msgid "Allowed email domain restriction only permitted for top-level groups"
+msgstr ""
+
 msgid "Allowed to fail"
 msgstr ""
 
@@ -5230,6 +5233,9 @@ msgstr ""
 msgid "Email address"
 msgstr ""
 
+msgid "Email domain is not editable in subgroups. Value inherited from top-level parent group."
+msgstr ""
+
 msgid "Email patch"
 msgstr ""
 
@@ -5416,6 +5422,9 @@ msgstr ""
 msgid "Enter board name"
 msgstr ""
 
+msgid "Enter domain"
+msgstr ""
+
 msgid "Enter in your Bitbucket Server URL and personal access token below"
 msgstr ""
 
@@ -10270,6 +10279,9 @@ msgstr ""
 msgid "Only these extensions are supported: %{extension_list}"
 msgstr ""
 
+msgid "Only users with an email address in this domain can be added to the group.<br>Example: <code>gitlab.com</code>. Some common domains are not allowed. %{read_more_link}."
+msgstr ""
+
 msgid "Oops, are you sure?"
 msgstr ""
 
@@ -12700,6 +12712,9 @@ msgstr ""
 msgid "Restrict access by IP address"
 msgstr ""
 
+msgid "Restrict membership by email"
+msgstr ""
+
 msgid "Resume"
 msgstr ""
 
@@ -14728,6 +14743,12 @@ msgstr ""
 msgid "The directory has been successfully created."
 msgstr ""
 
+msgid "The domain you entered is misformatted."
+msgstr ""
+
+msgid "The domain you entered is not allowed."
+msgstr ""
+
 msgid "The entered user map is not a valid JSON user map."
 msgstr ""
 
@@ -17986,6 +18007,9 @@ msgstr[1] ""
 msgid "element is not a hierarchy"
 msgstr ""
 
+msgid "email '%{email}' does not match the allowed domain of '%{email_domain}'"
+msgstr ""
+
 msgid "enabled"
 msgstr ""