From: Rodney Ewing Date: Wed, 22 May 2013 00:25:00 +0000 (-0700) Subject: added error handling on bad token, fixed route, and added tests X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=377db0e7ffdca6ce56041d28eba536ecd3b2a58a;p=mediagoblin.git added error handling on bad token, fixed route, and added tests --- diff --git a/mediagoblin/edit/routing.py b/mediagoblin/edit/routing.py index 67c2c7be..3592f708 100644 --- 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 78e47fe0..249fb8ba 100644 --- 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 08b4f8cf..76fd5ee9 100644 --- a/mediagoblin/tests/test_edit.py +++ b/mediagoblin/tests/test_edit.py @@ -15,14 +15,14 @@ # along with this program. If not, see . 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