Merge branch 'jej/remove-sso-enforcement-flags' into 'master'

Remove :enforced_sso and :enforced_sso_requires_session feature flags

Closes #11757 and #9672

See merge request gitlab-org/gitlab!31769

ee/app/controllers/groups/saml_providers_controller.rb

@@ -45,9 +45,8 @@ class Groups::SamlProvidersController < Groups::ApplicationController
   end
 
   def saml_provider_params
-    allowed_params = %i[sso_url certificate_fingerprint enabled]
+    allowed_params = %i[sso_url certificate_fingerprint enabled enforced_sso]
 
-    allowed_params += [:enforced_sso] if Feature.enabled?(:enforced_sso, group)
     if Feature.enabled?(:group_managed_accounts, group)
       allowed_params += [:enforced_group_managed_accounts, :prohibited_outer_forks]
     end

ee/app/models/saml_provider.rb

@@ -30,7 +30,7 @@ class SamlProvider < ApplicationRecord
   end
 
   def enforced_sso?
-    enabled? && super && group.feature_available?(:group_saml) && ::Feature.enabled?(:enforced_sso, group)
+    enabled? && super && group.feature_available?(:group_saml)
   end
 
   def enforced_group_managed_accounts?

ee/app/views/groups/saml_providers/_form.html.haml

@@ -6,15 +6,14 @@
         = render "shared/buttons/project_feature_toggle", is_checked: saml_provider.enabled?, label: s_("GroupSAML|Toggle SAML authentication"), class_list: "js-project-feature-toggle project-feature-toggle d-inline" do
           = f.hidden_field :enabled, { class: 'js-group-saml-enabled-input js-project-feature-toggle-input'}
         %span.d-inline.font-weight-normal.align-text-bottom.ml-3= s_('GroupSAML|Enable SAML authentication for this group.')
-    - if Feature.enabled?(:enforced_sso, group)
-      .form-group
-        %label.toggle-wrapper.mb-0.js-group-saml-enforced-sso-toggle-area
-          = render "shared/buttons/project_feature_toggle", is_checked: saml_provider.enforced_sso, disabled: !saml_provider.enabled?, label: s_("GroupSAML|Enforced SSO"), class_list: "js-project-feature-toggle js-group-saml-enforced-sso-toggle project-feature-toggle d-inline", data: { qa_selector: 'enforced_sso_toggle_button' } do
-            = f.hidden_field :enforced_sso, { class: 'js-group-saml-enforced-sso-input js-project-feature-toggle-input'}
-          %span.form-text.d-inline.font-weight-normal.align-text-bottom.ml-3= Feature.enabled?(:enforced_sso_requires_session, group) ? s_('GroupSAML|Enforce SSO-only authentication for this group.') : s_('GroupSAML|Enforce SSO-only membership for this group.')
-        .form-text.text-muted.js-helper-text{ style: "display: #{'none' if saml_provider.enabled?} #{'block' unless saml_provider.enabled?}" }
-          %span
-            = s_('GroupSAML|To be able to enable enforced SSO, you first need to enable SAML authentication.')
+    .form-group
+      %label.toggle-wrapper.mb-0.js-group-saml-enforced-sso-toggle-area
+        = render "shared/buttons/project_feature_toggle", is_checked: saml_provider.enforced_sso, disabled: !saml_provider.enabled?, label: s_("GroupSAML|Enforced SSO"), class_list: "js-project-feature-toggle js-group-saml-enforced-sso-toggle project-feature-toggle d-inline", data: { qa_selector: 'enforced_sso_toggle_button' } do
+          = f.hidden_field :enforced_sso, { class: 'js-group-saml-enforced-sso-input js-project-feature-toggle-input'}
+        %span.form-text.d-inline.font-weight-normal.align-text-bottom.ml-3= s_('GroupSAML|Enforce SSO-only authentication for this group.')
+      .form-text.text-muted.js-helper-text{ style: "display: #{'none' if saml_provider.enabled?} #{'block' unless saml_provider.enabled?}" }
+        %span
+          = s_('GroupSAML|To be able to enable enforced SSO, you first need to enable SAML authentication.')
     - if Feature.enabled?(:group_managed_accounts, group)
       .form-group
         %label.toggle-wrapper.mb-0.js-group-saml-enforced-group-managed-accounts-toggle-area

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

@@ -9,7 +9,6 @@ module Gitlab
         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)

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

@@ -25,13 +25,12 @@ module Gitlab
         end
 
         def access_restricted?
-          saml_enforced? && !active_session? && ::Feature.enabled?(:enforced_sso_requires_session, group)
+          saml_enforced? && !active_session?
         end
 
         def self.group_access_restricted?(group)
           return false unless group
           return false unless group.root_ancestor
