From b65c5a2bf61ced5a0ca27f38527f4757200947fb Mon Sep 17 00:00:00 2001 From: Geremia Taglialatela Date: Tue, 2 Jan 2024 14:21:24 +0100 Subject: [PATCH 01/22] Fix CodeClimate badge Use snippet from https://codeclimate.com/github/varvet/pundit/badges#maintainability-markdown [ci skip] --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 36d74d13..790962fa 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # Pundit [![Main](https://github.com/varvet/pundit/actions/workflows/main.yml/badge.svg)](https://github.com/varvet/pundit/actions/workflows/main.yml) -[![Code Climate](https://codeclimate.com/github/varvet/pundit.svg)](https://codeclimate.com/github/varvet/pundit) +[![Code Climate](https://api.codeclimate.com/v1/badges/a940030f96c9fb43046a/maintainability)](https://codeclimate.com/github/varvet/pundit/maintainability) [![Inline docs](http://inch-ci.org/github/varvet/pundit.svg?branch=main)](http://inch-ci.org/github/varvet/pundit) [![Gem Version](https://badge.fury.io/rb/pundit.svg)](http://badge.fury.io/rb/pundit) From 98dbab897bb0074c0fa358a25f6ec4d0d73e4bb8 Mon Sep 17 00:00:00 2001 From: Andy Waite Date: Fri, 12 Jan 2024 08:20:04 -0500 Subject: [PATCH 02/22] Qualify the Scope class name --- CHANGELOG.md | 2 ++ README.md | 4 ++-- lib/generators/pundit/policy/templates/policy.rb | 8 +++++++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d2195d5..b509deb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Update `ApplicationPolicy`` generator to qualify the `Scope` class name (#792) + - Policy generator uses `NoMethodError` to indicate `#resolve` is not implemented (#776) ## 2.3.1 (2023-07-17) diff --git a/README.md b/README.md index 36d74d13..d40d2f53 100644 --- a/README.md +++ b/README.md @@ -279,7 +279,7 @@ generator, or create your own base class to inherit from: ``` ruby class PostPolicy < ApplicationPolicy - class Scope < Scope + class Scope < ApplicationPolicy::Scope def resolve if user.admin? scope.all @@ -476,7 +476,7 @@ example, associations which might be `nil`. ```ruby class NilClassPolicy < ApplicationPolicy - class Scope < Scope + class Scope < ApplicationPolicy::Scope def resolve raise Pundit::NotDefinedError, "Cannot scope NilClass" end diff --git a/lib/generators/pundit/policy/templates/policy.rb b/lib/generators/pundit/policy/templates/policy.rb index 6798550b..a2102cb0 100644 --- a/lib/generators/pundit/policy/templates/policy.rb +++ b/lib/generators/pundit/policy/templates/policy.rb @@ -1,6 +1,12 @@ <% module_namespacing do -%> class <%= class_name %>Policy < ApplicationPolicy - class Scope < Scope + # NOTE: Up to Pundit v2.3.1, the inheritance was declared as + # `Scope < Scope` rather than `Scope < ApplicationPolicy::Scope`. + # In most cases the behavior will be identical, but if updating existing + # code, beware of possible changes to the ancestors: + # https://gist.github.com/Burgestrand/4b4bc22f31c8a95c425fc0e30d7ef1f5 + + class Scope < ApplicationPolicy::Scope # NOTE: Be explicit about which records you allow access to! # def resolve # scope.all From 14cf7697e6aa16aefab3d9a77ebd081e0c00ea0a Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Thu, 29 Feb 2024 19:48:01 +0100 Subject: [PATCH 03/22] Run for 3.2 and 3.3 too. 3.0 is soon EOL --- .github/workflows/main.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index afc481a2..6589890b 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -30,6 +30,8 @@ jobs: ruby-version: - '3.0' - '3.1' + - '3.2' + - '3.3' - 'jruby-9.3.10' # oldest supported jruby - 'jruby' include: # HEAD-versions From 03a4babc6069f4824f28c8afbb51cad799b19f51 Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Fri, 1 Mar 2024 10:51:02 +0100 Subject: [PATCH 04/22] Update rubocop target ruby to 3.1 We can't make a new release with this just yet, because we still support Ruby 3.0 for another 30 days. --- .rubocop.yml | 2 +- lib/pundit/authorization.rb | 2 +- lib/pundit/rspec.rb | 2 +- spec/policies/post_policy_spec.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index a4e374b2..84d701d4 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,5 +1,5 @@ AllCops: - TargetRubyVersion: 2.6 + TargetRubyVersion: 3.1 Exclude: - "lib/generators/**/templates/**/*" <% `git status --ignored --porcelain`.lines.grep(/^!! /).each do |path| %> diff --git a/lib/pundit/authorization.rb b/lib/pundit/authorization.rb index 1231f2a7..03096ca9 100644 --- a/lib/pundit/authorization.rb +++ b/lib/pundit/authorization.rb @@ -64,7 +64,7 @@ def authorize(record, query = nil, policy_class: nil) @_pundit_policy_authorized = true - Pundit.authorize(pundit_user, record, query, policy_class: policy_class, cache: policies) + Pundit.authorize(pundit_user, record, query, policy_class:, cache: policies) end # Allow this action not to perform authorization. diff --git a/lib/pundit/rspec.rb b/lib/pundit/rspec.rb index 243ebec7..6481b056 100644 --- a/lib/pundit/rspec.rb +++ b/lib/pundit/rspec.rb @@ -55,7 +55,7 @@ def permissions module DSL def permissions(*list, &block) - describe(list.to_sentence, permissions: list, caller: caller) { instance_eval(&block) } + describe(list.to_sentence, permissions: list, caller:) { instance_eval(&block) } end end diff --git a/spec/policies/post_policy_spec.rb b/spec/policies/post_policy_spec.rb index 54e301e0..eafeac0c 100644 --- a/spec/policies/post_policy_spec.rb +++ b/spec/policies/post_policy_spec.rb @@ -4,7 +4,7 @@ RSpec.describe PostPolicy do let(:user) { double } - let(:own_post) { double(user: user) } + let(:own_post) { double(user:) } let(:other_post) { double(user: double) } subject { described_class } From dc83235bcf2fcd27038ff6f83d170a760257e6f2 Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Fri, 1 Mar 2024 10:56:13 +0100 Subject: [PATCH 05/22] Update changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b509deb3..3af4cec9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,9 @@ ## Unreleased -- Update `ApplicationPolicy`` generator to qualify the `Scope` class name (#792) +- Dropped support for Ruby 3.0 (#796) +- Update `ApplicationPolicy` generator to qualify the `Scope` class name (#792) - Policy generator uses `NoMethodError` to indicate `#resolve` is not implemented (#776) ## 2.3.1 (2023-07-17) From 9a3e636851d92430ddfbcfe4e5b1886f2b4e8f64 Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Fri, 1 Mar 2024 10:56:38 +0100 Subject: [PATCH 06/22] Do not test on 3.0 --- .github/workflows/main.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 6589890b..89fb192a 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -28,7 +28,6 @@ jobs: fail-fast: false matrix: ruby-version: - - '3.0' - '3.1' - '3.2' - '3.3' From 3982ac387f4ac14cd4b328113976840d99642644 Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Thu, 7 Mar 2024 10:02:21 +0100 Subject: [PATCH 07/22] Do not use shorthand hash syntax because jruby JRuby targets 2.6, and doesn't support this syntax. JRuby 9.3 is still supported: https://github.com/jruby/jruby/wiki/Roadmap When JRuby 9.3 isn't supported any more, we can drop it. --- .rubocop.yml | 3 +++ lib/pundit/authorization.rb | 2 +- lib/pundit/rspec.rb | 2 +- spec/policies/post_policy_spec.rb | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 84d701d4..973e4a9d 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -73,3 +73,6 @@ Style/DoubleNegation: Style/Documentation: Enabled: false # TODO: Enable again once we have more docs + +Style/HashSyntax: + EnforcedShorthandSyntax: never diff --git a/lib/pundit/authorization.rb b/lib/pundit/authorization.rb index 03096ca9..1231f2a7 100644 --- a/lib/pundit/authorization.rb +++ b/lib/pundit/authorization.rb @@ -64,7 +64,7 @@ def authorize(record, query = nil, policy_class: nil) @_pundit_policy_authorized = true - Pundit.authorize(pundit_user, record, query, policy_class:, cache: policies) + Pundit.authorize(pundit_user, record, query, policy_class: policy_class, cache: policies) end # Allow this action not to perform authorization. diff --git a/lib/pundit/rspec.rb b/lib/pundit/rspec.rb index 6481b056..243ebec7 100644 --- a/lib/pundit/rspec.rb +++ b/lib/pundit/rspec.rb @@ -55,7 +55,7 @@ def permissions module DSL def permissions(*list, &block) - describe(list.to_sentence, permissions: list, caller:) { instance_eval(&block) } + describe(list.to_sentence, permissions: list, caller: caller) { instance_eval(&block) } end end diff --git a/spec/policies/post_policy_spec.rb b/spec/policies/post_policy_spec.rb index eafeac0c..54e301e0 100644 --- a/spec/policies/post_policy_spec.rb +++ b/spec/policies/post_policy_spec.rb @@ -4,7 +4,7 @@ RSpec.describe PostPolicy do let(:user) { double } - let(:own_post) { double(user:) } + let(:own_post) { double(user: user) } let(:other_post) { double(user: double) } subject { described_class } From f276b6a4f1180310e88b3e7c4ceda8ccf905876b Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Thu, 7 Mar 2024 10:05:21 +0100 Subject: [PATCH 08/22] Remove unused rubocops --- .rubocop.yml | 15 --------------- lib/pundit/policy_finder.rb | 2 +- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 973e4a9d..007b2a28 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -23,15 +23,6 @@ Metrics/ModuleLength: Layout/LineLength: Max: 120 -Metrics/AbcSize: - Enabled: false - -Metrics/CyclomaticComplexity: - Enabled: false - -Metrics/PerceivedComplexity: - Enabled: false - Gemspec/RequiredRubyVersion: Enabled: false @@ -62,12 +53,6 @@ Style/StringLiteralsInInterpolation: Style/StructInheritance: Enabled: false -Style/AndOr: - Enabled: false - -Style/Not: - Enabled: false - Style/DoubleNegation: Enabled: false diff --git a/lib/pundit/policy_finder.rb b/lib/pundit/policy_finder.rb index 40467d9d..ecc6e9bb 100644 --- a/lib/pundit/policy_finder.rb +++ b/lib/pundit/policy_finder.rb @@ -56,7 +56,7 @@ def policy! # @return [String] the name of the key this object would have in a params hash # - def param_key + def param_key # rubocop:disable Metrics/AbcSize model = object.is_a?(Array) ? object.last : object if model.respond_to?(:model_name) From dfceb9145c2db549c71c7e93eba1ca2043fc9f9f Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Thu, 28 Mar 2024 23:55:41 +0100 Subject: [PATCH 09/22] Fix broken sponsor logo in README --- README.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 65fe185a..29cc0bc9 100644 --- a/README.md +++ b/README.md @@ -9,16 +9,14 @@ Pundit provides a set of helpers which guide you in leveraging regular Ruby classes and object oriented design patterns to build a straightforward, robust, and scalable authorization system. -Links: +## Links: - [API documentation for the most recent version](http://www.rubydoc.info/gems/pundit) - [Source Code](https://github.com/varvet/pundit) - [Contributing](https://github.com/varvet/pundit/blob/main/CONTRIBUTING.md) - [Code of Conduct](https://github.com/varvet/pundit/blob/main/CODE_OF_CONDUCT.md) -Sponsored by: - -[Varvet](https://www.varvet.com) +Sponsored by: Varvet

