Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undefined method `reason' for #<Pundit::NotAuthorizedError> in Pundit 2.1.0 #658

Closed
Ashutoshkushwaha opened this issue Jun 15, 2020 · 5 comments

Comments

@Ashutoshkushwaha
Copy link

An object of Pundit::NotAuthorizedError class raises an undefined method reason error when the method .reason is called. This expectation was raised when I was accessing exception.reason after rescuing NotAuthorizedError.

In the console, I reproduced it by running

a = Pundit::NotAuthorizedError.new
a.reason

Pundit Version: 2.1.0
Rails Version: 6.0.2.2

@libin0120
Copy link

I believe this feature is not released yet, it is only in the master branch.

@Ashutoshkushwaha
Copy link
Author

When this feature is expected to release?

@rlue
Copy link
Contributor

rlue commented Sep 11, 2020

I am actually hoping that this feature is not released, as I have prepared an alternative approach in PR #655.

In fact, if you could have a look and let me know what you think of it, I'd really appreciate it. (If you like it, it might help persuade the maintainers to merge it.) 😉

@brightchimp
Copy link

I agree with @rlue. I don't think raising exceptions in policies is a good pattern and #655 looks better. With that said, this workaround might be useful if:

  • You want the behaviour that :reason supports (multiple error messages from the same policy method).
  • You want to use a tagged release of Pundit, not master.
  • You're OK with raising errors in policies until a solution is finalised.

I think this workaround is unsatisfactory for a couple of reasons:

  • It still raises exceptions in policies, but this is the pattern that :reason would require anyway.
  • It uses error_message to place an I18n key, which is not expected.

With those caveats, you can achieve the desired behaviour with something like this...

# ApplicationController
rescue_from Pundit::NotAuthorizedError do |exception|
  # Attempt to get custom message from Pundit error locale files.
  message = t(exception.message, scope: ["pundit","errors"], default: :default)
  if message == t("default", scope: ["pundit","errors"])
    # We have not specified a defined key in the I18n error scope.
    # Either set your own default or use the custom errors from locales
    # https://github.com/varvet/pundit#creating-custom-error-messages
    message = "You're not allowed to do this."
  end

  flash[:error] = message
  redirect_to(request.referrer || root_path)
end
# config/locales/pundit/errors.en.yml (better organisation)
# or
# config/locales/pundit.en.yml (if you just want it to try it out)
en:
  pundit:
    errors:
      # default is not used other that to check if a specific error was found.
      default: "magic_undefined_string"
      account:
        reached_max: "You've reached the maximum."
        is_locked: "This account is locked."
# AccountPolicy
def update?
  if maximum_reached?
    raise Pundit::NotAuthorizedError, message: "account.reached_max"
  elsif is_locked?
    raise Pundit::NotAuthorizedError, message: "account.is_locked"
  end
  return false if any_other_reason?
end

@Burgestrand
Copy link
Member

The feature this issue is referring to has been reverted, see #684 for more information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants