Discussing enhancements and changes for V3 rewrite

[eddyystop]

Transfer from https://github.com/feathers-plus/feathers-authentication-management/issues/114

[eddyystop]

Upgrade instructions will be added as needed to https://github.com/feathers-plus/authentication-local-management/blob/master/misc/upgrading.md

Breaking changes to feathers-authentication-management have been made.

[eddyystop]

@claustres @beeplin you may want to audit and comment on the ongoing changes mentioned in the above module.

The first change moves the hashing and comparing of tokens to outside the repo.

[eddyystop]

Move this here from feathers-authentication-management https://github.com/feathers-plus/feathers-authentication-management/issues/99

[eddyystop]

This is a summary of the changes. Please read the details at https://github.com/feathers-plus/authentication-local-management/blob/master/misc/upgrading.md

Previous status report. Converted feathers-authentication-management to async/await.

• feathers-plus/feathers-authentication management (f-a-m) has been rewritten using async/await as feathers-plus/authentication-local-management (a-l-m). No npm package has been published.
• The interfaces can be called using await or using Promises.
• Converting the tests took over 75% of the time. They now use async/await and Feathers instances. They no longer use stubs for Feathers.
• I expect what's there now should work as a drop-in replacement. If you want to help, drop it in into a copy of an existing app and test.
• DO NOT use a-l-m except for short-term testing. I feel no obligation to maintain compatibility as we move forward.

New status report. Outstanding issues in feathers-authentication-management have been addressed. Enhancements have been added.

Breaking changes

• Encryption is no longer performed internally but thru hooks. The dev may therefore customize these.
• The service is now automatically configured with hooks. By default only authorized clients may make checkUnique, passwordChange, identityChange calls. This hook configuration may be overridden with options.authManagementHooks.
• Client calls for passwordChange and identityChange may now only affect their own account. This can be controlled by options.ownAcctOnly whose default is true.
• isVerified no longer checks calls made by the server. isVerified may now be used on all service methods.

Enhancements

• An alternate hash function may be used instead of hashPassword. See external encryption hooks and option.bcryptCompare.
• Options now default to authentication.local.passwordField and .entity fields in config/default.json. passwordField may be overridden by with option.passwordField.
• Coerce fields for Sequelize and Knex. The service uses JS-like fields internally. The conversionSQL hook converts fields to SQL types as needed. The hook must be used in both the before and after hook configuration.
• Every service call may be customized. This, for example, would allow DB changes to be consolidated in a transaction which may be rolled back.
• Customization of errors. options.catchErr(err, options, data) allows the dev to customize any thrown error. The error message or the data returned to the client may be made non-specfic to harden against hacking.
• The addVerification hook now works with arrays of data.

Documentation

• How to configure the user service on the server.
• How to configure the client to prevent timeouts.

Next steps:

• Add a-l-m to an app generated by cli+.
• Expand a-l-m with templates, emailer, maybe an SMS push.
• We talk about other enhancements.

[eddyystop]

I need eyes to consider these changes, which are further described in https://github.com/feathers-plus/authentication-local-management/blob/master/misc/upgrading.md

@claustres @beeplin ^^^

[claustres]

Customization capabilities now seem great (hashing, service calls, etc.).

Async/Await is really great as well.

I wonder what you mean by Hooks are now automatically configured on the a-l-m service. ?

Also you explain which operations are accessible by (un)authenticated users like resendVerifySignup, etc. Is this just an example or is it somewhat hard-coded ? Indeed I think others setup are possible.

Well done master !

[eddyystop]

The service hooks are configured by default as follows

const actionsNoAuth = [
  'resendVerifySignup', 'verifySignupLong', 'verifySignupShort',
  'sendResetPwd', 'resetPwdLong', 'resetPwdShort'
];

const defaultOptions = {
  authManagementHooks: { before: { create: authManagementHook } },
};

async function authManagementHook(context) {
  if (!context.data || !actionsNoAuth.includes(context.data.action)) {
    context = await authenticate('jwt')(context);
  }

  context.data.authUser = context.params.user;
  context.data.provider = context.params.provider;
  return context;
}

 app.service('authManagement').hooks(options.authManagementHooks);

[eddyystop]

