Merge pull request #57970 from code-dot-org/stephen/ignore-observers

fix(lms): do not add observers or mentors as students
code-dot-org · Apr 15, 2024 · f0d8677 · f0d8677
2 parents 0d48efa + 390c9e3
commit f0d8677
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 1 deletion.
diff --git a/dashboard/lib/policies/lti.rb b/dashboard/lib/policies/lti.rb
@@ -28,6 +28,7 @@ module AccessTokenScopes
     ]
 ).freeze
   CONTEXT_LEARNER_ROLE = 'http://purl.imsglobal.org/vocab/lis/v2/membership#Learner'.freeze
+  CONTEXT_MENTOR_ROLE = 'http://purl.imsglobal.org/vocab/lis/v2/membership#Mentor'.freeze
   LTI_ROLES_KEY = 'https://purl.imsglobal.org/spec/lti/claim/roles'.freeze
   LTI_CUSTOM_CLAIMS = "https://purl.imsglobal.org/spec/lti/claim/custom".freeze
   LTI_CONTEXT_CLAIM = "https://purl.imsglobal.org/spec/lti/claim/context".freeze

diff --git a/dashboard/lib/services/lti.rb b/dashboard/lib/services/lti.rb
@@ -131,7 +131,7 @@ def self.parse_nrps_response(nrps_response, issuer)
       members = nrps_response[:members]
       context_title = nrps_response.dig(:context, :title)
       members.each do |member|
-        next if member[:status] == 'Inactive'
+        next if member[:status] == 'Inactive' || member[:roles].include?(Policies::Lti::CONTEXT_MENTOR_ROLE)
         # TODO: handle multiple messages. Shouldn't be needed until we support Deep Linking.
 
         # If the LMS hasn't implemented the resource link level membership service, we don't get the message property in the member object

diff --git a/dashboard/test/lib/services/lti_test.rb b/dashboard/test/lib/services/lti_test.rb
@@ -16,6 +16,7 @@ class Services::LtiTest < ActiveSupport::TestCase
     ]
     @lti_integration = create :lti_integration
     @student_role = Policies::Lti::CONTEXT_LEARNER_ROLE
+    @observer_role = Policies::Lti::CONTEXT_MENTOR_ROLE
     @teacher_role = Policies::Lti::TEACHER_ROLES.first
 
     @id_token = {
@@ -58,6 +59,19 @@ class Services::LtiTest < ActiveSupport::TestCase
       }],
     }.deep_symbolize_keys
 
+    @nrps_observer = {
+      status: 'Active',
+      user_id: SecureRandom.uuid,
+      roles: [@observer_role],
+      message: [{
+        @custom_claims_key => {
+          display_name: 'parent',
+          full_name: 'Test Parent',
+          email: 'test-parent@code.org'
+        }
+      }],
+    }.deep_symbolize_keys
+
     @course_name = 'Test Course'
     @lms_section_ids = [1, 2, 3]
     @lms_section_names = ['Section 1', 'Section 2', 'Section 3']
@@ -133,6 +147,27 @@ class Services::LtiTest < ActiveSupport::TestCase
             }
           ]
         },
+        {
+          status: "Active",
+          user_id: "observer-0",
+          roles: [Policies::Lti::CONTEXT_MENTOR_ROLE],
+          message: [
+            {
+              'https://purl.imsglobal.org/spec/lti/claim/message_type': "LtiResourceLinkRequest",
+              locale: "en",
+              'https://purl.imsglobal.org/spec/lti/claim/custom': {
+                email: "parent-0@code.org",
+                course_id: "115",
+                full_name: "Parent Zero",
+                given_name: "Test",
+                family_name: "Zero",
+                section_ids: @lms_section_ids.join(','),
+                display_name: "Test Zero",
+                section_names: @lms_section_names.to_s
+              },
+            }
+          ]
+        },
         {
           status: "Active",
           user_id: SecureRandom.uuid,