Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strict validations for signup view #1921

Merged
merged 4 commits into from
Sep 15, 2020
Merged

Strict validations for signup view #1921

merged 4 commits into from
Sep 15, 2020

Conversation

saltukalakus
Copy link
Contributor

@saltukalakus saltukalakus commented Sep 10, 2020

Changes

Username validation

In signup view if the user enters an email in the username field, we can't detect this and perform a backend call which ends up with a generic error message. This PR adds a client-side check to verify if the email was typed in the username field and shows an error message instead of making the call.

With this PR we see the following screen as an outcome:

Screen Shot 2020-09-11 at 18 24 28

Email validation

Current email validation is very basic and doesn't check all the different email patters. For example, following is considered a valid email, but it isn't: abc..@xyz.com

A few notes about the implementation

  • To ensure backward compatibility, validations are implemented behind a feature flag named signUpFieldsStrictValidation and the flag is disabled by default. The only use case I can think of which might break with this change is the custom DBs with migration option turned off.

  • While navigating between the login and the signup screen we weren't refreshing the state. This caused the same validation be in effect for the login screen which we don't want. This issue was fixed by updating the state machine during the component mount.

ezgif-5-a098915a4ada

  • If the Auth0 server was returning distinct error codes for the invalid usages, it could be possible to add translations for these errors. Here are the error codes returned from Auth0 server as of today. They aren't sufficient to implement a reliable error messaging with translations.

email
{"error":"error in email - email format validation failed: abc..@xyz.com"}

username
{"name":"BadRequestError","code":"missing_property","description":"Email address cannot be used as username.","statusCode":400,"data":{"min":1,"max":40,"errors":["NOT_PASSED"]}}

References

These changes are related to two support tickets.

Testing

Please describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. If this library has unit and/or integration testing, tests should be added for new functionality and existing tests should complete without errors.

  • This change adds unit test coverage
  • This change adds integration test coverage
  • This change has been tested on the latest version of the platform/language

Checklist

@saltukalakus saltukalakus requested a review from a team September 10, 2020 00:39
@saltukalakus saltukalakus changed the title Email schema verification & avoiding emails for username during signup. WIP: Email schema verification & avoiding emails for username during signup. Sep 11, 2020
@saltukalakus saltukalakus changed the title WIP: Email schema verification & avoiding emails for username during signup. WIP - No ready for merge: Email schema verification & avoiding emails for username during signup. Sep 11, 2020
@saltukalakus saltukalakus changed the title WIP - No ready for merge: Email schema verification & avoiding emails for username during signup. WIP - Not ready for merge - Email schema verification & avoiding emails for username during signup. Sep 11, 2020
…username was enabled in the login tab. An invalid username was thrown for the following sample abc..@xy.com. In login view we shouldn't throw validation error. Email verification is used in other places as well, so I'm not comfortable with updating it at this time.
@saltukalakus saltukalakus changed the title WIP - Not ready for merge - Email schema verification & avoiding emails for username during signup. Avoiding emails as username in signup view. Sep 11, 2020
@saltukalakus saltukalakus changed the title Avoiding emails as username in signup view. WIP - Avoiding emails as username in signup view. Sep 11, 2020
@stevehobbsdev
Copy link
Contributor

@saltukalakus Can you check how this behaves with #1918?

We've just implemented a change where, if you're using a custom resolver, we always use the username field which does not perform formatting validation, where they could use either a username or email address.

@stevehobbsdev
Copy link
Contributor

Also, could you please convert the PR into a draft until it is time to review. Thanks.

@saltukalakus saltukalakus marked this pull request as draft September 14, 2020 21:06
@saltukalakus saltukalakus changed the title WIP - Avoiding emails as username in signup view. Strict validations for signup view Sep 14, 2020
@saltukalakus saltukalakus marked this pull request as ready for review September 14, 2020 23:15
@saltukalakus
Copy link
Contributor Author

👋 @stevehobbsdev the PR is ready for review. I have also tested it with connection resolver option along with your PR. It seems to be working well. I didn't notice an issue.

Copy link
Contributor

@stevehobbsdev stevehobbsdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants