Opened 13 years ago

Last modified 13 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 13 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, 13 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/util.py#n421>`_
`http://code.creativecommons.org/viewgit/cc.engine.git/tree/cc/engine/tests/test\_util.py#n94 <http://code.creativecommons.org/viewgit/cc.engine.git/tree/cc/engine/tests/test_util.py#n94>`_



comment:2 by Will Kahn-Greene, 13 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, 13 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, 13 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, 13 years ago

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

comment:5 by joar, 13 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 Christopher Allan Webber, 13 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 Christopher Allan Webber, 13 years ago

Status: NewClosed
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!



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

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

Note: See TracTickets for help on using tickets.