ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix permission inheritance 

fixes CNVS-14451

test plan:
 * create a role in a root account
 * grant a permission to that role, and add an admin to that role
 * in a sub-account, revoke that permission from that role
 * ensure that the admin still has the permission in both the root
   account and a sub account (the sub account can't revoke permissions
   that admins got from further up the chain)
 * add an admin in that role to the sub account
 * ensure this new admin does not have the permission

 * and, sadly, permissions regression, cause we're messing with
   low level permissions stuff again

Change-Id: I7ccc32f36e4476d5df77eaec765abcb1c7b89b3e
Reviewed-on: https://gerrit.instructure.com/38604
Reviewed-by: Nick Cloward <ncloward@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
This commit is contained in:
Cody Cutrer 2014-07-31 21:28:10 -06:00
parent abea42ae86
commit 948a57a183
2 changed files with 33 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
app/models/role_override.rb
View File

@ -866,32 +866,44 @@ class RoleOverride < ActiveRecord::Base
@@role_override_chain ||= {}
overrides = @@role_override_chain[permissionless_key] ||= begin
role_context.shard.activate do
account_ids = context.account_chain_ids(include_site_admin: true)
case_string = ""
account_ids.each_with_index{|account_id, idx| case_string += " WHEN context_id='#{account_id}' THEN #{idx} " }
overrides = RoleOverride.where(:context_id => account_ids, :enrollment_type => generated_permission[:enrollment_type].to_s).order("CASE #{case_string} ELSE 9999 END DESC")
overrides.group_by(&:permission).freeze
account_ids = context.account_chain_ids(include_site_admin: true).reverse
overrides = RoleOverride.where(:context_id => account_ids, :enrollment_type => generated_permission[:enrollment_type].to_s).group_by(&:permission)
# every context has to be represented so that we can't miss role_context below
overrides.each_key do |permission|
overrides_by_account = overrides[permission].index_by { |override| [override.context_id, override.context.class.base_class.name] }
overrides[permission] = account_ids.map do |account_id|
overrides_by_account[[account_id, 'Account']] || RoleOverride.new { |ro| ro.context_id = account_id; ro.context_type = 'Account' }
end
end
overrides
end
end
# walk the overrides from most general (root account) to most specific (the role's account)
# walk the overrides from most general (site admin, root account) to most specific (the role's account)
# and apply them; short-circuit once someone has locked it
last_override = false
hit_role_context = false
(overrides[permission.to_s] || []).each do |override|
# set the flag that we have an override for the context we're on
last_override = override.context_id == context.id && override.context_type == context.class.base_class.name
generated_permission[:context_id] = override.context_id
generated_permission[:context_id] = override.context_id unless override.new_record?
generated_permission[:locked] = override.locked?
# keep track of the value for the parent
generated_permission[:prior_default] = generated_permission[:enabled]
unless override.enabled.nil?
generated_permission[:explicit] = true if last_override
generated_permission[:enabled] = override.enabled? ? override.applies_to : nil
if hit_role_context
generated_permission[:enabled] ||= override.enabled? ? override.applies_to : nil
else
generated_permission[:enabled] = override.enabled? ? override.applies_to : nil
end
end
hit_role_context ||= override.context_id == role_context.id && override.context_type == role_context.class.base_class.name
break if override.locked?
break if generated_permission[:enabled] && hit_role_context
end
# there was not an override matching this context, so do a half loop

16
spec/models/role_override_spec.rb
View File

@ -349,6 +349,7 @@ describe RoleOverride do
it "should use permission for role in parent account" do
@parent_account = @account
@sub = account_model(:parent_account => @account)
@course = @sub.courses.create!
@account = @parent_account
# create in parent
@ -363,13 +364,14 @@ describe RoleOverride do
create_override(@role_name, !@default_perm)
# check based on sub account
hash = RoleOverride.permission_for(@parent_account, @sub, @permission, @base_role.to_s, @role_name.to_s)
hash = RoleOverride.permission_for(@course, @sub, @permission, @base_role.to_s, @role_name.to_s)
(!!hash[:enabled]).should == !@default_perm
end
it "should use permission for role in parent account even if sub account doesn't have role" do
@parent_account = @account
@sub = account_model(:parent_account => @account)
@course = @sub.courses.create!
@account = @parent_account
create_role(@base_role, @role_name)
@ -378,13 +380,14 @@ describe RoleOverride do
create_override(@role_name, !@default_perm)
# check based on sub account
hash = RoleOverride.permission_for(@parent_account, @sub, @permission, @base_role.to_s, @role_name.to_s)
hash = RoleOverride.permission_for(@course, @sub, @permission, @base_role.to_s, @role_name.to_s)
(!!hash[:enabled]).should == !@default_perm
end
it "should use permission for role in sub account" do
@parent_account = @account
@sub = account_model(:parent_account => @account)
@course = @sub.courses.create!
create_role(@base_role, @role_name)
@ -392,7 +395,7 @@ describe RoleOverride do
create_override(@role_name, !@default_perm)
# check based on sub account
hash = RoleOverride.permission_for(@parent_account, @sub, @permission, @base_role.to_s, @role_name.to_s)
hash = RoleOverride.permission_for(@course, @sub, @permission, @base_role.to_s, @role_name.to_s)
(!!hash[:enabled]).should == !@default_perm
end
end
@ -435,6 +438,13 @@ describe RoleOverride do
@permission = :become_user
check_permission(AccountUser::BASE_ROLE_NAME, 'AccountAdmin', false)
end
it "should not allow a sub-account to revoke a permission granted to a parent account" do
@sub_account.role_overrides.create!(enrollment_type: 'AccountAdmin', enabled: false, permission: :manage_admin_users)
@sub_account.grants_right?(@site_admin, :manage_admin_users).should be_true
@sub_account.grants_right?(@root_admin, :manage_admin_users).should be_true
@sub_account.grants_right?(@sub_admin, :manage_admin_users).should be_false
end
end
context "sharding" do