Murali-group / GraphSpace

The interactive graph sharing website.
http://graphspace.org
GNU General Public License v2.0
30 stars 41 forks source link

Added functionality to validate the email address during signup #351

Closed lohani2280 closed 5 years ago

lohani2280 commented 6 years ago

Refers https://github.com/Murali-group/GraphSpace/issues/297

lohani2280 commented 6 years ago

@adbharadwaj As you suggested I have added 2 things-

  1. Validate email address - entered value should be valid email
  2. Change the error message to - "You will receive an email with link to update the password" even if the email id doesn't exist. Do we also need to change the Max input length of email on the registration page?
tmmurali commented 6 years ago

@lohani2280 @adbharadwaj I have added some comments to the PR.

@lohani2280 thank you for these changes!

tmmurali commented 6 years ago

It will be valuable to (i) let the user know what a valid email address should look like (as far as expect it) and (ii) if possible, what is wrong with the current email address typed by the user. (i) is easier to do but still important.

On Mon, Feb 12, 2018 at 11:39 AM, Aditya Bharadwaj <notifications@github.com

wrote:

@adbharadwaj commented on this pull request.

In static/js/main.js https://github.com/Murali-group/GraphSpace/pull/351#discussion_r167613140 :

@@ -62,6 +62,18 @@ var header = { var password = $("#password").val(); var verify_password = $("#verify_password").val();

  • if ($("#user_id")) {
  • var reg = /^(([^>()[\]\\.,;:\s@\"]+(\.[^<()[]\.,;:\s@\"]+)*)|(\".+\"))@(([[0-9]{1,3}.[0-9]{1,3}.[0-9]{1,3}.[0-9]{1,3}])|(([a-zA-Z-0-9]+.)+[a-zA-Z]{2,}))$/;
  • if (reg.test(user_id) == false) {
  • $.notify({
  • message: 'Please enter a valid email address!'

We can do that if split the regular expression case by case. I am going to let @lohani2280 https://github.com/lohani2280 document the regular expression above and then we can see what we should notify a user about.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Murali-group/GraphSpace/pull/351#discussion_r167613140, or mute the thread https://github.com/notifications/unsubscribe-auth/AGkWULISR1pTi37pSiXFkBrYVMYJLg-Rks5tUGkngaJpZM4SByH6 .

mitchwagner commented 6 years ago

@lohani2280 @adbharadwaj Should we not also validate on the server-side, if we are not already doing so? Perhaps that should be a part of the PR.

From the link the regex was taken from: https://stackoverflow.com/questions/46155/how-can-an-email-address-be-validated-in-javascript

"But keep in mind that one should not rely only upon JavaScript validation. JavaScript can easily be disabled. This should be validated on the server side as well."

That page seems to have an awful lot of contention on which regexes are rock-solid as well. A lot of people commenting along the lines of "But wait! This doesn't follow RCF XXXXXX, so it ignores valid emails," and a lot of contention about even using regexes in the first place. Another preferred method seems to be simply verifying the email address by having the user enter it in correctly and attempting to follow up with a confirmation email, rather than assuming the entered email is correct by default if it passes a regex, or presuming that a regex covers all possible corner cases and denying anyone with an email that might not be covered by that regex registration capability.

Django has built-in email validators: https://stackoverflow.com/questions/13996622/django-how-to-check-if-something-is-an-email-without-a-form

Validating on both sides seems to be a recommended practice: https://stackoverflow.com/questions/162159/javascript-client-side-vs-server-side-validation

If we do validate on both ends, we should make sure the regexes used on both ends are the same.

I believe Django's email validation regex can be found here: https://github.com/django/django/blob/master/django/core/validators.py

lohani2280 commented 6 years ago

@mitchwagner I agree with the fact that we should validate email on the server side as well. Let me look a bit more deep over this issue and the documentations and I will update the PR accordingly. Thanks for mentioning the links.

lohani2280 commented 6 years ago

I have read a lot of article on email validation and I think that despite the complex regular expression validation, one can just validate the format – not its existence. The only way to truly validate the email address is to send an email to that address and request the user to confirm by clicking on a unique link (or entering a confirmation code).I think we should follow this approach.

@tmmurali @adbharadwaj @mitchwagner Please suggest your views on how to proceed further over this issue.

adbharadwaj commented 6 years ago

@lohani2280 I think this issue is restricted to client-side validation. I am creating a separate issue for server-side email validation which will entail two things:

As @mitchwagner said, we would want to refer to the regex used on client side to validate email ids on the server side as well to maintain consistency.