Merge pull request #7451 from edx/pmitros/disable-email-auth

Make e-mail activation optional

common/djangoapps/student/tests/test_create_account.py

@@ -175,7 +175,7 @@ class TestCreateAccount(TestCase):
         profile = self.create_account_and_fetch_profile()
         self.assertIsNone(profile.year_of_birth)
 
-    def base_extauth_bypass_sending_activation_email(self, bypass_activation_email_for_extauth_setting):
+    def base_extauth_bypass_sending_activation_email(self, bypass_activation_email):
         """
         Tests user creation without sending activation email when
         doing external auth
@@ -196,7 +196,7 @@ class TestCreateAccount(TestCase):
             student.views.create_account(request)
 
         # check that send_mail is called
-        if bypass_activation_email_for_extauth_setting:
+        if bypass_activation_email:
             self.assertFalse(mock_send_mail.called)
         else:
             self.assertTrue(mock_send_mail.called)
@@ -219,6 +219,15 @@ class TestCreateAccount(TestCase):
         """
         self.base_extauth_bypass_sending_activation_email(False)
 
+    @unittest.skipUnless(settings.FEATURES.get('AUTH_USE_SHIB'), "AUTH_USE_SHIB not set")
+    @mock.patch.dict(settings.FEATURES, {'BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH': False, 'AUTOMATIC_AUTH_FOR_TESTING': False, 'SKIP_EMAIL_VALIDATION': True})
+    def test_extauth_bypass_sending_activation_email_without_bypass(self):
+        """
+        Tests user creation without sending activation email when
+        settings.FEATURES['BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH']=False and doing external auth
+        """
+        self.base_extauth_bypass_sending_activation_email(True)
+
     @ddt.data(True, False)
     def test_discussions_email_digest_pref(self, digest_enabled):
         with mock.patch.dict("student.models.settings.FEATURES", {"ENABLE_DISCUSSION_EMAIL_DIGEST": digest_enabled}):

common/djangoapps/student/views.py

@@ -23,7 +23,7 @@ from django.core.urlresolvers import reverse
 from django.core.validators import validate_email, ValidationError
 from django.db import IntegrityError, transaction
 from django.http import (HttpResponse, HttpResponseBadRequest, HttpResponseForbidden,
-                         Http404)
+                         HttpResponseServerError, Http404)
 from django.shortcuts import redirect
 from django.utils.translation import ungettext
 from django_future.csrf import ensure_csrf_cookie
@@ -1408,6 +1408,19 @@ def create_account_with_params(request, params):
     Raises AccountValidationError if an account with the username or email
     specified by params already exists, or ValidationError if any of the given
     parameters is invalid for any other reason.
+
+    Issues with this code:
+    * It is not transactional. If there is a failure part-way, an incomplete
+      account will be created and left in the database.
+    * Third-party auth passwords are not verified. There is a comment that
+      they are unused, but it would be helpful to have a sanity check that
+      they are sane.
+    * It is over 300 lines long (!) and includes disprate functionality, from
+      registration e-mails to all sorts of other things. It should be broken
+      up into semantically meaningful functions.
+    * The user-facing text is rather unfriendly (e.g. "Username must be a
+      minimum of two characters long" rather than "Please use a username of
+      at least two characters").
     """
     # Copy params so we can modify it; we can't just do dict(params) because if
     # params is request.POST, that results in a dict containing lists of values
@@ -1555,9 +1568,19 @@ def create_account_with_params(request, params):
     subject = ''.join(subject.splitlines())
     message = render_to_string('emails/activation_email.txt', context)
 
-    # don't send email if we are doing load testing or random user generation for some reason
-    # or external auth with bypass activated
+    # Don't send email if we are:
+    #
+    # 1. Doing load testing.
+    # 2. Random user generation for other forms of testing.
+    # 3. External auth bypassing activation.
+    # 4. Have the platform configured to not require e-mail activation.
+    #
+    # Note that this feature is only tested as a flag set one way or
+    # the other for *new* systems. we need to be careful about
+    # changing settings on a running system to make sure no users are
+    # left in an inconsistent state (or doing a migration if they are).
     send_email = (
+        not settings.FEATURES.get('SKIP_EMAIL_VALIDATION', None) and
         not settings.FEATURES.get('AUTOMATIC_AUTH_FOR_TESTING') and
         not (do_external_auth and settings.FEATURES.get('BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH'))
     )
@@ -1576,6 +1599,8 @@ def create_account_with_params(request, params):
                 user.email_user(subject, message, from_address)
         except Exception:  # pylint: disable=broad-except
             log.error(u'Unable to send activation email to user from "%s"', from_address, exc_info=True)
+    else:
+        registration.activate()
 
     # Immediately after a user creates an account, we log them in. They are only
     # logged in until they close the browser. They can't log in again until they click
@@ -1794,7 +1819,7 @@ def activate_account(request, key):
             "registration/activation_invalid.html",
             {'csrf': csrf(request)['csrf_token']}
         )
-    return HttpResponse(_("Unknown error. Please e-mail us to let us know how it happened."))
+    return HttpResponseServerError(_("Unknown error. Please e-mail us to let us know how it happened."))
 
 
 @csrf_exempt