diff --git a/CHANGELOG.md b/CHANGELOG.md index 96530278..18ad81d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Improve the `NotAuthorizedError` message to include the policy class. +Furthermore, in the case where the record passed is a class instead of an instance, the class name is given. (#812) + ## 2.3.2 (2024-05-08) - Refactor: First pass of Pundit::Context (#797) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a44e1c24..09f2c76f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -28,3 +28,4 @@ Pundit version, OS version and any stack traces you have are very valuable. - **Send coherent history**. Make sure each individual commit in your pull request is meaningful. If you had to make multiple intermediate commits while developing, please squash them before sending them to us. +- **Update the CHANGELOG.** Don't forget to add your new changes to the CHANGELOG. diff --git a/README.md b/README.md index daa30138..d1dd6339 100644 --- a/README.md +++ b/README.md @@ -116,7 +116,7 @@ and the given record. It then infers from the action name, that it should call ``` ruby unless PostPolicy.new(current_user, @post).update? - raise Pundit::NotAuthorizedError, "not allowed to update? this #{@post.inspect}" + raise Pundit::NotAuthorizedError, "not allowed to PostPolicy#update? this Post" end ``` diff --git a/lib/pundit.rb b/lib/pundit.rb index 27dd032a..a03f4d3a 100644 --- a/lib/pundit.rb +++ b/lib/pundit.rb @@ -36,7 +36,10 @@ def initialize(options = {}) @record = options[:record] @policy = options[:policy] - message = options.fetch(:message) { "not allowed to #{query} this #{record.class}" } + message = options.fetch(:message) do + record_name = record.is_a?(Class) ? record.to_s : "this #{record.class}" + "not allowed to #{policy.class}##{query} #{record_name}" + end end super(message) diff --git a/spec/pundit_spec.rb b/spec/pundit_spec.rb index fadddc85..bcec8633 100644 --- a/spec/pundit_spec.rb +++ b/spec/pundit_spec.rb @@ -57,11 +57,11 @@ expect { Pundit.authorize(user, article_tag, :destroy?) }.to raise_error(Pundit::NotAuthorizedError) end - it "raises an error with a query and action" do + it "raises an error with the policy, query and record" do # rubocop:disable Style/MultilineBlockChain expect do Pundit.authorize(user, post, :destroy?) - end.to raise_error(Pundit::NotAuthorizedError, "not allowed to destroy? this Post") do |error| + end.to raise_error(Pundit::NotAuthorizedError, "not allowed to PostPolicy#destroy? this Post") do |error| expect(error.query).to eq :destroy? expect(error.record).to eq post expect(error.policy).to have_attributes( @@ -73,11 +73,12 @@ # rubocop:enable Style/MultilineBlockChain end - it "raises an error with a the record, query and action when the record is namespaced" do + it "raises an error with the policy, query and record when the record is namespaced" do # rubocop:disable Style/MultilineBlockChain expect do Pundit.authorize(user, [:project, :admin, comment], :destroy?) - end.to raise_error(Pundit::NotAuthorizedError, "not allowed to destroy? this Comment") do |error| + end.to raise_error(Pundit::NotAuthorizedError, + "not allowed to Project::Admin::CommentPolicy#destroy? this Comment") do |error| expect(error.query).to eq :destroy? expect(error.record).to eq comment expect(error.policy).to have_attributes( @@ -89,6 +90,22 @@ # rubocop:enable Style/MultilineBlockChain end + it "raises an error with the policy, query and the class name when a Class is given" do + # rubocop:disable Style/MultilineBlockChain + expect do + Pundit.authorize(user, Post, :destroy?) + end.to raise_error(Pundit::NotAuthorizedError, "not allowed to PostPolicy#destroy? Post") do |error| + expect(error.query).to eq :destroy? + expect(error.record).to eq Post + expect(error.policy).to have_attributes( + user: user, + record: Post + ) + expect(error.policy).to be_a(PostPolicy) + end + # rubocop:enable Style/MultilineBlockChain + end + it "raises an error with a invalid policy constructor" do expect do Pundit.authorize(user, wiki, :update?)