expand masquerade capabilities to root account admins

closes #4332

 * Allow :become_user to be granted to any root account admin role,
   not just site admin roles.
 * Adjust policy on User objects to properly grant :become_user:
   * You can always become yourself (stop masquerading)
   * Site admins can become any user besides other site admins
   * Root account admins can only become users that are not account
     admins, and that belong to accounts that this root account admin
     has permissions to
 * Adjust masquerading code to check for :become_user on the user
   object itself, rather than checking just on the site admin account
   * This means we have to figure out the target user before checking
     permissions
   * Because the permission check already checks for becoming another
     site admin user, that special case was removed in the
     masquerading code
   * Special case the UI to not show the "become" link for the
     current user (i.e. you can't become yourself, and you can't
     become the user that you already are)

Change-Id: I69bc855b8ee24098b9a63b0b1c8d7edf2063b625
Reviewed-on: https://gerrit.instructure.com/4614
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Jon Jensen <jon@instructure.com>

Author: ccutrer

app/controllers/users_controller.rb

@@ -191,8 +191,8 @@ def index
   end
 
   def masquerade
-    if user_is_site_admin?(@real_current_user || @current_user, :become_user)
-      @user = User.find_by_id(params[:user_id])
+    @user = User.find_by_id(params[:user_id])
+    if (authorized_action(@user, @real_current_user || @current_user, :become_user))
       if request.post?
         if @user == @real_current_user
           session[:become_user_id] = nil
@@ -203,9 +203,6 @@ def masquerade
         session[:masquerade_return_to] = nil
         return return_to(return_url, request.referer)
       end
-      return
-    else
-      render_unauthorized_action
     end
   end
 

app/models/role_override.rb

@@ -477,7 +477,7 @@ def self.known_role_types
       },
       :become_user => {
         :label => lambda { t('permissions.become_user', "Become other users") },
-        :account_only => :site_admin,
+        :account_only => :root,
         :true_for => %w(AccountAdmin),
         :available_to => %w(AccountAdmin AccountMembership),
       },

app/models/user.rb

@@ -690,7 +690,7 @@ def courses_with_grades
 
   set_policy do
     given { |user| user == self }
-    set { can :rename and can :read and can :manage and can :manage_content and can :manage_files and can :manage_calendar }
+    set { can :rename and can :read and can :manage and can :manage_content and can :manage_files and can :manage_calendar and can :become_user }
 
     given {|user| self.courses.any?{|c| c.user_is_teacher?(user)}}
     set { can :rename and can :create_user_notes and can :read_user_notes}
@@ -718,7 +718,26 @@ def courses_with_grades
       )
     end
     set { can :manage_user_details and can :manage_logins and can :rename and can :view_statistics and can :create_user_notes and can :read_user_notes and can :delete_user_notes}
-    
+
+    given do |user|
+      user && ((
+        # or, if the user we are given can masquerade in *all* of this user's accounts
+        # (to prevent an account admin from masquerading and gaining access to another root account)
+        (self.associated_accounts.all?{|a| a.grants_right?(user, nil, :become_user) } && !self.associated_accounts.empty?)
+      ) && (
+        # account admins can't masquerade as other account admins
+        self.account_users.empty? || (
+          # unless they're a site admin
+          Account.site_admin.grants_right?(user, nil, :become_user) &&
+          # and not wanting to become a site admin
+          !Account.site_admin_user?(self)
+        )
+      ) || (
+        # only site admins can masquerade as users that don't belong to any account
+        self.associated_accounts.empty? && Account.site_admin.grants_right?(user, nil, :become_user)
+      ))
+    end
+    set { can :become_user }
   end
 
   def self.infer_id(obj)

app/views/role_overrides/index.html.erb

@@ -128,7 +128,7 @@
           <td><%= t 'headers.account_permissions', "Account Permissions" %></td>
           <td colspan='<%= @role_types.size %>'></td>
         </tr>
-        <%= render :partial => 'permission', :collection => permissions.select { |k, p| p[:account_only] == true } %>
+        <%= render :partial => 'permission', :collection => permissions.select { |k, p| p[:account_only] == true || p[:account_only] == :root && @context.root_account? } %>
         <tr class='ui-widget-header'>
           <td><%=  t 'headers.course_permissions',  "Course & Account Permissions" %></td>
           <td colspan='<%= @role_types.size %>'></td>

