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

enforce permissions for new course roles 

Updates the role permission checking to use base role types
(Account type and enrollment types) when calculating
permissions

Test Plan:
 * Test all the permissions
 * Test again
 * Create custom roles for accounts and each enrollment type
 * Change permissions and verify they work correctly
 * Delete roles and verify they still enforce permissions
 * Create roles in parent accounts and make changes in sub accounts
 * Verify those permissions too

closes #11740

Change-Id: Icce22f961065073b27252bb9e2f238de1e59e33b
Reviewed-on: https://gerrit.instructure.com/15735
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
This commit is contained in:
Bracken Mosbacker 2012-12-03 09:05:38 -07:00
parent fbdd2d6551
commit 481067601e
9 changed files with 195 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
app/models/account_user.rb
View File

@ -82,7 +82,11 @@ class AccountUser < ActiveRecord::Base
def has_permission_to?(action)
@permission_lookup ||= {}
@permission_lookup[action] ||= RoleOverride.permission_for(self, action, self.membership_type)[:enabled]
@permission_lookup[action] ||= RoleOverride.permission_for(self, action, base_role_name, self.membership_type)[:enabled]
end
def base_role_name
BASE_ROLE_NAME
end
def self.readable_type(type)

6
app/models/enrollment.rb
View File

@ -582,11 +582,15 @@ class Enrollment < ActiveRecord::Base
def has_permission_to?(action)
@permission_lookup ||= {}
unless @permission_lookup.has_key? action
@permission_lookup[action] = RoleOverride.permission_for(self, action, self.class.to_s)[:enabled]
@permission_lookup[action] = RoleOverride.permission_for(self, action, base_role_name, self.role_name)[:enabled]
end
@permission_lookup[action]
end
def base_role_name
self.class.to_s
end
# Determine if a user has permissions to conclude this enrollment.
#
# user - The user requesting permission to conclude/delete enrollment.

6
app/models/role.rb
View File

@ -23,7 +23,7 @@ class Role < ActiveRecord::Base
before_validation :infer_root_account_id
validates_presence_of :name
validates_inclusion_of :base_role_type, :in => RoleOverride::BASE_ROLE_TYPES
validates_exclusion_of :name, :in => RoleOverride::RESERVED_ROLES
validates_exclusion_of :name, :in => RoleOverride::KNOWN_ROLE_TYPES
validate :ensure_no_name_conflict_with_different_base_role_type
def infer_root_account_id
@ -35,7 +35,7 @@ class Role < ActiveRecord::Base
end
def ensure_no_name_conflict_with_different_base_role_type
unless self.root_account.all_roles.active.scoped(:conditions => ["name = ? AND base_role_type <> ?", self.name, self.base_role_type]).empty?
if self.root_account.all_roles.not_deleted.scoped(:conditions => ["name = ? AND base_role_type <> ?", self.name, self.base_role_type]).any?
self.errors.add(:name, 'is already taken by a different type of Role in the same root account')
end
end
@ -79,7 +79,7 @@ class Role < ActiveRecord::Base
elsif role = account.find_role(role_name)
[ role.base_role_type, role.workflow_state ]
else
[ 'NoPermissions', 'deleted' ]
[ RoleOverride::NO_PERMISSIONS_TYPE, 'deleted' ]
end
end
end

82
app/models/role_override.rb
View File

