diff --git a/app/logical/user_name_validator.rb b/app/logical/user_name_validator.rb index 8d44cb064..99ecce9dc 100644 --- a/app/logical/user_name_validator.rb +++ b/app/logical/user_name_validator.rb @@ -7,14 +7,33 @@ # # @see https://guides.rubyonrails.org/active_record_validations.html#custom-validators class UserNameValidator < ActiveModel::EachValidator - def validate_each(rec, attr, value) - name = value + ALLOWED_PUNCTUATION = "_.-" # All other punctuation characters are forbidden - rec.errors.add(attr, "already exists") if User.find_by_name(name).present? - rec.errors.add(attr, "must be more than 1 character long") if name.length <= 1 - rec.errors.add(attr, "must be less than 25 characters long") if name.length >= 25 - rec.errors.add(attr, "cannot have whitespace or colons") if name =~ /[[:space:]]|:/ - rec.errors.add(attr, "cannot begin or end with an underscore") if name =~ /\A_|_\z/ - rec.errors.add(attr, "is not allowed") if name =~ Regexp.union(Danbooru.config.user_name_blacklist) + def validate_each(rec, attr, name) + forbidden_characters = name.delete(ALLOWED_PUNCTUATION).chars.grep(/[[:punct:]]/).uniq + + if rec.new_record? && User.find_by_name(name).present? + rec.errors.add(attr, "already exists") + elsif name.length <= 1 + rec.errors.add(attr, "must be more than 1 character long") + elsif name.length >= 25 + rec.errors.add(attr, "must be less than 25 characters long") + elsif name =~ /[[:space:]]/ + rec.errors.add(attr, "can't contain whitespace") + elsif name =~ /\A[[:punct:]]/ + rec.errors.add(attr, "can't start with '#{name.first}'") + elsif name =~ /[[:punct:]]\z/ + rec.errors.add(attr, "can't end with '#{name.last}'") + elsif name =~ /__/ + rec.errors.add(attr, "can't contain multiple underscores in a row") + elsif forbidden_characters.present? + rec.errors.add(attr, "can't contain #{forbidden_characters.map { |c| "'#{c}'" }.to_sentence}") + elsif name !~ /\A([a-zA-Z0-9]|\p{Han}|\p{Hangul}|\p{Hiragana}|\p{Katakana}|[#{ALLOWED_PUNCTUATION}])+\z/ + rec.errors.add(attr, "must contain only basic letters or numbers") + elsif name =~ /\Auser_\d+\z/i + rec.errors.add(attr, "can't be the same as a deleted user") + elsif name =~ Regexp.union(Danbooru.config.user_name_blacklist) + rec.errors.add(attr, "is not allowed") + end end end diff --git a/app/models/user.rb b/app/models/user.rb index 7a55daa9d..323fe94d2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class User < ApplicationRecord + extend Memoist + class PrivilegeError < StandardError; end module Levels @@ -204,6 +206,18 @@ class User < ApplicationRecord errors.add(:base, "Can't enable privacy mode without a Gold account") end end + + def name_errors + User.validators_on(:name).each do |validator| + validator.validate_each(self, :name, name) + end + + errors + end + + def name_invalid? + name_errors.present? + end end concerning :AuthenticationMethods do @@ -695,4 +709,6 @@ class User < ApplicationRecord def self.available_includes [:inviter] end + + memoize :name_errors end diff --git a/app/views/layouts/default.html.erb b/app/views/layouts/default.html.erb index fe035f1fb..5c45e848c 100644 --- a/app/views/layouts/default.html.erb +++ b/app/views/layouts/default.html.erb @@ -104,6 +104,13 @@ <%= render "users/dmail_notice" %> <% end %> + <% if !CurrentUser.user.is_anonymous? && CurrentUser.user.name_invalid? %> +
+

Action required

+
You must <%= link_to "change your username", new_user_name_change_request_path %> to continue using <%= Danbooru.config.canonical_app_name %>.
+
+ <% end %> +
"> <%= format_text(flash[:notice], inline: true) %>. close diff --git a/app/views/user_name_change_requests/new.html.erb b/app/views/user_name_change_requests/new.html.erb index b1be8501d..0c7698c55 100644 --- a/app/views/user_name_change_requests/new.html.erb +++ b/app/views/user_name_change_requests/new.html.erb @@ -1,13 +1,32 @@
-
-

Name Change Request

+
+

Change Name

-

You can request a name change once per week. Your previous names will still - be visible on your profile to other Danbooru members, but they won't be visible - to search engines.

+ <% if CurrentUser.user.name_invalid? %> +

Your current username is invalid. You must change your username to continue + using <%= Danbooru.config.canonical_app_name %>.

+ +

+ Current name: <%= CurrentUser.user.name %>.
+ Error: <%= CurrentUser.user.name_errors.full_messages.join(". ").html_safe %>. +

+ <% end %> + +
+
Rules
+
    +
  • Names can contain only letters, numbers, underscore ('_'), period ('.'), and dash ('-') characters.
  • +
  • Names must start and end with a letter or number.
  • +
  • Names must be less than 25 characters long.
  • +
  • Names can't insult or impersonate other users.
  • +
  • Names are case-insensitive.
  • +
  • Your previous names will be visible on your profile to other Danbooru members, but they won't be visible to search engines.
  • +
  • You can't change your name more than once per week.
  • +
