Fix activate request email not sent bug on registration, and apply to shibboleth login

seahub/base/accounts.py

@@ -1,5 +1,6 @@
 # encoding: utf-8
 from django import forms
+from django.core.mail import send_mail
 from django.utils.encoding import smart_str
 from django.utils.translation import ugettext_lazy as _
 from django.conf import settings
@@ -230,7 +231,6 @@ class User(object):
 
     def email_user(self, subject, message, from_email=None):
         "Sends an e-mail to this User."
-        from django.core.mail import send_mail
         send_mail(subject, message, from_email, [self.email])
 
     def remove_repo_passwds(self):

tests/seahub/registraion/test_can_email_admins_on_registration.py

@@ -0,0 +1,29 @@
+from django.core import mail
+from django.core.urlresolvers import reverse
+
+from constance import config
+
+from seahub.test_utils import BaseTestCase
+
+
+class EmailAdminOnRegistrationTest(BaseTestCase):
+    def setUp(self):
+        config.ENABLE_SIGNUP = True
+        self.email = 'newuser@test.com'
+        self.remove_user(self.email)
+
+    def test_can_notify_admin(self):
+        assert bool(config.ENABLE_SIGNUP) is True
+        self.assertEqual(len(mail.outbox), 0)
+
+        config.ACTIVATE_AFTER_REGISTRATION = False
+        config.REGISTRATION_SEND_MAIL = False
+
+        resp = self.client.post(reverse('registration_register'), {
+            'email': self.email,
+            'password1': '123',
+            'password2': '123'
+        })
+        self.assertRedirects(resp, 'http://testserver/accounts/register/complete/')
+        assert len(mail.outbox) != 0
+        assert 'a newly registered account need to be activated' in mail.outbox[0].body

tests/seahub/thirdpart/shibboleth/test_backends.py

@@ -0,0 +1,88 @@
+from django.core import mail
+from django.conf import settings
+
+from shibboleth import backends
+
+from seahub.base.accounts import User
+from seahub.auth import authenticate
+from seahub.test_utils import BaseTestCase
+
+SAMPLE_HEADERS = {
+    "REMOTE_USER": 'sampledeveloper@school.edu',
+    "Shib-Application-ID": "default",
+    "Shib-Authentication-Method": "urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified",
+    "Shib-AuthnContext-Class": "urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified",
+    "Shib-Identity-Provider": "https://sso.college.edu/idp/shibboleth",
+    "Shib-Session-ID": "1",
+    "Shib-Session-Index": "12",
+    "Shibboleth-affiliation": "member@college.edu;staff@college.edu",
+    "Shibboleth-schoolBarCode": "12345678",
+    "Shibboleth-schoolNetId": "Sample_Developer",
+    "Shibboleth-schoolStatus": "active",
+    "Shibboleth-department": "University Library, Integrated Technology Services",
+    "Shibboleth-displayName": "Sample Developer",
+    "Shibboleth-eppn": "sampledeveloper@school.edu",
+    "Shibboleth-givenName": "Sample",
+    "Shibboleth-isMemberOf": "SCHOOL:COMMUNITY:EMPLOYEE:ADMINISTRATIVE:BASE;SCHOOL:COMMUNITY:EMPLOYEE:STAFF:SAC:P;COMMUNITY:ALL;SCHOOL:COMMUNITY:EMPLOYEE:STAFF:SAC:M;",
+    "Shibboleth-mail": "Sample_Developer@school.edu",
+    "Shibboleth-persistent-id": "https://sso.college.edu/idp/shibboleth!https://server.college.edu/shibboleth-sp!sk1Z9qKruvXY7JXvsq4GRb8GCUk=",
+    "Shibboleth-sn": "Developer",
+    "Shibboleth-title": "Library Developer",
+    "Shibboleth-unscoped-affiliation": "member;staff"
+}
+
+settings.SHIBBOLETH_ATTRIBUTE_MAP = {
+   "Shib-Identity-Provider": (True, "idp"),
+   "Shibboleth-mail": (True, "email"),
+   "Shibboleth-eppn": (True, "username"),
+   "Shibboleth-schoolStatus": (True, "status"),
+   "Shibboleth-affiliation": (True, "affiliation"),
+   "Shib-Session-ID": (True, "session_id"),
+   "Shibboleth-givenName": (True, "first_name"),
+   "Shibboleth-sn": (True, "last_name"),
+   "Shibboleth-mail": (True, "email"),
+   "Shibboleth-schoolBarCode": (False, "barcode")
+}
+
+settings.AUTHENTICATION_BACKENDS += (
+    'shibboleth.backends.ShibbolethRemoteUserBackend',
+)
+
+settings.MIDDLEWARE_CLASSES += (
+    'shibboleth.middleware.ShibbolethRemoteUserMiddleware',
+)
+
+
+class ShibbolethRemoteUserBackendTest(BaseTestCase):
+    def setUp(self):
+        self.remote_user = 'sampledeveloper@school.edu'
+        self.remove_user(self.remote_user)
+
+    def test_create_unknown_user(self):
+        with self.assertRaises(User.DoesNotExist):
+            self.assertFalse(User.objects.get(self.remote_user))
+
+        user = authenticate(remote_user=self.remote_user,
+                            shib_meta=SAMPLE_HEADERS)
+        assert user.is_active is True
+        self.assertEqual(user.username, 'sampledeveloper@school.edu')
+        self.assertEqual(User.objects.get(self.remote_user).username,
+                         'sampledeveloper@school.edu')
+
+    def test_notify_admins_on_activate_request(self):
+        self.assertEqual(len(mail.outbox), 0)
+        with self.assertRaises(User.DoesNotExist):
+            self.assertFalse(User.objects.get(self.remote_user))
+
+        with self.settings(SHIB_ACTIVATE_AFTER_CREATION=False):
+            # reload our shibboleth.backends module, so it picks up the settings change
+            reload(backends)
+            user = authenticate(remote_user=self.remote_user,
+                                shib_meta=SAMPLE_HEADERS)
+            self.assertEqual(user.username, 'sampledeveloper@school.edu')
+            assert user.is_active is False
+
+        assert len(mail.outbox) != 0
+        assert 'a newly registered account need to be activated' in mail.outbox[0].body
+        # now reload again, so it reverts to original settings
+        reload(backends)

