Remove custom deny_access handlers
authorAndy Allan <git@gravitystorm.co.uk>
Wed, 14 Nov 2018 10:35:30 +0000 (11:35 +0100)
committerAndy Allan <git@gravitystorm.co.uk>
Wed, 14 Nov 2018 13:10:51 +0000 (14:10 +0100)
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
app/controllers/issue_comments_controller.rb
app/controllers/issues_controller.rb
test/controllers/diary_entries_controller_test.rb
test/controllers/issue_comments_controller_test.rb
test/controllers/issues_controller_test.rb
test/system/issues_test.rb

index 456ce99..4a1da17 100644 (file)
@@ -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
index 0e4a707..3a6b357 100644 (file)
@@ -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
index 8943f2d..61f466f 100644 (file)
@@ -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
index 426bc38..b1f2216 100644 (file)
@@ -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
index d9b4062..db08ca3 100644 (file)
@@ -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
 
index 224f2f8..af0a860 100644 (file)
@@ -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
index 138de27..f242159 100644 (file)
@@ -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