From 83d005c2bccfbce5c3ddf7087e3c59742764ba8c Mon Sep 17 00:00:00 2001 From: Andrew Gardener Date: Thu, 21 Sep 2017 13:37:26 -0700 Subject: [PATCH] Upgrade lti user system roles automatically - Upgrade user's system role when launching an LTI request (system roles only gain more permission, never lose permissions) - Upgrade user's system role when linking an LTI account to an existing account (CAS or App login) --- compair/api/login.py | 6 +++-- compair/api/lti_launch.py | 3 +++ compair/models/lti_models/lti_user.py | 12 ++++++++- compair/tests/api/test_lti_launch.py | 38 +++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 3 deletions(-) diff --git a/compair/api/login.py b/compair/api/login.py index 00c8ed7f2..c81d29ad5 100644 --- 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 index def1038e7..40d902468 100644 --- 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 index e2a541059..54dcf2577 100644 --- 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 index 02ff91350..7aaedc083 100644 --- 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'