search

RedHatInsights / insights-api-common-rails

Header, Encryption, RBAC, Serialization, Pagination and other common behavior for Insights microservices built with Rails
Apache License 2.0
3 stars 25 forks source link

Handle Pundit::NotAuthorizedError exceptions differently than general exceptions #185

Closed eclarizio closed 4 years ago

eclarizio commented 4 years ago

In Catalog, we have an error that is being raised by a gem automatically, Pundit::NotAuthorizedError, which also auto-generates a not-so-user-friendly error message of "not authorized to <action?> this Class".

We needed a way to hook into the rescue_from process without duplicating the code in common, so I broke out the handler into a separate method and I'm using it to pass control over the error message to the rescue_from we have for specifically handling Pundit::NotAuthorizedErrors.

Example usage: Catalog PR.

Edit: I still broke out the handling code into a rescue_from_handler since it was a simple refactor and it allows us to potentially use it going forward, but we are not currently using it. Instead, common is just going to attempt to handle any Pundit::NotAuthorizedError exception similar to how it branches when handling api client errors.

eclarizio commented 4 years ago

@syncrou @lindgrenj6 It looks like travis is failing because it's using an older ruby version that doesn't have the #delete_suffix method for strings. Should we be updating ruby version dependency for this repo or should I use the older .chomp('?')? Seems like 2.4.5 is a bit old, but I don't know if 2.5.x breaks anything else we use in this repo.

gmcculloug commented 4 years ago

@eclarizio @lindgrenj6 My understanding is all the apps are running on 2.5.5 and travis for a number of repos is testing against 2.5 and 2.6.

Seems we should bump the travis rvm version here.

lindgrenj6 commented 4 years ago

@gmcculloug @eclarizio https://github.com/RedHatInsights/insights-api-common-rails/pull/187

Opened a PR for here and catalog/approval to keep us all in line.

gmcculloug commented 4 years ago

@eclarizio ruby version PR is merged. Please rebase.

eclarizio commented 4 years ago

@gmcculloug @lindgrenj6 Rebased/updated. Looks like we're green now! 👍

eclarizio commented 4 years ago

@syncrou Pushed changes, is this more what you're looking for?