Skip to content

Commit

Permalink
feat: link LTI Provider launches to authenticated users
Browse files Browse the repository at this point in the history
With this change, the platform users who access content via LTI will be 
automatically linked to their platform account instead of the new (anonymous) 
one. The following conditions need to be met:
* The `LtiConsumer` should be configured to auto-link the users via email.
* The LTI Consumer should share the user's email using the
`lis_person_contact_email_primary` parameter in the LTI Launch POST data.

This also replaces the one-to-one relationship of the `User` and `LtiUser` 
with one-to-many. This way, multiple `LtiUser` objects can refer to the same 
`edx_user`. With the auto-linking, multiple LTI Consumers can create 
independent `LtiUser` objects with the same `edx_user`.

Co-authored-by: Piotr Surowiec <[email protected]>
  • Loading branch information
tecoholic and Agrendalath authored Nov 13, 2023
1 parent 7050eca commit 5b2f012
Show file tree
Hide file tree
Showing 7 changed files with 227 additions and 34 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Generated by Django 3.2.22 on 2023-11-06 09:47

from django.conf import settings
from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
('lti_provider', '0003_auto_20161118_1040'),
]

operations = [
migrations.AddField(
model_name='lticonsumer',
name='require_user_account',
field=models.BooleanField(blank=True, default=False, help_text='When checked, the LTI content will load only for learners who have an account in this instance. This is required only for linking learner accounts with the LTI consumer. See the Open edX LTI Provider documentation for more details.'),
),
migrations.AlterField(
model_name='ltiuser',
name='edx_user',
field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL),
),
]
8 changes: 7 additions & 1 deletion lms/djangoapps/lti_provider/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user
from django.db import models
from django.utils.translation import gettext as _
from opaque_keys.edx.django.models import CourseKeyField, UsageKeyField

from openedx.core.djangolib.fields import CharNullField
Expand All @@ -34,6 +35,11 @@ class LtiConsumer(models.Model):
consumer_key = models.CharField(max_length=32, unique=True, db_index=True, default=short_token)
consumer_secret = models.CharField(max_length=32, unique=True, default=short_token)
instance_guid = CharNullField(max_length=255, blank=True, null=True, unique=True)
require_user_account = models.BooleanField(blank=True, default=False, help_text=_(
"When checked, the LTI content will load only for learners who have an account "
"in this instance. This is required only for linking learner accounts with "
"the LTI consumer. See the Open edX LTI Provider documentation for more details."
))

@staticmethod
def get_or_supplement(instance_guid, consumer_key):
Expand Down Expand Up @@ -140,7 +146,7 @@ class LtiUser(models.Model):
"""
lti_consumer = models.ForeignKey(LtiConsumer, on_delete=models.CASCADE)
lti_user_id = models.CharField(max_length=255)
edx_user = models.OneToOneField(User, on_delete=models.CASCADE)
edx_user = models.ForeignKey(User, on_delete=models.CASCADE)

class Meta:
unique_together = ('lti_consumer', 'lti_user_id')
77 changes: 71 additions & 6 deletions lms/djangoapps/lti_provider/tests/test_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
from unittest.mock import MagicMock, PropertyMock, patch

import pytest
from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user
from django.contrib.auth.models import AnonymousUser, User # lint-amnesty, pylint: disable=imported-auth-user
from django.core.exceptions import PermissionDenied
from django.db.utils import IntegrityError
from django.test import TestCase
from django.test.client import RequestFactory

Expand Down Expand Up @@ -92,15 +93,22 @@ def setUp(self):
self.old_user = UserFactory.create()
self.request = RequestFactory().post('/')
self.request.user = self.old_user
self.auto_linking_consumer = LtiConsumer(
consumer_name='AutoLinkingConsumer',
consumer_key='AutoLinkingKey',
consumer_secret='AutoLinkingSecret',
require_user_account=True
)
self.auto_linking_consumer.save()

def create_lti_user_model(self):
def create_lti_user_model(self, consumer=None):
"""
Generate and save a User and an LTI user model
"""
edx_user = User(username=self.edx_user_id)
edx_user.save()
lti_user = LtiUser(
lti_consumer=self.lti_consumer,
lti_consumer=consumer or self.lti_consumer,
lti_user_id=self.lti_user_id,
edx_user=edx_user
)
Expand Down Expand Up @@ -140,6 +148,38 @@ def test_authentication_with_wrong_user(self, create_user, switch_user):
assert not create_user.called
switch_user.assert_called_with(self.request, lti_user, self.lti_consumer)

def test_auto_linking_of_users_using_lis_person_contact_email_primary(self, create_user, switch_user):
request = RequestFactory().post("/", {"lis_person_contact_email_primary": self.old_user.email})
request.user = self.old_user

users.authenticate_lti_user(request, self.lti_user_id, self.lti_consumer)
create_user.assert_called_with(self.lti_user_id, self.lti_consumer)

users.authenticate_lti_user(request, self.lti_user_id, self.auto_linking_consumer)
create_user.assert_called_with(self.lti_user_id, self.auto_linking_consumer, self.old_user.email)

def test_raise_exception_trying_to_auto_link_unauthenticate_user(self, create_user, switch_user):
request = RequestFactory().post("/")
request.user = AnonymousUser()

with self.assertRaises(PermissionDenied):
users.authenticate_lti_user(request, self.lti_user_id, self.auto_linking_consumer)

def test_raise_exception_on_mismatched_user_and_lis_email(self, create_user, switch_user):
request = RequestFactory().post("/", {"lis_person_contact_email_primary": "[email protected]"})
request.user = self.old_user

with self.assertRaises(PermissionDenied):
users.authenticate_lti_user(request, self.lti_user_id, self.auto_linking_consumer)

def test_authenticate_unauthenticated_user_after_auto_linking_of_user_account(self, create_user, switch_user):
lti_user = self.create_lti_user_model(self.auto_linking_consumer)
self.request.user = AnonymousUser()

users.authenticate_lti_user(self.request, self.lti_user_id, self.auto_linking_consumer)
assert not create_user.called
switch_user.assert_called_with(self.request, lti_user, self.auto_linking_consumer)


class CreateLtiUserTest(TestCase):
"""
Expand All @@ -154,16 +194,17 @@ def setUp(self):
consumer_secret='TestSecret'
)
self.lti_consumer.save()
self.existing_user = UserFactory.create()

