enforceEmailVerification with other services and account-meld

[steph643]

The doc says:

Warning: experimental! Use it only if you have accounts-password as the only service!!!

Well, I have just realized my app uses enforceEmailVerification with Facebook and Google services, and also with account-meld.

What kind of risk am I taking?

[splendido]

This is what you get as login hook when you enable the enforceEmailVerification option.

I think the doc should be rephrased a bit. If you'r not using accounts-password you don't risk anything actually!

...the problem would be using OAuth services that let you in even if you're email address is not verified while telling you it is.. If you also use the password service you would risk to let a user in even if she did not verify any registered email.

[steph643]

Sorry, I forgot to say I use Facebook, Google and accounts-password.

I guess Facebook is ok, as it ensures that the email is verified. However, Google is a problem.

Am I right to say that your enforceEmailVerification hook could check for Google verified email, just like in account-meld? Then the doc could say:

enforceEmailVerification: works only if you use accounts-password and/or accounts-facebook and/or accounts-google. Using any other service will allow a user to log-in without having a verified email address

[splendido]

Looking better at the code I linked above, especially this line and this one, I recalled what could happen.

Since it's controlling the emails field of the user object, and not the registered_emails maintained for splendido:accounts-meld, this is what could happen:

1. A users register with the password but ignores the email with the verification link she get
2. Then come back to the app and logs in with ShameService which let you in even with non-verified email.
3. by chance (or by purpose...) the email address is the same used before and the two accounts get melded.
4. the user is let in since the OAuth login was successfull

So you wanted your users using password login to verify their emails before being allowed to access your app, but using also an OAuth service not enforcing email verification (e.g., Facebook or LikedIn see this table you frustrated the request for email verification.

I hope the example is clear...

How would you put this on the docs?

[steph643]

Is this line actually called? (i.e. is it possible to have info.type === "password" && info.methodName === "login" && info.methodArguments[0].user.email === falsey?)

[splendido]

If the user logs in using username and password you don't get an email address here, so you'll have to look among the emails within the user object (see here).

It would be awesome if you could test it out once again ;-)

[steph643]

Looking at your code, I have another question: Suppose I create an account with 2 email addresses, where one is verified and the other is not. If I try to login with the unverified email, my login will be rejected, right? Don't you think a more useful feature would be that my login is rejected only if I have no unverified email?

[steph643]

If the user logs in using username and password you don't get an email address here, so you'll have to look among the emails within the user object (see here).

Of course, sorry for the dumb question (I forgot about this "login with username" feature, as I don't use it in my app).

[splendido]

mmmm, I guess you're right. with the username login we're asking to have at least one verified email.. ...while with the email login we're asking the email used for the login attemp is verified itself.

Would you get rid of the if clause and keep only these lines then?

[splendido]

...or you meant you'd let a user login only if ALL her email are verified?

[steph643]

Would you get rid of the if clause and keep only these lines then?

Yes I would. I don't know if this is the general need, but it is definitely my need :-)

However, I also question the very opportunity of having this feature in this package. I am currently thinking about it, I will come back to you shortly.

[splendido]

...it will probably make more sense when also splendido:accounts-meld will be integrated into UA: this is likely to happen with useraccouns@2.0

[steph643]

Yes, definitely!

[steph643]

For your information, here is my code:

  // Ensure user has a verified email address
  Accounts.validateLoginAttempt(function (info) {
    // If the attempt has failed: nothing to do
    if (! info.allowed)
      return false;

    //
    // WE WILL CHECK THAT USER HAS AT LEAST ONE VERIFIED EMAIL IN AT LEAST ONE 
    // LOGIN SERVICE
    //
    // Notice that we can't check the "registered_emails" field of 
    // splendido:meteor-accounts-emails-field, because it has not been updated 
    // yet (it will be later in the "onLogin" hook).
    // However, at this stage, even for a first login, "info.user" has been
    // updated with latest login information.
    // 
    var found = false;

    // Check account-password. See:
    // https://github.com/meteor-useraccounts/core/blob/3862dd2ffee16902c9c94cc015cc0bbea3546c34/lib/server.js#L108-111
    found = found
      || _.find(info.user.emails, function (e) { return e.verified; });

    // Check Facebook: no need to do anything, as Facebook enforces email
    // verification
    found = found || info.user.services.facebook;

    // Check Google: see if verified_email is true.
    // THIS MIGHT NOT BE NECESSARY, see:
    // https://github.com/splendido/meteor-accounts-emails-field/issues/28
    found = found
      || (info.user.services.google && info.user.services.google.verified_email);

    // If a verified email address has been found, quit
    if (found)
      return true;

    // Address not found: display an error message based on the latest login
    // service user has tried
    var msg;
    switch (info.type) {
      case 'password':
        msg = 'Cannot sign-in because your email address has not been '
          + 'verified. Please check your inbox or spam folder and click the '
          + 'verification link.';
        break;
      case 'facebook':
        throw Error('should not happen (Facebook ensures email is verified)');
      case 'google':
        msg = 'Cannot sign-in with Google because your Google account has no '
          + 'verified email address. Please go through Google verification '
          + 'process or use another method to sign-in.';
        break;
      default:
        throw Error('should not happen (login with ' + info.type
          + ' is not supported)');
    }
    throw new Meteor.Error(401, msg);
  });

[splendido]

Awesome, It makes a lot of sense!

...@steph643, you seems to have dug well enough into both splendido:accounts-meld and splendido:accounts-emails-field: what about joining the team for useraccounts:meld? :-) There's a board on trello where I'm trying to organize things a bit better... Feel free to have a look at it!

[steph643]

Thanks @splendido. I would love to, but I am currently under heavy pressure and I cannot commit to anything more. I hope I can reconsider this in the future. In the meantime, I will try to help as much as I can.

[juliomac]

Thanks @steph643 and @splendido for this important discussion.

If I uderstood @splendido well, could the warning be rephrase as follows?

Warning: experimental! Use it only if you have accounts-password as the only service OR in combination only with services that do not allow login of unverified emails (such as Facebook and Linked-In).

For my use case, the whole point of user login-in in is to have at least its correct email. Currently I am using only facebook and accounts-password. I was planning to introduce more, but after seeing [this table] https://github.com/splendido/meteor-accounts-emails-field#already-tested-services, it wouldn't make much sense anyway to add those other services, except for Linked-In.

Could I use enforceEmailVerification with no worries in this scenario then?