Add logging for userkey / device trust rotation (#11682)
* Add logging for userkey rotation * Fix tests
This commit is contained in:
parent
a9d9130f01
commit
9264e6775c
|
@ -7,6 +7,7 @@ import { UserVerificationService } from "@bitwarden/common/auth/abstractions/use
|
||||||
import { WebauthnRotateCredentialRequest } from "@bitwarden/common/auth/models/request/webauthn-rotate-credential.request";
|
import { WebauthnRotateCredentialRequest } from "@bitwarden/common/auth/models/request/webauthn-rotate-credential.request";
|
||||||
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
|
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
|
||||||
import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service";
|
import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service";
|
||||||
|
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
|
||||||
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
|
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
|
||||||
import { SendWithIdRequest } from "@bitwarden/common/tools/send/models/request/send-with-id.request";
|
import { SendWithIdRequest } from "@bitwarden/common/tools/send/models/request/send-with-id.request";
|
||||||
import { SendService } from "@bitwarden/common/tools/send/services/send.service.abstraction";
|
import { SendService } from "@bitwarden/common/tools/send/services/send.service.abstraction";
|
||||||
|
@ -44,6 +45,7 @@ describe("KeyRotationService", () => {
|
||||||
let mockConfigService: MockProxy<ConfigService>;
|
let mockConfigService: MockProxy<ConfigService>;
|
||||||
let mockSyncService: MockProxy<SyncService>;
|
let mockSyncService: MockProxy<SyncService>;
|
||||||
let mockWebauthnLoginAdminService: MockProxy<WebauthnLoginAdminService>;
|
let mockWebauthnLoginAdminService: MockProxy<WebauthnLoginAdminService>;
|
||||||
|
let mockLogService: MockProxy<LogService>;
|
||||||
|
|
||||||
const mockUser = {
|
const mockUser = {
|
||||||
id: "mockUserId" as UserId,
|
id: "mockUserId" as UserId,
|
||||||
|
@ -66,6 +68,7 @@ describe("KeyRotationService", () => {
|
||||||
mockConfigService = mock<ConfigService>();
|
mockConfigService = mock<ConfigService>();
|
||||||
mockSyncService = mock<SyncService>();
|
mockSyncService = mock<SyncService>();
|
||||||
mockWebauthnLoginAdminService = mock<WebauthnLoginAdminService>();
|
mockWebauthnLoginAdminService = mock<WebauthnLoginAdminService>();
|
||||||
|
mockLogService = mock<LogService>();
|
||||||
|
|
||||||
keyRotationService = new UserKeyRotationService(
|
keyRotationService = new UserKeyRotationService(
|
||||||
mockUserVerificationService,
|
mockUserVerificationService,
|
||||||
|
@ -80,6 +83,7 @@ describe("KeyRotationService", () => {
|
||||||
mockEncryptService,
|
mockEncryptService,
|
||||||
mockSyncService,
|
mockSyncService,
|
||||||
mockWebauthnLoginAdminService,
|
mockWebauthnLoginAdminService,
|
||||||
|
mockLogService,
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
|
@ -7,6 +7,7 @@ import { UserVerificationService } from "@bitwarden/common/auth/abstractions/use
|
||||||
import { VerificationType } from "@bitwarden/common/auth/enums/verification-type";
|
import { VerificationType } from "@bitwarden/common/auth/enums/verification-type";
|
||||||
import { MasterPasswordVerification } from "@bitwarden/common/auth/types/verification";
|
import { MasterPasswordVerification } from "@bitwarden/common/auth/types/verification";
|
||||||
import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service";
|
import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service";
|
||||||
|
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
|
||||||
import { EncryptedString } from "@bitwarden/common/platform/models/domain/enc-string";
|
import { EncryptedString } from "@bitwarden/common/platform/models/domain/enc-string";
|
||||||
import { SendService } from "@bitwarden/common/tools/send/services/send.service.abstraction";
|
import { SendService } from "@bitwarden/common/tools/send/services/send.service.abstraction";
|
||||||
import { UserId } from "@bitwarden/common/types/guid";
|
import { UserId } from "@bitwarden/common/types/guid";
|
||||||
|
@ -38,6 +39,7 @@ export class UserKeyRotationService {
|
||||||
private encryptService: EncryptService,
|
private encryptService: EncryptService,
|
||||||
private syncService: SyncService,
|
private syncService: SyncService,
|
||||||
private webauthnLoginAdminService: WebauthnLoginAdminService,
|
private webauthnLoginAdminService: WebauthnLoginAdminService,
|
||||||
|
private logService: LogService,
|
||||||
) {}
|
) {}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -48,11 +50,14 @@ export class UserKeyRotationService {
|
||||||
masterPassword: string,
|
masterPassword: string,
|
||||||
user: { id: UserId } & AccountInfo,
|
user: { id: UserId } & AccountInfo,
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
|
this.logService.info("[Userkey rotation] Starting user key rotation...");
|
||||||
if (!masterPassword) {
|
if (!masterPassword) {
|
||||||
|
this.logService.info("[Userkey rotation] Invalid master password provided. Aborting!");
|
||||||
throw new Error("Invalid master password");
|
throw new Error("Invalid master password");
|
||||||
}
|
}
|
||||||
|
|
||||||
if ((await this.syncService.getLastSync()) === null) {
|
if ((await this.syncService.getLastSync()) === null) {
|
||||||
|
this.logService.info("[Userkey rotation] Client was never synced. Aborting!");
|
||||||
throw new Error(
|
throw new Error(
|
||||||
"The local vault is de-synced and the keys cannot be rotated. Please log out and log back in to resolve this issue.",
|
"The local vault is de-synced and the keys cannot be rotated. Please log out and log back in to resolve this issue.",
|
||||||
);
|
);
|
||||||
|
@ -74,6 +79,7 @@ export class UserKeyRotationService {
|
||||||
const [newUserKey, newEncUserKey] = await this.keyService.makeUserKey(masterKey);
|
const [newUserKey, newEncUserKey] = await this.keyService.makeUserKey(masterKey);
|
||||||
|
|
||||||
if (!newUserKey || !newEncUserKey) {
|
if (!newUserKey || !newEncUserKey) {
|
||||||
|
this.logService.info("[Userkey rotation] User key could not be created. Aborting!");
|
||||||
throw new Error("User key could not be created");
|
throw new Error("User key could not be created");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -91,6 +97,9 @@ export class UserKeyRotationService {
|
||||||
// Note: We distribute the legacy key, but not all domains actually use it. If any of those
|
// Note: We distribute the legacy key, but not all domains actually use it. If any of those
|
||||||
// domains break their legacy support it will break the migration process for legacy users.
|
// domains break their legacy support it will break the migration process for legacy users.
|
||||||
const originalUserKey = await this.keyService.getUserKeyWithLegacySupport(user.id);
|
const originalUserKey = await this.keyService.getUserKeyWithLegacySupport(user.id);
|
||||||
|
const isMasterKey =
|
||||||
|
(await firstValueFrom(this.keyService.userKey$(user.id))) != originalUserKey;
|
||||||
|
this.logService.info("[Userkey rotation] Is legacy user: " + isMasterKey);
|
||||||
|
|
||||||
// Add re-encrypted data
|
// Add re-encrypted data
|
||||||
request.privateKey = await this.encryptPrivateKey(newUserKey, user.id);
|
request.privateKey = await this.encryptPrivateKey(newUserKey, user.id);
|
||||||
|
@ -151,10 +160,14 @@ export class UserKeyRotationService {
|
||||||
request.webauthnKeys = rotatedWebauthnKeys;
|
request.webauthnKeys = rotatedWebauthnKeys;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
this.logService.info("[Userkey rotation] Posting user key rotation request to server");
|
||||||
await this.apiService.postUserKeyUpdate(request);
|
await this.apiService.postUserKeyUpdate(request);
|
||||||
|
this.logService.info("[Userkey rotation] Userkey rotation request posted to server");
|
||||||
|
|
||||||
// TODO PM-2199: Add device trust rotation support to the user key rotation endpoint
|
// TODO PM-2199: Add device trust rotation support to the user key rotation endpoint
|
||||||
|
this.logService.info("[Userkey rotation] Rotating device trust...");
|
||||||
await this.deviceTrustService.rotateDevicesTrust(user.id, newUserKey, masterPasswordHash);
|
await this.deviceTrustService.rotateDevicesTrust(user.id, newUserKey, masterPasswordHash);
|
||||||
|
this.logService.info("[Userkey rotation] Device trust rotation completed");
|
||||||
}
|
}
|
||||||
|
|
||||||
private async encryptPrivateKey(
|
private async encryptPrivateKey(
|
||||||
|
|
|
@ -175,6 +175,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction {
|
||||||
newUserKey: UserKey,
|
newUserKey: UserKey,
|
||||||
masterPasswordHash: string,
|
masterPasswordHash: string,
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
|
this.logService.info("[Device trust rotation] Rotating device trust...");
|
||||||
if (!userId) {
|
if (!userId) {
|
||||||
throw new Error("UserId is required. Cannot rotate device's trust.");
|
throw new Error("UserId is required. Cannot rotate device's trust.");
|
||||||
}
|
}
|
||||||
|
@ -183,11 +184,15 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction {
|
||||||
if (currentDeviceKey == null) {
|
if (currentDeviceKey == null) {
|
||||||
// If the current device doesn't have a device key available to it, then we can't
|
// If the current device doesn't have a device key available to it, then we can't
|
||||||
// rotate any trust at all, so early return.
|
// rotate any trust at all, so early return.
|
||||||
|
this.logService.info("[Device trust rotation] No device key available to rotate trust!");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
// At this point of rotating their keys, they should still have their old user key in state
|
// At this point of rotating their keys, they should still have their old user key in state
|
||||||
const oldUserKey = await firstValueFrom(this.keyService.userKey$(userId));
|
const oldUserKey = await firstValueFrom(this.keyService.userKey$(userId));
|
||||||
|
if (oldUserKey == newUserKey) {
|
||||||
|
this.logService.info("[Device trust rotation] Old user key is the same as the new user key.");
|
||||||
|
}
|
||||||
|
|
||||||
const deviceIdentifier = await this.appIdService.getAppId();
|
const deviceIdentifier = await this.appIdService.getAppId();
|
||||||
const secretVerificationRequest = new SecretVerificationRequest();
|
const secretVerificationRequest = new SecretVerificationRequest();
|
||||||
|
@ -229,7 +234,12 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction {
|
||||||
trustRequest.currentDevice = currentDeviceUpdateRequest;
|
trustRequest.currentDevice = currentDeviceUpdateRequest;
|
||||||
trustRequest.otherDevices = [];
|
trustRequest.otherDevices = [];
|
||||||
|
|
||||||
|
this.logService.info(
|
||||||
|
"[Device trust rotation] Posting device trust update with current device:",
|
||||||
|
deviceIdentifier,
|
||||||
|
);
|
||||||
await this.devicesApiService.updateTrust(trustRequest, deviceIdentifier);
|
await this.devicesApiService.updateTrust(trustRequest, deviceIdentifier);
|
||||||
|
this.logService.info("[Device trust rotation] Device trust update posted successfully.");
|
||||||
}
|
}
|
||||||
|
|
||||||
async getDeviceKey(userId: UserId): Promise<DeviceKey | null> {
|
async getDeviceKey(userId: UserId): Promise<DeviceKey | null> {
|
||||||
|
|
|
@ -193,6 +193,7 @@ export class NotificationsService implements NotificationsServiceAbstraction {
|
||||||
break;
|
break;
|
||||||
case NotificationType.LogOut:
|
case NotificationType.LogOut:
|
||||||
if (isAuthenticated) {
|
if (isAuthenticated) {
|
||||||
|
this.logService.info("[Notifications Service] Received logout notification");
|
||||||
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
|
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
|
||||||
// eslint-disable-next-line @typescript-eslint/no-floating-promises
|
// eslint-disable-next-line @typescript-eslint/no-floating-promises
|
||||||
this.logoutCallback("logoutNotification");
|
this.logoutCallback("logoutNotification");
|
||||||
|
|
Loading…
Reference in New Issue