This is a summary of the changes. Please read the details at https://github.com/feathers-plus/authentication-local-management/blob/master/misc/upgrading.md

[claustres]

If you can configure on a per operation basis which one requires auth or not by overriding the options then it is great.

[eddyystop]

options.authManagementHooks allows you to do that. Basically pass a modified version of the above hook.

[NickBolles]

I'm implementing f-a-m right now and found this. Is there any chance of publishing a beta npm package? It looks a lot easier to implement.

Another point I would suggest is creating Typescript Typings for the package. I'd be willing to chip in on them.

[eddyystop]

I won't be publishing a "beta" package on npm. You of course could do so for yourself.

f-a-m is stable, decently documented and has 2 sets of articles on Medium. You will finish your project using it.

I'm not sure a-l-m will be faster to implement considering the uncertainties involved. My guess is you'd spend fewer hours using f-a-m and later converting to a-l-m

Its a benefit for me if someone tests a-l-m, but it may impact the time to finish the project.

I'll take you up on embedding TS in the repo as you convert to a-l-m. It would have to be strict typings so its compatible with generated cli+ code.

[eddyystop]

New status report. Reference implementation with browser test bed completed.

Modifications made to previously documented changes

• a-l-m is now configured similarly to how authentication is, i.e. src/authentication.js. This means a-l-m no longer configures its hooks internally, the dev does so. This means the dev can easily customise these hooks.

• hashPasswordAndTokens has been removed as only hashPassword is needed now.

• options.actionsNoAuth has been added. It contains the service calls which unauthenticated clients may make. The default is [ 'resendVerifySignup', 'verifySignupLong', 'verifySignupShort', 'sendResetPwd', 'resetPwdLong', 'resetPwdShort' ]

Breaking changes

• The action: 'options' has been removed. The fully expanded options may be obtained with app.get('localManagement'). This is more secure.

• The client convenient methods in feathers-authentication-management/src/client.js have been removed. They were one line wrappers around service calls to the f-a-m service. They created a conceptual burden for little in return.

Enhancements

• A reference implementation has been developed. It contains a browser-based test bed for a-l-m. This was a substantial effort.

Next steps:

• Review the 2 articles published in Medium on feathers-authentication-management to identify pain points.
• Expand a-l-m with templates, emailer, maybe an SMS push.
• We talk about other enhancements.

Detailed docs

These are only a summary of the changes. You can read the details in the migration document https://github.com/feathers-plus/authentication-local-management/blob/master/misc/upgrading.md

[eddyystop]

"Luc - I can’t simply store the locale on the user record as some things I need the locale for happen with an unauthenticated user - an example is a “forgot password” email - the email’s language needs to match the language set on the iPhone when the request was dispatched."

[eddyystop]

New status report.

• Articles reviewed.
• Reference implementation complete enough for articles. Supports Email and SMS.
• I've realized after some thought that feathers-authentication-management and a-l-m need a different approach to documentation than other repos. These repos do not address a need in isolation. They must be integrated with other server code, with client code, with emailers and SMS pushers. This means the entire structure must be documented and not just this repo.
• Spent about 2 days making diagrams for articles and verifying their correctness. I find these diagrams useful myself to confirm what is going on, and to review looking for weaknesses and improvements.
• I will not push the reference implementation to support Email templates, nor internationalization. That would take attention away from the essential structure. Such an extension may be worthwhile if a longer set of articles is written.
• We may be at a point where we have to start evaluating fundamental enhancements.
• Some enhancements would require custom authentication strategies, which is outside the scope of this rewrite.

I will no longer keep extending the migration document. Its gotten too long, and I think the best way to migrate will be to read the articles and slide that code into your app.

[eddyystop]

https://www.eff.org/deeplinks/2016/12/how-enable-two-factor-authentication-facebook

[claustres]

which articles on Medium are you referring to ?

[eddyystop]

The ones mentioed here https://github.com/feathers-plus/authentication-local-management/issues/8

[eddyystop]

Candidate new features:

(0) Consider a-l-m making internal calls using _get, _find, _patch. These methods are standard (I believe) in all core adapters except feathers-memory. They will be made part of the adapter standard in v3. Before and after hooks are not run with them. Using them would prevent interference with configured hooks; we get and we write directly to the DB.

