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/app/tools/import/import-desktop.component.html b/apps/desktop/src/app/tools/import/import-desktop.component.html index 13cdea7470b..960e76845be 100644 --- a/apps/desktop/src/app/tools/import/import-desktop.component.html +++ b/apps/desktop/src/app/tools/import/import-desktop.component.html @@ -10,9 +10,11 @@ [onLoadProfilesFromBrowser]="this.onLoadProfilesFromBrowser" [class.tw-invisible]="loading" > -
- -
+ @if (loading) { +
+ +
+ } 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/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), }; } diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index ead2b15db0d..0e0545726e9 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/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/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 }}

+ } + + + + + + + + } + +}
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"), }); } 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",