Improvement to user accounts (#798)

* Improvement to user accounts

- Fixes #766 -- Records email verification
- Send email and deactivate user if email is not confirmed
- Fix #676 -- Send confirmation email when email is changed
- Bump django-skivvy to 0.1.2

* Add migration to activate confirmed emails

* Add note to account inactive page to confirm email address

* Rename migration file

cadasta/accounts/forms.py

@@ -1,6 +1,8 @@
+from datetime import datetime, timezone, timedelta
 from django import forms
 from django.conf import settings
 from django.utils.translation import ugettext as _
+from allauth.account.utils import send_email_confirmation
 
 from .models import User
 
@@ -47,6 +49,11 @@ class Meta:
         model = User
         fields = ['username', 'email', 'full_name']
 
+    def __init__(self, *args, **kwargs):
+        self._send_confirmation = False
+        self.request = kwargs.pop('request', None)
+        super().__init__(*args, **kwargs)
+
     def clean_username(self):
         username = self.data.get('username')
         if (self.instance.username != username and
@@ -60,8 +67,23 @@ def clean_username(self):
 
     def clean_email(self):
         email = self.data.get('email')
-        if (self.instance.email != email and
-                User.objects.filter(email=email).exists()):
-            raise forms.ValidationError(
-                _("Another user with this email already exists"))
+        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"))
         return email
+
+    def save(self, *args, **kwargs):
+        user = super().save(commit=False, *args, **kwargs)
+
+        if self._send_confirmation:
+            send_email_confirmation(self.request, user)
+            self._send_confirmation = False
+            user.email_verified = False
+            user.verify_email_by = (datetime.now(tz=timezone.utc) +
+                                    timedelta(hours=48))
+
+        user.save()
+        return user

cadasta/accounts/migrations/0002_activate_users_20161014_0846.py

@@ -0,0 +1,35 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.9.6 on 2016-10-14 08:46
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+def activate_user(apps, schema_editor):
+    User = apps.get_model('accounts', 'User')
+    EmailAddress = apps.get_model('account', 'EmailAddress')
+
+    inactive_users = User.objects.filter(email_verified=False)
+    for user in inactive_users:
+        try:
+            email = EmailAddress.objects.get(user=user, email=user.email)
+            if email.verified:
+                user.is_active = True
+                user.email_verified = True
+                user.save()
+        except EmailAddress.DoesNotExist:
+            pass
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('accounts', '0001_initial'),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            activate_user,
+            reverse_code=migrations.RunPython.noop
+        )
+    ]

cadasta/accounts/tests/test_forms.py

@@ -2,6 +2,9 @@
 
 from django.utils.translation import gettext as _
 from django.test import TestCase
+from django.core import mail
+from django.http import HttpRequest
+from django.contrib.messages.storage.fallback import FallbackStorage
 
 from ..models import User
 from ..forms import RegisterForm, ProfileForm
@@ -78,17 +81,28 @@ def test_signup_with_restricted_username(self):
 class ProfileFormTest(UserTestCase, TestCase):
     def test_update_user(self):
         user = UserFactory.create(username='imagine71',
-                                  email='[email protected]')
+                                  email='[email protected]',
+                                  email_verified=True)
         data = {
             'username': 'imagine71',
-            'email': 'john@beatles.uk',
+            'email': 'john2@beatles.uk',
             'full_name': 'John Lennon',
         }
-        form = ProfileForm(data, instance=user)
+
+        request = HttpRequest()
+        setattr(request, 'session', 'session')
+        self.messages = FallbackStorage(request)
+        setattr(request, '_messages', self.messages)
+        request.META['SERVER_NAME'] = 'testserver'
+        request.META['SERVER_PORT'] = '80'
+
+        form = ProfileForm(data, request=request, instance=user)
         form.save()
 
         user.refresh_from_db()
         assert user.full_name == 'John Lennon'
