Skip to content

Commit

Permalink
Fix #1140 -- Update change-email process (#1348)
Browse files Browse the repository at this point in the history
* Fix #1140 -- Update change-email process

* Addressing the review

* Forgot the other template

* Add custom authentication backend

* Add infor message when email is not verified

* Fix message
  • Loading branch information
oliverroick authored and amplifi committed Apr 4, 2017
1 parent b5049c5 commit 99f3054
Show file tree
Hide file tree
Showing 13 changed files with 167 additions and 20 deletions.
20 changes: 20 additions & 0 deletions cadasta/accounts/backends.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
from allauth.account.auth_backends import AuthenticationBackend as Backend
from .models import User


class AuthenticationBackend(Backend):
def _authenticate_by_email(self, **credentials):
# Even though allauth will pass along `email`, other apps may
# not respect this setting. For example, when using
# django-tastypie basic authentication, the login is always
# passed as `username`. So let's place nice with other apps
# and use username as fallback
email = credentials.get('email', credentials.get('username'))
try:
user = User.objects.get(email__iexact=email)
if user.check_password(credentials["password"]):
return user
except User.DoesNotExist:
pass

return None
13 changes: 6 additions & 7 deletions cadasta/accounts/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
from allauth.account.utils import send_email_confirmation
from allauth.account import forms as allauth_forms

from .models import User, now_plus_48_hours
from .utils import send_email_update_notification
from .models import User
from .validators import check_username_case_insensitive
from parsley.decorators import parsleyfy

Expand Down Expand Up @@ -71,6 +72,7 @@ def __init__(self, *args, **kwargs):
self._send_confirmation = False
self.request = kwargs.pop('request', None)
super().__init__(*args, **kwargs)
self.current_email = self.instance.email

def clean_username(self):
username = self.data.get('username')
Expand All @@ -84,8 +86,6 @@ def clean_username(self):
def clean_email(self):
email = self.data.get('email')
if self.instance.email != email:
self._send_confirmation = True

if User.objects.filter(email=email).exists():
raise forms.ValidationError(
_("Another user with this email already exists"))
Expand All @@ -98,11 +98,10 @@ def clean_email(self):

def save(self, *args, **kwargs):
user = super().save(commit=False, *args, **kwargs)
if self._send_confirmation:
if self.current_email != user.email:
send_email_confirmation(self.request, user)
self._send_confirmation = False
user.email_verified = False
user.verify_email_by = now_plus_48_hours()
send_email_update_notification(self.current_email)
user.email = self.current_email

user.save()
return user
Expand Down
37 changes: 37 additions & 0 deletions cadasta/accounts/tests/test_backend.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
from django.test import TestCase
from allauth.account.models import EmailAddress
from core.tests.utils.cases import UserTestCase
from ..backends import AuthenticationBackend
from .factories import UserFactory


class AuthBackendTest(UserTestCase, TestCase):
def setUp(self):
super().setUp()
self.user = UserFactory.create(
username='kindofblue',
email='[email protected]',
password='PlayTh3Trumpet!'
)
self.backend = AuthenticationBackend()

def test_login_with_email(self):
credentials = {'email': '[email protected]',
'password': 'PlayTh3Trumpet!'}
assert self.backend._authenticate_by_email(**credentials) == self.user

def test_login_with_unapproved_email(self):
EmailAddress.objects.create(
user=self.user,
email='[email protected]',
verified=False,
primary=True
)
credentials = {'email': '[email protected]',
'password': 'PlayTh3Trumpet!'}
assert self.backend._authenticate_by_email(**credentials) is None

def test_auth_in_username_field(self):
credentials = {'username': '[email protected]',
'password': 'PlayTh3Trumpet!'}
assert self.backend._authenticate_by_email(**credentials) == self.user
7 changes: 5 additions & 2 deletions cadasta/accounts/tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,11 @@ def test_update_user(self):

user.refresh_from_db()
assert user.full_name == 'John Lennon'
assert user.email_verified is False
assert len(mail.outbox) == 1
assert user.email == '[email protected]'
assert user.email_verified is True
assert len(mail.outbox) == 2
assert '[email protected]' in mail.outbox[0].to
assert '[email protected]' in mail.outbox[1].to