-          return false unless ::Feature.enabled?(:enforced_sso_requires_session, group.root_ancestor)
 
           saml_provider = group.root_ancestor.saml_provider
 

ee/spec/controllers/groups/saml_providers_controller_spec.rb

@@ -122,30 +122,11 @@ describe Groups::SamlProvidersController do
         group.add_owner(user)
       end
 
-      context 'enforced_sso feature flag enabled' do
-        before do
-          stub_feature_flags(enforced_sso: true)
-        end
-
-        it 'updates the flags' do
-          expect do
-            subject
-            saml_provider.reload
-          end.to change { saml_provider.enforced_sso? }.to(true)
-        end
-      end
-
-      context 'enforced_sso feature flag disabled' do
-        before do
-          stub_feature_flags(enforced_sso: false)
-        end
-
-        it 'does not update the setting' do
-          expect do
-            subject
-            saml_provider.reload
-          end.not_to change { saml_provider.enforced_sso? }.from(false)
-        end
+      it 'updates the setting' do
+        expect do
+          subject
+          saml_provider.reload
+        end.to change { saml_provider.enforced_sso? }.to(true)
       end
 
       context 'enabling group managed when owner has linked identity' do

ee/spec/features/groups/members/list_members_spec.rb

@@ -52,7 +52,6 @@ describe 'Groups > Members > List members' do
     let(:session) { { active_group_sso_sign_ins: { saml_provider.id => DateTime.now } } }
 
     before do
-      stub_feature_flags(enforced_sso: true, group_saml: true)
       stub_licensed_features(group_saml: true)
       allow(Gitlab::Session).to receive(:current).and_return(session)
 

ee/spec/features/groups/saml_providers_spec.rb

@@ -94,26 +94,12 @@ describe 'SAML provider settings' do
         expect(login_url).to end_with "?token=#{group.reload.saml_discovery_token}"
       end
 
-      context 'enforced sso enabled', :js do
-        it 'updates the flag' do
-          stub_feature_flags(enforced_sso: true)
-
-          visit group_saml_providers_path(group)
-
-          find('.js-group-saml-enforced-sso-toggle').click
-
-          expect { submit }.to change { saml_provider.reload.enforced_sso }.to(true)
-        end
-      end
+      it 'updates the enforced sso setting', :js do
+        visit group_saml_providers_path(group)
 
-      context 'enforced sso disabled' do
-        it 'does not update the flag' do
-          stub_feature_flags(enforced_sso: false)
+        find('.js-group-saml-enforced-sso-toggle').click
 
-          visit group_saml_providers_path(group)
-
-          expect(page).not_to have_selector('.js-group-saml-enforced-sso-toggle')
-        end
+        expect { submit }.to change { saml_provider.reload.enforced_sso }.to(true)
       end
 
       context 'enforced_group_managed_accounts enabled', :js do

ee/spec/features/groups/sso_spec.rb

@@ -23,7 +23,7 @@ describe 'Group SAML SSO', :js do
   end
 
   before do
-    stub_feature_flags(enforced_sso: true, group_managed_accounts: true, sign_up_on_sso: true, convert_user_to_group_managed_accounts: true)
+    stub_feature_flags(group_managed_accounts: true, sign_up_on_sso: true, convert_user_to_group_managed_accounts: true)
     stub_licensed_features(group_saml: true)
     allow(Devise).to receive(:omniauth_providers).and_return(%i(group_saml))
 

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

@@ -45,7 +45,7 @@ describe Gitlab::Auth::GroupSaml::SsoEnforcer do
 
     describe 'enforced sso expiry' do
       before do
-        stub_feature_flags(enforced_sso_requires_session: saml_provider.group)
+        stub_feature_flags(enforced_sso_expiry: saml_provider.group)
       end
 
       it 'returns true if a sign in is recently recorded' do
@@ -88,12 +88,6 @@ describe Gitlab::Auth::GroupSaml::SsoEnforcer do
       end
     end
 
-    it 'allows access when the sso enforcement feature is disabled' do
-      stub_feature_flags(enforced_sso: false)
-
-      expect(subject).not_to be_access_restricted
-    end
-
     it 'prevents access when sso enforcement active but there is no session' do
       expect(subject).to be_access_restricted
     end
@@ -109,10 +103,6 @@ describe Gitlab::Auth::GroupSaml::SsoEnforcer do
     let(:root_group) { create(:group, saml_provider: create(:saml_provider, enabled: true, enforced_sso: true)) }
 
     context 'is restricted' do
-      before do
-        stub_feature_flags(enforced_sso_requires_session: root_group)
-      end
-
       it 'for a group' do
         expect(described_class).to be_group_access_restricted(root_group)
       end
@@ -130,17 +120,10 @@ describe Gitlab::Auth::GroupSaml::SsoEnforcer do
       end
     end
 
