From 19f320450953de7f195423ab851da01033aa45ef Mon Sep 17 00:00:00 2001
From: Arunmozhi <arun@arunmozhi.in>
Date: Thu, 21 Sep 2023 17:50:41 +0530
Subject: [PATCH 01/19] feat: adds auto_link_users_using_email field to
 LtiConsumer

---
 ..._lticonsumer_auto_link_users_using_email.py | 18 ++++++++++++++++++
 lms/djangoapps/lti_provider/models.py          |  1 +
 2 files changed, 19 insertions(+)
 create mode 100644 lms/djangoapps/lti_provider/migrations/0004_lticonsumer_auto_link_users_using_email.py

diff --git a/lms/djangoapps/lti_provider/migrations/0004_lticonsumer_auto_link_users_using_email.py b/lms/djangoapps/lti_provider/migrations/0004_lticonsumer_auto_link_users_using_email.py
new file mode 100644
index 000000000000..2ea7f88244ce
--- /dev/null
+++ b/lms/djangoapps/lti_provider/migrations/0004_lticonsumer_auto_link_users_using_email.py
@@ -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),
+        ),
+    ]
diff --git a/lms/djangoapps/lti_provider/models.py b/lms/djangoapps/lti_provider/models.py
index 0cb165eeef69..93bde45d63bd 100644
--- a/lms/djangoapps/lti_provider/models.py
+++ b/lms/djangoapps/lti_provider/models.py
@@ -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):

From b85e743f1b9cfd630d195e76a41d20597e9e64d7 Mon Sep 17 00:00:00 2001
From: Arunmozhi <arun@arunmozhi.in>
Date: Mon, 25 Sep 2023 17:20:30 +0530
Subject: [PATCH 02/19] feat: link existing users to the LtiUser via email

With this change the platform users who access content via LTI will
automatically be linked their platform account instead of the anonymous
account when the following conditions are met:
* the LtiConsumer should be configured to auto link the users via email
* the LTI Consumer should share the email of the user using the
lis_person_contact_email_primary parameter in the LTI Launch POST data

Internal-ref: https://tasks.opencraft.com/browse/BB-7875
---
 .../lti_provider/tests/test_users.py          | 26 +++++++--
 lms/djangoapps/lti_provider/users.py          | 54 +++++++++++--------
 2 files changed, 53 insertions(+), 27 deletions(-)

diff --git a/lms/djangoapps/lti_provider/tests/test_users.py b/lms/djangoapps/lti_provider/tests/test_users.py
index d945f6b9a65a..867d4874e27a 100644
--- a/lms/djangoapps/lti_provider/tests/test_users.py
+++ b/lms/djangoapps/lti_provider/tests/test_users.py
@@ -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):
@@ -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):
     """
@@ -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 == 'edx_id@lti.example.com'
         uuid_mock.assert_called_with()
@@ -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 == '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)
+        assert lti_user.lti_consumer == self.lti_consumer
+        assert lti_user.edx_user == self.existing_user
+
 
 class LtiBackendTest(TestCase):
     """
diff --git a/lms/djangoapps/lti_provider/users.py b/lms/djangoapps/lti_provider/users.py
index 7c5a9c88ebaa..566fa22feb20 100644
--- a/lms/djangoapps/lti_provider/users.py
+++ b/lms/djangoapps/lti_provider/users.py
@@ -35,7 +35,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)
 
     if not (request.user.is_authenticated and
             request.user == lti_user.edx_user):
@@ -44,34 +47,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,

From 198f9797ae194e392e6282e1d58c3fc6959c85ae Mon Sep 17 00:00:00 2001
From: Arunmozhi <arun@arunmozhi.in>
Date: Thu, 28 Sep 2023 17:20:12 +0530
Subject: [PATCH 03/19] refactor: use None as a default value in
 create_lti_user

---
 lms/djangoapps/lti_provider/users.py | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/lms/djangoapps/lti_provider/users.py b/lms/djangoapps/lti_provider/users.py
index 566fa22feb20..3af42fa180e0 100644
--- a/lms/djangoapps/lti_provider/users.py
+++ b/lms/djangoapps/lti_provider/users.py
@@ -35,10 +35,11 @@ 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.
-        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)
+            lis_email = request.POST.get("lis_person_contact_email_primary")
+            lti_user = create_lti_user(lti_user_id, lti_consumer, lis_email)
+        else:
+            lti_user = create_lti_user(lti_user_id, lti_consumer)
 
     if not (request.user.is_authenticated and
             request.user == lti_user.edx_user):
@@ -47,19 +48,16 @@ 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=""):
+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
 
-    existing_user = User.objects.filter(email=email).first() if email else None
-
-    if existing_user:
-        edx_user = existing_user
-    else:
+    if not edx_user:
         created = False
+        edx_password = str(uuid.uuid4())
         while not created:
             try:
                 edx_user_id = generate_random_edx_username()

From 47922e4e6d559fe9cfb9167624dcb0d7b20b32df Mon Sep 17 00:00:00 2001
From: Arunmozhi <arun@arunmozhi.in>
Date: Thu, 28 Sep 2023 17:50:35 +0530
Subject: [PATCH 04/19] feat: make the auto link flag editable only during
 object creation

---
 lms/djangoapps/lti_provider/admin.py          |  6 ++++
 .../lti_provider/tests/test_admin.py          | 31 +++++++++++++++++++
 2 files changed, 37 insertions(+)
 create mode 100644 lms/djangoapps/lti_provider/tests/test_admin.py

