okland / accounts-phone

A login service based on mobile phone number for Meteor
118 stars 87 forks source link

undefined _id after createUserWithPhone server-side #18

Closed clarkritchie closed 8 years ago

clarkritchie commented 8 years ago

We're using Accounts.createUserWithPhone server-side so we can wrap it with necessary permission checks, add the user to necessary roles post-create, and so on.

Simplified a little, our Meteor method pretty much boils down to this:

createUser: function(user) {
    check(user, Object);

    // assign a random password if none was specified
    if (!user.password) {
        user.password = Random.id();
    }

    // default profile.offers to true
    user.profile.offers = true;

    var id = Accounts.createUserWithPhone(user); // no callback on server
    Roles.addUsersToRoles(id, 'users');
    // can't return the _id, breaks the phone package callback
}

The documentation says no callback on the server.

For grins I wrapped createUserWithPhone in a try/catch and logged out the _id.

            try {
                var id = Accounts.createUserWithPhone(user); // no callback on server
            } catch(e) {
                console.log("problem creating user: ", e);
            }
            console.log("User Created, _id: ", id);

I am seeing a lot of undefined values.

root@mycoolserver:/var/log# cat mycoolapp.log | grep "User Created"
User Created, _id:  oftMAFFEhh2jdK9bc
User Created, _id:  undefined
User Created, _id:  undefined
User Created, _id:  undefined
User Created, _id:  undefined
User Created, _id:  xCj2HuMSS5rBSABuP
User Created, _id:  undefined
User Created, _id:  undefined
User Created, _id:  3rh5ti2Q8W6kcsrdM
User Created, _id:  7E5iBBi2hKN8bfvJG
User Created, _id:  oBN6HJAk5Yh6pZEox
User Created, _id:  undefined
User Created, _id:  undefined

Any thoughts?

ahmedtabrez commented 8 years ago

Strange !!! (Niether creating user, nor throwing an exception.)

It may be possible that one of your fields may be empty. Not sure though.

Check to see if this is happening when you allow Random.id() to generate a password.

If I get time, I'll test this at my end.

clarkritchie commented 8 years ago

I've never seen Random.id() return blank but happy to add some checks around that. I probably can't release any code until next week due to the holiday...

The logic there was to set an initial password, force them to verify their phone, and then make them set their own password.

This is probably unrelated, but I've seen Meteor.userId() sometimes be null server side when it absolutely, positively should not be null. This is pretty rare but enough to notice. I haven't gone through your code, any chance you're relying on that?

clarkritchie commented 8 years ago

OK I think there are/were two issues.

Client side behavior in my app (users pressing enter on their phone vs. using the submit button) was sending incomplete data to my createUser method, which in turn called your createUserWithPhone with incomplete data. I fixed that with stronger client side data validation on my end.

I do think that createUserWithPhone was definitely not throwing an exception when it received incomplete data. So perhaps stronger arg checking on your end? I don't know, sort of swamped and can't test much right now.

Thanks for the package! Over 2k verified users in our app.