Merge pull request #8620 from mcgachey/lti-deployment

[LTI Provider] Added an authentication backend to log in LTI users

lms/djangoapps/lti_provider/tests/test_users.py

@@ -5,6 +5,7 @@ Tests for the LTI user management functionality
 import string
 
 from django.contrib.auth.models import User
+from django.core.exceptions import PermissionDenied
 from django.test import TestCase
 from django.test.client import RequestFactory
 from mock import patch, MagicMock
@@ -25,21 +26,37 @@ class UserManagementHelperTest(TestCase):
         self.new_user = UserFactory.create()
         self.new_user.save()
         self.request.user = self.old_user
+        self.lti_consumer = LtiConsumer(
+            consumer_name='TestConsumer',
+            consumer_key='TestKey',
+            consumer_secret='TestSecret'
+        )
+        self.lti_consumer.save()
         self.lti_user = LtiUser(
             lti_user_id='lti_user_id',
             edx_user=self.new_user
         )
 
+    @patch('django.contrib.auth.authenticate', return_value=None)
+    def test_permission_denied_for_unknown_user(self, _authenticate_mock):
+        with self.assertRaises(PermissionDenied):
+            users.switch_user(self.request, self.lti_user, self.lti_consumer)
+
     @patch('lti_provider.users.login')
-    def test_new_user_logged_in_by_switch_user(self, login_mock):
-        with patch('lti_provider.users.User.objects.get', return_value=self.new_user):
-            users.switch_user(self.request, self.lti_user)
-            login_mock.assert_called_with(self.request, self.new_user)
+    def test_authenticate_called(self, _login_mock):
+        with patch('lti_provider.users.authenticate', return_value=self.new_user) as authenticate:
+            users.switch_user(self.request, self.lti_user, self.lti_consumer)
+            authenticate.assert_called_with(
+                username=self.new_user.username,
+                lti_user_id=self.lti_user.lti_user_id,
+                lti_consumer=self.lti_consumer
+            )
 
     @patch('lti_provider.users.login')
-    def test_backend_set_in_switch_user(self, _login_mock):
-        users.switch_user(self.request, self.lti_user)
-        self.assertIsNotNone(self.new_user.backend, 'Backend not set on user')
+    def test_login_called(self, login_mock):
+        with patch('lti_provider.users.authenticate', return_value=self.new_user):
+            users.switch_user(self.request, self.lti_user, self.lti_consumer)
+            login_mock.assert_called_with(self.request, self.new_user)
 
     def test_random_username_generator(self):
         for _idx in range(1000):
@@ -93,7 +110,7 @@ class AuthenticateLtiUserTest(TestCase):
         with patch('lti_provider.users.create_lti_user', return_value=lti_user) as create_user:
             users.authenticate_lti_user(self.request, self.lti_user_id, self.lti_consumer)
             create_user.assert_called_with(self.lti_user_id, self.lti_consumer)
-            switch_user.assert_called_with(self.request, lti_user)
+            switch_user.assert_called_with(self.request, lti_user, self.lti_consumer)
 
     def test_authentication_with_authenticated_user(self, create_user, switch_user):
         lti_user = self.create_lti_user_model()
@@ -109,7 +126,7 @@ class AuthenticateLtiUserTest(TestCase):
         self.request.user.is_authenticated = MagicMock(return_value=False)
         users.authenticate_lti_user(self.request, self.lti_user_id, self.lti_consumer)
         self.assertFalse(create_user.called)
-        switch_user.assert_called_with(self.request, lti_user)
+        switch_user.assert_called_with(self.request, lti_user, self.lti_consumer)
 
     def test_authentication_with_wrong_user(self, create_user, switch_user):
         lti_user = self.create_lti_user_model()
@@ -117,7 +134,7 @@ class AuthenticateLtiUserTest(TestCase):
         self.request.user.is_authenticated = MagicMock(return_value=True)
         users.authenticate_lti_user(self.request, self.lti_user_id, self.lti_consumer)
         self.assertFalse(create_user.called)
-        switch_user.assert_called_with(self.request, lti_user)
+        switch_user.assert_called_with(self.request, lti_user, self.lti_consumer)
 
 
 class CreateLtiUserTest(TestCase):
