Policies custom parameters

[randoum]

This suggestion allows to add parameters to policies and scopes, and allow custom scopes.

I let you guys have a look before I write the documentation (if ever you consider it useful).
It is backward compatible, and wont break existing code.
Spec file included

Policy with parameters

Basically, you can create policy such as:

class UserPolicy
  def update?(new_attribute)
    # check on permission according to the new attribute
  end
end

And call authorize @user, :update?, the_new_param in your update action.

Custom scope

It is also possible to specify a custom scope:

class UserPolicy
  class Scope
    def resolve
      # Default Scope
    end

    def users_list_in_a_specific_case
      # Specific scope
    end
  end
end

Then you can call policy_scope(User, :users_list_in_a_specific_case)

Custom scope with parameters

A custom scope can accept parameters:

class UserPolicy
  class Scope
    def resolve
      # Default Scope
    end

    def users_list_in_a_specific_case(additional_data)
      # Specific scope
    end
  end
end

Then you can call policy_scope(User, :users_list_in_a_specific_case, the_important_data)

[thomasklemm]

Hi @randoum, thanks for your efforts and PR!

Permitting parameters with Pundit is a great idea. Pundit in fact already works nicely only permitting certain fields on models when you use strong_parameters, please see this Readme section and see if it fits your needs.

As a sidenote: Calling authorize @user, the_new_param would check for a the_new_param? action on your UserPolicy. We'd need to introduce a third argument that people might miss, so I'd love if you could see if the Readme section on integrating strong_parameters helps.

As for the custom scopes (+ arguments there), could you help us figuring out its potential value by quickly outlining a real life scenario where it can help developers? I've got the feeling that we can help you simplify it further down the road, so that it might even fit the existing Pundit behavior. Thanks in advance! :)

[randoum]

First, a note: I mistyped in my original message, yes a custom parameter come in third position: authorize @user, :update?, the_new_param. Edited my original message to fix that.

Yes, using strong_parameters with Pundit is working great but sometimes it is not enough. Let me give you some examples of why I came with this implementation of Pundit:

In my app there is 4 different user roles: freelance, employee, admin, and super_admin.
Employees and admins belong to a company. Freelances and super_admin do not belong to a company.
Companies are isolated and cannot share data with each other.
Admin can manage anything within his company
Super admin can manage anything in the system.

Policy with parameters

It is possible when editing a user record to modify the 'role' of this user, with the following restriction:

• a freelance or a super_admin cannot have his role changed
• a user cannot change its own role

Strong parameters cannot help me because by the time is it used to filter the parameters it does not know anything about the record being edited.
In Pundit after I defined update?(new_role) I was able in few line to control the whole behaviour. Without custom parameters, half my authorization code would sit in the controller or in the model and the rest in the policy class.

Custom scope

Each user own client_information records. It is possible to transfer the ownership of such a record to another user. While transferring we are given a list of users to transfer the client_information record to
I needed a custom scope to easily cover the following restriction:

• an employee or an admin can transfer to a user of their own company only
• a super admin can transfer a client_information record that belong to anyone in the system but can transfer to users of the same company only

My scope needed to know about the ownership of client_information record in order to provide the correct scoping of user records, without custom scope I could not have done it in Pundit.

[thomasklemm]

@randoum Let me clarify you intentions:

So you call authorize @target_user, :change_role?, target_role in your controller if the signed in user want to change the role of a target_user?

class UserPolicy < ApplicationPolicy
  alias_method :target_user, :record

  # `user` is the signed in user who wants to perform an action
  # `target_user` is the user the signed in user wants to perform an action on
  # `target_role` is the role the target user is intended to be assigned
  def change_role?(target_role)
    # The signed in super_admin can change the roles on any target_user
    user.super_admin? and return true

    # The signed in admin can change roles only if the target_user is a member of his company
    user.admin? and return target_user.member_of?(user.company)
  end
end

[thomasklemm]

@randoum Could you give some insights into how you structured your custom scopes in your app (either in code or pseudo-code)?

[randoum]

Your example with :change_role? is quite what I am doing, except that the role is changed during the update action.

class UsersController < ApplicationController
  def update
    @user = User.find(params[:id])
    authorize @user, :update?, user_params[:role]
    ...
  end
end

