graphql: fix loading users from node

closes RECNVS-610

The existing permission basically only allowed users to load themselves,
or admins to load about anyone, but teachers couldn't even load their
own students.

Test plan:
  - users should be able to load users they have permission to view via
    the top-level node/legacyNode fields

Change-Id: Ib9cfa750d7797253e678b057fc19cc2add4b39e8
Reviewed-on: https://gerrit.instructure.com/163605
Tested-by: Jenkins
Reviewed-by: Jonathan Featherstone <jfeatherstone@instructure.com>
QA-Review: Jonathan Featherstone <jfeatherstone@instructure.com>
Product-Review: Cameron Matheson <cameron@instructure.com>

app/graphql/graphql_node_loader.rb

@@ -28,9 +28,28 @@ module GraphQLNodeLoader
     when "Section"
       Loaders::IDLoader.for(CourseSection).load(id).then(check_read_permission)
     when "User"
-      Loaders::IDLoader.for(User).load(id).then(
-        make_permission_check(ctx, :manage, :manage_user_details)
-      )
+      Loaders::IDLoader.for(User).load(id).then(->(user) do
+        return nil unless user && ctx[:current_user]
+
+        return user if user.grants_right?(ctx[:current_user], :read_full_profile)
+
+        has_permission = Rails.cache.fetch(["node_user_perm", ctx[:current_user], user].cache_key) do
+          has_perm = Shard.with_each_shard(user.associated_shards & ctx[:current_user].associated_shards) do
+            shared_courses = Enrollment
+              .joins("INNER JOIN #{Enrollment.quoted_table_name} e2 ON e2.course_id = enrollments.course_id")
+              .where("enrollments.user_id = ? AND e2.user_id = ?", user.id, ctx[:current_user].id)
+              .select("enrollments.course_id")
+
+            break true if Course.where(id: shared_courses).any? do |course|
+              course.grants_right?(ctx[:current_user], :read_roster) &&
+                course.enrollments_visible_to(ctx[:current_user], include_concluded: true).where(user_id: user).exists?
+            end
+          end
+          has_perm == true
+        end
+
+        has_permission ? user : nil
+      end)
     when "Enrollment"
       Loaders::IDLoader.for(Enrollment).load(id).then do |enrollment|
         Loaders::IDLoader.for(Course).load(enrollment.course_id).then do |course|

spec/graphql/legacy_node_spec.rb

@@ -27,32 +27,6 @@ describe "legacyNode" do
     CanvasSchema.execute(query, context: {current_user: user})
   end
 
-  context "users" do
-    before(:once) do
-      @query = <<-GQL
-      query {
-        user: legacyNode(type: User, _id: "#{@student.id}") {
-          ... on User {
-            _id
-          }
-        }
-      }
-      GQL
-    end
-
-    it "works" do
-      expect(
-        run_query(@query, @student)["data"]["user"]["_id"]
-      ).to eq @student.id.to_s
-    end
-
-    it "requires read_full_profile permission" do
-      orig_student = @student
-      student_in_course
-      expect(run_query(@query, @student)["data"]["user"]).to be_nil
-    end
-  end
-
   context "enrollments" do
     before(:once) do
       @enrollment = @student.enrollments.first

spec/graphql/types/user_type_spec.rb

@@ -17,15 +17,50 @@
 #
 
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper')
+require File.expand_path(File.dirname(__FILE__) + '/../../helpers/graphql_type_tester')
 require File.expand_path(File.dirname(__FILE__) + '/../../helpers/legacy_type_tester')
 
 describe Types::UserType do
-  let_once(:user) { student_in_course(active_all: true); @student }
-  let(:user_type) { LegacyTypeTester.new(Types::UserType, user) }
+  before(:once) do
+    student = student_in_course(active_all: true).user
+    course = @course
+    teacher = @teacher
+    @other_student = student_in_course(active_all: true).user
 
-  it "works" do
-    expect(user_type._id).to eq user.id
-    expect(user_type.name).to eq "User"
+    @other_course = course_factory
+    @random_person = teacher_in_course(active_all: true).user
+
+    @course = course
+    @student = student
+    @teacher = teacher
+
+  end
+
+  let(:new_user_type) { GraphQLTypeTester.new(@student, current_user: @teacher) }
+  let(:user_type) { LegacyTypeTester.new(Types::UserType, @student) }
+  let(:user) { @student }
+
+  context "node" do
+    it "works" do
+      expect(new_user_type.resolve("_id")).to eq @student.id.to_s
+      expect(new_user_type.resolve("name")).to eq @student.name
+    end
+
+    it "works for users in the same course" do
+      expect(new_user_type.resolve("_id", current_user: @other_student)).to eq @student.id.to_s
+    end
+
+    it "doesn't work for just anyone" do
+      expect(new_user_type.resolve("_id", current_user: @random_person)).to be_nil
+    end
+
+    it "loads inactive and concluded users" do
+      @student.enrollments.update_all workflow_state: "inactive"
+      expect(new_user_type.resolve("_id", current_user: @other_student)).to eq @student.id.to_s
+
+      @student.enrollments.update_all workflow_state: "completed"
+      expect(new_user_type.resolve("_id", current_user: @other_student)).to eq @student.id.to_s
+    end
   end
 
   context "avatarUrl" do
@@ -43,7 +78,7 @@ describe Types::UserType do
     before(:once) do
       @course1 = @course
       @course2 = course_factory
-      student_in_course(user: @student, course: @course2, active_all: true)
+      @course2.enroll_student(@student, enrollment_state: "active")
     end
 
     it "returns enrollments for a given course" do
@@ -80,8 +115,7 @@ describe Types::UserType do
 
   context "email" do
     before(:once) do
-      user.email = "cooldude@example.com"
-      user.save!
+      @student.update_attributes! email: "cooldude@example.com"
     end
 
     it "returns email for teachers/admins" do
@@ -93,13 +127,9 @@ describe Types::UserType do
     end
 
     it "doesn't return email for others" do
-      _student = user
-      other_student = student_in_course(active_all: true).user
-      teacher_in_other_course = teacher_in_course(course: course_factory).user
-
       expect(user_type.email(current_user: nil)).to be_nil
-      expect(user_type.email(current_user: other_student)).to be_nil
-      expect(user_type.email(current_user: teacher_in_other_course)).to be_nil
+      expect(user_type.email(current_user: @other_student)).to be_nil
+      expect(user_type.email(current_user: @random_person)).to be_nil
     end
   end
 end