Merge pull request #16624 from open-craft/uman/force-sync-learner-data

[ENT-759] Add a TPA pipeline step to take forced sync into account.

common/djangoapps/third_party_auth/migrations/0013_sync_learner_profile_data.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('third_party_auth', '0012_auto_20170626_1135'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='ltiproviderconfig',
+            name='sync_learner_profile_data',
+            field=models.BooleanField(default=False, help_text='Synchronize user profile data received from the identity provider with the edX user account on each SSO login. The user will be notified if the email address associated with their account is changed as a part of this synchronization.'),
+        ),
+        migrations.AddField(
+            model_name='oauth2providerconfig',
+            name='sync_learner_profile_data',
+            field=models.BooleanField(default=False, help_text='Synchronize user profile data received from the identity provider with the edX user account on each SSO login. The user will be notified if the email address associated with their account is changed as a part of this synchronization.'),
+        ),
+        migrations.AddField(
+            model_name='samlproviderconfig',
+            name='sync_learner_profile_data',
+            field=models.BooleanField(default=False, help_text='Synchronize user profile data received from the identity provider with the edX user account on each SSO login. The user will be notified if the email address associated with their account is changed as a part of this synchronization.'),
+        ),
+    ]

common/djangoapps/third_party_auth/models.py

@@ -181,6 +181,14 @@ class ProviderConfig(ConfigurationModel):
             "immediately after authenticating with the third party instead of the login page."
         ),
     )
+    sync_learner_profile_data = models.BooleanField(
+        default=False,
+        help_text=_(
+            "Synchronize user profile data received from the identity provider with the edX user "
+            "account on each SSO login. The user will be notified if the email address associated "
+            "with their account is changed as a part of this synchronization."
+        )
+    )
     prefix = None  # used for provider_id. Set to a string value in subclass
     backend_name = None  # Set to a field or fixed value in subclass
     accepts_logins = True  # Whether to display a sign-in button when the provider is enabled

common/djangoapps/third_party_auth/pipeline.py

@@ -66,10 +66,12 @@ import string
 import urllib
 from collections import OrderedDict
 from logging import getLogger
+from smtplib import SMTPException
 
 import analytics
 from django.conf import settings
 from django.contrib.auth.models import User
+from django.core.mail.message import EmailMessage
 from django.core.urlresolvers import reverse
 from django.http import HttpResponseBadRequest
 from django.shortcuts import redirect
@@ -79,7 +81,9 @@ from social_core.pipeline import partial
 from social_core.pipeline.social_auth import associate_by_email
 
 import student
+from edxmako.shortcuts import render_to_string
 from eventtracking import tracker
+from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 
 from . import provider
 
@@ -563,7 +567,7 @@ def ensure_user_information(strategy, auth_entry, backend=None, user=None, socia
                 (current_provider.skip_email_verification or current_provider.send_to_registration_first))
 
     if not user:
-        if auth_entry in [AUTH_ENTRY_LOGIN_API, AUTH_ENTRY_REGISTER_API]:
+        if is_api(auth_entry):
             return HttpResponseBadRequest()
         elif auth_entry == AUTH_ENTRY_LOGIN:
             # User has authenticated with the third party provider but we don't know which edX
