Opened 15 years ago
Last modified 14 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: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 , 14 years ago
The email verification is now working. Merge request filed at `https://gitorious.org/mediagoblin/mediagoblin/merge\_requests/1 <https://gitorious.org/mediagoblin/mediagoblin/merge_requests/1>`_
comment:6 by , 14 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/ <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 <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 <mailto: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 <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 <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 <mailto:mediagoblin@example.org>`_').
`https://gitorious.org/mediagoblin/mediagoblin/blobs/master/mediagoblin/app.py#line141 <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 <mailto:mediagoblin@example.org>`_')
`https://gitorious.org/mediagoblin/mediagoblin/blobs/master/mediagoblin/celery\_setup/from\_celery.py#line80 <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 , 14 years ago
| Status: | New → Closed |
|---|
There was a bug,
list('`toemail@example.org <mailto: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!
Note:
See TracTickets
for help on using tickets.
