Merge pull request #598 from ubc/#589-fix-lti-system-role-issue

Upgrade lti user system roles automatically
ubc · Sep 23, 2017 · c3bcba5 · c3bcba5
2 parents 6a3d61c + 83d005c
commit c3bcba5
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 3 deletions.
diff --git a/compair/api/login.py b/compair/api/login.py
@@ -39,7 +39,8 @@ def login():
 
         if sess.get('LTI') and sess.get('oauth_create_user_link'):
             lti_user = LTIUser.query.get_or_404(sess['lti_user'])
-            lti_user.compair_user_id = user.id
+            lti_user.compair_user = user
+            lti_user.upgrade_system_role()
             sess.pop('oauth_create_user_link')
 
         if sess.get('LTI') and sess.get('lti_context') and sess.get('lti_user_resource_link'):
@@ -155,7 +156,8 @@ def cas_auth():
 
                 if sess.get('LTI') and sess.get('oauth_create_user_link'):
                     lti_user = LTIUser.query.get_or_404(sess['lti_user'])
-                    lti_user.compair_user_id = thirdpartyuser.user_id
+                    lti_user.compair_user = thirdpartyuser.user
+                    lti_user.upgrade_system_role()
                     sess.pop('oauth_create_user_link')
 
                 if sess.get('LTI') and sess.get('lti_context') and sess.get('lti_user_resource_link'):

diff --git a/compair/api/lti_launch.py b/compair/api/lti_launch.py
@@ -86,6 +86,9 @@ def post(self):
                 if lti_user.is_linked_to_user():
                     authenticate(lti_user.compair_user, login_method='LTI')
 
+                    # upgrade user system role if needed
+                    lti_user.upgrade_system_role()
+
                     # create/update enrollment if context exists
                     if lti_context and lti_context.is_linked_to_course():
                         lti_context.update_enrolment(lti_user.compair_user_id, lti_user_resource_link.course_role)

diff --git a/compair/models/lti_models/lti_user.py b/compair/models/lti_models/lti_user.py
@@ -55,7 +55,7 @@ def get_by_tool_provider(cls, lti_consumer, tool_provider):
             lti_user = LTIUser(
                 lti_consumer_id=lti_consumer.id,
                 user_id=tool_provider.user_id,
-                system_role= SystemRole.instructor  \
+                system_role=SystemRole.instructor  \
                     if tool_provider.is_instructor() \
                     else SystemRole.student
             )
@@ -70,6 +70,16 @@ def get_by_tool_provider(cls, lti_consumer, tool_provider):
 
         return lti_user
 
+    def upgrade_system_role(self):
+        # upgrade system role is needed
+        if self.is_linked_to_user():
+            if self.compair_user.system_role == SystemRole.student and self.system_role in [SystemRole.instructor, SystemRole.sys_admin]:
+                self.compair_user.system_role = self.system_role
+            elif self.compair_user.system_role == SystemRole.instructor and self.system_role == SystemRole.sys_admin:
+                self.compair_user.system_role = self.system_role
+
+            db.session.commit()
+
     @classmethod
     def __declare_last__(cls):
         super(cls, cls).__declare_last__()

diff --git a/compair/tests/api/test_lti_launch.py b/compair/tests/api/test_lti_launch.py
@@ -319,6 +319,44 @@ def test_lti_auth(self):
             db.session.commit()
             # done user_id_override ------
 
+        # test automatic upgrading of system role for existing accounts
+        for lti_role, (system_role, course_role) in roles.items():
+            for compair_system_role in [SystemRole.student, SystemRole.instructor, SystemRole.sys_admin]:
+                lti_context = self.lti_data.create_context(lti_consumer)
+                lti_user = self.lti_data.create_user(lti_consumer, system_role)
+                lti_resource_link = self.lti_data.create_resource_link(lti_consumer, lti_context)
+                lti_user_resource_link = self.lti_data.create_user_resource_link(
+                    lti_user, lti_resource_link, CourseRole.instructor)
+
+                user = self.data.create_user(compair_system_role)
+                lti_user.compair_user = user
+                course = self.data.create_course()
+                lti_context.compair_course = course
+
+                db.session.commit()
+
+                # valid request - user with account and existing context_id
+                with self.lti_launch(lti_consumer, lti_resource_link.resource_link_id,
+                        user_id=lti_user.user_id, context_id=lti_context.context_id, roles=lti_role,
+                        follow_redirects=False) as rv:
+                    self.assertRedirects(rv, '/app/#/course/'+course.uuid)
+
+                # compair user system role will upgrade
+                if compair_system_role == SystemRole.student:
+                    if system_role == SystemRole.sys_admin:
+                        self.assertEqual(user.system_role, SystemRole.sys_admin)
+                    elif system_role == SystemRole.instructor:
+                        self.assertEqual(user.system_role, SystemRole.instructor)
+                    else:
+                        self.assertEqual(user.system_role, SystemRole.student)
+                elif compair_system_role == SystemRole.instructor:
+                    if system_role == SystemRole.sys_admin:
+                        self.assertEqual(user.system_role, SystemRole.sys_admin)
+                    else:
+                        self.assertEqual(user.system_role, SystemRole.instructor)
+                elif compair_system_role == SystemRole.sys_admin:
+                    self.assertEqual(user.system_role, SystemRole.sys_admin)
+
     def test_lti_status(self):
         url = '/api/lti/status'