-    context 'is not restricted' do
-      it 'for the group without configured saml_provider' do
-        group = create(:group)
-        stub_feature_flags(enforced_sso_requires_session: group)
-
-        expect(described_class).not_to be_group_access_restricted(group)
-      end
-
-      it 'for the group without the feature flag' do
-        stub_feature_flags(enforced_sso_requires_session: false)
+    context 'for a group without a saml_provider configured' do
+      let(:root_group) { create(:group) }
 
+      it 'is not restricted' do
         expect(described_class).not_to be_group_access_restricted(root_group)
       end
     end

ee/spec/models/saml_provider_spec.rb

@@ -110,18 +110,6 @@ describe SamlProvider do
         expect(subject).not_to be_enforced_sso
       end
 
-      context 'and feature flag is disabled' do
-        before do
-          stub_feature_flags(enforced_sso: false)
-        end
-
-        it 'is false' do
-          subject.enforced_sso = true
-
-          expect(subject).not_to be_enforced_sso
-        end
-      end
-
       it 'does not enforce SSO when the feature is unavailable' do
         stub_licensed_features(group_saml: false)
         subject.enforced_sso = true

ee/spec/requests/api/graphql/group_query_spec.rb

@@ -17,7 +17,6 @@ describe 'getting group information' do
 
       before do
         stub_licensed_features(group_saml: true)
-        stub_feature_flags(enforced_sso_requires_session: true)
         saml_provider = create(:saml_provider, enforced_sso: true, group: group)
         create(:group_saml_identity, saml_provider: saml_provider, user: user)
         group.add_guest(user)

ee/spec/requests/api/projects_spec.rb

@@ -814,7 +814,6 @@ describe API::Projects do
       end
 
       before do
-        stub_feature_flags(enforced_sso_requires_session: false)
         stub_licensed_features(group_saml: true)
       end
 

ee/spec/requests/jwt_controller_spec.rb

@@ -21,10 +21,6 @@ describe JwtController do
       let!(:saml_provider) { create(:saml_provider, enforced_sso: true, group: group) }
       let!(:identity) { create(:group_saml_identity, saml_provider: saml_provider, user: user) }
 
-      before do
-        stub_feature_flags(enforced_sso_requires_session: true)
-      end
-
       it 'allows access' do
         get '/jwt/auth', params: parameters, headers: headers
 

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

@@ -8,10 +8,6 @@ RSpec.shared_examples 'member validations' do
       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)
@@ -37,17 +33,7 @@ RSpec.shared_examples 'member validations' 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

locale/gitlab.pot

@@ -10694,9 +10694,6 @@ msgstr ""
 msgid "GroupSAML|Enforce SSO-only authentication for this group."
 msgstr ""
 
-msgid "GroupSAML|Enforce SSO-only membership for this group."
-msgstr ""
-
 msgid "GroupSAML|Enforce users to have dedicated group managed accounts for this group."
 msgstr ""
 

qa/qa/specs/features/ee/browser_ui/1_manage/group/group_saml_enforced_sso_spec.rb

@@ -51,7 +51,7 @@ module QA
 
       after do
         page.visit Runtime::Scenario.gitlab_address
-        %w[enforced_sso enforced_sso_requires_session group_administration_nav_item].each do |flag|
+        %w[group_administration_nav_item].each do |flag|
           Runtime::Feature.remove(flag)
         end
 
@@ -64,7 +64,7 @@ module QA
     end
 
     def setup_and_enable_enforce_sso
-      %w[enforced_sso enforced_sso_requires_session group_administration_nav_item].each do |flag|
+      %w[group_administration_nav_item].each do |flag|
         Runtime::Feature.enable_and_verify(flag)
       end
 

qa/qa/specs/features/ee/browser_ui/1_manage/group/group_saml_group_managed_accounts_spec.rb

@@ -85,7 +85,7 @@ module QA
       after(:all) do
         page.visit Runtime::Scenario.gitlab_address
 
-        %w[enforced_sso enforced_sso_requires_session group_managed_accounts sign_up_on_sso group_scim group_administration_nav_item].each do |flag|
+        %w[group_managed_accounts sign_up_on_sso group_scim group_administration_nav_item].each do |flag|
           Runtime::Feature.remove(flag)
         end
 
@@ -119,7 +119,7 @@ module QA
     end
 
     def setup_and_enable_group_managed_accounts
-      %w[enforced_sso enforced_sso_requires_session group_managed_accounts sign_up_on_sso group_scim group_administration_nav_item].each do |flag|
+      %w[group_managed_accounts sign_up_on_sso group_scim group_administration_nav_item].each do |flag|
         Runtime::Feature.enable_and_verify(flag)
       end