PM-7392 - TokenSvc - Replace messageSender with logout callback per PR feedback.

This commit is contained in:
Jared Snider 2024-05-08 19:40:38 -04:00
parent d88766a028
commit bcf9099229
No known key found for this signature in database
GPG Key ID: A149DDD612516286
7 changed files with 52 additions and 59 deletions

View File

@ -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<void>;
};
};
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,
),
);
}

View File

@ -520,7 +520,7 @@ export default class MainBackground {
this.keyGenerationService,
this.encryptService,
this.logService,
this.messagingService,
logoutCallback,
);
const migrationRunner = new MigrationRunner(

View File

@ -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 */
});

View File

@ -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(

View File

@ -533,7 +533,7 @@ const safeProviders: SafeProvider[] = [
KeyGenerationServiceAbstraction,
EncryptService,
LogService,
MessagingServiceAbstraction,
LOGOUT_CALLBACK,
],
}),
safeProvider({

View File

@ -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<KeyGenerationService>;
let encryptService: MockProxy<EncryptService>;
let logService: MockProxy<LogService>;
let messagingService: MockProxy<MessagingService>;
let logoutCallback: jest.Mock<Promise<void>, [logoutReason: LogoutReason, userId?: string]>;
const memoryVaultTimeoutAction = VaultTimeoutAction.LogOut;
const memoryVaultTimeout = 30;
@ -99,7 +100,7 @@ describe("TokenService", () => {
keyGenerationService = mock<KeyGenerationService>();
encryptService = mock<EncryptService>();
logService = mock<LogService>();
messagingService = mock<MessagingService>();
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,
);
}
});

View File

@ -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<void>,
) {
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);
}
}