diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index c77fa959047..c957ac0e22b 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -8,7 +8,7 @@ apps/desktop/desktop_native @bitwarden/team-platform-dev apps/desktop/desktop_native/objc/src/native/autofill @bitwarden/team-autofill-desktop-dev apps/desktop/desktop_native/core/src/autofill @bitwarden/team-autofill-desktop-dev -apps/desktop/desktop_native/macos_provider @bitwarden/team-autofill-desktop-dev +apps/desktop/desktop_native/autofill_provider @bitwarden/team-autofill-desktop-dev apps/desktop/desktop_native/core/src/secure_memory @bitwarden/team-key-management-dev ## No ownership for Cargo.lock and Cargo.toml to allow dependency updates @@ -84,6 +84,7 @@ apps/web/src/app/billing @bitwarden/team-billing-dev libs/angular/src/billing @bitwarden/team-billing-dev libs/common/src/billing @bitwarden/team-billing-dev libs/billing @bitwarden/team-billing-dev +libs/pricing @bitwarden/team-billing-dev bitwarden_license/bit-web/src/app/billing @bitwarden/team-billing-dev ## Platform team files ## @@ -223,6 +224,9 @@ apps/web/src/locales/en/messages.json **/docker-compose.yml @bitwarden/team-appsec @bitwarden/dept-bre **/entrypoint.sh @bitwarden/team-appsec @bitwarden/dept-bre +# Scanning tools +.checkmarx/ @bitwarden/team-appsec + ## Overrides # For the time being platform owns tsconfig and jest config # These overrides will be removed after Nx is implemented @@ -230,7 +234,9 @@ apps/web/src/locales/en/messages.json **/tsconfig.json @bitwarden/team-platform-dev **/jest.config.js @bitwarden/team-platform-dev **/project.jsons @bitwarden/team-platform-dev -libs/pricing @bitwarden/team-billing-dev +# Platform override specifically for the package-lock.json in +# native-messaging-test-runner so that Platform can manage all lock file updates +apps/desktop/native-messaging-test-runner/package-lock.json @bitwarden/team-platform-dev # Claude related files .claude/ @bitwarden/team-ai-sme diff --git a/.github/renovate.json5 b/.github/renovate.json5 index d1a0ef12f4b..b0ede711f4c 100644 --- a/.github/renovate.json5 +++ b/.github/renovate.json5 @@ -189,6 +189,7 @@ "semver", "serde", "serde_json", + "serde_with", "simplelog", "style-loader", "sysinfo", diff --git a/.github/workflows/build-browser.yml b/.github/workflows/build-browser.yml index 7b35baf01e2..ef2c91f0a7d 100644 --- a/.github/workflows/build-browser.yml +++ b/.github/workflows/build-browser.yml @@ -152,7 +152,7 @@ jobs: persist-credentials: false - name: Set up Node - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: cache: 'npm' cache-dependency-path: '**/package-lock.json' @@ -260,7 +260,7 @@ jobs: persist-credentials: false - name: Set up Node - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: cache: 'npm' cache-dependency-path: '**/package-lock.json' @@ -392,7 +392,7 @@ jobs: persist-credentials: false - name: Set up Node - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: cache: 'npm' cache-dependency-path: '**/package-lock.json' diff --git a/.github/workflows/build-cli.yml b/.github/workflows/build-cli.yml index d0abe8e12e7..75820c54977 100644 --- a/.github/workflows/build-cli.yml +++ b/.github/workflows/build-cli.yml @@ -130,7 +130,7 @@ jobs: } >> "$GITHUB_ENV" - name: Set up Node - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: cache: 'npm' cache-dependency-path: '**/package-lock.json' @@ -326,7 +326,7 @@ jobs: choco install nasm --no-progress - name: Set up Node - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: cache: 'npm' cache-dependency-path: '**/package-lock.json' diff --git a/.github/workflows/build-desktop.yml b/.github/workflows/build-desktop.yml index 0d4009e54f9..c021dedd8e1 100644 --- a/.github/workflows/build-desktop.yml +++ b/.github/workflows/build-desktop.yml @@ -183,7 +183,7 @@ jobs: uses: bitwarden/gh-actions/free-disk-space@main - name: Set up Node - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: cache: 'npm' cache-dependency-path: '**/package-lock.json' @@ -339,7 +339,7 @@ jobs: persist-credentials: false - name: Set up Node - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: cache: 'npm' cache-dependency-path: '**/package-lock.json' @@ -487,7 +487,7 @@ jobs: persist-credentials: false - name: Set up Node - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: cache: 'npm' cache-dependency-path: '**/package-lock.json' @@ -755,7 +755,7 @@ jobs: persist-credentials: false - name: Set up Node - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: cache: 'npm' cache-dependency-path: '**/package-lock.json' @@ -1000,7 +1000,7 @@ jobs: persist-credentials: false - name: Set up Node - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: cache: 'npm' cache-dependency-path: '**/package-lock.json' @@ -1240,7 +1240,7 @@ jobs: persist-credentials: false - name: Set up Node - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: cache: 'npm' cache-dependency-path: '**/package-lock.json' @@ -1515,7 +1515,7 @@ jobs: persist-credentials: false - name: Set up Node - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: cache: 'npm' cache-dependency-path: '**/package-lock.json' diff --git a/.github/workflows/chromatic.yml b/.github/workflows/chromatic.yml index c7d80b82baa..6189744fe67 100644 --- a/.github/workflows/chromatic.yml +++ b/.github/workflows/chromatic.yml @@ -58,7 +58,7 @@ jobs: echo "node_version=$NODE_VERSION" >> "$GITHUB_OUTPUT" - name: Set up Node - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: node-version: ${{ steps.retrieve-node-version.outputs.node_version }} if: steps.get-changed-files-for-chromatic.outputs.storyFiles == 'true' diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 81d79df569c..7862c14c186 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -64,7 +64,7 @@ jobs: echo "node_version=$NODE_VERSION" >> "$GITHUB_OUTPUT" - name: Set up Node - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: cache: 'npm' cache-dependency-path: '**/package-lock.json' @@ -142,7 +142,7 @@ jobs: run: cargo +nightly udeps --workspace --all-features --all-targets - name: Install cargo-deny - uses: taiki-e/install-action@2e9d707ef49c9b094d45955b60c7e5c0dfedeb14 # v2.66.5 + uses: taiki-e/install-action@542cebaaed782771e619bd5609d97659d109c492 # v2.66.7 with: tool: cargo-deny@0.18.6 diff --git a/.github/workflows/nx.yml b/.github/workflows/nx.yml index 3a7431c07f0..e468ead4f1e 100644 --- a/.github/workflows/nx.yml +++ b/.github/workflows/nx.yml @@ -26,7 +26,7 @@ jobs: echo "node_version=$NODE_VERSION" >> "$GITHUB_OUTPUT" - name: Set up Node - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: cache: 'npm' cache-dependency-path: '**/package-lock.json' diff --git a/.github/workflows/publish-cli.yml b/.github/workflows/publish-cli.yml index ef287b0de08..5f6ee83e41f 100644 --- a/.github/workflows/publish-cli.yml +++ b/.github/workflows/publish-cli.yml @@ -216,7 +216,7 @@ jobs: echo "node_version=$NODE_VERSION" >> "$GITHUB_OUTPUT" - name: Set up Node - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: node-version: ${{ steps.retrieve-node-version.outputs.node_version }} registry-url: "https://registry.npmjs.org/" diff --git a/.github/workflows/sdk-breaking-change-check.yml b/.github/workflows/sdk-breaking-change-check.yml index 765e900af5c..eab0dffeda4 100644 --- a/.github/workflows/sdk-breaking-change-check.yml +++ b/.github/workflows/sdk-breaking-change-check.yml @@ -76,7 +76,7 @@ jobs: echo "node_version=$NODE_VERSION" >> "$GITHUB_OUTPUT" - name: Set up Node - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: cache: 'npm' cache-dependency-path: '**/package-lock.json' diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index eedf991d826..41b75c5a31d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -36,7 +36,7 @@ jobs: echo "node_version=$NODE_VERSION" >> "$GITHUB_OUTPUT" - name: Set up Node - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 with: cache: 'npm' cache-dependency-path: '**/package-lock.json' diff --git a/apps/browser/config/base.json b/apps/browser/config/base.json index 02bdc5d22af..0de1da0a648 100644 --- a/apps/browser/config/base.json +++ b/apps/browser/config/base.json @@ -1,7 +1,6 @@ { "devFlags": {}, "flags": { - "accountSwitching": false, "sdk": true } } diff --git a/apps/browser/config/development.json b/apps/browser/config/development.json index 042a98c2c39..12a34d8cbee 100644 --- a/apps/browser/config/development.json +++ b/apps/browser/config/development.json @@ -4,8 +4,5 @@ "base": "https://localhost:8080" }, "skipWelcomeOnInstall": true - }, - "flags": { - "accountSwitching": true } } diff --git a/apps/browser/config/production.json b/apps/browser/config/production.json deleted file mode 100644 index a43eee1d5c9..00000000000 --- a/apps/browser/config/production.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "flags": { - "accountSwitching": true - } -} diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 8e2c3279687..4c36a852f6a 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -990,6 +990,12 @@ "no": { "message": "No" }, + "noAuth": { + "message": "Anyone with the link" + }, + "anyOneWithPassword": { + "message": "Anyone with a password set by you" + }, "location": { "message": "Location" }, @@ -2048,6 +2054,9 @@ "email": { "message": "Email" }, + "emails": { + "message": "Emails" + }, "phone": { "message": "Phone" }, @@ -4610,11 +4619,11 @@ "message": "URI match detection is how Bitwarden identifies autofill suggestions.", "description": "Explains to the user that URI match detection determines how Bitwarden suggests autofill options, and clarifies that this default strategy applies when no specific match detection is set for a login item." }, - "regExAdvancedOptionWarning": { + "regExAdvancedOptionWarning": { "message": "\"Regular expression\" is an advanced option with increased risk of exposing credentials.", "description": "Content for dialog which warns a user when selecting 'regular expression' matching strategy as a cipher match strategy" }, - "startsWithAdvancedOptionWarning": { + "startsWithAdvancedOptionWarning": { "message": "\"Starts with\" is an advanced option with increased risk of exposing credentials.", "description": "Content for dialog which warns a user when selecting 'starts with' matching strategy as a cipher match strategy" }, @@ -4622,7 +4631,7 @@ "message": "More about match detection", "description": "Link to match detection docs on warning dialog for advance match strategy" }, - "uriAdvancedOption":{ + "uriAdvancedOption": { "message": "Advanced options", "description": "Advanced option placeholder for uri option component" }, @@ -4812,7 +4821,7 @@ } } }, - "copyFieldCipherName": { + "copyFieldCipherName": { "message": "Copy $FIELD$, $CIPHERNAME$", "description": "Title for a button that copies a field value to the clipboard.", "placeholders": { @@ -4844,7 +4853,7 @@ "adminConsole": { "message": "Admin Console" }, - "admin" :{ + "admin": { "message": "Admin" }, "automaticUserConfirmation": { @@ -4853,7 +4862,7 @@ "automaticUserConfirmationHint": { "message": "Automatically confirm pending users while this device is unlocked" }, - "autoConfirmOnboardingCallout":{ + "autoConfirmOnboardingCallout": { "message": "Save time with automatic user confirmation" }, "autoConfirmWarning": { @@ -5793,7 +5802,7 @@ "hasItemsVaultNudgeTitle": { "message": "Welcome to your vault!" }, - "phishingPageTitleV2":{ + "phishingPageTitleV2": { "message": "Phishing attempt detected" }, "phishingPageSummary": { @@ -5813,7 +5822,7 @@ "message": ", an open-source list of known phishing sites used for stealing personal and sensitive information.", "description": "This is in multiple parts to allow for bold text in the middle of the sentence. A proper name precedes this." }, - "phishingPageLearnMore" : { + "phishingPageLearnMore": { "message": "Learn more about phishing detection" }, "protectedBy": { @@ -5981,7 +5990,7 @@ "cardNumberLabel": { "message": "Card number" }, - "removeMasterPasswordForOrgUserKeyConnector":{ + "removeMasterPasswordForOrgUserKeyConnector": { "message": "Your organization is no longer using master passwords to log into Bitwarden. To continue, verify the organization and domain." }, "continueWithLogIn": { @@ -5999,10 +6008,10 @@ "verifyYourOrganization": { "message": "Verify your organization to log in" }, - "organizationVerified":{ + "organizationVerified": { "message": "Organization verified" }, - "domainVerified":{ + "domainVerified": { "message": "Domain verified" }, "leaveOrganizationContent": { @@ -6120,5 +6129,20 @@ }, "resizeSideNavigation": { "message": "Resize side navigation" + }, + "whoCanView": { + "message": "Who can view" + }, + "specificPeople": { + "message": "Specific people" + }, + "emailVerificationDesc": { + "message": "After sharing this Send link, individuals will need to verify their email with a code to view this Send." + }, + "enterMultipleEmailsSeparatedByComma": { + "message": "Enter multiple emails by separating with a comma." + }, + "emailPlaceholder": { + "message": "user@bitwarden.com , user@acme.com" } -} +} \ No newline at end of file diff --git a/apps/browser/src/auth/popup/account-switching/account-switcher.component.html b/apps/browser/src/auth/popup/account-switching/account-switcher.component.html index cef2a748d58..0a9e2a1dd9d 100644 --- a/apps/browser/src/auth/popup/account-switching/account-switcher.component.html +++ b/apps/browser/src/auth/popup/account-switching/account-switcher.component.html @@ -7,13 +7,16 @@ - + -
+
- +

