Authorization with Pundit doesn't allow Policy Namespacing

[bhtabor]

Punditize assumes that there is no Pundit Policy Namespacing being used. If we allow namespaces, we could even avoid the custom policy_scope_admin.

Quick work around is, instead of including Punditize, to use the following code:

# All Administrate controllers inherit from this `Admin::ApplicationController`,
# making it the ideal place to put authentication logic or other
# before_actions.
#
# If you want to add pagination or other controller-level concerns,
# you're free to overwrite the RESTful controller actions.
module Admin
  class ApplicationController < Administrate::ApplicationController
    include Pundit

    def policy_scope(scope)
      super([:admin, scope])
    end

    def authorize(record, query = nil)
      super([:admin, record], query)
    end

    def scoped_resource
      policy_scope super
    end

    def authorize_resource(resource)
      authorize resource
    end

    def show_action?(action, resource)
      Pundit.policy!(pundit_user, [:admin, resource]).send("#{action}?".to_sym)
    end

  end
end

[robinboening]

I had the same thoughts for namespacing after stumbling into an error because I like using Pundit's verify_policy_scoped to verify the use of policy_scope.

My approach for changing administrate's punditize is a bit different, though. I believe deprecating the policy_scope_admin and removing it on the next major version is the right way to go, not to build on top of it. Then policy_scope should always be used within punditize, without the need to add it manually. I believe software should needs to get rid of legacy code if we agree on a better approach.

[bhtabor]

I agree @robinboening We should just depricate resolve_admin. The whole idea behind policy_scope_admin is stated in the comment below:

# Like the policy_scope method in stock Pundit, but allows the 'resolve'
# to be overridden by 'resolve_admin' for a different index scope in Admin
# controllers.

This is the problem. It is trying to do what Pundit already does with Policy Namespacing. Admin scope is just like a Pundit Namespace.

We still need to allow Pundit Namespacing though, like I did in this pull request - https://github.com/thoughtbot/administrate/pull/1335. I kept the resolve_admin feature for backward compatibility only. But, it could also be used to even have an additional admin scope over a namespace.

[archonic]

I just hit this issue upgrading from 0.11 to 0.14 and I think @bhtabor's solution works perfectly. There's unreleased pundit stuff in the master branch but I think it would make sense to sort out all pundit stuff before releasing 0.15.

It's worth noting that I've created a folder with a bunch of admin policy files and each file specifies index?, show?, create?, update? and destroy? only to have user.super_admin? inside of every method. While the organization makes sense, it was cleaner to just authorize routes with a constraint. Once I realized this, I removed the include Pundit solution here and all the admin policy files. I was thrown off by the instructions in the readme saying to include Administrate::Punditize if you already have Pundit. If you're only ever checking one thing, a route constraint is the way to go.

[sedubois]

@archonic you'll find a proposed solution in #1663, feel free to take over that one if you'd like to help it get merged!

[Holist]

I think this has to be closed (https://github.com/thoughtbot/administrate/pull/2379)