From c133866cb7356616fb62fb99e2ce9be2e017ccc2 Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 6 Nov 2022 15:46:38 -0600 Subject: [PATCH] users: don't allow users to choose reserved names. Don't allow users to choose names that conflict with search syntax, like `any` or `none`, or names that impersonate user levels, like `Admin`, `Moderator`, `Anonymous`, etc. --- app/logical/user_name_validator.rb | 9 +++++++++ test/functional/bans_controller_test.rb | 4 ++-- test/functional/post_flags_controller_test.rb | 4 ++-- .../user_name_change_requests_controller_test.rb | 6 +++--- test/unit/post_vote_test.rb | 4 ++-- test/unit/user_test.rb | 7 +++++++ 6 files changed, 25 insertions(+), 9 deletions(-) diff --git a/app/logical/user_name_validator.rb b/app/logical/user_name_validator.rb index 1933c7c0d..4ac8368e9 100644 --- a/app/logical/user_name_validator.rb +++ b/app/logical/user_name_validator.rb @@ -9,6 +9,13 @@ class UserNameValidator < ActiveModel::EachValidator ALLOWED_PUNCTUATION = "_.-" # All other punctuation characters are forbidden + RESERVED_NAMES = [ + "any", "none", # conflicts with `approver:any` search syntax + "new", "deactivate", "custom_style", # conflicts with user routes (/users/new, /users/deactivate, /users/custom_style) + "mod", "administrator", # mod impersonation + *User::Roles.map(&:to_s) # owner, admin, moderator, anonymous, banned, etc + ] + def validate_each(rec, attr, name) forbidden_characters = name.delete(ALLOWED_PUNCTUATION).chars.grep(/[[:punct:]]/).uniq @@ -34,6 +41,8 @@ class UserNameValidator < ActiveModel::EachValidator 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.downcase.in?(RESERVED_NAMES) + rec.errors.add(attr, "is a reserved name and can't be used") elsif name =~ Regexp.union(Danbooru.config.user_name_blacklist) rec.errors.add(attr, "is not allowed") end diff --git a/test/functional/bans_controller_test.rb b/test/functional/bans_controller_test.rb index 3587ff27a..a72bf0b4a 100644 --- a/test/functional/bans_controller_test.rb +++ b/test/functional/bans_controller_test.rb @@ -32,7 +32,7 @@ class BansControllerTest < ActionDispatch::IntegrationTest context "index action" do setup do - @mod = create(:mod_user, name: "mod") + @mod = create(:mod_user, name: "mod123") @ban1 = create(:ban, created_at: 1.week.ago, duration: 1.day) @ban2 = create(:ban, user: build(:builder_user), reason: "blah", banner: @mod, duration: 100.years) end @@ -47,7 +47,7 @@ class BansControllerTest < ActionDispatch::IntegrationTest should respond_to_search(expired: "false").with { @ban2 } should respond_to_search(duration: "<1w").with { @ban1 } - should respond_to_search(banner_name: "mod").with { @ban2 } + should respond_to_search(banner_name: "mod123").with { @ban2 } should respond_to_search(banner: { level: User::Levels::MODERATOR }).with { @ban2 } end diff --git a/test/functional/post_flags_controller_test.rb b/test/functional/post_flags_controller_test.rb index 7ff7b425b..8e8826b36 100644 --- a/test/functional/post_flags_controller_test.rb +++ b/test/functional/post_flags_controller_test.rb @@ -6,7 +6,7 @@ class PostFlagsControllerTest < ActionDispatch::IntegrationTest @user = create(:user) @flagger = create(:gold_user, id: 999, created_at: 2.weeks.ago) @uploader = create(:mod_user, name: "chen", created_at: 2.weeks.ago) - @mod = create(:mod_user, name: "mod") + @mod = create(:mod_user, name: "mod123") @post = create(:post, id: 101, uploader: @uploader) @post_flag = create(:post_flag, reason: "xxx", post: @post, creator: @flagger) end @@ -113,7 +113,7 @@ class PostFlagsControllerTest < ActionDispatch::IntegrationTest @post_flag = create(:post_flag, creator: @mod, post: build(:post, uploader: @mod)) end - should respond_to_search(creator_name: "mod").with { @post_flag } + should respond_to_search(creator_name: "mod123").with { @post_flag } end context "when the user is the flagger" do diff --git a/test/functional/user_name_change_requests_controller_test.rb b/test/functional/user_name_change_requests_controller_test.rb index c3d688de1..974601970 100644 --- a/test/functional/user_name_change_requests_controller_test.rb +++ b/test/functional/user_name_change_requests_controller_test.rb @@ -67,18 +67,18 @@ class UserNameChangeRequestsControllerTest < ActionDispatch::IntegrationTest end should "fail for a moderator trying to change the name of someone above Builder level" do - @user = create(:moderator_user, name: "mod") + @user = create(:moderator_user, name: "bob") post_auth user_name_change_requests_path, create(:moderator_user), params: { user_name_change_request: { user_id: @user.id, desired_name: "zun" }} assert_response 403 - assert_equal("mod", @user.reload.name) + assert_equal("bob", @user.reload.name) end end context "show action" do setup do @change_request = as(@user) { create(:user_name_change_request, user_id: @user.id) } - @user.update!(name: "user_#{@user.id}") + @user.update!(is_deleted: true) end should "render" do diff --git a/test/unit/post_vote_test.rb b/test/unit/post_vote_test.rb index 46e3ee8e1..421c299a6 100644 --- a/test/unit/post_vote_test.rb +++ b/test/unit/post_vote_test.rb @@ -102,7 +102,7 @@ class PostVoteTest < ActiveSupport::TestCase context "deleting a vote by another user" do should "leave a mod action" do - admin = create(:admin_user, name: "admin") + admin = create(:admin_user) vote = create(:post_vote, post: @post, score: 1) vote.soft_delete!(updater: admin) @@ -114,7 +114,7 @@ class PostVoteTest < ActiveSupport::TestCase context "undeleting a vote by another user" do setup do - @admin = create(:admin_user, name: "admin") + @admin = create(:admin_user) @vote = create(:post_vote, post: @post, score: 1) @vote.soft_delete!(updater: @admin) diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index 39f495b1d..820d3ab3e 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -205,6 +205,13 @@ class UserTest < ActiveSupport::TestCase user.save assert_equal(["Name is not allowed"], user.errors.full_messages) end + + should_not allow_value("any").for(:name) + should_not allow_value("none").for(:name) + should_not allow_value("new").for(:name) + should_not allow_value("admin").for(:name) + should_not allow_value("mod").for(:name) + should_not allow_value("moderator").for(:name) end context "searching for users by name" do