def test_display_name(self):
user = UserFactory.create(username='imagine71',
Expand Down
6 changes: 4 additions & 2 deletions cadasta/accounts/tests/test_views_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@ def test_update_email_address(self):
data = {'email': '[email protected]', 'username': 'imagine71'}
response = self.request(method='PUT', post_data=data, user=self.user)
assert response.status_code == 200
assert len(mail.outbox) == 1
assert len(mail.outbox) == 2
assert '[email protected]' in mail.outbox[0].to
assert '[email protected]' in mail.outbox[1].to
self.user.refresh_from_db()
assert self.user.email_verified is False
assert self.user.email_verified is True

def test_keep_email_address(self):
"""Service should not send a verification email when the user does not
Expand Down
50 changes: 49 additions & 1 deletion cadasta/accounts/tests/test_views_default.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ class ProfileTest(ViewTestCase, UserTestCase, TestCase):

def setup_template_context(self):
return {
'form': ProfileForm(instance=self.user)
'form': ProfileForm(instance=self.user),
'emails_to_verify': False
}

def test_get_profile(self):
Expand All @@ -30,6 +31,32 @@ def test_get_profile(self):
assert response.status_code == 200
assert response.content == self.expected_content

def test_get_profile_with_unverified_email(self):
self.user = UserFactory.create()
EmailAddress.objects.create(
user=self.user,
email='[email protected]',
verified=False,
primary=True
)
response = self.request(user=self.user)

assert response.status_code == 200
assert response.content == self.render_content(emails_to_verify=True)

def test_get_profile_with_verified_email(self):
self.user = UserFactory.create()
EmailAddress.objects.create(
user=self.user,
email=self.user.email,
verified=True,
primary=True
)
response = self.request(user=self.user)

assert response.status_code == 200
assert response.content == self.expected_content

def test_update_profile(self):
user = UserFactory.create(username='John')
post_data = {
Expand Down Expand Up @@ -144,6 +171,27 @@ def test_activate(self):
self.email_address.refresh_from_db()
assert self.email_address.verified is True

def test_activate_changed_email(self):
self.email_address.email = '[email protected]'
self.email_address.save()

EmailConfirmation.objects.create(
email_address=self.email_address,
sent=datetime.datetime.now(),
key='456'
)
response = self.request(user=self.user, url_kwargs={'key': '456'})
assert response.status_code == 302
assert 'dashboard' in response.location

self.user.refresh_from_db()
assert self.user.email_verified is True
assert self.user.is_active is True
assert self.user.email == '[email protected]'

self.email_address.refresh_from_db()
assert self.email_address.verified is True

def test_activate_with_invalid_token(self):
response = self.request(user=self.user, url_kwargs={'key': 'abc'})
assert response.status_code == 200
Expand Down
16 changes: 16 additions & 0 deletions cadasta/accounts/utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
from django.conf import settings
from django.utils.translation import ugettext as _
from django.core.mail import send_mail
from django.template.loader import render_to_string


def send_email_update_notification(email):
msg_body = render_to_string(
'accounts/email/email_changed_notification.txt')
send_mail(
_("Change of email at Cadasta Platform"),
msg_body,
settings.DEFAULT_FROM_EMAIL,
[email],
fail_silently=False,
)
11 changes: 6 additions & 5 deletions cadasta/accounts/views/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from allauth.account.signals import password_changed

from .. import serializers
from ..models import now_plus_48_hours
from ..utils import send_email_update_notification
from ..exceptions import EmailNotVerifiedError


Expand All @@ -20,17 +20,18 @@ class AccountUser(djoser_views.UserView):

def perform_update(self, serializer):
user = self.get_object()
old_email = user.email
new_email = serializer.validated_data.get('email', user.email)

if user.email != new_email:
updated = serializer.save(
email_verified=False,
verify_email_by=now_plus_48_hours()
)
updated = serializer.save(email=old_email)
updated.email = new_email
try:
send_email_confirmation(self.request._request, updated)
except MessageFailure:
pass
send_email_update_notification(old_email)

else:
serializer.save()

Expand Down
9 changes: 9 additions & 0 deletions cadasta/accounts/views/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import allauth.account.views as allauth_views
from allauth.account.views import ConfirmEmailView, LoginView
from allauth.account.utils import send_email_confirmation
from allauth.account.models import EmailAddress

from ..models import User
from .. import forms
Expand Down Expand Up @@ -41,6 +42,13 @@ class AccountProfile(LoginRequiredMixin, UpdateView):
def get_object(self, *args, **kwargs):
return self.request.user

def get_context_data(self, *args, **kwargs):
context = super().get_context_data(*args, **kwargs)
emails_to_verify = EmailAddress.objects.filter(
user=self.object, verified=False).exists()
context['emails_to_verify'] = emails_to_verify
return context

def get_form_kwargs(self, *args, **kwargs):
form_kwargs = super().get_form_kwargs(*args, **kwargs)
form_kwargs['request'] = self.request
Expand Down Expand Up @@ -73,6 +81,7 @@ def post(self, *args, **kwargs):
response = super().post(*args, **kwargs)

user = self.get_object().email_address.user
user.email = self.get_object().email_address.email
user.email_verified = True
user.is_active = True
user.save()
Expand Down
2 changes: 1 addition & 1 deletion cadasta/config/settings/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@
AUTHENTICATION_BACKENDS = [
'core.backends.Auth',
'django.contrib.auth.backends.ModelBackend',
'allauth.account.auth_backends.AuthenticationBackend'
'accounts.backends.AuthenticationBackend'
]

ACCOUNT_AUTHENTICATION_METHOD = 'username_email'
Expand Down
11 changes: 11 additions & 0 deletions cadasta/templates/accounts/email/email_changed_notification.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{% load i18n %}{% autoescape off %}

{% blocktrans %}
You are receiving this email because a user at Cadasta Platform changed the email address for the account previously linked to this email address.

If it wasn't you who changed the email address, please contact us immediately under [email protected].
{% endblocktrans %}

{% blocktrans %}The Cadasta Team. {% endblocktrans %}

{% endautoescape %}
1 change: 1 addition & 0 deletions cadasta/templates/accounts/profile.html
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ <h1>{% trans "Update your profile" %}</h1>
<div class="form-group{% if form.email.errors %} has-error{% endif %}">
<label class="control-label" for="{{ form.email.id_for_label }}">{% trans "Email" %}</label>
{% render_field form.email class+="form-control input-lg" data-parsley-required="true" %}
{% if emails_to_verify %}<p class="help-block small">{% trans "The email for this account has been changed recently, but the new email address has not yet been verified. You should have received an email with a link to verify the new email address." %}</p>{% endif %}
<div class="error-block">{{ form.email.errors }}</div>
</div>

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{% load account %}{% user_display user as user_display %}{% load i18n %}{% autoescape off %}{% blocktrans %}
{% load account %}{% load i18n %}{% autoescape off %}{% blocktrans %}

You're receiving this email because user {{ user_display }} at Cadasta Platform has given yours as an email address to connect their account.
You are receiving this email because a user at Cadasta Platform has given yours as an email address to connect their account.

To confirm this is correct, go to {{ activate_url }}
{% endblocktrans %}{% endautoescape %}
Expand Down

0 comments on commit 99f3054

Please sign in to comment.