From 99846b7e3ddf6cc6775d4bda9efc206683ff4ca4 Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 1 Oct 2022 23:13:21 -0500 Subject: [PATCH] users: allow mods to change the names of other users. Allow moderators to forcibly change the username of other users. This is so mods can change abusive or invalid usernames. * A mod can only change the username of Builder-level users and below. * The user can't change their own name again until one week has passed. * A modaction is logged when a mod changes a user's name. * A dmail is sent to the user notifying them of the change. * The dmail does not send the user an email notification. This is so we don't spam users if their name is changed after they're banned, or if they haven't visited the site in a long time. The rename button is on the user's profile page, and when you hover over the user's name and open the "..." menu. --- app/controllers/application_controller.rb | 2 +- .../user_name_change_requests_controller.rb | 12 +++-- app/models/dmail.rb | 4 +- app/models/user_name_change_request.rb | 20 +++++++- .../user_name_change_request_policy.rb | 14 +++++- app/views/layouts/default.html.erb | 2 +- app/views/static/site_map.html.erb | 3 -- .../_secondary_links.html.erb | 2 +- .../user_name_change_requests/new.html.erb | 7 ++- .../user_name_change_requests/show.html.erb | 1 - app/views/users/_secondary_links.html.erb | 4 ++ app/views/users/edit.html.erb | 2 +- app/views/users/show.html+tooltip.erb | 8 +++ config/routes.rb | 5 +- .../functional/application_controller_test.rb | 2 +- ...er_name_change_requests_controller_test.rb | 49 ++++++++++++++++--- 16 files changed, 107 insertions(+), 30 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index e68c60f3f..32811a98f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -217,7 +217,7 @@ class ApplicationController < ActionController::Base def redirect_if_name_invalid? if request.format.html? && !CurrentUser.user.is_anonymous? && CurrentUser.user.name_invalid? flash[:notice] = "You must change your username to continue using #{Danbooru.config.app_name}" - redirect_to new_user_name_change_request_path + redirect_to change_name_user_path(CurrentUser.user) end end diff --git a/app/controllers/user_name_change_requests_controller.rb b/app/controllers/user_name_change_requests_controller.rb index 8953f91b8..a9ed1a234 100644 --- a/app/controllers/user_name_change_requests_controller.rb +++ b/app/controllers/user_name_change_requests_controller.rb @@ -6,15 +6,17 @@ class UserNameChangeRequestsController < ApplicationController skip_before_action :redirect_if_name_invalid? def new - @change_request = authorize UserNameChangeRequest.new(permitted_attributes(UserNameChangeRequest)) + user = User.find_by_name(params[:id]) || CurrentUser.user + @change_request = authorize UserNameChangeRequest.new(user: user, **permitted_attributes(UserNameChangeRequest)) respond_with(@change_request) end def create - @change_request = authorize UserNameChangeRequest.new(user: CurrentUser.user, original_name: CurrentUser.user.name) - @change_request.update(permitted_attributes(@change_request)) - flash[:notice] = "Your name has been changed" if @change_request.valid? - respond_with(@change_request, location: profile_path) + user = User.find(params.dig(:user_name_change_request, :user_id)) + @change_request = authorize UserNameChangeRequest.new(updater: CurrentUser.user, original_name: user.name, **permitted_attributes(UserNameChangeRequest)) + @change_request.save + flash[:notice] = "Name changed" if @change_request.valid? + respond_with(@change_request, location: @change_request.user) end def show diff --git a/app/models/dmail.rb b/app/models/dmail.rb index de68e9bbf..1b6cbf788 100644 --- a/app/models/dmail.rb +++ b/app/models/dmail.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Dmail < ApplicationRecord - attr_accessor :creator_ip_addr + attr_accessor :creator_ip_addr, :disable_email_notifications validate :validate_sender_is_not_limited, on: :create validates :title, presence: true, length: { maximum: 200 }, if: :title_changed? @@ -145,7 +145,7 @@ class Dmail < ApplicationRecord end def send_email - if is_recipient? && !is_deleted? && to.receive_email_notifications? + if is_recipient? && !is_deleted? && to.receive_email_notifications? && !disable_email_notifications UserMailer.with(headers: { "X-Danbooru-Dmail": Routes.dmail_url(self) }).dmail_notice(self).deliver_later end end diff --git a/app/models/user_name_change_request.rb b/app/models/user_name_change_request.rb index 494385b50..2c6f94e39 100644 --- a/app/models/user_name_change_request.rb +++ b/app/models/user_name_change_request.rb @@ -2,13 +2,16 @@ class UserNameChangeRequest < ApplicationRecord belongs_to :user - belongs_to :approver, class_name: "User", optional: true + + attr_accessor :updater validate :not_limited, on: :create validates :original_name, presence: true validates :desired_name, user_name: true, presence: true, on: :create after_create :update_name! + after_create :create_mod_action + after_create :send_dmail def self.visible(user) if user.is_moderator? @@ -31,9 +34,22 @@ class UserNameChangeRequest < ApplicationRecord def not_limited return if user.name_invalid? + return if updater && updater != user - if UserNameChangeRequest.unscoped.where(user: user).exists?(["created_at >= ?", 1.week.ago]) + if user.user_name_change_requests.exists?(created_at: (1.week.ago..)) errors.add(:base, "You can only submit one name change request per week") end end + + def create_mod_action + return if updater.nil? || user == updater + ModAction.log("changed user ##{user.id}'s name from #{original_name} to #{desired_name}", :user_name_change, subject: user, user: updater) + end + + def send_dmail + return if updater.nil? || user == updater + Dmail.create_automated(to: user, disable_email_notifications: true, title: "Your username has been changed", body: <<~EOS) + Your username has been changed from #{original_name} to #{desired_name}. Your old name was either no longer valid or it violated site rules. You can change it to something else after one week. Please make sure your name follows the [[help:community rules|community rules]]. + EOS + end end diff --git a/app/policies/user_name_change_request_policy.rb b/app/policies/user_name_change_request_policy.rb index 84078526e..ea7460e07 100644 --- a/app/policies/user_name_change_request_policy.rb +++ b/app/policies/user_name_change_request_policy.rb @@ -9,7 +9,17 @@ class UserNameChangeRequestPolicy < ApplicationPolicy user.is_moderator? || (!user.is_anonymous? && !record.user.is_deleted?) || (record.user == user) end - def permitted_attributes - [:desired_name] + def create? + !user.is_anonymous? && (name_change.user == user || can_rename_user?) end + + def can_rename_user? + user.is_moderator? && name_change.user.level < User::Levels::MODERATOR + end + + def permitted_attributes_for_create + [:user_id, :desired_name] + end + + alias_method :name_change, :record end diff --git a/app/views/layouts/default.html.erb b/app/views/layouts/default.html.erb index 28a82a3f2..77bc85df8 100644 --- a/app/views/layouts/default.html.erb +++ b/app/views/layouts/default.html.erb @@ -160,7 +160,7 @@ <% 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 %>.
+
You must <%= link_to "change your username", change_name_user_path(CurrentUser.user) %> to continue using <%= Danbooru.config.canonical_app_name %>.
<% end %> diff --git a/app/views/static/site_map.html.erb b/app/views/static/site_map.html.erb index cb811ed8a..cd2407b29 100644 --- a/app/views/static/site_map.html.erb +++ b/app/views/static/site_map.html.erb @@ -131,9 +131,6 @@ <% else %>
  • <%= link_to "Profile", profile_path %>
  • <%= link_to "Settings", settings_path %>
  • - <% if policy(UserNameChangeRequest).create? %> -
  • <%= link_to "Change name", new_user_name_change_request_path %>
  • - <% end %>
  • <%= link_to "Uploads", user_uploads_path(CurrentUser.user) %>
  • <%= link_to "Dmails", dmails_path(search: { folder: "received" }) %>
  • <%= link_to "Favorites", favorites_path %>
  • diff --git a/app/views/user_name_change_requests/_secondary_links.html.erb b/app/views/user_name_change_requests/_secondary_links.html.erb index 47803bb0c..569308136 100644 --- a/app/views/user_name_change_requests/_secondary_links.html.erb +++ b/app/views/user_name_change_requests/_secondary_links.html.erb @@ -1,5 +1,5 @@ <% content_for(:secondary_links) do %> <%= subnav_link_to "Listing", user_name_change_requests_path %> - <%= subnav_link_to "New", new_user_name_change_request_path %> + <%= subnav_link_to "New", change_name_user_path(CurrentUser.user) unless CurrentUser.user.is_anonymous? %> <%= subnav_link_to "Help", wiki_page_path("help:user_name_change_requests") %> <% end %> diff --git a/app/views/user_name_change_requests/new.html.erb b/app/views/user_name_change_requests/new.html.erb index 5d8270f19..a29db248b 100644 --- a/app/views/user_name_change_requests/new.html.erb +++ b/app/views/user_name_change_requests/new.html.erb @@ -1,6 +1,10 @@
    -

    Change Name

    + <% if CurrentUser.user != @change_request.user %> +

    Change Name: <%= link_to_user @change_request.user %>

    + <% else %> +

    Change Name

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

    Your current username is invalid. You must change your username to continue @@ -26,6 +30,7 @@

    <%= edit_form_for(@change_request) do |f| %> + <%= f.input :user_id, as: :hidden, input_html: { value: @change_request.user_id } %> <%= f.input :desired_name, label: "New name" %> <%= f.submit "Submit" %> <% end %> diff --git a/app/views/user_name_change_requests/show.html.erb b/app/views/user_name_change_requests/show.html.erb index e95f2bc96..6750b1066 100644 --- a/app/views/user_name_change_requests/show.html.erb +++ b/app/views/user_name_change_requests/show.html.erb @@ -8,7 +8,6 @@ Date <%= compact_time @change_request.created_at %> - <%= render "application/update_notice", record: @change_request %> diff --git a/app/views/users/_secondary_links.html.erb b/app/views/users/_secondary_links.html.erb index 66580a933..fd5b4bce3 100644 --- a/app/views/users/_secondary_links.html.erb +++ b/app/views/users/_secondary_links.html.erb @@ -27,6 +27,10 @@ <%= subnav_link_to "Promote", edit_admin_user_path(@user) %> <% end %> + <% if policy(UserNameChangeRequest.new(user: @user)).can_rename_user? %> + <%= subnav_link_to "Rename", change_name_user_path(@user) %> + <% end %> + <% if policy(Ban.new(user: @user)).create? %> <% if @user.is_banned? && @user.active_ban.present? %> <%= subnav_link_to "Unban", ban_path(@user.active_ban) %> diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index 351ba0a2b..046ebfe7f 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -14,7 +14,7 @@
    -

    <%= link_to "Change your name", new_user_name_change_request_path %>

    +

    <%= link_to "Change your name", change_name_user_path(@user) %>

    diff --git a/app/views/users/show.html+tooltip.erb b/app/views/users/show.html+tooltip.erb index 75c83966d..63849a4fc 100644 --- a/app/views/users/show.html+tooltip.erb +++ b/app/views/users/show.html+tooltip.erb @@ -83,6 +83,14 @@ <% end %> <% end %> <% end %> + + <% if policy(UserNameChangeRequest.new(user: @user)).can_rename_user? %> + <% menu.item do %> + <%= link_to change_name_user_path(@user) do %> + <%= feedback_icon %> Rename User + <% end %> + <% end %> + <% end %> <% end %>
    diff --git a/config/routes.rb b/config/routes.rb index 7f80ec25d..682c82153 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -270,9 +270,8 @@ Rails.application.routes.draw do resources :api_keys, only: [:new, :create, :edit, :update, :index, :destroy] resources :uploads, only: [:index] - collection do - get :custom_style - end + get :change_name, on: :member, to: "user_name_change_requests#new" + get :custom_style, on: :collection end get "/upgrade", to: "user_upgrades#new", as: "new_user_upgrade" get "/user_upgrades/new", to: redirect("/upgrade") diff --git a/test/functional/application_controller_test.rb b/test/functional/application_controller_test.rb index f0ab1423a..d8eb31429 100644 --- a/test/functional/application_controller_test.rb +++ b/test/functional/application_controller_test.rb @@ -95,7 +95,7 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest @user.update_columns(name: "foo__bar") get_auth posts_path, @user - assert_redirected_to new_user_name_change_request_path + assert_redirected_to change_name_user_path(@user) end end diff --git a/test/functional/user_name_change_requests_controller_test.rb b/test/functional/user_name_change_requests_controller_test.rb index 9539ae52d..c3d688de1 100644 --- a/test/functional/user_name_change_requests_controller_test.rb +++ b/test/functional/user_name_change_requests_controller_test.rb @@ -9,33 +9,70 @@ class UserNameChangeRequestsControllerTest < ActionDispatch::IntegrationTest context "new action" do should "render" do - get_auth new_user_name_change_request_path, @user + get_auth change_name_user_path(@user), @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 + get_auth change_name_user_path(@user), @user assert_response :success end end context "create action" do - should "work" do - post_auth user_name_change_requests_path, @user, params: { user_name_change_request: { desired_name: "zun" }} + should "work for a user changing their own name" do + post_auth user_name_change_requests_path, @user, params: { user_name_change_request: { user_id: @user.id, desired_name: "zun" }} - assert_redirected_to profile_path + assert_redirected_to @user assert_equal("zun", @user.reload.name) + + assert_equal(0, ModAction.user_name_change.count) + assert_equal(0, @user.dmails.received.count) + end + + should "work for a moderator changing a regular user's name" do + @user = create(:user, name: "bkub") + @mod = create(:moderator_user) + post_auth user_name_change_requests_path, @mod, params: { user_name_change_request: { user_id: @user.id, desired_name: "zun" }} + + assert_redirected_to @user + assert_equal("zun", @user.reload.name) + + assert_equal("user_name_change", ModAction.last.category) + assert_equal(@mod, ModAction.last.creator) + assert_equal(@user, ModAction.last.subject) + assert_equal("changed user ##{@user.id}'s name from bkub to zun", ModAction.last.description) + + assert_equal(1, @user.dmails.received.count) + assert_equal("Your username has been changed", @user.dmails.received.last.title) + assert_no_enqueued_emails end should "fail if the new name is invalid" do assert_no_changes(-> { @user.reload.name }) do - post_auth user_name_change_requests_path, @user, params: { user_name_change_request: { desired_name: "foo__bar" }} + post_auth user_name_change_requests_path, @user, params: { user_name_change_request: { user_id: @user.id, desired_name: "foo__bar" }} assert_response :success end end + + should "fail for a regular user trying to change another user's name" do + @user = create(:user, name: "bkub") + post_auth user_name_change_requests_path, create(:builder_user), params: { user_name_change_request: { user_id: @user.id, desired_name: "zun" }} + + assert_response 403 + assert_equal("bkub", @user.reload.name) + end + + should "fail for a moderator trying to change the name of someone above Builder level" do + @user = create(:moderator_user, name: "mod") + 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) + end end context "show action" do