From 89e1563f683c620387c782c363d63f90abd46a33 Mon Sep 17 00:00:00 2001 From: Rodney Ewing Date: Tue, 21 May 2013 16:21:33 -0700 Subject: [PATCH 01/29] added support for user to change email address --- mediagoblin/auth/lib.py | 25 +++-- mediagoblin/edit/forms.py | 14 ++- mediagoblin/edit/routing.py | 2 + mediagoblin/edit/views.py | 105 +++++++++++++++--- .../mediagoblin/edit/edit_account.html | 2 + .../mediagoblin/edit/verification.txt | 29 +++++ 6 files changed, 150 insertions(+), 27 deletions(-) create mode 100644 mediagoblin/templates/mediagoblin/edit/verification.txt diff --git a/mediagoblin/auth/lib.py b/mediagoblin/auth/lib.py index 8829995a..8bba8484 100644 --- a/mediagoblin/auth/lib.py +++ b/mediagoblin/auth/lib.py @@ -95,7 +95,8 @@ EMAIL_VERIFICATION_TEMPLATE = ( u"userid={userid}&token={verification_key}") -def send_verification_email(user, request): +def send_verification_email(user, request, email=None, + rendered_email=None): """ Send the verification email to users to activate their accounts. @@ -103,19 +104,23 @@ def send_verification_email(user, request): - user: a user object - request: the request """ - rendered_email = render_template( - request, 'mediagoblin/auth/verification_email.txt', - {'username': user.username, - 'verification_url': EMAIL_VERIFICATION_TEMPLATE.format( - host=request.host, - uri=request.urlgen('mediagoblin.auth.verify_email'), - userid=unicode(user.id), - verification_key=user.verification_key)}) + if not email: + email = user.email + + if not rendered_email: + rendered_email = render_template( + request, 'mediagoblin/auth/verification_email.txt', + {'username': user.username, + 'verification_url': EMAIL_VERIFICATION_TEMPLATE.format( + host=request.host, + uri=request.urlgen('mediagoblin.auth.verify_email'), + userid=unicode(user.id), + verification_key=user.verification_key)}) # TODO: There is no error handling in place send_email( mg_globals.app_config['email_sender_address'], - [user.email], + [email], # TODO # Due to the distributed nature of GNU MediaGoblin, we should # find a way to send some additional information about the diff --git a/mediagoblin/edit/forms.py b/mediagoblin/edit/forms.py index 3b2486de..3a502263 100644 --- a/mediagoblin/edit/forms.py +++ b/mediagoblin/edit/forms.py @@ -16,9 +16,11 @@ import wtforms -from mediagoblin.tools.text import tag_length_validator, TOO_LONG_TAG_WARNING +from mediagoblin.tools.text import tag_length_validator from mediagoblin.tools.translate import lazy_pass_to_ugettext as _ from mediagoblin.tools.licenses import licenses_as_choices +from mediagoblin.auth.forms import normalize_user_or_email_field + class EditForm(wtforms.Form): title = wtforms.TextField( @@ -59,6 +61,16 @@ class EditProfileForm(wtforms.Form): class EditAccountForm(wtforms.Form): + new_email = wtforms.TextField( + _('New email address'), + [wtforms.validators.Optional(), + normalize_user_or_email_field(allow_user=False)]) + password = wtforms.PasswordField( + _('Password'), + [wtforms.validators.Optional(), + wtforms.validators.Length(min=5, max=1024)], + description=_( + 'Enter your old password to prove you own this account.')) license_preference = wtforms.SelectField( _('License preference'), [ diff --git a/mediagoblin/edit/routing.py b/mediagoblin/edit/routing.py index 622729ac..67c2c7be 100644 --- a/mediagoblin/edit/routing.py +++ b/mediagoblin/edit/routing.py @@ -26,3 +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', + 'mediagoblin.edit.views:verify_email') diff --git a/mediagoblin/edit/views.py b/mediagoblin/edit/views.py index 508c380d..78e47fe0 100644 --- a/mediagoblin/edit/views.py +++ b/mediagoblin/edit/views.py @@ -23,18 +23,22 @@ from mediagoblin import messages from mediagoblin import mg_globals from mediagoblin.auth import lib as auth_lib +from mediagoblin.auth.views import email_debug_message from mediagoblin.edit import forms from mediagoblin.edit.lib import may_edit_media from mediagoblin.decorators import (require_active_login, active_user_from_url, - get_media_entry_by_id, - user_may_alter_collection, get_user_collection) -from mediagoblin.tools.response import render_to_response, \ - redirect, redirect_obj + get_media_entry_by_id, user_may_alter_collection, + get_user_collection) +from mediagoblin.tools.crypto import get_timed_signer_url +from mediagoblin.tools.response import (render_to_response, + redirect, redirect_obj, render_404) from mediagoblin.tools.translate import pass_to_ugettext as _ +from mediagoblin.tools.template import render_template from mediagoblin.tools.text import ( convert_to_tag_list_of_dicts, media_tags_as_string) from mediagoblin.tools.url import slugify from mediagoblin.db.util import check_media_slug_used, check_collection_slug_used +from mediagoblin.db.models import User import mimetypes @@ -212,6 +216,10 @@ def edit_profile(request, url_user=None): {'user': user, 'form': form}) +EMAIL_VERIFICATION_TEMPLATE = ( + u'{uri}?' + u'token={verification_key}') + @require_active_login def edit_account(request): @@ -220,27 +228,57 @@ def edit_account(request): wants_comment_notification=user.wants_comment_notification, license_preference=user.license_preference) - if request.method == 'POST': - form_validated = form.validate() - - if form_validated and \ - form.wants_comment_notification.validate(form): + if request.method == 'POST' and form.validate(): + if form.wants_comment_notification.validate(form): user.wants_comment_notification = \ form.wants_comment_notification.data - if form_validated and \ - form.license_preference.validate(form): + if form.license_preference.validate(form): user.license_preference = \ form.license_preference.data - if form_validated and not form.errors: + if form.new_email.data: + if not form.password.data: + form.password.errors.append( + _('This field is required.')) + elif not auth_lib.bcrypt_check_password( + form.password.data, user.pw_hash): + form.password.errors.append( + _('Wrong password.')) + else: + new_email = form.new_email.data + users_with_email = User.query.filter_by( + email=new_email).count() + if users_with_email: + form.new_email.errors.append( + _('Sorry, a user with that email address' + ' already exists.')) + else: + verification_key = get_timed_signer_url( + 'mail_verification_token').dumps({ + 'user': user.id, + 'email': new_email}) + + rendered_email = render_template( + request, 'mediagoblin/edit/verification.txt', + {'username': user.username, + 'verification_url': EMAIL_VERIFICATION_TEMPLATE.format( + uri=request.urlgen('mediagoblin.edit.verify_email', + qualified=True), + verification_key=verification_key)}) + + email_debug_message(request) + auth_lib.send_verification_email(user, request, new_email, + rendered_email) + + if not form.errors: user.save() messages.add_message(request, - messages.SUCCESS, - _("Account settings saved")) + messages.SUCCESS, + _("Account settings saved")) return redirect(request, - 'mediagoblin.user_pages.user_home', - user=user.username) + 'mediagoblin.user_pages.user_home', + user=user.username) return render_to_response( request, @@ -369,3 +407,38 @@ def change_pass(request): 'mediagoblin/edit/change_pass.html', {'form': form, 'user': user}) + + +def verify_email(request): + """ + Email verification view for changing email address + """ + # If no token, we can't do anything + 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) + + user = User.query.filter_by(id=int(token['user'])).first() + + if user: + user.email = token['email'] + user.save() + + messages.add_message( + request, + messages.SUCCESS, + _('Your email address has been verified.')) + + else: + messages.add_message( + request, + messages.ERROR, + _('The verification key or user id is incorrect.')) + + return redirect( + request, 'mediagoblin.user_pages.user_home', + user=user.username) diff --git a/mediagoblin/templates/mediagoblin/edit/edit_account.html b/mediagoblin/templates/mediagoblin/edit/edit_account.html index 4c4aaf95..d56b3ba0 100644 --- a/mediagoblin/templates/mediagoblin/edit/edit_account.html +++ b/mediagoblin/templates/mediagoblin/edit/edit_account.html @@ -46,6 +46,8 @@ {% trans %}Change your password.{% endtrans %}

