From 56df077931e42749ea254a9df32154a8aa13bddc Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Mon, 21 Oct 2024 14:38:37 +0800 Subject: [PATCH] FIX: Don't error out on empty reserved_usernames setting (#29305) We're seeing errors in logs due to some sites setting the reserved_usernames setting to nil. This is causing multiple use cases upstream of User#reserved_username? to error out. This commit changes from using the raw #reserved_usernames to using the #reserved_usernames_map helper which exists on list-type site settings. It returns an empty array if the raw value is nil or empty string. --- app/models/user.rb | 8 +++----- config/site_settings.yml | 1 + spec/lib/user_name_suggester_spec.rb | 9 +++++---- spec/models/discourse_connect_spec.rb | 6 ++++-- spec/models/user_spec.rb | 6 ++++++ 5 files changed, 19 insertions(+), 11 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 4932dcaddca..40b218ef2b3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -440,11 +440,9 @@ class User < ActiveRecord::Base return true if SiteSetting.here_mention == username - SiteSetting - .reserved_usernames - .unicode_normalize - .split("|") - .any? { |reserved| username.match?(/\A#{Regexp.escape(reserved).gsub('\*', ".*")}\z/) } + SiteSetting.reserved_usernames_map.any? do |reserved| + username.match?(/\A#{Regexp.escape(reserved.unicode_normalize).gsub('\*', ".*")}\z/) + end end def self.editable_user_custom_fields(by_staff: false) diff --git a/config/site_settings.yml b/config/site_settings.yml index 39d919dfc5f..9bf7445419a 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -652,6 +652,7 @@ users: type: list list_type: compact default: "admin|moderator|administrator|mod|sys|system|community|info|you|name|username|user|nickname|discourse|discourseorg|discourseforum|support|all|here" + mandatory_values: "admin|moderator|administrator|mod|sys|system|community|info|you|name|username|user|nickname|discourse|discourseorg|discourseforum|support|all|here" min_password_length: client: true default: 10 diff --git a/spec/lib/user_name_suggester_spec.rb b/spec/lib/user_name_suggester_spec.rb index a36fb648ba0..d27d0b2ab96 100644 --- a/spec/lib/user_name_suggester_spec.rb +++ b/spec/lib/user_name_suggester_spec.rb @@ -10,6 +10,8 @@ RSpec.describe UserNameSuggester do SiteSetting.reserved_usernames = "" end + let(:fallback_username) { I18n.t("fallback_username") + "1" } + it "keeps adding numbers to the username" do Fabricate(:user, username: "sam") Fabricate(:user, username: "sAm1") @@ -20,7 +22,7 @@ RSpec.describe UserNameSuggester do end 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")) + expect(UserNameSuggester.suggest(nil)).to eq(fallback_username) end it "doesn't raise an error on integer username" do @@ -86,7 +88,7 @@ RSpec.describe UserNameSuggester do it "suggest a fallback username if name contains only invalid characters" do suggestion = UserNameSuggester.suggest("---") - expect(suggestion).to eq(I18n.t("fallback_username")) + expect(suggestion).to eq(fallback_username) end it "allows dots in the middle" do @@ -164,7 +166,6 @@ RSpec.describe UserNameSuggester do end it "uses fallback username if there are Unicode characters only" do - fallback_username = I18n.t("fallback_username") expect(UserNameSuggester.suggest("طائر")).to eq(fallback_username) expect(UserNameSuggester.suggest("πουλί")).to eq(fallback_username) end @@ -218,7 +219,7 @@ RSpec.describe UserNameSuggester do it "uses allowlist" do SiteSetting.allowed_unicode_username_characters = "[äöüßÄÖÜẞ]" - expect(UserNameSuggester.suggest("πουλί")).to eq(I18n.t("fallback_username")) + expect(UserNameSuggester.suggest("πουλί")).to eq(fallback_username) expect(UserNameSuggester.suggest("a鳥b")).to eq("a_b") expect(UserNameSuggester.suggest("Löwe")).to eq("Löwe") diff --git a/spec/models/discourse_connect_spec.rb b/spec/models/discourse_connect_spec.rb index 0c05124a3c8..f0c6bff7cfc 100644 --- a/spec/models/discourse_connect_spec.rb +++ b/spec/models/discourse_connect_spec.rb @@ -12,6 +12,8 @@ RSpec.describe DiscourseConnect do Jobs.run_immediately! end + let(:fallback_username) { I18n.t("fallback_username") + "1" } + def make_sso sso = DiscourseConnectBase.new sso.sso_url = "http://meta.discorse.org/topics/111" @@ -576,7 +578,7 @@ RSpec.describe DiscourseConnect do sso.email = "mail@mail.com" user = sso.lookup_or_create_user(ip_address) - expect(user.username).to eq "user" + expect(user.username).to eq(fallback_username) end it "doesn't use email as a source for username suggestions by default" do @@ -589,7 +591,7 @@ RSpec.describe DiscourseConnect do sso.email = "mail@mail.com" user = sso.lookup_or_create_user(ip_address) - expect(user.username).to eq I18n.t("fallback_username") + expect(user.username).to eq(fallback_username) end it "uses email as a source for username suggestions if enabled" do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 18f3f97d45c..0d62b131305 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1076,6 +1076,12 @@ RSpec.describe User do expect(User.reserved_username?("löwe")).to eq(true) # NFC expect(User.reserved_username?("käfer")).to eq(true) # NFC end + + it "does not error out when there are no reserved usernames" do + SiteSetting.stubs(:reserved_usernames).returns(nil) + + expect { User.username_available?("Foo") }.not_to raise_error + end end describe "email_validator" do