Opened 15 years ago

Last modified 15 years ago

#12 closed defect (FIXED)

Email verification for new users

Reported by: Christopher Allan Webber Owned by: joar
Priority: minor Milestone: 0.0.2
Component: Keywords:
Cc: Parent Tickets:

Description

New users should be requested email verification.

Steps to make happen:

  • user['status'] should default to 'needs_email_verification'
  • After registering, a property should be set with a unique random verification key
  • Link embedding verification key and username should be emailed to user
  • Clicking the link should activate the user's account, setting user['status'] to active

Attachments (1)

0001-Added-functionality-to-support-user-email-verificati.patch (4.6 KB ) - added by joar 15 years ago.
0001-Added-functionality-to-support-user-email-verificati.patch

Download all attachments as: .zip

Change History (9)

comment:1 by Christopher Allan Webber, 15 years ago

Owner: set to Will Kahn-Greene

Will, I've assigned this to you. You don't have to do it... if you don't want to, assign back to me, but I think it would be a good first-mediagoblin-coding task for you if you'd like.

I've already indicated to the FSF that I'd be borrowing stuff from cc.engine in my copyright assignment, so if you like, you can borrow these email sending tools, which might make testing a bit easier.

http://code.creativecommons.org/viewgit/cc.engine.git/tree/cc/engine/util.py#n421 http://code.creativecommons.org/viewgit/cc.engine.git/tree/cc/engine/tests/test_util.py#n94

comment:2 by Will Kahn-Greene, 15 years ago

Milestone: 0.0.2

I'm going to tentatively put this in the 0.0.2 milestone.

comment:3 by Christopher Allan Webber, 15 years ago

Owner: changed from Will Kahn-Greene to Joar Wandborg

Assigning this to Joar, who is already taking initiative on it. (Thanks, Joar!)

comment:4 by joar, 15 years ago

  • Link embedding verification key and username should be emailed to user

Changes available at https://github.com/jwandborg/mediagoblin/commits/master

by joar, 15 years ago

0001-Added-functionality-to-support-user-email-verificati.patch

comment:5 by joar, 15 years ago

The email verification is now working.

Merge request filed at https://gitorious.org/mediagoblin/mediagoblin/merge_requests/1

comment:6 by Christopher Allan Webber, 15 years ago

So, this is looking good. Some comments:

  • Make sure you follow PEP8, the official python style guide: http://www.python.org/dev/peps/pep-0008/ Most especially make sure your lines don't got longer than 80 characters.
  • Create a template in mediagoblin/templates/mediagoblin/verify_email.txt to compose the email body from and use that. It'll make it easier for local instances to override things via custom templating. Since this is plaintext you might want to make things less irritating for yourself while using whitespace control: http://jinja.pocoo.org/docs/templates/#whitespace-control
  • Please remove the clever to_emails list or string added to the send_email() function... I understand and appreciate what you are trying to do but I like to avoid magical arguments as much as possible... just wrap your call to the parameter in a list or tuple please :)
  • You put three TODOs in register... please use comments instead of multi-line quotes to write up comments. As for these I think you can remove number three, it's not a huge issue, but keep 1 and 2, good catch. You don't need to resolve these before closing this ticket though.

As for the global EMAIL_SENDER_ADDRESS, I have some ideas for a more customizable email system, but for now let's just do the following:

Now you should be able to do:

from mediagoblin import globals as mgoblin_globals

[...]
            send_email(
                mgoblin_globals.from_email,

Now here's me being pedantic about verify_email.html:

{% if verification_successful %}
Your email address has been verified!
{% else %}
The verification key or user id is incorrect
{% endif %}

I think it looks nicer/clearer if you indent if sections (when it's not a plaintext template)

{% if verification_successful %}
  Your email address has been verified!
{% else %}
  The verification key or user id is incorrect
{% endif %}

I know this is a lot of commentary. :) Your patch is looking good though, and I think after you do these few things it's good to merge!

comment:7 by Christopher Allan Webber, 15 years ago

Status: NewClosed

There was a bug, list('toemail@example.org') made things go ['t', 'o', 'e', 'm', 'a', 'i', 'l', '@', 'e', 'x', 'a', 'm', 'p', 'l', 'e', '.', 'o', 'r', 'g'] but I fixed that one. Everything else was good. Merged into master. Congrats Joar on completing our first non-founder feature merged into master!

comment:8 by Will Kahn-Greene, 14 years ago

The original url for this bug was http://bugs.foocorp.net/issues/271 .

Note: See TracTickets for help on using tickets.