Signals removal

Author: mhindery

README.rst

@@ -493,28 +493,6 @@ Learn more about Django profile models at:
 https://docs.djangoproject.com/en/dev/topics/auth/customizing/#substituting-a-custom-user-model
 
 
-Sometimes you need to use special logic to update the user object
-depending on the SAML2 attributes and the mapping described above
-is simply not enough. For these cases djangosaml2 provides a Django
-signal that you can listen to. In order to do so you can add the
-following code to your app::
-
-  from djangosaml2.signals import pre_user_save
-
-  def custom_update_user(sender=User, instance, attributes, user_modified, **kargs)
-     ...
-     return True  # I modified the user object
-
-
-Your handler will receive the user object, the list of SAML attributes
-and a flag telling you if the user is already modified and need
-to be saved after your handler is executed. If your handler
-modifies the user object it should return True. Otherwise it should
-return False. This way djangosaml2 will know if it should save
-the user object so you don't need to do it and no more calls to
-the save method are issued.
-
-
 IdP setup
 =========
 Congratulations, you have finished configuring the SP side of the federation.

djangosaml2/apps.py

@@ -4,6 +4,3 @@
 class DjangoSaml2Config(AppConfig):
     name = 'djangosaml2'
     verbose_name = "DjangoSAML2"
-
-    def ready(self):
-        from . import signals  # noqa

djangosaml2/backends.py

@@ -24,8 +24,6 @@
 from django.core.exceptions import (ImproperlyConfigured,
                                     MultipleObjectsReturned)
 
-from .signals import pre_user_save
-
 logger = logging.getLogger('djangosaml2')
 
 