(1) Add support for 2FA. This would additionally require a custom authenticator. This should be a separate repo and I wouldn't be implementing it.

(2a) Maintain time user last logged on as well as the time before that. This allows the UI to display a "You last logged on at xxx" message. Should IP and some device indication also be kept? A longer chronological list of logon times, IP and device is more properly a logger responsibility.

(2b) Send a verification notification if last logon was too long ago. This is conceptually similar to 2FA.

(3a) For each user maintain a list of previous { hashedPassword, dateCreated } and prevent old passwords from being reused. Do we keep this list in the users service for simplicity or in a related service? May be best to implement as a plugin to a-l-m.

(3b) A new sendResetPwdHistorical request for the client. Its similar to sendResetPwd but requires one of the previous hashedPasswords.

(4a) Maintain a list of IPs and/or devices that have logged on. In users service or in a related service? May be best to implement as a plugin to a-l-m.

(4b) Email/SMS a short token when a new IP/device logs into. This is conceptually similar to 2FA.

(5) Invitation request allowing a user to invite another to join

(6) Gmail sends a 'signing in from another device?' SMS.

Potential features a-l-m won't be implementing.

• Email HTML templates with i18n. This should be a separate repo and it should support HTML templates, i18n and file attachment. What existing options are there to wrap in an adapter? (*) (**)

• Push notification using SMS, WhatsApp. This should be a separate repo. What existing options are there to wrap in an adapter? (***)

• Rate limiting. This should be a separate repo. What existing options are available? https://blog.feathersjs.com/feathersjs-in-production-password-policy-and-rate-limiting-32c9874dc563

Misc

• you could retrieve information like tokens directly from a test user gmail inbox, you can have a look to https://github.com/kalisio/kNotify/blob/master/test/account.test.js,

(*) Look at

• https://github.com/kalisio/kNotify
• https://github.com/kalisio/kNotify/blob/master/src/services/account/account.service.js#L64
• https://github.com/kalisio/aktnmap/tree/master/api/config/email-templates see https://github.com/feathers-plus/feathers-authentication-management/issues/114#issuecomment-418037582
• https://lad.js.org/#/?id=email-engine https://github.com/niftylettuce/email-templates (2k stars)

(*) Look at

• https://github.com/skolmer/es2015-i18n-tag

(***) Look at

• https://github.com/kalisio/kNotify

[eddyystop]

@claustres @beeplin @daffl @Marshall @j2L4e your comments on the above would be appreciated.

[eddyystop]

