varvet / pundit

Minimal authorization through OO design and pure Ruby classes
MIT License
8.24k stars 627 forks source link

Record is array when using namespaced policies with policy_class set #689

Closed vivekmiyani closed 2 years ago

vivekmiyani commented 2 years ago

Ruby, Rails and pundit versions

Ruby version: 3.0 Rails version: 6.1.4.1 Pundit version: master

How I am using

# app/controllers/admin_controller.rb

class AdminController < ApplicationController
  def authorize(record, query = nil, policy_class: nil)
    super([:admin, record], query, policy_class: policy_class)
  end
end
# app/controllers/admin/posts_controller.rb

class Admin::PostsController < AdminController
  def show
    @publication = authorize Post.find(params[:id]), policy_class: Admin::PublicationPolicy
  end
end

Observed behaviour

Inside Admin::PublicationPolicy I am getting record array (with namespace).

     4:       scope.all
     5:     end
     6:   end
     7:
     8:   def show?
 =>  9:     binding.irb
    10:   end
    11: end

3.0.2 :001 > record
 => [:admin, #<Post>]

Expected behaviour

Inside Admin::PublicationPolicy, I should able to get Post object.

vivekmiyani commented 2 years ago

Happy to raise an PR, if this is really an issue 😃 .

iggant commented 2 years ago

Hi all. We have a trouble with current changes: we were using this feature as current manner:

authorize([initial_country, target_country], policy_class: CountryPolicy)

and inside CountryPolicy:

  def copy_to_country?
    initial_country, target_country = record

    country_allowed?(initial_country.country) && country_allowed?(target_country)
  end

and now there is no way to get initial_country. Record is always target_country

dgmstuart commented 2 years ago

@iggant I'm afraid this doesn't look like a valid use of Pundit?

When passing an array, this is interpreted as namespacing, as documented in the README: https://github.com/varvet/pundit#policy-namespacing

It seems that the thing which made your code work before was a bug in the code for handling namespacing, which was raised in this issue.

Pundit is designed to authorise access to individual resources: passing arbitrary ruby objects (like this array of two objects) is not something we can reasonably support.

I'm not sure I understand your use case without additional context, but in the code you've shown, there's no comparison between the two countries, so I believe it could be implemented like this:

authorize(initial_country.country)
authorize(target_country)
dgmstuart commented 2 years ago

@iggant or I guess maybe this?

# controller
def copy 
  authorize(initial_country.country, :allowed)
  authorize(target_country, :allowed)

  # ...
end
# policies/country_policy.rb
def allowed?
  country_allowed?(record)
end

Without seeing the rest of your code, it's hard to tell what the best way to implement this is, but hopefully you get the idea.