app/views/users/_name.html.erb

@@ -44,7 +44,7 @@
       <% if can_manage_admin_users || (!@enrollments.any?(&:admin?) && !@user.creation_sis_batch_id && can_do(@context, @current_user, :manage_students)) %>
         <td class="links" colspan="2" style="text-align: right; font-size: 0.8em; padding-top: 10px;">
           <a href="#" class="edit_user_link"><%= t('edit', 'Edit') %></a>
-          <% if current_user_is_site_admin?(:become_user) %>
+          <% if @user.id != @current_user.id && can_do(@user, @real_current_user || @current_user, :become_user) %>
             |
             <% if @context && !@context.is_a?(Account) %>
               <a href="<%= context_url(@context, :context_url, :become_user_id => @user.id) %>"> <%= t('become', 'Become') %></a>

app/views/users/masquerade.html.erb

@@ -9,6 +9,13 @@
   <% else %>
     <span class="warning-text"><%= t('are_you_sure_start', 'Are you sure you want to masquerade as this user?') %></span>
     <% link_text = "Masquerade as user" %>
+    <p><%= mt 'details', <<-HEREDOC
+Masquerading is essentially logging in as this user without a password.
+You will be able to take any action as if you were this user, and from other users' points of views,
+it will be as if this user performed them. However, audit logs record the fact that
+**you** were the one that actually performed the actions on behalf of this user.
+HEREDOC
+      %></p>
   <% end %>
 
   <div style="float: right">

lib/authentication_methods.rb

@@ -84,49 +84,44 @@ def load_user
       @current_user = nil 
     end
 
-    if @current_user && %w(become_user_id me become_teacher become_student).any? { |k| params.key?(k) } && Account.site_admin.grants_right?(@current_user, session, :become_user)
-      request_become_user_id = nil
+    if @current_user && %w(become_user_id me become_teacher become_student).any? { |k| params.key?(k) }
+      request_become_user = nil
       if params[:become_user_id]
-        request_become_user_id = params[:become_user_id]
+        request_become_user = User.find_by_id(params[:become_user_id])
       elsif params.keys.include?('me')
-        request_become_user_id = nil
+        request_become_user = @current_user
       elsif params.keys.include?('become_teacher')
         course = Course.find(params[:course_id] || params[:id]) rescue nil
         teacher = course.teachers.first if course
         if teacher
-          request_become_user_id = teacher.id
+          request_become_user = teacher
         else
           flash[:error] = I18n.t('lib.auth.errors.teacher_not_found', "No teacher found")
         end
       elsif params.keys.include?('become_student')
         course = Course.find(params[:course_id] || params[:id]) rescue nil
         student = course.students.first if course
         if student
-          request_become_user_id = student.id
+          request_become_user = student
         else
           flash[:error] = I18n.t('lib.auth.errors.student_not_found', "No student found")
         end
       end
-      
-      if request_become_user_id != session[:become_user_id]
+
+      if request_become_user && request_become_user.id != session[:become_user_id].to_i && request_become_user.grants_right?(@current_user, session, :become_user)
         params_without_become = params.dup
         params_without_become.delete_if {|k,v| [ 'become_user_id', 'become_teacher', 'become_student', 'me' ].include? k }
         params_without_become[:only_path] = true
         session[:masquerade_return_to] = url_for(params_without_become)
-        return redirect_to user_masquerade_url(request_become_user_id || @current_user.id)
+        return redirect_to user_masquerade_url(request_become_user.id)
       end
     end
 
-    if session[:become_user_id] && Account.site_admin.grants_right?(@current_user, session, :become_user)
-      @real_current_user = @current_user
-      @current_user = User.find(session[:become_user_id]) rescue @current_user
-      if @current_user.id != @real_current_user.id && Account.site_admin_user?(@current_user)
-        # we can't let even site admins impersonate other site admins, since
-        # they may have different permissions.
-        logger.warn "#{@real_current_user.name}(#{@real_current_user.id}) attempting to impersonate another site admin (#{@current_user.name}). No dice."
-        flash.now[:error] = I18n.t('lib.auth.errors.admin_impersonation_disallowed', "You can't impersonate other site admins. Sorry.")
-        @current_user = @real_current_user
-      else
+    if session[:become_user_id]
+      user = User.find_by_id(session[:become_user_id])
+      if user && user.grants_right?(@current_user, session, :become_user)
+        @real_current_user = @current_user
+        @current_user = user
         logger.warn "#{@real_current_user.name}(#{@real_current_user.id}) impersonating #{@current_user.name} on page #{request.url}"
       end
     end