def test_create_lti_user_creates_auth_user_model(self):
users.create_lti_user('lti_user_id', self.lti_consumer)
assert User.objects.count() == 1
assert User.objects.count() == 2

@patch('uuid.uuid4', return_value='random_uuid')
@patch('lms.djangoapps.lti_provider.users.generate_random_edx_username', return_value='edx_id')
def test_create_lti_user_creates_correct_user(self, uuid_mock, _username_mock):
users.create_lti_user('lti_user_id', self.lti_consumer)
assert User.objects.count() == 1
assert User.objects.count() == 2
user = User.objects.get(username='edx_id')
assert user.email == '[email protected]'
uuid_mock.assert_called_with()
Expand All @@ -173,10 +214,34 @@ def test_unique_username_created(self, username_mock):
User(username='edx_id').save()
users.create_lti_user('lti_user_id', self.lti_consumer)
assert username_mock.call_count == 2
assert User.objects.count() == 2
assert User.objects.count() == 3
user = User.objects.get(username='new_edx_id')
assert user.email == '[email protected]'

def test_existing_user_is_linked(self):
lti_user = users.create_lti_user('lti_user_id', self.lti_consumer, self.existing_user.email)
assert lti_user.lti_consumer == self.lti_consumer
assert lti_user.edx_user == self.existing_user

def test_only_one_lti_user_edx_user_for_each_lti_consumer(self):
users.create_lti_user('lti_user_id', self.lti_consumer, self.existing_user.email)

with pytest.raises(IntegrityError):
users.create_lti_user('lti_user_id', self.lti_consumer, self.existing_user.email)

def test_create_multiple_lti_users_for_edx_user_if_lti_consumer_varies(self):
lti_consumer_2 = LtiConsumer(
consumer_name="SecondConsumer",
consumer_key="SecondKey",
consumer_secret="SecondSecret",
)
lti_consumer_2.save()

lti_user_1 = users.create_lti_user('lti_user_id', self.lti_consumer, self.existing_user.email)
lti_user_2 = users.create_lti_user('lti_user_id', lti_consumer_2, self.existing_user.email)

assert lti_user_1.edx_user == lti_user_2.edx_user


class LtiBackendTest(TestCase):
"""
Expand Down
41 changes: 40 additions & 1 deletion lms/djangoapps/lti_provider/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from unittest.mock import MagicMock, patch

from django.contrib.auth.models import AnonymousUser
from django.test import TestCase
from django.test.client import RequestFactory
from django.urls import reverse
Expand Down Expand Up @@ -58,7 +59,7 @@ def build_launch_request(extra_post_data=None, param_to_delete=None):
del post_data[param_to_delete]
request = RequestFactory().post('/', data=post_data)
request.user = UserFactory.create()
request.session = {}
request.session = MagicMock()
return request


Expand All @@ -82,6 +83,14 @@ def setUp(self):
)
self.consumer.save()

self.auto_link_consumer = models.LtiConsumer(
consumer_name='auto-link-consumer',
consumer_key='consumer_key_2',
consumer_secret='secret_2',
require_user_account=True
)
self.auto_link_consumer.save()


class LtiLaunchTest(LtiTestMixin, TestCase):
"""
Expand Down Expand Up @@ -189,6 +198,36 @@ def test_lti_consumer_record_supplemented_with_guid(self, _render):
)
assert consumer.instance_guid == 'consumer instance guid'

