Merge pull request #12 from appfolio/tcRecursiveRoles

Add support for recursive inclusion of roles

Appraisals

@@ -12,16 +12,19 @@ when '2.3.3' then
 
   appraise 'rails507' do
     gem 'rails', '5.0.7'
+    gem 'grape', '1.1.0'
     gem 'rails-controller-testing'
   end
 
   appraise 'rails516' do
     gem 'rails', '5.1.6'
+    gem 'grape', '1.1.0'
     gem 'rails-controller-testing'
   end
 
   appraise 'rails521' do
     gem 'rails', '5.2.1'
+    gem 'grape', '1.1.0'
     gem 'rails-controller-testing'
   end
 

gemfiles/rails507.gemfile

@@ -7,6 +7,6 @@ gem "mocha", "~> 1.0", require: false
 gem "sqlite3"
 gem "rails", "5.0.7"
 gem "rails-controller-testing"
-gem 'grape', "~> 1.1"
+gem 'grape', "1.1.0"
 
 gemspec path: "../"

gemfiles/rails516.gemfile

@@ -7,6 +7,6 @@ gem "mocha", "~> 1.0", require: false
 gem "sqlite3"
 gem "rails", "5.1.6"
 gem "rails-controller-testing"
-gem 'grape', "~> 1.1"
+gem 'grape', "1.1.0"
 
 gemspec path: "../"

gemfiles/rails521.gemfile

@@ -7,6 +7,6 @@ gem "mocha", "~> 1.0", require: false
 gem "sqlite3"
 gem "rails", "5.2.1"
 gem "rails-controller-testing"
-gem 'grape', "~> 1.1"
+gem 'grape', "1.1.0"
 
 gemspec path: "../"

lib/declarative_authorization/authorization.rb

@@ -345,17 +345,16 @@ def user_roles_privleges_from_options(privilege, options)
       [user, roles, privileges]
     end
 
-    def flatten_roles(roles)
+    def flatten_roles(roles, ignore: [])
       # TODO: caching?
       hierarchy = role_hierarchy
       flattened_roles = {}
       roles.each do |role|
         flattened_roles[role] = true
-        if (hierarchy_for_role = hierarchy[role])
-          hierarchy_for_role.each do |r|
-            flattened_roles[r] = true
-          end
-        end
+
+        children = (hierarchy[role] || []) - ignore
+        ignore += flattened_roles.keys
+        flattened_roles.merge!(flatten_roles(children, ignore: ignore)) if children.any?
       end
       flattened_roles
     end

test/authorization_test.rb

@@ -387,11 +387,11 @@ def test_role_hierarchy
       end
     }
     engine = Authorization::Engine.new(reader)
-    assert engine.permit?(:lower, :context => :permissions,
-      :user => MockUser.new(:test_role))
+    assert engine.permit?(:test, context: :permissions, user: MockUser.new(:test_role))
+    assert engine.permit?(:lower, context: :permissions, user: MockUser.new(:test_role))
   end
 
-  def test_role_hierarchy_infinity
+  def test_role_hierarchy__recursive
     reader = Authorization::Reader::DSLReader.new
     reader.parse %{
       authorization do
@@ -400,14 +400,61 @@ def test_role_hierarchy_infinity
           has_permission_on :permissions, :to => :test
         end
         role :lower_role do
-          includes :higher_role
           has_permission_on :permissions, :to => :lower
+          includes :lowest_role
+        end
+        role :lowest_role do
+          has_permission_on :permissions, :to => :lowest
         end
       end
     }
     engine = Authorization::Engine.new(reader)
-    assert engine.permit?(:lower, :context => :permissions,
-      :user => MockUser.new(:test_role))
+    assert engine.permit?(:test, context: :permissions, user: MockUser.new(:test_role))
+    assert engine.permit?(:lower, context: :permissions, user: MockUser.new(:test_role))
+    assert engine.permit?(:lowest, context: :permissions, user: MockUser.new(:test_role))
+  end
+
+  def test_role_hierarchy__circular
+    reader = Authorization::Reader::DSLReader.new
+    reader.parse %{
+      authorization do
+        role :test_role do
+          includes :lower_role
+          has_permission_on :permissions, :to => :test
+        end
+        role :lower_role do
+          includes :test_role
+          has_permission_on :permissions, :to => :lower
+        end
+      end
+    }
+    engine = Authorization::Engine.new(reader)
+    assert engine.permit?(:test, context: :permissions, user: MockUser.new(:test_role))
+    assert engine.permit?(:lower, context: :permissions, user: MockUser.new(:test_role))
+  end
+
+  def test_role_hierarchy__recursive__circular
+    reader = Authorization::Reader::DSLReader.new
+    reader.parse %{
+      authorization do
+        role :test_role do
+          includes :lower_role
+          has_permission_on :permissions, :to => :test
+        end
+        role :lower_role do
+          includes :lowest_role
+          has_permission_on :permissions, :to => :lower
+        end
+        role :lowest_role do
+          includes :test_role
+          has_permission_on :permissions, :to => :lowest
+        end
+      end
+    }
+    engine = Authorization::Engine.new(reader)
+    assert engine.permit?(:test, context: :permissions, user: MockUser.new(:test_role))
+    assert engine.permit?(:lower, context: :permissions, user: MockUser.new(:test_role))
+    assert engine.permit?(:lowest, context: :permissions, user: MockUser.new(:test_role))
   end
 
   def test_privilege_hierarchy
@@ -425,8 +472,29 @@ def test_privilege_hierarchy
       end
     }
     engine = Authorization::Engine.new(reader)
-    assert engine.permit?(:lower, :context => :permissions,
-      :user => MockUser.new(:test_role))
+    assert engine.permit?(:lower, context: :permissions, user: MockUser.new(:test_role))
+  end
+
+  def test_privilege_hierarchy__recursive
+    reader = Authorization::Reader::DSLReader.new
+    reader.parse %{
+      privileges do
+        privilege :test, :permissions do
+          includes :lower
+        end
+        privilege :lower, :permissions do
+          includes :lowest
+        end
+      end
+      authorization do
+        role :test_role do
+          has_permission_on :permissions, :to => :test
+        end
+      end
+    }
+    engine = Authorization::Engine.new(reader)
+    assert engine.permit?(:lower, context: :permissions, user: MockUser.new(:test_role))
+    assert engine.permit?(:lowest, context: :permissions, user: MockUser.new(:test_role))
   end
 
   def test_privilege_hierarchy_without_context

test/model_test.rb

@@ -1,5 +1,5 @@
 require 'test_helper'
-require File.join(File.dirname(__FILE__), %w{.. lib declarative_authorization in_model})
+require File.expand_path(File.join(File.dirname(__FILE__), %w{.. lib declarative_authorization in_model}))
 
 ActiveRecord::Base.send :include, Authorization::AuthorizationInModel
 #ActiveRecord::Base.logger = Logger.new(STDOUT)