thoughtbot / administrate

A Rails engine that helps you put together a super-flexible admin dashboard.
http://administrate-demo.herokuapp.com
MIT License
5.9k stars 1.13k forks source link

Punditize concern does not restrict index #1348

Closed ademars94 closed 4 years ago

ademars94 commented 5 years ago

Expected Behavior: In application_policy.rb, if index? returns false, the user should not be able to access the index page.

Some Code: In application_policy.rb, I've defined my index? action to return false:

def index?
    false
end

Actual Behavior: User is able to access the index page.

Note: Even if I set show? to return false, the index page in Administrate will still show empty cells with no data. If I set show? to return true, the index page appears as it should (it shows the data). No matter what though, the index action is never properly restricted.

v2lrf commented 5 years ago

Hi @ademars94.

In application_policy.rb, if index? returns false, the user should not be able to access the index

Pundit has no incidence on display or not a view. Pundit is about Authorization. Do you have sufficient right or not to access to this view ? Yes or No. if not you are redirected... no white page is display....

However .... my knowledge let me to propose you that and according me this is the max that we could do to hide stuff on index with Pundit. Why cause index is linked to a scope through its concern objects at the end ...

class FooPolicy < ApplicationPolicy
   def index?
       true
   end
   # ...
   class Scope < Scope
    def resolve
      scope.all
    end
    def resolve_admin
      if user.admin == true
        scope.all 
      else
        scope.none #  if no admin => no object close to white page but not what you seem to want
      end
    end
  end
end

Just : if I want to restrict the view index for a specific kind of user. I would probably do something inside the controller... i would probably start by writing something like this but I haven't try and there is probably something more sexy..

    def index
       if current_user == "foo"
         redirect_to....
         flash[...]
        else 
           # code 
       end
      super
    end

However... if you want to restrict the index view... there is no reason, at first glance, to show all other actions from that controller (show, edit....) => keep it working but don't show it .

So your question is maybe how to hide views (buttons, views... ) from specifics controllers for specific(s) user.class? or user.status ? Rather than trying to hide index with policy.

So it's a kind of Punditize

you re right or rules base on who is the current_user... but it's different from the pundit objective which is focus on Authorization.

Your questions seem to fit more to a front (Ux/Ui ) question

rather than a pure Pundit Authorization question. Both are compatible and have to be conjugate together.

Use case kind of index => collection (inside show view for instance)

case 1 there is a white row all resources are display then if there is a punditize , the row with still be there but will be empty.... Capture d’écran 2019-06-14 à 21 35 24

to fix it...

generate views inside app/views/admin/customers/_collection.html.erb do something like this

<% resources.each do |resource| %>
      <%if resource.seller == current_seller %> #  👈🏼  this logic
       <tr .....>
        # ......
        </tr>
   <%end %>
<% end %>

Capture d’écran 2019-06-14 à 21 36 18

to fix the Front question in that case #626

that issue could help you . Inside layout for instance you could build something like this

<% if current_seller.admin? %>
   <% Administrate::Namespace.new(namespace).resources.each do |resource| %>
       # that will display all buttons links
   <% end %>
<% else %>
    <% resources =  AdministrateHiddenPages.routes(Administrate::Namespace.new(namespace)) %>
        <% resources.each do |resource| %>
             # that will display selected  actions links
       <% end %>
<% end %>

inside app/services/administrate_hidden_pages.rb

class AdministrateHiddenPages
  #  set pages which you wanna remove from sidebar
  HIDDEN_PAGES = [
    "quotation_attachments",
    "order_attachments",
    "project_attachments",
    # ...
    # ...
    "line_items",
    "payments",
  ].freeze

  def self.routes(admin)
    admin.resources.reject { |i| i.resource.in?(HIDDEN_PAGES) }
  end
end

=> details

to fix the Back question

Index action and pundit is one story. All other actions and pundit are an other story.

class Seller < ApplicationRecord
 # ...
 has_many :quotations
 # ...
end

have a look of an example I have for one of my controller => QuotationsController.

class QuotationPolicy < ApplicationPolicy
  def index?
    true
  end

  def show?
    if user.class == Seller
      record.seller == user || user.admin
    else
      record.customer == user
    end
  end

  def create?
    true
  end

  def new?
    create?
  end

  def update?
    true
  end

  def edit?
    update?
  end

  def destroy?
    true
  end
  def create_quotation_pdf?
    true
  end
  class Scope < Scope
    def resolve
      scope.where(customer: user)
    end
    def resolve_admin
      if user.admin
        scope.all
      else
        # i need to display on index view only quotation that have been 
        # build by the current user (here current_seller) 
        # the relation is seller has_many quotations and quotation belongs to seller 
        # i agree it's simple but it's good to start...
        scope.where(seller: user)
      end
    end
  end
end

then have a look on this kind of user story Let say that sellers have many suppliers and suppliers have many seller ... A join table have to exist between them...

class Seller < ApplicationRecord
 # ...
 has_many :quotations
 has_many :seller_suppliers
 has_many :suppliers, through: :seller_suppliers
 # ...
end
class Supplier < ApplicationRecord
 # ...
 has_many :seller_suppliers
 has_many :sellers, through: :seller_suppliers
 # ...
end

Let's say also that suppliers have many products....

class Supplier < ApplicationRecord
 # ...
 has_many :seller_suppliers
 has_many :sellers, through: :seller_suppliers
 has_many :products
 # ...
end

Now let's say that sellers have many products through supplliers... (through their suppliers)

class Seller < ApplicationRecord
 # ...
 has_many :quotations
 has_many :seller_suppliers
 has_many :suppliers, through: :seller_suppliers
 has_many :products, through: :suppliers
 # ...
end

What is the business logic when you are connected as seller ? What do you want to display on the index view from ProductsController?
And...How make a filter through allowed products if the seller want to create a new quotation for instance ???

=> inside your ```app/policies/product_policy.rb```` you have to build the scope ...

class ProductPolicy < ApplicationPolicy
  def index?
    true
  end

  def show?
    # record.supplier.supplier_users.pluck(:user_id).include? user.id
    true
  end

  def create?
    user.admin
  end

  def new?
    create?
  end

  def update?
      true
  end

  def edit?
    update?
  end

  def destroy?
    true
  end

  class Scope < Scope
    def resolve
      scope.all
    end
    def resolve_admin
      if user.admin == true
        scope.all
      else
        # scope.all
        scope.joins(:supplier).includes(:seller_suppliers).where(seller_suppliers: { seller_id: user.id })

      end
      # def resolve_admin_custom
      #   Product.joins(:supplier).where("supplier_id" => 2)
      # end
    end
  end

end

For instance here when I create a quotation, connected as seller with id 1 I have only access to the products owned by suppliers that seller 1 deal with

final_5cf8cc58f93dfd001368fa07_830653

Let me know if that answer to your question

nickcharlton commented 4 years ago

I'm going to close this as it's been inactive for a long time, plus there's a solution which addresses the original problem mentioned.