From 88ecb83382e4dc5cec1d9599619a6c40a07825ae Mon Sep 17 00:00:00 2001 From: Andrei Prigorshnev Date: Tue, 12 Oct 2021 17:25:54 +0400 Subject: [PATCH] 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. --- app/models/discourse_single_sign_on.rb | 4 +-- lib/user_name_suggester.rb | 2 -- spec/components/user_name_suggester_spec.rb | 4 +-- spec/models/discourse_single_sign_on_spec.rb | 27 ++++++++++++++++++++ 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/app/models/discourse_single_sign_on.rb b/app/models/discourse_single_sign_on.rb index b30286f2e0b..7b198d36e4b 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 bf767d61627..3a294c0c36e 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 446fd7639cd..3adc48a6068 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 848f0e773d2..9c739771e62 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