feathersjs-ecosystem / feathers-authentication-management

Adds sign up verification, forgotten password reset, and other capabilities to local feathers-authentication
https://feathers-a-m.netlify.app/
MIT License
246 stars 98 forks source link

Notifier wont work with other opitions like 'identifyUserProps' #120

Closed guzz closed 5 years ago

guzz commented 5 years ago

Steps to reproduce

I am configuring authManagement in my app like this:

app.configure(authManagement({
    notifier: () => notifier(app),
    identifyUserProps: ['email', 'phone']
  }));

But notifier is not called when I make any actions from client like sendResetPwd.

If I configure it like this:

app.configure(authManagement(notifier(app)));

It will work, but them I cant use the phone as a identifying property to my users.

guzz commented 5 years ago

Ok I figured out what I was doing wrong:

My notifier was actualy returning an object I just changed it to export the function.

That was stupid.

gchokeen commented 4 years ago

Hi Guzz, I'm facing the same issue, can you elaborate the solution,

My notifier js looks similar to this

module.exports = function(app) {

  function getLink(type, hash) {
    const url = 'http://localhost:3030/' + type + '?token=' + hash
    return url
  }

  function sendEmail(email) {
    return app.service('mailer').create(email).then(function (result) {
      console.log('Sent email', result)
    }).catch(err => {
      console.log('Error sending email', err)
    })
  }

  return {
    notifier: function(type, user, notifierOptions) {
      let tokenLink
      let email
      switch (type) {
        case 'resendVerifySignup': //sending the user the verification email
          tokenLink = getLink('verify', user.verifyToken)
          email = {
             from: process.env.FROM_EMAIL,
             to: user.email,
             subject: 'Verify Signup',
             html: tokenLink
          }
          return sendEmail(email)
          break

        case 'verifySignup': // confirming verification
          tokenLink = getLink('verify', user.verifyToken)
          email = {
             from: process.env.FROM_EMAIL,
             to: user.email,
             subject: 'Confirm Signup',
             html: 'Thanks for verifying your email'
          }
          return sendEmail(email)
          break

        case 'sendResetPwd':
          tokenLink = getLink('reset', user.resetToken)
          email = {}
          return sendEmail(email)
          break

        case 'resetPwd':
          tokenLink = getLink('reset', user.resetToken)
          email = {}
          return sendEmail(email)
          break

        case 'passwordChange':
          email = {}
          return sendEmail(email)
          break

        case 'identityChange':
          tokenLink = getLink('verifyChanges', user.verifyToken)
          email = {}
          return sendEmail(email)
          break

        default:
          break
      }
    }
  }
}

Config look like this


  const options = {
    identifyUserProps,
    shortTokenLen,
    shortTokenDigits,
    notifier: () => notifier(app),

  };

  app.configure(authManagement(options));

Please can you tell me how to fix it?

janzheng commented 4 years ago

haha I did the same thing (probably used the same tutorial!)

@gchokeen this is probably too late, but you can configure authManagement like this, if your notifier is an object:

const authManagement = require('feathers-authentication-management');
...
app.configure(authManagement({
    delay: 1000 * 60 * 60 * 24 * 30, // 30 days
    // notifier: notifier(app), // old way
    notifier: accountService(app).notifier
}))
user-2015-r commented 3 years ago

@janzheng Could you please explain? I still have this issue.

chrisbag commented 3 years ago

Hello, I am experiencing the same issue as described @guzz. I am unable to modify options.identifyUserProps as well as use notifier (I am using feathers with Typescript)

The following works but I cannot change default identifyUserProps to use username instead of email. authmanagement.services.ts

export default function(app: Application): void {
  app.configure(authManagement(notifier(app)))

  const service = app.service('auth-management')

  // eslint-disable-next-line @typescript-eslint/ban-ts-ignore
  // @ts-ignore // seems like ts thinks that service doesn't exist
  service.hooks(hooks)
}

Trying to change default options gives me the following error : TypeError: Cannot read property 'hooks' of undefined

authmanagement.services.ts

export default function(app: Application): void {
  const options = {
    identifyUserProps: ['id', 'username'],
    notifier: () => notifier(app),
  }
  app.configure(authManagement(options))

  const service = app.service('auth-management')

  // eslint-disable-next-line @typescript-eslint/ban-ts-ignore
  // @ts-ignore // seems like ts thinks that service doesn't exist
  service.hooks(hooks)
}

Would you have any idea of the solution ? Thanks a lot for your help.

claustres commented 3 years ago

The expected notifier option should be your notifier function directly, so something like:

const notifier = function(type, user, notifierOptions) { ... }

app.configure(authManagement({
  notifier,
  ...
}))

I guess you are using this tutorial and the notifier function is actually wrapped in a module exporting a closure in order to get access to the app. That's the reason of the way it is written app.configure(authManagement(notifier(app))). The call notifier(app) actually returns an object like this { notifier }. So take care the way you are exporting/importing your notifier function.

chrisbag commented 3 years ago

@claustres Thank you for your fast answer, this is exactly what happened. Problem solved :)