diff --git a/lms/djangoapps/bulk_email/signals.py b/lms/djangoapps/bulk_email/signals.py new file mode 100644 index 0000000000..c86bb040c8 --- /dev/null +++ b/lms/djangoapps/bulk_email/signals.py @@ -0,0 +1,25 @@ +""" +Signal handlers for the bulk_email app +""" + +from django.dispatch import receiver + +from student.models import CourseEnrollment +from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_MAILINGS + +from .models import Optout + + +@receiver(USER_RETIRE_MAILINGS) +def force_optout_all(sender, **kwargs): # pylint: disable=unused-argument + """ + When a user is retired from all mailings this method will create an Optout + row for any courses they may be enrolled in. + """ + user = kwargs.get('user', None) + + if not user: + raise TypeError('Expected a User type, but received None.') + + for enrollment in CourseEnrollment.objects.filter(user=user): + Optout.objects.get_or_create(user=user, course_id=enrollment.course.id) diff --git a/lms/djangoapps/bulk_email/tests/test_signals.py b/lms/djangoapps/bulk_email/tests/test_signals.py new file mode 100644 index 0000000000..427708bd32 --- /dev/null +++ b/lms/djangoapps/bulk_email/tests/test_signals.py @@ -0,0 +1,87 @@ +""" +Unit tests for student optouts from course email +""" +import json + +from django.core import mail +from django.core.management import call_command +from django.core.urlresolvers import reverse +from mock import Mock, patch +from nose.plugins.attrib import attr +from six import text_type + +from bulk_email.models import BulkEmailFlag, Optout +from bulk_email.signals import force_optout_all +from student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + + +@attr(shard=1) +@patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message', autospec=True)) +class TestOptoutCourseEmailsBySignal(ModuleStoreTestCase): + """ + Tests that the force_optout_all signal receiver opts the user out of course emails + """ + def setUp(self): + super(TestOptoutCourseEmailsBySignal, self).setUp() + self.course = CourseFactory.create(run='testcourse1', display_name="Test Course Title") + self.instructor = AdminFactory.create() + self.student = UserFactory.create() + self.enrollment = CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id) + + # load initial content (since we don't run migrations as part of tests): + call_command("loaddata", "course_email_template.json") + + self.client.login(username=self.student.username, password="test") + + self.send_mail_url = reverse('send_email', kwargs={'course_id': text_type(self.course.id)}) + self.success_content = { + 'course_id': text_type(self.course.id), + 'success': True, + } + BulkEmailFlag.objects.create(enabled=True, require_course_email_auth=False) + + def test_optout_row_created_on_signal(self): + """ + Make sure the correct row is created for a user enrolled in a course + """ + force_optout_all(sender=self.__class__, user=self.student) + self.assertEqual(Optout.objects.filter(user=self.student, course_id=self.course.id).count(), 1) + + def send_test_email(self): + """ + Navigate to the instructor dash's email view to send bulk email + """ + # Pull up email view on instructor dashboard + url = reverse('instructor_dashboard', kwargs={'course_id': text_type(self.course.id)}) + response = self.client.get(url) + email_section = '
' + + # If this fails, it is likely because BulkEmailFlag.is_enabled() is set to False + self.assertIn(email_section, response.content) + + test_email = { + 'action': 'Send email', + 'send_to': '["myself", "staff", "learners"]', + 'subject': 'test subject for all', + 'message': 'test message for all' + } + response = self.client.post(self.send_mail_url, test_email) + self.assertEquals(json.loads(response.content), self.success_content) + + def test_optout_course(self): + """ + Make sure student does not receive course email after being opted out + """ + # Use the signal receiver to for the opt-out + force_optout_all(sender=self.__class__, user=self.student) + + # Try to send a bulk course email + self.client.login(username=self.instructor.username, password="test") + self.send_test_email() + + # Assert that self.student.email not in mail.to, outbox should only contain "myself" target + self.assertEqual(len(mail.outbox), 1) + self.assertEqual(len(mail.outbox[0].to), 1) + self.assertEqual(mail.outbox[0].to[0], self.instructor.email) diff --git a/lms/djangoapps/email_marketing/signals.py b/lms/djangoapps/email_marketing/signals.py index 79864f9a4d..3d8720e199 100644 --- a/lms/djangoapps/email_marketing/signals.py +++ b/lms/djangoapps/email_marketing/signals.py @@ -6,14 +6,17 @@ import logging from random import randint import crum +from celery.exceptions import TimeoutError from django.conf import settings from django.dispatch import receiver +from sailthru.sailthru_client import SailthruClient from sailthru.sailthru_error import SailthruClientError -from celery.exceptions import TimeoutError +from six import text_type import third_party_auth from course_modes.models import CourseMode from email_marketing.models import EmailMarketingConfiguration +from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_MAILINGS from openedx.core.djangoapps.waffle_utils import WaffleSwitchNamespace from lms.djangoapps.email_marketing.tasks import update_user, update_user_email, get_email_cookies_via_sailthru from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY @@ -94,10 +97,10 @@ def add_email_marketing_cookies(sender, response=None, user=None, _log_sailthru_api_call_time(time_before_call) except TimeoutError as exc: - log.error("Timeout error while attempting to obtain cookie from Sailthru: %s", unicode(exc)) + log.error("Timeout error while attempting to obtain cookie from Sailthru: %s", text_type(exc)) return response except SailthruClientError as exc: - log.error("Exception attempting to obtain cookie from Sailthru: %s", unicode(exc)) + log.error("Exception attempting to obtain cookie from Sailthru: %s", text_type(exc)) return response except Exception: log.error("Exception Connecting to celery task for %s", user.email) @@ -226,7 +229,7 @@ def _create_sailthru_user_vars(user, profile, registration=None): if profile.year_of_birth: sailthru_vars['year_of_birth'] = profile.year_of_birth - sailthru_vars['country'] = unicode(profile.country.code) + sailthru_vars['country'] = text_type(profile.country.code) if registration: sailthru_vars['activation_key'] = registration.activation_key @@ -258,3 +261,43 @@ def _log_sailthru_api_call_time(time_before_call): time_before_call.isoformat(' '), time_after_call.isoformat(' '), delta_sailthru_api_call_time.microseconds / 1000) + + +@receiver(USER_RETIRE_MAILINGS) +def force_unsubscribe_all(sender, **kwargs): # pylint: disable=unused-argument + """ + Synchronously(!) unsubscribes the given user from all Sailthru email lists. + + In the future this could be moved to a Celery task, however this is currently + only used as part of user retirement, where we need a very reliable indication + of success or failure. + + Args: + user(User): Django model of type returned from get_user_model() + Returns: + None + """ + user = kwargs.get('user', None) + + if not user: + raise TypeError('Expected a User type, but received None.') + + email_config = EmailMarketingConfiguration.current() + if not email_config.enabled: + return + + sailthru_parms = {"id": user.email, "keys": {"optout_email": "all"}} + + try: + sailthru_client = SailthruClient(email_config.sailthru_key, email_config.sailthru_secret) + sailthru_response = sailthru_client.api_post("user", sailthru_parms) + except SailthruClientError as exc: + error_msg = "Exception attempting to opt-out user %s from Sailthru - %s" % (user.email, text_type(exc)) + log.error(error_msg) + raise Exception(error_msg) + + if not sailthru_response.is_ok(): + error = sailthru_response.get_error() + error_msg = "Error attempting to opt-out user %s from Sailthru - %s" % (user.email, error.get_message()) + log.error(error_msg) + raise Exception(error_msg) diff --git a/lms/djangoapps/email_marketing/tests/test_signals.py b/lms/djangoapps/email_marketing/tests/test_signals.py index 69f9c1a8ea..29d5cb4746 100644 --- a/lms/djangoapps/email_marketing/tests/test_signals.py +++ b/lms/djangoapps/email_marketing/tests/test_signals.py @@ -20,6 +20,7 @@ from email_marketing.signals import ( add_email_marketing_cookies, email_marketing_register_user, email_marketing_user_field_changed, + force_unsubscribe_all, update_sailthru ) from email_marketing.tasks import ( @@ -578,6 +579,34 @@ class EmailMarketingTests(TestCase): email_marketing_user_field_changed(None, self.user, table='auth_user', setting='email', old_value='new@a.com') self.assertFalse(mock_update_user.called) + @patch('email_marketing.tasks.log.error') + @patch('email_marketing.tasks.SailthruClient.api_post') + def test_sailthru_on_success(self, mock_sailthru, mock_log_error): + """ + Ensure that a successful unsubscribe does not trigger an error log + """ + mock_sailthru.return_value = SailthruResponse(JsonResponse({'ok': True})) + force_unsubscribe_all(sender=self.__class__, user=self.user) + self.assertTrue(mock_sailthru.called) + self.assertFalse(mock_log_error.called) + + @patch('email_marketing.tasks.log.error') + @patch('email_marketing.tasks.SailthruClient.api_post') + def test_sailthru_on_connection_error(self, mock_sailthru, mock_log_error): + mock_sailthru.side_effect = SailthruClientError + with self.assertRaises(Exception): + force_unsubscribe_all(sender=self.__class__, user=self.user) + self.assertTrue(mock_log_error.called) + + @patch('email_marketing.tasks.log.error') + @patch('email_marketing.tasks.SailthruClient.api_post') + def test_sailthru_on_bad_response(self, mock_sailthru, mock_log_error): + mock_sailthru.return_value = SailthruResponse(JsonResponse({'error': 100, 'errormsg': 'Got an error'})) + with self.assertRaises(Exception): + force_unsubscribe_all(sender=self.__class__, user=self.user) + self.assertTrue(mock_sailthru.called) + self.assertTrue(mock_log_error.called) + class MockSailthruResponse(object): """ diff --git a/openedx/core/djangoapps/user_api/accounts/signals.py b/openedx/core/djangoapps/user_api/accounts/signals.py new file mode 100644 index 0000000000..6972c496c5 --- /dev/null +++ b/openedx/core/djangoapps/user_api/accounts/signals.py @@ -0,0 +1,7 @@ +""" +Django Signal related functionality for user_api accounts +""" + +from django.dispatch import Signal + +USER_RETIRE_MAILINGS = Signal(providing_args=["user"]) diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index 431e4ac720..98fdfdbf4a 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -15,18 +15,19 @@ from django.core.urlresolvers import reverse from django.test import TestCase from django.test.testcases import TransactionTestCase from django.test.utils import override_settings -from mock import patch +from mock import MagicMock, patch from nose.plugins.attrib import attr from pytz import UTC from rest_framework import status from rest_framework.test import APIClient, APITestCase from openedx.core.djangoapps.user_api.accounts import ACCOUNT_VISIBILITY_PREF_KEY -from openedx.core.djangoapps.user_api.models import UserPreference +from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_MAILINGS +from openedx.core.djangoapps.user_api.models import UserPreference, UserOrgTag from openedx.core.djangoapps.user_api.preferences.api import set_user_preference from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms from openedx.core.lib.token_utils import JwtBuilder -from student.models import LanguageProficiency, PendingEmailChange, UserProfile +from student.models import PendingEmailChange, UserProfile, get_retired_username_by_username from student.tests.factories import ( TEST_PASSWORD, ContentTypeFactory, @@ -918,3 +919,96 @@ class TestAccountDeactivation(TestCase): expected_status=status.HTTP_401_UNAUTHORIZED, expected_activation_status=True ) + + +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') +class TestAccountRetireMailings(TestCase): + """ + Tests the account retire mailings endpoint. + """ + + def setUp(self): + super(TestAccountRetireMailings, self).setUp() + self.test_user = UserFactory() + self.test_superuser = SuperuserFactory() + self.test_service_user = UserFactory() + + UserOrgTag.objects.create(user=self.test_user, key='email-optin', org="foo", value="True") + UserOrgTag.objects.create(user=self.test_user, key='email-optin', org="bar", value="True") + + self.url = reverse('accounts_retire_mailings', kwargs={'username': self.test_user.username}) + + def build_jwt_headers(self, user): + """ + Helper function for creating headers for the JWT authentication. + """ + token = JwtBuilder(user).build_token([]) + headers = { + 'HTTP_AUTHORIZATION': 'JWT ' + token + } + return headers + + def build_post(self, user): + retired_username = get_retired_username_by_username(user.username) + return {'retired_username': retired_username} + + def assert_status_and_tag_count(self, headers, expected_status=status.HTTP_204_NO_CONTENT, expected_tag_count=2, + expected_tag_value="False", expected_content=None): + """ + Helper function for making a request to the retire subscriptions endpoint, and asserting the status. + """ + response = self.client.post(self.url, self.build_post(self.test_user), **headers) + self.assertEqual(response.status_code, expected_status) + + # Check that the expected number of tags with the correct value exist + tag_count = UserOrgTag.objects.filter(user=self.test_user, value=expected_tag_value).count() + self.assertEqual(tag_count, expected_tag_count) + + if expected_content: + self.assertEqual(response.content.strip('"'), expected_content) + + def test_superuser_retires_user_subscriptions(self): + """ + Verify a user's subscriptions are retired when a superuser posts to the retire subscriptions endpoint. + """ + headers = self.build_jwt_headers(self.test_superuser) + self.assert_status_and_tag_count(headers) + + def test_superuser_retires_user_subscriptions_no_orgtags(self): + """ + Verify the call succeeds when the user doesn't have any org tags. + """ + UserOrgTag.objects.all().delete() + headers = self.build_jwt_headers(self.test_superuser) + self.assert_status_and_tag_count(headers, expected_tag_count=0) + + def test_unauthorized_rejection(self): + """ + Verify unauthorized users cannot retire subscriptions. + """ + headers = self.build_jwt_headers(self.test_user) + + # User should still have 2 "True" subscriptions. + self.assert_status_and_tag_count(headers, expected_status=status.HTTP_403_FORBIDDEN, expected_tag_value="True") + + def test_signal_failure(self): + """ + Verify that if a signal fails the transaction is rolled back and a proper error message is returned. + """ + headers = self.build_jwt_headers(self.test_superuser) + + mock_handler = MagicMock() + mock_handler.side_effect = Exception("Tango") + + try: + USER_RETIRE_MAILINGS.connect(mock_handler) + + # User should still have 2 "True" subscriptions. + self.assert_status_and_tag_count( + headers, + expected_status=status.HTTP_500_INTERNAL_SERVER_ERROR, + expected_tag_value="True", + expected_content="Tango" + ) + finally: + USER_RETIRE_MAILINGS.disconnect(mock_handler) diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 43d1da074f..1f3036a40a 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -6,22 +6,28 @@ https://openedx.atlassian.net/wiki/display/TNL/User+API """ from django.db import transaction +from django.contrib.auth import get_user_model from edx_rest_framework_extensions.authentication import JwtAuthentication from rest_framework import permissions from rest_framework import status from rest_framework.response import Response from rest_framework.views import APIView from rest_framework.viewsets import ViewSet +from six import text_type -from .api import get_account_settings, update_account_settings -from .permissions import CanDeactivateUser -from ..errors import UserNotFound, UserNotAuthorized, AccountUpdateError, AccountValidationError +from openedx.core.djangoapps.user_api.preferences.api import update_email_opt_in from openedx.core.lib.api.authentication import ( SessionAuthenticationAllowInactiveUser, OAuth2AuthenticationAllowInactiveUser, ) from openedx.core.lib.api.parsers import MergePatchParser -from student.models import User +from student.models import User, get_potentially_retired_user_by_username_and_hash + +from .api import get_account_settings, update_account_settings +from .permissions import CanDeactivateUser, CanRetireUser +from .signals import USER_RETIRE_MAILINGS +from ..errors import UserNotFound, UserNotAuthorized, AccountUpdateError, AccountValidationError +from ..models import UserOrgTag class AccountViewSet(ViewSet): @@ -249,3 +255,42 @@ class AccountDeactivationView(APIView): user.save() account_settings = get_account_settings(request, [username])[0] return Response(account_settings) + + +class AccountRetireMailingsView(APIView): + """ + Part of the retirement API, accepts POSTs to unsubscribe a user + from all email lists. + """ + authentication_classes = (JwtAuthentication, ) + permission_classes = (permissions.IsAuthenticated, CanRetireUser) + + def post(self, request, username): + """ + POST /api/user/v1/accounts/{username}/retire_mailings/ + + Allows an administrative user to take the following actions + on behalf of an LMS user: + - Update UserOrgTags to opt the user out of org emails + - Call Sailthru API to force opt-out the user from all email lists + """ + user_model = get_user_model() + retired_username = request.data['retired_username'] + + try: + user = get_potentially_retired_user_by_username_and_hash(username, retired_username) + + with transaction.atomic(): + # Take care of org emails first, using the existing API for consistency + for preference in UserOrgTag.objects.filter(user=user, key='email-optin'): + update_email_opt_in(user, preference.org, False) + + # This signal allows lms' email_marketing and other 3rd party email + # providers to unsubscribe the user as well + USER_RETIRE_MAILINGS.send(sender=self.__class__, user=user) + except user_model.DoesNotExist: + return Response(status=status.HTTP_404_NOT_FOUND) + except Exception as exc: # pylint: disable=broad-except + return Response(text_type(exc), status=status.HTTP_500_INTERNAL_SERVER_ERROR) + + return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/openedx/core/djangoapps/user_api/urls.py b/openedx/core/djangoapps/user_api/urls.py index 2537dd03af..ba4729d591 100644 --- a/openedx/core/djangoapps/user_api/urls.py +++ b/openedx/core/djangoapps/user_api/urls.py @@ -6,7 +6,7 @@ from django.conf import settings from django.conf.urls import url from ..profile_images.views import ProfileImageView -from .accounts.views import AccountDeactivationView, AccountViewSet +from .accounts.views import AccountDeactivationView, AccountRetireMailingsView, AccountViewSet from .preferences.views import PreferencesDetailView, PreferencesView from .verification_api.views import PhotoVerificationStatusView from .validation.views import RegistrationValidationView @@ -50,6 +50,11 @@ urlpatterns = [ AccountDeactivationView.as_view(), name='accounts_deactivation' ), + url( + r'^v1/accounts/{}/retire_mailings/$'.format(settings.USERNAME_PATTERN), + AccountRetireMailingsView.as_view(), + name='accounts_retire_mailings' + ), url( r'^v1/accounts/{}/verification_status/$'.format(settings.USERNAME_PATTERN), PhotoVerificationStatusView.as_view(),