From d40e9a36443f49bbd90389539ee3f190f53b56a4 Mon Sep 17 00:00:00 2001 From: Brad <44413459+lastbestdev@users.noreply.github.com> Date: Wed, 28 Jan 2026 12:47:38 -0800 Subject: [PATCH 1/6] [PM-30918] Migrate DIRT components to new Angular control flow syntax (#18416) * dirt: migrate apps/web components to new control flow * dirt: update control flow bitwarden licensed code * consolidate @if statements, use @else where appropriate * more cleanup * consolidate conditionals * remove unnecessary conditional --- .../pages/breach-report.component.html | 91 +++---- .../exposed-passwords-report.component.html | 217 +++++++++-------- .../inactive-two-factor-report.component.html | 229 +++++++++--------- .../reused-passwords-report.component.html | 216 +++++++++-------- .../unsecured-websites-report.component.html | 204 ++++++++-------- .../weak-passwords-report.component.html | 228 ++++++++--------- .../report-list/report-list.component.html | 20 +- .../activity/activity-card.component.ts | 3 +- .../password-change-metric.component.ts | 3 +- .../assign-tasks-view.component.ts | 2 - .../new-applications-dialog.component.ts | 2 - .../empty-state-card.component.html | 134 +++++----- .../empty-state-card.component.ts | 3 +- .../risk-insights.component.html | 17 +- .../app-table-row-scrollable.component.html | 98 ++++---- .../shared/report-loading.component.ts | 3 +- .../integration-grid.component.html | 35 +-- .../integrations.component.html | 48 ++-- .../member-access-report.component.html | 84 ++++--- 19 files changed, 838 insertions(+), 799 deletions(-) diff --git a/apps/web/src/app/dirt/reports/pages/breach-report.component.html b/apps/web/src/app/dirt/reports/pages/breach-report.component.html index d645fa39d69..0915902143e 100644 --- a/apps/web/src/app/dirt/reports/pages/breach-report.component.html +++ b/apps/web/src/app/dirt/reports/pages/breach-report.component.html @@ -12,45 +12,54 @@ {{ "checkBreaches" | i18n }} -
-

{{ "reportError" | i18n }}...

- - - {{ "breachUsernameNotFound" | i18n: checkedUsername }} - - - {{ "breachUsernameFound" | i18n: checkedUsername : breachedAccounts.length }} - - - -
+ @if (!loading && checkedUsername) { +
+ @if (error) { +

{{ "reportError" | i18n }}...

+ } @else { + @if (!breachedAccounts.length) { + + {{ "breachUsernameNotFound" | i18n: checkedUsername }} + + } @else { + + {{ "breachUsernameFound" | i18n: checkedUsername : breachedAccounts.length }} + + + } + } +
+ } diff --git a/apps/web/src/app/dirt/reports/pages/exposed-passwords-report.component.html b/apps/web/src/app/dirt/reports/pages/exposed-passwords-report.component.html index 55e6678bd58..ba118ea6663 100644 --- a/apps/web/src/app/dirt/reports/pages/exposed-passwords-report.component.html +++ b/apps/web/src/app/dirt/reports/pages/exposed-passwords-report.component.html @@ -5,108 +5,119 @@ -
- - {{ "noExposedPasswords" | i18n }} - - - - {{ "exposedPasswordsFoundReportDesc" | i18n: (ciphers.length | number) : vaultMsg }} - - - @if (showFilterToggle && !isAdminConsoleActive) { - @if (canDisplayToggleGroup()) { - - - - {{ getName(status) }} - {{ getCount(status) }} - - - - } @else { - - } - } - - - - - {{ "name" | i18n }} - - {{ "owner" | i18n }} - - - {{ "timesExposed" | i18n }} - - - - - - - - - - {{ row.name }} - - - - {{ row.name }} - - - - {{ "shared" | i18n }} - - - - {{ "attachments" | i18n }} - -
- {{ row.subTitle }} - - - + @if (!ciphers.length) { + + {{ "noExposedPasswords" | i18n }} + + } @else { + + {{ "exposedPasswordsFoundReportDesc" | i18n: (ciphers.length | number) : vaultMsg }} + + @if (showFilterToggle && !isAdminConsoleActive) { + @if (canDisplayToggleGroup()) { + - - - - - {{ "exposedXTimes" | i18n: (row.exposedXTimes | number) }} - - -
-
-
-
+ @for (status of filterStatus; track status) { + + {{ getName(status) }} + {{ getCount(status) }} + + } + + } @else { + + } + } + + + + {{ "name" | i18n }} + @if (!isAdminConsoleActive) { + + {{ "owner" | i18n }} + + } + + {{ "timesExposed" | i18n }} + + + + + + + + @if (!organization || canManageCipher(row)) { + + {{ row.name }} + + } @else { + {{ row.name }} + } + @if (!organization && row.organizationId) { + + {{ "shared" | i18n }} + } + @if (row.hasAttachments) { + + {{ "attachments" | i18n }} + } +
+ {{ row.subTitle }} + + @if (!isAdminConsoleActive) { + + @if (!organization) { + + + } + + } + + + {{ "exposedXTimes" | i18n: (row.exposedXTimes | number) }} + + +
+
+ } + + } diff --git a/apps/web/src/app/dirt/reports/pages/inactive-two-factor-report.component.html b/apps/web/src/app/dirt/reports/pages/inactive-two-factor-report.component.html index a1d3f2a38be..4999d572969 100644 --- a/apps/web/src/app/dirt/reports/pages/inactive-two-factor-report.component.html +++ b/apps/web/src/app/dirt/reports/pages/inactive-two-factor-report.component.html @@ -2,117 +2,124 @@

{{ "inactive2faReportDesc" | i18n }}

-
- - {{ "loading" | i18n }} -
-
- - {{ "noInactive2fa" | i18n }} - - - - {{ "inactive2faFoundReportDesc" | i18n: (ciphers.length | number) : vaultMsg }} - - - @if (showFilterToggle && !isAdminConsoleActive) { - @if (canDisplayToggleGroup()) { - - - - {{ getName(status) }} - {{ getCount(status) }} - - - - } @else { - + @if (!hasLoaded && loading) { +
+ + {{ "loading" | i18n }} +
+ } @else { +
+ @if (!ciphers.length) { + + {{ "noInactive2fa" | i18n }} + + } @else { + + {{ "inactive2faFoundReportDesc" | i18n: (ciphers.length | number) : vaultMsg }} + + @if (showFilterToggle && !isAdminConsoleActive) { + @if (canDisplayToggleGroup()) { + + @for (status of filterStatus; track status) { + + {{ getName(status) }} + {{ getCount(status) }} + + } + + } @else { + + } } + + @if (!isAdminConsoleActive) { + + + {{ "name" | i18n }} + {{ "owner" | i18n }} + + + } + + + + + + @if (!organization || canManageCipher(row)) { + + {{ row.name }} + + } @else { + + {{ row.name }} + + } + @if (!organization && row.organizationId) { + + + {{ "shared" | i18n }} + + } + @if (row.hasAttachments) { + + + {{ "attachments" | i18n }} + + } +
+ {{ row.subTitle }} + + + @if (!organization) { + + } + + + @if (cipherDocs.has(row.id)) { + + {{ "instructions" | i18n }} + } + +
+
} - - - - - {{ "name" | i18n }} - {{ "owner" | i18n }} - - - - - - - - - {{ row.name }} - - - {{ row.name }} - - - - {{ "shared" | i18n }} - - - - {{ "attachments" | i18n }} - -
- {{ row.subTitle }} - - - - - - - - {{ "instructions" | i18n }} - -
-
- -
+
+ }
diff --git a/apps/web/src/app/dirt/reports/pages/reused-passwords-report.component.html b/apps/web/src/app/dirt/reports/pages/reused-passwords-report.component.html index 62496dfad00..f08af8bda01 100644 --- a/apps/web/src/app/dirt/reports/pages/reused-passwords-report.component.html +++ b/apps/web/src/app/dirt/reports/pages/reused-passwords-report.component.html @@ -2,111 +2,115 @@

{{ "reusedPasswordsReportDesc" | i18n }}

-
- - {{ "loading" | i18n }} -
-
- - {{ "noReusedPasswords" | i18n }} - - - - {{ "reusedPasswordsFoundReportDesc" | i18n: (ciphers.length | number) : vaultMsg }} - - - @if (showFilterToggle && !isAdminConsoleActive) { - @if (canDisplayToggleGroup()) { - - - - {{ getName(status) }} - {{ getCount(status) }} - - - - } @else { - - } - } - - - - - {{ "name" | i18n }} - {{ "owner" | i18n }} - {{ "timesReused" | i18n }} - - - - - - - - {{ row.name }} - - - {{ row.name }} - - - - {{ "shared" | i18n }} - - - - {{ "attachments" | i18n }} - -
- {{ row.subTitle }} - - - + + {{ "loading" | i18n }} +
+ } @else { +
+ @if (!ciphers.length) { + + {{ "noReusedPasswords" | i18n }} + + } @else { + + {{ "reusedPasswordsFoundReportDesc" | i18n: (ciphers.length | number) : vaultMsg }} + + @if (showFilterToggle && !isAdminConsoleActive) { + @if (canDisplayToggleGroup()) { + - - - - - {{ "reusedXTimes" | i18n: passwordUseMap.get(row.login.password) }} - - - - - -
+ @for (status of filterStatus; track status) { + + {{ getName(status) }} + {{ getCount(status) }} + + } + + } @else { + + } + } + + @if (!isAdminConsoleActive) { + + + {{ "name" | i18n }} + {{ "owner" | i18n }} + {{ "timesReused" | i18n }} + + } + + + + + + @if (!organization || canManageCipher(row)) { + {{ row.name }} + } @else { + {{ row.name }} + } + @if (!organization && row.organizationId) { + + {{ "shared" | i18n }} + } + @if (row.hasAttachments) { + + {{ "attachments" | i18n }} + } +
+ {{ row.subTitle }} + + + @if (!organization) { + + + } + + + + {{ "reusedXTimes" | i18n: passwordUseMap.get(row.login.password) }} + + +
+
+ } + + }
diff --git a/apps/web/src/app/dirt/reports/pages/unsecured-websites-report.component.html b/apps/web/src/app/dirt/reports/pages/unsecured-websites-report.component.html index 276508b3801..810c1e384b0 100644 --- a/apps/web/src/app/dirt/reports/pages/unsecured-websites-report.component.html +++ b/apps/web/src/app/dirt/reports/pages/unsecured-websites-report.component.html @@ -2,105 +2,109 @@

{{ "unsecuredWebsitesReportDesc" | i18n }}

-
- - {{ "loading" | i18n }} -
-
- - {{ "noUnsecuredWebsites" | i18n }} - - - - {{ "unsecuredWebsitesFoundReportDesc" | i18n: (ciphers.length | number) : vaultMsg }} - - - @if (showFilterToggle && !isAdminConsoleActive) { - @if (canDisplayToggleGroup()) { - - - - {{ getName(status) }} - {{ getCount(status) }} - - - - } @else { - - } - } - - - - - {{ "name" | i18n }} - {{ "owner" | i18n }} - - - - - - - - {{ row.name }} - - - {{ row.name }} - - - - {{ "shared" | i18n }} - - - - {{ "attachments" | i18n }} - -
- {{ row.subTitle }} - - - + + {{ "loading" | i18n }} +
+ } @else { +
+ @if (!ciphers.length) { + + {{ "noUnsecuredWebsites" | i18n }} + + } @else { + + {{ "unsecuredWebsitesFoundReportDesc" | i18n: (ciphers.length | number) : vaultMsg }} + + @if (showFilterToggle && !isAdminConsoleActive) { + @if (canDisplayToggleGroup()) { + - - - - - -
+ @for (status of filterStatus; track status) { + + {{ getName(status) }} + {{ getCount(status) }} + + } + + } @else { + + } + } + + @if (!isAdminConsoleActive) { + + + {{ "name" | i18n }} + {{ "owner" | i18n }} + + } + + + + + + @if (!organization || canManageCipher(row)) { + {{ row.name }} + } @else { + {{ row.name }} + } + @if (!organization && row.organizationId) { + + {{ "shared" | i18n }} + } + @if (row.hasAttachments) { + + {{ "attachments" | i18n }} + } +
+ {{ row.subTitle }} + + + @if (!organization) { + + + } + +
+
+ } + + }
diff --git a/apps/web/src/app/dirt/reports/pages/weak-passwords-report.component.html b/apps/web/src/app/dirt/reports/pages/weak-passwords-report.component.html index 96bae4c3e0a..d96d083ffe0 100644 --- a/apps/web/src/app/dirt/reports/pages/weak-passwords-report.component.html +++ b/apps/web/src/app/dirt/reports/pages/weak-passwords-report.component.html @@ -2,115 +2,123 @@

{{ "weakPasswordsReportDesc" | i18n }}