+ {{ wtforms_util.render_field_div(form.new_email) }} + {{ wtforms_util.render_field_div(form.password) }}

{{ form.wants_comment_notification }} {{ wtforms_util.render_label(form.wants_comment_notification) }}

diff --git a/mediagoblin/templates/mediagoblin/edit/verification.txt b/mediagoblin/templates/mediagoblin/edit/verification.txt new file mode 100644 index 00000000..d53cd5e8 --- /dev/null +++ b/mediagoblin/templates/mediagoblin/edit/verification.txt @@ -0,0 +1,29 @@ +{# +# GNU MediaGoblin -- federated, autonomous media hosting +# Copyright (C) 2011, 2012 MediaGoblin contributors. See AUTHORS. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . +-#} + +{% trans username=username, verification_url=verification_url|safe -%} +Hi, + +We wanted to verify that you are {{ username }}. If this is the case, then +please follow the link below to verify your new email address. + +{{ verification_url }} + +If you are not {{ username }} or didn't request an email change, you can ignore +this email. +{%- endtrans %} From 377db0e7ffdca6ce56041d28eba536ecd3b2a58a Mon Sep 17 00:00:00 2001 From: Rodney Ewing Date: Tue, 21 May 2013 17:25:00 -0700 Subject: [PATCH 02/29] added error handling on bad token, fixed route, and added tests --- mediagoblin/edit/routing.py | 2 +- mediagoblin/edit/views.py | 19 ++++-- mediagoblin/tests/test_edit.py | 106 ++++++++++++++++++++++++++++++++- 3 files changed, 120 insertions(+), 7 deletions(-) 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 From f670f48ddd047f09566e196f86f6253412f801cd Mon Sep 17 00:00:00 2001 From: Rodney Ewing Date: Wed, 29 May 2013 13:13:50 -0700 Subject: [PATCH 03/29] form is already validated, no need to validate again --- mediagoblin/edit/views.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/mediagoblin/edit/views.py b/mediagoblin/edit/views.py index 249fb8ba..4c4e5131 100644 --- a/mediagoblin/edit/views.py +++ b/mediagoblin/edit/views.py @@ -230,13 +230,9 @@ def edit_account(request): license_preference=user.license_preference) if request.method == 'POST' and form.validate(): - if form.wants_comment_notification.validate(form): - user.wants_comment_notification = \ - form.wants_comment_notification.data + user.wants_comment_notification = form.wants_comment_notification.data - if form.license_preference.validate(form): - user.license_preference = \ - form.license_preference.data + user.license_preference = form.license_preference.data if form.new_email.data: if not form.password.data: From a90b350f718baf96c66c57fac2a5e7b0e97f7efd Mon Sep 17 00:00:00 2001 From: Rodney Ewing Date: Wed, 29 May 2013 13:19:36 -0700 Subject: [PATCH 04/29] send_verification_email was moved to auth/tools --- mediagoblin/edit/views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mediagoblin/edit/views.py b/mediagoblin/edit/views.py index 4c4e5131..df7db21b 100644 --- a/mediagoblin/edit/views.py +++ b/mediagoblin/edit/views.py @@ -24,6 +24,7 @@ from mediagoblin import messages from mediagoblin import mg_globals from mediagoblin.auth import lib as auth_lib +from mediagoblin.auth import tools as auth_tools from mediagoblin.auth.views import email_debug_message from mediagoblin.edit import forms from mediagoblin.edit.lib import may_edit_media @@ -265,7 +266,7 @@ def edit_account(request): verification_key=verification_key)}) email_debug_message(request) - auth_lib.send_verification_email(user, request, new_email, + auth_tools.send_verification_email(user, request, new_email, rendered_email) if not form.errors: From 342f06f7bd40ee47a48562b42006225e93f0c386 Mon Sep 17 00:00:00 2001 From: Rodney Ewing Date: Wed, 22 May 2013 14:51:30 -0700 Subject: [PATCH 05/29] modified verification emails to use itsdangerous tokens --- mediagoblin/auth/forms.py | 3 -- mediagoblin/auth/lib.py | 15 +++--- mediagoblin/auth/tools.py | 13 ++--- mediagoblin/auth/views.py | 95 ++++++++++++++++++++++------------ mediagoblin/db/migrations.py | 20 +++++++ mediagoblin/db/models.py | 3 -- mediagoblin/tests/test_auth.py | 44 ++++------------ 7 files changed, 107 insertions(+), 86 deletions(-) diff --git a/mediagoblin/auth/forms.py b/mediagoblin/auth/forms.py index 0a391d67..ec395d60 100644 --- a/mediagoblin/auth/forms.py +++ b/mediagoblin/auth/forms.py @@ -58,9 +58,6 @@ class ChangePassForm(wtforms.Form): 'Password', [wtforms.validators.Required(), wtforms.validators.Length(min=5, max=1024)]) - userid = wtforms.HiddenField( - '', - [wtforms.validators.Required()]) token = wtforms.HiddenField( '', [wtforms.validators.Required()]) diff --git a/mediagoblin/auth/lib.py b/mediagoblin/auth/lib.py index bfc36b28..0810bd1b 100644 --- a/mediagoblin/auth/lib.py +++ b/mediagoblin/auth/lib.py @@ -20,6 +20,7 @@ import bcrypt from mediagoblin.tools.mail import send_email from mediagoblin.tools.template import render_template +from mediagoblin.tools.crypto import get_timed_signer_url from mediagoblin import mg_globals @@ -91,8 +92,8 @@ def fake_login_attempt(): EMAIL_FP_VERIFICATION_TEMPLATE = ( - u"http://{host}{uri}?" - u"userid={userid}&token={fp_verification_key}") + u"{uri}?" + u"token={fp_verification_key}") def send_fp_verification_email(user, request): @@ -103,14 +104,16 @@ def send_fp_verification_email(user, request): - user: a user object - request: the request """ + fp_verification_key = get_timed_signer_url('mail_verification_token') \ + .dumps(user.id) + rendered_email = render_template( request, 'mediagoblin/auth/fp_verification_email.txt', {'username': user.username, 'verification_url': EMAIL_FP_VERIFICATION_TEMPLATE.format( - host=request.host, - uri=request.urlgen('mediagoblin.auth.verify_forgot_password'), - userid=unicode(user.id), - fp_verification_key=user.fp_verification_key)}) + uri=request.urlgen('mediagoblin.auth.verify_forgot_password', + qualified=True), + fp_verification_key=fp_verification_key)}) # TODO: There is no error handling in place send_email( diff --git a/mediagoblin/auth/tools.py b/mediagoblin/auth/tools.py index d86235b1..d17ee4e6 100644 --- a/mediagoblin/auth/tools.py +++ b/mediagoblin/auth/tools.py @@ -62,8 +62,8 @@ def normalize_user_or_email_field(allow_email=True, allow_user=True): EMAIL_VERIFICATION_TEMPLATE = ( - u"http://{host}{uri}?" - u"userid={userid}&token={verification_key}") + u"{uri}?" + u"token={verification_key}") def send_verification_email(user, request, email=None, @@ -79,14 +79,15 @@ def send_verification_email(user, request, email=None, email = user.email if not rendered_email: + verification_key = get_timed_signer_url('mail_verification_token') \ + .dumps(user.id) rendered_email = render_template( request, 'mediagoblin/auth/verification_email.txt', {'username': user.username, 'verification_url': EMAIL_VERIFICATION_TEMPLATE.format( - host=request.host, - uri=request.urlgen('mediagoblin.auth.verify_email'), - userid=unicode(user.id), - verification_key=user.verification_key)}) + uri=request.urlgen('mediagoblin.auth.verify_email', + qualified=True), + verification_key=verification_key)}) # TODO: There is no error handling in place send_email( diff --git a/mediagoblin/auth/views.py b/mediagoblin/auth/views.py index bb7bda77..45cb3a54 100644 --- a/mediagoblin/auth/views.py +++ b/mediagoblin/auth/views.py @@ -15,10 +15,11 @@ # along with this program. If not, see . import uuid -import datetime +from itsdangerous import BadSignature from mediagoblin import messages, mg_globals from mediagoblin.db.models import User +from mediagoblin.tools.crypto import get_timed_signer_url from mediagoblin.tools.response import render_to_response, redirect, render_404 from mediagoblin.tools.translate import pass_to_ugettext as _ from mediagoblin.tools.mail import email_debug_message @@ -115,16 +116,28 @@ def verify_email(request): you are lucky :) """ # If we don't have userid and token parameters, we can't do anything; 404 - if not 'userid' in request.GET or not 'token' in request.GET: + if not 'token' in request.GET: return render_404(request) - user = User.query.filter_by(id=request.args['userid']).first() + # Catch error if token is faked or expired + 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.')) - if user and user.verification_key == unicode(request.GET['token']): + return redirect( + request, + 'index') + + user = User.query.filter_by(id=int(token)).first() + + if user and user.email_verified is False: user.status = u'active' user.email_verified = True - user.verification_key = None - user.save() messages.add_message( @@ -166,9 +179,6 @@ def resend_activation(request): return redirect(request, "mediagoblin.user_pages.user_home", user=request.user['username']) - request.user.verification_key = unicode(uuid.uuid4()) - request.user.save() - email_debug_message(request) send_verification_email(request.user, request) @@ -235,11 +245,6 @@ def forgot_password(request): # SUCCESS. Send reminder and return to login page if user: - user.fp_verification_key = unicode(uuid.uuid4()) - user.fp_token_expire = datetime.datetime.now() + \ - datetime.timedelta(days=10) - user.save() - email_debug_message(request) send_fp_verification_email(user, request) @@ -254,31 +259,44 @@ def verify_forgot_password(request): """ # get form data variables, and specifically check for presence of token formdata = _process_for_token(request) - if not formdata['has_userid_and_token']: + if not formdata['has_token']: return render_404(request) - formdata_token = formdata['vars']['token'] - formdata_userid = formdata['vars']['userid'] formdata_vars = formdata['vars'] - # check if it's a valid user id - user = User.query.filter_by(id=formdata_userid).first() - if not user: - return render_404(request) + # Catch error if token is faked or expired + try: + token = get_timed_signer_url("mail_verification_token") \ + .loads(formdata_vars['token'], max_age=10*24*3600) + except BadSignature: + messages.add_message( + request, + messages.ERROR, + _('The verification key or user id is incorrect.')) - # check if we have a real user and correct token - if ((user and user.fp_verification_key and - user.fp_verification_key == unicode(formdata_token) and - datetime.datetime.now() < user.fp_token_expire - and user.email_verified and user.status == 'active')): + return redirect( + request, + 'index') + + # check if it's a valid user id + user = User.query.filter_by(id=int(token)).first() + + # no user in db + if not user: + messages.add_message( + request, messages.ERROR, + _('The user id is incorrect.')) + return redirect( + request, 'index') + + # check if user active and has email verified + if user.email_verified and user.status == 'active': cp_form = auth_forms.ChangePassForm(formdata_vars) if request.method == 'POST' and cp_form.validate(): user.pw_hash = auth_lib.bcrypt_gen_password_hash( cp_form.password.data) - user.fp_verification_key = None - user.fp_token_expire = None user.save() messages.add_message( @@ -292,10 +310,20 @@ def verify_forgot_password(request): 'mediagoblin/auth/change_fp.html', {'cp_form': cp_form}) - # in case there is a valid id but no user with that id in the db - # or the token expired - else: - return render_404(request) + if not user.email_verified: + messages.add_message( + request, messages.ERROR, + _('You need to verify your email before you can reset your' + ' password.')) + + if not user.status == 'active': + messages.add_message( + request, messages.ERROR, + _('You are no longer an active user. Please contact the system' + ' admin to reactivate your accoutn.')) + + return redirect( + request, 'index') def _process_for_token(request): @@ -313,7 +341,6 @@ def _process_for_token(request): formdata = { 'vars': formdata_vars, - 'has_userid_and_token': - 'userid' in formdata_vars and 'token' in formdata_vars} + 'has_token': 'token' in formdata_vars} return formdata diff --git a/mediagoblin/db/migrations.py b/mediagoblin/db/migrations.py index 2c553396..6c9bf5cc 100644 --- a/mediagoblin/db/migrations.py +++ b/mediagoblin/db/migrations.py @@ -287,3 +287,23 @@ def unique_collections_slug(db): constraint.create() db.commit() + + +@RegisterMigration(11, MIGRATIONS) +def drop_token_related_User_columns(db): + """ + Drop unneeded columns from the User table after switching to using + itsdangerous tokens for email and forgot password verification. + """ + metadata = MetaData(bind=db.bind) + user_table = inspect_table(metadata, 'core__users') + + verification_key = user_table.columns['verification_key'] + fp_verification_key = user_table.columns['fp_verification_key'] + fp_token_expire = user_table.columns['fp_token_expire'] + + verification_key.drop() + fp_verification_key.drop() + fp_token_expire.drop() + + db.commit() diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index 2b925983..f5470cb9 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -68,12 +68,9 @@ class User(Base, UserMixin): # set to nullable=True implicitly. wants_comment_notification = Column(Boolean, default=True) license_preference = Column(Unicode) - verification_key = Column(Unicode) is_admin = Column(Boolean, default=False, nullable=False) url = Column(Unicode) bio = Column(UnicodeText) # ?? - fp_verification_key = Column(Unicode) - fp_token_expire = Column(DateTime) ## TODO # plugin data would be in a separate model diff --git a/mediagoblin/tests/test_auth.py b/mediagoblin/tests/test_auth.py index 755727f9..48b148dd 100644 --- a/mediagoblin/tests/test_auth.py +++ b/mediagoblin/tests/test_auth.py @@ -156,20 +156,15 @@ def test_register_views(test_app): assert path == u'/auth/verify_email/' parsed_get_params = urlparse.parse_qs(get_params) - ### user should have these same parameters - assert parsed_get_params['userid'] == [ - unicode(new_user.id)] - assert parsed_get_params['token'] == [ - new_user.verification_key] - ## Try verifying with bs verification key, shouldn't work template.clear_test_template_context() response = test_app.get( - "/auth/verify_email/?userid=%s&token=total_bs" % unicode( - new_user.id)) + "/auth/verify_email/?token=total_bs") response.follow() - context = template.TEMPLATE_TEST_CONTEXT[ - 'mediagoblin/user_pages/user.html'] + + # Correct redirect? + assert urlparse.urlsplit(response.location)[2] == '/' + # assert context['verification_successful'] == True # TODO: Would be good to test messages here when we can do so... new_user = mg_globals.database.User.find_one( @@ -233,35 +228,17 @@ def test_register_views(test_app): path = urlparse.urlsplit(email_context['verification_url'])[2] get_params = urlparse.urlsplit(email_context['verification_url'])[3] - assert path == u'/auth/forgot_password/verify/' parsed_get_params = urlparse.parse_qs(get_params) - - # user should have matching parameters - new_user = mg_globals.database.User.find_one({'username': u'happygirl'}) - assert parsed_get_params['userid'] == [unicode(new_user.id)] - assert parsed_get_params['token'] == [new_user.fp_verification_key] - - ### The forgotten password token should be set to expire in ~ 10 days - # A few ticks have expired so there are only 9 full days left... - assert (new_user.fp_token_expire - datetime.datetime.now()).days == 9 + assert path == u'/auth/forgot_password/verify/' ## Try using a bs password-changing verification key, shouldn't work template.clear_test_template_context() response = test_app.get( - "/auth/forgot_password/verify/?userid=%s&token=total_bs" % unicode( - new_user.id), status=404) - assert response.status.split()[0] == u'404' # status="404 NOT FOUND" + "/auth/forgot_password/verify/?token=total_bs") + response.follow() - ## Try using an expired token to change password, shouldn't work - template.clear_test_template_context() - new_user = mg_globals.database.User.find_one({'username': u'happygirl'}) - real_token_expiration = new_user.fp_token_expire - new_user.fp_token_expire = datetime.datetime.now() - new_user.save() - response = test_app.get("%s?%s" % (path, get_params), status=404) - assert response.status.split()[0] == u'404' # status="404 NOT FOUND" - new_user.fp_token_expire = real_token_expiration - new_user.save() + # Correct redirect? + assert urlparse.urlsplit(response.location)[2] == '/' ## Verify step 1 of password-change works -- can see form to change password template.clear_test_template_context() @@ -272,7 +249,6 @@ def test_register_views(test_app): template.clear_test_template_context() response = test_app.post( '/auth/forgot_password/verify/', { - 'userid': parsed_get_params['userid'], 'password': 'iamveryveryhappy', 'token': parsed_get_params['token']}) response.follow() From 69b888c22c326b1e69ee8e050a415561b6ca6aac Mon Sep 17 00:00:00 2001 From: Rodney Ewing Date: Tue, 28 May 2013 10:43:57 -0700 Subject: [PATCH 06/29] cleanup after merge --- mediagoblin/auth/tools.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mediagoblin/auth/tools.py b/mediagoblin/auth/tools.py index d17ee4e6..c45944d3 100644 --- a/mediagoblin/auth/tools.py +++ b/mediagoblin/auth/tools.py @@ -22,6 +22,7 @@ from sqlalchemy import or_ from mediagoblin import mg_globals from mediagoblin.auth import lib as auth_lib +from mediagoblin.tools.crypto import get_timed_signer_url from mediagoblin.db.models import User from mediagoblin.tools.mail import (normalize_email, send_email, email_debug_message) From 3c48bb39b7e3b76e4168e0d13ccd80f4f33e745e Mon Sep 17 00:00:00 2001 From: Joar Wandborg Date: Fri, 7 Jun 2013 00:28:17 +0200 Subject: [PATCH 07/29] CloudFiles: Default to SSL URIs --- mediagoblin/storage/cloudfiles.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mediagoblin/storage/cloudfiles.py b/mediagoblin/storage/cloudfiles.py index b6e57c91..250f06d4 100644 --- a/mediagoblin/storage/cloudfiles.py +++ b/mediagoblin/storage/cloudfiles.py @@ -75,7 +75,7 @@ class CloudFilesStorage(StorageInterface): _log.debug('Container: {0}'.format( self.container.name)) - self.container_uri = self.container.public_uri() + self.container_uri = self.container.public_ssl_uri() def _resolve_filepath(self, filepath): return '/'.join( From c139d49d838a508f7741746704cfe2e0438ced82 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Thu, 6 Jun 2013 17:56:00 -0500 Subject: [PATCH 08/29] ./bin/gmg dbupdate, not ./bin/dbupdate Thanks to Tsyesica for catching this :) --- mediagoblin.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mediagoblin.ini b/mediagoblin.ini index 4906546a..cc45c08d 100644 --- a/mediagoblin.ini +++ b/mediagoblin.ini @@ -20,7 +20,7 @@ email_debug_mode = true allow_registration = true ## Uncomment this to turn on video or enable other media types -## You may have to install dependencies, and will have to run ./bin/dbupdate +## You may have to install dependencies, and will have to run ./bin/gmg dbupdate ## See http://docs.mediagoblin.org/siteadmin/media-types.html for details. # media_types = mediagoblin.media_types.image, mediagoblin.media_types.video From 25aad338d4921ec76484c6d2af5e40c97904917d Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Fri, 7 Jun 2013 11:45:07 -0500 Subject: [PATCH 09/29] Added some test-writing docs for plugins, but not sure if they're good. ;) This commit sponsored by Joe Lee. Thank you! --- docs/source/index.rst | 1 + docs/source/pluginwriter/tests.rst | 64 ++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 docs/source/pluginwriter/tests.rst diff --git a/docs/source/index.rst b/docs/source/index.rst index 7f692d57..d71f39f8 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -73,6 +73,7 @@ This guide covers writing new GNU MediaGoblin plugins. pluginwriter/quickstart pluginwriter/database pluginwriter/api + pluginwriter/tests Part 4: Developer's Zone diff --git a/docs/source/pluginwriter/tests.rst b/docs/source/pluginwriter/tests.rst new file mode 100644 index 00000000..fe99688f --- /dev/null +++ b/docs/source/pluginwriter/tests.rst @@ -0,0 +1,64 @@ +.. MediaGoblin Documentation + + Written in 2013 by MediaGoblin contributors + + To the extent possible under law, the author(s) have dedicated all + copyright and related and neighboring rights to this software to + the public domain worldwide. This software is distributed without + any warranty. + + You should have received a copy of the CC0 Public Domain + Dedication along with this software. If not, see + . + +============================== +Writing unit tests for plugins +============================== + +Here's a brief guide to writing unit tests for plugins. However, it +isn't really ideal. It also hasn't been well tested... yes, there's +some irony there :) + +Some notes: we're using py.test and webtest for unit testing stuff. +Keep that in mind. + +My suggestion is to mime the behavior of `mediagoblin/tests/` and put +that in your own plugin, like `myplugin/tests/`. Copy over +`conftest.py` and `pytest.ini` to your tests directory, but possibly +change the `test_app` fixture to match your own tests' config needs. +For example:: + + import pkg_resources + # [...] + + @pytest.fixture() + def test_app(request): + return get_app( + request, + mgoblin_config=pkg_resources.resource_filename( + 'myplugin.tests', 'myplugin_mediagoblin.ini')) + +In any test module in your tests directory you can then do:: + + def test_somethingorother(test_app): + # real code goes here + pass + +And you'll get a mediagoblin application wrapped in webtest passed in +to your environment. + +If your plugin needs to define multiple configuration setups, you can +actually set up multiple fixtures very easily for this. You can just +set up multiple fixtures with different names that point to different +configs and pass them in as that named argument. + +To run the tests, from mediagoblin's directory (make sure that your +plugin has been added to your mediagoblin checkout's virtualenv!) do:: + + ./runtests.sh /path/to/myplugin/tests/ + +replacing `/path/to/myplugin/` with the actual path to your plugin. + +NOTE: again, the above is untested, but it should probably work. If +you run into trouble, `contact us +`_, preferably on IRC! From 2d7b6bdef9f4aead59576b7bcbb2f42ba9c92ad7 Mon Sep 17 00:00:00 2001 From: Joar Wandborg Date: Sun, 7 Apr 2013 23:17:23 +0200 Subject: [PATCH 10/29] New notifications - Added request.notifications - Email configuration fixes - Set config_spec default SMTP port to `0` and switch to SSL/non-SSL default if `port == 0` - Added email_smtp_use_ssl configuration setting - Added migrations for notification tables - Added __repr__ to MediaComment(Mixin) - Added MediaComment.get_entry => MediaEntry - Added CommentSubscription, CommentNotification, Notification, ProcessingNotification tables - Added notifications.task to celery init - Fixed a bug in the video transcoder where pygst would hijack the --help argument. - Added notifications - views - silence - subscribe - routes - utility methods - celery task - Added half-hearted .active comment CSS style - Added quick JS to show header_dropdown - Added fragment template to show notifications in header_dropdown - Added fragment template to show subscribe/unsubscribe buttons on media/comment pages - Updated celery setup tests with notifications.task - Tried to fix test_misc tests that I broke - Added notification tests - Added and extended tests.tools fixtures - Integrated new notifications into media_home, media_post_comment views - Bumped SQLAlchemy dependency to >= 0.8.0 since we need polymorphic for the notifications to work --- mediagoblin/app.py | 3 + mediagoblin/config_spec.ini | 3 +- mediagoblin/db/migrations.py | 57 ++++++- mediagoblin/db/mixin.py | 9 ++ mediagoblin/db/models.py | 110 ++++++++++++- mediagoblin/init/celery/__init__.py | 18 ++- mediagoblin/media_types/stl/processing.py | 2 +- mediagoblin/media_types/video/transcoders.py | 6 + mediagoblin/notifications/__init__.py | 141 ++++++++++++++++ mediagoblin/notifications/routing.py | 25 +++ mediagoblin/notifications/task.py | 46 ++++++ mediagoblin/notifications/tools.py | 55 +++++++ mediagoblin/notifications/views.py | 54 +++++++ mediagoblin/routing.py | 1 + mediagoblin/static/css/base.css | 6 + mediagoblin/static/js/notifications.js | 18 +++ mediagoblin/submit/views.py | 4 + mediagoblin/templates/mediagoblin/base.html | 6 + .../fragments/header_notifications.html | 40 +++++ .../mediagoblin/user_pages/media.html | 2 + .../utils/comment-subscription.html | 36 +++++ mediagoblin/tests/test_celery_setup.py | 2 +- mediagoblin/tests/test_misc.py | 8 +- mediagoblin/tests/test_notifications.py | 151 ++++++++++++++++++ mediagoblin/tests/tools.py | 87 +++++++++- mediagoblin/tools/mail.py | 7 +- mediagoblin/user_pages/views.py | 21 ++- setup.py | 2 +- 28 files changed, 891 insertions(+), 29 deletions(-) create mode 100644 mediagoblin/notifications/__init__.py create mode 100644 mediagoblin/notifications/routing.py create mode 100644 mediagoblin/notifications/task.py create mode 100644 mediagoblin/notifications/tools.py create mode 100644 mediagoblin/notifications/views.py create mode 100644 mediagoblin/static/js/notifications.js create mode 100644 mediagoblin/templates/mediagoblin/fragments/header_notifications.html create mode 100644 mediagoblin/templates/mediagoblin/utils/comment-subscription.html create mode 100644 mediagoblin/tests/test_notifications.py diff --git a/mediagoblin/app.py b/mediagoblin/app.py index 1984ce77..58058360 100644 --- a/mediagoblin/app.py +++ b/mediagoblin/app.py @@ -37,6 +37,7 @@ from mediagoblin.init import (get_jinja_loader, get_staticdirector, setup_storage) from mediagoblin.tools.pluginapi import PluginManager, hook_transform from mediagoblin.tools.crypto import setup_crypto +from mediagoblin import notifications _log = logging.getLogger(__name__) @@ -186,6 +187,8 @@ class MediaGoblinApp(object): request.urlgen = build_proxy + request.notifications = notifications + mg_request.setup_user_in_request(request) request.controller_name = None diff --git a/mediagoblin/config_spec.ini b/mediagoblin/config_spec.ini index b213970d..4547ea54 100644 --- a/mediagoblin/config_spec.ini +++ b/mediagoblin/config_spec.ini @@ -22,9 +22,10 @@ direct_remote_path = string(default="/mgoblin_static/") # set to false to enable sending notices email_debug_mode = boolean(default=True) +email_smtp_use_ssl = boolean(default=False) email_sender_address = string(default="notice@mediagoblin.example.org") email_smtp_host = string(default='') -email_smtp_port = integer(default=25) +email_smtp_port = integer(default=0) email_smtp_user = string(default=None) email_smtp_pass = string(default=None) diff --git a/mediagoblin/db/migrations.py b/mediagoblin/db/migrations.py index 2c553396..29b2522a 100644 --- a/mediagoblin/db/migrations.py +++ b/mediagoblin/db/migrations.py @@ -26,7 +26,7 @@ from sqlalchemy.sql import and_ from migrate.changeset.constraint import UniqueConstraint from mediagoblin.db.migration_tools import RegisterMigration, inspect_table -from mediagoblin.db.models import MediaEntry, Collection, User +from mediagoblin.db.models import MediaEntry, Collection, User, MediaComment MIGRATIONS = {} @@ -287,3 +287,58 @@ def unique_collections_slug(db): constraint.create() db.commit() + +class CommentSubscription_v0(declarative_base()): + __tablename__ = 'core__comment_subscriptions' + id = Column(Integer, primary_key=True) + + created = Column(DateTime, nullable=False, default=datetime.datetime.now) + + media_entry_id = Column(Integer, ForeignKey(MediaEntry.id), nullable=False) + + user_id = Column(Integer, ForeignKey(User.id), nullable=False) + + notify = Column(Boolean, nullable=False, default=True) + send_email = Column(Boolean, nullable=False, default=True) + + +class Notification_v0(declarative_base()): + __tablename__ = 'core__notifications' + id = Column(Integer, primary_key=True) + type = Column(Unicode) + + created = Column(DateTime, nullable=False, default=datetime.datetime.now) + + user_id = Column(Integer, ForeignKey(User.id), nullable=False, + index=True) + seen = Column(Boolean, default=lambda: False, index=True) + + +class CommentNotification_v0(Notification_v0): + __tablename__ = 'core__comment_notifications' + id = Column(Integer, ForeignKey(Notification_v0.id), primary_key=True) + + subject_id = Column(Integer, ForeignKey(MediaComment.id)) + + +class ProcessingNotification_v0(Notification_v0): + __tablename__ = 'core__processing_notifications' + + id = Column(Integer, ForeignKey(Notification_v0.id), primary_key=True) + + subject_id = Column(Integer, ForeignKey(MediaEntry.id)) + + +@RegisterMigration(11, MIGRATIONS) +def add_new_notification_tables(db): + metadata = MetaData(bind=db.bind) + + user_table = inspect_table(metadata, 'core__users') + mediaentry_table = inspect_table(metadata, 'core__media_entries') + mediacomment_table = inspect_table(metadata, 'core__media_comments') + + CommentSubscription_v0.__table__.create(db.bind) + + Notification_v0.__table__.create(db.bind) + CommentNotification_v0.__table__.create(db.bind) + ProcessingNotification_v0.__table__.create(db.bind) diff --git a/mediagoblin/db/mixin.py b/mediagoblin/db/mixin.py index 9f566e36..1b32d838 100644 --- a/mediagoblin/db/mixin.py +++ b/mediagoblin/db/mixin.py @@ -31,6 +31,8 @@ import uuid import re import datetime +from datetime import datetime + from werkzeug.utils import cached_property from mediagoblin import mg_globals @@ -288,6 +290,13 @@ class MediaCommentMixin(object): """ return cleaned_markdown_conversion(self.content) + def __repr__(self): + return '<{klass} #{id} {author} "{comment}">'.format( + klass=self.__class__.__name__, + id=self.id, + author=self.get_author, + comment=self.content) + class CollectionMixin(GenerateSlugMixin): def check_slug_used(self, slug): diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index 2b925983..62090126 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -24,15 +24,17 @@ import datetime from sqlalchemy import Column, Integer, Unicode, UnicodeText, DateTime, \ Boolean, ForeignKey, UniqueConstraint, PrimaryKeyConstraint, \ SmallInteger -from sqlalchemy.orm import relationship, backref +from sqlalchemy.orm import relationship, backref, with_polymorphic from sqlalchemy.orm.collections import attribute_mapped_collection from sqlalchemy.sql.expression import desc from sqlalchemy.ext.associationproxy import association_proxy from sqlalchemy.util import memoized_property + from mediagoblin.db.extratypes import PathTupleWithSlashes, JSONEncoded from mediagoblin.db.base import Base, DictReadAttrProxy -from mediagoblin.db.mixin import UserMixin, MediaEntryMixin, MediaCommentMixin, CollectionMixin, CollectionItemMixin +from mediagoblin.db.mixin import UserMixin, MediaEntryMixin, \ + MediaCommentMixin, CollectionMixin, CollectionItemMixin from mediagoblin.tools.files import delete_media_files from mediagoblin.tools.common import import_component @@ -60,9 +62,9 @@ class User(Base, UserMixin): # the RFC) and because it would be a mess to implement at this # point. email = Column(Unicode, nullable=False) - created = Column(DateTime, nullable=False, default=datetime.datetime.now) pw_hash = Column(Unicode, nullable=False) email_verified = Column(Boolean, default=False) + created = Column(DateTime, nullable=False, default=datetime.datetime.now) status = Column(Unicode, default=u"needs_email_verification", nullable=False) # Intented to be nullable=False, but migrations would not work for it # set to nullable=True implicitly. @@ -392,6 +394,10 @@ class MediaComment(Base, MediaCommentMixin): backref=backref("posted_comments", lazy="dynamic", cascade="all, delete-orphan")) + get_entry = relationship(MediaEntry, + backref=backref("comments", + lazy="dynamic", + cascade="all, delete-orphan")) # Cascade: Comments are somewhat owned by their MediaEntry. # So do the full thing. @@ -484,9 +490,103 @@ class ProcessingMetaData(Base): return DictReadAttrProxy(self) +class CommentSubscription(Base): + __tablename__ = 'core__comment_subscriptions' + id = Column(Integer, primary_key=True) + + created = Column(DateTime, nullable=False, default=datetime.datetime.now) + + media_entry_id = Column(Integer, ForeignKey(MediaEntry.id), nullable=False) + media_entry = relationship(MediaEntry, + backref=backref('comment_subscriptions', + cascade='all, delete-orphan')) + + user_id = Column(Integer, ForeignKey(User.id), nullable=False) + user = relationship(User, + backref=backref('comment_subscriptions', + cascade='all, delete-orphan')) + + notify = Column(Boolean, nullable=False, default=True) + send_email = Column(Boolean, nullable=False, default=True) + + def __repr__(self): + return ('<{classname} #{id}: {user} {media} notify: ' + '{notify} email: {email}>').format( + id=self.id, + classname=self.__class__.__name__, + user=self.user, + media=self.media_entry, + notify=self.notify, + email=self.send_email) + + +class Notification(Base): + __tablename__ = 'core__notifications' + id = Column(Integer, primary_key=True) + type = Column(Unicode) + + created = Column(DateTime, nullable=False, default=datetime.datetime.now) + + user_id = Column(Integer, ForeignKey('core__users.id'), nullable=False, + index=True) + seen = Column(Boolean, default=lambda: False, index=True) + user = relationship( + User, + backref=backref('notifications', cascade='all, delete-orphan')) + + __mapper_args__ = { + 'polymorphic_identity': 'notification', + 'polymorphic_on': type + } + + def __repr__(self): + return '<{klass} #{id}: {user}: {subject} ({seen})>'.format( + id=self.id, + klass=self.__class__.__name__, + user=self.user, + subject=getattr(self, 'subject', None), + seen='unseen' if not self.seen else 'seen') + + +class CommentNotification(Notification): + __tablename__ = 'core__comment_notifications' + id = Column(Integer, ForeignKey(Notification.id), primary_key=True) + + subject_id = Column(Integer, ForeignKey(MediaComment.id)) + subject = relationship( + MediaComment, + backref=backref('comment_notifications', cascade='all, delete-orphan')) + + __mapper_args__ = { + 'polymorphic_identity': 'comment_notification' + } + + +class ProcessingNotification(Notification): + __tablename__ = 'core__processing_notifications' + + id = Column(Integer, ForeignKey(Notification.id), primary_key=True) + + subject_id = Column(Integer, ForeignKey(MediaEntry.id)) + subject = relationship( + MediaEntry, + backref=backref('processing_notifications', + cascade='all, delete-orphan')) + + __mapper_args__ = { + 'polymorphic_identity': 'processing_notification' + } + + +with_polymorphic( + Notification, + [ProcessingNotification, CommentNotification]) + MODELS = [ - User, MediaEntry, Tag, MediaTag, MediaComment, Collection, CollectionItem, MediaFile, FileKeynames, - MediaAttachmentFile, ProcessingMetaData] + User, MediaEntry, Tag, MediaTag, MediaComment, Collection, CollectionItem, + MediaFile, FileKeynames, MediaAttachmentFile, ProcessingMetaData, + Notification, CommentNotification, ProcessingNotification, + CommentSubscription] ###################################################### diff --git a/mediagoblin/init/celery/__init__.py b/mediagoblin/init/celery/__init__.py index 169cc935..57242bf6 100644 --- a/mediagoblin/init/celery/__init__.py +++ b/mediagoblin/init/celery/__init__.py @@ -16,12 +16,18 @@ import os import sys +import logging from celery import Celery from mediagoblin.tools.pluginapi import hook_runall -MANDATORY_CELERY_IMPORTS = ['mediagoblin.processing.task'] +_log = logging.getLogger(__name__) + + +MANDATORY_CELERY_IMPORTS = [ + 'mediagoblin.processing.task', + 'mediagoblin.notifications.task'] DEFAULT_SETTINGS_MODULE = 'mediagoblin.init.celery.dummy_settings_module' @@ -97,3 +103,13 @@ def setup_celery_from_config(app_config, global_config, if set_environ: os.environ['CELERY_CONFIG_MODULE'] = settings_module + + # Replace the default celery.current_app.conf if celery has already been + # initiated + from celery import current_app + + _log.info('Setting celery configuration from object "{0}"'.format( + settings_module)) + current_app.config_from_object(this_module) + + _log.debug('Celery broker host: {0}'.format(current_app.conf['BROKER_HOST'])) diff --git a/mediagoblin/media_types/stl/processing.py b/mediagoblin/media_types/stl/processing.py index 49382495..ce7a5d37 100644 --- a/mediagoblin/media_types/stl/processing.py +++ b/mediagoblin/media_types/stl/processing.py @@ -46,7 +46,7 @@ def sniff_handler(media_file, **kw): if kw.get('media') is not None: name, ext = os.path.splitext(kw['media'].filename) clean_ext = ext[1:].lower() - + if clean_ext in SUPPORTED_FILETYPES: _log.info('Found file extension in supported filetypes') return True diff --git a/mediagoblin/media_types/video/transcoders.py b/mediagoblin/media_types/video/transcoders.py index 90a767dd..9d6b7655 100644 --- a/mediagoblin/media_types/video/transcoders.py +++ b/mediagoblin/media_types/video/transcoders.py @@ -22,9 +22,15 @@ import logging import urllib import multiprocessing import gobject + +old_argv = sys.argv +sys.argv = [] + import pygst pygst.require('0.10') import gst + +sys.argv = old_argv import struct try: from PIL import Image diff --git a/mediagoblin/notifications/__init__.py b/mediagoblin/notifications/__init__.py new file mode 100644 index 00000000..4b7fbb8c --- /dev/null +++ b/mediagoblin/notifications/__init__.py @@ -0,0 +1,141 @@ +# GNU MediaGoblin -- federated, autonomous media hosting +# Copyright (C) 2011, 2012 MediaGoblin contributors. See AUTHORS. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +import logging + +from mediagoblin.db.models import Notification, \ + CommentNotification, CommentSubscription +from mediagoblin.notifications.task import email_notification_task +from mediagoblin.notifications.tools import generate_comment_message + +_log = logging.getLogger(__name__) + +def trigger_notification(comment, media_entry, request): + ''' + Send out notifications about a new comment. + ''' + subscriptions = CommentSubscription.query.filter_by( + media_entry_id=media_entry.id).all() + + for subscription in subscriptions: + if not subscription.notify: + continue + + if comment.get_author == subscription.user: + continue + + cn = CommentNotification( + user_id=subscription.user_id, + subject_id=comment.id) + + cn.save() + + if subscription.send_email: + message = generate_comment_message( + subscription.user, + comment, + media_entry, + request) + + email_notification_task.apply_async([cn.id, message]) + + +def mark_notification_seen(notification): + if notification: + notification.seen = True + notification.save() + + +def mark_comment_notification_seen(comment_id, user): + notification = CommentNotification.query.filter_by( + user_id=user.id, + subject_id=comment_id).first() + + _log.debug('Marking {0} as seen.'.format(notification)) + + mark_notification_seen(notification) + + +def get_comment_subscription(user_id, media_entry_id): + return CommentSubscription.query.filter_by( + user_id=user_id, + media_entry_id=media_entry_id).first() + +def add_comment_subscription(user, media_entry): + ''' + Create a comment subscription for a User on a MediaEntry. + + Uses the User's wants_comment_notification to set email notifications for + the subscription to enabled/disabled. + ''' + cn = get_comment_subscription(user.id, media_entry.id) + + if not cn: + cn = CommentSubscription( + user_id=user.id, + media_entry_id=media_entry.id) + + cn.notify = True + + if not user.wants_comment_notification: + cn.send_email = False + + cn.save() + + +def silence_comment_subscription(user, media_entry): + ''' + Silence a subscription so that the user is never notified in any way about + new comments on an entry + ''' + cn = get_comment_subscription(user.id, media_entry.id) + + if cn: + cn.notify = False + cn.send_email = False + cn.save() + + +def remove_comment_subscription(user, media_entry): + cn = get_comment_subscription(user.id, media_entry.id) + + if cn: + cn.delete() + + +NOTIFICATION_FETCH_LIMIT = 100 + + +def get_notifications(user_id, only_unseen=True): + query = Notification.query.filter_by(user_id=user_id) + + if only_unseen: + query = query.filter_by(seen=False) + + notifications = query.limit( + NOTIFICATION_FETCH_LIMIT).all() + + return notifications + +def get_notification_count(user_id, only_unseen=True): + query = Notification.query.filter_by(user_id=user_id) + + if only_unseen: + query = query.filter_by(seen=False) + + count = query.count() + + return count diff --git a/mediagoblin/notifications/routing.py b/mediagoblin/notifications/routing.py new file mode 100644 index 00000000..e57956d3 --- /dev/null +++ b/mediagoblin/notifications/routing.py @@ -0,0 +1,25 @@ +# GNU MediaGoblin -- federated, autonomous media hosting +# Copyright (C) 2011, 2012 MediaGoblin contributors. See AUTHORS. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +from mediagoblin.tools.routing import add_route + +add_route('mediagoblin.notifications.subscribe_comments', + '/u//m//notifications/subscribe/comments/', + 'mediagoblin.notifications.views:subscribe_comments') + +add_route('mediagoblin.notifications.silence_comments', + '/u//m//notifications/silence/', + 'mediagoblin.notifications.views:silence_comments') diff --git a/mediagoblin/notifications/task.py b/mediagoblin/notifications/task.py new file mode 100644 index 00000000..52573b57 --- /dev/null +++ b/mediagoblin/notifications/task.py @@ -0,0 +1,46 @@ +# GNU MediaGoblin -- federated, autonomous media hosting +# Copyright (C) 2011, 2012 MediaGoblin contributors. See AUTHORS. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +import logging + +from celery import registry +from celery.task import Task + +from mediagoblin.tools.mail import send_email +from mediagoblin.db.models import CommentNotification + + +_log = logging.getLogger(__name__) + + +class EmailNotificationTask(Task): + ''' + Celery notification task. + + This task is executed by celeryd to offload long-running operations from + the web server. + ''' + def run(self, notification_id, message): + cn = CommentNotification.query.filter_by(id=notification_id).first() + _log.info('Sending notification email about {0}'.format(cn)) + + return send_email( + message['from'], + [message['to']], + message['subject'], + message['body']) + +email_notification_task = registry.tasks[EmailNotificationTask.name] diff --git a/mediagoblin/notifications/tools.py b/mediagoblin/notifications/tools.py new file mode 100644 index 00000000..25432780 --- /dev/null +++ b/mediagoblin/notifications/tools.py @@ -0,0 +1,55 @@ +# GNU MediaGoblin -- federated, autonomous media hosting +# Copyright (C) 2011, 2012 MediaGoblin contributors. See AUTHORS. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +from mediagoblin.tools.template import render_template +from mediagoblin.tools.translate import pass_to_ugettext as _ +from mediagoblin import mg_globals + +def generate_comment_message(user, comment, media, request): + """ + Sends comment email to user when a comment is made on their media. + + Args: + - user: the user object to whom the email is sent + - comment: the comment object referencing user's media + - media: the media object the comment is about + - request: the request + """ + + comment_url = request.urlgen( + 'mediagoblin.user_pages.media_home.view_comment', + comment=comment.id, + user=media.get_uploader.username, + media=media.slug_or_id, + qualified=True) + '#comment' + + comment_author = comment.get_author.username + + rendered_email = render_template( + request, 'mediagoblin/user_pages/comment_email.txt', + {'username': user.username, + 'comment_author': comment_author, + 'comment_content': comment.content, + 'comment_url': comment_url}) + + return { + 'from': mg_globals.app_config['email_sender_address'], + 'to': user.email, + 'subject': '{instance_title} - {comment_author} '.format( + comment_author=comment_author, + instance_title=mg_globals.app_config['html_title']) \ + + _('commented on your post'), + 'body': rendered_email} diff --git a/mediagoblin/notifications/views.py b/mediagoblin/notifications/views.py new file mode 100644 index 00000000..d275bc92 --- /dev/null +++ b/mediagoblin/notifications/views.py @@ -0,0 +1,54 @@ +# GNU MediaGoblin -- federated, autonomous media hosting +# Copyright (C) 2011, 2012 MediaGoblin contributors. See AUTHORS. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +from mediagoblin.tools.response import render_to_response, render_404, redirect +from mediagoblin.tools.translate import pass_to_ugettext as _ +from mediagoblin.decorators import (uses_pagination, get_user_media_entry, + get_media_entry_by_id, + require_active_login, user_may_delete_media, user_may_alter_collection, + get_user_collection, get_user_collection_item, active_user_from_url) + +from mediagoblin import messages + +from mediagoblin.notifications import add_comment_subscription, \ + silence_comment_subscription + +from werkzeug.exceptions import BadRequest + +@get_user_media_entry +@require_active_login +def subscribe_comments(request, media): + + add_comment_subscription(request.user, media) + + messages.add_message(request, + messages.SUCCESS, + _('Subscribed to comments on %s!') + % media.title) + + return redirect(request, location=media.url_for_self(request.urlgen)) + +@get_user_media_entry +@require_active_login +def silence_comments(request, media): + silence_comment_subscription(request.user, media) + + messages.add_message(request, + messages.SUCCESS, + _('You will not receive notifications for comments on' + ' %s.') % media.title) + + return redirect(request, location=media.url_for_self(request.urlgen)) diff --git a/mediagoblin/routing.py b/mediagoblin/routing.py index a650f22f..986eb2ed 100644 --- a/mediagoblin/routing.py +++ b/mediagoblin/routing.py @@ -35,6 +35,7 @@ def get_url_map(): import mediagoblin.edit.routing import mediagoblin.webfinger.routing import mediagoblin.listings.routing + import mediagoblin.notifications.routing for route in PluginManager().get_routes(): add_route(*route) diff --git a/mediagoblin/static/css/base.css b/mediagoblin/static/css/base.css index 5b8226e6..888d4e42 100644 --- a/mediagoblin/static/css/base.css +++ b/mediagoblin/static/css/base.css @@ -384,6 +384,12 @@ a.comment_whenlink:hover { margin-top: 8px; } +.comment_active { + box-shadow: 0px 0px 15px 15px #378566; + background: #378566; + color: #f7f7f7; +} + textarea#comment_content { resize: vertical; width: 100%; diff --git a/mediagoblin/static/js/notifications.js b/mediagoblin/static/js/notifications.js new file mode 100644 index 00000000..77793b34 --- /dev/null +++ b/mediagoblin/static/js/notifications.js @@ -0,0 +1,18 @@ +'use strict'; +var notifications = {}; + +(function (n) { + n._base = '/'; + n._endpoint = 'notifications/json'; + + n.init = function () { + $('.notification-gem').on('click', function () { + $('.header_dropdown_down:visible').click(); + }); + } + +})(notifications) + +$(document).ready(function () { + notifications.init(); +}); diff --git a/mediagoblin/submit/views.py b/mediagoblin/submit/views.py index a70c89b4..64e6791b 100644 --- a/mediagoblin/submit/views.py +++ b/mediagoblin/submit/views.py @@ -34,6 +34,8 @@ from mediagoblin.media_types import sniff_media, \ from mediagoblin.submit.lib import check_file_field, prepare_queue_task, \ run_process_media, new_upload_entry +from mediagoblin.notifications import add_comment_subscription + @require_active_login def submit_start(request): @@ -92,6 +94,8 @@ def submit_start(request): run_process_media(entry, feed_url) add_message(request, SUCCESS, _('Woohoo! Submitted!')) + add_comment_subscription(request.user, entry) + return redirect(request, "mediagoblin.user_pages.user_home", user=request.user.username) except Exception as e: diff --git a/mediagoblin/templates/mediagoblin/base.html b/mediagoblin/templates/mediagoblin/base.html index 6c7c07d0..f2723edb 100644 --- a/mediagoblin/templates/mediagoblin/base.html +++ b/mediagoblin/templates/mediagoblin/base.html @@ -34,6 +34,8 @@ src="{{ request.staticdirect('/js/extlib/jquery.js') }}"> + {# For clarification, the difference between the extra_head.html template # and the head template hook is that the former should be used by @@ -57,6 +59,9 @@
{%- if request.user %} {% if request.user and request.user.status == 'active' %} + + + {{ request.notifications.get_notification_count(request.user.id) }}
▼
▲
{% elif request.user and request.user.status == "needs_email_verification" %} @@ -109,6 +114,7 @@

{% endif %} + {% include 'mediagoblin/fragments/header_notifications.html' %}
{% endif %} diff --git a/mediagoblin/templates/mediagoblin/fragments/header_notifications.html b/mediagoblin/templates/mediagoblin/fragments/header_notifications.html new file mode 100644 index 00000000..613100aa --- /dev/null +++ b/mediagoblin/templates/mediagoblin/fragments/header_notifications.html @@ -0,0 +1,40 @@ +{% set notifications = request.notifications.get_notifications(request.user.id) %} +{% if notifications %} +
+

{% trans %}New comments{% endtrans %}

+ +
+{% endif %} diff --git a/mediagoblin/templates/mediagoblin/user_pages/media.html b/mediagoblin/templates/mediagoblin/user_pages/media.html index fb892fd7..a2a8f3b6 100644 --- a/mediagoblin/templates/mediagoblin/user_pages/media.html +++ b/mediagoblin/templates/mediagoblin/user_pages/media.html @@ -167,6 +167,8 @@ {% include "mediagoblin/utils/exif.html" %} + {% include "mediagoblin/utils/comment-subscription.html" %} + {%- if media.attachment_files|count %}

{% trans %}Attachments{% endtrans %}