[EC-836] Trying to confirm 2018 user account to organization returns 404

[eliykat]

Type of change

- [x] Bug fix
- [ ] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

Fix this bug:

User is trying to confirm an account created in 2018 to their organization but receive no visible errors, just inactivity after clicking confirm. The dev tools show a 404 error

Analysis:

The 404 is occurring because the first step of the key exchange is to fetch the user’s public key, which doesn’t exist.

I was able to reproduce this locally by creating a new user and then nulling out the User.PrivateKey and User.PublicKey fields in the db.

We have migration logic in logIn.strategy.ts for old users who don't have RSA keys (specifically createKeyPairForOldAccount), but it fails because the user’s master key isn’t set yet. This means that we can't decrypt the EncKey which we need to encrypt the new private key once it's generated.

Code changes

• libs/common/src/misc/logInStrategies/logIn.strategy.ts - the user's key is set in onSuccessfulLogin by each subclass/strategy. It turns out that's the only thing this method is used for, so rename it to setUserKey, make it abstract (to force subclasses to implement it), and move it to immediately before the migration logic instead of immediately after it. This should be OK because simply setting the user's key does not require the encKey or keypair that is set in the code in between the old & new locations.

• all other files - update method name in subclasses

Screenshots

Before you submit

• Please add unit tests where it makes sense to do so (encouraged but not required)
• If this change requires a documentation update - notify the documentation team
• If this change has particular deployment requirements - notify the DevOps team

[eliykat]

Good point about the test case - there's actually already a test case for this, it was just giving a false negative.

To cover this, we need to assert the call order in the tests, which we can't do with the old SubstituteJS library. I updated all these tests to use jest-mock-extended instead (there's an ADR for this, it's a tech debt item), then added an assertion for the call order.