grails / grails-spring-security-ui

The Spring Security UI plugin provides CRUD screens and other user management workflows.
https://grails.github.io/grails-spring-security-ui/
73 stars 97 forks source link

Feature request: Activate/Deactivate email registration validation via config #49

Closed edwardotis closed 6 years ago

edwardotis commented 8 years ago

It appears that email validation for new users is required. This is not ideal for development and testing. It would be very helpful to be able to control this feature via a configuration. For example: grails.plugin.springsecurity.register.requireEmailValidation=True/False (default True)

amondel2 commented 8 years ago

I would like nothing to require e-mail validation, since one of the current project I am working on, there is no mail server. Overriding the register action of the RegisterController is easy enough to avoid e-mail if you just create this configuration option locally and then do an if/else case. The real security issues comes in with trying to make sure people can use the Forgot Password functionality. I am going to work around that by using challenge questions and as a fall back if they forget the challenge questions then tell the user to e-mail an admin.

Ignoring the issues with Forget Password there are four use cases where email validation comes into the picture. Validation on registration only, Never use e-mail validation, Validation for both registration and forgot password, and l validation on Forget Password only.

One option would be to make two properties like you have suggested. grails.plugin.springsecurity.register.requireEmailValidation=True/False (default True) grails.plugin.springsecurity.forgot.requireEmailValidation=True/False (default True)

The issue comes in if there are new e-mail validation flows added in the future, then we have to keep adding new properties. The other option would be a generic configuration property named grails.plugin.springsecurity.requireEmailValidation which would be a list of flows where you want e-mail enabled. The default would be grails.plugin.springsecurity.requireEmailValidation= [REGISTRATION, FORGOT]. Then if more e-mail flows are added in the future, the list of Values would be updated. For my use-case of never wanting e-mail I would make the config an empty list or null, which not matter how many new flows where added would never use e-mail. Applications using the default additionally wouldn’t need to make any changes. It would cause each application that wanted a partial e-mail validation to consider e-mail validation if a new flow was added and e-mail validation would be disabled until it was added since you were overriding the plugin default config, this maybe a good until since each developer can evaluate if the application warrants having e-mail validation turned on.

In circling back to the Forgot Password I think by default it should not allow password rests if e-mail validation is disabled. I would think you want to tell users to email some address that you can put in by customizing a view or would require the developer to override the controller action with an alternate flow (as I am doing).

I would want Burt/Graeme to weight in before I made a Pull request to determine how they would envision such a feature, if they would even accept a Pull request for it.

amondel2 commented 6 years ago

I have finished writing the code for this but still have to do the testing. I wrote it against the master branch which is probably not what I should have done :disappointed: I would want to go in the latest/next version of the code base, is that the 3.0.x branch?

I ended going with my two parameter system and then added some additional parameter around Password Reset validation that can be used with/without e-mail validation. The MVP feature was to add one parameter that a user could enter to compare against but that parameter can be stored in any domain object so long as that domain object has a reference back to the user.

Please check out the code in my branch and let me know any feedback before I submit a pull request...also if anybody want to write some tests that would be great!

amondel2 commented 6 years ago

I just created a Pull Request https://github.com/grails-plugins/grails-spring-security-ui/pull/94. I would still be open to some feedback on what I did or at least a code review

ddelponte commented 6 years ago

@amondel2 let me know if you'd like assistance with documentation. I'm happy to help.

amondel2 commented 6 years ago

I did check in most of configuration sections with the commit you already reviewed, but it hasn't grammar and spell checked. I also think I need some screen shots for some of the other sections so maybe if you could get them. I was meaning to get to it two days go but new goal is now this weekend.

ddelponte commented 6 years ago

Reviewed, squashed and merged.

All of your hard work is very much appreciated.

Thank-you @amondel2 ! ✨ 🎉