-
- - {{ "loading" | i18n }} -
-
- - {{ "noWeakPasswords" | i18n }} - - - - {{ "weakPasswordsFoundReportDesc" | i18n: (ciphers.length | number) : vaultMsg }} - - - @if (showFilterToggle && !isAdminConsoleActive) { - @if (canDisplayToggleGroup()) { - - - - {{ getName(status) }} - {{ getCount(status) }} - - - - } @else { - - } - } - - - - - {{ "name" | i18n }} - - {{ "owner" | i18n }} - - - {{ "weakness" | i18n }} - - - - - - - - - {{ row.name }} - - - {{ row.name }} - - - - {{ "shared" | i18n }} - - - - {{ "attachments" | i18n }} - -
- {{ row.subTitle }} - - - + + {{ "loading" | i18n }} +
+ } @else { +
+ @if (!ciphers.length) { + + {{ "noWeakPasswords" | i18n }} + + } @else { + + {{ "weakPasswordsFoundReportDesc" | i18n: (ciphers.length | number) : vaultMsg }} + + @if (showFilterToggle && !isAdminConsoleActive) { + @if (canDisplayToggleGroup()) { + - - - - - {{ row.reportValue.label | i18n }} - - - - - -
+ @for (status of filterStatus; track status) { + + {{ getName(status) }} + {{ getCount(status) }} + + } + + } @else { + + } + } + + + + {{ "name" | i18n }} + @if (!isAdminConsoleActive) { + + {{ "owner" | i18n }} + + } + + {{ "weakness" | i18n }} + + + + + + + + @if (!organization || canManageCipher(row)) { + {{ row.name }} + } @else { + {{ row.name }} + } + @if (!organization && row.organizationId) { + + {{ "shared" | i18n }} + } + @if (row.hasAttachments) { + + {{ "attachments" | i18n }} + } +
+ {{ row.subTitle }} + + @if (!isAdminConsoleActive) { + + @if (!organization) { + + + } + + } + + + {{ row.reportValue.label | i18n }} + + +
+
+ } + + }
diff --git a/apps/web/src/app/dirt/reports/shared/report-list/report-list.component.html b/apps/web/src/app/dirt/reports/shared/report-list/report-list.component.html index 2a03bf78dd4..bba57882027 100644 --- a/apps/web/src/app/dirt/reports/shared/report-list/report-list.component.html +++ b/apps/web/src/app/dirt/reports/shared/report-list/report-list.component.html @@ -1,13 +1,15 @@
-
- -
+ @for (report of reports; track report) { +
+ +
+ }
diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/activity-card.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/activity-card.component.ts index e7c54bc81d0..111cf3e4d01 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/activity-card.component.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/activity-card.component.ts @@ -1,4 +1,3 @@ -import { CommonModule } from "@angular/common"; import { Component, EventEmitter, Input, Output } from "@angular/core"; import { Router } from "@angular/router"; @@ -10,7 +9,7 @@ import { ButtonModule, ButtonType, LinkModule, TypographyModule } from "@bitward @Component({ selector: "dirt-activity-card", templateUrl: "./activity-card.component.html", - imports: [CommonModule, TypographyModule, JslibModule, LinkModule, ButtonModule], + imports: [TypographyModule, JslibModule, LinkModule, ButtonModule], host: { class: "tw-box-border tw-bg-background tw-block tw-text-main tw-border-solid tw-border tw-border-secondary-300 tw-border [&:not(bit-layout_*)]:tw-rounded-lg tw-rounded-lg tw-p-6 tw-min-h-56 tw-overflow-hidden", diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/activity-cards/password-change-metric.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/activity-cards/password-change-metric.component.ts index 30e1db7b438..60b53f7405d 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/activity-cards/password-change-metric.component.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/activity-cards/password-change-metric.component.ts @@ -1,4 +1,3 @@ -import { CommonModule } from "@angular/common"; import { ChangeDetectionStrategy, Component, @@ -44,7 +43,7 @@ export type PasswordChangeView = (typeof PasswordChangeView)[keyof typeof Passwo @Component({ changeDetection: ChangeDetectionStrategy.OnPush, selector: "dirt-password-change-metric", - imports: [CommonModule, TypographyModule, JslibModule, ProgressModule, ButtonModule], + imports: [TypographyModule, JslibModule, ProgressModule, ButtonModule], templateUrl: "./password-change-metric.component.html", }) export class PasswordChangeMetricComponent implements OnInit { diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/application-review-dialog/assign-tasks-view.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/application-review-dialog/assign-tasks-view.component.ts index 15d927a7714..619858fdffe 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/application-review-dialog/assign-tasks-view.component.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/application-review-dialog/assign-tasks-view.component.ts @@ -1,4 +1,3 @@ -import { CommonModule } from "@angular/common"; import { ChangeDetectionStrategy, Component, input } from "@angular/core"; import { @@ -25,7 +24,6 @@ import { DarkImageSourceDirective } from "@bitwarden/vault"; selector: "dirt-assign-tasks-view", templateUrl: "./assign-tasks-view.component.html", imports: [ - CommonModule, ButtonModule, TypographyModule, I18nPipe, diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/application-review-dialog/new-applications-dialog.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/application-review-dialog/new-applications-dialog.component.ts index 4de8ecd9cd0..796c0acf220 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/application-review-dialog/new-applications-dialog.component.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/activity/application-review-dialog/new-applications-dialog.component.ts @@ -1,4 +1,3 @@ -import { CommonModule } from "@angular/common"; import { ChangeDetectionStrategy, Component, @@ -79,7 +78,6 @@ export type NewApplicationsDialogResultType = selector: "dirt-new-applications-dialog", templateUrl: "./new-applications-dialog.component.html", imports: [ - CommonModule, ButtonModule, DialogModule, TypographyModule, diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/empty-state-card.component.html b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/empty-state-card.component.html index b1eda08481a..59aa680fa4e 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/empty-state-card.component.html +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/empty-state-card.component.html @@ -6,12 +6,11 @@ {{ title() }} -
- {{ description() }} -
+ @if (description()) { +
+ {{ description() }} +
+ } @if (benefits().length > 0) {
@for (benefit of benefits(); track $index) { @@ -38,69 +37,74 @@
} -
- -
+ @if (buttonText() && buttonAction()) { +
+ +
+ } -
-
- @if (videoSrc()) { - - } @else if (icon()) { -
- +
+ @if (videoSrc()) { +
- } + > + } @else if (icon()) { +
+ +
+ } +
-
- -
-
- @if (videoSrc()) { - - } @else if (icon()) { -
- +
+ @if (videoSrc()) { +
- } + > + } @else if (icon()) { +
+ +
+ } +
-
+ } diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/empty-state-card.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/empty-state-card.component.ts index c28de5e9952..a9ad86dc67c 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/empty-state-card.component.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/empty-state-card.component.ts @@ -1,4 +1,3 @@ -import { CommonModule } from "@angular/common"; import { ChangeDetectionStrategy, Component, input, isDevMode, OnInit } from "@angular/core"; import { BitSvg } from "@bitwarden/assets/svg"; @@ -7,7 +6,7 @@ import { ButtonModule, SvgModule } from "@bitwarden/components"; @Component({ selector: "empty-state-card", templateUrl: "./empty-state-card.component.html", - imports: [CommonModule, SvgModule, ButtonModule], + imports: [SvgModule, ButtonModule], changeDetection: ChangeDetectionStrategy.OnPush, }) export class EmptyStateCardComponent implements OnInit { diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.html b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.html index dfbd49d95f7..2a783e6dcc2 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.html +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.html @@ -44,10 +44,11 @@
-
- {{ "reviewAtRiskPasswords" | i18n }} -
- @let isRunningReport = dataService.isGeneratingReport$ | async; + @if (appsCount > 0) { +
+ {{ "reviewAtRiskPasswords" | i18n }} +
+ }
@@ -62,7 +63,6 @@ } - - -
diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/app-table-row-scrollable.component.html b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/app-table-row-scrollable.component.html index 0494f77bd46..0a72c76a550 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/app-table-row-scrollable.component.html +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/app-table-row-scrollable.component.html @@ -12,28 +12,32 @@ {{ "totalMembers" | i18n }} - - - - - - - + @if (showRowCheckBox) { + + @if (!row.isMarkedAsCritical) { + + } + @if (row.isMarkedAsCritical) { + + } + + } + @if (!showRowCheckBox) { + + @if (row.isMarkedAsCritical) { + + } + + } - + @if (row.iconCipher) { + + } {{ row.memberCount }} - - - - - - - + @if (showRowMenuForCriticalApps) { + + + + + + + } diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/report-loading.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/report-loading.component.ts index f3cb89dff55..45b28dae470 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/report-loading.component.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/report-loading.component.ts @@ -1,4 +1,3 @@ -import { CommonModule } from "@angular/common"; import { Component, input } from "@angular/core"; import { JslibModule } from "@bitwarden/angular/jslib.module"; @@ -19,7 +18,7 @@ const ProgressStepConfig = Object.freeze({ // eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @Component({ selector: "dirt-report-loading", - imports: [CommonModule, JslibModule, ProgressModule], + imports: [JslibModule, ProgressModule], templateUrl: "./report-loading.component.html", }) export class ReportLoadingComponent { diff --git a/bitwarden_license/bit-web/src/app/dirt/organization-integrations/integration-grid/integration-grid.component.html b/bitwarden_license/bit-web/src/app/dirt/organization-integrations/integration-grid/integration-grid.component.html index 9e14023d21b..8127c6a0343 100644 --- a/bitwarden_license/bit-web/src/app/dirt/organization-integrations/integration-grid/integration-grid.component.html +++ b/bitwarden_license/bit-web/src/app/dirt/organization-integrations/integration-grid/integration-grid.component.html @@ -1,21 +1,22 @@ diff --git a/bitwarden_license/bit-web/src/app/dirt/organization-integrations/integrations.component.html b/bitwarden_license/bit-web/src/app/dirt/organization-integrations/integrations.component.html index a35df3677bb..14f20a0b71c 100644 --- a/bitwarden_license/bit-web/src/app/dirt/organization-integrations/integrations.component.html +++ b/bitwarden_license/bit-web/src/app/dirt/organization-integrations/integrations.component.html @@ -24,28 +24,32 @@ @if (organization?.useScim || organization?.useDirectory) { -
-

- {{ "scimIntegration" | i18n }} -

-

- {{ "scimIntegrationDescStart" | i18n }} - {{ "scimIntegration" | i18n }} - {{ "scimIntegrationDescEnd" | i18n }} -

- -
-
-

- {{ "bwdc" | i18n }} -

-

{{ "bwdcDesc" | i18n }}

- -
+ @if (organization?.useScim) { +
+

+ {{ "scimIntegration" | i18n }} +

+

+ {{ "scimIntegrationDescStart" | i18n }} + {{ "scimIntegration" | i18n }} + {{ "scimIntegrationDescEnd" | i18n }} +

+ +
+ } + @if (organization?.useDirectory) { +
+

+ {{ "bwdc" | i18n }} +

+

{{ "bwdcDesc" | i18n }}

+ +
+ }
} diff --git a/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/member-access-report.component.html b/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/member-access-report.component.html index 0200e206327..440e955a226 100644 --- a/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/member-access-report.component.html +++ b/bitwarden_license/bit-web/src/app/dirt/reports/member-access-report/member-access-report.component.html @@ -1,21 +1,17 @@ - + @let isLoading = isLoading$ | async; - + @if (!isLoading) { + + + }
@@ -24,7 +20,7 @@

- +@if (isLoading) {

{{ "loading" | i18n }}

-
- - - {{ "members" | i18n }} - {{ "groups" | i18n }} - {{ "collections" | i18n }} - {{ "items" | i18n }} - - - -
- -
- - -
- {{ row.email }} +} @else { + + + {{ "members" | i18n }} + {{ "groups" | i18n }} + + {{ "collections" | i18n }} + + {{ "items" | i18n }} + + + +
+ +
+ +
+ {{ row.email }} +
-
- - {{ row.groupsCount }} - {{ row.collectionsCount }} - {{ row.itemsCount }} - - + + {{ row.groupsCount }} + {{ row.collectionsCount }} + {{ row.itemsCount }} + + +} From fa5f62e1bd9e00c17e8e216bc85b317f1f065d35 Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Wed, 28 Jan 2026 16:00:56 -0500 Subject: [PATCH 2/6] Revert "[PM-26821] Improve macOS fullscreen ux (#16838)" (#18606) This reverts commit 05ca57d538240d48cc28553e9f2dafe95b717a5a. --- .../browser/browser-popup-utils.spec.ts | 64 ------------------- .../platform/browser/browser-popup-utils.ts | 23 +------ 2 files changed, 1 insertion(+), 86 deletions(-) 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); } /** From 3a232c92963a5c2216e4de2f5f12ff4aa1dff4e6 Mon Sep 17 00:00:00 2001 From: Alex <55413326+AlexRubik@users.noreply.github.com> Date: Wed, 28 Jan 2026 16:16:06 -0500 Subject: [PATCH 3/6] [PM-31348] phish cleanup - Address code review feedback from PR #18561 (Cursor-based phishing URL search) (#18638) --- .../phishing-detection/phishing-resources.ts | 4 -- .../services/phishing-data.service.spec.ts | 64 +++++++++++++++++- .../services/phishing-data.service.ts | 54 ++++----------- .../services/phishing-detection.service.ts | 66 ++++++------------- 4 files changed, 94 insertions(+), 94 deletions(-) diff --git a/apps/browser/src/dirt/phishing-detection/phishing-resources.ts b/apps/browser/src/dirt/phishing-detection/phishing-resources.ts index 6595104207a..88068987dd7 100644 --- a/apps/browser/src/dirt/phishing-detection/phishing-resources.ts +++ b/apps/browser/src/dirt/phishing-detection/phishing-resources.ts @@ -7,8 +7,6 @@ export type PhishingResource = { todayUrl: string; /** Matcher used to decide whether a given URL matches an entry from this resource */ match: (url: URL, entry: string) => boolean; - /** Whether to use the custom matcher. If false, only exact hasUrl lookups are used. Default: true */ - useCustomMatcher?: boolean; }; export const PhishingResourceType = Object.freeze({ @@ -58,8 +56,6 @@ export const PHISHING_RESOURCES: Record { if (!entry) { return false; 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 2d6c7a5a651..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 @@ -186,12 +186,74 @@ describe("PhishingDataService", () => { expect(result).toBe(false); expect(logService.error).toHaveBeenCalledWith( - "[PhishingDataService] IndexedDB lookup via hasUrl failed", + "[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; + } + }); }); describe("data updates", () => { 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 c34a94ecced..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,12 +157,8 @@ export class PhishingDataService { * @returns True if the URL is a known phishing web address, false otherwise */ async isPhishingWebAddress(url: URL): Promise { - this.logService.debug("[PhishingDataService] isPhishingWebAddress called for: " + url.href); - // Skip non-http(s) protocols - phishing database only contains web URLs - // This prevents expensive fallback checks for chrome://, about:, file://, etc. if (url.protocol !== "http:" && url.protocol !== "https:") { - this.logService.debug("[PhishingDataService] Skipping non-http(s) protocol: " + url.protocol); return false; } @@ -176,69 +176,37 @@ export class PhishingDataService { const urlHref = url.href; const urlWithoutTrailingSlash = urlHref.endsWith("/") ? urlHref.slice(0, -1) : null; - this.logService.debug("[PhishingDataService] Checking hasUrl on this string: " + urlHref); let hasUrl = await this.indexedDbService.hasUrl(urlHref); - // If not found and URL has trailing slash, try without it if (!hasUrl && urlWithoutTrailingSlash) { - this.logService.debug( - "[PhishingDataService] Checking hasUrl without trailing slash: " + - urlWithoutTrailingSlash, - ); hasUrl = await this.indexedDbService.hasUrl(urlWithoutTrailingSlash); } if (hasUrl) { - this.logService.info( - "[PhishingDataService] Found phishing web address through direct lookup: " + urlHref, - ); + 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 and enabled, use cursor-based search. - // This avoids loading all URLs into memory and allows early exit on first match. - // Can be disabled via useCustomMatcher: false for performance reasons. - if (resource && resource.match && resource.useCustomMatcher !== false) { + // Custom matcher is disabled for performance (see USE_CUSTOM_MATCHER) + if (resource && resource.match && PhishingDataService.USE_CUSTOM_MATCHER) { try { - this.logService.debug( - "[PhishingDataService] Starting cursor-based search for: " + url.href, - ); - const startTime = performance.now(); - const found = await this.indexedDbService.findMatchingUrl((entry) => resource.match(url, entry), ); - const endTime = performance.now(); - const duration = (endTime - startTime).toFixed(2); - this.logService.debug( - `[PhishingDataService] Cursor-based search completed in ${duration}ms for: ${url.href} (found: ${found})`, - ); - if (found) { - this.logService.info( - "[PhishingDataService] Found phishing web address through custom matcher: " + url.href, - ); - } else { - this.logService.debug( - "[PhishingDataService] No match found, returning false for: " + url.href, - ); + 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.debug( - "[PhishingDataService] Returning false due to error for: " + url.href, - ); + this.logService.error("[PhishingDataService] Custom matcher failed", err); return false; } } - this.logService.debug( - "[PhishingDataService] No custom matcher, returning false for: " + url.href, - ); + 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 6ca5bad8942..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 { - distinctUntilChanged, - EMPTY, - filter, - map, - merge, - mergeMap, - 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"; @@ -43,7 +33,6 @@ export class PhishingDetectionService { private static _tabUpdated$ = new Subject(); private static _ignoredHostnames = new Set(); private static _didInit = false; - private static _activeSearchCount = 0; static initialize( logService: LogService, @@ -64,7 +53,7 @@ export class PhishingDetectionService { tap((message) => logService.debug(`[PhishingDetectionService] user selected continue for ${message.url}`), ), - mergeMap(async (message) => { + switchMap(async (message) => { const url = new URL(message.url); this._ignoredHostnames.add(url.hostname); await BrowserApi.navigateTabToUrl(message.tabId, url); @@ -89,40 +78,25 @@ export class PhishingDetectionService { prev.ignored === curr.ignored, ), tap((event) => logService.debug(`[PhishingDetectionService] processing event:`, event)), - // Use mergeMap for parallel processing - each tab check runs independently - // Concurrency limit of 5 prevents overwhelming IndexedDB - mergeMap(async ({ tabId, url, ignored }) => { - this._activeSearchCount++; - const searchId = `${tabId}-${Date.now()}`; - logService.debug( - `[PhishingDetectionService] Search STARTED [${searchId}] for ${url.href} (active: ${this._activeSearchCount}/5)`, - ); - const startTime = performance.now(); - - try { - if (ignored) { - // The next time this host is visited, block again - this._ignoredHostnames.delete(url.hostname); - return; - } - const isPhishing = await phishingDataService.isPhishingWebAddress(url); - if (!isPhishing) { - return; - } - - const phishingWarningPage = new URL( - BrowserApi.getRuntimeURL("popup/index.html#/security/phishing-warning") + - `?phishingUrl=${url.toString()}`, - ); - await BrowserApi.navigateTabToUrl(tabId, phishingWarningPage); - } finally { - this._activeSearchCount--; - const duration = (performance.now() - startTime).toFixed(2); - logService.debug( - `[PhishingDetectionService] Search FINISHED [${searchId}] for ${url.href} in ${duration}ms (active: ${this._activeSearchCount}/5)`, - ); + // 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); + return; } - }, 5), + const isPhishing = await phishingDataService.isPhishingWebAddress(url); + if (!isPhishing) { + return; + } + + const phishingWarningPage = new URL( + BrowserApi.getRuntimeURL("popup/index.html#/security/phishing-warning") + + `?phishingUrl=${url.toString()}`, + ); + await BrowserApi.navigateTabToUrl(tabId, phishingWarningPage); + }), ); const onCancelCommand$ = messageListener From 1dfd68bf5702b0f977caf39d832fdc8ed6585d45 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 28 Jan 2026 16:18:03 -0500 Subject: [PATCH 4/6] [deps] Autofill: Update concurrently to v9.2.1 (#17540) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- package-lock.json | 31 ++++++++++++++++++++----------- package.json | 2 +- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/package-lock.json b/package-lock.json index bf0c5196364..59bd89afce4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -127,7 +127,7 @@ "base64-loader": "1.0.0", "browserslist": "4.28.1", "chromatic": "13.3.4", - "concurrently": "9.2.0", + "concurrently": "9.2.1", "copy-webpack-plugin": "13.0.1", "cross-env": "10.1.0", "css-loader": "7.1.2", @@ -20558,19 +20558,18 @@ } }, "node_modules/concurrently": { - "version": "9.2.0", - "resolved": "https://registry.npmjs.org/concurrently/-/concurrently-9.2.0.tgz", - "integrity": "sha512-IsB/fiXTupmagMW4MNp2lx2cdSN2FfZq78vF90LBB+zZHArbIQZjQtzXCiXnvTxCZSvXanTqFLWBjw2UkLx1SQ==", + "version": "9.2.1", + "resolved": "https://registry.npmjs.org/concurrently/-/concurrently-9.2.1.tgz", + "integrity": "sha512-fsfrO0MxV64Znoy8/l1vVIjjHa29SZyyqPgQBwhiDcaW8wJc2W3XWVOGx4M3oJBnv/zdUZIIp1gDeS98GzP8Ng==", "dev": true, "license": "MIT", "dependencies": { - "chalk": "^4.1.2", - "lodash": "^4.17.21", - "rxjs": "^7.8.1", - "shell-quote": "^1.8.1", - "supports-color": "^8.1.1", - "tree-kill": "^1.2.2", - "yargs": "^17.7.2" + "chalk": "4.1.2", + "rxjs": "7.8.2", + "shell-quote": "1.8.3", + "supports-color": "8.1.1", + "tree-kill": "1.2.2", + "yargs": "17.7.2" }, "bin": { "conc": "dist/bin/concurrently.js", @@ -20583,6 +20582,16 @@ "url": "https://github.com/open-cli-tools/concurrently?sponsor=1" } }, + "node_modules/concurrently/node_modules/rxjs": { + "version": "7.8.2", + "resolved": "https://registry.npmjs.org/rxjs/-/rxjs-7.8.2.tgz", + "integrity": "sha512-dhKf903U/PQZY6boNNtAGdWbG85WAbjT/1xYoZIC7FAY0yWapOBQVsVrDl58W86//e1VpMNBtRV4MaXfdMySFA==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "tslib": "^2.1.0" + } + }, "node_modules/concurrently/node_modules/supports-color": { "version": "8.1.1", "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-8.1.1.tgz", diff --git a/package.json b/package.json index 3fabb6af099..1cc4cabbceb 100644 --- a/package.json +++ b/package.json @@ -94,7 +94,7 @@ "base64-loader": "1.0.0", "browserslist": "4.28.1", "chromatic": "13.3.4", - "concurrently": "9.2.0", + "concurrently": "9.2.1", "copy-webpack-plugin": "13.0.1", "cross-env": "10.1.0", "css-loader": "7.1.2", From 9d8f1af62bf5986b39dc3b6425fcd0b4df6246f6 Mon Sep 17 00:00:00 2001 From: Vijay Oommen Date: Wed, 28 Jan 2026 15:19:39 -0600 Subject: [PATCH 5/6] PM-30539 created new component and added a filter (#18630) --- apps/web/src/locales/en/messages.json | 21 ++ .../applications.component.html | 128 ++++++++++ .../applications.component.ts | 221 ++++++++++++++++++ .../risk-insights.component.html | 5 + .../risk-insights.component.ts | 10 + libs/common/src/enums/feature-flag.enum.ts | 2 + 6 files changed, 387 insertions(+) create mode 100644 bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.html create mode 100644 bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.ts diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index ecb5f8d2dfc..872509a81c2 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -14,6 +14,24 @@ "noCriticalAppsAtRisk": { "message": "No critical applications at risk" }, + "critical":{ + "message": "Critical ($COUNT$)", + "placeholders": { + "count": { + "content": "$1", + "example": "3" + } + } + }, + "notCritical": { + "message": "Not critical ($COUNT$)", + "placeholders": { + "count": { + "content": "$1", + "example": "5" + } + } + }, "accessIntelligence": { "message": "Access Intelligence" }, @@ -250,6 +268,9 @@ "application": { "message": "Application" }, + "applications": { + "message": "Applications" + }, "atRiskPasswords": { "message": "At-risk passwords" }, diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.html b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.html new file mode 100644 index 00000000000..092cc4b73d8 --- /dev/null +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.html @@ -0,0 +1,128 @@ +@if ((dataService.reportStatus$ | async) == ReportStatusEnum.Loading) { + +} @else { + @let drawerDetails = dataService.drawerDetails$ | async; +
+

{{ "allApplications" | i18n }}

+
+
+
+ {{ + "atRiskMembers" | i18n + }} +
+ {{ applicationSummary().totalAtRiskMemberCount }} + {{ + "cardMetrics" | i18n: applicationSummary().totalMemberCount + }} +
+
+

+ +

+
+
+
+
+
+ {{ "atRiskApplications" | i18n }} +
+ {{ applicationSummary().totalAtRiskApplicationCount }} + {{ + "cardMetrics" | i18n: applicationSummary().totalApplicationCount + }} +
+
+

+ +

+
+
+
+
+
+ + + + + +
+ + +
+} diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.ts new file mode 100644 index 00000000000..0a393b26974 --- /dev/null +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.ts @@ -0,0 +1,221 @@ +import { + Component, + DestroyRef, + inject, + OnInit, + ChangeDetectionStrategy, + signal, + computed, +} from "@angular/core"; +import { takeUntilDestroyed, toObservable } from "@angular/core/rxjs-interop"; +import { FormControl, ReactiveFormsModule } from "@angular/forms"; +import { ActivatedRoute } from "@angular/router"; +import { combineLatest, debounceTime, startWith } from "rxjs"; + +import { Security } from "@bitwarden/assets/svg"; +import { RiskInsightsDataService } from "@bitwarden/bit-common/dirt/reports/risk-insights"; +import { createNewSummaryData } from "@bitwarden/bit-common/dirt/reports/risk-insights/helpers"; +import { + OrganizationReportSummary, + ReportStatus, +} from "@bitwarden/bit-common/dirt/reports/risk-insights/models/report-models"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { + ButtonModule, + IconButtonModule, + LinkModule, + NoItemsModule, + SearchModule, + TableDataSource, + ToastService, + TypographyModule, + ChipSelectComponent, +} from "@bitwarden/components"; +import { HeaderModule } from "@bitwarden/web-vault/app/layouts/header/header.module"; +import { SharedModule } from "@bitwarden/web-vault/app/shared"; +import { PipesModule } from "@bitwarden/web-vault/app/vault/individual-vault/pipes/pipes.module"; + +import { + ApplicationTableDataSource, + AppTableRowScrollableComponent, +} from "../shared/app-table-row-scrollable.component"; +import { ReportLoadingComponent } from "../shared/report-loading.component"; + +export const ApplicationFilterOption = { + All: "all", + Critical: "critical", + NonCritical: "nonCritical", +} as const; + +export type ApplicationFilterOption = + (typeof ApplicationFilterOption)[keyof typeof ApplicationFilterOption]; + +@Component({ + changeDetection: ChangeDetectionStrategy.OnPush, + selector: "dirt-applications", + templateUrl: "./applications.component.html", + imports: [ + ReportLoadingComponent, + HeaderModule, + LinkModule, + SearchModule, + PipesModule, + NoItemsModule, + SharedModule, + AppTableRowScrollableComponent, + IconButtonModule, + TypographyModule, + ButtonModule, + ReactiveFormsModule, + ChipSelectComponent, + ], +}) +export class ApplicationsComponent implements OnInit { + destroyRef = inject(DestroyRef); + + protected ReportStatusEnum = ReportStatus; + protected noItemsIcon = Security; + + // Standard properties + protected readonly dataSource = new TableDataSource(); + protected readonly searchControl = new FormControl("", { nonNullable: true }); + + // Template driven properties + protected readonly selectedUrls = signal(new Set()); + protected readonly markingAsCritical = signal(false); + protected readonly applicationSummary = signal(createNewSummaryData()); + protected readonly criticalApplicationsCount = signal(0); + protected readonly totalApplicationsCount = signal(0); + protected readonly nonCriticalApplicationsCount = computed(() => { + return this.totalApplicationsCount() - this.criticalApplicationsCount(); + }); + + // filter related properties + protected readonly selectedFilter = signal(ApplicationFilterOption.All); + protected selectedFilterObservable = toObservable(this.selectedFilter); + protected readonly ApplicationFilterOption = ApplicationFilterOption; + protected readonly filterOptions = computed(() => [ + { + label: this.i18nService.t("critical", this.criticalApplicationsCount()), + value: ApplicationFilterOption.Critical, + }, + { + label: this.i18nService.t("notCritical", this.nonCriticalApplicationsCount()), + value: ApplicationFilterOption.NonCritical, + }, + ]); + + constructor( + protected i18nService: I18nService, + protected activatedRoute: ActivatedRoute, + protected toastService: ToastService, + protected dataService: RiskInsightsDataService, + ) {} + + async ngOnInit() { + this.dataService.enrichedReportData$.pipe(takeUntilDestroyed(this.destroyRef)).subscribe({ + next: (report) => { + if (report != null) { + this.applicationSummary.set(report.summaryData); + + // Map the report data to include the iconCipher for each application + const tableDataWithIcon = report.reportData.map((app) => ({ + ...app, + iconCipher: + app.cipherIds.length > 0 + ? this.dataService.getCipherIcon(app.cipherIds[0]) + : undefined, + })); + this.dataSource.data = tableDataWithIcon; + this.totalApplicationsCount.set(report.reportData.length); + } else { + this.dataSource.data = []; + } + }, + error: () => { + this.dataSource.data = []; + }, + }); + + this.dataService.criticalReportResults$.pipe(takeUntilDestroyed(this.destroyRef)).subscribe({ + next: (criticalReport) => { + if (criticalReport != null) { + this.criticalApplicationsCount.set(criticalReport.reportData.length); + } else { + this.criticalApplicationsCount.set(0); + } + }, + }); + + combineLatest([ + this.searchControl.valueChanges.pipe(startWith("")), + this.selectedFilterObservable, + ]) + .pipe(debounceTime(200), takeUntilDestroyed(this.destroyRef)) + .subscribe(([searchText, selectedFilter]) => { + let filterFunction = (app: ApplicationTableDataSource) => true; + + if (selectedFilter === ApplicationFilterOption.Critical) { + filterFunction = (app) => app.isMarkedAsCritical; + } else if (selectedFilter === ApplicationFilterOption.NonCritical) { + filterFunction = (app) => !app.isMarkedAsCritical; + } + + this.dataSource.filter = (app) => + filterFunction(app) && + app.applicationName.toLowerCase().includes(searchText.toLowerCase()); + }); + } + + setFilterApplicationsByStatus(value: ApplicationFilterOption) { + this.selectedFilter.set(value); + } + + isMarkedAsCriticalItem(applicationName: string) { + return this.selectedUrls().has(applicationName); + } + + markAppsAsCritical = async () => { + this.markingAsCritical.set(true); + const count = this.selectedUrls().size; + + this.dataService + .saveCriticalApplications(Array.from(this.selectedUrls())) + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe({ + next: () => { + this.toastService.showToast({ + variant: "success", + title: "", + message: this.i18nService.t("criticalApplicationsMarkedSuccess", count.toString()), + }); + this.selectedUrls.set(new Set()); + this.markingAsCritical.set(false); + }, + error: () => { + this.toastService.showToast({ + variant: "error", + title: "", + message: this.i18nService.t("applicationsMarkedAsCriticalFail"), + }); + }, + }); + }; + + showAppAtRiskMembers = async (applicationName: string) => { + await this.dataService.setDrawerForAppAtRiskMembers(applicationName); + }; + + onCheckboxChange = (applicationName: string, event: Event) => { + const isChecked = (event.target as HTMLInputElement).checked; + this.selectedUrls.update((selectedUrls) => { + const nextSelected = new Set(selectedUrls); + if (isChecked) { + nextSelected.add(applicationName); + } else { + nextSelected.delete(applicationName); + } + return nextSelected; + }); + }; +} diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.html b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.html index 2a783e6dcc2..1e58d334288 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.html +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.html @@ -81,6 +81,11 @@ + @if (milestone11Enabled) { + + + + } diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.ts index b307c91d29f..657bdb87d4a 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.ts @@ -21,6 +21,8 @@ import { ReportStatus, RiskInsightsDataService, } from "@bitwarden/bit-common/dirt/reports/risk-insights"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { FileDownloadService } from "@bitwarden/common/platform/abstractions/file-download/file-download.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -38,6 +40,7 @@ import { HeaderModule } from "@bitwarden/web-vault/app/layouts/header/header.mod import { AllActivityComponent } from "./activity/all-activity.component"; import { AllApplicationsComponent } from "./all-applications/all-applications.component"; +import { ApplicationsComponent } from "./all-applications/applications.component"; import { CriticalApplicationsComponent } from "./critical-applications/critical-applications.component"; import { EmptyStateCardComponent } from "./empty-state-card.component"; import { RiskInsightsTabType } from "./models/risk-insights.models"; @@ -53,6 +56,7 @@ type ProgressStep = ReportProgress | null; templateUrl: "./risk-insights.component.html", imports: [ AllApplicationsComponent, + ApplicationsComponent, AsyncActionsModule, ButtonModule, CommonModule, @@ -77,6 +81,7 @@ type ProgressStep = ReportProgress | null; export class RiskInsightsComponent implements OnInit, OnDestroy { private destroyRef = inject(DestroyRef); protected ReportStatusEnum = ReportStatus; + protected milestone11Enabled: boolean = false; tabIndex: RiskInsightsTabType = RiskInsightsTabType.AllActivity; @@ -114,6 +119,7 @@ export class RiskInsightsComponent implements OnInit, OnDestroy { protected dialogService: DialogService, private fileDownloadService: FileDownloadService, private logService: LogService, + private configService: ConfigService, ) { this.route.queryParams.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(({ tabIndex }) => { this.tabIndex = !isNaN(Number(tabIndex)) ? Number(tabIndex) : RiskInsightsTabType.AllActivity; @@ -121,6 +127,10 @@ export class RiskInsightsComponent implements OnInit, OnDestroy { } async ngOnInit() { + this.milestone11Enabled = await this.configService.getFeatureFlag( + FeatureFlag.Milestone11AppPageImprovements, + ); + this.route.paramMap .pipe( takeUntilDestroyed(this.destroyRef), diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 244bd80d1fa..ac5f3c10260 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -59,6 +59,7 @@ export enum FeatureFlag { EventManagementForDataDogAndCrowdStrike = "event-management-for-datadog-and-crowdstrike", EventManagementForHuntress = "event-management-for-huntress", PhishingDetection = "phishing-detection", + Milestone11AppPageImprovements = "pm-30538-dirt-milestone-11-app-page-improvements", /* Vault */ PM19941MigrateCipherDomainToSdk = "pm-19941-migrate-cipher-domain-to-sdk", @@ -121,6 +122,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.EventManagementForDataDogAndCrowdStrike]: FALSE, [FeatureFlag.EventManagementForHuntress]: FALSE, [FeatureFlag.PhishingDetection]: FALSE, + [FeatureFlag.Milestone11AppPageImprovements]: FALSE, /* Vault */ [FeatureFlag.CipherKeyEncryption]: FALSE, From 0740c037a66ee140506a1d252a34abf0ffc92239 Mon Sep 17 00:00:00 2001 From: John Harrington <84741727+harr1424@users.noreply.github.com> Date: Wed, 28 Jan 2026 14:31:48 -0700 Subject: [PATCH 6/6] [PM-30922] Client changes to encrypt send access email list (#18486) --- .../browser/src/background/main.background.ts | 2 + apps/cli/src/register-oss-programs.ts | 2 +- .../service-container/service-container.ts | 2 + .../send/commands/create.command.spec.ts | 386 +++++++++++++++++ .../src/tools/send/commands/create.command.ts | 21 +- .../tools/send/commands/edit.command.spec.ts | 400 ++++++++++++++++++ .../src/tools/send/commands/edit.command.ts | 29 +- .../src/tools/send/models/send.response.ts | 5 + apps/cli/src/tools/send/send.program.ts | 48 ++- .../src/services/jslib-services.module.ts | 2 + .../src/tools/send/models/data/send.data.ts | 5 +- .../src/tools/send/models/domain/send.spec.ts | 286 ++++++++++++- .../src/tools/send/models/domain/send.ts | 21 +- .../send/models/request/send.request.spec.ts | 192 +++++++++ .../tools/send/models/request/send.request.ts | 4 +- .../send/models/response/send.response.ts | 8 +- .../src/tools/send/models/view/send.view.ts | 3 +- .../tools/send/services/send-api.service.ts | 1 + .../tools/send/services/send.service.spec.ts | 260 +++++++++++- .../src/tools/send/services/send.service.ts | 67 ++- .../services/test-data/send-tests.data.ts | 7 + 21 files changed, 1685 insertions(+), 66 deletions(-) create mode 100644 apps/cli/src/tools/send/commands/create.command.spec.ts create mode 100644 apps/cli/src/tools/send/commands/edit.command.spec.ts create mode 100644 libs/common/src/tools/send/models/request/send.request.spec.ts 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/cli/src/register-oss-programs.ts b/apps/cli/src/register-oss-programs.ts index 71d7aaa0d52..f0b0475c808 100644 --- a/apps/cli/src/register-oss-programs.ts +++ b/apps/cli/src/register-oss-programs.ts @@ -18,5 +18,5 @@ export async function registerOssPrograms(serviceContainer: ServiceContainer) { await vaultProgram.register(); const sendProgram = new SendProgram(serviceContainer); - sendProgram.register(); + await sendProgram.register(); } diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index 7bb8da27040..3e78eb36577 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -608,6 +608,8 @@ export class ServiceContainer { this.keyGenerationService, this.sendStateProvider, this.encryptService, + this.cryptoFunctionService, + this.configService, ); this.cipherFileUploadService = new CipherFileUploadService( diff --git a/apps/cli/src/tools/send/commands/create.command.spec.ts b/apps/cli/src/tools/send/commands/create.command.spec.ts new file mode 100644 index 00000000000..d3702689812 --- /dev/null +++ b/apps/cli/src/tools/send/commands/create.command.spec.ts @@ -0,0 +1,386 @@ +// FIXME: Update this file to be type safe and remove this and next line +// @ts-strict-ignore +import { mock } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; +import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; +import { mockAccountInfoWith } from "@bitwarden/common/spec"; +import { SendApiService } from "@bitwarden/common/tools/send/services/send-api.service.abstraction"; +import { SendService } from "@bitwarden/common/tools/send/services/send.service.abstraction"; +import { AuthType } from "@bitwarden/common/tools/send/types/auth-type"; +import { SendType } from "@bitwarden/common/tools/send/types/send-type"; +import { UserId } from "@bitwarden/user-core"; + +import { SendCreateCommand } from "./create.command"; + +describe("SendCreateCommand", () => { + let command: SendCreateCommand; + + const sendService = mock(); + const environmentService = mock(); + const sendApiService = mock(); + const accountProfileService = mock(); + const accountService = mock(); + + const activeAccount = { + id: "user-id" as UserId, + ...mockAccountInfoWith({ + email: "user@example.com", + name: "User", + }), + }; + + beforeEach(() => { + jest.clearAllMocks(); + + accountService.activeAccount$ = of(activeAccount); + accountProfileService.hasPremiumFromAnySource$.mockReturnValue(of(false)); + environmentService.environment$ = of({ + getWebVaultUrl: () => "https://vault.bitwarden.com", + } as any); + + command = new SendCreateCommand( + sendService, + environmentService, + sendApiService, + accountProfileService, + accountService, + ); + }); + + describe("authType inference", () => { + const futureDate = new Date(Date.now() + 7 * 24 * 60 * 60 * 1000); + + describe("with CLI flags", () => { + it("should set authType to Email when emails are provided via CLI", async () => { + const requestJson = { + type: SendType.Text, + text: { text: "test content", hidden: false }, + deletionDate: futureDate, + }; + + const cmdOptions = { + email: ["test@example.com"], + }; + + sendService.encrypt.mockResolvedValue([ + { id: "send-id", emails: "test@example.com", authType: AuthType.Email } as any, + null as any, + ]); + sendApiService.save.mockResolvedValue(undefined as any); + sendService.getFromState.mockResolvedValue({ + decrypt: jest.fn().mockResolvedValue({}), + } as any); + + const response = await command.run(requestJson, cmdOptions); + + expect(response.success).toBe(true); + expect(sendService.encrypt).toHaveBeenCalledWith( + expect.objectContaining({ + type: SendType.Text, + }), + null, + undefined, + ); + const savedCall = sendApiService.save.mock.calls[0][0]; + expect(savedCall[0].authType).toBe(AuthType.Email); + expect(savedCall[0].emails).toBe("test@example.com"); + }); + + it("should set authType to Password when password is provided via CLI", async () => { + const requestJson = { + type: SendType.Text, + text: { text: "test content", hidden: false }, + deletionDate: futureDate, + }; + + const cmdOptions = { + password: "testPassword123", + }; + + sendService.encrypt.mockResolvedValue([ + { id: "send-id", authType: AuthType.Password } as any, + null as any, + ]); + sendApiService.save.mockResolvedValue(undefined as any); + sendService.getFromState.mockResolvedValue({ + decrypt: jest.fn().mockResolvedValue({}), + } as any); + + const response = await command.run(requestJson, cmdOptions); + + expect(response.success).toBe(true); + expect(sendService.encrypt).toHaveBeenCalledWith( + expect.any(Object), + null as any, + "testPassword123", + ); + const savedCall = sendApiService.save.mock.calls[0][0]; + expect(savedCall[0].authType).toBe(AuthType.Password); + }); + + it("should set authType to None when neither emails nor password provided", async () => { + const requestJson = { + type: SendType.Text, + text: { text: "test content", hidden: false }, + deletionDate: futureDate, + }; + + const cmdOptions = {}; + + sendService.encrypt.mockResolvedValue([ + { id: "send-id", authType: AuthType.None } as any, + null as any, + ]); + sendApiService.save.mockResolvedValue(undefined as any); + sendService.getFromState.mockResolvedValue({ + decrypt: jest.fn().mockResolvedValue({}), + } as any); + + const response = await command.run(requestJson, cmdOptions); + + expect(response.success).toBe(true); + expect(sendService.encrypt).toHaveBeenCalledWith(expect.any(Object), null, undefined); + const savedCall = sendApiService.save.mock.calls[0][0]; + expect(savedCall[0].authType).toBe(AuthType.None); + }); + + it("should return error when both emails and password provided via CLI", async () => { + const requestJson = { + type: SendType.Text, + text: { text: "test content", hidden: false }, + deletionDate: futureDate, + }; + + const cmdOptions = { + email: ["test@example.com"], + password: "testPassword123", + }; + + const response = await command.run(requestJson, cmdOptions); + + expect(response.success).toBe(false); + expect(response.message).toBe("--password and --emails are mutually exclusive."); + }); + }); + + describe("with JSON input", () => { + it("should set authType to Email when emails provided in JSON", async () => { + const requestJson = { + type: SendType.Text, + text: { text: "test content", hidden: false }, + deletionDate: futureDate, + emails: ["test@example.com", "another@example.com"], + }; + + sendService.encrypt.mockResolvedValue([ + { + id: "send-id", + emails: "test@example.com,another@example.com", + authType: AuthType.Email, + } as any, + null as any, + ]); + sendApiService.save.mockResolvedValue(undefined as any); + sendService.getFromState.mockResolvedValue({ + decrypt: jest.fn().mockResolvedValue({}), + } as any); + + const response = await command.run(requestJson, {}); + + expect(response.success).toBe(true); + const savedCall = sendApiService.save.mock.calls[0][0]; + expect(savedCall[0].authType).toBe(AuthType.Email); + expect(savedCall[0].emails).toBe("test@example.com,another@example.com"); + }); + + it("should set authType to Password when password provided in JSON", async () => { + const requestJson = { + type: SendType.Text, + text: { text: "test content", hidden: false }, + deletionDate: futureDate, + password: "jsonPassword123", + }; + + sendService.encrypt.mockResolvedValue([ + { id: "send-id", authType: AuthType.Password } as any, + null as any, + ]); + sendApiService.save.mockResolvedValue(undefined as any); + sendService.getFromState.mockResolvedValue({ + decrypt: jest.fn().mockResolvedValue({}), + } as any); + + const response = await command.run(requestJson, {}); + + expect(response.success).toBe(true); + const savedCall = sendApiService.save.mock.calls[0][0]; + expect(savedCall[0].authType).toBe(AuthType.Password); + }); + + it("should return error when both emails and password provided in JSON", async () => { + const requestJson = { + type: SendType.Text, + text: { text: "test content", hidden: false }, + deletionDate: futureDate, + emails: ["test@example.com"], + password: "jsonPassword123", + }; + + const response = await command.run(requestJson, {}); + + expect(response.success).toBe(false); + expect(response.message).toBe("--password and --emails are mutually exclusive."); + }); + }); + + describe("with mixed CLI and JSON input", () => { + it("should return error when CLI emails combined with JSON password", async () => { + const requestJson = { + type: SendType.Text, + text: { text: "test content", hidden: false }, + deletionDate: futureDate, + password: "jsonPassword123", + }; + + const cmdOptions = { + email: ["cli@example.com"], + }; + + const response = await command.run(requestJson, cmdOptions); + + expect(response.success).toBe(false); + expect(response.message).toBe("--password and --emails are mutually exclusive."); + }); + + it("should return error when CLI password combined with JSON emails", async () => { + const requestJson = { + type: SendType.Text, + text: { text: "test content", hidden: false }, + deletionDate: futureDate, + emails: ["json@example.com"], + }; + + const cmdOptions = { + password: "cliPassword123", + }; + + const response = await command.run(requestJson, cmdOptions); + + expect(response.success).toBe(false); + expect(response.message).toBe("--password and --emails are mutually exclusive."); + }); + + it("should use CLI value when JSON has different value of same type", async () => { + const requestJson = { + type: SendType.Text, + text: { text: "test content", hidden: false }, + deletionDate: futureDate, + emails: ["json@example.com"], + }; + + const cmdOptions = { + email: ["cli@example.com"], + }; + + sendService.encrypt.mockResolvedValue([ + { id: "send-id", emails: "cli@example.com", authType: AuthType.Email } as any, + null as any, + ]); + sendApiService.save.mockResolvedValue(undefined as any); + sendService.getFromState.mockResolvedValue({ + decrypt: jest.fn().mockResolvedValue({}), + } as any); + + const response = await command.run(requestJson, cmdOptions); + + expect(response.success).toBe(true); + const savedCall = sendApiService.save.mock.calls[0][0]; + expect(savedCall[0].authType).toBe(AuthType.Email); + expect(savedCall[0].emails).toBe("cli@example.com"); + }); + }); + + describe("edge cases", () => { + it("should set authType to None when emails array is empty", async () => { + const requestJson = { + type: SendType.Text, + text: { text: "test content", hidden: false }, + deletionDate: futureDate, + emails: [] as string[], + }; + + sendService.encrypt.mockResolvedValue([ + { id: "send-id", authType: AuthType.None } as any, + null as any, + ]); + sendApiService.save.mockResolvedValue(undefined as any); + sendService.getFromState.mockResolvedValue({ + decrypt: jest.fn().mockResolvedValue({}), + } as any); + + const response = await command.run(requestJson, {}); + + expect(response.success).toBe(true); + const savedCall = sendApiService.save.mock.calls[0][0]; + expect(savedCall[0].authType).toBe(AuthType.None); + }); + + it("should set authType to None when password is empty string", async () => { + const requestJson = { + type: SendType.Text, + text: { text: "test content", hidden: false }, + deletionDate: futureDate, + }; + + const cmdOptions = { + password: "", + }; + + sendService.encrypt.mockResolvedValue([ + { id: "send-id", authType: AuthType.None } as any, + null as any, + ]); + sendApiService.save.mockResolvedValue(undefined as any); + sendService.getFromState.mockResolvedValue({ + decrypt: jest.fn().mockResolvedValue({}), + } as any); + + const response = await command.run(requestJson, cmdOptions); + + expect(response.success).toBe(true); + const savedCall = sendApiService.save.mock.calls[0][0]; + expect(savedCall[0].authType).toBe(AuthType.None); + }); + + it("should set authType to None when password is whitespace only", async () => { + const requestJson = { + type: SendType.Text, + text: { text: "test content", hidden: false }, + deletionDate: futureDate, + }; + + const cmdOptions = { + password: " ", + }; + + sendService.encrypt.mockResolvedValue([ + { id: "send-id", authType: AuthType.None } as any, + null as any, + ]); + sendApiService.save.mockResolvedValue(undefined as any); + sendService.getFromState.mockResolvedValue({ + decrypt: jest.fn().mockResolvedValue({}), + } as any); + + const response = await command.run(requestJson, cmdOptions); + + expect(response.success).toBe(true); + const savedCall = sendApiService.save.mock.calls[0][0]; + expect(savedCall[0].authType).toBe(AuthType.None); + }); + }); + }); +}); diff --git a/apps/cli/src/tools/send/commands/create.command.ts b/apps/cli/src/tools/send/commands/create.command.ts index 91e579c26c1..ad4ff9c4e18 100644 --- a/apps/cli/src/tools/send/commands/create.command.ts +++ b/apps/cli/src/tools/send/commands/create.command.ts @@ -11,6 +11,7 @@ import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abs import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { SendApiService } from "@bitwarden/common/tools/send/services/send-api.service.abstraction"; import { SendService } from "@bitwarden/common/tools/send/services/send.service.abstraction"; +import { AuthType } from "@bitwarden/common/tools/send/types/auth-type"; import { SendType } from "@bitwarden/common/tools/send/types/send-type"; import { NodeUtils } from "@bitwarden/node/node-utils"; @@ -18,7 +19,6 @@ import { Response } from "../../../models/response"; import { CliUtils } from "../../../utils"; import { SendTextResponse } from "../models/send-text.response"; import { SendResponse } from "../models/send.response"; - export class SendCreateCommand { constructor( private sendService: SendService, @@ -81,12 +81,24 @@ export class SendCreateCommand { const emails = req.emails ?? options.emails ?? undefined; const maxAccessCount = req.maxAccessCount ?? options.maxAccessCount; - if (emails !== undefined && password !== undefined) { + const hasEmails = emails != null && emails.length > 0; + const hasPassword = password != null && password.trim().length > 0; + + if (hasEmails && hasPassword) { return Response.badRequest("--password and --emails are mutually exclusive."); } req.key = null; req.maxAccessCount = maxAccessCount; + req.emails = emails; + + if (hasEmails) { + req.authType = AuthType.Email; + } else if (hasPassword) { + req.authType = AuthType.Password; + } else { + req.authType = AuthType.None; + } const hasPremium$ = this.accountService.activeAccount$.pipe( switchMap(({ id }) => this.accountProfileService.hasPremiumFromAnySource$(id)), @@ -136,11 +148,6 @@ export class SendCreateCommand { const sendView = SendResponse.toView(req); const [encSend, fileData] = await this.sendService.encrypt(sendView, fileBuffer, password); - // Add dates from template - encSend.deletionDate = sendView.deletionDate; - encSend.expirationDate = sendView.expirationDate; - encSend.emails = emails && emails.join(","); - await this.sendApiService.save([encSend, fileData]); const newSend = await this.sendService.getFromState(encSend.id); const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); diff --git a/apps/cli/src/tools/send/commands/edit.command.spec.ts b/apps/cli/src/tools/send/commands/edit.command.spec.ts new file mode 100644 index 00000000000..5bac63d3821 --- /dev/null +++ b/apps/cli/src/tools/send/commands/edit.command.spec.ts @@ -0,0 +1,400 @@ +// FIXME: Update this file to be type safe and remove this and next line +// @ts-strict-ignore +import { mock } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; +import { mockAccountInfoWith } from "@bitwarden/common/spec"; +import { SendView } from "@bitwarden/common/tools/send/models/view/send.view"; +import { SendApiService } from "@bitwarden/common/tools/send/services/send-api.service.abstraction"; +import { SendService } from "@bitwarden/common/tools/send/services/send.service.abstraction"; +import { AuthType } from "@bitwarden/common/tools/send/types/auth-type"; +import { SendType } from "@bitwarden/common/tools/send/types/send-type"; +import { UserId } from "@bitwarden/user-core"; + +import { Response } from "../../../models/response"; +import { SendResponse } from "../models/send.response"; + +import { SendEditCommand } from "./edit.command"; +import { SendGetCommand } from "./get.command"; + +describe("SendEditCommand", () => { + let command: SendEditCommand; + + const sendService = mock(); + const getCommand = mock(); + const sendApiService = mock(); + const accountProfileService = mock(); + const accountService = mock(); + + const activeAccount = { + id: "user-id" as UserId, + ...mockAccountInfoWith({ + email: "user@example.com", + name: "User", + }), + }; + + const mockSendId = "send-123"; + const mockSendView = { + id: mockSendId, + type: SendType.Text, + name: "Test Send", + text: { text: "test content", hidden: false }, + deletionDate: new Date(Date.now() + 7 * 24 * 60 * 60 * 1000), + } as SendView; + + const mockSend = { + id: mockSendId, + type: SendType.Text, + decrypt: jest.fn().mockResolvedValue(mockSendView), + }; + + const encodeRequest = (data: any) => Buffer.from(JSON.stringify(data)).toString("base64"); + + beforeEach(() => { + jest.clearAllMocks(); + + accountService.activeAccount$ = of(activeAccount); + accountProfileService.hasPremiumFromAnySource$.mockReturnValue(of(false)); + sendService.getFromState.mockResolvedValue(mockSend as any); + getCommand.run.mockResolvedValue(Response.success(new SendResponse(mockSendView)) as any); + + command = new SendEditCommand( + sendService, + getCommand, + sendApiService, + accountProfileService, + accountService, + ); + }); + + describe("authType inference", () => { + describe("with CLI flags", () => { + it("should set authType to Email when emails are provided via CLI", async () => { + const requestData = { + id: mockSendId, + type: SendType.Text, + name: "Test Send", + }; + const requestJson = encodeRequest(requestData); + + const cmdOptions = { + email: ["test@example.com"], + }; + + sendService.encrypt.mockResolvedValue([ + { id: mockSendId, emails: "test@example.com", authType: AuthType.Email } as any, + null as any, + ]); + sendApiService.save.mockResolvedValue(undefined as any); + + const response = await command.run(requestJson, cmdOptions); + + expect(response.success).toBe(true); + const savedCall = sendApiService.save.mock.calls[0][0]; + expect(savedCall[0].authType).toBe(AuthType.Email); + expect(savedCall[0].emails).toBe("test@example.com"); + }); + + it("should set authType to Password when password is provided via CLI", async () => { + const requestData = { + id: mockSendId, + type: SendType.Text, + name: "Test Send", + }; + const requestJson = encodeRequest(requestData); + + const cmdOptions = { + password: "testPassword123", + }; + + sendService.encrypt.mockResolvedValue([ + { id: mockSendId, authType: AuthType.Password } as any, + null as any, + ]); + sendApiService.save.mockResolvedValue(undefined as any); + + const response = await command.run(requestJson, cmdOptions); + + expect(response.success).toBe(true); + const savedCall = sendApiService.save.mock.calls[0][0]; + expect(savedCall[0].authType).toBe(AuthType.Password); + }); + + it("should set authType to None when neither emails nor password provided", async () => { + const requestData = { + id: mockSendId, + type: SendType.Text, + name: "Test Send", + }; + const requestJson = encodeRequest(requestData); + + const cmdOptions = {}; + + sendService.encrypt.mockResolvedValue([ + { id: mockSendId, authType: AuthType.None } as any, + null as any, + ]); + sendApiService.save.mockResolvedValue(undefined as any); + + const response = await command.run(requestJson, cmdOptions); + + expect(response.success).toBe(true); + const savedCall = sendApiService.save.mock.calls[0][0]; + expect(savedCall[0].authType).toBe(AuthType.None); + }); + + it("should return error when both emails and password provided via CLI", async () => { + const requestData = { + id: mockSendId, + type: SendType.Text, + name: "Test Send", + }; + const requestJson = encodeRequest(requestData); + + const cmdOptions = { + email: ["test@example.com"], + password: "testPassword123", + }; + + const response = await command.run(requestJson, cmdOptions); + + expect(response.success).toBe(false); + expect(response.message).toBe("--password and --emails are mutually exclusive."); + }); + }); + + describe("with JSON input", () => { + it("should set authType to Email when emails provided in JSON", async () => { + const requestData = { + id: mockSendId, + type: SendType.Text, + name: "Test Send", + emails: ["test@example.com", "another@example.com"], + }; + const requestJson = encodeRequest(requestData); + + sendService.encrypt.mockResolvedValue([ + { id: mockSendId, authType: AuthType.Email } as any, + null as any, + ]); + sendApiService.save.mockResolvedValue(undefined as any); + + const response = await command.run(requestJson, {}); + + expect(response.success).toBe(true); + const savedCall = sendApiService.save.mock.calls[0][0]; + expect(savedCall[0].authType).toBe(AuthType.Email); + }); + + it("should set authType to Password when password provided in JSON", async () => { + const requestData = { + id: mockSendId, + type: SendType.Text, + name: "Test Send", + password: "jsonPassword123", + }; + const requestJson = encodeRequest(requestData); + + sendService.encrypt.mockResolvedValue([ + { id: mockSendId, authType: AuthType.Password } as any, + null as any, + ]); + sendApiService.save.mockResolvedValue(undefined as any); + + const response = await command.run(requestJson, {}); + + expect(response.success).toBe(true); + const savedCall = sendApiService.save.mock.calls[0][0]; + expect(savedCall[0].authType).toBe(AuthType.Password); + }); + + it("should return error when both emails and password provided in JSON", async () => { + const requestData = { + id: mockSendId, + type: SendType.Text, + name: "Test Send", + emails: ["test@example.com"], + password: "jsonPassword123", + }; + const requestJson = encodeRequest(requestData); + + const response = await command.run(requestJson, {}); + + expect(response.success).toBe(false); + expect(response.message).toBe("--password and --emails are mutually exclusive."); + }); + }); + + describe("with mixed CLI and JSON input", () => { + it("should return error when CLI emails combined with JSON password", async () => { + const requestData = { + id: mockSendId, + type: SendType.Text, + name: "Test Send", + password: "jsonPassword123", + }; + const requestJson = encodeRequest(requestData); + + const cmdOptions = { + email: ["cli@example.com"], + }; + + const response = await command.run(requestJson, cmdOptions); + + expect(response.success).toBe(false); + expect(response.message).toBe("--password and --emails are mutually exclusive."); + }); + + it("should return error when CLI password combined with JSON emails", async () => { + const requestData = { + id: mockSendId, + type: SendType.Text, + name: "Test Send", + emails: ["json@example.com"], + }; + const requestJson = encodeRequest(requestData); + + const cmdOptions = { + password: "cliPassword123", + }; + + const response = await command.run(requestJson, cmdOptions); + + expect(response.success).toBe(false); + expect(response.message).toBe("--password and --emails are mutually exclusive."); + }); + + it("should prioritize CLI value when JSON has different value of same type", async () => { + const requestData = { + id: mockSendId, + type: SendType.Text, + name: "Test Send", + emails: ["json@example.com"], + }; + const requestJson = encodeRequest(requestData); + + const cmdOptions = { + email: ["cli@example.com"], + }; + + sendService.encrypt.mockResolvedValue([ + { id: mockSendId, emails: "cli@example.com", authType: AuthType.Email } as any, + null as any, + ]); + sendApiService.save.mockResolvedValue(undefined as any); + + const response = await command.run(requestJson, cmdOptions); + + expect(response.success).toBe(true); + const savedCall = sendApiService.save.mock.calls[0][0]; + expect(savedCall[0].authType).toBe(AuthType.Email); + expect(savedCall[0].emails).toBe("cli@example.com"); + }); + }); + + describe("edge cases", () => { + it("should set authType to None when emails array is empty", async () => { + const requestData = { + id: mockSendId, + type: SendType.Text, + name: "Test Send", + emails: [] as string[], + }; + const requestJson = encodeRequest(requestData); + + sendService.encrypt.mockResolvedValue([ + { id: mockSendId, authType: AuthType.None } as any, + null as any, + ]); + sendApiService.save.mockResolvedValue(undefined as any); + + const response = await command.run(requestJson, {}); + + expect(response.success).toBe(true); + const savedCall = sendApiService.save.mock.calls[0][0]; + expect(savedCall[0].authType).toBe(AuthType.None); + }); + + it("should set authType to None when password is empty string", async () => { + const requestData = { + id: mockSendId, + type: SendType.Text, + name: "Test Send", + password: "", + }; + const requestJson = encodeRequest(requestData); + + sendService.encrypt.mockResolvedValue([ + { id: mockSendId, authType: AuthType.None } as any, + null as any, + ]); + sendApiService.save.mockResolvedValue(undefined as any); + + const response = await command.run(requestJson, {}); + + expect(response.success).toBe(true); + const savedCall = sendApiService.save.mock.calls[0][0]; + expect(savedCall[0].authType).toBe(AuthType.None); + }); + + it("should handle send not found", async () => { + sendService.getFromState.mockResolvedValue(null); + + const requestData = { + id: "nonexistent-id", + type: SendType.Text, + name: "Test Send", + }; + const requestJson = encodeRequest(requestData); + + const response = await command.run(requestJson, {}); + + expect(response.success).toBe(false); + }); + + it("should handle type mismatch", async () => { + const requestData = { + id: mockSendId, + type: SendType.File, + name: "Test Send", + }; + const requestJson = encodeRequest(requestData); + + const response = await command.run(requestJson, {}); + + expect(response.success).toBe(false); + expect(response.message).toBe("Cannot change a Send's type"); + }); + }); + }); + + describe("validation", () => { + it("should return error when requestJson is empty", async () => { + // Set BW_SERVE to prevent readStdin call + process.env.BW_SERVE = "true"; + + const response = await command.run("", {}); + + expect(response.success).toBe(false); + expect(response.message).toBe("`requestJson` was not provided."); + + delete process.env.BW_SERVE; + }); + + it("should return error when id is not provided", async () => { + const requestData = { + type: SendType.Text, + name: "Test Send", + }; + const requestJson = encodeRequest(requestData); + + const response = await command.run(requestJson, {}); + + expect(response.success).toBe(false); + expect(response.message).toBe("`itemid` was not provided."); + }); + }); +}); diff --git a/apps/cli/src/tools/send/commands/edit.command.ts b/apps/cli/src/tools/send/commands/edit.command.ts index 2c6d41d66ac..0709a33b88f 100644 --- a/apps/cli/src/tools/send/commands/edit.command.ts +++ b/apps/cli/src/tools/send/commands/edit.command.ts @@ -7,6 +7,7 @@ import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { SendApiService } from "@bitwarden/common/tools/send/services/send-api.service.abstraction"; import { SendService } from "@bitwarden/common/tools/send/services/send.service.abstraction"; +import { AuthType } from "@bitwarden/common/tools/send/types/auth-type"; import { SendType } from "@bitwarden/common/tools/send/types/send-type"; import { Response } from "../../../models/response"; @@ -53,14 +54,30 @@ export class SendEditCommand { req.id = normalizedOptions.itemId || req.id; if (normalizedOptions.emails) { req.emails = normalizedOptions.emails; - req.password = undefined; - } else if (normalizedOptions.password) { - req.emails = undefined; + } + if (normalizedOptions.password) { req.password = normalizedOptions.password; - } else if (req.password && (typeof req.password !== "string" || req.password === "")) { + } + if (req.password && (typeof req.password !== "string" || req.password === "")) { req.password = undefined; } + // Infer authType based on emails/password (mutually exclusive) + const hasEmails = req.emails != null && req.emails.length > 0; + const hasPassword = req.password != null && req.password.trim() !== ""; + + if (hasEmails && hasPassword) { + return Response.badRequest("--password and --emails are mutually exclusive."); + } + + if (hasEmails) { + req.authType = AuthType.Email; + } else if (hasPassword) { + req.authType = AuthType.Password; + } else { + req.authType = AuthType.None; + } + if (!req.id) { return Response.error("`itemid` was not provided."); } @@ -90,10 +107,6 @@ export class SendEditCommand { try { const [encSend, encFileData] = await this.sendService.encrypt(sendView, null, req.password); - // Add dates from template - encSend.deletionDate = sendView.deletionDate; - encSend.expirationDate = sendView.expirationDate; - await this.sendApiService.save([encSend, encFileData]); } catch (e) { return Response.error(e); diff --git a/apps/cli/src/tools/send/models/send.response.ts b/apps/cli/src/tools/send/models/send.response.ts index b7655226be0..c8182cbfaf8 100644 --- a/apps/cli/src/tools/send/models/send.response.ts +++ b/apps/cli/src/tools/send/models/send.response.ts @@ -2,6 +2,7 @@ // @ts-strict-ignore import { Utils } from "@bitwarden/common/platform/misc/utils"; import { SendView } from "@bitwarden/common/tools/send/models/view/send.view"; +import { AuthType } from "@bitwarden/common/tools/send/types/auth-type"; import { SendType } from "@bitwarden/common/tools/send/types/send-type"; import { BaseResponse } from "../../../models/response/base.response"; @@ -54,6 +55,7 @@ export class SendResponse implements BaseResponse { view.emails = send.emails ?? []; view.disabled = send.disabled; view.hideEmail = send.hideEmail; + view.authType = send.authType; return view; } @@ -92,6 +94,7 @@ export class SendResponse implements BaseResponse { emails?: Array; disabled: boolean; hideEmail: boolean; + authType: AuthType; constructor(o?: SendView, webVaultUrl?: string) { if (o == null) { @@ -116,8 +119,10 @@ export class SendResponse implements BaseResponse { this.deletionDate = o.deletionDate; this.expirationDate = o.expirationDate; this.passwordSet = o.password != null; + this.emails = o.emails ?? []; this.disabled = o.disabled; this.hideEmail = o.hideEmail; + this.authType = o.authType; if (o.type === SendType.Text && o.text != null) { this.text = new SendTextResponse(o.text); diff --git a/apps/cli/src/tools/send/send.program.ts b/apps/cli/src/tools/send/send.program.ts index 869d77a379c..a84b6c15ead 100644 --- a/apps/cli/src/tools/send/send.program.ts +++ b/apps/cli/src/tools/send/send.program.ts @@ -6,6 +6,7 @@ import * as path from "path"; import * as chalk from "chalk"; import { program, Command, Option, OptionValues } from "commander"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { SendType } from "@bitwarden/common/tools/send/types/send-type"; @@ -31,13 +32,16 @@ import { parseEmail } from "./util"; const writeLn = CliUtils.writeLn; export class SendProgram extends BaseProgram { - register() { - program.addCommand(this.sendCommand()); + async register() { + const emailAuthEnabled = await this.serviceContainer.configService.getFeatureFlag( + FeatureFlag.SendEmailOTP, + ); + program.addCommand(this.sendCommand(emailAuthEnabled)); // receive is accessible both at `bw receive` and `bw send receive` program.addCommand(this.receiveCommand()); } - private sendCommand(): Command { + private sendCommand(emailAuthEnabled: boolean): Command { return new Command("send") .argument("", "The data to Send. Specify as a filepath with the --file option") .description( @@ -59,9 +63,7 @@ export class SendProgram extends BaseProgram { new Option( "--email ", "optional emails to access this Send. Can also be specified in JSON.", - ) - .argParser(parseEmail) - .hideHelp(), + ).argParser(parseEmail), ) .option("-a, --maxAccessCount ", "The amount of max possible accesses.") .option("--hidden", "Hide in web by default. Valid only if --file is not set.") @@ -78,11 +80,18 @@ export class SendProgram extends BaseProgram { .addCommand(this.templateCommand()) .addCommand(this.getCommand()) .addCommand(this.receiveCommand()) - .addCommand(this.createCommand()) - .addCommand(this.editCommand()) + .addCommand(this.createCommand(emailAuthEnabled)) + .addCommand(this.editCommand(emailAuthEnabled)) .addCommand(this.removePasswordCommand()) .addCommand(this.deleteCommand()) .action(async (data: string, options: OptionValues) => { + if (options.email) { + if (!emailAuthEnabled) { + this.processResponse(Response.error("The --email feature is not currently available.")); + return; + } + } + const encodedJson = this.makeSendJson(data, options); let response: Response; @@ -199,7 +208,7 @@ export class SendProgram extends BaseProgram { }); } - private createCommand(): Command { + private createCommand(emailAuthEnabled: any): Command { return new Command("create") .argument("[encodedJson]", "JSON object to upload. Can also be piped in through stdin.") .description("create a Send") @@ -215,6 +224,14 @@ export class SendProgram extends BaseProgram { .action(async (encodedJson: string, options: OptionValues, args: { parent: Command }) => { // subcommands inherit flags from their parent; they cannot override them const { fullObject = false, email = undefined, password = undefined } = args.parent.opts(); + + if (email) { + if (!emailAuthEnabled) { + this.processResponse(Response.error("The --email feature is not currently available.")); + return; + } + } + const mergedOptions = { ...options, fullObject: fullObject, @@ -227,7 +244,7 @@ export class SendProgram extends BaseProgram { }); } - private editCommand(): Command { + private editCommand(emailAuthEnabled: any): Command { return new Command("edit") .argument( "[encodedJson]", @@ -243,6 +260,14 @@ export class SendProgram extends BaseProgram { }) .action(async (encodedJson: string, options: OptionValues, args: { parent: Command }) => { await this.exitIfLocked(); + const { email = undefined, password = undefined } = args.parent.opts(); + if (email) { + if (!emailAuthEnabled) { + this.processResponse(Response.error("The --email feature is not currently available.")); + return; + } + } + const getCmd = new SendGetCommand( this.serviceContainer.sendService, this.serviceContainer.environmentService, @@ -259,8 +284,6 @@ export class SendProgram extends BaseProgram { this.serviceContainer.accountService, ); - // subcommands inherit flags from their parent; they cannot override them - const { email = undefined, password = undefined } = args.parent.opts(); const mergedOptions = { ...options, email, @@ -328,6 +351,7 @@ export class SendProgram extends BaseProgram { file: sendFile, text: sendText, type: type, + emails: options.email ?? undefined, }); return Buffer.from(JSON.stringify(template), "utf8").toString("base64"); diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 1ecf7fe3e3d..5a582626e68 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -858,6 +858,8 @@ const safeProviders: SafeProvider[] = [ KeyGenerationService, SendStateProviderAbstraction, EncryptService, + CryptoFunctionServiceAbstraction, + ConfigService, ], }), safeProvider({ diff --git a/libs/common/src/tools/send/models/data/send.data.ts b/libs/common/src/tools/send/models/data/send.data.ts index 7eeb15f3ebe..4081eba2878 100644 --- a/libs/common/src/tools/send/models/data/send.data.ts +++ b/libs/common/src/tools/send/models/data/send.data.ts @@ -11,7 +11,6 @@ export class SendData { id: string; accessId: string; type: SendType; - authType: AuthType; name: string; notes: string; file: SendFileData; @@ -24,8 +23,10 @@ export class SendData { deletionDate: string; password: string; emails: string; + emailHashes: string; disabled: boolean; hideEmail: boolean; + authType: AuthType; constructor(response?: SendResponse) { if (response == null) { @@ -46,8 +47,10 @@ export class SendData { this.deletionDate = response.deletionDate; this.password = response.password; this.emails = response.emails; + this.emailHashes = ""; this.disabled = response.disable; this.hideEmail = response.hideEmail; + this.authType = response.authType; switch (this.type) { case SendType.Text: diff --git a/libs/common/src/tools/send/models/domain/send.spec.ts b/libs/common/src/tools/send/models/domain/send.spec.ts index cd51390908e..f660333c917 100644 --- a/libs/common/src/tools/send/models/domain/send.spec.ts +++ b/libs/common/src/tools/send/models/domain/send.spec.ts @@ -1,6 +1,7 @@ import { mock } from "jest-mock-extended"; import { of } from "rxjs"; +import { Send } from "@bitwarden/common/tools/send/models/domain/send"; import { emptyGuid, UserId } from "@bitwarden/common/types/guid"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports @@ -15,7 +16,6 @@ import { AuthType } from "../../types/auth-type"; import { SendType } from "../../types/send-type"; import { SendData } from "../data/send.data"; -import { Send } from "./send"; import { SendText } from "./send-text"; describe("Send", () => { @@ -26,7 +26,6 @@ describe("Send", () => { id: "id", accessId: "accessId", type: SendType.Text, - authType: AuthType.None, name: "encName", notes: "encNotes", text: { @@ -41,9 +40,11 @@ describe("Send", () => { expirationDate: "2022-01-31T12:00:00.000Z", deletionDate: "2022-01-31T12:00:00.000Z", password: "password", - emails: null!, + emails: "", + emailHashes: "", disabled: false, hideEmail: true, + authType: AuthType.None, }; mockContainerService(); @@ -69,6 +70,8 @@ describe("Send", () => { expirationDate: null, deletionDate: null, password: undefined, + emails: null, + emailHashes: undefined, disabled: undefined, hideEmail: undefined, }); @@ -81,7 +84,6 @@ describe("Send", () => { id: "id", accessId: "accessId", type: SendType.Text, - authType: AuthType.None, name: { encryptedString: "encName", encryptionType: 0 }, notes: { encryptedString: "encNotes", encryptionType: 0 }, text: { @@ -95,9 +97,11 @@ describe("Send", () => { expirationDate: new Date("2022-01-31T12:00:00.000Z"), deletionDate: new Date("2022-01-31T12:00:00.000Z"), password: "password", - emails: null!, + emails: null, + emailHashes: "", disabled: false, hideEmail: true, + authType: AuthType.None, }); }); @@ -121,14 +125,22 @@ describe("Send", () => { send.expirationDate = new Date("2022-01-31T12:00:00.000Z"); send.deletionDate = new Date("2022-01-31T12:00:00.000Z"); send.password = "password"; + send.emails = null; send.disabled = false; send.hideEmail = true; + send.authType = AuthType.None; const encryptService = mock(); const keyService = mock(); encryptService.decryptBytes .calledWith(send.key, userKey) .mockResolvedValue(makeStaticByteArray(32)); + encryptService.decryptString + .calledWith(send.name, "cryptoKey" as any) + .mockResolvedValue("name"); + encryptService.decryptString + .calledWith(send.notes, "cryptoKey" as any) + .mockResolvedValue("notes"); keyService.makeSendKey.mockResolvedValue("cryptoKey" as any); keyService.userKey$.calledWith(userId).mockReturnValue(of(userKey)); @@ -137,12 +149,6 @@ describe("Send", () => { const view = await send.decrypt(userId); expect(text.decrypt).toHaveBeenNthCalledWith(1, "cryptoKey"); - expect(send.name.decrypt).toHaveBeenNthCalledWith( - 1, - null, - "cryptoKey", - "Property: name; ObjectContext: No Domain Context", - ); expect(view).toMatchObject({ id: "id", @@ -150,7 +156,6 @@ describe("Send", () => { name: "name", notes: "notes", type: 0, - authType: 2, key: expect.anything(), cryptoKey: "cryptoKey", file: expect.anything(), @@ -161,8 +166,265 @@ describe("Send", () => { expirationDate: new Date("2022-01-31T12:00:00.000Z"), deletionDate: new Date("2022-01-31T12:00:00.000Z"), password: "password", + emails: [], disabled: false, hideEmail: true, + authType: AuthType.None, + }); + }); + + describe("Email decryption", () => { + let encryptService: jest.Mocked; + let keyService: jest.Mocked; + const userKey = new SymmetricCryptoKey(new Uint8Array(32)) as UserKey; + const userId = emptyGuid as UserId; + + beforeEach(() => { + encryptService = mock(); + keyService = mock(); + encryptService.decryptBytes.mockResolvedValue(makeStaticByteArray(32)); + keyService.makeSendKey.mockResolvedValue("cryptoKey" as any); + keyService.userKey$.mockReturnValue(of(userKey)); + (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService); + }); + + it("should decrypt and parse single email", async () => { + const send = new Send(); + send.id = "id"; + send.type = SendType.Text; + send.name = mockEnc("name"); + send.notes = mockEnc("notes"); + send.key = mockEnc("key"); + send.emails = mockEnc("test@example.com"); + send.text = mock(); + send.text.decrypt = jest.fn().mockResolvedValue("textView" as any); + + encryptService.decryptString.mockImplementation((encString, key) => { + if (encString === send.emails) { + return Promise.resolve("test@example.com"); + } + if (encString === send.name) { + return Promise.resolve("name"); + } + if (encString === send.notes) { + return Promise.resolve("notes"); + } + return Promise.resolve(""); + }); + + const view = await send.decrypt(userId); + + expect(encryptService.decryptString).toHaveBeenCalledWith(send.emails, "cryptoKey"); + expect(view.emails).toEqual(["test@example.com"]); + }); + + it("should decrypt and parse multiple emails", async () => { + const send = new Send(); + send.id = "id"; + send.type = SendType.Text; + send.name = mockEnc("name"); + send.notes = mockEnc("notes"); + send.key = mockEnc("key"); + send.emails = mockEnc("test@example.com,user@test.com,admin@domain.com"); + send.text = mock(); + send.text.decrypt = jest.fn().mockResolvedValue("textView" as any); + + encryptService.decryptString.mockImplementation((encString, key) => { + if (encString === send.emails) { + return Promise.resolve("test@example.com,user@test.com,admin@domain.com"); + } + if (encString === send.name) { + return Promise.resolve("name"); + } + if (encString === send.notes) { + return Promise.resolve("notes"); + } + return Promise.resolve(""); + }); + + const view = await send.decrypt(userId); + + expect(view.emails).toEqual(["test@example.com", "user@test.com", "admin@domain.com"]); + }); + + it("should trim whitespace from decrypted emails", async () => { + const send = new Send(); + send.id = "id"; + send.type = SendType.Text; + send.name = mockEnc("name"); + send.notes = mockEnc("notes"); + send.key = mockEnc("key"); + send.emails = mockEnc(" test@example.com , user@test.com "); + send.text = mock(); + send.text.decrypt = jest.fn().mockResolvedValue("textView" as any); + + encryptService.decryptString.mockImplementation((encString, key) => { + if (encString === send.emails) { + return Promise.resolve(" test@example.com , user@test.com "); + } + if (encString === send.name) { + return Promise.resolve("name"); + } + if (encString === send.notes) { + return Promise.resolve("notes"); + } + return Promise.resolve(""); + }); + + const view = await send.decrypt(userId); + + expect(view.emails).toEqual(["test@example.com", "user@test.com"]); + }); + + it("should return empty array when emails is null", async () => { + const send = new Send(); + send.id = "id"; + send.type = SendType.Text; + send.name = mockEnc("name"); + send.notes = mockEnc("notes"); + send.key = mockEnc("key"); + send.emails = null; + send.text = mock(); + send.text.decrypt = jest.fn().mockResolvedValue("textView" as any); + + const view = await send.decrypt(userId); + + expect(view.emails).toEqual([]); + expect(encryptService.decryptString).not.toHaveBeenCalledWith(expect.anything(), "cryptoKey"); + }); + + it("should return empty array when decrypted emails is empty string", async () => { + const send = new Send(); + send.id = "id"; + send.type = SendType.Text; + send.name = mockEnc("name"); + send.notes = mockEnc("notes"); + send.key = mockEnc("key"); + send.emails = mockEnc(""); + send.text = mock(); + send.text.decrypt = jest.fn().mockResolvedValue("textView" as any); + + encryptService.decryptString.mockImplementation((encString, key) => { + if (encString === send.emails) { + return Promise.resolve(""); + } + if (encString === send.name) { + return Promise.resolve("name"); + } + if (encString === send.notes) { + return Promise.resolve("notes"); + } + return Promise.resolve(""); + }); + + const view = await send.decrypt(userId); + + expect(view.emails).toEqual([]); + }); + + it("should return empty array when decrypted emails is null", async () => { + const send = new Send(); + send.id = "id"; + send.type = SendType.Text; + send.name = mockEnc("name"); + send.notes = mockEnc("notes"); + send.key = mockEnc("key"); + send.emails = mockEnc("something"); + send.text = mock(); + send.text.decrypt = jest.fn().mockResolvedValue("textView" as any); + + encryptService.decryptString.mockImplementation((encString, key) => { + if (encString === send.emails) { + return Promise.resolve(null); + } + if (encString === send.name) { + return Promise.resolve("name"); + } + if (encString === send.notes) { + return Promise.resolve("notes"); + } + return Promise.resolve(""); + }); + + const view = await send.decrypt(userId); + + expect(view.emails).toEqual([]); + }); + }); + + describe("Null handling for name and notes decryption", () => { + let encryptService: jest.Mocked; + let keyService: jest.Mocked; + const userKey = new SymmetricCryptoKey(new Uint8Array(32)) as UserKey; + const userId = emptyGuid as UserId; + + beforeEach(() => { + encryptService = mock(); + keyService = mock(); + encryptService.decryptBytes.mockResolvedValue(makeStaticByteArray(32)); + keyService.makeSendKey.mockResolvedValue("cryptoKey" as any); + keyService.userKey$.mockReturnValue(of(userKey)); + (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService); + }); + + it("should return null for name when name is null", async () => { + const send = new Send(); + send.id = "id"; + send.type = SendType.Text; + send.name = null; + send.notes = mockEnc("notes"); + send.key = mockEnc("key"); + send.emails = null; + send.text = mock(); + send.text.decrypt = jest.fn().mockResolvedValue("textView" as any); + + const view = await send.decrypt(userId); + + expect(view.name).toBeNull(); + expect(encryptService.decryptString).not.toHaveBeenCalledWith(null, expect.anything()); + }); + + it("should return null for notes when notes is null", async () => { + const send = new Send(); + send.id = "id"; + send.type = SendType.Text; + send.name = mockEnc("name"); + send.notes = null; + send.key = mockEnc("key"); + send.emails = null; + send.text = mock(); + send.text.decrypt = jest.fn().mockResolvedValue("textView" as any); + + const view = await send.decrypt(userId); + + expect(view.notes).toBeNull(); + }); + + it("should decrypt non-null name and notes", async () => { + const send = new Send(); + send.id = "id"; + send.type = SendType.Text; + send.name = mockEnc("Test Name"); + send.notes = mockEnc("Test Notes"); + send.key = mockEnc("key"); + send.emails = null; + send.text = mock(); + send.text.decrypt = jest.fn().mockResolvedValue("textView" as any); + + encryptService.decryptString.mockImplementation((encString, key) => { + if (encString === send.name) { + return Promise.resolve("Test Name"); + } + if (encString === send.notes) { + return Promise.resolve("Test Notes"); + } + return Promise.resolve(""); + }); + + const view = await send.decrypt(userId); + + expect(view.name).toBe("Test Name"); + expect(view.notes).toBe("Test Notes"); }); }); }); diff --git a/libs/common/src/tools/send/models/domain/send.ts b/libs/common/src/tools/send/models/domain/send.ts index 82c37a17528..5247d35c655 100644 --- a/libs/common/src/tools/send/models/domain/send.ts +++ b/libs/common/src/tools/send/models/domain/send.ts @@ -20,7 +20,6 @@ export class Send extends Domain { id: string; accessId: string; type: SendType; - authType: AuthType; name: EncString; notes: EncString; file: SendFile; @@ -32,9 +31,11 @@ export class Send extends Domain { expirationDate: Date; deletionDate: Date; password: string; - emails: string; + emails: EncString; + emailHashes: string; disabled: boolean; hideEmail: boolean; + authType: AuthType; constructor(obj?: SendData) { super(); @@ -51,6 +52,7 @@ export class Send extends Domain { name: null, notes: null, key: null, + emails: null, }, ["id", "accessId"], ); @@ -60,12 +62,13 @@ export class Send extends Domain { this.maxAccessCount = obj.maxAccessCount; this.accessCount = obj.accessCount; this.password = obj.password; - this.emails = obj.emails; + this.emailHashes = obj.emailHashes; this.disabled = obj.disabled; this.revisionDate = obj.revisionDate != null ? new Date(obj.revisionDate) : null; this.deletionDate = obj.deletionDate != null ? new Date(obj.deletionDate) : null; this.expirationDate = obj.expirationDate != null ? new Date(obj.expirationDate) : null; this.hideEmail = obj.hideEmail; + this.authType = obj.authType; switch (this.type) { case SendType.Text: @@ -91,8 +94,17 @@ export class Send extends Domain { // model.key is a seed used to derive a key, not a SymmetricCryptoKey model.key = await encryptService.decryptBytes(this.key, sendKeyEncryptionKey); model.cryptoKey = await keyService.makeSendKey(model.key); + model.name = + this.name != null ? await encryptService.decryptString(this.name, model.cryptoKey) : null; + model.notes = + this.notes != null ? await encryptService.decryptString(this.notes, model.cryptoKey) : null; - await this.decryptObj(this, model, ["name", "notes"], model.cryptoKey); + if (this.emails != null) { + const decryptedEmails = await encryptService.decryptString(this.emails, model.cryptoKey); + model.emails = decryptedEmails ? decryptedEmails.split(",").map((e) => e.trim()) : []; + } else { + model.emails = []; + } switch (this.type) { case SendType.File: @@ -121,6 +133,7 @@ export class Send extends Domain { key: EncString.fromJSON(obj.key), name: EncString.fromJSON(obj.name), notes: EncString.fromJSON(obj.notes), + emails: EncString.fromJSON(obj.emails), text: SendText.fromJSON(obj.text), file: SendFile.fromJSON(obj.file), revisionDate, diff --git a/libs/common/src/tools/send/models/request/send.request.spec.ts b/libs/common/src/tools/send/models/request/send.request.spec.ts new file mode 100644 index 00000000000..1daee1d01ff --- /dev/null +++ b/libs/common/src/tools/send/models/request/send.request.spec.ts @@ -0,0 +1,192 @@ +import { Send } from "@bitwarden/common/tools/send/models/domain/send"; + +import { EncString } from "../../../../key-management/crypto/models/enc-string"; +import { SendType } from "../../types/send-type"; +import { SendText } from "../domain/send-text"; + +import { SendRequest } from "./send.request"; + +describe("SendRequest", () => { + describe("constructor", () => { + it("should populate emails with encrypted string from Send.emails", () => { + const send = new Send(); + send.type = SendType.Text; + send.name = new EncString("encryptedName"); + send.notes = new EncString("encryptedNotes"); + send.key = new EncString("encryptedKey"); + send.emails = new EncString("encryptedEmailList"); + send.emailHashes = "HASH1,HASH2,HASH3"; + send.disabled = false; + send.hideEmail = false; + send.text = new SendText(); + send.text.text = new EncString("text"); + send.text.hidden = false; + + const request = new SendRequest(send); + + expect(request.emails).toBe("encryptedEmailList"); + }); + + it("should populate emailHashes from Send.emailHashes", () => { + const send = new Send(); + send.type = SendType.Text; + send.name = new EncString("encryptedName"); + send.notes = new EncString("encryptedNotes"); + send.key = new EncString("encryptedKey"); + send.emails = new EncString("encryptedEmailList"); + send.emailHashes = "HASH1,HASH2,HASH3"; + send.disabled = false; + send.hideEmail = false; + send.text = new SendText(); + send.text.text = new EncString("text"); + send.text.hidden = false; + + const request = new SendRequest(send); + + expect(request.emailHashes).toBe("HASH1,HASH2,HASH3"); + }); + + it("should set emails to null when Send.emails is null", () => { + const send = new Send(); + send.type = SendType.Text; + send.name = new EncString("encryptedName"); + send.notes = new EncString("encryptedNotes"); + send.key = new EncString("encryptedKey"); + send.emails = null; + send.emailHashes = ""; + send.disabled = false; + send.hideEmail = false; + send.text = new SendText(); + send.text.text = new EncString("text"); + send.text.hidden = false; + + const request = new SendRequest(send); + + expect(request.emails).toBeNull(); + expect(request.emailHashes).toBe(""); + }); + + it("should handle empty emailHashes", () => { + const send = new Send(); + send.type = SendType.Text; + send.name = new EncString("encryptedName"); + send.key = new EncString("encryptedKey"); + send.emails = null; + send.emailHashes = ""; + send.disabled = false; + send.hideEmail = false; + send.text = new SendText(); + send.text.text = new EncString("text"); + send.text.hidden = false; + + const request = new SendRequest(send); + + expect(request.emailHashes).toBe(""); + }); + + it("should not expose plaintext emails", () => { + const send = new Send(); + send.type = SendType.Text; + send.name = new EncString("encryptedName"); + send.key = new EncString("encryptedKey"); + send.emails = new EncString("2.encrypted|emaildata|here"); + send.emailHashes = "ABC123,DEF456"; + send.disabled = false; + send.hideEmail = false; + send.text = new SendText(); + send.text.text = new EncString("text"); + send.text.hidden = false; + + const request = new SendRequest(send); + + // Ensure the request contains the encrypted string format, not plaintext + expect(request.emails).toBe("2.encrypted|emaildata|here"); + expect(request.emails).not.toContain("@"); + }); + + it("should handle name being null", () => { + const send = new Send(); + send.type = SendType.Text; + send.name = null; + send.notes = new EncString("encryptedNotes"); + send.key = new EncString("encryptedKey"); + send.emails = null; + send.emailHashes = ""; + send.disabled = false; + send.hideEmail = false; + send.text = new SendText(); + send.text.text = new EncString("text"); + send.text.hidden = false; + + const request = new SendRequest(send); + + expect(request.name).toBeNull(); + }); + + it("should handle notes being null", () => { + const send = new Send(); + send.type = SendType.Text; + send.name = new EncString("encryptedName"); + send.notes = null; + send.key = new EncString("encryptedKey"); + send.emails = null; + send.emailHashes = ""; + send.disabled = false; + send.hideEmail = false; + send.text = new SendText(); + send.text.text = new EncString("text"); + send.text.hidden = false; + + const request = new SendRequest(send); + + expect(request.notes).toBeNull(); + }); + + it("should include fileLength when provided for text send", () => { + const send = new Send(); + send.type = SendType.Text; + send.name = new EncString("encryptedName"); + send.key = new EncString("encryptedKey"); + send.emails = null; + send.emailHashes = ""; + send.disabled = false; + send.hideEmail = false; + send.text = new SendText(); + send.text.text = new EncString("text"); + send.text.hidden = false; + + const request = new SendRequest(send, 1024); + + expect(request.fileLength).toBe(1024); + }); + }); + + describe("Email auth requirements", () => { + it("should create request with encrypted emails and plaintext emailHashes", () => { + // Setup: A Send with encrypted emails and computed hashes + const send = new Send(); + send.type = SendType.Text; + send.name = new EncString("encryptedName"); + send.key = new EncString("encryptedKey"); + send.emails = new EncString("2.encryptedEmailString|data"); + send.emailHashes = "A1B2C3D4,E5F6G7H8"; // Plaintext hashes + send.disabled = false; + send.hideEmail = false; + send.text = new SendText(); + send.text.text = new EncString("text"); + send.text.hidden = false; + + // Act: Create the request + const request = new SendRequest(send); + + // emails field contains encrypted value + expect(request.emails).toBe("2.encryptedEmailString|data"); + expect(request.emails).toContain("encrypted"); + + //emailHashes field contains plaintext comma-separated hashes + expect(request.emailHashes).toBe("A1B2C3D4,E5F6G7H8"); + expect(request.emailHashes).not.toContain("encrypted"); + expect(request.emailHashes.split(",")).toHaveLength(2); + }); + }); +}); diff --git a/libs/common/src/tools/send/models/request/send.request.ts b/libs/common/src/tools/send/models/request/send.request.ts index 902ca0a2c54..37590e40108 100644 --- a/libs/common/src/tools/send/models/request/send.request.ts +++ b/libs/common/src/tools/send/models/request/send.request.ts @@ -18,6 +18,7 @@ export class SendRequest { file: SendFileApi; password: string; emails: string; + emailHashes: string; disabled: boolean; hideEmail: boolean; @@ -31,7 +32,8 @@ export class SendRequest { this.deletionDate = send.deletionDate != null ? send.deletionDate.toISOString() : null; this.key = send.key != null ? send.key.encryptedString : null; this.password = send.password; - this.emails = send.emails; + this.emails = send.emails ? send.emails.encryptedString : null; + this.emailHashes = send.emailHashes; this.disabled = send.disabled; this.hideEmail = send.hideEmail; diff --git a/libs/common/src/tools/send/models/response/send.response.ts b/libs/common/src/tools/send/models/response/send.response.ts index 7a7885d5ae1..a51b1e8ac7a 100644 --- a/libs/common/src/tools/send/models/response/send.response.ts +++ b/libs/common/src/tools/send/models/response/send.response.ts @@ -1,8 +1,9 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore +import { AuthType } from "@bitwarden/common/tools/send/types/auth-type"; +import { SendType } from "@bitwarden/common/tools/send/types/send-type"; + import { BaseResponse } from "../../../../models/response/base.response"; -import { AuthType } from "../../types/auth-type"; -import { SendType } from "../../types/send-type"; import { SendFileApi } from "../api/send-file.api"; import { SendTextApi } from "../api/send-text.api"; @@ -10,7 +11,6 @@ export class SendResponse extends BaseResponse { id: string; accessId: string; type: SendType; - authType: AuthType; name: string; notes: string; file: SendFileApi; @@ -25,6 +25,7 @@ export class SendResponse extends BaseResponse { emails: string; disable: boolean; hideEmail: boolean; + authType: AuthType; constructor(response: any) { super(response); @@ -44,6 +45,7 @@ export class SendResponse extends BaseResponse { this.emails = this.getResponseProperty("Emails"); this.disable = this.getResponseProperty("Disabled") || false; this.hideEmail = this.getResponseProperty("HideEmail") || false; + this.authType = this.getResponseProperty("AuthType"); const text = this.getResponseProperty("Text"); if (text != null) { diff --git a/libs/common/src/tools/send/models/view/send.view.ts b/libs/common/src/tools/send/models/view/send.view.ts index d07de6d8293..150a649671b 100644 --- a/libs/common/src/tools/send/models/view/send.view.ts +++ b/libs/common/src/tools/send/models/view/send.view.ts @@ -19,7 +19,6 @@ export class SendView implements View { key: Uint8Array; cryptoKey: SymmetricCryptoKey; type: SendType = null; - authType: AuthType = null; text = new SendTextView(); file = new SendFileView(); maxAccessCount?: number = null; @@ -31,6 +30,7 @@ export class SendView implements View { emails: string[] = []; disabled = false; hideEmail = false; + authType: AuthType = null; constructor(s?: Send) { if (!s) { @@ -49,6 +49,7 @@ export class SendView implements View { this.disabled = s.disabled; this.password = s.password; this.hideEmail = s.hideEmail; + this.authType = s.authType; } get urlB64Key(): string { diff --git a/libs/common/src/tools/send/services/send-api.service.ts b/libs/common/src/tools/send/services/send-api.service.ts index f09117316d8..57004b6ff0e 100644 --- a/libs/common/src/tools/send/services/send-api.service.ts +++ b/libs/common/src/tools/send/services/send-api.service.ts @@ -189,6 +189,7 @@ export class SendApiService implements SendApiServiceAbstraction { private async upload(sendData: [Send, EncArrayBuffer]): Promise { const request = new SendRequest(sendData[0], sendData[1]?.buffer.byteLength); + let response: SendResponse; if (sendData[0].id == null) { if (sendData[0].type === SendType.Text) { diff --git a/libs/common/src/tools/send/services/send.service.spec.ts b/libs/common/src/tools/send/services/send.service.spec.ts index fb99ddbe3bc..1c587327098 100644 --- a/libs/common/src/tools/send/services/send.service.spec.ts +++ b/libs/common/src/tools/send/services/send.service.spec.ts @@ -1,6 +1,7 @@ import { mock } from "jest-mock-extended"; import { firstValueFrom, of } from "rxjs"; +import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports import { KeyService } from "@bitwarden/key-management"; @@ -16,6 +17,7 @@ import { import { KeyGenerationService } from "../../../key-management/crypto"; import { EncryptService } from "../../../key-management/crypto/abstractions/encrypt.service"; import { EncString } from "../../../key-management/crypto/models/enc-string"; +import { ConfigService } from "../../../platform/abstractions/config/config.service"; import { EnvironmentService } from "../../../platform/abstractions/environment.service"; import { I18nService } from "../../../platform/abstractions/i18n.service"; import { Utils } from "../../../platform/misc/utils"; @@ -29,6 +31,7 @@ import { SendTextApi } from "../models/api/send-text.api"; import { SendFileData } from "../models/data/send-file.data"; import { SendTextData } from "../models/data/send-text.data"; import { SendData } from "../models/data/send.data"; +import { SendTextView } from "../models/view/send-text.view"; import { SendView } from "../models/view/send.view"; import { SendType } from "../types/send-type"; @@ -48,7 +51,8 @@ describe("SendService", () => { const keyGenerationService = mock(); const encryptService = mock(); const environmentService = mock(); - + const cryptoFunctionService = mock(); + const configService = mock(); let sendStateProvider: SendStateProvider; let sendService: SendService; @@ -94,6 +98,8 @@ describe("SendService", () => { keyGenerationService, sendStateProvider, encryptService, + cryptoFunctionService, + configService, ); }); @@ -573,4 +579,256 @@ describe("SendService", () => { expect(sendsAfterDelete.length).toBe(0); }); }); + + describe("encrypt", () => { + let sendView: SendView; + const userKey = new SymmetricCryptoKey(new Uint8Array(32)) as UserKey; + const mockCryptoKey = new SymmetricCryptoKey(new Uint8Array(32)); + + beforeEach(() => { + sendView = new SendView(); + sendView.id = "sendId"; + sendView.type = SendType.Text; + sendView.name = "Test Send"; + sendView.notes = "Test Notes"; + const sendTextView = new SendTextView(); + sendTextView.text = "test text"; + sendTextView.hidden = false; + sendView.text = sendTextView; + sendView.key = new Uint8Array(16); + sendView.cryptoKey = mockCryptoKey; + sendView.maxAccessCount = 5; + sendView.disabled = false; + sendView.hideEmail = false; + sendView.deletionDate = new Date("2024-12-31"); + sendView.expirationDate = null; + + keyService.userKey$.mockReturnValue(of(userKey)); + keyService.makeSendKey.mockResolvedValue(mockCryptoKey); + encryptService.encryptBytes.mockResolvedValue({ encryptedString: "encryptedKey" } as any); + encryptService.encryptString.mockResolvedValue({ encryptedString: "encrypted" } as any); + }); + + describe("when SendEmailOTP feature flag is ON", () => { + beforeEach(() => { + configService.getFeatureFlag.mockResolvedValue(true); + cryptoFunctionService.hash.mockClear(); + }); + + describe("email encryption", () => { + it("should encrypt emails when email list is provided", async () => { + sendView.emails = ["test@example.com", "user@test.com"]; + cryptoFunctionService.hash.mockResolvedValue(new Uint8Array([0xab, 0xcd])); + + const [send] = await sendService.encrypt(sendView, null, null); + + expect(encryptService.encryptString).toHaveBeenCalledWith( + "test@example.com,user@test.com", + mockCryptoKey, + ); + expect(send.emails).toEqual({ encryptedString: "encrypted" }); + expect(send.password).toBeNull(); + }); + + it("should set emails to null when email list is empty", async () => { + sendView.emails = []; + + const [send] = await sendService.encrypt(sendView, null, null); + + expect(send.emails).toBeNull(); + expect(send.emailHashes).toBe(""); + }); + + it("should set emails to null when email list is null", async () => { + sendView.emails = null; + + const [send] = await sendService.encrypt(sendView, null, null); + + expect(send.emails).toBeNull(); + expect(send.emailHashes).toBe(""); + }); + + it("should set emails to null when email list is undefined", async () => { + sendView.emails = undefined; + + const [send] = await sendService.encrypt(sendView, null, null); + + expect(send.emails).toBeNull(); + expect(send.emailHashes).toBe(""); + }); + }); + + describe("email hashing", () => { + it("should hash emails using SHA-256 and return uppercase hex", async () => { + sendView.emails = ["test@example.com"]; + const mockHash = new Uint8Array([0xab, 0xcd, 0xef]); + + cryptoFunctionService.hash.mockResolvedValue(mockHash); + + const [send] = await sendService.encrypt(sendView, null, null); + + expect(cryptoFunctionService.hash).toHaveBeenCalledWith("test@example.com", "sha256"); + expect(send.emailHashes).toBe("ABCDEF"); + }); + + it("should hash multiple emails and return comma-separated hashes", async () => { + sendView.emails = ["test@example.com", "user@test.com"]; + const mockHash1 = new Uint8Array([0xab, 0xcd]); + const mockHash2 = new Uint8Array([0x12, 0x34]); + + cryptoFunctionService.hash + .mockResolvedValueOnce(mockHash1) + .mockResolvedValueOnce(mockHash2); + + const [send] = await sendService.encrypt(sendView, null, null); + + expect(cryptoFunctionService.hash).toHaveBeenCalledWith("test@example.com", "sha256"); + expect(cryptoFunctionService.hash).toHaveBeenCalledWith("user@test.com", "sha256"); + expect(send.emailHashes).toBe("ABCD,1234"); + }); + + it("should trim and lowercase emails before hashing", async () => { + sendView.emails = [" Test@Example.COM ", "USER@test.com"]; + const mockHash = new Uint8Array([0xff]); + + cryptoFunctionService.hash.mockResolvedValue(mockHash); + + await sendService.encrypt(sendView, null, null); + + expect(cryptoFunctionService.hash).toHaveBeenCalledWith("test@example.com", "sha256"); + expect(cryptoFunctionService.hash).toHaveBeenCalledWith("user@test.com", "sha256"); + }); + + it("should set emailHashes to empty string when no emails", async () => { + sendView.emails = []; + + const [send] = await sendService.encrypt(sendView, null, null); + + expect(send.emailHashes).toBe(""); + expect(cryptoFunctionService.hash).not.toHaveBeenCalled(); + }); + + it("should handle single email correctly", async () => { + sendView.emails = ["single@test.com"]; + const mockHash = new Uint8Array([0xa1, 0xb2, 0xc3]); + + cryptoFunctionService.hash.mockResolvedValue(mockHash); + + const [send] = await sendService.encrypt(sendView, null, null); + + expect(send.emailHashes).toBe("A1B2C3"); + }); + }); + + describe("emails and password mutual exclusivity", () => { + it("should set password to null when emails are provided", async () => { + sendView.emails = ["test@example.com"]; + + const [send] = await sendService.encrypt(sendView, null, "password123"); + + expect(send.emails).toBeDefined(); + expect(send.password).toBeNull(); + }); + + it("should set password when no emails are provided", async () => { + sendView.emails = []; + keyGenerationService.deriveKeyFromPassword.mockResolvedValue({ + keyB64: "hashedPassword", + } as any); + + const [send] = await sendService.encrypt(sendView, null, "password123"); + + expect(send.emails).toBeNull(); + expect(send.password).toBe("hashedPassword"); + }); + }); + }); + + describe("when SendEmailOTP feature flag is OFF", () => { + beforeEach(() => { + configService.getFeatureFlag.mockResolvedValue(false); + cryptoFunctionService.hash.mockClear(); + }); + + it("should NOT encrypt emails even when provided", async () => { + sendView.emails = ["test@example.com"]; + + const [send] = await sendService.encrypt(sendView, null, null); + + expect(send.emails).toBeNull(); + expect(send.emailHashes).toBe(""); + expect(cryptoFunctionService.hash).not.toHaveBeenCalled(); + }); + + it("should use password when provided and flag is OFF", async () => { + sendView.emails = []; + keyGenerationService.deriveKeyFromPassword.mockResolvedValue({ + keyB64: "hashedPassword", + } as any); + + const [send] = await sendService.encrypt(sendView, null, "password123"); + + expect(send.emails).toBeNull(); + expect(send.emailHashes).toBe(""); + expect(send.password).toBe("hashedPassword"); + }); + + it("should ignore emails and use password when both provided", async () => { + sendView.emails = ["test@example.com"]; + keyGenerationService.deriveKeyFromPassword.mockResolvedValue({ + keyB64: "hashedPassword", + } as any); + + const [send] = await sendService.encrypt(sendView, null, "password123"); + + expect(send.emails).toBeNull(); + expect(send.emailHashes).toBe(""); + expect(send.password).toBe("hashedPassword"); + expect(cryptoFunctionService.hash).not.toHaveBeenCalled(); + }); + + it("should set emails and password to null when neither provided", async () => { + sendView.emails = []; + + const [send] = await sendService.encrypt(sendView, null, null); + + expect(send.emails).toBeNull(); + expect(send.emailHashes).toBe(""); + expect(send.password).toBeUndefined(); + }); + }); + + describe("null handling for name and notes", () => { + it("should handle null name correctly", async () => { + sendView.name = null; + sendView.emails = []; + + const [send] = await sendService.encrypt(sendView, null, null); + + expect(send.name).toBeNull(); + }); + + it("should handle null notes correctly", async () => { + sendView.notes = null; + sendView.emails = []; + + const [send] = await sendService.encrypt(sendView, null, null); + + expect(send.notes).toBeNull(); + }); + + it("should encrypt non-null name and notes", async () => { + sendView.name = "Test Name"; + sendView.notes = "Test Notes"; + sendView.emails = []; + + const [send] = await sendService.encrypt(sendView, null, null); + + expect(encryptService.encryptString).toHaveBeenCalledWith("Test Name", mockCryptoKey); + expect(encryptService.encryptString).toHaveBeenCalledWith("Test Notes", mockCryptoKey); + expect(send.name).toEqual({ encryptedString: "encrypted" }); + expect(send.notes).toEqual({ encryptedString: "encrypted" }); + }); + }); + }); }); diff --git a/libs/common/src/tools/send/services/send.service.ts b/libs/common/src/tools/send/services/send.service.ts index c274d90146e..078e94b2563 100644 --- a/libs/common/src/tools/send/services/send.service.ts +++ b/libs/common/src/tools/send/services/send.service.ts @@ -7,9 +7,12 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv // eslint-disable-next-line no-restricted-imports import { PBKDF2KdfConfig, KeyService } from "@bitwarden/key-management"; +import { FeatureFlag } from "../../../enums/feature-flag.enum"; import { KeyGenerationService } from "../../../key-management/crypto"; +import { CryptoFunctionService } from "../../../key-management/crypto/abstractions/crypto-function.service"; import { EncryptService } from "../../../key-management/crypto/abstractions/encrypt.service"; import { EncString } from "../../../key-management/crypto/models/enc-string"; +import { ConfigService } from "../../../platform/abstractions/config/config.service"; import { I18nService } from "../../../platform/abstractions/i18n.service"; import { Utils } from "../../../platform/misc/utils"; import { EncArrayBuffer } from "../../../platform/models/domain/enc-array-buffer"; @@ -51,6 +54,8 @@ export class SendService implements InternalSendServiceAbstraction { private keyGenerationService: KeyGenerationService, private stateProvider: SendStateProvider, private encryptService: EncryptService, + private cryptoFunctionService: CryptoFunctionService, + private configService: ConfigService, ) {} async encrypt( @@ -80,19 +85,30 @@ export class SendService implements InternalSendServiceAbstraction { model.cryptoKey = key.derivedKey; } + // Check feature flag for email OTP authentication + const sendEmailOTPEnabled = await this.configService.getFeatureFlag(FeatureFlag.SendEmailOTP); + const hasEmails = (model.emails?.length ?? 0) > 0; - if (hasEmails) { - send.emails = model.emails.join(","); + + if (sendEmailOTPEnabled && hasEmails) { + const plaintextEmails = model.emails.join(","); + send.emails = await this.encryptService.encryptString(plaintextEmails, model.cryptoKey); + send.emailHashes = await this.hashEmails(plaintextEmails); send.password = null; - } else if (password != null) { - // Note: Despite being called key, the passwordKey is not used for encryption. - // It is used as a static proof that the client knows the password, and has the encryption key. - const passwordKey = await this.keyGenerationService.deriveKeyFromPassword( - password, - model.key, - new PBKDF2KdfConfig(SEND_KDF_ITERATIONS), - ); - send.password = passwordKey.keyB64; + } else { + send.emails = null; + send.emailHashes = ""; + + if (password != null) { + // Note: Despite being called key, the passwordKey is not used for encryption. + // It is used as a static proof that the client knows the password, and has the encryption key. + const passwordKey = await this.keyGenerationService.deriveKeyFromPassword( + password, + model.key, + new PBKDF2KdfConfig(SEND_KDF_ITERATIONS), + ); + send.password = passwordKey.keyB64; + } } const userId = (await firstValueFrom(this.accountService.activeAccount$)).id; if (userKey == null) { @@ -100,10 +116,14 @@ export class SendService implements InternalSendServiceAbstraction { } // Key is not a SymmetricCryptoKey, but key material used to derive the cryptoKey send.key = await this.encryptService.encryptBytes(model.key, userKey); - // FIXME: model.name can be null. encryptString should not be called with null values. - send.name = await this.encryptService.encryptString(model.name, model.cryptoKey); - // FIXME: model.notes can be null. encryptString should not be called with null values. - send.notes = await this.encryptService.encryptString(model.notes, model.cryptoKey); + send.name = + model.name != null + ? await this.encryptService.encryptString(model.name, model.cryptoKey) + : null; + send.notes = + model.notes != null + ? await this.encryptService.encryptString(model.notes, model.cryptoKey) + : null; if (send.type === SendType.Text) { send.text = new SendText(); // FIXME: model.text.text can be null. encryptString should not be called with null values. @@ -127,6 +147,8 @@ export class SendService implements InternalSendServiceAbstraction { } } + send.authType = model.authType; + return [send, fileData]; } @@ -371,4 +393,19 @@ export class SendService implements InternalSendServiceAbstraction { decryptedSends.sort(Utils.getSortFunction(this.i18nService, "name")); return decryptedSends; } + + private async hashEmails(emails: string): Promise { + if (!emails) { + return ""; + } + + const emailArray = emails.split(",").map((e) => e.trim().toLowerCase()); + const hashPromises = emailArray.map(async (email) => { + const hash: Uint8Array = await this.cryptoFunctionService.hash(email, "sha256"); + return Utils.fromBufferToHex(hash).toUpperCase(); + }); + + const hashes = await Promise.all(hashPromises); + return hashes.join(","); + } } diff --git a/libs/common/src/tools/send/services/test-data/send-tests.data.ts b/libs/common/src/tools/send/services/test-data/send-tests.data.ts index c1d04ab2926..9c4e121edc0 100644 --- a/libs/common/src/tools/send/services/test-data/send-tests.data.ts +++ b/libs/common/src/tools/send/services/test-data/send-tests.data.ts @@ -20,6 +20,7 @@ export function testSendViewData(id: string, name: string) { data.deletionDate = null; data.notes = "Notes!!"; data.key = null; + data.emails = []; return data; } @@ -39,6 +40,8 @@ export function createSendData(value: Partial = {}) { expirationDate: "2024-09-04", deletionDate: "2024-09-04", password: "password", + emails: "", + emailHashes: "", disabled: false, hideEmail: false, }; @@ -62,6 +65,8 @@ export function testSendData(id: string, name: string) { data.deletionDate = null; data.notes = "Notes!!"; data.key = null; + data.emails = ""; + data.emailHashes = ""; return data; } @@ -77,5 +82,7 @@ export function testSend(id: string, name: string) { data.deletionDate = null; data.notes = new EncString("Notes!!"); data.key = null; + data.emails = null; + data.emailHashes = ""; return data; }