{{ "availableAccounts" | i18n }}

 diff --git a/apps/browser/src/auth/popup/account-switching/account-switcher.component.ts b/apps/browser/src/auth/popup/account-switching/account-switcher.component.ts index d7d3c02ab14..ae7f66a9018 100644 --- a/apps/browser/src/auth/popup/account-switching/account-switcher.component.ts +++ b/apps/browser/src/auth/popup/account-switching/account-switcher.component.ts @@ -1,7 +1,7 @@ import { CommonModule, Location } from "@angular/common"; import { Component, OnDestroy, OnInit } from "@angular/core"; import { Router } from "@angular/router"; -import { Subject, firstValueFrom, map, of, startWith, switchMap } from "rxjs"; +import { Observable, Subject, firstValueFrom, map, of, startWith, switchMap } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { LockService, LogoutService } from "@bitwarden/auth/common"; @@ -24,7 +24,6 @@ import { TypographyModule, } from "@bitwarden/components"; -import { enableAccountSwitching } from "../../../platform/flags"; 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"; @@ -59,7 +58,7 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { loading = false; activeUserCanLock = false; - enableAccountSwitching = true; + enableAccountSwitching$: Observable; constructor( private accountSwitcherService: AccountSwitcherService, @@ -72,7 +71,9 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { private authService: AuthService, private lockService: LockService, private logoutService: LogoutService, - ) {} + ) { + this.enableAccountSwitching$ = this.accountSwitcherService.accountSwitchingEnabled$(); + } get accountLimit() { return this.accountSwitcherService.ACCOUNT_LIMIT; @@ -97,19 +98,21 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { switchMap((accounts) => { // If account switching is disabled, don't show the lock all button // as only one account should be shown. - if (!enableAccountSwitching()) { - return of(false); - } + return this.accountSwitcherService.accountSwitchingEnabled$().pipe( + switchMap((enabled) => { + if (!enabled) { + return of(false); + } - // When there are an inactive accounts provide the option to lock all accounts - // Note: "Add account" is counted as an inactive account, so check for more than one account - return of(accounts.length > 1); + // When there are inactive accounts provide the option to lock all accounts + // Note: "Add account" is counted as an inactive account, so check for more than one account + return of(accounts.length > 1); + }), + ); }), ); async ngOnInit() { - this.enableAccountSwitching = enableAccountSwitching(); - const availableVaultTimeoutActions = await firstValueFrom( this.vaultTimeoutSettingsService.availableVaultTimeoutActions$(), ); diff --git a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts index f3be535f00e..13f4a8635df 100644 --- a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts +++ b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts @@ -9,6 +9,7 @@ import { import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { Environment, EnvironmentService, @@ -37,6 +38,7 @@ describe("AccountSwitcherService", () => { const environmentService = mock(); const logService = mock(); const authService = mock(); + const configService = mock(); let accountSwitcherService: AccountSwitcherService; @@ -60,6 +62,7 @@ describe("AccountSwitcherService", () => { messagingService, environmentService, logService, + configService, authService, ); }); diff --git a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts index 99d2c83283e..0f25ea91c99 100644 --- a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts +++ b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts @@ -7,6 +7,7 @@ import { filter, firstValueFrom, map, + of, switchMap, throwError, timeout, @@ -17,11 +18,14 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { UserId } from "@bitwarden/common/types/guid"; +import { BrowserApi } from "../../../../platform/browser/browser-api"; import { fromChromeEvent } from "../../../../platform/browser/from-chrome-event"; export type AvailableAccount = { @@ -52,6 +56,7 @@ export class AccountSwitcherService { private messagingService: MessagingService, private environmentService: EnvironmentService, private logService: LogService, + private configService: ConfigService, authService: AuthService, ) { this.availableAccounts$ = combineLatest([ @@ -123,6 +128,19 @@ export class AccountSwitcherService { ); } + /* + * PM-5594: This was a compile-time flag (default true) which made an exception for Safari in platform/flags. + * The truthiness of AccountSwitching has been enshrined at this point, so those compile-time flags have been removed + * in favor of this method to allow easier access to the config service for controlling Safari. Unwinding the Safari + * flag should be more straightforward from this consolidation. + */ + accountSwitchingEnabled$(): Observable { + if (BrowserApi.isSafariApi) { + return this.configService.getFeatureFlag$(FeatureFlag.SafariAccountSwitching); + } + return of(true); + } + get specialAccountAddId() { return this.SPECIAL_ADD_ACCOUNT_ID; } 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 6a3378670bf..1789feebe4e 100644 --- a/apps/browser/src/auth/popup/settings/account-security.component.ts +++ b/apps/browser/src/auth/popup/settings/account-security.component.ts @@ -257,7 +257,7 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { pin: await this.pinService.isPinSet(activeAccount.id), pinLockWithMasterPassword: (await this.pinService.getPinLockType(activeAccount.id)) == "EPHEMERAL", - biometric: await this.vaultTimeoutSettingsService.isBiometricLockSet(), + biometric: await this.vaultTimeoutSettingsService.isBiometricLockSet(activeAccount.id), enableAutoBiometricsPrompt: await firstValueFrom( this.biometricStateService.promptAutomatically$, ), diff --git a/apps/browser/src/autofill/fido2/content/fido2-page-script.ts b/apps/browser/src/autofill/fido2/content/fido2-page-script.ts index 1cd614a9516..d55e0827352 100644 --- a/apps/browser/src/autofill/fido2/content/fido2-page-script.ts +++ b/apps/browser/src/autofill/fido2/content/fido2-page-script.ts @@ -1,12 +1,10 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { WebauthnUtils } from "../utils/webauthn-utils"; import { MessageTypes } from "./messaging/message"; import { Messenger } from "./messaging/messenger"; (function (globalContext) { - if (globalContext.document.currentScript) { + if (globalContext.document.currentScript?.parentNode) { globalContext.document.currentScript.parentNode.removeChild( globalContext.document.currentScript, ); @@ -86,7 +84,7 @@ import { Messenger } from "./messaging/messenger"; */ async function createWebAuthnCredential( options?: CredentialCreationOptions, - ): Promise { + ): Promise { if (!isWebauthnCall(options)) { return await browserCredentials.create(options); } @@ -106,13 +104,18 @@ import { Messenger } from "./messaging/messenger"; options?.signal, ); - if (response.type !== MessageTypes.CredentialCreationResponse) { + if (response.type !== MessageTypes.CredentialCreationResponse || !response.result) { throw new Error("Something went wrong."); } return WebauthnUtils.mapCredentialRegistrationResult(response.result); } catch (error) { - if (error && error.fallbackRequested && fallbackSupported) { + if ( + fallbackSupported && + error instanceof Object && + "fallbackRequested" in error && + error.fallbackRequested + ) { await waitForFocus(); return await browserCredentials.create(options); } @@ -127,7 +130,9 @@ import { Messenger } from "./messaging/messenger"; * @param options Options for creating new credentials. * @returns Promise that resolves to the new credential object. */ - async function getWebAuthnCredential(options?: CredentialRequestOptions): Promise { + async function getWebAuthnCredential( + options?: CredentialRequestOptions, + ): Promise { if (!isWebauthnCall(options)) { return await browserCredentials.get(options); } @@ -153,7 +158,7 @@ import { Messenger } from "./messaging/messenger"; internalAbortController.signal, ); internalAbortController.signal.removeEventListener("abort", abortListener); - if (response.type !== MessageTypes.CredentialGetResponse) { + if (response.type !== MessageTypes.CredentialGetResponse || !response.result) { throw new Error("Something went wrong."); } @@ -176,7 +181,7 @@ import { Messenger } from "./messaging/messenger"; abortSignal.removeEventListener("abort", abortListener); internalAbortControllers.forEach((controller) => controller.abort()); - return response; + return response ?? null; } try { @@ -188,13 +193,18 @@ import { Messenger } from "./messaging/messenger"; options?.signal, ); - if (response.type !== MessageTypes.CredentialGetResponse) { + if (response.type !== MessageTypes.CredentialGetResponse || !response.result) { throw new Error("Something went wrong."); } return WebauthnUtils.mapCredentialAssertResult(response.result); } catch (error) { - if (error && error.fallbackRequested && fallbackSupported) { + if ( + fallbackSupported && + error instanceof Object && + "fallbackRequested" in error && + error.fallbackRequested + ) { await waitForFocus(); return await browserCredentials.get(options); } @@ -203,8 +213,10 @@ import { Messenger } from "./messaging/messenger"; } } - function isWebauthnCall(options?: CredentialCreationOptions | CredentialRequestOptions) { - return options && "publicKey" in options; + function isWebauthnCall( + options?: CredentialCreationOptions | CredentialRequestOptions, + ): options is CredentialCreationOptions | CredentialRequestOptions { + return options != null && "publicKey" in options; } /** @@ -217,7 +229,7 @@ import { Messenger } from "./messaging/messenger"; */ async function waitForFocus(fallbackWait = 500, timeout = 5 * 60 * 1000) { try { - if (globalContext.top.document.hasFocus()) { + if (globalContext.top?.document.hasFocus()) { return; } } catch { @@ -225,9 +237,14 @@ import { Messenger } from "./messaging/messenger"; return await new Promise((resolve) => globalContext.setTimeout(resolve, fallbackWait)); } + if (!globalContext.top) { + return await new Promise((resolve) => globalContext.setTimeout(resolve, fallbackWait)); + } + + const topWindow = globalContext.top; const focusPromise = new Promise((resolve) => { focusListenerHandler = () => resolve(); - globalContext.top.addEventListener("focus", focusListenerHandler); + topWindow.addEventListener("focus", focusListenerHandler); }); const timeoutPromise = new Promise((_, reject) => { @@ -248,7 +265,7 @@ import { Messenger } from "./messaging/messenger"; } function clearWaitForFocus() { - globalContext.top.removeEventListener("focus", focusListenerHandler); + globalContext.top?.removeEventListener("focus", focusListenerHandler); if (waitForFocusTimeout) { globalContext.clearTimeout(waitForFocusTimeout); } diff --git a/apps/browser/src/autofill/fido2/content/messaging/messenger.ts b/apps/browser/src/autofill/fido2/content/messaging/messenger.ts index 257f7e9efd5..78bb9aa8f33 100644 --- a/apps/browser/src/autofill/fido2/content/messaging/messenger.ts +++ b/apps/browser/src/autofill/fido2/content/messaging/messenger.ts @@ -1,5 +1,3 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { Message, MessageTypes } from "./message"; const SENDER = "bitwarden-webauthn"; @@ -25,7 +23,9 @@ type Handler = ( * handling aborts and exceptions across separate execution contexts. */ export class Messenger { - private messageEventListener: (event: MessageEvent) => void | null = null; + private messageEventListener: + | ((event: MessageEvent) => void | Promise) + | null = null; private onDestroy = new EventTarget(); /** diff --git a/apps/browser/src/autofill/popup/settings/excluded-domains.component.html b/apps/browser/src/autofill/popup/settings/excluded-domains.component.html index 75be3bcc1a0..30170820a27 100644 --- a/apps/browser/src/autofill/popup/settings/excluded-domains.component.html +++ b/apps/browser/src/autofill/popup/settings/excluded-domains.component.html @@ -8,7 +8,9 @@

{{ - accountSwitcherEnabled ? ("excludedDomainsDescAlt" | i18n) : ("excludedDomainsDesc" | i18n) + (accountSwitcherEnabled$ | async) + ? ("excludedDomainsDescAlt" | i18n) + : ("excludedDomainsDesc" | i18n) }}