The features feathers-authentication-management provided, reimplemented here in a-l-m, were confusing enough to people. Articles were written (https://github.com/feathers-plus/authentication-local-management/issues/8) explaining its integration and they received a lot of applause.

I've spent considerable time preparing diagrams to help explain a-l-m. https://www.lucidchart.com/invitations/accept/f4ebc4fd-3297-47e4-8568-eb13778c535c

I think a-l-m is a handful already, and we now want to add more complex features to it. I think additional features should be implemented as plugins. That would keep a-l-m lean conceptually and users would not feel the need to learn everything at once.

The plugins could be released in a more controlled fashion.

[NickBolles]

@eddyystop those diagrams are awesome! It would be beneficial I think to have documentation ( maybe an article makes more sens though) that walks through each part of the diagrams to explains how it is done with code. I think a confusing part is what almost handles and what it doesn't and that an approach like that would help. Great job!

[claustres]

As usual you did a great work and diagrams are often better than written doc, with articles this could be sufficient IMHO. About the features:

0. I don't think it's a good idea to bypass the service, see my previous comment. As an example why it could lead to unexpected behaviour as well see what I learned the hard way: https://github.com/feathersjs/feathers/issues/1049. Often hidden states do exist in apps and are managed through hooks or events.
1. Hard to provide a generic module here, could only be a thin wrapper over SMS or push services like SNS since token generation and verification is already part of a-l-m. Not worth it IMHO.
2. & 4. Are pretty related to devices management. We implemented something like that in a dedicated service for mobiles: https://github.com/kalisio/kNotify/blob/master/src/services/devices/devices.service.js. This is often linked to push notifications and mobile frameworks, which really depends on your implementation. So yes separated plugins are the best choice since otherwise you will have to make too much opiniated choices for a core module
3. Easily implemented with a hook, maybe not worth a plugin, see https://blog.feathersjs.com/feathersjs-in-production-password-policy-and-rate-limiting-32c9874dc563. The difficulty here is to handle i18n for most common passwords. 3b could be also implemented as answer to secret questions since we often do not recall the history. Not sure if usefull with 2FA.
4. Seems to me the most similar feature than existing ones: also relies on email sending with temporary token to be checked, should be easy to implement. The difficult part here is where to store the token if we only create the user when he responds ? On our side we have chosen to create the user on invitation and remove his account if he did not respond in eg 48 hours.

[eddyystop]

@NickBolles Those diagrams are intended to be used in a series of articles. The potential enhancements would introduce new ways of authenticating, and additional diagrams would be needed for those.

@claustres

0. People could customize the default service calls to use _find if needed I guess.

1. 2FA would need tfaExpires, tfatoken, tfaShortToken, its own command and the notifier should know what its sending.

2. & 4. Code! Thanks.

3. Yes, it could be written outside a-l-m, however that may result in questions. My philosophy is to put more work up front rather than continuously deal with questions.

[lwhiteley]

@eddyystop Are you looking at adding in the possibility to manage multiple password/hashed fields?

Use case: an account system that has both a password and a pin.

Adding an override somewhere here should do the trick

options.passwordField = data.passwordField || options.passwordField;

The information about which password field being changed/reset can be passed to the notifier as well.

Relevant actions:

• 'sendResetPwd'
• 'resetPwdLong'
• 'resetPwdShort'
• 'passwordChange'

Let me know if that makes sense or can be supported.

[eddyystop]

@lwhiteley , I am not clear about the use case. Are you asking if a person can log on using email/phone as an account, plus either passord or pin? If so, that is handled via a customized authenticator in @feathersjs/authentication.

a-l-m does not handle the actual authentication, it manages some of the fields on the users record. For your case, I would first try making the pin field as an identity fields and change it using the identityChange action. The identityChange action sends a verification token and waits for a confirmation before updating the field.

A reset could not be done on the pin. A reset on the password followed by an identityChange would be the suggestion.

[xendarboh]

Potential features a-l-m won't be implementing.

Email HTML templates with i18n. This should be a separate repo and it should support HTML templates, i18n and file attachment. What existing options are there to wrap in an adapter? (*) (**)

Push notification using SMS, WhatsApp. This should be a separate repo. What existing options are there to wrap in an adapter? (***)

notifme-sdk is a notification abstraction for providers that include emails | SMS | pushes | webpushes | slack (I don't see WhatsApp).

Notification Catcher is a web interface for viewing and testing notifications during development.

notifme-template plugin features include: Easily implement i18n/l10n for all your notifications

feathers-notifme: Feathers notification service using notifme-sdk

[eddyystop]

@xendarboh That looks very interesting.

[NickBolles]

@xendarboh @eddyystop Hopefully this info adds some more info to help with implementing emails/notifications. I'm using notifme-sdk and notification catcher and it works awesome (at least during dev). I haven't dealt with I18N yet, and it seems to me that a feathers service is overkill, you can just setup notifme in it's own file and import it.

Here's my auth management notifier and notifme setup. Keep in mind it's a little bit customized to my client setup, and most of it is typescript types... https://gist.github.com/NickBolles/1bd3331c02feadcd34fd2042ca73b32c

[eddyystop]

Services have hooks which support authententication, etc., allowing the client to call the notifier. I also like the idea of wrapping what we can in services, even in thin wrappers, to reduce the cognative load.

It would be appreciated if someone wrote non-generated code showing how each of the features would be coded for. That would be the target for the generator, as well as a source from which to decide the prompts for cli+.

I did not see in my brief review of the repo how notifier would be coded to send one email of several. a-l-m needs that option.

[eddyystop]

Status update

Scaffolding to implement plugin support added as @feathers-plus/plugin-scaffolding.

• Plugins may erase previous plugins upon registration. This allows default plugins to be ignored.
• A plugin may have setup, run and teardown steps.
• Plugins share both a global context and one for their specific trigger. This allows plugins to communicate with one another.
• A sequence of plugins may be run for a trigger, reducing to a final result.

Initial plugin support added to a-l-m.

• Default plugins are in src/default-plugins.js .
• To do: override or add to default plugins.

feathers-plus/commons has been introduced as a respository of cross-cutting utilities.

• To do: transfer remaining utilities from a-l-m.

Lesson learned

// Prefer this
const [result] = await foo();
// to
const result = (await foo())[0];

Its too easy to miscode the latter and the consequence could be extremely confusing. Tracking down one such bug took longer than writing and using the plugin support.

[xendarboh]

// Prefer this
const [result] = await foo();
// to
const result = (await foo())[0];

nice! reminds me of the await to pattern for both result and error that I've seen recently in a few places. It also presents a way to use async/await without try/catch. await-to-js is pretty popular for a small one-function package.

const [err, result] = await to(bar());

[eddyystop]

I'm on G4 in a jungle but my understanding is that an await throwing in a function (without try-catch) will cause the function to throw. I don't see wrapping every await in a try-catch unless some param is needed for a user error message. And such cases are usually at the top level function.

[eddyystop]

I'm looking for critiques and comments about this technique and this article https://blog.feathersjs.com/use-plugins-to-publish-incredibly-flexible-code-a55e8faf27a8

[eddyystop]

Finished conversion to plugins.It took longer than anticipated. https://blog.feathersjs.com/use-plugins-to-publish-incredibly-flexible-code-a55e8faf27a8

[eddyystop]

I believe we are finished with converting feathers-authentication-management and can start implementing new features. COMMENTS ARE INVITED.

[eddyystop]

[Completed. Awaiting comments.]

(a) Send an invitation for a user to sign-up.

• Users can only be invited via email, not SMS. So notifiers for 'sendInvitationSignup' and 'resendInvitationSignup' must use email regardless of what the value of the user.preferredComm is.

• Create a user record with "isInvitation: true" (a newly introduced field which is protected from mutation by the existing before hook preventChangesVerification).

• The existing after hook sendVerifySignupNotification sees isInvitation: true and calls the notifier with 'sendInvitationSignup' (instead of 'sendVerifySignup'). This results in a URL being emailed with .../invite/token (rather than .../verify/token).

• The existing resendVerifySignup command sees isInvitation: true and calls the notifier with 'resendInvitationSignup' ( instead of 'resendVerifySignup'). This also results in a URL being sent with .../invite/token (rather than .../verify/token). The value of isInvitation is not changed.

• When the URL link is followed, the frontend router uses .../invite/... to route to the invited user handler. This handler differs from the .../verify/... handler in that, when it calls verifySigupLong, it includes a new password for the user, along with the usual token, etc. (No password was set when the record for the invited user was created; no password was included in the invitation email.) The frontend may ask the user to enter this new password, or it may autogenerate a temporary one.

• A sanitizedForClient user record is returned to the frontend on a successful verifySignupLong call.

• The invite handler is then expected to authenticate the newly invited user with the server. The frontend can then obtain additional/missing information from the user, persisting it with users.patch. (a-l-m related fields are protected from such mutation by the existing before hook preventChangesVerification.)

• A flow diagram of this is available at https://www.lucidchart.com/documents/view/d3ebd525-34c4-4aab-bd51-791e88176a2f/0

• A user with isInvitation: true would always have isVertified: false, so we still only have to check isVerified on authentication.

• We have a command to delete invited users who haven't accepted after a given interval, or users who have not verified in that interval. Selection of users can be customized in the filtering plugin.

[eddyystop]

@claustres ^^^

[eddyystop]

[Completed. Awaiting comments.]

(b) There has been discussion whether 2FA is useful or not. However the same underlying process can be used for 2FA, a sign-in after too long an interval, a sign-in for a new device, etc. I feel its worthwhile to implement enabling capabilities for these features.

• The repo supports one multiple factor authentication (mfa) token. Its a short token, regardless if it's emailed or pushed with SMS. (I don't see a use case where more than 1 mfa is needed simultaneously.)
• Fields mfaExpires, mfaShortToken, mfaType have been introduced. (They are protected from mutation by the existing before hook preventChangesVerification.) mfaType is a code indicating what the current mfa is for. By convension we'll use
  • '2fa' during a sign-in using 2 factor auth,
  • 'old' for a sign in attempt after too long an interval since the last sign-in,
  • 'dev' for a sign in attempt on a new device,
  • the dev may customize other codes.
• The frontend specifies the mfaType when it requests a push of the mfaShortToken. The frontend must specify the same value for mfaType when it is verifying the token entered by the user. This hardens against attacks where an mfa is generated by subterfuge for one reason, e.g. new device, and then used for another purpose, e.g. 2fa.
• The frontend can use the new sendMfa command (with a mfaType) to push the mfa token. It can verify the mfaShortToken entered by the user by using the new verifyMfa command (with the short token and mfaType).
• mfaExpires, mfaShortToken and mfaType are erased by the verifyMfa command whether the verification is successful or not.

[In design. Awaiting comments.]

Some new fields may be introduced to support some of these mfa usages:

• The last several (count?) old passwords, possibly with the datetime they were created.
  • How does this work with additional password-like fields?
  • Do we keep, say, passwords and PIN in an indiscriminate stack? A new password could not be any of the previous PINs then.

??? Is it sensible for a-l-m to keep the following as it is authentication itself that would set them.???

• lastSigin is the datetime of the last successful signin.
• lcountInvalidSigins is the number of unsuccessful sign-in attempts since the last successful one.
• Maybe keep a stack of the recent unsucessful attempts with a timestamp, IP or device id making the attempt.
• Maybe just give a-l-m a list of "extra" fields to protect with preventChangesVerification but then we're conflagating a-l-m and user enhancements.
• ??? Do we develop a companion repo with matching server authentication strategies for the above?
• David is interested in rewriting feathersjs/authentication. How does this impact the above thought?

Do we introduce a changePasswordWithHistory command which prevents use of a password that's in the historical stack of passwords? (This would implemented seperately from mfa development.) changePasswordWithHistory would maintain the stack. Is it correct the existing changePassword does not maintain that stack?

The design @marshallswain suggests for 2FA is as follows:

• Frontend asks for an identity-field and a password-like-field.
• Feathers server authenticates these.
• If the user record indicates 2FA is needed for that user, a short token is pushed to the user, and the frontend is informed.
• Frontend sends the identity-field and a password-like-field it stashed, along with the short token entered by the user.
• Feathers server authenticates this tuple.

[eddyystop]

Reserved.

[eddyystop]

[In design. Awaiting comments.]

(c) Support multiple password-like fields such as password & pin#, as suggested by @lwhiteley.

• We maintain an array of password-like fields instead of the single passwordField. All these fields are protected from uncontrolled mutation.
• I don't agree with the suggestion we change the 'sendResetPwd', 'resetPwdLong', 'resetPwdShort' and 'passwordChange' commands to indicate which password-like field is being changed. First, there could be any number of password-like fields. Second, its awkward to identify which password-like field is being changed; we wouldn't want to use the internal field name.
• My suggestion is only the passwordChange command support changing all the password-like fields. If a user forgets the PIN they primarily sign in with, they will first have to reset their password if they forgot that too, sign in with the (new) password, then change the PIN.
• However this makes the passwordField first-among-equals of the password-like fields, which makes me feel uneasy.

[eddyystop]

Reserved.

[claustres]

I took some time to read your article about plugins and it seems to me simple, elegant and flexible, so far so good.

About invitation the process seems fine to me, it is more secure than the one we created awaiting enhancement in a-l-m: no password generated upfront. Using plugins or options it would be easy to customize things like to add the sponsor who invited the user or whatever.

About 2FA I agree it is useful for new device singin so MFA code type is probably a great thing. We should take care to prevent concurrency here so that if the MFA started for a given code is not closed you cannot start a new one I guess ? About all your questions I think this could be implemented by plugins to keep the module lighter: password history, signin count/date, device/IP management, etc. But all of this makes sense.

Thanks for the damn big work done, now it will be time for us to start migrating our code base and provide feedback, I am afraid it will not be before a couple of months or so...

[eddyystop]

@claustres

(1) " if the MFA started for a given code is not closed you cannot start a new one I guess ?" I think you could get stuck with this.

(2) I'm not sure what the advantage would be keeping the server code light. I am intending to break the plugins in 2 sets, doing a conditional require for the second set. This has more to do with policy than code size.

@lwhiteley @claustres

My issue at the moment is how to handle multiple passwords as mentioned by lwhiteley.

• An array of passwords, along with an array of password field names ('password', 'PIN', 'badge') seems most CS. However that adds a cognative load (including another 'weird' coersion for SQL) on everyone when I don't think this feature will be used much.
• Duplicating passwordChange, sendResetPassword, resetPasswordLong, resetPasswordShort for a second password is not DRY, but it wouldn't unnecessarily increase cognative load as new plugins.
• Do we stop at 2 passwords with 2 sets of commands, or do we support an array of passwords with the second set.

[claustres]

Can't see why we would stop at 2 passwords, if we start supporting more password yes go with an array.

I wonder if handling multiple passwords could be managed by a plugin that simply "overrides" the standard call eg https://github.com/feathers-plus/authentication-local-management/blob/master/src/password-change.js#L48 by looking in options as an array instead of a single value. In this case what we need is a way to send to plugins additional input data from the frontend like the name of the password field name or index to be changed. For instance in a reserved sanatized field like clientData, then there is no need to create a new set of commands.

What do you think ?

[eddyystop]

Overridding the standard plugin is still a new command, as it would need a different signature. Using the same plugin name could, I feel, just make it more confusing.

There is also the issue that the standard hashPassword() hook couldn't be used.

On the other hand, having a separate plugin, we now have 'password' plus an array of extra passwords.

[claustres]

The hashPassword() takes as an option the field to be used so combined with appropriate iff on the additional input data from client indicating which field to be changed this can be easily setup I guess or do I miss something ?

I don't think same name will be confusing as long as the plugin managing password arrays also calls the standard plugin for single password so that the old behavior is kept. To avoid the cognitive load the plugin should be optional so that the single mode is still the default one.

[eddyystop]

You would need a hashPassword for each password, or write a custom one to loop through each one.

Your second suggestion makes sense. But I still feel drawn to the idea of separating out the "extra" passwords. I feel it's easier on people to explain a new addon feature than to undo a previous one. Lemme think on it.

[eddyystop]

@claustres

(1) The following make it more awkward to have a multi-password plugin which overrides the single-password one.

• The identifyChange call requires the password value as a parm.
• The verifySignupLong and verifySignupShort calls require the initial password value for an invited user.

(2) Let's look at this from a completely different direction. What do we really want when we say we want additional passwords?

• The additional passwords have to be hashPassword() on create or patch. But the dev does this with hooks on usersService, so its not something a-l-m has to do, nor know about.
• The ability to use the additional passwords in Feathers authentication. But its sufficient these password fields exist anywhere in the users record.
• We want to control changes to the additional passwords:
  • We don't want the client changing them. This can be controlled by the current preventChangesVerification hook, which protects some hardcoded fields, e.g. isVerified, and the fields in the identifyUserProps option.
  • If the user forgets an additional password, we want the change to be made only the user verifies the change via email or SMS. But the identityChange command already does this for fields whose names are in identifyUserProps.
  • If the user is signed-in and wants to change an additional password, we want the server to make the change (thus bypassing preventChangesVerification) after the service confirms some information like the current password.

Am I missing something here or can additional passwords be implemented using

• The current identityChange command. That command stashes wanted changes to fields whose names are in identifyUserProps until an email or SMS are verified. See identityChange in https://www.lucidchart.com/invitations/accept/f4ebc4fd-3297-47e4-8568-eb13778c535c .
• A new additionalPasswordsChange command to immediately change the additional passwords. But do we really even need this? Can't we live with getting an email or SMS confirmation request from identityChange? Isn't that even safer?
• An indication of which identityUserProps are "passwords" so we can add changes to the password change history. So we can write a custom hashPasswords() hook which iterates through passwords?
• Of course we could have seperate identifyUserProps and identifyAdditionalPasswordProps arrays, concatenating their elements when required.

The "best" code is the code you don't have to write.

[eddyystop]

[In design. Awaiting comments.]

(d) Password strength hook

We might as well have a convenience hook which evaluates password strength. The Dropbox one ( https://github.com/dropbox/zxcvbn ) seems to be recommended. It uses an algorithmic approach rather than "at least ? chars with at least ? digits and ? capital letters". Its English centric unfortunately.

The hook's signature would be minimumPasswordStrength(level) where level is from 0, too guessable, to 4, very unguessable.