questions on docs

[beeplin]

question 1: In the docs I read:

Isn't populateUser has been deprecated?

[beeplin]

question 2:

I saw both preferredCommunication and preferredComm, and even prefersTransport. Which is final?

[beeplin]

question 3:

I see an example like this:

Two concerns here:

1. this is authv0.7, not auth v1.0

2. In fact, fo an unverified user, we at first log him in, then log him out. Since this is javascript running at the front end, a unverified user can always manually log himself in via browser's console (simply run app.authenticate({ type: 'local', email, password }) as long as he could find how to access the app), bypassing the !user.verified check. I wonder if there is any decent way to avoid this.

I like the v0.x authenticate wrapper because it hides the details of first logging in and then logging out, making it a little bit harder for users to hack and bypass the verification. But it seems this wrapper is deprecated in auth v1.0?

[beeplin]

q 4:

param name unmatched.

And, I haven't checked the source code and the docs are unclear on how to use ownId and ifErrMsg.

[beeplin]

q 5:

wrong comment line.

[eddyystop]

Thanks for reviewing things. Much appreciated.

feathers-authentication-management is not dependent on a particular version of authentication/permissions other than in examples in its docs.

V1 authentication and permissions has been a large undertaking that ekryski is still wrapping up. I did not use the v1 API in the docs as parts were in flux or outstanding. I decided to either wait until v1 was wrapped up or, even better, have ekryski update the docs for feathers-authentication-management.

So the examples are v0.7 and I have yet to understand how to convert them to V1.

I saw both preferredCommunication and preferredComm, and even prefersTransport. Which is final?

The notifier handler options.notifier would need to reference this field to decide how to notify the user. The rest of the package doesn't care what it is called.

However we should standardize on a name and make the docs consistent. [Edited] I've changed the names to preferredComm.

a unverified user can always manually log himself in via browser's console. I wonder if there is any decent way to avoid this.

Agreed on the issue. I was thinking authenticate, if configured to do so, would check user.isVerified if it existed. I want v1 to stablize before bringing this up with ekryski.

I like the v0.x authenticate ... But it seems this wrapper is deprecated in auth v1.0?

I frankly don't know. Its best to ask ekrsyki.

And, I haven't checked the source code and the docs are unclear on how to use ownId and ifErrMsg.

ownId allows us to ignore "ourself" when checking if a field value is unique amoung users. So if "we" are the only user having that field value, the field value is considered unique.

[Edited] noErrMsg decides if error.BadRequest('Values already taken.', { errors }) or error.BadRequest(null, { errors }) is thrown. I found it convenient to have the option when doing an async validation on the client.

the docs are unclear on how to use ownId and ifErrMsg

[Edited} Updated.

wrong comment line.

Thanks for the catch.

Let me know if I missed answering anything. My apologies for not being able to be more helpful on the v1 stuff.

[beeplin]

Thank you @eddyystop for the quick reply. ;)

As for the naming of APIs, I do agree that since this is now an official package and will go wide public, we need to refine some names. Here are my thoughts:

In general, we now have some long camelCase names, which I think is fine as long as their meanings are clear. Being long is not the problem since we do have some complex logic and contexts that can only be made clear via long names. Long and clear, this is my criteria in the following suggestions.

• Some of APIs use shorten names while others do not. For example, sometimes we have 'pwd' and sometimes 'password'. I suggest all names should not be shorten, say, Len -> Length, 'Pwd->Password`.
  

• The concepts identity or identifier and transport or communicaton are new (I brought the rough concepts up lately), and I feel we lack clear names/terms for them. I suggest doing this: an identity is a field in the users table in the database which is unique and can be used to indicate which user we are dealing with, eg, 'username' or 'email' or 'facebook'; an transport is a field in the users table in the database. which is used to communicate with the user. A transport must be an identity, but an identity might not be a transport, like username. I will use this terminology below.

For the service options:

