[EC-836] Trying to confirm 2018 user account to organization returns 404 (#4214)

* Fix migration logic to create keypair for old account

* Rename onSuccessfulLogin to reflect usage

* Rewrite loginStrategy spec with jest-mock-ex

* Rewrite tests with jest-mock-extended

* Assert call order

* Fix linting

libs/common/spec/misc/logInStrategies/logIn.strategy.spec.ts

@@ -1,5 +1,4 @@
-// eslint-disable-next-line no-restricted-imports
+import { mock, MockProxy } from "jest-mock-extended";
-import { Arg, Substitute, SubstituteOf } from "@fluffy-spoon/substitute";
 
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { AppIdService } from "@bitwarden/common/abstractions/appId.service";
@@ -67,33 +66,34 @@ export function identityTokenResponseFactory() {
 }
 
 describe("LogInStrategy", () => {
-  let cryptoService: SubstituteOf<CryptoService>;
+  let cryptoService: MockProxy<CryptoService>;
-  let apiService: SubstituteOf<ApiService>;
+  let apiService: MockProxy<ApiService>;
-  let tokenService: SubstituteOf<TokenService>;
+  let tokenService: MockProxy<TokenService>;
-  let appIdService: SubstituteOf<AppIdService>;
+  let appIdService: MockProxy<AppIdService>;
-  let platformUtilsService: SubstituteOf<PlatformUtilsService>;
+  let platformUtilsService: MockProxy<PlatformUtilsService>;
-  let messagingService: SubstituteOf<MessagingService>;
+  let messagingService: MockProxy<MessagingService>;
-  let logService: SubstituteOf<LogService>;
+  let logService: MockProxy<LogService>;
-  let stateService: SubstituteOf<StateService>;
+  let stateService: MockProxy<StateService>;
-  let twoFactorService: SubstituteOf<TwoFactorService>;
+  let twoFactorService: MockProxy<TwoFactorService>;
-  let authService: SubstituteOf<AuthService>;
+  let authService: MockProxy<AuthService>;
 
   let passwordLogInStrategy: PasswordLogInStrategy;
   let credentials: PasswordLogInCredentials;
 
   beforeEach(async () => {
-    cryptoService = Substitute.for<CryptoService>();
+    cryptoService = mock<CryptoService>();
-    apiService = Substitute.for<ApiService>();
+    apiService = mock<ApiService>();
-    tokenService = Substitute.for<TokenService>();
+    tokenService = mock<TokenService>();
-    appIdService = Substitute.for<AppIdService>();
+    appIdService = mock<AppIdService>();
-    platformUtilsService = Substitute.for<PlatformUtilsService>();
+    platformUtilsService = mock<PlatformUtilsService>();
-    messagingService = Substitute.for<MessagingService>();
+    messagingService = mock<MessagingService>();
-    logService = Substitute.for<LogService>();
+    logService = mock<LogService>();
-    stateService = Substitute.for<StateService>();
+    stateService = mock<StateService>();
-    twoFactorService = Substitute.for<TwoFactorService>();
+    twoFactorService = mock<TwoFactorService>();
-    authService = Substitute.for<AuthService>();
+    authService = mock<AuthService>();
 
-    appIdService.getAppId().resolves(deviceId);
+    appIdService.getAppId.mockResolvedValue(deviceId);
+    tokenService.decodeToken.calledWith(accessToken).mockResolvedValue(decodedToken);
 
     // The base class is abstract so we test it via PasswordLogInStrategy
     passwordLogInStrategy = new PasswordLogInStrategy(
@@ -113,12 +113,11 @@ describe("LogInStrategy", () => {
 
   describe("base class", () => {
     it("sets the local environment after a successful login", async () => {
-      apiService.postIdentityToken(Arg.any()).resolves(identityTokenResponseFactory());
+      apiService.postIdentityToken.mockResolvedValue(identityTokenResponseFactory());
-      tokenService.decodeToken(accessToken).resolves(decodedToken);
 
       await passwordLogInStrategy.logIn(credentials);
 
-      stateService.received(1).addAccount(
+      expect(stateService.addAccount).toHaveBeenCalledWith(
         new Account({
           profile: {
             ...new AccountProfile(),
@@ -140,10 +139,9 @@ describe("LogInStrategy", () => {
           },
         })
       );
-      cryptoService.received(1).setEncKey(encKey);
+      expect(cryptoService.setEncKey).toHaveBeenCalledWith(encKey);
-      cryptoService.received(1).setEncPrivateKey(privateKey);
+      expect(cryptoService.setEncPrivateKey).toHaveBeenCalledWith(privateKey);
-
+      expect(messagingService.send).toHaveBeenCalledWith("loggedIn");
-      messagingService.received(1).send("loggedIn");
     });
 
     it("builds AuthResult", async () => {
@@ -151,16 +149,16 @@ describe("LogInStrategy", () => {
       tokenResponse.forcePasswordReset = true;
       tokenResponse.resetMasterPassword = true;
 
-      apiService.postIdentityToken(Arg.any()).resolves(tokenResponse);
+      apiService.postIdentityToken.mockResolvedValue(tokenResponse);
 
       const result = await passwordLogInStrategy.logIn(credentials);
 
-      const expected = new AuthResult();
+      expect(result).toEqual({
-      expected.forcePasswordReset = true;
+        forcePasswordReset: true,
-      expected.resetMasterPassword = true;
+        resetMasterPassword: true,
-      expected.twoFactorProviders = null;
+        twoFactorProviders: null,
-      expected.captchaSiteKey = "";
+        captchaSiteKey: "",
-      expect(result).toEqual(expected);
+      } as AuthResult);
     });
 
     it("rejects login if CAPTCHA is required", async () => {
@@ -171,12 +169,12 @@ describe("LogInStrategy", () => {
         HCaptcha_SiteKey: captchaSiteKey,
       });
 
-      apiService.postIdentityToken(Arg.any()).resolves(tokenResponse);
+      apiService.postIdentityToken.mockResolvedValue(tokenResponse);
 
       const result = await passwordLogInStrategy.logIn(credentials);
 
-      stateService.didNotReceive().addAccount(Arg.any());
+      expect(stateService.addAccount).not.toHaveBeenCalled();
-      messagingService.didNotReceive().send(Arg.any());
+      expect(messagingService.send).not.toHaveBeenCalled();
 
       const expected = new AuthResult();
       expected.captchaSiteKey = captchaSiteKey;
@@ -186,13 +184,20 @@ describe("LogInStrategy", () => {
     it("makes a new public and private key for an old account", async () => {
       const tokenResponse = identityTokenResponseFactory();
       tokenResponse.privateKey = null;
-      cryptoService.makeKeyPair(Arg.any()).resolves(["PUBLIC_KEY", new EncString("PRIVATE_KEY")]);
+      cryptoService.makeKeyPair.mockResolvedValue(["PUBLIC_KEY", new EncString("PRIVATE_KEY")]);
 
-      apiService.postIdentityToken(Arg.any()).resolves(tokenResponse);
+      apiService.postIdentityToken.mockResolvedValue(tokenResponse);
 
       await passwordLogInStrategy.logIn(credentials);
 
-      apiService.received(1).postAccountKeys(Arg.any());
+      // User key must be set before the new RSA keypair is generated, otherwise we can't decrypt the EncKey
+      expect(cryptoService.setKey).toHaveBeenCalled();
+      expect(cryptoService.makeKeyPair).toHaveBeenCalled();
+      expect(cryptoService.setKey.mock.invocationCallOrder[0]).toBeLessThan(
+        cryptoService.makeKeyPair.mock.invocationCallOrder[0]
+      );
+
+      expect(apiService.postAccountKeys).toHaveBeenCalled();
     });
   });
 
@@ -206,12 +211,12 @@ describe("LogInStrategy", () => {
         error_description: "Two factor required.",
       });
 
-      apiService.postIdentityToken(Arg.any()).resolves(tokenResponse);
+      apiService.postIdentityToken.mockResolvedValue(tokenResponse);
 
       const result = await passwordLogInStrategy.logIn(credentials);
 
-      stateService.didNotReceive().addAccount(Arg.any());
+      expect(stateService.addAccount).not.toHaveBeenCalled();
-      messagingService.didNotReceive().send(Arg.any());
+      expect(messagingService.send).not.toHaveBeenCalled();
 
       const expected = new AuthResult();
       expected.twoFactorProviders = new Map<TwoFactorProviderType, { [key: string]: string }>();
@@ -220,26 +225,25 @@ describe("LogInStrategy", () => {
     });
 
     it("sends stored 2FA token to server", async () => {
-      tokenService.getTwoFactorToken().resolves(twoFactorToken);
+      tokenService.getTwoFactorToken.mockResolvedValue(twoFactorToken);
-      apiService.postIdentityToken(Arg.any()).resolves(identityTokenResponseFactory());
+      apiService.postIdentityToken.mockResolvedValue(identityTokenResponseFactory());
 
       await passwordLogInStrategy.logIn(credentials);
 
-      apiService.received(1).postIdentityToken(
+      expect(apiService.postIdentityToken).toHaveBeenCalledWith(
-        Arg.is((actual) => {
+        expect.objectContaining({
-          const passwordTokenRequest = actual as any;
+          twoFactor: {
-          return (
+            provider: TwoFactorProviderType.Remember,
-            passwordTokenRequest.twoFactor.provider === TwoFactorProviderType.Remember &&
+            token: twoFactorToken,
-            passwordTokenRequest.twoFactor.token === twoFactorToken &&
+            remember: false,
-            passwordTokenRequest.twoFactor.remember === false
+          } as TokenTwoFactorRequest,
-          );
         })
       );
     });
 
     it("sends 2FA token provided by user to server (single step)", async () => {
       // This occurs if the user enters the 2FA code as an argument in the CLI
-      apiService.postIdentityToken(Arg.any()).resolves(identityTokenResponseFactory());
+      apiService.postIdentityToken.mockResolvedValue(identityTokenResponseFactory());
       credentials.twoFactor = new TokenTwoFactorRequest(
         twoFactorProviderType,
         twoFactorToken,
@@ -248,14 +252,13 @@ describe("LogInStrategy", () => {
 
       await passwordLogInStrategy.logIn(credentials);
 
-      apiService.received(1).postIdentityToken(
+      expect(apiService.postIdentityToken).toHaveBeenCalledWith(
-        Arg.is((actual) => {
+        expect.objectContaining({
-          const passwordTokenRequest = actual as any;
+          twoFactor: {
-          return (
+            provider: twoFactorProviderType,
-            passwordTokenRequest.twoFactor.provider === twoFactorProviderType &&
+            token: twoFactorToken,
-            passwordTokenRequest.twoFactor.token === twoFactorToken &&
+            remember: twoFactorRemember,
-            passwordTokenRequest.twoFactor.remember === twoFactorRemember
+          } as TokenTwoFactorRequest,
-          );
         })
       );
     });
@@ -269,21 +272,20 @@ describe("LogInStrategy", () => {
         null
       );
 
-      apiService.postIdentityToken(Arg.any()).resolves(identityTokenResponseFactory());
+      apiService.postIdentityToken.mockResolvedValue(identityTokenResponseFactory());
 
       await passwordLogInStrategy.logInTwoFactor(
         new TokenTwoFactorRequest(twoFactorProviderType, twoFactorToken, twoFactorRemember),
         null
       );
 
-      apiService.received(1).postIdentityToken(
+      expect(apiService.postIdentityToken).toHaveBeenCalledWith(
-        Arg.is((actual) => {
+        expect.objectContaining({
-          const passwordTokenRequest = actual as any;
+          twoFactor: {
-          return (
+            provider: twoFactorProviderType,
-            passwordTokenRequest.twoFactor.provider === twoFactorProviderType &&
+            token: twoFactorToken,
-            passwordTokenRequest.twoFactor.token === twoFactorToken &&
+            remember: twoFactorRemember,
-            passwordTokenRequest.twoFactor.remember === twoFactorRemember
+          } as TokenTwoFactorRequest,
-          );
         })
       );
     });

libs/common/spec/misc/logInStrategies/passwordLogIn.strategy.spec.ts

@@ -1,5 +1,4 @@
-// eslint-disable-next-line no-restricted-imports
+import { mock, MockProxy } from "jest-mock-extended";
-import { Arg, Substitute, SubstituteOf } from "@fluffy-spoon/substitute";
 
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { AppIdService } from "@bitwarden/common/abstractions/appId.service";
@@ -31,41 +30,43 @@ const preloginKey = new SymmetricCryptoKey(
 const deviceId = Utils.newGuid();
 
 describe("PasswordLogInStrategy", () => {
-  let cryptoService: SubstituteOf<CryptoService>;
+  let cryptoService: MockProxy<CryptoService>;
-  let apiService: SubstituteOf<ApiService>;
+  let apiService: MockProxy<ApiService>;
-  let tokenService: SubstituteOf<TokenService>;
+  let tokenService: MockProxy<TokenService>;
-  let appIdService: SubstituteOf<AppIdService>;
+  let appIdService: MockProxy<AppIdService>;
-  let platformUtilsService: SubstituteOf<PlatformUtilsService>;
+  let platformUtilsService: MockProxy<PlatformUtilsService>;
-  let messagingService: SubstituteOf<MessagingService>;
+  let messagingService: MockProxy<MessagingService>;
-  let logService: SubstituteOf<LogService>;
+  let logService: MockProxy<LogService>;
-  let stateService: SubstituteOf<StateService>;
+  let stateService: MockProxy<StateService>;
-  let twoFactorService: SubstituteOf<TwoFactorService>;
+  let twoFactorService: MockProxy<TwoFactorService>;
-  let authService: SubstituteOf<AuthService>;
+  let authService: MockProxy<AuthService>;
 
   let passwordLogInStrategy: PasswordLogInStrategy;
   let credentials: PasswordLogInCredentials;
 
   beforeEach(async () => {
-    cryptoService = Substitute.for<CryptoService>();
+    cryptoService = mock<CryptoService>();
-    apiService = Substitute.for<ApiService>();
+    apiService = mock<ApiService>();
-    tokenService = Substitute.for<TokenService>();
+    tokenService = mock<TokenService>();
-    appIdService = Substitute.for<AppIdService>();
+    appIdService = mock<AppIdService>();
-    platformUtilsService = Substitute.for<PlatformUtilsService>();
+    platformUtilsService = mock<PlatformUtilsService>();
-    messagingService = Substitute.for<MessagingService>();
+    messagingService = mock<MessagingService>();
-    logService = Substitute.for<LogService>();
+    logService = mock<LogService>();
-    stateService = Substitute.for<StateService>();
+    stateService = mock<StateService>();
-    twoFactorService = Substitute.for<TwoFactorService>();
+    twoFactorService = mock<TwoFactorService>();
-    authService = Substitute.for<AuthService>();
+    authService = mock<AuthService>();
 
-    appIdService.getAppId().resolves(deviceId);
+    appIdService.getAppId.mockResolvedValue(deviceId);
-    tokenService.getTwoFactorToken().resolves(null);
+    tokenService.decodeToken.mockResolvedValue({});
 
-    authService.makePreloginKey(Arg.any(), Arg.any()).resolves(preloginKey);
+    authService.makePreloginKey.mockResolvedValue(preloginKey);
 
-    cryptoService.hashPassword(masterPassword, Arg.any()).resolves(hashedPassword);
+    cryptoService.hashPassword
-    cryptoService
+      .calledWith(masterPassword, expect.anything(), undefined)
-      .hashPassword(masterPassword, Arg.any(), HashPurpose.LocalAuthorization)
+      .mockResolvedValue(hashedPassword);
-      .resolves(localHashedPassword);
+    cryptoService.hashPassword
+      .calledWith(masterPassword, expect.anything(), HashPurpose.LocalAuthorization)
+      .mockResolvedValue(localHashedPassword);
 
     passwordLogInStrategy = new PasswordLogInStrategy(
       cryptoService,
@@ -81,23 +82,24 @@ describe("PasswordLogInStrategy", () => {
     );
     credentials = new PasswordLogInCredentials(email, masterPassword);
 
-    apiService.postIdentityToken(Arg.any()).resolves(identityTokenResponseFactory());
+    apiService.postIdentityToken.mockResolvedValue(identityTokenResponseFactory());
   });
 
   it("sends master password credentials to the server", async () => {
     await passwordLogInStrategy.logIn(credentials);
 
-    apiService.received(1).postIdentityToken(
+    expect(apiService.postIdentityToken).toHaveBeenCalledWith(
-      Arg.is((actual) => {
+      expect.objectContaining({
-        const passwordTokenRequest = actual as any; // Need to access private fields
+        email: email,
-        return (
+        masterPasswordHash: hashedPassword,
-          passwordTokenRequest.email === email &&
+        device: expect.objectContaining({
-          passwordTokenRequest.masterPasswordHash === hashedPassword &&
+          identifier: deviceId,
-          passwordTokenRequest.device.identifier === deviceId &&
+        }),
-          passwordTokenRequest.twoFactor.provider == null &&
+        twoFactor: expect.objectContaining({
-          passwordTokenRequest.twoFactor.token == null &&
+          provider: null,
-          passwordTokenRequest.captchaResponse == null
+          token: null,
-        );
+        }),
+        captchaResponse: undefined,
       })
     );
   });
@@ -105,7 +107,7 @@ describe("PasswordLogInStrategy", () => {
   it("sets the local environment after a successful login", async () => {
     await passwordLogInStrategy.logIn(credentials);
 
-    cryptoService.received(1).setKey(preloginKey);
+    expect(cryptoService.setKey).toHaveBeenCalledWith(preloginKey);
-    cryptoService.received(1).setKeyHash(localHashedPassword);
+    expect(cryptoService.setKeyHash).toHaveBeenCalledWith(localHashedPassword);
   });
 });

libs/common/spec/misc/logInStrategies/ssoLogIn.strategy.spec.ts

@@ -1,5 +1,4 @@
-// eslint-disable-next-line no-restricted-imports
+import { mock, MockProxy } from "jest-mock-extended";
-import { Arg, Substitute, SubstituteOf } from "@fluffy-spoon/substitute";
 
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { AppIdService } from "@bitwarden/common/abstractions/appId.service";
@@ -18,23 +17,21 @@ import { SsoLogInCredentials } from "@bitwarden/common/models/domain/log-in-cred
 import { identityTokenResponseFactory } from "./logIn.strategy.spec";
 
 describe("SsoLogInStrategy", () => {
-  let cryptoService: SubstituteOf<CryptoService>;
+  let cryptoService: MockProxy<CryptoService>;
-  let apiService: SubstituteOf<ApiService>;
+  let apiService: MockProxy<ApiService>;
-  let tokenService: SubstituteOf<TokenService>;
+  let tokenService: MockProxy<TokenService>;
-  let appIdService: SubstituteOf<AppIdService>;
+  let appIdService: MockProxy<AppIdService>;
-  let platformUtilsService: SubstituteOf<PlatformUtilsService>;
+  let platformUtilsService: MockProxy<PlatformUtilsService>;
-  let messagingService: SubstituteOf<MessagingService>;
+  let messagingService: MockProxy<MessagingService>;
-  let logService: SubstituteOf<LogService>;
+  let logService: MockProxy<LogService>;
-  let keyConnectorService: SubstituteOf<KeyConnectorService>;
+  let stateService: MockProxy<StateService>;
-  let stateService: SubstituteOf<StateService>;
+  let twoFactorService: MockProxy<TwoFactorService>;
-  let twoFactorService: SubstituteOf<TwoFactorService>;
+  let keyConnectorService: MockProxy<KeyConnectorService>;
 
   let ssoLogInStrategy: SsoLogInStrategy;
   let credentials: SsoLogInCredentials;
 
   const deviceId = Utils.newGuid();
-  const encKey = "ENC_KEY";
-  const privateKey = "PRIVATE_KEY";
   const keyConnectorUrl = "KEY_CONNECTOR_URL";
 
   const ssoCode = "SSO_CODE";
@@ -43,19 +40,20 @@ describe("SsoLogInStrategy", () => {
   const ssoOrgId = "SSO_ORG_ID";
 
   beforeEach(async () => {
-    cryptoService = Substitute.for<CryptoService>();
+    cryptoService = mock<CryptoService>();
-    apiService = Substitute.for<ApiService>();
+    apiService = mock<ApiService>();
-    tokenService = Substitute.for<TokenService>();
+    tokenService = mock<TokenService>();
-    appIdService = Substitute.for<AppIdService>();
+    appIdService = mock<AppIdService>();
-    platformUtilsService = Substitute.for<PlatformUtilsService>();
+    platformUtilsService = mock<PlatformUtilsService>();
-    messagingService = Substitute.for<MessagingService>();
+    messagingService = mock<MessagingService>();
-    logService = Substitute.for<LogService>();
+    logService = mock<LogService>();
-    stateService = Substitute.for<StateService>();
+    stateService = mock<StateService>();
-    keyConnectorService = Substitute.for<KeyConnectorService>();
+    twoFactorService = mock<TwoFactorService>();
-    twoFactorService = Substitute.for<TwoFactorService>();
+    keyConnectorService = mock<KeyConnectorService>();
 
-    tokenService.getTwoFactorToken().resolves(null);
+    tokenService.getTwoFactorToken.mockResolvedValue(null);
-    appIdService.getAppId().resolves(deviceId);
+    appIdService.getAppId.mockResolvedValue(deviceId);
+    tokenService.decodeToken.mockResolvedValue({});
 
     ssoLogInStrategy = new SsoLogInStrategy(
       cryptoService,
@@ -73,21 +71,22 @@ describe("SsoLogInStrategy", () => {
   });
 
   it("sends SSO information to server", async () => {
-    apiService.postIdentityToken(Arg.any()).resolves(identityTokenResponseFactory());
+    apiService.postIdentityToken.mockResolvedValue(identityTokenResponseFactory());
 
     await ssoLogInStrategy.logIn(credentials);
 
-    apiService.received(1).postIdentityToken(
+    expect(apiService.postIdentityToken).toHaveBeenCalledWith(
-      Arg.is((actual) => {
+      expect.objectContaining({
-        const ssoTokenRequest = actual as any;
+        code: ssoCode,
-        return (
+        codeVerifier: ssoCodeVerifier,
-          ssoTokenRequest.code === ssoCode &&
+        redirectUri: ssoRedirectUrl,
-          ssoTokenRequest.codeVerifier === ssoCodeVerifier &&
+        device: expect.objectContaining({
-          ssoTokenRequest.redirectUri === ssoRedirectUrl &&
+          identifier: deviceId,
-          ssoTokenRequest.device.identifier === deviceId &&
+        }),
-          ssoTokenRequest.twoFactor.provider == null &&
+        twoFactor: expect.objectContaining({
-          ssoTokenRequest.twoFactor.token == null
+          provider: null,
-        );
+          token: null,
+        }),
       })
     );
   });
@@ -95,23 +94,23 @@ describe("SsoLogInStrategy", () => {
   it("does not set keys for new SSO user flow", async () => {
     const tokenResponse = identityTokenResponseFactory();
     tokenResponse.key = null;
-    apiService.postIdentityToken(Arg.any()).resolves(tokenResponse);
+    apiService.postIdentityToken.mockResolvedValue(tokenResponse);
 
     await ssoLogInStrategy.logIn(credentials);
 
-    cryptoService.didNotReceive().setEncPrivateKey(privateKey);
+    expect(cryptoService.setEncPrivateKey).not.toHaveBeenCalled();
-    cryptoService.didNotReceive().setEncKey(encKey);
+    expect(cryptoService.setEncKey).not.toHaveBeenCalled();
   });
 
   it("gets and sets KeyConnector key for enrolled user", async () => {
     const tokenResponse = identityTokenResponseFactory();
     tokenResponse.keyConnectorUrl = keyConnectorUrl;
 
-    apiService.postIdentityToken(Arg.any()).resolves(tokenResponse);
+    apiService.postIdentityToken.mockResolvedValue(tokenResponse);
 
     await ssoLogInStrategy.logIn(credentials);
 
-    keyConnectorService.received(1).getAndSetKey(keyConnectorUrl);
+    expect(keyConnectorService.getAndSetKey).toHaveBeenCalledWith(keyConnectorUrl);
   });
 
   it("converts new SSO user to Key Connector on first login", async () => {
@@ -119,10 +118,13 @@ describe("SsoLogInStrategy", () => {
     tokenResponse.keyConnectorUrl = keyConnectorUrl;
     tokenResponse.key = null;
 
-    apiService.postIdentityToken(Arg.any()).resolves(tokenResponse);
+    apiService.postIdentityToken.mockResolvedValue(tokenResponse);
 
     await ssoLogInStrategy.logIn(credentials);
 
-    keyConnectorService.received(1).convertNewSsoUserToKeyConnector(tokenResponse, ssoOrgId);
+    expect(keyConnectorService.convertNewSsoUserToKeyConnector).toHaveBeenCalledWith(
+      tokenResponse,
+      ssoOrgId
+    );
   });
 });

libs/common/spec/misc/logInStrategies/user-api-login.strategy.spec.ts

@@ -1,5 +1,4 @@
-// eslint-disable-next-line no-restricted-imports
+import { mock, MockProxy } from "jest-mock-extended";
-import { Arg, Substitute, SubstituteOf } from "@fluffy-spoon/substitute";
 
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { AppIdService } from "@bitwarden/common/abstractions/appId.service";
@@ -19,17 +18,17 @@ import { UserApiLogInCredentials } from "@bitwarden/common/models/domain/log-in-
 import { identityTokenResponseFactory } from "./logIn.strategy.spec";
 
 describe("UserApiLogInStrategy", () => {
-  let cryptoService: SubstituteOf<CryptoService>;
+  let cryptoService: MockProxy<CryptoService>;
-  let apiService: SubstituteOf<ApiService>;
+  let apiService: MockProxy<ApiService>;
-  let tokenService: SubstituteOf<TokenService>;
+  let tokenService: MockProxy<TokenService>;
-  let appIdService: SubstituteOf<AppIdService>;
+  let appIdService: MockProxy<AppIdService>;
-  let platformUtilsService: SubstituteOf<PlatformUtilsService>;
+  let platformUtilsService: MockProxy<PlatformUtilsService>;
-  let messagingService: SubstituteOf<MessagingService>;
+  let messagingService: MockProxy<MessagingService>;
-  let logService: SubstituteOf<LogService>;
+  let logService: MockProxy<LogService>;
-  let environmentService: SubstituteOf<EnvironmentService>;
+  let stateService: MockProxy<StateService>;
-  let keyConnectorService: SubstituteOf<KeyConnectorService>;
+  let twoFactorService: MockProxy<TwoFactorService>;
-  let stateService: SubstituteOf<StateService>;
+  let keyConnectorService: MockProxy<KeyConnectorService>;
-  let twoFactorService: SubstituteOf<TwoFactorService>;
+  let environmentService: MockProxy<EnvironmentService>;
 
   let apiLogInStrategy: UserApiLogInStrategy;
   let credentials: UserApiLogInCredentials;
@@ -40,20 +39,21 @@ describe("UserApiLogInStrategy", () => {
   const apiClientSecret = "API_CLIENT_SECRET";
 
   beforeEach(async () => {
-    cryptoService = Substitute.for<CryptoService>();
+    cryptoService = mock<CryptoService>();
-    apiService = Substitute.for<ApiService>();
+    apiService = mock<ApiService>();
-    tokenService = Substitute.for<TokenService>();
+    tokenService = mock<TokenService>();
-    appIdService = Substitute.for<AppIdService>();
+    appIdService = mock<AppIdService>();
-    platformUtilsService = Substitute.for<PlatformUtilsService>();
+    platformUtilsService = mock<PlatformUtilsService>();
-    messagingService = Substitute.for<MessagingService>();
+    messagingService = mock<MessagingService>();
-    logService = Substitute.for<LogService>();
+    logService = mock<LogService>();
-    environmentService = Substitute.for<EnvironmentService>();
+    stateService = mock<StateService>();
-    stateService = Substitute.for<StateService>();
+    twoFactorService = mock<TwoFactorService>();
-    keyConnectorService = Substitute.for<KeyConnectorService>();
+    keyConnectorService = mock<KeyConnectorService>();
-    twoFactorService = Substitute.for<TwoFactorService>();
+    environmentService = mock<EnvironmentService>();
 
-    appIdService.getAppId().resolves(deviceId);
+    appIdService.getAppId.mockResolvedValue(deviceId);
-    tokenService.getTwoFactorToken().resolves(null);
+    tokenService.getTwoFactorToken.mockResolvedValue(null);
+    tokenService.decodeToken.mockResolvedValue({});
 
     apiLogInStrategy = new UserApiLogInStrategy(
       cryptoService,
@@ -73,43 +73,43 @@ describe("UserApiLogInStrategy", () => {
   });
 
   it("sends api key credentials to the server", async () => {
-    apiService.postIdentityToken(Arg.any()).resolves(identityTokenResponseFactory());
+    apiService.postIdentityToken.mockResolvedValue(identityTokenResponseFactory());
     await apiLogInStrategy.logIn(credentials);
 
-    apiService.received(1).postIdentityToken(
+    expect(apiService.postIdentityToken).toHaveBeenCalledWith(
-      Arg.is((actual) => {
+      expect.objectContaining({
-        const apiTokenRequest = actual as any;
+        clientId: apiClientId,
-        return (
+        clientSecret: apiClientSecret,
-          apiTokenRequest.clientId === apiClientId &&
+        device: expect.objectContaining({
-          apiTokenRequest.clientSecret === apiClientSecret &&
+          identifier: deviceId,
-          apiTokenRequest.device.identifier === deviceId &&
+        }),
-          apiTokenRequest.twoFactor.provider == null &&
+        twoFactor: expect.objectContaining({
-          apiTokenRequest.twoFactor.token == null &&
+          provider: null,
-          apiTokenRequest.captchaResponse == null
+          token: null,
-        );
+        }),
       })
     );
   });
 
   it("sets the local environment after a successful login", async () => {
-    apiService.postIdentityToken(Arg.any()).resolves(identityTokenResponseFactory());
+    apiService.postIdentityToken.mockResolvedValue(identityTokenResponseFactory());
 
     await apiLogInStrategy.logIn(credentials);
 
-    stateService.received(1).setApiKeyClientId(apiClientId);
+    expect(stateService.setApiKeyClientId).toHaveBeenCalledWith(apiClientId);
-    stateService.received(1).setApiKeyClientSecret(apiClientSecret);
+    expect(stateService.setApiKeyClientSecret).toHaveBeenCalledWith(apiClientSecret);
-    stateService.received(1).addAccount(Arg.any());
+    expect(stateService.addAccount).toHaveBeenCalled();
   });
 
   it("gets and sets the Key Connector key from environmentUrl", async () => {
     const tokenResponse = identityTokenResponseFactory();
     tokenResponse.apiUseKeyConnector = true;
 
-    apiService.postIdentityToken(Arg.any()).resolves(tokenResponse);
+    apiService.postIdentityToken.mockResolvedValue(tokenResponse);
-    environmentService.getKeyConnectorUrl().returns(keyConnectorUrl);
+    environmentService.getKeyConnectorUrl.mockReturnValue(keyConnectorUrl);
 
     await apiLogInStrategy.logIn(credentials);
 
-    keyConnectorService.received(1).getAndSetKey(keyConnectorUrl);
+    expect(keyConnectorService.getAndSetKey).toHaveBeenCalledWith(keyConnectorUrl);
   });
 });

libs/common/src/misc/logInStrategies/logIn.strategy.ts

@@ -50,6 +50,9 @@ export abstract class LogInStrategy {
       | PasswordlessLogInCredentials
   ): Promise<AuthResult>;
 
+  // The user key comes from different sources depending on the login strategy
+  protected abstract setUserKey(response: IdentityTokenResponse): Promise<void>;
+
   async logInTwoFactor(
     twoFactor: TokenTwoFactorRequest,
     captchaResponse: string = null
@@ -74,11 +77,6 @@ export abstract class LogInStrategy {
     throw new Error("Invalid response object.");
   }
 
-  protected onSuccessfulLogin(response: IdentityTokenResponse): Promise<void> {
-    // Implemented in subclass if required
-    return null;
-  }
-
   protected async buildDeviceRequest() {
     const appId = await this.appIdService.getAppId();
     return new DeviceRequest(appId, this.platformUtilsService);
@@ -134,6 +132,9 @@ export abstract class LogInStrategy {
       await this.tokenService.setTwoFactorToken(response);
     }
 
+    await this.setUserKey(response);
+
+    // Must come after the user Key is set, otherwise createKeyPairForOldAccount will fail
     const newSsoUser = response.key == null;
     if (!newSsoUser) {
       await this.cryptoService.setEncKey(response.key);
@@ -142,8 +143,6 @@ export abstract class LogInStrategy {
       );
     }
 
-    await this.onSuccessfulLogin(response);
-
     this.messagingService.send("loggedIn");
 
     return result;

libs/common/src/misc/logInStrategies/passwordLogin.strategy.ts

@@ -56,7 +56,7 @@ export class PasswordLogInStrategy extends LogInStrategy {
     );
   }
 
-  async onSuccessfulLogin() {
+  async setUserKey() {
     await this.cryptoService.setKey(this.key);
     await this.cryptoService.setKeyHash(this.localHashedPassword);
   }

libs/common/src/misc/logInStrategies/passwordlessLogin.strategy.ts

@@ -56,7 +56,7 @@ export class PasswordlessLogInStrategy extends LogInStrategy {
     );
   }
 
-  async onSuccessfulLogin() {
+  async setUserKey() {
     await this.cryptoService.setKey(this.passwordlessCredentials.decKey);
     await this.cryptoService.setKeyHash(this.passwordlessCredentials.localPasswordHash);
   }

libs/common/src/misc/logInStrategies/ssoLogin.strategy.ts

@@ -43,7 +43,7 @@ export class SsoLogInStrategy extends LogInStrategy {
     );
   }
 
-  async onSuccessfulLogin(tokenResponse: IdentityTokenResponse) {
+  async setUserKey(tokenResponse: IdentityTokenResponse) {
     const newSsoUser = tokenResponse.key == null;
 
     if (tokenResponse.keyConnectorUrl != null) {

libs/common/src/misc/logInStrategies/user-api-login.strategy.ts

@@ -44,7 +44,7 @@ export class UserApiLogInStrategy extends LogInStrategy {
     );
   }
 
-  async onSuccessfulLogin(tokenResponse: IdentityTokenResponse) {
+  async setUserKey(tokenResponse: IdentityTokenResponse) {
     if (tokenResponse.apiUseKeyConnector) {
       const keyConnectorUrl = this.environmentService.getKeyConnectorUrl();
       await this.keyConnectorService.getAndSetKey(keyConnectorUrl);