diff --git a/lms/djangoapps/lti_provider/tests/test_users.py b/lms/djangoapps/lti_provider/tests/test_users.py index 508ab703ac..90025e3f98 100644 --- a/lms/djangoapps/lti_provider/tests/test_users.py +++ b/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('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) + @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_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_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_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) diff --git a/lms/djangoapps/lti_provider/users.py b/lms/djangoapps/lti_provider/users.py index e6525707fa..dadaf8d7cc 100644 --- a/lms/djangoapps/lti_provider/users.py +++ b/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 diff --git a/lms/envs/aws.py b/lms/envs/aws.py index a4ae2dfb9b..0af1c480d8 100644 --- a/lms/envs/aws.py +++ b/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', ) diff --git a/lms/envs/common.py b/lms/envs/common.py index 43dadd2970..2658d1866a 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2543,3 +2543,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' diff --git a/lms/envs/test.py b/lms/envs/test.py index 326572da14..4870c5513a 100644 --- a/lms/envs/test.py +++ b/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',)