jamie-mh / AuthenticatorPro

📱 Two-Factor Authentication (2FA) client for Android + Wear OS
https://authenticatorpro.jmh.me
GNU General Public License v3.0
2.91k stars 195 forks source link

Confusing UX for importing #1165

Open Nowaker opened 1 month ago

Nowaker commented 1 month ago

Original title: "Password is incorrect" when AuthPro generates a "corrupted" export file

Describe the bug

When two entries with the same issuer and username but a different secret exist, AuthPro will generate a JSON backup file that it will refuse to import claiming the password is incorrect.

Related to https://github.com/jamie-mh/AuthenticatorPro/issues/119

To Reproduce Steps to reproduce the behavior:

  1. Get AuthPro to a state where it has two secrets for the same account.
  2. Export.
  3. Import.
  4. Error.

Expected behavior

  1. Auth Pro generates an export that AuthPro can import (crazy idea, right? 😆)
    • If it's a valid state in my current AuthPro, it should be a valid state in my new AuthPro. So just import it as-is.
  2. Fix non-factual and confusing error message "Password is incorrect" when in reality, something else is an issue.
    • If the export is password-protected, it should not say "Password is incorrect" since there is no password.
    • If the export is password-protected, and the supplied password is correct, it should not say "Password is incorrect" since it's fine.
    • Errors related to "corruption" / failure to import (on AuthPro level) should be messaged in a different way compared to decryption errors.
  3. Validate an export file after it's generated.
    • The same function that validates if an export is "corrupted" during import should be proactively run after an export completes to validate the format, and inform the user that they may encounter a problem during the import due to an unknown bug.

App Version: 1.25.2

Additional context

JSON file had these two entries. EMAIL and ABC and DEF are snipped.

{
  "Authenticators": [
    {
      "Type": 2,
      "Icon": "stripe",
      "Issuer": "Stripe",
      "Username": "EMAIL",
      "Secret": "ABC",
      "Pin": null,
      "Algorithm": 0,
      "Digits": 6,
      "Period": 30,
      "Counter": 0,
      "CopyCount": 0,
      "Ranking": 19
    },
    {
      "Type": 2,
      "Icon": "stripe",
      "Issuer": "Stripe",
      "Username": "EMAIL",
      "Secret": "DEF",
      "Pin": null,
      "Algorithm": 0,
      "Digits": 6,
      "Period": 30,
      "Counter": 0,
      "CopyCount": 0,
      "Ranking": 20
    }
  ]
}
Nowaker commented 1 month ago

Oh shit...

I was selecting "Import from other apps", and then choosing "Authenticator Plus".

The UX is to blame.

"Import from other apps" wasn't the first place where I'd look to import... After all, it's called "Import from other apps", so I was clicking around, and around, and couldn't find the import UI. So I clicked "Import from other apps", and there it was Authenticator Pro, or so I thought! I even joked about it to my wife.

The core UX problem is the import feature present under the [ + ] button. As a long-time user of AuthPro, I've used this button tens of times - always for one purpose - adding a new QR codes to the application. It's not a place where I'd look for import from backup / other apps feature. Especially when the home screen says "Nothing here", gives a "Getting started guide" and "Import from other apps".

To avoid confusion, the second option on an empty screen should be "Import from Authenticator Pro backup", and the third "Import from other apps". Or, rename "Import from other apps" to "Import from other apps or backup", with Authenticator Pro being listed aside other third party apps on the list.

The original report partially stands: error handling and messaging needs to be improved. But the "duplicate" entry wasn't a problem - I read about duplicates being at issue previously, so I assumed that was the case.

Red6785 commented 1 month ago

I agree, the UX for importing backups is more than a bit confusing. I think for ease of organization/understanding, you should rename your issue to reflect the actual issue.

d3vnate commented 1 week ago

Ugh. Yes, annoying for sure. Thanks to this post, helped alleviate a fair amount of frustration.