diff --git a/lms/djangoapps/lti_provider/admin.py b/lms/djangoapps/lti_provider/admin.py
index 7776b4821ff2..c3610e9d0a1d 100644
--- a/lms/djangoapps/lti_provider/admin.py
+++ b/lms/djangoapps/lti_provider/admin.py
@@ -13,4 +13,10 @@ class LtiConsumerAdmin(admin.ModelAdmin):
     search_fields = ('consumer_name', 'consumer_key', 'instance_guid')
     list_display = ('id', 'consumer_name', 'consumer_key', 'instance_guid')
 
+    def get_readonly_fields(self, request, obj=None):
+        if obj and obj.pk:
+            return ("auto_link_users_using_email",)
+        return super().get_readonly_fields(request, obj)
+
+
 admin.site.register(LtiConsumer, LtiConsumerAdmin)
diff --git a/lms/djangoapps/lti_provider/tests/test_admin.py b/lms/djangoapps/lti_provider/tests/test_admin.py
new file mode 100644
index 000000000000..7c2d1d18381c
--- /dev/null
+++ b/lms/djangoapps/lti_provider/tests/test_admin.py
@@ -0,0 +1,31 @@
+"""
+Tests for the LTI Provider's Admin Views
+"""
+from unittest.mock import Mock
+
+from django.contrib.admin.sites import AdminSite
+from django.test import TestCase
+
+from lms.djangoapps.lti_provider.admin import LtiConsumerAdmin
+from lms.djangoapps.lti_provider.models import LtiConsumer
+
+
+class LtiConsumerAdminTests(TestCase):
+    """
+    Test the customizations applied for the LtiConsumerAdmin
+    """
+    def setUp(self):
+        self.site = AdminSite()
+        self.consumer = LtiConsumer(
+            consumer_name="Test Consumer",
+            consumer_key="test-key",
+            consumer_secret="secret",
+        )
+        self.consumer.save()
+
+    def test_lticonsumeradmin_read_only_fields(self):
+        ma = LtiConsumerAdmin(LtiConsumer, self.site)
+        request = Mock()
+
+        self.assertEqual(ma.get_readonly_fields(request, None), ())
+        self.assertEqual(ma.get_readonly_fields(request, self.consumer), ('auto_link_users_using_email',))

From 50a4a6ffc9ea36df3e9925a7674b15c36b7925a1 Mon Sep 17 00:00:00 2001
From: Arunmozhi <arun@arunmozhi.in>
Date: Thu, 28 Sep 2023 19:04:50 +0530
Subject: [PATCH 05/19] fix: remove the empty strings as args from the tests

---
 lms/djangoapps/lti_provider/tests/test_users.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lms/djangoapps/lti_provider/tests/test_users.py b/lms/djangoapps/lti_provider/tests/test_users.py
index 867d4874e27a..f807013dee78 100644
--- a/lms/djangoapps/lti_provider/tests/test_users.py
+++ b/lms/djangoapps/lti_provider/tests/test_users.py
@@ -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):
@@ -145,7 +145,7 @@ def test_auto_linking_of_users_using_lis_person_contact_email_primary(self, crea
         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, "")
+        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()

From f9857ad895c4543516c1a50f309a533d52edad88 Mon Sep 17 00:00:00 2001
From: Arunmozhi <arun@arunmozhi.in>
Date: Fri, 29 Sep 2023 12:57:37 +0530
Subject: [PATCH 06/19] fix: remove unique constraint on edx_user

With the auto linking of edx_user with the lti_users, the scenario where
multiple LTI consumers will create independent LtiUsers depending on the
same edx_user is created. This will not be possible if the edx_user has
a one-to-one relationship with the lti_user. This commit replaces the
one-to-one relationship with an one-to-many relationship so that
multiple LtiUser objects can be created referencing the same edx_user.
---
 .../0004_auto_link_users_using_email.py       | 26 +++++++++++++++++++
 ...lticonsumer_auto_link_users_using_email.py | 18 -------------
 lms/djangoapps/lti_provider/models.py         |  2 +-
 .../lti_provider/tests/test_users.py          | 20 ++++++++++++++
 4 files changed, 47 insertions(+), 19 deletions(-)
 create mode 100644 lms/djangoapps/lti_provider/migrations/0004_auto_link_users_using_email.py
 delete mode 100644 lms/djangoapps/lti_provider/migrations/0004_lticonsumer_auto_link_users_using_email.py

diff --git a/lms/djangoapps/lti_provider/migrations/0004_auto_link_users_using_email.py b/lms/djangoapps/lti_provider/migrations/0004_auto_link_users_using_email.py
new file mode 100644
index 000000000000..c9b9a346b560
--- /dev/null
+++ b/lms/djangoapps/lti_provider/migrations/0004_auto_link_users_using_email.py
@@ -0,0 +1,26 @@
+# Generated by Django 3.2.21 on 2023-09-29 07:22
+
+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='auto_link_users_using_email',
+            field=models.BooleanField(blank=True, default=False),
+        ),
+        migrations.AlterField(
+            model_name='ltiuser',
+            name='edx_user',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL),
+        ),
+    ]
diff --git a/lms/djangoapps/lti_provider/migrations/0004_lticonsumer_auto_link_users_using_email.py b/lms/djangoapps/lti_provider/migrations/0004_lticonsumer_auto_link_users_using_email.py
deleted file mode 100644
index 2ea7f88244ce..000000000000
--- a/lms/djangoapps/lti_provider/migrations/0004_lticonsumer_auto_link_users_using_email.py
+++ /dev/null
@@ -1,18 +0,0 @@
-# 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),
-        ),
-    ]
diff --git a/lms/djangoapps/lti_provider/models.py b/lms/djangoapps/lti_provider/models.py
index 93bde45d63bd..ba8fda5c5283 100644
--- a/lms/djangoapps/lti_provider/models.py
+++ b/lms/djangoapps/lti_provider/models.py
@@ -141,7 +141,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')
diff --git a/lms/djangoapps/lti_provider/tests/test_users.py b/lms/djangoapps/lti_provider/tests/test_users.py
index f807013dee78..d8761658d378 100644
--- a/lms/djangoapps/lti_provider/tests/test_users.py
+++ b/lms/djangoapps/lti_provider/tests/test_users.py
@@ -9,6 +9,7 @@
 import pytest
 from django.contrib.auth.models import 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
 
