From 688c6a85e2a3fe77255a3595490e6941439de45e Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Fri, 13 Sep 2024 17:00:34 +0300 Subject: [PATCH 01/13] 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 ca291f9a..32bb14c6 100644 --- a/spec/authorization_spec.rb +++ b/spec/authorization_spec.rb @@ -272,4 +272,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 e234c02ea929663f7356bf339052eb2ba202e9ce Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Fri, 13 Sep 2024 17:15:39 +0300 Subject: [PATCH 02/13] 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 b528a31a70e7106eae3e14afccff92e69a99ebdd Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Fri, 13 Sep 2024 17:15:55 +0300 Subject: [PATCH 03/13] Add Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 63415401..4aa1e2cc 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) + ## Changed - Deprecated `Pundit::SUFFIX`, moved it to `Pundit::PolicyFinder::SUFFIX` (#835) From cabf5b7223e9dd7b52d562d161132d254732ff07 Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Tue, 8 Oct 2024 13:15:36 +0300 Subject: [PATCH 04/13] 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 4aa1e2cc..d3102949 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) ## Changed 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 32bb14c6..8c487d28 100644 --- a/spec/authorization_spec.rb +++ b/spec/authorization_spec.rb @@ -273,7 +273,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 @@ -282,7 +282,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 @@ -290,7 +290,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 @@ -298,7 +298,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 03392c9dfb04452535db58cf78cfa53c8e898c75 Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Tue, 8 Oct 2024 13:16:24 +0300 Subject: [PATCH 05/13] 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 8c487d28..da7c792c 100644 --- a/spec/authorization_spec.rb +++ b/spec/authorization_spec.rb @@ -287,19 +287,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 cd6bff933bd5aa45be4aa262a810716802e547f7 Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Tue, 8 Oct 2024 13:19:46 +0300 Subject: [PATCH 06/13] 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 5180aaf298829df7a2c715b30ee41ddd79a7e57b Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Tue, 22 Oct 2024 16:41:45 +0300 Subject: [PATCH 07/13] 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 16899332dccb04a7e493448a68b14e7358e0da06 Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Tue, 22 Oct 2024 16:42:01 +0300 Subject: [PATCH 08/13] 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 4a77389f94ff9d3b931894024c3fd1d38bed078f Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Tue, 22 Oct 2024 16:42:19 +0300 Subject: [PATCH 09/13] 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 da7c792c..568afcb4 100644 --- a/spec/authorization_spec.rb +++ b/spec/authorization_spec.rb @@ -274,35 +274,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 0031a15623a20a7b06e959b9082f71547599ec63 Mon Sep 17 00:00:00 2001 From: Furkan Ural Date: Tue, 22 Oct 2024 16:53:14 +0300 Subject: [PATCH 10/13] 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 From c0e2d0e0bef0bbd4013347e3133d1f1249c28e6a Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Fri, 22 Nov 2024 10:00:08 +0100 Subject: [PATCH 11/13] Make sure we test clearing policy scope cache --- spec/authorization_spec.rb | 23 ++++++++++++++----- spec/support/models/dummy_current_user.rb | 7 ++++++ .../policies/dummy_current_user_policy.rb | 9 ++++++++ 3 files changed, 33 insertions(+), 6 deletions(-) create mode 100644 spec/support/models/dummy_current_user.rb create mode 100644 spec/support/policies/dummy_current_user_policy.rb diff --git a/spec/authorization_spec.rb b/spec/authorization_spec.rb index 568afcb4..5bc1d255 100644 --- a/spec/authorization_spec.rb +++ b/spec/authorization_spec.rb @@ -9,7 +9,7 @@ def to_params(*args, **kwargs, &block) end let(:controller) { Controller.new(user, "update", to_params({})) } - let(:user) { double } + let(:user) { double("user") } let(:post) { Post.new(user) } let(:comment) { Comment.new } let(:article) { Article.new } @@ -276,20 +276,31 @@ def to_params(*args, **kwargs, &block) describe "#pundit_reset!" do 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 "allows policy to react to a user change" do + expect(controller.policy(DummyCurrentUser).user).to be user + + new_user = double("new user") + controller.current_user = new_user + controller.pundit_reset! + expect(controller.policy(DummyCurrentUser).user).to be new_user + end + 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 + expect(controller.policy_scope(DummyCurrentUser)).to be user + + new_user = double("new user") + controller.current_user = new_user controller.pundit_reset! - expect { controller.verify_policy_scoped }.to raise_error(Pundit::PolicyScopingNotPerformedError) + expect(controller.policy_scope(DummyCurrentUser)).to be new_user end - it "clears the pundit context user" do + it "resets the pundit context" do expect(controller.pundit.user).to be(user) new_user = double diff --git a/spec/support/models/dummy_current_user.rb b/spec/support/models/dummy_current_user.rb new file mode 100644 index 00000000..9c9af62a --- /dev/null +++ b/spec/support/models/dummy_current_user.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class DummyCurrentUser + def update? + user + end +end diff --git a/spec/support/policies/dummy_current_user_policy.rb b/spec/support/policies/dummy_current_user_policy.rb new file mode 100644 index 00000000..043e779a --- /dev/null +++ b/spec/support/policies/dummy_current_user_policy.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class DummyCurrentUserPolicy < BasePolicy + class Scope < BasePolicy::BaseScope + def resolve + user + end + end +end From bfa0de5ea8856066779f9fe18f3b5629b1a59031 Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Fri, 22 Nov 2024 10:06:40 +0100 Subject: [PATCH 12/13] Adjust README for switching users to use Rails 8 I'm not sure about the `should_switch_user?` pattern we're implicitly suggesting here. I rewrote the code sample to use the Rails 8 generators. I think this is roughly what it should look like for real. --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 9bbe9816..8c013798 100644 --- a/README.md +++ b/README.md @@ -591,11 +591,11 @@ 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 + def switch_user_to(user) + terminate_session if authenticated? + start_new_session_for user + pundit_reset! end end ``` From ddaa1e6ee822e7249541e6de2ec79ad4b7f55570 Mon Sep 17 00:00:00 2001 From: Kim Burgestrand Date: Fri, 22 Nov 2024 10:17:57 +0100 Subject: [PATCH 13/13] Regroup documentation for pundit_reset! --- lib/pundit/authorization.rb | 43 ++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/lib/pundit/authorization.rb b/lib/pundit/authorization.rb index 93a62405..2e57cff8 100644 --- a/lib/pundit/authorization.rb +++ b/lib/pundit/authorization.rb @@ -40,12 +40,33 @@ def pundit # Hook method which allows customizing which user is passed to policies and # scopes initialized by {#authorize}, {#policy} and {#policy_scope}. # + # @note Make sure to call `pundit_reset!` if this changes during a request. # @see https://github.com/varvet/pundit#customize-pundit-user + # @see #pundit + # @see #pundit_reset! # @return [Object] the user object to be used with pundit def pundit_user current_user end + # 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. + # + # @return [void] + def pundit_reset! + @pundit = nil + @_pundit_policies = nil + @_pundit_policy_scopes = nil + @_pundit_policy_authorized = nil + @_pundit_policy_scoped = nil + end + # @!group Policies # Retrieves the policy for the given record, initializing it with the record @@ -217,27 +238,5 @@ def pundit_params_for(record) end # @!endgroup - - # @!group Customize Pundit user - - # 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. - # - # @return [void] - def pundit_reset! - @pundit = nil - @_pundit_policies = nil - @_pundit_policy_scopes = nil - @_pundit_policy_authorized = nil - @_pundit_policy_scoped = nil - end - - # @!endgroup end end