Support strict with_role queries. Issue #362 (#543)

* Support strict with_role queries

Why:

* There is an outstanding issues #362
* strict mode should be enforced on role queries and resource queries

How:

* Pass a strict flag to the scope function
* call where or where_strict depending on strict flag in active_record adapter
* Add tests for strict mode resource queries

Note:

* This PR does not address strict mode for mongoid since I am unfamiliar with that ORM

* Update where_strict for mongoid

Why:

* The tests will fail until mongo works with strict resource querying

This change addresses the need by:

* Copy the where_strict implementation from the active record adapter

* Make sure to call strict where

* Refactor to reduce mutations

Co-authored-by: Thomas McDonald <thomas-mcdonald@users.noreply.github.com>

Author: Mrjaco12

lib/rolify/adapters/active_record/role_adapter.rb

@@ -9,14 +9,20 @@ def where(relation, *args)
       end
 
       def where_strict(relation, args)
-        return relation.where(:name => args[:name]) if args[:resource].blank?
-        resource = if args[:resource].is_a?(Class)
-                     {class: args[:resource].to_s, id: nil}
-                   else
-                     {class: args[:resource].class.name, id: args[:resource].id}
-                   end
-
-        relation.where(:name => args[:name], :resource_type => resource[:class], :resource_id => resource[:id])
+        wrap_conditions = relation.name != role_class.name
+
+        conditions = if args[:resource].is_a?(Class)
+                       {:resource_type => args[:resource].to_s, :resource_id => nil }
+                     elsif args[:resource].present?
+                       {:resource_type => args[:resource].class.name, :resource_id => args[:resource].id}
+                     else
+                       {}
+                     end
+
+        conditions.merge!(:name => args[:name])
+        conditions = wrap_conditions ? { role_table => conditions } : conditions
+
+        relation.where(conditions)
       end
 
       def find_cached(relation, args)
@@ -67,9 +73,9 @@ def exists?(relation, column)
         relation.where("#{column} IS NOT NULL")
       end
 
-      def scope(relation, conditions)
+      def scope(relation, conditions, strict)
         query = relation.joins(:roles)
-        query = where(query, conditions)
+        query = strict ? where_strict(query, conditions) : where(query, conditions)
         query
       end
 

lib/rolify/adapters/mongoid/role_adapter.rb

@@ -9,14 +9,20 @@ def where(relation, *args)
       end
 
       def where_strict(relation, args)
-        return relation.where(:name => args[:name]) if args[:resource].blank?
-        resource = if args[:resource].is_a?(Class)
-                     {class: args[:resource].to_s, id: nil}
-                   else
-                     {class: args[:resource].class.name, id: args[:resource].id}
-                   end
-
-        relation.where(:name => args[:name], :resource_type => resource[:class], :resource_id => resource[:id])
+        wrap_conditions = relation.name != role_class.name
+
+        conditions = if args[:resource].is_a?(Class)
+                       {:resource_type => args[:resource].to_s, :resource_id => nil }
+                     elsif args[:resource].present?
+                       {:resource_type => args[:resource].class.name, :resource_id => args[:resource].id}
+                     else
+                       {}
+                     end
+
+        conditions.merge!(:name => args[:name])
+        conditions = wrap_conditions ? { role_table => conditions } : conditions
+
+        relation.where(conditions)
       end
 
       def find_cached(relation, args)
@@ -84,8 +90,9 @@ def exists?(relation, column)
         relation.where(column.to_sym.ne => nil)
       end
 
-      def scope(relation, conditions)
-        roles = where(role_class, conditions).map { |role| role.id }
+      def scope(relation, conditions, strict)
+        query = strict ? where_strict(role_class, conditions) : where(role_class, conditions)
+        roles = query.map { |role| role.id }
         return [] if roles.size.zero?
         query = relation.any_in(:role_ids => roles)
         query

lib/rolify/finders.rb

@@ -1,7 +1,12 @@
 module Rolify
   module Finders
     def with_role(role_name, resource = nil)
-      self.adapter.scope(self, :name => role_name, :resource => resource)
+      strict = self.strict_rolify and resource and resource != :any
+      self.adapter.scope(
+        self,
+        { :name => role_name, :resource => resource },
+        strict
+      )
     end
 
     def without_role(role_name, resource = nil)

spec/rolify/shared_examples/shared_examples_for_finders.rb

@@ -4,47 +4,65 @@
       it { should respond_to(:with_role).with(1).argument }
       it { should respond_to(:with_role).with(2).arguments }
 
