Pundit: Support custom arguments for policy

[sobrinho]

I would to allow only customer's owner to manage profiles (the customer is given by the controller):

class ProfilePolicy
  def initialize(user, customer, record)
    @user = user
    @customer = customer
    @record = record
  end

  def create?
    @customer.owner == @user
  end
end

What I'm currently doing is:

class ApplicationController
  def policy(record)
    policy = PolicyFinder.new(record).policy
    policy.new(current_user, current_customer, record) if policy
  end
end

If that's correctly, we should add a section on README explaining that.

[pas256]

Cool. What do you do for the scope?

[rupurt]

@sobrinho nice solution.

You can also set the policy attribute on the controller. I do that in a before_action instead of overriding the policy method on the controller.

def index
   person = Person.new
   company = Company.find_by_name("github")
   policy_class = Pundit::PolicyFinder.new(person).policy

   self.policy = policy_class.new(pundit_user, person, company)
   authorize(person)
end

[chrisalley]

This issue is similar to #181. I suggest using an attribute instead of a third parameter.

[pas256]

Could you use Pundit to build the permissions system GitHub uses?

• A repo belongs to either a user or an organization.
• An organization is owned by a team called Owners with the "admin" permission
• An organization can have many teams, and each team many users
• Each team can have either "read only" or "read/write" permissions on repos

I am duplicating just this for my own project, and have not yet found clean ways of doing it without extra arguments for both policies and scopes.

[chrisalley]

You can access both the user's relationships and the repo's relationships from within the policy method. Given that these entities are related in real life, you just need to set up the models in a way that they can all be referenced (and then compared) from either the user or repo record. Then you can use nested conditional statements within the policy method to check if you should authorise each of ther repo's actions.

It may help to break down some of the things you're checking into smaller methods and place these in the model - for example, you could have a user.member_of?(repo.team) method in your user model, and then check the truth of the method from within your policy methods. Pundit is unopinionated regarding how you go about this.

[pas256]

Thanks @chrisalley. I can see that working for policies, but for a controller index scope, I can't work it out. For example, the user wants to see all the teams for a particular Organization. The controller would have:

def index
  @teams = policy_scope(Team)
end

At this point, within the TeamPolicy class, I have no idea which Organization the user is looking at, so don't know which Organization to list the teams for. If I pass in anything other than Team to policy_scope, then a different policy is used. I could pass in @organization since the controller knows, but then OrganizationPolicy::Scope.resolve is called.

I know scopes are still new, so perhaps this just hasn't been worked out yet. Or, I am missing something.

[chrisalley]

You could make the organisation that the user is browsing a virtual attribute of the User model. This attribute could be set in the controller, based on the organisation's ID or slug in the URL. Then, within the policy (either the scope or an action), just reference user.current_organisation.team.

[jnicklas]

@pas256 I would model it the following way: Create a domain object which represents the context that a user is in. In the GitHub UI, a user can switch between different organizations, and this is maintained in the session, we also have the user information. So we could model this as a UserContext object, which has references to a user and organization, and then pass this in as the user. For example:

class ApplicationController
  include Pundit

  def pundit_user
    if session[:organization_id]
      UserContext.new(current_user, Organization.find(session[:organization_id]))
    else
      UserContext.new(current_user)
    end
  end
end

[pas256]

GitHub doesn't use a session to change context. I can go from https://github.com/pas256/myrepo to https://github.com/Answers4AWS/myotherrepo just by changing the URL. The Organization is determined entirely from the URL, no session information.

I get the idea though of a UserContext object.

To me, this just spreads the authorization logic around the code, rather than keeping it nicely within the Policy classes.

[johno]

It looks like Github uses a a nested resource relationship for organizations. Likely something similar to:

resources :repositories
resources :issues

resources :organizations do
  resources :repositories
  resources :issues
end

What I've been doing for this relationship is authorizing the parent resource, if it is a nested route:

class RepositoriesController < ApplicationController
  before_action :set_repository, except: :index
  before_action :set_organization

  after_action :authorize_repository
  after_action :authorize_organization

  # ...

  private

    def set_repository
      @repository = Repository.find(params[:id])
    end

    def set_organization
      @organization = Organization.find(params[:organization_id]) if params[:organization_id]
    end

    def authorize_repository
      authorize(@repository)
    end

    def authorize_organization
       authorize(@organization) if @organization
    end
end

That way, both the RepositoryPolicy and OrganizationPolicy remain vanilla Pundit policies. This also ensures that all authorization logic remains in the Policy classes.

Then for scoping, if it's a nested route and @organization is instantiated, you can just modify the query with policy_scope(Repository).where(organization_id: @organization.id).

[chrisalley]

To me, this just spreads the authorization logic around the code, rather than keeping it nicely within the Policy classes.

The user's context or current organisation isn't authorisation logic though; rather, it is descriptive of your domain model. So long as the _checking_ of the context is done within the policy, the authorisation logic remains centralised.

[sobrinho]

I liked the suggestion of UserContext object.

If that's the recommended way, can we add a section to README about this suggestion?

[jnicklas]

Added to README: https://github.com/elabs/pundit/blob/master/README.md#additional-context

[chrisalley]

The updated documentation looks good @jnicklas.

[sobrinho]

Thanks! :)

[vedmant]

It doesn't make sense for me, I have many cases when create? method requires another (parent) model to authorize against. Should I just authorize against that parent model policy instead? That looks like a messy approach, what if I have more logic for authorizing child including parent model, how I'm supposed to do so without passing additional params? Passing everything through a user context looks like a messy hack.

[Linuus]

@vedmant I say it depends. If it's like in the example above that you have a User and Organization that's always needed, use the UserContext approach.

Can you give an example where you need multiple objects?

Also, see: https://github.com/varvet/pundit#additional-context

Pundit strongly encourages you to model your application in such a way that the only context you need for authorization is a user object and a domain model that you want to check authorization for. If you find yourself needing more context than that, consider whether you are authorizing the right domain model, maybe another domain model (or a wrapper around multiple domain models) can provide the context you need.

Pundit does not allow you to pass additional arguments to policies for precisely this reason.

[vedmant]

@Linuus For example I have a "board" model that belongs to user, only owner can add posts to the board. In the posts controller new and create methods I do "authorize :post", that logical that I need to use PostPolicy in the PostsContoller, but I can't check if user owns board there unless I pass board model to PostPolicy, which is needed only for create? method. Passing board model in the UserContext doesn't make sense, it has nothing to do with user, furthermore why to write so much code just to pass one param for a single create? method.

[Linuus]

How about this:

# In controller
@post = Post.new(permitted_params) # Here the board_id is assigned to the post
authorize(@post)
if @post.save
  ...
else
  ...
end

# In policy
record # Is the post
record.board # is the board

Doesn't that work?

[vedmant]

@Linuus Yes, this works, but this is not obvious, creating model and passing prams through model before saving looks like a workaround, it also will require to change quite much code in my controllers and add new variables. If this is official approach it would be nice to have in the documentation as it's not obvious.

[Linuus]

@vedmant That's the idiomatic approach in Rails projects so I don't think we need it in the Pundit docs.

See: https://guides.rubyonrails.org/action_controller_overview.html#parameters