@@ -195,6 +196,25 @@ def test_existing_user_is_linked(self):
         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):
     """

From a589dee5ecc8124dd063645fce2f9e54b3c3c111 Mon Sep 17 00:00:00 2001
From: Arunmozhi <arun@arunmozhi.in>
Date: Fri, 27 Oct 2023 12:13:12 +0530
Subject: [PATCH 07/19] refactor: remove read-only from Lti Consumer config
 admin

---
 lms/djangoapps/lti_provider/admin.py          |  5 ---
 .../lti_provider/tests/test_admin.py          | 31 -------------------
 2 files changed, 36 deletions(-)
 delete mode 100644 lms/djangoapps/lti_provider/tests/test_admin.py

diff --git a/lms/djangoapps/lti_provider/admin.py b/lms/djangoapps/lti_provider/admin.py
index c3610e9d0a1d..1311ae70fc00 100644
--- a/lms/djangoapps/lti_provider/admin.py
+++ b/lms/djangoapps/lti_provider/admin.py
@@ -13,10 +13,5 @@ class LtiConsumerAdmin(admin.ModelAdmin):
     search_fields = ('consumer_name', 'consumer_key', 'instance_guid')
     list_display = ('id', 'consumer_name', 'consumer_key', 'instance_guid')
 
-    def get_readonly_fields(self, request, obj=None):
-        if obj and obj.pk:
-            return ("auto_link_users_using_email",)
-        return super().get_readonly_fields(request, obj)
-
 
 admin.site.register(LtiConsumer, LtiConsumerAdmin)
diff --git a/lms/djangoapps/lti_provider/tests/test_admin.py b/lms/djangoapps/lti_provider/tests/test_admin.py
deleted file mode 100644
index 7c2d1d18381c..000000000000
--- a/lms/djangoapps/lti_provider/tests/test_admin.py
+++ /dev/null
@@ -1,31 +0,0 @@
-"""
-Tests for the LTI Provider's Admin Views
-"""
-from unittest.mock import Mock
-
-from django.contrib.admin.sites import AdminSite
-from django.test import TestCase
-
-from lms.djangoapps.lti_provider.admin import LtiConsumerAdmin
-from lms.djangoapps.lti_provider.models import LtiConsumer
-
-
-class LtiConsumerAdminTests(TestCase):
-    """
-    Test the customizations applied for the LtiConsumerAdmin
-    """
-    def setUp(self):
-        self.site = AdminSite()
-        self.consumer = LtiConsumer(
-            consumer_name="Test Consumer",
-            consumer_key="test-key",
-            consumer_secret="secret",
-        )
-        self.consumer.save()
-
-    def test_lticonsumeradmin_read_only_fields(self):
-        ma = LtiConsumerAdmin(LtiConsumer, self.site)
-        request = Mock()
-
-        self.assertEqual(ma.get_readonly_fields(request, None), ())
-        self.assertEqual(ma.get_readonly_fields(request, self.consumer), ('auto_link_users_using_email',))

From 1f3473777c165b5e481cfc78dbb054c2d7f13c2a Mon Sep 17 00:00:00 2001
From: Arunmozhi <arun@arunmozhi.in>
Date: Mon, 30 Oct 2023 18:38:07 +0530
Subject: [PATCH 08/19] fix: allow only authenticated users to be auto-linked

---
 .../lti_provider/tests/test_users.py          | 11 +++++++
 .../lti_provider/tests/test_views.py          | 32 ++++++++++++++++++-
 lms/djangoapps/lti_provider/users.py          | 10 ++++++
 lms/djangoapps/lti_provider/views.py          | 13 ++++++++
 .../lti_provider/user-auth-error.html         | 24 ++++++++++++++
 5 files changed, 89 insertions(+), 1 deletion(-)
 create mode 100644 lms/templates/lti_provider/user-auth-error.html

diff --git a/lms/djangoapps/lti_provider/tests/test_users.py b/lms/djangoapps/lti_provider/tests/test_users.py
index d8761658d378..10b02cf1f601 100644
--- a/lms/djangoapps/lti_provider/tests/test_users.py
+++ b/lms/djangoapps/lti_provider/tests/test_users.py
@@ -153,6 +153,17 @@ def test_auto_linking_of_users_using_lis_person_contact_email_primary(self, crea
         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)
 
+    def test_switch_the_associated_edx_user_when_auto_linking_existing_user(self, create_user, switch_user):
+        lti_user = self.create_lti_user_model()
+        new_user = UserFactory.create()
+        self.lti_consumer.auto_link_users_using_email = True
+        self.lti_consumer.save()
+        request = RequestFactory().post("/", {"lis_person_contact_email_primary": new_user.email})
+        request.user = new_user
+
+        users.authenticate_lti_user(request, self.lti_user_id, self.lti_consumer)
+        assert LtiUser.objects.get(pk=lti_user.id).edx_user == new_user
+
 
 class CreateLtiUserTest(TestCase):
     """
