8000 Support default Pundit policy class. by seban · Pull Request #3323 · activeadmin/activeadmin · GitHub
[go: up one dir, main page]
Skip to content

Support default Pundit policy class. #3323

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Support default Pundit policy class. #3323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/active_admin/application.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ def initialize
# The authorization adapter to use
inheritable_setting :authorization_adapter, ActiveAdmin::AuthorizationAdapter

inheritable_setting :pundit_default_policy, nil

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of being in application.rb, this should be at the top of pundit_adapter.rb similar to cancan_adapter.rb

# A proc to be used when a user is not authorized to view the current resource
inheritable_setting :on_unauthorized_access, :rescue_active_admin_access_denied

Expand Down
18 changes: 18 additions & 0 deletions lib/active_admin/pundit_adapter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ def scope_collection(collection, action = Auth::READ)
# scoping is appliable only to read/index action
# which means there is no way how to scope other actions
Pundit.policy_scope!(user, collection)
rescue Pundit::NotDefinedError
default_policy_class::Scope.new(user, collection).resolve
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be:

rescue Pundit::NotDefinedError => e
  if default_policy_class && default_policy_class.const_defined?(:Scope)
    default_policy_class::Scope.new(user, collection).resolve
  else
    raise e
  end
end

end

def retrieve_policy(subject)
Expand All @@ -25,6 +27,12 @@ def retrieve_policy(subject)
when Class then Pundit.policy!(user, subject.new)
else Pundit.policy!(user, subject)
end
rescue Pundit::NotDefinedError => e
if default_policy_class
default_policy(user, subject)
else
raise e
end
end

def format_action(action, subject)
Expand All @@ -38,6 +46,16 @@ def format_action(action, subject)
end
end

private

def default_policy_class
ActiveAdmin.application.pundit_default_policy
end

def default_policy(user, subject)
default_policy_class.new(user, subject)
end

end

end
5 changes: 5 additions & 0 deletions lib/generators/active_admin/install/templates/active_admin.rb.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ ActiveAdmin.setup do |config|
# CanCanAdapter or make your own. Please refer to documentation.
# config.authorization_adapter = ActiveAdmin::CanCanAdapter

# In case you prefer Pundit over other solutions you can here pass
# the name of default policy class. This policy will be used in every
# case when Pundit is unable to find suitable policy.
# config.pundit_default_policy = MyDefaultPunditPolicy

# You can customize your CanCan Ability class name here.
# config.cancan_ability_class = "Ability"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird that pundit_default_policy accepts a class, and cancan_ability_class accepts a string. I'm not sure which is better, but in general it'd be good to have a consistent DSL.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is even another reason for that, setting a class here results in a caching problem in dev env.
Make the setting saving a String and constatize him in default_policy_class


Expand Down
4 changes: 4 additions & 0 deletions spec/unit/application_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@
expect(application.allow_comments).to eq true
end

it "should set default Pundit policy class" do
application.default_pundit_policy = policy_klass
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't only check the setting, you should check that is set too.


describe "authentication settings" do

it "should have no default current_user_method" do
Expand Down
44 changes: 44 additions & 0 deletions spec/unit/pundit_adapter_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,25 @@
let(:namespace) { ActiveAdmin::Namespace.new(application, "Admin") }
let(:resource) { namespace.register(Post) }
let(:auth) { namespace.authorization_adapter.new(resource, double) }
let(:default_policy_klass) do
class DefaultPolicy < ApplicationPolicy
class Scope

attr_reader :user, :scope

def initialize(user, scope)
@user = user
@scope = scope
end

def resolve
scope
end
end
end

DefaultPolicy
end

before do
namespace.authorization_adapter = ActiveAdmin::PunditAdapter
Expand All @@ -31,6 +50,31 @@ def resolve
auth.scope_collection(collection, :read)
expect(collection).to eq collection
end

context 'when Pundit is unable to find policy scope' do
let(:collection) { double("collection", to_sym: :collection) }
subject(:scope) { auth.scope_collection(collection, :read) }

before do
allow(ActiveAdmin.application).to receive(:pundit_default_policy).and_return default_policy_klass
allow(Pundit).to receive(:policy_scope!) { raise Pundit::NotDefinedError.new }
end

it("should return default policy's scope if defined") { is_expected.to eq(collection) }
end

context "when Pundit is unable to find policy" do
let(:record) { double }

subject(:policy) { auth.retrieve_policy(record) }

before do
allow(ActiveAdmin.application).to receive(:pundit_default_policy).and_return default_policy_klass
allow(Pundit).to receive(:policy!) { raise Pundit::NotDefinedError.new }
end

it("should return default policy instance") { is_expected.to be_instance_of(default_policy_klass) }
end
end

end
0