From 09ff12fc028545d149a74f4c432ed729d511d7e7 Mon Sep 17 00:00:00 2001 From: Jake Fink Date: Mon, 6 May 2024 11:15:33 -0400 Subject: [PATCH] [PM-7919] Add more tde logging (#9035) * adds additional logging to TDE service * remove base catch swallowing errors * add dependency to cli * fix comment --- .../device-trust-service.factory.ts | 2 + .../browser/src/background/main.background.ts | 1 + apps/cli/src/bw.ts | 1 + ...base-login-decryption-options.component.ts | 3 +- .../src/services/jslib-services.module.ts | 1 + .../login-strategies/sso-login.strategy.ts | 2 +- .../device-trust.service.implementation.ts | 49 ++++++++++++------- .../services/device-trust.service.spec.ts | 3 ++ 8 files changed, 40 insertions(+), 22 deletions(-) diff --git a/apps/browser/src/auth/background/service-factories/device-trust-service.factory.ts b/apps/browser/src/auth/background/service-factories/device-trust-service.factory.ts index 106bcbcf72..42a8232c3e 100644 --- a/apps/browser/src/auth/background/service-factories/device-trust-service.factory.ts +++ b/apps/browser/src/auth/background/service-factories/device-trust-service.factory.ts @@ -34,6 +34,7 @@ import { KeyGenerationServiceInitOptions, keyGenerationServiceFactory, } from "../../../platform/background/service-factories/key-generation-service.factory"; +import { logServiceFactory } from "../../../platform/background/service-factories/log-service.factory"; import { PlatformUtilsServiceInitOptions, platformUtilsServiceFactory, @@ -88,6 +89,7 @@ export function deviceTrustServiceFactory( await stateProviderFactory(cache, opts), await secureStorageServiceFactory(cache, opts), await userDecryptionOptionsServiceFactory(cache, opts), + await logServiceFactory(cache, opts), ), ); } diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 5ce7b0a408..5cd4113bae 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -631,6 +631,7 @@ export default class MainBackground { this.stateProvider, this.secureStorageService, this.userDecryptionOptionsService, + this.logService, ); this.devicesService = new DevicesServiceImplementation(this.devicesApiService); diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index 114765a789..a038f3aa90 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -486,6 +486,7 @@ export class Main { this.stateProvider, this.secureStorageService, this.userDecryptionOptionsService, + this.logService, ); this.authRequestService = new AuthRequestService( diff --git a/libs/angular/src/auth/components/base-login-decryption-options.component.ts b/libs/angular/src/auth/components/base-login-decryption-options.component.ts index 0e58c03a54..b5cc50d847 100644 --- a/libs/angular/src/auth/components/base-login-decryption-options.component.ts +++ b/libs/angular/src/auth/components/base-login-decryption-options.component.ts @@ -272,6 +272,7 @@ export class BaseLoginDecryptionOptionsComponent implements OnInit, OnDestroy { // this.loading to support clients without async-actions-support this.loading = true; + // errors must be caught in child components to prevent navigation try { const { publicKey, privateKey } = await this.cryptoService.initAccount(); const keysRequest = new KeysRequest(publicKey, privateKey.encryptedString); @@ -288,8 +289,6 @@ export class BaseLoginDecryptionOptionsComponent implements OnInit, OnDestroy { if (this.rememberDeviceForm.value.rememberDevice) { await this.deviceTrustService.trustDevice(this.activeAccountId); } - } catch (error) { - this.validationService.showError(error); } finally { this.loading = false; } diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 80024bb0b6..fdbf5e9ecb 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -967,6 +967,7 @@ const safeProviders: SafeProvider[] = [ StateProvider, SECURE_STORAGE, UserDecryptionOptionsServiceAbstraction, + LogService, ], }), safeProvider({ diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.ts index f328547772..c37ef683ed 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.ts @@ -241,7 +241,7 @@ export class SsoLoginStrategy extends LoginStrategy { if (userDecryptionOptions?.trustedDeviceOption) { await this.trySetUserKeyWithApprovedAdminRequestIfExists(userId); - const hasUserKey = await this.cryptoService.hasUserKey(); + const hasUserKey = await this.cryptoService.hasUserKey(userId); // Only try to set user key with device key if admin approval request was not successful if (!hasUserKey) { diff --git a/libs/common/src/auth/services/device-trust.service.implementation.ts b/libs/common/src/auth/services/device-trust.service.implementation.ts index ccf87acaf8..dd98ce2b44 100644 --- a/libs/common/src/auth/services/device-trust.service.implementation.ts +++ b/libs/common/src/auth/services/device-trust.service.implementation.ts @@ -8,6 +8,7 @@ import { CryptoService } from "../../platform/abstractions/crypto.service"; import { EncryptService } from "../../platform/abstractions/encrypt.service"; import { I18nService } from "../../platform/abstractions/i18n.service"; import { KeyGenerationService } from "../../platform/abstractions/key-generation.service"; +import { LogService } from "../../platform/abstractions/log.service"; import { PlatformUtilsService } from "../../platform/abstractions/platform-utils.service"; import { AbstractStorageService } from "../../platform/abstractions/storage.service"; import { StorageLocation } from "../../platform/enums"; @@ -61,6 +62,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { private stateProvider: StateProvider, private secureStorageService: AbstractStorageService, private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, + private logService: LogService, ) { this.supportsDeviceTrust$ = this.userDecryptionOptionsService.userDecryptionOptions$.pipe( map((options) => options?.trustedDeviceOption != null ?? false), @@ -110,7 +112,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { } // Attempt to get user key - const userKey: UserKey = await this.cryptoService.getUserKey(); + const userKey: UserKey = await this.cryptoService.getUserKey(userId); // If user key is not found, throw error if (!userKey) { @@ -223,19 +225,23 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { throw new Error("UserId is required. Cannot get device key."); } - if (this.platformSupportsSecureStorage) { - const deviceKeyB64 = await this.secureStorageService.get< - ReturnType - >(`${userId}${this.deviceKeySecureStorageKey}`, this.getSecureStorageOptions(userId)); + try { + if (this.platformSupportsSecureStorage) { + const deviceKeyB64 = await this.secureStorageService.get< + ReturnType + >(`${userId}${this.deviceKeySecureStorageKey}`, this.getSecureStorageOptions(userId)); - const deviceKey = SymmetricCryptoKey.fromJSON(deviceKeyB64) as DeviceKey; + const deviceKey = SymmetricCryptoKey.fromJSON(deviceKeyB64) as DeviceKey; + + return deviceKey; + } + + const deviceKey = await firstValueFrom(this.stateProvider.getUserState$(DEVICE_KEY, userId)); return deviceKey; + } catch (e) { + this.logService.error("Failed to get device key", e); } - - const deviceKey = await firstValueFrom(this.stateProvider.getUserState$(DEVICE_KEY, userId)); - - return deviceKey; } private async setDeviceKey(userId: UserId, deviceKey: DeviceKey | null): Promise { @@ -243,16 +249,20 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { throw new Error("UserId is required. Cannot set device key."); } - if (this.platformSupportsSecureStorage) { - await this.secureStorageService.save( - `${userId}${this.deviceKeySecureStorageKey}`, - deviceKey, - this.getSecureStorageOptions(userId), - ); - return; - } + try { + if (this.platformSupportsSecureStorage) { + await this.secureStorageService.save( + `${userId}${this.deviceKeySecureStorageKey}`, + deviceKey, + this.getSecureStorageOptions(userId), + ); + return; + } - await this.stateProvider.setUserState(DEVICE_KEY, deviceKey?.toJSON(), userId); + await this.stateProvider.setUserState(DEVICE_KEY, deviceKey?.toJSON(), userId); + } catch (e) { + this.logService.error("Failed to set device key", e); + } } private async makeDeviceKey(): Promise { @@ -293,6 +303,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { return new SymmetricCryptoKey(userKey) as UserKey; } catch (e) { // If either decryption effort fails, we want to remove the device key + this.logService.error("Failed to decrypt using device key. Removing device key."); await this.setDeviceKey(userId, null); return null; diff --git a/libs/common/src/auth/services/device-trust.service.spec.ts b/libs/common/src/auth/services/device-trust.service.spec.ts index 12b8cf2eaa..f61bce563f 100644 --- a/libs/common/src/auth/services/device-trust.service.spec.ts +++ b/libs/common/src/auth/services/device-trust.service.spec.ts @@ -14,6 +14,7 @@ import { CryptoService } from "../../platform/abstractions/crypto.service"; import { EncryptService } from "../../platform/abstractions/encrypt.service"; import { I18nService } from "../../platform/abstractions/i18n.service"; import { KeyGenerationService } from "../../platform/abstractions/key-generation.service"; +import { LogService } from "../../platform/abstractions/log.service"; import { PlatformUtilsService } from "../../platform/abstractions/platform-utils.service"; import { AbstractStorageService } from "../../platform/abstractions/storage.service"; import { StorageLocation } from "../../platform/enums"; @@ -48,6 +49,7 @@ describe("deviceTrustService", () => { const i18nService = mock(); const platformUtilsService = mock(); const secureStorageService = mock(); + const logService = mock(); const userDecryptionOptionsService = mock(); const decryptionOptions = new BehaviorSubject(null); @@ -726,6 +728,7 @@ describe("deviceTrustService", () => { stateProvider, secureStorageService, userDecryptionOptionsService, + logService, ); } });