From ec950853bc4f60f050ab0c63c48b8d862dfe6cda Mon Sep 17 00:00:00 2001
From: rr-bw <102181210+rr-bw@users.noreply.github.com>
Date: Fri, 29 Aug 2025 12:25:31 -0700
Subject: [PATCH 1/3] fix(2fa-recovery-code-error): [Auth/PM-19885] Better
error handling when 2FA recovery code is invalid (#16145)
Implements better error handling when a user enters an invalid 2FA recovery code. Upon entering an invalid code:
- Keep the user on the `/recover-2fa` page (This also makes it so the incorrect code remains in the form field so the user can see what they entered, if they mistyped the code, etc.)
- Show an inline error: "Invalid recovery code"
---
.../auth/recover-two-factor.component.html | 1 -
.../auth/recover-two-factor.component.spec.ts | 74 +++++++++++++++----
.../app/auth/recover-two-factor.component.ts | 33 +++++++--
apps/web/src/locales/en/messages.json | 3 +
4 files changed, 90 insertions(+), 21 deletions(-)
diff --git a/apps/web/src/app/auth/recover-two-factor.component.html b/apps/web/src/app/auth/recover-two-factor.component.html
index dee3bec1520..da2501efe26 100644
--- a/apps/web/src/app/auth/recover-two-factor.component.html
+++ b/apps/web/src/app/auth/recover-two-factor.component.html
@@ -28,7 +28,6 @@
{{ "recoveryCodeTitle" | i18n }}
-
{{ "submit" | i18n }}
diff --git a/apps/web/src/app/auth/recover-two-factor.component.spec.ts b/apps/web/src/app/auth/recover-two-factor.component.spec.ts
index 5977d994521..c3792cfd3f3 100644
--- a/apps/web/src/app/auth/recover-two-factor.component.spec.ts
+++ b/apps/web/src/app/auth/recover-two-factor.component.spec.ts
@@ -9,10 +9,12 @@ import {
} from "@bitwarden/auth/common";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result";
+import { ErrorResponse } from "@bitwarden/common/models/response/error.response";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
+import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service";
import { ToastService } from "@bitwarden/components";
import { KeyService } from "@bitwarden/key-management";
import { I18nPipe } from "@bitwarden/ui-common";
@@ -34,6 +36,7 @@ describe("RecoverTwoFactorComponent", () => {
let mockConfigService: MockProxy;
let mockLoginSuccessHandlerService: MockProxy;
let mockLogService: MockProxy;
+ let mockValidationService: MockProxy;
beforeEach(() => {
mockRouter = mock();
@@ -46,6 +49,7 @@ describe("RecoverTwoFactorComponent", () => {
mockConfigService = mock();
mockLoginSuccessHandlerService = mock();
mockLogService = mock();
+ mockValidationService = mock();
TestBed.configureTestingModule({
declarations: [RecoverTwoFactorComponent],
@@ -60,6 +64,7 @@ describe("RecoverTwoFactorComponent", () => {
{ provide: ConfigService, useValue: mockConfigService },
{ provide: LoginSuccessHandlerService, useValue: mockLoginSuccessHandlerService },
{ provide: LogService, useValue: mockLogService },
+ { provide: ValidationService, useValue: mockValidationService },
],
imports: [I18nPipe],
// FIXME(PM-18598): Replace unknownElements and unknownProperties with actual imports
@@ -70,16 +75,21 @@ describe("RecoverTwoFactorComponent", () => {
component = fixture.componentInstance;
});
- afterEach(() => {
- jest.resetAllMocks();
- });
-
describe("handleRecoveryLogin", () => {
+ let email: string;
+ let recoveryCode: string;
+
+ beforeEach(() => {
+ email = "test@example.com";
+ recoveryCode = "testRecoveryCode";
+ });
+
+ afterEach(() => {
+ jest.resetAllMocks();
+ });
+
it("should log in successfully and navigate to the two-factor settings page", async () => {
// Arrange
- const email = "test@example.com";
- const recoveryCode = "testRecoveryCode";
-
const authResult = new AuthResult();
mockLoginStrategyService.logIn.mockResolvedValue(authResult);
@@ -98,14 +108,16 @@ describe("RecoverTwoFactorComponent", () => {
expect(mockRouter.navigate).toHaveBeenCalledWith(["/settings/security/two-factor"]);
});
- it("should handle login errors and redirect to login page", async () => {
+ it("should log an error and set an inline error on the recoveryCode form control upon receiving an ErrorResponse due to an invalid token", async () => {
// Arrange
- const email = "test@example.com";
- const recoveryCode = "testRecoveryCode";
-
- const error = new Error("Login failed");
+ const error = new ErrorResponse("mockError", 400);
+ error.message = "Two-step token is invalid";
mockLoginStrategyService.logIn.mockRejectedValue(error);
+ const recoveryCodeControl = component.formGroup.get("recoveryCode");
+ jest.spyOn(recoveryCodeControl, "setErrors");
+ mockI18nService.t.mockReturnValue("Invalid recovery code");
+
// Act
await component["loginWithRecoveryCode"](email, recoveryCode);
@@ -114,9 +126,43 @@ describe("RecoverTwoFactorComponent", () => {
"Error logging in automatically: ",
error.message,
);
- expect(mockRouter.navigate).toHaveBeenCalledWith(["/login"], {
- queryParams: { email: email },
+ expect(recoveryCodeControl.setErrors).toHaveBeenCalledWith({
+ invalidRecoveryCode: { message: "Invalid recovery code" },
});
});
+
+ it("should log an error and show validation but not set an inline error on the recoveryCode form control upon receiving some other ErrorResponse", async () => {
+ // Arrange
+ const error = new ErrorResponse("mockError", 400);
+ error.message = "Some other error";
+ mockLoginStrategyService.logIn.mockRejectedValue(error);
+
+ const recoveryCodeControl = component.formGroup.get("recoveryCode");
+ jest.spyOn(recoveryCodeControl, "setErrors");
+
+ // Act
+ await component["loginWithRecoveryCode"](email, recoveryCode);
+
+ // Assert
+ expect(mockLogService.error).toHaveBeenCalledWith(
+ "Error logging in automatically: ",
+ error.message,
+ );
+ expect(mockValidationService.showError).toHaveBeenCalledWith(error.message);
+ expect(recoveryCodeControl.setErrors).not.toHaveBeenCalled();
+ });
+
+ it("should log an error and show validation upon receiving a non-ErrorResponse error", async () => {
+ // Arrange
+ const error = new Error("Generic error");
+ mockLoginStrategyService.logIn.mockRejectedValue(error);
+
+ // Act
+ await component["loginWithRecoveryCode"](email, recoveryCode);
+
+ // Assert
+ expect(mockLogService.error).toHaveBeenCalledWith("Error logging in automatically: ", error);
+ expect(mockValidationService.showError).toHaveBeenCalledWith(error);
+ });
});
});
diff --git a/apps/web/src/app/auth/recover-two-factor.component.ts b/apps/web/src/app/auth/recover-two-factor.component.ts
index a69da0a66bf..f606e803df3 100644
--- a/apps/web/src/app/auth/recover-two-factor.component.ts
+++ b/apps/web/src/app/auth/recover-two-factor.component.ts
@@ -1,4 +1,5 @@
-import { Component, OnInit } from "@angular/core";
+import { Component, DestroyRef, OnInit } from "@angular/core";
+import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
import { FormControl, FormGroup, Validators } from "@angular/forms";
import { Router } from "@angular/router";
@@ -9,8 +10,10 @@ import {
} from "@bitwarden/auth/common";
import { TwoFactorProviderType } from "@bitwarden/common/auth/enums/two-factor-provider-type";
import { TokenTwoFactorRequest } from "@bitwarden/common/auth/models/request/identity-token/token-two-factor.request";
+import { ErrorResponse } from "@bitwarden/common/models/response/error.response";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service";
import { ToastService } from "@bitwarden/components";
@Component({
@@ -19,7 +22,7 @@ import { ToastService } from "@bitwarden/components";
standalone: false,
})
export class RecoverTwoFactorComponent implements OnInit {
- protected formGroup = new FormGroup({
+ formGroup = new FormGroup({
email: new FormControl("", [Validators.required]),
masterPassword: new FormControl("", [Validators.required]),
recoveryCode: new FormControl("", [Validators.required]),
@@ -31,16 +34,23 @@ export class RecoverTwoFactorComponent implements OnInit {
recoveryCodeMessage = "";
constructor(
+ private destroyRef: DestroyRef,
private router: Router,
private i18nService: I18nService,
private loginStrategyService: LoginStrategyServiceAbstraction,
private toastService: ToastService,
private loginSuccessHandlerService: LoginSuccessHandlerService,
private logService: LogService,
+ private validationService: ValidationService,
) {}
async ngOnInit() {
this.recoveryCodeMessage = this.i18nService.t("logInBelowUsingYourSingleUseRecoveryCode");
+
+ // Clear any existing recovery code inline error when user updates the form
+ this.formGroup.valueChanges.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(() => {
+ this.formGroup.get("recoveryCode")?.setErrors(null);
+ });
}
get email(): string {
@@ -99,10 +109,21 @@ export class RecoverTwoFactorComponent implements OnInit {
await this.loginSuccessHandlerService.run(authResult.userId);
await this.router.navigate(["/settings/security/two-factor"]);
- } catch (error) {
- // If login errors, redirect to login page per product. Don't show error
- this.logService.error("Error logging in automatically: ", (error as Error).message);
- await this.router.navigate(["/login"], { queryParams: { email: email } });
+ } catch (error: unknown) {
+ if (error instanceof ErrorResponse) {
+ this.logService.error("Error logging in automatically: ", error.message);
+
+ if (error.message.includes("Two-step token is invalid")) {
+ this.formGroup.get("recoveryCode")?.setErrors({
+ invalidRecoveryCode: { message: this.i18nService.t("invalidRecoveryCode") },
+ });
+ } else {
+ this.validationService.showError(error.message);
+ }
+ } else {
+ this.logService.error("Error logging in automatically: ", error);
+ this.validationService.showError(error);
+ }
}
}
}
diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json
index 08be03f0bed..2c68a219baa 100644
--- a/apps/web/src/locales/en/messages.json
+++ b/apps/web/src/locales/en/messages.json
@@ -1517,6 +1517,9 @@
"recoveryCodeTitle": {
"message": "Recovery code"
},
+ "invalidRecoveryCode": {
+ "message": "Invalid recovery code"
+ },
"authenticatorAppTitle": {
"message": "Authenticator app"
},
From de928e9ba19b77de69d2c7e5fdfc7e5fdf8ebd57 Mon Sep 17 00:00:00 2001
From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com>
Date: Fri, 29 Aug 2025 14:27:49 -0500
Subject: [PATCH 2/3] Fix SDK typings (#16223)
* fix SDK typings for `uuidAsString`
* add `load_flags` to mock SDK instance
---
.../src/platform/services/sdk/default-sdk.service.spec.ts | 1 +
libs/vault/src/services/copy-cipher-field.service.ts | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/libs/common/src/platform/services/sdk/default-sdk.service.spec.ts b/libs/common/src/platform/services/sdk/default-sdk.service.spec.ts
index 8ebc2dcb62b..2b94305f0ec 100644
--- a/libs/common/src/platform/services/sdk/default-sdk.service.spec.ts
+++ b/libs/common/src/platform/services/sdk/default-sdk.service.spec.ts
@@ -232,6 +232,7 @@ function createMockClient(): MockProxy {
client.platform.mockReturnValue({
state: jest.fn().mockReturnValue(mock()),
free: mock(),
+ load_flags: jest.fn(),
});
return client;
}
diff --git a/libs/vault/src/services/copy-cipher-field.service.ts b/libs/vault/src/services/copy-cipher-field.service.ts
index d0086ab5ac7..0a7d9e1f68d 100644
--- a/libs/vault/src/services/copy-cipher-field.service.ts
+++ b/libs/vault/src/services/copy-cipher-field.service.ts
@@ -145,9 +145,9 @@ export class CopyCipherFieldService {
if (action.event !== undefined) {
await this.eventCollectionService.collect(
action.event,
- uuidAsString(cipher.id),
+ cipher.id ? uuidAsString(cipher.id) : undefined,
false,
- uuidAsString(cipher.organizationId),
+ cipher.organizationId ? uuidAsString(cipher.organizationId) : undefined,
);
}
From e4c75b3c49a0c5ec921e58272398518dd9eb7b77 Mon Sep 17 00:00:00 2001
From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com>
Date: Fri, 29 Aug 2025 14:57:19 -0500
Subject: [PATCH 3/3] Revert "PM-23386 Display autofill options after sync
(#15906)" (#16222)
This reverts commit 8c51050eda06016c936cfb12d39dd887ddcd14ef.
---
libs/common/src/vault/services/cipher.service.ts | 1 -
1 file changed, 1 deletion(-)
diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts
index a3915d6ce3d..d89a41aba1f 100644
--- a/libs/common/src/vault/services/cipher.service.ts
+++ b/libs/common/src/vault/services/cipher.service.ts
@@ -1139,7 +1139,6 @@ export class CipherService implements CipherServiceAbstraction {
}
async replace(ciphers: { [id: string]: CipherData }, userId: UserId): Promise {
- await this.clearEncryptedCiphersState(userId);
await this.updateEncryptedCipherState(() => ciphers, userId);
}