@@ -710,3 +714,79 @@ def associate_by_email_if_login_api(auth_entry, backend, details, user, current_
             # email address and the legitimate user would now login to the illegitimate
             # account.
             return association_response
+
+
+def user_details_force_sync(auth_entry, strategy, details, user=None, *args, **kwargs):
+    """
+    Update normally protected user details using data from provider.
+
+    This step in the pipeline is akin to `social_core.pipeline.user.user_details`, which updates
+    the user details but has an unconfigurable protection over updating the username & email, and
+    is unable to update information such as the user's full name which isn't on the user model, but
+    rather on the user profile model.
+
+    Additionally, because the email field is normally used to log in, if the email is changed by this
+    forced synchronization, we send an email to both the old and new emails, letting the user know.
+
+    This step is controlled by the `sync_learner_profile_data` flag on the provider's configuration.
+    """
+    current_provider = provider.Registry.get_from_pipeline({'backend': strategy.request.backend.name, 'kwargs': kwargs})
+    if user and current_provider.sync_learner_profile_data:
+        # Keep track of which incoming values get applied.
+        changed = {}
+
+        # Map each incoming field from the provider to the name on the user model (by default, they always match).
+        field_mapping = {field: (user, field) for field in details.keys() if hasattr(user, field)}
+
+        # This is a special case where the field mapping should go to the user profile object and not the user object,
+        # in some cases with differing field names (i.e. 'fullname' vs. 'name').
+        field_mapping.update({
+            'fullname': (user.profile, 'name'),
+            'country': (user.profile, 'country'),
+        })
+
+        # Track any fields that would raise an integrity error if there was a conflict.
+        integrity_conflict_fields = {'email': user.email, 'username': user.username}
+
+        for provider_field, (model, field) in field_mapping.items():
+            provider_value = details.get(provider_field)
+            current_value = getattr(model, field)
+            if provider_value is not None and current_value != provider_value:
+                if field in integrity_conflict_fields and User.objects.filter(**{field: provider_value}).exists():
+                    logger.warning('User with ID [%s] tried to synchronize profile data through [%s] '
+                                   'but there was a conflict with an existing [%s]: [%s].',
+                                   user.id, current_provider.name, field, provider_value)
+                    continue
+                changed[provider_field] = current_value
+                setattr(model, field, provider_value)
+
+        if changed:
+            logger.info(
+                "User [%s] performed SSO through [%s] who synchronizes profile data, and the "
+                "following fields were changed: %s", user.username, current_provider.name, changed.keys(),
+            )
+
+            # Save changes to user and user.profile models.
+            strategy.storage.user.changed(user)
+            user.profile.save()
+
+            # Send an email to the old and new email to alert the user that their login email changed.
+            if changed.get('email'):
+                old_email = changed['email']
+                new_email = user.email
+                email_context = {'old_email': old_email, 'new_email': new_email}
+                # Subjects shouldn't have new lines.
+                subject = ''.join(render_to_string(
+                    'emails/sync_learner_profile_data_email_change_subject.txt',
+                    email_context
+                ).splitlines())
+                body = render_to_string('emails/sync_learner_profile_data_email_change_body.txt', email_context)
+                from_email = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL)
+
+                email = EmailMessage(subject=subject, body=body, from_email=from_email, to=[old_email, new_email])
+                email.content_subtype = "html"
+                try:
+                    email.send()
+                except SMTPException:
+                    logger.exception('Error sending IdP learner data sync-initiated email change '
+                                     'notification email for user [%s].', user.username)

common/djangoapps/third_party_auth/settings.py

@@ -57,6 +57,7 @@ def apply_settings(django_settings):
         'social_core.pipeline.social_auth.associate_user',
         'social_core.pipeline.social_auth.load_extra_data',
         'social_core.pipeline.user.user_details',
+        'third_party_auth.pipeline.user_details_force_sync',
         'third_party_auth.pipeline.set_logged_in_cookies',
         'third_party_auth.pipeline.login_analytics',
     ]

common/djangoapps/third_party_auth/tests/test_pipeline_integration.py

@@ -6,8 +6,10 @@ import mock
 import ddt
 from django import test
 from django.contrib.auth import models
+from django.core import mail
 from social_django import models as social_models
 
+from student.tests.factories import UserFactory
 from third_party_auth import pipeline, provider
 from third_party_auth.tests import testutil
 
@@ -303,9 +305,6 @@ class TestPipelineUtilityFunctions(TestCase, test.TestCase):
 class EnsureUserInformationTestCase(testutil.TestCase, test.TestCase):
     """Tests ensuring that we have the necessary user information to proceed with the pipeline."""
 
-    def setUp(self):
-        super(EnsureUserInformationTestCase, self).setUp()
-
     @ddt.data(
         (True, '/register'),
         (False, '/login')
@@ -335,3 +334,105 @@ class EnsureUserInformationTestCase(testutil.TestCase, test.TestCase):
                 )
                 assert response.status_code == 302
                 assert response.url == expected_redirect_url