diff --git a/apps/browser/src/autofill/popup/settings/excluded-domains.component.ts b/apps/browser/src/autofill/popup/settings/excluded-domains.component.ts index e67c826cac6..6714f749d2d 100644 --- a/apps/browser/src/autofill/popup/settings/excluded-domains.component.ts +++ b/apps/browser/src/autofill/popup/settings/excluded-domains.component.ts @@ -15,7 +15,7 @@ import { FormArray, } from "@angular/forms"; import { RouterModule } from "@angular/router"; -import { Subject, takeUntil } from "rxjs"; +import { Observable, Subject, takeUntil } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service"; @@ -35,7 +35,7 @@ import { TypographyModule, } from "@bitwarden/components"; -import { enableAccountSwitching } from "../../../platform/flags"; +import { AccountSwitcherService } from "../../../auth/popup/account-switching/services/account-switcher.service"; import { PopOutComponent } from "../../../platform/popup/components/pop-out.component"; import { PopupFooterComponent } from "../../../platform/popup/layout/popup-footer.component"; import { PopupHeaderComponent } from "../../../platform/popup/layout/popup-header.component"; @@ -74,7 +74,8 @@ export class ExcludedDomainsComponent implements AfterViewInit, OnDestroy { @ViewChildren("uriInput") uriInputElements: QueryList> = new QueryList(); - accountSwitcherEnabled = false; + readonly accountSwitcherEnabled$: Observable = + this.accountSwitcherService.accountSwitchingEnabled$(); dataIsPristine = true; isLoading = false; excludedDomainsState: string[] = []; @@ -95,9 +96,8 @@ export class ExcludedDomainsComponent implements AfterViewInit, OnDestroy { private toastService: ToastService, private formBuilder: FormBuilder, private popupRouterCacheService: PopupRouterCacheService, - ) { - this.accountSwitcherEnabled = enableAccountSwitching(); - } + private accountSwitcherService: AccountSwitcherService, + ) {} get domainForms() { return this.domainListForm.get("domains") as FormArray; diff --git a/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts b/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts index 66a692dbe20..58f3ad11166 100644 --- a/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts +++ b/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts @@ -158,7 +158,7 @@ describe("CollectAutofillContentService", () => { type: "text", value: "", checked: false, - autoCompleteType: "", + autoCompleteType: null, disabled: false, readonly: false, selectInfo: null, @@ -346,7 +346,7 @@ describe("CollectAutofillContentService", () => { type: "text", value: "", checked: false, - autoCompleteType: "", + autoCompleteType: null, disabled: false, readonly: false, selectInfo: null, @@ -379,7 +379,7 @@ describe("CollectAutofillContentService", () => { type: "password", value: "", checked: false, - autoCompleteType: "", + autoCompleteType: null, disabled: false, readonly: false, selectInfo: null, @@ -588,7 +588,7 @@ describe("CollectAutofillContentService", () => { "aria-disabled": false, "aria-haspopup": false, "aria-hidden": false, - autoCompleteType: "", + autoCompleteType: null, checked: false, "data-stripe": null, disabled: false, @@ -621,7 +621,7 @@ describe("CollectAutofillContentService", () => { "aria-disabled": false, "aria-haspopup": false, "aria-hidden": false, - autoCompleteType: "", + autoCompleteType: null, checked: false, "data-stripe": null, disabled: false, @@ -2507,9 +2507,7 @@ describe("CollectAutofillContentService", () => { "class", "tabindex", "title", - "value", "rel", - "tagname", "checked", "disabled", "readonly", 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 117c7c5e2a4..1d464e1313f 100644 --- a/apps/browser/src/autofill/services/collect-autofill-content.service.ts +++ b/apps/browser/src/autofill/services/collect-autofill-content.service.ts @@ -1,5 +1,7 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore +import { AUTOFILL_ATTRIBUTES } from "@bitwarden/common/autofill/constants"; + import AutofillField from "../models/autofill-field"; import AutofillForm from "../models/autofill-form"; import AutofillPageDetails from "../models/autofill-page-details"; @@ -242,10 +244,10 @@ export class CollectAutofillContentService implements CollectAutofillContentServ this._autofillFormElements.set(formElement, { opid: formElement.opid, htmlAction: this.getFormActionAttribute(formElement), - htmlName: this.getPropertyOrAttribute(formElement, "name"), - htmlClass: this.getPropertyOrAttribute(formElement, "class"), - htmlID: this.getPropertyOrAttribute(formElement, "id"), - htmlMethod: this.getPropertyOrAttribute(formElement, "method"), + htmlName: this.getPropertyOrAttribute(formElement, AUTOFILL_ATTRIBUTES.NAME), + htmlClass: this.getPropertyOrAttribute(formElement, AUTOFILL_ATTRIBUTES.CLASS), + htmlID: this.getPropertyOrAttribute(formElement, AUTOFILL_ATTRIBUTES.ID), + htmlMethod: this.getPropertyOrAttribute(formElement, AUTOFILL_ATTRIBUTES.METHOD), }); } @@ -260,7 +262,10 @@ export class CollectAutofillContentService implements CollectAutofillContentServ * @private */ private getFormActionAttribute(element: ElementWithOpId): string { - return new URL(this.getPropertyOrAttribute(element, "action"), globalThis.location.href).href; + return new URL( + this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.ACTION), + globalThis.location.href, + ).href; } /** @@ -335,7 +340,10 @@ export class CollectAutofillContentService implements CollectAutofillContentServ return priorityFormFields; } - const fieldType = this.getPropertyOrAttribute(element, "type")?.toLowerCase(); + const fieldType = this.getPropertyOrAttribute( + element, + AUTOFILL_ATTRIBUTES.TYPE, + )?.toLowerCase(); if (unimportantFieldTypesSet.has(fieldType)) { unimportantFormFields.push(element); continue; @@ -384,11 +392,11 @@ export class CollectAutofillContentService implements CollectAutofillContentServ elementNumber: index, maxLength: this.getAutofillFieldMaxLength(element), viewable: await this.domElementVisibilityService.isElementViewable(element), - htmlID: this.getPropertyOrAttribute(element, "id"), - htmlName: this.getPropertyOrAttribute(element, "name"), - htmlClass: this.getPropertyOrAttribute(element, "class"), - tabindex: this.getPropertyOrAttribute(element, "tabindex"), - title: this.getPropertyOrAttribute(element, "title"), + htmlID: this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.ID), + htmlName: this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.NAME), + htmlClass: this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.CLASS), + tabindex: this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.TABINDEX), + title: this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.TITLE), tagName: this.getAttributeLowerCase(element, "tagName"), dataSetValues: this.getDataSetValues(element), }; @@ -404,16 +412,16 @@ export class CollectAutofillContentService implements CollectAutofillContentServ } let autofillFieldLabels = {}; - const elementType = this.getAttributeLowerCase(element, "type"); + const elementType = this.getAttributeLowerCase(element, AUTOFILL_ATTRIBUTES.TYPE); if (elementType !== "hidden") { autofillFieldLabels = { "label-tag": this.createAutofillFieldLabelTag(element as FillableFormFieldElement), - "label-data": this.getPropertyOrAttribute(element, "data-label"), - "label-aria": this.getPropertyOrAttribute(element, "aria-label"), + "label-data": this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.DATA_LABEL), + "label-aria": this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.ARIA_LABEL), "label-top": this.createAutofillFieldTopLabel(element), "label-right": this.createAutofillFieldRightLabel(element), "label-left": this.createAutofillFieldLeftLabel(element), - placeholder: this.getPropertyOrAttribute(element, "placeholder"), + placeholder: this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.PLACEHOLDER), }; } @@ -421,21 +429,21 @@ export class CollectAutofillContentService implements CollectAutofillContentServ const autofillField = { ...autofillFieldBase, ...autofillFieldLabels, - rel: this.getPropertyOrAttribute(element, "rel"), + rel: this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.REL), type: elementType, value: this.getElementValue(element), - checked: this.getAttributeBoolean(element, "checked"), + checked: this.getAttributeBoolean(element, AUTOFILL_ATTRIBUTES.CHECKED), autoCompleteType: this.getAutoCompleteAttribute(element), - disabled: this.getAttributeBoolean(element, "disabled"), - readonly: this.getAttributeBoolean(element, "readonly"), + disabled: this.getAttributeBoolean(element, AUTOFILL_ATTRIBUTES.DISABLED), + readonly: this.getAttributeBoolean(element, AUTOFILL_ATTRIBUTES.READONLY), selectInfo: elementIsSelectElement(element) ? this.getSelectElementOptions(element as HTMLSelectElement) : null, form: fieldFormElement ? this.getPropertyOrAttribute(fieldFormElement, "opid") : null, - "aria-hidden": this.getAttributeBoolean(element, "aria-hidden", true), - "aria-disabled": this.getAttributeBoolean(element, "aria-disabled", true), - "aria-haspopup": this.getAttributeBoolean(element, "aria-haspopup", true), - "data-stripe": this.getPropertyOrAttribute(element, "data-stripe"), + "aria-hidden": this.getAttributeBoolean(element, AUTOFILL_ATTRIBUTES.ARIA_HIDDEN, true), + "aria-disabled": this.getAttributeBoolean(element, AUTOFILL_ATTRIBUTES.ARIA_DISABLED, true), + "aria-haspopup": this.getAttributeBoolean(element, AUTOFILL_ATTRIBUTES.ARIA_HASPOPUP, true), + "data-stripe": this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.DATA_STRIPE), }; this.cacheAutofillFieldElement(index, element, autofillField); @@ -467,9 +475,9 @@ export class CollectAutofillContentService implements CollectAutofillContentServ */ private getAutoCompleteAttribute(element: ElementWithOpId): string { return ( - this.getPropertyOrAttribute(element, "x-autocompletetype") || - this.getPropertyOrAttribute(element, "autocompletetype") || - this.getPropertyOrAttribute(element, "autocomplete") + this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.AUTOCOMPLETE) || + this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.X_AUTOCOMPLETE_TYPE) || + this.getPropertyOrAttribute(element, AUTOFILL_ATTRIBUTES.AUTOCOMPLETE_TYPE) ); } @@ -957,6 +965,8 @@ export class CollectAutofillContentService implements CollectAutofillContentServ this.mutationObserver = new MutationObserver(this.handleMutationObserverMutation); this.mutationObserver.observe(document.documentElement, { attributes: true, + /** Mutations to node attributes NOT on this list will not be observed! */ + attributeFilter: Object.values(AUTOFILL_ATTRIBUTES), childList: true, subtree: true, }); @@ -1321,6 +1331,7 @@ export class CollectAutofillContentService implements CollectAutofillContentServ action: () => (dataTarget.htmlAction = this.getFormActionAttribute(element)), name: () => updateAttribute("htmlName"), id: () => updateAttribute("htmlID"), + class: () => updateAttribute("htmlClass"), method: () => updateAttribute("htmlMethod"), }; @@ -1350,29 +1361,49 @@ export class CollectAutofillContentService implements CollectAutofillContentServ this.updateAutofillDataAttribute({ element, attributeName, dataTarget, dataTargetKey }); }; const updateActions: Record = { - maxlength: () => (dataTarget.maxLength = this.getAutofillFieldMaxLength(element)), - id: () => updateAttribute("htmlID"), - name: () => updateAttribute("htmlName"), - class: () => updateAttribute("htmlClass"), - tabindex: () => updateAttribute("tabindex"), - title: () => updateAttribute("tabindex"), - rel: () => updateAttribute("rel"), - tagname: () => (dataTarget.tagName = this.getAttributeLowerCase(element, "tagName")), - type: () => (dataTarget.type = this.getAttributeLowerCase(element, "type")), - value: () => (dataTarget.value = this.getElementValue(element)), - checked: () => (dataTarget.checked = this.getAttributeBoolean(element, "checked")), - disabled: () => (dataTarget.disabled = this.getAttributeBoolean(element, "disabled")), - readonly: () => (dataTarget.readonly = this.getAttributeBoolean(element, "readonly")), - autocomplete: () => (dataTarget.autoCompleteType = this.getAutoCompleteAttribute(element)), - "data-label": () => updateAttribute("label-data"), + "aria-describedby": () => updateAttribute(AUTOFILL_ATTRIBUTES.ARIA_DESCRIBEDBY), "aria-label": () => updateAttribute("label-aria"), + "aria-labelledby": () => updateAttribute(AUTOFILL_ATTRIBUTES.ARIA_LABELLEDBY), "aria-hidden": () => - (dataTarget["aria-hidden"] = this.getAttributeBoolean(element, "aria-hidden", true)), + (dataTarget["aria-hidden"] = this.getAttributeBoolean( + element, + AUTOFILL_ATTRIBUTES.ARIA_HIDDEN, + true, + )), "aria-disabled": () => - (dataTarget["aria-disabled"] = this.getAttributeBoolean(element, "aria-disabled", true)), + (dataTarget["aria-disabled"] = this.getAttributeBoolean( + element, + AUTOFILL_ATTRIBUTES.ARIA_DISABLED, + true, + )), "aria-haspopup": () => - (dataTarget["aria-haspopup"] = this.getAttributeBoolean(element, "aria-haspopup", true)), - "data-stripe": () => updateAttribute("data-stripe"), + (dataTarget["aria-haspopup"] = this.getAttributeBoolean( + element, + AUTOFILL_ATTRIBUTES.ARIA_HASPOPUP, + true, + )), + autocomplete: () => (dataTarget.autoCompleteType = this.getAutoCompleteAttribute(element)), + autocompletetype: () => + (dataTarget.autoCompleteType = this.getAutoCompleteAttribute(element)), + "x-autocompletetype": () => + (dataTarget.autoCompleteType = this.getAutoCompleteAttribute(element)), + class: () => updateAttribute("htmlClass"), + checked: () => + (dataTarget.checked = this.getAttributeBoolean(element, AUTOFILL_ATTRIBUTES.CHECKED)), + "data-label": () => updateAttribute("label-data"), + "data-stripe": () => updateAttribute(AUTOFILL_ATTRIBUTES.DATA_STRIPE), + disabled: () => + (dataTarget.disabled = this.getAttributeBoolean(element, AUTOFILL_ATTRIBUTES.DISABLED)), + id: () => updateAttribute("htmlID"), + maxlength: () => (dataTarget.maxLength = this.getAutofillFieldMaxLength(element)), + name: () => updateAttribute("htmlName"), + placeholder: () => updateAttribute(AUTOFILL_ATTRIBUTES.PLACEHOLDER), + readonly: () => + (dataTarget.readonly = this.getAttributeBoolean(element, AUTOFILL_ATTRIBUTES.READONLY)), + rel: () => updateAttribute(AUTOFILL_ATTRIBUTES.REL), + tabindex: () => updateAttribute(AUTOFILL_ATTRIBUTES.TABINDEX), + title: () => updateAttribute(AUTOFILL_ATTRIBUTES.TITLE), + type: () => (dataTarget.type = this.getAttributeLowerCase(element, AUTOFILL_ATTRIBUTES.TYPE)), }; if (!updateActions[attributeName]) { diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 660fcb97bcf..8d741039b31 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -1031,6 +1031,8 @@ export default class MainBackground { this.keyGenerationService, this.sendStateProvider, this.encryptService, + this.cryptoFunctionService, + this.configService, ); this.sendApiService = new SendApiService( this.apiService, diff --git a/apps/browser/src/dirt/phishing-detection/popup/phishing-warning.component.ts b/apps/browser/src/dirt/phishing-detection/popup/phishing-warning.component.ts index d8e9895237c..419de04d9f4 100644 --- a/apps/browser/src/dirt/phishing-detection/popup/phishing-warning.component.ts +++ b/apps/browser/src/dirt/phishing-detection/popup/phishing-warning.component.ts @@ -10,7 +10,7 @@ import { ButtonModule, CheckboxModule, FormFieldModule, - IconModule, + SvgModule, IconTileComponent, LinkModule, CalloutComponent, @@ -31,7 +31,7 @@ import { templateUrl: "phishing-warning.component.html", imports: [ CommonModule, - IconModule, + SvgModule, JslibModule, LinkModule, FormFieldModule, 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 d633c0612f5..0cbb765ce0e 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 @@ -40,6 +40,7 @@ describe("PhishingDataService", () => { // Set default mock behaviors mockIndexedDbService.hasUrl.mockResolvedValue(false); mockIndexedDbService.loadAllUrls.mockResolvedValue([]); + mockIndexedDbService.findMatchingUrl.mockResolvedValue(false); mockIndexedDbService.saveUrls.mockResolvedValue(undefined); mockIndexedDbService.addUrls.mockResolvedValue(undefined); mockIndexedDbService.saveUrlsFromStream.mockResolvedValue(undefined); @@ -90,7 +91,7 @@ describe("PhishingDataService", () => { it("should NOT detect QA test addresses - different subpath", async () => { mockIndexedDbService.hasUrl.mockResolvedValue(false); - mockIndexedDbService.loadAllUrls.mockResolvedValue([]); + mockIndexedDbService.findMatchingUrl.mockResolvedValue(false); const url = new URL("https://phishing.testcategory.com/other"); const result = await service.isPhishingWebAddress(url); @@ -120,83 +121,138 @@ describe("PhishingDataService", () => { expect(result).toBe(true); expect(mockIndexedDbService.hasUrl).toHaveBeenCalledWith("http://phish.com/testing-param"); // Should not fall back to custom matcher when hasUrl returns true - expect(mockIndexedDbService.loadAllUrls).not.toHaveBeenCalled(); + expect(mockIndexedDbService.findMatchingUrl).not.toHaveBeenCalled(); }); - it("should fall back to custom matcher when hasUrl returns false", async () => { + it("should return false when hasUrl returns false (custom matcher disabled)", async () => { // Mock hasUrl to return false (no direct href match) mockIndexedDbService.hasUrl.mockResolvedValue(false); - // Mock loadAllUrls to return phishing URLs for custom matcher - mockIndexedDbService.loadAllUrls.mockResolvedValue(["http://phish.com/path"]); const url = new URL("http://phish.com/path"); const result = await service.isPhishingWebAddress(url); - expect(result).toBe(true); + // Custom matcher is currently disabled (useCustomMatcher: false), so result is false + expect(result).toBe(false); expect(mockIndexedDbService.hasUrl).toHaveBeenCalledWith("http://phish.com/path"); - expect(mockIndexedDbService.loadAllUrls).toHaveBeenCalled(); + // Custom matcher should NOT be called since it's disabled + expect(mockIndexedDbService.findMatchingUrl).not.toHaveBeenCalled(); }); it("should not detect a safe web address", async () => { // Mock hasUrl to return false mockIndexedDbService.hasUrl.mockResolvedValue(false); - // Mock loadAllUrls to return phishing URLs that don't match - mockIndexedDbService.loadAllUrls.mockResolvedValue(["http://phish.com", "http://badguy.net"]); const url = new URL("http://safe.com"); const result = await service.isPhishingWebAddress(url); expect(result).toBe(false); expect(mockIndexedDbService.hasUrl).toHaveBeenCalledWith("http://safe.com/"); - expect(mockIndexedDbService.loadAllUrls).toHaveBeenCalled(); + // Custom matcher is disabled, so findMatchingUrl should NOT be called + expect(mockIndexedDbService.findMatchingUrl).not.toHaveBeenCalled(); }); - it("should not match against root web address with subpaths using custom matcher", async () => { + it("should not match against root web address with subpaths (custom matcher disabled)", async () => { // Mock hasUrl to return false (no direct href match) mockIndexedDbService.hasUrl.mockResolvedValue(false); - // Mock loadAllUrls to return entry that matches with subpath - mockIndexedDbService.loadAllUrls.mockResolvedValue(["http://phish.com/login"]); const url = new URL("http://phish.com/login/page"); const result = await service.isPhishingWebAddress(url); expect(result).toBe(false); expect(mockIndexedDbService.hasUrl).toHaveBeenCalledWith("http://phish.com/login/page"); - expect(mockIndexedDbService.loadAllUrls).toHaveBeenCalled(); + // Custom matcher is disabled, so findMatchingUrl should NOT be called + expect(mockIndexedDbService.findMatchingUrl).not.toHaveBeenCalled(); }); - it("should not match against root web address with different subpaths using custom matcher", async () => { + it("should not match against root web address with different subpaths (custom matcher disabled)", async () => { // Mock hasUrl to return false (no direct hostname match) mockIndexedDbService.hasUrl.mockResolvedValue(false); - // Mock loadAllUrls to return entry that matches with subpath - mockIndexedDbService.loadAllUrls.mockResolvedValue(["http://phish.com/login/page1"]); const url = new URL("http://phish.com/login/page2"); const result = await service.isPhishingWebAddress(url); expect(result).toBe(false); expect(mockIndexedDbService.hasUrl).toHaveBeenCalledWith("http://phish.com/login/page2"); - expect(mockIndexedDbService.loadAllUrls).toHaveBeenCalled(); + // Custom matcher is disabled, so findMatchingUrl should NOT be called + expect(mockIndexedDbService.findMatchingUrl).not.toHaveBeenCalled(); }); it("should handle IndexedDB errors gracefully", async () => { // Mock hasUrl to throw error mockIndexedDbService.hasUrl.mockRejectedValue(new Error("hasUrl error")); - // Mock loadAllUrls to also throw error - mockIndexedDbService.loadAllUrls.mockRejectedValue(new Error("IndexedDB error")); const url = new URL("http://phish.com/about"); const result = await service.isPhishingWebAddress(url); expect(result).toBe(false); expect(logService.error).toHaveBeenCalledWith( - "[PhishingDataService] IndexedDB lookup via hasUrl failed", - expect.any(Error), - ); - expect(logService.error).toHaveBeenCalledWith( - "[PhishingDataService] Error running custom matcher", + "[PhishingDataService] IndexedDB lookup failed", expect.any(Error), ); + // Custom matcher is disabled, so no custom matcher error is expected + expect(mockIndexedDbService.findMatchingUrl).not.toHaveBeenCalled(); + }); + + it("should use cursor-based search when useCustomMatcher is enabled", async () => { + // Temporarily enable custom matcher for this test + const originalValue = (PhishingDataService as any).USE_CUSTOM_MATCHER; + (PhishingDataService as any).USE_CUSTOM_MATCHER = true; + + try { + // Mock hasUrl to return false (no direct match) + mockIndexedDbService.hasUrl.mockResolvedValue(false); + // Mock findMatchingUrl to return true (custom matcher finds it) + mockIndexedDbService.findMatchingUrl.mockResolvedValue(true); + + const url = new URL("http://phish.com/path"); + const result = await service.isPhishingWebAddress(url); + + expect(result).toBe(true); + expect(mockIndexedDbService.hasUrl).toHaveBeenCalled(); + expect(mockIndexedDbService.findMatchingUrl).toHaveBeenCalled(); + } finally { + // Restore original value + (PhishingDataService as any).USE_CUSTOM_MATCHER = originalValue; + } + }); + + it("should return false when custom matcher finds no match (when enabled)", async () => { + const originalValue = (PhishingDataService as any).USE_CUSTOM_MATCHER; + (PhishingDataService as any).USE_CUSTOM_MATCHER = true; + + try { + mockIndexedDbService.hasUrl.mockResolvedValue(false); + mockIndexedDbService.findMatchingUrl.mockResolvedValue(false); + + const url = new URL("http://safe.com/path"); + const result = await service.isPhishingWebAddress(url); + + expect(result).toBe(false); + expect(mockIndexedDbService.findMatchingUrl).toHaveBeenCalled(); + } finally { + (PhishingDataService as any).USE_CUSTOM_MATCHER = originalValue; + } + }); + + it("should handle custom matcher errors gracefully (when enabled)", async () => { + const originalValue = (PhishingDataService as any).USE_CUSTOM_MATCHER; + (PhishingDataService as any).USE_CUSTOM_MATCHER = true; + + try { + mockIndexedDbService.hasUrl.mockResolvedValue(false); + mockIndexedDbService.findMatchingUrl.mockRejectedValue(new Error("Cursor error")); + + const url = new URL("http://error.com/path"); + const result = await service.isPhishingWebAddress(url); + + expect(result).toBe(false); + expect(logService.error).toHaveBeenCalledWith( + "[PhishingDataService] Custom matcher failed", + expect.any(Error), + ); + } finally { + (PhishingDataService as any).USE_CUSTOM_MATCHER = originalValue; + } }); }); 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 10268fa7f93..03759ba14bc 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 @@ -78,6 +78,10 @@ export const PHISHING_DOMAINS_BLOB_KEY = new KeyDefinition( /** Coordinates fetching, caching, and patching of known phishing web addresses */ export class PhishingDataService { + // Cursor-based search is disabled due to performance (6+ minutes on large databases) + // Enable when performance is optimized via indexing or other improvements + private static readonly USE_CUSTOM_MATCHER = false; + // While background scripts do not necessarily need destroying, // processes in PhishingDataService are memory intensive. // We are adding the destroy to guard against accidental leaks. @@ -153,8 +157,14 @@ export class PhishingDataService { * @returns True if the URL is a known phishing web address, false otherwise */ async isPhishingWebAddress(url: URL): Promise { + // Skip non-http(s) protocols - phishing database only contains web URLs + if (url.protocol !== "http:" && url.protocol !== "https:") { + return false; + } + // Quick check for QA/dev test addresses if (this._testWebAddresses.includes(url.href)) { + this.logService.info("[PhishingDataService] Found test web address: " + url.href); return true; } @@ -162,28 +172,41 @@ export class PhishingDataService { try { // Quick lookup: check direct presence of href in IndexedDB - const hasUrl = await this.indexedDbService.hasUrl(url.href); + // Also check without trailing slash since browsers add it but DB entries may not have it + const urlHref = url.href; + const urlWithoutTrailingSlash = urlHref.endsWith("/") ? urlHref.slice(0, -1) : null; + + let hasUrl = await this.indexedDbService.hasUrl(urlHref); + + if (!hasUrl && urlWithoutTrailingSlash) { + hasUrl = await this.indexedDbService.hasUrl(urlWithoutTrailingSlash); + } + if (hasUrl) { + this.logService.info("[PhishingDataService] Found phishing URL: " + urlHref); return true; } } catch (err) { - this.logService.error("[PhishingDataService] IndexedDB lookup via hasUrl failed", err); + this.logService.error("[PhishingDataService] IndexedDB lookup failed", err); } - // If a custom matcher is provided, iterate stored entries and apply the matcher. - if (resource && resource.match) { + // Custom matcher is disabled for performance (see USE_CUSTOM_MATCHER) + if (resource && resource.match && PhishingDataService.USE_CUSTOM_MATCHER) { try { - const entries = await this.indexedDbService.loadAllUrls(); - for (const entry of entries) { - if (resource.match(url, entry)) { - return true; - } + const found = await this.indexedDbService.findMatchingUrl((entry) => + resource.match(url, entry), + ); + + if (found) { + this.logService.info("[PhishingDataService] Found phishing URL via matcher: " + url.href); } + return found; } catch (err) { - this.logService.error("[PhishingDataService] Error running custom matcher", err); + this.logService.error("[PhishingDataService] Custom matcher failed", err); + return false; } - return false; } + return false; } 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 815007e1d4c..2fa7bf8ec9e 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 @@ -1,14 +1,4 @@ -import { - concatMap, - distinctUntilChanged, - EMPTY, - filter, - map, - merge, - Subject, - switchMap, - tap, -} from "rxjs"; +import { distinctUntilChanged, EMPTY, filter, map, merge, Subject, switchMap, tap } from "rxjs"; import { PhishingDetectionSettingsServiceAbstraction } from "@bitwarden/common/dirt/services/abstractions/phishing-detection-settings.service.abstraction"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -63,7 +53,7 @@ export class PhishingDetectionService { tap((message) => logService.debug(`[PhishingDetectionService] user selected continue for ${message.url}`), ), - concatMap(async (message) => { + switchMap(async (message) => { const url = new URL(message.url); this._ignoredHostnames.add(url.hostname); await BrowserApi.navigateTabToUrl(message.tabId, url); @@ -88,7 +78,9 @@ export class PhishingDetectionService { prev.ignored === curr.ignored, ), tap((event) => logService.debug(`[PhishingDetectionService] processing event:`, event)), - concatMap(async ({ tabId, url, ignored }) => { + // Use switchMap to cancel any in-progress check when navigating to a new URL + // This prevents race conditions where a stale check redirects the user incorrectly + switchMap(async ({ tabId, url, ignored }) => { if (ignored) { // The next time this host is visited, block again this._ignoredHostnames.delete(url.hostname); diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.spec.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.spec.ts index 99e101cc199..98835a5b366 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.spec.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.spec.ts @@ -435,6 +435,89 @@ describe("PhishingIndexedDbService", () => { }); }); + describe("findMatchingUrl", () => { + it("returns true when matcher finds a match", async () => { + mockStore.set("https://example.com", { url: "https://example.com" }); + mockStore.set("https://phishing.net", { url: "https://phishing.net" }); + mockStore.set("https://test.org", { url: "https://test.org" }); + + const matcher = (url: string) => url.includes("phishing"); + const result = await service.findMatchingUrl(matcher); + + expect(result).toBe(true); + expect(mockDb.transaction).toHaveBeenCalledWith("phishing-urls", "readonly"); + expect(mockObjectStore.openCursor).toHaveBeenCalled(); + }); + + it("returns false when no URLs match", async () => { + mockStore.set("https://example.com", { url: "https://example.com" }); + mockStore.set("https://test.org", { url: "https://test.org" }); + + const matcher = (url: string) => url.includes("notfound"); + const result = await service.findMatchingUrl(matcher); + + expect(result).toBe(false); + }); + + it("returns false when store is empty", async () => { + const matcher = (url: string) => url.includes("anything"); + const result = await service.findMatchingUrl(matcher); + + expect(result).toBe(false); + }); + + it("exits early on first match without iterating all records", async () => { + mockStore.set("https://match1.com", { url: "https://match1.com" }); + mockStore.set("https://match2.com", { url: "https://match2.com" }); + mockStore.set("https://match3.com", { url: "https://match3.com" }); + + const matcherCallCount = jest + .fn() + .mockImplementation((url: string) => url.includes("match2")); + await service.findMatchingUrl(matcherCallCount); + + // Matcher should be called for match1.com and match2.com, but NOT match3.com + // because it exits early on first match + expect(matcherCallCount).toHaveBeenCalledWith("https://match1.com"); + expect(matcherCallCount).toHaveBeenCalledWith("https://match2.com"); + expect(matcherCallCount).not.toHaveBeenCalledWith("https://match3.com"); + expect(matcherCallCount).toHaveBeenCalledTimes(2); + }); + + it("supports complex matcher logic", async () => { + mockStore.set("https://example.com/path", { url: "https://example.com/path" }); + mockStore.set("https://test.org", { url: "https://test.org" }); + mockStore.set("https://phishing.net/login", { url: "https://phishing.net/login" }); + + const matcher = (url: string) => { + return url.includes("phishing") && url.includes("login"); + }; + const result = await service.findMatchingUrl(matcher); + + expect(result).toBe(true); + }); + + it("returns false on error", async () => { + const error = new Error("IndexedDB error"); + mockOpenRequest.error = error; + (global.indexedDB.open as jest.Mock).mockImplementation(() => { + setTimeout(() => { + mockOpenRequest.onerror?.(); + }, 0); + return mockOpenRequest; + }); + + const matcher = (url: string) => url.includes("test"); + const result = await service.findMatchingUrl(matcher); + + expect(result).toBe(false); + expect(logService.error).toHaveBeenCalledWith( + "[PhishingIndexedDbService] Cursor search failed", + expect.any(Error), + ); + }); + }); + describe("database initialization", () => { it("creates object store with keyPath on upgrade", async () => { mockDb.objectStoreNames.contains.mockReturnValue(false); diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.ts index fe0f10da221..ea4b7987607 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.ts @@ -195,6 +195,60 @@ export class PhishingIndexedDbService { }); } + /** + * Checks if any URL in the database matches the given matcher function. + * Uses a cursor to iterate through records without loading all into memory. + * Returns immediately on first match for optimal performance. + * + * @param matcher - Function that tests each URL and returns true if it matches + * @returns `true` if any URL matches, `false` if none match or on error + */ + async findMatchingUrl(matcher: (url: string) => boolean): Promise { + this.logService.debug("[PhishingIndexedDbService] Searching for matching URL with cursor..."); + + let db: IDBDatabase | null = null; + try { + db = await this.openDatabase(); + return await this.cursorSearch(db, matcher); + } catch (error) { + this.logService.error("[PhishingIndexedDbService] Cursor search failed", error); + return false; + } finally { + db?.close(); + } + } + + /** + * Performs cursor-based search through all URLs. + * Tests each URL with the matcher without accumulating records in memory. + */ + private cursorSearch(db: IDBDatabase, matcher: (url: string) => boolean): Promise { + return new Promise((resolve, reject) => { + const req = db + .transaction(this.STORE_NAME, "readonly") + .objectStore(this.STORE_NAME) + .openCursor(); + req.onerror = () => reject(req.error); + req.onsuccess = (e) => { + const cursor = (e.target as IDBRequest).result; + if (cursor) { + const url = (cursor.value as PhishingUrlRecord).url; + // Test the URL immediately without accumulating in memory + if (matcher(url)) { + // Found a match + resolve(true); + return; + } + // No match, continue to next record + cursor.continue(); + } else { + // Reached end of records without finding a match + resolve(false); + } + }; + }); + } + /** * Saves phishing URLs directly from a stream. * Processes data incrementally to minimize memory usage. diff --git a/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.ts b/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.ts index c8be58b0bde..d7e755b34ea 100644 --- a/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.ts +++ b/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.ts @@ -35,7 +35,7 @@ export class BackgroundBrowserBiometricsService extends BiometricsService { super(); // Always connect to the native messaging background if biometrics are enabled, not just when it is used // so that there is no wait when used. - const biometricsEnabled = this.biometricStateService.biometricUnlockEnabled$; + const biometricsEnabled = this.biometricStateService.biometricUnlockEnabled$(); combineLatest([timer(0, this.BACKGROUND_POLLING_INTERVAL), biometricsEnabled]) .pipe( diff --git a/apps/browser/src/key-management/lock/services/extension-lock-component.service.spec.ts b/apps/browser/src/key-management/lock/services/extension-lock-component.service.spec.ts index ecdb899b9a7..934fb9307ee 100644 --- a/apps/browser/src/key-management/lock/services/extension-lock-component.service.spec.ts +++ b/apps/browser/src/key-management/lock/services/extension-lock-component.service.spec.ts @@ -375,7 +375,7 @@ describe("ExtensionLockComponentService", () => { platformUtilsService.supportsSecureStorage.mockReturnValue( mockInputs.platformSupportsSecureStorage, ); - biometricStateService.biometricUnlockEnabled$ = of(true); + biometricStateService.biometricUnlockEnabled$.mockReturnValue(of(true)); // PIN pinService.isPinDecryptionAvailable.mockResolvedValue(mockInputs.pinDecryptionAvailable); @@ -386,6 +386,7 @@ describe("ExtensionLockComponentService", () => { const unlockOptions = await firstValueFrom(service.getAvailableUnlockOptions$(userId)); expect(unlockOptions).toEqual(expectedOutput); + expect(biometricStateService.biometricUnlockEnabled$).toHaveBeenCalledWith(userId); }); }); }); diff --git a/apps/browser/src/key-management/lock/services/extension-lock-component.service.ts b/apps/browser/src/key-management/lock/services/extension-lock-component.service.ts index 5e6e564bbc2..1ed9d1ea967 100644 --- a/apps/browser/src/key-management/lock/services/extension-lock-component.service.ts +++ b/apps/browser/src/key-management/lock/services/extension-lock-component.service.ts @@ -69,7 +69,7 @@ export class ExtensionLockComponentService implements LockComponentService { return combineLatest([ // Note: defer is preferable b/c it delays the execution of the function until the observable is subscribed to defer(async () => { - if (!(await firstValueFrom(this.biometricStateService.biometricUnlockEnabled$))) { + if (!(await firstValueFrom(this.biometricStateService.biometricUnlockEnabled$(userId)))) { return BiometricsStatus.NotEnabledLocally; } else { // TODO remove after 2025.3 diff --git a/apps/browser/src/platform/browser/browser-popup-utils.spec.ts b/apps/browser/src/platform/browser/browser-popup-utils.spec.ts index cb04f30b589..89459523843 100644 --- a/apps/browser/src/platform/browser/browser-popup-utils.spec.ts +++ b/apps/browser/src/platform/browser/browser-popup-utils.spec.ts @@ -140,11 +140,6 @@ describe("BrowserPopupUtils", () => { describe("openPopout", () => { beforeEach(() => { - jest.spyOn(BrowserApi, "getPlatformInfo").mockResolvedValueOnce({ - os: "linux", - arch: "x86-64", - nacl_arch: "x86-64", - }); jest.spyOn(BrowserApi, "getWindow").mockResolvedValueOnce({ id: 1, left: 100, @@ -155,8 +150,6 @@ describe("BrowserPopupUtils", () => { width: PopupWidthOptions.default, }); jest.spyOn(BrowserApi, "createWindow").mockImplementation(); - jest.spyOn(BrowserApi, "updateWindowProperties").mockImplementation(); - jest.spyOn(BrowserApi, "getPlatformInfo").mockImplementation(); }); it("creates a window with the default window options", async () => { @@ -274,63 +267,6 @@ describe("BrowserPopupUtils", () => { url: `chrome-extension://id/${url}?uilocation=popout&singleActionPopout=123`, }); }); - - it("exits fullscreen and focuses popout window if the current window is fullscreen and platform is mac", async () => { - const url = "popup/index.html"; - jest.spyOn(BrowserPopupUtils as any, "isSingleActionPopoutOpen").mockResolvedValueOnce(false); - jest.spyOn(BrowserApi, "getPlatformInfo").mockReset().mockResolvedValueOnce({ - os: "mac", - arch: "x86-64", - nacl_arch: "x86-64", - }); - jest.spyOn(BrowserApi, "getWindow").mockReset().mockResolvedValueOnce({ - id: 1, - left: 100, - top: 100, - focused: false, - alwaysOnTop: false, - incognito: false, - width: PopupWidthOptions.default, - state: "fullscreen", - }); - jest - .spyOn(BrowserApi, "createWindow") - .mockResolvedValueOnce({ id: 2 } as chrome.windows.Window); - - await BrowserPopupUtils.openPopout(url, { senderWindowId: 1 }); - expect(BrowserApi.updateWindowProperties).toHaveBeenCalledWith(1, { - state: "maximized", - }); - expect(BrowserApi.updateWindowProperties).toHaveBeenCalledWith(2, { - focused: true, - }); - }); - - it("doesnt exit fullscreen if the platform is not mac", async () => { - const url = "popup/index.html"; - jest.spyOn(BrowserPopupUtils as any, "isSingleActionPopoutOpen").mockResolvedValueOnce(false); - jest.spyOn(BrowserApi, "getPlatformInfo").mockReset().mockResolvedValueOnce({ - os: "win", - arch: "x86-64", - nacl_arch: "x86-64", - }); - jest.spyOn(BrowserApi, "getWindow").mockResolvedValueOnce({ - id: 1, - left: 100, - top: 100, - focused: false, - alwaysOnTop: false, - incognito: false, - width: PopupWidthOptions.default, - state: "fullscreen", - }); - - await BrowserPopupUtils.openPopout(url); - - expect(BrowserApi.updateWindowProperties).not.toHaveBeenCalledWith(1, { - state: "maximized", - }); - }); }); describe("openCurrentPagePopout", () => { diff --git a/apps/browser/src/platform/browser/browser-popup-utils.ts b/apps/browser/src/platform/browser/browser-popup-utils.ts index c8dba57e708..7333023d178 100644 --- a/apps/browser/src/platform/browser/browser-popup-utils.ts +++ b/apps/browser/src/platform/browser/browser-popup-utils.ts @@ -168,29 +168,8 @@ export default class BrowserPopupUtils { ) { return; } - const platform = await BrowserApi.getPlatformInfo(); - const isMacOS = platform.os === "mac"; - const isFullscreen = senderWindow.state === "fullscreen"; - const isFullscreenAndMacOS = isFullscreen && isMacOS; - //macOS specific handling for improved UX when sender in fullscreen aka green button; - if (isFullscreenAndMacOS) { - await BrowserApi.updateWindowProperties(senderWindow.id, { - state: "maximized", - }); - //wait for macOS animation to finish - await new Promise((resolve) => setTimeout(resolve, 1000)); - } - - const newWindow = await BrowserApi.createWindow(popoutWindowOptions); - - if (isFullscreenAndMacOS) { - await BrowserApi.updateWindowProperties(newWindow.id, { - focused: true, - }); - } - - return newWindow; + return await BrowserApi.createWindow(popoutWindowOptions); } /** diff --git a/apps/browser/src/platform/flags.ts b/apps/browser/src/platform/flags.ts index 2b1040bcd8a..30441d42979 100644 --- a/apps/browser/src/platform/flags.ts +++ b/apps/browser/src/platform/flags.ts @@ -8,12 +8,8 @@ import { import { GroupPolicyEnvironment } from "../admin-console/types/group-policy-environment"; -import { BrowserApi } from "./browser/browser-api"; - // required to avoid linting errors when there are no flags -export type Flags = { - accountSwitching?: boolean; -} & SharedFlags; +export type Flags = SharedFlags; // required to avoid linting errors when there are no flags export type DevFlags = { @@ -31,14 +27,3 @@ export function devFlagEnabled(flag: keyof DevFlags) { export function devFlagValue(flag: keyof DevFlags) { return baseDevFlagValue(flag); } - -/** Helper method to sync flag specifically for account switching, which as platform-based values. - * If this pattern needs to be repeated, it's better handled by increasing complexity of webpack configurations - * Not by expanding these flag getters. - */ -export function enableAccountSwitching(): boolean { - if (BrowserApi.isSafariApi) { - return false; - } - return flagEnabled("accountSwitching"); -} diff --git a/apps/browser/src/platform/ipc/ipc-content-script-manager.service.ts b/apps/browser/src/platform/ipc/ipc-content-script-manager.service.ts index e5fe95e2018..d53347b9dce 100644 --- a/apps/browser/src/platform/ipc/ipc-content-script-manager.service.ts +++ b/apps/browser/src/platform/ipc/ipc-content-script-manager.service.ts @@ -15,7 +15,7 @@ export class IpcContentScriptManagerService { } configService - .getFeatureFlag$(FeatureFlag.IpcChannelFramework) + .getFeatureFlag$(FeatureFlag.ContentScriptIpcChannelFramework) .pipe( mergeMap(async (enabled) => { if (!enabled) { diff --git a/apps/browser/src/platform/popup/layout/popup-tab-navigation.component.html b/apps/browser/src/platform/popup/layout/popup-tab-navigation.component.html index bce2b5033ae..e04d302ea2c 100644 --- a/apps/browser/src/platform/popup/layout/popup-tab-navigation.component.html +++ b/apps/browser/src/platform/popup/layout/popup-tab-navigation.component.html @@ -18,11 +18,11 @@ type="button" role="link" > - + > {{ button.label | i18n }} diff --git a/apps/browser/src/platform/popup/layout/popup-tab-navigation.component.ts b/apps/browser/src/platform/popup/layout/popup-tab-navigation.component.ts index 26138d57954..5a40b72daff 100644 --- a/apps/browser/src/platform/popup/layout/popup-tab-navigation.component.ts +++ b/apps/browser/src/platform/popup/layout/popup-tab-navigation.component.ts @@ -3,15 +3,15 @@ import { Component, Input } from "@angular/core"; import { RouterModule } from "@angular/router"; import { JslibModule } from "@bitwarden/angular/jslib.module"; -import { Icon } from "@bitwarden/assets/svg"; +import { BitSvg } from "@bitwarden/assets/svg"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { IconModule, LinkModule } from "@bitwarden/components"; +import { SvgModule, LinkModule } from "@bitwarden/components"; export type NavButton = { label: string; page: string; - icon: Icon; - iconActive: Icon; + icon: BitSvg; + iconActive: BitSvg; showBerry?: boolean; }; @@ -20,7 +20,7 @@ export type NavButton = { @Component({ selector: "popup-tab-navigation", templateUrl: "popup-tab-navigation.component.html", - imports: [CommonModule, LinkModule, RouterModule, JslibModule, IconModule], + imports: [CommonModule, LinkModule, RouterModule, JslibModule, SvgModule], host: { class: "tw-block tw-size-full tw-flex tw-flex-col", }, diff --git a/apps/browser/src/popup/components/extension-anon-layout-wrapper/extension-anon-layout-wrapper.component.html b/apps/browser/src/popup/components/extension-anon-layout-wrapper/extension-anon-layout-wrapper.component.html index 484f9680519..2cf1998bb05 100644 --- a/apps/browser/src/popup/components/extension-anon-layout-wrapper/extension-anon-layout-wrapper.component.html +++ b/apps/browser/src/popup/components/extension-anon-layout-wrapper/extension-anon-layout-wrapper.component.html @@ -6,7 +6,7 @@ [pageTitle]="''" >
- +
diff --git a/apps/browser/src/popup/components/extension-anon-layout-wrapper/extension-anon-layout-wrapper.component.ts b/apps/browser/src/popup/components/extension-anon-layout-wrapper/extension-anon-layout-wrapper.component.ts index 3a50f03e982..e07e9c50554 100644 --- a/apps/browser/src/popup/components/extension-anon-layout-wrapper/extension-anon-layout-wrapper.component.ts +++ b/apps/browser/src/popup/components/extension-anon-layout-wrapper/extension-anon-layout-wrapper.component.ts @@ -5,10 +5,10 @@ import { Component, OnDestroy, OnInit } from "@angular/core"; import { ActivatedRoute, Data, NavigationEnd, Router, RouterModule } from "@angular/router"; import { Subject, filter, switchMap, takeUntil, tap } from "rxjs"; -import { BitwardenLogo, Icon } from "@bitwarden/assets/svg"; +import { BitwardenLogo, BitSvg } from "@bitwarden/assets/svg"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { - IconModule, + SvgModule, Translation, AnonLayoutComponent, AnonLayoutWrapperData, @@ -38,7 +38,7 @@ export interface ExtensionAnonLayoutWrapperData extends AnonLayoutWrapperData { CommonModule, CurrentAccountComponent, I18nPipe, - IconModule, + SvgModule, PopOutComponent, PopupPageComponent, PopupHeaderComponent, @@ -54,7 +54,7 @@ export class ExtensionAnonLayoutWrapperComponent implements OnInit, OnDestroy { protected pageTitle: string; protected pageSubtitle: string; - protected pageIcon: Icon; + protected pageIcon: BitSvg; protected showReadonlyHostname: boolean; protected maxWidth: "md" | "3xl"; protected hasLoggedInAccount: boolean = false; diff --git a/apps/browser/src/tools/popup/send-v2/send-created/send-created.component.html b/apps/browser/src/tools/popup/send-v2/send-created/send-created.component.html index 828c1667c57..94c1df46eea 100644 --- a/apps/browser/src/tools/popup/send-v2/send-created/send-created.component.html +++ b/apps/browser/src/tools/popup/send-v2/send-created/send-created.component.html @@ -14,7 +14,7 @@ class="tw-flex tw-bg-background-alt tw-flex-col tw-justify-center tw-items-center tw-gap-2 tw-h-full tw-px-5" >
- +

{{ "createdSendSuccessfully" | i18n }} diff --git a/apps/browser/src/tools/popup/send-v2/send-created/send-created.component.spec.ts b/apps/browser/src/tools/popup/send-v2/send-created/send-created.component.spec.ts index 521d72bba0c..a19897b6bbc 100644 --- a/apps/browser/src/tools/popup/send-v2/send-created/send-created.component.spec.ts +++ b/apps/browser/src/tools/popup/send-v2/send-created/send-created.component.spec.ts @@ -14,7 +14,7 @@ import { SelfHostedEnvironment } from "@bitwarden/common/platform/services/defau import { SendView } from "@bitwarden/common/tools/send/models/view/send.view"; import { SendService } from "@bitwarden/common/tools/send/services/send.service.abstraction"; import { SendType } from "@bitwarden/common/tools/send/types/send-type"; -import { ButtonModule, I18nMockService, IconModule, ToastService } from "@bitwarden/components"; +import { ButtonModule, I18nMockService, SvgModule, ToastService } from "@bitwarden/components"; import { PopOutComponent } from "../../../../platform/popup/components/pop-out.component"; import { PopupFooterComponent } from "../../../../platform/popup/layout/popup-footer.component"; @@ -76,7 +76,7 @@ describe("SendCreatedComponent", () => { RouterTestingModule, JslibModule, ButtonModule, - IconModule, + SvgModule, PopOutComponent, PopupHeaderComponent, PopupPageComponent, diff --git a/apps/browser/src/tools/popup/send-v2/send-created/send-created.component.ts b/apps/browser/src/tools/popup/send-v2/send-created/send-created.component.ts index e9109ec6c21..e3717075e24 100644 --- a/apps/browser/src/tools/popup/send-v2/send-created/send-created.component.ts +++ b/apps/browser/src/tools/popup/send-v2/send-created/send-created.component.ts @@ -13,7 +13,7 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { SendView } from "@bitwarden/common/tools/send/models/view/send.view"; import { SendService } from "@bitwarden/common/tools/send/services/send.service.abstraction"; -import { ButtonModule, IconModule, ToastService } from "@bitwarden/components"; +import { ButtonModule, SvgModule, ToastService } from "@bitwarden/components"; import { PopOutComponent } from "../../../../platform/popup/components/pop-out.component"; import { PopupFooterComponent } from "../../../../platform/popup/layout/popup-footer.component"; @@ -34,7 +34,7 @@ import { PopupPageComponent } from "../../../../platform/popup/layout/popup-page PopupPageComponent, RouterModule, PopupFooterComponent, - IconModule, + SvgModule, ], }) export class SendCreatedComponent { diff --git a/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.html b/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.html index 47ef0284d6a..0e9a12b504e 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.html @@ -5,8 +5,7 @@ [showRefresh]="showRefresh" (onRefresh)="refreshCurrentTab()" [description]="(showEmptyAutofillTip$ | async) ? ('autofillSuggestionsTip' | i18n) : undefined" - showAutofillButton + isAutofillList [disableDescriptionMargin]="showEmptyAutofillTip$ | async" - [primaryActionAutofill]="clickItemsToAutofillVaultView$ | async" [groupByType]="groupByType()" > diff --git a/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.ts b/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.ts index 64f662ab840..adb4e6b6866 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.ts @@ -1,7 +1,7 @@ import { CommonModule } from "@angular/common"; import { Component } from "@angular/core"; import { toSignal } from "@angular/core/rxjs-interop"; -import { combineLatest, map, Observable, startWith } from "rxjs"; +import { combineLatest, map, Observable } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { VaultSettingsService } from "@bitwarden/common/vault/abstractions/vault-settings/vault-settings.service"; @@ -42,12 +42,6 @@ export class AutofillVaultListItemsComponent { */ protected showRefresh: boolean = BrowserPopupUtils.inSidebar(window); - /** Flag indicating whether the login item should automatically autofill when clicked */ - protected clickItemsToAutofillVaultView$: Observable = - this.vaultSettingsService.clickItemsToAutofillVaultView$.pipe( - startWith(true), // Start with true to avoid flashing the fill button on first load - ); - protected readonly groupByType = toSignal( this.vaultPopupItemsService.hasFilterApplied$.pipe(map((hasFilter) => !hasFilter)), ); diff --git a/apps/browser/src/vault/popup/components/vault-v2/intro-carousel/intro-carousel.component.html b/apps/browser/src/vault/popup/components/vault-v2/intro-carousel/intro-carousel.component.html index 5f19092d6b0..1980e8aa356 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/intro-carousel/intro-carousel.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/intro-carousel/intro-carousel.component.html @@ -2,7 +2,7 @@
- +

{{ "securityPrioritized" | i18n }}

{{ "securityPrioritizedBody" | i18n }}

@@ -11,7 +11,7 @@
- +

{{ "quickLogin" | i18n }}

{{ "quickLoginBody" | i18n }}

@@ -20,7 +20,7 @@
- +

{{ "secureUser" | i18n }}

{{ "secureUserBody" | i18n }}

@@ -29,7 +29,7 @@
- +

{{ "secureDevices" | i18n }}

{{ "secureDevicesBody" | i18n }}

diff --git a/apps/browser/src/vault/popup/components/vault-v2/intro-carousel/intro-carousel.component.ts b/apps/browser/src/vault/popup/components/vault-v2/intro-carousel/intro-carousel.component.ts index 48c8f5682bc..5ad44c2f545 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/intro-carousel/intro-carousel.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/intro-carousel/intro-carousel.component.ts @@ -3,7 +3,7 @@ import { Router } from "@angular/router"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { ItemTypes, LoginCards, NoCredentialsIcon, DevicesIcon } from "@bitwarden/assets/svg"; -import { ButtonModule, DialogModule, IconModule, TypographyModule } from "@bitwarden/components"; +import { ButtonModule, DialogModule, SvgModule, TypographyModule } from "@bitwarden/components"; import { I18nPipe } from "@bitwarden/ui-common"; import { VaultCarouselModule } from "@bitwarden/vault"; @@ -17,7 +17,7 @@ import { IntroCarouselService } from "../../../services/intro-carousel.service"; imports: [ VaultCarouselModule, ButtonModule, - IconModule, + SvgModule, DialogModule, TypographyModule, JslibModule, diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html index be67869d3df..223029aed0a 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html @@ -8,14 +8,14 @@ > @if (!decryptionFailure) { - + - + diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts index d7de51ad20f..7a6c1db8026 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts @@ -1,5 +1,5 @@ import { CommonModule } from "@angular/common"; -import { booleanAttribute, Component, Input } from "@angular/core"; +import { booleanAttribute, Component, input, Input } from "@angular/core"; import { Router, RouterModule } from "@angular/router"; import { BehaviorSubject, combineLatest, firstValueFrom, map, Observable, switchMap } from "rxjs"; import { filter } from "rxjs/operators"; @@ -76,22 +76,10 @@ export class ItemMoreOptionsComponent { } /** - * Flag to show view item menu option. Used when something else is - * assigned as the primary action for the item, such as autofill. - */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input({ transform: booleanAttribute }) - showViewOption = false; - - /** - * Flag to hide the autofill menu options. Used for items that are + * Flag to show the autofill menu options. Used for items that are * already in the autofill list suggestion. */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input({ transform: booleanAttribute }) - hideAutofillOptions = false; + readonly showAutofill = input(false, { transform: booleanAttribute }); protected autofillAllowed$ = this.vaultPopupAutofillService.autofillAllowed$; diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html b/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html index 3dac158b8e1..d3bc025905e 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html @@ -90,11 +90,11 @@ - + + - + - - - + @if (!cipher.isDeleted && action === "view") { + + } + + @if (action === "edit" || action === "clone" || action === "add") { + + } + + @if (cipher.isDeleted && cipher.permissions.restore) { + + } + @if (showCloneOption) { } -
- - - -
+ @if (hasFooterAction) { +
+ @if (showArchiveButton) { + + } + + @if (showUnarchiveButton) { + + } + + +
+ }
diff --git a/apps/web/src/app/admin-console/common/people-table-data-source.spec.ts b/apps/web/src/app/admin-console/common/people-table-data-source.spec.ts index e9cf87a114d..e174d01a75d 100644 --- a/apps/web/src/app/admin-console/common/people-table-data-source.spec.ts +++ b/apps/web/src/app/admin-console/common/people-table-data-source.spec.ts @@ -2,7 +2,6 @@ import { TestBed } from "@angular/core/testing"; import { ReplaySubject } from "rxjs"; import { OrganizationUserStatusType } from "@bitwarden/common/admin-console/enums"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { Environment, EnvironmentService, @@ -46,23 +45,16 @@ describe("PeopleTableDataSource", () => { isCloud: () => false, } as Environment); - const mockConfigService = { - getFeatureFlag$: jest.fn(() => featureFlagSubject.asObservable()), - } as any; - const mockEnvironmentService = { environment$: environmentSubject.asObservable(), } as any; TestBed.configureTestingModule({ - providers: [ - { provide: ConfigService, useValue: mockConfigService }, - { provide: EnvironmentService, useValue: mockEnvironmentService }, - ], + providers: [{ provide: EnvironmentService, useValue: mockEnvironmentService }], }); dataSource = TestBed.runInInjectionContext( - () => new TestPeopleTableDataSource(mockConfigService, mockEnvironmentService), + () => new TestPeopleTableDataSource(mockEnvironmentService), ); }); diff --git a/apps/web/src/app/admin-console/common/people-table-data-source.ts b/apps/web/src/app/admin-console/common/people-table-data-source.ts index d39a4f29653..a3ffbaeb7b5 100644 --- a/apps/web/src/app/admin-console/common/people-table-data-source.ts +++ b/apps/web/src/app/admin-console/common/people-table-data-source.ts @@ -1,6 +1,6 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { computed, Signal } from "@angular/core"; +import { Signal } from "@angular/core"; import { toSignal } from "@angular/core/rxjs-interop"; import { Observable, Subject, map } from "rxjs"; @@ -9,8 +9,6 @@ import { ProviderUserStatusType, } from "@bitwarden/common/admin-console/enums"; import { ProviderUserUserDetailsResponse } from "@bitwarden/common/admin-console/models/response/provider/provider-user.response"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { TableDataSource } from "@bitwarden/components"; @@ -27,8 +25,7 @@ export type ProviderUser = ProviderUserUserDetailsResponse; export const MaxCheckedCount = 500; /** - * Maximum for bulk reinvite operations when the IncreaseBulkReinviteLimitForCloud - * feature flag is enabled on cloud environments. + * Maximum for bulk reinvite limit in cloud environments. */ export const CloudBulkReinviteLimit = 8000; @@ -78,18 +75,15 @@ export abstract class PeopleTableDataSource extends Tab confirmedUserCount: number; revokedUserCount: number; - /** True when increased bulk limit feature is enabled (feature flag + cloud environment) */ + /** True when increased bulk limit feature is enabled (cloud environment) */ readonly isIncreasedBulkLimitEnabled: Signal; - constructor(configService: ConfigService, environmentService: EnvironmentService) { + constructor(environmentService: EnvironmentService) { super(); - const featureFlagEnabled = toSignal( - configService.getFeatureFlag$(FeatureFlag.IncreaseBulkReinviteLimitForCloud), + this.isIncreasedBulkLimitEnabled = toSignal( + environmentService.environment$.pipe(map((env) => env.isCloud())), ); - const isCloud = toSignal(environmentService.environment$.pipe(map((env) => env.isCloud()))); - - this.isIncreasedBulkLimitEnabled = computed(() => featureFlagEnabled() && isCloud()); } override set data(data: T[]) { @@ -224,12 +218,9 @@ export abstract class PeopleTableDataSource extends Tab } /** - * Gets checked users with optional limiting based on the IncreaseBulkReinviteLimitForCloud feature flag. + * Returns checked users in visible order, optionally limited to the specified count. * - * When the feature flag is enabled: Returns checked users in visible order, limited to the specified count. - * When the feature flag is disabled: Returns all checked users without applying any limit. - * - * @param limit The maximum number of users to return (only applied when feature flag is enabled) + * @param limit The maximum number of users to return * @returns The checked users array */ getCheckedUsersWithLimit(limit: number): T[] { diff --git a/apps/web/src/app/admin-console/organizations/layouts/organization-layout.component.ts b/apps/web/src/app/admin-console/organizations/layouts/organization-layout.component.ts index b00e4d9840d..2d1fde10856 100644 --- a/apps/web/src/app/admin-console/organizations/layouts/organization-layout.component.ts +++ b/apps/web/src/app/admin-console/organizations/layouts/organization-layout.component.ts @@ -26,7 +26,7 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { getById } from "@bitwarden/common/platform/misc"; -import { BannerModule, IconModule } from "@bitwarden/components"; +import { BannerModule, SvgModule } from "@bitwarden/components"; import { OrganizationWarningsModule } from "@bitwarden/web-vault/app/billing/organizations/warnings/organization-warnings.module"; import { OrganizationWarningsService } from "@bitwarden/web-vault/app/billing/organizations/warnings/services"; import { NonIndividualSubscriber } from "@bitwarden/web-vault/app/billing/types"; @@ -47,7 +47,7 @@ import { WebLayoutModule } from "../../../layouts/web-layout.module"; RouterModule, JslibModule, WebLayoutModule, - IconModule, + SvgModule, OrgSwitcherComponent, BannerModule, TaxIdWarningComponent, diff --git a/apps/web/src/app/admin-console/organizations/members/deprecated_members.component.ts b/apps/web/src/app/admin-console/organizations/members/deprecated_members.component.ts index 99fd81aa48d..93960820fbb 100644 --- a/apps/web/src/app/admin-console/organizations/members/deprecated_members.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/deprecated_members.component.ts @@ -33,7 +33,6 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { OrganizationMetadataServiceAbstraction } from "@bitwarden/common/billing/abstractions/organization-metadata.service.abstraction"; import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -124,7 +123,6 @@ export class MembersComponent extends BaseMembersComponent private policyApiService: PolicyApiServiceAbstraction, private organizationMetadataService: OrganizationMetadataServiceAbstraction, private memberExportService: MemberExportService, - private configService: ConfigService, private environmentService: EnvironmentService, ) { super( @@ -139,7 +137,7 @@ export class MembersComponent extends BaseMembersComponent toastService, ); - this.dataSource = new MembersTableDataSource(this.configService, this.environmentService); + this.dataSource = new MembersTableDataSource(this.environmentService); const organization$ = this.route.params.pipe( concatMap((params) => diff --git a/apps/web/src/app/admin-console/organizations/members/members.component.ts b/apps/web/src/app/admin-console/organizations/members/members.component.ts index 9d367657d55..e3ed575d81b 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/members.component.ts @@ -33,7 +33,6 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { OrganizationMetadataServiceAbstraction } from "@bitwarden/common/billing/abstractions/organization-metadata.service.abstraction"; import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -100,7 +99,6 @@ export class vNextMembersComponent { private policyService = inject(PolicyService); private policyApiService = inject(PolicyApiServiceAbstraction); private organizationMetadataService = inject(OrganizationMetadataServiceAbstraction); - private configService = inject(ConfigService); private environmentService = inject(EnvironmentService); private memberExportService = inject(MemberExportService); @@ -114,7 +112,7 @@ export class vNextMembersComponent { protected statusToggle = new BehaviorSubject(undefined); protected readonly dataSource: Signal = signal( - new MembersTableDataSource(this.configService, this.environmentService), + new MembersTableDataSource(this.environmentService), ); protected readonly organization: Signal; protected readonly firstLoaded: WritableSignal = signal(false); @@ -389,7 +387,7 @@ export class vNextMembersComponent { // Capture the original count BEFORE enforcing the limit const originalInvitedCount = allInvitedUsers.length; - // When feature flag is enabled, limit invited users and uncheck the excess + // In cloud environments, limit invited users and uncheck the excess let filteredUsers: OrganizationUserView[]; if (this.dataSource().isIncreasedBulkLimitEnabled()) { filteredUsers = this.dataSource().limitAndUncheckExcess( @@ -418,7 +416,7 @@ export class vNextMembersComponent { this.validationService.showError(result.failed); } - // When feature flag is enabled, show toast instead of dialog + // In cloud environments, show toast instead of dialog if (this.dataSource().isIncreasedBulkLimitEnabled()) { const selectedCount = originalInvitedCount; const invitedCount = filteredUsers.length; @@ -441,7 +439,7 @@ export class vNextMembersComponent { }); } } else { - // Feature flag disabled - show legacy dialog + // In self-hosted environments, show legacy dialog await this.memberDialogManager.openBulkStatusDialog( users, filteredUsers, @@ -514,7 +512,7 @@ export class vNextMembersComponent { if (result.error != null) { this.toastService.showToast({ variant: "error", - message: this.i18nService.t(result.error), + message: result.error, }); this.logService.error(result.error); return; diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts index 1df285d7ba2..423977e73c4 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts @@ -17,7 +17,6 @@ import { import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { OrganizationMetadataServiceAbstraction } from "@bitwarden/common/billing/abstractions/organization-metadata.service.abstraction"; import { ListResponse } from "@bitwarden/common/models/response/list.response"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { DialogService } from "@bitwarden/components"; @@ -32,7 +31,6 @@ describe("MemberActionsService", () => { let service: MemberActionsService; let organizationUserApiService: MockProxy; let organizationUserService: MockProxy; - let configService: MockProxy; let organizationMetadataService: MockProxy; const organizationId = newGuid() as OrganizationId; @@ -44,7 +42,6 @@ describe("MemberActionsService", () => { beforeEach(() => { organizationUserApiService = mock(); organizationUserService = mock(); - configService = mock(); organizationMetadataService = mock(); mockOrganization = { @@ -68,7 +65,6 @@ describe("MemberActionsService", () => { MemberActionsService, { provide: OrganizationUserApiService, useValue: organizationUserApiService }, { provide: OrganizationUserService, useValue: organizationUserService }, - { provide: ConfigService, useValue: configService }, { provide: OrganizationMetadataServiceAbstraction, useValue: organizationMetadataService, @@ -279,308 +275,247 @@ describe("MemberActionsService", () => { }); describe("bulkReinvite", () => { - const userIds = [newGuid() as UserId, newGuid() as UserId, newGuid() as UserId]; + it("should process users in a single batch when count equals REQUESTS_PER_BATCH", async () => { + const userIdsBatch = Array.from({ length: REQUESTS_PER_BATCH }, () => newGuid() as UserId); + const mockResponse = new ListResponse( + { + data: userIdsBatch.map((id) => ({ + id, + error: null, + })), + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); - describe("when feature flag is false", () => { - beforeEach(() => { - configService.getFeatureFlag$.mockReturnValue(of(false)); - }); + organizationUserApiService.postManyOrganizationUserReinvite.mockResolvedValue(mockResponse); - it("should successfully reinvite multiple users", async () => { - const mockResponse = new ListResponse( - { - data: userIds.map((id) => ({ - id, - error: null, - })), - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + const result = await service.bulkReinvite(mockOrganization, userIdsBatch); - organizationUserApiService.postManyOrganizationUserReinvite.mockResolvedValue(mockResponse); - - const result = await service.bulkReinvite(mockOrganization, userIds); - - expect(result.failed).toEqual([]); - expect(result.successful).toBeDefined(); - expect(result.successful).toEqual(mockResponse); - expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledWith( - organizationId, - userIds, - ); - }); - - it("should handle bulk reinvite errors", async () => { - const errorMessage = "Bulk reinvite failed"; - organizationUserApiService.postManyOrganizationUserReinvite.mockRejectedValue( - new Error(errorMessage), - ); - - const result = await service.bulkReinvite(mockOrganization, userIds); - - expect(result.successful).toBeUndefined(); - expect(result.failed).toHaveLength(3); - expect(result.failed[0]).toEqual({ id: userIds[0], error: errorMessage }); - }); + expect(result.successful).toBeDefined(); + expect(result.successful?.response).toHaveLength(REQUESTS_PER_BATCH); + expect(result.failed).toHaveLength(0); + expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledTimes(1); + expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledWith( + organizationId, + userIdsBatch, + ); }); - describe("when feature flag is true (batching behavior)", () => { - beforeEach(() => { - configService.getFeatureFlag$.mockReturnValue(of(true)); - }); - it("should process users in a single batch when count equals REQUESTS_PER_BATCH", async () => { - const userIdsBatch = Array.from({ length: REQUESTS_PER_BATCH }, () => newGuid() as UserId); - const mockResponse = new ListResponse( - { - data: userIdsBatch.map((id) => ({ - id, - error: null, - })), - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + it("should process users in multiple batches when count exceeds REQUESTS_PER_BATCH", async () => { + const totalUsers = REQUESTS_PER_BATCH + 100; + const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); - organizationUserApiService.postManyOrganizationUserReinvite.mockResolvedValue(mockResponse); + const mockResponse1 = new ListResponse( + { + data: userIdsBatch.slice(0, REQUESTS_PER_BATCH).map((id) => ({ + id, + error: null, + })), + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); - const result = await service.bulkReinvite(mockOrganization, userIdsBatch); + const mockResponse2 = new ListResponse( + { + data: userIdsBatch.slice(REQUESTS_PER_BATCH).map((id) => ({ + id, + error: null, + })), + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); - expect(result.successful).toBeDefined(); - expect(result.successful?.response).toHaveLength(REQUESTS_PER_BATCH); - expect(result.failed).toHaveLength(0); - expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledTimes( - 1, - ); - expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledWith( - organizationId, - userIdsBatch, - ); - }); + organizationUserApiService.postManyOrganizationUserReinvite + .mockResolvedValueOnce(mockResponse1) + .mockResolvedValueOnce(mockResponse2); - it("should process users in multiple batches when count exceeds REQUESTS_PER_BATCH", async () => { - const totalUsers = REQUESTS_PER_BATCH + 100; - const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); + const result = await service.bulkReinvite(mockOrganization, userIdsBatch); - const mockResponse1 = new ListResponse( - { - data: userIdsBatch.slice(0, REQUESTS_PER_BATCH).map((id) => ({ - id, - error: null, - })), - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + expect(result.successful).toBeDefined(); + expect(result.successful?.response).toHaveLength(totalUsers); + expect(result.failed).toHaveLength(0); + expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledTimes(2); + expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenNthCalledWith( + 1, + organizationId, + userIdsBatch.slice(0, REQUESTS_PER_BATCH), + ); + expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenNthCalledWith( + 2, + organizationId, + userIdsBatch.slice(REQUESTS_PER_BATCH), + ); + }); - const mockResponse2 = new ListResponse( - { - data: userIdsBatch.slice(REQUESTS_PER_BATCH).map((id) => ({ - id, - error: null, - })), - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + it("should aggregate results across multiple successful batches", async () => { + const totalUsers = REQUESTS_PER_BATCH + 50; + const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); - organizationUserApiService.postManyOrganizationUserReinvite - .mockResolvedValueOnce(mockResponse1) - .mockResolvedValueOnce(mockResponse2); + const mockResponse1 = new ListResponse( + { + data: userIdsBatch.slice(0, REQUESTS_PER_BATCH).map((id) => ({ + id, + error: null, + })), + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); - const result = await service.bulkReinvite(mockOrganization, userIdsBatch); + const mockResponse2 = new ListResponse( + { + data: userIdsBatch.slice(REQUESTS_PER_BATCH).map((id) => ({ + id, + error: null, + })), + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); - expect(result.successful).toBeDefined(); - expect(result.successful?.response).toHaveLength(totalUsers); - expect(result.failed).toHaveLength(0); - expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledTimes( - 2, - ); - expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenNthCalledWith( - 1, - organizationId, - userIdsBatch.slice(0, REQUESTS_PER_BATCH), - ); - expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenNthCalledWith( - 2, - organizationId, - userIdsBatch.slice(REQUESTS_PER_BATCH), - ); - }); + organizationUserApiService.postManyOrganizationUserReinvite + .mockResolvedValueOnce(mockResponse1) + .mockResolvedValueOnce(mockResponse2); - it("should aggregate results across multiple successful batches", async () => { - const totalUsers = REQUESTS_PER_BATCH + 50; - const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); + const result = await service.bulkReinvite(mockOrganization, userIdsBatch); - const mockResponse1 = new ListResponse( - { - data: userIdsBatch.slice(0, REQUESTS_PER_BATCH).map((id) => ({ - id, - error: null, - })), - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + expect(result.successful).toBeDefined(); + expect(result.successful?.response).toHaveLength(totalUsers); + expect(result.successful?.response.slice(0, REQUESTS_PER_BATCH)).toEqual(mockResponse1.data); + expect(result.successful?.response.slice(REQUESTS_PER_BATCH)).toEqual(mockResponse2.data); + expect(result.failed).toHaveLength(0); + }); - const mockResponse2 = new ListResponse( - { - data: userIdsBatch.slice(REQUESTS_PER_BATCH).map((id) => ({ - id, - error: null, - })), - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + it("should handle mixed individual errors across multiple batches", async () => { + const totalUsers = REQUESTS_PER_BATCH + 4; + const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); - organizationUserApiService.postManyOrganizationUserReinvite - .mockResolvedValueOnce(mockResponse1) - .mockResolvedValueOnce(mockResponse2); + const mockResponse1 = new ListResponse( + { + data: userIdsBatch.slice(0, REQUESTS_PER_BATCH).map((id, index) => ({ + id, + error: index % 10 === 0 ? "Rate limit exceeded" : null, + })), + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); - const result = await service.bulkReinvite(mockOrganization, userIdsBatch); + const mockResponse2 = new ListResponse( + { + data: [ + { id: userIdsBatch[REQUESTS_PER_BATCH], error: null }, + { id: userIdsBatch[REQUESTS_PER_BATCH + 1], error: "Invalid email" }, + { id: userIdsBatch[REQUESTS_PER_BATCH + 2], error: null }, + { id: userIdsBatch[REQUESTS_PER_BATCH + 3], error: "User suspended" }, + ], + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); - expect(result.successful).toBeDefined(); - expect(result.successful?.response).toHaveLength(totalUsers); - expect(result.successful?.response.slice(0, REQUESTS_PER_BATCH)).toEqual( - mockResponse1.data, - ); - expect(result.successful?.response.slice(REQUESTS_PER_BATCH)).toEqual(mockResponse2.data); - expect(result.failed).toHaveLength(0); - }); + organizationUserApiService.postManyOrganizationUserReinvite + .mockResolvedValueOnce(mockResponse1) + .mockResolvedValueOnce(mockResponse2); - it("should handle mixed individual errors across multiple batches", async () => { - const totalUsers = REQUESTS_PER_BATCH + 4; - const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); + const result = await service.bulkReinvite(mockOrganization, userIdsBatch); - const mockResponse1 = new ListResponse( - { - data: userIdsBatch.slice(0, REQUESTS_PER_BATCH).map((id, index) => ({ - id, - error: index % 10 === 0 ? "Rate limit exceeded" : null, - })), - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + // Count expected failures: every 10th index (0, 10, 20, ..., 490) in first batch + 2 explicit in second batch + // Indices 0 to REQUESTS_PER_BATCH-1 where index % 10 === 0: that's floor((BATCH_SIZE-1)/10) + 1 values + const expectedFailuresInBatch1 = Math.floor((REQUESTS_PER_BATCH - 1) / 10) + 1; + const expectedFailuresInBatch2 = 2; + const expectedTotalFailures = expectedFailuresInBatch1 + expectedFailuresInBatch2; + const expectedSuccesses = totalUsers - expectedTotalFailures; - const mockResponse2 = new ListResponse( - { - data: [ - { id: userIdsBatch[REQUESTS_PER_BATCH], error: null }, - { id: userIdsBatch[REQUESTS_PER_BATCH + 1], error: "Invalid email" }, - { id: userIdsBatch[REQUESTS_PER_BATCH + 2], error: null }, - { id: userIdsBatch[REQUESTS_PER_BATCH + 3], error: "User suspended" }, - ], - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + expect(result.successful).toBeDefined(); + expect(result.successful?.response).toHaveLength(expectedSuccesses); + expect(result.failed).toHaveLength(expectedTotalFailures); + expect(result.failed.some((f) => f.error === "Rate limit exceeded")).toBe(true); + expect(result.failed.some((f) => f.error === "Invalid email")).toBe(true); + expect(result.failed.some((f) => f.error === "User suspended")).toBe(true); + }); - organizationUserApiService.postManyOrganizationUserReinvite - .mockResolvedValueOnce(mockResponse1) - .mockResolvedValueOnce(mockResponse2); + it("should aggregate all failures when all batches fail", async () => { + const totalUsers = REQUESTS_PER_BATCH + 100; + const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); + const errorMessage = "All batches failed"; - const result = await service.bulkReinvite(mockOrganization, userIdsBatch); + organizationUserApiService.postManyOrganizationUserReinvite.mockRejectedValue( + new Error(errorMessage), + ); - // Count expected failures: every 10th index (0, 10, 20, ..., 490) in first batch + 2 explicit in second batch - // Indices 0 to REQUESTS_PER_BATCH-1 where index % 10 === 0: that's floor((BATCH_SIZE-1)/10) + 1 values - const expectedFailuresInBatch1 = Math.floor((REQUESTS_PER_BATCH - 1) / 10) + 1; - const expectedFailuresInBatch2 = 2; - const expectedTotalFailures = expectedFailuresInBatch1 + expectedFailuresInBatch2; - const expectedSuccesses = totalUsers - expectedTotalFailures; + const result = await service.bulkReinvite(mockOrganization, userIdsBatch); - expect(result.successful).toBeDefined(); - expect(result.successful?.response).toHaveLength(expectedSuccesses); - expect(result.failed).toHaveLength(expectedTotalFailures); - expect(result.failed.some((f) => f.error === "Rate limit exceeded")).toBe(true); - expect(result.failed.some((f) => f.error === "Invalid email")).toBe(true); - expect(result.failed.some((f) => f.error === "User suspended")).toBe(true); - }); + expect(result.successful).toBeUndefined(); + expect(result.failed).toHaveLength(totalUsers); + expect(result.failed.every((f) => f.error === errorMessage)).toBe(true); + expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledTimes(2); + }); - it("should aggregate all failures when all batches fail", async () => { - const totalUsers = REQUESTS_PER_BATCH + 100; - const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); - const errorMessage = "All batches failed"; + it("should handle empty data in batch response", async () => { + const totalUsers = REQUESTS_PER_BATCH + 50; + const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); - organizationUserApiService.postManyOrganizationUserReinvite.mockRejectedValue( - new Error(errorMessage), - ); + const mockResponse1 = new ListResponse( + { + data: userIdsBatch.slice(0, REQUESTS_PER_BATCH).map((id) => ({ + id, + error: null, + })), + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); - const result = await service.bulkReinvite(mockOrganization, userIdsBatch); + const mockResponse2 = new ListResponse( + { + data: [], + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); - expect(result.successful).toBeUndefined(); - expect(result.failed).toHaveLength(totalUsers); - expect(result.failed.every((f) => f.error === errorMessage)).toBe(true); - expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledTimes( - 2, - ); - }); + organizationUserApiService.postManyOrganizationUserReinvite + .mockResolvedValueOnce(mockResponse1) + .mockResolvedValueOnce(mockResponse2); - it("should handle empty data in batch response", async () => { - const totalUsers = REQUESTS_PER_BATCH + 50; - const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); + const result = await service.bulkReinvite(mockOrganization, userIdsBatch); - const mockResponse1 = new ListResponse( - { - data: userIdsBatch.slice(0, REQUESTS_PER_BATCH).map((id) => ({ - id, - error: null, - })), - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + expect(result.successful).toBeDefined(); + expect(result.successful?.response).toHaveLength(REQUESTS_PER_BATCH); + expect(result.failed).toHaveLength(0); + }); - const mockResponse2 = new ListResponse( - { - data: [], - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + it("should process batches sequentially in order", async () => { + const totalUsers = REQUESTS_PER_BATCH * 2; + const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); + const callOrder: number[] = []; - organizationUserApiService.postManyOrganizationUserReinvite - .mockResolvedValueOnce(mockResponse1) - .mockResolvedValueOnce(mockResponse2); + organizationUserApiService.postManyOrganizationUserReinvite.mockImplementation( + async (orgId, ids) => { + const batchIndex = ids.includes(userIdsBatch[0]) ? 1 : 2; + callOrder.push(batchIndex); - const result = await service.bulkReinvite(mockOrganization, userIdsBatch); + return new ListResponse( + { + data: ids.map((id) => ({ + id, + error: null, + })), + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); + }, + ); - expect(result.successful).toBeDefined(); - expect(result.successful?.response).toHaveLength(REQUESTS_PER_BATCH); - expect(result.failed).toHaveLength(0); - }); + await service.bulkReinvite(mockOrganization, userIdsBatch); - it("should process batches sequentially in order", async () => { - const totalUsers = REQUESTS_PER_BATCH * 2; - const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); - const callOrder: number[] = []; - - organizationUserApiService.postManyOrganizationUserReinvite.mockImplementation( - async (orgId, ids) => { - const batchIndex = ids.includes(userIdsBatch[0]) ? 1 : 2; - callOrder.push(batchIndex); - - return new ListResponse( - { - data: ids.map((id) => ({ - id, - error: null, - })), - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); - }, - ); - - await service.bulkReinvite(mockOrganization, userIdsBatch); - - expect(callOrder).toEqual([1, 2]); - expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledTimes( - 2, - ); - }); + expect(callOrder).toEqual([1, 2]); + expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledTimes(2); }); }); diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts index 5833238209c..e8c4a21d675 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts @@ -16,9 +16,7 @@ import { import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { assertNonNullish } from "@bitwarden/common/auth/utils"; import { OrganizationMetadataServiceAbstraction } from "@bitwarden/common/billing/abstractions/organization-metadata.service.abstraction"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ListResponse } from "@bitwarden/common/models/response/list.response"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { DialogService } from "@bitwarden/components"; @@ -45,7 +43,6 @@ export interface BulkActionResult { export class MemberActionsService { private organizationUserApiService = inject(OrganizationUserApiService); private organizationUserService = inject(OrganizationUserService); - private configService = inject(ConfigService); private organizationMetadataService = inject(OrganizationMetadataServiceAbstraction); private apiService = inject(ApiService); private dialogService = inject(DialogService); @@ -175,18 +172,9 @@ export class MemberActionsService { async bulkReinvite(organization: Organization, userIds: UserId[]): Promise { this.startProcessing(); try { - const increaseBulkReinviteLimitForCloud = await firstValueFrom( - this.configService.getFeatureFlag$(FeatureFlag.IncreaseBulkReinviteLimitForCloud), + return this.processBatchedOperation(userIds, REQUESTS_PER_BATCH, (batch) => + this.organizationUserApiService.postManyOrganizationUserReinvite(organization.id, batch), ); - if (increaseBulkReinviteLimitForCloud) { - return await this.vNextBulkReinvite(organization, userIds); - } else { - const result = await this.organizationUserApiService.postManyOrganizationUserReinvite( - organization.id, - userIds, - ); - return { successful: result, failed: [] }; - } } catch (error) { return { failed: userIds.map((id) => ({ id, error: (error as Error).message ?? String(error) })), @@ -196,15 +184,6 @@ export class MemberActionsService { } } - async vNextBulkReinvite( - organization: Organization, - userIds: UserId[], - ): Promise { - return this.processBatchedOperation(userIds, REQUESTS_PER_BATCH, (batch) => - this.organizationUserApiService.postManyOrganizationUserReinvite(organization.id, batch), - ); - } - allowResetPassword( orgUser: OrganizationUserView, organization: Organization, diff --git a/apps/web/src/app/admin-console/organizations/policies/index.ts b/apps/web/src/app/admin-console/organizations/policies/index.ts index eb614e180e1..8e730d3a6b8 100644 --- a/apps/web/src/app/admin-console/organizations/policies/index.ts +++ b/apps/web/src/app/admin-console/organizations/policies/index.ts @@ -5,3 +5,4 @@ export { POLICY_EDIT_REGISTER } from "./policy-register-token"; export { AutoConfirmPolicy } from "./policy-edit-definitions"; export { PolicyEditDialogResult } from "./policy-edit-dialog.component"; export * from "./policy-edit-dialogs"; +export { PolicyOrderPipe } from "./pipes/policy-order.pipe"; diff --git a/apps/web/src/app/admin-console/organizations/policies/pipes/policy-order.pipe.ts b/apps/web/src/app/admin-console/organizations/policies/pipes/policy-order.pipe.ts new file mode 100644 index 00000000000..02092f05b92 --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/policies/pipes/policy-order.pipe.ts @@ -0,0 +1,66 @@ +import { Pipe, PipeTransform } from "@angular/core"; + +import { BasePolicyEditDefinition } from "../base-policy-edit.component"; + +/** + * Order mapping for policies. Policies are ordered according to this mapping. + * Policies not in this mapping will appear at the end, maintaining their relative order. + */ +const POLICY_ORDER_MAP = new Map([ + ["singleOrg", 1], + ["organizationDataOwnership", 2], + ["centralizeDataOwnership", 2], + ["masterPassPolicyTitle", 3], + ["accountRecoveryPolicy", 4], + ["requireSso", 5], + ["automaticAppLoginWithSSO", 6], + ["twoStepLoginPolicyTitle", 7], + ["blockClaimedDomainAccountCreation", 8], + ["sessionTimeoutPolicyTitle", 9], + ["removeUnlockWithPinPolicyTitle", 10], + ["passwordGenerator", 11], + ["uriMatchDetectionPolicy", 12], + ["activateAutofillPolicy", 13], + ["sendOptions", 14], + ["disableSend", 15], + ["restrictedItemTypePolicy", 16], + ["freeFamiliesSponsorship", 17], + ["disableExport", 18], +]); + +/** + * Default order for policies not in the mapping. This ensures unmapped policies + * appear at the end while maintaining their relative order. + */ +const DEFAULT_ORDER = 999; + +@Pipe({ + name: "policyOrder", + standalone: true, +}) +export class PolicyOrderPipe implements PipeTransform { + transform( + policies: readonly BasePolicyEditDefinition[] | null | undefined, + ): BasePolicyEditDefinition[] { + if (policies == null || policies.length === 0) { + return []; + } + + const sortedPolicies = [...policies]; + + sortedPolicies.sort((a, b) => { + const orderA = POLICY_ORDER_MAP.get(a.name) ?? DEFAULT_ORDER; + const orderB = POLICY_ORDER_MAP.get(b.name) ?? DEFAULT_ORDER; + + if (orderA !== orderB) { + return orderA - orderB; + } + + const indexA = policies.indexOf(a); + const indexB = policies.indexOf(b); + return indexA - indexB; + }); + + return sortedPolicies; + } +} diff --git a/apps/web/src/app/admin-console/organizations/policies/policies.component.html b/apps/web/src/app/admin-console/organizations/policies/policies.component.html index c38092146ab..902c7e79d55 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policies.component.html +++ b/apps/web/src/app/admin-console/organizations/policies/policies.component.html @@ -15,7 +15,7 @@ } @else { - @for (p of policies$ | async; track $index) { + @for (p of policies$ | async | policyOrder; track $index) { @if (p.display$(organization, configService) | async) { diff --git a/apps/web/src/app/admin-console/organizations/policies/policies.component.ts b/apps/web/src/app/admin-console/organizations/policies/policies.component.ts index 1f9a8deaa85..d13a2097628 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policies.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policies.component.ts @@ -21,13 +21,14 @@ import { HeaderModule } from "../../../layouts/header/header.module"; import { SharedModule } from "../../../shared"; import { BasePolicyEditDefinition, PolicyDialogComponent } from "./base-policy-edit.component"; +import { PolicyOrderPipe } from "./pipes/policy-order.pipe"; import { PolicyEditDialogComponent } from "./policy-edit-dialog.component"; import { PolicyListService } from "./policy-list.service"; import { POLICY_EDIT_REGISTER } from "./policy-register-token"; @Component({ templateUrl: "policies.component.html", - imports: [SharedModule, HeaderModule], + imports: [SharedModule, HeaderModule, PolicyOrderPipe], providers: [ safeProvider({ provide: PolicyListService, diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/auto-confirm-policy.component.html b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/auto-confirm-policy.component.html index 54f166b662e..a8e3236dad8 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/auto-confirm-policy.component.html +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/auto-confirm-policy.component.html @@ -44,7 +44,7 @@
- +
  1. 1. {{ "autoConfirmExtension1" | i18n }}
    2. diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.spec.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.spec.ts new file mode 100644 index 00000000000..09b2f8961f3 --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.spec.ts @@ -0,0 +1,270 @@ +import { NO_ERRORS_SCHEMA } from "@angular/core"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { FormBuilder } from "@angular/forms"; +import { Router } from "@angular/router"; +import { mock, MockProxy } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; +import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { PolicyType } from "@bitwarden/common/admin-console/enums"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; +import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; +import { DIALOG_DATA, DialogRef, ToastService } from "@bitwarden/components"; +import { newGuid } from "@bitwarden/guid"; +import { KeyService } from "@bitwarden/key-management"; + +import { + AutoConfirmPolicyDialogComponent, + AutoConfirmPolicyDialogData, +} from "./auto-confirm-edit-policy-dialog.component"; + +describe("AutoConfirmPolicyDialogComponent", () => { + let component: AutoConfirmPolicyDialogComponent; + let fixture: ComponentFixture; + + let mockPolicyApiService: MockProxy; + let mockAccountService: FakeAccountService; + let mockOrganizationService: MockProxy; + let mockPolicyService: MockProxy; + let mockRouter: MockProxy; + let mockAutoConfirmService: MockProxy; + let mockDialogRef: MockProxy; + let mockToastService: MockProxy; + let mockI18nService: MockProxy; + let mockKeyService: MockProxy; + + const mockUserId = newGuid() as UserId; + const mockOrgId = newGuid() as OrganizationId; + + const mockDialogData: AutoConfirmPolicyDialogData = { + organizationId: mockOrgId, + policy: { + name: "autoConfirm", + description: "Auto Confirm Policy", + type: PolicyType.AutoConfirm, + component: {} as any, + showDescription: true, + display$: () => of(true), + }, + firstTimeDialog: false, + }; + + const mockOrg = { + id: mockOrgId, + name: "Test Organization", + enabled: true, + isAdmin: true, + canManagePolicies: true, + } as Organization; + + beforeEach(async () => { + mockPolicyApiService = mock(); + mockAccountService = mockAccountServiceWith(mockUserId); + mockOrganizationService = mock(); + mockPolicyService = mock(); + mockRouter = mock(); + mockAutoConfirmService = mock(); + mockDialogRef = mock(); + mockToastService = mock(); + mockI18nService = mock(); + mockKeyService = mock(); + + mockPolicyService.policies$.mockReturnValue(of([])); + mockOrganizationService.organizations$.mockReturnValue(of([mockOrg])); + + await TestBed.configureTestingModule({ + imports: [AutoConfirmPolicyDialogComponent], + providers: [ + FormBuilder, + { provide: DIALOG_DATA, useValue: mockDialogData }, + { provide: AccountService, useValue: mockAccountService }, + { provide: PolicyApiServiceAbstraction, useValue: mockPolicyApiService }, + { provide: I18nService, useValue: mockI18nService }, + { provide: DialogRef, useValue: mockDialogRef }, + { provide: ToastService, useValue: mockToastService }, + { provide: KeyService, useValue: mockKeyService }, + { provide: OrganizationService, useValue: mockOrganizationService }, + { provide: PolicyService, useValue: mockPolicyService }, + { provide: Router, useValue: mockRouter }, + { provide: AutomaticUserConfirmationService, useValue: mockAutoConfirmService }, + ], + schemas: [NO_ERRORS_SCHEMA], + }) + .overrideComponent(AutoConfirmPolicyDialogComponent, { + set: { template: "
    " }, + }) + .compileComponents(); + + fixture = TestBed.createComponent(AutoConfirmPolicyDialogComponent); + component = fixture.componentInstance; + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + it("should create", () => { + expect(component).toBeTruthy(); + }); + + describe("handleSubmit", () => { + beforeEach(() => { + // Mock the policyComponent + component.policyComponent = { + buildRequest: jest.fn().mockResolvedValue({ enabled: true, data: null }), + enabled: { value: true }, + setSingleOrgEnabled: jest.fn(), + } as any; + + mockAutoConfirmService.configuration$.mockReturnValue( + of({ enabled: false, showSetupDialog: true, showBrowserNotification: undefined }), + ); + mockAutoConfirmService.upsert.mockResolvedValue(undefined); + mockI18nService.t.mockReturnValue("Policy updated"); + }); + + it("should enable SingleOrg policy when it was not already enabled", async () => { + mockPolicyApiService.putPolicyVNext.mockResolvedValue({} as any); + + // Call handleSubmit with singleOrgEnabled = false (meaning it needs to be enabled) + await component["handleSubmit"](false); + + // First call should be SingleOrg enable + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenNthCalledWith( + 1, + mockOrgId, + PolicyType.SingleOrg, + { policy: { enabled: true, data: null } }, + ); + }); + + it("should not enable SingleOrg policy when it was already enabled", async () => { + mockPolicyApiService.putPolicyVNext.mockResolvedValue({} as any); + + // Call handleSubmit with singleOrgEnabled = true (meaning it's already enabled) + await component["handleSubmit"](true); + + // Should only call putPolicyVNext once (for AutoConfirm, not SingleOrg) + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledTimes(1); + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledWith( + mockOrgId, + PolicyType.AutoConfirm, + { policy: { enabled: true, data: null } }, + ); + }); + + it("should rollback SingleOrg policy when AutoConfirm fails and SingleOrg was enabled during action", async () => { + const autoConfirmError = new Error("AutoConfirm failed"); + + // First call (SingleOrg enable) succeeds, second call (AutoConfirm) fails, third call (SingleOrg rollback) succeeds + mockPolicyApiService.putPolicyVNext + .mockResolvedValueOnce({} as any) // SingleOrg enable + .mockRejectedValueOnce(autoConfirmError) // AutoConfirm fails + .mockResolvedValueOnce({} as any); // SingleOrg rollback + + await expect(component["handleSubmit"](false)).rejects.toThrow("AutoConfirm failed"); + + // Verify: SingleOrg enabled, AutoConfirm attempted, SingleOrg rolled back + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledTimes(3); + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenNthCalledWith( + 1, + mockOrgId, + PolicyType.SingleOrg, + { policy: { enabled: true, data: null } }, + ); + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenNthCalledWith( + 2, + mockOrgId, + PolicyType.AutoConfirm, + { policy: { enabled: true, data: null } }, + ); + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenNthCalledWith( + 3, + mockOrgId, + PolicyType.SingleOrg, + { policy: { enabled: false, data: null } }, + ); + }); + + it("should not rollback SingleOrg policy when AutoConfirm fails but SingleOrg was already enabled", async () => { + const autoConfirmError = new Error("AutoConfirm failed"); + + // AutoConfirm call fails (SingleOrg was already enabled, so no SingleOrg calls) + mockPolicyApiService.putPolicyVNext.mockRejectedValue(autoConfirmError); + + await expect(component["handleSubmit"](true)).rejects.toThrow("AutoConfirm failed"); + + // Verify only AutoConfirm was called (no SingleOrg enable/rollback) + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledTimes(1); + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledWith( + mockOrgId, + PolicyType.AutoConfirm, + { policy: { enabled: true, data: null } }, + ); + }); + + it("should keep both policies enabled when both submissions succeed", async () => { + mockPolicyApiService.putPolicyVNext.mockResolvedValue({} as any); + + await component["handleSubmit"](false); + + // Verify two calls: SingleOrg enable and AutoConfirm enable + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledTimes(2); + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenNthCalledWith( + 1, + mockOrgId, + PolicyType.SingleOrg, + { policy: { enabled: true, data: null } }, + ); + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenNthCalledWith( + 2, + mockOrgId, + PolicyType.AutoConfirm, + { policy: { enabled: true, data: null } }, + ); + }); + + it("should re-throw the error after rollback", async () => { + const autoConfirmError = new Error("Network error"); + + mockPolicyApiService.putPolicyVNext + .mockResolvedValueOnce({} as any) // SingleOrg enable + .mockRejectedValueOnce(autoConfirmError) // AutoConfirm fails + .mockResolvedValueOnce({} as any); // SingleOrg rollback + + await expect(component["handleSubmit"](false)).rejects.toThrow("Network error"); + }); + }); + + describe("setSingleOrgPolicy", () => { + it("should call putPolicyVNext with enabled: true when enabling", async () => { + mockPolicyApiService.putPolicyVNext.mockResolvedValue({} as any); + + await component["setSingleOrgPolicy"](true); + + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledWith( + mockOrgId, + PolicyType.SingleOrg, + { policy: { enabled: true, data: null } }, + ); + }); + + it("should call putPolicyVNext with enabled: false when disabling", async () => { + mockPolicyApiService.putPolicyVNext.mockResolvedValue({} as any); + + await component["setSingleOrgPolicy"](false); + + expect(mockPolicyApiService.putPolicyVNext).toHaveBeenCalledWith( + mockOrgId, + PolicyType.SingleOrg, + { policy: { enabled: false, data: null } }, + ); + }); + }); +}); diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.ts index fbdeffc71bb..f0146225b8d 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialogs/auto-confirm-edit-policy-dialog.component.ts @@ -181,10 +181,21 @@ export class AutoConfirmPolicyDialogComponent } private async handleSubmit(singleOrgEnabled: boolean) { - if (!singleOrgEnabled) { - await this.submitSingleOrg(); + const enabledSingleOrgDuringAction = !singleOrgEnabled; + + if (enabledSingleOrgDuringAction) { + await this.setSingleOrgPolicy(true); + } + + try { + await this.submitAutoConfirm(); + } catch (error) { + // Roll back SingleOrg if we enabled it during this action + if (enabledSingleOrgDuringAction) { + await this.setSingleOrgPolicy(false); + } + throw error; } - await this.submitAutoConfirm(); } /** @@ -198,11 +209,9 @@ export class AutoConfirmPolicyDialogComponent const autoConfirmRequest = await this.policyComponent.buildRequest(); - await this.policyApiService.putPolicy( - this.data.organizationId, - this.data.policy.type, - autoConfirmRequest, - ); + await this.policyApiService.putPolicyVNext(this.data.organizationId, this.data.policy.type, { + policy: autoConfirmRequest, + }); const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); @@ -225,17 +234,15 @@ export class AutoConfirmPolicyDialogComponent } } - private async submitSingleOrg(): Promise { + private async setSingleOrgPolicy(enabled: boolean): Promise { const singleOrgRequest: PolicyRequest = { - enabled: true, + enabled, data: null, }; - await this.policyApiService.putPolicyVNext( - this.data.organizationId, - PolicyType.SingleOrg, - singleOrgRequest, - ); + await this.policyApiService.putPolicyVNext(this.data.organizationId, PolicyType.SingleOrg, { + policy: singleOrgRequest, + }); } private async openBrowserExtension() { diff --git a/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.html b/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.html index e509692aba7..431d7711331 100644 --- a/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.html +++ b/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.html @@ -63,7 +63,7 @@ - +
    {{ "readOnlyCollectionAccess" | i18n }} diff --git a/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts b/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts index 4f40ea701d2..2f9ddddd8cb 100644 --- a/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts +++ b/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts @@ -361,6 +361,12 @@ export class CollectionDialogComponent implements OnInit, OnDestroy { return this.params.readonly === true; } + protected get accessTabLabel(): string { + return this.dialogReadonly + ? this.i18nService.t("viewAccess") + : this.i18nService.t("editAccess"); + } + protected async cancel() { this.close(CollectionDialogAction.Canceled); } diff --git a/apps/web/src/app/admin-console/organizations/sponsorships/accept-family-sponsorship.component.html b/apps/web/src/app/admin-console/organizations/sponsorships/accept-family-sponsorship.component.html index ca1264829b9..0255e1a6a99 100644 --- a/apps/web/src/app/admin-console/organizations/sponsorships/accept-family-sponsorship.component.html +++ b/apps/web/src/app/admin-console/organizations/sponsorships/accept-family-sponsorship.component.html @@ -1,7 +1,7 @@
    - - + +