feat: Add support for using LTI data to populate user profile
Currently the LTI provider implementation auto-creates a random user when logging in, however, the LTI launch can include relevant user details such as their email, full name and even a username. This change makes the LTI code use the provided details if the "Use lti pii" setting is set in the Django admin.
This commit is contained in:
committed by
Farhaan Bukhsh
parent
a4d4ddf9e3
commit
0bed7d7127
@@ -0,0 +1,18 @@
|
||||
# Generated by Django 4.2.23 on 2025-08-29 12:43
|
||||
|
||||
from django.db import migrations, models
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('lti_provider', '0004_require_user_account'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.AddField(
|
||||
model_name='lticonsumer',
|
||||
name='use_lti_pii',
|
||||
field=models.BooleanField(blank=True, default=False, help_text='When checked, the platform will automatically use any personal information provided via LTI to create an account. Otherwise an anonymous account will be created.'),
|
||||
),
|
||||
]
|
||||
@@ -40,6 +40,10 @@ class LtiConsumer(models.Model):
|
||||
"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."
|
||||
))
|
||||
use_lti_pii = models.BooleanField(blank=True, default=False, help_text=_(
|
||||
"When checked, the platform will automatically use any personal information provided "
|
||||
"via LTI to create an account. Otherwise an anonymous account will be created."
|
||||
))
|
||||
|
||||
@staticmethod
|
||||
def get_or_supplement(instance_guid, consumer_key):
|
||||
|
||||
@@ -2,10 +2,11 @@
|
||||
Tests for the LTI user management functionality
|
||||
"""
|
||||
|
||||
|
||||
import itertools
|
||||
import string
|
||||
from unittest.mock import MagicMock, PropertyMock, patch
|
||||
|
||||
import ddt
|
||||
import pytest
|
||||
from django.contrib.auth.models import AnonymousUser, User # lint-amnesty, pylint: disable=imported-auth-user
|
||||
from django.core.exceptions import PermissionDenied
|
||||
@@ -73,6 +74,7 @@ class UserManagementHelperTest(TestCase):
|
||||
f"Username has forbidden character '{username[char]}'"
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
@patch('lms.djangoapps.lti_provider.users.switch_user', autospec=True)
|
||||
@patch('lms.djangoapps.lti_provider.users.create_lti_user', autospec=True)
|
||||
class AuthenticateLtiUserTest(TestCase):
|
||||
@@ -156,7 +158,10 @@ class AuthenticateLtiUserTest(TestCase):
|
||||
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)
|
||||
create_user.assert_called_with(self.lti_user_id, self.auto_linking_consumer, {
|
||||
"email": self.old_user.email,
|
||||
"full_name": "",
|
||||
})
|
||||
|
||||
def test_auto_linking_of_users_using_lis_person_contact_email_primary_case_insensitive(self, create_user, switch_user): # pylint: disable=line-too-long
|
||||
request = RequestFactory().post("/", {"lis_person_contact_email_primary": self.old_user.email.upper()})
|
||||
@@ -166,7 +171,10 @@ class AuthenticateLtiUserTest(TestCase):
|
||||
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, request.user.email)
|
||||
create_user.assert_called_with(self.lti_user_id, self.auto_linking_consumer, {
|
||||
"email": self.old_user.email,
|
||||
"full_name": "",
|
||||
})
|
||||
|
||||
def test_raise_exception_trying_to_auto_link_unauthenticate_user(self, create_user, switch_user):
|
||||
request = RequestFactory().post("/")
|
||||
@@ -190,7 +198,57 @@ class AuthenticateLtiUserTest(TestCase):
|
||||
assert not create_user.called
|
||||
switch_user.assert_called_with(self.request, lti_user, self.auto_linking_consumer)
|
||||
|
||||
@ddt.data(
|
||||
*itertools.product(
|
||||
(
|
||||
(
|
||||
{
|
||||
"lis_person_contact_email_primary": "some_email@example.com",
|
||||
"lis_person_name_given": "John",
|
||||
"lis_person_name_family": "Doe",
|
||||
},
|
||||
"some_email@example.com",
|
||||
"John Doe",
|
||||
),
|
||||
(
|
||||
{
|
||||
"lis_person_contact_email_primary": "some_email@example.com",
|
||||
"lis_person_name_full": "John Doe",
|
||||
"lis_person_name_given": "Jacob",
|
||||
},
|
||||
"some_email@example.com",
|
||||
"John Doe",
|
||||
),
|
||||
(
|
||||
{"lis_person_contact_email_primary": "some_email@example.com", "lis_person_name_full": "John Doe"},
|
||||
"some_email@example.com",
|
||||
"John Doe",
|
||||
),
|
||||
({"lis_person_contact_email_primary": "some_email@example.com"}, "some_email@example.com", ""),
|
||||
({"lis_person_contact_email_primary": ""}, "", ""),
|
||||
({"lis_person_contact_email_primary": ""}, "", ""),
|
||||
({}, "", ""),
|
||||
),
|
||||
[True, False],
|
||||
)
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_create_user_when_user_account_not_required(self, params, enable_lti_pii, create_user, switch_user):
|
||||
post_params, email, name = params
|
||||
self.auto_linking_consumer.require_user_account = False
|
||||
self.auto_linking_consumer.use_lti_pii = enable_lti_pii
|
||||
self.auto_linking_consumer.save()
|
||||
request = RequestFactory().post("/", post_params)
|
||||
request.user = AnonymousUser()
|
||||
users.authenticate_lti_user(request, self.lti_user_id, self.auto_linking_consumer)
|
||||
if enable_lti_pii:
|
||||
profile = {"email": email, "full_name": name, "username": self.lti_user_id}
|
||||
create_user.assert_called_with(self.lti_user_id, self.auto_linking_consumer, profile)
|
||||
else:
|
||||
create_user.assert_called_with(self.lti_user_id, self.auto_linking_consumer)
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class CreateLtiUserTest(TestCase):
|
||||
"""
|
||||
Tests for the create_lti_user function in users.py
|
||||
@@ -222,22 +280,22 @@ class CreateLtiUserTest(TestCase):
|
||||
@patch('lms.djangoapps.lti_provider.users.generate_random_edx_username', side_effect=['edx_id', 'new_edx_id'])
|
||||
def test_unique_username_created(self, username_mock):
|
||||
User(username='edx_id').save()
|
||||
users.create_lti_user('lti_user_id', self.lti_consumer)
|
||||
users.create_lti_user('lti_user_id', self.lti_consumer, None)
|
||||
assert username_mock.call_count == 2
|
||||
assert User.objects.count() == 3
|
||||
user = User.objects.get(username='new_edx_id')
|
||||
assert user.email == 'new_edx_id@lti.example.com'
|
||||
|
||||
def test_existing_user_is_linked(self):
|
||||
lti_user = users.create_lti_user('lti_user_id', self.lti_consumer, self.existing_user.email)
|
||||
lti_user = users.create_lti_user('lti_user_id', self.lti_consumer, {"email": 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)
|
||||
users.create_lti_user('lti_user_id', self.lti_consumer, {"email": self.existing_user.email})
|
||||
|
||||
with pytest.raises(IntegrityError):
|
||||
users.create_lti_user('lti_user_id', self.lti_consumer, self.existing_user.email)
|
||||
users.create_lti_user('lti_user_id', self.lti_consumer, {"email": self.existing_user.email})
|
||||
|
||||
def test_create_multiple_lti_users_for_edx_user_if_lti_consumer_varies(self):
|
||||
lti_consumer_2 = LtiConsumer(
|
||||
@@ -247,11 +305,42 @@ class CreateLtiUserTest(TestCase):
|
||||
)
|
||||
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)
|
||||
lti_user_1 = users.create_lti_user('lti_user_id', self.lti_consumer, {"email": self.existing_user.email})
|
||||
lti_user_2 = users.create_lti_user('lti_user_id', lti_consumer_2, {"email": self.existing_user.email})
|
||||
|
||||
assert lti_user_1.edx_user == lti_user_2.edx_user
|
||||
|
||||
def test_create_lti_user_with_full_profile(self):
|
||||
lti_user = users.create_lti_user('lti_user_id', self.lti_consumer, {
|
||||
"email": "some.user@example.com",
|
||||
"full_name": "John Doe",
|
||||
"username": "john_doe",
|
||||
})
|
||||
assert lti_user.edx_user.email == "some.user@example.com"
|
||||
assert lti_user.edx_user.username == "john_doe"
|
||||
assert lti_user.edx_user.profile.name == "John Doe"
|
||||
|
||||
@patch('lms.djangoapps.lti_provider.users.generate_random_edx_username', side_effect=['edx_id'])
|
||||
def test_create_lti_user_with_missing_username_in_profile(self, mock):
|
||||
lti_user = users.create_lti_user('lti_user_id', self.lti_consumer, {
|
||||
"email": "some.user@example.com",
|
||||
"full_name": "John Doe",
|
||||
})
|
||||
assert lti_user.edx_user.email == "some.user@example.com"
|
||||
assert lti_user.edx_user.username == "edx_id"
|
||||
assert lti_user.edx_user.profile.name == "John Doe"
|
||||
|
||||
@patch('lms.djangoapps.lti_provider.users.generate_random_edx_username', side_effect=['edx_id', 'edx_id123'])
|
||||
def test_create_lti_user_with_duplicate_username_in_profile(self, mock):
|
||||
lti_user = users.create_lti_user('lti_user_id', self.lti_consumer, {
|
||||
"email": "some.user@example.com",
|
||||
"full_name": "John Doe",
|
||||
"username": self.existing_user.username,
|
||||
})
|
||||
assert lti_user.edx_user.email == "some.user@example.com"
|
||||
assert lti_user.edx_user.username == "edx_id"
|
||||
assert lti_user.edx_user.profile.name == "John Doe"
|
||||
|
||||
|
||||
class LtiBackendTest(TestCase):
|
||||
"""
|
||||
|
||||
@@ -19,6 +19,20 @@ from lms.djangoapps.lti_provider.models import LtiUser
|
||||
from openedx.core.djangoapps.safe_sessions.middleware import mark_user_change_as_expected
|
||||
|
||||
|
||||
def get_lti_user_details(request):
|
||||
"""
|
||||
Returns key LTI user details from the LTI launch request.
|
||||
"""
|
||||
post_data = request.POST
|
||||
email = post_data.get("lis_person_contact_email_primary", "").lower()
|
||||
full_name = post_data.get("lis_person_name_full", "")
|
||||
given_name = post_data.get("lis_person_name_given", "")
|
||||
family_name = post_data.get("lis_person_name_family", "")
|
||||
if not full_name and given_name:
|
||||
full_name = f"{given_name} {family_name}"
|
||||
return dict(email=email, full_name=full_name)
|
||||
|
||||
|
||||
def authenticate_lti_user(request, lti_user_id, lti_consumer):
|
||||
"""
|
||||
Determine whether the user specified by the LTI launch has an existing
|
||||
@@ -28,7 +42,7 @@ 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")
|
||||
profile = get_lti_user_details(request)
|
||||
|
||||
try:
|
||||
lti_user = LtiUser.objects.get(
|
||||
@@ -40,11 +54,14 @@ def authenticate_lti_user(request, 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.lower() == lis_email.lower():
|
||||
lti_user = create_lti_user(lti_user_id, lti_consumer, request.user.email)
|
||||
if request.user.is_authenticated and request.user.email.lower() == profile["email"]:
|
||||
lti_user = create_lti_user(lti_user_id, lti_consumer, profile)
|
||||
else:
|
||||
# Ask the user to login before linking.
|
||||
raise PermissionDenied() from exc
|
||||
elif lti_consumer.use_lti_pii:
|
||||
profile["username"] = lti_user_id
|
||||
lti_user = create_lti_user(lti_user_id, lti_consumer, profile)
|
||||
else:
|
||||
lti_user = create_lti_user(lti_user_id, lti_consumer)
|
||||
|
||||
@@ -55,11 +72,15 @@ 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, email=None):
|
||||
def create_lti_user(lti_user_id, lti_consumer, profile=None):
|
||||
"""
|
||||
Generate a new user on the edX platform with a random username and password,
|
||||
and associates that account with the LTI identity.
|
||||
"""
|
||||
if profile is None:
|
||||
profile = {}
|
||||
email = profile.get("email")
|
||||
edx_user_id = profile.get("username") or generate_random_edx_username()
|
||||
edx_user = User.objects.filter(email=email).first() if email else None
|
||||
|
||||
if not edx_user:
|
||||
@@ -67,8 +88,7 @@ def create_lti_user(lti_user_id, lti_consumer, email=None):
|
||||
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}"
|
||||
edx_email = email if email else f"{edx_user_id}@{settings.LTI_USER_EMAIL_DOMAIN}"
|
||||
with transaction.atomic():
|
||||
edx_user = User.objects.create_user(
|
||||
username=edx_user_id,
|
||||
@@ -78,13 +98,13 @@ def create_lti_user(lti_user_id, lti_consumer, email=None):
|
||||
# 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 = UserProfile(user=edx_user, name=profile.get("full_name", ""))
|
||||
edx_user_profile.save()
|
||||
created = True
|
||||
except IntegrityError:
|
||||
edx_user_id = generate_random_edx_username()
|
||||
# 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,
|
||||
|
||||
@@ -35,7 +35,9 @@ REQUIRED_PARAMETERS = [
|
||||
|
||||
OPTIONAL_PARAMETERS = [
|
||||
'context_title', 'context_label', 'lis_result_sourcedid',
|
||||
'lis_outcome_service_url', 'tool_consumer_instance_guid'
|
||||
'lis_outcome_service_url', 'tool_consumer_instance_guid',
|
||||
"lis_person_name_full", "lis_person_name_given", "lis_person_name_family",
|
||||
"lis_person_contact_email_primary",
|
||||
]
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user