Pundit: The creation of a policy finder resolver

[aaronbhansen]

Before I created a pull request, I wanted to see if this is something you would be open to merging in.

I created a quick monkey patch version in my app you can see below.

The general idea we would like to accomplish is to have policies resolved based on

1. The context (The context user and their role)
2. Allow falling back to a default pundit if the specific model pundit isn't found.

This helps separate authorization based on specific user roles (or anonymous if the user isn't logged in). And it allows use to fallback to a generic authorization if the specific model can't be found. For our api most of the authorization is the exact same except in a few very specific cases.

The pull request wouldn't be structured as it is below, instead it would allow a custom policy finder resolver to be created and called that would resolve the policy.

The changes would include

1. Always passing in the user context into the policy finder
2. Allowing a custom resolver to be created and configured on the app to find the correct policy
3. Possibly not caching previously found policies (like is done today), since they would be context specific.

Anyway, if this is something you would be interested in allowing, I'd be happy to put together a more formal pull request with better separation to allow the creation of a custom policy finder resolver. But the extra flexibility of having a custom resolver would allow for something like:

policies\
  {user_role}\ 
    base_policy.rb
    {model}_policy.rb

Monkey Patch example

module Pundit

  class << self
   # patched these just so the policy finder had access to the context object
    def policy_scope(user, scope)
      policy_scope = PolicyFinder.new(scope, user).scope
      policy_scope.new(user, scope).resolve if policy_scope
    end

    def policy_scope!(user, scope)
      PolicyFinder.new(scope, user).scope!.new(user, scope).resolve
    end

    def policy(user, record)
      policy = PolicyFinder.new(record, user).policy
      policy.new(user, record) if policy
    end

    def policy!(user, record)
      PolicyFinder.new(record, user).policy!.new(user, record)
    end
  end

  class PolicyFinder
    attr_reader :context

    def initialize(object, context)
      @object = object
      @context = context
    end

    def policy
      klass = find
      if klass.is_a?(String)
        # if we don't have a context, just apply the default
        if context.blank?
          # try to find anonymous
          klass = "Anonymous::#{klass}".safe_constantize
          unless klass
            klass = "Anonymous::Application#{SUFFIX}".constantize
          end
        else
          # Otherwise try to find it based context role (Owner, etc)
          klass = "#{context.role.capitalize}::#{klass}".safe_constantize
          unless klass
            klass = "#{context.role.capitalize}::Application#{SUFFIX}".constantize
          end
        end
      end
      klass
    rescue NameError
      nil
    end

  end

end

[damien]

I'm not the maintainer, but this seems like an interesting idea. From a design standpoint it may even be generic enough to include in pundit rather than an extension.

@jnicklas Thoughts? I'm not a big contributor, but I keep an eye on the issues on this gem. Something like this seems to be a common pain point in the usage of Pundit in non-trivial use cases.

[aaronbhansen]

In the pull request I would create a generic pundit resolver with the current functionality as it exists today, but allow the configuration of a different resolver so you could replace the generic one and resolve any policy you would like. The only catch I mentioned above was the catching, but maybe that could be a generic cacher also, or a configuration to disable it.

[ricardotk002]

It's a good feature. Right now I'm dealing with separating authorization rules for different roles, this would make it easier.

[sepastian]

:+1: from me, too.

I am implementing different policies based on the plan the user is on. So, for user 1 with _PlanA_ I need to be able to select PlanAPolicy; for another user with _PlanB_, select PlanBPolicy.

I require a way of selecting policies based on an user#plan, which could be achieved with a custom PolicyFinder.

[jnicklas]

So I had a rather tough time figuring out what this actually does, which isn't a good thing. Pundit is supposed to be simple. But even after I figured it out, I'm not sure that this is a good idea.

First of all to address your use case, I see a few problems with it:

• It introduces a lot of implicit behaviour. If a policy class can't be found for some reason, the default rules apply. This is not good. Imagine someone has misspelled the name of their policy class, causing inference to fail so AdminUser falls back to the default policy. Suddenly everyone will be able to see or maybe even create admin users. Not good. Explicit is better. Having to create a AdminUserPolicy is a _good_ thing, imo. Locked down by default. White list over black list. Etc...
• If you need to define a class anyway, and you want to have a lot of default behaviour, use normal Ruby to abstract that. For example, you could create a module called DefaultPermission, with all of the most common permissions, then:

class PostPolicy
  include DefaultPermissions
end

class AdminUserPolicy
end

Nice and explicit, and much safer than an implicit fallback.

• Pundit _is_ its inference algorithm. It is what defines the library. When people see Pundit in a Rails project, they expect it to work a certain way. Making it too configurable is not a good idea imo. If you really need an inference algorithm which is very different from Pundit's then creating your own pundit style helpers is very easy, and I think that's a better solution if you have very specific needs.

[aaronbhansen]

My idea was more around having a methodology to create more custom solution if needed. I understand you want to keep it simple and in your default configurations you can, but if someone else needs more complex resolvers, having an option to replace the default would allow us to do that.

We've thought through your points and feel comfortable with our solution of defaulting and fall through policies, since an application policy has to exist per role still. So a user role would never have access to an admin roles base policy.

We also don't like having a lot of empty classes when it can be defined another way through the resolver. It's a matter of preference. Some like to have everything defined, we only like to have classes that change the base defined.

If you're comfortable with a pull request to allow a base configuration thats configurable I'd be happy to create one, if not I'm also fine branching and creating something different. Thats why I wanted to reach out in an issue before creating any pull requests. Let me know what you think

[scottjacobsen]

I think it wouldn't take much to make the PolicyFinder pluggable
with a little bit of dependency injection.

Essentially take hard coded instances of PolicyFinder and replace
them with a call to a policy_finder accessor that could be
overridden in whatever is mixing in Pundit.

Another option, that doesn't require any changes to Pundit, is to just
create a policy that sort of acts as a custom policy resolver.

You could use a strategy pattern for example:

class DynamicPolicy
  def initialize(user, record)
    @user = user
    @record = record
  end

  def create?
    strategy.create?
  end

  def destroy?
    strategy.destroy?
  end

  #.. etc

  def strategy
    if @user.plan_a?
      PlanAPolicy.new(@user, @record)
    elsif @user.plan_b?
      PlanBPolicy.new(@user, @record)
    else
      DefaultPolicy.new(@user, @record)
    end
  end
end

Then in your objects that need the special resolver:

class Post < ActiveRecord::Base
  def self.policy_class
    DynamicPolicy
  end
end

Or even monkey patch AR::Base

class ActiveRecord::Base
  def self.policy_class
    DynamicPolicy
  end
end

[jnicklas]

I am closing this issue. I feel very strongly that this adds undesired complexity to Pundit, and there are many more explicit, less confusing ways of adding behaviour like this, such as the one I proposed, or @scottjacobsen's suggestion above.

[tommyalvarez]

For anyone else arriving into this issue googling how to apply custom policies based on context or different controllers for same model, etc.... see: https://github.com/varvet/pundit/issues/241. The answer is namespaces!