fix permissions around "Login ID" column in course roster

test plan:
 - a custom role with "Users - View login IDs" permission granted
   and without "add/remove students" or "add/remove teachers"
   should still see the login ID column in the course roster
   and it should be populated
 - a custom role without "Users - View login IDs" permission granted
   should not see the login ID column in the course roster
   even if they do have "add/remove students" or "add/remove teachers"

fixes ADMIN-2628

Change-Id: I876fbcd7cae705cf78c2b422fb365adc629235fc
Reviewed-on: https://gerrit.instructure.com/208837
QA-Review: Rex Fleischer <rfleischer@instructure.com>
Tested-by: Jenkins
Reviewed-by: Mysti Lilla <mysti@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>

app/coffeescripts/views/courses/roster/RosterUserView.js

@@ -97,8 +97,7 @@ export default class RosterUserView extends Backbone.View {
 
     json.canEditSections = !json.isInactive && !_.isEmpty(this.model.sectionEditableEnrollments())
     json.canLinkStudents = json.isObserver && !ENV.course.concluded
-    json.canViewLoginIdColumn =
-      ENV.permissions.manage_admin_users || ENV.permissions.manage_students
+    json.canViewLoginIdColumn = ENV.permissions.view_user_logins
     json.canViewSisIdColumn = ENV.permissions.read_sis
     json.canManage = _.some(['TeacherEnrollment', 'DesignerEnrollment', 'TaEnrollment'], et =>
       this.model.hasEnrollmentType(et)

app/controllers/context_controller.rb

@@ -143,6 +143,7 @@ class ContextController < ApplicationController
         :resend_invitations_url => course_re_send_invitations_url(@context),
         :permissions => {
           :read_sis => @context.grants_any_right?(@current_user, session, :read_sis, :manage_sis),
+          :view_user_logins => @context.grants_right?(@current_user, session, :view_user_logins),
           :manage_students => (manage_students = @context.grants_right?(@current_user, session, :manage_students) && !MasterCourses::MasterTemplate.is_master_course?(@context)),
           :manage_admin_users => (manage_admins = @context.grants_right?(@current_user, session, :manage_admin_users)),
           :add_users => manage_students || manage_admins,

app/jsx/bundles/roster.js

@@ -54,7 +54,7 @@ const usersView = new PaginatedCollectionView({
   itemViewOptions: {
     course: ENV.course
   },
-  canViewLoginIdColumn: ENV.permissions.manage_admin_users || ENV.permissions.manage_students,
+  canViewLoginIdColumn: ENV.permissions.view_user_logins,
   canViewSisIdColumn: ENV.permissions.read_sis,
   buffer: 1000,
   template: rosterUsersTemplate

lib/api/v1/user.rb

@@ -218,7 +218,7 @@ module Api::V1::User
         permissions_account = context.is_a?(Account) ? context : context.account
       end
       !!(
-        permissions_context.grants_any_right?(current_user, :manage_students, :read_sis) ||
+        permissions_context.grants_any_right?(current_user, :manage_students, :read_sis, :view_user_logins) ||
         permissions_account.membership_for_user(current_user) ||
         permissions_account.root_account.grants_right?(current_user, :manage_sis)
       )

spec/apis/v1/enrollments_api_spec.rb

@@ -1577,6 +1577,7 @@ describe EnrollmentsApiController, type: :request do
               'sortable_name' => e.user.sortable_name,
               'short_name' => e.user.short_name,
               'id' => e.user.id,
+              'login_id' => @user.pseudonym.unique_id,
               'created_at' => e.user.created_at.iso8601
             },
             'html_url' => course_user_url(e.course_id, e.user_id),

spec/apis/v1/users_api_spec.rb

@@ -336,7 +336,7 @@ describe Api::V1::User do
     def test_context(mock_context, context_to_pass)
       expect(mock_context).to receive(:account).and_return(mock_context)
       expect(mock_context).to receive(:global_id).and_return(42).twice
-      expect(mock_context).to receive(:grants_any_right?).with(@admin, :manage_students, :read_sis).and_return(true)
+      expect(mock_context).to receive(:grants_any_right?).with(@admin, :manage_students, :read_sis, :view_user_logins).and_return(true)
       expect(mock_context).to receive(:grants_right?).with(@admin, {}, :view_user_logins).and_return(true)
       json = if context_to_pass
         @test_api.user_json(@student, @admin, {}, [], context_to_pass)
@@ -617,7 +617,7 @@ describe Api::V1::User do
       @test_api.context = double()
       expect(@test_api.context).to receive(:global_id).and_return(42)
       expect(@test_api.context).to receive(:account).and_return(@test_api.context)
-      expect(@test_api.context).to receive(:grants_any_right?).with(@admin, :manage_students, :read_sis).and_return(true)
+      expect(@test_api.context).to receive(:grants_any_right?).with(@admin, :manage_students, :read_sis, :view_user_logins).and_return(true)
       @test_api.current_user = @admin
       expect(@test_api.user_json_is_admin?).to eq true
     end
@@ -626,7 +626,7 @@ describe Api::V1::User do
       mock_context = double()
       expect(mock_context).to receive(:global_id).and_return(42)
       expect(mock_context).to receive(:account).and_return(mock_context)
-      expect(mock_context).to receive(:grants_any_right?).with(@admin, :manage_students, :read_sis).and_return(true)
+      expect(mock_context).to receive(:grants_any_right?).with(@admin, :manage_students, :read_sis, :view_user_logins).and_return(true)
       @test_api.current_user = @admin
       expect(@test_api.user_json_is_admin?(mock_context, @admin)).to eq true
     end

spec/selenium/people_spec.rb

@@ -385,6 +385,22 @@ describe "people" do
 
     # TODO reimplement per CNVS-29609, but make sure we're testing at the right level
     it "should validate that a TA cannot rename a teacher"
+
+    it "includes login id column if the user has :view_user_logins, even if they don't have :manage_students" do
+      RoleOverride.create!(:context => Account.default, :permission => 'manage_students', :role => ta_role, :enabled => false)
+      get "/courses/#{@course.id}/users"
+      index = ff('table.roster th').map(&:text).find_index('Login ID')
+      expect(index).not_to be_nil
+      ta_row = ff("table.roster #user_#{@ta.id} td").map(&:text)
+      expect(ta_row[index].strip).to eq @ta.pseudonym.unique_id
+    end
+
+    it "does not include login id column if the user does not have :view_user_logins, even if they do have :manage_students" do
+      RoleOverride.create!(:context => Account.default, :permission => 'view_user_logins', :role => ta_role, :enabled => false)
+      get "/courses/#{@course.id}/users"
+      index = ff('table.roster th').map(&:text).find_index('Login ID')
+      expect(index).to be_nil
+    end
   end
 
   context "people as a student" do