From 012abc3f3ec156ced78d1829a6b9deba6481a123 Mon Sep 17 00:00:00 2001 From: thedevjoao Date: Tue, 23 May 2023 17:40:44 -0300 Subject: [PATCH 1/4] add tests to cover overriding policies - tests when called from a controller - tests when a record is called from another namespace --- spec/authorization_spec.rb | 73 ++++++++++++++++++++++++++++++++++++++ spec/spec_helper.rb | 10 ++++++ 2 files changed, 83 insertions(+) diff --git a/spec/authorization_spec.rb b/spec/authorization_spec.rb index 2995bfdb..adfb65db 100644 --- a/spec/authorization_spec.rb +++ b/spec/authorization_spec.rb @@ -8,6 +8,7 @@ let(:post) { Post.new(user) } let(:customer_post) { Customer::Post.new(user) } let(:comment) { Comment.new } + let(:admin_post) { Project::Admin::Post.new(user) } let(:article) { Article.new } let(:article_tag) { ArticleTag.new } let(:wiki) { Wiki.new } @@ -116,6 +117,39 @@ it "raises an error with a invalid policy constructor" do expect { controller.authorize(wiki, :destroy?) }.to raise_error(Pundit::InvalidConstructorError) end + + context "when passed a record with the admin namespace" do + it "returns the record when the permission check passes" do + allow(controller).to receive(:authorize).with(admin_post, :update?).and_return(admin_post) + expect(controller.authorize(admin_post, :update?)).to eq(admin_post) + end + + it "raises an exception when the permission check fails" do + allow(controller).to receive(:authorize).with(admin_post, :update?).and_raise(Pundit::NotAuthorizedError) + expect { controller.authorize(admin_post, :update?) }.to raise_error(Pundit::NotAuthorizedError) + end + end + + context "when called in a controller" do + let(:controller) { Controller.new(user, "update", {}) } + + before do + allow(controller).to receive(:kind_of?).with(Controller).and_return(true) + allow(controller).to receive(:current_user).and_return(user) + end + + it "uses the Admin namespace for policy calls" do + expect(controller).to receive(:authorize).with(admin_post, :update?).and_return(admin_post) + expect(controller.authorize(admin_post, :update?)).to eq(admin_post) + end + + it "allows overriding the policy class" do + allow(controller).to receive(:policy_class).and_return(Project::Admin::CommentPolicy) + expect(controller).to receive(:authorize).with(admin_post, :update?, + policy_class: Project::Admin::Post).and_return(admin_post) + expect(controller.authorize(admin_post, :update?, policy_class: Project::Admin::Post)).to eq(admin_post) + end + end end describe "#skip_authorization" do @@ -159,6 +193,45 @@ expect(controller.policy(post)).to eq new_policy end + + context "when the policy is overriden" do + before do + allow(controller).to receive(:policy) do |record| + case record + when Post + PostPolicy.new(user, record) + when Blog + BlogPolicy.new(user, record) + else + raise Pundit::NotDefinedError, "Policy not defined for record: #{record.class}" + end + end + end + + it "returns the correct overriden policy for a Post record" do + post = Post.new(user) + + policy = controller.policy(post) + expect(policy).to be_instance_of(PostPolicy) + expect(policy.user).to eq(user) + expect(policy.post).to eq(post) + end + + it "returns the correct overriden policy for a Blog record" do + blog = Blog.new + + policy = controller.policy(blog) + expect(policy).to be_instance_of(BlogPolicy) + expect(policy.user).to eq(user) + expect(policy.blog).to eq(blog) + end + + it "raises an error if the given policy can't be found" do + record = Article.new + + expect { controller.policy(record) }.to raise_error(Pundit::NotDefinedError) + end + end end describe "#policy_scope" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 192146f2..621ea43c 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -191,6 +191,16 @@ def destroy? false end end + + class Post < Post + def model_name + OpenStruct.new(param_key: "admin_post") + end + + def self.policy_class + PostPolicy + end + end end end From 13eee12e7f29d33e87af0656728c77b9bc4ed0c6 Mon Sep 17 00:00:00 2001 From: thedevjoao Date: Tue, 23 May 2023 17:45:56 -0300 Subject: [PATCH 2/4] feat: adds relevant lines to the changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f789c84b..df37522e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Pundit +### Added + +- Adds tests to cover policy overrides and when authorize is called in other namespaces (#772) + ## Unreleased ### Fixed From 733e7b6193062ccf6d72911a94cf454110a2f56e Mon Sep 17 00:00:00 2001 From: thedevjoao Date: Fri, 26 May 2023 17:32:04 -0300 Subject: [PATCH 3/4] refactor: tests now properly call authorize - refactors the authorization_spec and spec_helper --- spec/authorization_spec.rb | 54 ++++++++------------------------------ spec/spec_helper.rb | 4 +++ 2 files changed, 15 insertions(+), 43 deletions(-) diff --git a/spec/authorization_spec.rb b/spec/authorization_spec.rb index adfb65db..3457aae2 100644 --- a/spec/authorization_spec.rb +++ b/spec/authorization_spec.rb @@ -8,7 +8,7 @@ let(:post) { Post.new(user) } let(:customer_post) { Customer::Post.new(user) } let(:comment) { Comment.new } - let(:admin_post) { Project::Admin::Post.new(user) } + let(:admin_comment) { Project::Admin::CommentPolicy.new(user, comment) } let(:article) { Article.new } let(:article_tag) { ArticleTag.new } let(:wiki) { Wiki.new } @@ -118,37 +118,12 @@ expect { controller.authorize(wiki, :destroy?) }.to raise_error(Pundit::InvalidConstructorError) end - context "when passed a record with the admin namespace" do - it "returns the record when the permission check passes" do - allow(controller).to receive(:authorize).with(admin_post, :update?).and_return(admin_post) - expect(controller.authorize(admin_post, :update?)).to eq(admin_post) - end - - it "raises an exception when the permission check fails" do - allow(controller).to receive(:authorize).with(admin_post, :update?).and_raise(Pundit::NotAuthorizedError) - expect { controller.authorize(admin_post, :update?) }.to raise_error(Pundit::NotAuthorizedError) - end - end - context "when called in a controller" do - let(:controller) { Controller.new(user, "update", {}) } - - before do - allow(controller).to receive(:kind_of?).with(Controller).and_return(true) - allow(controller).to receive(:current_user).and_return(user) - end - it "uses the Admin namespace for policy calls" do - expect(controller).to receive(:authorize).with(admin_post, :update?).and_return(admin_post) - expect(controller.authorize(admin_post, :update?)).to eq(admin_post) - end - - it "allows overriding the policy class" do - allow(controller).to receive(:policy_class).and_return(Project::Admin::CommentPolicy) - expect(controller).to receive(:authorize).with(admin_post, :update?, - policy_class: Project::Admin::Post).and_return(admin_post) - expect(controller.authorize(admin_post, :update?, policy_class: Project::Admin::Post)).to eq(admin_post) - end + policy = Project::Admin::CommentPolicy.new(user, comment) + + expect(controller.authorize(admin_comment, :update?)).to eq(policy) + end end end @@ -194,20 +169,13 @@ expect(controller.policy(post)).to eq new_policy end - context "when the policy is overriden" do - before do - allow(controller).to receive(:policy) do |record| - case record - when Post - PostPolicy.new(user, record) - when Blog - BlogPolicy.new(user, record) - else - raise Pundit::NotDefinedError, "Policy not defined for record: #{record.class}" - end - end - end + it "uses the Admin namespace for policy calls" do + policy = Project::Admin::CommentPolicy.new(user, comment) + + expect(policy.update?).to eq(true) + end + context "when the policy is overriden" do it "returns the correct overriden policy for a Post record" do post = Post.new(user) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 621ea43c..55a9234f 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -183,6 +183,10 @@ def resolve module Admin class CommentPolicy < Struct.new(:user, :comment) + def self.policy_class + CommentPolicy + end + def update? true end From 545a2e64c32d3cb361cdafe829167b6b311d7655 Mon Sep 17 00:00:00 2001 From: thedevjoao Date: Fri, 26 May 2023 17:46:47 -0300 Subject: [PATCH 4/4] fix: removes trailing whitespace with rubocop --- spec/authorization_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/authorization_spec.rb b/spec/authorization_spec.rb index 3457aae2..df32f4cb 100644 --- a/spec/authorization_spec.rb +++ b/spec/authorization_spec.rb @@ -121,9 +121,9 @@ context "when called in a controller" do it "uses the Admin namespace for policy calls" do policy = Project::Admin::CommentPolicy.new(user, comment) - + expect(controller.authorize(admin_comment, :update?)).to eq(policy) - end + end end end @@ -171,7 +171,7 @@ it "uses the Admin namespace for policy calls" do policy = Project::Admin::CommentPolicy.new(user, comment) - + expect(policy.update?).to eq(true) end