@@ -143,14 +141,15 @@ def _update_user(self, user, attributes: dict, attribute_mapping: dict, force_sa
             SAML_ATTRIBUTE_MAPPING. For each attribute, if the user object has
             that field defined it will be set.
         """
-        # Always save a brand new user instance
-        user_modified = user.pk is None
 
+        # No attributes to set on the user instance, nothing to update
         if not attribute_mapping:
-            if user_modified:
-                user.save()
+            # Always save a brand new user instance
+            if user.pk is None:
+                user = self.save_user(user)
             return user
 
+        has_updated_fields = False
         for saml_attr, django_attrs in attribute_mapping.items():
             attr_value_list = attributes.get(saml_attr)
             if not attr_value_list:
@@ -167,15 +166,12 @@ def _update_user(self, user, attributes: dict, attribute_mapping: dict, force_sa
                     else:
                         modified = set_attribute(user, attr, attr_value_list[0])
 
-                    user_modified = user_modified or modified
+                    has_updated_fields = has_updated_fields or modified
                 else:
                     logger.debug('Could not find attribute "%s" on user "%s"', attr, user)
 
-        signal_modified = self.send_user_update_signal(user, attributes, user_modified)
-
-        if user_modified or signal_modified or force_save:
-            user.save()
-            logger.debug('User updated with incoming attributes')
+        if has_updated_fields or force_save:
+            user = self.save_user(user)
 
         return user
 
@@ -228,20 +224,18 @@ def get_or_create_user(self,
 
         return user, created
 
-    def send_user_update_signal(self, user: settings.AUTH_USER_MODEL, attributes: dict, user_modified: bool) -> bool:
-        """ Send out a pre-save signal after the user has been updated with the SAML attributes.
-            This does not have to be overwritten, but depending on your custom implementation of get_or_create_user,
-            you might want to not send out this signal. In that case, just override this method to return False.
+    def save_user(self, user: settings.AUTH_USER_MODEL, *args, **kwargs) -> settings.AUTH_USER_MODEL:
+        """ Hook to add custom logic around saving a user. Return the saved user instance.
         """
-        logger.debug('Sending the pre_save signal')
-        signal_modified = any(
-            [response for receiver, response
-             in pre_user_save.send_robust(sender=user.__class__,
-                                          instance=user,
-                                          attributes=attributes,
-                                          user_modified=user_modified)]
-            )
-        return signal_modified
+        is_new_instance = user.pk is None
+        user.save()
+
+        if is_new_instance:
+            logger.debug('New user created')
+        else:
+            logger.debug('User %s updated with incoming attributes', user)
+
+        return user
 
     # ############################################
     # Backwards-compatibility stubs

djangosaml2/signals.py

@@ -35,7 +35,6 @@
 from djangosaml2.cache import OutstandingQueriesCache
 from djangosaml2.conf import get_config
 from djangosaml2.middleware import SamlSessionMiddleware
-from djangosaml2.signals import post_authenticated, pre_user_save
 from djangosaml2.tests import conf
 from djangosaml2.utils import (get_custom_setting,
                                get_idp_sso_supported_bindings,
@@ -599,52 +598,6 @@ def _test_metadata(self):
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.content, expected_metadata)
 
-    def test_post_authenticated_signal(self):
-        self.called = []
-
-        def signal_handler(sender, instance, session_info, request, **kwargs):
-            self.called.append({'sender': sender, 'instance': instance, 'request': request, 'session_info': session_info})
-
-        post_authenticated.connect(signal_handler, dispatch_uid='test_signal')
-
-        self.do_login()
-
-        # make sure the handler is only called once
-        self.assertEqual(len(self.called), 1)
-        # test 'sender', this should be User.__class__
-        self.assertEqual(self.called[0]['sender'], get_user_model(), 'post_authenticated signal sender is not a User')
-        # test 'instance', this should be User
-        self.assertIsInstance(self.called[0]['instance'], get_user_model(), 'post_authenticated signal did not send a User instance')
-        # test the request
-        self.assertIsInstance(self.called[0]['request'], HttpRequest, 'post_authenticated signal did not send a request')
-        # test the session_info
-        self.assertIsInstance(self.called[0]['session_info'], dict, 'post_authenticated signal did not send a session_info dict')
-
-        post_authenticated.disconnect(dispatch_uid='test_signal')
-
-    def test_pre_user_save_signal(self):
-        self.called = []
-
-        def signal_handler(sender, instance, attributes, user_modified, **kwargs):
-            self.called.append({'sender': sender, 'instance': instance, 'attributes': attributes, 'user_modified': user_modified})
-
-        pre_user_save.connect(signal_handler, dispatch_uid='test_signal')
-
-        self.do_login()
-
-        # make sure the handler is only called once
-        self.assertEqual(len(self.called), 1)
-        # test 'sender', this should be User.__class__
-        self.assertEqual(self.called[0]['sender'], get_user_model(), 'pre_user_save signal sender is not a User')
-        # test 'instance', this should be User
-        self.assertIsInstance(self.called[0]['instance'], get_user_model(), 'pre_user_save signal did not send a User instance')
-        # test the attributes
-        self.assertIsInstance(self.called[0]['attributes'], dict, 'pre_user_save signal did not send attributes')
-        # test the user_modified
-        self.assertIsInstance(self.called[0]['user_modified'], bool, 'pre_user_save signal did not send a user_modified bool')
-
-        pre_user_save.disconnect(dispatch_uid='test_signal')
-
     def test_sigalg_not_passed_when_not_signing_request(self):
         # monkey patch SAML configuration
         settings.SAML_CONFIG = conf.create_conf(

djangosaml2/tests/__init__.py

@@ -48,7 +48,6 @@
 from .conf import get_config
 from .exceptions import IdPConfigurationMissing
 from .overrides import Saml2Client
-from .signals import post_authenticated
 from .utils import (available_idps, fail_acs_response, get_custom_setting,
                     get_idp_sso_supported_bindings, get_location,
                     validate_referral_url)
@@ -340,14 +339,8 @@ def post(self, request, attribute_mapping=None, create_unknown_user=None):
         auth.login(self.request, user)
         _set_subject_id(request.saml_session, session_info['name_id'])
         logger.debug("User %s authenticated via SSO.", user)
-        logger.debug('Sending the post_authenticated signal')
-
-        # post_authenticated.send_robust(sender=user, session_info=session_info)
-        # https://github.com/knaperek/djangosaml2/issues/117
-        post_authenticated.send_robust(sender=user.__class__,
-                                       instance=user,
-                                       session_info=session_info,
-                                       request=request)
+
+        self.post_login_hook(request, user, session_info)
         self.customize_session(user, session_info)
 
         relay_state = self.build_relay_state()
@@ -358,6 +351,11 @@ def post(self, request, attribute_mapping=None, create_unknown_user=None):
         logger.debug('Redirecting to the RelayState: %s', relay_state)
         return HttpResponseRedirect(relay_state)
 
+    def post_login_hook(self, request: HttpRequest, user: settings.AUTH_USER_MODEL, session_info: dict) -> None:
+        """ If desired, a hook to add logic after a user has succesfully logged in.
+        """
+        pass
+
     def build_relay_state(self) -> str:
         """ The relay state is a URL used to redirect the user to the view where they came from.
         """