diff --git a/lms/djangoapps/lti_provider/tests/test_views.py b/lms/djangoapps/lti_provider/tests/test_views.py
index ebb7eda154d6..8dbff97a1e61 100644
--- a/lms/djangoapps/lti_provider/tests/test_views.py
+++ b/lms/djangoapps/lti_provider/tests/test_views.py
@@ -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
@@ -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
 
 
@@ -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',
+            auto_link_users_using_email=True
+        )
+        self.auto_link_consumer.save()
+
 
 class LtiLaunchTest(LtiTestMixin, TestCase):
     """
@@ -189,6 +198,27 @@ 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_auto_linking_is_enabled(self, render_error):
+        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):
+        # lis email different from logged in user
+        request = build_launch_request({
+            'oauth_consumer_key': 'consumer_key_2',
+            'lis_person_contact_email_primary':  'random_email@test.com'
+        })
+
+        views.lti_launch(request, str(COURSE_KEY), str(USAGE_KEY))
+        render_error.assert_called()
+
 
 class LtiLaunchTestRender(LtiTestMixin, RenderXBlockTestMixin, ModuleStoreTestCase):
     """
diff --git a/lms/djangoapps/lti_provider/users.py b/lms/djangoapps/lti_provider/users.py
index 3af42fa180e0..64bb7087822c 100644
--- a/lms/djangoapps/lti_provider/users.py
+++ b/lms/djangoapps/lti_provider/users.py
@@ -41,6 +41,16 @@ def authenticate_lti_user(request, lti_user_id, lti_consumer):
         else:
             lti_user = create_lti_user(lti_user_id, lti_consumer)
 
+    # If auto-linking is enabled, the lti_user should be linked to the logged-in user
+    if (
+        lti_consumer.auto_link_users_using_email and
+        request.user.is_authenticated and
+        lti_user.edx_user != request.user
+    ):
+        print("Switching to request.user")
+        lti_user.edx_user = request.user
+        lti_user.save()
+
     if not (request.user.is_authenticated and
             request.user == lti_user.edx_user):
         # The user is not authenticated, or is logged in as somebody else.
diff --git a/lms/djangoapps/lti_provider/views.py b/lms/djangoapps/lti_provider/views.py
index ea2102e41893..915fb868654a 100644
--- a/lms/djangoapps/lti_provider/views.py
+++ b/lms/djangoapps/lti_provider/views.py
@@ -8,6 +8,7 @@
 from django.conf import settings
 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
 
@@ -86,6 +87,18 @@ def lti_launch(request, course_id, usage_id):
     params['course_key'] = course_key
     params['usage_key'] = usage_key
 
+    # Verify that the email from the LTI Launch and the logged-in user are the same.
+    if lti_consumer.auto_link_users_using_email:
+        lis_email = request.POST.get("lis_person_contact_email_primary")
+        if not request.user.is_authenticated or (lis_email and request.user.email != lis_email):
+            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)
+
     # 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)
diff --git a/lms/templates/lti_provider/user-auth-error.html b/lms/templates/lti_provider/user-auth-error.html
new file mode 100644
index 000000000000..e9c718ab7d46
--- /dev/null
+++ b/lms/templates/lti_provider/user-auth-error.html
@@ -0,0 +1,24 @@
+<%page expression_filter="h" />
+<%namespace name='static' file='../static_content.html'/>
+<%!
+from django.utils.translation import gettext as _
+from openedx.core.djangolib.markup import HTML, Text
+%>
+<%inherit file="../main.html" />
+
+<section class="outside-app">
+  <h1>
+    ${Text(_("There was an error when loading this module!"))}
+  </h1>
+  <p>
+    ${Text(_("This module is available only for users who are signed into {platform}. "
+        "Kindly sign in and refresh this page to load the content."
+      )).format(
+        platform=HTML(u"<em>{}</em>").format(static.get_platform_name())
+      )}
+  </p>
+
+  <p class="mt-3">
+    <a class="btn btn-primary" href="${login_link}" target="_blank">${_("Sign in")}</a>
+  </p>
+</section>

From 9ab823343b861abea425ab442f1d78603f2ec84f Mon Sep 17 00:00:00 2001
From: Arunmozhi <arun@arunmozhi.in>
Date: Mon, 30 Oct 2023 19:03:45 +0530
Subject: [PATCH 09/19] fix: extra space linting issue

---
 lms/djangoapps/lti_provider/tests/test_views.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lms/djangoapps/lti_provider/tests/test_views.py b/lms/djangoapps/lti_provider/tests/test_views.py
index 8dbff97a1e61..3e49b92787b3 100644
--- a/lms/djangoapps/lti_provider/tests/test_views.py
+++ b/lms/djangoapps/lti_provider/tests/test_views.py
@@ -213,7 +213,7 @@ def test_auth_error_shown_when_lis_email_is_different_from_user_email(self, rend
         # lis email different from logged in user
         request = build_launch_request({
             'oauth_consumer_key': 'consumer_key_2',
-            'lis_person_contact_email_primary':  'random_email@test.com'
+            'lis_person_contact_email_primary': 'random_email@test.com'
         })
 
         views.lti_launch(request, str(COURSE_KEY), str(USAGE_KEY))

From 274234fa03386b958f94bf910fd069f39149fbfb Mon Sep 17 00:00:00 2001
From: Arunmozhi <arun@arunmozhi.in>
Date: Thu, 2 Nov 2023 17:25:54 +0530
Subject: [PATCH 10/19] refactor: move the email comparison check to auth
 function

---
 .../lti_provider/tests/test_users.py          | 41 +++++++++++++------
 lms/djangoapps/lti_provider/users.py          | 11 ++++-
 lms/djangoapps/lti_provider/views.py          | 12 +++++-
 3 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/lms/djangoapps/lti_provider/tests/test_users.py b/lms/djangoapps/lti_provider/tests/test_users.py
index 10b02cf1f601..19622d4d344e 100644
--- a/lms/djangoapps/lti_provider/tests/test_users.py
+++ b/lms/djangoapps/lti_provider/tests/test_users.py
@@ -7,7 +7,7 @@
 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
@@ -93,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',
+            auto_link_users_using_email=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
         )
