Merge branch '9255-implement-access-controls-when-sso-enforcement-enabled' into 'master'

Resolve "Implement access controls when SSO enforcement enabled"

Closes #9413

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

ee/app/models/ee/group_member.rb

@@ -7,6 +7,8 @@ module EE
     prepended do
       extend ::Gitlab::Utils::Override
 
+      validate :sso_enforcement, if: :group
+
       scope :with_ldap_dn, -> { joins(user: :identities).where("identities.provider LIKE ?", 'ldap%') }
       scope :with_identity_provider, ->(provider) do
         joins(user: :identities).where(identities: { provider: provider })

ee/app/models/ee/member.rb

@@ -27,5 +27,11 @@ module EE
         super
       end
     end
+
+    def sso_enforcement
+      unless ::Gitlab::Auth::GroupSaml::MembershipEnforcer.new(group).can_add_user?(user)
+        errors.add(:user, 'is not linked to a SAML account')
+      end
+    end
   end
 end

ee/app/models/ee/project_member.rb

@@ -7,9 +7,15 @@ module EE
     prepended do
       extend ::Gitlab::Utils::Override
 
+      validate :sso_enforcement, if: :group
+
       before_destroy :delete_member_branch_protection
     end
 
+    def group
+      source&.group
+    end
+
     def delete_member_branch_protection
       if user.present? && project.present?
         project.protected_branches.merge_access_by_user(user).destroy_all # rubocop: disable DestroyAll

ee/changelogs/unreleased/9255-implement-access-controls-when-sso-enforcement-enabled.yml

+---
+title: Resolve Implement access controls when SSO enforcement enabled
+merge_request: 9270
+author:
+type: added

ee/lib/gitlab/auth/group_saml/membership_enforcer.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Auth
+    module GroupSaml
+      class MembershipEnforcer
+        def initialize(group)
+          @group = group
+        end
+
+        def can_add_user?(user)
+          return true unless ::Feature.enabled?(:enforced_sso, @group)
+          return true unless root_group&.saml_provider&.enforced_sso
+
+          GroupSamlIdentityFinder.new(user: user).find_linked(group: root_group)
+        end
+
+        private
+
+        def root_group
+          @root_group ||= @group&.root_ancestor
+        end
+      end
+    end
+  end
+end

ee/spec/lib/gitlab/auth/group_saml/membership_enforcer_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::Auth::GroupSaml::MembershipEnforcer do
+  let(:user) { create(:user) }
+  let(:identity) { create(:group_saml_identity, user: user) }
+  let(:group) { identity.saml_provider.group }
+
+  before do
+    allow_any_instance_of(SamlProvider).to receive(:enforced_sso).and_return(true)
+  end
+
+  it 'allows adding the group member' do
+    expect(described_class.new(group).can_add_user?(user)).to be_truthy
+  end
+
+  it 'does not add the group member' do
+    non_saml_user = create(:user)
+
+    expect(described_class.new(group).can_add_user?(non_saml_user)).to be_falsey
+  end
+end

ee/spec/models/group_member_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe GroupMember do
+  it { is_expected.to include_module(EE::GroupMember) }
+
+  it_behaves_like 'member validations'
+end

ee/spec/models/project_member_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe ProjectMember do
+  it { is_expected.to include_module(EE::ProjectMember) }
+
+  it_behaves_like 'member validations' do
+    let(:entity) { create(:project, group: group)}
+  end
+end

ee/spec/support/shared_examples/models/member_shared_examples.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+shared_examples_for 'member validations' do
+  describe 'validations' do
+    context 'validates SSO enforcement' do
+      let(:user) { create(:user) }
+      let(:identity) { create(:group_saml_identity, user: user) }
+      let(:group) { identity.saml_provider.group }
+      let(:entity) { group }
+
+      before do
+        stub_feature_flags(enforced_sso: true)
+      end
+
+      context 'enforced SSO enabled' do
+        before do
+          allow_any_instance_of(SamlProvider).to receive(:enforced_sso).and_return(true)
+        end
+
+        it 'allows adding the group member' do
+          member = described_class.add_user(entity, user, Member::DEVELOPER)
+
+          expect(member).to be_valid
+        end
+
+        it 'does not add the group member' do
+          member = described_class.add_user(entity, create(:user), Member::DEVELOPER)
+
+          expect(member).not_to be_valid
+          expect(member.errors.messages[:user]).to eq(['is not linked to a SAML account'])
+        end
+
+        context 'subgroups', :nested_groups do
+          let!(:subgroup) { create(:group, parent: group) }
+
+          before do
+            entity.update(group: subgroup) if entity.is_a?(Project)
+          end
+
+          it 'allows adding a group member without SSO enforced on subgroup' do
+            stub_feature_flags(enforced_sso: false, group: subgroup)
+
+            member = described_class.add_user(entity, create(:user), ProjectMember::DEVELOPER)
+
+            expect(member).to be_valid
+          end
+
+          it 'does not allow adding a group member with SSO enforced on subgroup' do
+            stub_feature_flags(enforced_sso: true, group: subgroup)
+
+            member = described_class.add_user(entity, create(:user), ProjectMember::DEVELOPER)
+
+            expect(member).not_to be_valid
+            expect(member.errors.messages[:user]).to eq(['is not linked to a SAML account'])
+          end
+        end
+      end
+
+      context 'enforced SSO disabled' do
+        it 'allows adding the group member' do
+          member = described_class.add_user(entity, user, Member::DEVELOPER)
+
+          expect(member).to be_valid
+        end
+      end
+    end
+  end
+end

spec/migrations/clean_up_for_members_spec.rb

@@ -2,6 +2,10 @@ require 'spec_helper'
 require Rails.root.join('db', 'migrate', '20171216111734_clean_up_for_members.rb')
 
 describe CleanUpForMembers, :migration do
+  before do
+    stub_feature_flags(enforced_sso: false)
+  end
+
   let(:migration) { described_class.new }
   let(:groups) { table(:namespaces) }
   let!(:group_member) { create_group_member }