spec/models/user_spec.rb

@@ -297,4 +297,81 @@
       @user1.submissions.map(&:id).should be_include(s3.id)
     end
   end
+
+  context "permissions" do
+    it "should grant become_user to self" do
+      @user = user_with_pseudonym(:username => 'nobody1@example.com')
+      @user.grants_right?(@user, nil, :become_user).should be_true
+    end
+
+    it "should not grant become_user to other users" do
+      @user1 = user_with_pseudonym(:username => 'nobody1@example.com')
+      @user2 = user_with_pseudonym(:username => 'nobody2@example.com')
+      @user1.grants_right?(@user2, nil, :become_user).should be_false
+      @user2.grants_right?(@user1, nil, :become_user).should be_false
+    end
+
+    it "should grant become_user to site and account admins" do
+      user = user_with_pseudonym(:username => 'nobody1@example.com')
+      @admin = user_with_pseudonym(:username => 'nobody2@example.com')
+      @site_admin = user_with_pseudonym(:username => 'nobody3@example.com')
+      Account.site_admin.add_user(@site_admin)
+      Account.default.add_user(@admin)
+      user.grants_right?(@site_admin, nil, :become_user).should be_true
+      @admin.grants_right?(@site_admin, nil, :become_user).should be_true
+      user.grants_right?(@admin, nil, :become_user).should be_true
+      @admin.grants_right?(@admin, nil, :become_user).should be_true
+      @admin.grants_right?(user, nil, :become_user).should be_false
+      @site_admin.grants_right?(@site_admin, nil, :become_user).should be_true
+      @site_admin.grants_right?(user, nil, :become_user).should be_false
+      @site_admin.grants_right?(@admin, nil, :become_user).should be_false
+    end
+
+    it "should not grant become_user to other site admins" do
+      @site_admin1 = user_with_pseudonym(:username => 'nobody1@example.com')
+      @site_admin2 = user_with_pseudonym(:username => 'nobody2@example.com')
+      Account.site_admin.add_user(@site_admin1)
+      Account.site_admin.add_user(@site_admin2)
+      @site_admin1.grants_right?(@site_admin2, nil, :become_user).should be_false
+      @site_admin2.grants_right?(@site_admin1, nil, :become_user).should be_false
+    end
+
+    it "should not grant become_user to other account admins" do
+      @admin1 = user_with_pseudonym(:username => 'nobody1@example.com')
+      @admin2 = user_with_pseudonym(:username => 'nobody2@example.com')
+      Account.default.add_user(@admin1)
+      Account.default.add_user(@admin2)
+      @admin1.grants_right?(@admin2, nil, :become_user).should be_false
+      @admin2.grants_right?(@admin1, nil, :become_user).should be_false
+    end
+
+    it "should grant become_user for users in multiple accounts to site admins but not account admins" do
+      user = user_with_pseudonym(:username => 'nobody1@example.com')
+      @account2 = Account.create!
+      user.pseudonyms.create!(:unique_id => 'nobodyelse@example.com', :account => @account2)
+      @admin = user_with_pseudonym(:username => 'nobody2@example.com')
+      @site_admin = user_with_pseudonym(:username => 'nobody3@example.com')
+      Account.default.add_user(@admin)
+      Account.site_admin.add_user(@site_admin)
+      user.grants_right?(@admin, nil, :become_user).should be_false
+      user.grants_right?(@site_admin, nil, :become_user).should be_true
+      @account2.add_user(@admin)
+      user.grants_right?(@admin, nil, :become_user).should be_true
+    end
+
+    it "should not grant become_user for dis-associated users" do
+      @user1 = user_model
+      @user2 = user_model
+      @user1.grants_right?(@user2, nil, :become_user).should be_false
+      @user2.grants_right?(@user1, nil, :become_user).should be_false
+    end
+
+    it "should grant become_user for dis-associated users to site admins" do
+      user = user_model
+      @site_admin = user_model
+      Account.site_admin.add_user(@site_admin)
+      user.grants_right?(@site_admin, nil, :become_user).should be_true
+      @site_admin.grants_right?(user, nil, :become_user).should be_false
+    end
+  end
 end