Before/after hooks on authorize and policy_scope

[prschmid]

As there is one policy file per model (and an application_policy) file, there is currently no easy way to set up standard setup/teardown behavior in all policy files if the individual methods are overwritten. I'd like to propose adding before/after hooks that are called as part of the normal life cycle of evaluating a policy. E.g. very simplistically it would be something like

def authorize(user, record, query)
      policy = policy!(user, record)

      policy.before
      unless policy.public_send(query)
        raise NotAuthorizedError, query: query, record: record, policy: policy
      end
      policy.after

      record
end

I'd suggest having multiple hooks so that there is a top level before/after (as shown above) and then auto_generated before_[query] methods that can also be implemented.

[tom-lord]

Can you give an example of something you'd like to place in the before_<query> methods? I suspect you may be putting something in the policy that belongs elsewhere.

[WaKeMaTTa]

I'm expecting the same expectation as @tom-lord . Share an example @prschmid please.

[prschmid]

Hi @tom-lord @WaKeMaTTa. Sorry for not responding sooner, must have gotten lost in my inbox.

It's been 2 months since I posted this issue, and for the life of me I can't remember what I was trying to do and why I was requesting this. Clearly I was able to work around it some other way.

Feel free to close/"wont_fix" this issue.

[pacop]

Hi, let us imagine a case where we have two different roles, a plain user and a supervisor.

User is able to access to his own resources, in this sense I am checking on each policy that current_user == resource.owner.

However, supervisor only can access to resources from another user, and he CANNOT access to his own resources. To the best of my knowledge it means that I should add in each line as the previous one: current_user == resource.owner && !current_user.supervisor?

Am I right? I think that it could be interesting to have a hook, where I could put sth like that:

class ApplicationPolicy
  before_action :not_supervisor!

  def not_supervisor
    !current_user.supervisor
  end
end

[tom-lord]

@pacop That example sounds a little contrived; but regardless, you could code that trivially in 1 line, without any additional Pundit extension, by use of the XOR operator:

class MyResourcePolicy
  def show?
    user.supervisor? ^ user == resource.owner
  end
end

[pacop]

@tom-lord is a real example, I came to this issue because I gonna open a new one but I search first. I can find many examples: a blog where the supervisor cannot create posts but it can accept or refuse posts, a consultant reviewing invoices from another user where he cannot create invoices....

Yeah I know that it solves the problem, but my project has so many policies that add this line to each policy could perfectly by two day of work. When If I were able to have a hook, the problem would be solved in only one line and in one unique place, much faster and elegant

[tom-lord]

@pacop

a blog where the supervisor cannot create posts but it can accept or refuse posts

So a supervisor is not allowed to create?, but they are allowed to publish?. That's two distinct actions; not a single create action.

a consultant reviewing invoices from another user where he cannot create invoices

So a consultant is not allowed to create?, but they are allowed to review?. That's two distinct actions; not a single create action.

my project has so many policies that add this line to each policy could perfectly by two day of work. When If I were able to have a hook, the problem would be solved in only one line and in one unique place, much faster and elegant

If the policy logic is identical, then you could consider using some form of composition/inheritance in the policy class, or a helper method, to remove repetition.

For example:

class MyResourcePolicy < ApplicationPolicy
  def show?
    supervisor_xor_owner
  end
end

class ApplicationPolicy
  # ...
  protected

  def supervisor_xor_owner
    user.supervisor? ^ user == resource.owner
  end
end

Or even, if you feel the need to take the abstraction further, for example:

class MyResourcePolicy < ApplicationPolicy
  include SupervisorXorOwnerPolicyConcern
end

module SupervisorXorOwnerPolicyConcern
  def show?
    supervisor_xor_owner
  end

  def update?
    supervisor_xor_owner
  end

  def destroy?
    supervisor_xor_owner
  end
end

Regardless, what you are suggesting as an alternative solution would be to add a before hook to the controller, not the policy. I wouldn't do this personally, but you are already free to do so if you prefer:

class MyResourcesController < ApplicationController
  before_action :not_supervisor, only: :create

  private

  def not_supervisor
    raise Pundit::NotAuthorizedError, "supervisors cannot create" if user.supervisor?
  end
end

This issue was requesting a policy hook, which - as far as I can tell - is not really what you are suggesting.

[pacop]

So a consultant is not allowed to create?, but they are allowed to review?. That's two distinct actions; not a single create action.

You are completely right, in this example is pretty easy to discern. In real life anything is that easy.

For example:

Regarding your first example, I know that perfectly works but in this case I still have to go in each policy adding this chunk of code. This part is which I am trying to avoid since I want to apply this restriction always, so for me it does not make sense having to repeat the same call. What happens if I forget one policy?

Or even, if you feel the need to take the abstraction further, for example:
Same thing, my policies are complex and in a example is pretty easy to do that. What happens when you have a complex policy and you have to add a restriction as I said before?

Composition is not a option since I still have to go for each policy including the module or doing whatever is required to avoid repetition.

Regardless, what you are suggesting as an alternative solution would be to add a before hook to the controller, not the policy. I wouldn't do this personally, but you are already free to do so if you prefer:

This very example is what I am trying to do. Now I see that you know what I am trying to achieve. But If you use a controller you would be mixing your policies with controllers which it is not a good practice. I just was trying to do this but with ApplicationPolicy.

class ApplicationPolicy
  before_action :not_supervisor!

  def not_supervisor!
    !current_user.supervisor
  end
end
class MyResource1 < ApplicationPolicy
   def create?
      # My complex logic here, I don't have to check for !supervisor since I know that is always applied.
   end
end
... many more resources where supervisor cannot access
class MyResouce3000 < ApplicationPolicy
    #Same as 1..3000
end
# 
class MyResource30001 < ApplicationPolicy
    skip_before_action :not_supervisor!, only: %i[create?]
    def show?
       # Supervisor cannot access here
    end
    def create?
      #Some additional logic here. Here supervisor is free to do whatever he wants
    end
end

For me this solution is much more elegant don't you think? Let me know your thoughts in that case, BTW thanks for being open to discuss.

[tom-lord]

For me this solution is much more elegant don't you think? Let me know your thoughts in that case

I get what you're trying to achieve, but I still feel it would be relatively easy to achieve with vanilla ruby patterns. For example:

class ApplicationPolicy
  def create?
    !user.supervisor?
  end
end

class MyResource1 < ApplicationPolicy
   def create?
      super || # My complex logic here
   end
end

class MyResource30001 < ApplicationPolicy
  def create?
     super || # Supervisor cannot access here
  end
  def create?
    # Do not call super
    # Some additional logic here. Here supervisor is free to do whatever he wants
  end
end

Or if you want to write even less code, how about using Module#prepend. I haven't thought this through too thoroughly, but something vaguely along the lines of:

class MyResource1 < NonSuperVisorPolicy
 def create?
    # My complex logic here
 end
end

class MyResource30001 < ApplicationPolicy
 def create?
    # My complex logic here
 end
end

class NonSuperVisorPolicy
  prepend NonSuperVisorRule
end

module NonSuperVisorRule
  def show?
    !user.supervisor? || super
  end
end

Given this is a relatively unusual requirement, I'm not personally convinced it warrants additional functionality to the Pundit library, which prides itself in being as minimal as possible.

[javierav]

If anyone is interested in solving this problem, I have implemented a solution in pundit-before gem.