SAML Group Links permission policies

Adds a new permission for group SAML group links and refactors
a helper to centralize all the logic.

ee/app/helpers/ee/saml_providers_helper.rb

@@ -2,18 +2,14 @@
 
 module EE
   module SamlProvidersHelper
-    def group_saml_configured?
-      ::Gitlab::Auth::GroupSaml::Config.enabled?
-    end
-
     def show_saml_in_sidebar?(group)
-      return false unless group_saml_configured?
-      return false unless group.feature_available?(:group_saml)
-      return false if group.subgroup?
-
       can?(current_user, :admin_group_saml, group)
     end
 
+    def show_saml_group_links_in_sidebar?(group)
+      can?(current_user, :admin_saml_group_links, group)
+    end
+
     def saml_link_for_provider(text, provider, **args)
       saml_link(text, provider.group.full_path, **args)
     end

ee/app/policies/ee/group_policy.rb

@@ -69,8 +69,16 @@ module EE
         @subject.feature_available?(:cluster_deployments)
       end
 
-      condition(:group_saml_enabled) do
-        @subject.saml_provider&.enabled?
+      condition(:group_saml_config_enabled, scope: :global) do
+        ::Gitlab::Auth::GroupSaml::Config.enabled?
+      end
+
+      condition(:group_saml_available, scope: :subject) do
+        !@subject.subgroup? && @subject.feature_available?(:group_saml)
+      end
+
+      condition(:group_saml_enabled, scope: :subject) do
+        @subject.saml_enabled?
       end
 
       condition(:group_timelogs_available) do
@@ -198,7 +206,9 @@ module EE
         enable :read_group_security_dashboard
       end
 
-      rule { admin | owner }.enable :admin_group_saml
+      rule { group_saml_config_enabled & group_saml_available & (admin | owner) }.enable :admin_group_saml
+
+      rule { group_saml_enabled & can?(:admin_group_saml) }.enable :admin_saml_group_links
 
       rule { admin | (can_owners_manage_ldap & owner) }.policy do
         enable :admin_ldap_group_links

ee/spec/helpers/ee/saml_providers_helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe EE::SamlProvidersHelper do
+  def stub_can(permission, value)
+    allow(helper).to receive(:can?).with(user, permission, group).and_return(value)
+  end
+
+  let_it_be(:group) { create(:group) }
+  let_it_be(:user) { create(:user) }
+
+  before do
+    allow(helper).to receive(:current_user).and_return(user)
+  end
+
+  describe '#show_saml_in_sidebar?' do
+    subject { helper.show_saml_in_sidebar?(group) }
+
+    context 'when the user can admin group saml' do
+      before do
+        stub_can(:admin_group_saml, true)
+      end
+
+      it { is_expected.to eq(true) }
+    end
+
+    context 'when the user cannot admin group saml' do
+      before do
+        stub_can(:admin_group_saml, false)
+      end
+
+      it { is_expected.to eq(false) }
+    end
+  end
+
+  describe '#show_saml_group_links_in_sidebar?' do
+    subject { helper.show_saml_group_links_in_sidebar?(group) }
+
+    context 'when the user can admin saml group links' do
+      before do
+        stub_can(:admin_saml_group_links, true)
+      end
+
+      it { is_expected.to eq(true) }
+    end
+
+    context 'when the user cannot admin saml group links' do
+      before do
+        stub_can(:admin_saml_group_links, false)
+      end
+
+      it { is_expected.to eq(false) }
+    end
+  end
+end

ee/spec/policies/group_policy_spec.rb

@@ -285,20 +285,91 @@ RSpec.describe GroupPolicy do
   end
 
   describe 'per group SAML' do
+    context 'when group_saml is unavailable' do
+      def stub_group_saml_config(enabled)
+        allow(::Gitlab::Auth::GroupSaml::Config).to receive_messages(enabled?: enabled)
+      end
+
+      let(:current_user) { owner }
+
+      context 'when group saml config is disabled' do
+        before do
+          stub_group_saml_config(false)
+        end
+
+        it { is_expected.to be_disallowed(:admin_group_saml) }
+      end
+
+      context 'when the group is a subgroup' do
+        let_it_be(:subgroup) { create(:group, :private, parent: group) }
+
+        before do
+          stub_group_saml_config(true)
+        end
+
+        subject { described_class.new(current_user, subgroup) }
+
+        it { is_expected.to be_disallowed(:admin_group_saml) }
+      end
+
+      context 'when the feature is not licensed' do
+        before do
+          stub_group_saml_config(true)
+          stub_licensed_features(group_saml: false)
+        end
+
+        it { is_expected.to be_disallowed(:admin_group_saml) }
+      end
+    end
+
+    context 'when group_saml is available' do
+      before do
+        stub_licensed_features(group_saml: true)
+      end
+
+      context 'without an enabled SAML provider' do
+        context 'maintainer' do
           let(:current_user) { maintainer }
 
           it { is_expected.to be_disallowed(:admin_group_saml) }
+          it { is_expected.to be_disallowed(:admin_saml_group_links) }
+        end
 
         context 'owner' do
           let(:current_user) { owner }
 
           it { is_expected.to be_allowed(:admin_group_saml) }
+          it { is_expected.to be_disallowed(:admin_saml_group_links) }
         end
 
         context 'admin' do
           let(:current_user) { admin }
 
           it { is_expected.to be_allowed(:admin_group_saml) }
+          it { is_expected.to be_disallowed(:admin_saml_group_links) }
+        end
+      end
+
+      context 'with an enabled SAML provider' do
+        let_it_be(:saml_provider) { create(:saml_provider, group: group, enabled: true) }
+
+        context 'maintainer' do
+          let(:current_user) { maintainer }
+
+          it { is_expected.to be_disallowed(:admin_saml_group_links) }
+        end
+
+        context 'owner' do
+          let(:current_user) { owner }
+
+          it { is_expected.to be_allowed(:admin_saml_group_links) }
+        end
+
+        context 'admin' do
+          let(:current_user) { admin }
+
+          it { is_expected.to be_allowed(:admin_saml_group_links) }
+        end
       end
 
       context 'with sso enforcement enabled' do
@@ -306,10 +377,6 @@ RSpec.describe GroupPolicy do
 
         let_it_be(:saml_provider) { create(:saml_provider, group: group, enforced_sso: true) }
 
-      before do
-        stub_licensed_features(group_saml: true)
-      end
-
         context 'when the session has been set globally' do
           around do |example|
             Gitlab::Session.with_session({}) do
@@ -345,6 +412,7 @@ RSpec.describe GroupPolicy do
         end
       end
     end
+  end
 
   context 'with ip restriction' do
     let(:current_user) { developer }