diff --git a/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.ts b/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.ts index f155680d32d..21d15245bed 100644 --- a/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.ts +++ b/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.ts @@ -62,10 +62,10 @@ const matchOptions: IsActiveMatchOptions = { providers: [{ provide: LoginViaAuthRequestCacheService }], }) export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { - private authRequest: AuthRequest | undefined = undefined; private authRequestKeyPair: | { publicKey: Uint8Array | undefined; privateKey: Uint8Array | undefined } | undefined = undefined; + private accessCode: string | undefined = undefined; private authStatus: AuthenticationStatus | undefined = undefined; private showResendNotificationTimeoutSeconds = 12; @@ -200,7 +200,7 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { this.loginViaAuthRequestCacheService.getCachedLoginViaAuthRequestView(); if (cachedAuthRequest) { - await this.processAuthRequestResponse(cachedAuthRequest.authRequestResponse.id); + await this.processAuthRequestResponse(cachedAuthRequest.id); } else { await this.startStandardAuthRequestLogin(); } @@ -223,9 +223,9 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { private async startAdminAuthRequestLogin(): Promise { try { - await this.buildAuthRequest(AuthRequestType.AdminApproval); + const authRequest = await this.buildAuthRequest(AuthRequestType.AdminApproval); - if (!this.authRequest) { + if (!authRequest) { this.logService.error("Auth request failed to build."); return; } @@ -235,9 +235,9 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { return; } - const authRequestResponse = await this.authRequestApiService.postAdminAuthRequest( - this.authRequest as AuthRequest, - ); + const authRequestResponse = + await this.authRequestApiService.postAdminAuthRequest(authRequest); + const adminAuthReqStorable = new AdminAuthRequestStorable({ id: authRequestResponse.id, privateKey: this.authRequestKeyPair.privateKey, @@ -271,24 +271,33 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { // the verifyAndHandleApprovedAuthReq function will receive the public key back // from the looked up auth request and all we need is to make sure that // we can use the cached private key that is associated with it. - this.authRequest = cachedAuthRequest.authRequest; - this.fingerprintPhrase = cachedAuthRequest.fingerprintPhrase; - this.authRequestKeyPair = { - privateKey: cachedAuthRequest.privateKey - ? Utils.fromB64ToArray(cachedAuthRequest.privateKey) - : undefined, - publicKey: undefined, - }; - if (!cachedAuthRequest.authRequestResponse) { - this.logService.error("No cached auth request response."); + if (!this.email) { + this.logService.error("Email not defined when handling an existing auth request."); return; } - if (cachedAuthRequest.authRequestResponse.id) { - await this.anonymousHubService.createHubConnection( - cachedAuthRequest.authRequestResponse.id, - ); + const privateKey = Utils.fromB64ToArray(cachedAuthRequest.privateKey); + + // Re-derive the user's fingerprint phrase + // It is important to not use the server's public key here as it could have been compromised via MITM + const derivedPublicKeyArrayBuffer = + await this.cryptoFunctionService.rsaExtractPublicKey(privateKey); + + this.fingerprintPhrase = await this.authRequestService.getFingerprintPhrase( + this.email, + derivedPublicKeyArrayBuffer, + ); + + // Request still pending response from admin set keypair and create hub connection + // so that any approvals will be received via push notification + this.authRequestKeyPair = { + privateKey: privateKey, + publicKey: undefined, + }; + + if (cachedAuthRequest.id) { + await this.anonymousHubService.createHubConnection(cachedAuthRequest.id); } } } @@ -297,12 +306,12 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { this.showResendNotification = false; try { - await this.buildAuthRequest(AuthRequestType.AuthenticateAndUnlock); + const authRequest = await this.buildAuthRequest(AuthRequestType.AuthenticateAndUnlock); // I tried several ways to get the IDE/linter to play nice with checking for null values // in less code / more efficiently, but it struggles to identify code paths that // are more complicated than this. - if (!this.authRequest) { + if (!authRequest) { this.logService.error("AuthRequest failed to initialize from buildAuthRequest."); return; } @@ -318,14 +327,13 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { } const authRequestResponse: AuthRequestResponse = - await this.authRequestApiService.postAuthRequest(this.authRequest); + await this.authRequestApiService.postAuthRequest(authRequest); if (await this.configService.getFeatureFlag(FeatureFlag.PM9112_DeviceApprovalPersistence)) { this.loginViaAuthRequestCacheService.cacheLoginView( - this.authRequest, - authRequestResponse, - this.fingerprintPhrase, - this.authRequestKeyPair, + authRequestResponse.id, + this.authRequestKeyPair.privateKey, + this.accessCode, ); } @@ -341,7 +349,7 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { }, this.showResendNotificationTimeoutSeconds * 1000); } - private async buildAuthRequest(authRequestType: AuthRequestType): Promise { + private async buildAuthRequest(authRequestType: AuthRequestType): Promise { const authRequestKeyPairArray = await this.cryptoFunctionService.rsaGenerateKeyPair(2048); this.authRequestKeyPair = { @@ -356,12 +364,6 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { return; } - const publicKey = Utils.fromBufferToB64(this.authRequestKeyPair.publicKey); - const accessCode = await this.passwordGenerationService.generatePassword({ - type: "password", - length: 25, - }); - if (!this.email) { this.logService.error("Email not defined when building auth request."); return; @@ -372,12 +374,19 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { this.authRequestKeyPair.publicKey, ); - this.authRequest = new AuthRequest( + this.accessCode = await this.passwordGenerationService.generatePassword({ + type: "password", + length: 25, + }); + + const b64PublicKey = Utils.fromBufferToB64(this.authRequestKeyPair.publicKey); + + return new AuthRequest( this.email, deviceIdentifier, - publicKey, + b64PublicKey, authRequestType, - accessCode, + this.accessCode, ); } @@ -537,8 +546,8 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { await this.reloadCachedStandardAuthRequestIfOneExists(); } else { - if (!this.authRequest) { - this.logService.error("No auth request defined when handling approved auth request."); + if (!this.accessCode) { + this.logService.error("No access code available when handling approved auth request."); return; } @@ -546,7 +555,7 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { // User is unauthenticated, therefore the endpoint requires an access code for user verification. const authRequestResponse = await this.authRequestApiService.getAuthResponse( requestId, - this.authRequest.accessCode, + this.accessCode, ); // Request doesn't exist anymore @@ -695,9 +704,9 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { return; } - if (!this.authRequest) { + if (!this.accessCode) { this.logService.error( - "AuthRequest not defined when building auth request login credentials.", + "Access code not defined when building auth request login credentials.", ); return; } @@ -720,7 +729,7 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { return new AuthRequestLoginCredentials( this.email, - this.authRequest.accessCode, + this.accessCode, requestId, null, // no userKey masterKey, @@ -734,7 +743,7 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { ); return new AuthRequestLoginCredentials( this.email, - this.authRequest.accessCode, + this.accessCode, requestId, userKey, null, // no masterKey diff --git a/libs/auth/src/common/services/auth-request/default-login-via-auth-request-cache.service.spec.ts b/libs/auth/src/common/services/auth-request/default-login-via-auth-request-cache.service.spec.ts index 82ac0f1006d..89b78500f1f 100644 --- a/libs/auth/src/common/services/auth-request/default-login-via-auth-request-cache.service.spec.ts +++ b/libs/auth/src/common/services/auth-request/default-login-via-auth-request-cache.service.spec.ts @@ -2,11 +2,9 @@ import { signal } from "@angular/core"; import { TestBed } from "@angular/core/testing"; import { ViewCacheService } from "@bitwarden/angular/platform/abstractions/view-cache.service"; -import { AuthRequestType } from "@bitwarden/common/auth/enums/auth-request-type"; -import { AuthRequest } from "@bitwarden/common/auth/models/request/auth.request"; -import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response"; import { LoginViaAuthRequestView } from "@bitwarden/common/auth/models/view/login-via-auth-request.view"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; import { LoginViaAuthRequestCacheService } from "./default-login-via-auth-request-cache.service"; @@ -39,12 +37,12 @@ describe("LoginViaAuthRequestCache", () => { }); it("`getCachedLoginViaAuthRequestView` returns the cached data", async () => { - cacheSignal.set({ ...buildAuthenticMockAuthView() }); + cacheSignal.set({ ...buildMockState() }); service = testBed.inject(LoginViaAuthRequestCacheService); await service.init(); expect(service.getCachedLoginViaAuthRequestView()).toEqual({ - ...buildAuthenticMockAuthView(), + ...buildMockState(), }); }); @@ -54,20 +52,19 @@ describe("LoginViaAuthRequestCache", () => { const parameters = buildAuthenticMockAuthView(); - service.cacheLoginView( - parameters.authRequest, - parameters.authRequestResponse, - parameters.fingerprintPhrase, - { publicKey: new Uint8Array(), privateKey: new Uint8Array() }, - ); + service.cacheLoginView(parameters.id, parameters.privateKey, parameters.accessCode); - expect(cacheSignal.set).toHaveBeenCalledWith(parameters); + expect(cacheSignal.set).toHaveBeenCalledWith({ + id: parameters.id, + privateKey: Utils.fromBufferToB64(parameters.privateKey), + accessCode: parameters.accessCode, + }); }); }); describe("feature disabled", () => { beforeEach(async () => { - cacheSignal.set({ ...buildAuthenticMockAuthView() } as LoginViaAuthRequestView); + cacheSignal.set({ ...buildMockState() } as LoginViaAuthRequestView); getFeatureFlag.mockResolvedValue(false); cacheSetMock.mockClear(); @@ -82,12 +79,7 @@ describe("LoginViaAuthRequestCache", () => { it("does not update the signal value", () => { const params = buildAuthenticMockAuthView(); - service.cacheLoginView( - params.authRequest, - params.authRequestResponse, - params.fingerprintPhrase, - { publicKey: new Uint8Array(), privateKey: new Uint8Array() }, - ); + service.cacheLoginView(params.id, params.privateKey, params.accessCode); expect(cacheSignal.set).not.toHaveBeenCalled(); }); @@ -95,17 +87,17 @@ describe("LoginViaAuthRequestCache", () => { const buildAuthenticMockAuthView = () => { return { - fingerprintPhrase: "", - privateKey: "", - publicKey: "", - authRequest: new AuthRequest( - "test@gmail.com", - "deviceIdentifier", - "publicKey", - AuthRequestType.Unlock, - "accessCode", - ), - authRequestResponse: new AuthRequestResponse({}), + id: "testId", + privateKey: new Uint8Array(), + accessCode: "testAccessCode", + }; + }; + + const buildMockState = () => { + return { + id: "testId", + privateKey: Utils.fromBufferToB64(new Uint8Array()), + accessCode: "testAccessCode", }; }; }); diff --git a/libs/auth/src/common/services/auth-request/default-login-via-auth-request-cache.service.ts b/libs/auth/src/common/services/auth-request/default-login-via-auth-request-cache.service.ts index 30ba8879546..493fea5c14b 100644 --- a/libs/auth/src/common/services/auth-request/default-login-via-auth-request-cache.service.ts +++ b/libs/auth/src/common/services/auth-request/default-login-via-auth-request-cache.service.ts @@ -1,8 +1,6 @@ import { inject, Injectable, WritableSignal } from "@angular/core"; import { ViewCacheService } from "@bitwarden/angular/platform/abstractions/view-cache.service"; -import { AuthRequest } from "@bitwarden/common/auth/models/request/auth.request"; -import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response"; import { LoginViaAuthRequestView } from "@bitwarden/common/auth/models/view/login-via-auth-request.view"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; @@ -45,12 +43,7 @@ export class LoginViaAuthRequestCacheService { /** * Update the cache with the new LoginView. */ - cacheLoginView( - authRequest: AuthRequest, - authRequestResponse: AuthRequestResponse, - fingerprintPhrase: string, - keys: { privateKey: Uint8Array | undefined; publicKey: Uint8Array | undefined }, - ): void { + cacheLoginView(id: string, privateKey: Uint8Array, accessCode: string): void { if (!this.featureEnabled) { return; } @@ -59,11 +52,9 @@ export class LoginViaAuthRequestCacheService { // data can be properly formed when json-ified. If not done, they are not stored properly and // will not be parsable by the cryptography library after coming out of storage. this.defaultLoginViaAuthRequestCache.set({ - authRequest, - authRequestResponse, - fingerprintPhrase, - privateKey: keys.privateKey ? Utils.fromBufferToB64(keys.privateKey.buffer) : undefined, - publicKey: keys.publicKey ? Utils.fromBufferToB64(keys.publicKey.buffer) : undefined, + id: id, + privateKey: Utils.fromBufferToB64(privateKey.buffer), + accessCode: accessCode, } as LoginViaAuthRequestView); } diff --git a/libs/common/src/auth/models/view/login-via-auth-request.view.ts b/libs/common/src/auth/models/view/login-via-auth-request.view.ts index 0691b8efd86..c5b4e5df967 100644 --- a/libs/common/src/auth/models/view/login-via-auth-request.view.ts +++ b/libs/common/src/auth/models/view/login-via-auth-request.view.ts @@ -1,15 +1,11 @@ import { Jsonify } from "type-fest"; -import { AuthRequest } from "@bitwarden/common/auth/models/request/auth.request"; -import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response"; import { View } from "@bitwarden/common/models/view/view"; export class LoginViaAuthRequestView implements View { - authRequest: AuthRequest | undefined = undefined; - authRequestResponse: AuthRequestResponse | undefined = undefined; - fingerprintPhrase: string | undefined = undefined; + id: string | undefined = undefined; + accessCode: string | undefined = undefined; privateKey: string | undefined = undefined; - publicKey: string | undefined = undefined; static fromJSON(obj: Partial>): LoginViaAuthRequestView { return Object.assign(new LoginViaAuthRequestView(), obj);