+        assert user.email_verified is False
+        assert len(mail.outbox) == 1
 
     def test_display_name(self):
         user = UserFactory.create(username='imagine71',

cadasta/accounts/tests/test_urls_default.py

@@ -10,3 +10,17 @@ def test_profile(self):
 
         resolved = resolve('/account/profile/')
         assert resolved.func.__name__ == default.AccountProfile.__name__
+
+    def test_login(self):
+        assert reverse('account:login') == '/account/login/'
+
+        resolved = resolve('/account/login/')
+        assert resolved.func.__name__ == default.AccountLogin.__name__
+
+    def test_verify_email(self):
+        assert reverse('account:verify_email',
+                       kwargs={'key': '123'}) == '/account/confirm-email/123/'
+
+        resolved = resolve('/account/confirm-email/123/')
+        assert resolved.func.__name__ == default.ConfirmEmail.__name__
+        assert resolved.kwargs['key'] == '123'

cadasta/accounts/tests/test_views_default.py

@@ -1,9 +1,14 @@
+import datetime
 from django.test import TestCase
+from django.core import mail
 from skivvy import ViewTestCase
 
 from accounts.tests.factories import UserFactory
 from core.tests.utils.cases import UserTestCase
-from ..views.default import AccountProfile
+
+from allauth.account.models import EmailConfirmation, EmailAddress
+
+from ..views.default import AccountProfile, AccountLogin, ConfirmEmail
 from ..forms import ProfileForm
 
 
@@ -58,3 +63,72 @@ def test_update_profile_duplicate_email(self):
 
         response = self.request(method='POST', user=user2, post_data=post_data)
         assert 'Failed to update profile information' in response.messages
+
+
+class LoginTest(ViewTestCase, UserTestCase, TestCase):
+    view_class = AccountLogin
+
+    def setup_models(self):
+        self.user = UserFactory.create(username='imagine71',
+                                       email='[email protected]',
+                                       password='iloveyoko79')
+
+    def test_successful_login(self):
+        data = {'login': 'imagine71', 'password': 'iloveyoko79'}
+        response = self.request(method='POST', post_data=data)
+        assert response.status_code == 302
+        assert 'dashboard' in response.location
+
+    def test_successful_login_with_unverified_user(self):
+        self.user.verify_email_by = datetime.datetime.now()
+        self.user.save()
+
+        data = {'login': 'imagine71', 'password': 'iloveyoko79'}
+        response = self.request(method='POST', post_data=data)
+        assert response.status_code == 302
+        assert 'account/inactive' in response.location
+        assert len(mail.outbox) == 1
+        self.user.refresh_from_db()
+        assert self.user.is_active is False
+
+
+class ConfirmEmailTest(ViewTestCase, UserTestCase, TestCase):
+    view_class = ConfirmEmail
+    url_kwargs = {'key': '123'}
+
+    def setup_models(self):
+        self.user = UserFactory.create(email='[email protected]')
+        self.email_address = EmailAddress.objects.create(
+            user=self.user,
+            email='[email protected]',
+            verified=False,
+            primary=True
+        )
+        self.confirmation = EmailConfirmation.objects.create(
+            email_address=self.email_address,
+            sent=datetime.datetime.now(),
+            key='123'
+        )
+
+    def test_activate(self):
+        response = self.request(user=self.user)
+        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
+
+        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
+
+        self.user.refresh_from_db()
+        assert self.user.email_verified is False
+        assert self.user.is_active is True
+
+        self.email_address.refresh_from_db()
+        assert self.email_address.verified is False

cadasta/accounts/urls/default.py

@@ -4,4 +4,7 @@
 
 urlpatterns = [
     url(r'^profile/$', default.AccountProfile.as_view(), name='profile'),
+    url(r'^login/$', default.AccountLogin.as_view(), name='login'),
+    url(r'^confirm-email/(?P<key>[-:\w]+)/$', default.ConfirmEmail.as_view(),
+        name='verify_email'),
 ]

cadasta/accounts/views/api.py

@@ -37,7 +37,6 @@ def perform_update(self, serializer):
             self.send_email(**self.get_send_email_kwargs(user))
 
     def put(self, *args, **kwargs):
-        print(self.request.method)
         return super().put(*args, **kwargs)
 
 

cadasta/accounts/views/default.py

@@ -1,9 +1,13 @@
 from django.core.urlresolvers import reverse_lazy
-from core.views.generic import UpdateView
 from django.utils.translation import ugettext as _
-
 from django.contrib import messages
 from django.contrib.auth.mixins import LoginRequiredMixin
+from django.utils import timezone
+
+from core.views.generic import UpdateView
+
+from allauth.account.views import ConfirmEmailView, LoginView
+from allauth.account.utils import send_email_confirmation
 
 from ..models import User
 from ..forms import ProfileForm
@@ -18,6 +22,11 @@ class AccountProfile(LoginRequiredMixin, UpdateView):
     def get_object(self, *args, **kwargs):
         return self.request.user
 
+    def get_form_kwargs(self, *args, **kwargs):
+        form_kwargs = super().get_form_kwargs(*args, **kwargs)
+        form_kwargs['request'] = self.request
+        return form_kwargs
+
     def form_valid(self, form):
         messages.add_message(self.request, messages.SUCCESS,
                              _("Successfully updated profile information"))
@@ -27,3 +36,26 @@ def form_invalid(self, form):
         messages.add_message(self.request, messages.ERROR,
                              _("Failed to update profile information"))
         return super().form_invalid(form)
+
+
+class AccountLogin(LoginView):
+    def form_valid(self, form):
+        user = form.user
+        if not user.email_verified and timezone.now() > user.verify_email_by:
+            user.is_active = False
+            user.save()
+            send_email_confirmation(self.request, user)
+
+        return super().form_valid(form)
+
+
+class ConfirmEmail(ConfirmEmailView):
+    def post(self, *args, **kwargs):
+        response = super().post(*args, **kwargs)
+
+        user = self.get_object().email_address.user
+        user.email_verified = True
+        user.is_active = True
+        user.save()
+
+        return response

cadasta/templates/allauth/account/account_inactive.html

@@ -8,7 +8,11 @@
 
 <div class="narrow">
 	<h1>{% trans "Account inactive" %}</h1>
+    {% if not user.email_verified %}
+    <p>{% blocktrans %}This account is inactive because your email address has not been confirmed.</p><p> We have sent you another email to confirm this address. Please click on the link inside this email. <a href="mailto:[email protected]">Contact us</a> if you do not receive an email within a few minutes.{% endblocktrans %}</p>
+    {% else %}
 	<p>{% blocktrans %}This account is inactive. Please <a href="mailto:[email protected]">contact us</a> if you need assistance.{% endblocktrans %}</p>
+    {% endif %}
 </div>
 
 {% endblock %}