• Following the terminology above, I think identityUserProps is not very clear and doesn't seem proper English (English is not my native language so I am not very sure about this). I guess userIdentities would be better. We indicate an array of identities here. Then developers must indicate a transport which must in this array, either via writing to the transport field in the users table of db, or via dynamically calling a create method or a wrapper function with a transport option.

• shortTokenDigits -> shotTokenIsDigital, might be better for a boolean property.

• delay -> verificationDuration (I don't think delay is the best word for this. Any better word?)

• resetDelay -> resetDuration

For database field names:

• Names with verifyxxxxx might have grammatical problems: verify is a verb, so verifyToken (or verifyExpires, etc.) might be misunderstand as an action, a function, since it's actually a verb followed by an noun (verb's object). I guess verificationToken would be better. (I know it's longer.. is there any similar but shorter word for that?) (reset is both a verb and a noun, so resetToken is OK.)

• verifyChanges seems not very accurate. identityChanges might be better, following the identity-transport terminology above.

• verifyExpires and resetExpire could be changed to verificationExpiresAt and resetExpiresAt, following feathers' norm of date naming (like createdAt and updatedAt).

• preferredComm -> transport (which I think is much plainer and clearer to tell developers what this property is for. This is the default transport to communicate with the user, but it can be dynamically overridden if calling a method/wrapper with a 'transport' option. I don't think 'prefered' is necessary here.)

For the create method's actions and wrapper functions:

• I prefer checkUniqueness which is better English than checkUnique.
• resendVerifySignup -> resendTokenToVerifySignup
• verifySignupLong -> verifySignupWithLongToken
• verifySignupShort -> verifySignupWithShortToken
• sendResetPwd -> sendTokenToResetPassword
• resetPwdLong -> resetPasswordWithLongToken
• resetPwdShort -> resetPasswordWithShortToken
• passwordChange -> changePassword (all actions should start with a verb)
• identityChange -> changeIdentities (ditto)

(the names seem so long... but when I used the old APIs I was always puzzled due to the unclear names. I think these long names could help developers to precisely understand what these actions or wrapper functions do.)

[beeplin]

Some API reconsiderations:

For notifierOptions, in the docs:

notifier: function(type, user, notifierOptions) returns a Promise.
...
notifierOptions: notifierOptions option from resendVerifySignup and sendResetPwd 

// resend sign up verification notification
authManagement.create({ action: 'resendVerifySignup',
  value: identifyUser, // {email}, {token: verifyToken}
  notifierOptions: {}, // options passed to options.notifier, e.g. {prefersTransport: 'cellphone'}
})

I wonder why we have to make this a three-layer object ({value: {notifierOptions: {prefersTransport: 'cellphone'}}})? Will there be any other options besides prefersTransport (I would like to change it to transport as stated above) in the future? Even if we will have new options in the future, I think it is better to make the API like this:

// resend sign up verification notification
authManagement.create({ 
   action: 'resendTokenToVerifySignup',
   user: {email: email},
   transport: 'cellphone' // bring this to the top level
   someOtherProp: ... // put possible new options here at the top level too, making the param flat.
})

So we eliminate the name notifierOptions, and we pass all props of the above param to the notifier function:

notifier: function(options) returns a Promise.  
Here 'options' is an object including the following props:
action: ... (not changed)
user: the full user item minus password (*HERE we get 'user' prop from the param of
'resendTokenToVerifySignup'  or 'sendTokenToResetPassword' method call, then get the full 
user info from database (without password), and then replace the original 'user' prop with 
full user info, and then pass it into this notifier function. *)
...
transport: transport prop from resendTokenToVerifySignup and sendTokenToResetPassword
someOtherProps: ...

Here my idea is: If we make the param structure of action method calls simple and flat, then we only need to pass this original param to the notifier function as a whole. We don't need to touch other props in this whole param object before pass them to here notifier function, only need to populate the user prop with the user's full info.

[beeplin]

Following the above comment we might be able to flatten the structures of params of all the methods/wrappers like this:

// check props are unique in the users items
authManagement.create({ 
  action: 'checkUniqueness',
  user: ..., // e.g. {email, username}. Props with null or undefined are ignored. SIMPLY USE 'USER' HERE.
  ownId, // excludes your current user from the search
  ErrMsg, // if return an error.message if not unique. FLATTEN, REMOVE 'meta'
})

// resend sign up verification notification
authManagement.create({ 
  action: 'resendTokenToVerifySignup',
  user: ..., // {email}, {token: verifyToken} *DON"T UNDERSTAND WHY 'token' could also be used here?*
  transport: ..., // 'cellphone',
  otherProps: ...
})

// sign up or identityChange verification with long token
authManagement.create({ 
  action: 'verifySignupWithLongToken',
  token: ...
})

// sign up or identityChange verification with short token
authManagement.create({ 
  action: 'verifySignupWithShortToken',
  user: ..., // identify user, e.g. {email: 'a@a.com'}. See options.identityUserProps.
  token: ..., // compares to .verifyTokenShort
  }
})

// send forgotten password notification
authManagement.create({ 
  action: 'sendTokenToResetPassword',
  user: ..., // {email}, {token: verifyToken} SAME ABOVE, WHY token here?
  transport: ..., // 'email'
  otherProps: ...
})

// forgotten password verification with long token
authManagement.create({
  action: 'resetPassorwdWithLongToken',
  token: ..., // compares to .resetToken
  password: ..., // new password
})

// forgotten password verification with short token
authManagement.create({ 
  action: 'resetPasswordWithShortToken',
  user: ..., // identify user, e.g. {email: 'a@a.com'}. See options.identityUserProps.
  token: ..., // compares to .resetTokenShort
  password: ..., // new password
})

// change password
authManagement.create({ 
  action: 'changePassword',
  user: ..., // identify user, e.g. {email: 'a@a.com'}. See options.identityUserProps.
  oldPassword: ..., // old password for verification
  newPassword: ..., // new password
})

// change communications
authManagement.create({ 
  action: 'changeIdentities',{
  user: ..., // identify user, e.g. {email: 'a@a.com'}. See options.identityUserProps.
  password:..., // current password for verification
  changes: ..., // {email: 'a@a.com'} or {email: 'a@a.com', cellphone: '+1-800-555-1212'}
})

[beeplin]

As for the wrapper functions, I strongly suggest not using multiple params, but using one single option param and put all things in it as its props. See this:

// forgotten password verification with short token
authManagement.resetPwdShort(resetShortToken, identifyUser, password)

// change password
authManagement.passwordChange(oldPassword, password, identifyUser)

It's hard for developers to remember the order of multiple params: when identifyUser is before password and when after? Is the new password in front of the old or in opposite? That's awful.

It would be much nicer in this way (I guess since this is a new repo so we don't need to make the API backward compatible with the previous verifyReste reop?):

// check props are unique in the users items
authManagement.checkUniqueness({user, ownId, errMsg}) //RENAME TO errMsg

// resend sign up verification notification
authManagement.resendTokenTOVerifySignup({user, transport, ...otherProps})

// sign up or identityChange verification with long token
authManagement.verifySignupWithLongToken({token})

// sign up or identityChange verification with short token
authManagement.verifySignupWithShortToken({user, token})

// send forgotten password notification
authManagement.sendTokenToResetPassword({user, tranport, ...otherProps})

// forgotten password verification with long token
authManagement.resetPassorwdWithLongToken({token, password})

// forgotten password verification with short token
authManagement.resetPassorwdWithShortToken({user, token, password})

// change password
authManagement.changePassword({user, oldPassword, newPassword})

// change identity
authManagement.changeIdentities({user, password, changes})

// Authenticate user and log on if user is verified. SURELY CAN WORK IN auth v1.0
authManagement.authenticate({user, password})

[eddyystop]

Wow.

I made an effort to use clear names but that gets difficult when you are so close to the code. So I appreciate your more clear-sighted comments. I agree clarity is more important than the length of a name.

I know a few people have started using the package, so we have the issue of backwards compatibility to consider. We can alias some names but it seems too early in this version's lifecycle to start doing that.

I need to consider the pros and cons, once I have more coffee. :)

The concepts identity or identifier and transport or communicaton are new ... we lack clear names/terms for them.

Thank you.

I wonder why we have to make this a three-layer object ({value: {notifierOptions: {prefersTransport: 'cellphone'}}})?

The dev can include any params he wants in notifierOptions and use them in the notifier function as notifierOptions is passed to it as is. prefersTransport, for example, is just a suggestion for a field that is likely to be needed.

English is not my native language

LOL. You write better English that most people.

[eddyystop]

We've been told several times we need a better introduction to Feathers. The main point seems to be to get across that you can work with the DB on the client just as you would on the server, without any boilerplate.

We suspect we may be too close to the code to write for the person just starting to look at Feathers.

Do you anyone just starting to look at or use Feathers that could write notes on their experience? We could then correctany misunderstandings and flesh out the intro.

[beeplin]

The dev can include any params he wants in notifierOptions and use them in the notifier function as notifierOptions is passed to it as is. prefersTransport, for example, is just a suggestion for a field that is likely to be needed.

I mean we could flatten the structure of the params and meanwhile still allow the dev to do so. For example, as I understand, currently we have this API:

authManagement.create({
  action: 'resendVerifySignup',
  value: {email: email},
  notifierOptions: {prefersTransport: 'cellphone'}
})

When calling this method, the notifier function(type, user, notifierOptions) is called, with the params mapped like this:

action          -> type
value           -> (get the full user item from db) -> user
notifierOptions -> notifierOptions

While I suggest this:

authManagement.create({ 
  action: 'resendTokenToVerifySignup',
  user: ..., // {email}, {token: verifyToken} *DON"T UNDERSTAND WHY 'token' could also be used here?*
  transport: ..., // 'cellphone',
  otherProps: ...
})

Then the notifier function(options) is called with the params mapped like this:

action     -> options.type (or options.action, keeping the name the same like the following others)
user       -> (get the full user item from db according to it) -> options.user
transport  -> options.transport
otherProps -> options.otherProps

So basically we don't eliminate the notifierOptions, but make it the only param of the notifier function (renamed to simple options), making type and user part of it. By doing so we don't need to have this 3-layer nested params: `{value: {notifierOptions: {transport: 'email'}}}, but a beautiful and clearly mapped param structure.

[beeplin]

Do you anyone just starting to look at or use Feathers that could write notes on their experience? We could then correctany misunderstandings and flesh out the intro.

My understanding of Feathers at the very beginning is exactly like that: 'Wow, this is a db connector which wraps REST in nice APIs, and these APIs are nearly universal for all kinds of dbs! And I like the way to write hooks!'

After several months my understanding evolved, and I find Feathers useful not only at the backend, but also at the frontend. The key point is how to understand and make use of service in Feathers. At the beginning I only understood services as wrappers of database tables/collections. Then I found verifyReset service which uses service as purely a transmitter between frontend and backend. That's cool.

Then I figured out that a service could also work only at the frontend. That means Feathers can also be used as a frontend data model manager. I can define services at the frontend, write hooks for them, make them communicate with backend services if needed. (my 'feathers-cache' repo uses services in this way).

Currently I use feathers in this way:

frontend UI View: vuejs
|
| (use vue-rx & feathers-reactive to connect the two, rxjs observables in between as transmitter)
|
frontend data model: feathers services (for caching and filtering data from backend)
|
backend data model: feathers services
|
backend database: mongoDB

[eddyystop]

You make a great point about "frontend data model manager". I moved away from client work after doing the basics and before comprehending that. +1