Better API for auth

[allaire]

Our app accepts both login with username and email address.

I currently need to do in our app:

    NSString *user = self.usernameEmailTextField.text;
    NSString *password = self.passwordTextField.text;
    void (^meteorCallback)(NSDictionary *response, NSError *error);
    meteorCallback = ^(NSDictionary *response, NSError *error) {
        if (error) {
            [self handleFailedAuth:error];
        } else {
            [self handleSuccessfulAuth];
        }
    };

    if ([CLAHelper validEmail:user]) {
        // Without userParameters, ObjectiveDDP user email address
        [[CLAMeteorClient sharedClient] logonWithUsername:user password:password responseCallback:meteorCallback];
    } else {
        NSDictionary *userParameters = @{@"user": @{@"username": user}};
        [[CLAMeteorClient sharedClient] logonWithUserParameters:userParameters username:user password:password responseCallback:meteorCallback];
    }

The current API is not really well designed to accept both types (email and username).

IMO, we should create a new method called:

- (void)logonWithUser:(NSString *)user password:(NSString *)password responseCallback:(MeteorClientMethodCallback)responseCallback;

And detect in there if we need to create a login using email or username (like I did in the first snippet above).

That would be 100% in line with http://docs.meteor.com/#meteor_loginwithpassword

user Object or String
Either a string interpreted as a username or an email; or an object with a single key: email, username or id.

I'm willing to contribute @boundsj and PR if you give me the green light about this, thanks!

[mikey0000]

@allaire I think @boundsj must be on holiday, I'm keen to merge this into my fork, can always make a pull request back later.

[boundsj]

@allaire thanks for this. As you say, it seems like the current logonWithUserParameters:username:password:responseCallback: method around line 127 gives users flexibility but leaks the details of having to sort out if a username is a "username" or "valid email". And now, with the changes from #81 a client also needs to know to send a password sha and algorithm string which is no good.

If it would still be useful for you and you want to send a PR for this I'd be happy to accept it. In any case, I will keep this issue open and take care of it in a future commit. Thanks and sorry for being a 🐌

[allaire]

@boundsj Thanks for the heads up, we'll try to submit a PR asap.

[charlesvallieres]

I just made some changes to the auth API, so we might have to change the readme? (if you accept them)

I think it was kind of useless to pass a nil parameter and username and password to logonWithUserParameters, now it always expects a not nil userParameters.

Since username can be a username or an email (or id) I made it that the logonWithUsername now accepts a username or an email.

Maybe that last change is more of an app thing. Then I can remove the _validEmail stuff and add 2 methods : logonWithEmail and logonWithID.

thoughts? @allaire @boundsj @mikey0000

:)

[charlesvallieres]

And I am not sure what is the usage of _logonParams and why it was passed as the default parameter, maybeI broke something, was it for the reconnection if it failed?

[mikey0000]

@charlesvallieres I see what you've done, my concern is that trying to validate emails is probably the wrong way to go about it unless all the validation checks is for the @ symbol due to the number of variations emails can have also this is an app concern not really this lib. I would be more inclined to separate out into two methods as you've mentioned. I would also try to match syntax with Meteors own api to reduce confusion as much as possible.

I would pass the user params

userParameters[@"user"] = @{@"email": email}; or @{@"username": username};

from each of the two methods onto the logonWithUserParameters method

so a loginWithUsername
and a loginWithEmail

EDIT: just had a think about your specific concern

With this approach you could also have a third method called login(WithUsernameOrEmail) that should only check for the @ symbol @boundsj I think this is a good way of extending. what are your thoughts?

@charlesvallieres yes the _loginParams are for re-connecting if the socket disconnects.

[charlesvallieres]

Yeah I wasn't sure about the _validEmail after I pushed that's why I suggested it. It is better this way, what do you think?

[mikey0000]

I'm happy, @boundsj if you're happy I can merge and check tonight.

[boundsj]

👍