From 359b6e02d99ce1cbcfcc99a42962869bddb0b92c Mon Sep 17 00:00:00 2001 From: Oleksii Holub <1935960+Tyrrrz@users.noreply.github.com> Date: Mon, 7 Oct 2024 17:55:17 +0300 Subject: [PATCH 01/10] PM-12102 | Fix LastPass importer not properly de-encrypting URLs (#11366) * PM-12102 | Fix LastPass importer not properly de-encrypting URLs * Reuse the original code for the unencrypted path * Add some comments --- .../importers/lastpass/access/services/parser.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/libs/importer/src/importers/lastpass/access/services/parser.ts b/libs/importer/src/importers/lastpass/access/services/parser.ts index 19ef55bc739..31efadd6c02 100644 --- a/libs/importer/src/importers/lastpass/access/services/parser.ts +++ b/libs/importer/src/importers/lastpass/access/services/parser.ts @@ -22,7 +22,7 @@ export class Parser { /* May return null when the chunk does not represent an account. All secure notes are ACCTs but not all of them store account information. - + TODO: Add a test for the folder case! TODO: Add a test case that covers secure note account! */ @@ -60,9 +60,17 @@ export class Parser { // 3: url step = 3; - let url = Utils.fromBufferToUtf8( - this.decodeHexLoose(Utils.fromBufferToUtf8(this.readItem(reader))), - ); + const urlEncoded = this.readItem(reader); + let url = + urlEncoded.length > 0 && urlEncoded[0] === 33 // 33 = '!' + ? // URL is encrypted + await this.cryptoUtils.decryptAes256PlainWithDefault( + urlEncoded, + encryptionKey, + placeholder, + ) + : // URL is not encrypted + Utils.fromBufferToUtf8(this.decodeHexLoose(Utils.fromBufferToUtf8(urlEncoded))); // Ignore "group" accounts. They have no credentials. if (url == "http://group") { From c98b4553f22b8e0e38af97e7690955a4c7a790d7 Mon Sep 17 00:00:00 2001 From: "bw-ghapp[bot]" <178206702+bw-ghapp[bot]@users.noreply.github.com> Date: Mon, 7 Oct 2024 15:06:39 +0000 Subject: [PATCH 02/10] Bumped client version(s) (#11439) Co-authored-by: bitwarden-devops-bot <106330231+bitwarden-devops-bot@users.noreply.github.com> --- apps/browser/package.json | 2 +- apps/browser/src/manifest.json | 2 +- apps/browser/src/manifest.v3.json | 2 +- apps/cli/package.json | 2 +- apps/desktop/package.json | 2 +- apps/desktop/src/package-lock.json | 12 ++---------- apps/desktop/src/package.json | 2 +- apps/web/package.json | 2 +- package-lock.json | 8 ++++---- 9 files changed, 13 insertions(+), 21 deletions(-) diff --git a/apps/browser/package.json b/apps/browser/package.json index 9d878694594..15caff9eff2 100644 --- a/apps/browser/package.json +++ b/apps/browser/package.json @@ -1,6 +1,6 @@ { "name": "@bitwarden/browser", - "version": "2024.10.0", + "version": "2024.10.1", "scripts": { "build": "cross-env MANIFEST_VERSION=3 webpack", "build:mv2": "webpack", diff --git a/apps/browser/src/manifest.json b/apps/browser/src/manifest.json index 598c6ff0286..850c5c4727a 100644 --- a/apps/browser/src/manifest.json +++ b/apps/browser/src/manifest.json @@ -2,7 +2,7 @@ "manifest_version": 2, "name": "__MSG_extName__", "short_name": "__MSG_appName__", - "version": "2024.10.0", + "version": "2024.10.1", "description": "__MSG_extDesc__", "default_locale": "en", "author": "Bitwarden Inc.", diff --git a/apps/browser/src/manifest.v3.json b/apps/browser/src/manifest.v3.json index 762e5e01f83..0b89a36d700 100644 --- a/apps/browser/src/manifest.v3.json +++ b/apps/browser/src/manifest.v3.json @@ -3,7 +3,7 @@ "minimum_chrome_version": "102.0", "name": "__MSG_extName__", "short_name": "__MSG_appName__", - "version": "2024.10.0", + "version": "2024.10.1", "description": "__MSG_extDesc__", "default_locale": "en", "author": "Bitwarden Inc.", diff --git a/apps/cli/package.json b/apps/cli/package.json index 759fb14ba5a..502e186d346 100644 --- a/apps/cli/package.json +++ b/apps/cli/package.json @@ -1,7 +1,7 @@ { "name": "@bitwarden/cli", "description": "A secure and free password manager for all of your devices.", - "version": "2024.9.1", + "version": "2024.10.0", "keywords": [ "bitwarden", "password", diff --git a/apps/desktop/package.json b/apps/desktop/package.json index fbf598327f4..1014412c0d5 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -1,7 +1,7 @@ { "name": "@bitwarden/desktop", "description": "A secure and free password manager for all of your devices.", - "version": "2024.9.2", + "version": "2024.10.0", "keywords": [ "bitwarden", "password", diff --git a/apps/desktop/src/package-lock.json b/apps/desktop/src/package-lock.json index acd5f97a3f4..602f35872e1 100644 --- a/apps/desktop/src/package-lock.json +++ b/apps/desktop/src/package-lock.json @@ -1,26 +1,18 @@ { "name": "@bitwarden/desktop", - "version": "2024.9.2", + "version": "2024.10.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@bitwarden/desktop", - "version": "2024.9.2", + "version": "2024.10.0", "license": "GPL-3.0", "dependencies": { "@bitwarden/desktop-napi": "file:../desktop_native/napi", "argon2": "0.40.1" } }, - "../desktop_native/napi": { - "name": "@bitwarden/desktop-napi", - "version": "0.1.0", - "license": "GPL-3.0", - "devDependencies": { - "@napi-rs/cli": "2.16.2" - } - }, "../desktop_native/napi": { "version": "0.1.0", "license": "GPL-3.0", diff --git a/apps/desktop/src/package.json b/apps/desktop/src/package.json index 29b3f8ed7d1..c98d31f10a2 100644 --- a/apps/desktop/src/package.json +++ b/apps/desktop/src/package.json @@ -2,7 +2,7 @@ "name": "@bitwarden/desktop", "productName": "Bitwarden", "description": "A secure and free password manager for all of your devices.", - "version": "2024.9.2", + "version": "2024.10.0", "author": "Bitwarden Inc. (https://bitwarden.com)", "homepage": "https://bitwarden.com", "license": "GPL-3.0", diff --git a/apps/web/package.json b/apps/web/package.json index bc7a96e239f..dd737fb5b43 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -1,6 +1,6 @@ { "name": "@bitwarden/web-vault", - "version": "2024.10.0", + "version": "2024.10.1", "scripts": { "build:oss": "webpack", "build:bit": "webpack -c ../../bitwarden_license/bit-web/webpack.config.js", diff --git a/package-lock.json b/package-lock.json index 7ef9cf23d9c..791976f9071 100644 --- a/package-lock.json +++ b/package-lock.json @@ -194,11 +194,11 @@ }, "apps/browser": { "name": "@bitwarden/browser", - "version": "2024.10.0" + "version": "2024.10.1" }, "apps/cli": { "name": "@bitwarden/cli", - "version": "2024.9.1", + "version": "2024.10.0", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@koa/multer": "3.0.2", @@ -234,7 +234,7 @@ }, "apps/desktop": { "name": "@bitwarden/desktop", - "version": "2024.9.2", + "version": "2024.10.0", "hasInstallScript": true, "license": "GPL-3.0" }, @@ -248,7 +248,7 @@ }, "apps/web": { "name": "@bitwarden/web-vault", - "version": "2024.10.0" + "version": "2024.10.1" }, "libs/admin-console": { "name": "@bitwarden/admin-console", From 7098a243ca03d42f5994dfc863f652b8905b4c5b Mon Sep 17 00:00:00 2001 From: Jason Ng Date: Mon, 7 Oct 2024 11:15:00 -0400 Subject: [PATCH 03/10] [PM-10378] Unassigned Items Readonly After Edit Bug Fix (#11340) --- libs/common/src/vault/services/cipher.service.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index a06ca4d793d..77e696b5cdd 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -673,6 +673,8 @@ export class CipherService implements CipherServiceAbstraction { if (orgAdmin && cipher.organizationId != null) { const request = new CipherCreateRequest(cipher); response = await this.apiService.postCipherAdmin(request); + const data = new CipherData(response, cipher.collectionIds); + return new Cipher(data); } else if (cipher.collectionIds != null) { const request = new CipherCreateRequest(cipher); response = await this.apiService.postCipherCreate(request); @@ -697,6 +699,8 @@ export class CipherService implements CipherServiceAbstraction { if (orgAdmin && isNotClone) { const request = new CipherRequest(cipher); response = await this.apiService.putCipherAdmin(cipher.id, request); + const data = new CipherData(response, cipher.collectionIds); + return new Cipher(data, cipher.localData); } else if (cipher.edit) { const request = new CipherRequest(cipher); response = await this.apiService.putCipher(cipher.id, request); From a6db7e30861dff7803f053c4b91be721966b7f89 Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Mon, 7 Oct 2024 10:59:23 -0500 Subject: [PATCH 04/10] [PM-10426] Admin Console - Edit Modal (#11249) * add `hideFolderSelection` for admin console ciphers * hide folder form field when configuration has `hideFolderSelection` set to true * add `addCipherV2` method in the admin console vault * add browser refresh logic for add/edit form * add admin console implementation of `AdminConsoleCipherFormConfigService` * only allow edit dialog in admin console * remove duplicate check * refactor comments * initial integration of combined dialog * integrate add cipher with admin console vault * account for special admin console collection permissions * add `edit` variable to AC ciphers when the user has permissions * Move comment to JSDoc * pass full cipher to view component * validate edit access when opening view form * partial-edit not applicable for admin console * refactor hideIndividualFields to be more generic and hide favorite button * pass entire cipher into edit logic to match view logic * add null check for cipher when attempting to view * remove logic for personal ownership, not needed in AC --- ...console-cipher-form-config.service.spec.ts | 119 +++++++++++ ...dmin-console-cipher-form-config.service.ts | 99 ++++++++++ .../app/vault/org-vault/vault.component.ts | 186 +++++++++++++----- .../cipher-form-config.service.ts | 3 + .../item-details-section.component.html | 8 +- 5 files changed, 361 insertions(+), 54 deletions(-) create mode 100644 apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.spec.ts create mode 100644 apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.ts diff --git a/apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.spec.ts b/apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.spec.ts new file mode 100644 index 00000000000..f0624e6b2f2 --- /dev/null +++ b/apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.spec.ts @@ -0,0 +1,119 @@ +import { TestBed } from "@angular/core/testing"; +import { BehaviorSubject } from "rxjs"; + +import { CollectionAdminService } from "@bitwarden/admin-console/common"; +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { CipherId } from "@bitwarden/common/types/guid"; +import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; + +import { RoutedVaultFilterService } from "../../individual-vault/vault-filter/services/routed-vault-filter.service"; + +import { AdminConsoleCipherFormConfigService } from "./admin-console-cipher-form-config.service"; + +describe("AdminConsoleCipherFormConfigService", () => { + let adminConsoleConfigService: AdminConsoleCipherFormConfigService; + + const cipherId = "333-444-555" as CipherId; + const testOrg = { id: "333-44-55", name: "Test Org", canEditAllCiphers: false }; + const organization$ = new BehaviorSubject(testOrg as Organization); + const getCipherAdmin = jest.fn().mockResolvedValue(null); + const getCipher = jest.fn().mockResolvedValue(null); + + beforeEach(async () => { + getCipherAdmin.mockClear(); + getCipher.mockClear(); + getCipher.mockResolvedValue({ id: cipherId, name: "Test Cipher - (non-admin)" }); + getCipherAdmin.mockResolvedValue({ id: cipherId, name: "Test Cipher - (admin)" }); + + await TestBed.configureTestingModule({ + providers: [ + AdminConsoleCipherFormConfigService, + { provide: OrganizationService, useValue: { get$: () => organization$ } }, + { provide: CipherService, useValue: { get: getCipher } }, + { provide: CollectionAdminService, useValue: { getAll: () => Promise.resolve([]) } }, + { + provide: RoutedVaultFilterService, + useValue: { filter$: new BehaviorSubject({ organizationId: testOrg.id }) }, + }, + { provide: ApiService, useValue: { getCipherAdmin } }, + ], + }); + }); + + describe("buildConfig", () => { + it("sets individual attributes", async () => { + adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService); + + const { folders, hideIndividualVaultFields } = await adminConsoleConfigService.buildConfig( + "add", + cipherId, + ); + + expect(folders).toEqual([]); + expect(hideIndividualVaultFields).toBe(true); + }); + + it("sets mode based on passed mode", async () => { + adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService); + + const { mode } = await adminConsoleConfigService.buildConfig("edit", cipherId); + + expect(mode).toBe("edit"); + }); + + it("sets admin flag based on `canEditAllCiphers`", async () => { + // Disable edit all ciphers on org + testOrg.canEditAllCiphers = false; + adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService); + + let result = await adminConsoleConfigService.buildConfig("add", cipherId); + + expect(result.admin).toBe(false); + + // Enable edit all ciphers on org + testOrg.canEditAllCiphers = true; + result = await adminConsoleConfigService.buildConfig("add", cipherId); + + expect(result.admin).toBe(true); + }); + + it("sets `allowPersonalOwnership` to false", async () => { + adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService); + + const result = await adminConsoleConfigService.buildConfig("clone", cipherId); + + expect(result.allowPersonalOwnership).toBe(false); + }); + + describe("getCipher", () => { + it("retrieves the cipher from the cipher service", async () => { + testOrg.canEditAllCiphers = false; + + adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService); + + const result = await adminConsoleConfigService.buildConfig("clone", cipherId); + + expect(getCipher).toHaveBeenCalledWith(cipherId); + expect(result.originalCipher.name).toBe("Test Cipher - (non-admin)"); + + // Admin service not needed when cipher service can return the cipher + expect(getCipherAdmin).not.toHaveBeenCalled(); + }); + + it("retrieves the cipher from the admin service", async () => { + getCipher.mockResolvedValueOnce(null); + getCipherAdmin.mockResolvedValue({ id: cipherId, name: "Test Cipher - (admin)" }); + + adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService); + + await adminConsoleConfigService.buildConfig("add", cipherId); + + expect(getCipherAdmin).toHaveBeenCalledWith(cipherId); + + expect(getCipher).toHaveBeenCalledWith(cipherId); + }); + }); + }); +}); diff --git a/apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.ts b/apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.ts new file mode 100644 index 00000000000..fa5cbedfca8 --- /dev/null +++ b/apps/web/src/app/vault/org-vault/services/admin-console-cipher-form-config.service.ts @@ -0,0 +1,99 @@ +import { inject, Injectable } from "@angular/core"; +import { combineLatest, filter, firstValueFrom, map, switchMap } from "rxjs"; + +import { CollectionAdminService } from "@bitwarden/admin-console/common"; +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { CipherId } from "@bitwarden/common/types/guid"; +import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { CipherType } from "@bitwarden/common/vault/enums"; +import { CipherData } from "@bitwarden/common/vault/models/data/cipher.data"; +import { Cipher } from "@bitwarden/common/vault/models/domain/cipher"; + +import { + CipherFormConfig, + CipherFormConfigService, + CipherFormMode, +} from "../../../../../../../libs/vault/src/cipher-form/abstractions/cipher-form-config.service"; +import { RoutedVaultFilterService } from "../../individual-vault/vault-filter/services/routed-vault-filter.service"; + +/** Admin Console implementation of the `CipherFormConfigService`. */ +@Injectable() +export class AdminConsoleCipherFormConfigService implements CipherFormConfigService { + private organizationService: OrganizationService = inject(OrganizationService); + private cipherService: CipherService = inject(CipherService); + private routedVaultFilterService: RoutedVaultFilterService = inject(RoutedVaultFilterService); + private collectionAdminService: CollectionAdminService = inject(CollectionAdminService); + private apiService: ApiService = inject(ApiService); + + private organizationId$ = this.routedVaultFilterService.filter$.pipe( + map((filter) => filter.organizationId), + filter((filter) => filter !== undefined), + ); + + private organization$ = this.organizationId$.pipe( + switchMap((organizationId) => this.organizationService.get$(organizationId)), + ); + + private editableCollections$ = this.organization$.pipe( + switchMap(async (org) => { + const collections = await this.collectionAdminService.getAll(org.id); + // Users that can edit all ciphers can implicitly add to / edit within any collection + if (org.canEditAllCiphers) { + return collections; + } + // The user is only allowed to add/edit items to assigned collections that are not readonly + return collections.filter((c) => c.assigned && !c.readOnly); + }), + ); + + async buildConfig( + mode: CipherFormMode, + cipherId?: CipherId, + cipherType?: CipherType, + ): Promise { + const [organization, allCollections] = await firstValueFrom( + combineLatest([this.organization$, this.editableCollections$]), + ); + + const cipher = await this.getCipher(organization, cipherId); + + const collections = allCollections.filter( + (c) => c.organizationId === organization.id && c.assigned && !c.readOnly, + ); + + return { + mode, + cipherType: cipher?.type ?? cipherType ?? CipherType.Login, + admin: organization.canEditAllCiphers ?? false, + allowPersonalOwnership: false, + originalCipher: cipher, + collections, + organizations: [organization], // only a single org is in context at a time + folders: [], // folders not applicable in the admin console + hideIndividualVaultFields: true, + }; + } + + private async getCipher(organization: Organization, id?: CipherId): Promise { + if (id == null) { + return Promise.resolve(null); + } + + // Check to see if the user has direct access to the cipher + const cipherFromCipherService = await this.cipherService.get(id); + + // If the organization doesn't allow admin/owners to edit all ciphers return the cipher + if (!organization.canEditAllCiphers && cipherFromCipherService != null) { + return cipherFromCipherService; + } + + // Retrieve the cipher through the means of an admin + const cipherResponse = await this.apiService.getCipherAdmin(id); + cipherResponse.edit = true; + + const cipherData = new CipherData(cipherResponse); + return new Cipher(cipherData); + } +} diff --git a/apps/web/src/app/vault/org-vault/vault.component.ts b/apps/web/src/app/vault/org-vault/vault.component.ts index a52530dde1a..9c8ff3f2ff5 100644 --- a/apps/web/src/app/vault/org-vault/vault.component.ts +++ b/apps/web/src/app/vault/org-vault/vault.component.ts @@ -1,3 +1,4 @@ +import { DialogRef } from "@angular/cdk/dialog"; import { ChangeDetectorRef, Component, @@ -42,16 +43,17 @@ import { EventCollectionService } from "@bitwarden/common/abstractions/event/eve import { SearchService } from "@bitwarden/common/abstractions/search.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { EventType } from "@bitwarden/common/enums"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { SyncService } from "@bitwarden/common/platform/sync"; -import { OrganizationId } from "@bitwarden/common/types/guid"; +import { CipherId, CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service"; import { TotpService } from "@bitwarden/common/vault/abstractions/totp.service"; @@ -62,7 +64,12 @@ import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view"; import { ServiceUtils } from "@bitwarden/common/vault/service-utils"; import { DialogService, Icons, NoItemsModule, ToastService } from "@bitwarden/components"; -import { CollectionAssignmentResult, PasswordRepromptService } from "@bitwarden/vault"; +import { + CipherFormConfig, + CipherFormConfigService, + CollectionAssignmentResult, + PasswordRepromptService, +} from "@bitwarden/vault"; import { GroupService, GroupView } from "../../admin-console/organizations/core"; import { openEntityEventsDialog } from "../../admin-console/organizations/manage/entity-events.component"; @@ -75,6 +82,11 @@ import { CollectionDialogTabType, openCollectionDialog, } from "../components/collection-dialog"; +import { + VaultItemDialogComponent, + VaultItemDialogMode, + VaultItemDialogResult, +} from "../components/vault-item-dialog/vault-item-dialog.component"; import { VaultItemEvent } from "../components/vault-items/vault-item-event"; import { VaultItemsModule } from "../components/vault-items/vault-items.module"; import { @@ -89,12 +101,6 @@ import { All, RoutedVaultFilterModel, } from "../individual-vault/vault-filter/shared/models/routed-vault-filter.model"; -import { - openViewCipherDialog, - ViewCipherDialogCloseResult, - ViewCipherDialogResult, - ViewComponent, -} from "../individual-vault/view.component"; import { VaultHeaderComponent } from "../org-vault/vault-header/vault-header.component"; import { getNestedCollectionTree } from "../utils/collection-utils"; @@ -106,8 +112,8 @@ import { } from "./bulk-collections-dialog"; import { CollectionAccessRestrictedComponent } from "./collection-access-restricted.component"; import { openOrgVaultCollectionsDialog } from "./collections.component"; +import { AdminConsoleCipherFormConfigService } from "./services/admin-console-cipher-form-config.service"; import { VaultFilterModule } from "./vault-filter/vault-filter.module"; - const BroadcasterSubscriptionId = "OrgVaultComponent"; const SearchTextDebounceInterval = 200; @@ -127,9 +133,12 @@ enum AddAccessStatusType { VaultItemsModule, SharedModule, NoItemsModule, - ViewComponent, ], - providers: [RoutedVaultFilterService, RoutedVaultFilterBridgeService], + providers: [ + RoutedVaultFilterService, + RoutedVaultFilterBridgeService, + { provide: CipherFormConfigService, useClass: AdminConsoleCipherFormConfigService }, + ], }) export class VaultComponent implements OnInit, OnDestroy { protected Unassigned = Unassigned; @@ -174,6 +183,8 @@ export class VaultComponent implements OnInit, OnDestroy { private refresh$ = new BehaviorSubject(null); private destroy$ = new Subject(); protected addAccessStatus$ = new BehaviorSubject(0); + private extensionRefreshEnabled: boolean; + private vaultItemDialogRef?: DialogRef | undefined; constructor( private route: ActivatedRoute, @@ -203,10 +214,15 @@ export class VaultComponent implements OnInit, OnDestroy { private apiService: ApiService, private collectionService: CollectionService, private toastService: ToastService, - private accountService: AccountService, + private configService: ConfigService, + private cipherFormConfigService: CipherFormConfigService, ) {} async ngOnInit() { + this.extensionRefreshEnabled = await this.configService.getFeatureFlag( + FeatureFlag.ExtensionRefresh, + ); + this.trashCleanupWarning = this.i18nService.t( this.platformUtilsService.isSelfHost() ? "trashCleanupWarningSelfHosted" @@ -466,22 +482,27 @@ export class VaultComponent implements OnInit, OnDestroy { firstSetup$ .pipe( switchMap(() => this.route.queryParams), + // Only process the queryParams if the dialog is not open (only when extension refresh is enabled) + filter(() => this.vaultItemDialogRef == undefined || !this.extensionRefreshEnabled), withLatestFrom(allCipherMap$, allCollections$, organization$), - switchMap(async ([qParams, allCiphersMap, allCollections]) => { + switchMap(async ([qParams, allCiphersMap]) => { const cipherId = getCipherIdFromParams(qParams); if (!cipherId) { return; } const cipher = allCiphersMap[cipherId]; - const cipherCollections = allCollections.filter((c) => - cipher.collectionIds.includes(c.id), - ); if (cipher) { - if (qParams.action === "view") { - await this.viewCipher(cipher, cipherCollections); + let action = qParams.action; + // Default to "view" if extension refresh is enabled + if (action == null && this.extensionRefreshEnabled) { + action = "view"; + } + + if (action === "view") { + await this.viewCipherById(cipher); } else { - await this.editCipherId(cipherId); + await this.editCipherId(cipher, false); } } else { this.toastService.showToast({ @@ -730,12 +751,16 @@ export class VaultComponent implements OnInit, OnDestroy { } async addCipher(cipherType?: CipherType) { + if (this.extensionRefreshEnabled) { + return this.addCipherV2(cipherType); + } + let collections: CollectionView[] = []; // Admins limited to only adding items to collections they have access to. collections = await firstValueFrom(this.editableCollections$); - await this.editCipher(null, (comp) => { + await this.editCipher(null, false, (comp) => { comp.type = cipherType || this.activeFilter.cipherType; comp.collections = collections; if (this.activeFilter.collectionId) { @@ -744,20 +769,46 @@ export class VaultComponent implements OnInit, OnDestroy { }); } + /** Opens the Add/Edit Dialog. Only to be used when the BrowserExtension feature flag is active */ + async addCipherV2(cipherType?: CipherType) { + const cipherFormConfig = await this.cipherFormConfigService.buildConfig( + "add", + null, + cipherType, + ); + + const collectionId: CollectionId | undefined = this.activeFilter.collectionId as CollectionId; + + cipherFormConfig.initialValues = { + organizationId: this.organization.id as OrganizationId, + collectionIds: collectionId ? [collectionId] : [], + }; + + await this.openVaultItemDialog("form", cipherFormConfig); + } + + /** + * Edit the given cipher + * @param cipherView - The cipher to be edited + * @param cloneCipher - `true` when the cipher should be cloned. + * Used in place of the `additionalComponentParameters`, as + * the `editCipherIdV2` method has a differing implementation. + * @param defaultComponentParameters - A method that takes in an instance of + * the `AddEditComponent` to edit methods directly. + */ async editCipher( cipher: CipherView, + cloneCipher: boolean, additionalComponentParameters?: (comp: AddEditComponent) => void, ) { - return this.editCipherId(cipher?.id, additionalComponentParameters); + return this.editCipherId(cipher, cloneCipher, additionalComponentParameters); } async editCipherId( - cipherId: string, + cipher: CipherView, + cloneCipher: boolean, additionalComponentParameters?: (comp: AddEditComponent) => void, ) { - const cipher = await this.cipherService.get(cipherId); - // if cipher exists (cipher is null when new) and MP reprompt - // is on for this cipher, then show password reprompt if ( cipher && cipher.reprompt !== 0 && @@ -768,10 +819,15 @@ export class VaultComponent implements OnInit, OnDestroy { return; } + if (this.extensionRefreshEnabled) { + await this.editCipherIdV2(cipher, cloneCipher); + return; + } + const defaultComponentParameters = (comp: AddEditComponent) => { comp.organization = this.organization; comp.organizationId = this.organization.id; - comp.cipherId = cipherId; + comp.cipherId = cipher.id; comp.onSavedCipher.pipe(takeUntil(this.destroy$)).subscribe(() => { modal.close(); this.refresh(); @@ -807,46 +863,70 @@ export class VaultComponent implements OnInit, OnDestroy { } /** - * Takes a cipher and its assigned collections to opens dialog where it can be viewed. - * @param cipher - the cipher to view - * @param collections - the collections the cipher is assigned to + * Edit a cipher using the new AddEditCipherDialogV2 component. + * Only to be used behind the ExtensionRefresh feature flag. */ - async viewCipher(cipher: CipherView, collections: CollectionView[] = []) { + private async editCipherIdV2(cipher: CipherView, cloneCipher: boolean) { + const cipherFormConfig = await this.cipherFormConfigService.buildConfig( + cloneCipher ? "clone" : "edit", + cipher.id as CipherId, + ); + + await this.openVaultItemDialog("form", cipherFormConfig, cipher); + } + + /** Opens the view dialog for the given cipher unless password reprompt fails */ + async viewCipherById(cipher: CipherView) { if (!cipher) { - this.go({ cipherId: null, itemId: null }); return; } - if (cipher.reprompt !== 0 && !(await this.passwordRepromptService.showPasswordPrompt())) { - // didn't pass password prompt, so don't open the dialog - this.go({ cipherId: null, itemId: null }); + if ( + cipher && + cipher.reprompt !== 0 && + !(await this.passwordRepromptService.showPasswordPrompt()) + ) { + // Didn't pass password prompt, so don't open add / edit modal. + await this.go({ cipherId: null, itemId: null, action: null }); return; } - const dialogRef = openViewCipherDialog(this.dialogService, { - data: { - cipher: cipher, - collections: collections, - disableEdit: !cipher.edit && !this.organization.canEditAllCiphers, - }, + const cipherFormConfig = await this.cipherFormConfigService.buildConfig( + "edit", + cipher.id as CipherId, + cipher.type, + ); + + await this.openVaultItemDialog("view", cipherFormConfig, cipher); + } + + /** + * Open the combined view / edit dialog for a cipher. + */ + async openVaultItemDialog( + mode: VaultItemDialogMode, + formConfig: CipherFormConfig, + cipher?: CipherView, + ) { + const disableForm = cipher ? !cipher.edit && !this.organization.canEditAllCiphers : false; + // If the form is disabled, force the mode into `view` + const dialogMode = disableForm ? "view" : mode; + this.vaultItemDialogRef = VaultItemDialogComponent.open(this.dialogService, { + mode: dialogMode, + formConfig, + disableForm, }); - // Wait for the dialog to close. - const result: ViewCipherDialogCloseResult = await lastValueFrom(dialogRef.closed); - - // If the dialog was closed by clicking the edit button, navigate to open the edit dialog. - if (result?.action === ViewCipherDialogResult.Edited) { - this.go({ itemId: cipher.id, action: "edit" }); - return; - } + const result = await lastValueFrom(this.vaultItemDialogRef.closed); + this.vaultItemDialogRef = undefined; // If the dialog was closed by deleting the cipher, refresh the vault. - if (result?.action === ViewCipherDialogResult.Deleted) { + if (result === VaultItemDialogResult.Deleted || result === VaultItemDialogResult.Saved) { this.refresh(); } - // Clear the query params when the view dialog closes - this.go({ cipherId: null, itemId: null, action: null }); + // Clear the query params when the dialog closes + await this.go({ cipherId: null, itemId: null, action: null }); } async cloneCipher(cipher: CipherView) { @@ -867,7 +947,7 @@ export class VaultComponent implements OnInit, OnDestroy { // Admins limited to only adding items to collections they have access to. collections = await firstValueFrom(this.editableCollections$); - await this.editCipher(cipher, (comp) => { + await this.editCipher(cipher, true, (comp) => { comp.cloneMode = true; comp.collections = collections; comp.collectionIds = cipher.collectionIds; diff --git a/libs/vault/src/cipher-form/abstractions/cipher-form-config.service.ts b/libs/vault/src/cipher-form/abstractions/cipher-form-config.service.ts index 25c0e411243..28c18b3665f 100644 --- a/libs/vault/src/cipher-form/abstractions/cipher-form-config.service.ts +++ b/libs/vault/src/cipher-form/abstractions/cipher-form-config.service.ts @@ -79,6 +79,9 @@ type BaseCipherFormConfig = { * List of organizations that the user can create ciphers for. */ organizations?: Organization[]; + + /** Hides the fields that are only applicable to individuals, useful in the Admin Console where folders aren't applicable */ + hideIndividualVaultFields?: true; }; /** diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html index 26f7f7fd9e8..6c6bd8a801e 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html @@ -2,6 +2,7 @@

{{ "itemDetails" | i18n }}

@@ -35,7 +36,7 @@ bitIconButton="bwi-clone" bitSuffix [appA11yTitle]="'copyPassword' | i18n" - [disabled]="!sendOptionsForm.get('password').value" + [disabled]="!config.areSendsAllowed || !sendOptionsForm.get('password').value" [valueLabel]="'password' | i18n" [appCopyClick]="sendOptionsForm.get('password').value" showToast From b6ea6075b321ef28881744f837f2ec66de277929 Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Tue, 8 Oct 2024 05:40:11 -0500 Subject: [PATCH 09/10] [PM-13188] Update auto-submit to act on uri hash instead of query param (#11416) --- .../auto-submit-login.background.spec.ts | 6 +++--- .../background/auto-submit-login.background.ts | 16 ++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/apps/browser/src/autofill/background/auto-submit-login.background.spec.ts b/apps/browser/src/autofill/background/auto-submit-login.background.spec.ts index ea86a84d63f..73f936bb591 100644 --- a/apps/browser/src/autofill/background/auto-submit-login.background.spec.ts +++ b/apps/browser/src/autofill/background/auto-submit-login.background.spec.ts @@ -42,7 +42,7 @@ describe("AutoSubmitLoginBackground", () => { const validIpdUrl1 = "https://example.com"; const validIpdUrl2 = "https://subdomain.example3.com"; const validAutoSubmitHost = "some-valid-url.com"; - const validAutoSubmitUrl = `https://${validAutoSubmitHost}/?autofill=1`; + const validAutoSubmitUrl = `https://${validAutoSubmitHost}/#autosubmit=1`; beforeEach(() => { logService = mock(); @@ -122,7 +122,7 @@ describe("AutoSubmitLoginBackground", () => { await autoSubmitLoginBackground.init(); }); - it("sets up the auto-submit workflow when the web request occurs in the main frame and the destination URL contains a valid auto-fill param", () => { + it("sets up the auto-submit workflow when the web request occurs in the main frame and the destination URL contains a valid auto-fill hash", () => { triggerWebRequestOnBeforeRequestEvent(webRequestDetails); expect(autoSubmitLoginBackground["currentAutoSubmitHostData"]).toStrictEqual({ @@ -226,7 +226,7 @@ describe("AutoSubmitLoginBackground", () => { it("disables the auto-submit workflow if a web request is initiated after the auto-submit route has been visited", () => { webRequestDetails.url = `https://${validAutoSubmitHost}`; - webRequestDetails.initiator = `https://${validAutoSubmitHost}?autofill=1`; + webRequestDetails.initiator = `https://${validAutoSubmitHost}#autosubmit=1`; triggerWebRequestOnBeforeRequestEvent(webRequestDetails); diff --git a/apps/browser/src/autofill/background/auto-submit-login.background.ts b/apps/browser/src/autofill/background/auto-submit-login.background.ts index 52d4cb2b419..180a6ba5462 100644 --- a/apps/browser/src/autofill/background/auto-submit-login.background.ts +++ b/apps/browser/src/autofill/background/auto-submit-login.background.ts @@ -234,7 +234,7 @@ export class AutoSubmitLoginBackground implements AutoSubmitLoginBackgroundAbstr ) => { if ( details.tabId === this.currentAutoSubmitHostData.tabId && - this.urlContainsAutoFillParam(details.url) + this.urlContainsAutoSubmitHash(details.url) ) { this.injectAutoSubmitLoginScript(details.tabId).catch((error) => this.logService.error(error), @@ -277,7 +277,7 @@ export class AutoSubmitLoginBackground implements AutoSubmitLoginBackgroundAbstr private handleWebRequestOnBeforeRedirect = ( details: chrome.webRequest.WebRedirectionResponseDetails, ) => { - if (this.isRequestInMainFrame(details) && this.urlContainsAutoFillParam(details.redirectUrl)) { + if (this.isRequestInMainFrame(details) && this.urlContainsAutoSubmitHash(details.redirectUrl)) { this.validAutoSubmitHosts.add(this.getUrlHost(details.redirectUrl)); this.validAutoSubmitHosts.add(this.getUrlHost(details.url)); } @@ -369,7 +369,7 @@ export class AutoSubmitLoginBackground implements AutoSubmitLoginBackgroundAbstr /** * Determines if the provided URL is a valid auto-submit host. If the request is occurring - * in the main frame, we will check for the presence of the `autofill=1` query parameter. + * in the main frame, we will check for the presence of the `autosubmit=1` uri hash. * If the request is occurring in a sub frame, the main frame URL should be set as a * valid auto-submit host and can be used to validate the request. * @@ -382,7 +382,7 @@ export class AutoSubmitLoginBackground implements AutoSubmitLoginBackgroundAbstr ) => { if (this.isRequestInMainFrame(details)) { return !!( - this.urlContainsAutoFillParam(details.url) || + this.urlContainsAutoSubmitHash(details.url) || this.triggerAutoSubmitAfterRedirectOnSafari(details.url) ); } @@ -391,14 +391,14 @@ export class AutoSubmitLoginBackground implements AutoSubmitLoginBackgroundAbstr }; /** - * Determines if the provided URL contains the `autofill=1` query parameter. + * Determines if the provided URL contains the `autosubmit=1` uri hash. * - * @param url - The URL to check for the `autofill=1` query parameter. + * @param url - The URL to check for the `autosubmit=1` uri hash. */ - private urlContainsAutoFillParam = (url: string) => { + private urlContainsAutoSubmitHash = (url: string) => { try { const urlObj = new URL(url); - return urlObj.search.indexOf("autofill=1") !== -1; + return urlObj.hash.indexOf("autosubmit=1") !== -1; } catch { return false; } From df14e3f030c49d1266b79c967c6e5866b69dabd7 Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk <167752252+mzieniukbw@users.noreply.github.com> Date: Tue, 8 Oct 2024 14:02:58 +0200 Subject: [PATCH 10/10] [PM-13207] Detect incompatible locale changes (#11425) * detecting unsupported modifications in locales * typo fix * limit to english locales, increased verbosity * increased verbosity --- .github/workflows/locales-lint.yml | 34 ++++++++++ package.json | 1 + scripts/.eslintrc.json | 5 ++ scripts/test-locales.ts | 105 +++++++++++++++++++++++++++++ scripts/tsconfig.json | 9 +++ tsconfig.eslint.json | 4 +- 6 files changed, 156 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/locales-lint.yml create mode 100644 scripts/.eslintrc.json create mode 100644 scripts/test-locales.ts create mode 100644 scripts/tsconfig.json diff --git a/.github/workflows/locales-lint.yml b/.github/workflows/locales-lint.yml new file mode 100644 index 00000000000..db2c66f7b88 --- /dev/null +++ b/.github/workflows/locales-lint.yml @@ -0,0 +1,34 @@ +--- +name: Locales lint for Crowdin + +on: + pull_request: + branches-ignore: + - 'l10n_master' + - 'cf-pages' + paths: + - '**/messages.json' + +jobs: + lint: + name: Lint + runs-on: ubuntu-22.04 + steps: + - name: Checkout repo + uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + - name: Checkout base branch repo + uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + with: + ref: ${{ github.event.pull_request.base.sha }} + path: base + - name: Install dependencies + run: npm ci + - name: Compare + run: | + npm run test:locales + if [ $? -eq 0 ]; then + echo "Lint check successful." + else + echo "Lint check failed." + exit 1 + fi diff --git a/package.json b/package.json index 2926fd095fc..6e4e6c1199e 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "test:watch": "jest --clearCache && jest --watch", "test:watch:all": "jest --watchAll", "test:types": "node ./scripts/test-types.js", + "test:locales": "tsc --project ./scripts/tsconfig.json && node ./scripts/dist/test-locales.js", "docs:json": "compodoc -p ./tsconfig.json -e json -d . --disableRoutesGraph", "storybook": "ng run components:storybook", "build-storybook": "ng run components:build-storybook", diff --git a/scripts/.eslintrc.json b/scripts/.eslintrc.json new file mode 100644 index 00000000000..10d22388378 --- /dev/null +++ b/scripts/.eslintrc.json @@ -0,0 +1,5 @@ +{ + "env": { + "node": true + } +} diff --git a/scripts/test-locales.ts b/scripts/test-locales.ts new file mode 100644 index 00000000000..85ed6e7afa7 --- /dev/null +++ b/scripts/test-locales.ts @@ -0,0 +1,105 @@ +/* eslint no-console:0 */ +import fs from "fs"; +import path from "path"; + +type Messages = { + [id: string]: { + message: string; + }; +}; + +function findLocaleFiles(dir: string): string[] { + return fs + .readdirSync(dir, { encoding: null, recursive: true }) + .filter((file) => path.basename(file) === "messages.json") + .filter((file) => path.dirname(file).endsWith("en")) + .map((file) => path.join(dir, file)); +} + +function findAllLocaleFiles(rootDir: string): string[] { + return [ + ...findLocaleFiles(path.join(rootDir, "apps", "browser", "src")), + ...findLocaleFiles(path.join(rootDir, "apps", "cli", "src")), + ...findLocaleFiles(path.join(rootDir, "apps", "desktop", "src")), + ...findLocaleFiles(path.join(rootDir, "apps", "web", "src")), + ].map((file) => path.relative(rootDir, file)); +} + +function readMessagesJson(file: string): Messages { + let content = fs.readFileSync(file, { encoding: "utf-8" }); + // Strip BOM + content = content.replace(/^\uFEFF/, ""); + try { + return JSON.parse(content); + } catch (e: unknown) { + console.error(`ERROR: Invalid JSON file ${file}`, e); + throw e; + } +} + +function compareMessagesJson(beforeFile: string, afterFile: string): boolean { + try { + console.log("Comparing locale files:", beforeFile, afterFile); + + const messagesBeforeJson = readMessagesJson(beforeFile); + const messagesAfterJson = readMessagesJson(afterFile); + + const messagesIdMapBefore = toMessageIdMap(messagesBeforeJson); + const messagesIdMapAfter = toMessageIdMap(messagesAfterJson); + + let changed = false; + + for (const [id, message] of messagesIdMapAfter.entries()) { + if (!messagesIdMapBefore.has(id)) { + console.log("New message:", id); + continue; + } + + if (messagesIdMapBefore.get(id) !== message) { + console.error("ERROR: Message changed:", id); + changed = true; + } + } + + return changed; + } catch (e: unknown) { + console.error(`ERROR: Unable to compare files ${beforeFile} and ${afterFile}`, e); + throw e; + } +} + +function toMessageIdMap(messagesJson: Messages): Map { + return Object.entries(messagesJson).reduce((map, [id, value]) => { + map.set(id, value.message); + return map; + }, new Map()); +} + +const rootDir = path.join(__dirname, "..", ".."); +const baseBranchRootDir = path.join(rootDir, "base"); + +const files = findAllLocaleFiles(rootDir); + +console.log("Detected valid English locale files:", files); + +let changedFiles = false; + +for (const file of files) { + const baseBranchFile = path.join(baseBranchRootDir, file); + if (!fs.existsSync(baseBranchFile)) { + console.error("ERROR: File not found in base branch:", file); + continue; + } + + const changed = compareMessagesJson(baseBranchFile, path.join(rootDir, file)); + changedFiles ||= changed; +} + +if (changedFiles) { + console.error( + "ERROR: Incompatible Crowdin locale files. " + + "All messages in messages.json locale files needs to be immutable and cannot be updated. " + + "If a message needs to be changed, create a new message id and update your code to use it instead.", + ); + process.exit(1); +} diff --git a/scripts/tsconfig.json b/scripts/tsconfig.json new file mode 100644 index 00000000000..018fb133932 --- /dev/null +++ b/scripts/tsconfig.json @@ -0,0 +1,9 @@ +{ + "extends": "../libs/shared/tsconfig", + "compilerOptions": { + "outDir": "dist", + "module": "NodeNext", + "target": "ESNext" + }, + "include": ["*.ts"] +} diff --git a/tsconfig.eslint.json b/tsconfig.eslint.json index 23696d35944..f8ab18b498f 100644 --- a/tsconfig.eslint.json +++ b/tsconfig.eslint.json @@ -42,6 +42,6 @@ } ] }, - "include": ["apps/**/*", "libs/**/*", "bitwarden_license/**/*"], - "exclude": ["**/build"] + "include": ["apps/**/*", "libs/**/*", "bitwarden_license/**/*", "scripts/**/*"], + "exclude": ["**/build", "**/dist"] }