added error handling on bad token, fixed route, and added tests

diff --git a/mediagoblin/edit/routing.py b/mediagoblin/edit/routing.py
index 67c2c7bee35d8d2ad3a95bba57303834604251c6..3592f70824cdc8d967558b07c111a97aa3513f02 100644 (file)
--- a/mediagoblin/edit/routing.py
+++ b/mediagoblin/edit/routing.py
@@ -26,5 +26,5 @@ add_route('mediagoblin.edit.delete_account', '/edit/account/delete/',
     'mediagoblin.edit.views:delete_account')
 add_route('mediagoblin.edit.pass', '/edit/password/',
     'mediagoblin.edit.views:change_pass')
-add_route('mediagoblin.edit.verify_email', '/edit/verify_email',
+add_route('mediagoblin.edit.verify_email', '/edit/verify_email/',
     'mediagoblin.edit.views:verify_email')

diff --git a/mediagoblin/edit/views.py b/mediagoblin/edit/views.py
index 78e47fe05735a924054667ad6a41d840d939cff4..249fb8ba5dd3c92448688726b3151f1672b8c15a 100644 (file)
--- a/mediagoblin/edit/views.py
+++ b/mediagoblin/edit/views.py
@@ -16,6 +16,7 @@
 
 from datetime import datetime
 
+from itsdangerous import BadSignature
 from werkzeug.exceptions import Forbidden
 from werkzeug.utils import secure_filename
 
@@ -417,10 +418,20 @@ def verify_email(request):
     if not 'token' in request.GET:
         return render_404(request)
 
-    # This throws an error, if the thing is faked or expired
-    # should be catched, probably.
-    token = get_timed_signer_url("mail_verification_token") \
-            .loads(request.GET['token'], max_age=10*24*3600)
+    # Catch error if token is faked or expired
+    token = None
+    try:
+        token = get_timed_signer_url("mail_verification_token") \
+                .loads(request.GET['token'], max_age=10*24*3600)
+    except BadSignature:
+        messages.add_message(
+            request,
+            messages.ERROR,
+            _('The verification key or user id is incorrect.'))
+
+        return redirect(
+            request,
+            'index')
 
     user = User.query.filter_by(id=int(token['user'])).first()
 

diff --git a/mediagoblin/tests/test_edit.py b/mediagoblin/tests/test_edit.py
index 08b4f8cf752fc0679767b8d4bc3b0a0a4c4ca3e3..76fd5ee9c2a60fcbe3e3b0ea7cfd9192d797de3a 100644 (file)
--- a/mediagoblin/tests/test_edit.py
+++ b/mediagoblin/tests/test_edit.py
@@ -15,14 +15,14 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import urlparse
-import pytest
 
 from mediagoblin import mg_globals
 from mediagoblin.db.models import User
 from mediagoblin.tests.tools import fixture_add_user
-from mediagoblin.tools import template
+from mediagoblin.tools import template, mail
 from mediagoblin.auth.lib import bcrypt_check_password
 
+
 class TestUserEdit(object):
     def setup(self):
         # set up new user
@@ -141,4 +141,106 @@ class TestUserEdit(object):
         assert form.url.errors == [
             u'This address contains errors']
 
+    def test_email_change(self, test_app):
+        self.login(test_app)
+
+        # Test email change without password
+        template.clear_test_template_context()
+        test_app.post(
+            '/edit/account/', {
+                'new_email': 'new@example.com'})
+
+        # Check form errors
+        context = template.TEMPLATE_TEST_CONTEXT[
+            'mediagoblin/edit/edit_account.html']
+        assert context['form'].password.errors == [
+            u'This field is required.']
+
+        # Test email change with wrong password
+        template.clear_test_template_context()
+        test_app.post(
+            '/edit/account/', {
+                'new_email': 'new@example.com',
+                'password': 'wrong'})
+
+        # Check form errors
+        context = template.TEMPLATE_TEST_CONTEXT[
+            'mediagoblin/edit/edit_account.html']
+        assert context['form'].password.errors == [
+            u'Wrong password.']
+
+        # Test email already in db
+        template.clear_test_template_context()
+        test_app.post(
+            '/edit/account/', {
+                'new_email': 'chris@example.com',
+                'password': 'toast'})
+
+        # Check form errors
+        context = template.TEMPLATE_TEST_CONTEXT[
+            'mediagoblin/edit/edit_account.html']
+        assert context['form'].new_email.errors == [
+            u'Sorry, a user with that email address already exists.']
+
+        # Test password is too short
+        template.clear_test_template_context()
+        test_app.post(
+            '/edit/account/', {
+                'new_email': 'new@example.com',
+                'password': 't'})
+
+        # Check form errors
+        context = template.TEMPLATE_TEST_CONTEXT[
+            'mediagoblin/edit/edit_account.html']
+        assert context['form'].password.errors == [
+            u'Field must be between 5 and 1024 characters long.']
+
+        # Test successful email change
+        template.clear_test_template_context()
+        res = test_app.post(
+            '/edit/account/', {
+                'new_email': 'new@example.com',
+                'password': 'toast'})
+        res.follow()
+
+        # Correct redirect?
+        assert urlparse.urlsplit(res.location)[2] == '/u/chris/'
+
+        # Make sure we get email verification and try verifying
+        assert len(mail.EMAIL_TEST_INBOX) == 1
+        message = mail.EMAIL_TEST_INBOX.pop()
+        assert message['To'] == 'new@example.com'
+        email_context = template.TEMPLATE_TEST_CONTEXT[
+            'mediagoblin/edit/verification.txt']
+        assert email_context['verification_url'] in \
+            message.get_payload(decode=True)
+
+        path = urlparse.urlsplit(email_context['verification_url'])[2]
+        assert path == u'/edit/verify_email/'
+
+        ## Try verifying with bs verification key, shouldn't work
+        template.clear_test_template_context()
+        res = test_app.get(
+            "/edit/verify_email/?token=total_bs")
+        res.follow()
+
+        # Correct redirect?
+        assert urlparse.urlsplit(res.location)[2] == '/'
+
+        # Email shouldn't be saved
+        email_in_db = mg_globals.database.User.find_one(
+            {'email': 'new@example.com'})
+        email = User.query.filter_by(username='chris').first().email
+        assert email_in_db is None
+        assert email == 'chris@example.com'
+
+        # Verify email activation works
+        template.clear_test_template_context()
+        get_params = urlparse.urlsplit(email_context['verification_url'])[3]
+        res = test_app.get('%s?%s' % (path, get_params))
+        res.follow()
+
+        # New email saved?
+        email = User.query.filter_by(username='chris').first().email
+        assert email == 'new@example.com'
 # test changing the url inproperly