@@ -148,21 +155,31 @@ def test_auto_linking_of_users_using_lis_person_contact_email_primary(self, crea
         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)
+        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_switch_the_associated_edx_user_when_auto_linking_existing_user(self, create_user, switch_user):
-        lti_user = self.create_lti_user_model()
+        lti_user = self.create_lti_user_model(self.auto_linking_consumer)
         new_user = UserFactory.create()
-        self.lti_consumer.auto_link_users_using_email = True
-        self.lti_consumer.save()
         request = RequestFactory().post("/", {"lis_person_contact_email_primary": new_user.email})
         request.user = new_user
 
-        users.authenticate_lti_user(request, self.lti_user_id, self.lti_consumer)
-        assert LtiUser.objects.get(pk=lti_user.id).edx_user == new_user
+        users.authenticate_lti_user(request, self.lti_user_id, self.auto_linking_consumer)
+        assert LtiUser.objects.get(id=lti_user.id).edx_user == new_user
+
+    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": "wrong_email@example.com"})
+        request.user = self.old_user
+
+        with self.assertRaises(PermissionDenied):
+            users.authenticate_lti_user(request, self.lti_user_id, self.auto_linking_consumer)
 
 
 class CreateLtiUserTest(TestCase):
diff --git a/lms/djangoapps/lti_provider/users.py b/lms/djangoapps/lti_provider/users.py
index 64bb7087822c..9c53b29f0b1e 100644
--- a/lms/djangoapps/lti_provider/users.py
+++ b/lms/djangoapps/lti_provider/users.py
@@ -28,6 +28,15 @@ 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")
+
+    # Verify that the email from the LTI Launch and the logged-in user are the same.
+    if lti_consumer.auto_link_users_using_email and (
+       not request.user.is_authenticated or
+       (lis_email and request.user.email != lis_email)
+    ):
+        raise PermissionDenied()
+
     try:
         lti_user = LtiUser.objects.get(
             lti_user_id=lti_user_id,
@@ -36,7 +45,6 @@ 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.
         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)
         else:
             lti_user = create_lti_user(lti_user_id, lti_consumer)
@@ -47,7 +55,6 @@ def authenticate_lti_user(request, lti_user_id, lti_consumer):
         request.user.is_authenticated and
         lti_user.edx_user != request.user
     ):
-        print("Switching to request.user")
         lti_user.edx_user = request.user
         lti_user.save()
 
diff --git a/lms/djangoapps/lti_provider/views.py b/lms/djangoapps/lti_provider/views.py
index 915fb868654a..d8ad37e1579f 100644
--- a/lms/djangoapps/lti_provider/views.py
+++ b/lms/djangoapps/lti_provider/views.py
@@ -6,6 +6,7 @@
 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
@@ -101,7 +102,16 @@ 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:
+        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

From 1b783f6e42e735badc2f672e2a32acdc06f0ff23 Mon Sep 17 00:00:00 2001
From: Arunmozhi <arun@arunmozhi.in>
Date: Thu, 2 Nov 2023 17:29:14 +0530
Subject: [PATCH 11/19] fix: apply code deletion missed in last commit

---
 lms/djangoapps/lti_provider/admin.py |  1 -
 lms/djangoapps/lti_provider/views.py | 12 ------------
 2 files changed, 13 deletions(-)

diff --git a/lms/djangoapps/lti_provider/admin.py b/lms/djangoapps/lti_provider/admin.py
index 1311ae70fc00..7776b4821ff2 100644
--- a/lms/djangoapps/lti_provider/admin.py
+++ b/lms/djangoapps/lti_provider/admin.py
@@ -13,5 +13,4 @@ class LtiConsumerAdmin(admin.ModelAdmin):
     search_fields = ('consumer_name', 'consumer_key', 'instance_guid')
     list_display = ('id', 'consumer_name', 'consumer_key', 'instance_guid')
 
-
 admin.site.register(LtiConsumer, LtiConsumerAdmin)
diff --git a/lms/djangoapps/lti_provider/views.py b/lms/djangoapps/lti_provider/views.py
index d8ad37e1579f..fa0ce30eef11 100644
--- a/lms/djangoapps/lti_provider/views.py
+++ b/lms/djangoapps/lti_provider/views.py
@@ -88,18 +88,6 @@ def lti_launch(request, course_id, usage_id):
     params['course_key'] = course_key
     params['usage_key'] = usage_key
 
-    # Verify that the email from the LTI Launch and the logged-in user are the same.
-    if lti_consumer.auto_link_users_using_email:
-        lis_email = request.POST.get("lis_person_contact_email_primary")
-        if not request.user.is_authenticated or (lis_email and request.user.email != lis_email):
-            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)
-
     # 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.
     try:

From 6950940ca0ed15fe02e600f16e2041cad67829a5 Mon Sep 17 00:00:00 2001
From: Arunmozhi <arun@arunmozhi.in>
Date: Thu, 2 Nov 2023 17:34:37 +0530
Subject: [PATCH 12/19] fix: flush the session before showing the error message

