From bcf9099229d49a96506ec452f1d9fb3f2eed5be3 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 8 May 2024 19:40:38 -0400 Subject: [PATCH] PM-7392 - TokenSvc - Replace messageSender with logout callback per PR feedback. --- .../token-service.factory.ts | 16 +++---- .../browser/src/background/main.background.ts | 2 +- apps/cli/src/bw.ts | 13 +++--- apps/desktop/src/main.ts | 3 +- .../src/services/jslib-services.module.ts | 2 +- .../src/auth/services/token.service.spec.ts | 43 ++++++++++--------- .../common/src/auth/services/token.service.ts | 32 +++++--------- 7 files changed, 52 insertions(+), 59 deletions(-) diff --git a/apps/browser/src/auth/background/service-factories/token-service.factory.ts b/apps/browser/src/auth/background/service-factories/token-service.factory.ts index da39afcda2..e8cec63c26 100644 --- a/apps/browser/src/auth/background/service-factories/token-service.factory.ts +++ b/apps/browser/src/auth/background/service-factories/token-service.factory.ts @@ -1,3 +1,4 @@ +import { LogoutReason } from "@bitwarden/auth/common"; import { TokenService as AbstractTokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { TokenService } from "@bitwarden/common/auth/services/token.service"; @@ -22,10 +23,6 @@ import { LogServiceInitOptions, logServiceFactory, } from "../../../platform/background/service-factories/log-service.factory"; -import { - MessagingServiceInitOptions, - messagingServiceFactory, -} from "../../../platform/background/service-factories/messaging-service.factory"; import { PlatformUtilsServiceInitOptions, platformUtilsServiceFactory, @@ -39,7 +36,11 @@ import { secureStorageServiceFactory, } from "../../../platform/background/service-factories/storage-service.factory"; -type TokenServiceFactoryOptions = FactoryOptions; +type TokenServiceFactoryOptions = FactoryOptions & { + tokenServiceOptions: { + logoutCallback: (logoutReason: LogoutReason, userId?: string) => Promise; + }; +}; export type TokenServiceInitOptions = TokenServiceFactoryOptions & SingleUserStateProviderInitOptions & @@ -48,8 +49,7 @@ export type TokenServiceInitOptions = TokenServiceFactoryOptions & SecureStorageServiceInitOptions & KeyGenerationServiceInitOptions & EncryptServiceInitOptions & - LogServiceInitOptions & - MessagingServiceInitOptions; + LogServiceInitOptions; export function tokenServiceFactory( cache: { tokenService?: AbstractTokenService } & CachedServices, @@ -68,7 +68,7 @@ export function tokenServiceFactory( await keyGenerationServiceFactory(cache, opts), await encryptServiceFactory(cache, opts), await logServiceFactory(cache, opts), - await messagingServiceFactory(cache, opts), + opts.tokenServiceOptions.logoutCallback, ), ); } diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 9c5f7ff334..e4d643c5bd 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -520,7 +520,7 @@ export default class MainBackground { this.keyGenerationService, this.encryptService, this.logService, - this.messagingService, + logoutCallback, ); const migrationRunner = new MigrationRunner( diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index 4a68027681..e2c6323dfc 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -258,6 +258,9 @@ export class Main { p = path.join(process.env.HOME, ".config/Bitwarden CLI"); } + const logoutCallback = async (logoutReason: LogoutReason, userId?: UserId) => + await this.logout(logoutReason, userId); + this.platformUtilsService = new CliPlatformUtilsService(ClientType.Cli, packageJson); this.logService = new ConsoleLogService( this.platformUtilsService.isDev(), @@ -340,7 +343,7 @@ export class Main { this.keyGenerationService, this.encryptService, this.logService, - this.messagingService, + logoutCallback, ); const migrationRunner = new MigrationRunner( @@ -397,7 +400,7 @@ export class Main { throw new Error("Refresh Access token error"); }, this.logService, - async (logoutReason: LogoutReason) => await this.logout(), + logoutCallback, customUserAgent, ); @@ -459,7 +462,7 @@ export class Main { this.logService, this.organizationService, this.keyGenerationService, - async (logoutReason: LogoutReason) => await this.logout(), + logoutCallback, this.stateProvider, ); @@ -652,7 +655,7 @@ export class Main { this.sendApiService, this.userDecryptionOptionsService, this.avatarService, - async (logoutReason: LogoutReason) => await this.logout(), + logoutCallback, this.billingAccountProfileStateService, this.tokenService, ); @@ -733,7 +736,7 @@ export class Main { } } - async logout() { + async logout(logoutReason: LogoutReason, passedInUserId?: UserId) { this.authService.logOut(() => { /* Do nothing */ }); diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index 7ed92a7eec..ad7ebb494f 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -3,6 +3,7 @@ import * as path from "path"; import { app } from "electron"; import { Subject, firstValueFrom } from "rxjs"; +import { LogoutReason } from "@bitwarden/auth/common"; import { TokenService as TokenServiceAbstraction } from "@bitwarden/common/auth/abstractions/token.service"; import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; import { TokenService } from "@bitwarden/common/auth/services/token.service"; @@ -203,7 +204,7 @@ export class Main { this.keyGenerationService, this.encryptService, this.logService, - this.messagingService, + async (logoutReason: LogoutReason) => {}, ); this.migrationRunner = new MigrationRunner( diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index e36769cc4a..4025c88061 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -533,7 +533,7 @@ const safeProviders: SafeProvider[] = [ KeyGenerationServiceAbstraction, EncryptService, LogService, - MessagingServiceAbstraction, + LOGOUT_CALLBACK, ], }), safeProvider({ diff --git a/libs/common/src/auth/services/token.service.spec.ts b/libs/common/src/auth/services/token.service.spec.ts index f2baec8352..97a02b82f4 100644 --- a/libs/common/src/auth/services/token.service.spec.ts +++ b/libs/common/src/auth/services/token.service.spec.ts @@ -1,12 +1,13 @@ import { MockProxy, mock } from "jest-mock-extended"; import { firstValueFrom } from "rxjs"; +import { LogoutReason } from "@bitwarden/auth/common"; + import { FakeSingleUserStateProvider, FakeGlobalStateProvider } from "../../../spec"; import { VaultTimeoutAction } from "../../enums/vault-timeout-action.enum"; import { EncryptService } from "../../platform/abstractions/encrypt.service"; import { KeyGenerationService } from "../../platform/abstractions/key-generation.service"; import { LogService } from "../../platform/abstractions/log.service"; -import { MessagingService } from "../../platform/abstractions/messaging.service"; import { AbstractStorageService } from "../../platform/abstractions/storage.service"; import { StorageLocation } from "../../platform/enums"; import { StorageOptions } from "../../platform/models/domain/storage-options"; @@ -38,7 +39,7 @@ describe("TokenService", () => { let keyGenerationService: MockProxy; let encryptService: MockProxy; let logService: MockProxy; - let messagingService: MockProxy; + let logoutCallback: jest.Mock, [logoutReason: LogoutReason, userId?: string]>; const memoryVaultTimeoutAction = VaultTimeoutAction.LogOut; const memoryVaultTimeout = 30; @@ -99,7 +100,7 @@ describe("TokenService", () => { keyGenerationService = mock(); encryptService = mock(); logService = mock(); - messagingService = mock(); + logoutCallback = jest.fn(); const supportsSecureStorage = false; // default to false; tests will override as needed tokenService = createTokenService(supportsSecureStorage); @@ -533,10 +534,10 @@ describe("TokenService", () => { ); // assert that we logged the user out - expect(messagingService.send).toHaveBeenCalledWith("logout", { - userId: userIdFromAccessToken, - reason: "accessTokenUnableToBeDecrypted", - }); + expect(logoutCallback).toHaveBeenCalledWith( + "accessTokenUnableToBeDecrypted", + userIdFromAccessToken, + ); }); it("logs the error and logs the user out when secure storage errors on trying to get an access token key", async () => { @@ -568,10 +569,10 @@ describe("TokenService", () => { ); // assert that we logged the user out - expect(messagingService.send).toHaveBeenCalledWith("logout", { - userId: userIdFromAccessToken, - reason: "accessTokenUnableToBeDecrypted", - }); + expect(logoutCallback).toHaveBeenCalledWith( + "accessTokenUnableToBeDecrypted", + userIdFromAccessToken, + ); }); }); }); @@ -1366,7 +1367,7 @@ describe("TokenService", () => { // assert that we did not log an error or log the user out expect(logService.error).not.toHaveBeenCalled(); - expect(messagingService.send).not.toHaveBeenCalledWith("logout", expect.anything()); + expect(logoutCallback).not.toHaveBeenCalled(); }); it("does not error and fallback to disk storage when passed a null value for the refresh token", async () => { @@ -1425,10 +1426,10 @@ describe("TokenService", () => { ); // assert that we logged the user out - expect(messagingService.send).toHaveBeenCalledWith("logout", { - userId: userIdFromAccessToken, - reason: "accessTokenUnableToBeDecrypted", - }); + expect(logoutCallback).toHaveBeenCalledWith( + "accessTokenUnableToBeDecrypted", + userIdFromAccessToken, + ); }); }); }); @@ -1688,10 +1689,10 @@ describe("TokenService", () => { new Error(secureStorageSvcMockErrorMsg), ); - expect(messagingService.send).toHaveBeenCalledWith("logout", { - userId: userIdFromAccessToken, - reason: "refreshTokenSecureStorageRetrievalFailure", - }); + expect(logoutCallback).toHaveBeenCalledWith( + "refreshTokenSecureStorageRetrievalFailure", + userIdFromAccessToken, + ); }); }); }); @@ -2651,7 +2652,7 @@ describe("TokenService", () => { keyGenerationService, encryptService, logService, - messagingService, + logoutCallback, ); } }); diff --git a/libs/common/src/auth/services/token.service.ts b/libs/common/src/auth/services/token.service.ts index d7399387ee..4501c4aa87 100644 --- a/libs/common/src/auth/services/token.service.ts +++ b/libs/common/src/auth/services/token.service.ts @@ -1,7 +1,7 @@ import { Observable, combineLatest, firstValueFrom, map } from "rxjs"; import { Opaque } from "type-fest"; -import { decodeJwtTokenToJson } from "@bitwarden/auth/common"; +import { LogoutReason, decodeJwtTokenToJson } from "@bitwarden/auth/common"; import { VaultTimeoutAction } from "../../enums/vault-timeout-action.enum"; import { EncryptService } from "../../platform/abstractions/encrypt.service"; @@ -9,7 +9,6 @@ import { KeyGenerationService } from "../../platform/abstractions/key-generation import { LogService } from "../../platform/abstractions/log.service"; import { AbstractStorageService } from "../../platform/abstractions/storage.service"; import { StorageLocation } from "../../platform/enums"; -import { MessageSender } from "../../platform/messaging"; import { EncString, EncryptedString } from "../../platform/models/domain/enc-string"; import { StorageOptions } from "../../platform/models/domain/storage-options"; import { SymmetricCryptoKey } from "../../platform/models/domain/symmetric-crypto-key"; @@ -132,7 +131,7 @@ export class TokenService implements TokenServiceAbstraction { private keyGenerationService: KeyGenerationService, private encryptService: EncryptService, private logService: LogService, - private messageSender: MessageSender, + private logoutCallback: (logoutReason: LogoutReason, userId?: string) => Promise, ) { this.initializeState(); } @@ -410,10 +409,7 @@ export class TokenService implements TokenServiceAbstraction { "Access token key retrieval failed. Unable to decrypt encrypted access token. Logging user out.", error, ); - this.messageSender.send("logout", { - userId, - reason: "accessTokenUnableToBeDecrypted", - }); + await this.logoutCallback("accessTokenUnableToBeDecrypted", userId); return null; } @@ -432,10 +428,9 @@ export class TokenService implements TokenServiceAbstraction { this.logService.error( "Access token key not found to decrypt encrypted access token. Logging user out.", ); - this.messageSender.send("logout", { - userId, - reason: "accessTokenUnableToBeDecrypted", - }); + + await this.logoutCallback("accessTokenUnableToBeDecrypted", userId); + return null; } @@ -456,10 +451,9 @@ export class TokenService implements TokenServiceAbstraction { // We don't try to recover here since we'd like to know // if access token and key are getting out of sync. this.logService.error(`Failed to decrypt access token`, error); - this.messageSender.send("logout", { - userId, - reason: "accessTokenUnableToBeDecrypted", - }); + + await this.logoutCallback("accessTokenUnableToBeDecrypted", userId); + return null; } } @@ -588,13 +582,7 @@ export class TokenService implements TokenServiceAbstraction { this.logService.error(`Failed to retrieve refresh token from secure storage`, error); - // This is not ideal as we would like to use the LOGOUT_CALLBACK injection token - // instead of directly using the MessagingService here. However, we can't customize the - // logout reason if we do so. - this.messageSender.send("logout", { - userId, - reason: "refreshTokenSecureStorageRetrievalFailure", - }); + await this.logoutCallback("refreshTokenSecureStorageRetrievalFailure", userId); } }