From d4c43af1ddc9c8e94a8ce3b6200a218d222eeb6c Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 24 Aug 2019 22:55:36 -0500 Subject: [PATCH] app controller: replace calls to access_denied with PrivilegeError. Standardize controllers to raise User::PrivilegeError instead of calling `access_denied` directly. --- app/controllers/application_controller.rb | 9 ++------ .../bulk_update_requests_controller.rb | 22 +++++++------------ app/controllers/tag_aliases_controller.rb | 10 ++++----- .../tag_implications_controller.rb | 15 ++++--------- test/functional/comments_controller_test.rb | 2 +- 5 files changed, 19 insertions(+), 39 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1a1aae416..40428f965 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -14,11 +14,6 @@ class ApplicationController < ActionController::Base rescue_from Exception, :with => :rescue_exception rescue_from User::PrivilegeError, :with => :access_denied - rescue_from ActionController::UnpermittedParameters, :with => :access_denied - - # This is raised on requests to `/blah.js`. Rails has already rendered StaticController#not_found - # here, so calling `rescue_exception` would cause a double render error. - rescue_from ActionController::InvalidCrossOriginRequest, with: -> {} protected @@ -62,7 +57,7 @@ class ApplicationController < ActionController::Base render_error_page(400, exception) when SessionLoader::AuthenticationFailure render_error_page(401, exception) - when ActionController::InvalidAuthenticityToken + when ActionController::InvalidAuthenticityToken, ActionController::UnpermittedParameters, ActionController::InvalidCrossOriginRequest render_error_page(403, exception) when ActiveRecord::RecordNotFound render_error_page(404, exception, message: "That record was not found.") @@ -144,7 +139,7 @@ class ApplicationController < ActionController::Base User::Roles.each do |role| define_method("#{role}_only") do if !CurrentUser.user.send("is_#{role}?") || CurrentUser.user.is_banned? || IpBan.is_banned?(CurrentUser.ip_addr) - access_denied + raise User::PrivilegeError end end end diff --git a/app/controllers/bulk_update_requests_controller.rb b/app/controllers/bulk_update_requests_controller.rb index b0d6894e9..6f3878c7b 100644 --- a/app/controllers/bulk_update_requests_controller.rb +++ b/app/controllers/bulk_update_requests_controller.rb @@ -23,13 +23,10 @@ class BulkUpdateRequestsController < ApplicationController end def update - if @bulk_update_request.editable?(CurrentUser.user) - @bulk_update_request.update(bur_params(:update)) - flash[:notice] = "Bulk update request updated" - respond_with(@bulk_update_request, :location => bulk_update_requests_path) - else - access_denied() - end + raise User::PrivilegeError unless @bulk_update_request.editable?(CurrentUser.user) + + @bulk_update_request.update(bur_params(:update)) + respond_with(@bulk_update_request, location: bulk_update_requests_path, notice: "Bulk update request updated") end def approve @@ -38,13 +35,10 @@ class BulkUpdateRequestsController < ApplicationController end def destroy - if @bulk_update_request.rejectable?(CurrentUser.user) - @bulk_update_request.reject!(CurrentUser.user) - flash[:notice] = "Bulk update request rejected" - respond_with(@bulk_update_request, :location => bulk_update_requests_path) - else - access_denied() - end + raise User::PrivilegeError unless @bulk_update_request.rejectable?(CurrentUser.user) + + @bulk_update_request.reject!(CurrentUser.user) + respond_with(@bulk_update_request, location: bulk_update_requests_path, notice: "Bulk update request rejected") end def index diff --git a/app/controllers/tag_aliases_controller.rb b/app/controllers/tag_aliases_controller.rb index 3e3832632..1d29ef1a3 100644 --- a/app/controllers/tag_aliases_controller.rb +++ b/app/controllers/tag_aliases_controller.rb @@ -32,12 +32,10 @@ class TagAliasesController < ApplicationController def destroy @tag_alias = TagAlias.find(params[:id]) - if @tag_alias.deletable_by?(CurrentUser.user) - @tag_alias.reject! - respond_with(@tag_alias, :location => tag_aliases_path) - else - access_denied - end + raise User::PrivilegeError unless @tag_alias.deletable_by?(CurrentUser.user) + + @tag_alias.reject! + respond_with(@tag_alias, location: tag_aliases_path, notice: "Tag alias was deleted") end def approve diff --git a/app/controllers/tag_implications_controller.rb b/app/controllers/tag_implications_controller.rb index fc11b1782..947fe9c8a 100644 --- a/app/controllers/tag_implications_controller.rb +++ b/app/controllers/tag_implications_controller.rb @@ -32,17 +32,10 @@ class TagImplicationsController < ApplicationController def destroy @tag_implication = TagImplication.find(params[:id]) - if @tag_implication.deletable_by?(CurrentUser.user) - @tag_implication.reject! - respond_with(@tag_implication) do |format| - format.html do - flash[:notice] = "Tag implication was deleted" - redirect_to(tag_implications_path) - end - end - else - access_denied - end + raise User::PrivilegeError unless @tag_implication.deletable_by?(CurrentUser.user) + + @tag_implication.reject! + respond_with(@tag_implication, location: tag_implications_path, notice: "Tag implication was deleted") end def approve diff --git a/test/functional/comments_controller_test.rb b/test/functional/comments_controller_test.rb index 49e70ed93..6f9de1a73 100644 --- a/test/functional/comments_controller_test.rb +++ b/test/functional/comments_controller_test.rb @@ -23,7 +23,7 @@ class CommentsControllerTest < ActionDispatch::IntegrationTest context "index action" do should "render for post" do - get comments_path(post_id: @post.id, group_by: "post", format: "js") + get comments_path(post_id: @post.id, group_by: "post", format: "js"), xhr: true assert_response :success end