Unable to access namespaced policy in view

[jrmehle]

Given a TagsController and an Admin::TagsController each with a corresponding policy, how does one access the admin policy from a view? I've got a global nav link I'd like to only expose to admins.

policy(Tag).index? references the non-Admin namespace policy

I assumed something like this would work: policy(Admin::Tag).index? However, Admin::Tag is not defined and thus, it throws an error.

Ideally, I'd do something like so: policy(Tag, namespace: 'admin').index?

[thomasklemm]

@jnicklas Do we need to address this before releasing 0.3.0?

[jnicklas]

I want to push 0.3.0 out the door ASAP. But all this namespacing stuff makes me uneasy. I'm considering reverting all of it. I never really liked the idea anyway, and it seems to cause all kinds of weird inconsistencies and problems.

[jrmehle]

@jnicklas Are you talking about removing name-spacing for good? Or just pulling it back from the 0.3.0 release so it can be given more attention for a future release?

[jnicklas]

@jrmehle a bit of both. It'd be completely removed for 0.3.0. We might add it again in the future, if all of these problems can be addressed in some way. I'm personally not convinced of the utility of this feature, but it seems that this has had some adoption. I'm unsure of the best course going forward.

[thomasklemm]

@jnicklas I'm d'accord with rolling back namespacing changes and maybe moving them to a seperate branch first. I'm not sure how widely they've been adopted yet, but I guess we should maybe take a very different approach towards handling different kinds of application domains, since these Rails autoloading issues might cause pain for lots of people in their development and test environments. We could go back to the drawing board altogether and work on a solution in which only a PostPolicy exists, with different namespaces / user roles represented in there.

[thomasklemm]

@jnicklas I'll open a PR for that.

[jrmehle]

I would much appreciate a branch containing the current state of namespacing. That way those of us who have decided to utilize the feature can still continue doing so without interruption.

[chrisalley]

@jnicklas I'm not convinced of it's utility either, and it could be detrimental to the clarity of the system as there is no longer a single source of truth (about how to authorise a particular model). I like @thomasklemm's idea of investigating a solution which adds further authorisation conditions inside the single policy class.

[jnicklas]

Alright, namespaces have been yanked. And 0.3.0 has been released without namespaces. I'm with @thomasklemm and @chrisalley on this. I think having multiple policies for the same object is confusing and counter productive and I wouldn't want to have it in my app. Since it seemed like this was a light-weight feature I was okay with adding it anyway, since many people were requesting it, but since this is turning out to be a source of bugs and a maintenance nightmare, I don't think they'll be making a comeback any time soon.

I've created a branch called 'namespaces' which has a reference to the last commit before namespaces were yanked for anyone who needs it.

[onemanstartup]

@jnicklas I found a way to autoload namespace

class ApplicationPolicy
  extend ActiveSupport::Autoload
  autoload :Admin
end

That's all. It will fix such bugs. What are the other ways to restrict users to access actions in admin namespace?
I have 2 policies and 2 controllers, how will I do that without namespaces?

class ArticlePolicy < ApplicationPolicy
  def index?
    true
  end
end

class Admin::ArticlePolicy < ArticlePolicy
  def index?
    if user && user.admin?
      true
    else
      false
    end
  end
end

[jnicklas]

I would simply do two separate query methods on the same policy.

class ArticlePolicy < ApplicationPolicy
  def index?
    true
  end

  def index_admin? # or whatever name you prefer
    user and user.admin?
  end
end

[onemanstartup]

@jnicklas
I'm trying to do REST approach and automate everything i can, so I'm not doing authorize @article manually. However, how about this approach.

class ApplicationPolicy
  attr_reader :user, :record, :controller

  def initialize(user, record, controller=nil)
    @user = user
    @record = record
    @controller = controller
  end
end

class ArticlePolicy < ApplicationPolicy
  def index?
    if controller == Admin
      return false unless user && user.admin?
    end
    true
  end
end

module Pundit
   # ...
    def policy(user, record, controller=nil)
      policy = PolicyFinder.new(record).policy
      policy.new(user, record, controller) if policy
    end

    def policy!(user, record, controller=nil)
      PolicyFinder.new(record).policy!.new(user, record, controller)
    end
   #....
end

With access to what you want, you can access in policy what you want.
Maybe not exactly controller, but something else. Maybe hash.

[onemanstartup]

Ok, I've managed to do that with overwriting policy method in my concern https://gist.github.com/onemanstartup/9564137fc482a327f0e5#file-accessable-rb-L30 but it will be great if such posibilty were out of the box.

[jnicklas]

@onemanstartup it's certainly your prerogative to use something like InheritedResources, but it's never, ever worked out for me in the dozen or so projects I've tried it on. We've always ended up ripping it out in the end. Learn to live with the un-DRY-ness of controllers, and you'll be way happier.

I think the approach you are suggesting is very flawed. The whole point of having policy objects in the first place is to decouple them from controllers. Otherwise we could have simply added all of this stuff to the controller. You reintroduce the coupling and trust me, in two weeks you won't be able to untangle them anymore.

[chrisalley]

@onemanstartup Another approach is to make "browsing admin panel" part of your user context. As explained in the documentation you can create a custom Pundit user which can be linked to a particular user context. A "browsing admin panel" boolean flag could be part of that context.

[YanhaoYang]

@onemanstartup's example of 2 policies and 2 controllers is a typical usage of namespaces. @jnicklas's solution to define a policy as "index_admin?" is not very elegant, I think. With this solution, shall we also name the action as "index_admin", breaking all Rails' conventions, or pass the name of the permission every time?

[chrisalley]

