administrate
administrate copied to clipboard
Authorization errors should not return 500
At the moment, when Pundit::NotAuthorizedError or Administrate::NotAuthorizedError is raised within Administrate, the app returns an HTTP 500 status.
To reproduce:
- Visit the example app: https://administrate-demo.herokuapp.com/admin/
- On the Customers dashboard, click to "Become" any customer.
- Visit the payments dashboard at https://administrate-demo.herokuapp.com/admin/payments (it will have disappeared from the navigation as customers are not authorized).
Expected result: an appropriate HTTP 4xx status, probably 403, perhaps with an error message to match. Actual result: HTTP 500 status and "We're sorry, but something went wrong" message.
Seems like these exceptions should be rescued by Admin::ApplicationController. More questions:
- Should we also show an error message? What should it look like? Rails doesn't provide (that I know) error pages for errors other than 404, 422 and 500, so we would have to provide something ourselves.
- If a user integrates their own authorization mechanism, the raised exceptions will be different (eg: they integrate with cancancan and raise
CanCan::Error). Should we provide a way to tell Administrate what these exceptions might be, so that they are rescued the same way?
One way to handle authorization errors in Rails is to use the rescue_from method within the admin/ApplicationController. This method allows developers to catch exceptions and define custom actions for them. For example, if a user tries to access a resource that they are not authorized to view, the rescue_from method can redirect them to a different page or show an error message. Developers can also adapt their own authorization package or gem to work with the rescue_from method.
it's what example to work with cancancan and devise
module Admin
class ApplicationController < Administrate::ApplicationController
before_action :authenticate_user!
before_action :set_paper_trail_whodunnit
def authorized_action?(_resource, _action_name)
can? _action_name.to_sym, _resource
end
rescue_from Administrate::NotAuthorizedError do |_exception|
redirect_to root_url, alert: 'Not Authorized Error'
end
# Override this value to specify the number of elements to display at a time
# on index pages. Defaults to 20.
# def records_per_page
# params[:per_page] || 20
# end
end
end
Yeah, we may need something like that. Ideally I was thinking we could have it in Administrate::ApplicationController, so that users don't have to set it up. However if we just say the following:
rescue_from Administrate::NotAuthorizedError, Pundit::NotAuthorizedError do |_exception|
show_authorization_error # or something like that
end
Then that doesn't cover other exceptions that users might be raising (eg: CanCan::Error). Options:
- We add to the documentation so that they set their own
rescue_from. - Offer some sort of configuration where they can list the errors they may raise, and this is used in the common
rescue_fromabove.