def update?(new_role)
  # Allow to update itself / Cannot change its own role
  if record.id == current_user.id
    return if new_role and current_user.role != new_role

  # Updating another user
  else
    ### Access rights
    # Super Admin can update any user
    # Admin can update company's user / Cannot update deleted users
    return unless current_user.super_admin? or
                ( current_user.admin? and record.company_id == current_user.company_id and not record.deleted )

    ### user.role authorization
    if new_role 
      # Cannot change a company user to a freelance or a super_admin
      return if %w[admin employee].include? record.role and %w[super_admin freelance].include? new_role

      # Cannot change role of a freelance or a super_admin
      return if %w[super_admin freelance].include? record.role and record.role != new_role
    end
  end

  true
end

Note: this code looks ugly but it is un-factorized so you can read it without the helper methods

[zeevallin]

Though this can be useful, I would take another approach to the problem all-together. I love how pundit only has two perspectives, the user performing an action and the entity or scoped collection it's performed on, I'd argue you should treat relations, parameters and other meta the same way.

First. Touching on the topic of #50. Even if your association between certain things might vary in implementation, representing them with a class makes it more comprehensible in my opinion.

class ProjectPolicy < Struct.new(:user,:project)
  def update?
    project.owners.includes? user
  end
end

class ProjectMembershipPolicy < Struct.new(:user,:membership)
  def create?
    ProjectPolicy.new(user,membership.project).update? and membership.user.available?
  end
  def update?
    create? and not membership.locked?
  end
  def destroy?
    update?
  end
end

Second. When it comes to parameters, strong_parameters is quite the robust approach to solve this issue. If you're interested, I created a gem inspired by Pundit to easily structure your parameter constraints https://github.com/cloudsdaleapp/arcane – If you're not going to use it, at least check out the README for some clever examples of using strong parameters.

Third, any values for attributes should be handled with your ORM validations for consistency.

[randoum]

As for the scoping:

class UserPolicy
  class Scope
    def client_transferable_to(client)
      if current_user.super_admin? or current_user.admin? or current_user.employee?
        # A client can be transfered to another person of the same company (this is mainly a restriction for super_admin)
        User.where(company_id: client.owner.company_id, deleted: false).where.not(id: client.owner_id)
      else
        User.none
      end
    end
  end
end

class ClientsController < ApplicationController
  # GET /clients/1/start_transfer
  def start_transfer
    @client = Client.find(params[:id])
    @users = policy_scope(User, :client_transferable_to, @client).order(:first_name)
  end
end

[randoum]

Hi there,

Are going through with this or you decided to give up?
Thanks to let me know :)

Cheers

[thomasklemm]

@randoum @jnicklas I'm in favor of merging #50 regarding policy actions with parameters.
@jnicklas Please see @randoum's last comment for an example use case. What's your opinion on custom policy scopes?

[jnicklas]

I echo @Zeeraw's comment. If you really need additional data, you should probably wrap it in a class. I know this issue is brought up time and again, but I've yet to see anyone post a use case which is actually convincing.

Also, in this PR, the parameters to authorize are kind of ambiguous. It's very weird to have both default arguments and rest arguments in the same method signature.

[thomasklemm]

Let me sum that up: If you feel the need for passing in extra information, you're trying to do something that should more likely be done on another level. Options for that other level include:

• Modeling your associations so they reflect the "access rights" to a record, then go through associations in the policy
• Instantiating policies for associated models in your policy (as in @Zeeraw's example above)

[thomasklemm]

@jnicklas Triaging issues and PRs. 8 months later, I'm +1 on both ideas of

policy_scope(Post, :custom_resolve)
policy(@video, :show, embedded: true)

#140 is the first time I really feel this really has an application. I'll take a look at the other open PRs for this general issue.

[Austio]

@randoum I just implemented something similar by putting higher level logic in the application_policy and referenced it using the @record attribute. Would be happy to show you if you ping me. The gist is this though

I had 2 items that helped me segregate authorization, if someone was in a if an organization owned a record or if the record was owned by a user

def is_inside_organization(org_id)
  @user.organization_id == org_id
end

def is_mine(item_owner_id)
  @user.id == item_owner_id
end

Then in my policies I called these like this in order to check if something was inside

def show?
  is_mine(@record.user_id)
end

@record is the item i am checking authorization on, @user is provided by pundit and is the current_user.

Hope that helped you or someone in the future

[randoum]

@Austio thanks for that :) But I switched to another library long time ago