#7704 Option to require email for user registration NEEDS MIGRATION

v1.2.0
closed
Igor Bondarenko
sf-4 (350) 42cc (432)
General
Alexander Luberg
2015-08-20
2014-09-19
Dave Brondsema
No

Allura doesn't currently require an email address for user registration. There should be an option to require it. We have an option called auth.require_email_addr which is enforced on the /auth/preferences page, so we could use the same setting to control how user registration works.

Users should be in a 'pending' state (new flag) initially, and system should send a password verification email (similar to what we already do on /auth/preferences). After that email is verified, user is no longer pending.

We will have to update many places in the code to check the new 'pending' state. Probable best to search everywhere users are searched by 'disabled' or someuser.disabled is checked, and determine if the pending flag needs to be checked too. Since pending users shouldn't be allowed to do anything yet.

Discussion

  • Dave Brondsema

    Dave Brondsema - 2014-09-22
    • Size: --> 4
     

  • Igor Bondarenko - 2014-09-23
    • Owner: Anonymous --> Igor Bondarenko
    • Labels: --> 42cc
    • Status: open --> in-progress
     

  • Dave Brondsema

    Dave Brondsema - 2014-10-06
    • Milestone: forge-oct-3 --> forge-oct-17
     

  • Igor Bondarenko - 2014-10-15
    • status: in-progress --> code-review
     

  • Igor Bondarenko - 2014-10-15

    Closed #662. je/42cc_7704

    There's one issue I want to discuss. Registration uses User.claim_address and that can cause troubles. If second user will enter existing email during registration, than it will be reassigned to this new user and that's bad. We can check that email already exists and show an error message, but that's enables password enumeration attacks.

    Just rechecked and it seems like email gets reassigned when second user confirms address. And that's probably okay, since only email owner can confirm it.

    What do you think?

    Everything else should work fine.

     

  • Dave Brondsema

    Dave Brondsema - 2014-10-20
    • Milestone: forge-oct-17 --> forge-oct-31
     

  • Dave Brondsema

    Dave Brondsema - 2014-10-30
    • Labels: 42cc, sf-4 --> sf-4, 42cc, sf-current
     

  • Alexander Luberg - 2014-11-03
    • status: review --> in-progress
    • Reviewer: Alexander Luberg
     

  • Alexander Luberg - 2014-11-03

    1) register user1 with email: abc@domain.net
    2) confirm it
    3) login - it works correctly
    4) logout & register user2 with same email
    5) You will get an email that someone tried to register an account with the email - good
    5) login as user1 - you will be redirected to /user/pending instead of homepage

     

    Last edit: Alexander Luberg 2014-11-03

    • Igor Bondarenko - 2014-11-04

      I was not able to reproduce it on local Allura instance. Did you test it on a sandbox? What configuration did you use?

       

      • Igor Bondarenko - 2014-11-10

        I've tried on a sandbox and still was not able to reproduce. After going though all steps I end up logged in as user1 (on Allura part of the site).

        Config I've used:

        auth.method = local
auth.allow_user_registration = true
auth.ldap.autoregister = false
registration.method = local
user_prefs_storage.method = local

        upd: I've tested on rebased branch, pushed it to ib/7704

         

        Last edit: Igor Bondarenko 2014-11-10

  • Dave Brondsema

    Dave Brondsema - 2014-11-03
    • labels: sf-4, 42cc, sf-current --> sf-4, 42cc
     

  • Alexander Luberg - 2014-11-10

    I have retested - it is working fine, sorry for that.

     

  • Dave Brondsema

    Dave Brondsema - 2014-11-10
     

  • Dave Brondsema

    Dave Brondsema - 2014-11-10

    Need a migration to set all existing users to 'pending: false', else queries won't find them.

    Log message should be updated here:

    -        elif user.disabled:
+        elif user.disabled or user.pending:
             log.debug('LdapAuth: user {} is disabled in Allura'.format(username))

    And claimed_by_user() doesn't check for pending. I think it should, so that places like identify_sender() won't find a pending user. But it looks like _verify_addr needs it to match pending users. So I'd suggest adding an option to the claimed_by_user() method whether to include pending users or not.

     

  • Igor Bondarenko - 2014-11-12
    • status: in-progress --> review
     

  • Igor Bondarenko - 2014-11-12

    Closed #683.

    + 49b62f3...3a959f0 t683_migration -> ib/7704 (forced update)

     

  • Dave Brondsema

    Dave Brondsema - 2014-11-17
    • summary: Option to require email for user registration --> Option to require email for user registration NEEDS MIGRATION
    • status: review --> closed
     

  • Dave Brondsema

    Dave Brondsema - 2014-11-17

    Need to run migration: paster script development.ini ../scripts/migrations/031-set-user-pending-to-false.py

     

  • Dave Brondsema

    Dave Brondsema - 2015-01-05
    • Milestone: unreleased --> asf_release_1.2.0
     

Log in to post a comment.