From 234afb3f4277130f233fb3b01fcd56cc1c2d1678 Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 14 Nov 2018 11:35:30 +0100 Subject: [PATCH] Remove custom deny_access handlers Since these pages are not accessed by normal users, except for url fiddling, it's fine to respond with a generic access denied. --- app/controllers/diary_entries_controller.rb | 16 ---------------- app/controllers/issue_comments_controller.rb | 9 --------- app/controllers/issues_controller.rb | 9 --------- .../controllers/diary_entries_controller_test.rb | 4 ++-- .../issue_comments_controller_test.rb | 2 +- test/controllers/issues_controller_test.rb | 10 +++++----- test/system/issues_test.rb | 2 +- 7 files changed, 9 insertions(+), 43 deletions(-) diff --git a/app/controllers/diary_entries_controller.rb b/app/controllers/diary_entries_controller.rb index 456ce99ad..4a1da178a 100644 --- a/app/controllers/diary_entries_controller.rb +++ b/app/controllers/diary_entries_controller.rb @@ -216,22 +216,6 @@ class DiaryEntriesController < ApplicationController private - # This is required because, being a default-deny system, cancancan - # _cannot_ tell you the reason you were denied access; and so - # the "nice" feedback presenting next steps can't be gleaned from - # the exception - ## - # for the hide actions, require that the user is a administrator, or fill out - # a helpful error message and return them to the user page. - def deny_access(exception) - if current_user && exception.action.in?([:hide, :hidecomment]) - flash[:error] = t("users.filter.not_an_administrator") - redirect_to :action => "show" - else - super - end - end - ## # return permitted diary entry parameters def entry_params diff --git a/app/controllers/issue_comments_controller.rb b/app/controllers/issue_comments_controller.rb index 0e4a7079e..3a6b357f9 100644 --- a/app/controllers/issue_comments_controller.rb +++ b/app/controllers/issue_comments_controller.rb @@ -22,15 +22,6 @@ class IssueCommentsController < ApplicationController params.require(:issue_comment).permit(:body) end - def deny_access(_exception) - if current_user - flash[:error] = t("application.require_moderator_or_admin.not_a_moderator_or_admin") - redirect_to root_path - else - super - end - end - # This sort of assumes there are only two roles def reassign_issue(issue) role = (Issue::ASSIGNED_ROLES - [issue.assigned_role]).first diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 8943f2d4a..61f466f62 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -82,13 +82,4 @@ class IssuesController < ApplicationController def find_issue @issue = Issue.find(params[:id]) end - - def deny_access(_exception) - if current_user - flash[:error] = t("application.require_moderator_or_admin.not_a_moderator_or_admin") - redirect_to root_path - else - super - end - end end diff --git a/test/controllers/diary_entries_controller_test.rb b/test/controllers/diary_entries_controller_test.rb index 426bc3851..b1f2216c7 100644 --- a/test/controllers/diary_entries_controller_test.rb +++ b/test/controllers/diary_entries_controller_test.rb @@ -726,7 +726,7 @@ class DiaryEntriesControllerTest < ActionController::TestCase :params => { :display_name => user.display_name, :id => diary_entry.id }, :session => { :user => user } assert_response :redirect - assert_redirected_to :action => :show, :display_name => user.display_name, :id => diary_entry.id + assert_redirected_to :controller => :errors, :action => :forbidden assert_equal true, DiaryEntry.find(diary_entry.id).visible # Finally try as an administrator @@ -754,7 +754,7 @@ class DiaryEntriesControllerTest < ActionController::TestCase :params => { :display_name => user.display_name, :id => diary_entry.id, :comment => diary_comment.id }, :session => { :user => user } assert_response :redirect - assert_redirected_to :action => :show, :display_name => user.display_name, :id => diary_entry.id + assert_redirected_to :controller => :errors, :action => :forbidden assert_equal true, DiaryComment.find(diary_comment.id).visible # Finally try as an administrator diff --git a/test/controllers/issue_comments_controller_test.rb b/test/controllers/issue_comments_controller_test.rb index d9b4062c7..db08ca3cf 100644 --- a/test/controllers/issue_comments_controller_test.rb +++ b/test/controllers/issue_comments_controller_test.rb @@ -9,7 +9,7 @@ class IssueCommentsControllerTest < ActionController::TestCase post :create, :params => { :issue_id => issue.id } assert_response :redirect - assert_redirected_to root_path + assert_redirected_to :controller => :errors, :action => :forbidden assert_equal 0, issue.comments.length end diff --git a/test/controllers/issues_controller_test.rb b/test/controllers/issues_controller_test.rb index 224f2f8cf..af0a86028 100644 --- a/test/controllers/issues_controller_test.rb +++ b/test/controllers/issues_controller_test.rb @@ -11,7 +11,7 @@ class IssuesControllerTest < ActionController::TestCase session[:user] = create(:user).id get :index assert_response :redirect - assert_redirected_to root_path + assert_redirected_to :controller => :errors, :action => :forbidden # Access issues list as administrator session[:user] = create(:administrator_user).id @@ -37,7 +37,7 @@ class IssuesControllerTest < ActionController::TestCase session[:user] = create(:user).id get :show, :params => { :id => issue.id } assert_response :redirect - assert_redirected_to root_path + assert_redirected_to :controller => :errors, :action => :forbidden # Access issue as administrator session[:user] = create(:administrator_user).id @@ -63,7 +63,7 @@ class IssuesControllerTest < ActionController::TestCase session[:user] = create(:user).id get :resolve, :params => { :id => issue.id } assert_response :redirect - assert_redirected_to root_path + assert_redirected_to :controller => :errors, :action => :forbidden # Resolve issue as administrator session[:user] = create(:administrator_user).id @@ -93,7 +93,7 @@ class IssuesControllerTest < ActionController::TestCase session[:user] = create(:user).id get :ignore, :params => { :id => issue.id } assert_response :redirect - assert_redirected_to root_path + assert_redirected_to :controller => :errors, :action => :forbidden # Ignore issue as administrator session[:user] = create(:administrator_user).id @@ -125,7 +125,7 @@ class IssuesControllerTest < ActionController::TestCase session[:user] = create(:user).id get :reopen, :params => { :id => issue.id } assert_response :redirect - assert_redirected_to root_path + assert_redirected_to :controller => :errors, :action => :forbidden # Reopen issue as administrator session[:user] = create(:administrator_user).id diff --git a/test/system/issues_test.rb b/test/system/issues_test.rb index 138de2721..f2421599f 100644 --- a/test/system/issues_test.rb +++ b/test/system/issues_test.rb @@ -12,7 +12,7 @@ class IssuesTest < ApplicationSystemTestCase sign_in_as(create(:user)) visit issues_path - assert page.has_content?(I18n.t("application.require_moderator_or_admin.not_a_moderator_or_admin")) + assert page.has_content?("Forbidden") end def test_view_no_issues -- 2.43.2