-
Notifications
You must be signed in to change notification settings - Fork 2
Discussing enhancements and changes for V3 rewrite #1
Comments
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. |
@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. |
Move this here from feathers-authentication-management feathersjs-ecosystem/feathers-authentication-management#99 |
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
Breaking changes
Enhancements
Documentation
|
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 ^^^ |
Customization capabilities now seem great (hashing, service calls, etc.). Async/Await is really great as well. I wonder what you mean by 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 ! |
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); |
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 |
If you can configure on a per operation basis which one requires auth or not by overriding the options then it is great. |
options.authManagementHooks allows you to do that. Basically pass a modified version of the above hook. |
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. |
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. |
Modifications made to previously documented changes
Breaking changes
Enhancements
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 |
"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." |
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. |
which articles on Medium are you referring to ? |
The ones mentioed here #8 |
Candidate new features: (0) Consider a-l-m making internal calls using (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.
Misc
(*) Look at
(*) Look at (***) Look at |
@claustres @beeplin @daffl @marshall @j2L4e your comments on the above would be appreciated. |
The features feathers-authentication-management provided, reimplemented here in a-l-m, were confusing enough to people. Articles were written (#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. |
@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! |
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:
|
@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.
|
@eddyystop Are you looking at adding in the possibility to manage multiple password/hashed fields? Use case: an account system that has both a 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 Relevant actions:
Let me know if that makes sense or can be supported. |
@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. |
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 |
@xendarboh That looks very interesting. |
@xendarboh @eddyystop Hopefully this info adds some more info to help with implementing emails/notifications. 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... |
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. |
Scaffolding to implement plugin support added as @feathers-plus/plugin-scaffolding.
Initial plugin support added to a-l-m.
feathers-plus/commons has been introduced as a respository of cross-cutting utilities.
// 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. |
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()); |
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. |
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 |
Finished conversion to plugins.It took longer than anticipated. https://blog.feathersjs.com/use-plugins-to-publish-incredibly-flexible-code-a55e8faf27a8 |
I believe we are finished with converting feathers-authentication-management and can start implementing new features. COMMENTS ARE INVITED. |
[Completed. Awaiting comments.] (a) Send an invitation for a user to sign-up.
|
@claustres ^^^ |
[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.
[In design. Awaiting comments.] Some new fields may be introduced to support some of these mfa usages:
??? Is it sensible for a-l-m to keep the following as it is authentication itself that would set them.???
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:
|
Reserved. |
[In design. Awaiting comments.] (c) Support multiple password-like fields such as password & pin#, as suggested by @lwhiteley.
|
Reserved. |
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... |
(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 My issue at the moment is how to handle multiple passwords as mentioned by lwhiteley.
|
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 What do you think ? |
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. |
The 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. |
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. |
(1) The following make it more awkward to have a multi-password plugin which overrides the single-password one.
(2) Let's look at this from a completely different direction. What do we really want when we say we want additional passwords?
Am I missing something here or can additional passwords be implemented using
The "best" code is the code you don't have to write. |
[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. |
Reserved. |
[In design. Awaiting comments.] (e) List of previous passwords, so they are not reused. Questions:
Related comments:
|
Reserved. |
About multiple passwords I like your idea of somewhat considering we have a "master" password (the default field or another one we have chosen), then others are simply additional info attached to the identity we can only change giving the master password. This will avoid a lot of code and IMHO covers the use cases. Indeed having multiple "master" password does not make sense, people doesn't want to randomly use one or the other "just for fun". Si if they need additional "secrets" this is to be used in others things than the app itself (eg hardware lock) or to strengthen auth using both (it seems to me however already covered by MFA). About password strength we have avoided using https://github.com/dropbox/zxcvbn in our product due to its english centric approach. Moreover, it seems to be a complement to a password policy, you can have a policy and still want to check strength, you can also enforce some password properties whatever the strength. I think you will have multiple use cases and integrating such opiniated lib is not a good idea IMHO. |
I wanted to chime in before much more time passes. I'm excited to see this development, much thanks! I still need to digest details of the a-l-m article, plugin implementation, and recent comments here, so I could be a bit fuzzy and out of context till then (and likely a while longer till I get there). I'm working with an in-progress feathers-next project using f-a-m that is ready to convert to a-l-m. The project is intentionally underdeveloped and weird in places considering where it came from (a fork), where the concept is going (a larger established private project), and what else I thought it could be: a minimal proof-of-concept playground for local user auth basics using feathers + a-l-m + next.js + redux. With clean commits, it may be an example reference for One thing I encountered with my initial implementation with f-a-m that I'm wondering if there's a recommended solution with a-l-m, is how to independently verify user communication channels to avoid sending future communications to unverified channels. For example, if a user's email was verified but their SMS was not, don't send a password reset (or any communications) by SMS until/unless SMS has been verified – or vice versa. Maybe this is already covered? MFA is essentially self-verifying since it's typically verified from user input in the process of enabling it for an account (to ensure a user has it set up properly on their end). There's |
Neither f-a-m nor a-l-m have the concept of authenticating a communication channel, let alone multiple ones. I see the use for these concepts but introducting it would take a significant effort which I'm frankly not willing to undertake under the (increasingly broken) OS model. As an aside, neither f-a-m nor a-l-m are email centric. They are just as happy using the phone#. However all articles and discussions have been email-centric which leads to this impression. |
Transfer from feathersjs-ecosystem/feathers-authentication-management#114
The text was updated successfully, but these errors were encountered: