From 5c6d26ea2456ebe8ffda899007915fb8641ec854 Mon Sep 17 00:00:00 2001 From: evazion Date: Mon, 16 Mar 2020 00:18:50 -0500 Subject: [PATCH] pundit: convert users to pundit. --- app/controllers/admin/users_controller.rb | 6 +-- .../user/count_fixes_controller.rb | 4 +- app/controllers/user_upgrades_controller.rb | 2 +- app/controllers/users_controller.rb | 51 +++++-------------- app/policies/user_policy.rb | 44 ++++++++++++++++ app/views/users/index.html.erb | 2 +- .../functional/admin/users_controller_test.rb | 9 ++-- .../user_upgrades_controller_test.rb | 19 +++++++ test/functional/users_controller_test.rb | 10 ++-- 9 files changed, 88 insertions(+), 59 deletions(-) create mode 100644 app/policies/user_policy.rb create mode 100644 test/functional/user_upgrades_controller_test.rb diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 9860f0c8d..1422bd3de 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -1,13 +1,11 @@ module Admin class UsersController < ApplicationController - before_action :moderator_only - def edit - @user = User.find(params[:id]) + @user = authorize User.find(params[:id]), :promote? end def update - @user = User.find(params[:id]) + @user = authorize User.find(params[:id]), :promote? @user.promote_to!(params[:user][:level], params[:user]) redirect_to edit_admin_user_path(@user), :notice => "User updated" end diff --git a/app/controllers/maintenance/user/count_fixes_controller.rb b/app/controllers/maintenance/user/count_fixes_controller.rb index da1816efc..c2ebd564a 100644 --- a/app/controllers/maintenance/user/count_fixes_controller.rb +++ b/app/controllers/maintenance/user/count_fixes_controller.rb @@ -1,12 +1,12 @@ module Maintenance module User class CountFixesController < ApplicationController - before_action :member_only - def new + @user = authorize CurrentUser.user, :fix_counts? end def create + @user = authorize CurrentUser.user, :fix_counts? CurrentUser.user.refresh_counts! flash[:notice] = "Counts have been refreshed" redirect_to profile_path diff --git a/app/controllers/user_upgrades_controller.rb b/app/controllers/user_upgrades_controller.rb index 263a908d8..8001b1d88 100644 --- a/app/controllers/user_upgrades_controller.rb +++ b/app/controllers/user_upgrades_controller.rb @@ -1,5 +1,4 @@ class UserUpgradesController < ApplicationController - before_action :member_only, only: [:show] helper_method :user skip_before_action :verify_authenticity_token, only: [:create] @@ -13,6 +12,7 @@ class UserUpgradesController < ApplicationController end def show + authorize User, :upgrade? end def user diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 2bac49942..6f8a54572 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -3,19 +3,18 @@ class UsersController < ApplicationController skip_before_action :api_check def new - @user = User.new + @user = authorize User.new @user.email_address = EmailAddress.new respond_with(@user) end def edit - @user = User.find(params[:id]) - check_privilege(@user) + @user = authorize User.find(params[:id]) respond_with(@user) end def settings - @user = CurrentUser.user + @user = authorize CurrentUser.user if @user.is_anonymous? redirect_to login_path(url: settings_path) @@ -32,7 +31,7 @@ class UsersController < ApplicationController return end - @users = User.paginated_search(params) + @users = authorize User.paginated_search(params) @users = @users.includes(:inviter) if request.format.html? respond_with(@users) @@ -42,12 +41,12 @@ class UsersController < ApplicationController end def show - @user = User.find(params[:id]) + @user = authorize User.find(params[:id]) respond_with(@user, methods: @user.full_attributes) end def profile - @user = CurrentUser.user + @user = authorize CurrentUser.user if @user.is_member? params[:action] = "show" @@ -60,7 +59,8 @@ class UsersController < ApplicationController end def create - @user = User.new(last_ip_addr: CurrentUser.ip_addr, **user_params(:create)) + @user = authorize User.new(last_ip_addr: CurrentUser.ip_addr, **permitted_attributes(User)) + if !Danbooru.config.enable_recaptcha? || verify_recaptcha(model: @user) @user.save if @user.errors.empty? @@ -78,14 +78,15 @@ class UsersController < ApplicationController end def update - @user = User.find(params[:id]) - check_privilege(@user) - @user.update(user_params(:update)) + @user = authorize User.find(params[:id]) + @user.update(permitted_attributes(@user)) + if @user.errors.any? flash[:notice] = @user.errors.full_messages.join("; ") else flash[:notice] = "Settings updated" end + respond_with(@user) do |format| format.html { redirect_back fallback_location: edit_user_path(@user) } end @@ -105,32 +106,4 @@ class UsersController < ApplicationController true end end - - def check_privilege(user) - raise User::PrivilegeError unless user.id == CurrentUser.id || CurrentUser.is_admin? - end - - def user_params(context) - permitted_params = %i[ - password old_password password_confirmation - comment_threshold default_image_size favorite_tags blacklisted_tags - time_zone per_page custom_style theme - - receive_email_notifications always_resize_images enable_post_navigation - new_post_navigation_layout enable_private_favorites - enable_sequential_post_navigation hide_deleted_posts style_usernames - enable_auto_complete show_deleted_children - disable_categorized_saved_searches disable_tagged_filenames - disable_cropped_thumbnails disable_mobile_gestures - enable_safe_mode enable_desktop_mode disable_post_tooltips - ] - - if context == :create - permitted_params += [:name, { email_address_attributes: [:address] }] - end - - permitted_params << :level if CurrentUser.is_admin? - - params.require(:user).permit(permitted_params) - end end diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb new file mode 100644 index 000000000..1ed65844f --- /dev/null +++ b/app/policies/user_policy.rb @@ -0,0 +1,44 @@ +class UserPolicy < ApplicationPolicy + def create? + true + end + + def update? + record.id == user.id || user.is_admin? + end + + def promote? + user.is_moderator? + end + + def upgrade? + user.is_member? + end + + def fix_counts? + user.is_member? + end + + def permitted_attributes_for_create + [:name, :password, :password_confirmation, { email_address_attributes: [:address] }] + end + + def permitted_attributes_for_update + [ + :comment_threshold, :default_image_size, :favorite_tags, + :blacklisted_tags, :time_zone, :per_page, :custom_style, :theme, + :receive_email_notifications, :always_resize_images, + :enable_post_navigation, :new_post_navigation_layout, + :enable_private_favorites, :enable_sequential_post_navigation, + :hide_deleted_posts, :style_usernames, :enable_auto_complete, + :show_deleted_children, :disable_categorized_saved_searches, + :disable_tagged_filenames, :disable_cropped_thumbnails, + :disable_mobile_gestures, :enable_safe_mode, :enable_desktop_mode, + :disable_post_tooltips, + (:level if CurrentUser.is_admin?) + ].compact + end + + alias_method :profile?, :show? + alias_method :settings?, :edit? +end diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb index d7c051cae..2c3bae957 100644 --- a/app/views/users/index.html.erb +++ b/app/views/users/index.html.erb @@ -4,7 +4,7 @@ <%= table_for @users, width: "100%" do |t| %> <% t.column column: "control" do |user| %> - <% if CurrentUser.is_admin? %> + <% if policy(CurrentUser.user).promote? %> <%= link_to "Edit", edit_admin_user_path(user) %> <% end %> <% end %> diff --git a/test/functional/admin/users_controller_test.rb b/test/functional/admin/users_controller_test.rb index 55e707ed7..2cc47a0fe 100644 --- a/test/functional/admin/users_controller_test.rb +++ b/test/functional/admin/users_controller_test.rb @@ -20,8 +20,7 @@ class Admin::UsersControllerTest < ActionDispatch::IntegrationTest should "succeed" do put_auth admin_user_path(@user), @mod, params: {:user => {:level => "30"}} assert_redirected_to(edit_admin_user_path(@user)) - @user.reload - assert_equal(30, @user.level) + assert_equal(30, @user.reload.level) assert_equal(@mod.id, @user.inviter_id) end @@ -29,8 +28,7 @@ class Admin::UsersControllerTest < ActionDispatch::IntegrationTest should "fail" do put_auth admin_user_path(@user), @mod, params: {:user => {:level => "50"}} assert_response(403) - @user.reload - assert_equal(20, @user.level) + assert_equal(20, @user.reload.level) end end end @@ -39,8 +37,7 @@ class Admin::UsersControllerTest < ActionDispatch::IntegrationTest should "fail" do put_auth admin_user_path(@admin), @mod, params: {:user => {:level => "30"}} assert_response(403) - @admin.reload - assert_equal(50, @admin.level) + assert_equal(50, @admin.reload.level) end end end diff --git a/test/functional/user_upgrades_controller_test.rb b/test/functional/user_upgrades_controller_test.rb new file mode 100644 index 000000000..b4450f8a1 --- /dev/null +++ b/test/functional/user_upgrades_controller_test.rb @@ -0,0 +1,19 @@ +require 'test_helper' + +class UserUpgradesControllerTest < ActionDispatch::IntegrationTest + context "The user upgrades controller" do + context "new action" do + should "render" do + get new_user_upgrade_path + assert_response :success + end + end + + context "show action" do + should "render" do + get_auth user_upgrade_path, create(:user) + assert_response :success + end + end + end +end diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 246c8d7dc..9ff3370f2 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -186,14 +186,12 @@ class UsersControllerTest < ActionDispatch::IntegrationTest end context "changing the level" do - setup do - @cuser = create(:user) - end - should "not work" do + @cuser = create(:user) put_auth user_path(@user), @cuser, params: {:user => {:level => 40}} - @user.reload - assert_equal(20, @user.level) + + assert_response 403 + assert_equal(20, @user.reload.level) end end