Skip to content

Commit

Permalink
Merge pull request #220 from CUCWD/feature.maple/ztraboo/merge-lti-ed…
Browse files Browse the repository at this point in the history
…x-authentication-using-email

Link existing platform users to the LtiUser via email `lis_person_contact_email_primary` payload data.
  • Loading branch information
ztraboo authored Sep 28, 2023
2 parents 31829e2 + f1f9fc0 commit 03614fd
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 27 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 3.2.21 on 2023-09-21 12:18

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('lti_provider', '0003_auto_20161118_1040'),
]

operations = [
migrations.AddField(
model_name='lticonsumer',
name='auto_link_users_using_email',
field=models.BooleanField(blank=True, default=False),
),
]
1 change: 1 addition & 0 deletions lms/djangoapps/lti_provider/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ 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)
auto_link_users_using_email = models.BooleanField(blank=True, default=False)

@staticmethod
def get_or_supplement(instance_guid, consumer_key):
Expand Down
26 changes: 22 additions & 4 deletions lms/djangoapps/lti_provider/tests/test_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def test_authentication_with_new_user(self, _create_user, switch_user):
lti_user.edx_user_id = self.edx_user_id
with patch('lms.djangoapps.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)
create_user.assert_called_with(self.lti_user_id, self.lti_consumer, "")
switch_user.assert_called_with(self.request, lti_user, self.lti_consumer)

def test_authentication_with_authenticated_user(self, create_user, switch_user):
Expand Down Expand Up @@ -140,6 +140,18 @@ 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, "")

self.lti_consumer.auto_link_users_using_email = True
self.lti_consumer.save()
users.authenticate_lti_user(request, self.lti_user_id, self.lti_consumer)
create_user.assert_called_with(self.lti_user_id, self.lti_consumer, self.old_user.email)


class CreateLtiUserTest(TestCase):
"""
Expand All @@ -154,16 +166,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 +186,15 @@ 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


class LtiBackendTest(TestCase):
"""
Expand Down
54 changes: 31 additions & 23 deletions lms/djangoapps/lti_provider/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ def authenticate_lti_user(request, lti_user_id, lti_consumer):
)
except LtiUser.DoesNotExist:
# This is the first time that the user has been here. Create an account.
lti_user = create_lti_user(lti_user_id, lti_consumer)
lis_email = ""
if lti_consumer.auto_link_users_using_email:
lis_email = request.POST.get("lis_person_contact_email_primary", "")
lti_user = create_lti_user(lti_user_id, lti_consumer, lis_email)
log.info(
"authenticate_lti_user %s",
lti_user
Expand All @@ -51,34 +54,39 @@ 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=""):
"""
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())

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
existing_user = User.objects.filter(email=email).first() if email else None

if existing_user:
edx_user = existing_user
else:
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

lti_user = LtiUser(
lti_consumer=lti_consumer,
Expand Down
2 changes: 2 additions & 0 deletions lms/djangoapps/lti_provider/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

from django.conf import settings
from django.http import Http404, HttpResponseBadRequest, HttpResponseForbidden
from django.views.decorators.clickjacking import xframe_options_exempt
from django.views.decorators.csrf import csrf_exempt
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey, UsageKey
Expand Down Expand Up @@ -38,6 +39,7 @@

@csrf_exempt
@add_p3p_header
@xframe_options_exempt
def lti_launch(request, course_id, usage_id):
"""
Endpoint for all requests to embed edX content via the LTI protocol. This
Expand Down

0 comments on commit 03614fd

Please sign in to comment.