thoughtbot / thoughtbot/administrate

Authorization errors should not return 500

Open
#2,372 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
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:

  1. Visit the example app: https://administrate-demo.herokuapp.com/admin/
  2. On the Customers dashboard, click to "Become" any customer.
  3. 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.