@ -24,9 +24,9 @@ class RoleOverride < ActiveRecord::Base
attr_accessible :context, :permission, :enrollment_type, :enabled
def self.account_membership_types(account)
res = [{:name => "AccountAdmin", :label => t('roles.account_admin', "Account Admin")}]
res = [{:name => "AccountAdmin", :base_role_name => AccountUser::BASE_ROLE_NAME, :label => t('roles.account_admin', "Account Admin")}]
(account.available_account_roles - ['AccountAdmin']).each do |t|
res << {:name => t, :label => t}
res << {:name => t, :base_role_name => AccountUser::BASE_ROLE_NAME, :label => t}
end
res
end
@ -34,11 +34,11 @@ class RoleOverride < ActiveRecord::Base
ENROLLMENT_TYPES =
[
# StudentViewEnrollment permissions will mirror StudentPermissions
{:name => 'StudentEnrollment', :label => lambda { t('roles.student', 'Student') } },
{:name => 'TaEnrollment', :label => lambda { t('roles.ta', 'TA') } },
{:name => 'TeacherEnrollment', :label => lambda { t('roles.teacher', 'Teacher') } },
{:name => 'DesignerEnrollment', :label => lambda { t('roles.designer', 'Course Designer') } },
{:name => 'ObserverEnrollment', :label => lambda { t('roles.observer', 'Observer') } }
{:base_role_name => 'StudentEnrollment', :name => 'StudentEnrollment', :label => lambda { t('roles.student', 'Student') } },
{:base_role_name => 'TaEnrollment', :name => 'TaEnrollment', :label => lambda { t('roles.ta', 'TA') } },
{:base_role_name => 'TeacherEnrollment', :name => 'TeacherEnrollment', :label => lambda { t('roles.teacher', 'Teacher') } },
{:base_role_name => 'DesignerEnrollment', :name => 'DesignerEnrollment', :label => lambda { t('roles.designer', 'Course Designer') } },
{:base_role_name => 'ObserverEnrollment', :name => 'ObserverEnrollment', :label => lambda { t('roles.observer', 'Observer') } }
].freeze
def self.enrollment_types
@ -51,6 +51,8 @@ class RoleOverride < ActiveRecord::Base
BASE_ROLE_TYPES
end
NO_PERMISSIONS_TYPE = 'NoPermissions'
KNOWN_ROLE_TYPES =
[
'TeacherEnrollment',
@ -60,7 +62,9 @@ class RoleOverride < ActiveRecord::Base
'StudentViewEnrollment',
'ObserverEnrollment',
'TeacherlessStudentEnrollment',
'AccountAdmin'
'AccountAdmin',
'AccountMembership',
NO_PERMISSIONS_TYPE
].freeze
def self.known_role_types
KNOWN_ROLE_TYPES
@ -630,13 +634,6 @@ class RoleOverride < ActiveRecord::Base
}
})
RESERVED_ROLES =
[
'AccountAdmin', 'AccountMembership', 'DesignerEnrollment',
'ObserverEnrollment', 'StudentEnrollment', 'StudentViewEnrollment',
'TaEnrollment', 'TeacherEnrollment', 'TeacherlessStudentEnrollment'
].freeze
def self.permissions
Permissions.retrieve
end
@ -649,8 +646,8 @@ class RoleOverride < ActiveRecord::Base
permissions
end
def self.css_class_for(context, permission, enrollment_type)
generated_permission = self.permission_for(context, permission, enrollment_type)
def self.css_class_for(context, permission, base_role, custom_role=nil)
generated_permission = self.permission_for(context, permission, base_role, custom_role)
css = []
if generated_permission[:readonly]
@ -664,12 +661,12 @@ class RoleOverride < ActiveRecord::Base
css.join(' ')
end
def self.readonly_for(context, permission, enrollment_type)
self.permission_for(context, permission, enrollment_type)[:readonly]
def self.readonly_for(context, permission, base_role, custom_role=nil)
self.permission_for(context, permission, base_role, custom_role)[:readonly]
end
def self.title_for(context, permission, enrollment_type)
generated_permission = self.permission_for(context, permission, enrollment_type)
def self.title_for(context, permission, base_role, custom_role=nil)
generated_permission = self.permission_for(context, permission, base_role, custom_role)
if generated_permission[:readonly]
t 'tooltips.readonly', "you do not have permission to change this."
else
@ -677,12 +674,12 @@ class RoleOverride < ActiveRecord::Base
end
end
def self.locked_for(context, permission, enrollment_type=nil)
self.permission_for(context, permission, enrollment_type)[:locked]
def self.locked_for(context, permission, base_role, custom_role=nil)
self.permission_for(context, permission, base_role, custom_role)[:locked]
end
def self.hidden_value_for(context, permission, enrollment_type=nil)
generated_permission = self.permission_for(context, permission, enrollment_type)
def self.hidden_value_for(context, permission, base_role, custom_role=nil)
generated_permission = self.permission_for(context, permission, base_role, custom_role)
if !generated_permission[:readonly] && generated_permission[:explicit]
generated_permission[:enabled] ? 'checked' : 'unchecked'
else
@ -699,24 +696,35 @@ class RoleOverride < ActiveRecord::Base
@cached_permissions = {}
end
def self.permission_for(context, permission, enrollment_type=nil)
enrollment_type = 'StudentEnrollment' if enrollment_type == 'StudentViewEnrollment'
def self.permission_for(context, permission, base_role, custom_role=nil)
base_role = 'StudentEnrollment' if base_role == 'StudentViewEnrollment'
custom_role = nil if base_role == NO_PERMISSIONS_TYPE
if custom_role && custom_role == 'AccountAdmin'
raise ArgumentError.new("Can't have AccountAdmin with base_role #{base_role}") unless base_role == AccountUser::BASE_ROLE_NAME
# An AccountAdmin is the default account user and uses a different base
# permission set. So set its base_role to AccountAdmin instead of AccountMembership
base_role = 'AccountAdmin'
end
custom_role ||= base_role
@cached_permissions ||= {}
key = [context.cache_key, permission.to_s, enrollment_type.to_s].join
permissionless_key = [context.cache_key, enrollment_type.to_s].join
key = [context.cache_key, permission.to_s, custom_role.to_s].join
permissionless_key = [context.cache_key, custom_role.to_s].join
return @cached_permissions[key] if @cached_permissions[key]
fallback_enrollment_type = enrollment_type
fallback_enrollment_type = 'AccountMembership' if !self.known_role_types.include?(enrollment_type)
if !self.known_role_types.include?(base_role)
raise ArgumentError.new("Invalid base_role #{base_role}")
end
generated_permission = {
:permission => self.permissions[permission],
:enabled => self.permissions[permission][:true_for].include?(fallback_enrollment_type),
:locked => !self.permissions[permission][:available_to].include?(fallback_enrollment_type),
:readonly => !self.permissions[permission][:available_to].include?(fallback_enrollment_type),
:enabled => self.permissions[permission][:true_for].include?(base_role),
:locked => !self.permissions[permission][:available_to].include?(base_role),
:readonly => !self.permissions[permission][:available_to].include?(base_role),
:explicit => false,
:enrollment_type => enrollment_type
:base_role_type => base_role,
:enrollment_type => custom_role
}
@@role_override_chain ||= {}
overrides = @@role_override_chain[permissionless_key]
unless overrides
@ -738,7 +746,7 @@ class RoleOverride < ActiveRecord::Base
account_ids.each_with_index{|account_id, idx| case_string += " WHEN context_id='#{account_id}' THEN #{idx} " }
overrides = RoleOverride.find(:all, :conditions => {:context_id => account_ids, :enrollment_type => generated_permission[:enrollment_type].to_s}, :order => (case_string.empty? ? nil : "CASE #{case_string} ELSE 9999 END DESC"))
end
@@role_override_chain[permissionless_key] = overrides
overrides.each do |override|
if override.permission == permission.to_s

