thoughtbot / thoughtbot/administrate
Authorization errors should not return 500
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 6k
- Forks
- 1.1k
- PR merge metrics
- No merged PRs in 30d
Description
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?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in Admin::ApplicationController and reproduce the failure through the Customers and payments dashboards in the example app. Trace how Pundit::NotAuthorizedError and Administrate::NotAuthorizedError are handled, then determine the appropriate 4xx response and error message. Done means unauthorized access no longer returns HTTP 500, with a clear approach for integrations using other authorization exceptions.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rails, ruby
- Domain
- authorization, backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100