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.
This commit is contained in:
Ted Johansson 2024-10-21 14:38:37 +08:00 committed by GitHub
parent 6f55457652
commit 56df077931
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 19 additions and 11 deletions

View File

@ -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)

View File

@ -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

View File

@ -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")

View File

@ -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

View File

@ -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