identityChange doesn't verify the user's password

Question

identityChange doesn't verify the user's password

Closed this issue 3 years ago · 17 comments

POST: http://localhost:3030/authManagement
{ "action": "identityChange", "value": { "user": {"email": "aaa@live.com" }, "password": "xxxxx", "changes": {"email": "bbb@live.com"} } }

Both HTTP and Feathers call have the same issue, whatever password I have put there, it will assume the password is correct and send the verification email directly, after I clicked the url in email, the id(email) will successfully get updated.

Answer 1 · 2022-01-07T15:02:38.000Z

Pretty strange, as far as I can see the CI is still running fine, there is a specific test on this. Could you provide more context/code sample ?

Answer 2 · 2022-01-07T17:45:51.000Z

Just checked my code, I'm using "feathers-authentication-management-ts": "^2.1.2", not sure if this is the reason.
@MohammedFaragallah

From my understanding, once I installed the feathers-authentication-management, I can use
this.feathers.service('authManagement').create(data); or regular HTTP method to call identityChange right?
{ "action": "identityChange", "value": { "user": {"email": "aaa@live.com" }, "password": "xxxxx", "changes": {"email": "bbb@live.com"} } }

Both of them have the same issue, I tried to hard code the password, then any password allows the method to send the email.

Answer 3 · 2022-01-07T17:47:39.000Z

If your point is to use typescript please wait for this PR to be merged #164. @fratzinger is currently generating a pre-release.

Answer 4 · 2022-01-07T17:52:37.000Z

thx a lot! that's great, we gonna have an official ts version!
any ideal when is this going to release? because I'm currently developing a project base on it.

Answer 5 · 2022-01-07T18:19:12.000Z

npm i feathers-authentication-management@4.0.0-pre.0

It's alive. It's an early version though! There shouldn't be breaking changes. Please make sure to check out https://feathers-a-m.netlify.app/ .
Please let me know how it goes.

Answer 6 · 2022-01-07T18:27:13.000Z

I just testet the identityChange action in an app using the latest version (v3.1.0) of f-a-m. And I cannot confirm the reported issue. The service throws the expected "Password is incorrect." error.

@Millenniunn, what version are you using?

Answer 7 · 2022-01-07T19:42:05.000Z

I just testet the identityChange action in an app using the latest version (v3.1.0) of f-a-m. And I cannot confirm the reported issue. The service throws the expected "Password is incorrect." error.

@Millenniunn, what version are you using?

@OnnoGabriel Ya, im using the 3rd party ts version. going to try the pre-release version that @fratzinger just mentioned.
Thanks all!

npm i feathers-authentication-management@4.0.0-pre.0
It's alive. It's an early version though! There shouldn't be breaking changes. Please make sure to check out https://feathers-a-m.netlify.app/ .
Please let me know how it goes.

@fratzinger Thank you! going to try this version!

Answer 8 · 2022-01-07T22:42:20.000Z

@fratzinger What's the proper way to export the notifier function with TS? I followed the .js instruction here but couldn't make it works.
https://feathers-a-m.netlify.app/getting-started.html#implementation-of-the-notifier-function

Another question, just curious, what's the point to use feathers-mailer? since I need to install nodemailer or nodemailer-smtp-transport for the transport anyway, why don't I just use the nodemailer to do all the jobs?

Answer 9 · 2022-01-07T22:56:19.000Z

Thanks for the good explanation and screenshots! Highly appreciated!
There where a few errors in the guide. I fixed it in 8473dbe
Please look closely at the changes and follow the changes.
Last thing I see is the difference between type string and NotificationType in:
(type: NotificationType, user: User, notifierOptions?: any). It's in your first screenshot.

Answer 10 · 2022-01-07T23:06:51.000Z

feathers-mailer vs. nodemailer. It's just a convenience level. I like to have all things in app at my fingertips. Wether it is app.service(...) or app.get(...). There where times I thought:

Nah, I don't need this elsewhere in the app. Just a quick hack here

and weeks later I saw me repairing the hack and moving it to a separate service with disallow('external')

Answer 11 · 2022-01-07T23:20:04.000Z

@fratzinger Thank you for the quick response!
it does solved the error on user.hooks.ts (image 3 from my last post), but still getting error on auth-management.service.ts page.

Also two questions,

is TS: import notifier from './notifier' same as JS: const notifier = require("./notifier"); ?
1. is TS: export default function (app: Application) same as JS: module.exports = ?

not sure any of these may caused the issue.

Answer 12 · 2022-01-07T23:47:01.000Z

@fratzinger I also just tried to follow the example in your branch, also got the error here

Answer 13 · 2022-01-08T00:04:13.000Z

feathers-mailer vs. nodemailer. It's just a convenience level. I like to have all things in app at my fingertips. Wether it is app.service(...) or app.get(...). There where times I thought:

Nah, I don't need this elsewhere in the app. Just a quick hack here

and weeks later I saw me repairing the hack and moving it to a separate service with disallow('external')

Isn't this same thing? I can also just use this app.service("mailer") method to send emails with nodemailer
const email: Service<SendMailOptions> = app.service("mailer");
email.create(payload) .then(() => console.log("New ticket email sent successfully")) .catch(console.error)

Answer 14 · 2022-01-08T05:08:59.000Z

@fratzinger I have finally figured it out..
I changed export default function (app: Application) to module.exports = (app: Application) => in notifier.ts
and import notifier from './notifier' to const notifier = require("./notifier"); in auth-management.service.ts

But Im still curious why... isn't module.exports a JS method? what should be the proper way to write it? I have tried all those but didn't work.

One more thing... I didn't realize it should be addVerification("auth-management") in the users.hook, cost me some time to debug.
This is weird since the regular JS branch doesn't need the "auth-management" and your removeVerification() also doesn't need it...

Answer 15 · 2022-01-08T07:45:40.000Z

That's weird. But it has nothing todo with feathers-authentication-management, I think. Do you have other export default code in your code base that is working?

Please try the following:

// notifier.ts
export const notifier = (app: Application) => {

// auth-management.service.ts
import { notifier } from './notifier'

Answer 16 · 2022-01-08T07:50:47.000Z

About "auth-management" in addVerification:
In v3 the recommended/default path of f-a-m was: /authManagement. addVerification uses this as default.

I wanted to change the camelCase to kebab-case because this feels more natural. But I don't want to break the migration from v3 to v4. New people coming to f-a-m just copy&paste the code from the docs and don't realize this.

Answer 17 · 2022-01-08T17:19:14.000Z

That's weird. But it has nothing todo with feathers-authentication-management, I think. Do you have other export default code in your code base that is working?

Please try the following:
// notifier.ts
export const notifier = (app: Application) => {
// auth-management.service.ts
import { notifier } from './notifier'

I have tried that before, not working as well.

About "auth-management" in addVerification: In v3 the recommended/default path of f-a-m was: /authManagement. addVerification uses this as default.

I wanted to change the camelCase to kebab-case because this feels more natural. But I don't want to break the migration from v3 to v4. New people coming to f-a-m just copy&paste the code from the docs and don't realize this.

Thank you for the explanation!

And the identityChange issue is solved, thank you all. You may want to close the issue ticket anytime.