Allow multiple username fields

[sdbondi]

Usage:

options.usernameFields = ['username', 'user[email]', 'email'];

Defaults to ['username']

This allows the lookup to find for e.g. an email or username and pass that to the strategy verify function.

[coveralls]

Coverage increased (+2.4%) to 100.0% when pulling 5b4b4bb7c608e611d6e5550221a6f9a05675e03b on fixate:feature/multipleusernames into 3b9980f6467fd1203ecf8844fb017b5db2941606 on jaredhanson:master.

[jaredhanson]

What's the use case for this? The submitted form has a single key, why would the strategy itself not be in sync with that?

[ekryski]

I'll chime in and say that I'm not sure this is needed. @sdbondi regarding Feathers the new Auth 1.0.0-alpha allows you to register strategies the same way you would passport so you can have custom strategies.

Furthermore, you can register multiple local strategies with different username fields as options and chain them. Here's how you would accomplish that with the new feathers-authentication-local plugin.

// On the server
app.configure(auth({ secret: 'super secret' }))
  .configure(local()) // defaults usernameField to 'email'
  .configure(local({ // support username
    name: 'local-username',
    usernameField: 'username'
  }));

app.service('authentication').hooks({
  before: {
    create: auth.hooks.authenticate(['local', 'local-username'])
  }
});

// On the client

// for email
app.authenticate({
  strategy: 'local',
  email: 'me@mydomain.com',
  password: 'password'
}).then(...);

// username
app.authenticate({
  strategy: 'local-username',
  username: 'hulkhogan23',
  password: 'password'
}).then(...);

@jaredhanson my personal opinion is that you could close this but I'll leave that for you two to discuss. 🍻

[sdbondi]

@jaredhanson I see your point, the use case is probably too specific - we have two forms, one with a username, one with an email address but they submit to the same endpoint.

@ekryski Thanks, I'll definitely make use of the decoupled version of feathers-auth when I get a chance - that does indeed make this unnecessary :+1:

I'll close this.

[ekryski]

Sounds good @sdbondi. I'm just wrapping up the auth client right now and publishing the new permissions and all the new pieces should be ready to try out.

[euglv]

@ekryski wrote:

// On the client

// for email
app.authenticate({
  strategy: 'local',
  email: 'me@mydomain.com',
  password: 'password'
}).then(...);

// username
app.authenticate({
  strategy: 'local-username',
  username: 'hulkhogan23',
  password: 'password'
}).then(...);

And what if I want to implement login via login or phone with only one login form and if login can be numeric? I will need to do something like this:

// login
app.authenticate({
  strategy: 'local',
  login: loginOrPhone,
  password: password
}).then(
  ...
).catch(() => {
  // phone
  app.authenticate({
    strategy: 'local-phone',
    login: loginOrPhone,
    password: password
  }).then(...);
});

But it is not very efficient. It would be better to have an ability to authenticate with only one API call. So I think this was useful pull request.

[Atif252]

I think instead of an array of fields it would be better if we had an option to bypass this conditional check and handle and validate the username and password fields manually

https://github.com/jaredhanson/passport-local/blob/0aefeb66e56c790b34a674682d4e6d8a5e9d0a05/lib/strategy.js#L74-L76