-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
COMP-6957 Add roleScope methods and indexQueryBuilder #15
Changes from 5 commits
50e2d48
49a3233
55f1c70
aadf363
2df3f3d
2040ed2
0c55d25
a30e827
ffe51dd
fc842da
7fbd30d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
require 'forwardable' | ||
require 'role_scope' | ||
require 'rubygems' | ||
|
||
module IndexRoleDSL | ||
|
||
def self.included(base) | ||
base.before_filter(:check_where_or) if base.respond_to? :before_filter | ||
base.extend(ClassMethods) | ||
base.send(:include, InstanceMethods) | ||
end | ||
|
||
module InstanceMethods | ||
extend Forwardable | ||
|
||
def self.included(base) | ||
unless base.method_defined?(:current_user_roles) | ||
base.send(:define_method, :current_user_roles) do | ||
raise NotImplementedError | ||
end | ||
end | ||
end | ||
|
||
def_delegators 'self.class', :role_scopes | ||
def_delegators :role_scopes, :public? | ||
|
||
def apply_scopes | ||
return query if role_scopes.all_access?(current_user_roles) | ||
return query.none if scope_hash.empty? | ||
return scope_hash.inject(query) { |query, (scope_name, ids)| query.or(query.public_send(scope_name, ids)) } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In regards to my comments about rails/rails#24055. The main issue is that |
||
end | ||
|
||
def scope_hash | ||
@scope_hash ||= role_scopes.scope_hash(current_user_roles) | ||
end | ||
|
||
def allowed_roles(scope_name) | ||
role_scopes.allowed_roles(current_user_roles, scope_name) | ||
end | ||
|
||
protected def check_rails | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like this should be done on start in an initializer. |
||
rails_gem = Gem::Specification.select {|z| z.name == "rails"}.max_by {|a| a.version} | ||
if Gem::Version.new(rails_gem.version.version) < Gem::Version.new('5.0.0') | ||
true | ||
else | ||
false | ||
end | ||
end | ||
|
||
protected def check_where_or | ||
whereor_gem = Gem::Specification.select {|z| z.name == "where-or"} | ||
unless whereor_gem && check_rails | ||
rescue_error status: 500, message: 'It appears you are using Rails version 4.X.X or lower, please install the "where-or" gem or upgrade to rails 5.X.X' | ||
end | ||
end | ||
end | ||
|
||
module ClassMethods | ||
extend Forwardable | ||
|
||
def inherited(subclass) | ||
super | ||
subclass.instance_variable_set('@role_scopes', role_scopes.dup) | ||
end | ||
|
||
def_delegators :role_scopes, :all_public, :allow, :allowed_roles, :on | ||
|
||
def role_scopes | ||
@role_scopes ||= RoleScopes.new | ||
end | ||
end | ||
|
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
class ResourceIndexQueryBuilder | ||
include IndexRoleDSL | ||
|
||
attr_reader :query, :user | ||
|
||
allow(:third_north).to_all | ||
allow(:org_admin).on(:organization).to_access(:org_id) | ||
allow(:tournament_director).on(:organization).to_access(:org_id) | ||
|
||
def initialize(query, user) | ||
@query = query | ||
@user = user | ||
end | ||
|
||
private def current_user_roles | ||
user.role_assignments | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
require 'set' | ||
|
||
class RoleScope | ||
attr_reader :roles, :actions | ||
def initialize(roles, actions, resource = nil) | ||
self.roles = Set.new Array(roles).map(&:to_s) | ||
self.actions = Set.new Array(actions).map(&:to_s) | ||
self.resource = resource | ||
self.all_actions = false | ||
end | ||
|
||
def initialize_copy(other) | ||
@roles = @roles.dup | ||
@actions = @actions.dup | ||
end | ||
|
||
# on(resource).allow(*roles).to_access(*actions) | ||
def allow(*roles) | ||
self.roles = self.roles.merge(roles.flatten.compact.map(&:to_s)) | ||
self | ||
end | ||
|
||
# allow(*roles).on(resource).to_access(*actions) | ||
def on(resource) | ||
self.resource = resource | ||
self | ||
end | ||
|
||
# allow(*roles).to_access *actions | ||
def to_access(*actions) | ||
self.actions = self.actions.merge actions.flatten.compact.map(&:to_s) | ||
end | ||
|
||
# allow role access to all actions | ||
# allow(*roles).to_all | ||
def to_all | ||
self.all_actions = true | ||
end | ||
|
||
def allow?(current_roles, action) | ||
action?(action) && role?(current_roles) | ||
end | ||
|
||
def allowed_roles(current_roles, action) | ||
return [] unless action?(action) | ||
match_roles(current_roles) | ||
end | ||
|
||
def role?(check_roles) | ||
Array(check_roles).any? { |check_role| allowed_role?(check_role) } | ||
end | ||
|
||
def action?(check_actions) | ||
check_actions = Set.new Array(check_actions).map(&:to_s) | ||
all_actions? || !(check_actions & actions).empty? | ||
end | ||
|
||
def all_actions? | ||
!!all_actions | ||
end | ||
|
||
private def allowed_resource?(check_role, required_resource) | ||
return false unless check_role.respond_to?(:resource?) | ||
|
||
required_resources = type_id_resource?(required_resource) ? [required_resource] : Array(required_resource) | ||
required_resources.any? { |r| check_role.resource?(r) } | ||
end | ||
|
||
private def type_id_resource?(required_resource) | ||
required_resource.is_a?(Array) && %w(String Symbol).include?(required_resource.first.class.name) | ||
end | ||
|
||
protected | ||
attr_writer :roles | ||
attr_writer :actions | ||
attr_accessor :all_actions | ||
attr_accessor :resource | ||
|
||
private def match_roles(check_roles) | ||
Array(check_roles).select do |check_role| | ||
allowed_role?(check_role) | ||
end | ||
end | ||
|
||
private def allowed_role?(role_object) | ||
role_string = role_object.respond_to?(:to_role_string) ? role_object.to_role_string : role_object.to_s | ||
|
||
roles.include?(role_string.to_s) | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
require 'forwardable' | ||
require 'role_scope' | ||
|
||
class RoleScopes | ||
extend Forwardable | ||
include Enumerable | ||
|
||
def_delegators :build_role_scope, :allow, :on | ||
def_delegators :@role_scopes, :clear, :each, :empty? | ||
|
||
def initialize(role_scopes = []) | ||
@role_scopes = Array(role_scopes) | ||
end | ||
|
||
def initialize_copy(other) | ||
@role_scopes = @role_scopes.map(&:dup) | ||
end | ||
|
||
def allowed_roles(user_role_objects, action) | ||
return [] if empty? | ||
|
||
reduce([]) do |allowed_role_objects, role_scope| | ||
allowed_role_objects | role_scope.allowed_roles(user_role_objects, action) | ||
end | ||
end | ||
|
||
def scope_hash(user_role_objects) | ||
actions.each_with_object({}) do |action, memo| | ||
roles = allowed_roles(user_role_objects, action) | ||
ids = roles.map(&:resource_id).compact.uniq | ||
memo[action] = ids if ids.any? | ||
memo | ||
end | ||
end | ||
|
||
def all_access?(current_user_roles) | ||
any? { |role_scope| role_scope.allow?(current_user_roles, nil) } | ||
end | ||
|
||
def actions | ||
map(&:actions).each_with_object(Set.new) { |action_set, memo| memo.merge(action_set) } | ||
end | ||
|
||
private def build_role_scope(roles = nil, actions = nil, resource = nil) | ||
new_role_scope = RoleScope.new(roles, actions, resource) | ||
@role_scopes << new_role_scope | ||
new_role_scope | ||
end | ||
|
||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still going to run before every request. I don't think that is necessary or desired. Add it to the documentation if we can't add a check without affecting too much overhead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will only run before every
index
request. Is that still too much overhead?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still prefer it doesn't. Running for every index request isn't fool proof anyhow.