securesocial icon indicating copy to clipboard operation
securesocial copied to clipboard

One-step user registration

Open tabdulradi opened this issue 11 years ago • 9 comments

This pull request creates another registration controller, that works in parallel with the old one, without breaking it. It allows users to register in one step, but marked as non-verified, later they can verify their account by following a link send in a verification email.

These are changes in the code:

  • Moves old registration controller inside "registration" package
  • Creates a new registration controller that creates the User in one step
  • Creates an action for email verification
  • Add views for the new actions/emails
  • Adds methods for new views, in Mailer and TemplatePlugin (without breaking backward compatibilty)
  • Adds a state field in Identity trait, to hold the state of email verification
  • Adds configuration options to choose the registration method
  • Changes "SecuredAction" to prevent users with non-verified email

I have to note that this feature was implemented by one of our trainees, then reviewed by me. You can see her original changes here

Todo: We are planning to change the SecuredAction (or extend it) to take additional parameters, to allow non-verified users in specific pages, or even allow (recent) non-verified users only.

tabdulradi avatar Aug 13 '13 14:08 tabdulradi

Hi @tabdulradi

Thanks for your contribution. I did a quick look and would need to see this in more detail but here are my first comments:

  1. The registration flow leaks user information (error saying the email is taken). The default flow prevents that.
  2. Adding Active SocialUser might to be needed. Since the change in SecuredAction line 86 does a filter on the result, it really would be the same if your UserService.find method would return None for inactive users.
  3. As of your planned changes to SecuredAction please consider using an Authorization object as an alternative. That would allow your users to access certain pages: in the implementation of that object you could check if the user is active or not. If you make UserService.find return your own model that should be easy. Eg:
 case class WithActiveState extends Authorization {
       def isAuthorized(user: Identity): Boolean {
             user match {
                   case myUser: MyUserClass => myUser.isActive
                   case _ => // did not get a MyUserClass instance, log/handle error. Should not happen.
             }
        }
  }

jaliss avatar Aug 14 '13 21:08 jaliss

Thanks for your reply,

  1. I think we can remove the validation on email field. Then take a decision, either we should send a welcome/validation email, or send instructions email. We will work on it by next week.
  2. UserAwareAction uses UserService.find method too, so returning None for inactive users will prevent us from allowing Inactive users to access specific pages in the future.
  3. Thanks for the note. We will take this into consideration.

tabdulradi avatar Aug 21 '13 12:08 tabdulradi

What do you think now?

tabdulradi avatar Sep 05 '13 16:09 tabdulradi

@jaliss are you going to integrate this feature into securesocial in the near future?

tmbo avatar Nov 12 '13 19:11 tmbo

Hi @jaliss . Are you interested in integrating this feature into SecureSocial? I need it for my project so I'm currently updating it to the latest code in master, and doing some small improvements. Shall I create a new pull request when I'm done? Thanks for your hard work on this great framework.

paiou avatar Feb 15 '14 21:02 paiou

Great work all of you! +1

jakob85 avatar Feb 22 '14 19:02 jakob85

+1 for the great works!

Is there any conclusion this would eventually be pulled or not?

@jaliss I understand that saying the email is taken might leak some user information, but it seems really few sites didn't do that. I feel this feature is essential to have, and it's developer's call to use it or not.

by the way, it seems one of the user in list in securesocial's user list, carambla, is using one step registration https://carambla.com/register

zfy0701 avatar Jul 15 '14 07:07 zfy0701

Hi, what about merge? Will you resolve conflicts?

biokys avatar Nov 03 '14 08:11 biokys

+1 from my side

olmaga avatar Jan 01 '15 19:01 olmaga