diff --git a/.github/renovate.json5 b/.github/renovate.json5 index c4c24799da1..b402d01e209 100644 --- a/.github/renovate.json5 +++ b/.github/renovate.json5 @@ -132,6 +132,7 @@ "@yao-pkg/pkg", "anyhow", "arboard", + "ashpd", "babel-loader", "base64-loader", "base64", @@ -142,6 +143,7 @@ "core-foundation", "copy-webpack-plugin", "css-loader", + "ctor", "dirs", "electron", "electron-builder", @@ -179,6 +181,7 @@ "sass", "sass-loader", "scopeguard", + "secmem-proc", "security-framework", "security-framework-sys", "semver", @@ -187,6 +190,7 @@ "simplelog", "style-loader", "sysinfo", + "thiserror", "tokio", "tokio-util", "tracing", @@ -210,6 +214,7 @@ "windows-registry", "zbus", "zbus_polkit", + "zeroizing-alloc", ], description: "Platform owned dependencies", commitMessagePrefix: "[deps] Platform:", @@ -285,6 +290,7 @@ "@types/jsdom", "@types/papaparse", "@types/zxcvbn", + "aes-gcm", "async-trait", "clap", "jsdom", @@ -337,6 +343,7 @@ "aes", "big-integer", "cbc", + "chacha20poly1305", "linux-keyutils", "memsec", "node-forge", @@ -445,6 +452,7 @@ matchPackageNames: [ "anyhow", "arboard", + "ashpd", "babel-loader", "base64-loader", "base64", @@ -454,6 +462,7 @@ "core-foundation", "copy-webpack-plugin", "css-loader", + "ctor", "dirs", "electron-builder", "electron-log", @@ -488,6 +497,7 @@ "sass", "sass-loader", "scopeguard", + "secmem-proc", "security-framework", "security-framework-sys", "semver", @@ -496,6 +506,7 @@ "simplelog", "style-loader", "sysinfo", + "thiserror", "tokio", "tokio-util", "tracing", @@ -517,6 +528,7 @@ "windows-registry", "zbus", "zbus_polkit", + "zeroizing-alloc", ], matchUpdateTypes: ["minor", "patch"], dependencyDashboardApproval: true, diff --git a/.github/workflows/build-desktop.yml b/.github/workflows/build-desktop.yml index f3cdf80f710..8e43127770c 100644 --- a/.github/workflows/build-desktop.yml +++ b/.github/workflows/build-desktop.yml @@ -179,18 +179,8 @@ jobs: ref: ${{ github.event.pull_request.head.sha }} persist-credentials: false - - name: Free disk space for build - run: | - sudo rm -rf /usr/share/dotnet - sudo rm -rf /usr/share/swift - sudo rm -rf /usr/local/.ghcup - sudo rm -rf /usr/share/miniconda - sudo rm -rf /usr/share/az_* - sudo rm -rf /usr/local/julia* - sudo rm -rf /usr/lib/mono - sudo rm -rf /usr/lib/heroku - sudo rm -rf /usr/local/aws-cli - sudo rm -rf /usr/local/aws-sam-cli + - name: Free disk space + uses: bitwarden/gh-actions/free-disk-space@main - name: Set up Node uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 @@ -265,7 +255,7 @@ jobs: # Note: It is important that we use the release build because some compute heavy # operations such as key derivation for oo7 on linux are too slow in debug mode run: | - node build.js --release + node build.js --target=x86_64-unknown-linux-gnu --release - name: Build application run: npm run dist:lin @@ -428,7 +418,7 @@ jobs: # Note: It is important that we use the release build because some compute heavy # operations such as key derivation for oo7 on linux are too slow in debug mode run: | - node build.js --release + node build.js --target=aarch64-unknown-linux-gnu --release - name: Check index.d.ts generated if: github.event_name == 'pull_request' && steps.cache.outputs.cache-hit != 'true' diff --git a/.github/workflows/review-code.yml b/.github/workflows/review-code.yml index 0e0597fccf0..000f4020961 100644 --- a/.github/workflows/review-code.yml +++ b/.github/workflows/review-code.yml @@ -2,7 +2,7 @@ name: Code Review on: pull_request: - types: [opened, synchronize, reopened, ready_for_review] + types: [opened, synchronize, reopened] permissions: {} diff --git a/apps/browser/src/auth/popup/settings/account-security.component.spec.ts b/apps/browser/src/auth/popup/settings/account-security.component.spec.ts index 0f799fe7d4d..ebabbadf71c 100644 --- a/apps/browser/src/auth/popup/settings/account-security.component.spec.ts +++ b/apps/browser/src/auth/popup/settings/account-security.component.spec.ts @@ -1,5 +1,5 @@ import { Component } from "@angular/core"; -import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { ComponentFixture, fakeAsync, TestBed, tick } from "@angular/core/testing"; import { By } from "@angular/platform-browser"; import { ActivatedRoute } from "@angular/router"; import { mock } from "jest-mock-extended"; @@ -37,7 +37,12 @@ import { UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { DialogService, ToastService } from "@bitwarden/components"; import { newGuid } from "@bitwarden/guid"; -import { BiometricStateService, BiometricsService, KeyService } from "@bitwarden/key-management"; +import { + BiometricStateService, + BiometricsService, + BiometricsStatus, + KeyService, +} from "@bitwarden/key-management"; import { BrowserApi } from "../../../platform/browser/browser-api"; import BrowserPopupUtils from "../../../platform/browser/browser-popup-utils"; @@ -64,6 +69,7 @@ describe("AccountSecurityComponent", () => { const apiService = mock(); const billingService = mock(); const biometricStateService = mock(); + const biometricsService = mock(); const configService = mock(); const dialogService = mock(); const keyService = mock(); @@ -75,6 +81,7 @@ describe("AccountSecurityComponent", () => { const validationService = mock(); const vaultNudgesService = mock(); const vaultTimeoutSettingsService = mock(); + const mockI18nService = mock(); // Mock subjects to control the phishing detection observables let phishingAvailableSubject: BehaviorSubject; @@ -91,14 +98,14 @@ describe("AccountSecurityComponent", () => { provide: BillingAccountProfileStateService, useValue: billingService, }, - { provide: BiometricsService, useValue: mock() }, + { provide: BiometricsService, useValue: biometricsService }, { provide: BiometricStateService, useValue: biometricStateService }, { provide: CipherService, useValue: mock() }, { provide: CollectionService, useValue: mock() }, { provide: ConfigService, useValue: configService }, { provide: DialogService, useValue: dialogService }, { provide: EnvironmentService, useValue: mock() }, - { provide: I18nService, useValue: mock() }, + { provide: I18nService, useValue: mockI18nService }, { provide: KeyService, useValue: keyService }, { provide: LockService, useValue: lockService }, { provide: LogService, useValue: mock() }, @@ -153,6 +160,7 @@ describe("AccountSecurityComponent", () => { pinServiceAbstraction.isPinSet.mockResolvedValue(false); configService.getFeatureFlag$.mockReturnValue(of(false)); billingService.hasPremiumPersonally$.mockReturnValue(of(true)); + mockI18nService.t.mockImplementation((key) => `${key}-used-i18n`); policyService.policiesByType$.mockReturnValue(of([null])); @@ -459,4 +467,118 @@ describe("AccountSecurityComponent", () => { }); }); }); + + describe("biometrics polling timer", () => { + let browserApiSpy: jest.SpyInstance; + + beforeEach(() => { + browserApiSpy = jest.spyOn(BrowserApi, "permissionsGranted"); + }); + + afterEach(() => { + component.ngOnDestroy(); + }); + + it("disables biometric control when canEnableBiometricUnlock is false", fakeAsync(async () => { + biometricsService.canEnableBiometricUnlock.mockResolvedValue(false); + + await component.ngOnInit(); + tick(); + + expect(component.form.controls.biometric.disabled).toBe(true); + })); + + it("enables biometric control when canEnableBiometricUnlock is true", fakeAsync(async () => { + biometricsService.canEnableBiometricUnlock.mockResolvedValue(true); + + await component.ngOnInit(); + tick(); + + expect(component.form.controls.biometric.disabled).toBe(false); + })); + + it("skips status check when nativeMessaging permission is not granted and not Safari", fakeAsync(async () => { + biometricsService.canEnableBiometricUnlock.mockResolvedValue(true); + browserApiSpy.mockResolvedValue(false); + platformUtilsService.isSafari.mockReturnValue(false); + + await component.ngOnInit(); + tick(); + + expect(biometricsService.getBiometricsStatusForUser).not.toHaveBeenCalled(); + expect(component.biometricUnavailabilityReason).toBeUndefined(); + })); + + it("checks biometrics status when nativeMessaging permission is granted", fakeAsync(async () => { + biometricsService.canEnableBiometricUnlock.mockResolvedValue(true); + browserApiSpy.mockResolvedValue(true); + platformUtilsService.isSafari.mockReturnValue(false); + biometricsService.getBiometricsStatusForUser.mockResolvedValue( + BiometricsStatus.DesktopDisconnected, + ); + + await component.ngOnInit(); + tick(); + + expect(biometricsService.getBiometricsStatusForUser).toHaveBeenCalledWith(mockUserId); + })); + + it("should check status on Safari", fakeAsync(async () => { + biometricsService.canEnableBiometricUnlock.mockResolvedValue(true); + browserApiSpy.mockResolvedValue(false); + platformUtilsService.isSafari.mockReturnValue(true); + biometricsService.getBiometricsStatusForUser.mockResolvedValue( + BiometricsStatus.DesktopDisconnected, + ); + + await component.ngOnInit(); + tick(); + + expect(biometricsService.getBiometricsStatusForUser).toHaveBeenCalledWith(mockUserId); + })); + + test.each([ + [ + BiometricsStatus.DesktopDisconnected, + "biometricsStatusHelptextDesktopDisconnected-used-i18n", + ], + [ + BiometricsStatus.NotEnabledInConnectedDesktopApp, + "biometricsStatusHelptextNotEnabledInDesktop-used-i18n", + ], + [ + BiometricsStatus.HardwareUnavailable, + "biometricsStatusHelptextHardwareUnavailable-used-i18n", + ], + ])( + "sets expected unavailability reason for %s status when biometric not available", + fakeAsync(async (biometricStatus: BiometricsStatus, expected: string) => { + biometricsService.canEnableBiometricUnlock.mockResolvedValue(false); + browserApiSpy.mockResolvedValue(true); + platformUtilsService.isSafari.mockReturnValue(false); + biometricsService.getBiometricsStatusForUser.mockResolvedValue(biometricStatus); + + await component.ngOnInit(); + tick(); + + expect(component.biometricUnavailabilityReason).toBe(expected); + }), + ); + + it("should not set unavailability reason for error statuses when biometric is available", fakeAsync(async () => { + biometricsService.canEnableBiometricUnlock.mockResolvedValue(true); + browserApiSpy.mockResolvedValue(true); + platformUtilsService.isSafari.mockReturnValue(false); + biometricsService.getBiometricsStatusForUser.mockResolvedValue( + BiometricsStatus.DesktopDisconnected, + ); + + await component.ngOnInit(); + tick(); + + // Status is DesktopDisconnected but biometric IS available, so don't show error + expect(component.biometricUnavailabilityReason).toBe(""); + component.ngOnDestroy(); + })); + }); }); diff --git a/apps/browser/src/auth/popup/settings/account-security.component.ts b/apps/browser/src/auth/popup/settings/account-security.component.ts index 7c36754c894..6a3378670bf 100644 --- a/apps/browser/src/auth/popup/settings/account-security.component.ts +++ b/apps/browser/src/auth/popup/settings/account-security.component.ts @@ -149,6 +149,7 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { protected refreshTimeoutSettings$ = new BehaviorSubject(undefined); private destroy$ = new Subject(); + private readonly BIOMETRICS_POLLING_INTERVAL = 2000; constructor( private accountService: AccountService, @@ -264,10 +265,9 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { }; this.form.patchValue(initialValues, { emitEvent: false }); - timer(0, 1000) + timer(0, this.BIOMETRICS_POLLING_INTERVAL) .pipe( switchMap(async () => { - const status = await this.biometricsService.getBiometricsStatusForUser(activeAccount.id); const biometricSettingAvailable = await this.biometricsService.canEnableBiometricUnlock(); if (!biometricSettingAvailable) { this.form.controls.biometric.disable({ emitEvent: false }); @@ -275,6 +275,15 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { this.form.controls.biometric.enable({ emitEvent: false }); } + // Biometrics status shouldn't be checked if permissions are needed. + const needsPermissionPrompt = + !(await BrowserApi.permissionsGranted(["nativeMessaging"])) && + !this.platformUtilsService.isSafari(); + if (needsPermissionPrompt) { + return; + } + + const status = await this.biometricsService.getBiometricsStatusForUser(activeAccount.id); if (status === BiometricsStatus.DesktopDisconnected && !biometricSettingAvailable) { this.biometricUnavailabilityReason = this.i18nService.t( "biometricsStatusHelptextDesktopDisconnected", diff --git a/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.ts b/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.ts index 64ef7d180ed..ad1241e98d2 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.ts @@ -1,5 +1,3 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { EVENTS } from "@bitwarden/common/autofill/constants"; import { ThemeTypes } from "@bitwarden/common/platform/enums"; @@ -15,14 +13,17 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe private readonly setElementStyles = setElementStyles; private readonly sendExtensionMessage = sendExtensionMessage; private port: chrome.runtime.Port | null = null; - private portKey: string; + private portKey?: string; private readonly extensionOrigin: string; private iframeMutationObserver: MutationObserver; - private iframe: HTMLIFrameElement; - private ariaAlertElement: HTMLDivElement; - private ariaAlertTimeout: number | NodeJS.Timeout; - private delayedCloseTimeout: number | NodeJS.Timeout; - private fadeInTimeout: number | NodeJS.Timeout; + /** + * Initialized in initMenuIframe which makes it safe to assert non null by lifecycle. + */ + private iframe!: HTMLIFrameElement; + private ariaAlertElement?: HTMLDivElement; + private ariaAlertTimeout: number | NodeJS.Timeout | null = null; + private delayedCloseTimeout: number | NodeJS.Timeout | null = null; + private fadeInTimeout: number | NodeJS.Timeout | null = null; private readonly fadeInOpacityTransition = "opacity 125ms ease-out 0s"; private readonly fadeOutOpacityTransition = "opacity 65ms ease-out 0s"; private iframeStyles: Partial = { @@ -50,7 +51,7 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe }; private foreignMutationsCount = 0; private mutationObserverIterations = 0; - private mutationObserverIterationsResetTimeout: number | NodeJS.Timeout; + private mutationObserverIterationsResetTimeout: number | NodeJS.Timeout | null = null; private readonly backgroundPortMessageHandlers: BackgroundPortMessageHandlers = { initAutofillInlineMenuButton: ({ message }) => this.initAutofillInlineMenu(message), initAutofillInlineMenuList: ({ message }) => this.initAutofillInlineMenu(message), @@ -134,7 +135,9 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe this.port.onDisconnect.addListener(this.handlePortDisconnect); this.port.onMessage.addListener(this.handlePortMessage); - this.announceAriaAlert(this.ariaAlert, 2000); + if (this.ariaAlert) { + this.announceAriaAlert(this.ariaAlert, 2000); + } }; /** @@ -155,7 +158,7 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe this.ariaAlertTimeout = globalThis.setTimeout(async () => { const isFieldFocused = await this.sendExtensionMessage("checkIsFieldCurrentlyFocused"); - if (isFieldFocused || triggeredByUser) { + if ((isFieldFocused || triggeredByUser) && this.ariaAlertElement) { this.shadow.appendChild(this.ariaAlertElement); } this.ariaAlertTimeout = null; @@ -242,7 +245,7 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe */ private initAutofillInlineMenuList(message: AutofillInlineMenuIframeExtensionMessage) { const { theme } = message; - let borderColor: string; + let borderColor: string | undefined; let verifiedTheme = theme; if (verifiedTheme === ThemeTypes.System) { verifiedTheme = globalThis.matchMedia("(prefers-color-scheme: dark)").matches @@ -274,8 +277,8 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe * * @param position - The position styles to apply to the iframe */ - private updateIframePosition(position: Partial) { - if (!globalThis.document.hasFocus()) { + private updateIframePosition(position?: Partial) { + if (!position || !globalThis.document.hasFocus()) { return; } @@ -295,7 +298,9 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe this.handleFadeInInlineMenuIframe(); } - this.announceAriaAlert(this.ariaAlert, 2000); + if (this.ariaAlert) { + this.announceAriaAlert(this.ariaAlert, 2000); + } } /** @@ -359,8 +364,8 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe * @param customElement - The element to update the styles for * @param styles - The styles to apply to the element */ - private updateElementStyles(customElement: HTMLElement, styles: Partial) { - if (!customElement) { + private updateElementStyles(customElement: HTMLElement, styles?: Partial) { + if (!customElement || !styles) { return; } diff --git a/apps/browser/src/autofill/popup/settings/autofill.component.html b/apps/browser/src/autofill/popup/settings/autofill.component.html index 1153ad58719..085145adb19 100644 --- a/apps/browser/src/autofill/popup/settings/autofill.component.html +++ b/apps/browser/src/autofill/popup/settings/autofill.component.html @@ -6,16 +6,18 @@
-
- -
+ @if (showSpotlightNudge$ | async) { +
+ +
+ }

