From 33424381c13878233c84af8ab91a0c69715a305d Mon Sep 17 00:00:00 2001 From: Linus Marton Date: Sun, 17 Jun 2018 09:18:41 +0200 Subject: [PATCH 1/5] Pass the last thing in array to policy When passing an array to Pundit to specify namespacing we should only pass the last element of the array to the policy. --- lib/pundit.rb | 10 ++++++++-- spec/pundit_spec.rb | 16 ++++++++-------- spec/spec_helper.rb | 2 +- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/lib/pundit.rb b/lib/pundit.rb index 7d0b088..ba2c215 100644 --- a/lib/pundit.rb +++ b/lib/pundit.rb @@ -109,7 +109,7 @@ module Pundit # @return [Object, nil] instance of policy class with query methods def policy(user, record) policy = PolicyFinder.new(record).policy - policy.new(user, record) if policy + policy.new(user, pundit_model(record)) if policy rescue ArgumentError raise InvalidConstructorError, "Invalid #<#{policy}> constructor is called" end @@ -124,10 +124,16 @@ module Pundit # @return [Object] instance of policy class with query methods def policy!(user, record) policy = PolicyFinder.new(record).policy! - policy.new(user, record) + policy.new(user, pundit_model(record)) if policy rescue ArgumentError raise InvalidConstructorError, "Invalid #<#{policy}> constructor is called" end + + private + + def pundit_model(record) + record.is_a?(Array) ? record.last : record + end end # @api private diff --git a/spec/pundit_spec.rb b/spec/pundit_spec.rb index 5ed3787..7c6d0e2 100644 --- a/spec/pundit_spec.rb +++ b/spec/pundit_spec.rb @@ -144,35 +144,35 @@ describe Pundit do policy = Pundit.policy(user, %i[project criteria]) expect(policy.class).to eq Project::CriteriaPolicy expect(policy.user).to eq user - expect(policy.criteria).to eq %i[project criteria] + expect(policy.criteria).to eq :criteria end it "returns an instantiated policy given an array of a symbol and plain model instance" do policy = Pundit.policy(user, [:project, post]) expect(policy.class).to eq Project::PostPolicy expect(policy.user).to eq user - expect(policy.post).to eq [:project, post] + expect(policy.post).to eq post end it "returns an instantiated policy given an array of a symbol and a model instance with policy_class override" do policy = Pundit.policy(user, [:project, customer_post]) expect(policy.class).to eq Project::PostPolicy expect(policy.user).to eq user - expect(policy.post).to eq [:project, customer_post] + expect(policy.post).to eq customer_post end it "returns an instantiated policy given an array of a symbol and an active model instance" do policy = Pundit.policy(user, [:project, comment]) expect(policy.class).to eq Project::CommentPolicy expect(policy.user).to eq user - expect(policy.post).to eq [:project, comment] + expect(policy.comment).to eq comment end it "returns an instantiated policy given an array of a symbol and a plain model class" do policy = Pundit.policy(user, [:project, Post]) expect(policy.class).to eq Project::PostPolicy expect(policy.user).to eq user - expect(policy.post).to eq [:project, Post] + expect(policy.post).to eq Post end it "raises an error with a invalid policy constructor" do @@ -185,14 +185,14 @@ describe Pundit do policy = Pundit.policy(user, [:project, Comment]) expect(policy.class).to eq Project::CommentPolicy expect(policy.user).to eq user - expect(policy.post).to eq [:project, Comment] + expect(policy.comment).to eq Comment end it "returns an instantiated policy given an array of a symbol and a class with policy_class override" do policy = Pundit.policy(user, [:project, Customer::Post]) expect(policy.class).to eq Project::PostPolicy expect(policy.user).to eq user - expect(policy.post).to eq [:project, Customer::Post] + expect(policy.post).to eq Customer::Post end it "returns correct policy class for an array of a multi-word symbols" do @@ -312,7 +312,7 @@ describe Pundit do policy = Pundit.policy!(user, %i[project criteria]) expect(policy.class).to eq Project::CriteriaPolicy expect(policy.user).to eq user - expect(policy.criteria).to eq %i[project criteria] + expect(policy.criteria).to eq :criteria end it "throws an exception if the given policy can't be found" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 3ed641b..9e790be 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -155,7 +155,7 @@ end class CriteriaPolicy < Struct.new(:user, :criteria); end module Project - class CommentPolicy < Struct.new(:user, :post); end + class CommentPolicy < Struct.new(:user, :comment); end class CriteriaPolicy < Struct.new(:user, :criteria); end class PostPolicy < Struct.new(:user, :post); end end From ac372bcf0dcaf6b6d31f130036990e90264caf83 Mon Sep 17 00:00:00 2001 From: Linus Marton Date: Sun, 17 Jun 2018 09:26:10 +0200 Subject: [PATCH 2/5] Only pass last element of array to policy scope --- lib/pundit.rb | 6 +++--- spec/pundit_spec.rb | 16 ++++++++++++++++ spec/spec_helper.rb | 22 ++++++++++++++++++++-- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/lib/pundit.rb b/lib/pundit.rb index ba2c215..546e306 100644 --- a/lib/pundit.rb +++ b/lib/pundit.rb @@ -80,7 +80,7 @@ module Pundit # @return [Scope{#resolve}, nil] instance of scope class which can resolve to a scope def policy_scope(user, scope) policy_scope = PolicyFinder.new(scope).scope - policy_scope.new(user, scope).resolve if policy_scope + policy_scope.new(user, pundit_model(scope)).resolve if policy_scope rescue ArgumentError raise InvalidConstructorError, "Invalid #<#{policy_scope}> constructor is called" end @@ -95,7 +95,7 @@ module Pundit # @return [Scope{#resolve}] instance of scope class which can resolve to a scope def policy_scope!(user, scope) policy_scope = PolicyFinder.new(scope).scope! - policy_scope.new(user, scope).resolve + policy_scope.new(user, pundit_model(scope)).resolve rescue ArgumentError raise InvalidConstructorError, "Invalid #<#{policy_scope}> constructor is called" end @@ -124,7 +124,7 @@ module Pundit # @return [Object] instance of policy class with query methods def policy!(user, record) policy = PolicyFinder.new(record).policy! - policy.new(user, pundit_model(record)) if policy + policy.new(user, pundit_model(record)) rescue ArgumentError raise InvalidConstructorError, "Invalid #<#{policy}> constructor is called" end diff --git a/spec/pundit_spec.rb b/spec/pundit_spec.rb index 7c6d0e2..380bb6b 100644 --- a/spec/pundit_spec.rb +++ b/spec/pundit_spec.rb @@ -63,6 +63,14 @@ describe Pundit do expect(Pundit.policy_scope(user, empty_comments_relation)).to eq CommentScope.new(empty_comments_relation) end + it "returns an instantiated policy scope given an array of a symbol and plain model class" do + expect(Pundit.policy_scope(user, [:project, Post])).to eq :read + end + + it "returns an instantiated policy scope given an array of a symbol and active model class" do + expect(Pundit.policy_scope(user, [:project, Comment])).to eq Comment + end + it "returns nil if the given policy scope can't be found" do expect(Pundit.policy_scope(user, Article)).to be_nil end @@ -101,6 +109,14 @@ describe Pundit do end.to raise_error(Pundit::NotDefinedError, "Cannot scope NilClass") end + it "returns an instantiated policy scope given an array of a symbol and plain model class" do + expect(Pundit.policy_scope!(user, [:project, Post])).to eq :read + end + + it "returns an instantiated policy scope given an array of a symbol and active model class" do + expect(Pundit.policy_scope!(user, [:project, Comment])).to eq Comment + end + it "raises an error with a invalid policy scope constructor" do expect do Pundit.policy_scope(user, Wiki) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9e790be..72cc2e0 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -62,6 +62,10 @@ class Post < Struct.new(:user) :published end + def self.read + :read + end + def to_s "Post" end @@ -155,9 +159,23 @@ end class CriteriaPolicy < Struct.new(:user, :criteria); end module Project - class CommentPolicy < Struct.new(:user, :comment); end + class CommentPolicy < Struct.new(:user, :comment) + class Scope < Struct.new(:user, :scope) + def resolve + scope + end + end + end + class CriteriaPolicy < Struct.new(:user, :criteria); end - class PostPolicy < Struct.new(:user, :post); end + + class PostPolicy < Struct.new(:user, :post) + class Scope < Struct.new(:user, :scope) + def resolve + scope.read + end + end + end end class DenierPolicy < Struct.new(:user, :record) From 7b21ca027842782c126a226c15922d49febc3fec Mon Sep 17 00:00:00 2001 From: Linus Marton Date: Sun, 17 Jun 2018 09:32:53 +0200 Subject: [PATCH 3/5] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dc898e..fd46942 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Only pass last element of "namespace array" to policy and scope. (#529) - Allow specification of a `NilClassPolicy`. (#525) - Make sure `policy_class` override is called when passed an array. (#475) - Raise `InvalidConstructorError` if a policy or policy scope with an invalid constructor is called. (#462) From 55b53c5f7899f50394a4108f5ddeb712106252b9 Mon Sep 17 00:00:00 2001 From: Linus Marton Date: Sun, 17 Jun 2018 09:45:02 +0200 Subject: [PATCH 4/5] Fix `param_key` when passed an array for namespacing --- lib/pundit/policy_finder.rb | 12 +++++++----- spec/policy_finder_spec.rb | 12 ++++++++++++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/lib/pundit/policy_finder.rb b/lib/pundit/policy_finder.rb index 0a5a69a..413c4bd 100644 --- a/lib/pundit/policy_finder.rb +++ b/lib/pundit/policy_finder.rb @@ -55,12 +55,14 @@ module Pundit # @return [String] the name of the key this object would have in a params hash # def param_key - if object.respond_to?(:model_name) - object.model_name.param_key.to_s - elsif object.is_a?(Class) - object.to_s.demodulize.underscore + model = object.is_a?(Array) ? object.last : object + + if model.respond_to?(:model_name) + model.model_name.param_key.to_s + elsif model.is_a?(Class) + model.to_s.demodulize.underscore else - object.class.to_s.demodulize.underscore + model.class.to_s.demodulize.underscore end end diff --git a/spec/policy_finder_spec.rb b/spec/policy_finder_spec.rb index bd8a988..36386ca 100644 --- a/spec/policy_finder_spec.rb +++ b/spec/policy_finder_spec.rb @@ -106,5 +106,17 @@ describe Pundit::PolicyFinder do expect(subject.param_key).to eq "article" end end + + context "object is an array" do + subject { described_class.new([:project, article]) } + + it "returns the param_key for the last element of the array" do + expect(subject.object).not_to respond_to(:model_name) + expect(subject.object).not_to be_a Class + expect(subject.object).to be_an_instance_of Array + + expect(subject.param_key).to eq "article" + end + end end end From d23ba077910e1cab1c65a710e4b3d3ae40cc31d0 Mon Sep 17 00:00:00 2001 From: Linus Marton Date: Sun, 17 Jun 2018 09:53:26 +0200 Subject: [PATCH 5/5] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd46942..d56c606 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Fix `param_key` issue when passed an array. (#529) - Only pass last element of "namespace array" to policy and scope. (#529) - Allow specification of a `NilClassPolicy`. (#525) - Make sure `policy_class` override is called when passed an array. (#475)