@@ -155,3 +172,71 @@ class CreateLtiUserTest(TestCase):
         self.assertEqual(User.objects.count(), 2)
         user = User.objects.get(username='new_edx_id')
         self.assertEqual(user.email, 'new_edx_id@lti.example.com')
+
+
+class LtiBackendTest(TestCase):
+    """
+    Tests for the authentication backend that authenticates LTI users.
+    """
+
+    def setUp(self):
+        super(LtiBackendTest, self).setUp()
+        self.edx_user = UserFactory.create()
+        self.edx_user.save()
+        self.lti_consumer = LtiConsumer(
+            consumer_key="Consumer Key",
+            consumer_secret="Consumer Secret"
+        )
+        self.lti_consumer.save()
+        self.lti_user_id = 'LTI User ID'
+        LtiUser(
+            lti_consumer=self.lti_consumer,
+            lti_user_id=self.lti_user_id,
+            edx_user=self.edx_user
+        ).save()
+
+    def test_valid_user_authenticates(self):
+        user = users.LtiBackend().authenticate(
+            username=self.edx_user.username,
+            lti_user_id=self.lti_user_id,
+            lti_consumer=self.lti_consumer
+        )
+        self.assertEqual(user, self.edx_user)
+
+    def test_missing_user_returns_none(self):
+        user = users.LtiBackend().authenticate(
+            username=self.edx_user.username,
+            lti_user_id='Invalid Username',
+            lti_consumer=self.lti_consumer
+        )
+        self.assertIsNone(user)
+
+    def test_non_lti_user_returns_none(self):
+        non_edx_user = UserFactory.create()
+        non_edx_user.save()
+        user = users.LtiBackend().authenticate(
+            username=non_edx_user.username,
+        )
+        self.assertIsNone(user)
+
+    def test_missing_lti_id_returns_null(self):
+        user = users.LtiBackend().authenticate(
+            username=self.edx_user.username,
+            lti_consumer=self.lti_consumer
+        )
+        self.assertIsNone(user)
+
+    def test_missing_lti_consumer_returns_null(self):
+        user = users.LtiBackend().authenticate(
+            username=self.edx_user.username,
+            lti_user_id=self.lti_user_id,
+        )
+        self.assertIsNone(user)
+
+    def test_existing_user_returned_by_get_user(self):
+        user = users.LtiBackend().get_user(self.edx_user.id)
+        self.assertEqual(user, self.edx_user)
+
+    def test_get_user_returns_none_for_invalid_user(self):
+        user = users.LtiBackend().get_user(-1)
+        self.assertIsNone(user)

lms/djangoapps/lti_provider/users.py

@@ -7,11 +7,13 @@ import string
 import random
 import uuid
 
-from django.contrib.auth import login
+from django.conf import settings
+from django.contrib.auth import authenticate, login
 from django.contrib.auth.models import User
+from django.core.exceptions import PermissionDenied
 from django.db import IntegrityError
-
 from lti_provider.models import LtiUser
+from student.models import UserProfile
 
 
 def authenticate_lti_user(request, lti_user_id, lti_consumer):
@@ -36,7 +38,7 @@ def authenticate_lti_user(request, lti_user_id, lti_consumer):
             request.user == lti_user.edx_user):
         # The user is not authenticated, or is logged in as somebody else.
         # Switch them to the LTI user
-        switch_user(request, lti_user)
+        switch_user(request, lti_user, lti_consumer)
 
 
 def create_lti_user(lti_user_id, lti_consumer):
@@ -50,12 +52,17 @@ def create_lti_user(lti_user_id, lti_consumer):
     while not created:
         try:
             edx_user_id = generate_random_edx_username()
+            edx_email = "{}@{}".format(edx_user_id, settings.LTI_USER_EMAIL_DOMAIN)
             edx_user = User.objects.create_user(
                 username=edx_user_id,
                 password=edx_password,
-                email='{}@lti.example.com'.format(edx_user_id)
+                email=edx_email,
             )
-            edx_user.save()
+            # A profile is required if PREVENT_CONCURRENT_LOGINS flag is set.
+            # TODO: We could populate user information from the LTI launch here,
+            # but it's not necessary for our current uses.
+            edx_user_profile = UserProfile(user=edx_user)
+            edx_user_profile.save()
             created = True
         except IntegrityError:
             # The random edx_user_id wasn't unique. Since 'created' is still
