Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reregistration Flows #215

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions djoser/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
'USE_HTML_EMAIL_TEMPLATES': False,
'SEND_ACTIVATION_EMAIL': False,
'SEND_CONFIRMATION_EMAIL': False,
'RESEND_REGISTRATION_EMAIL': False,
'SET_PASSWORD_RETYPE': False,
'SET_USERNAME_RETYPE': False,
'PASSWORD_RESET_CONFIRM_RETYPE': False,
Expand All @@ -38,6 +39,7 @@
},
'LOGOUT_ON_PASSWORD_CHANGE': False,
'USER_EMAIL_FIELD_NAME': 'email',
'REGISTRATION_SHOW_EMAIL_FOUND': True,
}

SETTINGS_TO_IMPORT = ['TOKEN_MODEL']
Expand Down
1 change: 0 additions & 1 deletion djoser/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ def perform_create(self, validated_data):
return user



class LoginSerializer(serializers.Serializer):
password = serializers.CharField(
required=False, style={'input_type': 'password'}
Expand Down
17 changes: 17 additions & 0 deletions djoser/templates/reregistration_email_body.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{% load i18n %}{% autoescape off %}
<html>
<body>
<p>
{% blocktrans %}You're receiving this email because someone tried to reregister a user account with this email at {{ site_name }}.{% endblocktrans %}</p>

<p>{% trans "If this wasn't you, please go to the following page and choose a new password:" %}</p>
<p>{% block reset_link %}
<a href="{{ protocol }}://{{ domain }}/{{ url }}">{{ protocol }}://{{ domain }}/{{ url }}</a>
{% endblock %}</p>
<p>{% trans "Your username, in case you've forgotten:" %} <b>{{ user.get_username }}</b></p>

<p>{% trans "Thanks for using our site!" %}</p>

<p>{% blocktrans %}The {{ site_name }} team{% endblocktrans %}</p>
</body></html>
{% endautoescape %}
14 changes: 14 additions & 0 deletions djoser/templates/reregistration_email_body.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{% load i18n %}{% autoescape off %}
{% blocktrans %}You're receiving this email because someone tried to reregister a user account with this email at {{ site_name }}.{% endblocktrans %}

{% trans "If this wasn't you, please go to the following page and choose a new password:" %}
{% block reset_link %}
{{ protocol }}://{{ domain }}/{{ url }}
{% endblock %}
{% trans "Your username, in case you've forgotten:" %} {{ user.get_username }}

{% trans "Thanks for using our site!" %}

{% blocktrans %}The {{ site_name }} team{% endblocktrans %}

{% endautoescape %}
3 changes: 3 additions & 0 deletions djoser/templates/reregistration_email_subject.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{% load i18n %}{% autoescape off %}
{% blocktrans %}Reregistration requested on {{ site_name }}{% endblocktrans %}
{% endautoescape %}
14 changes: 14 additions & 0 deletions djoser/templates/reregistration_inactive_email_body.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{% load i18n %}{% autoescape off %}
<html>
<body>
<p>
{% blocktrans %}You're receiving this email because someone tried to reregister your user account at {{ site_name }}.{% endblocktrans %}</p>
{% blocktrans %}Please find the previously sent activation email, or request a new one to properly set this account up{% endblocktrans %}

<p>{% trans "Your username, in case you've forgotten:" %} <b>{{ user.get_username }}</b></p>

<p>{% trans "Thanks for using our site!" %}</p>

<p>{% blocktrans %}The {{ site_name }} team{% endblocktrans %}</p>
</body></html>
{% endautoescape %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This template as well as djoser/templates/reregistration_inactive_email_body.txt could be merged with djoser/templates/reregistration_email_body.[html|txt] and you could simply use the if statement controlled by context variable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, wouldn't we prefer to resend activation email in case of such API call? Emails tend to (rarely) get lost e.g. temporary mail server downtime.

It might be a good idea to have a discussion on that @haxoza @KaczuH

11 changes: 11 additions & 0 deletions djoser/templates/reregistration_inactive_email_body.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{% load i18n %}{% autoescape off %}
{% blocktrans %}You're receiving this email because someone tried to reregister your user account at {{ site_name }}.{% endblocktrans %}
{% blocktrans %}Please find the previously sent activation email, or request a new one to properly set this account up{% endblocktrans %}

{% trans "Your username, in case you've forgotten:" %} {{ user.get_username }}

{% trans "Thanks for using our site!" %}

{% blocktrans %}The {{ site_name }} team{% endblocktrans %}

{% endautoescape %}
28 changes: 28 additions & 0 deletions djoser/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,34 @@ def get_context(self):
return context


class UserReregistrationEmailFactory(UserEmailFactoryBase):
subject_template_name = 'reregistration_email_subject.txt'
plain_body_template_name = 'reregistration_email_body.txt'
if settings.USE_HTML_EMAIL_TEMPLATES:
html_body_template_name = 'reregistration_email_body.html'

def get_context(self):
context = super(UserReregistrationEmailFactory, self).get_context()
context['url'] = settings.PASSWORD_RESET_CONFIRM_URL.format(
**context
)
return context


class UserReregistrationInactiveEmailFactory(UserEmailFactoryBase):
subject_template_name = 'reregistration_email_subject.txt'
plain_body_template_name = 'reregistration_inactive_email_body.txt'
if settings.USE_HTML_EMAIL_TEMPLATES:
html_body_template_name = 'reregistration_inactive_email_body.html'

def get_context(self):
context = super(UserReregistrationInactiveEmailFactory, self).get_context()
context['url'] = settings.PASSWORD_RESET_CONFIRM_URL.format(
**context
)
return context
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two factories are almost the same. I think you could merge them into one and control whether it's inactive or active use via context variable.



class UserConfirmationEmailFactory(UserEmailFactoryBase):
subject_template_name = 'confirmation_email_subject.txt'
plain_body_template_name = 'confirmation_email_body.txt'
Expand Down
39 changes: 39 additions & 0 deletions djoser/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,23 @@ class RegistrationView(generics.CreateAPIView):
permission_classes = (
permissions.AllowAny,
)
_users = None

def create(self, request, *args, **kwargs):
try:
email_field_name = get_user_email_field_name(User)
users = self.get_email_users(request.data.get(email_field_name))
for user in users:
serializer = self.get_serializer(instance=user)
if settings.RESEND_REGISTRATION_EMAIL:
self.resend_registration_email(user)
headers = self.get_success_headers(serializer.data)
if not settings.REGISTRATION_SHOW_EMAIL_FOUND and users:
return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers)
except User.DoesNotExist:
pass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if this try ... except ... block could not be replaced with some smart if statement, since you are not doing anything with the exception.

response = super(RegistrationView, self).create(request, *args, **kwargs)
return response
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be simplified into one line:

return super(RegistrationView, self).create(request, *args, **kwargs)


def perform_create(self, serializer):
user = serializer.save()
Expand All @@ -79,6 +96,28 @@ def send_confirmation_email(self, user):
email = email_factory.create()
email.send()

def resend_registration_email(self, user):
if user.is_active:
email_factory = utils.UserReregistrationEmailFactory.from_request(
self.request, user=user
)
else:
email_factory = utils.UserReregistrationInactiveEmailFactory.from_request(
self.request, user=user
)
email = email_factory.create()
email.send()

def get_email_users(self, email):
if self._users is None:
email_field_name = get_user_email_field_name(User)
email_users_kwargs = {
email_field_name + '__iexact': email,
}
email_users = User._default_manager.filter(**email_users_kwargs)
self._users = [u for u in email_users if u.has_usable_password()]
return self._users


class LoginView(utils.ActionViewMixin, generics.GenericAPIView):
"""
Expand Down
20 changes: 20 additions & 0 deletions docs/source/settings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,26 @@ If ``True``, register or activation endpoint will send confirmation email to use

**Default**: ``False``

RESEND_REGISTRATION_EMAIL
-------------------------

If ``True``, register endpoint will send warning and reminder email to user.
Only active users are able to reset their passwords (User.is_active is True).
The email will ask the User to reset their password since someone is trying to
re-register their account.
If User.is_active is False, will send a warning
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of code values are decorated with higlight like ``True``, but it seems that you have forgot about the others.


**Default**: ``False``

REGISTRATION_SHOW_EMAIL_FOUND
----------------------------

If ``True`` (default), the ``/register/`` endpoint will always return
a ``HTTP_201_CREATED`` response, as well as the UserSerializer serializer.data response
of one of those users, so that it makes it difficult to distinguish if a User has this email

**Default**: ``True``

ACTIVATION_URL
--------------

Expand Down