FEATURE: when suggesting usernames skip input that consist entirely o…

…f disallowed characters (#15368)

app/models/discourse_single_sign_on.rb

@@ -373,12 +373,9 @@ def change_external_attributes_and_override(sso_record, user)
   end
 
   def resolve_username
-    username_suggester_input = username.presence || name.presence
-    if SiteSetting.use_email_for_username_and_name_suggestions
-      username_suggester_input = username_suggester_input || email
-    end
-
-    UserNameSuggester.suggest(username_suggester_input)
+    suggester_input = [username, name]
+    suggester_input << email if SiteSetting.use_email_for_username_and_name_suggestions
+    UserNameSuggester.suggest(*suggester_input)
   end
 
   def resolve_name

app/services/username_changer.rb

@@ -19,7 +19,7 @@ def self.override(user, new_username)
       UsernameChanger.change(user, new_username, user)
       true
     elsif user.username != UserNameSuggester.fix_username(new_username)
-      suggested_username = UserNameSuggester.suggest(new_username, user.username)
+      suggested_username = UserNameSuggester.suggest(new_username, current_username: user.username)
       UsernameChanger.change(user, suggested_username, user)
       true
     else

lib/auth/result.rb

@@ -189,7 +189,7 @@ def staged_user
   end
 
   def username_suggester_attributes
-    username || name || email
+    [username, name, email]
   end
 
   def authenticator
@@ -203,6 +203,6 @@ def resolve_username
       end
     end
 
-    UserNameSuggester.suggest(username_suggester_attributes)
+    UserNameSuggester.suggest(*username_suggester_attributes)
   end
 end

lib/user_name_suggester.rb

@@ -4,8 +4,12 @@ module UserNameSuggester
   GENERIC_NAMES = ['i', 'me', 'info', 'support', 'admin', 'webmaster', 'hello', 'mail', 'office', 'contact', 'team']
   LAST_RESORT_USERNAME = "user"
 
-  def self.suggest(name_or_email,  current_username = nil)
-    name = parse_name_from_email(name_or_email)
+  def self.suggest(*input,  current_username: nil)
+    name = input.find do |item|
+      parsed_name = parse_name_from_email(item)
+      break parsed_name if sanitize_username(parsed_name).present?
+    end
+
     name = fix_username(name)
     find_available_username_based_on(name, current_username)
   end

spec/components/user_name_suggester_spec.rb

@@ -128,11 +128,29 @@
       Fabricate(:user, username: "bill4")
 
       # the number should be preserved, bill3 should remain bill3
-      suggestion = UserNameSuggester.suggest("bill", "bill3")
+      suggestion = UserNameSuggester.suggest("bill", current_username: "bill3")
 
       expect(suggestion).to eq "bill3"
     end
 
+    it "skips input made entirely of disallowed characters" do
+      SiteSetting.unicode_usernames = false
+
+      input = %w[Πλάτων علي William]
+      suggestion = UserNameSuggester.suggest(*input)
+
+      expect(suggestion).to eq "William"
+    end
+
+    it "uses the first item if it isn't made entirely of disallowed characters" do
+      SiteSetting.unicode_usernames = false
+
+      input = %w[William علي Πλάτων]
+      suggestion = UserNameSuggester.suggest(*input)
+
+      expect(suggestion).to eq "William"
+    end
+
     context "with Unicode usernames disabled" do
       before { SiteSetting.unicode_usernames = false }
 

spec/models/discourse_single_sign_on_spec.rb

@@ -438,6 +438,20 @@ def test_parsed(parsed, sso)
     expect(user.username).to eq "John_Smith"
   end
 
+  it "uses name for username suggestions if username consists entirely of disallowed characters" do
+    SiteSetting.unicode_usernames = false
+
+    sso = new_discourse_sso
+    sso.external_id = "100"
+
+    sso.username = "Πλάτων"
+    sso.name = "Plato"
+    sso.email = "mail@mail.com"
+    user = sso.lookup_or_create_user(ip_address)
+
+    expect(user.username).to eq sso.name
+  end
+
   it "doesn't use email as a source for username suggestions by default" do
     sso = new_discourse_sso
     sso.external_id = "100"
@@ -451,7 +465,7 @@ def test_parsed(parsed, sso)
     expect(user.username).to eq I18n.t('fallback_username')
   end
 
-  it "use email as a source for username suggestions if enabled" do
+  it "uses email as a source for username suggestions if enabled" do
     SiteSetting.use_email_for_username_and_name_suggestions = true
     sso = new_discourse_sso
     sso.external_id = "100"
@@ -478,7 +492,7 @@ def test_parsed(parsed, sso)
     expect(user.name).to eq ""
   end
 
-  it "use email as a source for name suggestions if enabled" do
+  it "uses email as a source for name suggestions if enabled" do
     SiteSetting.use_email_for_username_and_name_suggestions = true
     sso = new_discourse_sso
     sso.external_id = "100"
@@ -492,6 +506,21 @@ def test_parsed(parsed, sso)
     expect(user.name).to eq "Mail"
   end
 
+  it "uses email for username suggestions if username and name consist entirely of disallowed characters" do
+    SiteSetting.use_email_for_username_and_name_suggestions = true
+    SiteSetting.unicode_usernames = false
+
+    sso = new_discourse_sso
+    sso.external_id = "100"
+
+    sso.username = "Πλάτων"
+    sso.name = "Πλάτων"
+    sso.email = "mail@mail.com"
+    user = sso.lookup_or_create_user(ip_address)
+
+    expect(user.username).to eq "mail"
+  end
+
   it "can override username with a number at the end to a simpler username without a number" do
     SiteSetting.auth_overrides_username = true