From 4807ccc42696df9a2d681985c29ecb8e8cb68e57 Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Wed, 19 Dec 2018 12:44:40 +1300 Subject: [PATCH 1/2] Add support for recursive inclusion of roles --- .../authorization.rb | 11 ++- test/authorization_test.rb | 84 +++++++++++++++++-- 2 files changed, 81 insertions(+), 14 deletions(-) diff --git a/lib/declarative_authorization/authorization.rb b/lib/declarative_authorization/authorization.rb index ac84564..709d5cf 100644 --- a/lib/declarative_authorization/authorization.rb +++ b/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 diff --git a/test/authorization_test.rb b/test/authorization_test.rb index ab9999f..a861a62 100644 --- a/test/authorization_test.rb +++ b/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 From 0fe95350e551326567555b79b35b295adba79df4 Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Wed, 19 Dec 2018 13:40:37 +1300 Subject: [PATCH 2/2] Appraise against grape 1.1.0 --- Appraisals | 3 +++ gemfiles/rails507.gemfile | 2 +- gemfiles/rails516.gemfile | 2 +- gemfiles/rails521.gemfile | 2 +- test/model_test.rb | 2 +- 5 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Appraisals b/Appraisals index bd1f04c..8766cb2 100644 --- a/Appraisals +++ b/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 diff --git a/gemfiles/rails507.gemfile b/gemfiles/rails507.gemfile index ae0bc8a..a684b25 100644 --- a/gemfiles/rails507.gemfile +++ b/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: "../" diff --git a/gemfiles/rails516.gemfile b/gemfiles/rails516.gemfile index 288367c..aea7c37 100644 --- a/gemfiles/rails516.gemfile +++ b/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: "../" diff --git a/gemfiles/rails521.gemfile b/gemfiles/rails521.gemfile index f676cd2..b9e9853 100644 --- a/gemfiles/rails521.gemfile +++ b/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: "../" diff --git a/test/model_test.rb b/test/model_test.rb index 0d6d227..22e11e8 100644 --- a/test/model_test.rb +++ b/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)