Require user names to be unique after unicode normalisation

As with the previous checks on case sensitivity this only affects
new users, and changes to names of existing users.
This commit is contained in:
Tom Hughes 2023-12-13 20:53:38 +00:00
parent 75042a04a1
commit c12f8959dd
5 changed files with 49 additions and 12 deletions

View file

@ -34,12 +34,13 @@
#
# Indexes
#
# users_auth_idx (auth_provider,auth_uid) UNIQUE
# users_display_name_idx (display_name) UNIQUE
# users_display_name_lower_idx (lower((display_name)::text))
# users_email_idx (email) UNIQUE
# users_email_lower_idx (lower((email)::text))
# users_home_idx (home_tile)
# users_auth_idx (auth_provider,auth_uid) UNIQUE
# users_display_name_canonical_idx (lower(NORMALIZE(display_name, NFKC)))
# users_display_name_idx (display_name) UNIQUE
# users_display_name_lower_idx (lower((display_name)::text))
# users_email_idx (email) UNIQUE
# users_email_lower_idx (lower((email)::text))
# users_home_idx (home_tile)
#
class User < ApplicationRecord
@ -91,7 +92,7 @@ class User < ApplicationRecord
validates :display_name, :presence => true, :length => 3..255,
:exclusion => %w[new terms save confirm confirm-email go_public reset-password forgot-password suspended]
validates :display_name, :if => proc { |u| u.display_name_changed? },
:uniqueness => { :case_sensitive => false }
:normalized_uniqueness => { :case_sensitive => false }
validates :display_name, :if => proc { |u| u.display_name_changed? },
:characters => { :url_safe => true },
:whitespace => { :leading => false, :trailing => false }
@ -129,7 +130,7 @@ class User < ApplicationRecord
user = find_by("email = ? OR display_name = ?", options[:username].strip, options[:username])
if user.nil?
users = where("LOWER(email) = LOWER(?) OR LOWER(display_name) = LOWER(?)", options[:username].strip, options[:username])
users = where("LOWER(email) = LOWER(?) OR LOWER(NORMALIZE(display_name, NFKC)) = LOWER(NORMALIZE(?, NFKC))", options[:username].strip, options[:username])
user = users.first if users.count == 1
end

View file

@ -0,0 +1,18 @@
class NormalizedUniquenessValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value)
relation = if options.fetch(:case_sensitive, true)
record.class.where("NORMALIZE(#{attribute}, NFKC) = NORMALIZE(?, NFKC)", value)
else
record.class.where("LOWER(NORMALIZE(#{attribute}, NFKC)) = LOWER(NORMALIZE(?, NFKC))", value)
end
relation = relation.where.not(record.class.primary_key => [record.id_in_database]) if record.persisted?
if relation.exists?
error_options = options.except(:case_sensitive)
error_options[:value] = value
record.errors.add(attribute, :taken, **error_options)
end
end
end

View file

@ -0,0 +1,7 @@
class AddCanonicalUserIndex < ActiveRecord::Migration[7.1]
disable_ddl_transaction!
def change
add_index :users, "LOWER(NORMALIZE(display_name, NFKC))", :name => "users_display_name_canonical_idx", :algorithm => :concurrently
end
end

View file

@ -2867,6 +2867,13 @@ CREATE INDEX user_tokens_user_id_idx ON public.user_tokens USING btree (user_id)
CREATE UNIQUE INDEX users_auth_idx ON public.users USING btree (auth_provider, auth_uid);
--
-- Name: users_display_name_canonical_idx; Type: INDEX; Schema: public; Owner: -
--
CREATE INDEX users_display_name_canonical_idx ON public.users USING btree (lower(NORMALIZE(display_name, NFKC)));
--
-- Name: users_display_name_idx; Type: INDEX; Schema: public; Owner: -
--
@ -3510,6 +3517,7 @@ INSERT INTO "schema_migrations" (version) VALUES
('23'),
('22'),
('21'),
('20231213182102'),
('20231206141457'),
('20231117170422'),
('20231101222146'),

View file

@ -27,10 +27,13 @@ class UserTest < ActiveSupport::TestCase
end
def test_unique_display_name
existing_user = create(:user)
new_user = build(:user, :display_name => existing_user.display_name)
assert_not new_user.save
assert_includes new_user.errors[:display_name], "has already been taken"
create(:user, :display_name => "H\u{e9}nryIV")
%W[H\u{e9}nryIV he\u{301}nryiv H\u{c9}nry\u2163 he\u{301}nry\u2173].each do |name|
new_user = build(:user, :display_name => name)
assert_not new_user.save
assert_includes new_user.errors[:display_name], "has already been taken"
end
end
def test_email_valid