---
 lms/djangoapps/lti_provider/views.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lms/djangoapps/lti_provider/views.py b/lms/djangoapps/lti_provider/views.py
index fa0ce30eef11..1106908112ed 100644
--- a/lms/djangoapps/lti_provider/views.py
+++ b/lms/djangoapps/lti_provider/views.py
@@ -93,6 +93,7 @@ def lti_launch(request, course_id, usage_id):
     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,

From be2c179a3a1738e096024ba3be2bb5853bc8783c Mon Sep 17 00:00:00 2001
From: Arunmozhi <arun@arunmozhi.in>
Date: Thu, 2 Nov 2023 17:59:46 +0530
Subject: [PATCH 13/19] fix: linting issue

---
 lms/djangoapps/lti_provider/users.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lms/djangoapps/lti_provider/users.py b/lms/djangoapps/lti_provider/users.py
index 9c53b29f0b1e..fd9c12af6071 100644
--- a/lms/djangoapps/lti_provider/users.py
+++ b/lms/djangoapps/lti_provider/users.py
@@ -32,8 +32,8 @@ def authenticate_lti_user(request, lti_user_id, lti_consumer):
 
     # Verify that the email from the LTI Launch and the logged-in user are the same.
     if lti_consumer.auto_link_users_using_email and (
-       not request.user.is_authenticated or
-       (lis_email and request.user.email != lis_email)
+        not request.user.is_authenticated or
+        (lis_email and request.user.email != lis_email)
     ):
         raise PermissionDenied()
 

From 8b7142a7d5da7778332f90561585b4d999892c5e Mon Sep 17 00:00:00 2001
From: Arunmozhi <arun@arunmozhi.in>
Date: Mon, 6 Nov 2023 15:26:42 +0530
Subject: [PATCH 14/19] refactor: use a better name for the flag with helptext

---
 ...k_users_using_email.py => 0004_require_user_account.py} | 6 +++---
 lms/djangoapps/lti_provider/models.py                      | 7 ++++++-
 lms/djangoapps/lti_provider/tests/test_users.py            | 2 +-
 lms/djangoapps/lti_provider/tests/test_views.py            | 4 ++--
 lms/djangoapps/lti_provider/users.py                       | 6 +++---
 lms/templates/lti_provider/user-auth-error.html            | 2 +-
 6 files changed, 16 insertions(+), 11 deletions(-)
 rename lms/djangoapps/lti_provider/migrations/{0004_auto_link_users_using_email.py => 0004_require_user_account.py} (60%)

diff --git a/lms/djangoapps/lti_provider/migrations/0004_auto_link_users_using_email.py b/lms/djangoapps/lti_provider/migrations/0004_require_user_account.py
similarity index 60%
rename from lms/djangoapps/lti_provider/migrations/0004_auto_link_users_using_email.py
rename to lms/djangoapps/lti_provider/migrations/0004_require_user_account.py
index c9b9a346b560..d01ed4300f7f 100644
--- a/lms/djangoapps/lti_provider/migrations/0004_auto_link_users_using_email.py
+++ b/lms/djangoapps/lti_provider/migrations/0004_require_user_account.py
@@ -1,4 +1,4 @@
-# Generated by Django 3.2.21 on 2023-09-29 07:22
+# Generated by Django 3.2.22 on 2023-11-06 09:47
 
 from django.conf import settings
 from django.db import migrations, models
@@ -15,8 +15,8 @@ class Migration(migrations.Migration):
     operations = [
         migrations.AddField(
             model_name='lticonsumer',
-            name='auto_link_users_using_email',
-            field=models.BooleanField(blank=True, default=False),
+            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 leaner accounts with the LTI consumer. See the Open edX LTI Provider documentation for more details.'),
         ),
         migrations.AlterField(
             model_name='ltiuser',
diff --git a/lms/djangoapps/lti_provider/models.py b/lms/djangoapps/lti_provider/models.py
index ba8fda5c5283..94582d4bf0b4 100644
--- a/lms/djangoapps/lti_provider/models.py
+++ b/lms/djangoapps/lti_provider/models.py
@@ -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
@@ -34,7 +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)
-    auto_link_users_using_email = models.BooleanField(blank=True, default=False)
+    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):
diff --git a/lms/djangoapps/lti_provider/tests/test_users.py b/lms/djangoapps/lti_provider/tests/test_users.py
index 19622d4d344e..8fc7937a17db 100644
--- a/lms/djangoapps/lti_provider/tests/test_users.py
+++ b/lms/djangoapps/lti_provider/tests/test_users.py
@@ -97,7 +97,7 @@ def setUp(self):
             consumer_name='AutoLinkingConsumer',
             consumer_key='AutoLinkingKey',
             consumer_secret='AutoLinkingSecret',
-            auto_link_users_using_email=True
+            require_user_account=True
         )
         self.auto_linking_consumer.save()
 
diff --git a/lms/djangoapps/lti_provider/tests/test_views.py b/lms/djangoapps/lti_provider/tests/test_views.py
index 3e49b92787b3..0686c2371344 100644
--- a/lms/djangoapps/lti_provider/tests/test_views.py
+++ b/lms/djangoapps/lti_provider/tests/test_views.py
@@ -87,7 +87,7 @@ def setUp(self):
             consumer_name='auto-link-consumer',
             consumer_key='consumer_key_2',
             consumer_secret='secret_2',
-            auto_link_users_using_email=True
+            require_user_account=True
         )
         self.auto_link_consumer.save()
 
