FEATURE: stop using email as source for username and name suggestions for Single Sign On (#14541)

FEATURE: stop using email as source for username and name suggestions for Single Sign On (#14541)

We don’t want to be using emails as source for username and name suggestions in cases when it’s possible that a user have no chance to intervene and correct a suggested username. It risks exposing email addresses.

diff --git a/app/models/discourse_single_sign_on.rb b/app/models/discourse_single_sign_on.rb
index b30286f..7b198d3 100644
--- a/app/models/discourse_single_sign_on.rb
+++ b/app/models/discourse_single_sign_on.rb
@@ -242,8 +242,8 @@ class DiscourseSingleSignOn < SingleSignOn
 
         user_params = {
           primary_email: UserEmail.new(email: email, primary: true),
-          name: try_name || User.suggest_name(try_username || email),
-          username: UserNameSuggester.suggest(try_username || try_name || email),
+          name: try_name || User.suggest_name(try_username),
+          username: UserNameSuggester.suggest(try_username || try_name),
           ip_address: ip_address
         }
 
diff --git a/lib/user_name_suggester.rb b/lib/user_name_suggester.rb
index bf767d6..3a294c0 100644
--- a/lib/user_name_suggester.rb
+++ b/lib/user_name_suggester.rb
@@ -5,8 +5,6 @@ module UserNameSuggester
   LAST_RESORT_USERNAME = "user"
 
   def self.suggest(name_or_email, allowed_username = nil)
-    return unless name_or_email.present?
-
     name = parse_name_from_email(name_or_email)
     find_available_username_based_on(name, allowed_username)
   end
diff --git a/spec/components/user_name_suggester_spec.rb b/spec/components/user_name_suggester_spec.rb
index 446fd76..3adc48a 100644
--- a/spec/components/user_name_suggester_spec.rb
+++ b/spec/components/user_name_suggester_spec.rb
@@ -20,8 +20,8 @@ describe UserNameSuggester do
       expect(UserNameSuggester.suggest('saM')).to eq('saM3')
     end
 
-    it "doesn't raise an error on nil username" do
-      expect(UserNameSuggester.suggest(nil)).to eq(nil)
+    it "doesn't raise an error on nil username and suggest the fallback username" do
+      expect(UserNameSuggester.suggest(nil)).to eq(I18n.t('fallback_username'))
     end
 
     it "doesn't raise an error on integer username" do
diff --git a/spec/models/discourse_single_sign_on_spec.rb b/spec/models/discourse_single_sign_on_spec.rb
index 848f0e7..9c73977 100644
--- a/spec/models/discourse_single_sign_on_spec.rb
+++ b/spec/models/discourse_single_sign_on_spec.rb
@@ -10,6 +10,7 @@ describe DiscourseSingleSignOn do
     SiteSetting.discourse_connect_url = @discourse_connect_url
     SiteSetting.enable_discourse_connect = true
     SiteSetting.discourse_connect_secret = @discourse_connect_secret
+    SiteSetting.reserved_usernames = ''
     Jobs.run_immediately!
   end
 
@@ -346,6 +347,32 @@ describe DiscourseSingleSignOn do
     expect(admin.name).to eq "Louis C.K."
   end
 
+  it "doesn't use email as a source for username suggestions" do
+    sso = new_discourse_sso
+    sso.external_id = "100"
+
+    # set username and name to nil, so they cannot be used as a source for suggestions
+    sso.username = nil
+    sso.name = nil
+    sso.email = "mail@mail.com"
+
+    user = sso.lookup_or_create_user(ip_address)
+    expect(user.username).to eq I18n.t('fallback_username')
+  end
+
+  it "doesn't use email as a source for name suggestions" do
+    sso = new_discourse_sso
+    sso.external_id = "100"
+
+    # set username and name to nil, so they cannot be used as a source for suggestions
+    sso.username = nil
+    sso.name = nil
+    sso.email = "mail@mail.com"
+
+    user = sso.lookup_or_create_user(ip_address)
+    expect(user.name).to eq ""
+  end
+
   it "can override username with a number at the end to a simpler username without a number" do
     SiteSetting.auth_overrides_username = true
 

GitHub sha: 88ecb83382e4dc5cec1d9599619a6c40a07825ae

This commit appears in #14541 which was approved by davidtaylorhq. It was merged by AndrewPrigorshnev.

This commit has been mentioned on Discourse Meta. There might be relevant details there: