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 ba42998209..da39afcda2 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 @@ -22,6 +22,10 @@ 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, @@ -44,7 +48,8 @@ export type TokenServiceInitOptions = TokenServiceFactoryOptions & SecureStorageServiceInitOptions & KeyGenerationServiceInitOptions & EncryptServiceInitOptions & - LogServiceInitOptions; + LogServiceInitOptions & + MessagingServiceInitOptions; export function tokenServiceFactory( cache: { tokenService?: AbstractTokenService } & CachedServices, @@ -63,6 +68,7 @@ export function tokenServiceFactory( await keyGenerationServiceFactory(cache, opts), await encryptServiceFactory(cache, opts), await logServiceFactory(cache, opts), + await messagingServiceFactory(cache, opts), ), ); } diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index dc93de7803..be94d6221c 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -517,6 +517,7 @@ export default class MainBackground { this.keyGenerationService, this.encryptService, this.logService, + this.messagingService, ); const migrationRunner = new MigrationRunner( diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index be3ad9ea0e..17c8ad0be0 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -335,6 +335,7 @@ export class Main { this.keyGenerationService, this.encryptService, this.logService, + this.messagingService, ); const migrationRunner = new MigrationRunner( diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index da4c14b4aa..14a3d321d1 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -176,6 +176,16 @@ export class Main { // Note: secure storage service is not available and should not be called in the main background process. const illegalSecureStorageService = new IllegalSecureStorageService(); + const messageSubject = new Subject>(); + this.messagingService = MessageSender.combine( + new SubjectMessageSender(messageSubject), // For local messages + new ElectronMainMessagingService(this.windowMain), + ); + + messageSubject.asObservable().subscribe((message) => { + this.messagingMain.onMessage(message); + }); + this.tokenService = new TokenService( singleUserStateProvider, globalStateProvider, @@ -184,6 +194,7 @@ export class Main { this.keyGenerationService, this.encryptService, this.logService, + this.messagingService, ); this.migrationRunner = new MigrationRunner( @@ -224,16 +235,6 @@ export class Main { this.updaterMain = new UpdaterMain(this.i18nService, this.windowMain); this.trayMain = new TrayMain(this.windowMain, this.i18nService, this.desktopSettingsService); - const messageSubject = new Subject>(); - this.messagingService = MessageSender.combine( - new SubjectMessageSender(messageSubject), // For local messages - new ElectronMainMessagingService(this.windowMain), - ); - - messageSubject.asObservable().subscribe((message) => { - this.messagingMain.onMessage(message); - }); - this.powerMonitorMain = new PowerMonitorMain(this.messagingService); this.menuMain = new MenuMain( this.i18nService, diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 42879a8424..ea0fadfa0f 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -523,6 +523,7 @@ const safeProviders: SafeProvider[] = [ KeyGenerationServiceAbstraction, EncryptService, LogService, + MessagingServiceAbstraction, ], }), safeProvider({ diff --git a/libs/common/src/auth/services/token.service.spec.ts b/libs/common/src/auth/services/token.service.spec.ts index 3e92053d2f..5b8af2e834 100644 --- a/libs/common/src/auth/services/token.service.spec.ts +++ b/libs/common/src/auth/services/token.service.spec.ts @@ -6,6 +6,7 @@ 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"; @@ -35,6 +36,7 @@ describe("TokenService", () => { let keyGenerationService: MockProxy; let encryptService: MockProxy; let logService: MockProxy; + let messagingService: MockProxy; const memoryVaultTimeoutAction = VaultTimeoutAction.LogOut; const memoryVaultTimeout = 30; @@ -92,6 +94,7 @@ describe("TokenService", () => { keyGenerationService = mock(); encryptService = mock(); logService = mock(); + messagingService = mock(); const supportsSecureStorage = false; // default to false; tests will override as needed tokenService = createTokenService(supportsSecureStorage); @@ -2280,6 +2283,7 @@ describe("TokenService", () => { keyGenerationService, encryptService, logService, + messagingService, ); } }); diff --git a/libs/common/src/auth/services/token.service.ts b/libs/common/src/auth/services/token.service.ts index 40036a8453..9bfdf21240 100644 --- a/libs/common/src/auth/services/token.service.ts +++ b/libs/common/src/auth/services/token.service.ts @@ -7,6 +7,7 @@ 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 { EncString, EncryptedString } from "../../platform/models/domain/enc-string"; @@ -114,6 +115,8 @@ type AccessTokenKey = Opaque; export class TokenService implements TokenServiceAbstraction { private readonly accessTokenKeySecureStorageKey: string = "_accessTokenKey"; + private readonly accessTokenKeyFailedToSaveToSecureStorageError = + "Access token key not saved to secure storage."; private readonly refreshTokenSecureStorageKey: string = "_refreshToken"; @@ -131,6 +134,7 @@ export class TokenService implements TokenServiceAbstraction { private keyGenerationService: KeyGenerationService, private encryptService: EncryptService, private logService: LogService, + private messagingService: MessagingService, ) { this.initializeState(); } @@ -208,6 +212,17 @@ export class TokenService implements TokenServiceAbstraction { this.getSecureStorageOptions(userId), ); + // We are having intermittent issues with access token keys not saving into secure storage on windows 10/11. + // So, let's add a check to ensure we can read the value after writing it. + + const accessTokenKey = await this.getAccessTokenKey(userId); // replace a + + // accessTokenKey = null; // TODO: remove this after testing + + if (!accessTokenKey) { + throw new Error(this.accessTokenKeyFailedToSaveToSecureStorageError); + } + return newAccessTokenKey; } @@ -287,17 +302,31 @@ export class TokenService implements TokenServiceAbstraction { // store the access token directly. Instead, we encrypt with accessTokenKey and store that // in secure storage. - const encryptedAccessToken: EncString = await this.encryptAccessToken(accessToken, userId); + try { + const encryptedAccessToken: EncString = await this.encryptAccessToken( + accessToken, + userId, + ); + // Save the encrypted access token to disk + await this.singleUserStateProvider + .get(userId, ACCESS_TOKEN_DISK) + .update((_) => encryptedAccessToken.encryptedString); - // Save the encrypted access token to disk - await this.singleUserStateProvider - .get(userId, ACCESS_TOKEN_DISK) - .update((_) => encryptedAccessToken.encryptedString); - - // TODO: PM-6408 - https://bitwarden.atlassian.net/browse/PM-6408 - // 2024-02-20: Remove access token from memory so that we migrate to encrypt the access token over time. - // Remove this call to remove the access token from memory after 3 releases. - await this.singleUserStateProvider.get(userId, ACCESS_TOKEN_MEMORY).update((_) => null); + // TODO: PM-6408 - https://bitwarden.atlassian.net/browse/PM-6408 + // 2024-02-20: Remove access token from memory so that we migrate to encrypt the access token over time. + // Remove this call to remove the access token from memory after 3 releases. + await this.singleUserStateProvider.get(userId, ACCESS_TOKEN_MEMORY).update((_) => null); + } catch (error) { + // check if error is Access token key not saved to secure storage. + if (error.message === this.accessTokenKeyFailedToSaveToSecureStorageError) { + // Fall back to disk storage for unecrypted access token + await this.singleUserStateProvider + .get(userId, ACCESS_TOKEN_DISK) + .update((_) => accessToken); + } + // re-throw the error if it's not the expected error + throw error; + } return; } @@ -383,7 +412,15 @@ export class TokenService implements TokenServiceAbstraction { const accessTokenKey = await this.getAccessTokenKey(userId); if (!accessTokenKey) { - // We know this is an unencrypted access token because we don't have an access token key + if (this.encryptService.stringIsEncString(accessTokenDisk)) { + // We have to log the user out if we can't decrypt the access token + this.logService.error( + "Access token key not found to decrypt encrypted access token. Logging user out.", + ); + this.messagingService.send("logout", { userId }); + } + + // We know this is an unencrypted access token return accessTokenDisk; }