Skip to content

Commit

Permalink
rename key rotation service to user key and add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
jlf0dev committed Dec 15, 2023
1 parent 5469ed5 commit b63e987
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 32 deletions.
9 changes: 0 additions & 9 deletions apps/web/src/app/auth/key-rotation/key-rotation.module.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { UpdateKeyRequest } from "./request/update-key.request";

@Injectable()
export class KeyRotationApiService {
export class UserKeyRotationApiService {
readonly apiService = inject(ApiService);

postAccountKey(request: UpdateKeyRequest): Promise<any> {
postUserKeyUpdate(request: UpdateKeyRequest): Promise<any> {
return this.apiService.send("POST", "/accounts/key", request, true, false);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { NgModule } from "@angular/core";

import { UserKeyRotationApiService } from "./user-key-rotation-api.service";
import { UserKeyRotationService } from "./user-key-rotation.service";

@NgModule({
providers: [UserKeyRotationService, UserKeyRotationApiService],
})
export class UserKeyRotationModule {}
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ import { OrganizationUserResetPasswordService } from "../../admin-console/organi
import { StateService } from "../../core";
import { EmergencyAccessService } from "../emergency-access";

import { KeyRotationApiService } from "./key-rotation-api.service";
import { KeyRotationService } from "./key-rotation.service";
import { UserKeyRotationApiService } from "./user-key-rotation-api.service";
import { UserKeyRotationService } from "./user-key-rotation.service";

describe("KeyRotationService", () => {
let keyRotationService: KeyRotationService;
let keyRotationService: UserKeyRotationService;

let mockApiService: MockProxy<KeyRotationApiService>;
let mockApiService: MockProxy<UserKeyRotationApiService>;
let mockCipherService: MockProxy<CipherService>;
let mockFolderService: MockProxy<FolderService>;
let mockSendService: MockProxy<SendService>;
Expand All @@ -44,7 +44,7 @@ describe("KeyRotationService", () => {
let mockConfigService: MockProxy<ConfigServiceAbstraction>;

beforeAll(() => {
mockApiService = mock<KeyRotationApiService>();
mockApiService = mock<UserKeyRotationApiService>();
mockCipherService = mock<CipherService>();
mockFolderService = mock<FolderService>();
mockSendService = mock<SendService>();
Expand All @@ -56,7 +56,7 @@ describe("KeyRotationService", () => {
mockStateService = mock<StateService>();
mockConfigService = mock<ConfigServiceAbstraction>();

keyRotationService = new KeyRotationService(
keyRotationService = new UserKeyRotationService(
mockApiService,
mockCipherService,
mockFolderService,
Expand Down Expand Up @@ -137,6 +137,15 @@ describe("KeyRotationService", () => {
});
});

it("rotates the user key and encrypted data", async () => {
await keyRotationService.rotateUserKeyAndEncryptedData("mockMasterPassword");

expect(mockApiService.postUserKeyUpdate).toHaveBeenCalled();
const arg = mockApiService.postUserKeyUpdate.mock.calls[0][0];
expect(arg.ciphers.length).toBe(2);
expect(arg.folders.length).toBe(2);
});

it("throws if master password provided is falsey", async () => {
await expect(keyRotationService.rotateUserKeyAndEncryptedData("")).rejects.toThrow();
});
Expand Down Expand Up @@ -168,13 +177,13 @@ describe("KeyRotationService", () => {

await keyRotationService.rotateUserKeyAndEncryptedData("mockMasterPassword");

expect(mockApiService.postAccountKey).toHaveBeenCalled();
expect(mockApiService.postUserKeyUpdate).toHaveBeenCalled();
expect(mockEmergencyAccessService.postLegacyRotation).toHaveBeenCalled();
expect(mockResetPasswordService.postLegacyRotation).toHaveBeenCalled();
});

it("throws if server rotation fails", async () => {
mockApiService.postAccountKey.mockRejectedValueOnce(new Error("mockError"));
mockApiService.postUserKeyUpdate.mockRejectedValueOnce(new Error("mockError"));

await expect(
keyRotationService.rotateUserKeyAndEncryptedData("mockMasterPassword"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ import { FolderWithIdRequest } from "@bitwarden/common/vault/models/request/fold
import { OrganizationUserResetPasswordService } from "../../admin-console/organizations/members/services/organization-user-reset-password/organization-user-reset-password.service";
import { EmergencyAccessService } from "../emergency-access";

import { KeyRotationApiService } from "./key-rotation-api.service";
import { UpdateKeyRequest } from "./request/update-key.request";
import { UserKeyRotationApiService } from "./user-key-rotation-api.service";

@Injectable()
export class KeyRotationService {
export class UserKeyRotationService {
constructor(
private apiService: KeyRotationApiService,
private apiService: UserKeyRotationApiService,
private cipherService: CipherService,
private folderService: FolderService,
private sendService: SendService,
Expand All @@ -37,6 +37,10 @@ export class KeyRotationService {
private configService: ConfigServiceAbstraction,
) {}

/**
* Creates a new user key and re-encrypts all required data with the it.
* @param masterPassword current master password (used for validation)
*/
async rotateUserKeyAndEncryptedData(masterPassword: string): Promise<void> {
if (!masterPassword) {
throw new Error("Invalid master password");
Expand Down Expand Up @@ -81,7 +85,7 @@ export class KeyRotationService {
request.resetPasswordKeys = await this.resetPasswordService.getRotatedKeys(newUserKey);

if (await this.configService.getFeatureFlag<boolean>(FeatureFlag.KeyRotationImprovements)) {
await this.apiService.postAccountKey(request);
await this.apiService.postUserKeyUpdate(request);
} else {
await this.rotateUserKeyAndEncryptedDataLegacy(request);
}
Expand Down Expand Up @@ -127,7 +131,7 @@ export class KeyRotationService {

private async rotateUserKeyAndEncryptedDataLegacy(request: UpdateKeyRequest): Promise<void> {
// Update keys, ciphers, folders, and sends
await this.apiService.postAccountKey(request);
await this.apiService.postUserKeyUpdate(request);

// Update emergency access keys
await this.emergencyAccessService.postLegacyRotation(request.emergencyAccessKeys);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ import { MessagingService } from "@bitwarden/common/platform/abstractions/messag
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";

import { SharedModule } from "../../shared";
import { KeyRotationModule } from "../key-rotation/key-rotation.module";
import { KeyRotationService } from "../key-rotation/key-rotation.service";
import { UserKeyRotationModule } from "../key-rotation/user-key-rotation.module";
import { UserKeyRotationService } from "../key-rotation/user-key-rotation.service";

// The master key was originally used to encrypt user data, before the user key was introduced.
// This component is used to migrate from the old encryption scheme to the new one.
@Component({
standalone: true,
imports: [SharedModule, KeyRotationModule],
imports: [SharedModule, UserKeyRotationModule],
templateUrl: "migrate-legacy-encryption.component.html",
})
export class MigrateFromLegacyEncryptionComponent {
Expand All @@ -24,7 +24,7 @@ export class MigrateFromLegacyEncryptionComponent {
});

constructor(
private keyRotationService: KeyRotationService,
private keyRotationService: UserKeyRotationService,
private i18nService: I18nService,
private platformUtilsService: PlatformUtilsService,
private cryptoService: CryptoService,
Expand Down
4 changes: 2 additions & 2 deletions apps/web/src/app/auth/settings/change-password.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.servi
import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction";
import { DialogService } from "@bitwarden/components";

import { KeyRotationService } from "../key-rotation/key-rotation.service";
import { UserKeyRotationService } from "../key-rotation/user-key-rotation.service";

@Component({
selector: "app-change-password",
Expand Down Expand Up @@ -53,7 +53,7 @@ export class ChangePasswordComponent extends BaseChangePasswordComponent {
dialogService: DialogService,
private userVerificationService: UserVerificationService,
private configService: ConfigServiceAbstraction,
private keyRotationService: KeyRotationService,
private keyRotationService: UserKeyRotationService,
) {
super(
i18nService,
Expand Down
4 changes: 2 additions & 2 deletions apps/web/src/app/auth/settings/settings.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { PasswordCalloutComponent } from "@bitwarden/auth";

import { SharedModule } from "../../shared";
import { EmergencyAccessModule } from "../emergency-access";
import { KeyRotationModule } from "../key-rotation/key-rotation.module";
import { UserKeyRotationModule } from "../key-rotation/user-key-rotation.module";

import { ChangePasswordComponent } from "./change-password.component";
import { WebauthnLoginSettingsModule } from "./webauthn-login-settings";
Expand All @@ -15,7 +15,7 @@ import { WebauthnLoginSettingsModule } from "./webauthn-login-settings";
WebauthnLoginSettingsModule,
EmergencyAccessModule,
PasswordCalloutComponent,
KeyRotationModule,
UserKeyRotationModule,
],
declarations: [ChangePasswordComponent],
providers: [],
Expand Down

0 comments on commit b63e987

Please sign in to comment.