Varvet logo ## Installation From 17616e64aee1692fd3481de1c67345912d73ed35 Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Tue, 26 Mar 2024 01:51:14 +0100 Subject: [PATCH 10/22] Refactor: do not use Struct in our test suite It has unwanted behaviour, such as being lenient in initialisation parameters and being loose in equality check. --- spec/pundit_spec.rb | 12 ++++- spec/spec_helper.rb | 116 ++++++++++++++++++++++++++++++++------------ 2 files changed, 94 insertions(+), 34 deletions(-) diff --git a/spec/pundit_spec.rb b/spec/pundit_spec.rb index 6911ba63..fadddc85 100644 --- a/spec/pundit_spec.rb +++ b/spec/pundit_spec.rb @@ -64,7 +64,11 @@ end.to raise_error(Pundit::NotAuthorizedError, "not allowed to destroy? this Post") do |error| expect(error.query).to eq :destroy? expect(error.record).to eq post - expect(error.policy).to eq Pundit.policy(user, post) + expect(error.policy).to have_attributes( + user: user, + record: post + ) + expect(error.policy).to be_a(PostPolicy) end # rubocop:enable Style/MultilineBlockChain end @@ -76,7 +80,11 @@ end.to raise_error(Pundit::NotAuthorizedError, "not allowed to destroy? this Comment") do |error| expect(error.query).to eq :destroy? expect(error.record).to eq comment - expect(error.policy).to eq Pundit.policy(user, [:project, :admin, comment]) + expect(error.policy).to have_attributes( + user: user, + record: comment + ) + expect(error.policy).to be_a(Project::Admin::CommentPolicy) end # rubocop:enable Style/MultilineBlockChain end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 26e41b53..6cc57f61 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -18,13 +18,33 @@ require "active_model/naming" require "action_controller/metal/strong_parameters" -class PostPolicy < Struct.new(:user, :post) - class Scope < Struct.new(:user, :scope) +class BasePolicy + class BaseScope + def initialize(user, scope) + @user = user + @scope = scope + end + + attr_reader :user, :scope + end + + def initialize(user, record) + @user = user + @record = record + end + + attr_reader :user, :record +end + +class PostPolicy < BasePolicy + class Scope < BaseScope def resolve scope.published end end + alias post record + def update? post.user == user end @@ -50,7 +70,13 @@ def permitted_attributes_for_revise end end -class Post < Struct.new(:user) +class Post + def initialize(user = nil) + @user = user + end + + attr_reader :user + def self.published :published end @@ -69,7 +95,7 @@ def inspect end module Customer - class Post < Post + class Post < ::Post def model_name OpenStruct.new(param_key: "customer_post") end @@ -92,16 +118,18 @@ def ==(other) end end -class CommentPolicy < Struct.new(:user, :comment) - class Scope < Struct.new(:user, :scope) +class CommentPolicy < BasePolicy + class Scope < BaseScope def resolve CommentScope.new(scope) end end + + alias comment record end -class PublicationPolicy < Struct.new(:user, :publication) - class Scope < Struct.new(:user, :scope) +class PublicationPolicy < BasePolicy + class Scope < BaseScope def resolve scope.published end @@ -132,7 +160,9 @@ def self.model_name class Article; end -class BlogPolicy < Struct.new(:user, :blog); end +class BlogPolicy < BasePolicy + alias blog record +end class Blog; end @@ -142,7 +172,7 @@ def self.policy_class end end -class ArticleTagOtherNamePolicy < Struct.new(:user, :tag) +class ArticleTagOtherNamePolicy < BasePolicy def show? true end @@ -150,6 +180,8 @@ def show? def destroy? false end + + alias tag record end class ArticleTag @@ -158,33 +190,41 @@ def self.policy_class end end -class CriteriaPolicy < Struct.new(:user, :criteria); end +class CriteriaPolicy < BasePolicy + alias criteria record +end module Project - class CommentPolicy < Struct.new(:user, :comment) - def update? - true - end - - class Scope < Struct.new(:user, :scope) + class CommentPolicy < BasePolicy + class Scope < BaseScope def resolve scope end end + + def update? + true + end + + alias comment record end - class CriteriaPolicy < Struct.new(:user, :criteria); end + class CriteriaPolicy < BasePolicy + alias criteria record + end - class PostPolicy < Struct.new(:user, :post) - class Scope < Struct.new(:user, :scope) + class PostPolicy < BasePolicy + class Scope < BaseScope def resolve scope.read end end + + alias post record end module Admin - class CommentPolicy < Struct.new(:user, :comment) + class CommentPolicy < BasePolicy def update? true end @@ -196,7 +236,7 @@ def destroy? end end -class DenierPolicy < Struct.new(:user, :record) +class DenierPolicy < BasePolicy def update? false end @@ -218,7 +258,7 @@ def initialize(current_user, action_name, params) end end -class NilClassPolicy < Struct.new(:user, :record) +class NilClassPolicy < BasePolicy class Scope def initialize(*) raise Pundit::NotDefinedError, "Cannot scope NilClass" @@ -247,8 +287,8 @@ class Thread def self.all; end end -class ThreadPolicy < Struct.new(:user, :thread) - class Scope < Struct.new(:user, :scope) +class ThreadPolicy < BasePolicy + class Scope < BaseScope def resolve # deliberate wrong useage of the method scope.all(:unvalid, :parameters) @@ -256,22 +296,34 @@ def resolve end end -class PostFourFiveSix < Struct.new(:user); end +class PostFourFiveSix + def initialize(user) + @user = user + end + + attr_reader(:user) +end class CommentFourFiveSix; extend ActiveModel::Naming; end module ProjectOneTwoThree - class CommentFourFiveSixPolicy < Struct.new(:user, :post); end + class CommentFourFiveSixPolicy < BasePolicy; end + + class CriteriaFourFiveSixPolicy < BasePolicy; end - class CriteriaFourFiveSixPolicy < Struct.new(:user, :criteria); end + class PostFourFiveSixPolicy < BasePolicy; end - class PostFourFiveSixPolicy < Struct.new(:user, :post); end + class TagFourFiveSix + def initialize(user) + @user = user + end - class TagFourFiveSix < Struct.new(:user); end + attr_reader(:user) + end - class TagFourFiveSixPolicy < Struct.new(:user, :tag); end + class TagFourFiveSixPolicy < BasePolicy; end class AvatarFourFiveSix; extend ActiveModel::Naming; end - class AvatarFourFiveSixPolicy < Struct.new(:user, :avatar); end + class AvatarFourFiveSixPolicy < BasePolicy; end end From 4f4127876865827788acdc298e7149208897edd4 Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Tue, 26 Mar 2024 21:08:43 +0100 Subject: [PATCH 11/22] Add missing test around cache usage This is not strictly a feature that was intended, but it's how it works so it needs to be supported in the future. --- spec/authorization_spec.rb | 28 ++++++++++++++++++++++------ spec/spec_helper.rb | 23 +++++++++++++++++++++++ 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/spec/authorization_spec.rb b/spec/authorization_spec.rb index 2995bfdb..4ff6c19c 100644 --- a/spec/authorization_spec.rb +++ b/spec/authorization_spec.rb @@ -3,10 +3,13 @@ require "spec_helper" describe Pundit::Authorization do - let(:controller) { Controller.new(user, "update", {}) } + def to_params(*args, **kwargs, &block) + ActionController::Parameters.new(*args, **kwargs, &block) + end + + let(:controller) { Controller.new(user, "update", to_params({})) } let(:user) { double } let(:post) { Post.new(user) } - let(:customer_post) { Customer::Post.new(user) } let(:comment) { Comment.new } let(:article) { Article.new } let(:article_tag) { ArticleTag.new } @@ -188,7 +191,7 @@ describe "#permitted_attributes" do it "checks policy for permitted attributes" do - params = ActionController::Parameters.new( + params = to_params( post: { title: "Hello", votes: 5, @@ -206,7 +209,8 @@ end it "checks policy for permitted attributes for record of a ActiveModel type" do - params = ActionController::Parameters.new( + customer_post = Customer::Post.new(user) + params = to_params( customer_post: { title: "Hello", votes: 5, @@ -224,11 +228,23 @@ "votes" => 5 ) end + + it "goes through the policy cache" do + params = to_params(post: { title: "Hello" }) + user = double + post = Post.new(user) + controller = Controller.new(user, "update", params) + + expect do + expect(controller.permitted_attributes(post)).to be_truthy + expect(controller.permitted_attributes(post)).to be_truthy + end.to change { PostPolicy.instances }.by(1) + end end describe "#permitted_attributes_for_action" do it "is checked if it is defined in the policy" do - params = ActionController::Parameters.new( + params = to_params( post: { title: "Hello", body: "blah", @@ -242,7 +258,7 @@ end it "can be explicitly set" do - params = ActionController::Parameters.new( + params = to_params( post: { title: "Hello", body: "blah", diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 6cc57f61..a66d8103 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -18,8 +18,31 @@ require "active_model/naming" require "action_controller/metal/strong_parameters" +module InstanceTracking + module ClassMethods + def instances + @instances || 0 + end + + attr_writer :instances + end + + def self.prepended(other) + other.extend(ClassMethods) + end + + def initialize(*args, **kwargs, &block) + self.class.instances += 1 + super(*args, **kwargs, &block) + end +end + class BasePolicy + prepend InstanceTracking + class BaseScope + prepend InstanceTracking + def initialize(user, scope) @user = user @scope = scope From 9045680e4faef2ee87a2273c09d7d77d7d024f6e Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Fri, 1 Mar 2024 10:49:47 +0100 Subject: [PATCH 12/22] First pass of Pundit::Context See https://github.com/varvet/pundit/discussions/774 An API-change in this commit is that we're introducing a new method named `pundit` in `Pundit::Authorization`. We should not take this change lightly, because this is a namespace that is technically not ours as we're being included in controllers. The hope is that the context method will help _reduce_ the overall API surface. Additionally this will hopefully help us introduce an API for a namespaced Pundit. --- lib/pundit.rb | 106 +++++--------------------------- lib/pundit/authorization.rb | 14 ++++- lib/pundit/context.rb | 118 ++++++++++++++++++++++++++++++++++++ 3 files changed, 145 insertions(+), 93 deletions(-) create mode 100644 lib/pundit/context.rb diff --git a/lib/pundit.rb b/lib/pundit.rb index 5eab334f..e234ea92 100644 --- a/lib/pundit.rb +++ b/lib/pundit.rb @@ -8,6 +8,7 @@ require "active_support/core_ext/module/introspection" require "active_support/dependencies/autoload" require "pundit/authorization" +require "pundit/context" # @api private # To avoid name clashes with common Error naming when mixing in Pundit, @@ -64,104 +65,29 @@ def self.included(base) end class << self - # Retrieves the policy for the given record, initializing it with the - # record and user and finally throwing an error if the user is not - # authorized to perform the given action. - # - # @param user [Object] the user that initiated the action - # @param possibly_namespaced_record [Object, Array] the object we're checking permissions of - # @param query [Symbol, String] the predicate method to check on the policy (e.g. `:show?`) - # @param policy_class [Class] the policy class we want to force use of - # @param cache [#[], #[]=] a Hash-like object to cache the found policy instance in - # @raise [NotAuthorizedError] if the given query method returned false - # @return [Object] Always returns the passed object record - def authorize(user, possibly_namespaced_record, query, policy_class: nil, cache: {}) - record = pundit_model(possibly_namespaced_record) - policy = if policy_class - policy_class.new(user, record) - else - cache[possibly_namespaced_record] ||= policy!(user, possibly_namespaced_record) - end - - raise NotAuthorizedError, query: query, record: record, policy: policy unless policy.public_send(query) - - record + # @see [Pundit::Context#authorize] + def authorize(user, record, query, policy_class: nil, cache: {}) + Context.new(user: user, policy_cache: cache).authorize(record, query: query, policy_class: policy_class) end - # Retrieves the policy scope for the given record. - # - # @see https://github.com/varvet/pundit#scopes - # @param user [Object] the user that initiated the action - # @param scope [Object] the object we're retrieving the policy scope for - # @raise [InvalidConstructorError] if the policy constructor called incorrectly - # @return [Scope{#resolve}, nil] instance of scope class which can resolve to a scope - def policy_scope(user, scope) - policy_scope_class = PolicyFinder.new(scope).scope - return unless policy_scope_class - - begin - policy_scope = policy_scope_class.new(user, pundit_model(scope)) - rescue ArgumentError - raise InvalidConstructorError, "Invalid #<#{policy_scope_class}> constructor is called" - end - - policy_scope.resolve + # @see [Pundit::Context#policy_scope] + def policy_scope(user, *args, **kwargs, &block) + Context.new(user: user).policy_scope(*args, **kwargs, &block) end - # Retrieves the policy scope for the given record. - # - # @see https://github.com/varvet/pundit#scopes - # @param user [Object] the user that initiated the action - # @param scope [Object] the object we're retrieving the policy scope for - # @raise [NotDefinedError] if the policy scope cannot be found - # @raise [InvalidConstructorError] if the policy constructor called incorrectly - # @return [Scope{#resolve}] instance of scope class which can resolve to a scope - def policy_scope!(user, scope) - policy_scope_class = PolicyFinder.new(scope).scope! - return unless policy_scope_class - - begin - policy_scope = policy_scope_class.new(user, pundit_model(scope)) - rescue ArgumentError - raise InvalidConstructorError, "Invalid #<#{policy_scope_class}> constructor is called" - end - - policy_scope.resolve + # @see [Pundit::Context#policy_scope!] + def policy_scope!(user, *args, **kwargs, &block) + Context.new(user: user).policy_scope!(*args, **kwargs, &block) end - # Retrieves the policy for the given record. - # - # @see https://github.com/varvet/pundit#policies - # @param user [Object] the user that initiated the action - # @param record [Object] the object we're retrieving the policy for - # @raise [InvalidConstructorError] if the policy constructor called incorrectly - # @return [Object, nil] instance of policy class with query methods - def policy(user, record) - policy = PolicyFinder.new(record).policy - policy&.new(user, pundit_model(record)) - rescue ArgumentError - raise InvalidConstructorError, "Invalid #<#{policy}> constructor is called" + # @see [Pundit::Context#policy] + def policy(user, *args, **kwargs, &block) + Context.new(user: user).policy(*args, **kwargs, &block) end - # Retrieves the policy for the given record. - # - # @see https://github.com/varvet/pundit#policies - # @param user [Object] the user that initiated the action - # @param record [Object] the object we're retrieving the policy for - # @raise [NotDefinedError] if the policy cannot be found - # @raise [InvalidConstructorError] if the policy constructor called incorrectly - # @return [Object] instance of policy class with query methods - def policy!(user, record) - policy = PolicyFinder.new(record).policy! - policy.new(user, pundit_model(record)) - rescue ArgumentError - raise InvalidConstructorError, "Invalid #<#{policy}> constructor is called" - end - - private - - def pundit_model(record) - record.is_a?(Array) ? record.last : record + # @see [Pundit::Context#policy!] + def policy!(user, *args, **kwargs, &block) + Context.new(user: user).policy!(*args, **kwargs, &block) end end diff --git a/lib/pundit/authorization.rb b/lib/pundit/authorization.rb index 1231f2a7..b47cb20c 100644 --- a/lib/pundit/authorization.rb +++ b/lib/pundit/authorization.rb @@ -15,6 +15,14 @@ module Authorization protected + # @return [Pundit::Context] a new instance of {Pundit::Context} with the current user + def pundit + @pundit ||= Pundit::Context.new( + user: pundit_user, + policy_cache: policies + ) + end + # @return [Boolean] whether authorization has been performed, i.e. whether # one {#authorize} or {#skip_authorization} has been called def pundit_policy_authorized? @@ -64,7 +72,7 @@ def authorize(record, query = nil, policy_class: nil) @_pundit_policy_authorized = true - Pundit.authorize(pundit_user, record, query, policy_class: policy_class, cache: policies) + pundit.authorize(record, query: query, policy_class: policy_class) end # Allow this action not to perform authorization. @@ -100,7 +108,7 @@ def policy_scope(scope, policy_scope_class: nil) # @param record [Object] the object we're retrieving the policy for # @return [Object, nil] instance of policy class with query methods def policy(record) - policies[record] ||= Pundit.policy!(pundit_user, record) + policies[record] ||= pundit.policy!(record) end # Retrieves a set of permitted attributes from the policy by instantiating @@ -162,7 +170,7 @@ def pundit_user private def pundit_policy_scope(scope) - policy_scopes[scope] ||= Pundit.policy_scope!(pundit_user, scope) + policy_scopes[scope] ||= pundit.policy_scope!(scope) end end end diff --git a/lib/pundit/context.rb b/lib/pundit/context.rb new file mode 100644 index 00000000..9e8d6078 --- /dev/null +++ b/lib/pundit/context.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +module Pundit + class Context + def initialize(user:, policy_cache: {}) + @user = user + @policy_cache = policy_cache + end + + attr_reader :user + + # @api private + attr_reader :policy_cache + + # Retrieves the policy for the given record, initializing it with the + # record and user and finally throwing an error if the user is not + # authorized to perform the given action. + # + # @param user [Object] the user that initiated the action + # @param possibly_namespaced_record [Object, Array] the object we're checking permissions of + # @param query [Symbol, String] the predicate method to check on the policy (e.g. `:show?`) + # @param policy_class [Class] the policy class we want to force use of + # @raise [NotAuthorizedError] if the given query method returned false + # @return [Object] Always returns the passed object record + def authorize(possibly_namespaced_record, query:, policy_class:) + record = pundit_model(possibly_namespaced_record) + policy = if policy_class + policy_class.new(user, record) + else + policy_cache[possibly_namespaced_record] ||= policy!(possibly_namespaced_record) + end + + raise NotAuthorizedError, query: query, record: record, policy: policy unless policy.public_send(query) + + record + end + + # Retrieves the policy scope for the given record. + # + # @see https://github.com/varvet/pundit#scopes + # @param user [Object] the user that initiated the action + # @param scope [Object] the object we're retrieving the policy scope for + # @raise [InvalidConstructorError] if the policy constructor called incorrectly + # @return [Scope{#resolve}, nil] instance of scope class which can resolve to a scope + def policy_scope(scope) + policy_scope_class = policy_finder(scope).scope + return unless policy_scope_class + + begin + policy_scope = policy_scope_class.new(user, pundit_model(scope)) + rescue ArgumentError + raise InvalidConstructorError, "Invalid #<#{policy_scope_class}> constructor is called" + end + + policy_scope.resolve + end + + # Retrieves the policy scope for the given record. + # + # @see https://github.com/varvet/pundit#scopes + # @param user [Object] the user that initiated the action + # @param scope [Object] the object we're retrieving the policy scope for + # @raise [NotDefinedError] if the policy scope cannot be found + # @raise [InvalidConstructorError] if the policy constructor called incorrectly + # @return [Scope{#resolve}] instance of scope class which can resolve to a scope + def policy_scope!(scope) + policy_scope_class = policy_finder(scope).scope! + return unless policy_scope_class + + begin + policy_scope = policy_scope_class.new(user, pundit_model(scope)) + rescue ArgumentError + raise InvalidConstructorError, "Invalid #<#{policy_scope_class}> constructor is called" + end + + policy_scope.resolve + end + + # Retrieves the policy for the given record. + # + # @see https://github.com/varvet/pundit#policies + # @param user [Object] the user that initiated the action + # @param record [Object] the object we're retrieving the policy for + # @raise [InvalidConstructorError] if the policy constructor called incorrectly + # @return [Object, nil] instance of policy class with query methods + def policy(record) + policy = policy_finder(record).policy + policy&.new(user, pundit_model(record)) + rescue ArgumentError + raise InvalidConstructorError, "Invalid #<#{policy}> constructor is called" + end + + # Retrieves the policy for the given record. + # + # @see https://github.com/varvet/pundit#policies + # @param user [Object] the user that initiated the action + # @param record [Object] the object we're retrieving the policy for + # @raise [NotDefinedError] if the policy cannot be found + # @raise [InvalidConstructorError] if the policy constructor called incorrectly + # @return [Object] instance of policy class with query methods + def policy!(record) + policy = policy_finder(record).policy! + policy.new(user, pundit_model(record)) + rescue ArgumentError + raise InvalidConstructorError, "Invalid #<#{policy}> constructor is called" + end + + private + + def policy_finder(record) + PolicyFinder.new(record) + end + + def pundit_model(record) + record.is_a?(Array) ? record.last : record + end + end +end From 266cae036279a7623cc0626be82eb9a392f93177 Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Mon, 25 Mar 2024 23:13:11 +0100 Subject: [PATCH 13/22] Make it clearer why these methods have a bang An `!` means caution, but not much more. These comments help highlight what to be cautious about. Co-authored-by: mattzollinhofer --- lib/pundit/context.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pundit/context.rb b/lib/pundit/context.rb index 9e8d6078..0028ba1e 100644 --- a/lib/pundit/context.rb +++ b/lib/pundit/context.rb @@ -55,7 +55,7 @@ def policy_scope(scope) policy_scope.resolve end - # Retrieves the policy scope for the given record. + # Retrieves the policy scope for the given record. Raises if not found. # # @see https://github.com/varvet/pundit#scopes # @param user [Object] the user that initiated the action @@ -90,7 +90,7 @@ def policy(record) raise InvalidConstructorError, "Invalid #<#{policy}> constructor is called" end - # Retrieves the policy for the given record. + # Retrieves the policy for the given record. Raises if not found. # # @see https://github.com/varvet/pundit#policies # @param user [Object] the user that initiated the action From aedc8620fd267f4b0219975fe76589c1812906b4 Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Tue, 26 Mar 2024 01:35:59 +0100 Subject: [PATCH 14/22] Refactor: API contract for policy cache See https://github.com/varvet/pundit/pull/801#issuecomment-2018996706 We'd like to support caching a few more lookups, but that's backwards-incompatible. By swapping out the cache, we could have a legacy cache (that only caches "the old way"), and a new cache that will cache _more_ things. This allows backwards-compatibility with the same API interface. --- lib/pundit.rb | 12 ++++++++++-- lib/pundit/authorization.rb | 2 +- lib/pundit/cache_store/hash_store.rb | 16 ++++++++++++++++ lib/pundit/cache_store/null_store.rb | 18 ++++++++++++++++++ lib/pundit/context.rb | 6 ++++-- 5 files changed, 49 insertions(+), 5 deletions(-) create mode 100644 lib/pundit/cache_store/hash_store.rb create mode 100644 lib/pundit/cache_store/null_store.rb diff --git a/lib/pundit.rb b/lib/pundit.rb index e234ea92..9175cb55 100644 --- a/lib/pundit.rb +++ b/lib/pundit.rb @@ -9,6 +9,8 @@ require "active_support/dependencies/autoload" require "pundit/authorization" require "pundit/context" +require "pundit/cache_store/null_store" +require "pundit/cache_store/hash_store" # @api private # To avoid name clashes with common Error naming when mixing in Pundit, @@ -66,8 +68,14 @@ def self.included(base) class << self # @see [Pundit::Context#authorize] - def authorize(user, record, query, policy_class: nil, cache: {}) - Context.new(user: user, policy_cache: cache).authorize(record, query: query, policy_class: policy_class) + def authorize(user, record, query, policy_class: nil, cache: nil) + context = if cache + Context.new(user: user, policy_cache: cache) + else + Context.new(user: user) + end + + context.authorize(record, query: query, policy_class: policy_class) end # @see [Pundit::Context#policy_scope] diff --git a/lib/pundit/authorization.rb b/lib/pundit/authorization.rb index b47cb20c..5d02c92c 100644 --- a/lib/pundit/authorization.rb +++ b/lib/pundit/authorization.rb @@ -19,7 +19,7 @@ module Authorization def pundit @pundit ||= Pundit::Context.new( user: pundit_user, - policy_cache: policies + policy_cache: Pundit::CacheStore::HashStore.new(policies) ) end diff --git a/lib/pundit/cache_store/hash_store.rb b/lib/pundit/cache_store/hash_store.rb new file mode 100644 index 00000000..e6c47f17 --- /dev/null +++ b/lib/pundit/cache_store/hash_store.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Pundit + module CacheStore + # @api private + class HashStore + def initialize(hash = {}) + @store = hash + end + + def fetch(key) + @store[key] ||= yield + end + end + end +end diff --git a/lib/pundit/cache_store/null_store.rb b/lib/pundit/cache_store/null_store.rb new file mode 100644 index 00000000..60f484e0 --- /dev/null +++ b/lib/pundit/cache_store/null_store.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Pundit + module CacheStore + # @api private + class NullStore + @instance = new + + class << self + attr_reader :instance + end + + def fetch(*, **) + yield + end + end + end +end diff --git a/lib/pundit/context.rb b/lib/pundit/context.rb index 0028ba1e..a3387490 100644 --- a/lib/pundit/context.rb +++ b/lib/pundit/context.rb @@ -2,7 +2,7 @@ module Pundit class Context - def initialize(user:, policy_cache: {}) + def initialize(user:, policy_cache: CacheStore::NullStore.instance) @user = user @policy_cache = policy_cache end @@ -27,7 +27,9 @@ def authorize(possibly_namespaced_record, query:, policy_class:) policy = if policy_class policy_class.new(user, record) else - policy_cache[possibly_namespaced_record] ||= policy!(possibly_namespaced_record) + policy_cache.fetch(possibly_namespaced_record) do + policy!(possibly_namespaced_record) + end end raise NotAuthorizedError, query: query, record: record, policy: policy unless policy.public_send(query) From 8136c08d8405f2ba5d82e3e824c35fa48e0f1613 Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Tue, 26 Mar 2024 22:15:58 +0100 Subject: [PATCH 15/22] Apply the cache strategy for all policy lookups This should be backwards-compatible because: * `Pundit.policy` and `Pundit.policy!` are unaffected by this, since they use a non-cache cache. * `Pundit::Context` is new, so nobody is relying on these methods yet. * `Pundit::Authorization#policy` already cached the policy anyway. * `policies` is explicitly discouraged, but even if it is relied upon it works as it always has, by default. This is an improvement because we can change the cache strategy to introduce new features without breaking backwards-compatibility. --- lib/pundit/authorization.rb | 4 ++-- lib/pundit/context.rb | 27 ++++++++++++++++----------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/lib/pundit/authorization.rb b/lib/pundit/authorization.rb index 5d02c92c..08b3f115 100644 --- a/lib/pundit/authorization.rb +++ b/lib/pundit/authorization.rb @@ -106,9 +106,9 @@ def policy_scope(scope, policy_scope_class: nil) # # @see https://github.com/varvet/pundit#policies # @param record [Object] the object we're retrieving the policy for - # @return [Object, nil] instance of policy class with query methods + # @return [Object] instance of policy class with query methods def policy(record) - policies[record] ||= pundit.policy!(record) + pundit.policy!(record) end # Retrieves a set of permitted attributes from the policy by instantiating diff --git a/lib/pundit/context.rb b/lib/pundit/context.rb index a3387490..732be8d9 100644 --- a/lib/pundit/context.rb +++ b/lib/pundit/context.rb @@ -27,9 +27,7 @@ def authorize(possibly_namespaced_record, query:, policy_class:) policy = if policy_class policy_class.new(user, record) else - policy_cache.fetch(possibly_namespaced_record) do - policy!(possibly_namespaced_record) - end + policy!(possibly_namespaced_record) end raise NotAuthorizedError, query: query, record: record, policy: policy unless policy.public_send(query) @@ -86,10 +84,7 @@ def policy_scope!(scope) # @raise [InvalidConstructorError] if the policy constructor called incorrectly # @return [Object, nil] instance of policy class with query methods def policy(record) - policy = policy_finder(record).policy - policy&.new(user, pundit_model(record)) - rescue ArgumentError - raise InvalidConstructorError, "Invalid #<#{policy}> constructor is called" + cached_policy(record, &:policy) end # Retrieves the policy for the given record. Raises if not found. @@ -101,14 +96,24 @@ def policy(record) # @raise [InvalidConstructorError] if the policy constructor called incorrectly # @return [Object] instance of policy class with query methods def policy!(record) - policy = policy_finder(record).policy! - policy.new(user, pundit_model(record)) - rescue ArgumentError - raise InvalidConstructorError, "Invalid #<#{policy}> constructor is called" + cached_policy(record, &:policy!) end private + def cached_policy(record) + policy_cache.fetch(record) do + policy = yield policy_finder(record) + next unless policy + + begin + policy.new(user, pundit_model(record)) + rescue ArgumentError + raise InvalidConstructorError, "Invalid #<#{policy}> constructor is called" + end + end + end + def policy_finder(record) PolicyFinder.new(record) end From 6f04482534f5d0cf093f2f7dd8d0ce185d297508 Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Thu, 28 Mar 2024 13:03:37 +0100 Subject: [PATCH 16/22] Open cache strategy API up for including user in cache key ... and possibly by extension also the `policy_class`. --- lib/pundit.rb | 2 +- lib/pundit/authorization.rb | 2 +- .../{hash_store.rb => legacy_store.rb} | 7 ++++--- lib/pundit/context.rb | 18 ++++++++++-------- 4 files changed, 16 insertions(+), 13 deletions(-) rename lib/pundit/cache_store/{hash_store.rb => legacy_store.rb} (62%) diff --git a/lib/pundit.rb b/lib/pundit.rb index 9175cb55..27dd032a 100644 --- a/lib/pundit.rb +++ b/lib/pundit.rb @@ -10,7 +10,7 @@ require "pundit/authorization" require "pundit/context" require "pundit/cache_store/null_store" -require "pundit/cache_store/hash_store" +require "pundit/cache_store/legacy_store" # @api private # To avoid name clashes with common Error naming when mixing in Pundit, diff --git a/lib/pundit/authorization.rb b/lib/pundit/authorization.rb index 08b3f115..bc4cc4f4 100644 --- a/lib/pundit/authorization.rb +++ b/lib/pundit/authorization.rb @@ -19,7 +19,7 @@ module Authorization def pundit @pundit ||= Pundit::Context.new( user: pundit_user, - policy_cache: Pundit::CacheStore::HashStore.new(policies) + policy_cache: Pundit::CacheStore::LegacyStore.new(policies) ) end diff --git a/lib/pundit/cache_store/hash_store.rb b/lib/pundit/cache_store/legacy_store.rb similarity index 62% rename from lib/pundit/cache_store/hash_store.rb rename to lib/pundit/cache_store/legacy_store.rb index e6c47f17..d8ca6e9b 100644 --- a/lib/pundit/cache_store/hash_store.rb +++ b/lib/pundit/cache_store/legacy_store.rb @@ -3,13 +3,14 @@ module Pundit module CacheStore # @api private - class HashStore + class LegacyStore def initialize(hash = {}) @store = hash end - def fetch(key) - @store[key] ||= yield + def fetch(user:, record:) + _ = user + @store[record] ||= yield end end end diff --git a/lib/pundit/context.rb b/lib/pundit/context.rb index 732be8d9..a5f86716 100644 --- a/lib/pundit/context.rb +++ b/lib/pundit/context.rb @@ -84,7 +84,7 @@ def policy_scope!(scope) # @raise [InvalidConstructorError] if the policy constructor called incorrectly # @return [Object, nil] instance of policy class with query methods def policy(record) - cached_policy(record, &:policy) + cached_find(record, &:policy) end # Retrieves the policy for the given record. Raises if not found. @@ -96,20 +96,22 @@ def policy(record) # @raise [InvalidConstructorError] if the policy constructor called incorrectly # @return [Object] instance of policy class with query methods def policy!(record) - cached_policy(record, &:policy!) + cached_find(record, &:policy!) end private - def cached_policy(record) - policy_cache.fetch(record) do - policy = yield policy_finder(record) - next unless policy + def cached_find(record) + policy_cache.fetch(user: user, record: record) do + klass = yield policy_finder(record) + next unless klass + + model = pundit_model(record) begin - policy.new(user, pundit_model(record)) + klass.new(user, model) rescue ArgumentError - raise InvalidConstructorError, "Invalid #<#{policy}> constructor is called" + raise InvalidConstructorError, "Invalid #<#{klass}> constructor is called" end end end From 02d8e11f9a239092362f0aa073d84276312509f3 Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Wed, 8 May 2024 15:18:57 -0400 Subject: [PATCH 17/22] Add trusted publishing (rubygems) --- .github/workflows/push_gem.yml | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 .github/workflows/push_gem.yml diff --git a/.github/workflows/push_gem.yml b/.github/workflows/push_gem.yml new file mode 100644 index 00000000..67745e0c --- /dev/null +++ b/.github/workflows/push_gem.yml @@ -0,0 +1,33 @@ +name: Push Gem + +on: + workflow_dispatch: + +permissions: + contents: read + +jobs: + push: + if: github.repository == 'varvet/pundit' + runs-on: ubuntu-latest + + permissions: + contents: write + id-token: write + + steps: + # Set up + - name: Harden Runner + uses: step-security/harden-runner@a4aa98b93cab29d9b1101a6143fb8bce00e2eac4 # v2.7.1 + with: + egress-policy: audit + + - uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 + - name: Set up Ruby + uses: ruby/setup-ruby@cacc9f1c0b3f4eb8a16a6bb0ed10897b43b9de49 # v1.176.0 + with: + bundler-cache: true + ruby-version: ruby + + # Release + - uses: rubygems/release-gem@612653d273a73bdae1df8453e090060bb4db5f31 # v1 From d033604522220446af61515edef2839d42f63af5 Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Wed, 8 May 2024 15:19:19 -0400 Subject: [PATCH 18/22] Bump to v2.3.2 --- CHANGELOG.md | 10 +++++++++- lib/pundit/version.rb | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3af4cec9..96530278 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,11 +2,19 @@ ## Unreleased -- Dropped support for Ruby 3.0 (#796) +## 2.3.2 (2024-05-08) + +- Refactor: First pass of Pundit::Context (#797) + +## Changed - Update `ApplicationPolicy` generator to qualify the `Scope` class name (#792) - Policy generator uses `NoMethodError` to indicate `#resolve` is not implemented (#776) +## Deprecated + +- Dropped support for Ruby 3.0 (#796) + ## 2.3.1 (2023-07-17) ### Fixed diff --git a/lib/pundit/version.rb b/lib/pundit/version.rb index c7eec700..6cdaa20e 100644 --- a/lib/pundit/version.rb +++ b/lib/pundit/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Pundit - VERSION = "2.3.1" + VERSION = "2.3.2" end From 3f68dcea76414c27eebaa9511217e2d3de2bf7c7 Mon Sep 17 00:00:00 2001 From: Sean Devine Date: Sat, 18 Feb 2023 14:21:11 +0000 Subject: [PATCH 19/22] feat: Add customizable permit description --- lib/pundit/rspec.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/pundit/rspec.rb b/lib/pundit/rspec.rb index 243ebec7..221c77eb 100644 --- a/lib/pundit/rspec.rb +++ b/lib/pundit/rspec.rb @@ -5,6 +5,10 @@ module RSpec module Matchers extend ::RSpec::Matchers::DSL + def self.description(&block) + @description = block.call + end + # rubocop:disable Metrics/BlockLength matcher :permit do |user, record| match_proc = lambda do |policy| @@ -33,6 +37,14 @@ module Matchers "#{record} but #{@violating_permissions.to_sentence} #{was_were} granted" end + description do + if Pundit::RSpec::Matchers.instance_variable_defined?(:@description) + Pundit::RSpec::Matchers.instance_variable_get(:@description) + else + super() + end + end + if respond_to?(:match_when_negated) match(&match_proc) match_when_negated(&match_when_negated_proc) From f5e7ca36c049177c0e9be9e1f70fdccfde85a7dd Mon Sep 17 00:00:00 2001 From: Sean Devine Date: Sat, 18 Feb 2023 14:38:35 +0000 Subject: [PATCH 20/22] docs: Update the README about description customization --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index 29cc0bc9..9097e39d 100644 --- a/README.md +++ b/README.md @@ -783,6 +783,14 @@ describe PostPolicy do end ``` +You can customize the description used for the `permit` matcher: + +``` ruby +Pundit::RSpec::Matchers.description do + "permit the user" +end +``` + An alternative approach to Pundit policy specs is scoping them to a user context as outlined in this [excellent post](http://thunderboltlabs.com/blog/2013/03/27/testing-pundit-policies-with-rspec/) and implemented in the third party [pundit-matchers](https://github.com/punditcommunity/pundit-matchers) gem. From 4ca1e4516496194a71ba9d60a43e3028b8a165cd Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Sun, 19 Feb 2023 22:09:17 -0600 Subject: [PATCH 21/22] chore: adjust interface, update readme --- README.md | 25 ++++++++++++++++++++++++- lib/pundit/rspec.rb | 16 +++++++++------- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 9097e39d..85416dc3 100644 --- a/README.md +++ b/README.md @@ -786,11 +786,34 @@ end You can customize the description used for the `permit` matcher: ``` ruby -Pundit::RSpec::Matchers.description do +Pundit::RSpec::Matchers.description = "permit the user" +``` + +given the spec + +```ruby +permissions :update?, :show? do + it { expect(policy).to permit(user, record) } end ``` +will change the output from + +``` +update? and show? + is expected to permit # and # +``` + +to + +``` +update? and show? + is expected to permit the user +``` + +which may be desirable when distributing policy specs as documentation. + An alternative approach to Pundit policy specs is scoping them to a user context as outlined in this [excellent post](http://thunderboltlabs.com/blog/2013/03/27/testing-pundit-policies-with-rspec/) and implemented in the third party [pundit-matchers](https://github.com/punditcommunity/pundit-matchers) gem. diff --git a/lib/pundit/rspec.rb b/lib/pundit/rspec.rb index 221c77eb..c88407d2 100644 --- a/lib/pundit/rspec.rb +++ b/lib/pundit/rspec.rb @@ -5,8 +5,14 @@ module RSpec module Matchers extend ::RSpec::Matchers::DSL - def self.description(&block) - @description = block.call + def self.default_description=(default_description) + @default_description = default_description + end + + def self.default_description + return @default_description if instance_variable_defined?(:@default_description) + + nil end # rubocop:disable Metrics/BlockLength @@ -38,11 +44,7 @@ def self.description(&block) end description do - if Pundit::RSpec::Matchers.instance_variable_defined?(:@description) - Pundit::RSpec::Matchers.instance_variable_get(:@description) - else - super() - end + Pundit::RSpec::Matchers.default_description || super() end if respond_to?(:match_when_negated) From 86c72511c21cdf9b8b97bb2a314b3d8f97248aa4 Mon Sep 17 00:00:00 2001 From: Greg Fletcher Date: Wed, 8 May 2024 18:24:10 -0400 Subject: [PATCH 22/22] refactor: Update proposed change to address feedback from Pundit maintainers --- lib/pundit/rspec.rb | 14 +++++++------- spec/policies/post_policy_spec.rb | 27 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/lib/pundit/rspec.rb b/lib/pundit/rspec.rb index c88407d2..5becb308 100644 --- a/lib/pundit/rspec.rb +++ b/lib/pundit/rspec.rb @@ -5,14 +5,14 @@ module RSpec module Matchers extend ::RSpec::Matchers::DSL - def self.default_description=(default_description) - @default_description = default_description - end + class << self + attr_writer :description - def self.default_description - return @default_description if instance_variable_defined?(:@default_description) + def description(user, record) + return @description.call(user, record) if defined?(@description) && @description.respond_to?(:call) - nil + @description + end end # rubocop:disable Metrics/BlockLength @@ -44,7 +44,7 @@ def self.default_description end description do - Pundit::RSpec::Matchers.default_description || super() + Pundit::RSpec::Matchers.description(user, record) || super() end if respond_to?(:match_when_negated) diff --git a/spec/policies/post_policy_spec.rb b/spec/policies/post_policy_spec.rb index 54e301e0..912fe74b 100644 --- a/spec/policies/post_policy_spec.rb +++ b/spec/policies/post_policy_spec.rb @@ -18,5 +18,32 @@ should permit(user, other_post) end.to raise_error(RSpec::Expectations::ExpectationNotMetError) end + + it "uses the default description if not overridden" do + expect(permit(user, own_post).description).to eq("permit #{user.inspect} and #{own_post.inspect}") + end + + context "when the matcher description is overridden" do + after do + Pundit::RSpec::Matchers.description = nil + end + + it "sets a custom matcher description with a Proc" do + allow(user).to receive(:role).and_return("default_role") + allow(own_post).to receive(:id).and_return(1) + + Pundit::RSpec::Matchers.description = lambda { |user, record| + "permit user with role #{user.role} to access record with ID #{record.id}" + } + + description = permit(user, own_post).description + expect(description).to eq("permit user with role default_role to access record with ID 1") + end + + it "sets a custom matcher description with a string" do + Pundit::RSpec::Matchers.description = "permit user" + expect(permit(user, own_post).description).to eq("permit user") + end + end end end