Better login error messaging

[mark-prins]

What went wrong? 😲

I've had this one a few times when trying to login:

Looking at the explanatory comment in the code:

  VerifyPasswordError::InvalidCredentialsBackend(_) => {
      LoginError::InternalError("Failed to read credentials".to_string())
...

    /// Invalid account data on the backend
    InvalidCredentialsBackend(bcrypt::BcryptError),

does not explain the problem.

Expected behaviour 🤔

I would prefer a message which told me something in English. A couple of thoughts:

1. The system shouldn't get into this state. If the problem is a duplicate user (see below) then how did it create a second user with the same login? If the issue is a blank password (also below) how is this possible?
2. The message should tell me how to get out of this mess. I want to login and it isn't helping me resolve the problem. Perhaps The user account has invalid credentials stored in the database. Please try logging in as another user, or contact support. This should show instead of The server experienced a problem and not require the tooltip

How to Reproduce 🔨

Looking at my database, I have this:

and from memory, having a duplicate user record has been a problem before.

These are the steps I took:

1. Have a site configured in mSupply
2. Sync this to oms and then login to the site
3. Delete the oms database
4. Recreate the database and sync, try to login

Your environment 🌱

• Open mSupply Version:
• Legacy mSupply Central Server Version:
• Platform:
• Database type:

[mark-prins]

I can confirm that doing this

delete from user_store_join where user_id = '21F40ED44E444C1291DA7133AF906431';
delete from user_permission where user_id = '21F40ED44E444C1291DA7133AF906431';
delete from user_account where id = '21F40ED44E444C1291DA7133AF906431';

in the above database resolves the problem

[andreievg]

At some point i wanted every error to be structured error and very concrete (named to explain exactly what it is), at some point Clemens mentioned it's too much work mapping it all, and at this stage I thought it was reasonable to agree that some errors are ok as standards errors with less description as they are not likely to happen and if they do there is something is badly wrong.

There is an error! being logged btw.

Yes agree if it happened in the wild, the message doesn't help the user think of next action.

Maybe all InternalError should have something like contact support ?

Btw i can replicate this consistently, if you re-initialise. Then quit mSupply central server so it's not accessible anymore.

Then try to login (users sync now but without passwords).

I think as part of solving this issue we should also not search for users with empty passwords when checking passwords against user

[mark-prins]

Good investigating - thanks Andrei! Would be good to investigate the bug scenario and prevent the empty-password users from being created and/or being returned in the login query

Maybe all InternalError should have something like contact support ?

yeah, good idea. at least that login one - it's a bit of a blocker

[roxy-dao]

I can't seem to reproduce at all? The user login upserts users on id and translator also upserts on id... I'm wondering how the id changed? 👀

Steps I did:

• Initialise
• Turn off central
• Try login -> error orz
• Turn central back on
• Login -> user gets upserted...

[mark-prins]

I had this yesterday - was restoring a database and testing locally.. but didn't track the steps! was in a rush sorry, so don't know exactly what I did to recreate.

[roxy-dao]

Ties in with #2654 and all the other login error issues linked in 2654 😄