don't give account only permissions down the chain fixes #CNVS-1973

i.e. an AccountAdmin in a root account doesn't get site admin only
permissions, even on that root account

test plan:
 * create a custom role in site admin with all account and course
   permissions, but no site admin permissions
 * add a user to that role
 * add a user as an Account Admin in a regular root account
 * when logged in as (masquerading doesn't count) the site admin
   user, you should be able to masquerade as the regular root account
   admin

Change-Id: I72130a1c0751c36fdb1a7c2367dc7c5fa0daea11
Reviewed-on: https://gerrit.instructure.com/15865
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: Clare Hetherington <clare@instructure.com>

app/models/account_user.rb

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

app/models/enrollment.rb

@@ -597,7 +597,7 @@ 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, base_role_name, self.role_name)[:enabled]
+      @permission_lookup[action] = RoleOverride.permission_for(course, action, base_role_name, self.role_name)[:enabled]
     end
     @permission_lookup[action]
   end

app/models/role_override.rb

@@ -696,7 +696,7 @@ class RoleOverride < ActiveRecord::Base
     @cached_permissions = {}
   end
   
-  def self.permission_for(context, permission, base_role, custom_role=nil)
+  def self.permission_for(role_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'
@@ -708,28 +708,37 @@ class RoleOverride < ActiveRecord::Base
     custom_role ||= base_role
 
     @cached_permissions ||= {}
-    key = [context.cache_key, permission.to_s, custom_role.to_s].join
-    permissionless_key = [context.cache_key, custom_role.to_s].join
+    key = [role_context.cache_key, permission.to_s, custom_role.to_s].join
+    permissionless_key = [role_context.cache_key, custom_role.to_s].join
     return @cached_permissions[key] if @cached_permissions[key]
     
     if !self.known_role_types.include?(base_role)
       raise ArgumentError.new("Invalid base_role #{base_role}")
     end
+    default_data = self.permissions[permission]
     generated_permission = {
-      :permission =>  self.permissions[permission],
-      :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),
+      :permission =>  default_data,
+      :enabled    =>  default_data[:true_for].include?(base_role),
+      :locked     => !default_data[:available_to].include?(base_role),
+      :readonly   => !default_data[:available_to].include?(base_role),
       :explicit   => false,
       :base_role_type => base_role,
       :enrollment_type => custom_role
     }
+    if default_data[:account_only]
+      if role_context.is_a? Account
+        generated_permission[:enabled] = false if default_data[:account_only] == :root && !role_context.root_account?
+        generated_permission[:enabled] = false if default_data[:account_only] == :site_admin && !role_context.site_admin?
+      else
+        generated_permission[:enabled] = false
+      end
+    end
 
     @@role_override_chain ||= {}
     overrides = @@role_override_chain[permissionless_key]
     unless overrides
       account_ids = []
-      context_walk = context
+      context_walk = role_context
       while context_walk
         account_ids << context_walk.id if context_walk.is_a? Account
         if context_walk.respond_to?(:course)
@@ -750,7 +759,7 @@ class RoleOverride < ActiveRecord::Base
     @@role_override_chain[permissionless_key] = overrides
     overrides.each do |override|
       if override.permission == permission.to_s
