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.
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.
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
I was not able to reproduce it on local Allura instance. Did you test it on a sandbox? What configuration did you use?
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:
upd: I've tested on rebased branch, pushed it to
ib/7704
Last edit: Igor Bondarenko 2014-11-10
I have retested - it is working fine, sorry for that.
Need a migration to set all existing users to 'pending: false', else queries won't find them.
Log message should be updated here:
And
claimed_by_user()
doesn't check for pending. I think it should, so that places likeidentify_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 theclaimed_by_user()
method whether to include pending users or not.Closed #683.
+ 49b62f3...3a959f0 t683_migration -> ib/7704 (forced update)
Need to run migration:
paster script development.ini ../scripts/migrations/031-set-user-pending-to-false.py