diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index e2032bf27b..da8d9ea0e3 100644 --- a/apps/desktop/src/locales/en/messages.json +++ b/apps/desktop/src/locales/en/messages.json @@ -69,6 +69,9 @@ } } }, + "noEditPermissions": { + "message": "You don't have permission to edit this item" + }, "welcomeBack": { "message": "Welcome back" }, diff --git a/apps/web/src/app/auth/settings/emergency-access/view/emergency-view-dialog.component.spec.ts b/apps/web/src/app/auth/settings/emergency-access/view/emergency-view-dialog.component.spec.ts index 60993924de..d13987f2e8 100644 --- a/apps/web/src/app/auth/settings/emergency-access/view/emergency-view-dialog.component.spec.ts +++ b/apps/web/src/app/auth/settings/emergency-access/view/emergency-view-dialog.component.spec.ts @@ -8,6 +8,7 @@ import { CollectionService } from "@bitwarden/admin-console/common"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service"; +import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -16,6 +17,7 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl import { Utils } from "@bitwarden/common/platform/misc/utils"; import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; import { UserId, EmergencyAccessId } from "@bitwarden/common/types/guid"; +import { CipherRiskService } from "@bitwarden/common/vault/abstractions/cipher-risk.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; import { CipherType } from "@bitwarden/common/vault/enums"; @@ -68,6 +70,12 @@ describe("EmergencyViewDialogComponent", () => { useValue: { environment$: of({ getIconsUrl: () => "https://icons.example.com" }) }, }, { provide: DomainSettingsService, useValue: { showFavicons$: of(true) } }, + { provide: CipherRiskService, useValue: mock() }, + { + provide: BillingAccountProfileStateService, + useValue: mock(), + }, + { provide: ConfigService, useValue: mock() }, ], }) .overrideComponent(EmergencyViewDialogComponent, { @@ -78,7 +86,6 @@ describe("EmergencyViewDialogComponent", () => { provide: ChangeLoginPasswordService, useValue: ChangeLoginPasswordService, }, - { provide: ConfigService, useValue: ConfigService }, { provide: CipherService, useValue: mock() }, ], }, @@ -89,7 +96,6 @@ describe("EmergencyViewDialogComponent", () => { provide: ChangeLoginPasswordService, useValue: mock(), }, - { provide: ConfigService, useValue: mock() }, { provide: CipherService, useValue: mock() }, ], }, diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index e91464cb17..0a0152c596 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -23,6 +23,9 @@ "passwordRisk": { "message": "Password Risk" }, + "noEditPermissions": { + "message": "You don't have permission to edit this item" + }, "reviewAtRiskPasswords": { "message": "Review at-risk passwords (weak, exposed, or reused) across applications. Select your most critical applications to prioritize security actions for your users to address at-risk passwords." }, diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 38ce3c0fcc..c60bc2e2f0 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -282,6 +282,7 @@ import { } from "@bitwarden/common/tools/send/services/send.service.abstraction"; import { CipherArchiveService } from "@bitwarden/common/vault/abstractions/cipher-archive.service"; import { CipherEncryptionService } from "@bitwarden/common/vault/abstractions/cipher-encryption.service"; +import { CipherRiskService } from "@bitwarden/common/vault/abstractions/cipher-risk.service"; import { CipherService as CipherServiceAbstraction } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherFileUploadService as CipherFileUploadServiceAbstraction } from "@bitwarden/common/vault/abstractions/file-upload/cipher-file-upload.service"; import { FolderApiServiceAbstraction } from "@bitwarden/common/vault/abstractions/folder/folder-api.service.abstraction"; @@ -303,6 +304,7 @@ import { import { CipherService } from "@bitwarden/common/vault/services/cipher.service"; import { DefaultCipherArchiveService } from "@bitwarden/common/vault/services/default-cipher-archive.service"; import { DefaultCipherEncryptionService } from "@bitwarden/common/vault/services/default-cipher-encryption.service"; +import { DefaultCipherRiskService } from "@bitwarden/common/vault/services/default-cipher-risk.service"; import { CipherFileUploadService } from "@bitwarden/common/vault/services/file-upload/cipher-file-upload.service"; import { FolderApiService } from "@bitwarden/common/vault/services/folder/folder-api.service"; import { FolderService } from "@bitwarden/common/vault/services/folder/folder.service"; @@ -605,6 +607,11 @@ const safeProviders: SafeProvider[] = [ MessagingServiceAbstraction, ], }), + safeProvider({ + provide: CipherRiskService, + useClass: DefaultCipherRiskService, + deps: [SdkService, CipherServiceAbstraction], + }), safeProvider({ provide: InternalFolderService, useClass: FolderService, diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index d9effd21b3..d14ad8a64f 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -58,6 +58,7 @@ export enum FeatureFlag { PM22136_SdkCipherEncryption = "pm-22136-sdk-cipher-encryption", CipherKeyEncryption = "cipher-key-encryption", AutofillConfirmation = "pm-25083-autofill-confirm-from-search", + RiskInsightsForPremium = "pm-23904-risk-insights-for-premium", /* Platform */ IpcChannelFramework = "ipc-channel-framework", @@ -106,6 +107,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.PM22134SdkCipherListView]: FALSE, [FeatureFlag.PM22136_SdkCipherEncryption]: FALSE, [FeatureFlag.AutofillConfirmation]: FALSE, + [FeatureFlag.RiskInsightsForPremium]: FALSE, /* Auth */ [FeatureFlag.PM22110_DisableAlternateLoginMethods]: FALSE, diff --git a/libs/common/src/vault/abstractions/cipher-risk.service.spec.ts b/libs/common/src/vault/abstractions/cipher-risk.service.spec.ts new file mode 100644 index 0000000000..2c87191cd9 --- /dev/null +++ b/libs/common/src/vault/abstractions/cipher-risk.service.spec.ts @@ -0,0 +1,88 @@ +import type { CipherRiskResult, CipherId } from "@bitwarden/sdk-internal"; + +import { isPasswordAtRisk } from "./cipher-risk.service"; + +describe("isPasswordAtRisk", () => { + const mockId = "00000000-0000-0000-0000-000000000000" as unknown as CipherId; + + const createRisk = (overrides: Partial = {}): CipherRiskResult => ({ + id: mockId, + password_strength: 4, + exposed_result: { type: "NotChecked" }, + reuse_count: 1, + ...overrides, + }); + + describe("exposed password risk", () => { + it.each([ + { value: 5, expected: true, desc: "found with value > 0" }, + { value: 0, expected: false, desc: "found but value is 0" }, + ])("should return $expected when password is $desc", ({ value, expected }) => { + const risk = createRisk({ exposed_result: { type: "Found", value } }); + expect(isPasswordAtRisk(risk)).toBe(expected); + }); + + it("should return false when password is not checked", () => { + expect(isPasswordAtRisk(createRisk())).toBe(false); + }); + }); + + describe("password reuse risk", () => { + it.each([ + { count: 2, expected: true, desc: "reused (reuse_count > 1)" }, + { count: 1, expected: false, desc: "not reused" }, + { count: undefined, expected: false, desc: "undefined" }, + ])("should return $expected when reuse_count is $desc", ({ count, expected }) => { + const risk = createRisk({ reuse_count: count }); + expect(isPasswordAtRisk(risk)).toBe(expected); + }); + }); + + describe("password strength risk", () => { + it.each([ + { strength: 0, expected: true }, + { strength: 1, expected: true }, + { strength: 2, expected: true }, + { strength: 3, expected: false }, + { strength: 4, expected: false }, + ])("should return $expected when password strength is $strength", ({ strength, expected }) => { + const risk = createRisk({ password_strength: strength }); + expect(isPasswordAtRisk(risk)).toBe(expected); + }); + }); + + describe("multiple risk factors", () => { + it.each<{ desc: string; overrides: Partial; expected: boolean }>([ + { + desc: "exposed and reused", + overrides: { + exposed_result: { type: "Found" as const, value: 3 }, + reuse_count: 2, + }, + expected: true, + }, + { + desc: "reused and weak strength", + overrides: { password_strength: 2, reuse_count: 2 }, + expected: true, + }, + { + desc: "all three risk factors", + overrides: { + password_strength: 1, + exposed_result: { type: "Found" as const, value: 10 }, + reuse_count: 3, + }, + expected: true, + }, + { + desc: "no risk factors", + overrides: { reuse_count: undefined }, + expected: false, + }, + ])("should return $expected when $desc present", ({ overrides, expected }) => { + const risk = createRisk(overrides); + expect(isPasswordAtRisk(risk)).toBe(expected); + }); + }); +}); diff --git a/libs/common/src/vault/abstractions/cipher-risk.service.ts b/libs/common/src/vault/abstractions/cipher-risk.service.ts index 6bbd9d7791..78f1c50da1 100644 --- a/libs/common/src/vault/abstractions/cipher-risk.service.ts +++ b/libs/common/src/vault/abstractions/cipher-risk.service.ts @@ -1,12 +1,10 @@ import type { CipherRiskResult, CipherRiskOptions, - ExposedPasswordResult, PasswordReuseMap, - CipherId, } from "@bitwarden/sdk-internal"; -import { UserId } from "../../types/guid"; +import { UserId, CipherId } from "../../types/guid"; import { CipherView } from "../models/view/cipher.view"; export abstract class CipherRiskService { @@ -51,5 +49,21 @@ export abstract class CipherRiskService { abstract buildPasswordReuseMap(ciphers: CipherView[], userId: UserId): Promise; } -// Re-export SDK types for convenience -export type { CipherRiskResult, CipherRiskOptions, ExposedPasswordResult, PasswordReuseMap }; +/** + * Evaluates if a password represented by a CipherRiskResult is considered at risk. + * + * A password is considered at risk if any of the following conditions are true: + * - The password has been exposed in data breaches + * - The password is reused across multiple ciphers + * - The password has weak strength (password_strength < 3) + * + * @param risk - The CipherRiskResult to evaluate + * @returns true if the password is at risk, false otherwise + */ +export function isPasswordAtRisk(risk: CipherRiskResult): boolean { + return ( + (risk.exposed_result.type === "Found" && risk.exposed_result.value > 0) || + (risk.reuse_count ?? 1) > 1 || + risk.password_strength < 3 + ); +} diff --git a/libs/common/src/vault/models/view/cipher.view.ts b/libs/common/src/vault/models/view/cipher.view.ts index 7412c68d69..0d4ab8e520 100644 --- a/libs/common/src/vault/models/view/cipher.view.ts +++ b/libs/common/src/vault/models/view/cipher.view.ts @@ -113,6 +113,12 @@ export class CipherView implements View, InitializerMetadata { return this.passwordHistory && this.passwordHistory.length > 0; } + get hasLoginPassword(): boolean { + return ( + this.type === CipherType.Login && this.login?.password != null && this.login.password !== "" + ); + } + get hasAttachments(): boolean { return !!this.attachments && this.attachments.length > 0; } diff --git a/libs/common/src/vault/services/default-cipher-risk.service.spec.ts b/libs/common/src/vault/services/default-cipher-risk.service.spec.ts index afd52bde6c..e523124146 100644 --- a/libs/common/src/vault/services/default-cipher-risk.service.spec.ts +++ b/libs/common/src/vault/services/default-cipher-risk.service.spec.ts @@ -1,11 +1,11 @@ import { mock } from "jest-mock-extended"; -import { BehaviorSubject } from "rxjs"; +import { BehaviorSubject, Observable } from "rxjs"; -import type { CipherRiskOptions, CipherId, CipherRiskResult } from "@bitwarden/sdk-internal"; +import type { CipherRiskOptions, CipherRiskResult } from "@bitwarden/sdk-internal"; import { asUuid } from "../../platform/abstractions/sdk/sdk.service"; import { MockSdkService } from "../../platform/spec/mock-sdk.service"; -import { UserId } from "../../types/guid"; +import { UserId, CipherId } from "../../types/guid"; import { CipherService } from "../abstractions/cipher.service"; import { CipherType } from "../enums/cipher-type"; import { CipherView } from "../models/view/cipher.view"; @@ -19,9 +19,9 @@ describe("DefaultCipherRiskService", () => { let mockCipherService: jest.Mocked; const mockUserId = "test-user-id" as UserId; - const mockCipherId1 = "cbea34a8-bde4-46ad-9d19-b05001228ab2"; - const mockCipherId2 = "cbea34a8-bde4-46ad-9d19-b05001228ab3"; - const mockCipherId3 = "cbea34a8-bde4-46ad-9d19-b05001228ab4"; + const mockCipherId1 = "cbea34a8-bde4-46ad-9d19-b05001228ab2" as CipherId; + const mockCipherId2 = "cbea34a8-bde4-46ad-9d19-b05001228ab3" as CipherId; + const mockCipherId3 = "cbea34a8-bde4-46ad-9d19-b05001228ab4" as CipherId; beforeEach(() => { sdkService = new MockSdkService(); @@ -534,5 +534,56 @@ describe("DefaultCipherRiskService", () => { // Verify password_reuse_map was called twice (fresh computation each time) expect(mockCipherRiskClient.password_reuse_map).toHaveBeenCalledTimes(2); }); + + it("should wait for a decrypted vault before computing risk", async () => { + const mockClient = sdkService.simulate.userLogin(mockUserId); + const mockCipherRiskClient = mockClient.vault.mockDeep().cipher_risk.mockDeep(); + + const cipher = new CipherView(); + cipher.id = mockCipherId1; + cipher.type = CipherType.Login; + cipher.login = new LoginView(); + cipher.login.password = "password1"; + + // Simulate the observable emitting null (undecrypted vault) first, then the decrypted ciphers + const cipherViewsSubject = new BehaviorSubject(null); + mockCipherService.cipherViews$.mockReturnValue( + cipherViewsSubject as Observable, + ); + + mockCipherRiskClient.password_reuse_map.mockReturnValue({}); + mockCipherRiskClient.compute_risk.mockResolvedValue([ + { + id: mockCipherId1 as any, + password_strength: 4, + exposed_result: { type: "NotChecked" }, + reuse_count: 1, + }, + ]); + + // Initiate the async call but don't await yet + const computePromise = cipherRiskService.computeCipherRiskForUser( + asUuid(mockCipherId1), + mockUserId, + true, + ); + + // Simulate a tick to allow the service to process the null emission + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Now emit the actual decrypted ciphers + cipherViewsSubject.next([cipher]); + + const result = await computePromise; + + expect(mockCipherRiskClient.compute_risk).toHaveBeenCalledWith( + [expect.objectContaining({ password: "password1" })], + { + passwordMap: expect.any(Object), + checkExposed: true, + }, + ); + expect(result).toEqual(expect.objectContaining({ id: expect.anything() })); + }); }); }); diff --git a/libs/common/src/vault/services/default-cipher-risk.service.ts b/libs/common/src/vault/services/default-cipher-risk.service.ts index d9f0243edf..4b4558e5e7 100644 --- a/libs/common/src/vault/services/default-cipher-risk.service.ts +++ b/libs/common/src/vault/services/default-cipher-risk.service.ts @@ -1,16 +1,17 @@ import { firstValueFrom, switchMap } from "rxjs"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { filterOutNullish } from "@bitwarden/common/vault/utils/observable-utilities"; import { CipherLoginDetails, CipherRiskOptions, PasswordReuseMap, - CipherId, CipherRiskResult, + CipherId as SdkCipherId, } from "@bitwarden/sdk-internal"; import { SdkService, asUuid } from "../../platform/abstractions/sdk/sdk.service"; -import { UserId } from "../../types/guid"; +import { UserId, CipherId } from "../../types/guid"; import { CipherRiskService as CipherRiskServiceAbstraction } from "../abstractions/cipher-risk.service"; import { CipherType } from "../enums/cipher-type"; import { CipherView } from "../models/view/cipher.view"; @@ -52,7 +53,9 @@ export class DefaultCipherRiskService implements CipherRiskServiceAbstraction { checkExposed: boolean = true, ): Promise { // Get all ciphers for the user - const allCiphers = await firstValueFrom(this.cipherService.cipherViews$(userId)); + const allCiphers = await firstValueFrom( + this.cipherService.cipherViews$(userId).pipe(filterOutNullish()), + ); // Find the specific cipher const targetCipher = allCiphers?.find((c) => asUuid(c.id) === cipherId); @@ -106,7 +109,7 @@ export class DefaultCipherRiskService implements CipherRiskServiceAbstraction { .map( (cipher) => ({ - id: asUuid(cipher.id), + id: asUuid(cipher.id), password: cipher.login.password!, username: cipher.login.username, }) satisfies CipherLoginDetails, diff --git a/libs/vault/src/cipher-view/cipher-view.component.html b/libs/vault/src/cipher-view/cipher-view.component.html index b523c11c7e..3d0cc4c441 100644 --- a/libs/vault/src/cipher-view/cipher-view.component.html +++ b/libs/vault/src/cipher-view/cipher-view.component.html @@ -1,89 +1,85 @@ - - + + {{ "cardExpiredMessage" | i18n }} {{ "changeAtRiskPasswordAndAddWebsite" | i18n }} - - + + {{ "changeAtRiskPassword" | i18n }} -

+

{{ "noEditPermissions" | i18n }}

- + - + - + - - + + - - + + - + - + diff --git a/libs/vault/src/cipher-view/cipher-view.component.spec.ts b/libs/vault/src/cipher-view/cipher-view.component.spec.ts new file mode 100644 index 0000000000..18a5132781 --- /dev/null +++ b/libs/vault/src/cipher-view/cipher-view.component.spec.ts @@ -0,0 +1,287 @@ +import { NO_ERRORS_SCHEMA } from "@angular/core"; +import { ComponentFixture, TestBed, fakeAsync, tick } from "@angular/core/testing"; +import { mock } from "jest-mock-extended"; +import { BehaviorSubject } from "rxjs"; + +// eslint-disable-next-line no-restricted-imports +import { CollectionService } from "@bitwarden/admin-console/common"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { AccountService, Account } from "@bitwarden/common/auth/abstractions/account.service"; +import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; +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 { CipherRiskService } from "@bitwarden/common/vault/abstractions/cipher-risk.service"; +import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; +import { ViewPasswordHistoryService } from "@bitwarden/common/vault/abstractions/view-password-history.service"; +import { CipherType } from "@bitwarden/common/vault/enums"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { TaskService } from "@bitwarden/common/vault/tasks"; + +import { ChangeLoginPasswordService } from "../abstractions/change-login-password.service"; + +import { CipherViewComponent } from "./cipher-view.component"; + +describe("CipherViewComponent", () => { + let component: CipherViewComponent; + let fixture: ComponentFixture; + + // Mock services + let mockAccountService: AccountService; + let mockOrganizationService: OrganizationService; + let mockCollectionService: CollectionService; + let mockFolderService: FolderService; + let mockTaskService: TaskService; + let mockPlatformUtilsService: PlatformUtilsService; + let mockChangeLoginPasswordService: ChangeLoginPasswordService; + let mockCipherService: CipherService; + let mockViewPasswordHistoryService: ViewPasswordHistoryService; + let mockI18nService: I18nService; + let mockLogService: LogService; + let mockCipherRiskService: CipherRiskService; + let mockBillingAccountProfileStateService: BillingAccountProfileStateService; + let mockConfigService: ConfigService; + + // Mock data + let mockCipherView: CipherView; + let featureFlagEnabled$: BehaviorSubject; + let hasPremiumFromAnySource$: BehaviorSubject; + let activeAccount$: BehaviorSubject; + + beforeEach(async () => { + // Setup mock observables + activeAccount$ = new BehaviorSubject({ + id: "test-user-id", + email: "test@example.com", + } as Account); + + featureFlagEnabled$ = new BehaviorSubject(false); + hasPremiumFromAnySource$ = new BehaviorSubject(true); + + // Create service mocks + mockAccountService = mock(); + mockAccountService.activeAccount$ = activeAccount$; + + mockOrganizationService = mock(); + mockCollectionService = mock(); + mockFolderService = mock(); + mockTaskService = mock(); + mockPlatformUtilsService = mock(); + mockChangeLoginPasswordService = mock(); + mockCipherService = mock(); + mockViewPasswordHistoryService = mock(); + mockI18nService = mock({ + t: (key: string) => key, + }); + mockLogService = mock(); + mockCipherRiskService = mock(); + + mockBillingAccountProfileStateService = mock(); + mockBillingAccountProfileStateService.hasPremiumFromAnySource$ = jest + .fn() + .mockReturnValue(hasPremiumFromAnySource$); + + mockConfigService = mock(); + mockConfigService.getFeatureFlag$ = jest.fn().mockReturnValue(featureFlagEnabled$); + + // Setup mock cipher view + mockCipherView = new CipherView(); + mockCipherView.id = "cipher-id"; + mockCipherView.name = "Test Cipher"; + + await TestBed.configureTestingModule({ + imports: [CipherViewComponent], + providers: [ + { provide: AccountService, useValue: mockAccountService }, + { provide: OrganizationService, useValue: mockOrganizationService }, + { provide: CollectionService, useValue: mockCollectionService }, + { provide: FolderService, useValue: mockFolderService }, + { provide: TaskService, useValue: mockTaskService }, + { provide: PlatformUtilsService, useValue: mockPlatformUtilsService }, + { provide: ChangeLoginPasswordService, useValue: mockChangeLoginPasswordService }, + { provide: CipherService, useValue: mockCipherService }, + { provide: ViewPasswordHistoryService, useValue: mockViewPasswordHistoryService }, + { provide: I18nService, useValue: mockI18nService }, + { provide: LogService, useValue: mockLogService }, + { provide: CipherRiskService, useValue: mockCipherRiskService }, + { + provide: BillingAccountProfileStateService, + useValue: mockBillingAccountProfileStateService, + }, + { provide: ConfigService, useValue: mockConfigService }, + ], + schemas: [NO_ERRORS_SCHEMA], + }) + // Override the component template to avoid rendering child components + // Allows testing component logic without + // needing to provide dependencies for all child components. + .overrideComponent(CipherViewComponent, { + set: { + template: "
{{ passwordIsAtRisk() }}
", + imports: [], + }, + }) + .compileComponents(); + + fixture = TestBed.createComponent(CipherViewComponent); + component = fixture.componentInstance; + }); + + describe("passwordIsAtRisk signal", () => { + // Helper to create a cipher view with login credentials + const createLoginCipherView = (): CipherView => { + const cipher = new CipherView(); + cipher.id = "cipher-id"; + cipher.name = "Test Login"; + cipher.type = CipherType.Login; + cipher.edit = true; + cipher.organizationId = undefined; + // Set up login with password so hasLoginPassword returns true + cipher.login = { password: "test-password" } as any; + return cipher; + }; + + beforeEach(() => { + // Reset observables to default values for this test suite + featureFlagEnabled$.next(true); + hasPremiumFromAnySource$.next(true); + + // Setup default mock for computeCipherRiskForUser (individual tests can override) + mockCipherRiskService.computeCipherRiskForUser = jest.fn().mockResolvedValue({ + password_strength: 4, + exposed_result: { type: "NotFound" }, + reuse_count: 1, + }); + + // Recreate the fixture for each test in this suite. + // This ensures that the signal's observable subscribes with the correct + // initial state + fixture = TestBed.createComponent(CipherViewComponent); + component = fixture.componentInstance; + }); + + it("returns false when feature flag is disabled", fakeAsync(() => { + featureFlagEnabled$.next(false); + + const cipher = createLoginCipherView(); + fixture.componentRef.setInput("cipher", cipher); + fixture.detectChanges(); + tick(); + + expect(mockCipherRiskService.computeCipherRiskForUser).not.toHaveBeenCalled(); + expect(component.passwordIsAtRisk()).toBe(false); + })); + + it("returns false when cipher has no login password", fakeAsync(() => { + const cipher = createLoginCipherView(); + cipher.login = {} as any; // No password + + fixture.componentRef.setInput("cipher", cipher); + fixture.detectChanges(); + tick(); + + expect(mockCipherRiskService.computeCipherRiskForUser).not.toHaveBeenCalled(); + expect(component.passwordIsAtRisk()).toBe(false); + })); + + it("returns false when user does not have edit access", fakeAsync(() => { + const cipher = createLoginCipherView(); + cipher.edit = false; + + fixture.componentRef.setInput("cipher", cipher); + fixture.detectChanges(); + tick(); + + expect(mockCipherRiskService.computeCipherRiskForUser).not.toHaveBeenCalled(); + expect(component.passwordIsAtRisk()).toBe(false); + })); + + it("returns false when cipher is deleted", fakeAsync(() => { + const cipher = createLoginCipherView(); + cipher.deletedDate = new Date(); + + fixture.componentRef.setInput("cipher", cipher); + fixture.detectChanges(); + tick(); + + expect(mockCipherRiskService.computeCipherRiskForUser).not.toHaveBeenCalled(); + expect(component.passwordIsAtRisk()).toBe(false); + })); + + it("returns false for organization-owned ciphers", fakeAsync(() => { + const cipher = createLoginCipherView(); + cipher.organizationId = "org-id"; + + fixture.componentRef.setInput("cipher", cipher); + fixture.detectChanges(); + tick(); + + expect(mockCipherRiskService.computeCipherRiskForUser).not.toHaveBeenCalled(); + expect(component.passwordIsAtRisk()).toBe(false); + })); + + it("returns false when user is not premium", fakeAsync(() => { + hasPremiumFromAnySource$.next(false); + + const cipher = createLoginCipherView(); + fixture.componentRef.setInput("cipher", cipher); + fixture.detectChanges(); + tick(); + + expect(mockCipherRiskService.computeCipherRiskForUser).not.toHaveBeenCalled(); + expect(component.passwordIsAtRisk()).toBe(false); + })); + + it("returns true when password is weak", fakeAsync(() => { + // Setup mock to return weak password + const mockRiskyResult = { + password_strength: 2, // Weak password (< 3) + exposed_result: { type: "NotFound" }, + reuse_count: 1, + }; + mockCipherRiskService.computeCipherRiskForUser = jest.fn().mockResolvedValue(mockRiskyResult); + + const cipher = createLoginCipherView(); + fixture.componentRef.setInput("cipher", cipher); + fixture.detectChanges(); + + // Initial value should be false (from startWith(false)) + expect(component.passwordIsAtRisk()).toBe(false); + + // Wait for async operations to complete + tick(); + fixture.detectChanges(); + + // After async completes, should reflect the weak password + expect(mockCipherRiskService.computeCipherRiskForUser).toHaveBeenCalled(); + expect(component.passwordIsAtRisk()).toBe(true); + })); + + it("returns false when password is strong and not exposed", fakeAsync(() => { + // Setup mock to return safe password + const mockSafeResult = { + password_strength: 4, // Strong password + exposed_result: { type: "NotFound" }, // Not exposed + reuse_count: 1, // Not reused + }; + mockCipherRiskService.computeCipherRiskForUser = jest.fn().mockResolvedValue(mockSafeResult); + + const cipher = createLoginCipherView(); + fixture.componentRef.setInput("cipher", cipher); + fixture.detectChanges(); + + // Initial value should be false + expect(component.passwordIsAtRisk()).toBe(false); + + // Wait for async operations to complete + tick(); + fixture.detectChanges(); + + // Should remain false for safe password + expect(mockCipherRiskService.computeCipherRiskForUser).toHaveBeenCalled(); + expect(component.passwordIsAtRisk()).toBe(false); + })); + }); +}); diff --git a/libs/vault/src/cipher-view/cipher-view.component.ts b/libs/vault/src/cipher-view/cipher-view.component.ts index 15cb7d4651..043a5a63c4 100644 --- a/libs/vault/src/cipher-view/cipher-view.component.ts +++ b/libs/vault/src/cipher-view/cipher-view.component.ts @@ -1,30 +1,38 @@ import { CommonModule } from "@angular/common"; -import { Component, Input, OnChanges, OnDestroy } from "@angular/core"; -import { firstValueFrom, Observable, Subject, takeUntil } from "rxjs"; +import { ChangeDetectionStrategy, Component, computed, input } from "@angular/core"; +import { toObservable, toSignal } from "@angular/core/rxjs-interop"; +import { combineLatest, of, switchMap, map, catchError, from, Observable, startWith } 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 import { CollectionService, CollectionView } from "@bitwarden/admin-console/common"; import { JslibModule } from "@bitwarden/angular/jslib.module"; -import { - getOrganizationById, - OrganizationService, -} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; -import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { isCardExpired } from "@bitwarden/common/autofill/utils"; +import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { getByIds } from "@bitwarden/common/platform/misc"; import { CipherId, EmergencyAccessId, UserId } from "@bitwarden/common/types/guid"; +import { + CipherRiskService, + isPasswordAtRisk, +} from "@bitwarden/common/vault/abstractions/cipher-risk.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; -import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; import { SecurityTaskType, TaskService } from "@bitwarden/common/vault/tasks"; -import { AnchorLinkDirective, CalloutModule, SearchModule } from "@bitwarden/components"; +import { + CalloutModule, + SearchModule, + TypographyModule, + AnchorLinkDirective, +} from "@bitwarden/components"; import { ChangeLoginPasswordService } from "../abstractions/change-login-password.service"; @@ -39,11 +47,10 @@ import { LoginCredentialsViewComponent } from "./login-credentials/login-credent import { SshKeyViewComponent } from "./sshkey-sections/sshkey-view.component"; import { ViewIdentitySectionsComponent } from "./view-identity-sections/view-identity-sections.component"; -// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush -// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @Component({ selector: "app-cipher-view", templateUrl: "cipher-view.component.html", + changeDetection: ChangeDetectionStrategy.OnPush, imports: [ CalloutModule, CommonModule, @@ -60,38 +67,37 @@ import { ViewIdentitySectionsComponent } from "./view-identity-sections/view-ide LoginCredentialsViewComponent, AutofillOptionsViewComponent, AnchorLinkDirective, + TypographyModule, ], }) -export class CipherViewComponent implements OnChanges, OnDestroy { - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input({ required: true }) cipher: CipherView | null = null; +export class CipherViewComponent { + /** + * The cipher to display details for + */ + readonly cipher = input.required(); - // Required for fetching attachment data when viewed from cipher via emergency access - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() emergencyAccessId?: EmergencyAccessId; + /** + * Observable version of the cipher input + */ + private readonly cipher$ = toObservable(this.cipher); - activeUserId$ = getUserId(this.accountService.activeAccount$); + /** + * Required for fetching attachment data when viewed from cipher via emergency access + */ + readonly emergencyAccessId = input(); /** * Optional list of collections the cipher is assigned to. If none are provided, they will be fetched using the * `CipherService` and the `collectionIds` property of the cipher. */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() collections?: CollectionView[]; + readonly collections = input(undefined); - /** Should be set to true when the component is used within the Admin Console */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() isAdminConsole?: boolean = false; + /** + * Should be set to true when the component is used within the Admin Console + */ + readonly isAdminConsole = input(false); - organization$: Observable | undefined; - folder$: Observable | undefined; - private destroyed$: Subject = new Subject(); - cardIsExpired: boolean = false; - hadPendingChangePasswordTask: boolean = false; + readonly activeUserId$ = getUserId(this.accountService.activeAccount$); constructor( private organizationService: OrganizationService, @@ -103,126 +109,206 @@ export class CipherViewComponent implements OnChanges, OnDestroy { private changeLoginPasswordService: ChangeLoginPasswordService, private cipherService: CipherService, private logService: LogService, + private cipherRiskService: CipherRiskService, + private billingAccountService: BillingAccountProfileStateService, + private configService: ConfigService, ) {} - async ngOnChanges() { - if (this.cipher == null) { - return; - } + readonly resolvedCollections = toSignal( + combineLatest([this.activeUserId$, this.cipher$, toObservable(this.collections)]).pipe( + switchMap(([userId, cipher, providedCollections]) => { + // Use provided collections if available + if (providedCollections && providedCollections.length > 0) { + return of(providedCollections); + } + // Otherwise, load collections based on cipher's collectionIds + if (cipher.collectionIds && cipher.collectionIds.length > 0) { + return this.collectionService + .decryptedCollections$(userId) + .pipe(getByIds(cipher.collectionIds)); + } + return of(undefined); + }), + ), + ); - await this.loadCipherData(); + readonly organization = toSignal( + combineLatest([this.activeUserId$, this.cipher$]).pipe( + switchMap(([userId, cipher]) => { + if (!userId || !cipher?.organizationId) { + return of(undefined); + } + return this.organizationService.organizations$(userId).pipe( + map((organizations) => { + return organizations.find((org) => org.id === cipher.organizationId); + }), + ); + }), + ), + ); + readonly folder = toSignal( + combineLatest([this.activeUserId$, this.cipher$]).pipe( + switchMap(([userId, cipher]) => { + if (!userId || !cipher?.folderId) { + return of(undefined); + } + return this.folderService.getDecrypted$(cipher.folderId, userId); + }), + ), + ); - this.cardIsExpired = isCardExpired(this.cipher.card); - } + readonly hadPendingChangePasswordTask = toSignal( + combineLatest([this.activeUserId$, this.cipher$]).pipe( + switchMap(([userId, cipher]) => { + // Early exit if not a Login cipher owned by an organization + if (cipher?.type !== CipherType.Login || !cipher?.organizationId) { + return of(false); + } - ngOnDestroy(): void { - this.destroyed$.next(); - this.destroyed$.complete(); - } + return combineLatest([ + this.cipherService.ciphers$(userId), + this.defaultTaskService.pendingTasks$(userId), + ]).pipe( + map(([allCiphers, tasks]) => { + const cipherServiceCipher = allCiphers[cipher?.id as CipherId]; - get hasCard() { - if (!this.cipher) { + // Show tasks only for Manage and Edit permissions + if (!cipherServiceCipher?.edit || !cipherServiceCipher?.viewPassword) { + return false; + } + + return ( + tasks?.some( + (task) => + task.cipherId === cipher?.id && + task.type === SecurityTaskType.UpdateAtRiskCredential, + ) ?? false + ); + }), + catchError((error: unknown) => { + this.logService.error("Failed to retrieve change password tasks for cipher", error); + return of(false); + }), + ); + }), + ), + { initialValue: false }, + ); + + readonly hasCard = computed(() => { + const cipher = this.cipher(); + if (!cipher) { return false; } - const { cardholderName, code, expMonth, expYear, number } = this.cipher.card; + const { cardholderName, code, expMonth, expYear, number } = cipher.card; return cardholderName || code || expMonth || expYear || number; - } + }); - get hasLogin() { - if (!this.cipher) { + readonly cardIsExpired = computed(() => { + const cipher = this.cipher(); + if (cipher == null) { + return false; + } + return isCardExpired(cipher.card); + }); + + readonly hasLogin = computed(() => { + const cipher = this.cipher(); + if (!cipher) { return false; } - const { username, password, totp, fido2Credentials } = this.cipher.login; + const { username, password, totp, fido2Credentials } = cipher.login; return username || password || totp || fido2Credentials?.length > 0; - } + }); - get hasAutofill() { - const uris = this.cipher?.login?.uris.length ?? 0; + readonly hasAutofill = computed(() => { + const cipher = this.cipher(); + const uris = cipher?.login?.uris.length ?? 0; return uris > 0; - } + }); - get hasSshKey() { - return !!this.cipher?.sshKey?.privateKey; - } + readonly hasSshKey = computed(() => { + const cipher = this.cipher(); + return !!cipher?.sshKey?.privateKey; + }); - get hasLoginUri() { - return this.cipher?.login?.hasUris; - } + readonly hasLoginUri = computed(() => { + const cipher = this.cipher(); + return cipher?.login?.hasUris; + }); - async loadCipherData() { - if (!this.cipher) { - return; - } - - const userId = await firstValueFrom(this.activeUserId$); - - // Load collections if not provided and the cipher has collectionIds - if ( - this.cipher.collectionIds && - this.cipher.collectionIds.length > 0 && - (!this.collections || this.collections.length === 0) - ) { - this.collections = await firstValueFrom( - this.collectionService - .decryptedCollections$(userId) - .pipe(getByIds(this.cipher.collectionIds)), - ); - } - - if (this.cipher.organizationId) { - this.organization$ = this.organizationService - .organizations$(userId) - .pipe(getOrganizationById(this.cipher.organizationId)) - .pipe(takeUntil(this.destroyed$)); - - if (this.cipher.type === CipherType.Login) { - await this.checkPendingChangePasswordTasks(userId); - } - } - - if (this.cipher.folderId) { - this.folder$ = this.folderService - .getDecrypted$(this.cipher.folderId, userId) - .pipe(takeUntil(this.destroyed$)); - } - } - - async checkPendingChangePasswordTasks(userId: UserId): Promise { - try { - // Show Tasks for Manage and Edit permissions - // Using cipherService to see if user has access to cipher in a non-AC context to address with Edit Except Password permissions - const allCiphers = await firstValueFrom(this.cipherService.ciphers$(userId)); - const cipherServiceCipher = allCiphers[this.cipher?.id as CipherId]; - - if (!cipherServiceCipher?.edit || !cipherServiceCipher?.viewPassword) { - this.hadPendingChangePasswordTask = false; - return; - } - - const tasks = await firstValueFrom(this.defaultTaskService.pendingTasks$(userId)); - - this.hadPendingChangePasswordTask = tasks?.some((task) => { - return ( - task.cipherId === this.cipher?.id && task.type === SecurityTaskType.UpdateAtRiskCredential + /** + * Whether the login password for the cipher is considered at risk. + * The password is only evaluated when the user is premium and has edit access to the cipher. + */ + readonly passwordIsAtRisk = toSignal( + combineLatest([ + this.activeUserId$, + this.cipher$, + this.configService.getFeatureFlag$(FeatureFlag.RiskInsightsForPremium), + ]).pipe( + switchMap(([userId, cipher, featureEnabled]) => { + if ( + !featureEnabled || + !cipher.hasLoginPassword || + !cipher.edit || + cipher.organizationId || + cipher.isDeleted + ) { + return of(false); + } + return this.switchPremium$( + userId, + () => + from(this.checkIfPasswordIsAtRisk(cipher.id as CipherId, userId as UserId)).pipe( + startWith(false), + ), + () => of(false), ); - }); - } catch (error) { - this.hadPendingChangePasswordTask = false; - this.logService.error("Failed to retrieve change password tasks for cipher", error); - } - } + }), + ), + { initialValue: false }, + ); + + readonly showChangePasswordLink = computed(() => { + return this.hasLoginUri() && (this.hadPendingChangePasswordTask() || this.passwordIsAtRisk()); + }); launchChangePassword = async () => { - if (this.cipher != null) { - const url = await this.changeLoginPasswordService.getChangePasswordUrl(this.cipher); + const cipher = this.cipher(); + if (cipher != null) { + const url = await this.changeLoginPasswordService.getChangePasswordUrl(cipher); if (url == null) { return; } this.platformUtilsService.launchUri(url); } }; + + /** + * Switches between two observables based on whether the user has a premium from any source. + */ + private switchPremium$( + userId: UserId, + ifPremium$: () => Observable, + ifNonPremium$: () => Observable, + ): Observable { + return this.billingAccountService + .hasPremiumFromAnySource$(userId) + .pipe(switchMap((isPremium) => (isPremium ? ifPremium$() : ifNonPremium$()))); + } + + private async checkIfPasswordIsAtRisk(cipherId: CipherId, userId: UserId): Promise { + try { + const risk = await this.cipherRiskService.computeCipherRiskForUser(cipherId, userId, true); + return isPasswordAtRisk(risk); + } catch (error: unknown) { + this.logService.error("Failed to check if password is at risk", error); + return false; + } + } } diff --git a/libs/vault/src/cipher-view/login-credentials/login-credentials-view.component.html b/libs/vault/src/cipher-view/login-credentials/login-credentials-view.component.html index 02b8be552b..be33f7a556 100644 --- a/libs/vault/src/cipher-view/login-credentials/login-credentials-view.component.html +++ b/libs/vault/src/cipher-view/login-credentials/login-credentials-view.component.html @@ -90,13 +90,13 @@ data-testid="copy-password" (click)="logCopyEvent()" > + + + {{ "changeAtRiskPassword" | i18n }} + + + - - - {{ "changeAtRiskPassword" | i18n }} - - -
();