-        generated_permission[:readonly] = true if override.locked && (override.context_id != context.id || !context.is_a?(Account))
+        generated_permission[:readonly] = true if override.locked && (override.context_id != role_context.id || !role_context.is_a?(Account))
         generated_permission.merge!({
           :readonly => generated_permission[:readonly] || generated_permission[:locked],
           :explicit => false
@@ -758,7 +767,7 @@ class RoleOverride < ActiveRecord::Base
 
         if !generated_permission[:locked]
           unless override.enabled.nil?
-            if override.context == context
+            if override.context == role_context
               # if the explicit override is for the target context, the prior default
               # is the parent context's value
               generated_permission[:prior_default] = generated_permission[:enabled]

spec/models/account_spec.rb

@@ -388,9 +388,11 @@ describe Account do
     end
 
     limited_access = [ :read, :manage, :update, :delete, :read_outcomes ]
-    full_access = RoleOverride.permissions.map { |k, v| k } + limited_access
+    full_access = RoleOverride.permissions.keys + limited_access
     index = full_access.index(:manage_courses)
     full_access = full_access[0..index] + [:create_courses] + full_access[index+1..-1]
+    full_root_access = full_access - RoleOverride.permissions.select { |k, v| v[:account_only] == :site_admin }.map(&:first)
+    full_sub_access = full_root_access - RoleOverride.permissions.select { |k, v| v[:account_only] == :root }.map(&:first)
     # site admin has access to everything everywhere
     hash.each do |k, v|
       account = v[:account]
@@ -405,7 +407,7 @@ describe Account do
     hash.each do |k, v|
       next if k == :site_admin
       account = v[:account]
-      account.check_policy(hash[:root][:admin]).should == full_access
+      account.check_policy(hash[:root][:admin]).should == full_root_access
       account.check_policy(hash[:root][:user]).should == limited_access
     end
 
@@ -419,7 +421,7 @@ describe Account do
     hash.each do |k, v|
       next if k == :site_admin || k == :root
       account = v[:account]
-      account.check_policy(hash[:sub][:admin]).should == full_access
+      account.check_policy(hash[:sub][:admin]).should == full_sub_access
       account.check_policy(hash[:sub][:user]).should == limited_access
     end
 
@@ -442,7 +444,7 @@ describe Account do
     hash.each do |k, v|
       next if k == :site_admin
       account = v[:account]
-      account.check_policy(hash[:root][:admin]).should == full_access
+      account.check_policy(hash[:root][:admin]).should == full_root_access
       account.check_policy(hash[:root][:user]).should == some_access
     end
 
@@ -456,7 +458,7 @@ describe Account do
     hash.each do |k, v|
       next if k == :site_admin || k == :root
       account = v[:account]
-      account.check_policy(hash[:sub][:admin]).should == full_access
+      account.check_policy(hash[:sub][:admin]).should == full_sub_access
       account.check_policy(hash[:sub][:user]).should == some_access
     end
   end

spec/models/role_override_spec.rb

@@ -320,7 +320,43 @@ describe RoleOverride do
       end
     end
 
+    context "account_only" do
+      before do
+        @site_admin = User.create!
+        Account.site_admin.add_user(@site_admin)
+        @root_admin = User.create!
+        Account.default.add_user(@root_admin)
+        @sub_admin = User.create!
+        @sub_account = Account.default.sub_accounts.create!
+        @sub_account.add_user(@sub_admin)
+      end
 
+      it "should not grant site admin permissions to normal account admins" do
+        Account.default.grants_right?(@root_admin, :manage_site_settings).should be_false
+        # check against the normal root account, but granted rights from Site Admin
+        Account.default.grants_right?(@site_admin, :manage_site_settings).should be_true
+        # check against Site Admin
+        Account.site_admin.grants_right?(@site_admin, :manage_site_settings).should be_true
+      end
+
+      it "should not grant root only permissions to sub account admins" do
+        Account.default.grants_right?(@root_admin, :become_user).should be_true
+        @sub_account.grants_right?(@sub_admin, :become_user).should be_false
+        # check against the sub account, but granted rights from the root account
+        @sub_account.grants_right?(@root_admin, :become_user).should be_true
+      end
+
+      it "should grant root only permissions in courses when the user is a root account admin" do
+        @course = @account.courses.create!
+        @course.grants_right?(@root_admin, :become_user).should be_true
+      end
+
+      it "should not grant account only permissions to malicious course users" do
+        @account = @account.courses.create!
+        @permission = :become_user
+        check_permission(AccountUser::BASE_ROLE_NAME, 'AccountAdmin', false)
+      end
+    end
 
   end
 end