From a129293e44a03190792c4477f1891fbe2e5dbab1 Mon Sep 17 00:00:00 2001 From: Ryan Lue Date: Thu, 14 May 2020 22:26:44 +0800 Subject: [PATCH] Support custom error messages in policies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit allows policies to define the error messages set on `Pundit::NotAuthorizedError` exceptions when `#authorize` fails. The rationale is described in detail in GitHub issue #654[0], and summarized below. Some queries can fail in multiple ways; for instance, class PostPolicy def update? if record.author != user ... # failure case 1 elsif record.archived ... # failure case 2 end true end end In their controllers, users might wish to handle different failure modes in different ways, but prior to this commit, there was only one way to tell the difference— namely, by raising errors inside the query method: def update? if record.author != user raise Pundit::NotAuthorizedError, 'You’re not the author!' elsif record.archived raise Pundit::NotAuthorizedError, 'This post is old news!' end true end This breaks the expectation that query methods should return booleans, which in turn breaks a pattern for using query methods in views: <% if policy(@post).update? %> <%= link_to "Edit post", edit_post_path(@post) %> <% end %> 973b63b added a `reason` option to the NotAuthorizedError initializer, but ultimatly required the same approach of raising errors in queries. --- This commit enables a cleaner method of passing a custom error message to exceptions from within policies, without violating the expectations of where exceptions are raised from. class PostPolicy attr_accessor :error_message def update? self.error_message = if record.author != user 'You’re not the author!' elsif record.archived 'This post is old news!' end error_message.nil? end end [0]: https://github.com/varvet/pundit/issues/654 --- README.md | 181 ++++++++++++++++++++++++++++++-------------- lib/pundit.rb | 4 +- spec/pundit_spec.rb | 46 ++++++++++- spec/spec_helper.rb | 2 +- 4 files changed, 173 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index 439928d3..2fc15624 100644 --- a/README.md +++ b/README.md @@ -483,17 +483,24 @@ class NilClassPolicy < ApplicationPolicy end ``` -## Rescuing a denied Authorization in Rails +## Handling `Pundit::NotAuthorizedError` -Pundit raises a `Pundit::NotAuthorizedError` you can -[rescue_from](http://guides.rubyonrails.org/action_controller_overview.html#rescue-from) -in your `ApplicationController`. You can customize the `user_not_authorized` -method in every controller. +When `#authorize` fails, Pundit raises an error. +For a coarse, one-line approach to handling all failed authorizations, +set up an application-wide 403 Forbidden response +any time this error is encountered: ```ruby -class ApplicationController < ActionController::Base - include Pundit +# application.rb + +config.action_dispatch.rescue_responses["Pundit::NotAuthorizedError"] = :forbidden +``` +For more fine-grained control, [`rescue_from`][] this exception +in your application controller: + +```ruby +class ApplicationController < ActionController::Base rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized private @@ -505,85 +512,149 @@ class ApplicationController < ActionController::Base end ``` -Alternatively, you can globally handle Pundit::NotAuthorizedError's by having rails handle them as a 403 error and serving a 403 error page. Add the following to application.rb: +To customize this response on a per-controller basis, simply define +`#user_not_authorized` in other controllers as necessary. -```config.action_dispatch.rescue_responses["Pundit::NotAuthorizedError"] = :forbidden``` +[`rescue_from`]: http://guides.rubyonrails.org/action_controller_overview.html#rescue-from -## Creating custom error messages +### Customizing the flash message -`NotAuthorizedError`s provide information on what query (e.g. `:create?`), what -record (e.g. an instance of `Post`), and what policy (e.g. an instance of -`PostPolicy`) caused the error to be raised. +In the example above, +we show the user the same error message any time authorization fails. +But we can also dynamically generate more detailed messages +using metadata attributes stored on the exception object itself: -One way to use these `query`, `record`, and `policy` properties is to connect -them with `I18n` to generate error messages. Here's how you might go about doing -that. +```ruby +def user_not_authorized(exception) + flash[:alert] = case exception.query + when :show? + "That #{exception.record.class} is not available." + when :update? + "That #{exception.record.class} cannot be modified." + else + exception.message + end + + redirect_to(request.referrer || root_path) +end +``` + +These metadata attributes are described below: ```ruby -class ApplicationController < ActionController::Base - rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized +begin + authorize(@post, :show?) +rescue Pundit::NotAuthorizedError => e + e.query # => :show? + e.record # => @post + e.policy # => # + e.message # => "not allowed to show? this Post" + e.reason # => nil +end +``` - private +#### Internationalization (I18n) - def user_not_authorized(exception) - policy_name = exception.policy.class.to_s.underscore +With the help of [ActionView’s translation helpers][#t], +these attributes can be used to display error messages in different languages, +or simply to encode them in a config file: + +```ruby +def user_not_authorized(exception) + # e.g., "post_policy.update?" + translation_key = "#{exception.policy.class.name.underscore}.#{exception.query}" - flash[:error] = t "#{policy_name}.#{exception.query}", scope: "pundit", default: :default - redirect_to(request.referrer || root_path) - end + flash[:alert] = t "#{translation_key}", scope: "pundit", default: :default + redirect_to(request.referrer || root_path) end ``` ```yaml +# config/locales/en.yml + en: - pundit: - default: 'You cannot perform this action.' - post_policy: - update?: 'You cannot edit this post!' - create?: 'You cannot create posts!' + pundit: + default: 'You cannot perform this action.' + post_policy: + update?: 'You cannot edit this post!' + create?: 'You cannot create posts!' ``` -Of course, this is just an example. Pundit is agnostic as to how you implement -your error messaging. - -## Multiple error messages per one policy action - -If there are multiple reasons that authorization can be denied, you can show different messages by raising exceptions in your policy: - -In your policy class raise `Pundit::NotAuthorizedError` with custom error message or I18n key in `reason` argument: +For even more control with this approach, you can raise the error explicitly, +which allows you to manually set the `exception.reason` metadata attribute: ```ruby class ProjectPolicy < ApplicationPolicy def create? - if user.has_paid_subscription? - if user.project_limit_reached? - raise Pundit::NotAuthorizedError, reason: 'user.project_limit_reached' - else - true - end - else + if !user.has_paid_subscription? raise Pundit::NotAuthorizedError, reason: 'user.paid_subscription_required' + elsif user.project_limit_reached? + raise Pundit::NotAuthorizedError, reason: 'user.project_limit_reached' end + + true end end -``` -Then you can get this error message in exception handler: -```ruby -rescue_from Pundit::NotAuthorizedError do |e| - message = e.reason ? I18n.t("pundit.errors.#{e.reason}") : e.message - flash[:error] = message, scope: "pundit", default: :default - redirect_to(request.referrer || root_path) +class ProjectsController < ApplicationController + private + + def user_not_authorized(exception) + message = if exception.reason + t "#{exception.reason}", scope: "pundit", default: :default + else + exception.message + end + + flash[:error] = message + redirect_to(request.referrer || root_path) + end end ``` ```yaml +# config/locales/en.yml + en: pundit: - errors: - user: - paid_subscription_required: 'Paid subscription is required' - project_limit_reached: 'Project limit is reached' + default: 'You cannot perform this action.' + user: + paid_subscription_required: 'Paid subscription is required' + project_limit_reached: 'Project limit is reached' +``` + +[#t]: https://api.rubyonrails.org/classes/ActionView/Helpers/TranslationHelper.html#method-i-translate + +### Customizing `exception.message` + +Here’s the default error message that’s given when authorization fails: + +```ruby +>> authorize(@post) +Pundit::NotAuthorizedError: not allowed to update? this Post +``` + +To define your own, set an `#error_message` attribute on your policies: + +```ruby +class ApplicationPolicy + attr_accessor :error_message +end + +class PostPolicy < ApplicationPolicy + def update? + self.error_message = if record.author != user + %("#{post.title}" cannot be edited by #{user}) + elsif record.archived? + 'cannot update an archived post' + end + + error_message.nil? + end +end + +>> authorize(@post) +Pundit::NotAuthorizedError: cannot update an archived post ``` ## Manually retrieving policies and scopes diff --git a/lib/pundit.rb b/lib/pundit.rb index 0b3e31a7..9b5dffec 100644 --- a/lib/pundit.rb +++ b/lib/pundit.rb @@ -33,7 +33,9 @@ def initialize(options = {}) @policy = options[:policy] @reason = options[:reason] - message = options.fetch(:message) { "not allowed to #{query} this #{record.class}" } + message = options[:message] || + policy.try(:error_message) || + "not allowed to #{query} this #{record.class}" end super(message) diff --git a/spec/pundit_spec.rb b/spec/pundit_spec.rb index b07da956..13396940 100644 --- a/spec/pundit_spec.rb +++ b/spec/pundit_spec.rb @@ -625,9 +625,49 @@ end describe "Pundit::NotAuthorizedError" do - it "can be initialized with a string as message" do - error = Pundit::NotAuthorizedError.new("must be logged in") - expect(error.message).to eq "must be logged in" + let(:error) { Pundit::NotAuthorizedError.new(init_arg) } + let(:expected_message) { "lorem ipsum dolor" } + + shared_examples "for error message" do + it "sets the appropriate error message" do + expect(error.message).to eq expected_message + end + end + + context "initialized with a string" do + let(:init_arg) { expected_message } + + include_examples "for error message" + end + + context "initialized with a hash" do + let(:init_arg) do + { message: expected_message, + query: :show?, + record: post, + policy: Pundit.policy(user, post) } + end + + context "containing :message" do + include_examples "for error message" + end + + context "containing :policy with an #error_message" do + before do + init_arg.except!(:message) + init_arg[:policy].error_message = expected_message + end + + include_examples "for error message" + end + + context "containing :policy with no #error_message" do + before { init_arg.except!(:message) } + + let(:expected_message) { "not allowed to show? this #{post.class}" } + + include_examples "for error message" + end end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 4197d548..9ada84ca 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -16,7 +16,7 @@ require "active_model/naming" require "action_controller/metal/strong_parameters" -class PostPolicy < Struct.new(:user, :post) +class PostPolicy < Struct.new(:user, :post, :error_message) class Scope < Struct.new(:user, :scope) def resolve scope.published