From 1af8fe2012063894cd29e755f1f3b0eac94cce1c Mon Sep 17 00:00:00 2001 From: Patrick-Pimentel-Bitwarden Date: Fri, 4 Apr 2025 15:44:48 -0400 Subject: [PATCH] feat(device-approval-persistence): [PM-19380] Device Approval Persistence (#13958) * feat(device-approval-persistence): [PM-19380] Device Approval Persistence - Added lookup on standard auth requests. * fix(device-approval-persistence): [PM-19380] Device Approval Persistence - Fixed issue with null value trying to be parsed from the fromJSON function. --------- Co-authored-by: Todd Martin --- .../local-backed-session-storage.service.ts | 1 - .../abstractions/view-cache.service.ts | 2 +- .../auth_request_login_readme.md | 203 +++++++ .../login-via-auth-request.component.html | 110 ++-- .../login-via-auth-request.component.ts | 564 ++++++++++-------- ...gin-via-auth-request-cache.service.spec.ts | 52 +- ...lt-login-via-auth-request-cache.service.ts | 17 +- .../view/login-via-auth-request.view.ts | 13 +- 8 files changed, 605 insertions(+), 357 deletions(-) create mode 100644 libs/auth/src/angular/login-via-auth-request/auth_request_login_readme.md diff --git a/apps/browser/src/platform/services/local-backed-session-storage.service.ts b/apps/browser/src/platform/services/local-backed-session-storage.service.ts index b030ff4627..0e6922e308 100644 --- a/apps/browser/src/platform/services/local-backed-session-storage.service.ts +++ b/apps/browser/src/platform/services/local-backed-session-storage.service.ts @@ -90,7 +90,6 @@ export class LocalBackedSessionStorageService this.logService.warning( `Possible unnecessary write to local session storage. Key: ${key}`, ); - this.logService.warning(obj as any); } } catch (err) { this.logService.warning(`Error while comparing values for key: ${key}`); diff --git a/libs/angular/src/platform/abstractions/view-cache.service.ts b/libs/angular/src/platform/abstractions/view-cache.service.ts index 0ee09afb81..a282ef6796 100644 --- a/libs/angular/src/platform/abstractions/view-cache.service.ts +++ b/libs/angular/src/platform/abstractions/view-cache.service.ts @@ -9,7 +9,7 @@ type Deserializer = { * @param jsonValue The JSON object representation of your state. * @returns The fully typed version of your state. */ - readonly deserializer?: (jsonValue: Jsonify) => T; + readonly deserializer?: (jsonValue: Jsonify) => T | null; }; type BaseCacheOptions = { diff --git a/libs/auth/src/angular/login-via-auth-request/auth_request_login_readme.md b/libs/auth/src/angular/login-via-auth-request/auth_request_login_readme.md new file mode 100644 index 0000000000..240316f788 --- /dev/null +++ b/libs/auth/src/angular/login-via-auth-request/auth_request_login_readme.md @@ -0,0 +1,203 @@ +# Authentication Flows Documentation + +## Standard Auth Request Flows + +### Flow 1: Unauthed user requests approval from device; Approving device has a masterKey in memory + +1. Unauthed user clicks "Login with device" +2. Navigates to /login-with-device which creates a StandardAuthRequest +3. Receives approval from a device with authRequestPublicKey(masterKey) +4. Decrypts masterKey +5. Decrypts userKey +6. Proceeds to vault + +### Flow 2: Unauthed user requests approval from device; Approving device does NOT have a masterKey in memory + +1. Unauthed user clicks "Login with device" +2. Navigates to /login-with-device which creates a StandardAuthRequest +3. Receives approval from a device with authRequestPublicKey(userKey) +4. Decrypts userKey +5. Proceeds to vault + +**Note:** This flow is an uncommon scenario and relates to TDE off-boarding. The following describes how a user could +get into this flow: + +1. An SSO TD user logs into a device via an Admin auth request approval, therefore this device does NOT have a masterKey + in memory +2. The org admin: + - Changes the member decryption options from "Trusted devices" to "Master password" AND + - Turns off the "Require single sign-on authentication" policy +3. On another device, the user clicks "Login with device", which they can do because the org no longer requires SSO +4. The user approves from the device they had previously logged into with SSO TD, which does NOT have a masterKey in + memory + +### Flow 3: Authed SSO TD user requests approval from device; Approving device has a masterKey in memory + +1. SSO TD user authenticates via SSO +2. Navigates to /login-initiated +3. Clicks "Approve from your other device" +4. Navigates to /login-with-device which creates a StandardAuthRequest +5. Receives approval from device with authRequestPublicKey(masterKey) +6. Decrypts masterKey +7. Decrypts userKey +8. Establishes trust (if required) +9. Proceeds to vault + +### Flow 4: Authed SSO TD user requests approval from device; Approving device does NOT have a masterKey in memory + +1. SSO TD user authenticates via SSO +2. Navigates to /login-initiated +3. Clicks "Approve from your other device" +4. Navigates to /login-with-device which creates a StandardAuthRequest +5. Receives approval from device with authRequestPublicKey(userKey) +6. Decrypts userKey +7. Establishes trust (if required) +8. Proceeds to vault + +## Admin Auth Request Flow + +### Flow: Authed SSO TD user requests admin approval + +1. SSO TD user authenticates via SSO +2. Navigates to /login-initiated +3. Clicks "Request admin approval" +4. Navigates to /admin-approval-requested which creates an AdminAuthRequest +5. Receives approval from device with authRequestPublicKey(userKey) +6. Decrypts userKey +7. Establishes trust (if required) +8. Proceeds to vault + +**Note:** TDE users are required to be enrolled in admin account recovery, which gives the admin access to the user's +userKey. This is how admins are able to send over the authRequestPublicKey(userKey) to the user to allow them to unlock. + +## Summary Table + +| Flow | Auth Status | Clicks Button [active route] | Navigates to | Approving device has masterKey in memory\* | +| --------------- | ----------- | --------------------------------------------------- | ------------------------- | ------------------------------------------------- | +| Standard Flow 1 | unauthed | "Login with device" [/login] | /login-with-device | yes | +| Standard Flow 2 | unauthed | "Login with device" [/login] | /login-with-device | no | +| Standard Flow 3 | authed | "Approve from your other device" [/login-initiated] | /login-with-device | yes | +| Standard Flow 4 | authed | "Approve from your other device" [/login-initiated] | /login-with-device | no | +| Admin Flow | authed | "Request admin approval" [/login-initiated] | /admin-approval-requested | NA - admin requests always send encrypted userKey | + +**Note:** The phrase "in memory" here is important. It is possible for a user to have a master password for their +account, but not have a masterKey IN MEMORY for a specific device. For example, if a user registers an account with a +master password, then joins an SSO TD org, then logs in to a device via SSO and admin auth request, they are now logged +into that device but that device does not have masterKey IN MEMORY. + +## State Management + +### View Cache + +The component uses `LoginViaAuthRequestCacheService` to manage persistent state across extension close and reopen. +This cache stores: + +- Auth Request ID +- Private Key +- Access Code + +The cache is used to: + +1. Preserve authentication state during extension close +2. Allow resumption of pending auth requests +3. Enable processing of approved requests after extension close and reopen. + +### Component State Variables + +Key state variables maintained during the authentication process: + +#### Authentication Keys + +``` +private authRequestKeyPair: { + publicKey: Uint8Array | undefined; + privateKey: Uint8Array | undefined; +} | undefined +``` + +- Stores the RSA key pair used for secure communication +- Generated during auth request initialization +- Required for decrypting approved auth responses + +#### Access Code + +``` +private accessCode: string | undefined +``` + +- 25-character generated password +- Used for retrieving auth responses when user is not authenticated +- Required for standard auth flows + +#### Authentication Status + +``` +private authStatus: AuthenticationStatus | undefined +``` + +- Tracks whether user is authenticated via SSO +- Determines available flows and API endpoints +- Affects navigation paths (`/login` vs `/login-initiated`) + +#### Flow Control + +``` +protected flow = Flow.StandardAuthRequest +``` + +- Determines current authentication flow (Standard vs Admin) +- Affects UI rendering and request handling +- Set based on route and authentication state + +### State Flow Examples + +#### Standard Auth Request Cache Flow + +1. User initiates login with device +2. Component generates auth request and keys +3. Cache service stores: + ``` + cacheLoginView( + authRequestResponse.id, + authRequestKeyPair.privateKey, + accessCode + ) + ``` +4. On page refresh/revisit: + - Component retrieves cached view + - Reestablishes connection using cached credentials + - Continues monitoring for approval + +#### Admin Auth Request State Flow + +1. User requests admin approval +2. Component stores admin request in `AuthRequestService`: + ``` + setAdminAuthRequest( + new AdminAuthRequestStorable({ + id: authRequestResponse.id, + privateKey: authRequestKeyPair.privateKey + }), + userId + ) + ``` +3. On subsequent visits: + - Component checks for existing admin requests + - Either resumes monitoring or starts new request + - Clears state after successful approval + +### State Cleanup + +State cleanup occurs in several scenarios: + +- Component destruction (`ngOnDestroy`) +- Successful authentication +- Request denial or timeout +- Manual navigation away + +Key cleanup actions: + +1. Hub connection termination +2. Cache clearance +3. Admin request state removal +4. Key pair disposal diff --git a/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.html b/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.html index d6b91b960b..38dc874cd0 100644 --- a/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.html +++ b/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.html @@ -1,57 +1,65 @@ -
- -

- {{ "notificationSentDevicePart1" | i18n }} - {{ "notificationSentDeviceAnchor" | i18n }}. {{ "notificationSentDevicePart2" | i18n }} -

-

- {{ "notificationSentDeviceComplete" | i18n }} -

+ +
+ +
+ -
{{ "fingerprintPhraseHeader" | i18n }}
- {{ fingerprintPhrase }} + +
+ +

+ {{ "notificationSentDevicePart1" | i18n }} + {{ "notificationSentDeviceAnchor" | i18n }}. {{ "notificationSentDevicePart2" | i18n }} +

+

+ {{ "notificationSentDeviceComplete" | i18n }} +

- +
{{ "fingerprintPhraseHeader" | i18n }}
+ {{ fingerprintPhrase }} -
- {{ "needAnotherOptionV1" | i18n }}  - {{ - "viewAllLogInOptions" | i18n - }} -
- + - -

{{ "youWillBeNotifiedOnceTheRequestIsApproved" | i18n }}

+
+ {{ "needAnotherOptionV1" | i18n }}  + {{ + "viewAllLogInOptions" | i18n + }} +
+ -
{{ "fingerprintPhraseHeader" | i18n }}
- {{ fingerprintPhrase }} + +

{{ "youWillBeNotifiedOnceTheRequestIsApproved" | i18n }}

-
- {{ "troubleLoggingIn" | i18n }}  - {{ - "viewAllLogInOptions" | i18n - }} -
- -
+
{{ "fingerprintPhraseHeader" | i18n }}
+ {{ fingerprintPhrase }} + +
+ {{ "troubleLoggingIn" | i18n }}  + {{ + "viewAllLogInOptions" | i18n + }} +
+ +
+ 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 ea753a3f0c..8e79bd54f7 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,12 +62,13 @@ 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; + protected loading = true; protected backToRoute = "/login"; protected clientType: ClientType; @@ -110,13 +111,14 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { this.authRequestService.authRequestPushNotification$ .pipe(takeUntilDestroyed()) .subscribe((requestId) => { - this.verifyAndHandleApprovedAuthReq(requestId).catch((e: Error) => { + this.loading = true; + this.handleExistingAuthRequestLogin(requestId).catch((e: Error) => { this.toastService.showToast({ variant: "error", title: this.i18nService.t("error"), message: e.message, }); - + this.loading = false; this.logService.error("Failed to use approved auth request: " + e.message); }); }); @@ -149,24 +151,12 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { } else { await this.initStandardAuthRequestFlow(); } + this.loading = false; } private async initAdminAuthRequestFlow(): Promise { this.flow = Flow.AdminAuthRequest; - // Get email from state for admin auth requests because it is available and also - // prevents it from being lost on refresh as the loginEmailService email does not persist. - this.email = await firstValueFrom( - this.accountService.activeAccount$.pipe(map((a) => a?.email)), - ); - - if (!this.email) { - await this.handleMissingEmail(); - return; - } - - // We only allow a single admin approval request to be active at a time - // so we must check state to see if we have an existing one or not const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; if (!userId) { this.logService.error( @@ -175,12 +165,13 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { return; } - const existingAdminAuthRequest = await this.authRequestService.getAdminAuthRequest(userId); + // [Admin Request Flow State Management] Check cached auth request + const existingAdminAuthRequest = await this.reloadCachedAdminAuthRequest(userId); if (existingAdminAuthRequest) { - await this.handleExistingAdminAuthRequest(existingAdminAuthRequest, userId); + await this.handleExistingAdminAuthRequestLogin(existingAdminAuthRequest, userId); } else { - await this.startAdminAuthRequestLogin(); + await this.handleNewAdminAuthRequestLogin(); } } @@ -194,7 +185,24 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { return; } - await this.startStandardAuthRequestLogin(); + // [Standard Flow State Management] Check cached auth request + const cachedAuthRequest: LoginViaAuthRequestView | null = + this.loginViaAuthRequestCacheService.getCachedLoginViaAuthRequestView(); + + if (cachedAuthRequest) { + this.logService.info("Found cached auth request."); + if (!cachedAuthRequest.id) { + this.logService.error( + "No id on the cached auth request when in the standard auth request flow.", + ); + return; + } + + await this.reloadCachedStandardAuthRequest(cachedAuthRequest); + await this.handleExistingAuthRequestLogin(cachedAuthRequest.id); + } else { + await this.handleNewStandardAuthRequestLogin(); + } } private async handleMissingEmail(): Promise { @@ -212,11 +220,17 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { this.loginViaAuthRequestCacheService.clearCacheLoginView(); } - private async startAdminAuthRequestLogin(): Promise { + private async handleNewAdminAuthRequestLogin(): Promise { try { - await this.buildAuthRequest(AuthRequestType.AdminApproval); + if (!this.email) { + this.logService.error("No email when starting admin auth request login."); + return; + } - if (!this.authRequest) { + // At this point we know there is no + const authRequest = await this.buildAuthRequest(this.email, AuthRequestType.AdminApproval); + + if (!authRequest) { this.logService.error("Auth request failed to build."); return; } @@ -226,9 +240,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, @@ -253,104 +267,154 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { } } - protected async startStandardAuthRequestLogin( - clearCachedRequest: boolean = false, + /** + * We only allow a single admin approval request to be active at a time + * so we can check to see if it's stored in state with the state service + * provider. + * @param userId + * @protected + */ + protected async reloadCachedAdminAuthRequest( + userId: UserId, + ): Promise { + // Get email from state for admin auth requests because it is available and also + // prevents it from being lost on refresh as the loginEmailService email does not persist. + this.email = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((a) => a?.email)), + ); + + if (!this.email) { + await this.handleMissingEmail(); + return null; + } + + return await this.authRequestService.getAdminAuthRequest(userId); + } + + /** + * Restores a cached authentication request into the component's state. + * + * This function checks for the presence of a cached authentication request and, + * if available, updates the component's state with the necessary details to + * continue processing the request. It ensures that the user's email and the + * private key from the cached request are available. + * + * The private key is converted from Base64 to an ArrayBuffer, and a fingerprint + * phrase is derived to verify the request's integrity. The function then sets + * the authentication request key pair in the component's state, preparing it + * to handle any responses or approvals. + * + * @param cachedAuthRequest The request to load into the component state + * @returns Promise to await for completion + */ + protected async reloadCachedStandardAuthRequest( + cachedAuthRequest: LoginViaAuthRequestView, ): Promise { + if (cachedAuthRequest) { + if (!this.email) { + this.logService.error( + "Email not defined when trying to reload cached standard auth request.", + ); + return; + } + + if (!cachedAuthRequest.privateKey) { + this.logService.error( + "No private key on the cached auth request when trying to reload cached standard auth request.", + ); + return; + } + + if (!cachedAuthRequest.accessCode) { + this.logService.error( + "No access code on the cached auth request when trying to reload cached standard auth request.", + ); + return; + } + + 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, + ); + + // We don't need the public key for handling the authentication request because + // the handleExistingAuthRequestLogin 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.authRequestKeyPair = { + privateKey: privateKey, + publicKey: undefined, + }; + + this.accessCode = cachedAuthRequest.accessCode; + } + } + + protected async handleNewStandardAuthRequestLogin(): Promise { this.showResendNotification = false; - if (await this.configService.getFeatureFlag(FeatureFlag.PM9112_DeviceApprovalPersistence)) { - // Used for manually refreshing the auth request when clicking the resend auth request - // on the ui. - if (clearCachedRequest) { - this.loginViaAuthRequestCacheService.clearCacheLoginView(); + try { + if (!this.email) { + this.logService.error("Email not defined when starting standard auth request login."); + return; } - try { - const loginAuthRequestView: LoginViaAuthRequestView | null = - this.loginViaAuthRequestCacheService.getCachedLoginViaAuthRequestView(); + const authRequest = await this.buildAuthRequest( + this.email, + AuthRequestType.AuthenticateAndUnlock, + ); - if (!loginAuthRequestView) { - 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) { - this.logService.error("AuthRequest failed to initialize from buildAuthRequest."); - return; - } - - if (!this.fingerprintPhrase) { - this.logService.error("FingerprintPhrase failed to initialize from buildAuthRequest."); - return; - } - - if (!this.authRequestKeyPair) { - this.logService.error("KeyPair failed to initialize from buildAuthRequest."); - return; - } - - const authRequestResponse: AuthRequestResponse = - await this.authRequestApiService.postAuthRequest(this.authRequest); - - this.loginViaAuthRequestCacheService.cacheLoginView( - this.authRequest, - authRequestResponse, - this.fingerprintPhrase, - this.authRequestKeyPair, - ); - - if (authRequestResponse.id) { - await this.anonymousHubService.createHubConnection(authRequestResponse.id); - } - } else { - // Grab the cached information and store it back in component state. - // We don't need the public key for handling the authentication request because - // 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 = loginAuthRequestView.authRequest; - this.fingerprintPhrase = loginAuthRequestView.fingerprintPhrase; - this.authRequestKeyPair = { - privateKey: loginAuthRequestView.privateKey - ? Utils.fromB64ToArray(loginAuthRequestView.privateKey) - : undefined, - publicKey: undefined, - }; - - if (!loginAuthRequestView.authRequestResponse) { - this.logService.error("No cached auth request response."); - return; - } - - if (loginAuthRequestView.authRequestResponse.id) { - await this.anonymousHubService.createHubConnection( - loginAuthRequestView.authRequestResponse.id, - ); - } - } - } catch (e) { - this.logService.error(e); + // 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 (!authRequest) { + this.logService.error("AuthRequest failed to initialize from buildAuthRequest."); + return; } - } else { - try { - await this.buildAuthRequest(AuthRequestType.AuthenticateAndUnlock); - if (!this.authRequest) { - this.logService.error("No auth request found."); + if (!this.fingerprintPhrase) { + this.logService.error("FingerprintPhrase failed to initialize from buildAuthRequest."); + return; + } + + if (!this.authRequestKeyPair) { + this.logService.error("KeyPair failed to initialize from buildAuthRequest."); + return; + } + + const authRequestResponse: AuthRequestResponse = + await this.authRequestApiService.postAuthRequest(authRequest); + + if (await this.configService.getFeatureFlag(FeatureFlag.PM9112_DeviceApprovalPersistence)) { + if (!this.authRequestKeyPair.privateKey) { + this.logService.error("No private key when trying to cache the login view."); return; } - const authRequestResponse = await this.authRequestApiService.postAuthRequest( - this.authRequest, - ); - - if (authRequestResponse.id) { - await this.anonymousHubService.createHubConnection(authRequestResponse.id); + if (!this.accessCode) { + this.logService.error("No access code when trying to cache the login view."); + return; } - } catch (e) { - this.logService.error(e); + + this.loginViaAuthRequestCacheService.cacheLoginView( + authRequestResponse.id, + this.authRequestKeyPair.privateKey, + this.accessCode, + ); } + + if (authRequestResponse.id) { + await this.anonymousHubService.createHubConnection(authRequestResponse.id); + } + } catch (e) { + this.logService.error(e); } setTimeout(() => { @@ -358,7 +422,10 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { }, this.showResendNotificationTimeoutSeconds * 1000); } - private async buildAuthRequest(authRequestType: AuthRequestType): Promise { + private async buildAuthRequest( + email: string, + authRequestType: AuthRequestType, + ): Promise { const authRequestKeyPairArray = await this.cryptoFunctionService.rsaGenerateKeyPair(2048); this.authRequestKeyPair = { @@ -369,36 +436,27 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { const deviceIdentifier = await this.appIdService.getAppId(); if (!this.authRequestKeyPair.publicKey) { - this.logService.error("AuthRequest public key not set to value in building auth request."); - return; + const errorMessage = "No public key when building an auth request."; + this.logService.error(errorMessage); + throw new Error(errorMessage); } - const publicKey = Utils.fromBufferToB64(this.authRequestKeyPair.publicKey); - const accessCode = await this.passwordGenerationService.generatePassword({ + this.fingerprintPhrase = await this.authRequestService.getFingerprintPhrase( + email, + this.authRequestKeyPair.publicKey, + ); + + this.accessCode = await this.passwordGenerationService.generatePassword({ type: "password", length: 25, }); - if (!this.email) { - this.logService.error("Email not defined when building auth request."); - return; - } + const b64PublicKey = Utils.fromBufferToB64(this.authRequestKeyPair.publicKey); - this.fingerprintPhrase = await this.authRequestService.getFingerprintPhrase( - this.email, - this.authRequestKeyPair.publicKey, - ); - - this.authRequest = new AuthRequest( - this.email, - deviceIdentifier, - publicKey, - authRequestType, - accessCode, - ); + return new AuthRequest(email, deviceIdentifier, b64PublicKey, authRequestType, this.accessCode); } - private async handleExistingAdminAuthRequest( + private async handleExistingAdminAuthRequestLogin( adminAuthRequestStorable: AdminAuthRequestStorable, userId: UserId, ): Promise { @@ -414,7 +472,7 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { ); } catch (error) { if (error instanceof ErrorResponse && error.statusCode === HttpStatusCode.NotFound) { - return await this.handleExistingAdminAuthReqDeletedOrDenied(userId); + return await this.clearExistingAdminAuthRequestAndStartNewRequest(userId); } this.logService.error(error); return; @@ -422,28 +480,12 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { // Request doesn't exist anymore if (!adminAuthRequestResponse) { - return await this.handleExistingAdminAuthReqDeletedOrDenied(userId); + return await this.clearExistingAdminAuthRequestAndStartNewRequest(userId); } - // 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( - adminAuthRequestStorable.privateKey, - ); - - if (!this.email) { - this.logService.error("Email not defined when handling an existing an admin auth request."); - return; - } - - this.fingerprintPhrase = await this.authRequestService.getFingerprintPhrase( - this.email, - derivedPublicKeyArrayBuffer, - ); - // Request denied if (adminAuthRequestResponse.isAnswered && !adminAuthRequestResponse.requestApproved) { - return await this.handleExistingAdminAuthReqDeletedOrDenied(userId); + return await this.clearExistingAdminAuthRequestAndStartNewRequest(userId); } // Request approved @@ -455,6 +497,22 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { ); } + if (!this.email) { + this.logService.error("Email not defined when handling an existing an admin auth request."); + return; + } + + // 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( + adminAuthRequestStorable.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 = { @@ -464,117 +522,99 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { await this.anonymousHubService.createHubConnection(adminAuthRequestStorable.id); } - private async verifyAndHandleApprovedAuthReq(requestId: string): Promise { - /** - * *********************************** - * Standard Auth Request Flows - * *********************************** - * - * Flow 1: Unauthed user requests approval from device; Approving device has a masterKey in memory. - * - * Unauthed user clicks "Login with device" > navigates to /login-with-device which creates a StandardAuthRequest - * > receives approval from a device with authRequestPublicKey(masterKey) > decrypts masterKey > decrypts userKey > proceed to vault - * - * Flow 2: Unauthed user requests approval from device; Approving device does NOT have a masterKey in memory. - * - * Unauthed user clicks "Login with device" > navigates to /login-with-device which creates a StandardAuthRequest - * > receives approval from a device with authRequestPublicKey(userKey) > decrypts userKey > proceeds to vault - * - * Note: this flow is an uncommon scenario and relates to TDE off-boarding. The following describes how a user could get into this flow: - * 1) An SSO TD user logs into a device via an Admin auth request approval, therefore this device does NOT have a masterKey in memory. - * 2) The org admin... - * (2a) Changes the member decryption options from "Trusted devices" to "Master password" AND - * (2b) Turns off the "Require single sign-on authentication" policy - * 3) On another device, the user clicks "Login with device", which they can do because the org no longer requires SSO. - * 4) The user approves from the device they had previously logged into with SSO TD, which does NOT have a masterKey in memory (see step 1 above). - * - * Flow 3: Authed SSO TD user requests approval from device; Approving device has a masterKey in memory. - * - * SSO TD user authenticates via SSO > navigates to /login-initiated > clicks "Approve from your other device" - * > navigates to /login-with-device which creates a StandardAuthRequest > receives approval from device with authRequestPublicKey(masterKey) - * > decrypts masterKey > decrypts userKey > establishes trust (if required) > proceeds to vault - * - * Flow 4: Authed SSO TD user requests approval from device; Approving device does NOT have a masterKey in memory. - * - * SSO TD user authenticates via SSO > navigates to /login-initiated > clicks "Approve from your other device" - * > navigates to /login-with-device which creates a StandardAuthRequest > receives approval from device with authRequestPublicKey(userKey) - * > decrypts userKey > establishes trust (if required) > proceeds to vault - * - * *********************************** - * Admin Auth Request Flow - * *********************************** - * - * Flow: Authed SSO TD user requests admin approval. - * - * SSO TD user authenticates via SSO > navigates to /login-initiated > clicks "Request admin approval" - * > navigates to /admin-approval-requested which creates an AdminAuthRequest > receives approval from device with authRequestPublicKey(userKey) - * > decrypts userKey > establishes trust (if required) > proceeds to vault - * - * Note: TDE users are required to be enrolled in admin password reset, which gives the admin access to the user's userKey. - * This is how admins are able to send over the authRequestPublicKey(userKey) to the user to allow them to unlock. - * - * - * Summary Table - * |-------------------------------------------------------------------------------------------------------------------------------------------------------------------------| - * | Flow | Auth Status | Clicks Button [active route] | Navigates to | Approving device has masterKey in memory (see note 1) | - * |-------------------------------------------------------------------------------------------------------------------------------------------------------------------------| - * | Standard Flow 1 | unauthed | "Login with device" [/login] | /login-with-device | yes | - * | Standard Flow 2 | unauthed | "Login with device" [/login] | /login-with-device | no | - * | Standard Flow 3 | authed | "Approve from your other device" [/login-initiated] | /login-with-device | yes | - * | Standard Flow 4 | authed | "Approve from your other device" [/login-initiated] | /login-with-device | no | - * | Admin Flow | authed | "Request admin approval" [/login-initiated] | /admin-approval-requested | NA - admin requests always send encrypted userKey | - * |-------------------------------------------------------------------------------------------------------------------------------------------------------------------------| - * * Note 1: The phrase "in memory" here is important. It is possible for a user to have a master password for their account, but not have a masterKey IN MEMORY for - * a specific device. For example, if a user registers an account with a master password, then joins an SSO TD org, then logs in to a device via SSO and - * admin auth request, they are now logged into that device but that device does not have masterKey IN MEMORY. - */ - + /** + * This is used for trying to get the auth request back out of state. + * @param requestId + * @private + */ + private async retrieveAuthRequest(requestId: string): Promise { + let authRequestResponse: AuthRequestResponse | undefined = undefined; try { + // There are two cases here, the first being const userHasAuthenticatedViaSSO = this.authStatus === AuthenticationStatus.Locked; + // Get the response based on whether we've authenticated or not. We need to call a different API method + // based on whether we have a token or need to use the accessCode. if (userHasAuthenticatedViaSSO) { - // Get the auth request from the server - // User is authenticated, therefore the endpoint does not require an access code. - const authRequestResponse = await this.authRequestApiService.getAuthRequest(requestId); - - if (authRequestResponse.requestApproved) { - // Handles Standard Flows 3-4 and Admin Flow - await this.handleAuthenticatedFlows(authRequestResponse); - } + authRequestResponse = await this.authRequestApiService.getAuthRequest(requestId); } else { - if (!this.authRequest) { - this.logService.error("No auth request defined when handling approved auth request."); - return; + if (!this.accessCode) { + const errorMessage = "No access code available when handling approved auth request."; + this.logService.error(errorMessage); + throw new Error(errorMessage); } - - // Get the auth request from the server - // User is unauthenticated, therefore the endpoint requires an access code for user verification. - const authRequestResponse = await this.authRequestApiService.getAuthResponse( + authRequestResponse = await this.authRequestApiService.getAuthResponse( requestId, - this.authRequest.accessCode, + this.accessCode, ); + } + } catch (error) { + // If the request no longer exists, we treat it as if it's been answered (and denied). + if (error instanceof ErrorResponse && error.statusCode === HttpStatusCode.NotFound) { + authRequestResponse = undefined; + } else { + this.logService.error(error); + } + } - if (authRequestResponse.requestApproved) { - // Handles Standard Flows 1-2 - await this.handleUnauthenticatedFlows(authRequestResponse, requestId); + if (authRequestResponse === undefined) { + throw new Error("Auth request response not generated"); + } + + return authRequestResponse; + } + + /** + * Determines if the Auth Request has been approved, deleted or denied, and handles + * the response accordingly. + * @param requestId The ID of the Auth Request to process + * @returns A boolean indicating whether the Auth Request was successfully processed + */ + private async handleExistingAuthRequestLogin(requestId: string): Promise { + this.showResendNotification = false; + + try { + const authRequestResponse = await this.retrieveAuthRequest(requestId); + + // Request doesn't exist anymore, so we'll clear the cache and start a new request. + if (!authRequestResponse) { + return await this.clearExistingStandardAuthRequestAndStartNewRequest(); + } + + // Request denied, so we'll clear the cache and start a new request. + if (authRequestResponse.isAnswered && !authRequestResponse.requestApproved) { + return await this.clearExistingStandardAuthRequestAndStartNewRequest(); + } + + // Request approved, so we'll log the user in. + if (authRequestResponse.requestApproved) { + const userHasAuthenticatedViaSSO = this.authStatus === AuthenticationStatus.Locked; + if (userHasAuthenticatedViaSSO) { + // [Standard Flow 3-4] Handle authenticated SSO TD user flows + return await this.handleAuthenticatedFlows(authRequestResponse); + } else { + // [Standard Flow 1-2] Handle unauthenticated user flows + return await this.handleUnauthenticatedFlows(authRequestResponse, requestId); } } + + // At this point, we know that the request is still pending, so we'll start a hub connection to listen for a response. + await this.anonymousHubService.createHubConnection(requestId); } catch (error) { if (error instanceof ErrorResponse) { await this.router.navigate([this.backToRoute]); this.validationService.showError(error); - return; } - this.logService.error(error); - } finally { - // Manually clean out the cache to make sure sensitive - // data does not persist longer than it needs to. - this.loginViaAuthRequestCacheService.clearCacheLoginView(); } + + setTimeout(() => { + this.showResendNotification = true; + }, this.showResendNotificationTimeoutSeconds * 1000); } private async handleAuthenticatedFlows(authRequestResponse: AuthRequestResponse) { + // [Standard Flow 3-4] Handle authenticated SSO TD user flows const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; if (!userId) { this.logService.error( @@ -599,6 +639,7 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { authRequestResponse: AuthRequestResponse, requestId: string, ) { + // [Standard Flow 1-2] Handle unauthenticated user flows const authRequestLoginCredentials = await this.buildAuthRequestLoginCredentials( requestId, authRequestResponse, @@ -609,6 +650,9 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { return; } + // Clear the cached auth request from state since we're using it to log in. + this.loginViaAuthRequestCacheService.clearCacheLoginView(); + // Note: keys are set by AuthRequestLoginStrategy success handling const authResult = await this.loginStrategyService.logIn(authRequestLoginCredentials); @@ -621,21 +665,20 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { userId: UserId, ): Promise { /** - * See verifyAndHandleApprovedAuthReq() for flow details. - * + * [Flow Type Detection] * We determine the type of `key` based on the presence or absence of `masterPasswordHash`: - * - If `masterPasswordHash` has a value, we receive the `key` as an authRequestPublicKey(masterKey) [plus we have authRequestPublicKey(masterPasswordHash)] - * - If `masterPasswordHash` does not have a value, we receive the `key` as an authRequestPublicKey(userKey) + * - If `masterPasswordHash` exists: Standard Flow 1 or 3 (device has masterKey) + * - If no `masterPasswordHash`: Standard Flow 2, 4, or Admin Flow (device sends userKey) */ if (authRequestResponse.masterPasswordHash) { - // ...in Standard Auth Request Flow 3 + // [Standard Flow 1 or 3] Device has masterKey await this.authRequestService.setKeysAfterDecryptingSharedMasterKeyAndHash( authRequestResponse, privateKey, userId, ); } else { - // ...in Standard Auth Request Flow 4 or Admin Auth Request Flow + // [Standard Flow 2, 4, or Admin Flow] Device sends userKey await this.authRequestService.setUserKeyAfterDecryptingSharedUserKey( authRequestResponse, privateKey, @@ -643,15 +686,20 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { ); } + // [Admin Flow Cleanup] Clear one-time use admin auth request // clear the admin auth request from state so it cannot be used again (it's a one time use) // TODO: this should eventually be enforced via deleting this on the server once it is used await this.authRequestService.clearAdminAuthRequest(userId); + // [Standard Flow Cleanup] Clear the cached auth request from state + this.loginViaAuthRequestCacheService.clearCacheLoginView(); + this.toastService.showToast({ variant: "success", message: this.i18nService.t("loginApproved"), }); + // [Device Trust] Establish trust if required // Now that we have a decrypted user key in memory, we can check if we // need to establish trust on the current device const activeAccount = await firstValueFrom(this.accountService.activeAccount$); @@ -686,9 +734,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; } @@ -711,7 +759,7 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { return new AuthRequestLoginCredentials( this.email, - this.authRequest.accessCode, + this.accessCode, requestId, null, // no userKey masterKey, @@ -725,7 +773,7 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { ); return new AuthRequestLoginCredentials( this.email, - this.authRequest.accessCode, + this.accessCode, requestId, userKey, null, // no masterKey @@ -734,12 +782,20 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { } } - private async handleExistingAdminAuthReqDeletedOrDenied(userId: UserId) { + private async clearExistingAdminAuthRequestAndStartNewRequest(userId: UserId) { // clear the admin auth request from state await this.authRequestService.clearAdminAuthRequest(userId); // start new auth request - await this.startAdminAuthRequestLogin(); + await this.handleNewAdminAuthRequestLogin(); + } + + private async clearExistingStandardAuthRequestAndStartNewRequest(): Promise { + // clear the auth request from state + this.loginViaAuthRequestCacheService.clearCacheLoginView(); + + // start new auth request + await this.handleNewStandardAuthRequestLogin(); } private async handlePostLoginNavigation(loginResponse: AuthResult) { 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 82ac0f1006..89b78500f1 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 30ba887954..493fea5c14 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 0691b8efd8..c2113415cf 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,17 +1,16 @@ 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 { + static fromJSON(obj: Partial>): LoginViaAuthRequestView | null { + if (obj == null) { + return null; + } return Object.assign(new LoginViaAuthRequestView(), obj); } }