14
app/views/role_overrides/_permission.html.erb
View File

@ -4,21 +4,21 @@
<% @role_types.each do |enrollment_type| %>
<td>
<%= image_tag "locked_small.png", :class => "lock #{
RoleOverride.readonly_for(@context, key, enrollment_type[:name]) ?
RoleOverride.readonly_for(@context, key, enrollment_type[:base_role_name], enrollment_type[:name]) ?
'disabled' : (
RoleOverride.locked_for(@context, key, enrollment_type[:name]) ?
RoleOverride.locked_for(@context, key, enrollment_type[:base_role_name], enrollment_type[:name]) ?
'locked' :
''
)}"
%>
<% unless RoleOverride.readonly_for(@context, key, enrollment_type[:name]) %>
<% unless RoleOverride.readonly_for(@context, key, enrollment_type[:base_role_name], enrollment_type[:name]) %>
<input class="locked_input" type="hidden" name="<%= "permissions[#{key}][#{enrollment_type[:name]}][locked]" %>"
value="<%= RoleOverride.locked_for(@context, key, enrollment_type[:name]) %>" />
value="<%= RoleOverride.locked_for(@context, key, enrollment_type[:base_role_name], enrollment_type[:name]) %>" />
<% end %>
<span class="six-checkbox <%= RoleOverride.css_class_for(@context, key, enrollment_type[:name]) %>"
title="<%= RoleOverride.title_for(@context, key, enrollment_type[:name]) %>">
<span class="six-checkbox <%= RoleOverride.css_class_for(@context, key, enrollment_type[:base_role_name], enrollment_type[:name]) %>"
title="<%= RoleOverride.title_for(@context, key, enrollment_type[:base_role_name], enrollment_type[:name]) %>">
<input type="hidden" name="<%= "permissions[#{key}][#{enrollment_type[:name]}][override]" %>"
value="<%= RoleOverride.hidden_value_for(@context, key, enrollment_type[:name]) %>" />
value="<%= RoleOverride.hidden_value_for(@context, key, enrollment_type[:base_role_name], enrollment_type[:name]) %>" />
</span>
</td>
<% end %>

2
lib/api/v1/role.rb
View File

@ -31,7 +31,7 @@ module Api::V1::Role
}
RoleOverride.manageable_permissions(account).keys.each do |permission|
json[:permissions][permission] = permission_json(RoleOverride.permission_for(account, permission, role), current_user, session)
json[:permissions][permission] = permission_json(RoleOverride.permission_for(account, permission, base_role_type, role), current_user, session)
end
json

2
lib/data_fixup/move_account_membership_types_to_roles.rb
View File

@ -41,7 +41,7 @@ module DataFixup::MoveAccountMembershipTypesToRoles
# Step 2.
# then look for the role overrides that are referencing to a (presumably) deleted membership type
# and make 'inactive' roles for each of them, if they don't exist already
RoleOverride.all(:conditions => ['context_type = ? AND enrollment_type NOT IN (?)', 'Account', RoleOverride::RESERVED_ROLES],
RoleOverride.all(:conditions => ['context_type = ? AND enrollment_type NOT IN (?)', 'Account', RoleOverride::KNOWN_ROLE_TYPES],
:group => 'context_id, enrollment_type',
:select => 'context_id, enrollment_type').each_slice(500) do |role_overrides|
roles = Role.all(:conditions => {:account_id => role_overrides.collect(&:context_id).uniq}, :select => "account_id, name")

125
spec/models/role_override_spec.rb
View File

@ -194,4 +194,129 @@ describe RoleOverride do
a.grants_right?(@user, :manage_user_notes).should be_false
end
end
describe "#permissions_for" do
before :each do
@account = account_model(:parent_account => Account.default)
@role_name = 'NewRole'
@permission = :view_group_pages
end
def check_permission(base_role, role, enabled)
hash = RoleOverride.permission_for(@account, @permission, base_role.to_s, role.to_s)
hash[:enabled].should == enabled
end
def create_role(base_role, role_name)
@role = @account.roles.build(:name => role_name.to_s)
@role.base_role_type = base_role.to_s
@role.workflow_state = 'active'
@role.save!
end
def create_override(role_name, enabled)
RoleOverride.create!(:context => @account, :permission => @permission.to_s,
:enrollment_type => role_name.to_s, :enabled => enabled)
end
it "should error with unknown base role" do
expect{RoleOverride.permission_for(@account, @permission, "DodoBird")}.to raise_error
end
it "should give no permissions if basetype is no permissions regardless of role" do
check_permission(RoleOverride::NO_PERMISSIONS_TYPE, 'TeacherEnrollment', false)
end
context "admin roles" do
it "should special case AccountAdmin role to use AccountAdmin as base role" do
# the default base role type has no permissions, so this tests it is getting
# them from the AccountAdmin type.
check_permission(AccountUser::BASE_ROLE_NAME, 'AccountAdmin', true)
end
it "should reject AccountAdmin role with wrong base role" do
expect{RoleOverride.permission_for(@account, @permission, "DodoBird", "AccountAdmin")}.to raise_error
end
it "should use role override for role" do
create_role(AccountUser::BASE_ROLE_NAME, @role_name)
create_override(@role_name, true)
check_permission(AccountUser::BASE_ROLE_NAME, @role_name, true)
end
it "should fall back to base role permissions" do
create_role(AccountUser::BASE_ROLE_NAME, @role_name)
check_permission(AccountUser::BASE_ROLE_NAME, @role_name, false)
end
end
context "course roles" do
RoleOverride.enrollment_types.each do |base_role|
context "#{base_role[:name]} enrollments" do
before do
@base_role = base_role[:name]
@default_perm = RoleOverride.permissions[@permission][:true_for].include?(@base_role)
end
it "should use default permissions" do
create_role(@base_role, @role_name)
check_permission(@base_role, @role_name, @default_perm)
end
it "should use permission for role" do
create_role(@base_role, @role_name)
create_override(@role_name, !@default_perm)
check_permission(@base_role, @role_name, !@default_perm)
end
it "should not find override for base type of role" do
create_role(@base_role, @role_name)
create_override(@role_name, @default_perm)
create_override(@base_role, !@default_perm)
check_permission(@base_role, @role_name, @default_perm)
check_permission(@base_role, @base_role, !@default_perm)
end
it "should use permission for role in parent account" do
@sub = account_model(:parent_account => @account)
# create in parent
create_role(@base_role, @role_name)
# create in sub account
@role = @sub.roles.build(:name => @role_name.to_s)
@role.base_role_type = @base_role.to_s
@role.workflow_state = 'active'
@role.save!
#create permission in parent
create_override(@role_name, !@default_perm)
# check based on sub account
hash = RoleOverride.permission_for(@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
@sub = account_model(:parent_account => @account)
create_role(@base_role, @role_name)
#create permission in parent
create_override(@role_name, !@default_perm)
# check based on sub account
hash = RoleOverride.permission_for(@sub, @permission, @base_role.to_s, @role_name.to_s)
hash[:enabled].should == !@default_perm
end
end
end
end
end
end

6
spec/models/user_spec.rb
View File

@ -925,9 +925,9 @@ describe User do
context "permissions" do
it "should not allow account admin to modify admin privileges of other account admins" do
RoleOverride.readonly_for(Account.default, :manage_role_overrides, 'AccountAdmin').should be_true
RoleOverride.readonly_for(Account.default, :manage_account_memberships, 'AccountAdmin').should be_true
RoleOverride.readonly_for(Account.default, :manage_account_settings, 'AccountAdmin').should be_true
RoleOverride.readonly_for(Account.default, :manage_role_overrides, AccountUser::BASE_ROLE_NAME, 'AccountAdmin').should be_true
RoleOverride.readonly_for(Account.default, :manage_account_memberships, AccountUser::BASE_ROLE_NAME, 'AccountAdmin').should be_true
RoleOverride.readonly_for(Account.default, :manage_account_settings, AccountUser::BASE_ROLE_NAME, 'AccountAdmin').should be_true
end
end