{{ "autofillSuggestionsSectionTitle" | i18n }}

diff --git a/apps/browser/src/autofill/popup/settings/autofill.component.ts b/apps/browser/src/autofill/popup/settings/autofill.component.ts index 49be3104dc1..acb2aa7a970 100644 --- a/apps/browser/src/autofill/popup/settings/autofill.component.ts +++ b/apps/browser/src/autofill/popup/settings/autofill.component.ts @@ -611,6 +611,10 @@ export class AutofillComponent implements OnInit { if (this.canOverrideBrowserAutofillSetting) { this.defaultBrowserAutofillDisabled = true; await this.updateDefaultBrowserAutofillDisabled(); + await this.nudgesService.dismissNudge( + NudgeType.AutofillNudge, + await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)), + ); } else { await this.openURI(event, this.disablePasswordManagerURI); } diff --git a/apps/browser/src/autofill/services/collect-autofill-content.service.ts b/apps/browser/src/autofill/services/collect-autofill-content.service.ts index 367599f7ad0..18eb8e2baf8 100644 --- a/apps/browser/src/autofill/services/collect-autofill-content.service.ts +++ b/apps/browser/src/autofill/services/collect-autofill-content.service.ts @@ -60,6 +60,15 @@ export class CollectAutofillContentService implements CollectAutofillContentServ "button", "image", "file", + "search", + "url", + "date", + "time", + "datetime", // Note: datetime is deprecated in HTML5; keeping here for backwards compatibility + "datetime-local", + "week", + "color", + "range", ]); constructor( diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index da5bf390d20..9226fad909c 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -126,6 +126,7 @@ import { FileUploadService as FileUploadServiceAbstraction } from "@bitwarden/co import { I18nService as I18nServiceAbstraction } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService as LogServiceAbstraction } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService as PlatformUtilsServiceAbstraction } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { RegisterSdkService } from "@bitwarden/common/platform/abstractions/sdk/register-sdk.service"; import { SdkLoadService } from "@bitwarden/common/platform/abstractions/sdk/sdk-load.service"; import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; import { StateService as StateServiceAbstraction } from "@bitwarden/common/platform/abstractions/state.service"; @@ -164,6 +165,7 @@ import { MigrationRunner } from "@bitwarden/common/platform/services/migration-r import { DefaultSdkClientFactory } from "@bitwarden/common/platform/services/sdk/default-sdk-client-factory"; import { DefaultSdkService } from "@bitwarden/common/platform/services/sdk/default-sdk.service"; import { NoopSdkClientFactory } from "@bitwarden/common/platform/services/sdk/noop-sdk-client-factory"; +import { DefaultRegisterSdkService } from "@bitwarden/common/platform/services/sdk/register-sdk.service"; import { SystemService } from "@bitwarden/common/platform/services/system.service"; import { UserAutoUnlockKeyService } from "@bitwarden/common/platform/services/user-auto-unlock-key.service"; import { PrimarySecondaryStorageService } from "@bitwarden/common/platform/storage/primary-secondary-storage.service"; @@ -463,6 +465,7 @@ export default class MainBackground { themeStateService: DefaultThemeStateService; autoSubmitLoginBackground: AutoSubmitLoginBackground; sdkService: SdkService; + registerSdkService: RegisterSdkService; sdkLoadService: SdkLoadService; cipherAuthorizationService: CipherAuthorizationService; endUserNotificationService: EndUserNotificationService; @@ -578,7 +581,7 @@ export default class MainBackground { "ephemeral", "bitwarden-ephemeral", ); - await sessionStorage.save("session-key", derivedKey); + await sessionStorage.save("session-key", derivedKey.toJSON()); return derivedKey; }); @@ -797,18 +800,6 @@ export default class MainBackground { this.apiService, this.accountService, ); - this.keyConnectorService = new KeyConnectorService( - this.accountService, - this.masterPasswordService, - this.keyService, - this.apiService, - this.tokenService, - this.logService, - this.organizationService, - this.keyGenerationService, - logoutCallback, - this.stateProvider, - ); this.authService = new AuthService( this.accountService, @@ -846,6 +837,37 @@ export default class MainBackground { this.configService, ); + this.registerSdkService = new DefaultRegisterSdkService( + sdkClientFactory, + this.environmentService, + this.platformUtilsService, + this.accountService, + this.apiService, + this.stateProvider, + this.configService, + ); + + this.accountCryptographicStateService = new DefaultAccountCryptographicStateService( + this.stateProvider, + ); + + this.keyConnectorService = new KeyConnectorService( + this.accountService, + this.masterPasswordService, + this.keyService, + this.apiService, + this.tokenService, + this.logService, + this.organizationService, + this.keyGenerationService, + logoutCallback, + this.stateProvider, + this.configService, + this.registerSdkService, + this.securityStateService, + this.accountCryptographicStateService, + ); + this.pinService = new PinService( this.encryptService, this.logService, @@ -1014,9 +1036,7 @@ export default class MainBackground { this.avatarService = new AvatarService(this.apiService, this.stateProvider); this.providerService = new ProviderService(this.stateProvider); - this.accountCryptographicStateService = new DefaultAccountCryptographicStateService( - this.stateProvider, - ); + this.syncService = new DefaultSyncService( this.masterPasswordService, this.accountService, diff --git a/apps/browser/src/dirt/phishing-detection/phishing-resources.ts b/apps/browser/src/dirt/phishing-detection/phishing-resources.ts new file mode 100644 index 00000000000..262d6cf833b --- /dev/null +++ b/apps/browser/src/dirt/phishing-detection/phishing-resources.ts @@ -0,0 +1,98 @@ +export type PhishingResource = { + name?: string; + remoteUrl: string; + checksumUrl: string; + todayUrl: string; + /** Matcher used to decide whether a given URL matches an entry from this resource */ + match: (url: URL, entry: string) => boolean; +}; + +export const PhishingResourceType = Object.freeze({ + Domains: "domains", + Links: "links", +} as const); + +export type PhishingResourceType = (typeof PhishingResourceType)[keyof typeof PhishingResourceType]; + +export const PHISHING_RESOURCES: Record = { + [PhishingResourceType.Domains]: [ + { + name: "Phishing.Database Domains", + remoteUrl: + "https://raw.githubusercontent.com/Phishing-Database/Phishing.Database/master/phishing-domains-ACTIVE.txt", + checksumUrl: + "https://raw.githubusercontent.com/Phishing-Database/checksums/refs/heads/master/phishing-domains-ACTIVE.txt.md5", + todayUrl: + "https://raw.githubusercontent.com/Phishing-Database/Phishing.Database/refs/heads/master/phishing-domains-NEW-today.txt", + match: (url: URL, entry: string) => { + if (!entry) { + return false; + } + const candidate = entry.trim().toLowerCase().replace(/\/$/, ""); + // If entry contains a scheme, strip it for comparison + const e = candidate.replace(/^https?:\/\//, ""); + // Compare against hostname or host+path + if (e === url.hostname.toLowerCase()) { + return true; + } + const urlNoProto = url.href + .toLowerCase() + .replace(/https?:\/\//, "") + .replace(/\/$/, ""); + return urlNoProto === e || urlNoProto.startsWith(e + "/"); + }, + }, + ], + [PhishingResourceType.Links]: [ + { + name: "Phishing.Database Links", + remoteUrl: + "https://raw.githubusercontent.com/Phishing-Database/Phishing.Database/master/phishing-links-ACTIVE.txt", + checksumUrl: + "https://raw.githubusercontent.com/Phishing-Database/checksums/refs/heads/master/phishing-links-ACTIVE.txt.md5", + todayUrl: + "https://raw.githubusercontent.com/Phishing-Database/Phishing.Database/refs/heads/master/phishing-links-NEW-today.txt", + match: (url: URL, entry: string) => { + if (!entry) { + return false; + } + // Basic HTML entity decode for common cases (the lists sometimes contain &) + const decodeHtml = (s: string) => s.replace(/&/g, "&"); + + const normalizedEntry = decodeHtml(entry.trim()).toLowerCase().replace(/\/$/, ""); + + // Normalize URL for comparison - always strip protocol for consistent matching + const normalizedUrl = decodeHtml(url.href).toLowerCase().replace(/\/$/, ""); + const urlNoProto = normalizedUrl.replace(/^https?:\/\//, ""); + + // Strip protocol from entry if present (http:// and https:// should be treated as equivalent) + const entryNoProto = normalizedEntry.replace(/^https?:\/\//, ""); + + // Compare full path (without protocol) - exact match + if (urlNoProto === entryNoProto) { + return true; + } + + // Check if URL starts with entry (prefix match for subpaths/query/hash) + // e.g., entry "site.com/phish" matches "site.com/phish/subpage" or "site.com/phish?id=1" + if ( + urlNoProto.startsWith(entryNoProto + "/") || + urlNoProto.startsWith(entryNoProto + "?") || + urlNoProto.startsWith(entryNoProto + "#") + ) { + return true; + } + + return false; + }, + }, + ], +}; + +export function getPhishingResources( + type: PhishingResourceType, + index = 0, +): PhishingResource | undefined { + const list = PHISHING_RESOURCES[type] ?? []; + return list[index]; +} diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.spec.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.spec.ts index 94f3e99f8be..30aa947092d 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.spec.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.spec.ts @@ -25,7 +25,7 @@ describe("PhishingDataService", () => { }; let fetchChecksumSpy: jest.SpyInstance; - let fetchDomainsSpy: jest.SpyInstance; + let fetchWebAddressesSpy: jest.SpyInstance; beforeEach(() => { jest.useFakeTimers(); @@ -45,113 +45,113 @@ describe("PhishingDataService", () => { platformUtilsService, ); - fetchChecksumSpy = jest.spyOn(service as any, "fetchPhishingDomainsChecksum"); - fetchDomainsSpy = jest.spyOn(service as any, "fetchPhishingDomains"); + fetchChecksumSpy = jest.spyOn(service as any, "fetchPhishingChecksum"); + fetchWebAddressesSpy = jest.spyOn(service as any, "fetchPhishingWebAddresses"); }); - describe("isPhishingDomains", () => { - it("should detect a phishing domain", async () => { + describe("isPhishingWebAddress", () => { + it("should detect a phishing web address", async () => { setMockState({ - domains: ["phish.com", "badguy.net"], + webAddresses: ["phish.com", "badguy.net"], timestamp: Date.now(), checksum: "abc123", applicationVersion: "1.0.0", }); const url = new URL("http://phish.com"); - const result = await service.isPhishingDomain(url); + const result = await service.isPhishingWebAddress(url); expect(result).toBe(true); }); - it("should not detect a safe domain", async () => { + it("should not detect a safe web address", async () => { setMockState({ - domains: ["phish.com", "badguy.net"], + webAddresses: ["phish.com", "badguy.net"], timestamp: Date.now(), checksum: "abc123", applicationVersion: "1.0.0", }); const url = new URL("http://safe.com"); - const result = await service.isPhishingDomain(url); + const result = await service.isPhishingWebAddress(url); expect(result).toBe(false); }); - it("should match against root domain", async () => { + it("should match against root web address", async () => { setMockState({ - domains: ["phish.com", "badguy.net"], + webAddresses: ["phish.com", "badguy.net"], timestamp: Date.now(), checksum: "abc123", applicationVersion: "1.0.0", }); const url = new URL("http://phish.com/about"); - const result = await service.isPhishingDomain(url); + const result = await service.isPhishingWebAddress(url); expect(result).toBe(true); }); it("should not error on empty state", async () => { setMockState(undefined as any); const url = new URL("http://phish.com/about"); - const result = await service.isPhishingDomain(url); + const result = await service.isPhishingWebAddress(url); expect(result).toBe(false); }); }); - describe("getNextDomains", () => { - it("refetches all domains if applicationVersion has changed", async () => { + describe("getNextWebAddresses", () => { + it("refetches all web addresses if applicationVersion has changed", async () => { const prev: PhishingData = { - domains: ["a.com"], + webAddresses: ["a.com"], timestamp: Date.now() - 60000, checksum: "old", applicationVersion: "1.0.0", }; fetchChecksumSpy.mockResolvedValue("new"); - fetchDomainsSpy.mockResolvedValue(["d.com", "e.com"]); + fetchWebAddressesSpy.mockResolvedValue(["d.com", "e.com"]); platformUtilsService.getApplicationVersion.mockResolvedValue("2.0.0"); - const result = await service.getNextDomains(prev); + const result = await service.getNextWebAddresses(prev); - expect(result!.domains).toEqual(["d.com", "e.com"]); + expect(result!.webAddresses).toEqual(["d.com", "e.com"]); expect(result!.checksum).toBe("new"); expect(result!.applicationVersion).toBe("2.0.0"); }); it("only updates timestamp if checksum matches", async () => { const prev: PhishingData = { - domains: ["a.com"], + webAddresses: ["a.com"], timestamp: Date.now() - 60000, checksum: "abc", applicationVersion: "1.0.0", }; fetchChecksumSpy.mockResolvedValue("abc"); - const result = await service.getNextDomains(prev); - expect(result!.domains).toEqual(prev.domains); + const result = await service.getNextWebAddresses(prev); + expect(result!.webAddresses).toEqual(prev.webAddresses); expect(result!.checksum).toBe("abc"); expect(result!.timestamp).not.toBe(prev.timestamp); }); it("patches daily domains if cache is fresh", async () => { const prev: PhishingData = { - domains: ["a.com"], + webAddresses: ["a.com"], timestamp: Date.now() - 60000, checksum: "old", applicationVersion: "1.0.0", }; fetchChecksumSpy.mockResolvedValue("new"); - fetchDomainsSpy.mockResolvedValue(["b.com", "c.com"]); - const result = await service.getNextDomains(prev); - expect(result!.domains).toEqual(["a.com", "b.com", "c.com"]); + fetchWebAddressesSpy.mockResolvedValue(["b.com", "c.com"]); + const result = await service.getNextWebAddresses(prev); + expect(result!.webAddresses).toEqual(["a.com", "b.com", "c.com"]); expect(result!.checksum).toBe("new"); }); it("fetches all domains if cache is old", async () => { const prev: PhishingData = { - domains: ["a.com"], + webAddresses: ["a.com"], timestamp: Date.now() - 2 * 24 * 60 * 60 * 1000, checksum: "old", applicationVersion: "1.0.0", }; fetchChecksumSpy.mockResolvedValue("new"); - fetchDomainsSpy.mockResolvedValue(["d.com", "e.com"]); - const result = await service.getNextDomains(prev); - expect(result!.domains).toEqual(["d.com", "e.com"]); + fetchWebAddressesSpy.mockResolvedValue(["d.com", "e.com"]); + const result = await service.getNextWebAddresses(prev); + expect(result!.webAddresses).toEqual(["d.com", "e.com"]); expect(result!.checksum).toBe("new"); }); }); diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts index 6e1bf07c647..21fe74f1873 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts @@ -20,14 +20,16 @@ import { ScheduledTaskNames, TaskSchedulerService } from "@bitwarden/common/plat import { LogService } from "@bitwarden/logging"; import { GlobalStateProvider, KeyDefinition, PHISHING_DETECTION_DISK } from "@bitwarden/state"; +import { getPhishingResources, PhishingResourceType } from "../phishing-resources"; + export type PhishingData = { - domains: string[]; + webAddresses: string[]; timestamp: number; checksum: string; /** * We store the application version to refetch the entire dataset on a new client release. - * This counteracts daily appends updates not removing inactive or false positive domains. + * This counteracts daily appends updates not removing inactive or false positive web addresses. */ applicationVersion: string; }; @@ -37,34 +39,27 @@ export const PHISHING_DOMAINS_KEY = new KeyDefinition( "phishingDomains", { deserializer: (value: PhishingData) => - value ?? { domains: [], timestamp: 0, checksum: "", applicationVersion: "" }, + value ?? { webAddresses: [], timestamp: 0, checksum: "", applicationVersion: "" }, }, ); -/** Coordinates fetching, caching, and patching of known phishing domains */ +/** Coordinates fetching, caching, and patching of known phishing web addresses */ export class PhishingDataService { - private static readonly RemotePhishingDatabaseUrl = - "https://raw.githubusercontent.com/Phishing-Database/Phishing.Database/master/phishing-domains-ACTIVE.txt"; - private static readonly RemotePhishingDatabaseChecksumUrl = - "https://raw.githubusercontent.com/Phishing-Database/checksums/refs/heads/master/phishing-domains-ACTIVE.txt.md5"; - private static readonly RemotePhishingDatabaseTodayUrl = - "https://raw.githubusercontent.com/Phishing-Database/Phishing.Database/refs/heads/master/phishing-domains-NEW-today.txt"; - - private _testDomains = this.getTestDomains(); + private _testWebAddresses = this.getTestWebAddresses(); private _cachedState = this.globalStateProvider.get(PHISHING_DOMAINS_KEY); - private _domains$ = this._cachedState.state$.pipe( + private _webAddresses$ = this._cachedState.state$.pipe( map( (state) => new Set( - (state?.domains?.filter((line) => line.trim().length > 0) ?? []).concat( - this._testDomains, + (state?.webAddresses?.filter((line) => line.trim().length > 0) ?? []).concat( + this._testWebAddresses, "phishing.testcategory.com", // Included for QA to test in prod ), ), ), ); - // How often are new domains added to the remote? + // How often are new web addresses added to the remote? readonly UPDATE_INTERVAL_DURATION = 24 * 60 * 60 * 1000; // 24 hours private _triggerUpdate$ = new Subject(); @@ -75,7 +70,7 @@ export class PhishingDataService { this._cachedState.state$.pipe( first(), // Only take the first value to avoid an infinite loop when updating the cache below switchMap(async (cachedState) => { - const next = await this.getNextDomains(cachedState); + const next = await this.getNextWebAddresses(cachedState); if (next) { await this._cachedState.update(() => next); this.logService.info(`[PhishingDataService] cache updated`); @@ -85,7 +80,7 @@ export class PhishingDataService { count: 3, delay: (err, count) => { this.logService.error( - `[PhishingDataService] Unable to update domains. Attempt ${count}.`, + `[PhishingDataService] Unable to update web addresses. Attempt ${count}.`, err, ); return timer(5 * 60 * 1000); // 5 minutes @@ -97,7 +92,7 @@ export class PhishingDataService { err: unknown /** Eslint actually crashed if you remove this type: https://github.com/cartant/eslint-plugin-rxjs/issues/122 */, ) => { this.logService.error( - "[PhishingDataService] Retries unsuccessful. Unable to update domains.", + "[PhishingDataService] Retries unsuccessful. Unable to update web addresses.", err, ); return EMPTY; @@ -114,6 +109,7 @@ export class PhishingDataService { private globalStateProvider: GlobalStateProvider, private logService: LogService, private platformUtilsService: PlatformUtilsService, + private resourceType: PhishingResourceType = PhishingResourceType.Links, ) { this.taskSchedulerService.registerTaskHandler(ScheduledTaskNames.phishingDomainUpdate, () => { this._triggerUpdate$.next(); @@ -125,22 +121,31 @@ export class PhishingDataService { } /** - * Checks if the given URL is a known phishing domain + * Checks if the given URL is a known phishing web address * * @param url The URL to check - * @returns True if the URL is a known phishing domain, false otherwise + * @returns True if the URL is a known phishing web address, false otherwise */ - async isPhishingDomain(url: URL): Promise { - const domains = await firstValueFrom(this._domains$); - const result = domains.has(url.hostname); - if (result) { - return true; + async isPhishingWebAddress(url: URL): Promise { + // Use domain (hostname) matching for domain resources, and link matching for links resources + const entries = await firstValueFrom(this._webAddresses$); + + const resource = getPhishingResources(this.resourceType); + if (resource && resource.match) { + for (const entry of entries) { + if (resource.match(url, entry)) { + return true; + } + } + return false; } - return false; + + // Default/domain behavior: exact hostname match as a fallback + return entries.has(url.hostname); } - async getNextDomains(prev: PhishingData | null): Promise { - prev = prev ?? { domains: [], timestamp: 0, checksum: "", applicationVersion: "" }; + async getNextWebAddresses(prev: PhishingData | null): Promise { + prev = prev ?? { webAddresses: [], timestamp: 0, checksum: "", applicationVersion: "" }; const timestamp = Date.now(); const prevAge = timestamp - prev.timestamp; this.logService.info(`[PhishingDataService] Cache age: ${prevAge}`); @@ -148,7 +153,7 @@ export class PhishingDataService { const applicationVersion = await this.platformUtilsService.getApplicationVersion(); // If checksum matches, return existing data with new timestamp & version - const remoteChecksum = await this.fetchPhishingDomainsChecksum(); + const remoteChecksum = await this.fetchPhishingChecksum(this.resourceType); if (remoteChecksum && prev.checksum === remoteChecksum) { this.logService.info( `[PhishingDataService] Remote checksum matches local checksum, updating timestamp only.`, @@ -157,66 +162,66 @@ export class PhishingDataService { } // Checksum is different, data needs to be updated. - // Approach 1: Fetch only new domains and append + // Approach 1: Fetch only new web addresses and append const isOneDayOldMax = prevAge <= this.UPDATE_INTERVAL_DURATION; if (isOneDayOldMax && applicationVersion === prev.applicationVersion) { - const dailyDomains: string[] = await this.fetchPhishingDomains( - PhishingDataService.RemotePhishingDatabaseTodayUrl, - ); + const webAddressesTodayUrl = getPhishingResources(this.resourceType)!.todayUrl; + const dailyWebAddresses: string[] = + await this.fetchPhishingWebAddresses(webAddressesTodayUrl); this.logService.info( - `[PhishingDataService] ${dailyDomains.length} new phishing domains added`, + `[PhishingDataService] ${dailyWebAddresses.length} new phishing web addresses added`, ); return { - domains: prev.domains.concat(dailyDomains), + webAddresses: prev.webAddresses.concat(dailyWebAddresses), checksum: remoteChecksum, timestamp, applicationVersion, }; } - // Approach 2: Fetch all domains - const domains = await this.fetchPhishingDomains(PhishingDataService.RemotePhishingDatabaseUrl); + // Approach 2: Fetch all web addresses + const remoteUrl = getPhishingResources(this.resourceType)!.remoteUrl; + const remoteWebAddresses = await this.fetchPhishingWebAddresses(remoteUrl); return { - domains, + webAddresses: remoteWebAddresses, timestamp, checksum: remoteChecksum, applicationVersion, }; } - private async fetchPhishingDomainsChecksum() { - const response = await this.apiService.nativeFetch( - new Request(PhishingDataService.RemotePhishingDatabaseChecksumUrl), - ); + private async fetchPhishingChecksum(type: PhishingResourceType = PhishingResourceType.Domains) { + const checksumUrl = getPhishingResources(type)!.checksumUrl; + const response = await this.apiService.nativeFetch(new Request(checksumUrl)); if (!response.ok) { throw new Error(`[PhishingDataService] Failed to fetch checksum: ${response.status}`); } return response.text(); } - private async fetchPhishingDomains(url: string) { + private async fetchPhishingWebAddresses(url: string) { const response = await this.apiService.nativeFetch(new Request(url)); if (!response.ok) { - throw new Error(`[PhishingDataService] Failed to fetch domains: ${response.status}`); + throw new Error(`[PhishingDataService] Failed to fetch web addresses: ${response.status}`); } return response.text().then((text) => text.split("\n")); } - private getTestDomains() { + private getTestWebAddresses() { const flag = devFlagEnabled("testPhishingUrls"); if (!flag) { return []; } - const domains = devFlagValue("testPhishingUrls") as unknown[]; - if (domains && domains instanceof Array) { + const webAddresses = devFlagValue("testPhishingUrls") as unknown[]; + if (webAddresses && webAddresses instanceof Array) { this.logService.debug( - "[PhishingDetectionService] Dev flag enabled for testing phishing detection. Adding test phishing domains:", - domains, + "[PhishingDetectionService] Dev flag enabled for testing phishing detection. Adding test phishing web addresses:", + webAddresses, ); - return domains as string[]; + return webAddresses as string[]; } return []; } diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts index e04d08559ab..d90e872eef8 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts @@ -94,7 +94,7 @@ export class PhishingDetectionService { this._ignoredHostnames.delete(url.hostname); return; } - const isPhishing = await phishingDataService.isPhishingDomain(url); + const isPhishing = await phishingDataService.isPhishingWebAddress(url); if (!isPhishing) { return; } diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 739166ff6f8..7a2ded5bb83 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -23,6 +23,8 @@ import { WINDOW, } from "@bitwarden/angular/services/injection-tokens"; import { JslibServicesModule } from "@bitwarden/angular/services/jslib-services.module"; +import { AUTOFILL_NUDGE_SERVICE } from "@bitwarden/angular/vault"; +import { SingleNudgeService } from "@bitwarden/angular/vault/services/default-single-nudge.service"; import { LoginComponentService, TwoFactorAuthComponentService, @@ -208,6 +210,7 @@ import { } from "../../platform/system-notifications/browser-system-notification.service"; import { fromChromeRuntimeMessaging } from "../../platform/utils/from-chrome-runtime-messaging"; import { FilePopoutUtilsService } from "../../tools/popup/services/file-popout-utils.service"; +import { BrowserAutofillNudgeService } from "../../vault/popup/services/browser-autofill-nudge.service"; import { Fido2UserVerificationService } from "../../vault/services/fido2-user-verification.service"; import { ExtensionAnonLayoutWrapperDataService } from "../components/extension-anon-layout-wrapper/extension-anon-layout-wrapper-data.service"; @@ -756,6 +759,11 @@ const safeProviders: SafeProvider[] = [ MessagingServiceAbstraction, ], }), + safeProvider({ + provide: AUTOFILL_NUDGE_SERVICE as SafeInjectionToken, + useClass: BrowserAutofillNudgeService, + deps: [], + }), ]; @NgModule({ diff --git a/apps/browser/src/tools/popup/settings/settings-v2.component.html b/apps/browser/src/tools/popup/settings/settings-v2.component.html index 683b7d70ed6..06c89e15f59 100644 --- a/apps/browser/src/tools/popup/settings/settings-v2.component.html +++ b/apps/browser/src/tools/popup/settings/settings-v2.component.html @@ -34,13 +34,11 @@

