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

fix(email): Check unquiness of email #15205

Closed
wants to merge 1 commit into from
Closed

fix(email): Check unquiness of email #15205

wants to merge 1 commit into from

Conversation

raisedadead
Copy link
Member

@raisedadead raisedadead commented Jun 4, 2017

This should let us check for uniqueness of email when the user attributes are updated via the API.

@raisedadead raisedadead added the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label Jun 4, 2017
@raisedadead
Copy link
Member Author

Note: I have although left the error message display to a raw message returned from loopback, this is done intentionally because the entire reset logic is going away in any case with passwordless.

@BerkeleyTrue:
Also, any users already duplicated will be affected. To find out we should do some backend purging or notifications to such users.

@raisedadead
Copy link
Member Author

Also do we need this on production?

@camperbot
Copy link
Contributor

@raisedadead updated the pull request.

// see:
// https://github.com/strongloop/loopback/issues/1137#issuecomment-271437926
User.validate('email', function( err ) {
if ( this.email && !isEmail(this.email)) {

This comment was marked as off-topic.

@BerkeleyTrue
Copy link
Contributor

@raisedadead We originally removed the email check because we only allowed third party sign ups, and that resulted in only having usernames as unique to everyone.

Since we are now moving to email first validation, should we just remove the email validation deletion?

@camperbot
Copy link
Contributor

@raisedadead updated the pull request.

@raisedadead
Copy link
Member Author

@BerkeleyTrue Excellent point, yes we should do that when we remove GitHub auth completely (maybe after passwordless goes in).

Until then this is we would still need this (as we are still creating user accounts, with GitHub)

@BerkeleyTrue
Copy link
Contributor

Ok, then an addition to passwordless would be to remove the ability to create accounts with github and re-add the original email validations.

@raisedadead
Copy link
Member Author

@BerkeleyTrue Yes, that is correct and #10407 should address this.

I have updated the user stories so that this is tracked there.

// Updated workaround for email validation
// see:
// https://github.com/strongloop/loopback/issues/1137#issuecomment-271437926
User.validate('email', function( err ) {

This comment was marked as off-topic.

// https://github.com/strongloop/loopback/issues/1137#issuecomment-271437926
User.validate('email', function( err ) {
if (this.email && !isEmail(this.email)) {
debug(err);

This comment was marked as off-topic.

debug(err);
}
}, {
message: 'Email format is invalid, please check again'

This comment was marked as off-topic.

@camperbot
Copy link
Contributor

@raisedadead updated the pull request.

@raisedadead
Copy link
Member Author

Just confirming again, do we need this on production?

@camperbot
Copy link
Contributor

@raisedadead updated the pull request.

@camperbot
Copy link
Contributor

@raisedadead updated the pull request.

@@ -608,7 +608,14 @@ module.exports = function(app) {
return User.findById(req.accessToken.userId, function(err, user) {
if (err) { return next(err); }
return user.updateAttribute('password', password, function(err) {
if (err) { return next(err); }
if (err) {

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

if (err) {
debug(err);
req.flash('error', {
msg: err.message ||

This comment was marked as off-topic.

This comment was marked as off-topic.

if (this.email && !isEmail(this.email)) {
// Here err is a callback
// See loopback api docs loopback-datasource-juggler/#validatable-validate
debug(err());

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@raisedadead raisedadead added the status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer. label Jul 13, 2017
@BerkeleyTrue BerkeleyTrue removed status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer. status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. labels Sep 2, 2017
@raisedadead raisedadead deleted the fix/unique-email-validation branch September 2, 2017 15:17
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.

None yet

3 participants