@jnicklas I think of a policy method as a gateway to a model. Would this be an accurate description of your intention with Pundit? If so, would it not make more sense to have a single gateway for each action, where the action "update" is the same action to the model, just attempted in two different contexts (admin and non admin mode). My point being that this seems to be a user context difference rather than an action difference.

[jnicklas]

@chrisalley yes, I agree with that, and it's very clear for update actions. There's no point in being allowed to update in one context and not in another. It's debatable if this is the right approach, authorization is a controller concern after all, but controllers are also interfaces to models in some way.

It also definitely gets tricky for index? actions. @YanhaoYang I would definitely not recommend adding other query methods for something like update?, update_admin? is an antipattern, I think it should be obvious why. I also definitely would not change the action name.

The problem here is that the context actually does matter. Maybe the admin list shows some additional data which would otherwise be hidden to regular users, so there is some need to differentiate it. I'm not sure about what the best way to do that is, and maybe namespaces is it, but that's not why I yanked it. The reason that namespaces was yanked was that the feature as implemented was terribly broken. And since I didn't particularly agree with it in the first place, and we desperately needed to push 0.3.0, because people were screaming for it, I felt that a solution to this problem was secondary, since it's after all quite easy to work around. If we do decide to tackle this again, I would like to explore other options as well.

[chrisalley]

@jnicklas Yeah, I can see how authorising the index action differently based on the controller might be required (although I think just scoping a different query in the admin controller would be enough in many cases - no need to deny the current user access to the whole index action).

One of the things I like about Ruby is that it is a very expressive language; reading Ruby code is often like reading a story. From a story telling point of view, the namespaces approach doesn't reveal the intention behind authorising the admin articles controller differently. Both articles controllers might need to authorise the index action in a different way depending on a combination of the (1) the relationship between the current user and the articles and (2) the particular controller that the action is being authorised within. The real world story behind (1) is usually quite clear - for example, the user is denied access to articles that she is not the author of. It is less clear why using a different controller/interface matters though. I think the question we should be asking is: if does matter, how can we express the story behind authorising a different controller/interface clearly in code?

Making the current controller/interface part of the user context seems cleanest to me as it represents part of the current user's state in the real world - browsing a different UI. But perhaps Pundit's authorize method should automatically pass (to the policy) the current user as well as some details about the user's current environment? It then becomes difficult to decide which information to include. The more environmental information is passed to the policy, the tighter it is coupled to the controller. I dislike this as it increases complexity, but maybe authorising the current user alone isn't enough.

[onemanstartup]

I solved this issue by creating class method for controller authorizable_by :admin which enables before_filter to access protected controller, all other authorization rules are in pundit files. Most common(imho) situation is protect admin controller from accessing by other people. That's all, all other rules can be expressed by pundit permissions.

[chrisalley]

@onemanstartup I've done something similar in an app that I've been working on, but the before filter is placed in the admin base controller which then applies to all child admin controllers. Inside these controllers there is no need to authorise actions with Pundit since the user either has full access to all admin controllers (if they are an admin) or no access. Pundit is just used to authorise non-admin controller actions.

[onemanstartup]

@chrisalley I have many roles, accountant, content-manager, etc.. so admin area is not specifically for admins :)

[chibicode]

I've been using Pundit inside a Rails engine, and this would have been very useful, since by default every class in my engine would go under a module...

[Systho]

I've been using this module to solve my namespace problem. It is not covered by automated tests (but is used on production ) and I just put it here so that it might help other people. Feel free to use it, change it or say it's crap, it has served me well and it might help you, too :)

https://gist.github.com/Systho/3d7632b5aa999cf88d87

[hwhelchel]

What about using simple delegators?

Let's say we are a news media app with a paywall for articles so we want have a policy for article preview and for the full article.

class Api::ArticlePreviewsController < Api::BaseController

  def index
    respond_with :api, article_previews, each_serializer: ArticlePreviewSerializer
  end

  def show
    respond_with :api, article_preview, serializer: ArticlePreviewSerializer
  end

  private

  def article_previews
    @article_previews ||= policy_scope ArticlePreview.new(Article)
  end

  def article_preview
    @article_preview ||= authorize ArticlePreview.new(Article.find(params[:id]))
  end
end

class ArticlePreview < SimpleDelegator; end

Since ArticlePreview is a delegator it will delegate to anything passed in the initializer so that is why we can pass in the class or the instance of the Article. However if you ask an instance of ArticlePreview what it's #class is it returns ArticlePreview

[rapind]

My 2 cents are that I'm convinced that authorization should be tied to controllers and not models. To that end, namespacing would be nice.

However if the Pundit philosophy is of a model based authorization, then it's almost definitely a bad idea to add namespacing.

[chendrix]

Even if we believe that Pundit should be tied to authorization around models and not controllers, there is such a thing as namespaced models.

[chendrix]

As a potential work around for those trying to get namespaced policies in views, you can do something like:

If you have a Foo::BarPolicy and you're trying to access the show? method

policy('Foo::Bar'.to_sym).show?

You can even wrap this into a helper for easier use

module ApplicationHelper
  def foo_policy(policy_symbol)
    policy_class_name = policy_symbol.to_s.classify
    policy("Foo::#{policy_class_name}".to_sym)
  end
end

Then used in a view like:

foo_policy(:bar).show?

[pascalbetz]

I'm with @rapind on this one. Having self.policy_class on the model seems wrong to me. At least in the context of a rails application, where the controller defines the context (Admin::, Api:: namespace), the action (index/show...) as well as the resource (FoosController which might exist inside Admin:: and Api::).

[edjarris]

Recently I open an issue to ckeditor gem because I am trying to integrate with pundit galetahub/ckeditor#609. I posted here because I think is related. If someone could bring me some light to the darkness I'll be very greatful :)

Thanks in advance,