@@ -199,7 +199,7 @@ 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_auto_linking_is_enabled(self, render_error):
+    def test_unauthenticated_user_shown_error_when_require_user_account_is_enabled(self, render_error):
         request = build_launch_request({'oauth_consumer_key': 'consumer_key_2'})
         request.user = AnonymousUser()
 
diff --git a/lms/djangoapps/lti_provider/users.py b/lms/djangoapps/lti_provider/users.py
index fd9c12af6071..28b9ef5969aa 100644
--- a/lms/djangoapps/lti_provider/users.py
+++ b/lms/djangoapps/lti_provider/users.py
@@ -31,7 +31,7 @@ def authenticate_lti_user(request, lti_user_id, lti_consumer):
     lis_email = request.POST.get("lis_person_contact_email_primary")
 
     # Verify that the email from the LTI Launch and the logged-in user are the same.
-    if lti_consumer.auto_link_users_using_email and (
+    if lti_consumer.require_user_account and (
         not request.user.is_authenticated or
         (lis_email and request.user.email != lis_email)
     ):
@@ -44,14 +44,14 @@ 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.
-        if lti_consumer.auto_link_users_using_email:
+        if lti_consumer.require_user_account:
             lti_user = create_lti_user(lti_user_id, lti_consumer, lis_email)
         else:
             lti_user = create_lti_user(lti_user_id, lti_consumer)
 
     # If auto-linking is enabled, the lti_user should be linked to the logged-in user
     if (
-        lti_consumer.auto_link_users_using_email and
+        lti_consumer.require_user_account and
         request.user.is_authenticated and
         lti_user.edx_user != request.user
     ):
diff --git a/lms/templates/lti_provider/user-auth-error.html b/lms/templates/lti_provider/user-auth-error.html
index e9c718ab7d46..c90f5c9d4ebf 100644
--- a/lms/templates/lti_provider/user-auth-error.html
+++ b/lms/templates/lti_provider/user-auth-error.html
@@ -14,7 +14,7 @@ <h1>
     ${Text(_("This module is available only for users who are signed into {platform}. "
         "Kindly sign in and refresh this page to load the content."
       )).format(
-        platform=HTML(u"<em>{}</em>").format(static.get_platform_name())
+        platform=HTML("<em>{}</em>").format(static.get_platform_name())
       )}
   </p>
 

From 96769d70d974eaa3dbf0d3f2c98420d287139741 Mon Sep 17 00:00:00 2001
From: Arunmozhi <tecoholic@users.noreply.github.com>
Date: Tue, 7 Nov 2023 15:19:20 +0530
Subject: [PATCH 15/19] fix: typo in migration help text

Co-authored-by: Piotr Surowiec <piotr@surowiec.it>
---
 .../lti_provider/migrations/0004_require_user_account.py        | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lms/djangoapps/lti_provider/migrations/0004_require_user_account.py b/lms/djangoapps/lti_provider/migrations/0004_require_user_account.py
index d01ed4300f7f..e6238e312da0 100644
--- a/lms/djangoapps/lti_provider/migrations/0004_require_user_account.py
+++ b/lms/djangoapps/lti_provider/migrations/0004_require_user_account.py
@@ -16,7 +16,7 @@ class Migration(migrations.Migration):
         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 leaner accounts with the LTI consumer. See the Open edX LTI Provider documentation for more details.'),
+            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',

From cc34afe6c7780beca38a46e5f361dde47ed30b36 Mon Sep 17 00:00:00 2001
From: Arunmozhi <arun@arunmozhi.in>
Date: Tue, 7 Nov 2023 16:15:40 +0530
Subject: [PATCH 16/19] fix: explicitly mention emails should match in the
 error page

---
 lms/templates/lti_provider/user-auth-error.html | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lms/templates/lti_provider/user-auth-error.html b/lms/templates/lti_provider/user-auth-error.html
index c90f5c9d4ebf..a807e49f56b1 100644
--- a/lms/templates/lti_provider/user-auth-error.html
+++ b/lms/templates/lti_provider/user-auth-error.html
@@ -17,6 +17,14 @@ <h1>
         platform=HTML("<em>{}</em>").format(static.get_platform_name())
       )}
   </p>
+  <p class="my-3">
+    <strong>${_("NOTE:")}</strong>
+    ${Text(_(
+      "The email used to sign into this platform and {platform} should be the same."
+    )).format(
+      platform=HTML("<em>{}</em>").format(static.get_platform_name())
+    )}
+  </p>
 
   <p class="mt-3">
     <a class="btn btn-primary" href="${login_link}" target="_blank">${_("Sign in")}</a>

From f9dc7179391088a7596d4f9766c2f687fa12abad Mon Sep 17 00:00:00 2001
From: Arunmozhi <arun@arunmozhi.in>
Date: Thu, 9 Nov 2023 22:25:54 +0530
Subject: [PATCH 17/19] fix: lti launch failing when auto-linked user was
 logged out of platform