{{ "autofill" | i18n }}

- 1 + @if (showAutofillBadge$ | async) { + 1 + }
diff --git a/apps/browser/src/tools/popup/settings/settings-v2.component.spec.ts b/apps/browser/src/tools/popup/settings/settings-v2.component.spec.ts index f51d514289e..4cc3ed0149c 100644 --- a/apps/browser/src/tools/popup/settings/settings-v2.component.spec.ts +++ b/apps/browser/src/tools/popup/settings/settings-v2.component.spec.ts @@ -148,31 +148,7 @@ describe("SettingsV2Component", () => { expect(openSpy).toHaveBeenCalledWith(dialogService); }); - it("isBrowserAutofillSettingOverridden$ emits the value from the AutofillBrowserSettingsService", async () => { - pushActiveAccount(); - - mockAutofillSettings.isBrowserAutofillSettingOverridden.mockResolvedValue(true); - - const fixture = TestBed.createComponent(SettingsV2Component); - const component = fixture.componentInstance; - fixture.detectChanges(); - await fixture.whenStable(); - - const value = await firstValueFrom(component["isBrowserAutofillSettingOverridden$"]); - expect(value).toBe(true); - - mockAutofillSettings.isBrowserAutofillSettingOverridden.mockResolvedValue(false); - - const fixture2 = TestBed.createComponent(SettingsV2Component); - const component2 = fixture2.componentInstance; - fixture2.detectChanges(); - await fixture2.whenStable(); - - const value2 = await firstValueFrom(component2["isBrowserAutofillSettingOverridden$"]); - expect(value2).toBe(false); - }); - - it("showAutofillBadge$ emits true when default autofill is NOT disabled and nudge is true", async () => { + it("showAutofillBadge$ emits true when showNudgeBadge is true", async () => { pushActiveAccount(); mockNudges.showNudgeBadge$.mockImplementation((type: NudgeType) => @@ -184,30 +160,10 @@ describe("SettingsV2Component", () => { fixture.detectChanges(); await fixture.whenStable(); - mockAutofillSettings.defaultBrowserAutofillDisabled$.next(false); - const value = await firstValueFrom(component.showAutofillBadge$); expect(value).toBe(true); }); - it("showAutofillBadge$ emits false when default autofill IS disabled even if nudge is true", async () => { - pushActiveAccount(); - - mockNudges.showNudgeBadge$.mockImplementation((type: NudgeType) => - of(type === NudgeType.AutofillNudge), - ); - - const fixture = TestBed.createComponent(SettingsV2Component); - const component = fixture.componentInstance; - fixture.detectChanges(); - await fixture.whenStable(); - - mockAutofillSettings.defaultBrowserAutofillDisabled$.next(true); - - const value = await firstValueFrom(component.showAutofillBadge$); - expect(value).toBe(false); - }); - it("dismissBadge dismisses when showVaultBadge$ emits true", async () => { const acct = pushActiveAccount(); diff --git a/apps/browser/src/tools/popup/settings/settings-v2.component.ts b/apps/browser/src/tools/popup/settings/settings-v2.component.ts index 95aeeb2f480..e10d41b9445 100644 --- a/apps/browser/src/tools/popup/settings/settings-v2.component.ts +++ b/apps/browser/src/tools/popup/settings/settings-v2.component.ts @@ -1,16 +1,7 @@ import { CommonModule } from "@angular/common"; import { ChangeDetectionStrategy, Component } from "@angular/core"; import { RouterModule } from "@angular/router"; -import { - combineLatest, - filter, - firstValueFrom, - from, - map, - Observable, - shareReplay, - switchMap, -} from "rxjs"; +import { filter, firstValueFrom, Observable, shareReplay, switchMap } from "rxjs"; import { PremiumUpgradeDialogComponent } from "@bitwarden/angular/billing/components"; import { JslibModule } from "@bitwarden/angular/jslib.module"; @@ -28,8 +19,6 @@ import { } from "@bitwarden/components"; import { CurrentAccountComponent } from "../../../auth/popup/account-switching/current-account.component"; -import { AutofillBrowserSettingsService } from "../../../autofill/services/autofill-browser-settings.service"; -import { BrowserApi } from "../../../platform/browser/browser-api"; import { PopOutComponent } from "../../../platform/popup/components/pop-out.component"; import { PopupHeaderComponent } from "../../../platform/popup/layout/popup-header.component"; import { PopupPageComponent } from "../../../platform/popup/layout/popup-page.component"; @@ -55,12 +44,6 @@ import { PopupPageComponent } from "../../../platform/popup/layout/popup-page.co export class SettingsV2Component { NudgeType = NudgeType; - protected isBrowserAutofillSettingOverridden$ = from( - this.autofillBrowserSettingsService.isBrowserAutofillSettingOverridden( - BrowserApi.getBrowserClientVendor(window), - ), - ); - private authenticatedAccount$: Observable = this.accountService.activeAccount$.pipe( filter((account): account is Account => account !== null), shareReplay({ bufferSize: 1, refCount: true }), @@ -82,23 +65,13 @@ export class SettingsV2Component { ), ); - showAutofillBadge$: Observable = combineLatest([ - this.autofillBrowserSettingsService.defaultBrowserAutofillDisabled$, - this.authenticatedAccount$, - ]).pipe( - switchMap(([defaultBrowserAutofillDisabled, account]) => - this.nudgesService.showNudgeBadge$(NudgeType.AutofillNudge, account.id).pipe( - map((badgeStatus) => { - return !defaultBrowserAutofillDisabled && badgeStatus; - }), - ), - ), + showAutofillBadge$: Observable = this.authenticatedAccount$.pipe( + switchMap((account) => this.nudgesService.showNudgeBadge$(NudgeType.AutofillNudge, account.id)), ); constructor( private readonly nudgesService: NudgesService, private readonly accountService: AccountService, - private readonly autofillBrowserSettingsService: AutofillBrowserSettingsService, private readonly accountProfileStateService: BillingAccountProfileStateService, private readonly dialogService: DialogService, ) {} diff --git a/apps/browser/src/vault/popup/services/browser-autofill-nudge.service.spec.ts b/apps/browser/src/vault/popup/services/browser-autofill-nudge.service.spec.ts new file mode 100644 index 00000000000..40782760283 --- /dev/null +++ b/apps/browser/src/vault/popup/services/browser-autofill-nudge.service.spec.ts @@ -0,0 +1,157 @@ +import { TestBed } from "@angular/core/testing"; +import { mock, MockProxy } from "jest-mock-extended"; +import { firstValueFrom } from "rxjs"; + +import { NudgeStatus, NudgeType } from "@bitwarden/angular/vault"; +import { VaultProfileService } from "@bitwarden/angular/vault/services/vault-profile.service"; +import { BrowserClientVendors } from "@bitwarden/common/autofill/constants"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { StateProvider } from "@bitwarden/common/platform/state"; +import { UserId } from "@bitwarden/common/types/guid"; + +import { FakeStateProvider, mockAccountServiceWith } from "../../../../../../libs/common/spec"; +import { BrowserApi } from "../../../platform/browser/browser-api"; + +import { BrowserAutofillNudgeService } from "./browser-autofill-nudge.service"; + +describe("BrowserAutofillNudgeService", () => { + let service: BrowserAutofillNudgeService; + let vaultProfileService: MockProxy; + let fakeStateProvider: FakeStateProvider; + + const userId = "test-user-id" as UserId; + const nudgeType = NudgeType.AutofillNudge; + + const notDismissedStatus: NudgeStatus = { + hasBadgeDismissed: false, + hasSpotlightDismissed: false, + }; + + const dismissedStatus: NudgeStatus = { + hasBadgeDismissed: true, + hasSpotlightDismissed: true, + }; + + // Set profile creation date to now (new account, within 30 days) + const recentProfileDate = new Date(); + + beforeEach(() => { + vaultProfileService = mock(); + vaultProfileService.getProfileCreationDate.mockResolvedValue(recentProfileDate); + + fakeStateProvider = new FakeStateProvider(mockAccountServiceWith(userId)); + + TestBed.configureTestingModule({ + providers: [ + BrowserAutofillNudgeService, + { + provide: VaultProfileService, + useValue: vaultProfileService, + }, + { + provide: StateProvider, + useValue: fakeStateProvider, + }, + { + provide: LogService, + useValue: mock(), + }, + ], + }); + + service = TestBed.inject(BrowserAutofillNudgeService); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe("nudgeStatus$", () => { + it("returns parent status when browser client is Unknown", async () => { + jest + .spyOn(BrowserApi, "getBrowserClientVendor") + .mockReturnValue(BrowserClientVendors.Unknown); + jest.spyOn(BrowserApi, "browserAutofillSettingsOverridden").mockResolvedValue(true); + + const result = await firstValueFrom(service.nudgeStatus$(nudgeType, userId)); + + expect(result).toEqual(notDismissedStatus); + }); + + it("returns parent status when browser autofill is not overridden", async () => { + jest.spyOn(BrowserApi, "getBrowserClientVendor").mockReturnValue(BrowserClientVendors.Chrome); + jest.spyOn(BrowserApi, "browserAutofillSettingsOverridden").mockResolvedValue(false); + + const result = await firstValueFrom(service.nudgeStatus$(nudgeType, userId)); + + expect(result).toEqual(notDismissedStatus); + }); + + it("returns dismissed status when browser autofill is overridden", async () => { + jest.spyOn(BrowserApi, "getBrowserClientVendor").mockReturnValue(BrowserClientVendors.Chrome); + jest.spyOn(BrowserApi, "browserAutofillSettingsOverridden").mockResolvedValue(true); + + const result = await firstValueFrom(service.nudgeStatus$(nudgeType, userId)); + + expect(result).toEqual(dismissedStatus); + }); + + it("preserves parent dismissed status when account is older than 30 days", async () => { + // Set profile creation date to more than 30 days ago + const oldProfileDate = new Date(Date.now() - 31 * 24 * 60 * 60 * 1000); + vaultProfileService.getProfileCreationDate.mockResolvedValue(oldProfileDate); + + jest.spyOn(BrowserApi, "getBrowserClientVendor").mockReturnValue(BrowserClientVendors.Chrome); + jest.spyOn(BrowserApi, "browserAutofillSettingsOverridden").mockResolvedValue(false); + + const result = await firstValueFrom(service.nudgeStatus$(nudgeType, userId)); + + expect(result).toEqual(dismissedStatus); + }); + + it("combines parent dismissed and browser autofill overridden status", async () => { + // Set profile creation date to more than 30 days ago (parent dismisses) + const oldProfileDate = new Date(Date.now() - 31 * 24 * 60 * 60 * 1000); + vaultProfileService.getProfileCreationDate.mockResolvedValue(oldProfileDate); + + jest.spyOn(BrowserApi, "getBrowserClientVendor").mockReturnValue(BrowserClientVendors.Chrome); + jest.spyOn(BrowserApi, "browserAutofillSettingsOverridden").mockResolvedValue(true); + + const result = await firstValueFrom(service.nudgeStatus$(nudgeType, userId)); + + expect(result).toEqual(dismissedStatus); + }); + + it.each([ + BrowserClientVendors.Chrome, + BrowserClientVendors.Edge, + BrowserClientVendors.Opera, + BrowserClientVendors.Vivaldi, + ])("checks browser autofill settings for %s browser", async (browserVendor) => { + const getBrowserClientVendorSpy = jest + .spyOn(BrowserApi, "getBrowserClientVendor") + .mockReturnValue(browserVendor); + const browserAutofillSettingsOverriddenSpy = jest + .spyOn(BrowserApi, "browserAutofillSettingsOverridden") + .mockResolvedValue(true); + + await firstValueFrom(service.nudgeStatus$(nudgeType, userId)); + + expect(getBrowserClientVendorSpy).toHaveBeenCalledWith(window); + expect(browserAutofillSettingsOverriddenSpy).toHaveBeenCalled(); + }); + + it("does not check browser autofill settings for Unknown browser", async () => { + jest + .spyOn(BrowserApi, "getBrowserClientVendor") + .mockReturnValue(BrowserClientVendors.Unknown); + const browserAutofillSettingsOverriddenSpy = jest + .spyOn(BrowserApi, "browserAutofillSettingsOverridden") + .mockResolvedValue(true); + + await firstValueFrom(service.nudgeStatus$(nudgeType, userId)); + + expect(browserAutofillSettingsOverriddenSpy).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/apps/browser/src/vault/popup/services/browser-autofill-nudge.service.ts b/apps/browser/src/vault/popup/services/browser-autofill-nudge.service.ts new file mode 100644 index 00000000000..7fe5f527bcb --- /dev/null +++ b/apps/browser/src/vault/popup/services/browser-autofill-nudge.service.ts @@ -0,0 +1,37 @@ +import { Injectable } from "@angular/core"; +import { Observable, switchMap } from "rxjs"; + +import { NudgeStatus, NudgeType } from "@bitwarden/angular/vault"; +import { NewAccountNudgeService } from "@bitwarden/angular/vault/services/custom-nudges-services/new-account-nudge.service"; +import { BrowserClientVendors } from "@bitwarden/common/autofill/constants"; +import { UserId } from "@bitwarden/common/types/guid"; + +import { BrowserApi } from "../../../platform/browser/browser-api"; + +/** + * Browser-specific autofill nudge service. + * Extends NewAccountNudgeService (30-day account age check) and adds + * browser autofill setting detection. + * + * Nudge is dismissed if: + * - Account is older than 30 days (inherited from NewAccountNudgeService) + * - Browser's built-in password manager is already disabled via privacy settings + */ +@Injectable() +export class BrowserAutofillNudgeService extends NewAccountNudgeService { + override nudgeStatus$(nudgeType: NudgeType, userId: UserId): Observable { + return super.nudgeStatus$(nudgeType, userId).pipe( + switchMap(async (status) => { + const browserClient = BrowserApi.getBrowserClientVendor(window); + const browserAutofillOverridden = + browserClient !== BrowserClientVendors.Unknown && + (await BrowserApi.browserAutofillSettingsOverridden()); + + return { + hasBadgeDismissed: status.hasBadgeDismissed || browserAutofillOverridden, + hasSpotlightDismissed: status.hasSpotlightDismissed || browserAutofillOverridden, + }; + }), + ); + } +} diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index 276573a2bd1..77b40b4268c 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -104,6 +104,7 @@ import { EnvironmentService, RegionConfig, } from "@bitwarden/common/platform/abstractions/environment.service"; +import { RegisterSdkService } from "@bitwarden/common/platform/abstractions/sdk/register-sdk.service"; import { SdkLoadService } from "@bitwarden/common/platform/abstractions/sdk/sdk-load.service"; import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; import { LogLevelType } from "@bitwarden/common/platform/enums"; @@ -124,6 +125,7 @@ import { MigrationRunner } from "@bitwarden/common/platform/services/migration-r import { DefaultSdkClientFactory } from "@bitwarden/common/platform/services/sdk/default-sdk-client-factory"; import { DefaultSdkService } from "@bitwarden/common/platform/services/sdk/default-sdk.service"; import { NoopSdkClientFactory } from "@bitwarden/common/platform/services/sdk/noop-sdk-client-factory"; +import { DefaultRegisterSdkService } from "@bitwarden/common/platform/services/sdk/register-sdk.service"; import { StorageServiceProvider } from "@bitwarden/common/platform/services/storage-service.provider"; import { UserAutoUnlockKeyService } from "@bitwarden/common/platform/services/user-auto-unlock-key.service"; import { SyncService } from "@bitwarden/common/platform/sync"; @@ -323,6 +325,7 @@ export class ServiceContainer { kdfConfigService: KdfConfigService; taskSchedulerService: TaskSchedulerService; sdkService: SdkService; + registerSdkService: RegisterSdkService; sdkLoadService: SdkLoadService; cipherAuthorizationService: CipherAuthorizationService; ssoUrlService: SsoUrlService; @@ -632,26 +635,10 @@ export class ServiceContainer { this.accountService, ); - this.keyConnectorService = new KeyConnectorService( - this.accountService, - this.masterPasswordService, - this.keyService, - this.apiService, - this.tokenService, - this.logService, - this.organizationService, - this.keyGenerationService, - logoutCallback, + this.accountCryptographicStateService = new DefaultAccountCryptographicStateService( this.stateProvider, ); - this.twoFactorService = new DefaultTwoFactorService( - this.i18nService, - this.platformUtilsService, - this.globalStateProvider, - this.twoFactorApiService, - ); - const sdkClientFactory = flagEnabled("sdk") ? new DefaultSdkClientFactory() : new NoopSdkClientFactory(); @@ -670,6 +657,41 @@ export class ServiceContainer { customUserAgent, ); + this.registerSdkService = new DefaultRegisterSdkService( + sdkClientFactory, + this.environmentService, + this.platformUtilsService, + this.accountService, + this.apiService, + this.stateProvider, + this.configService, + customUserAgent, + ); + + this.keyConnectorService = new KeyConnectorService( + this.accountService, + this.masterPasswordService, + this.keyService, + this.apiService, + this.tokenService, + this.logService, + this.organizationService, + this.keyGenerationService, + logoutCallback, + this.stateProvider, + this.configService, + this.registerSdkService, + this.securityStateService, + this.accountCryptographicStateService, + ); + + this.twoFactorService = new DefaultTwoFactorService( + this.i18nService, + this.platformUtilsService, + this.globalStateProvider, + this.twoFactorApiService, + ); + this.passwordStrengthService = new PasswordStrengthService(); this.passwordGenerationService = legacyPasswordGenerationServiceFactory( @@ -719,10 +741,6 @@ export class ServiceContainer { this.accountService, ); - this.accountCryptographicStateService = new DefaultAccountCryptographicStateService( - this.stateProvider, - ); - this.loginStrategyService = new LoginStrategyService( this.accountService, this.masterPasswordService, diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs b/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs index 16cf778b575..8ba64618ffa 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs @@ -226,7 +226,7 @@ impl BitwardenDesktopAgent { keystore.0.write().expect("RwLock is not poisoned").clear(); self.needs_unlock - .store(false, std::sync::atomic::Ordering::Relaxed); + .store(true, std::sync::atomic::Ordering::Relaxed); for (key, name, cipher_id) in new_keys.iter() { match parse_key_safe(key) { @@ -307,87 +307,3 @@ fn parse_key_safe(pem: &str) -> Result Err(anyhow::Error::msg(format!("Failed to parse key: {e}"))), } } - -#[cfg(test)] -mod tests { - use super::*; - - fn create_test_agent() -> ( - BitwardenDesktopAgent, - tokio::sync::mpsc::Receiver, - tokio::sync::broadcast::Sender<(u32, bool)>, - ) { - let (tx, rx) = tokio::sync::mpsc::channel(10); - let (response_tx, response_rx) = tokio::sync::broadcast::channel(10); - let agent = BitwardenDesktopAgent::new(tx, Arc::new(Mutex::new(response_rx))); - (agent, rx, response_tx) - } - - const TEST_ED25519_KEY: &str = "-----BEGIN OPENSSH PRIVATE KEY----- -b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW -QyNTUxOQAAACCWETEIh/JX+ZaK0Xlg5xZ9QIfjiKD2Qs57PjhRY45trwAAAIhqmvSbapr0 -mwAAAAtzc2gtZWQyNTUxOQAAACCWETEIh/JX+ZaK0Xlg5xZ9QIfjiKD2Qs57PjhRY45trw -AAAEAHVflTgR/OEl8mg9UEKcO7SeB0FH4AiaUurhVfBWT4eZYRMQiH8lf5lorReWDnFn1A -h+OIoPZCzns+OFFjjm2vAAAAAAECAwQF ------END OPENSSH PRIVATE KEY-----"; - - #[tokio::test] - async fn test_needs_unlock_initial_state() { - let (agent, _rx, _response_tx) = create_test_agent(); - - // Initially, needs_unlock should be true - assert!(agent - .needs_unlock - .load(std::sync::atomic::Ordering::Relaxed)); - } - - #[tokio::test] - async fn test_needs_unlock_after_set_keys() { - let (mut agent, _rx, _response_tx) = create_test_agent(); - agent - .is_running - .store(true, std::sync::atomic::Ordering::Relaxed); - - // Set keys should set needs_unlock to false - let keys = vec![( - TEST_ED25519_KEY.to_string(), - "test_key".to_string(), - "cipher_id".to_string(), - )]; - - agent.set_keys(keys).unwrap(); - - assert!(!agent - .needs_unlock - .load(std::sync::atomic::Ordering::Relaxed)); - } - - #[tokio::test] - async fn test_needs_unlock_after_clear_keys() { - let (mut agent, _rx, _response_tx) = create_test_agent(); - agent - .is_running - .store(true, std::sync::atomic::Ordering::Relaxed); - - // Set keys first - let keys = vec![( - TEST_ED25519_KEY.to_string(), - "test_key".to_string(), - "cipher_id".to_string(), - )]; - agent.set_keys(keys).unwrap(); - - // Verify needs_unlock is false - assert!(!agent - .needs_unlock - .load(std::sync::atomic::Ordering::Relaxed)); - - // Clear keys should set needs_unlock back to true - agent.clear_keys().unwrap(); - - // Verify needs_unlock is true - assert!(agent - .needs_unlock - .load(std::sync::atomic::Ordering::Relaxed)); - } -} diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index 836328142b5..d75702ee8b8 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -70,6 +70,7 @@ import { SyncService } from "@bitwarden/common/platform/sync"; import { UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { InternalFolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; +import { PremiumUpgradePromptService } from "@bitwarden/common/vault/abstractions/premium-upgrade-prompt.service"; import { SearchService } from "@bitwarden/common/vault/abstractions/search.service"; import { CipherType } from "@bitwarden/common/vault/enums"; import { RestrictedItemTypesService } from "@bitwarden/common/vault/services/restricted-item-types.service"; @@ -198,6 +199,7 @@ export class AppComponent implements OnInit, OnDestroy { private readonly tokenService: TokenService, private desktopAutotypeDefaultSettingPolicy: DesktopAutotypeDefaultSettingPolicy, private readonly lockService: LockService, + private premiumUpgradePromptService: PremiumUpgradePromptService, ) { this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe(); @@ -305,7 +307,7 @@ export class AppComponent implements OnInit, OnDestroy { await this.openModal(SettingsComponent, this.settingsRef); break; case "openPremium": - this.dialogService.open(PremiumComponent); + await this.premiumUpgradePromptService.promptForPremium(); break; case "showFingerprintPhrase": { const activeUserId = await firstValueFrom( diff --git a/apps/desktop/src/app/app.module.ts b/apps/desktop/src/app/app.module.ts index 31131c6202a..52a52ffd225 100644 --- a/apps/desktop/src/app/app.module.ts +++ b/apps/desktop/src/app/app.module.ts @@ -8,6 +8,7 @@ import { BrowserAnimationsModule } from "@angular/platform-browser/animations"; import { ColorPasswordCountPipe } from "@bitwarden/angular/pipes/color-password-count.pipe"; import { ColorPasswordPipe } from "@bitwarden/angular/pipes/color-password.pipe"; +import { PremiumUpgradePromptService } from "@bitwarden/common/vault/abstractions/premium-upgrade-prompt.service"; import { CalloutModule, DialogModule } from "@bitwarden/components"; import { AssignCollectionsComponent } from "@bitwarden/vault"; @@ -15,6 +16,7 @@ import { DeleteAccountComponent } from "../auth/delete-account.component"; import { LoginModule } from "../auth/login/login.module"; import { SshAgentService } from "../autofill/services/ssh-agent.service"; import { PremiumComponent } from "../billing/app/accounts/premium.component"; +import { DesktopPremiumUpgradePromptService } from "../services/desktop-premium-upgrade-prompt.service"; import { VaultFilterModule } from "../vault/app/vault/vault-filter/vault-filter.module"; import { VaultV2Component } from "../vault/app/vault/vault-v2.component"; @@ -51,7 +53,13 @@ import { SharedModule } from "./shared/shared.module"; PremiumComponent, SearchComponent, ], - providers: [SshAgentService], + providers: [ + SshAgentService, + { + provide: PremiumUpgradePromptService, + useClass: DesktopPremiumUpgradePromptService, + }, + ], bootstrap: [AppComponent], }) export class AppModule {} diff --git a/apps/desktop/src/services/desktop-premium-upgrade-prompt.service.spec.ts b/apps/desktop/src/services/desktop-premium-upgrade-prompt.service.spec.ts index 1eee4cd54f6..3c36d648167 100644 --- a/apps/desktop/src/services/desktop-premium-upgrade-prompt.service.spec.ts +++ b/apps/desktop/src/services/desktop-premium-upgrade-prompt.service.spec.ts @@ -4,26 +4,24 @@ import { mock, MockProxy } from "jest-mock-extended"; import { PremiumUpgradeDialogComponent } from "@bitwarden/angular/billing/components"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; -import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { DialogService } from "@bitwarden/components"; +import { PremiumComponent } from "../billing/app/accounts/premium.component"; + import { DesktopPremiumUpgradePromptService } from "./desktop-premium-upgrade-prompt.service"; describe("DesktopPremiumUpgradePromptService", () => { let service: DesktopPremiumUpgradePromptService; - let messager: MockProxy; let configService: MockProxy; let dialogService: MockProxy; beforeEach(async () => { - messager = mock(); configService = mock(); dialogService = mock(); await TestBed.configureTestingModule({ providers: [ DesktopPremiumUpgradePromptService, - { provide: MessagingService, useValue: messager }, { provide: ConfigService, useValue: configService }, { provide: DialogService, useValue: dialogService }, ], @@ -52,10 +50,10 @@ describe("DesktopPremiumUpgradePromptService", () => { FeatureFlag.PM23713_PremiumBadgeOpensNewPremiumUpgradeDialog, ); expect(openSpy).toHaveBeenCalledWith(dialogService); - expect(messager.send).not.toHaveBeenCalled(); + expect(dialogService.open).not.toHaveBeenCalled(); }); - it("sends openPremium message when feature flag is disabled", async () => { + it("opens the PremiumComponent when feature flag is disabled", async () => { configService.getFeatureFlag.mockResolvedValue(false); await service.promptForPremium(); @@ -63,7 +61,7 @@ describe("DesktopPremiumUpgradePromptService", () => { expect(configService.getFeatureFlag).toHaveBeenCalledWith( FeatureFlag.PM23713_PremiumBadgeOpensNewPremiumUpgradeDialog, ); - expect(messager.send).toHaveBeenCalledWith("openPremium"); + expect(dialogService.open).toHaveBeenCalledWith(PremiumComponent); expect(openSpy).not.toHaveBeenCalled(); }); }); diff --git a/apps/desktop/src/services/desktop-premium-upgrade-prompt.service.ts b/apps/desktop/src/services/desktop-premium-upgrade-prompt.service.ts index 5004e5ed547..0161baba801 100644 --- a/apps/desktop/src/services/desktop-premium-upgrade-prompt.service.ts +++ b/apps/desktop/src/services/desktop-premium-upgrade-prompt.service.ts @@ -3,15 +3,15 @@ import { inject } from "@angular/core"; import { PremiumUpgradeDialogComponent } from "@bitwarden/angular/billing/components"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; -import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PremiumUpgradePromptService } from "@bitwarden/common/vault/abstractions/premium-upgrade-prompt.service"; import { DialogService } from "@bitwarden/components"; +import { PremiumComponent } from "../billing/app/accounts/premium.component"; + /** * This class handles the premium upgrade process for the desktop. */ export class DesktopPremiumUpgradePromptService implements PremiumUpgradePromptService { - private messagingService = inject(MessagingService); private configService = inject(ConfigService); private dialogService = inject(DialogService); @@ -23,7 +23,7 @@ export class DesktopPremiumUpgradePromptService implements PremiumUpgradePromptS if (showNewDialog) { PremiumUpgradeDialogComponent.open(this.dialogService); } else { - this.messagingService.send("openPremium"); + this.dialogService.open(PremiumComponent); } } } diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 83d8ccbe05d..511af51c2e3 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -1149,6 +1149,10 @@ const safeProviders: SafeProvider[] = [ KeyGenerationService, LOGOUT_CALLBACK, StateProvider, + ConfigService, + RegisterSdkService, + SecurityStateService, + AccountCryptographicStateService, ], }), safeProvider({ diff --git a/libs/angular/src/vault/index.ts b/libs/angular/src/vault/index.ts index cb43fadb3bc..b9131338a45 100644 --- a/libs/angular/src/vault/index.ts +++ b/libs/angular/src/vault/index.ts @@ -1,3 +1,4 @@ // Note: Nudge related code is exported from `libs/angular` because it is consumed by multiple // `libs/*` packages. Exporting from the `libs/vault` package creates circular dependencies. export { NudgesService, NudgeStatus, NudgeType } from "./services/nudges.service"; +export { AUTOFILL_NUDGE_SERVICE } from "./services/nudge-injection-tokens"; diff --git a/libs/angular/src/vault/services/custom-nudges-services/index.ts b/libs/angular/src/vault/services/custom-nudges-services/index.ts index f60592b9c71..d4bfe80a525 100644 --- a/libs/angular/src/vault/services/custom-nudges-services/index.ts +++ b/libs/angular/src/vault/services/custom-nudges-services/index.ts @@ -4,3 +4,4 @@ export * from "./empty-vault-nudge.service"; export * from "./vault-settings-import-nudge.service"; export * from "./new-item-nudge.service"; export * from "./new-account-nudge.service"; +export * from "./noop-nudge.service"; diff --git a/libs/angular/src/vault/services/custom-nudges-services/noop-nudge.service.ts b/libs/angular/src/vault/services/custom-nudges-services/noop-nudge.service.ts new file mode 100644 index 00000000000..eabf1d6fc4a --- /dev/null +++ b/libs/angular/src/vault/services/custom-nudges-services/noop-nudge.service.ts @@ -0,0 +1,27 @@ +import { Injectable } from "@angular/core"; +import { Observable, of } from "rxjs"; + +import { UserId } from "@bitwarden/common/types/guid"; + +import { SingleNudgeService } from "../default-single-nudge.service"; +import { NudgeStatus, NudgeType } from "../nudges.service"; + +/** + * A no-op nudge service that always returns dismissed status. + * Use this for nudges that should be completely ignored/hidden in certain clients. + * For example, browser-specific nudges can use this as the default in non-browser clients. + */ +@Injectable({ providedIn: "root" }) +export class NoOpNudgeService implements SingleNudgeService { + nudgeStatus$(_nudgeType: NudgeType, _userId: UserId): Observable { + return of({ hasBadgeDismissed: true, hasSpotlightDismissed: true }); + } + + async setNudgeStatus( + _nudgeType: NudgeType, + _newStatus: NudgeStatus, + _userId: UserId, + ): Promise { + // No-op: state changes are ignored + } +} diff --git a/libs/angular/src/vault/services/nudge-injection-tokens.ts b/libs/angular/src/vault/services/nudge-injection-tokens.ts new file mode 100644 index 00000000000..52a0838d356 --- /dev/null +++ b/libs/angular/src/vault/services/nudge-injection-tokens.ts @@ -0,0 +1,7 @@ +import { InjectionToken } from "@angular/core"; + +import { SingleNudgeService } from "./default-single-nudge.service"; + +export const AUTOFILL_NUDGE_SERVICE = new InjectionToken( + "AutofillNudgeService", +); diff --git a/libs/angular/src/vault/services/nudges.service.ts b/libs/angular/src/vault/services/nudges.service.ts index 05d565eb499..19acf690d32 100644 --- a/libs/angular/src/vault/services/nudges.service.ts +++ b/libs/angular/src/vault/services/nudges.service.ts @@ -12,8 +12,10 @@ import { NewItemNudgeService, AccountSecurityNudgeService, VaultSettingsImportNudgeService, + NoOpNudgeService, } from "./custom-nudges-services"; import { DefaultSingleNudgeService, SingleNudgeService } from "./default-single-nudge.service"; +import { AUTOFILL_NUDGE_SERVICE } from "./nudge-injection-tokens"; export type NudgeStatus = { hasBadgeDismissed: boolean; @@ -56,6 +58,12 @@ export class NudgesService { private newItemNudgeService = inject(NewItemNudgeService); private newAcctNudgeService = inject(NewAccountNudgeService); + // NoOp service that always returns dismissed + private noOpNudgeService = inject(NoOpNudgeService); + + // Optional Browser-specific service provided via injection token (not all clients have autofill) + private autofillNudgeService = inject(AUTOFILL_NUDGE_SERVICE, { optional: true }); + /** * Custom nudge services to use for specific nudge types * Each nudge type can have its own service to determine when to show the nudge @@ -66,7 +74,7 @@ export class NudgesService { [NudgeType.EmptyVaultNudge]: inject(EmptyVaultNudgeService), [NudgeType.VaultSettingsImportNudge]: inject(VaultSettingsImportNudgeService), [NudgeType.AccountSecurity]: inject(AccountSecurityNudgeService), - [NudgeType.AutofillNudge]: this.newAcctNudgeService, + [NudgeType.AutofillNudge]: this.autofillNudgeService ?? this.noOpNudgeService, [NudgeType.DownloadBitwarden]: this.newAcctNudgeService, [NudgeType.GeneratorNudgeStatus]: this.newAcctNudgeService, [NudgeType.NewLoginItemStatus]: this.newItemNudgeService, diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 5124e007a8a..71074e88650 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -45,6 +45,7 @@ export enum FeatureFlag { DataRecoveryTool = "pm-28813-data-recovery-tool", ConsolidatedSessionTimeoutComponent = "pm-26056-consolidated-session-timeout-component", PM27279_V2RegistrationTdeJit = "pm-27279-v2-registration-tde-jit", + EnableAccountEncryptionV2KeyConnectorRegistration = "enable-account-encryption-v2-key-connector-registration", /* Tools */ DesktopSendUIRefresh = "desktop-send-ui-refresh", @@ -154,6 +155,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.DataRecoveryTool]: FALSE, [FeatureFlag.ConsolidatedSessionTimeoutComponent]: FALSE, [FeatureFlag.PM27279_V2RegistrationTdeJit]: FALSE, + [FeatureFlag.EnableAccountEncryptionV2KeyConnectorRegistration]: FALSE, /* Platform */ [FeatureFlag.IpcChannelFramework]: FALSE, diff --git a/libs/common/src/key-management/key-connector/models/new-sso-user-key-connector-conversion.ts b/libs/common/src/key-management/key-connector/models/new-sso-user-key-connector-conversion.ts index 12996747c96..3d686697a4e 100644 --- a/libs/common/src/key-management/key-connector/models/new-sso-user-key-connector-conversion.ts +++ b/libs/common/src/key-management/key-connector/models/new-sso-user-key-connector-conversion.ts @@ -5,5 +5,6 @@ import { KdfConfig } from "@bitwarden/key-management"; export interface NewSsoUserKeyConnectorConversion { kdfConfig: KdfConfig; keyConnectorUrl: string; + // SSO organization identifier, not UUID organizationId: string; } diff --git a/libs/common/src/key-management/key-connector/services/key-connector.service.spec.ts b/libs/common/src/key-management/key-connector/services/key-connector.service.spec.ts index 45b4f5e4ac6..b8ee5d7df64 100644 --- a/libs/common/src/key-management/key-connector/services/key-connector.service.spec.ts +++ b/libs/common/src/key-management/key-connector/services/key-connector.service.spec.ts @@ -7,7 +7,8 @@ import { SetKeyConnectorKeyRequest } from "@bitwarden/common/key-management/key- import { KeysRequest } from "@bitwarden/common/models/request/keys.request"; // 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 { Argon2KdfConfig, PBKDF2KdfConfig, KeyService, KdfType } from "@bitwarden/key-management"; +import { Argon2KdfConfig, KdfType, KeyService, PBKDF2KdfConfig } from "@bitwarden/key-management"; +import { BitwardenClient } from "@bitwarden/sdk-internal"; import { FakeAccountService, FakeStateProvider, mockAccountServiceWith } from "../../../../spec"; import { ApiService } from "../../../abstractions/api.service"; @@ -16,21 +17,26 @@ import { Organization } from "../../../admin-console/models/domain/organization" import { ProfileOrganizationResponse } from "../../../admin-console/models/response/profile-organization.response"; import { KeyConnectorUserKeyResponse } from "../../../auth/models/response/key-connector-user-key.response"; import { TokenService } from "../../../auth/services/token.service"; +import { ConfigService } from "../../../platform/abstractions/config/config.service"; import { LogService } from "../../../platform/abstractions/log.service"; +import { RegisterSdkService } from "../../../platform/abstractions/sdk/register-sdk.service"; +import { Rc } from "../../../platform/misc/reference-counting/rc"; import { Utils } from "../../../platform/misc/utils"; import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; import { OrganizationId, UserId } from "../../../types/guid"; import { MasterKey, UserKey } from "../../../types/key"; +import { AccountCryptographicStateService } from "../../account-cryptography/account-cryptographic-state.service"; import { KeyGenerationService } from "../../crypto"; import { EncString } from "../../crypto/models/enc-string"; import { FakeMasterPasswordService } from "../../master-password/services/fake-master-password.service"; +import { SecurityStateService } from "../../security-state/abstractions/security-state.service"; import { KeyConnectorUserKeyRequest } from "../models/key-connector-user-key.request"; import { NewSsoUserKeyConnectorConversion } from "../models/new-sso-user-key-connector-conversion"; import { - USES_KEY_CONNECTOR, - NEW_SSO_USER_KEY_CONNECTOR_CONVERSION, KeyConnectorService, + NEW_SSO_USER_KEY_CONNECTOR_CONVERSION, + USES_KEY_CONNECTOR, } from "./key-connector.service"; describe("KeyConnectorService", () => { @@ -43,6 +49,10 @@ describe("KeyConnectorService", () => { const organizationService = mock(); const keyGenerationService = mock(); const logoutCallback = jest.fn(); + const configService = mock(); + const registerSdkService = mock(); + const securityStateService = mock(); + const accountCryptographicStateService = mock(); let stateProvider: FakeStateProvider; @@ -50,6 +60,7 @@ describe("KeyConnectorService", () => { let masterPasswordService: FakeMasterPasswordService; const mockUserId = Utils.newGuid() as UserId; + const mockSsoOrgIdentifier = "test-sso-org-id"; const mockOrgId = Utils.newGuid() as OrganizationId; const mockMasterKeyResponse: KeyConnectorUserKeyResponse = new KeyConnectorUserKeyResponse({ @@ -61,7 +72,7 @@ describe("KeyConnectorService", () => { const conversion: NewSsoUserKeyConnectorConversion = { kdfConfig: new PBKDF2KdfConfig(600_000), keyConnectorUrl, - organizationId: mockOrgId, + organizationId: mockSsoOrgIdentifier, }; beforeEach(() => { @@ -82,6 +93,10 @@ describe("KeyConnectorService", () => { keyGenerationService, logoutCallback, stateProvider, + configService, + registerSdkService, + securityStateService, + accountCryptographicStateService, ); }); @@ -419,44 +434,52 @@ describe("KeyConnectorService", () => { }); describe("convertNewSsoUserToKeyConnector", () => { - const passwordKey = new SymmetricCryptoKey(new Uint8Array(64)); - const mockUserKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey; - const mockEmail = "test@example.com"; - const mockMasterKey = getMockMasterKey(); - const mockKeyPair = ["mockPubKey", new EncString("mockEncryptedPrivKey")] as [ - string, - EncString, - ]; - let mockMakeUserKeyResult: [UserKey, EncString]; + describe("V2", () => { + const mockKeyConnectorKey = Utils.fromBufferToB64(new Uint8Array(64)); + const mockUserKeyString = Utils.fromBufferToB64(new Uint8Array(64)); + const mockPrivateKey = "mockPrivateKey789"; + const mockKeyConnectorKeyWrappedUserKey = "2.mockWrappedUserKey"; + const mockSigningKey = "mockSigningKey"; + const mockSignedPublicKey = "mockSignedPublicKey"; + const mockSecurityState = "mockSecurityState"; - beforeEach(() => { - const mockUserKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey; - const encString = new EncString("mockEncryptedString"); - mockMakeUserKeyResult = [mockUserKey, encString] as [UserKey, EncString]; + let mockSdkRef: any; + let mockSdk: any; - keyGenerationService.createKey.mockResolvedValue(passwordKey); - keyService.makeMasterKey.mockResolvedValue(mockMasterKey); - keyService.makeUserKey.mockResolvedValue(mockMakeUserKeyResult); - keyService.makeKeyPair.mockResolvedValue(mockKeyPair); - tokenService.getEmail.mockResolvedValue(mockEmail); - }); + beforeEach(() => { + configService.getFeatureFlag$.mockReturnValue(of(true)); - it.each([ - [KdfType.PBKDF2_SHA256, 700_000, undefined, undefined], - [KdfType.Argon2id, 11, 65, 5], - ])( - "sets up a new SSO user with key connector", - async (kdfType, kdfIterations, kdfMemory, kdfParallelism) => { - const expectedKdfConfig = - kdfType == KdfType.PBKDF2_SHA256 - ? new PBKDF2KdfConfig(kdfIterations) - : new Argon2KdfConfig(kdfIterations, kdfMemory, kdfParallelism); - - const conversion: NewSsoUserKeyConnectorConversion = { - kdfConfig: expectedKdfConfig, - keyConnectorUrl: keyConnectorUrl, - organizationId: mockOrgId, + mockSdkRef = { + value: { + auth: jest.fn().mockReturnValue({ + registration: jest.fn().mockReturnValue({ + post_keys_for_key_connector_registration: jest.fn().mockResolvedValue({ + key_connector_key: mockKeyConnectorKey, + user_key: mockUserKeyString, + key_connector_key_wrapped_user_key: mockKeyConnectorKeyWrappedUserKey, + account_cryptographic_state: { + V2: { + private_key: mockPrivateKey, + signing_key: mockSigningKey, + signed_public_key: mockSignedPublicKey, + security_state: mockSecurityState, + }, + }, + }), + }), + }), + }, + [Symbol.dispose]: jest.fn(), }; + + mockSdk = { + take: jest.fn().mockReturnValue(mockSdkRef), + }; + + registerSdkService.registerClient$.mockReturnValue(of(mockSdk)); + }); + + it("should set up a new SSO user with key connector using V2", async () => { const conversionState = stateProvider.singleUser.getFake( mockUserId, NEW_SSO_USER_KEY_CONNECTOR_CONVERSION, @@ -465,11 +488,253 @@ describe("KeyConnectorService", () => { await keyConnectorService.convertNewSsoUserToKeyConnector(mockUserId); + expect(registerSdkService.registerClient$).toHaveBeenCalledWith(mockUserId); + expect(mockSdk.take).toHaveBeenCalled(); + expect(mockSdkRef.value.auth).toHaveBeenCalled(); + + const mockRegistration = mockSdkRef.value + .auth() + .registration().post_keys_for_key_connector_registration; + expect(mockRegistration).toHaveBeenCalledWith( + keyConnectorUrl, + mockSsoOrgIdentifier, + mockUserId, + ); + + expect(masterPasswordService.mock.setMasterKey).toHaveBeenCalledWith( + expect.any(SymmetricCryptoKey), + mockUserId, + ); + expect(keyService.setUserKey).toHaveBeenCalledWith( + expect.any(SymmetricCryptoKey), + mockUserId, + ); + expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith( + expect.any(EncString), + mockUserId, + ); + expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalledWith( + { + V2: { + private_key: mockPrivateKey, + signing_key: mockSigningKey, + signed_public_key: mockSignedPublicKey, + security_state: mockSecurityState, + }, + }, + mockUserId, + ); + expect(keyService.setPrivateKey).toHaveBeenCalledWith(mockPrivateKey, mockUserId); + expect(keyService.setUserSigningKey).toHaveBeenCalledWith(mockSigningKey, mockUserId); + expect(securityStateService.setAccountSecurityState).toHaveBeenCalledWith( + mockSecurityState, + mockUserId, + ); + expect(keyService.setSignedPublicKey).toHaveBeenCalledWith(mockSignedPublicKey, mockUserId); + + expect(await firstValueFrom(conversionState.state$)).toBeNull(); + }); + + it("should throw error when SDK is not available", async () => { + registerSdkService.registerClient$.mockReturnValue( + of(null as unknown as Rc), + ); + + const conversionState = stateProvider.singleUser.getFake( + mockUserId, + NEW_SSO_USER_KEY_CONNECTOR_CONVERSION, + ); + conversionState.nextState(conversion); + + await expect( + keyConnectorService.convertNewSsoUserToKeyConnector(mockUserId), + ).rejects.toThrow("SDK not available"); + + expect(await firstValueFrom(conversionState.state$)).toEqual(conversion); + expect(masterPasswordService.mock.setMasterKey).not.toHaveBeenCalled(); + expect(keyService.setUserKey).not.toHaveBeenCalled(); + expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).not.toHaveBeenCalled(); + expect( + accountCryptographicStateService.setAccountCryptographicState, + ).not.toHaveBeenCalled(); + expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + expect(keyService.setUserSigningKey).not.toHaveBeenCalled(); + expect(securityStateService.setAccountSecurityState).not.toHaveBeenCalled(); + expect(keyService.setSignedPublicKey).not.toHaveBeenCalled(); + }); + + it("should throw error when account cryptographic state is not V2", async () => { + mockSdkRef.value + .auth() + .registration() + .post_keys_for_key_connector_registration.mockResolvedValue({ + key_connector_key: mockKeyConnectorKey, + user_key: mockUserKeyString, + key_connector_key_wrapped_user_key: mockKeyConnectorKeyWrappedUserKey, + account_cryptographic_state: { + V1: { + private_key: mockPrivateKey, + }, + }, + }); + + const conversionState = stateProvider.singleUser.getFake( + mockUserId, + NEW_SSO_USER_KEY_CONNECTOR_CONVERSION, + ); + conversionState.nextState(conversion); + + await expect( + keyConnectorService.convertNewSsoUserToKeyConnector(mockUserId), + ).rejects.toThrow("Unexpected account cryptographic state version"); + + expect(await firstValueFrom(conversionState.state$)).toEqual(conversion); + expect(masterPasswordService.mock.setMasterKey).not.toHaveBeenCalled(); + expect(keyService.setUserKey).not.toHaveBeenCalled(); + expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).not.toHaveBeenCalled(); + expect( + accountCryptographicStateService.setAccountCryptographicState, + ).not.toHaveBeenCalled(); + expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + expect(keyService.setUserSigningKey).not.toHaveBeenCalled(); + expect(securityStateService.setAccountSecurityState).not.toHaveBeenCalled(); + expect(keyService.setSignedPublicKey).not.toHaveBeenCalled(); + }); + + it("should throw error when post_keys_for_key_connector_registration fails", async () => { + const sdkError = new Error("Key Connector registration failed"); + mockSdkRef.value + .auth() + .registration() + .post_keys_for_key_connector_registration.mockRejectedValue(sdkError); + + const conversionState = stateProvider.singleUser.getFake( + mockUserId, + NEW_SSO_USER_KEY_CONNECTOR_CONVERSION, + ); + conversionState.nextState(conversion); + + await expect( + keyConnectorService.convertNewSsoUserToKeyConnector(mockUserId), + ).rejects.toThrow("Key Connector registration failed"); + + expect(await firstValueFrom(conversionState.state$)).toEqual(conversion); + expect(masterPasswordService.mock.setMasterKey).not.toHaveBeenCalled(); + expect(keyService.setUserKey).not.toHaveBeenCalled(); + expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).not.toHaveBeenCalled(); + expect( + accountCryptographicStateService.setAccountCryptographicState, + ).not.toHaveBeenCalled(); + expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + expect(keyService.setUserSigningKey).not.toHaveBeenCalled(); + expect(securityStateService.setAccountSecurityState).not.toHaveBeenCalled(); + expect(keyService.setSignedPublicKey).not.toHaveBeenCalled(); + }); + }); + + describe("V1", () => { + const passwordKey = new SymmetricCryptoKey(new Uint8Array(64)); + const mockUserKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey; + const mockEmail = "test@example.com"; + const mockMasterKey = getMockMasterKey(); + const mockKeyPair = ["mockPubKey", new EncString("mockEncryptedPrivKey")] as [ + string, + EncString, + ]; + let mockMakeUserKeyResult: [UserKey, EncString]; + + beforeEach(() => { + const mockUserKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey; + const encString = new EncString("mockEncryptedString"); + mockMakeUserKeyResult = [mockUserKey, encString] as [UserKey, EncString]; + + keyGenerationService.createKey.mockResolvedValue(passwordKey); + keyService.makeMasterKey.mockResolvedValue(mockMasterKey); + keyService.makeUserKey.mockResolvedValue(mockMakeUserKeyResult); + keyService.makeKeyPair.mockResolvedValue(mockKeyPair); + tokenService.getEmail.mockResolvedValue(mockEmail); + configService.getFeatureFlag$.mockReturnValue(of(false)); + }); + + it.each([ + [KdfType.PBKDF2_SHA256, 700_000, undefined, undefined], + [KdfType.Argon2id, 11, 65, 5], + ])( + "sets up a new SSO user with key connector", + async (kdfType, kdfIterations, kdfMemory, kdfParallelism) => { + const expectedKdfConfig = + kdfType == KdfType.PBKDF2_SHA256 + ? new PBKDF2KdfConfig(kdfIterations) + : new Argon2KdfConfig(kdfIterations, kdfMemory, kdfParallelism); + + const conversion: NewSsoUserKeyConnectorConversion = { + kdfConfig: expectedKdfConfig, + keyConnectorUrl: keyConnectorUrl, + organizationId: mockSsoOrgIdentifier, + }; + const conversionState = stateProvider.singleUser.getFake( + mockUserId, + NEW_SSO_USER_KEY_CONNECTOR_CONVERSION, + ); + conversionState.nextState(conversion); + + await keyConnectorService.convertNewSsoUserToKeyConnector(mockUserId); + + expect(keyGenerationService.createKey).toHaveBeenCalledWith(512); + expect(keyService.makeMasterKey).toHaveBeenCalledWith( + passwordKey.keyB64, + mockEmail, + expectedKdfConfig, + ); + expect(masterPasswordService.mock.setMasterKey).toHaveBeenCalledWith( + mockMasterKey, + mockUserId, + ); + expect(keyService.makeUserKey).toHaveBeenCalledWith(mockMasterKey); + expect(keyService.setUserKey).toHaveBeenCalledWith(mockUserKey, mockUserId); + expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith( + mockMakeUserKeyResult[1], + mockUserId, + ); + expect(keyService.makeKeyPair).toHaveBeenCalledWith(mockMakeUserKeyResult[0]); + expect(apiService.postUserKeyToKeyConnector).toHaveBeenCalledWith( + keyConnectorUrl, + new KeyConnectorUserKeyRequest( + Utils.fromBufferToB64(mockMasterKey.inner().encryptionKey), + ), + ); + expect(apiService.postSetKeyConnectorKey).toHaveBeenCalledWith( + new SetKeyConnectorKeyRequest( + mockMakeUserKeyResult[1].encryptedString!, + expectedKdfConfig, + mockSsoOrgIdentifier, + new KeysRequest(mockKeyPair[0], mockKeyPair[1].encryptedString!), + ), + ); + + // Verify that conversion data is cleared from conversionState + expect(await firstValueFrom(conversionState.state$)).toBeNull(); + }, + ); + + it("handles api error", async () => { + apiService.postUserKeyToKeyConnector.mockRejectedValue(new Error("API error")); + + const conversionState = stateProvider.singleUser.getFake( + mockUserId, + NEW_SSO_USER_KEY_CONNECTOR_CONVERSION, + ); + conversionState.nextState(conversion); + + await expect( + keyConnectorService.convertNewSsoUserToKeyConnector(mockUserId), + ).rejects.toThrow(new Error("Key Connector error")); + expect(keyGenerationService.createKey).toHaveBeenCalledWith(512); expect(keyService.makeMasterKey).toHaveBeenCalledWith( passwordKey.keyB64, mockEmail, - expectedKdfConfig, + new PBKDF2KdfConfig(600_000), ); expect(masterPasswordService.mock.setMasterKey).toHaveBeenCalledWith( mockMasterKey, @@ -488,76 +753,29 @@ describe("KeyConnectorService", () => { Utils.fromBufferToB64(mockMasterKey.inner().encryptionKey), ), ); - expect(apiService.postSetKeyConnectorKey).toHaveBeenCalledWith( - new SetKeyConnectorKeyRequest( - mockMakeUserKeyResult[1].encryptedString!, - expectedKdfConfig, - mockOrgId, - new KeysRequest(mockKeyPair[0], mockKeyPair[1].encryptedString!), - ), + expect(apiService.postSetKeyConnectorKey).not.toHaveBeenCalled(); + expect(await firstValueFrom(conversionState.state$)).toEqual(conversion); + + expect(logoutCallback).toHaveBeenCalledWith("keyConnectorError"); + }); + + it("should throw error when conversion data is null", async () => { + const conversionState = stateProvider.singleUser.getFake( + mockUserId, + NEW_SSO_USER_KEY_CONNECTOR_CONVERSION, ); + conversionState.nextState(null); - // Verify that conversion data is cleared from conversionState - expect(await firstValueFrom(conversionState.state$)).toBeNull(); - }, - ); + await expect( + keyConnectorService.convertNewSsoUserToKeyConnector(mockUserId), + ).rejects.toThrow(new Error("Key Connector conversion not found")); - it("handles api error", async () => { - apiService.postUserKeyToKeyConnector.mockRejectedValue(new Error("API error")); - - const conversionState = stateProvider.singleUser.getFake( - mockUserId, - NEW_SSO_USER_KEY_CONNECTOR_CONVERSION, - ); - conversionState.nextState(conversion); - - await expect(keyConnectorService.convertNewSsoUserToKeyConnector(mockUserId)).rejects.toThrow( - new Error("Key Connector error"), - ); - - expect(keyGenerationService.createKey).toHaveBeenCalledWith(512); - expect(keyService.makeMasterKey).toHaveBeenCalledWith( - passwordKey.keyB64, - mockEmail, - new PBKDF2KdfConfig(600_000), - ); - expect(masterPasswordService.mock.setMasterKey).toHaveBeenCalledWith( - mockMasterKey, - mockUserId, - ); - expect(keyService.makeUserKey).toHaveBeenCalledWith(mockMasterKey); - expect(keyService.setUserKey).toHaveBeenCalledWith(mockUserKey, mockUserId); - expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith( - mockMakeUserKeyResult[1], - mockUserId, - ); - expect(keyService.makeKeyPair).toHaveBeenCalledWith(mockMakeUserKeyResult[0]); - expect(apiService.postUserKeyToKeyConnector).toHaveBeenCalledWith( - keyConnectorUrl, - new KeyConnectorUserKeyRequest(Utils.fromBufferToB64(mockMasterKey.inner().encryptionKey)), - ); - expect(apiService.postSetKeyConnectorKey).not.toHaveBeenCalled(); - expect(await firstValueFrom(conversionState.state$)).toEqual(conversion); - - expect(logoutCallback).toHaveBeenCalledWith("keyConnectorError"); - }); - - it("should throw error when conversion data is null", async () => { - const conversionState = stateProvider.singleUser.getFake( - mockUserId, - NEW_SSO_USER_KEY_CONNECTOR_CONVERSION, - ); - conversionState.nextState(null); - - await expect(keyConnectorService.convertNewSsoUserToKeyConnector(mockUserId)).rejects.toThrow( - new Error("Key Connector conversion not found"), - ); - - // Verify that no key generation or API calls were made - expect(keyGenerationService.createKey).not.toHaveBeenCalled(); - expect(keyService.makeMasterKey).not.toHaveBeenCalled(); - expect(apiService.postUserKeyToKeyConnector).not.toHaveBeenCalled(); - expect(apiService.postSetKeyConnectorKey).not.toHaveBeenCalled(); + // Verify that no key generation or API calls were made + expect(keyGenerationService.createKey).not.toHaveBeenCalled(); + expect(keyService.makeMasterKey).not.toHaveBeenCalled(); + expect(apiService.postUserKeyToKeyConnector).not.toHaveBeenCalled(); + expect(apiService.postSetKeyConnectorKey).not.toHaveBeenCalled(); + }); }); }); diff --git a/libs/common/src/key-management/key-connector/services/key-connector.service.ts b/libs/common/src/key-management/key-connector/services/key-connector.service.ts index 8a75034cae1..751f1ec8594 100644 --- a/libs/common/src/key-management/key-connector/services/key-connector.service.ts +++ b/libs/common/src/key-management/key-connector/services/key-connector.service.ts @@ -9,22 +9,36 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { NewSsoUserKeyConnectorConversion } from "@bitwarden/common/key-management/key-connector/models/new-sso-user-key-connector-conversion"; // 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 { Argon2KdfConfig, KdfType, KeyService, PBKDF2KdfConfig } from "@bitwarden/key-management"; +import { + Argon2KdfConfig, + KdfConfig, + KdfType, + KeyService, + PBKDF2KdfConfig, +} from "@bitwarden/key-management"; +import { LogService } from "@bitwarden/logging"; import { ApiService } from "../../../abstractions/api.service"; import { OrganizationService } from "../../../admin-console/abstractions/organization/organization.service.abstraction"; import { OrganizationUserType } from "../../../admin-console/enums"; import { Organization } from "../../../admin-console/models/domain/organization"; import { TokenService } from "../../../auth/abstractions/token.service"; +import { FeatureFlag } from "../../../enums/feature-flag.enum"; import { KeysRequest } from "../../../models/request/keys.request"; -import { LogService } from "../../../platform/abstractions/log.service"; +import { ConfigService } from "../../../platform/abstractions/config/config.service"; +import { RegisterSdkService } from "../../../platform/abstractions/sdk/register-sdk.service"; +import { asUuid } from "../../../platform/abstractions/sdk/sdk.service"; import { Utils } from "../../../platform/misc/utils"; import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; import { KEY_CONNECTOR_DISK, StateProvider, UserKeyDefinition } from "../../../platform/state"; import { UserId } from "../../../types/guid"; -import { MasterKey } from "../../../types/key"; +import { MasterKey, UserKey } from "../../../types/key"; +import { AccountCryptographicStateService } from "../../account-cryptography/account-cryptographic-state.service"; import { KeyGenerationService } from "../../crypto"; +import { EncString } from "../../crypto/models/enc-string"; import { InternalMasterPasswordServiceAbstraction } from "../../master-password/abstractions/master-password.service.abstraction"; +import { SecurityStateService } from "../../security-state/abstractions/security-state.service"; +import { SignedPublicKey, SignedSecurityState, WrappedSigningKey } from "../../types"; import { KeyConnectorService as KeyConnectorServiceAbstraction } from "../abstractions/key-connector.service"; import { KeyConnectorDomainConfirmation } from "../models/key-connector-domain-confirmation"; import { KeyConnectorUserKeyRequest } from "../models/key-connector-user-key.request"; @@ -75,6 +89,10 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { private keyGenerationService: KeyGenerationService, private logoutCallback: (logoutReason: LogoutReason, userId?: string) => Promise, private stateProvider: StateProvider, + private configService: ConfigService, + private registerSdkService: RegisterSdkService, + private securityStateService: SecurityStateService, + private accountCryptographicStateService: AccountCryptographicStateService, ) { this.convertAccountRequired$ = accountService.activeAccount$.pipe( filter((account) => account != null), @@ -152,8 +170,106 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { throw new Error("Key Connector conversion not found"); } - const { kdfConfig, keyConnectorUrl, organizationId } = conversion; + const { kdfConfig, keyConnectorUrl, organizationId: ssoOrganizationIdentifier } = conversion; + if ( + await firstValueFrom( + this.configService.getFeatureFlag$( + FeatureFlag.EnableAccountEncryptionV2KeyConnectorRegistration, + ), + ) + ) { + await this.convertNewSsoUserToKeyConnectorV2( + userId, + keyConnectorUrl, + ssoOrganizationIdentifier, + ); + } else { + await this.convertNewSsoUserToKeyConnectorV1( + userId, + kdfConfig, + keyConnectorUrl, + ssoOrganizationIdentifier, + ); + } + + await this.stateProvider + .getUser(userId, NEW_SSO_USER_KEY_CONNECTOR_CONVERSION) + .update(() => null); + } + + async convertNewSsoUserToKeyConnectorV2( + userId: UserId, + keyConnectorUrl: string, + ssoOrganizationIdentifier: string, + ) { + const result = await firstValueFrom( + this.registerSdkService.registerClient$(userId).pipe( + map((sdk) => { + if (!sdk) { + throw new Error("SDK not available"); + } + + using ref = sdk.take(); + + return ref.value + .auth() + .registration() + .post_keys_for_key_connector_registration( + keyConnectorUrl, + ssoOrganizationIdentifier, + asUuid(userId), + ); + }), + ), + ); + + if (!("V2" in result.account_cryptographic_state)) { + const version = Object.keys(result.account_cryptographic_state); + throw new Error(`Unexpected account cryptographic state version ${version}`); + } + + await this.masterPasswordService.setMasterKey( + SymmetricCryptoKey.fromString(result.key_connector_key) as MasterKey, + userId, + ); + await this.keyService.setUserKey( + SymmetricCryptoKey.fromString(result.user_key) as UserKey, + userId, + ); + await this.masterPasswordService.setMasterKeyEncryptedUserKey( + new EncString(result.key_connector_key_wrapped_user_key), + userId, + ); + + await this.accountCryptographicStateService.setAccountCryptographicState( + result.account_cryptographic_state, + userId, + ); + // Legacy states + await this.keyService.setPrivateKey(result.account_cryptographic_state.V2.private_key, userId); + await this.keyService.setUserSigningKey( + result.account_cryptographic_state.V2.signing_key as WrappedSigningKey, + userId, + ); + await this.securityStateService.setAccountSecurityState( + result.account_cryptographic_state.V2.security_state as SignedSecurityState, + userId, + ); + if (result.account_cryptographic_state.V2.signed_public_key != null) { + await this.keyService.setSignedPublicKey( + result.account_cryptographic_state.V2.signed_public_key as SignedPublicKey, + userId, + ); + } + } + + async convertNewSsoUserToKeyConnectorV1( + userId: UserId, + kdfConfig: KdfConfig, + keyConnectorUrl: string, + ssoOrganizationIdentifier: string, + ) { const password = await this.keyGenerationService.createKey(512); const masterKey = await this.keyService.makeMasterKey( @@ -182,14 +298,10 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { const setPasswordRequest = new SetKeyConnectorKeyRequest( userKey[1].encryptedString, kdfConfig, - organizationId, + ssoOrganizationIdentifier, keys, ); await this.apiService.postSetKeyConnectorKey(setPasswordRequest); - - await this.stateProvider - .getUser(userId, NEW_SSO_USER_KEY_CONNECTOR_CONVERSION) - .update(() => null); } async setNewSsoUserKeyConnectorConversionData( diff --git a/package-lock.json b/package-lock.json index c6df231c0a1..e2556015113 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23,8 +23,8 @@ "@angular/platform-browser": "20.3.15", "@angular/platform-browser-dynamic": "20.3.15", "@angular/router": "20.3.15", - "@bitwarden/commercial-sdk-internal": "0.2.0-main.448", - "@bitwarden/sdk-internal": "0.2.0-main.448", + "@bitwarden/commercial-sdk-internal": "0.2.0-main.450", + "@bitwarden/sdk-internal": "0.2.0-main.450", "@electron/fuses": "1.8.0", "@emotion/css": "11.13.5", "@koa/multer": "4.0.0", @@ -4973,9 +4973,9 @@ "link": true }, "node_modules/@bitwarden/commercial-sdk-internal": { - "version": "0.2.0-main.448", - "resolved": "https://registry.npmjs.org/@bitwarden/commercial-sdk-internal/-/commercial-sdk-internal-0.2.0-main.448.tgz", - "integrity": "sha512-1lAquLZACXDlSNxXv95dF7Q/+NVRqZE2VuKcXV2n5yRSNkRny9QgBv1K5EH1miVyQKU7vXmrpvX/AyL91qeNAw==", + "version": "0.2.0-main.450", + "resolved": "https://registry.npmjs.org/@bitwarden/commercial-sdk-internal/-/commercial-sdk-internal-0.2.0-main.450.tgz", + "integrity": "sha512-WCihR6ykpIfaqJBHl4Wou4xDB8mp+5UPi94eEKYUdkx/9/19YyX33SX9H56zEriOuOMCD8l2fymhzAFjAAB++g==", "license": "BITWARDEN SOFTWARE DEVELOPMENT KIT LICENSE AGREEMENT", "dependencies": { "type-fest": "^4.41.0" @@ -5078,9 +5078,9 @@ "link": true }, "node_modules/@bitwarden/sdk-internal": { - "version": "0.2.0-main.448", - "resolved": "https://registry.npmjs.org/@bitwarden/sdk-internal/-/sdk-internal-0.2.0-main.448.tgz", - "integrity": "sha512-QYa/imUJlM1E3oLXZwZhwHkPoRDdq8iEt4UDhCLkXhsbCqi8LAAUL9yNCYqyJ0F94yuYIJjozPo28N2rpPnVEA==", + "version": "0.2.0-main.450", + "resolved": "https://registry.npmjs.org/@bitwarden/sdk-internal/-/sdk-internal-0.2.0-main.450.tgz", + "integrity": "sha512-XRhrBN0uoo66ONx7dYo9glhe9N451+VhwtC/oh3wo3j3qYxbPwf9yE98szlQ52u3iUExLisiYJY7sQNzhZrbZw==", "license": "GPL-3.0", "dependencies": { "type-fest": "^4.41.0" diff --git a/package.json b/package.json index cc5abd9fdf0..be6658964a0 100644 --- a/package.json +++ b/package.json @@ -162,8 +162,8 @@ "@angular/platform-browser": "20.3.15", "@angular/platform-browser-dynamic": "20.3.15", "@angular/router": "20.3.15", - "@bitwarden/sdk-internal": "0.2.0-main.448", - "@bitwarden/commercial-sdk-internal": "0.2.0-main.448", + "@bitwarden/sdk-internal": "0.2.0-main.450", + "@bitwarden/commercial-sdk-internal": "0.2.0-main.450", "@electron/fuses": "1.8.0", "@emotion/css": "11.13.5", "@koa/multer": "4.0.0", diff --git a/scripts/dep-ownership.ts b/scripts/dep-ownership.ts index f0bcb1f7dd8..ae1a19bb170 100644 --- a/scripts/dep-ownership.ts +++ b/scripts/dep-ownership.ts @@ -1,6 +1,6 @@ /* eslint-disable no-console */ -/// Ensure that all dependencies in package.json have an owner in the renovate.json file. +/// Ensure that all dependencies in package.json and Cargo.toml have an owner in the renovate.json5 file. import fs from "fs"; import path from "path"; @@ -11,22 +11,67 @@ const renovateConfig = JSON5.parse( fs.readFileSync(path.join(__dirname, "..", "..", ".github", "renovate.json5"), "utf8"), ); +// Extract all packages with owners from renovate config const packagesWithOwners = renovateConfig.packageRules .flatMap((rule: any) => rule.matchPackageNames) .filter((packageName: string) => packageName != null); +function hasOwner(packageName: string): boolean { + return packagesWithOwners.includes(packageName); +} + +// Collect npm dependencies const packageJson = JSON.parse( fs.readFileSync(path.join(__dirname, "..", "..", "package.json"), "utf8"), ); -const dependencies = Object.keys(packageJson.dependencies).concat( - Object.keys(packageJson.devDependencies), +const npmDependencies = [ + ...Object.keys(packageJson.dependencies || {}), + ...Object.keys(packageJson.devDependencies || {}), +]; + +// Collect Cargo dependencies from workspace Cargo.toml +const cargoTomlPath = path.join( + __dirname, + "..", + "..", + "apps", + "desktop", + "desktop_native", + "Cargo.toml", ); +const cargoTomlContent = fs.existsSync(cargoTomlPath) ? fs.readFileSync(cargoTomlPath, "utf8") : ""; -const missingOwners = dependencies.filter((dep) => !packagesWithOwners.includes(dep)); +const cargoDependencies = new Set(); -if (missingOwners.length > 0) { +// Extract dependency names from [workspace.dependencies] section by +// extracting everything between [workspace.dependencies] and the next section start +// (indicated by a "\n["). +const workspaceSection = + cargoTomlContent.split("[workspace.dependencies]")[1]?.split(/\n\[/)[0] ?? ""; + +// Process each line to extract dependency names +workspaceSection + .split("\n") // Process each line + .map((line) => line.match(/^([a-zA-Z0-9_-]+)\s*=/)?.[1]) // Find the dependency name + .filter((depName): depName is string => depName != null && !depName.startsWith("bitwarden")) // Make sure it's not an empty line or a Bitwarden dependency + .forEach((depName) => cargoDependencies.add(depName)); + +// Check for missing owners +const missingNpmOwners = npmDependencies.filter((dep) => !hasOwner(dep)); +const missingCargoOwners = Array.from(cargoDependencies).filter((dep) => !hasOwner(dep)); + +const allMissing = [...missingNpmOwners, ...missingCargoOwners]; + +if (allMissing.length > 0) { console.error("Missing owners for the following dependencies:"); - console.error(missingOwners.join("\n")); + if (missingNpmOwners.length > 0) { + console.error("\nNPM dependencies:"); + console.error(missingNpmOwners.join("\n")); + } + if (missingCargoOwners.length > 0) { + console.error("\nCargo dependencies:"); + console.error(missingCargoOwners.join("\n")); + } process.exit(1); }