thirdpart/registration/models.py

@@ -302,6 +302,29 @@ from constance import config
 # Get an instance of a logger
 logger = logging.getLogger(__name__)
 
+def notify_admins_on_activate_request(reg_email):
+    ctx_dict = {
+        "site_name": settings.SITE_NAME,
+        "user_search_link": "%s%s?email=%s" % (
+            get_site_scheme_and_netloc(), reverse("user_search"),
+            urlquote(reg_email)),
+    }
+
+    subject = render_to_string('registration/activate_request_email_subject.txt',
+                               ctx_dict)
+    # Email subject *must not* contain newlines
+    subject = ''.join(subject.splitlines())
+
+    message = render_to_string('registration/activate_request_email.txt',
+                               ctx_dict)
+
+    admins = User.objects.get_superusers()
+    for admin in admins:
+        try:
+            admin.email_user(subject, message, settings.DEFAULT_FROM_EMAIL)
+        except Exception as e:
+            logger.error(e)
+
 @receiver(user_registered)
 def email_admin_on_registration(sender, **kwargs):
     """Send an email notification to admin when a newly registered user need
@@ -310,28 +333,7 @@ def email_admin_on_registration(sender, **kwargs):
     This email will be sent when both ``ACTIVATE_AFTER_REGISTRATION`` and
     ``REGISTRATION_SEND_MAIL`` are set to False.
     """
-
+    if bool(config.ACTIVATE_AFTER_REGISTRATION) is False and \
-    if config.ACTIVATE_AFTER_REGISTRATION is False and \
+            bool(config.REGISTRATION_SEND_MAIL) is False:
-            config.REGISTRATION_SEND_MAIL is False:
         reg_email = kwargs['user'].email
-
+        notify_admins_on_activate_request(reg_email)
-        ctx_dict = {
-            "site_name": settings.SITE_NAME,
-            "user_search_link": "%s%s?email=%s" % (
-                get_site_scheme_and_netloc(), reverse("user_search"),
-                urlquote(reg_email)),
-            }
-        subject = render_to_string('registration/activate_request_email_subject.txt',
-                                   ctx_dict)
-        # Email subject *must not* contain newlines
-        subject = ''.join(subject.splitlines())
-        
-        message = render_to_string('registration/activate_request_email.txt',
-                                   ctx_dict)
-
-        admins = User.objects.get_superusers()
-        for admin in admins:
-            try:
-                admin.email_user(subject, message, settings.DEFAULT_FROM_EMAIL)
-            except Exception as e:
-                logger.error(e)

thirdpart/shibboleth/backends.py

@@ -1,7 +1,9 @@
+from django.conf import settings
 from django.db import connection
 from django.contrib.auth.backends import RemoteUserBackend
 
 from seahub.base.accounts import User
+from registration.models import notify_admins_on_activate_request
 
 class ShibbolethRemoteUserBackend(RemoteUserBackend):
     """
@@ -17,6 +19,8 @@ class ShibbolethRemoteUserBackend(RemoteUserBackend):
 
     # Create a User object if not already in the database?
     create_unknown_user = True
+    # Create active user by default.
+    activate_after_creation = getattr(settings, 'SHIB_ACTIVATE_AFTER_CREATION', True)
 
     def get_user(self, username):
         try:
@@ -36,17 +40,17 @@ class ShibbolethRemoteUserBackend(RemoteUserBackend):
         """
         if not remote_user:
             return
-        user = None
+
         username = self.clean_username(remote_user)
-        # Note that this could be accomplished in one try-except clause, but
-        # instead we use get_or_create when creating unknown users since it has
-        # built-in safeguards for multiple threads.
         try:
             user = User.objects.get(email=username)
         except User.DoesNotExist:
             if self.create_unknown_user:
-                user = User.objects.create_user(email=username, is_active=True)
+                user = User.objects.create_user(
+                    email=username, is_active=self.activate_after_creation)
+                if user and self.activate_after_creation is False:
+                    notify_admins_on_activate_request(user.email)
             else:
-                pass
+                user = None
 
         return user