Use pundit policy_namespace in controllers (#2379)

app/controllers/concerns/administrate/punditize.rb

@@ -12,32 +12,53 @@ module Punditize
       included do
         private
 
+        def policy_namespace
+          []
+        end
+
         def scoped_resource
-          policy_scope_admin super
+          namespaced_scope = policy_namespace + [super]
+          policy_scope!(pundit_user, namespaced_scope)
         end
 
         def authorize_resource(resource)
-          authorize resource
+          namespaced_resource = policy_namespace + [resource]
+          authorize namespaced_resource
         end
 
         def authorized_action?(resource, action)
-          Pundit.policy!(pundit_user, resource).send("#{action}?".to_sym)
+          namespaced_resource = policy_namespace + [resource]
+          policy = Pundit.policy!(pundit_user, namespaced_resource)
+          policy.send("#{action}?".to_sym)
         end
       end
 
       private
 
-      # 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.
-      def policy_scope_admin(scope)
-        ps = Pundit::PolicyFinder.new(scope).scope!.new(pundit_user, scope)
-        if ps.respond_to? :resolve_admin
-          ps.resolve_admin
+      def policy_scope!(user, scope)
+        policy_scope_class = Pundit::PolicyFinder.new(scope).scope!
+
+        begin
+          policy_scope = policy_scope_class.new(user, pundit_model(scope))
+        rescue ArgumentError
+          raise(Pundit::InvalidConstructorError,
+                "Invalid #<#{policy_scope_class}> constructor is called")
+        end
+
+        if policy_scope.respond_to? :resolve_admin
+          ActiveSupport::Deprecation.warn(
+            "Pundit policy scope `resolve_admin` method is deprecated. " +
+            "Please use a namespaced pundit policy instead.",
+          )
+          policy_scope.resolve_admin
         else
-          ps.resolve
+          policy_scope.resolve
         end
       end
+
+      def pundit_model(record)
+        record.is_a?(Array) ? record.last : record
+      end
     end
   end
 end

docs/authorization.md

@@ -28,20 +28,30 @@ technically have access to see in the main app. For example, a user may
 have all public records in their scope, but you want to only show *their*
 records in the admin interface to reduce confusion.
 
-In this case, you can add an additional `resolve_admin` to your policy's
-scope and Administrate will use this instead of the `resolve` method.
+In this case, you can add additional pundit `policy_namespace` in your controller
+and Administrate will use the namespaced pundit policy instead.
 
 For example:
 
 ```ruby
-class PostPolicy < ApplicationPolicy
-  class Scope < Scope
-    def resolve
-      scope.all
+# app/controllers/admin/posts_controller.rb
+module Admin
+  class PostsController < ApplicationController
+    include Administrate::Punditize
+
+    def policy_namespace
+      [:admin]
     end
+  end
+end
 
-    def resolve_admin
-      scope.where(owner: user)
+# app/policies/admin/post_policy.rb
+module Admin
+  class PostPolicy < ApplicationPolicy
+    class Scope < Scope
+      def resolve
+        scope.where(owner: user)
+      end
     end
   end
 end

spec/controllers/admin/orders_controller_spec.rb

@@ -4,9 +4,132 @@
 # which will test all the authorization functionality.
 
 describe Admin::OrdersController, type: :controller do
-  context "with Punditize concern" do
+  context "with namespaced Punditize concern" do
     controller(Admin::OrdersController) do
       include Administrate::Punditize
+
+      def policy_namespace
+        [:own]
+      end
+
+      def pundit_user
+        Customer.find_by(name: "Current User")
+      end
+    end
+
+    let!(:user) { create(:customer, name: "Current User") }
+
+    describe "GET index" do
+      it "shows only your own orders" do
+        someone = create(:customer)
+        order1 = create(:order, customer: user)
+        _order2 = create(:order, customer: someone)
+        order3 = create(:order, customer: user)
+
+        locals = capture_view_locals { get :index }
+
+        expect(locals[:resources]).to contain_exactly(order1, order3)
+      end
+    end
+
+    describe "GET new" do
+      it "raises a Pundit error" do
+        expect { get :new }.to raise_error(Pundit::NotAuthorizedError)
+      end
+    end
+
+    describe "GET edit" do
+      it "allows me to edit my records" do
+        order = create :order, customer: user
+        expect { get :edit, params: { id: order.id } }.not_to raise_error
+      end
+
+      it "does not allow me to see other users' records" do
+        other_user = create(:customer)
+        order = create(:order, customer: other_user)
+        expect { get :show, params: { id: order.id } }.
+          to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+
+    describe "PUT update" do
+      def send_request(order:)
+        put(
+          :update,
+          params: {
+            id: order.id,
+            order: { address_line_one: "22 Acacia Avenue" },
+          },
+        )
+      end
+
+      it "allows me to update my records" do
+        order = create(:order, customer: user)
+        send_request(order: order)
+        expect(response).to redirect_to([:admin, order])
+        expect(order.reload.address_line_one).to eq("22 Acacia Avenue")
+      end
+
+      it "does not allow me to update other users' records" do
+        other_user = create(:customer)
+        order = create(:order, customer: other_user)
+        expect do
+          send_request(order: order)
+        end.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+
+    describe "DELETE destroy" do
+      it "never allows me to delete a record" do
+        o = create :order, customer: user, address_state: "AZ"
+        expect { delete :destroy, params: { id: o.id } }.
+          to raise_error(Pundit::NotAuthorizedError)
+      end
+    end
+
+    describe "#authorized_action?" do
+      it "shows edit actions for records by the user" do
+        o = create(:order, customer: user)
+        expect(controller.send(:authorized_action?, o, :edit)).to be true
+      end
+
+      it "does not show edit actions for records from other users" do
+        someone = create(:customer)
+        o = create(:order, customer: someone)
+        expect(controller.send(:authorized_action?, o, :edit)).to be false
+      end
+
+      it "never shows destroy actions" do
+        o = create :order, customer: user, address_state: "AZ"
+        expect(controller.send(:authorized_action?, o, :destroy)).to be false
+      end
+    end
+  end
+
+  context "with deprecated Punditize concern" do
+    before do
+      allow(ActiveSupport::Deprecation).to receive(:warn)
+
+      class OrderPolicy
+        class Scope
+          def resolve_admin
+            scope.where(customer: user)
+          end
+        end
+      end
+    end
+
+    after do
+      class OrderPolicy
+        class Scope
+          undef resolve_admin
+        end
+      end
+    end
+
+    controller(Admin::OrdersController) do
+      include Administrate::Punditize
+
       def pundit_user
         Customer.find_by(name: "Current User")
       end
@@ -24,6 +147,8 @@ def pundit_user
         locals = capture_view_locals { get :index }
 
         expect(locals[:resources]).to contain_exactly(order1, order3)
+        expect(ActiveSupport::Deprecation).to have_received(:warn).
+          with(/`resolve_admin` method is deprecated/)
       end
     end
 
@@ -37,13 +162,17 @@ def pundit_user
       it "allows me to edit my records" do
         order = create :order, customer: user
         expect { get :edit, params: { id: order.id } }.not_to raise_error
+        expect(ActiveSupport::Deprecation).to have_received(:warn).
+          with(/`resolve_admin` method is deprecated/)
       end
 
       it "does not allow me to see other users' records" do
         other_user = create(:customer)
         order = create(:order, customer: other_user)
         expect { get :show, params: { id: order.id } }.
           to raise_error(ActiveRecord::RecordNotFound)
+        expect(ActiveSupport::Deprecation).to have_received(:warn).
+          with(/`resolve_admin` method is deprecated/)
       end
     end
 
@@ -63,6 +192,8 @@ def send_request(order:)
         send_request(order: order)
         expect(response).to redirect_to([:admin, order])
         expect(order.reload.address_line_one).to eq("22 Acacia Avenue")
+        expect(ActiveSupport::Deprecation).to have_received(:warn).
+          with(/`resolve_admin` method is deprecated/)
       end
 
       it "does not allow me to update other users' records" do
@@ -71,6 +202,8 @@ def send_request(order:)
         expect do
           send_request(order: order)
         end.to raise_error(ActiveRecord::RecordNotFound)
+        expect(ActiveSupport::Deprecation).to have_received(:warn).
+          with(/`resolve_admin` method is deprecated/)
       end
     end
 

spec/example_app/app/policies/order_policy.rb

@@ -3,14 +3,6 @@ class Scope < Scope
     def resolve
       scope.all
     end
-
-    def resolve_admin
-      if user.admin?
-        scope
-      else
-        scope.where(customer: user)
-      end
-    end
   end
 
   def create?

spec/example_app/app/policies/own/order_policy.rb

@@ -0,0 +1,9 @@
+module Own
+  class OrderPolicy < ::OrderPolicy
+    class Scope < Scope
+      def resolve
+        scope.where(customer: user)
+      end
+    end
+  end
+end