@@ -71,14 +78,21 @@ def create_lti_user(lti_user_id, lti_consumer):
     return lti_user
 
 
-def switch_user(request, lti_user):
+def switch_user(request, lti_user, lti_consumer):
     """
     Log out the current user, and log in using the edX identity associated with
     the LTI ID.
     """
-    # The login function wants to know what backend authenticated the user.
-    lti_user.edx_user.backend = 'LTI_Provider'
-    login(request, lti_user.edx_user)
+    edx_user = authenticate(
+        username=lti_user.edx_user.username,
+        lti_user_id=lti_user.lti_user_id,
+        lti_consumer=lti_consumer
+    )
+    if not edx_user:
+        # This shouldn't happen, since we've created edX accounts for any LTI
+        # users by this point, but just in case we can return a 403.
+        raise PermissionDenied()
+    login(request, edx_user)
 
 
 def generate_random_edx_username():
@@ -92,3 +106,46 @@ def generate_random_edx_username():
     for _index in range(30):
         username = username + random.SystemRandom().choice(allowable_chars)
     return username
+
+
+class LtiBackend(object):
+    """
+    A Django authentication backend that authenticates users via LTI. This
+    backend will only return a User object if it is associated with an LTI
+    identity (i.e. the user was created by the create_lti_user method above).
+    """
+
+    def authenticate(self, username=None, lti_user_id=None, lti_consumer=None):
+        """
+        Try to authenticate a user. This method will return a Django user object
+        if a user with the corresponding username exists in the database, and
+        if a record that links that user with an LTI user_id field exists in
+        the LtiUser collection.
+
+        If such a user is not found, the method returns None (in line with the
+        authentication backend specification).
+        """
+        try:
+            edx_user = User.objects.get(username=username)
+        except User.DoesNotExist:
+            return None
+
+        try:
+            LtiUser.objects.get(
+                edx_user_id=edx_user.id,
+                lti_user_id=lti_user_id,
+                lti_consumer=lti_consumer
+            )
+        except LtiUser.DoesNotExist:
+            return None
+        return edx_user
+
+    def get_user(self, user_id):
+        """
+        Return the User object for a user that has already been authenticated by
+        this backend.
+        """
+        try:
+            return User.objects.get(id=user_id)
+        except User.DoesNotExist:
+            return None

lms/envs/aws.py

@@ -647,6 +647,10 @@ EDXNOTES_INTERNAL_API = ENV_TOKENS.get('EDXNOTES_INTERNAL_API', EDXNOTES_INTERNA
 
 CREDIT_PROVIDER_SECRET_KEYS = AUTH_TOKENS.get("CREDIT_PROVIDER_SECRET_KEYS", {})
 
-
 ############ CERTIFICATE VERIFICATION URL (STATIC FILES) ###########
 ENV_TOKENS.get('CERTIFICATES_STATIC_VERIFY_URL', CERTIFICATES_STATIC_VERIFY_URL)
+
+##################### LTI Provider #####################
+if FEATURES.get('ENABLE_LTI_PROVIDER'):
+    INSTALLED_APPS += ('lti_provider',)
+    AUTHENTICATION_BACKENDS += ('lti_provider.users.LtiBackend', )

lms/envs/common.py

@@ -2549,3 +2549,9 @@ CREDIT_PROVIDER_SECRET_KEYS = {}
 # when a credit provider notifies us that a student has been approved
 # or denied for credit.
 CREDIT_PROVIDER_TIMESTAMP_EXPIRATION = 15 * 60
+
+# Default domain for the e-mail address associated with users who are created
+# via the LTI Provider feature. Note that the generated e-mail addresses are
+# not expected to be active; this setting simply allows administrators to
+# route any messages intended for LTI users to a common domain.
+LTI_USER_EMAIL_DOMAIN = 'lti.example.com'

lms/envs/test.py

@@ -495,3 +495,4 @@ PROFILE_IMAGE_MIN_BYTES = 100
 # Enable the LTI provider feature for testing
 FEATURES['ENABLE_LTI_PROVIDER'] = True
 INSTALLED_APPS += ('lti_provider',)
+AUTHENTICATION_BACKENDS += ('lti_provider.users.LtiBackend',)