@patch('lms.djangoapps.lti_provider.views.render_to_response')
def test_unauthenticated_user_shown_error_when_require_user_account_is_enabled(self, render_error):
"""
Verify that an error page is shown instead of LTI Content for an unauthenticated user,
when the `require_user_account` flag is enabled for the LTI Consumer.
"""
request = build_launch_request({'oauth_consumer_key': 'consumer_key_2'})
request.user = AnonymousUser()

views.lti_launch(request, str(COURSE_KEY), str(USAGE_KEY))

render_error.assert_called()
assert render_error.call_args[0][0] == "lti_provider/user-auth-error.html"

@patch('lms.djangoapps.lti_provider.views.render_to_response')
def test_auth_error_shown_when_lis_email_is_different_from_user_email(self, render_error):
"""
When the `require_user_account` flag is enabled for the LTI Consumer, verify that
an error page is shown instead of LTI Content if the authenticated user's email
doesn't match the `lis_person_contact_email_primary` value from LTI Launch.
"""
# lis email different from logged in user
request = build_launch_request({
'oauth_consumer_key': 'consumer_key_2',
'lis_person_contact_email_primary': '[email protected]'
})

views.lti_launch(request, str(COURSE_KEY), str(USAGE_KEY))
render_error.assert_called()


class LtiLaunchTestRender(LtiTestMixin, RenderXBlockTestMixin, ModuleStoreTestCase):
"""
Expand Down
63 changes: 38 additions & 25 deletions lms/djangoapps/lti_provider/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,25 @@ def authenticate_lti_user(request, lti_user_id, lti_consumer):
If the currently logged-in user does not match the user specified by the LTI
launch, log out the old user and log in the LTI identity.
"""
lis_email = request.POST.get("lis_person_contact_email_primary")

try:
lti_user = LtiUser.objects.get(
lti_user_id=lti_user_id,
lti_consumer=lti_consumer
)
except LtiUser.DoesNotExist:
except LtiUser.DoesNotExist as exc:
# This is the first time that the user has been here. Create an account.
lti_user = create_lti_user(lti_user_id, lti_consumer)
if lti_consumer.require_user_account:
# Verify that the email from the LTI Launch and the logged-in user are the same
# before linking the LtiUser with the edx_user.
if request.user.is_authenticated and request.user.email == lis_email:
lti_user = create_lti_user(lti_user_id, lti_consumer, lis_email)
else:
# Ask the user to login before linking.
raise PermissionDenied() from exc
else:
lti_user = create_lti_user(lti_user_id, lti_consumer)

if not (request.user.is_authenticated and
request.user == lti_user.edx_user):
Expand All @@ -44,34 +55,36 @@ def authenticate_lti_user(request, lti_user_id, lti_consumer):
switch_user(request, lti_user, lti_consumer)


def create_lti_user(lti_user_id, lti_consumer):
def create_lti_user(lti_user_id, lti_consumer, email=None):
"""
Generate a new user on the edX platform with a random username and password,
and associates that account with the LTI identity.
"""
edx_password = str(uuid.uuid4())
edx_user = User.objects.filter(email=email).first() if email else None

created = False
while not created:
try:
edx_user_id = generate_random_edx_username()
edx_email = f"{edx_user_id}@{settings.LTI_USER_EMAIL_DOMAIN}"
with transaction.atomic():
edx_user = User.objects.create_user(
username=edx_user_id,
password=edx_password,
email=edx_email,
)
# 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
# False, we will retry with a different random ID.
pass
if not edx_user:
created = False
edx_password = str(uuid.uuid4())
while not created:
try:
edx_user_id = generate_random_edx_username()
edx_email = f"{edx_user_id}@{settings.LTI_USER_EMAIL_DOMAIN}"
with transaction.atomic():
edx_user = User.objects.create_user(
username=edx_user_id,
password=edx_password,
email=edx_email,
)
# 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
# False, we will retry with a different random ID.
pass

lti_user = LtiUser(
lti_consumer=lti_consumer,
Expand Down
14 changes: 13 additions & 1 deletion lms/djangoapps/lti_provider/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
import logging

from django.conf import settings
from django.core.exceptions import PermissionDenied
from django.http import Http404, HttpResponseBadRequest, HttpResponseForbidden
from django.views.decorators.csrf import csrf_exempt
from common.djangoapps.edxmako.shortcuts import render_to_response
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey, UsageKey

Expand Down Expand Up @@ -88,7 +90,17 @@ def lti_launch(request, course_id, usage_id):

# Create an edX account if the user identifed by the LTI launch doesn't have
# one already, and log the edX account into the platform.
authenticate_lti_user(request, params['user_id'], lti_consumer)
try:
authenticate_lti_user(request, params['user_id'], lti_consumer)
except PermissionDenied:
request.session.flush()
context = {
"login_link": request.build_absolute_uri(settings.LOGIN_URL),
"allow_iframing": True,
"disable_header": True,
"disable_footer": True,
}
return render_to_response("lti_provider/user-auth-error.html", context)

# Store any parameters required by the outcome service in order to report
# scores back later. We know that the consumer exists, since the record was
Expand Down
Loading

0 comments on commit 5b2f012

Please sign in to comment.