+
<%= edit_form_for(@change_request) do |f| %> - <%= f.input :desired_name, label: "Name" %> + <%= f.input :desired_name, label: "New name" %> <%= f.input :desired_name_confirmation, label: "Confirm name" %> <%= f.submit "Submit", "data-confirm": "Are you sure you want to change your name?" %> <% end %> diff --git a/test/functional/application_controller_test.rb b/test/functional/application_controller_test.rb index 53ff6a86e..8fa07e359 100644 --- a/test/functional/application_controller_test.rb +++ b/test/functional/application_controller_test.rb @@ -75,6 +75,17 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest end end + context "when a user has an invalid username" do + should "show a warning banner" do + @user = create(:user) + @user.update_columns(name: "foo__bar") + + get_auth posts_path, @user + assert_response :success + assert_select "#invalid-name-notice" + end + end + context "on api authentication" do setup do @user = create(:user, password: "password") diff --git a/test/functional/user_name_change_requests_controller_test.rb b/test/functional/user_name_change_requests_controller_test.rb index b9276d24b..d8e6abf40 100644 --- a/test/functional/user_name_change_requests_controller_test.rb +++ b/test/functional/user_name_change_requests_controller_test.rb @@ -12,6 +12,13 @@ class UserNameChangeRequestsControllerTest < ActionDispatch::IntegrationTest get_auth new_user_name_change_request_path, @user assert_response :success end + + should "render when the current user's name is invalid" do + @user.update_columns(name: "foo__bar") + get_auth new_user_name_change_request_path, @user + + assert_response :success + end end context "create action" do diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index 1006bc6cb..3bcbf157c 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -148,27 +148,51 @@ class UserTest < ActiveSupport::TestCase context "name" do should "not contain whitespace" do # U+2007: https://en.wikipedia.org/wiki/Figure_space - user = FactoryBot.build(:user, :name => "foo\u2007bar") + user = build(:user, name: "foo\u2007bar") user.save - assert_equal(["Name cannot have whitespace or colons"], user.errors.full_messages) + assert_equal(["Name can't contain whitespace"], user.errors.full_messages) + end + + should "be less than 25 characters long" do + user = build(:user, name: "a"*25) + user.save + assert_equal(["Name must be less than 25 characters long"], user.errors.full_messages) end should "not contain a colon" do - user = FactoryBot.build(:user, :name => "a:b") + user = build(:user, name: "a:b") user.save - assert_equal(["Name cannot have whitespace or colons"], user.errors.full_messages) + assert_equal(["Name can't contain ':'"], user.errors.full_messages) end should "not begin with an underscore" do - user = FactoryBot.build(:user, :name => "_x") + user = build(:user, name: "_x") user.save - assert_equal(["Name cannot begin or end with an underscore"], user.errors.full_messages) + assert_equal(["Name can't start with '_'"], user.errors.full_messages) end should "not end with an underscore" do - user = FactoryBot.build(:user, :name => "x_") + user = build(:user, name: "x_") user.save - assert_equal(["Name cannot begin or end with an underscore"], user.errors.full_messages) + assert_equal(["Name can't end with '_'"], user.errors.full_messages) + end + + should "not contain consecutive underscores" do + user = build(:user, name: "x__y") + user.save + assert_equal(["Name can't contain multiple underscores in a row"], user.errors.full_messages) + end + + should "not allow non-ASCII characters" do + user = build(:user, name: "Pokémon") + user.save + assert_equal(["Name must contain only basic letters or numbers"], user.errors.full_messages) + end + + should "not be in the same format as a deleted user" do + user = build(:user, name: "user_1234") + user.save + assert_equal(["Name can't be the same as a deleted user"], user.errors.full_messages) end should "not allow blacklisted names" do @@ -177,11 +201,6 @@ class UserTest < ActiveSupport::TestCase user.save assert_equal(["Name is not allowed"], user.errors.full_messages) end - - should "be updated" do - @user = FactoryBot.create(:user) - @user.update_attribute(:name, "danzig") - end end context "searching for users by name" do @@ -206,10 +225,14 @@ class UserTest < ActiveSupport::TestCase assert_nil(User.name_to_id("does_not_exist")) end - should "work for names containing asterisks or backlashes" do - @user1 = create(:user, name: "user*1") - @user2 = create(:user, name: "user*2") - @user3 = create(:user, name: "user\*3") + should "work for names containing asterisks or backslashes" do + @user1 = build(:user, name: "user*1") + @user2 = build(:user, name: "user*2") + @user3 = build(:user, name: "user\*3") + + @user1.save(validate: false) + @user2.save(validate: false) + @user3.save(validate: false) assert_equal(@user1.id, User.find_by_name("user*1").id) assert_equal(@user2.id, User.find_by_name("user*2").id) @@ -259,9 +282,13 @@ class UserTest < ActiveSupport::TestCase context "when searched by name" do should "match wildcards" do - user1 = FactoryBot.create(:user, :name => "foo") - user2 = FactoryBot.create(:user, :name => "foo*bar") - user3 = FactoryBot.create(:user, :name => "bar\*baz") + user1 = build(:user, name: "foo") + user2 = build(:user, name: "foo*bar") + user3 = build(:user, name: "bar\*baz") + + user1.save(validate: false) + user2.save(validate: false) + user3.save(validate: false) assert_equal([user2.id, user1.id], User.search(name: "foo*").map(&:id)) assert_equal([user2.id], User.search(name: "foo\*bar").map(&:id))