ghostfolio / ghostfolio

Open Source Wealth Management Software. Angular + NestJS + Prisma + Nx + TypeScript 🤍
https://Ghostfol.io
GNU Affero General Public License v3.0
3.76k stars 354 forks source link

[BUG] Exception when disabling unconfigured Biometric Authentication #3453

Closed dittodhole closed 22 hours ago

dittodhole commented 2 weeks ago

Bug Description

Just wanted to try out Biometric Authentication, did not configure anything, and then wanted to disable it again. Still experiencing #3439

To Reproduce

  1. Open ghostfolio http://ghostfolio:3333 and login as admin
  2. Goto "My Ghostfolio" at /en/account
  3. Enable "Biometric Authentication"
  4. Disable "Biometric Authentication"
  5. Confirm "Do you really want to remove this sign in method?"
  6. Error "Oops! Something went wrong. Please try again later."

https://github.com/ghostfolio/ghostfolio/assets/429216/06dec3b9-2cb9-4b33-b2aa-cc7f801ae5b7

Expected behavior

No failure

Logs

[Nest] 115  - 06/02/2024, 2:55:44 PM   ERROR [ExceptionsHandler] 
Invalid `prisma.authDevice.delete()` invocation:
An operation failed because it depends on one or more records that were required but not found. Record to delete does not exist.
PrismaClientKnownRequestError: 
Invalid `prisma.authDevice.delete()` invocation:
An operation failed because it depends on one or more records that were required but not found. Record to delete does not exist.
    at In.handleRequestError (/ghostfolio/apps/api/node_modules/@prisma/client/runtime/library.js:122:6877)
    at In.handleAndLogRequestError (/ghostfolio/apps/api/node_modules/@prisma/client/runtime/library.js:122:6211)
    at In.request (/ghostfolio/apps/api/node_modules/@prisma/client/runtime/library.js:122:5919)
    at async l (/ghostfolio/apps/api/node_modules/@prisma/client/runtime/library.js:127:11167)
    at async AuthDeviceController.deleteAuthDevice (/ghostfolio/apps/api/main.js:1:522555)
    at async /ghostfolio/apps/api/node_modules/@nestjs/core/router/router-execution-context.js:46:28
    at async /ghostfolio/apps/api/node_modules/@nestjs/core/router/router-proxy.js:9:17

Environment

juanabascal commented 3 days ago

The problem here is that when the set up of the biometric authentication fails (code pointer) the toggle remains checked. When you click the toggle again, it tries to deactivate biometric authentication but it's not found. An unhandled error is raised and you see the toast with the message "Oops! Something went wrong. Please try again later".

The issue goes away after refreshing the page because the set up never reached the db.

I will raise a PR to do two things in case the biometric authentication set up is not completed.

  1. It till set the toggle back to "not checked" status
  2. Show a toast to the user mentioning there was an error

@dtslvr do you see any issue with the approach? How can I set up i18n of the string I need to use?

dtslvr commented 3 days ago

I will raise a PR to do two things in case the biometric authentication set up is not completed.

That would be great, @juanabascal! Do you have an idea why the registration may fail in the first place?

How can I set up i18n of the string I need to use?

You can set up the snackbar like this: import-activities-dialog.component.ts The english string is just fine, you don't have to worry about localization.

juanabascal commented 3 days ago

Do you have an idea why the registration may fail in the first place?

On my laptop (Macbook 2019) I made it fail by cancelling the browser's dialog to set up the biometric authentication.

After taking a second look to the issue description video I noticed that such dialog doesn't even appear on the screen. I'll dig deeper into this.

@dittodhole does you system have any biometric authentication hardware?

juanabascal commented 3 days ago

@dittodhole my guess is that the browser you are using doesn't support Authn. However, we could have more visibility on the issue if you log the actual error you're getting.

Just change this code with the following. The error message will be printed in the browser's console.

catchError((error: Error) => {
  console.log(error);
  this.update();

  return EMPTY;
})