Is your community open to everyone, or is it restricted by membership, invitation, or approval? If you prefer, you can set things up privately, then switch over to public later.
" + description: "Is your community open to everyone, or is it restricted by membership, invitation, or approval? If you prefer, you can set things up privately, then switch over to public later.
" fields: privacy: diff --git a/config/routes.rb b/config/routes.rb index a81e46e1c4e..e30d78263d0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -6,7 +6,7 @@ require_dependency "homepage_constraint" require_dependency "permalink_constraint" # The following constants have been replaced with `RouteFormat` and are deprecated. -USERNAME_ROUTE_FORMAT = /[\w.\-]+?/ unless defined? USERNAME_ROUTE_FORMAT +USERNAME_ROUTE_FORMAT = /[%\w.\-]+?/ unless defined? USERNAME_ROUTE_FORMAT BACKUP_ROUTE_FORMAT = /.+\.(sql\.gz|tar\.gz|tgz)/i unless defined? BACKUP_ROUTE_FORMAT Discourse::Application.routes.draw do diff --git a/config/site_settings.yml b/config/site_settings.yml index aec3fef5f2e..5ec0106d476 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -450,6 +450,15 @@ users: min: 8 max: 60 validator: "MaxUsernameLengthValidator" + unicode_usernames: + default: false + client: true + validator: "UnicodeUsernameValidator" + unicode_username_character_whitelist: + validator: "UnicodeUsernameWhitelistValidator" + default: "" + locale_default: + de: "[äöüßÄÖÜẞ]" reserved_usernames: type: list list_type: compact @@ -1107,8 +1116,9 @@ files: default: true client: true shadowed_by_global: true + validator: "ExternalSystemAvatarsValidator" external_system_avatars_url: - default: "/letter_avatar_proxy/v3/letter/{first_letter}/{color}/{size}.png" + default: "/letter_avatar_proxy/v4/letter/{first_letter}/{color}/{size}.png" client: true regex: '^((https?:)?\/)?\/.+[^\/]' shadowed_by_global: true diff --git a/lib/route_format.rb b/lib/route_format.rb index 0ebd43fbdf0..14bc0398b1c 100644 --- a/lib/route_format.rb +++ b/lib/route_format.rb @@ -1,7 +1,7 @@ module RouteFormat def self.username - /[\w.\-]+?/ + /[%\w.\-]+?/ end def self.backup diff --git a/lib/user_name_suggester.rb b/lib/user_name_suggester.rb index afff6f1223b..d9c72edebd6 100644 --- a/lib/user_name_suggester.rb +++ b/lib/user_name_suggester.rb @@ -1,33 +1,35 @@ module UserNameSuggester GENERIC_NAMES = ['i', 'me', 'info', 'support', 'admin', 'webmaster', 'hello', 'mail', 'office', 'contact', 'team'] - def self.suggest(name, allow_username = nil) - return unless name.present? - name = parse_name_from_email(name) - find_available_username_based_on(name, allow_username) + 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 - def self.parse_name_from_email(name) - if name =~ User::EMAIL - # When 'walter@white.com' take 'walter' - name = Regexp.last_match[1] - # When 'me@eviltrout.com' take 'eviltrout' - name = Regexp.last_match[2] if GENERIC_NAMES.include?(name) - end + def self.parse_name_from_email(name_or_email) + return name_or_email if name_or_email !~ User::EMAIL + + # When 'walter@white.com' take 'walter' + name = Regexp.last_match[1] + + # When 'me@eviltrout.com' take 'eviltrout' + name = Regexp.last_match[2] if GENERIC_NAMES.include?(name) name end - def self.find_available_username_based_on(name, allow_username = nil) + def self.find_available_username_based_on(name, allowed_username = nil) name = fix_username(name) i = 1 attempt = name - until attempt == allow_username || User.username_available?(attempt) || i > 100 + until attempt == allowed_username || User.username_available?(attempt) || i > 100 suffix = i.to_s - max_length = User.username_length.end - suffix.length - 1 - attempt = "#{name[0..max_length]}#{suffix}" + max_length = User.username_length.end - suffix.length + attempt = "#{truncate(name, max_length)}#{suffix}" i += 1 end - until attempt == allow_username || User.username_available?(attempt) || i > 200 + until attempt == allowed_username || User.username_available?(attempt) || i > 200 attempt = SecureRandom.hex[1..SiteSetting.max_username_length] i += 1 end @@ -39,28 +41,45 @@ module UserNameSuggester end def self.sanitize_username(name) - name = ActiveSupport::Inflector.transliterate(name.to_s) - # 1. replace characters that aren't allowed with '_' - name.gsub!(UsernameValidator::CONFUSING_EXTENSIONS, "_") - name.gsub!(/[^\w.-]/, "_") - # 2. removes unallowed leading characters - name.gsub!(/^\W+/, "") - # 3. removes unallowed trailing characters - name = remove_unallowed_trailing_characters(name) - # 4. unify special characters - name.gsub!(/[-_.]{2,}/, "_") - name - end + name = name.to_s - def self.remove_unallowed_trailing_characters(name) - name.gsub!(/[^A-Za-z0-9]+$/, "") + if SiteSetting.unicode_usernames + name.unicode_normalize! + else + name = ActiveSupport::Inflector.transliterate(name) + end + + name.gsub!(UsernameValidator.invalid_char_pattern, '_') + name.chars.map! { |c| UsernameValidator.whitelisted_char?(c) ? c : '_' } if UsernameValidator.char_whitelist_exists? + name.gsub!(UsernameValidator::INVALID_LEADING_CHAR_PATTERN, '') + name.gsub!(UsernameValidator::CONFUSING_EXTENSIONS, "_") + name.gsub!(UsernameValidator::INVALID_TRAILING_CHAR_PATTERN, '') + name.gsub!(UsernameValidator::REPEATED_SPECIAL_CHAR_PATTERN, '_') name end def self.rightsize_username(name) - name = name[0, User.username_length.end] - name = remove_unallowed_trailing_characters(name) - name.ljust(User.username_length.begin, '1') + name = truncate(name, User.username_length.end) + name.gsub!(UsernameValidator::INVALID_TRAILING_CHAR_PATTERN, '') + + missing_char_count = User.username_length.begin - name.grapheme_clusters.size + name << '1' * missing_char_count if missing_char_count > 0 + name end + def self.truncate(name, max_grapheme_clusters) + clusters = name.grapheme_clusters + + if clusters.size > max_grapheme_clusters + clusters = clusters[0..max_grapheme_clusters - 1] + name = clusters.join + end + + while name.length > UsernameValidator::MAX_CHARS + clusters.pop + name = clusters.join + end + + name + end end diff --git a/lib/validators/external_system_avatars_validator.rb b/lib/validators/external_system_avatars_validator.rb new file mode 100644 index 00000000000..663da04d4fa --- /dev/null +++ b/lib/validators/external_system_avatars_validator.rb @@ -0,0 +1,13 @@ +class ExternalSystemAvatarsValidator + def initialize(opts = {}) + @opts = opts + end + + def valid_value?(value) + @valid = value == "t" || !SiteSetting.unicode_usernames + end + + def error_message + I18n.t("site_settings.errors.unicode_usernames_avatars") if !@valid + end +end diff --git a/lib/validators/unicode_username_validator.rb b/lib/validators/unicode_username_validator.rb new file mode 100644 index 00000000000..9b81f9235ee --- /dev/null +++ b/lib/validators/unicode_username_validator.rb @@ -0,0 +1,13 @@ +class UnicodeUsernameValidator + def initialize(opts = {}) + @opts = opts + end + + def valid_value?(value) + @valid = SiteSetting.external_system_avatars_enabled || value == "f" + end + + def error_message + I18n.t("site_settings.errors.unicode_usernames_avatars") if !@valid + end +end diff --git a/lib/validators/unicode_username_whitelist_validator.rb b/lib/validators/unicode_username_whitelist_validator.rb new file mode 100644 index 00000000000..f3e4691f8f7 --- /dev/null +++ b/lib/validators/unicode_username_whitelist_validator.rb @@ -0,0 +1,26 @@ +class UnicodeUsernameWhitelistValidator + def initialize(opts = {}) + @opts = opts + end + + def valid_value?(value) + @error_message = nil + return true if value.blank? + + if value.match?(/^\/.*\/[imxo]*$/) + @error_message = I18n.t("site_settings.errors.unicode_username_whitelist.leading_trailing_slash") + else + begin + Regexp.new(value) + rescue RegexpError => e + @error_message = I18n.t("site_settings.errors.unicode_username_whitelist.regex_invalid", error: e.message) + end + end + + @error_message.blank? + end + + def error_message + @error_message + end +end diff --git a/spec/components/pretty_text_spec.rb b/spec/components/pretty_text_spec.rb index 8b6763c3c5e..902f8046526 100644 --- a/spec/components/pretty_text_spec.rb +++ b/spec/components/pretty_text_spec.rb @@ -336,6 +336,21 @@ describe PrettyText do expect(PrettyText.cook(". http://test/@sam")).not_to include('mention') end + context "with Unicode usernames disabled" do + before { SiteSetting.unicode_usernames = false } + + it 'does not detect mention' do + expect(PrettyText.cook("Hello @狮子")).to_not include("mention") + end + end + + context "with Unicode usernames enabled" do + before { SiteSetting.unicode_usernames = true } + + it 'does detect mention' do + expect(PrettyText.cook("Hello @狮子")).to match_html 'Hello @狮子
' + end + end end describe "code fences" do diff --git a/spec/components/user_name_suggester_spec.rb b/spec/components/user_name_suggester_spec.rb index a9137d8b747..37f4a37a931 100644 --- a/spec/components/user_name_suggester_spec.rb +++ b/spec/components/user_name_suggester_spec.rb @@ -2,16 +2,10 @@ require 'rails_helper' require 'user_name_suggester' describe UserNameSuggester do - - describe 'name heuristics' do - it 'is able to guess a decent username from an email' do - expect(UserNameSuggester.suggest('bob@bob.com')).to eq('bob') - end - end - describe '.suggest' do before do - User.stubs(:username_length).returns(3..15) + SiteSetting.min_username_length = 3 + SiteSetting.max_username_length = 15 end it "doesn't raise an error on nil username" do @@ -26,10 +20,6 @@ describe UserNameSuggester do expect(UserNameSuggester.suggest("Darth%^Vader")).to eq('Darth_Vader') end - it "transliterates some characters" do - expect(UserNameSuggester.suggest("Jørn")).to eq('Jorn') - end - it 'adds 1 to an existing username' do user = Fabricate(:user) expect(UserNameSuggester.suggest(user.username)).to eq("#{user.username}1") @@ -39,6 +29,10 @@ describe UserNameSuggester do expect(UserNameSuggester.suggest('a')).to eq('a11') end + it 'is able to guess a decent username from an email' do + expect(UserNameSuggester.suggest('bob@example.com')).to eq('bob') + end + it "has a special case for me and i emails" do expect(UserNameSuggester.suggest('me@eviltrout.com')).to eq('eviltrout') expect(UserNameSuggester.suggest('i@eviltrout.com')).to eq('eviltrout') @@ -106,6 +100,57 @@ describe UserNameSuggester do User.stubs(:username_length).returns(8..8) expect(UserNameSuggester.suggest('uuuuuuu_u')).to eq('uuuuuuu1') end - end + context "with Unicode usernames disabled" do + before { SiteSetting.unicode_usernames = false } + + it "transliterates some characters" do + expect(UserNameSuggester.suggest('Jørn')).to eq('Jorn') + end + + it "replaces Unicode characters" do + expect(UserNameSuggester.suggest('طائر')).to eq('111') + expect(UserNameSuggester.suggest('πουλί')).to eq('111') + end + end + + context "with Unicode usernames enabled" do + before { SiteSetting.unicode_usernames = true } + + it "does not transliterate" do + expect(UserNameSuggester.suggest("Jørn")).to eq('Jørn') + end + + it "does not replace Unicode characters" do + expect(UserNameSuggester.suggest('طائر')).to eq('طائر') + expect(UserNameSuggester.suggest('πουλί')).to eq('πουλί') + end + + it "shortens usernames by counting grapheme clusters" do + SiteSetting.max_username_length = 10 + expect(UserNameSuggester.suggest('बहुत-लंबा-उपयोगकर्ता-नाम')).to eq('बहुत-लंबा-उपयो') + end + + it "adds numbers if it's too short" do + expect(UserNameSuggester.suggest('鳥')).to eq('鳥11') + + # grapheme cluster consists of 3 code points + expect(UserNameSuggester.suggest('য়া')).to eq('য়া11') + end + + it "normalizes usernames" do + actual = 'Löwe' # NFD, "Lo\u0308we" + expected = 'Löwe' # NFC, "L\u00F6we" + + expect(UserNameSuggester.suggest(actual)).to eq(expected) + end + + it "does not suggest a username longer than max column size" do + SiteSetting.max_username_length = 40 + + expect(UserNameSuggester.suggest('য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া')) + .to eq('য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া-য়া') + end + end + end end diff --git a/spec/components/validators/external_system_avatars_validator_spec.rb b/spec/components/validators/external_system_avatars_validator_spec.rb new file mode 100644 index 00000000000..1057f20b3f3 --- /dev/null +++ b/spec/components/validators/external_system_avatars_validator_spec.rb @@ -0,0 +1,25 @@ +require 'rails_helper' + +describe ExternalSystemAvatarsValidator do + subject { described_class.new } + + it "disallows disabling external system avatars when Unicode usernames are enabled" do + SiteSetting.unicode_usernames = true + + expect(subject.valid_value?("f")).to eq(false) + expect(subject.error_message).to eq(I18n.t("site_settings.errors.unicode_usernames_avatars")) + + expect(subject.valid_value?("t")).to eq(true) + expect(subject.error_message).to be_blank + end + + it "allows disabling external system avatars when Unicode usernames are disabled" do + SiteSetting.unicode_usernames = false + + expect(subject.valid_value?("t")).to eq(true) + expect(subject.error_message).to be_blank + + expect(subject.valid_value?("f")).to eq(true) + expect(subject.error_message).to be_blank + end +end diff --git a/spec/components/validators/unicode_username_validator_spec.rb b/spec/components/validators/unicode_username_validator_spec.rb new file mode 100644 index 00000000000..d8d8f59cb13 --- /dev/null +++ b/spec/components/validators/unicode_username_validator_spec.rb @@ -0,0 +1,25 @@ +require 'rails_helper' + +describe UnicodeUsernameValidator do + subject { described_class.new } + + it "disallows Unicode usernames when external system avatars are disabled" do + SiteSetting.external_system_avatars_enabled = false + + expect(subject.valid_value?("t")).to eq(false) + expect(subject.error_message).to eq(I18n.t("site_settings.errors.unicode_usernames_avatars")) + + expect(subject.valid_value?("f")).to eq(true) + expect(subject.error_message).to be_blank + end + + it "allows Unicode usernames when external system avatars are enabled" do + SiteSetting.external_system_avatars_enabled = true + + expect(subject.valid_value?("t")).to eq(true) + expect(subject.error_message).to be_blank + + expect(subject.valid_value?("f")).to eq(true) + expect(subject.error_message).to be_blank + end +end diff --git a/spec/components/validators/unicode_username_whitelist_validator_spec.rb b/spec/components/validators/unicode_username_whitelist_validator_spec.rb new file mode 100644 index 00000000000..8176e120483 --- /dev/null +++ b/spec/components/validators/unicode_username_whitelist_validator_spec.rb @@ -0,0 +1,36 @@ +require 'rails_helper' + +describe UnicodeUsernameWhitelistValidator do + subject { described_class.new } + + it "allows an empty whitelist" do + expect(subject.valid_value?("")).to eq(true) + expect(subject.error_message).to be_blank + end + + it "disallows leading and trailing slashes" do + expected_error = I18n.t("site_settings.errors.unicode_username_whitelist.leading_trailing_slash") + + expect(subject.valid_value?("/foo/")).to eq(false) + expect(subject.error_message).to eq(expected_error) + + expect(subject.valid_value?("foo/")).to eq(true) + expect(subject.error_message).to be_blank + + expect(subject.valid_value?("/foo")).to eq(true) + expect(subject.error_message).to be_blank + + expect(subject.valid_value?("f/o/o")).to eq(true) + expect(subject.error_message).to be_blank + + expect(subject.valid_value?("/foo/i")).to eq(false) + expect(subject.error_message).to eq(expected_error) + end + + it "detects invalid regular expressions" do + expected_error = I18n.t("site_settings.errors.unicode_username_whitelist.regex_invalid", error: "") + + expect(subject.valid_value?("\\p{Foo}")).to eq(false) + expect(subject.error_message).to start_with(expected_error) + end +end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 5c196e90483..45988094cd6 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -844,4 +844,13 @@ describe Group do group = Group.find(group.id) expect(group.flair_url).to eq("fab fa-bandcamp") end + + context "Unicode usernames and group names" do + before { SiteSetting.unicode_usernames = true } + + it "should normalize the name" do + group = Fabricate(:group, name: "Bücherwurm") # NFD + expect(group.name).to eq("Bücherwurm") # NFC + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a2b2d59fad3..94a1fb9536e 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -477,13 +477,13 @@ describe User do describe 'username format' do def assert_bad(username) - user = Fabricate.build(:user) + user = Fabricate(:user) user.username = username expect(user.valid?).to eq(false) end def assert_good(username) - user = Fabricate.build(:user) + user = Fabricate(:user) user.username = username expect(user.valid?).to eq(true) end @@ -494,39 +494,78 @@ describe User do assert_good("abcde") end - %w{ first.last - first first-last - _name first_last + context 'when Unicode usernames are disabled' do + before { SiteSetting.unicode_usernames = false } + + %w{ + first.last + first + first-last + _name + first_last mc.hammer_nose UPPERCASE sgif - }.each do |username| - it "allows #{username}" do - assert_good(username) + }.each do |username| + it "allows #{username}" do + assert_good(username) + end + end + + %w{ + traildot. + has\ space + double__underscore + with%symbol + Exclamation! + @twitter + my@email.com + .tester + sa$sy + sam.json + sam.xml + sam.html + sam.htm + sam.js + sam.woff + sam.Png + sam.gif + }.each do |username| + it "disallows #{username}" do + assert_bad(username) + end end end - %w{ - traildot. - has\ space - double__underscore - with%symbol - Exclamation! - @twitter - my@email.com - .tester - sa$sy - sam.json - sam.xml - sam.html - sam.htm - sam.js - sam.woff - sam.Png - sam.gif - }.each do |username| - it "disallows #{username}" do - assert_bad(username) + context 'when Unicode usernames are enabled' do + before { SiteSetting.unicode_usernames = true } + + %w{ + Джофрэй + Джо.фрэй + Джофр-эй + Д.жофрэй + 乔夫雷 + 乔夫_雷 + _乔夫雷 + }.each do |username| + it "allows #{username}" do + assert_good(username) + end + end + + %w{ + .Джофрэй + Джофрэй. + Джо\ фрэй + Джоф__рэй + 乔夫雷.js + 乔夫雷. + 乔夫%雷 + }.each do |username| + it "disallows #{username}" do + assert_bad(username) + end end end end @@ -540,12 +579,12 @@ describe User do it "should not allow saving if username is reused" do @codinghorror.username = @user.username - expect(@codinghorror.save).to eq(false) + expect(@codinghorror.save).to eq(false) end it "should not allow saving if username is reused in different casing" do @codinghorror.username = @user.username.upcase - expect(@codinghorror.save).to eq(false) + expect(@codinghorror.save).to eq(false) end end @@ -585,6 +624,21 @@ describe User do Fabricate(:group, name: 'foo') expect(User.username_available?('Foo')).to eq(false) end + + context "with Unicode usernames enabled" do + before { SiteSetting.unicode_usernames = true } + + it 'returns false when the username is taken, but the Unicode normalization form is different' do + Fabricate(:user, username: "L\u00F6we") # NFC + requested_username = "Lo\u0308we" # NFD + expect(User.username_available?(requested_username)).to eq(false) + end + + it 'returns false when the username is taken and the case differs' do + Fabricate(:user, username: 'LÖWE') + expect(User.username_available?('löwe')).to eq(false) + end + end end describe '.reserved_username?' do @@ -597,7 +651,7 @@ describe User do end it 'should not allow usernames matched against an expession' do - SiteSetting.reserved_usernames = 'test)|*admin*|foo*|*bar|abc.def' + SiteSetting.reserved_usernames = "test)|*admin*|foo*|*bar|abc.def|löwe|ka\u0308fer" expect(User.reserved_username?('test')).to eq(false) expect(User.reserved_username?('abc9def')).to eq(false) @@ -610,6 +664,11 @@ describe User do expect(User.reserved_username?('bar.foo')).to eq(false) expect(User.reserved_username?('foo.bar')).to eq(true) expect(User.reserved_username?('baz.bar')).to eq(true) + + expect(User.reserved_username?('LÖwe')).to eq(true) + expect(User.reserved_username?("Lo\u0308we")).to eq(true) # NFD + expect(User.reserved_username?('löwe')).to eq(true) # NFC + expect(User.reserved_username?('käfer')).to eq(true) # NFC end end @@ -989,6 +1048,14 @@ describe User do expect(found_user).to eq bob end + it 'finds users with Unicode username' do + SiteSetting.unicode_usernames = true + user = Fabricate(:user, username: 'löwe') + + expect(User.find_by_username('LÖWE')).to eq(user) # NFC + expect(User.find_by_username("LO\u0308WE")).to eq(user) # NFD + expect(User.find_by_username("lo\u0308we")).to eq(user) # NFD + end end describe "#new_user_posting_on_first_day?" do @@ -1099,9 +1166,9 @@ describe User do before do Jobs.run_immediately! PostCreator.new(Fabricate(:user), - raw: 'whatever this is a raw post', - topic_id: topic.id, - reply_to_post_number: post.post_number).create + raw: 'whatever this is a raw post', + topic_id: topic.id, + reply_to_post_number: post.post_number).create end it "resets the `posted_too_much` threshold" do @@ -1170,7 +1237,7 @@ describe User do expect(user.small_avatar_url).to eq("//test.localhost/letter_avatar/sam/45/#{LetterAvatar.version}.png") SiteSetting.external_system_avatars_enabled = true - expect(user.small_avatar_url).to eq("//test.localhost/letter_avatar_proxy/v3/letter/s/5f9b8f/45.png") + expect(user.small_avatar_url).to eq("//test.localhost/letter_avatar_proxy/v4/letter/s/5f9b8f/45.png") end end @@ -1193,7 +1260,7 @@ describe User do describe "update_posts_read!" do context "with a UserVisit record" do let!(:user) { Fabricate(:user) } - let!(:now) { Time.zone.now } + let!(:now) { Time.zone.now } before { user.update_last_seen!(now) } it "with existing UserVisit record, increments the posts_read value" do @@ -1301,17 +1368,17 @@ describe User do before do PostCreator.new(Discourse.system_user, - title: "Welcome to our Discourse", - raw: "This is a welcome message", - archetype: Archetype.private_message, - target_usernames: [unactivated_old_with_system_pm.username], + title: "Welcome to our Discourse", + raw: "This is a welcome message", + archetype: Archetype.private_message, + target_usernames: [unactivated_old_with_system_pm.username], ).create PostCreator.new(user, - title: "Welcome to our Discourse", - raw: "This is a welcome message", - archetype: Archetype.private_message, - target_usernames: [unactivated_old_with_human_pm.username], + title: "Welcome to our Discourse", + raw: "This is a welcome message", + archetype: Archetype.private_message, + target_usernames: [unactivated_old_with_human_pm.username], ).create end @@ -1357,10 +1424,10 @@ describe User do let!(:group) { Fabricate(:group, - automatic_membership_email_domains: "bar.com|wat.com", - grant_trust_level: 1, - title: "bars and wats", - primary_group: true + automatic_membership_email_domains: "bar.com|wat.com", + grant_trust_level: 1, + title: "bars and wats", + primary_group: true ) } @@ -1394,10 +1461,10 @@ describe User do it "get attributes from the group" do user = Fabricate.build(:user, - active: true, - trust_level: 0, - email: "foo@bar.com", - password: "strongpassword4Uguys" + active: true, + trust_level: 0, + email: "foo@bar.com", + password: "strongpassword4Uguys" ) user.password_required! @@ -1659,8 +1726,8 @@ describe User do end.first expect(message.data[:recent]).to eq([ - [notification2.id, true], [notification.id, false] - ]) + [notification2.id, true], [notification.id, false] + ]) end end @@ -2015,4 +2082,26 @@ describe User do expect(Discourse.system_user).not_to be_human end end + + context "Unicode username" do + before { SiteSetting.unicode_usernames = true } + + let(:user) { Fabricate(:user, username: "Lo\u0308we") } # NFD + + it "normalizes usernames" do + expect(user.username).to eq("L\u00F6we") # NFC + expect(user.username_lower).to eq("l\u00F6we") # NFC + end + + describe ".username_exists?" do + it "normalizes username before executing query" do + expect(User.username_exists?(user.username)).to eq(true) + expect(User.username_exists?("Lo\u0308we")).to eq(true) # NFD + expect(User.username_exists?("L\u00F6we")).to eq(true) # NFC + expect(User.username_exists?("LO\u0308WE")).to eq(true) # NFD + expect(User.username_exists?("l\u00D6wE")).to eq(true) # NFC + expect(User.username_exists?("foo")).to eq(false) + end + end + end end diff --git a/spec/models/username_validator_spec.rb b/spec/models/username_validator_spec.rb index 700cea48505..e85f613220d 100644 --- a/spec/models/username_validator_spec.rb +++ b/spec/models/username_validator_spec.rb @@ -1,17 +1,205 @@ require 'rails_helper' describe UsernameValidator do - context "#valid_format?" do - it 'returns true when username is both valid and available' do - expect(UsernameValidator.new('Available').valid_format?).to eq true + def expect_valid(*usernames) + usernames.each do |username| + validator = UsernameValidator.new(username) + + aggregate_failures do + expect(validator.valid_format?).to eq(true), "expected '#{username}' to be valid" + expect(validator.errors).to be_empty + end + end + end + + def expect_invalid(*usernames, error_message:) + usernames.each do |username| + validator = UsernameValidator.new(username) + + aggregate_failures do + expect(validator.valid_format?).to eq(false), "expected '#{username}' to be invalid" + expect(validator.errors).to include(error_message) + end + end + end + + shared_examples 'ASCII username' do + it 'is invalid when the username is blank' do + expect_invalid('', error_message: I18n.t(:'user.username.blank')) end - it 'returns true when the username is valid but not available' do - expect(UsernameValidator.new(Fabricate(:user).username).valid_format?).to eq true + it 'is invalid when the username is too short' do + SiteSetting.min_username_length = 4 + + expect_invalid('a', 'ab', 'abc', + error_message: I18n.t(:'user.username.short', min: 4)) end - it 'returns false when the username is not valid' do - expect(UsernameValidator.new('not valid.name').valid_format?).to eq false + it 'is valid when the username has the minimum lenght' do + SiteSetting.min_username_length = 4 + + expect_valid('abcd') + end + + it 'is invalid when the username is too long' do + SiteSetting.max_username_length = 8 + + expect_invalid('abcdefghi', + error_message: I18n.t(:'user.username.long', max: 8)) + end + + it 'is valid when the username has the maximum lenght' do + SiteSetting.max_username_length = 8 + + expect_valid('abcdefgh') + end + + it 'is valid when the username contains alphanumeric characters, dots, underscores and dashes' do + expect_valid('ab-cd.123_ABC-xYz') + end + + it 'is invalid when the username contains non-alphanumeric characters other than dots, underscores and dashes' do + expect_invalid('abc|', 'a#bc', 'abc xyz', + error_message: I18n.t(:'user.username.characters')) + end + + it 'is valid when the username starts with a alphanumeric character or underscore' do + expect_valid('abcd', '1abc', '_abc') + end + + it 'is invalid when the username starts with a dot or dash' do + expect_invalid('.abc', '-abc', + error_message: I18n.t(:'user.username.must_begin_with_alphanumeric_or_underscore')) + end + + it 'is valid when the username ends with a alphanumeric character' do + expect_valid('abcd', 'abc9') + end + + it 'is invalid when the username ends with an underscore, a dot or dash' do + expect_invalid('abc_', 'abc.', 'abc-', + error_message: I18n.t(:'user.username.must_end_with_alphanumeric')) + end + + it 'is invalid when the username contains consecutive underscores, dots or dashes' do + expect_invalid('a__bc', 'a..bc', 'a--bc', + error_message: I18n.t(:'user.username.must_not_contain_two_special_chars_in_seq')) + end + + it 'is invalid when the username ends with certain file extensions' do + expect_invalid('abc.json', 'abc.png', + error_message: I18n.t(:'user.username.must_not_end_with_confusing_suffix')) + end + end + + context 'when Unicode usernames are disabled' do + before { SiteSetting.unicode_usernames = false } + + include_examples 'ASCII username' + + it 'is invalid when the username contains non-ASCII characters except dots, underscores and dashes' do + expect_invalid('abcö', 'abc象', + error_message: I18n.t(:'user.username.characters')) + end + end + + context 'when Unicode usernames are enabled' do + before { SiteSetting.unicode_usernames = true } + + context "ASCII usernames" do + include_examples 'ASCII username' + end + + context "Unicode usernames" do + before { SiteSetting.min_username_length = 1 } + + it 'is invalid when the username is too short' do + SiteSetting.min_username_length = 3 + + expect_invalid('鳥', 'পাখি', + error_message: I18n.t(:'user.username.short', min: 3)) + end + + it 'is valid when the username has the minimum lenght' do + SiteSetting.min_username_length = 2 + + expect_valid('পাখি', 'طائر') + end + + it 'is invalid when the username is too long' do + SiteSetting.max_username_length = 8 + + expect_invalid('חוטב_עצים', 'Holzfäller', + error_message: I18n.t(:'user.username.long', max: 8)) + end + + it 'is valid when the username has the maximum lenght' do + SiteSetting.max_username_length = 9 + + expect_valid('Дровосек', 'چوب-لباسی', 'தமிழ்-தமிழ்') + end + + it 'is invalid when the username has too many Unicode codepoints' do + SiteSetting.max_username_length = 30 + + expect_invalid('য়ায়ায়ায়ায়ায়ায়ায়ায়ায়ায়ায়ায়ায়ায়ায়ায়ায়ায়ায়ায়ায়ায়ায়ায়া', + error_message: I18n.t(:'user.username.too_long')) + end + + it 'is valid when the username contains Unicode letters' do + expect_valid('鳥', 'طائر', 'թռչուն', 'πουλί', 'পাখি', 'madár', '새', + 'پرنده', 'птица', 'fågel', 'นก', 'پرندے', 'ציפור') + end + + it 'is valid when the username contains numbers from the Nd or Nl Unicode category' do + expect_valid('arabic٠١٢٣٤٥٦٧٨٩', 'bengali০১২৩৪৫৬৭৮৯', 'romanⅥ', 'hangzhou〺') + end + + it 'is invalid when the username contains numbers from the No Unicode category' do + expect_invalid('circled㊸', 'fraction¾', + error_message: I18n.t(:'user.username.characters')) + end + + it 'is invalid when the username contains symbols or emojis' do + SiteSetting.min_username_length = 1 + + expect_invalid('©', '⇨', '“', '±', '‿', '😃', '🚗', + error_message: I18n.t(:'user.username.characters')) + end + + it 'is invalid when the username contains zero width join characters' do + expect_invalid('ണ്', 'র্যাম', + error_message: I18n.t(:'user.username.characters')) + end + + it 'is valid when the username ends with a Unicode Mark' do + expect_valid('தமிழ்') + end + + it 'allows all Unicode letters when the whitelist is empty' do + expect_valid('鳥') + end + + context "with Unicode whitelist" do + before { SiteSetting.unicode_username_character_whitelist = "[äöüÄÖÜß]" } + + it 'is invalid when username contains non-whitelisted letters' do + expect_invalid('鳥', 'francès', error_message: I18n.t(:'user.username.characters')) + end + + it 'is valid when username contains only whitelisted letters' do + expect_valid('Löwe', 'Ötzi') + end + + it 'is valid when username contains only ASCII letters and numbers regardless of whitelist' do + expect_valid('a-z_A-Z.0-9') + end + + it 'is valid after resetting the site setting' do + SiteSetting.unicode_username_character_whitelist = "" + expect_valid('鳥') + end + end end end end diff --git a/spec/services/username_changer_spec.rb b/spec/services/username_changer_spec.rb index 15bb760a13f..ef3ce2f9cb1 100644 --- a/spec/services/username_changer_spec.rb +++ b/spec/services/username_changer_spec.rb @@ -97,7 +97,7 @@ describe UsernameChanger do block.call(post) if block - UsernameChanger.change(user, 'bar') + UsernameChanger.change(user, args[:target_username] || 'bar') post.reload end @@ -130,13 +130,13 @@ describe UsernameChanger do expect(post.raw).to eq(".@bar -@bar %@bar _@bar ,@bar ;@bar @@bar") expect(post.cooked).to match_html(<<~HTML) -.@bar - -@bar - %@bar - _@bar - ,@bar - ;@bar - @@bar
+.@bar + -@bar + %@bar + _@bar + ,@bar + ;@bar + @@bar
HTML end @@ -147,38 +147,55 @@ describe UsernameChanger do expect(post.cooked).to eq(%Q()) end - it 'replaces mentions when there are trailing symbols' do - post = create_post_and_change_username(raw: "@foo. @foo, @foo: @foo; @foo-") + it 'replaces Markdown formatted mentions' do + post = create_post_and_change_username(raw: "**@foo** *@foo* _@foo_ ~~@foo~~") - expect(post.raw).to eq("@bar. @bar, @bar: @bar; @bar-") + expect(post.raw).to eq("**@bar** *@bar* _@bar_ ~~@bar~~") expect(post.cooked).to match_html(<<~HTML) -@bar. - @bar, - @bar: - @bar; - @bar-
+ HTML end - it 'does not replace mention when followed by an underscore' do - post = create_post_and_change_username(raw: "@foo_") + it 'replaces mentions when there are trailing symbols' do + post = create_post_and_change_username(raw: "@foo. @foo, @foo: @foo; @foo_ @foo-") - expect(post.raw).to eq("@foo_") - expect(post.cooked).to eq(%Q(@foo_
)) + expect(post.raw).to eq("@bar. @bar, @bar: @bar; @bar_ @bar-") + expect(post.cooked).to match_html(<<~HTML) +@bar. + @bar, + @bar: + @bar; + @bar_ + @bar-
+ HTML + end + + it 'does not replace mention in cooked when mention contains a trailing underscore' do + # Older versions of Discourse detected a trailing underscore as part of a username. + # That doesn't happen anymore, so we need to do create the `cooked` for this test manually. + post = create_post_and_change_username(raw: "@foobar @foo") do |p| + p.update_columns(raw: p.raw.gsub("@foobar", "@foo_"), cooked: p.cooked.gsub("@foobar", "@foo_")) + end + + expect(post.raw).to eq("@bar_ @bar") + expect(post.cooked).to eq(%Q(@foo_ @bar
)) end it 'does not replace mentions when there are leading alphanumeric chars' do - post = create_post_and_change_username(raw: "a@foo 2@foo") + post = create_post_and_change_username(raw: "@foo a@foo 2@foo") - expect(post.raw).to eq("a@foo 2@foo") - expect(post.cooked).to eq(%Q(a@foo 2@foo
)) + expect(post.raw).to eq("@bar a@foo 2@foo") + expect(post.cooked).to eq(%Q(@bar a@foo 2@foo
)) end it 'does not replace username within email address' do - post = create_post_and_change_username(raw: "mail@foo.com") + post = create_post_and_change_username(raw: "@foo mail@foo.com") - expect(post.raw).to eq("mail@foo.com") - expect(post.cooked).to eq(%Q()) + expect(post.raw).to eq("@bar mail@foo.com") + expect(post.cooked).to eq(%Q()) end it 'does not replace username in a mention of a similar username' do @@ -191,11 +208,11 @@ describe UsernameChanger do expect(post.raw).to eq("@bar @foobar @foo-bar @foo_bar @foo1") expect(post.cooked).to match_html(<<~HTML) -@bar - @foobar - @foo-bar - @foo_bar - @foo1
+@bar + @foobar + @foo-bar + @foo_bar + @foo1
HTML end @@ -253,6 +270,44 @@ describe UsernameChanger do expect(post.raw).to eq('@bar and @someuser') expect(post.cooked).to match_html('') end + + context "Unicode usernames" do + before { SiteSetting.unicode_usernames = true } + let(:user) { Fabricate(:user, username: 'թռչուն') } + + it 'it correctly updates mentions' do + post = create_post_and_change_username(raw: "Hello @թռչուն", target_username: 'птица') + + expect(post.raw).to eq("Hello @птица") + expect(post.cooked).to eq(%Q(Hello @птица
)) + end + + it 'does not replace mentions when there are leading alphanumeric chars' do + post = create_post_and_change_username(raw: "Hello @թռչուն 鳥@թռչուն 2@թռչուն ٩@թռչուն", target_username: 'птица') + + expect(post.raw).to eq("Hello @птица 鳥@թռչուն 2@թռչուն ٩@թռչուն") + expect(post.cooked).to eq(%Q(Hello @птица 鳥@թռչուն 2@թռչուն ٩@թռչուն
)) + end + + it 'does not replace username in a mention of a similar username' do + Fabricate(:user, username: 'թռչուն鳥') + Fabricate(:user, username: 'թռչուն-鳥') + Fabricate(:user, username: 'թռչուն_鳥') + Fabricate(:user, username: 'թռչուն٩') + + post = create_post_and_change_username(raw: "@թռչուն @թռչուն鳥 @թռչուն-鳥 @թռչուն_鳥 @թռչուն٩", target_username: 'птица') + + expect(post.raw).to eq("@птица @թռչուն鳥 @թռչուն-鳥 @թռչուն_鳥 @թռչուն٩") + expect(post.cooked).to match_html(<<~HTML) +@птица + @թռչուն鳥 + @թռչուն-鳥 + @թռչուն_鳥 + @թռչուն٩
+ HTML + end + + end end context 'quotes' do diff --git a/test/javascripts/helpers/site-settings.js b/test/javascripts/helpers/site-settings.js index c449c3c0348..6dd997e67c8 100644 --- a/test/javascripts/helpers/site-settings.js +++ b/test/javascripts/helpers/site-settings.js @@ -94,7 +94,8 @@ Discourse.SiteSettingsOriginal = { emoji_set: "emoji_one", desktop_category_page_style: "categories_and_latest_topics", enable_mentions: true, - enable_personal_messages: true + enable_personal_messages: true, + unicode_usernames: false }; Discourse.SiteSettings = jQuery.extend( true, diff --git a/test/javascripts/lib/pretty-text-test.js.es6 b/test/javascripts/lib/pretty-text-test.js.es6 index 9495122a87c..90c503f2c46 100644 --- a/test/javascripts/lib/pretty-text-test.js.es6 +++ b/test/javascripts/lib/pretty-text-test.js.es6 @@ -555,6 +555,48 @@ QUnit.test("Mentions", assert => { 'a @sam c
', "it allows mentions within HTML tags" ); + + assert.cooked( + "@_sam @1sam @ab-cd.123_ABC-xYz @sam1", + '@_sam @1sam @ab-cd.123_ABC-xYz @sam1
', + "it detects mentions of valid usernames" + ); + + assert.cooked( + "@.sam @-sam @sam. @sam_ @sam-", + '@.sam @-sam @sam. @sam_ @sam-
', + "it does not detect mentions of invalid usernames" + ); + + assert.cookedOptions( + "Hello @狮子", + { siteSettings: { unicode_usernames: false } }, + "Hello @狮子
", + "it does not detect mentions of Unicode usernames" + ); +}); + +QUnit.test("Mentions - Unicode usernames enabled", assert => { + assert.cookedOptions( + "Hello @狮子", + { siteSettings: { unicode_usernames: true } }, + 'Hello @狮子
', + "it detects mentions of Unicode usernames" + ); + + assert.cookedOptions( + "@狮子 @_狮子 @1狮子 @狮-ø.١٢٣_Ö-ழ் @狮子1", + { siteSettings: { unicode_usernames: true } }, + '@狮子 @_狮子 @1狮子 @狮-ø.١٢٣_Ö-ழ் @狮子1
', + "it detects mentions of valid Unicode usernames" + ); + + assert.cookedOptions( + "@.狮子 @-狮子 @狮子. @狮子_ @狮子-", + { siteSettings: { unicode_usernames: true } }, + '@.狮子 @-狮子 @狮子. @狮子_ @狮子-
', + "it does not detect mentions of invalid Unicode usernames" + ); }); QUnit.test("Mentions - disabled", assert => {