From c4e169f290c606da7fd8470a47609caece105e0e Mon Sep 17 00:00:00 2001
From: Jacob <realshin@gmail.com>
Date: Fri, 29 May 2020 12:01:38 -0400
Subject: [PATCH 1/4] 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
---
.../adapters/active_record/role_adapter.rb | 26 +++---
lib/rolify/adapters/mongoid/role_adapter.rb | 2 +-
lib/rolify/finders.rb | 7 +-
.../shared_examples_for_finders.rb | 82 +++++++++++--------
4 files changed, 73 insertions(+), 44 deletions(-)
diff --git a/lib/rolify/adapters/active_record/role_adapter.rb b/lib/rolify/adapters/active_record/role_adapter.rb
index 011a0411..f8dd00f0 100644
--- a/lib/rolify/adapters/active_record/role_adapter.rb
+++ b/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
+ processed_args = { :name => args[:name] }
+ conditions = wrap_conditions ? { role_table => processed_args } : processed_args
+ return relation.where(conditions) if args[:resource].blank?
+
+ if args[:resource].is_a?(Class)
+ processed_args[:resource_type] = args[:resource].to_s
+ processed_args[:resource_id] = nil
+ else
+ processed_args[:resource_type] = args[:resource].class.name
+ processed_args[:resource_id] = args[:resource].id
+ end
+
+ 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
diff --git a/lib/rolify/adapters/mongoid/role_adapter.rb b/lib/rolify/adapters/mongoid/role_adapter.rb
index 042a8b73..4bd3f3f8 100644
--- a/lib/rolify/adapters/mongoid/role_adapter.rb
+++ b/lib/rolify/adapters/mongoid/role_adapter.rb
@@ -84,7 +84,7 @@ def exists?(relation, column)
relation.where(column.to_sym.ne => nil)
end
- def scope(relation, conditions)
+ def scope(relation, conditions, strict)
roles = where(role_class, conditions).map { |role| role.id }
return [] if roles.size.zero?
query = relation.any_in(:role_ids => roles)
diff --git a/lib/rolify/finders.rb b/lib/rolify/finders.rb
index b17557d5..c7306ac4 100644
--- a/lib/rolify/finders.rb
+++ b/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)
diff --git a/spec/rolify/shared_examples/shared_examples_for_finders.rb b/spec/rolify/shared_examples/shared_examples_for_finders.rb
index 0c9194c7..216d12eb 100644
--- a/spec/rolify/shared_examples/shared_examples_for_finders.rb
+++ b/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
From 3d75c6543019b3cec9b219cbc6c4f3604f2471e7 Mon Sep 17 00:00:00 2001
From: Jacob <realshin@gmail.com>
Date: Fri, 29 May 2020 18:04:32 -0600
Subject: [PATCH 2/4] 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
---
lib/rolify/adapters/mongoid/role_adapter.rb | 22 +++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/lib/rolify/adapters/mongoid/role_adapter.rb b/lib/rolify/adapters/mongoid/role_adapter.rb
index 4bd3f3f8..f3542bf7 100644
--- a/lib/rolify/adapters/mongoid/role_adapter.rb
+++ b/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
+ processed_args = { :name => args[:name] }
+ conditions = wrap_conditions ? { role_table => processed_args } : processed_args
+ return relation.where(conditions) if args[:resource].blank?
+
+ if args[:resource].is_a?(Class)
+ processed_args[:resource_type] = args[:resource].to_s
+ processed_args[:resource_id] = nil
+ else
+ processed_args[:resource_type] = args[:resource].class.name
+ processed_args[:resource_id] = args[:resource].id
+ end
+
+ relation.where(conditions)
end
def find_cached(relation, args)
From 71dcbecf1e5f8c3344e49e9e1c8db28a5ce0a3ec Mon Sep 17 00:00:00 2001
From: Jacob <realshin@gmail.com>
Date: Fri, 29 May 2020 19:22:56 -0600
Subject: [PATCH 3/4] Make sure to call strict where
---
lib/rolify/adapters/mongoid/role_adapter.rb | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/rolify/adapters/mongoid/role_adapter.rb b/lib/rolify/adapters/mongoid/role_adapter.rb
index f3542bf7..42d2692c 100644
--- a/lib/rolify/adapters/mongoid/role_adapter.rb
+++ b/lib/rolify/adapters/mongoid/role_adapter.rb
@@ -91,7 +91,8 @@ def exists?(relation, column)
end
def scope(relation, conditions, strict)
- roles = where(role_class, conditions).map { |role| role.id }
+ 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
From 1988e8e8f1d1daa6aa97e4159ed75c714683872c Mon Sep 17 00:00:00 2001
From: Jacob <realshin@gmail.com>
Date: Mon, 15 Jun 2020 09:41:30 -0400
Subject: [PATCH 4/4] Refactor to reduce mutations
---
.../adapters/active_record/role_adapter.rb | 22 +++++++++----------
lib/rolify/adapters/mongoid/role_adapter.rb | 22 +++++++++----------
2 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/lib/rolify/adapters/active_record/role_adapter.rb b/lib/rolify/adapters/active_record/role_adapter.rb
index f8dd00f0..c0c5f280 100644
--- a/lib/rolify/adapters/active_record/role_adapter.rb
+++ b/lib/rolify/adapters/active_record/role_adapter.rb
@@ -10,17 +10,17 @@ def where(relation, *args)
def where_strict(relation, args)
wrap_conditions = relation.name != role_class.name
- processed_args = { :name => args[:name] }
- conditions = wrap_conditions ? { role_table => processed_args } : processed_args
- return relation.where(conditions) if args[:resource].blank?
-
- if args[:resource].is_a?(Class)
- processed_args[:resource_type] = args[:resource].to_s
- processed_args[:resource_id] = nil
- else
- processed_args[:resource_type] = args[:resource].class.name
- processed_args[:resource_id] = args[:resource].id
- end
+
+ conditions = if args[:resource].is_a?(Class)
+ {:resource_type => args[:resource].to_s, :resource_id => nil }
+ elsif arg[: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
diff --git a/lib/rolify/adapters/mongoid/role_adapter.rb b/lib/rolify/adapters/mongoid/role_adapter.rb
index 42d2692c..d8345f2c 100644
--- a/lib/rolify/adapters/mongoid/role_adapter.rb
+++ b/lib/rolify/adapters/mongoid/role_adapter.rb
@@ -10,17 +10,17 @@ def where(relation, *args)
def where_strict(relation, args)
wrap_conditions = relation.name != role_class.name
- processed_args = { :name => args[:name] }
- conditions = wrap_conditions ? { role_table => processed_args } : processed_args
- return relation.where(conditions) if args[:resource].blank?
-
- if args[:resource].is_a?(Class)
- processed_args[:resource_type] = args[:resource].to_s
- processed_args[:resource_id] = nil
- else
- processed_args[:resource_type] = args[:resource].class.name
- processed_args[:resource_id] = args[:resource].id
- end
+
+ conditions = if args[:resource].is_a?(Class)
+ {:resource_type => args[:resource].to_s, :resource_id => nil }
+ elsif arg[: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