-      context "with a global role" do
-        it { subject.with_role("admin".send(param_method)).should eq([ root ]) }
-        it { subject.with_role("moderator".send(param_method)).should be_empty }
-        it { subject.with_role("visitor".send(param_method)).should be_empty }
-      end
-
-      context "with a class scoped role" do
-        context "on Forum class" do
-          it { subject.with_role("admin".send(param_method), Forum).should eq([ root ]) }
-          it { subject.with_role("moderator".send(param_method), Forum).should eq([ modo ]) }
-          it { subject.with_role("visitor".send(param_method), Forum).should be_empty }
+      context "when resource setting: strict is set to false" do
+        context "with a global role" do
+          it { subject.with_role("admin".send(param_method)).should eq([ root ]) }
+          it { subject.with_role("moderator".send(param_method)).should be_empty }
+          it { subject.with_role("visitor".send(param_method)).should be_empty }
         end
 
-        context "on Group class" do
-          it { subject.with_role("admin".send(param_method), Group).should eq([ root ]) }
-          it { subject.with_role("moderator".send(param_method), Group).should eq([ root ]) }
-          it { subject.with_role("visitor".send(param_method), Group).should be_empty }
+        context "with a class scoped role" do
+          context "on Forum class" do
+            it { subject.with_role("admin".send(param_method), Forum).should eq([ root ]) }
+            it { subject.with_role("moderator".send(param_method), Forum).should eq([ modo ]) }
+            it { subject.with_role("visitor".send(param_method), Forum).should be_empty }
+          end
+
+          context "on Group class" do
+            it { subject.with_role("admin".send(param_method), Group).should eq([ root ]) }
+            it { subject.with_role("moderator".send(param_method), Group).should eq([ root ]) }
+            it { subject.with_role("visitor".send(param_method), Group).should be_empty }
+          end
         end
-      end
 
-      context "with an instance scoped role" do
-        context "on Forum.first instance" do
-          it { subject.with_role("admin".send(param_method), Forum.first).should eq([ root ]) }
-          it { subject.with_role("moderator".send(param_method), Forum.first).should eq([ modo ]) }
-          it { subject.with_role("visitor".send(param_method), Forum.first).should be_empty }
+        context "with an instance scoped role" do
+          context "on Forum.first instance" do
+            it { subject.with_role("admin".send(param_method), Forum.first).should eq([ root ]) }
+            it { subject.with_role("moderator".send(param_method), Forum.first).should eq([ modo ]) }
+            it { subject.with_role("visitor".send(param_method), Forum.first).should be_empty }
+          end
+
+          context "on Forum.last instance" do
+            it { subject.with_role("admin".send(param_method), Forum.last).should eq([ root ]) }
+            it { subject.with_role("moderator".send(param_method), Forum.last).should eq([ modo ]) }
+            it { subject.with_role("visitor".send(param_method), Forum.last).should include(root, visitor) } # =~ doesn't pass using mongoid, don't know why...
+          end
+
+          context "on Group.first instance" do
+            it { subject.with_role("admin".send(param_method), Group.first).should eq([ root ]) }
+            it { subject.with_role("moderator".send(param_method), Group.first).should eq([ root ]) }
+            it { subject.with_role("visitor".send(param_method), Group.first).should eq([ modo ]) }
+          end
+
+          context "on Company.first_instance" do
+            it { subject.with_role("owner".send(param_method), Company.first).should eq([ owner ]) }
+          end
         end
+      end
 
-        context "on Forum.last instance" do
-          it { subject.with_role("admin".send(param_method), Forum.last).should eq([ root ]) }
-          it { subject.with_role("moderator".send(param_method), Forum.last).should eq([ modo ]) }
-          it { subject.with_role("visitor".send(param_method), Forum.last).should include(root, visitor) } # =~ doesn't pass using mongoid, don't know why...
+      context "when resource setting: strict is set to true" do
+        before(:context) do
+          user_class.strict_rolify = true
         end
-
-        context "on Group.first instance" do
-          it { subject.with_role("admin".send(param_method), Group.first).should eq([ root ]) }
-          it { subject.with_role("moderator".send(param_method), Group.first).should eq([ root ]) }
-          it { subject.with_role("visitor".send(param_method), Group.first).should eq([ modo ]) }
+        after(:context) do
+          user_class.strict_rolify = false
         end
 
-        context "on Company.first_instance" do
-          it { subject.with_role("owner".send(param_method), Company.first).should eq([ owner ]) }
+        context "with an instance scoped role" do
+          context "on Forum.first instance" do
+            it { subject.with_role("admin".send(param_method), Forum.first).should be_empty }
+            it { subject.with_role("moderator".send(param_method), Forum.first).should be_empty }
+          end
         end
       end
     end