mirror of
https://github.com/bitwarden/browser
synced 2025-12-06 00:13:28 +00:00
feat(auth-tech-debt): [PM-24103] Remove Get User Key to UserKey$ (#16589)
* fix(auth-tech-debt): [PM-24103] Remove Get User Key to UserKey$ - Fixed and updated tests. * fix(auth-tech-debt): [PM-24103] Remove Get User Key to UserKey$ - Fixed test variable being made more vague.
This commit is contained in:
committed by
GitHub
parent
9b2fbdba1c
commit
94cb1fe07b
@@ -47,7 +47,7 @@ export class SetPinComponent implements OnInit {
|
||||
}
|
||||
|
||||
const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
|
||||
const userKey = await this.keyService.getUserKey();
|
||||
const userKey = await firstValueFrom(this.keyService.userKey$(userId));
|
||||
|
||||
const userKeyEncryptedPin = await this.pinService.createUserKeyEncryptedPin(
|
||||
pinFormControl.value,
|
||||
|
||||
@@ -1291,6 +1291,7 @@ const safeProviders: SafeProvider[] = [
|
||||
ApiServiceAbstraction,
|
||||
StateProvider,
|
||||
AuthRequestApiServiceAbstraction,
|
||||
AccountServiceAbstraction,
|
||||
],
|
||||
}),
|
||||
safeProvider({
|
||||
|
||||
@@ -55,6 +55,7 @@ export abstract class AuthRequestServiceAbstraction {
|
||||
* Approve or deny an auth request.
|
||||
* @param approve True to approve, false to deny.
|
||||
* @param authRequest The auth request to approve or deny, must have an id and key.
|
||||
* @param activeUserId the active user id
|
||||
* @returns The updated auth request, the `requestApproved` field will be true if
|
||||
* approval was successful.
|
||||
* @throws If the auth request is missing an id or key.
|
||||
|
||||
@@ -337,7 +337,7 @@ describe("LoginStrategy", () => {
|
||||
const tokenResponse = identityTokenResponseFactory();
|
||||
tokenResponse.privateKey = null;
|
||||
keyService.makeKeyPair.mockResolvedValue(["PUBLIC_KEY", new EncString("PRIVATE_KEY")]);
|
||||
keyService.getUserKey.mockResolvedValue(userKey);
|
||||
keyService.userKey$.mockReturnValue(new BehaviorSubject<UserKey>(userKey).asObservable());
|
||||
|
||||
apiService.postIdentityToken.mockResolvedValue(tokenResponse);
|
||||
masterPasswordService.masterKeySubject.next(masterKey);
|
||||
@@ -356,9 +356,11 @@ describe("LoginStrategy", () => {
|
||||
});
|
||||
|
||||
it("throws if userKey is CoseEncrypt0 (V2 encryption) in createKeyPairForOldAccount", async () => {
|
||||
keyService.getUserKey.mockResolvedValue({
|
||||
inner: () => ({ type: 7 }),
|
||||
} as UserKey);
|
||||
keyService.userKey$.mockReturnValue(
|
||||
new BehaviorSubject<UserKey>({
|
||||
inner: () => ({ type: 7 }),
|
||||
} as unknown as UserKey).asObservable(),
|
||||
);
|
||||
await expect(passwordLoginStrategy["createKeyPairForOldAccount"](userId)).resolves.toBe(
|
||||
undefined,
|
||||
);
|
||||
|
||||
@@ -306,7 +306,11 @@ export abstract class LoginStrategy {
|
||||
|
||||
protected async createKeyPairForOldAccount(userId: UserId) {
|
||||
try {
|
||||
const userKey = await this.keyService.getUserKey(userId);
|
||||
const userKey = await firstValueFrom(this.keyService.userKey$(userId));
|
||||
if (userKey === null) {
|
||||
throw new Error("User key is null when creating key pair for old account");
|
||||
}
|
||||
|
||||
if (userKey.inner().type == EncryptionType.CoseEncrypt0) {
|
||||
throw new Error("Cannot create key pair for account on V2 encryption");
|
||||
}
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
import { mock } from "jest-mock-extended";
|
||||
import { firstValueFrom } from "rxjs";
|
||||
import { firstValueFrom, of } from "rxjs";
|
||||
|
||||
import { ApiService } from "@bitwarden/common/abstractions/api.service";
|
||||
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
|
||||
import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response";
|
||||
import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
|
||||
import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string";
|
||||
@@ -9,11 +10,11 @@ import { FakeMasterPasswordService } from "@bitwarden/common/key-management/mast
|
||||
import { ListResponse } from "@bitwarden/common/models/response/list.response";
|
||||
import { AuthRequestPushNotification } from "@bitwarden/common/models/response/notification.response";
|
||||
import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
|
||||
import { Utils } from "@bitwarden/common/platform/misc/utils";
|
||||
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
|
||||
import { StateProvider } from "@bitwarden/common/platform/state";
|
||||
import { UserId } from "@bitwarden/common/types/guid";
|
||||
import { MasterKey, UserKey } from "@bitwarden/common/types/key";
|
||||
import { newGuid } from "@bitwarden/guid";
|
||||
import { KeyService } from "@bitwarden/key-management";
|
||||
|
||||
import { DefaultAuthRequestApiService } from "./auth-request-api.service";
|
||||
@@ -29,10 +30,11 @@ describe("AuthRequestService", () => {
|
||||
const encryptService = mock<EncryptService>();
|
||||
const apiService = mock<ApiService>();
|
||||
const authRequestApiService = mock<DefaultAuthRequestApiService>();
|
||||
const accountService = mock<AccountService>();
|
||||
|
||||
let mockPrivateKey: Uint8Array;
|
||||
let mockPublicKey: Uint8Array;
|
||||
const mockUserId = Utils.newGuid() as UserId;
|
||||
const mockUserId = newGuid() as UserId;
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
@@ -46,6 +48,7 @@ describe("AuthRequestService", () => {
|
||||
apiService,
|
||||
stateProvider,
|
||||
authRequestApiService,
|
||||
accountService,
|
||||
);
|
||||
|
||||
mockPrivateKey = new Uint8Array(64);
|
||||
@@ -95,6 +98,8 @@ describe("AuthRequestService", () => {
|
||||
const authRequestNoId = new AuthRequestResponse({ id: "", key: "KEY" });
|
||||
const authRequestNoPublicKey = new AuthRequestResponse({ id: "123", publicKey: "" });
|
||||
|
||||
accountService.activeAccount$ = of({ id: mockUserId } as any);
|
||||
|
||||
await expect(sut.approveOrDenyAuthRequest(true, authRequestNoId)).rejects.toThrow(
|
||||
"Auth request has no id",
|
||||
);
|
||||
@@ -104,8 +109,9 @@ describe("AuthRequestService", () => {
|
||||
});
|
||||
|
||||
it("should use the user key if the master key and hash do not exist", async () => {
|
||||
keyService.getUserKey.mockResolvedValueOnce(
|
||||
new SymmetricCryptoKey(new Uint8Array(64)) as UserKey,
|
||||
accountService.activeAccount$ = of({ id: mockUserId } as any);
|
||||
keyService.userKey$.mockReturnValue(
|
||||
of(new SymmetricCryptoKey(new Uint8Array(64)) as UserKey),
|
||||
);
|
||||
|
||||
await sut.approveOrDenyAuthRequest(
|
||||
|
||||
@@ -4,9 +4,11 @@ import { Observable, Subject, defer, firstValueFrom, map } from "rxjs";
|
||||
import { Jsonify } from "type-fest";
|
||||
|
||||
import { ApiService } from "@bitwarden/common/abstractions/api.service";
|
||||
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
|
||||
import { AdminAuthRequestStorable } from "@bitwarden/common/auth/models/domain/admin-auth-req-storable";
|
||||
import { PasswordlessAuthRequest } from "@bitwarden/common/auth/models/request/passwordless-auth.request";
|
||||
import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response";
|
||||
import { getUserId } from "@bitwarden/common/auth/services/account.service";
|
||||
import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
|
||||
import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string";
|
||||
import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction";
|
||||
@@ -56,6 +58,7 @@ export class AuthRequestService implements AuthRequestServiceAbstraction {
|
||||
private apiService: ApiService,
|
||||
private stateProvider: StateProvider,
|
||||
private authRequestApiService: AuthRequestApiServiceAbstraction,
|
||||
private accountService: AccountService,
|
||||
) {
|
||||
this.authRequestPushNotification$ = this.authRequestPushNotificationSubject.asObservable();
|
||||
this.adminLoginApproved$ = this.adminLoginApprovedSubject.asObservable();
|
||||
@@ -124,15 +127,19 @@ export class AuthRequestService implements AuthRequestServiceAbstraction {
|
||||
approve: boolean,
|
||||
authRequest: AuthRequestResponse,
|
||||
): Promise<AuthRequestResponse> {
|
||||
const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId));
|
||||
|
||||
if (!authRequest.id) {
|
||||
throw new Error("Auth request has no id");
|
||||
}
|
||||
if (!authRequest.publicKey) {
|
||||
throw new Error("Auth request has no public key");
|
||||
}
|
||||
if (activeUserId == null) {
|
||||
throw new Error("User ID is required");
|
||||
}
|
||||
const pubKey = Utils.fromB64ToArray(authRequest.publicKey);
|
||||
|
||||
const keyToEncrypt = await this.keyService.getUserKey();
|
||||
const keyToEncrypt = await firstValueFrom(this.keyService.userKey$(activeUserId));
|
||||
const encryptedKey = await this.encryptService.encapsulateKeyUnsigned(keyToEncrypt, pubKey);
|
||||
|
||||
const response = new PasswordlessAuthRequest(
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { mock, MockProxy } from "jest-mock-extended";
|
||||
import { BehaviorSubject } from "rxjs";
|
||||
import { BehaviorSubject, of } from "rxjs";
|
||||
|
||||
// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
|
||||
// eslint-disable-next-line no-restricted-imports
|
||||
@@ -103,7 +103,7 @@ describe("PasswordResetEnrollmentServiceImplementation", () => {
|
||||
};
|
||||
activeAccountSubject.next(Object.assign(user1AccountInfo, { id: "userId" as UserId }));
|
||||
|
||||
keyService.getUserKey.mockResolvedValue({ key: "key" } as any);
|
||||
keyService.userKey$.mockReturnValue(of({ key: "key" } as any));
|
||||
encryptService.encapsulateKeyUnsigned.mockResolvedValue(encryptedKey as any);
|
||||
|
||||
await service.enroll("orgId");
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
// FIXME: Update this file to be type safe and remove this and next line
|
||||
// @ts-strict-ignore
|
||||
import { firstValueFrom, map } from "rxjs";
|
||||
import { firstValueFrom } from "rxjs";
|
||||
|
||||
// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
|
||||
// eslint-disable-next-line no-restricted-imports
|
||||
@@ -8,9 +8,11 @@ import {
|
||||
OrganizationUserApiService,
|
||||
OrganizationUserResetPasswordEnrollmentRequest,
|
||||
} from "@bitwarden/admin-console/common";
|
||||
import { getUserId } from "@bitwarden/common/auth/services/account.service";
|
||||
// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
|
||||
// eslint-disable-next-line no-restricted-imports
|
||||
import { KeyService } from "@bitwarden/key-management";
|
||||
import { UserId } from "@bitwarden/user-core";
|
||||
|
||||
import { OrganizationApiServiceAbstraction } from "../../admin-console/abstractions/organization/organization-api.service.abstraction";
|
||||
import { EncryptService } from "../../key-management/crypto/abstractions/encrypt.service";
|
||||
@@ -43,7 +45,7 @@ export class PasswordResetEnrollmentServiceImplementation
|
||||
|
||||
async enroll(organizationId: string): Promise<void>;
|
||||
async enroll(organizationId: string, userId: string, userKey: UserKey): Promise<void>;
|
||||
async enroll(organizationId: string, userId?: string, userKey?: UserKey): Promise<void> {
|
||||
async enroll(organizationId: string, activeUserId?: string, userKey?: UserKey): Promise<void> {
|
||||
const orgKeyResponse = await this.organizationApiService.getKeys(organizationId);
|
||||
if (orgKeyResponse == null) {
|
||||
throw new Error(this.i18nService.t("resetPasswordOrgKeysError"));
|
||||
@@ -51,9 +53,12 @@ export class PasswordResetEnrollmentServiceImplementation
|
||||
|
||||
const orgPublicKey = Utils.fromB64ToArray(orgKeyResponse.publicKey);
|
||||
|
||||
userId =
|
||||
userId ?? (await firstValueFrom(this.accountService.activeAccount$.pipe(map((a) => a?.id))));
|
||||
userKey = userKey ?? (await this.keyService.getUserKey(userId));
|
||||
activeUserId =
|
||||
activeUserId ?? (await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)));
|
||||
if (activeUserId == null) {
|
||||
throw new Error("User ID is required");
|
||||
}
|
||||
userKey = userKey ?? (await firstValueFrom(this.keyService.userKey$(activeUserId as UserId)));
|
||||
// RSA Encrypt user's userKey.key with organization public key
|
||||
const encryptedKey = await this.encryptService.encapsulateKeyUnsigned(userKey, orgPublicKey);
|
||||
|
||||
@@ -62,7 +67,7 @@ export class PasswordResetEnrollmentServiceImplementation
|
||||
|
||||
await this.organizationUserApiService.putOrganizationUserResetPasswordEnrollment(
|
||||
organizationId,
|
||||
userId,
|
||||
activeUserId,
|
||||
resetRequest,
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user