---
 lms/djangoapps/lti_provider/tests/test_users.py |  8 ++++++++
 lms/djangoapps/lti_provider/tests/test_views.py |  9 +++++++++
 lms/djangoapps/lti_provider/users.py            | 15 +++++++--------
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/lms/djangoapps/lti_provider/tests/test_users.py b/lms/djangoapps/lti_provider/tests/test_users.py
index 8fc7937a17db..9b344ef4859e 100644
--- a/lms/djangoapps/lti_provider/tests/test_users.py
+++ b/lms/djangoapps/lti_provider/tests/test_users.py
@@ -181,6 +181,14 @@ def test_raise_exception_on_mismatched_user_and_lis_email(self, create_user, swi
         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):
     """
diff --git a/lms/djangoapps/lti_provider/tests/test_views.py b/lms/djangoapps/lti_provider/tests/test_views.py
index 0686c2371344..8328c669cf5a 100644
--- a/lms/djangoapps/lti_provider/tests/test_views.py
+++ b/lms/djangoapps/lti_provider/tests/test_views.py
@@ -200,6 +200,10 @@ def test_lti_consumer_record_supplemented_with_guid(self, _render):
 
     @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()
 
@@ -210,6 +214,11 @@ def test_unauthenticated_user_shown_error_when_require_user_account_is_enabled(s
 
     @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',
diff --git a/lms/djangoapps/lti_provider/users.py b/lms/djangoapps/lti_provider/users.py
index 28b9ef5969aa..302e316da483 100644
--- a/lms/djangoapps/lti_provider/users.py
+++ b/lms/djangoapps/lti_provider/users.py
@@ -30,13 +30,6 @@ def authenticate_lti_user(request, lti_user_id, lti_consumer):
     """
     lis_email = request.POST.get("lis_person_contact_email_primary")
 
-    # Verify that the email from the LTI Launch and the logged-in user are the same.
-    if lti_consumer.require_user_account and (
-        not request.user.is_authenticated or
-        (lis_email and request.user.email != lis_email)
-    ):
-        raise PermissionDenied()
-
     try:
         lti_user = LtiUser.objects.get(
             lti_user_id=lti_user_id,
@@ -45,7 +38,13 @@ 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.
         if lti_consumer.require_user_account:
-            lti_user = create_lti_user(lti_user_id, lti_consumer, lis_email)
+            # 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()
         else:
             lti_user = create_lti_user(lti_user_id, lti_consumer)
 

From 7404a966a29d5e3f8c277c62cd3234e4dd90ed1c Mon Sep 17 00:00:00 2001
From: Arunmozhi <arun@arunmozhi.in>
Date: Thu, 9 Nov 2023 22:59:03 +0530
Subject: [PATCH 18/19] fix: linting issue with raising exception

---
 lms/djangoapps/lti_provider/users.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lms/djangoapps/lti_provider/users.py b/lms/djangoapps/lti_provider/users.py
index 302e316da483..57bb9ca0a148 100644
--- a/lms/djangoapps/lti_provider/users.py
+++ b/lms/djangoapps/lti_provider/users.py
@@ -35,7 +35,7 @@ def authenticate_lti_user(request, lti_user_id, lti_consumer):
             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.
         if lti_consumer.require_user_account:
             # Verify that the email from the LTI Launch and the logged-in user are the same
@@ -44,7 +44,7 @@ def authenticate_lti_user(request, lti_user_id, lti_consumer):
                 lti_user = create_lti_user(lti_user_id, lti_consumer, lis_email)
             else:
                 # Ask the user to login before linking.
-                raise PermissionDenied()
+                raise PermissionDenied() from exc
         else:
             lti_user = create_lti_user(lti_user_id, lti_consumer)
 

From 2fb1a0625acefbdc4ecba6be6fa4e1770d3ab666 Mon Sep 17 00:00:00 2001
From: Arunmozhi <arun@arunmozhi.in>
Date: Mon, 13 Nov 2023 22:08:24 +0530
Subject: [PATCH 19/19] fix: remove re-linking from authenticate_user function

---
 lms/djangoapps/lti_provider/tests/test_users.py | 9 ---------
 lms/djangoapps/lti_provider/users.py            | 9 ---------
 2 files changed, 18 deletions(-)

diff --git a/lms/djangoapps/lti_provider/tests/test_users.py b/lms/djangoapps/lti_provider/tests/test_users.py
index 9b344ef4859e..1d19d8995be9 100644
--- a/lms/djangoapps/lti_provider/tests/test_users.py
+++ b/lms/djangoapps/lti_provider/tests/test_users.py
@@ -158,15 +158,6 @@ def test_auto_linking_of_users_using_lis_person_contact_email_primary(self, crea
         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_switch_the_associated_edx_user_when_auto_linking_existing_user(self, create_user, switch_user):
-        lti_user = self.create_lti_user_model(self.auto_linking_consumer)
-        new_user = UserFactory.create()
-        request = RequestFactory().post("/", {"lis_person_contact_email_primary": new_user.email})
-        request.user = new_user
-
-        users.authenticate_lti_user(request, self.lti_user_id, self.auto_linking_consumer)
-        assert LtiUser.objects.get(id=lti_user.id).edx_user == new_user
-
     def test_raise_exception_trying_to_auto_link_unauthenticate_user(self, create_user, switch_user):
         request = RequestFactory().post("/")
         request.user = AnonymousUser()
diff --git a/lms/djangoapps/lti_provider/users.py b/lms/djangoapps/lti_provider/users.py
index 57bb9ca0a148..c2373522b960 100644
--- a/lms/djangoapps/lti_provider/users.py
+++ b/lms/djangoapps/lti_provider/users.py
@@ -48,15 +48,6 @@ def authenticate_lti_user(request, lti_user_id, lti_consumer):
         else:
             lti_user = create_lti_user(lti_user_id, lti_consumer)
 
-    # If auto-linking is enabled, the lti_user should be linked to the logged-in user
-    if (
-        lti_consumer.require_user_account and
-        request.user.is_authenticated and
-        lti_user.edx_user != request.user
-    ):
-        lti_user.edx_user = request.user
-        lti_user.save()
-
     if not (request.user.is_authenticated and
             request.user == lti_user.edx_user):
         # The user is not authenticated, or is logged in as somebody else.