From 944c1f09af03c3b60c2a08952047cee1e9200155 Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Fri, 13 Sep 2024 17:00:34 +0300 Subject: [PATCH 01/10] add hook for clear pundit context --- lib/pundit/authorization.rb | 10 ++++++++++ spec/authorization_spec.rb | 30 ++++++++++++++++++++++++++++++ spec/support/lib/controller.rb | 3 ++- 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/lib/pundit/authorization.rb b/lib/pundit/authorization.rb index c0a77834..c01f0002 100644 --- a/lib/pundit/authorization.rb +++ b/lib/pundit/authorization.rb @@ -217,5 +217,15 @@ def pundit_params_for(record) end # @!endgroup + + # Hook method which allows to clear the Pundit context. + # + # @see https://github.com/varvet/pundit#additional-context + # @return [void] + def clear_pundit_context! + @pundit = nil + @_pundit_policies = nil + @_pundit_policy_scopes = nil + end end end diff --git a/spec/authorization_spec.rb b/spec/authorization_spec.rb index 8bfa3fcb..b5f45e95 100644 --- a/spec/authorization_spec.rb +++ b/spec/authorization_spec.rb @@ -271,4 +271,34 @@ def to_params(*args, **kwargs, &block) expect(Controller.new(user, action, params).permitted_attributes(post, :revise).to_h).to eq("body" => "blah") end end + + describe "#clear_pundit_context!" do + let(:new_user) { double } + + it "clears the current user" do + expect(controller.pundit.user).to eq controller.current_user + + controller.current_user = new_user + expect(controller.pundit.user).not_to eq controller.current_user + + controller.clear_pundit_context! + expect(controller.pundit.user).to eq controller.current_user + end + + it "clears the policy cache" do + controller.policy(post) + expect(controller.policies).not_to be_empty + + controller.clear_pundit_context! + expect(controller.policies).to be_empty + end + + it "clears the policy scope cache" do + controller.policy_scope(Post) + expect(controller.policy_scopes).not_to be_empty + + controller.clear_pundit_context! + expect(controller.policy_scopes).to be_empty + end + end end diff --git a/spec/support/lib/controller.rb b/spec/support/lib/controller.rb index 4077de25..8715ecf1 100644 --- a/spec/support/lib/controller.rb +++ b/spec/support/lib/controller.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true class Controller - attr_reader :current_user, :action_name, :params + attr_accessor :current_user + attr_reader :action_name, :params class View def initialize(controller) From 55fac53d01eeb981dcb48627857969fcb6ff12ce Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Fri, 13 Sep 2024 17:15:39 +0300 Subject: [PATCH 02/10] Add instructions to Readme --- README.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/README.md b/README.md index a1b59868..0a2f7b34 100644 --- a/README.md +++ b/README.md @@ -660,6 +660,24 @@ class ApplicationController end ``` +If you need to change the context for any reason, you will need to clear the caches stored in the context. You can use the hook below to do this. + +```ruby +class ApplicationController + include Pundit::Authorization + before_action :switch_account, if: :should_switch_account? + + def switch_account + set_current_account(Account.find(params[:account_id])) + clear_pundit_context! + end + + def pundit_user + UserContext.new(current_user, current_account) + end +end +``` + ## Strong parameters In Rails, From e8767a888c9a7f2a40a918b4d8cdbf9715a37052 Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Fri, 13 Sep 2024 17:15:55 +0300 Subject: [PATCH 03/10] Add Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b16cddbc..98f6778b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +## Added + +- Add `Pundit::Authorization#clear_pundit_context!` hook to clear the context (#830) + ## 2.4.0 (2024-08-26) ## Changed From e73340a79323bdb2066e523b3524673c80a47fe2 Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Tue, 8 Oct 2024 13:15:36 +0300 Subject: [PATCH 04/10] chore: rename method name to pundit_reset! --- CHANGELOG.md | 2 +- README.md | 2 +- lib/pundit/authorization.rb | 2 +- spec/authorization_spec.rb | 8 ++++---- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98f6778b..95362c86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ## Added -- Add `Pundit::Authorization#clear_pundit_context!` hook to clear the context (#830) +- Add `Pundit::Authorization#pundit_reset!` hook to reset the policy and policy scope cache. (#830) ## 2.4.0 (2024-08-26) diff --git a/README.md b/README.md index 0a2f7b34..625a4991 100644 --- a/README.md +++ b/README.md @@ -669,7 +669,7 @@ class ApplicationController def switch_account set_current_account(Account.find(params[:account_id])) - clear_pundit_context! + pundit_reset! end def pundit_user diff --git a/lib/pundit/authorization.rb b/lib/pundit/authorization.rb index c01f0002..bfa44d25 100644 --- a/lib/pundit/authorization.rb +++ b/lib/pundit/authorization.rb @@ -222,7 +222,7 @@ def pundit_params_for(record) # # @see https://github.com/varvet/pundit#additional-context # @return [void] - def clear_pundit_context! + def pundit_reset! @pundit = nil @_pundit_policies = nil @_pundit_policy_scopes = nil diff --git a/spec/authorization_spec.rb b/spec/authorization_spec.rb index b5f45e95..45d40ac5 100644 --- a/spec/authorization_spec.rb +++ b/spec/authorization_spec.rb @@ -272,7 +272,7 @@ def to_params(*args, **kwargs, &block) end end - describe "#clear_pundit_context!" do + describe "#pundit_reset!" do let(:new_user) { double } it "clears the current user" do @@ -281,7 +281,7 @@ def to_params(*args, **kwargs, &block) controller.current_user = new_user expect(controller.pundit.user).not_to eq controller.current_user - controller.clear_pundit_context! + controller.pundit_reset! expect(controller.pundit.user).to eq controller.current_user end @@ -289,7 +289,7 @@ def to_params(*args, **kwargs, &block) controller.policy(post) expect(controller.policies).not_to be_empty - controller.clear_pundit_context! + controller.pundit_reset! expect(controller.policies).to be_empty end @@ -297,7 +297,7 @@ def to_params(*args, **kwargs, &block) controller.policy_scope(Post) expect(controller.policy_scopes).not_to be_empty - controller.clear_pundit_context! + controller.pundit_reset! expect(controller.policy_scopes).to be_empty end end From dc2a1e476f71650f9ca593857699e684628036b9 Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Tue, 8 Oct 2024 13:16:24 +0300 Subject: [PATCH 05/10] chore: reset authorized? and scoped? caches --- lib/pundit/authorization.rb | 2 ++ spec/authorization_spec.rb | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/pundit/authorization.rb b/lib/pundit/authorization.rb index bfa44d25..44d1cd70 100644 --- a/lib/pundit/authorization.rb +++ b/lib/pundit/authorization.rb @@ -226,6 +226,8 @@ def pundit_reset! @pundit = nil @_pundit_policies = nil @_pundit_policy_scopes = nil + @_pundit_policy_authorized = nil + @_pundit_policy_scoped = nil end end end diff --git a/spec/authorization_spec.rb b/spec/authorization_spec.rb index 45d40ac5..5c2b81fb 100644 --- a/spec/authorization_spec.rb +++ b/spec/authorization_spec.rb @@ -286,19 +286,23 @@ def to_params(*args, **kwargs, &block) end it "clears the policy cache" do - controller.policy(post) + controller.authorize(post) expect(controller.policies).not_to be_empty + expect(controller.pundit_policy_authorized?).to be true controller.pundit_reset! expect(controller.policies).to be_empty + expect(controller.pundit_policy_authorized?).to be false end it "clears the policy scope cache" do controller.policy_scope(Post) expect(controller.policy_scopes).not_to be_empty + expect(controller.pundit_policy_scoped?).to be true controller.pundit_reset! expect(controller.policy_scopes).to be_empty + expect(controller.pundit_policy_scoped?).to be false end end end From 2d7d4e2d0b814fa0d94697c60b8eb3e52ab8d80c Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Tue, 8 Oct 2024 13:19:46 +0300 Subject: [PATCH 06/10] doc: use switch_user instead of switch_account --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 625a4991..2eec9c76 100644 --- a/README.md +++ b/README.md @@ -660,20 +660,20 @@ class ApplicationController end ``` -If you need to change the context for any reason, you will need to clear the caches stored in the context. You can use the hook below to do this. +If you need to change the context for any reason, you will need to clear the caches stored in the Pundit. You can use the hook below to do this. ```ruby class ApplicationController include Pundit::Authorization - before_action :switch_account, if: :should_switch_account? + before_action :switch_user, if: :should_switch_user? def switch_account - set_current_account(Account.find(params[:account_id])) + set_current_user(User.find(params[:user_id])) pundit_reset! end def pundit_user - UserContext.new(current_user, current_account) + UserContext.new(current_user, request.ip) end end ``` From cfded458e6937101011ae122cb616a1a3107618f Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Tue, 22 Oct 2024 16:41:45 +0300 Subject: [PATCH 07/10] chore: update readme section to describe method --- README.md | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 2eec9c76..9bbe9816 100644 --- a/README.md +++ b/README.md @@ -582,6 +582,25 @@ def pundit_user User.find_by_other_means end ``` +### Handling User Switching in Pundit + +When switching users in your application, it's important to reset the Pundit user context to ensure that authorization policies are applied correctly for the new user. Pundit caches the user context, so failing to reset it could result in incorrect permissions being applied. + +To handle user switching, you can use the following pattern in your controller: + +```ruby +class ApplicationController + include Pundit::Authorization + before_action :switch_user, if: :should_switch_user? + + def switch_user + current_user = User.find(params[:user_id]) + pundit_reset! # Ensure that the Pundit context is reset for the new user + end +end +``` + +Make sure to invoke `pundit_reset!` whenever changing the user. This ensures the cached authorization context is reset, preventing any incorrect permissions from being applied. ## Policy Namespacing In some cases it might be helpful to have multiple policies that serve different contexts for a @@ -660,24 +679,6 @@ class ApplicationController end ``` -If you need to change the context for any reason, you will need to clear the caches stored in the Pundit. You can use the hook below to do this. - -```ruby -class ApplicationController - include Pundit::Authorization - before_action :switch_user, if: :should_switch_user? - - def switch_account - set_current_user(User.find(params[:user_id])) - pundit_reset! - end - - def pundit_user - UserContext.new(current_user, request.ip) - end -end -``` - ## Strong parameters In Rails, From cd0ad40ae17cff628155e0fbbcc5a24cf0d53923 Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Tue, 22 Oct 2024 16:42:01 +0300 Subject: [PATCH 08/10] chore: explain more details on method --- lib/pundit/authorization.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/pundit/authorization.rb b/lib/pundit/authorization.rb index 44d1cd70..cf2a2ae3 100644 --- a/lib/pundit/authorization.rb +++ b/lib/pundit/authorization.rb @@ -218,9 +218,15 @@ def pundit_params_for(record) # @!endgroup - # Hook method which allows to clear the Pundit context. + # Clears the cached Pundit authorization data. + # + # This method should be called when the pundit_user is changed, + # such as during user switching, to ensure that stale authorization + # data is not used. Pundit caches authorization policies and scopes + # for the pundit_user, so calling this method will reset those + # caches and ensure that the next authorization checks are performed + # with the correct context for the new pundit_user. # - # @see https://github.com/varvet/pundit#additional-context # @return [void] def pundit_reset! @pundit = nil From 9515c99dc688f9b5ced21feaf4584ec1cd9f7676 Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Tue, 22 Oct 2024 16:42:19 +0300 Subject: [PATCH 09/10] test: add more clear specs for clear context --- spec/authorization_spec.rb | 41 ++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/spec/authorization_spec.rb b/spec/authorization_spec.rb index 5c2b81fb..24ec2310 100644 --- a/spec/authorization_spec.rb +++ b/spec/authorization_spec.rb @@ -273,35 +273,46 @@ def to_params(*args, **kwargs, &block) end describe "#pundit_reset!" do - let(:new_user) { double } + it "allows authorize to react to a user change" do + expect(controller.authorize(post)).to be_truthy + controller.current_user = double + controller.pundit_reset! + expect { controller.authorize(post) }.to raise_error(Pundit::NotAuthorizedError) + end - it "clears the current user" do - expect(controller.pundit.user).to eq controller.current_user + it "allows policy scope to react to a user change" do + expect(controller.policy_scope(Post)).to eq :published + expect { controller.verify_policy_scoped }.not_to raise_error + controller.current_user = double + controller.pundit_reset! + expect { controller.verify_policy_scoped }.to raise_error(Pundit::PolicyScopingNotPerformedError) + end - controller.current_user = new_user - expect(controller.pundit.user).not_to eq controller.current_user + it "clears the pundit context user" do + expect(controller.pundit.user).to be(user) - controller.pundit_reset! - expect(controller.pundit.user).to eq controller.current_user + new_user = double + controller.current_user = new_user + expect { controller.pundit_reset! }.to change { controller.pundit.user }.from(user).to(new_user) end - it "clears the policy cache" do - controller.authorize(post) - expect(controller.policies).not_to be_empty + it "clears pundit_policy_authorized? flag" do + expect(controller.pundit_policy_authorized?).to be false + + controller.skip_authorization expect(controller.pundit_policy_authorized?).to be true controller.pundit_reset! - expect(controller.policies).to be_empty expect(controller.pundit_policy_authorized?).to be false end - it "clears the policy scope cache" do - controller.policy_scope(Post) - expect(controller.policy_scopes).not_to be_empty + it "clears pundit_policy_scoped? flag" do + expect(controller.pundit_policy_scoped?).to be false + + controller.skip_policy_scope expect(controller.pundit_policy_scoped?).to be true controller.pundit_reset! - expect(controller.policy_scopes).to be_empty expect(controller.pundit_policy_scoped?).to be false end end From a80c98b170fd07decdc637aec5fb6b68ccc9cc49 Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Tue, 22 Oct 2024 16:53:14 +0300 Subject: [PATCH 10/10] chore: add endgroup --- lib/pundit/authorization.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/pundit/authorization.rb b/lib/pundit/authorization.rb index cf2a2ae3..93a62405 100644 --- a/lib/pundit/authorization.rb +++ b/lib/pundit/authorization.rb @@ -218,6 +218,8 @@ def pundit_params_for(record) # @!endgroup + # @!group Customize Pundit user + # Clears the cached Pundit authorization data. # # This method should be called when the pundit_user is changed, @@ -235,5 +237,7 @@ def pundit_reset! @_pundit_policy_authorized = nil @_pundit_policy_scoped = nil end + + # @!endgroup end end