Merge branch 'ee-52940-fix-internal-email-pattern-not-respected' into 'master'

[EE] Fix a bug where internal email pattern wasn't respected

Closes gitlab-ce#52940

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

app/services/users/build_service.rb

@@ -97,10 +97,6 @@ module Users
         if params[:reset_password]
           user_params.merge!(force_random_password: true, password_expires_at: nil)
         end
-
-        if user_default_internal_regex_enabled? && !user_params.key?(:external)
-          user_params[:external] = user_external?
-        end
       else
         allowed_signup_params = signup_params
         allowed_signup_params << :skip_confirmation if skip_authorization
@@ -111,6 +107,10 @@ module Users
         end
       end
 
+      if user_default_internal_regex_enabled? && !user_params.key?(:external)
+        user_params[:external] = user_external?
+      end
+
       user_params
     end
 

changelogs/unreleased/52940-fix-internal-email-pattern-not-respected.yml

+---
+title: Fix a bug where internal email pattern wasn't respected
+merge_request: 22516
+author:
+type: fixed

ee/spec/lib/gitlab/auth/ldap/user_spec.rb

@@ -68,6 +68,37 @@ describe Gitlab::Auth::LDAP::User do
           end
         end
       end
+
+      context 'with "user_default_external" application setting' do
+        using RSpec::Parameterized::TableSyntax
+
+        where(:user_default_external, :user_default_internal_regex, :user_is_in_external_group, :expected_to_be_external) do
+          true  | nil           | false | false
+          true  | 'example.com' | false | false
+
+          true  | nil           | true | true
+          true  | 'example.com' | true | true
+
+          false | nil           | false | false
+          false | 'example.com' | false | false
+
+          false | nil           | true | true
+          false | 'example.com' | true | true
+        end
+
+        with_them do
+          let(:group_member_dns) { ['uid=someone_else,ou=people,dc=example,dc=com', user_is_in_external_group ? auth_hash.uid : nil].compact }
+
+          before do
+            stub_application_setting(user_default_external: user_default_external)
+            stub_application_setting(user_default_internal_regex: user_default_internal_regex)
+          end
+
+          it "sets the user's external flag appropriately" do
+            expect(gl_user.external).to eq(expected_to_be_external)
+          end
+        end
+      end
     end
 
     context 'when there is more than one external group' do

spec/services/users/build_service_spec.rb

@@ -159,9 +159,9 @@ describe Users::BuildService do
           true  | true  | 'fl@example.com'        | ''                      | true
           true  | false | 'fl@example.com'        | ''                      | true
 
-          true  | nil   | 'fl@example.com'        | '^(?:(?!\.ext@).)*$\r?' | true
-          true  | true  | 'fl@example.com'        | '^(?:(?!\.ext@).)*$\r?' | true
-          true  | false | 'fl@example.com'        | '^(?:(?!\.ext@).)*$\r?' | true
+          true  | nil   | 'fl@example.com'        | '^(?:(?!\.ext@).)*$\r?' | false
+          true  | true  | 'fl@example.com'        | '^(?:(?!\.ext@).)*$\r?' | false
+          true  | false | 'fl@example.com'        | '^(?:(?!\.ext@).)*$\r?' | false
 
           true  | nil   | 'tester.ext@domain.com' | '^(?:(?!\.ext@).)*$\r?' | true
           true  | true  | 'tester.ext@domain.com' | '^(?:(?!\.ext@).)*$\r?' | true