+
+
+@unittest.skipUnless(testutil.AUTH_FEATURE_ENABLED, testutil.AUTH_FEATURES_KEY + ' not enabled')
+class UserDetailsForceSyncTestCase(testutil.TestCase, test.TestCase):
+    """Tests to ensure learner profile data is properly synced if the provider requires it."""
+
+    def setUp(self):
+        super(UserDetailsForceSyncTestCase, self).setUp()
+        # pylint: disable=attribute-defined-outside-init
+        self.user = UserFactory.create()
+        self.old_email = self.user.email
+        self.old_username = self.user.username
+        self.old_fullname = self.user.profile.name
+        self.details = {
+            'email': 'new+{}'.format(self.user.email),
+            'username': 'new_{}'.format(self.user.username),
+            'fullname': 'Grown Up {}'.format(self.user.profile.name),
+            'country': 'PK',
+            'non_existing_field': 'value',
+        }
+
+        # Mocks
+        self.strategy = mock.MagicMock()
+        self.strategy.storage.user.changed.side_effect = lambda user: user.save()
+
+        get_from_pipeline = mock.patch('third_party_auth.pipeline.provider.Registry.get_from_pipeline')
+        self.get_from_pipeline = get_from_pipeline.start()
+        self.get_from_pipeline.return_value = mock.MagicMock(sync_learner_profile_data=True)
+        self.addCleanup(get_from_pipeline.stop)
+
+    def test_user_details_force_sync(self):
+        """
+        The user details are synced properly and an email is sent when the email is changed.
+        """
+        # Begin the pipeline.
+        pipeline.user_details_force_sync(
+            auth_entry=pipeline.AUTH_ENTRY_LOGIN,
+            strategy=self.strategy,
+            details=self.details,
+            user=self.user,
+        )
+
+        # User now has updated information in the DB.
+        user = User.objects.get()
+        assert user.email == 'new+{}'.format(self.old_email)
+        assert user.username == 'new_{}'.format(self.old_username)
+        assert user.profile.name == 'Grown Up {}'.format(self.old_fullname)
+        assert user.profile.country == 'PK'
+
+        assert len(mail.outbox) == 1
+
+    def test_user_details_force_sync_email_conflict(self):
+        """
+        The user details were attempted to be synced but the incoming email already exists for another account.
+        """
+        # Create a user with an email that conflicts with the incoming value.
+        UserFactory.create(email='new+{}'.format(self.old_email))
+
+        # Begin the pipeline.
+        pipeline.user_details_force_sync(
+            auth_entry=pipeline.AUTH_ENTRY_LOGIN,
+            strategy=self.strategy,
+            details=self.details,
+            user=self.user,
+        )
+
+        # The email is not changed, but everything else is.
+        user = User.objects.get(pk=self.user.pk)
+        assert user.email == self.old_email
+        assert user.username == 'new_{}'.format(self.old_username)
+        assert user.profile.name == 'Grown Up {}'.format(self.old_fullname)
+        assert user.profile.country == 'PK'
+
+        # No email should be sent for an email change.
+        assert len(mail.outbox) == 0
+
+    def test_user_details_force_sync_username_conflict(self):
+        """
+        The user details were attempted to be synced but the incoming username already exists for another account.
+
+        An email should still be sent in this case.
+        """
+        # Create a user with an email that conflicts with the incoming value.
+        UserFactory.create(username='new_{}'.format(self.old_username))
+
+        # Begin the pipeline.
+        pipeline.user_details_force_sync(
+            auth_entry=pipeline.AUTH_ENTRY_LOGIN,
+            strategy=self.strategy,
+            details=self.details,
+            user=self.user,
+        )
+
+        # The username is not changed, but everything else is.
+        user = User.objects.get(pk=self.user.pk)
+        assert user.email == 'new+{}'.format(self.old_email)
+        assert user.username == self.old_username
+        assert user.profile.name == 'Grown Up {}'.format(self.old_fullname)
+        assert user.profile.country == 'PK'
+
+        # An email should still be sent because the email changed.
+        assert len(mail.outbox) == 1

common/templates/emails/sync_learner_profile_data_email_change_body.txt

+<%! from django.utils.translation import ugettext as _ %>
+<%! from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers %>
+
+<p>
+${_("The email associated with your {platform_name} account has changed from {old_email} to {new_email}.").format(
+    platform_name=configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME),
+    old_email=old_email,
+    new_email=new_email,
+)}
+</p>
+
+
+<p>${_("No action is needed on your part.")}</p>
+
+<p>
+${_("If this change is not correct, contact {link_start}{platform_name} Support{link_end} "
+    "or your administrator.").format(
+        platform_name=configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME),
+        link_start=u"<a href='{support_link}'>".format(
+            support_link=configuration_helpers.get_value('SUPPORT_SITE_LINK', settings.SUPPORT_SITE_LINK),
+        ),
+        link_end=u"</a>",
+    )
+}
+</p>

common/templates/emails/sync_learner_profile_data_email_change_subject.txt

+<%! from django.utils.translation import ugettext as _ %>
+<%! from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers %>
+
+${_("Your {platform_name} account email has been updated").format(
+    platform_name=configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME),
+)}