From a0943c310004e7ad215cd9e786211a83f8f64366 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Mon, 8 Dec 2025 15:30:29 +0000 Subject: [PATCH 1/6] [PM-28252] Increase Bulk Reinvite limit for cloud (#17587) * Add IncreaseBulkReinviteLimitForCloud feature flag * Enhance PeopleTableDataSource with bulk operation limits and feature flag integration - Introduced a new feature flag to increase the bulk reinvite limit for cloud environments. - Added an observable to determine if the increased limit is enabled based on the feature flag and environment. - Updated the logic for enforcing checked user limits in bulk operations, allowing for a maximum of 4000 users when the feature flag is active. - Refactored the constructor to initialize the new observable and manage the maximum allowed checked count dynamically. * Add unit tests for PeopleTableDataSource to validate user limit enforcement and status counts * Refactor MembersComponent to integrate increased bulk limit feature - Added support for conditional user limit enforcement in bulk operations based on a feature flag. - Introduced new dependencies for ConfigService and EnvironmentService to manage configuration settings. - Updated methods to utilize the new getCheckedUsers function, which enforces limits when the feature is enabled. - Refactored data source initialization to accommodate the new logic for handling checked users. * Refactor enforceCheckedUserLimit method in PeopleTableDataSource to use filtered data for user limit enforcement and to keep checked the top rows. Removed unnecessary comments and improved readability. * Add bulk reinvite success messages to localization files This update introduces new localization keys for bulk reinvite success notifications, including a general success message and a limited success message that provides details on the number of users re-invited and those excluded due to limits. This enhances user feedback during bulk operations. * Enhance bulk reinvite functionality with toast notifications This update modifies the MembersComponent to display success messages via toast notifications when the feature flag for increased bulk limits is enabled. If the limit is exceeded, a detailed message is shown, otherwise a general success message is displayed. The legacy dialog is retained for cases when the feature flag is disabled, ensuring consistent user feedback during bulk reinvite operations. * Rename MaxBulkReinviteCount to CloudBulkReinviteLimit * Refactor user retrieval logic in MembersComponent to conditionally enforce bulk limits This update modifies the MembersComponent in both the admin console and provider management sections to replace the direct calls to getCheckedUsers() with a conditional check for increased bulk limit feature. If enabled, it enforces user limits; otherwise, it retrieves all checked users. The deprecated getCheckedUsers method has been removed to streamline the code. * Add constructor to MembersTableDataSource for improved dependency injection This update introduces a constructor to the MembersTableDataSource class in both the admin console and provider management sections, allowing for better dependency injection of ConfigService, EnvironmentService, and DestroyRef. This change enhances the overall structure and maintainability of the code. * Refactor PeopleTableDataSource and MembersComponent to implement new bulk limit logic This update modifies the PeopleTableDataSource to introduce a new property for managing increased bulk limits and refactors the MembersComponent to utilize this logic. The enforcement of user limits during bulk operations is now conditional based on the feature flag, allowing for a more flexible handling of user selections. Additionally, the method for limiting and unchecking excess users has been updated for clarity and efficiency. * Refactor PeopleTableDataSource tests to update limit enforcement logic This update modifies the test cases for the PeopleTableDataSource to reflect the new method for limiting and unchecking excess users. The method name has been changed from `enforceCheckedUserLimit` to `limitAndUncheckExcess`, and the tests have been adjusted accordingly to ensure they accurately validate the new functionality. Additionally, unnecessary tests have been removed to streamline the test suite. * Change CloudBulkReinviteLimit back to 4000 * Refactor MembersComponent to utilize new getCheckedUsersInVisibleOrder method This update modifies the MembersComponent to conditionally retrieve checked users based on the increased bulk limit feature. If enabled, it uses the new getCheckedUsersInVisibleOrder method to maintain visual consistency in the filtered/sorted table view. This change enhances the handling of user selections during bulk operations. * Refactor PeopleTableDataSource to use Signals for increased bulk limit feature and update related tests. Removed unused imports and dependencies on DestroyRef in MembersTableDataSource components. * Refactor MembersComponent to remove unused Signal for increased bulk limit and directly utilize dataSource method for feature flag checks. * Implement getCheckedUsersWithLimit method to streamline user retrieval based on feature flag; update MembersComponent to utilize this new method for bulk actions. --- .../common/people-table-data-source.spec.ts | 130 ++++++++++++++++++ .../common/people-table-data-source.ts | 86 +++++++++++- .../members/members.component.ts | 107 ++++++++++---- apps/web/src/locales/en/messages.json | 26 ++++ .../providers/manage/members.component.ts | 100 +++++++++++--- libs/common/src/enums/feature-flag.enum.ts | 2 + 6 files changed, 399 insertions(+), 52 deletions(-) create mode 100644 apps/web/src/app/admin-console/common/people-table-data-source.spec.ts diff --git a/apps/web/src/app/admin-console/common/people-table-data-source.spec.ts b/apps/web/src/app/admin-console/common/people-table-data-source.spec.ts new file mode 100644 index 00000000000..e9cf87a114d --- /dev/null +++ b/apps/web/src/app/admin-console/common/people-table-data-source.spec.ts @@ -0,0 +1,130 @@ +import { TestBed } from "@angular/core/testing"; +import { ReplaySubject } from "rxjs"; + +import { OrganizationUserStatusType } from "@bitwarden/common/admin-console/enums"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { + Environment, + EnvironmentService, +} from "@bitwarden/common/platform/abstractions/environment.service"; + +import { PeopleTableDataSource } from "./people-table-data-source"; + +interface MockUser { + id: string; + name: string; + email: string; + status: OrganizationUserStatusType; + checked?: boolean; +} + +class TestPeopleTableDataSource extends PeopleTableDataSource { + protected statusType = OrganizationUserStatusType; +} + +describe("PeopleTableDataSource", () => { + let dataSource: TestPeopleTableDataSource; + + const createMockUser = (id: string, checked: boolean = false): MockUser => ({ + id, + name: `User ${id}`, + email: `user${id}@example.com`, + status: OrganizationUserStatusType.Confirmed, + checked, + }); + + const createMockUsers = (count: number, checked: boolean = false): MockUser[] => { + return Array.from({ length: count }, (_, i) => createMockUser(`${i + 1}`, checked)); + }; + + beforeEach(() => { + const featureFlagSubject = new ReplaySubject(1); + featureFlagSubject.next(false); + + const environmentSubject = new ReplaySubject(1); + environmentSubject.next({ + isCloud: () => false, + } as Environment); + + const mockConfigService = { + getFeatureFlag$: jest.fn(() => featureFlagSubject.asObservable()), + } as any; + + const mockEnvironmentService = { + environment$: environmentSubject.asObservable(), + } as any; + + TestBed.configureTestingModule({ + providers: [ + { provide: ConfigService, useValue: mockConfigService }, + { provide: EnvironmentService, useValue: mockEnvironmentService }, + ], + }); + + dataSource = TestBed.runInInjectionContext( + () => new TestPeopleTableDataSource(mockConfigService, mockEnvironmentService), + ); + }); + + describe("limitAndUncheckExcess", () => { + it("should return all users when under limit", () => { + const users = createMockUsers(10, true); + dataSource.data = users; + + const result = dataSource.limitAndUncheckExcess(users, 500); + + expect(result).toHaveLength(10); + expect(result).toEqual(users); + expect(users.every((u) => u.checked)).toBe(true); + }); + + it("should limit users and uncheck excess", () => { + const users = createMockUsers(600, true); + dataSource.data = users; + + const result = dataSource.limitAndUncheckExcess(users, 500); + + expect(result).toHaveLength(500); + expect(result).toEqual(users.slice(0, 500)); + expect(users.slice(0, 500).every((u) => u.checked)).toBe(true); + expect(users.slice(500).every((u) => u.checked)).toBe(false); + }); + + it("should only affect users in the provided array", () => { + const allUsers = createMockUsers(1000, true); + dataSource.data = allUsers; + + // Pass only a subset (simulates filtering by status) + const subset = allUsers.slice(0, 600); + + const result = dataSource.limitAndUncheckExcess(subset, 500); + + expect(result).toHaveLength(500); + expect(subset.slice(0, 500).every((u) => u.checked)).toBe(true); + expect(subset.slice(500).every((u) => u.checked)).toBe(false); + // Users outside subset remain checked + expect(allUsers.slice(600).every((u) => u.checked)).toBe(true); + }); + }); + + describe("status counts", () => { + it("should correctly count users by status", () => { + const users: MockUser[] = [ + { ...createMockUser("1"), status: OrganizationUserStatusType.Invited }, + { ...createMockUser("2"), status: OrganizationUserStatusType.Invited }, + { ...createMockUser("3"), status: OrganizationUserStatusType.Accepted }, + { ...createMockUser("4"), status: OrganizationUserStatusType.Confirmed }, + { ...createMockUser("5"), status: OrganizationUserStatusType.Confirmed }, + { ...createMockUser("6"), status: OrganizationUserStatusType.Confirmed }, + { ...createMockUser("7"), status: OrganizationUserStatusType.Revoked }, + ]; + dataSource.data = users; + + expect(dataSource.invitedUserCount).toBe(2); + expect(dataSource.acceptedUserCount).toBe(1); + expect(dataSource.confirmedUserCount).toBe(3); + expect(dataSource.revokedUserCount).toBe(1); + expect(dataSource.activeUserCount).toBe(6); // All except revoked + }); + }); +}); diff --git a/apps/web/src/app/admin-console/common/people-table-data-source.ts b/apps/web/src/app/admin-console/common/people-table-data-source.ts index 4696f8a6738..0228edb1e8c 100644 --- a/apps/web/src/app/admin-console/common/people-table-data-source.ts +++ b/apps/web/src/app/admin-console/common/people-table-data-source.ts @@ -1,14 +1,30 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore +import { computed, Signal } from "@angular/core"; +import { toSignal } from "@angular/core/rxjs-interop"; +import { map } from "rxjs"; + import { OrganizationUserStatusType, ProviderUserStatusType, } from "@bitwarden/common/admin-console/enums"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { TableDataSource } from "@bitwarden/components"; import { StatusType, UserViewTypes } from "./base-members.component"; -const MaxCheckedCount = 500; +/** + * Default maximum for most bulk operations (confirm, remove, delete, etc.) + */ +export const MaxCheckedCount = 500; + +/** + * Maximum for bulk reinvite operations when the IncreaseBulkReinviteLimitForCloud + * feature flag is enabled on cloud environments. + */ +export const CloudBulkReinviteLimit = 4000; /** * Returns true if the user matches the status, or where the status is `null`, if the user is active (not revoked). @@ -56,6 +72,20 @@ export abstract class PeopleTableDataSource extends Tab confirmedUserCount: number; revokedUserCount: number; + /** True when increased bulk limit feature is enabled (feature flag + cloud environment) */ + readonly isIncreasedBulkLimitEnabled: Signal; + + constructor(configService: ConfigService, environmentService: EnvironmentService) { + super(); + + const featureFlagEnabled = toSignal( + configService.getFeatureFlag$(FeatureFlag.IncreaseBulkReinviteLimitForCloud), + ); + const isCloud = toSignal(environmentService.environment$.pipe(map((env) => env.isCloud()))); + + this.isIncreasedBulkLimitEnabled = computed(() => featureFlagEnabled() && isCloud()); + } + override set data(data: T[]) { super.data = data; @@ -89,6 +119,14 @@ export abstract class PeopleTableDataSource extends Tab return this.data.filter((u) => (u as any).checked); } + /** + * Gets checked users in the order they appear in the filtered/sorted table view. + * Use this when enforcing limits to ensure visual consistency (top N visible rows stay checked). + */ + getCheckedUsersInVisibleOrder() { + return this.filteredData.filter((u) => (u as any).checked); + } + /** * Check all filtered users (i.e. those rows that are currently visible) * @param select check the filtered users (true) or uncheck the filtered users (false) @@ -101,8 +139,13 @@ export abstract class PeopleTableDataSource extends Tab const filteredUsers = this.filteredData; - const selectCount = - filteredUsers.length > MaxCheckedCount ? MaxCheckedCount : filteredUsers.length; + // When the increased bulk limit feature is enabled, allow checking all users. + // Individual bulk operations will enforce their specific limits. + // When disabled, enforce the legacy limit at check time. + const selectCount = this.isIncreasedBulkLimitEnabled() + ? filteredUsers.length + : Math.min(filteredUsers.length, MaxCheckedCount); + for (let i = 0; i < selectCount; i++) { this.checkUser(filteredUsers[i], select); } @@ -132,4 +175,41 @@ export abstract class PeopleTableDataSource extends Tab this.data = updatedData; } } + + /** + * Limits an array of users and unchecks those beyond the limit. + * Returns the limited array. + * + * @param users The array of users to limit + * @param limit The maximum number of users to keep + * @returns The users array limited to the specified count + */ + limitAndUncheckExcess(users: T[], limit: number): T[] { + if (users.length <= limit) { + return users; + } + + // Uncheck users beyond the limit + users.slice(limit).forEach((user) => this.checkUser(user, false)); + + return users.slice(0, limit); + } + + /** + * Gets checked users with optional limiting based on the IncreaseBulkReinviteLimitForCloud feature flag. + * + * When the feature flag is enabled: Returns checked users in visible order, limited to the specified count. + * When the feature flag is disabled: Returns all checked users without applying any limit. + * + * @param limit The maximum number of users to return (only applied when feature flag is enabled) + * @returns The checked users array + */ + getCheckedUsersWithLimit(limit: number): T[] { + if (this.isIncreasedBulkLimitEnabled()) { + const allUsers = this.getCheckedUsersInVisibleOrder(); + return this.limitAndUncheckExcess(allUsers, limit); + } else { + return this.getCheckedUsers(); + } + } } diff --git a/apps/web/src/app/admin-console/organizations/members/members.component.ts b/apps/web/src/app/admin-console/organizations/members/members.component.ts index 59c4c4898ea..ac25278a636 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/members.component.ts @@ -33,6 +33,8 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { OrganizationMetadataServiceAbstraction } from "@bitwarden/common/billing/abstractions/organization-metadata.service.abstraction"; import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; @@ -44,7 +46,11 @@ import { BillingConstraintService } from "@bitwarden/web-vault/app/billing/membe import { OrganizationWarningsService } from "@bitwarden/web-vault/app/billing/organizations/warnings/services"; import { BaseMembersComponent } from "../../common/base-members.component"; -import { PeopleTableDataSource } from "../../common/people-table-data-source"; +import { + CloudBulkReinviteLimit, + MaxCheckedCount, + PeopleTableDataSource, +} from "../../common/people-table-data-source"; import { OrganizationUserView } from "../core/views/organization-user.view"; import { AccountRecoveryDialogResultType } from "./components/account-recovery/account-recovery-dialog.component"; @@ -70,7 +76,7 @@ export class MembersComponent extends BaseMembersComponent userType = OrganizationUserType; userStatusType = OrganizationUserStatusType; memberTab = MemberDialogTab; - protected dataSource = new MembersTableDataSource(); + protected dataSource: MembersTableDataSource; readonly organization: Signal; status: OrganizationUserStatusType | undefined; @@ -113,6 +119,8 @@ export class MembersComponent extends BaseMembersComponent private policyService: PolicyService, private policyApiService: PolicyApiServiceAbstraction, private organizationMetadataService: OrganizationMetadataServiceAbstraction, + private configService: ConfigService, + private environmentService: EnvironmentService, ) { super( apiService, @@ -126,6 +134,8 @@ export class MembersComponent extends BaseMembersComponent toastService, ); + this.dataSource = new MembersTableDataSource(this.configService, this.environmentService); + const organization$ = this.route.params.pipe( concatMap((params) => this.userId$.pipe( @@ -356,10 +366,9 @@ export class MembersComponent extends BaseMembersComponent return; } - await this.memberDialogManager.openBulkRemoveDialog( - organization, - this.dataSource.getCheckedUsers(), - ); + const users = this.dataSource.getCheckedUsersWithLimit(MaxCheckedCount); + + await this.memberDialogManager.openBulkRemoveDialog(organization, users); this.organizationMetadataService.refreshMetadataCache(); await this.load(organization); } @@ -369,10 +378,9 @@ export class MembersComponent extends BaseMembersComponent return; } - await this.memberDialogManager.openBulkDeleteDialog( - organization, - this.dataSource.getCheckedUsers(), - ); + const users = this.dataSource.getCheckedUsersWithLimit(MaxCheckedCount); + + await this.memberDialogManager.openBulkDeleteDialog(organization, users); await this.load(organization); } @@ -389,11 +397,9 @@ export class MembersComponent extends BaseMembersComponent return; } - await this.memberDialogManager.openBulkRestoreRevokeDialog( - organization, - this.dataSource.getCheckedUsers(), - isRevoking, - ); + const users = this.dataSource.getCheckedUsersWithLimit(MaxCheckedCount); + + await this.memberDialogManager.openBulkRestoreRevokeDialog(organization, users, isRevoking); await this.load(organization); } @@ -402,8 +408,28 @@ export class MembersComponent extends BaseMembersComponent return; } - const users = this.dataSource.getCheckedUsers(); - const filteredUsers = users.filter((u) => u.status === OrganizationUserStatusType.Invited); + let users: OrganizationUserView[]; + if (this.dataSource.isIncreasedBulkLimitEnabled()) { + users = this.dataSource.getCheckedUsersInVisibleOrder(); + } else { + users = this.dataSource.getCheckedUsers(); + } + + const allInvitedUsers = users.filter((u) => u.status === OrganizationUserStatusType.Invited); + + // Capture the original count BEFORE enforcing the limit + const originalInvitedCount = allInvitedUsers.length; + + // When feature flag is enabled, limit invited users and uncheck the excess + let filteredUsers: OrganizationUserView[]; + if (this.dataSource.isIncreasedBulkLimitEnabled()) { + filteredUsers = this.dataSource.limitAndUncheckExcess( + allInvitedUsers, + CloudBulkReinviteLimit, + ); + } else { + filteredUsers = allInvitedUsers; + } if (filteredUsers.length <= 0) { this.toastService.showToast({ @@ -424,13 +450,37 @@ export class MembersComponent extends BaseMembersComponent throw new Error(); } - // Bulk Status component open - await this.memberDialogManager.openBulkStatusDialog( - users, - filteredUsers, - Promise.resolve(result.successful), - this.i18nService.t("bulkReinviteMessage"), - ); + // When feature flag is enabled, show toast instead of dialog + if (this.dataSource.isIncreasedBulkLimitEnabled()) { + const selectedCount = originalInvitedCount; + const invitedCount = filteredUsers.length; + + if (selectedCount > CloudBulkReinviteLimit) { + const excludedCount = selectedCount - CloudBulkReinviteLimit; + this.toastService.showToast({ + variant: "success", + message: this.i18nService.t( + "bulkReinviteLimitedSuccessToast", + CloudBulkReinviteLimit.toLocaleString(), + selectedCount.toLocaleString(), + excludedCount.toLocaleString(), + ), + }); + } else { + this.toastService.showToast({ + variant: "success", + message: this.i18nService.t("bulkReinviteSuccessToast", invitedCount.toString()), + }); + } + } else { + // Feature flag disabled - show legacy dialog + await this.memberDialogManager.openBulkStatusDialog( + users, + filteredUsers, + Promise.resolve(result.successful), + this.i18nService.t("bulkReinviteMessage"), + ); + } } catch (e) { this.validationService.showError(e); } @@ -442,15 +492,14 @@ export class MembersComponent extends BaseMembersComponent return; } - await this.memberDialogManager.openBulkConfirmDialog( - organization, - this.dataSource.getCheckedUsers(), - ); + const users = this.dataSource.getCheckedUsersWithLimit(MaxCheckedCount); + + await this.memberDialogManager.openBulkConfirmDialog(organization, users); await this.load(organization); } async bulkEnableSM(organization: Organization) { - const users = this.dataSource.getCheckedUsers(); + const users = this.dataSource.getCheckedUsersWithLimit(MaxCheckedCount); await this.memberDialogManager.openBulkEnableSecretsManagerDialog(organization, users); diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index efd33ff64b8..3d5303c8e82 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -6457,6 +6457,32 @@ "bulkReinviteMessage": { "message": "Reinvited successfully" }, + "bulkReinviteSuccessToast": { + "message": "$COUNT$ users re-invited", + "placeholders": { + "count": { + "content": "$1", + "example": "12" + } + } + }, + "bulkReinviteLimitedSuccessToast": { + "message": "$LIMIT$ of $SELECTEDCOUNT$ users re-invited. $EXCLUDEDCOUNT$ were not invited due to the $LIMIT$ invite limit.", + "placeholders": { + "limit": { + "content": "$1", + "example": "4,000" + }, + "selectedCount": { + "content": "$2", + "example": "4,005" + }, + "excludedCount": { + "content": "$3", + "example": "5" + } + } + }, "bulkRemovedMessage": { "message": "Removed successfully" }, diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/manage/members.component.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/manage/members.component.ts index 268a82ac12f..6e9209be882 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/manage/members.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/manage/members.component.ts @@ -19,6 +19,8 @@ import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { assertNonNullish } from "@bitwarden/common/auth/utils"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { ListResponse } from "@bitwarden/common/models/response/list.response"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; @@ -27,6 +29,8 @@ import { DialogRef, DialogService, ToastService } from "@bitwarden/components"; import { KeyService } from "@bitwarden/key-management"; import { BaseMembersComponent } from "@bitwarden/web-vault/app/admin-console/common/base-members.component"; import { + CloudBulkReinviteLimit, + MaxCheckedCount, peopleFilter, PeopleTableDataSource, } from "@bitwarden/web-vault/app/admin-console/common/people-table-data-source"; @@ -56,7 +60,7 @@ class MembersTableDataSource extends PeopleTableDataSource { }) export class MembersComponent extends BaseMembersComponent { accessEvents = false; - dataSource = new MembersTableDataSource(); + dataSource: MembersTableDataSource; loading = true; providerId: string; rowHeight = 70; @@ -81,6 +85,8 @@ export class MembersComponent extends BaseMembersComponent { private providerService: ProviderService, private router: Router, private accountService: AccountService, + private configService: ConfigService, + private environmentService: EnvironmentService, ) { super( apiService, @@ -94,6 +100,8 @@ export class MembersComponent extends BaseMembersComponent { toastService, ); + this.dataSource = new MembersTableDataSource(this.configService, this.environmentService); + combineLatest([ this.activatedRoute.parent.params, this.activatedRoute.queryParams.pipe(first()), @@ -134,10 +142,12 @@ export class MembersComponent extends BaseMembersComponent { return; } + const users = this.dataSource.getCheckedUsersWithLimit(MaxCheckedCount); + const dialogRef = BulkConfirmDialogComponent.open(this.dialogService, { data: { providerId: this.providerId, - users: this.dataSource.getCheckedUsers(), + users: users, }, }); @@ -150,10 +160,28 @@ export class MembersComponent extends BaseMembersComponent { return; } - const checkedUsers = this.dataSource.getCheckedUsers(); - const checkedInvitedUsers = checkedUsers.filter( - (user) => user.status === ProviderUserStatusType.Invited, - ); + let users: ProviderUser[]; + if (this.dataSource.isIncreasedBulkLimitEnabled()) { + users = this.dataSource.getCheckedUsersInVisibleOrder(); + } else { + users = this.dataSource.getCheckedUsers(); + } + + const allInvitedUsers = users.filter((user) => user.status === ProviderUserStatusType.Invited); + + // Capture the original count BEFORE enforcing the limit + const originalInvitedCount = allInvitedUsers.length; + + // When feature flag is enabled, limit invited users and uncheck the excess + let checkedInvitedUsers: ProviderUser[]; + if (this.dataSource.isIncreasedBulkLimitEnabled()) { + checkedInvitedUsers = this.dataSource.limitAndUncheckExcess( + allInvitedUsers, + CloudBulkReinviteLimit, + ); + } else { + checkedInvitedUsers = allInvitedUsers; + } if (checkedInvitedUsers.length <= 0) { this.toastService.showToast({ @@ -165,20 +193,50 @@ export class MembersComponent extends BaseMembersComponent { } try { - const request = this.apiService.postManyProviderUserReinvite( - this.providerId, - new ProviderUserBulkRequest(checkedInvitedUsers.map((user) => user.id)), - ); + // When feature flag is enabled, show toast instead of dialog + if (this.dataSource.isIncreasedBulkLimitEnabled()) { + await this.apiService.postManyProviderUserReinvite( + this.providerId, + new ProviderUserBulkRequest(checkedInvitedUsers.map((user) => user.id)), + ); - const dialogRef = BulkStatusComponent.open(this.dialogService, { - data: { - users: checkedUsers, - filteredUsers: checkedInvitedUsers, - request, - successfulMessage: this.i18nService.t("bulkReinviteMessage"), - }, - }); - await lastValueFrom(dialogRef.closed); + const selectedCount = originalInvitedCount; + const invitedCount = checkedInvitedUsers.length; + + if (selectedCount > CloudBulkReinviteLimit) { + const excludedCount = selectedCount - CloudBulkReinviteLimit; + this.toastService.showToast({ + variant: "success", + message: this.i18nService.t( + "bulkReinviteLimitedSuccessToast", + CloudBulkReinviteLimit.toLocaleString(), + selectedCount.toLocaleString(), + excludedCount.toLocaleString(), + ), + }); + } else { + this.toastService.showToast({ + variant: "success", + message: this.i18nService.t("bulkReinviteSuccessToast", invitedCount.toString()), + }); + } + } else { + // Feature flag disabled - show legacy dialog + const request = this.apiService.postManyProviderUserReinvite( + this.providerId, + new ProviderUserBulkRequest(checkedInvitedUsers.map((user) => user.id)), + ); + + const dialogRef = BulkStatusComponent.open(this.dialogService, { + data: { + users: users, + filteredUsers: checkedInvitedUsers, + request, + successfulMessage: this.i18nService.t("bulkReinviteMessage"), + }, + }); + await lastValueFrom(dialogRef.closed); + } } catch (error) { this.validationService.showError(error); } @@ -193,10 +251,12 @@ export class MembersComponent extends BaseMembersComponent { return; } + const users = this.dataSource.getCheckedUsersWithLimit(MaxCheckedCount); + const dialogRef = BulkRemoveDialogComponent.open(this.dialogService, { data: { providerId: this.providerId, - users: this.dataSource.getCheckedUsers(), + users: users, }, }); diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 23c1a07601e..371081a89d9 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -14,6 +14,7 @@ export enum FeatureFlag { CreateDefaultLocation = "pm-19467-create-default-location", AutoConfirm = "pm-19934-auto-confirm-organization-users", BlockClaimedDomainAccountCreation = "pm-28297-block-uninvited-claimed-domain-registration", + IncreaseBulkReinviteLimitForCloud = "pm-28251-increase-bulk-reinvite-limit-for-cloud", /* Auth */ PM23801_PrefetchPasswordPrelogin = "pm-23801-prefetch-password-prelogin", @@ -97,6 +98,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.CreateDefaultLocation]: FALSE, [FeatureFlag.AutoConfirm]: FALSE, [FeatureFlag.BlockClaimedDomainAccountCreation]: FALSE, + [FeatureFlag.IncreaseBulkReinviteLimitForCloud]: FALSE, /* Autofill */ [FeatureFlag.MacOsNativeCredentialSync]: FALSE, From f89c9b0f84803ecf6bf979575f43e50093389f24 Mon Sep 17 00:00:00 2001 From: Jeffrey Holland <124393578+jholland-livefront@users.noreply.github.com> Date: Mon, 8 Dec 2025 17:23:04 +0100 Subject: [PATCH 2/6] Fix typo in `xPositionOffset` variable name (#17857) --- .../desktop/src/autofill/services/desktop-autofill.service.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/autofill/services/desktop-autofill.service.ts b/apps/desktop/src/autofill/services/desktop-autofill.service.ts index 1f58b03dbda..c50964e31e3 100644 --- a/apps/desktop/src/autofill/services/desktop-autofill.service.ts +++ b/apps/desktop/src/autofill/services/desktop-autofill.service.ts @@ -448,10 +448,10 @@ export class DesktopAutofillService implements OnDestroy { function normalizePosition(position: { x: number; y: number }): { x: number; y: number } { // Add 100 pixels to the x-coordinate to offset the native OS dialog positioning. - const xPostionOffset = 100; + const xPositionOffset = 100; return { - x: Math.round(position.x + xPostionOffset), + x: Math.round(position.x + xPositionOffset), y: Math.round(position.y), }; } From 9f5dab05a265749b3b4338c7c8addce09078da3c Mon Sep 17 00:00:00 2001 From: Shane Melton Date: Mon, 8 Dec 2025 09:14:41 -0800 Subject: [PATCH 3/6] [PM-22750] Reimplement fix old attachment logic (#17689) * [PM-22750] Add upgradeOldCipherAttachment method to CipherService * [PM-22750] Refactor download attachment component to use signals * [PM-22750] Better download url handling * [PM-22750] Cleanup upgradeOldCipherAttachments method * [PM-22750] Refactor cipher-attachments.component to use Signals and OnPush * [PM-22750] Use the correct legacy decryption key for attachments without their own content encryption key * [PM-22750] Add fix attachment button back to attachments component * [PM-22750] Fix newly added output signals * [PM-22750] Fix failing test due to signal refactor * [PM-22750] Update copy --- apps/browser/src/_locales/en/messages.json | 9 + .../attachments-v2.component.spec.ts | 22 +- apps/desktop/src/locales/en/messages.json | 9 + apps/web/src/locales/en/messages.json | 9 + .../src/vault/abstractions/cipher.service.ts | 13 +- .../src/vault/services/cipher.service.ts | 103 ++++++++- .../cipher-attachments.component.html | 79 ++++--- .../cipher-attachments.component.spec.ts | 170 ++++++++------ .../cipher-attachments.component.ts | 216 ++++++++++-------- .../download-attachment.component.html | 19 +- .../download-attachment.component.spec.ts | 9 +- .../download-attachment.component.ts | 93 ++++---- 12 files changed, 491 insertions(+), 260 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index cb858930650..e3b7c69e163 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -1457,6 +1457,15 @@ "attachmentSaved": { "message": "Attachment saved" }, + "fixEncryption": { + "message": "Fix encryption" + }, + "fixEncryptionTooltip": { + "message": "This file is using an outdated encryption method." + }, + "attachmentUpdated": { + "message": "Attachment updated" + }, "file": { "message": "File" }, diff --git a/apps/browser/src/vault/popup/components/vault-v2/attachments/attachments-v2.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/attachments/attachments-v2.component.spec.ts index 871163ac80b..1da2d352c14 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/attachments/attachments-v2.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/attachments/attachments-v2.component.spec.ts @@ -1,4 +1,4 @@ -import { Component, Input } from "@angular/core"; +import { Component, input, ChangeDetectionStrategy } from "@angular/core"; import { ComponentFixture, TestBed, fakeAsync, tick } from "@angular/core/testing"; import { By } from "@angular/platform-browser"; import { ActivatedRoute, Router } from "@angular/router"; @@ -25,31 +25,23 @@ import { PopupRouterCacheService } from "../../../../../platform/popup/view-cach import { AttachmentsV2Component } from "./attachments-v2.component"; -// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush -// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @Component({ selector: "popup-header", template: ``, + changeDetection: ChangeDetectionStrategy.OnPush, }) class MockPopupHeaderComponent { - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() pageTitle: string; - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() backAction: () => void; + readonly pageTitle = input(); + readonly backAction = input<() => void>(); } -// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush -// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @Component({ selector: "popup-footer", template: ``, + changeDetection: ChangeDetectionStrategy.OnPush, }) class MockPopupFooterComponent { - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() pageTitle: string; + readonly pageTitle = input(); } describe("AttachmentsV2Component", () => { @@ -120,7 +112,7 @@ describe("AttachmentsV2Component", () => { const submitBtn = fixture.debugElement.queryAll(By.directive(ButtonComponent))[1] .componentInstance; - expect(cipherAttachment.submitBtn).toEqual(submitBtn); + expect(cipherAttachment.submitBtn()).toEqual(submitBtn); }); it("navigates the user to the edit view `onUploadSuccess`", fakeAsync(() => { diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index 33582c857aa..92e350fab90 100644 --- a/apps/desktop/src/locales/en/messages.json +++ b/apps/desktop/src/locales/en/messages.json @@ -708,6 +708,15 @@ "addAttachment": { "message": "Add attachment" }, + "fixEncryption": { + "message": "Fix encryption" + }, + "fixEncryptionTooltip": { + "message": "This file is using an outdated encryption method." + }, + "attachmentUpdated": { + "message": "Attachment updated" + }, "maxFileSizeSansPunctuation": { "message": "Maximum file size is 500 MB" }, diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 3d5303c8e82..4be70b102d1 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -5173,6 +5173,15 @@ "message": "Fix", "description": "This is a verb. ex. 'Fix The Car'" }, + "fixEncryption": { + "message": "Fix encryption" + }, + "fixEncryptionTooltip": { + "message": "This file is using an outdated encryption method." + }, + "attachmentUpdated": { + "message": "Attachment updated" + }, "oldAttachmentsNeedFixDesc": { "message": "There are old file attachments in your vault that need to be fixed before you can rotate your account's encryption key." }, diff --git a/libs/common/src/vault/abstractions/cipher.service.ts b/libs/common/src/vault/abstractions/cipher.service.ts index b95d9023a7c..8472a359c51 100644 --- a/libs/common/src/vault/abstractions/cipher.service.ts +++ b/libs/common/src/vault/abstractions/cipher.service.ts @@ -161,6 +161,17 @@ export abstract class CipherService implements UserKeyRotationDataProvider; + /** + * Upgrade all old attachments for a cipher by downloading, decrypting, re-uploading with new key, and deleting old. + * @param cipher - The cipher with old attachments to upgrade + * @param userId - The user ID + * @param attachmentId - If provided, only upgrade the attachment with this ID + */ + abstract upgradeOldCipherAttachments( + cipher: CipherView, + userId: UserId, + attachmentId?: string, + ): Promise; /** * Save the collections for a cipher with the server * @@ -274,7 +285,7 @@ export abstract class CipherService implements UserKeyRotationDataProvider; + ): Promise; /** * Decrypts the full `CipherView` for a given `CipherViewLike`. diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 7eebe960a7f..402b8ed1030 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -1656,12 +1656,14 @@ export class CipherService implements CipherServiceAbstraction { const key = attachment.key != null ? attachment.key - : await firstValueFrom( - this.keyService.orgKeys$(userId).pipe( - filterOutNullish(), - map((orgKeys) => orgKeys[cipherDomain.organizationId as OrganizationId] as OrgKey), - ), - ); + : cipherDomain.organizationId + ? await firstValueFrom( + this.keyService.orgKeys$(userId).pipe( + filterOutNullish(), + map((orgKeys) => orgKeys[cipherDomain.organizationId as OrganizationId] as OrgKey), + ), + ) + : await firstValueFrom(this.keyService.userKey$(userId).pipe(filterOutNullish())); return await this.encryptService.decryptFileData(encBuf, key); } @@ -1829,6 +1831,95 @@ export class CipherService implements CipherServiceAbstraction { } } + /** + * Upgrade all old attachments for a cipher by downloading, decrypting, re-uploading with new key, and deleting old. + * @param cipher + * @param userId + * @param attachmentId Optional specific attachment ID to upgrade. If not provided, all old attachments will be upgraded. + */ + async upgradeOldCipherAttachments( + cipher: CipherView, + userId: UserId, + attachmentId?: string, + ): Promise { + if (!cipher.hasOldAttachments) { + return cipher; + } + + let cipherDomain = await this.get(cipher.id, userId); + + for (const attachmentView of cipher.attachments) { + if ( + attachmentView.key != null || + (attachmentId != null && attachmentView.id !== attachmentId) + ) { + continue; + } + + try { + // 1. Get download URL + const downloadUrl = await this.getAttachmentDownloadUrl(cipher.id, attachmentView); + + // 2. Download attachment data + const dataResponse = await this.apiService.nativeFetch( + new Request(downloadUrl, { cache: "no-store" }), + ); + + if (dataResponse.status !== 200) { + throw new Error(`Failed to download attachment. Status: ${dataResponse.status}`); + } + + // 3. Decrypt the attachment + const decryptedBuffer = await this.getDecryptedAttachmentBuffer( + cipher.id as CipherId, + attachmentView, + dataResponse, + userId, + ); + + // 4. Re-upload with attachment key + cipherDomain = await this.saveAttachmentRawWithServer( + cipherDomain, + attachmentView.fileName, + decryptedBuffer, + userId, + ); + + // 5. Delete the old attachment + const cipherData = await this.deleteAttachmentWithServer( + cipher.id, + attachmentView.id, + userId, + ); + cipherDomain = new Cipher(cipherData); + } catch (e) { + this.logService.error(`Failed to upgrade attachment ${attachmentView.id}`, e); + throw e; + } + } + + return await this.decrypt(cipherDomain, userId); + } + + private async getAttachmentDownloadUrl( + cipherId: string, + attachmentView: AttachmentView, + ): Promise { + try { + const attachmentResponse = await this.apiService.getAttachmentData( + cipherId, + attachmentView.id, + ); + return attachmentResponse.url; + } catch (e) { + // Fall back to the attachment's stored URL + if (e instanceof ErrorResponse && e.statusCode === 404 && attachmentView.url) { + return attachmentView.url; + } + throw new Error(`Failed to get download URL for attachment ${attachmentView.id}`); + } + } + private async encryptObjProperty( model: V, obj: D, diff --git a/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.html b/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.html index 83e5956a067..855c37ecab5 100644 --- a/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.html +++ b/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.html @@ -1,32 +1,57 @@

{{ "attachments" | i18n }}

-
    -
  • - - - {{ attachment.fileName }} - {{ attachment.sizeName }} - - - - - - - - - - -
  • -
+@if (cipher()?.attachments; as attachments) { +
    + @for (attachment of attachments; track attachment.id) { +
  • + + + {{ + attachment.fileName + }} + {{ attachment.sizeName }} + + + + + + @if (attachment.key != null) { + + } @else { + + } + + + + + + +
  • + } +
+}
diff --git a/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.spec.ts b/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.spec.ts index 06f62976548..2e54d3b539a 100644 --- a/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.spec.ts +++ b/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.spec.ts @@ -1,7 +1,8 @@ -import { Component, Input } from "@angular/core"; +import { ChangeDetectionStrategy, Component, input } from "@angular/core"; import { ComponentFixture, TestBed } from "@angular/core/testing"; import { By } from "@angular/platform-browser"; import { mock } from "jest-mock-extended"; +import { BehaviorSubject } from "rxjs"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; @@ -13,7 +14,7 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { CipherId, UserId } from "@bitwarden/common/types/guid"; +import { CipherId, OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherType } from "@bitwarden/common/vault/enums"; import { AttachmentView } from "@bitwarden/common/vault/models/view/attachment.view"; @@ -26,27 +27,21 @@ import { FakeAccountService, mockAccountServiceWith } from "../../../../../commo import { CipherAttachmentsComponent } from "./cipher-attachments.component"; import { DeleteAttachmentComponent } from "./delete-attachment/delete-attachment.component"; -// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush -// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @Component({ selector: "app-download-attachment", template: "", + changeDetection: ChangeDetectionStrategy.OnPush, }) class MockDownloadAttachmentComponent { - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() attachment: AttachmentView; - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() cipher: CipherView; - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() admin: boolean = false; + readonly attachment = input(); + readonly cipher = input(); + readonly admin = input(false); } describe("CipherAttachmentsComponent", () => { let component: CipherAttachmentsComponent; let fixture: ComponentFixture; + let submitBtnFixture: ComponentFixture; const showToast = jest.fn(); const cipherView = { id: "5555-444-3333", @@ -63,17 +58,21 @@ describe("CipherAttachmentsComponent", () => { }; const organization = new Organization(); + organization.id = "org-123" as OrganizationId; organization.type = OrganizationUserType.Admin; organization.allowAdminAccessToAllCollectionItems = true; const cipherServiceGet = jest.fn().mockResolvedValue(cipherDomain); + const cipherServiceDecrypt = jest.fn().mockResolvedValue(cipherView); const saveAttachmentWithServer = jest.fn().mockResolvedValue(cipherDomain); const mockUserId = Utils.newGuid() as UserId; const accountService: FakeAccountService = mockAccountServiceWith(mockUserId); + const organizations$ = new BehaviorSubject([organization]); beforeEach(async () => { cipherServiceGet.mockClear(); + cipherServiceDecrypt.mockClear().mockResolvedValue(cipherView); showToast.mockClear(); saveAttachmentWithServer.mockClear().mockResolvedValue(cipherDomain); @@ -87,7 +86,7 @@ describe("CipherAttachmentsComponent", () => { get: cipherServiceGet, saveAttachmentWithServer, getKeyForCipherKeyDecryption: () => Promise.resolve(null), - decrypt: jest.fn().mockResolvedValue(cipherView), + decrypt: cipherServiceDecrypt, }, }, { @@ -110,7 +109,9 @@ describe("CipherAttachmentsComponent", () => { }, { provide: OrganizationService, - useValue: mock(), + useValue: { + organizations$: () => organizations$.asObservable(), + }, }, ], }) @@ -128,70 +129,67 @@ describe("CipherAttachmentsComponent", () => { beforeEach(() => { fixture = TestBed.createComponent(CipherAttachmentsComponent); component = fixture.componentInstance; - component.cipherId = "5555-444-3333" as CipherId; - component.submitBtn = TestBed.createComponent(ButtonComponent).componentInstance; + submitBtnFixture = TestBed.createComponent(ButtonComponent); + + fixture.componentRef.setInput("cipherId", "5555-444-3333" as CipherId); + fixture.componentRef.setInput("submitBtn", submitBtnFixture.componentInstance); fixture.detectChanges(); }); + /** + * Helper to wait for the async initialization effect to complete + */ + async function waitForInitialization(): Promise { + await fixture.whenStable(); + fixture.detectChanges(); + } + it("fetches cipherView using `cipherId`", async () => { - await component.ngOnInit(); + await waitForInitialization(); expect(cipherServiceGet).toHaveBeenCalledWith("5555-444-3333", mockUserId); - expect(component.cipher).toEqual(cipherView); }); - it("sets testids for automation testing", () => { + it("sets testids for automation testing", async () => { const attachment = { id: "1234-5678", fileName: "test file.txt", sizeName: "244.2 KB", } as AttachmentView; - component.cipher.attachments = [attachment]; + const cipherWithAttachments = { ...cipherView, attachments: [attachment] }; + cipherServiceDecrypt.mockResolvedValue(cipherWithAttachments); + // Create fresh fixture to pick up the mock + fixture = TestBed.createComponent(CipherAttachmentsComponent); + component = fixture.componentInstance; + fixture.componentRef.setInput("cipherId", "5555-444-3333" as CipherId); fixture.detectChanges(); + await waitForInitialization(); + const fileName = fixture.debugElement.query(By.css('[data-testid="file-name"]')); const fileSize = fixture.debugElement.query(By.css('[data-testid="file-size"]')); - expect(fileName.nativeElement.textContent).toEqual(attachment.fileName); + expect(fileName.nativeElement.textContent.trim()).toEqual(attachment.fileName); expect(fileSize.nativeElement.textContent).toEqual(attachment.sizeName); }); describe("bitSubmit", () => { - beforeEach(() => { - component.submitBtn.disabled.set(undefined); - component.submitBtn.loading.set(undefined); - }); - it("updates sets initial state of the submit button", async () => { - await component.ngOnInit(); + // Create fresh fixture to properly test initial state + submitBtnFixture = TestBed.createComponent(ButtonComponent); + submitBtnFixture.componentInstance.disabled.set(undefined as unknown as boolean); - expect(component.submitBtn.disabled()).toBe(true); - }); + fixture = TestBed.createComponent(CipherAttachmentsComponent); + component = fixture.componentInstance; + fixture.componentRef.setInput("submitBtn", submitBtnFixture.componentInstance); + fixture.componentRef.setInput("cipherId", "5555-444-3333" as CipherId); + fixture.detectChanges(); - it("sets submitBtn loading state", () => { - jest.useFakeTimers(); + await waitForInitialization(); - component.bitSubmit.loading = true; - - jest.runAllTimers(); - - expect(component.submitBtn.loading()).toBe(true); - - component.bitSubmit.loading = false; - - expect(component.submitBtn.loading()).toBe(false); - }); - - it("sets submitBtn disabled state", () => { - component.bitSubmit.disabled = true; - - expect(component.submitBtn.disabled()).toBe(true); - - component.bitSubmit.disabled = false; - - expect(component.submitBtn.disabled()).toBe(false); + expect(submitBtnFixture.componentInstance.disabled()).toBe(true); }); }); @@ -199,7 +197,7 @@ describe("CipherAttachmentsComponent", () => { let file: File; beforeEach(() => { - component.submitBtn.disabled.set(undefined); + submitBtnFixture.componentInstance.disabled.set(undefined as unknown as boolean); file = new File([""], "attachment.txt", { type: "text/plain" }); const inputElement = fixture.debugElement.query(By.css("input[type=file]")); @@ -215,11 +213,11 @@ describe("CipherAttachmentsComponent", () => { }); it("sets value of `file` control when input changes", () => { - expect(component.attachmentForm.controls.file.value.name).toEqual(file.name); + expect(component.attachmentForm.controls.file.value?.name).toEqual(file.name); }); it("updates disabled state of submit button", () => { - expect(component.submitBtn.disabled()).toBe(false); + expect(submitBtnFixture.componentInstance.disabled()).toBe(false); }); }); @@ -250,6 +248,8 @@ describe("CipherAttachmentsComponent", () => { }); it("shows error toast with server message when saveAttachmentWithServer fails", async () => { + await waitForInitialization(); + const file = { size: 100 } as File; component.attachmentForm.controls.file.setValue(file); @@ -265,6 +265,8 @@ describe("CipherAttachmentsComponent", () => { }); it("shows error toast with fallback message when error has no message property", async () => { + await waitForInitialization(); + const file = { size: 100 } as File; component.attachmentForm.controls.file.setValue(file); @@ -279,6 +281,8 @@ describe("CipherAttachmentsComponent", () => { }); it("shows error toast with string error message", async () => { + await waitForInitialization(); + const file = { size: 100 } as File; component.attachmentForm.controls.file.setValue(file); @@ -296,13 +300,27 @@ describe("CipherAttachmentsComponent", () => { describe("success", () => { const file = { size: 524287999 } as File; - beforeEach(() => { + async function setupWithOrganization(adminAccess: boolean): Promise { + // Create fresh fixture with organization set before cipherId + organization.allowAdminAccessToAllCollectionItems = adminAccess; + + fixture = TestBed.createComponent(CipherAttachmentsComponent); + component = fixture.componentInstance; + submitBtnFixture = TestBed.createComponent(ButtonComponent); + + // Set organizationId BEFORE cipherId so the effect picks it up + fixture.componentRef.setInput("organizationId", organization.id); + fixture.componentRef.setInput("submitBtn", submitBtnFixture.componentInstance); + fixture.componentRef.setInput("cipherId", "5555-444-3333" as CipherId); + fixture.detectChanges(); + + await waitForInitialization(); component.attachmentForm.controls.file.setValue(file); - component.organization = organization; - }); + } it("calls `saveAttachmentWithServer` with admin=false when admin permission is false for organization", async () => { - component.organization.allowAdminAccessToAllCollectionItems = false; + await setupWithOrganization(false); + await component.submit(); expect(saveAttachmentWithServer).toHaveBeenCalledWith( @@ -314,13 +332,16 @@ describe("CipherAttachmentsComponent", () => { }); it("calls `saveAttachmentWithServer` with admin=true when using admin API", async () => { - component.organization.allowAdminAccessToAllCollectionItems = true; + await setupWithOrganization(true); + await component.submit(); expect(saveAttachmentWithServer).toHaveBeenCalledWith(cipherDomain, file, mockUserId, true); }); it("resets form and input values", async () => { + await setupWithOrganization(true); + await component.submit(); const fileInput = fixture.debugElement.query(By.css("input[type=file]")); @@ -330,16 +351,19 @@ describe("CipherAttachmentsComponent", () => { }); it("shows success toast", async () => { + await setupWithOrganization(true); + await component.submit(); expect(showToast).toHaveBeenCalledWith({ variant: "success", - title: null, message: "attachmentSaved", }); }); it('emits "onUploadSuccess"', async () => { + await setupWithOrganization(true); + const emitSpy = jest.spyOn(component.onUploadSuccess, "emit"); await component.submit(); @@ -350,22 +374,36 @@ describe("CipherAttachmentsComponent", () => { }); describe("removeAttachment", () => { - const attachment = { id: "1234-5678" } as AttachmentView; + const attachment = { id: "1234-5678", fileName: "test.txt" } as AttachmentView; - beforeEach(() => { - component.cipher.attachments = [attachment]; + it("removes attachment from cipher", async () => { + // Create a new fixture with cipher that has attachments + const cipherWithAttachments = { ...cipherView, attachments: [attachment] }; + cipherServiceDecrypt.mockResolvedValue(cipherWithAttachments); + // Create fresh fixture + fixture = TestBed.createComponent(CipherAttachmentsComponent); + component = fixture.componentInstance; + fixture.componentRef.setInput("cipherId", "5555-444-3333" as CipherId); fixture.detectChanges(); - }); - it("removes attachment from cipher", () => { + await waitForInitialization(); + + // Verify attachment is rendered + const attachmentsBefore = fixture.debugElement.queryAll(By.css('[data-testid="file-name"]')); + expect(attachmentsBefore.length).toEqual(1); + const deleteAttachmentComponent = fixture.debugElement.query( By.directive(DeleteAttachmentComponent), ).componentInstance as DeleteAttachmentComponent; deleteAttachmentComponent.onDeletionSuccess.emit(); - expect(component.cipher.attachments).toEqual([]); + fixture.detectChanges(); + + // After removal, there should be no attachments displayed + const attachmentItems = fixture.debugElement.queryAll(By.css('[data-testid="file-name"]')); + expect(attachmentItems.length).toEqual(0); }); }); }); diff --git a/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.ts b/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.ts index a5306606199..f75611b995e 100644 --- a/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.ts +++ b/libs/vault/src/cipher-form/components/attachments/cipher-attachments.component.ts @@ -1,17 +1,15 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { CommonModule } from "@angular/common"; import { - AfterViewInit, + ChangeDetectionStrategy, Component, DestroyRef, ElementRef, - EventEmitter, - Input, - OnInit, - Output, - ViewChild, + effect, inject, + input, + output, + signal, + viewChild, } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { @@ -56,11 +54,10 @@ type CipherAttachmentForm = FormGroup<{ file: FormControl; }>; -// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush -// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @Component({ selector: "app-cipher-attachments", templateUrl: "./cipher-attachments.component.html", + changeDetection: ChangeDetectionStrategy.OnPush, imports: [ AsyncActionsModule, ButtonModule, @@ -74,70 +71,50 @@ type CipherAttachmentForm = FormGroup<{ DownloadAttachmentComponent, ], }) -export class CipherAttachmentsComponent implements OnInit, AfterViewInit { +export class CipherAttachmentsComponent { /** `id` associated with the form element */ static attachmentFormID = "attachmentForm"; /** Reference to the file HTMLInputElement */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @ViewChild("fileInput", { read: ElementRef }) private fileInput: ElementRef; + private readonly fileInput = viewChild("fileInput", { read: ElementRef }); /** Reference to the BitSubmitDirective */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @ViewChild(BitSubmitDirective) bitSubmit: BitSubmitDirective; + readonly bitSubmit = viewChild(BitSubmitDirective); /** The `id` of the cipher in context */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input({ required: true }) cipherId: CipherId; + readonly cipherId = input.required(); /** The organization ID if this cipher belongs to an organization */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() organizationId?: OrganizationId; + readonly organizationId = input(); /** Denotes if the action is occurring from within the admin console */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() admin: boolean = false; + readonly admin = input(false); /** An optional submit button, whose loading/disabled state will be tied to the form state. */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() submitBtn?: ButtonComponent; + readonly submitBtn = input(); /** Emits when a file upload is started */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-output-emitter-ref - @Output() onUploadStarted = new EventEmitter(); + readonly onUploadStarted = output(); /** Emits after a file has been successfully uploaded */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-output-emitter-ref - @Output() onUploadSuccess = new EventEmitter(); + readonly onUploadSuccess = output(); /** Emits when a file upload fails */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-output-emitter-ref - @Output() onUploadFailed = new EventEmitter(); + readonly onUploadFailed = output(); /** Emits after a file has been successfully removed */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-output-emitter-ref - @Output() onRemoveSuccess = new EventEmitter(); + readonly onRemoveSuccess = output(); - organization: Organization; - cipher: CipherView; + protected readonly organization = signal(null); + protected readonly cipher = signal(null); attachmentForm: CipherAttachmentForm = this.formBuilder.group({ - file: new FormControl(null, [Validators.required]), + file: new FormControl(null, [Validators.required]), }); - private cipherDomain: Cipher; - private activeUserId: UserId; - private destroy$ = inject(DestroyRef); + private cipherDomain: Cipher | null = null; + private activeUserId: UserId | null = null; + private readonly destroyRef = inject(DestroyRef); constructor( private cipherService: CipherService, @@ -150,43 +127,52 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit { private organizationService: OrganizationService, ) { this.attachmentForm.statusChanges.pipe(takeUntilDestroyed()).subscribe((status) => { - if (!this.submitBtn) { + const btn = this.submitBtn(); + if (!btn) { return; } - this.submitBtn.disabled.set(status !== "VALID"); - }); - } - - async ngOnInit(): Promise { - this.activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); - // Get the organization to check admin permissions - this.organization = await this.getOrganization(); - this.cipherDomain = await this.getCipher(this.cipherId); - - this.cipher = await this.cipherService.decrypt(this.cipherDomain, this.activeUserId); - - // Update the initial state of the submit button - if (this.submitBtn) { - this.submitBtn.disabled.set(!this.attachmentForm.valid); - } - } - - ngAfterViewInit(): void { - this.bitSubmit.loading$.pipe(takeUntilDestroyed(this.destroy$)).subscribe((loading) => { - if (!this.submitBtn) { - return; - } - - this.submitBtn.loading.set(loading); + btn.disabled.set(status !== "VALID"); }); - this.bitSubmit.disabled$.pipe(takeUntilDestroyed(this.destroy$)).subscribe((disabled) => { - if (!this.submitBtn) { + // Initialize data when cipherId input is available + effect(async () => { + const cipherId = this.cipherId(); + if (!cipherId) { return; } - this.submitBtn.disabled.set(disabled); + this.activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + // Get the organization to check admin permissions + this.organization.set(await this.getOrganization()); + this.cipherDomain = await this.getCipher(cipherId); + + if (this.cipherDomain && this.activeUserId) { + this.cipher.set(await this.cipherService.decrypt(this.cipherDomain, this.activeUserId)); + } + + // Update the initial state of the submit button + const btn = this.submitBtn(); + if (btn) { + btn.disabled.set(!this.attachmentForm.valid); + } + }); + + // Sync bitSubmit loading/disabled state with submitBtn + effect(() => { + const bitSubmit = this.bitSubmit(); + const btn = this.submitBtn(); + if (!bitSubmit || !btn) { + return; + } + + bitSubmit.loading$.pipe(takeUntilDestroyed(this.destroyRef)).subscribe((loading) => { + btn.loading.set(loading); + }); + + bitSubmit.disabled$.pipe(takeUntilDestroyed(this.destroyRef)).subscribe((disabled) => { + btn.disabled.set(disabled); + }); }); } @@ -209,7 +195,7 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit { this.onUploadStarted.emit(); const file = this.attachmentForm.value.file; - if (file === null) { + if (file == null) { this.toastService.showToast({ variant: "error", title: this.i18nService.t("errorOccurred"), @@ -228,24 +214,30 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit { return; } + if (!this.cipherDomain || !this.activeUserId) { + return; + } + try { this.cipherDomain = await this.cipherService.saveAttachmentWithServer( this.cipherDomain, file, this.activeUserId, - this.organization?.canEditAllCiphers, + this.organization()?.canEditAllCiphers, ); // re-decrypt the cipher to update the attachments - this.cipher = await this.cipherService.decrypt(this.cipherDomain, this.activeUserId); + this.cipher.set(await this.cipherService.decrypt(this.cipherDomain, this.activeUserId)); // Reset reactive form and input element - this.fileInput.nativeElement.value = ""; + const fileInputEl = this.fileInput(); + if (fileInputEl) { + fileInputEl.nativeElement.value = ""; + } this.attachmentForm.controls.file.setValue(null); this.toastService.showToast({ variant: "success", - title: null, message: this.i18nService.t("attachmentSaved"), }); @@ -257,7 +249,7 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit { let errorMessage = this.i18nService.t("unexpectedError"); if (typeof e === "string") { errorMessage = e; - } else if (e?.message) { + } else if (e instanceof Error && e?.message) { errorMessage = e.message; } @@ -271,10 +263,19 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit { /** Removes the attachment from the cipher */ removeAttachment(attachment: AttachmentView) { - const index = this.cipher.attachments.indexOf(attachment); + const currentCipher = this.cipher(); + if (!currentCipher?.attachments) { + return; + } + + const index = currentCipher.attachments.indexOf(attachment); if (index > -1) { - this.cipher.attachments.splice(index, 1); + currentCipher.attachments.splice(index, 1); + // Trigger signal update by creating a new reference + this.cipher.set( + Object.assign(Object.create(Object.getPrototypeOf(currentCipher)), currentCipher), + ); } this.onRemoveSuccess.emit(); @@ -286,7 +287,7 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit { * it will retrieve the cipher using the admin endpoint. */ private async getCipher(id: CipherId): Promise { - if (id == null) { + if (id == null || !this.activeUserId) { return null; } @@ -294,12 +295,13 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit { const localCipher = await this.cipherService.get(id, this.activeUserId); // If we got the cipher or there's no organization context, return the result - if (localCipher != null || !this.organizationId) { + if (localCipher != null || !this.organizationId()) { return localCipher; } // Only try the admin API if the user has admin permissions - if (this.organization != null && this.organization.canEditAllCiphers) { + const org = this.organization(); + if (org != null && org.canEditAllCiphers) { const cipherResponse = await this.apiService.getCipherAdmin(id); const cipherData = new CipherData(cipherResponse); return new Cipher(cipherData); @@ -312,7 +314,8 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit { * Gets the organization for the given organization ID */ private async getOrganization(): Promise { - if (!this.organizationId) { + const orgId = this.organizationId(); + if (!orgId || !this.activeUserId) { return null; } @@ -320,6 +323,41 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit { this.organizationService.organizations$(this.activeUserId), ); - return organizations.find((o) => o.id === this.organizationId) || null; + return organizations.find((o) => o.id === orgId) || null; } + + protected fixOldAttachment = (attachment: AttachmentView) => { + return async () => { + const cipher = this.cipher(); + const userId = this.activeUserId; + + if (!attachment.id || !userId || !cipher) { + this.toastService.showToast({ + variant: "error", + message: this.i18nService.t("errorOccurred"), + }); + return; + } + + try { + const updatedCipher = await this.cipherService.upgradeOldCipherAttachments( + cipher, + userId, + attachment.id, + ); + + this.cipher.set(updatedCipher); + this.toastService.showToast({ + variant: "success", + message: this.i18nService.t("attachmentUpdated"), + }); + this.onUploadSuccess.emit(); + } catch { + this.toastService.showToast({ + variant: "error", + message: this.i18nService.t("errorOccurred"), + }); + } + }; + }; } diff --git a/libs/vault/src/components/download-attachment/download-attachment.component.html b/libs/vault/src/components/download-attachment/download-attachment.component.html index c2c2f1d4ebd..9d80f36818a 100644 --- a/libs/vault/src/components/download-attachment/download-attachment.component.html +++ b/libs/vault/src/components/download-attachment/download-attachment.component.html @@ -1,9 +1,10 @@ - +@if (!isDecryptionFailure()) { + +} diff --git a/libs/vault/src/components/download-attachment/download-attachment.component.spec.ts b/libs/vault/src/components/download-attachment/download-attachment.component.spec.ts index ec5a9ce96fd..3bbc375fdfc 100644 --- a/libs/vault/src/components/download-attachment/download-attachment.component.spec.ts +++ b/libs/vault/src/components/download-attachment/download-attachment.component.spec.ts @@ -100,8 +100,8 @@ describe("DownloadAttachmentComponent", () => { beforeEach(() => { fixture = TestBed.createComponent(DownloadAttachmentComponent); component = fixture.componentInstance; - component.attachment = attachment; - component.cipher = cipherView; + fixture.componentRef.setInput("attachment", attachment); + fixture.componentRef.setInput("cipher", cipherView); fixture.detectChanges(); }); @@ -123,7 +123,8 @@ describe("DownloadAttachmentComponent", () => { }); it("hides download button when the attachment has decryption failure", () => { - component.attachment.fileName = DECRYPT_ERROR; + const decryptFailureAttachment = { ...attachment, fileName: DECRYPT_ERROR }; + fixture.componentRef.setInput("attachment", decryptFailureAttachment); fixture.detectChanges(); expect(fixture.debugElement.query(By.css("button"))).toBeNull(); @@ -156,7 +157,6 @@ describe("DownloadAttachmentComponent", () => { expect(showToast).toHaveBeenCalledWith({ message: "errorOccurred", - title: null, variant: "error", }); }); @@ -172,7 +172,6 @@ describe("DownloadAttachmentComponent", () => { expect(showToast).toHaveBeenCalledWith({ message: "errorOccurred", - title: null, variant: "error", }); }); diff --git a/libs/vault/src/components/download-attachment/download-attachment.component.ts b/libs/vault/src/components/download-attachment/download-attachment.component.ts index 2f9cd528990..31ed609637c 100644 --- a/libs/vault/src/components/download-attachment/download-attachment.component.ts +++ b/libs/vault/src/components/download-attachment/download-attachment.component.ts @@ -1,7 +1,5 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { CommonModule } from "@angular/common"; -import { Component, Input } from "@angular/core"; +import { ChangeDetectionStrategy, Component, computed, input } from "@angular/core"; import { firstValueFrom } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; @@ -17,38 +15,27 @@ import { AttachmentView } from "@bitwarden/common/vault/models/view/attachment.v import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { AsyncActionsModule, IconButtonModule, ToastService } from "@bitwarden/components"; -// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush -// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @Component({ selector: "app-download-attachment", templateUrl: "./download-attachment.component.html", imports: [AsyncActionsModule, CommonModule, JslibModule, IconButtonModule], + changeDetection: ChangeDetectionStrategy.OnPush, }) export class DownloadAttachmentComponent { /** Attachment to download */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input({ required: true }) attachment: AttachmentView; + readonly attachment = input.required(); /** The cipher associated with the attachment */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input({ required: true }) cipher: CipherView; + readonly cipher = input.required(); - // When in view mode, we will want to check for the master password reprompt - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() checkPwReprompt?: boolean = false; + /** When in view mode, we will want to check for the master password reprompt */ + readonly checkPwReprompt = input(false); - // Required for fetching attachment data when viewed from cipher via emergency access - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() emergencyAccessId?: EmergencyAccessId; + /** Required for fetching attachment data when viewed from cipher via emergency access */ + readonly emergencyAccessId = input(); - /** When owners/admins can mange all items and when accessing from the admin console, use the admin endpoint */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() admin?: boolean = false; + /** When owners/admins can manage all items and when accessing from the admin console, use the admin endpoint */ + readonly admin = input(false); constructor( private i18nService: I18nService, @@ -59,26 +46,36 @@ export class DownloadAttachmentComponent { private cipherService: CipherService, ) {} - protected get isDecryptionFailure(): boolean { - return this.attachment.fileName === DECRYPT_ERROR; - } + protected readonly isDecryptionFailure = computed( + () => this.attachment().fileName === DECRYPT_ERROR, + ); /** Download the attachment */ download = async () => { - let url: string; + const attachment = this.attachment(); + const cipher = this.cipher(); + let url: string | undefined; + + if (!attachment.id) { + this.toastService.showToast({ + variant: "error", + message: this.i18nService.t("errorOccurred"), + }); + return; + } try { - const attachmentDownloadResponse = this.admin - ? await this.apiService.getAttachmentDataAdmin(this.cipher.id, this.attachment.id) + const attachmentDownloadResponse = this.admin() + ? await this.apiService.getAttachmentDataAdmin(cipher.id, attachment.id) : await this.apiService.getAttachmentData( - this.cipher.id, - this.attachment.id, - this.emergencyAccessId, + cipher.id, + attachment.id, + this.emergencyAccessId(), ); url = attachmentDownloadResponse.url; } catch (e) { if (e instanceof ErrorResponse && (e as ErrorResponse).statusCode === 404) { - url = this.attachment.url; + url = attachment.url; } else if (e instanceof ErrorResponse) { throw new Error((e as ErrorResponse).getSingleMessage()); } else { @@ -86,11 +83,18 @@ export class DownloadAttachmentComponent { } } + if (!url) { + this.toastService.showToast({ + variant: "error", + message: this.i18nService.t("errorOccurred"), + }); + return; + } + const response = await fetch(new Request(url, { cache: "no-store" })); if (response.status !== 200) { this.toastService.showToast({ variant: "error", - title: null, message: this.i18nService.t("errorOccurred"), }); return; @@ -99,26 +103,31 @@ export class DownloadAttachmentComponent { try { const userId = await firstValueFrom(this.stateProvider.activeUserId$); + if (!userId || !attachment.fileName) { + this.toastService.showToast({ + variant: "error", + message: this.i18nService.t("errorOccurred"), + }); + return; + } + const decBuf = await this.cipherService.getDecryptedAttachmentBuffer( - this.cipher.id as CipherId, - this.attachment, + cipher.id as CipherId, + attachment, response, userId, // When the emergency access ID is present, the cipher is being viewed via emergency access. // Force legacy decryption in these cases. - this.emergencyAccessId ? true : false, + Boolean(this.emergencyAccessId()), ); this.fileDownloadService.download({ - fileName: this.attachment.fileName, + fileName: attachment.fileName, blobData: decBuf, }); - // FIXME: Remove when updating file. Eslint update - // eslint-disable-next-line @typescript-eslint/no-unused-vars - } catch (e) { + } catch { this.toastService.showToast({ variant: "error", - title: null, message: this.i18nService.t("errorOccurred"), }); } From 97adae2864b7d7257690e56896d36ad356533395 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 8 Dec 2025 13:46:54 -0500 Subject: [PATCH 4/6] [deps] UI Foundation: Update @types/react to v18.3.27 (#17558) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- package-lock.json | 14 +++++++------- package.json | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/package-lock.json b/package-lock.json index 505c12b320c..21afa10e3e8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14278,15 +14278,15 @@ "license": "MIT" }, "node_modules/@types/react": { - "version": "18.3.20", - "resolved": "https://registry.npmjs.org/@types/react/-/react-18.3.20.tgz", - "integrity": "sha512-IPaCZN7PShZK/3t6Q87pfTkRm6oLTd4vztyoj+cbHUF1g3FfVb2tFIL79uCRKEfv16AhqDMBywP2VW3KIZUvcg==", + "version": "18.3.27", + "resolved": "https://registry.npmjs.org/@types/react/-/react-18.3.27.tgz", + "integrity": "sha512-cisd7gxkzjBKU2GgdYrTdtQx1SORymWyaAFhaxQPK9bYO9ot3Y5OikQRvY0VYQtvwjeQnizCINJAenh/V7MK2w==", "dev": true, "license": "MIT", "peer": true, "dependencies": { "@types/prop-types": "*", - "csstype": "^3.0.2" + "csstype": "^3.2.2" } }, "node_modules/@types/responselike": { @@ -19574,9 +19574,9 @@ } }, "node_modules/csstype": { - "version": "3.1.3", - "resolved": "https://registry.npmjs.org/csstype/-/csstype-3.1.3.tgz", - "integrity": "sha512-M1uQkMl8rQK/szD0LNhtqxIPLpimGm8sOBwU7lLnCpSbTyY3yeU1Vc7l4KT5zT4s/yOxHH5O7tIuuLOCnLADRw==", + "version": "3.2.3", + "resolved": "https://registry.npmjs.org/csstype/-/csstype-3.2.3.tgz", + "integrity": "sha512-z1HGKcYy2xA8AGQfwrn0PAy+PB7X/GSj3UVJW9qKyn43xWa+gl5nXmU4qqLMRzWVLFC8KusUX8T/0kCiOYpAIQ==", "license": "MIT" }, "node_modules/cwd": { diff --git a/package.json b/package.json index 94019fafea8..613c4042f8f 100644 --- a/package.json +++ b/package.json @@ -220,7 +220,7 @@ "parse5": "7.2.1", "react": "18.3.1", "react-dom": "18.3.1", - "@types/react": "18.3.20" + "@types/react": "18.3.27" }, "lint-staged": { "*": "prettier --cache --ignore-unknown --write", From 2d3b017cc24a8706fb4204b9b1b3e431f202aa94 Mon Sep 17 00:00:00 2001 From: Mike Amirault Date: Mon, 8 Dec 2025 13:52:45 -0500 Subject: [PATCH 5/6] [PM-24095] Ensure long Send file names do not overflow parent container (#17774) * [PM-24095] Ensure long Send file names do not overflow parent container * [PM-24095] Add styling to a couple other spots --- apps/desktop/src/app/tools/send/add-edit.component.html | 4 +++- .../tools/send/send-access/send-access-file.component.html | 2 +- .../components/send-details/send-file-details.component.html | 4 +++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/apps/desktop/src/app/tools/send/add-edit.component.html b/apps/desktop/src/app/tools/send/add-edit.component.html index 0bf2e1778e0..639c80c9060 100644 --- a/apps/desktop/src/app/tools/send/add-edit.component.html +++ b/apps/desktop/src/app/tools/send/add-edit.component.html @@ -46,7 +46,9 @@
-
{{ send.file.fileName }} ({{ send.file.sizeName }})
+
+ {{ send.file.fileName }} ({{ send.file.sizeName }}) +
diff --git a/apps/web/src/app/tools/send/send-access/send-access-file.component.html b/apps/web/src/app/tools/send/send-access/send-access-file.component.html index 82880407809..8cbe6a975ef 100644 --- a/apps/web/src/app/tools/send/send-access/send-access-file.component.html +++ b/apps/web/src/app/tools/send/send-access/send-access-file.component.html @@ -1,4 +1,4 @@ -

{{ send.file.fileName }}

+

{{ send.file.fileName }}