From c82e05d8280d1fc590dbcaf57d62a37499fcbef6 Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 13 Dec 2020 16:54:45 -0600 Subject: [PATCH] users: add stricter checks for user promotions. New rules for user promotions: * Moderators can no longer promote other users to moderator level. Only Admins can promote users to Mod level. Mods can only promote up to Builder level. * Admins can no longer promote other users to Admin level. Only Owners can promote users to Admin. Admins can only promote up to Mod level. * Admins can no longer demote themselves or other admins. These rules are being changed to account for the new Owner user level. Also change it so that when a user upgrades their account, the promotion is done by DanbooruBot. This means that the inviter and the mod action will show DanbooruBot as the promoter instead of the user themselves. --- app/controllers/admin/users_controller.rb | 8 +- app/controllers/user_upgrades_controller.rb | 2 +- app/logical/user_promotion.rb | 50 +++++------ app/models/user.rb | 4 +- db/populate.rb | 2 +- .../functional/admin/users_controller_test.rb | 18 ++++ test/unit/user_test.rb | 85 +++++++++++++++++-- 7 files changed, 131 insertions(+), 38 deletions(-) diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 1422bd3de..0337c892a 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -6,7 +6,13 @@ module Admin def update @user = authorize User.find(params[:id]), :promote? - @user.promote_to!(params[:user][:level], params[:user]) + + @level = params.dig(:user, :level) + @can_upload_free = params.dig(:user, :can_upload_free) + @can_approve_posts = params.dig(:user, :can_approve_posts) + + @user.promote_to!(@level, CurrentUser.user, can_upload_free: @can_upload_free, can_approve_posts: @can_approve_posts) + redirect_to edit_admin_user_path(@user), :notice => "User updated" end end diff --git a/app/controllers/user_upgrades_controller.rb b/app/controllers/user_upgrades_controller.rb index 8001b1d88..903f7b6ec 100644 --- a/app/controllers/user_upgrades_controller.rb +++ b/app/controllers/user_upgrades_controller.rb @@ -48,7 +48,7 @@ class UserUpgradesController < ApplicationController :card => params[:stripeToken], :description => params[:desc] ) - @user.promote_to!(level, is_upgrade: true) + @user.promote_to!(level, User.system, is_upgrade: true) flash[:success] = true rescue Stripe::CardError => e DanbooruLogger.log(e) diff --git a/app/logical/user_promotion.rb b/app/logical/user_promotion.rb index b3b7a32bb..457c67cce 100644 --- a/app/logical/user_promotion.rb +++ b/app/logical/user_promotion.rb @@ -1,33 +1,28 @@ class UserPromotion - attr_reader :user, :promoter, :new_level, :options, :old_can_approve_posts, :old_can_upload_free + attr_reader :user, :promoter, :new_level, :old_can_approve_posts, :old_can_upload_free, :can_upload_free, :can_approve_posts, :is_upgrade - def initialize(user, promoter, new_level, options = {}) + def initialize(user, promoter, new_level, can_upload_free: nil, can_approve_posts: nil, is_upgrade: false) @user = user @promoter = promoter - @new_level = new_level - @options = options + @new_level = new_level.to_i + @can_upload_free = can_upload_free + @can_approve_posts = can_approve_posts + @is_upgrade = is_upgrade end def promote! - validate + validate! @old_can_approve_posts = user.can_approve_posts? @old_can_upload_free = user.can_upload_free? user.level = new_level + user.can_upload_free = can_upload_free unless can_upload_free.nil? + user.can_approve_posts = can_approve_posts unless can_approve_posts.nil? + user.inviter = promoter - if options.key?(:can_approve_posts) - user.can_approve_posts = options[:can_approve_posts] - end - - if options.key?(:can_upload_free) - user.can_upload_free = options[:can_upload_free] - end - - user.inviter_id = promoter.id - - create_user_feedback unless options[:is_upgrade] - create_dmail unless options[:skip_dmail] + create_user_feedback unless is_upgrade + create_dmail create_mod_actions user.save @@ -45,20 +40,21 @@ class UserPromotion end if user.level_changed? - category = options[:is_upgrade] ? :user_account_upgrade : :user_level_change + category = is_upgrade ? :user_account_upgrade : :user_level_change ModAction.log(%{"#{user.name}":/users/#{user.id} level changed #{user.level_string_was} -> #{user.level_string}}, category) end end - def validate - # admins can do anything - return if promoter.is_admin? - - # can't promote/demote moderators - raise User::PrivilegeError if user.is_moderator? - - # can't promote to admin - raise User::PrivilegeError if new_level.to_i >= User::Levels::ADMIN + def validate! + if !promoter.is_moderator? + raise User::PrivilegeError, "You can't promote or demote other users" + elsif promoter == user + raise User::PrivilegeError, "You can't promote or demote yourself" + elsif new_level >= promoter.level + raise User::PrivilegeError, "You can't promote other users to your rank or above" + elsif user.level >= promoter.level + raise User::PrivilegeError, "You can't promote or demote other users at your rank or above" + end end def build_messages diff --git a/app/models/user.rb b/app/models/user.rb index 73e2cb116..ec14ed950 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -250,8 +250,8 @@ class User < ApplicationRecord end end - def promote_to!(new_level, options = {}) - UserPromotion.new(self, CurrentUser.user, new_level, options).promote! + def promote_to!(new_level, promoter = CurrentUser.user, **options) + UserPromotion.new(self, promoter, new_level, **options).promote! end def promote_to_admin_if_first_user diff --git a/db/populate.rb b/db/populate.rb index c6451e7b7..7a66a7080 100644 --- a/db/populate.rb +++ b/db/populate.rb @@ -53,7 +53,7 @@ if User.count == 0 password: "password1", password_confirmation: "password1" ) - newuser.promote_to!(User::Levels.const_get(level), :is_upgrade => true, :skip_dmail => true) + newuser.promote_to!(User::Levels.const_get(level), user) end newuser = User.create( diff --git a/test/functional/admin/users_controller_test.rb b/test/functional/admin/users_controller_test.rb index 2cc47a0fe..c56470544 100644 --- a/test/functional/admin/users_controller_test.rb +++ b/test/functional/admin/users_controller_test.rb @@ -24,6 +24,24 @@ class Admin::UsersControllerTest < ActionDispatch::IntegrationTest assert_equal(@mod.id, @user.inviter_id) end + should "promote the user to unrestricted uploads" do + put_auth admin_user_path(@user), @mod, params: { user: { level: User::Levels::BUILDER, can_upload_free: true }} + + assert_redirected_to(edit_admin_user_path(@user.reload)) + assert_equal(true, @user.is_builder?) + assert_equal(true, @user.can_upload_free?) + assert_equal(false, @user.can_approve_posts?) + end + + should "promote the user to approver" do + put_auth admin_user_path(@user), @mod, params: { user: { level: User::Levels::BUILDER, can_approve_posts: true }} + + assert_redirected_to(edit_admin_user_path(@user.reload)) + assert_equal(true, @user.is_builder?) + assert_equal(false, @user.can_upload_free?) + assert_equal(true, @user.can_approve_posts?) + end + context "promoted to an admin" do should "fail" do put_auth admin_user_path(@user), @mod, params: {:user => {:level => "50"}} diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index 012d1779b..cfa83b76a 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -1,6 +1,19 @@ require 'test_helper' class UserTest < ActiveSupport::TestCase + def assert_promoted_to(new_level, user, promoter) + user.promote_to!(new_level, promoter) + assert_equal(new_level, user.reload.level) + end + + def assert_not_promoted_to(new_level, user, promoter) + assert_raise(User::PrivilegeError) do + user.promote_to!(new_level, promoter) + end + + assert_not_equal(new_level, user.reload.level) + end + context "A user" do setup do @user = FactoryBot.create(:user) @@ -15,14 +28,74 @@ class UserTest < ActiveSupport::TestCase context "promoting a user" do setup do - CurrentUser.user = FactoryBot.create(:moderator_user) + @builder = create(:builder_user) + @mod = create(:moderator_user) + @admin = create(:admin_user) + @owner = create(:owner_user) + end + + should "allow moderators to promote users up to builder level" do + assert_promoted_to(User::Levels::GOLD, @user, @mod) + assert_promoted_to(User::Levels::PLATINUM, @user, @mod) + assert_promoted_to(User::Levels::BUILDER, @user, @mod) + + assert_not_promoted_to(User::Levels::MODERATOR, @user, @mod) + assert_not_promoted_to(User::Levels::ADMIN, @user, @mod) + assert_not_promoted_to(User::Levels::OWNER, @user, @mod) + end + + should "allow admins to promote users up to moderator level" do + assert_promoted_to(User::Levels::GOLD, @user, @admin) + assert_promoted_to(User::Levels::PLATINUM, @user, @admin) + assert_promoted_to(User::Levels::BUILDER, @user, @admin) + assert_promoted_to(User::Levels::MODERATOR, @user, @admin) + + assert_not_promoted_to(User::Levels::ADMIN, @user, @admin) + assert_not_promoted_to(User::Levels::OWNER, @user, @admin) + end + + should "allow the owner to promote users up to admin level" do + assert_promoted_to(User::Levels::GOLD, @user, @owner) + assert_promoted_to(User::Levels::PLATINUM, @user, @owner) + assert_promoted_to(User::Levels::BUILDER, @user, @owner) + assert_promoted_to(User::Levels::MODERATOR, @user, @owner) + assert_promoted_to(User::Levels::ADMIN, @user, @owner) + + assert_not_promoted_to(User::Levels::OWNER, @user, @owner) + end + + should "not allow non-moderators to promote other users" do + assert_not_promoted_to(User::Levels::GOLD, @user, @builder) + assert_not_promoted_to(User::Levels::PLATINUM, @user, @builder) + assert_not_promoted_to(User::Levels::BUILDER, @user, @builder) + assert_not_promoted_to(User::Levels::MODERATOR, @user, @builder) + assert_not_promoted_to(User::Levels::ADMIN, @user, @builder) + assert_not_promoted_to(User::Levels::OWNER, @user, @builder) + end + + should "not allow users to promote or demote other users at their rank or above" do + assert_not_promoted_to(User::Levels::ADMIN, create(:moderator_user), @mod) + assert_not_promoted_to(User::Levels::BUILDER, create(:moderator_user), @mod) + + assert_not_promoted_to(User::Levels::OWNER, create(:admin_user), @admin) + assert_not_promoted_to(User::Levels::MODERATOR, create(:admin_user), @admin) + + assert_not_promoted_to(User::Levels::ADMIN, create(:owner_user), @owner) + end + + should "not allow users to promote themselves" do + assert_not_promoted_to(User::Levels::ADMIN, @mod, @mod) + assert_not_promoted_to(User::Levels::OWNER, @admin, @admin) + end + + should "not allow users to demote themselves" do + assert_not_promoted_to(User::Levels::MEMBER, @mod, @mod) + assert_not_promoted_to(User::Levels::MEMBER, @admin, @admin) + assert_not_promoted_to(User::Levels::MEMBER, @owner, @owner) end should "create a neutral feedback" do - assert_difference("UserFeedback.count") do - @user.promote_to!(User::Levels::GOLD) - end - + @user.promote_to!(User::Levels::GOLD, @mod) assert_equal("You have been promoted to a Gold level account from Member.", @user.feedback.last.body) end @@ -31,7 +104,7 @@ class UserTest < ActiveSupport::TestCase User.stubs(:system).returns(bot) assert_difference("Dmail.count", 1) do - @user.promote_to!(User::Levels::GOLD) + @user.promote_to!(User::Levels::GOLD, @admin) end assert(@user.dmails.exists?(from: bot, to: @user, title: "Your account has been updated"))