user_logged_in instead of update_last_login

[Ou7law007]

I did not see a reason to call update_last_login directly.

This allows code like the following to function properly.

# my_app.signals
# Track last_login & previous_login (custom field)
def update_last_and_previous_login(sender, user, **kwargs):
    User.objects.filter(pk=user.pk).update(previous_login=user.last_login, last_login=timezone.now())

user_logged_in.disconnect(update_last_login, dispatch_uid='update_last_login')
user_logged_in.connect(update_last_and_previous_login, dispatch_uid='update_last_and_previous_login')

I also thought of providing the data variable as a kwarg to allow even more flexibility.

user_logged_in.send(sender=self.user.__class__, user=self.user, **data) or user_logged_in.send(sender=self.user.__class__, user=self.user, data=data)

[Ou7law007]

By pure coincidence, I just tried the library django-axes and without this code, it raises:

TypeError: AxesProxyHandler.user_logged_in() missing 1 required positional argument: 'request'

So this code seems to fix this issue as well. I don't understand the reason for calling the signals directly anyway!

[Ou7law007]

The new commit fixes the django-axes issue mentioned above.

[Ou7law007]

Also, the 2 user_logged_in signals should no be tied to api_settings.UPDATE_LAST_LOGIN.

I think api_settings.UPDATE_LAST_LOGIN is something else and that's why the original code called djangorestframework-simplejwt's signal directly.

This must be updated. Example:

# Send this signal anyway
user_logged_in.send(sender=self.__class__, user=self.user, data=data, request=self.context.get('request'))

if api_settings.UPDATE_LAST_LOGIN:
    # Send the signal that updates the user's last login (as in the original code)

[Ou7law007]

One could (or even should) also add user_logged_out and user_login_failed signals to TokenObtainSerializer and TokenBlacklistSerializer respectively.

In which case, I don't know if any of the signals also applies to any of the rest of the serializers (TokenRefreshSerializer, TokenRefreshSlidingSerializer, TokenVerifySerializer).

[Ou7law007]

Let me know if this is going anywhere or not.

[Andrew-Chen-Wang]

Hi please rebase with master

[mdefeche]

@Andrew-Chen-Wang, is it possible to revive this PR ? Or create another one ? We need to track user logins and need to implement new serializers just to be able to send signals. As discussed here, sending user_login_failed would also make sense.

[Andrew-Chen-Wang]

if you'd like to use signals, I'd love if you could open a new PR and create a custom function with a custom setting pointing to the path of the function. It is similar to what we did with custom authentication rules! Thanks!

I personally dislike signals for performance and robustness reasons, but a custom function will work to accommodate old and customized functionality