Distinguishing between identityChange and initial verification in notifier

[amxmln]

Hi,

is it possible to distinguish between the verification of an identityChange and the initial verification in verifySignup in the notifier?

That would be good for sending different emails depending on what happened.

I’ve been trying to achieve that for a while now, but nothing I tried (hooks, inspecting the data arriving in the notifier) worked, but I could’ve very well missed something.

Cheers and keep up the great work!

[eddyystop]

Can't you differentiate by having different values in the notifier options?

[amxmln]

Thanks for the quick reply.

I thought that might be possible, because you suggested that in other issues as well, but I couldn't figure out how to pass something into the notifier options and where to pass it in from. Do you have any pointers?

[morenoh149]

maybe share your code and suggestions can be made for how exactly to change it?

[amxmln]

I’m using the code from the example linked in the README: https://hackernoon.com/setting-up-email-verification-in-feathersjs-ce764907e4f2

All I’ve added is an additional hook on the authManagement service that makes sure the User is logged in when trying to change identity or password as suggested in the docs.md of this repo.

From the source-code of this repository it looks like the notifierOptions are only passed during resendVerifySignup and sendResetPassword, so I’m not sure how I could use them to pass data to verifySignup even if I could figure out how and where to actually pass data into the notifierOptions object.

Sorry if I’m missing something really obvious here, this is my first time working with this module.

[eddyystop]

The notifier options param is not passed with all actions. The authentication-local-management rewrite has fixed this.

I'm not sure if there's any way to handle this within the repo itself. Could you post what you did for informatory purposes if you find another way to do it.

[amxmln]

Sure, I can do that if I find a way. Is there an ETA on when authentication-local-management will be ready for production?

[eddyystop]

I'd say its at least 5-6 weeks away, assuming the first production implementation does indeed start in 2 weeks.

[morenoh149]

@amxmln maybe you can help test the rewrite. Are you in the feathers slack?

[eddyystop]

The rewrite will start with a closed test.

[amxmln]

Thanks, I guess I’ll keep using fam for now and upgrade once alm is out.

@morenoh149 no, I’m not. However, I’m in the middle of a really busy period at uni right now, so I wouldn’t have time unfortunately, I’m sorry.

[demorose]

Do you have an ETA for alm? I need a way to add notifierOptions to identityChange too.

[claustres]

This issue being old we close it as we are trying to give a fresh look, please reopen if required and provide as much information as possible to reproduce it.

[jd1378]

@claustres this is still relevant, for example how can we send an email to the original email before identityChange to let them know that the email on their account has been changed ? both identityChange and verification use the same type for notifier

[claustres]

As far as I can see the notifier is called with a different type in the current version like identityChange or verifySignupLong so that you can take the appropriate actions depending on it. For instance when changing the identity you should have the new/previous email in user.verifyChanges.email/user.email . You can also probably create a specific hook that will call the notifier with your own custom type, for instance I have a hook to send an invitation email like this with a notifier handling the additional type:

export async function sendInvitationEmail (hook) {
  // Before because we need to send the clear password by email
  if (hook.type !== 'before') {
    throw new Error('The \'sendInvitationEmail\' hook should only be used as a \'before\' hook.')
  }

  const accountService = hook.app.getService('account')
  await accountService.options.notifier('sendInvitation', hook.data)
  return hook
}

[jd1378]

I mean after the identity change happens, which is done by verifying, both types are the same and it can't be done with a hook because notifier is called inside the verify here: https://github.com/feathersjs-ecosystem/feathers-authentication-management/blob/7b73637bb1b8204a368d11e3c9117a4a6e6f3d92/src/methods/verify-signup.ts#L117

and it passes the cleaned up user to the notifier, so it doesn't have the verifyChanges anymore

this can be easily solved by passing the original mail address to the notifier as option just a line before like so:

notifierOptions.originalEmail = user.email;
// or better ?
notifierOptions.originalUser = user;

which makes it easy to know if email has been changed

[claustres]

OK I see now, the usual workflow is to send an email when asking for identity change to the previous email then an email to confirm the changes on the new email after verification because as the user has asked to change it he should want to use the new email after the change.

What you propose could probably be done but will break backward compatibility, as it seems to me that the user will then somewhat receive the same information on both emails, not sure if it's worth it. Let me know the reason it will be relevent for your use case.

Thanks for feedback.

[jd1378]

Well why make it a breaking change when what I proposed does not? And it allows the dev to decide if he wants to send what to the old or new email

Also it's probably a good idea to document the identityChange flow, as the way I'm doing it is to send an email verification token to the new email because the way I see it is that a logged in user has already confirmed their current email once in past, and he must confirm their new email instead. Because how else we are gonna know their new email is theirs? Then send a change notification to their old email.

But If you mean we send a verification code first to the current email, then change when verified, and make user unverified and send another verification email to the new email, then this sounds like an unnecessary hassle for the user

Thank you for putting time on this

[jd1378]

And maybe for added security in case of a hijacked session, we can add an option to require current password from user, as we do in password change, in a way that doesn't break compatibility

[claustres]

Sorry you are right I was confused, as you said documenting more workflow should be better for correct understanding and remembrance :wink:

1) It can be backward compatible as I did not correctly understood the change you propose, maybe to make it more generic as verification is used in different use cases we can pass to the notifier options user.verifyChanges although not really designed for this. I wonder if there is any security hole doing so as a maintainer of the library took care of erasing verification properties prior to call the notifier :thinking: By the way I think this behaviour is new because I still use v4 and I have a code like this in my notifier:

const email = {
      from: xxx,
      // When changing email send to the new one so that it can be verified
      to: (type === 'identityChange' ? user.verifyChanges.email : user.email),
      ...
    }

2) Identity change actually requires the password verification so that we know the user with the current email is making the change, so no need to send him a notification on the previous email and as you say we need instead to send him a notification on the new email to ensure it can be verified.

Maybe @fratzinger or @dallinbjohnson can help with their views ?