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)
Change History (9)
comment:1 by , 15 years ago
| Owner: | set to |
|---|
comment:2 by , 15 years ago
| Milestone: | → 0.0.2 |
|---|
I'm going to tentatively put this in the 0.0.2 milestone.
comment:3 by , 15 years ago
| Owner: | changed from to |
|---|
Assigning this to Joar, who is already taking initiative on it. (Thanks, Joar!)
comment:4 by , 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 , 15 years ago
| Attachment: | 0001-Added-functionality-to-support-user-email-verificati.patch added |
|---|
0001-Added-functionality-to-support-user-email-verificati.patch
comment:5 by , 15 years ago
The email verification is now working.
Merge request filed at https://gitorious.org/mediagoblin/mediagoblin/merge_requests/1
comment:6 by , 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:
- add a line to mediagoblin.ini under [app:mediagoblin] that's like "from_email = mediagoblin@example.org"
- add a new global variable storing the from_email address in
mediagoblin.globals. The way you should do this is:
- Add a from_email line right after "staticdirector" in MediaGoblinApp.**init** https://gitorious.org/mediagoblin/mediagoblin/blobs/master/mediagoblin/app.py#line38
- Pass that into setup_globals like from_email=from_email https://gitorious.org/mediagoblin/mediagoblin/blobs/master/mediagoblin/app.py#line61
- Pass that in to the MediaGoblinApp instantiation in the paste_app_factory... grab from the config like app_config.get('from_email', 'mediagoblin@example.org'). https://gitorious.org/mediagoblin/mediagoblin/blobs/master/mediagoblin/app.py#line141
- As a bonus I'd appreciate it if you could add it to the setup_globals_func in mediagoblin/celery_setup/from_celery.py ... just grab it from mgoblin_section.get('from_email', 'mediagoblin@example.org') https://gitorious.org/mediagoblin/mediagoblin/blobs/master/mediagoblin/celery_setup/from_celery.py#line80
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 , 15 years ago
| Status: | New → Closed |
|---|
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!

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