diff --git a/.github/workflows/build-browser.yml b/.github/workflows/build-browser.yml index 34c69912f50..ecd1e404944 100644 --- a/.github/workflows/build-browser.yml +++ b/.github/workflows/build-browser.yml @@ -1,7 +1,8 @@ name: Build Browser on: - pull_request: + pull_request_target: + types: [opened, synchronize] branches-ignore: - 'l10n_master' - 'cf-pages' @@ -33,6 +34,10 @@ defaults: shell: bash jobs: + check-run: + name: Check PR run + uses: bitwarden/gh-actions/.github/workflows/check-run.yml@main + setup: name: Setup runs-on: ubuntu-22.04 @@ -41,8 +46,10 @@ jobs: adj_build_number: ${{ steps.gen_vars.outputs.adj_build_number }} node_version: ${{ steps.retrieve-node-version.outputs.node_version }} steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Get Package Version id: gen_vars @@ -71,8 +78,10 @@ jobs: run: working-directory: apps/browser steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Testing locales - extName length run: | @@ -109,8 +118,10 @@ jobs: _BUILD_NUMBER: ${{ needs.setup.outputs.adj_build_number }} _NODE_VERSION: ${{ needs.setup.outputs.node_version }} steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Set up Node uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0 @@ -225,12 +236,15 @@ jobs: needs: - setup - locales-test + - check-run env: _BUILD_NUMBER: ${{ needs.setup.outputs.adj_build_number }} _NODE_VERSION: ${{ needs.setup.outputs.node_version }} steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Set up Node uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0 @@ -342,8 +356,10 @@ jobs: - build - build-safari steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Login to Azure uses: Azure/login@e15b166166a8746d1a47596803bd8c1b595455cf # v1.6.0 @@ -381,7 +397,10 @@ jobs: - crowdin-push steps: - name: Check if any job failed - if: (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc') && contains(needs.*.result, 'failure') + if: | + github.event_name != 'pull_request_target' + && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc') + && contains(needs.*.result, 'failure') run: exit 1 - name: Login to Azure - Prod Subscription diff --git a/.github/workflows/build-cli.yml b/.github/workflows/build-cli.yml index 7994e508b3c..98ba5b9fd8a 100644 --- a/.github/workflows/build-cli.yml +++ b/.github/workflows/build-cli.yml @@ -1,7 +1,8 @@ name: Build CLI on: - pull_request: + pull_request_target: + types: [opened, synchronize] branches-ignore: - 'l10n_master' - 'cf-pages' @@ -34,6 +35,10 @@ defaults: working-directory: apps/cli jobs: + check-run: + name: Check PR run + uses: bitwarden/gh-actions/.github/workflows/check-run.yml@main + setup: name: Setup runs-on: ubuntu-22.04 @@ -41,8 +46,10 @@ jobs: package_version: ${{ steps.retrieve-package-version.outputs.package_version }} node_version: ${{ steps.retrieve-node-version.outputs.node_version }} steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Get Package Version id: retrieve-package-version @@ -58,7 +65,6 @@ jobs: NODE_VERSION=${NODE_NVMRC/v/''} echo "node_version=$NODE_VERSION" >> $GITHUB_OUTPUT - cli: name: "${{ matrix.os.base }} - ${{ matrix.license_type.readable }}" strategy: @@ -82,8 +88,10 @@ jobs: _WIN_PKG_FETCH_VERSION: 20.11.1 _WIN_PKG_VERSION: 3.5 steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Setup Unix Vars run: | @@ -160,8 +168,10 @@ jobs: _WIN_PKG_FETCH_VERSION: 20.11.1 _WIN_PKG_VERSION: 3.5 steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Setup Windows builder run: | @@ -310,8 +320,10 @@ jobs: env: _PACKAGE_VERSION: ${{ needs.setup.outputs.package_version }} steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Print environment run: | @@ -386,10 +398,14 @@ jobs: - cli - cli-windows - snap + - check-run steps: - name: Check if any job failed working-directory: ${{ github.workspace }} - if: (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc') && contains(needs.*.result, 'failure') + if: | + github.event_name != 'pull_request_target' + && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc') + && contains(needs.*.result, 'failure') run: exit 1 - name: Login to Azure - Prod Subscription diff --git a/.github/workflows/build-desktop.yml b/.github/workflows/build-desktop.yml index 4667a937113..83389c5bbec 100644 --- a/.github/workflows/build-desktop.yml +++ b/.github/workflows/build-desktop.yml @@ -1,7 +1,8 @@ name: Build Desktop on: - pull_request: + pull_request_target: + types: [opened, synchronize] branches-ignore: - 'l10n_master' - 'cf-pages' @@ -32,12 +33,18 @@ defaults: shell: bash jobs: + check-run: + name: Check PR run + uses: bitwarden/gh-actions/.github/workflows/check-run.yml@main + electron-verify: name: Verify Electron Version runs-on: ubuntu-22.04 steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Verify run: | @@ -65,8 +72,10 @@ jobs: run: working-directory: apps/desktop steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Get Package Version id: retrieve-version @@ -138,8 +147,10 @@ jobs: run: working-directory: apps/desktop steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Set up Node uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0 @@ -238,7 +249,9 @@ jobs: windows: name: Windows Build runs-on: windows-2022 - needs: setup + needs: + - setup + - check-run defaults: run: shell: pwsh @@ -248,8 +261,10 @@ jobs: _NODE_VERSION: ${{ needs.setup.outputs.node_version }} NODE_OPTIONS: --max_old_space_size=4096 steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Set up Node uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0 @@ -447,7 +462,9 @@ jobs: macos-build: name: MacOS Build runs-on: macos-13 - needs: setup + needs: + - setup + - check-run env: _PACKAGE_VERSION: ${{ needs.setup.outputs.package_version }} _NODE_VERSION: ${{ needs.setup.outputs.node_version }} @@ -456,8 +473,10 @@ jobs: run: working-directory: apps/desktop steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Set up Node uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0 @@ -622,8 +641,10 @@ jobs: run: working-directory: apps/desktop steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Set up Node uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0 @@ -841,8 +862,10 @@ jobs: run: working-directory: apps/desktop steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Set up Node uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0 @@ -1088,8 +1111,10 @@ jobs: run: working-directory: apps/desktop steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Set up Node uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0 @@ -1279,8 +1304,10 @@ jobs: - macos-package-mas runs-on: ubuntu-22.04 steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Login to Azure uses: Azure/login@e15b166166a8746d1a47596803bd8c1b595455cf # v1.6.0 @@ -1323,7 +1350,10 @@ jobs: - crowdin-push steps: - name: Check if any job failed - if: (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc') && contains(needs.*.result, 'failure') + if: | + github.event_name != 'pull_request_target' + && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc') + && contains(needs.*.result, 'failure') run: exit 1 - name: Login to Azure - Prod Subscription diff --git a/.github/workflows/build-web.yml b/.github/workflows/build-web.yml index 31f800d5b37..4ce5bad790f 100644 --- a/.github/workflows/build-web.yml +++ b/.github/workflows/build-web.yml @@ -1,7 +1,8 @@ name: Build Web on: - pull_request: + pull_request_target: + types: [opened, synchronize] branches-ignore: - 'l10n_master' - 'cf-pages' @@ -36,6 +37,10 @@ env: _AZ_REGISTRY: bitwardenprod.azurecr.io jobs: + check-run: + name: Check PR run + uses: bitwarden/gh-actions/.github/workflows/check-run.yml@main + setup: name: Setup runs-on: ubuntu-22.04 @@ -43,8 +48,10 @@ jobs: version: ${{ steps.version.outputs.value }} node_version: ${{ steps.retrieve-node-version.outputs.node_version }} steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Get GitHub sha as version id: version @@ -89,8 +96,10 @@ jobs: git_metadata: true steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Set up Node uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0 @@ -142,6 +151,7 @@ jobs: needs: - setup - build-artifacts + - check-run strategy: fail-fast: false matrix: @@ -155,8 +165,10 @@ jobs: env: _VERSION: ${{ needs.setup.outputs.version }} steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Check Branch to Publish env: @@ -250,11 +262,15 @@ jobs: crowdin-push: name: Crowdin Push if: github.ref == 'refs/heads/main' - needs: build-artifacts + needs: + - build-artifacts + - check-run runs-on: ubuntu-22.04 steps: - - name: Checkout repo + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Login to Azure uses: Azure/login@e15b166166a8746d1a47596803bd8c1b595455cf # v1.6.0 @@ -282,9 +298,11 @@ jobs: trigger-web-vault-deploy: name: Trigger web vault deploy - if: github.ref == 'refs/heads/main' + if: github.event_name != 'pull_request_target' && github.ref == 'refs/heads/main' runs-on: ubuntu-22.04 - needs: build-artifacts + needs: + - build-artifacts + - check-run steps: - name: Login to Azure - CI Subscription uses: Azure/login@e15b166166a8746d1a47596803bd8c1b595455cf # v1.6.0 @@ -326,7 +344,10 @@ jobs: - trigger-web-vault-deploy steps: - name: Check if any job failed - if: (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc') && contains(needs.*.result, 'failure') + if: | + github.event_name != 'pull_request_target' + && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc') + && contains(needs.*.result, 'failure') run: exit 1 - name: Login to Azure - Prod Subscription diff --git a/apps/browser/src/autofill/popup/settings/autofill.component.ts b/apps/browser/src/autofill/popup/settings/autofill.component.ts index b395808f57a..ac247609b13 100644 --- a/apps/browser/src/autofill/popup/settings/autofill.component.ts +++ b/apps/browser/src/autofill/popup/settings/autofill.component.ts @@ -219,7 +219,11 @@ export class AutofillComponent implements OnInit { : AutofillOverlayVisibility.Off; await this.autofillSettingsService.setInlineMenuVisibility(newInlineMenuVisibilityValue); - await this.requestPrivacyPermission(); + + // No need to initiate browser permission request if a feature is being turned off + if (newInlineMenuVisibilityValue !== AutofillOverlayVisibility.Off) { + await this.requestPrivacyPermission(); + } } async updateAutofillOnPageLoad() { diff --git a/apps/browser/src/tools/popup/send-v2/add-edit/send-add-edit.component.ts b/apps/browser/src/tools/popup/send-v2/add-edit/send-add-edit.component.ts index 585f6067e3d..d1005883651 100644 --- a/apps/browser/src/tools/popup/send-v2/add-edit/send-add-edit.component.ts +++ b/apps/browser/src/tools/popup/send-v2/add-edit/send-add-edit.component.ts @@ -24,9 +24,9 @@ import { SendFormConfig, SendFormConfigService, SendFormMode, + SendFormModule, } from "@bitwarden/send-ui"; -import { SendFormModule } from "../../../../../../../libs/tools/send/send-ui/src/send-form/send-form.module"; import { PopupFooterComponent } from "../../../../platform/popup/layout/popup-footer.component"; import { PopupHeaderComponent } from "../../../../platform/popup/layout/popup-header.component"; import { PopupPageComponent } from "../../../../platform/popup/layout/popup-page.component"; diff --git a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts index ae2cf88fd1f..bf623e729a1 100644 --- a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts +++ b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts @@ -6,6 +6,7 @@ import { firstValueFrom, Observable, Subject } from "rxjs"; import { map } from "rxjs/operators"; import { CollectionView } from "@bitwarden/admin-console/common"; +import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; @@ -17,6 +18,8 @@ import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.servi import { PremiumUpgradePromptService } from "@bitwarden/common/vault/abstractions/premium-upgrade-prompt.service"; import { ViewPasswordHistoryService } from "@bitwarden/common/vault/abstractions/view-password-history.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 { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service"; import { @@ -231,6 +234,7 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy { private billingAccountProfileStateService: BillingAccountProfileStateService, private premiumUpgradeService: PremiumUpgradePromptService, private cipherAuthorizationService: CipherAuthorizationService, + private apiService: ApiService, ) { this.updateTitle(); } @@ -278,7 +282,20 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy { if (this._originalFormMode === "add" || this._originalFormMode === "clone") { this.formConfig.mode = "edit"; } - this.formConfig.originalCipher = await this.cipherService.get(cipherView.id); + + let cipher: Cipher; + + // When the form config is used within the Admin Console, retrieve the cipher from the admin endpoint + if (this.formConfig.isAdminConsole) { + const cipherResponse = await this.apiService.getCipherAdmin(cipherView.id); + const cipherData = new CipherData(cipherResponse); + cipher = new Cipher(cipherData); + } else { + cipher = await this.cipherService.get(cipherView.id); + } + + // Store the updated cipher so any following edits use the most up to date cipher + this.formConfig.originalCipher = cipher; this._cipherModified = true; await this.changeMode("view"); } diff --git a/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.ts b/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.ts index 3b7db72a09d..57eb9b1bdd9 100644 --- a/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.ts +++ b/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.ts @@ -1,5 +1,5 @@ import { Component, Inject, OnDestroy, OnInit } from "@angular/core"; -import { combineLatest, map, Observable, Subject, takeUntil } from "rxjs"; +import { combineLatest, map, Observable, of, Subject, switchMap, takeUntil } from "rxjs"; import { OrganizationUserApiService, @@ -8,11 +8,14 @@ import { import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { OrganizationApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization-api.service.abstraction"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +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 { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; @@ -53,6 +56,8 @@ export class OrganizationOptionsComponent implements OnInit, OnDestroy { private resetPasswordService: OrganizationUserResetPasswordService, private userVerificationService: UserVerificationService, private toastService: ToastService, + private configService: ConfigService, + private organizationService: OrganizationService, ) {} async ngOnInit() { @@ -60,23 +65,39 @@ export class OrganizationOptionsComponent implements OnInit, OnDestroy { map((policies) => policies.filter((policy) => policy.type === PolicyType.ResetPassword)), ); + const managingOrg$ = this.configService + .getFeatureFlag$(FeatureFlag.AccountDeprovisioning) + .pipe( + switchMap((isAccountDeprovisioningEnabled) => + isAccountDeprovisioningEnabled + ? this.organizationService.organizations$.pipe( + map((organizations) => + organizations.find((o) => o.userIsManagedByOrganization === true), + ), + ) + : of(null), + ), + ); + combineLatest([ this.organization$, resetPasswordPolicies$, this.userDecryptionOptionsService.userDecryptionOptions$, + managingOrg$, ]) .pipe(takeUntil(this.destroy$)) - .subscribe(([organization, resetPasswordPolicies, decryptionOptions]) => { + .subscribe(([organization, resetPasswordPolicies, decryptionOptions, managingOrg]) => { this.organization = organization; this.resetPasswordPolicy = resetPasswordPolicies.find( (p) => p.organizationId === organization.id, ); - // A user can leave an organization if they are NOT using TDE and Key Connector, or they have a master password. + // A user can leave an organization if they are NOT a managed user and they are NOT using TDE and Key Connector, or they have a master password. this.showLeaveOrgOption = - (decryptionOptions.trustedDeviceOption == undefined && + managingOrg?.id !== organization.id && + ((decryptionOptions.trustedDeviceOption == undefined && decryptionOptions.keyConnectorOption == undefined) || - decryptionOptions.hasMasterPassword; + decryptionOptions.hasMasterPassword); // Hide the 3 dot menu if the user has no available actions this.hideMenu = 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 index 02d280f5ff9..05c40fe2e79 100644 --- 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 @@ -1,14 +1,13 @@ import { TestBed } from "@angular/core/testing"; import { BehaviorSubject } from "rxjs"; -import { CollectionAdminService } from "@bitwarden/admin-console/common"; +import { CollectionAdminService, CollectionAdminView } 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 { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { OrganizationUserStatusType } from "@bitwarden/common/admin-console/enums"; 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"; @@ -35,27 +34,41 @@ describe("AdminConsoleCipherFormConfigService", () => { status: OrganizationUserStatusType.Confirmed, }; const policyAppliesToActiveUser$ = new BehaviorSubject(true); + const collection = { + id: "12345-5555", + organizationId: "234534-34334", + name: "Test Collection 1", + assigned: false, + readOnly: true, + } as CollectionAdminView; + const collection2 = { + id: "12345-6666", + organizationId: "22222-2222", + name: "Test Collection 2", + assigned: true, + readOnly: false, + } as CollectionAdminView; + const organization$ = new BehaviorSubject(testOrg as Organization); const organizations$ = new BehaviorSubject([testOrg, testOrg2] 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$, organizations$ } }, + { + provide: CollectionAdminService, + useValue: { getAll: () => Promise.resolve([collection, collection2]) }, + }, { provide: PolicyService, useValue: { policyAppliesToActiveUser$: () => policyAppliesToActiveUser$ }, }, - { provide: OrganizationService, useValue: { get$: () => organization$, organizations$ } }, - { provide: CipherService, useValue: { get: getCipher } }, - { provide: CollectionAdminService, useValue: { getAll: () => Promise.resolve([]) } }, { provide: RoutedVaultFilterService, useValue: { filter$: new BehaviorSubject({ organizationId: testOrg.id }) }, @@ -86,6 +99,12 @@ describe("AdminConsoleCipherFormConfigService", () => { expect(mode).toBe("edit"); }); + it("returns all collections", async () => { + const { collections } = await adminConsoleConfigService.buildConfig("edit", cipherId); + + expect(collections).toEqual([collection, collection2]); + }); + it("sets admin flag based on `canEditAllCiphers`", async () => { // Disable edit all ciphers on org testOrg.canEditAllCiphers = false; @@ -153,33 +172,14 @@ describe("AdminConsoleCipherFormConfigService", () => { expect(result.organizations).toEqual([testOrg, testOrg2]); }); - describe("getCipher", () => { - it("retrieves the cipher from the cipher service", async () => { - testOrg.canEditAllCiphers = false; + it("retrieves the cipher from the admin service", async () => { + getCipherAdmin.mockResolvedValue({ id: cipherId, name: "Test Cipher - (admin)" }); - adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService); + adminConsoleConfigService = TestBed.inject(AdminConsoleCipherFormConfigService); - const result = await adminConsoleConfigService.buildConfig("clone", cipherId); + await adminConsoleConfigService.buildConfig("add", 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); - }); + expect(getCipherAdmin).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 index 328ab4475dc..457b4e83d03 100644 --- 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 @@ -6,9 +6,7 @@ import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyType, OrganizationUserStatusType } from "@bitwarden/common/admin-console/enums"; -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"; @@ -25,7 +23,6 @@ import { RoutedVaultFilterService } from "../../individual-vault/vault-filter/se export class AdminConsoleCipherFormConfigService implements CipherFormConfigService { private policyService: PolicyService = inject(PolicyService); 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); @@ -51,20 +48,8 @@ export class AdminConsoleCipherFormConfigService implements CipherFormConfigServ map(([orgs, orgId]) => orgs.find((o) => o.id === orgId)), ); - private editableCollections$ = this.organization$.pipe( - switchMap(async (org) => { - if (!org) { - return []; - } - - 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); - }), + private allCollections$ = this.organization$.pipe( + switchMap(async (org) => await this.collectionAdminService.getAll(org.id)), ); async buildConfig( @@ -72,21 +57,17 @@ export class AdminConsoleCipherFormConfigService implements CipherFormConfigServ cipherId?: CipherId, cipherType?: CipherType, ): Promise { + const cipher = await this.getCipher(cipherId); const [organization, allowPersonalOwnership, allOrganizations, allCollections] = await firstValueFrom( combineLatest([ this.organization$, this.allowPersonalOwnership$, this.allOrganizations$, - this.editableCollections$, + this.allCollections$, ]), ); - const cipher = await this.getCipher(organization, cipherId); - - const collections = allCollections.filter( - (c) => c.organizationId === organization.id && c.assigned && !c.readOnly, - ); // When cloning from within the Admin Console, all organizations should be available. // Otherwise only the one in context should be const organizations = mode === "clone" ? allOrganizations : [organization]; @@ -100,7 +81,7 @@ export class AdminConsoleCipherFormConfigService implements CipherFormConfigServ admin: organization.canEditAllCiphers ?? false, allowPersonalOwnership: allowPersonalOwnershipOnlyForClone, originalCipher: cipher, - collections, + collections: allCollections, organizations, folders: [], // folders not applicable in the admin console hideIndividualVaultFields: true, @@ -108,19 +89,11 @@ export class AdminConsoleCipherFormConfigService implements CipherFormConfigServ }; } - private async getCipher(organization: Organization, id?: CipherId): Promise { + private async getCipher(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; diff --git a/libs/common/src/services/api.service.ts b/libs/common/src/services/api.service.ts index 2d4a0522636..0c508bfeb88 100644 --- a/libs/common/src/services/api.service.ts +++ b/libs/common/src/services/api.service.ts @@ -584,7 +584,7 @@ export class ApiService implements ApiServiceAbstraction { } putCipherCollectionsAdmin(id: string, request: CipherCollectionsRequest): Promise { - return this.send("PUT", "/ciphers/" + id + "/collections-admin", request, true, false); + return this.send("PUT", "/ciphers/" + id + "/collections-admin", request, true, true); } postPurgeCiphers( @@ -1886,7 +1886,7 @@ export class ApiService implements ApiServiceAbstraction { }); if (flagEnabled("prereleaseBuild")) { - headers.set("Is-Prerelease", "true"); + headers.set("Is-Prerelease", "1"); } if (this.customUserAgent != null) { headers.set("User-Agent", this.customUserAgent); diff --git a/libs/common/src/vault/abstractions/cipher.service.ts b/libs/common/src/vault/abstractions/cipher.service.ts index 444c922fe31..5221f4cf0a6 100644 --- a/libs/common/src/vault/abstractions/cipher.service.ts +++ b/libs/common/src/vault/abstractions/cipher.service.ts @@ -119,7 +119,7 @@ export abstract class CipherService implements UserKeyRotationDataProvider Promise; + saveCollectionsWithServerAdmin: (cipher: Cipher) => Promise; /** * Bulk update collections for many ciphers with the server * @param orgId diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 154042601e9..6b618e25502 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -880,9 +880,11 @@ export class CipherService implements CipherServiceAbstraction { return new Cipher(updated[cipher.id as CipherId], cipher.localData); } - async saveCollectionsWithServerAdmin(cipher: Cipher): Promise { + async saveCollectionsWithServerAdmin(cipher: Cipher): Promise { const request = new CipherCollectionsRequest(cipher.collectionIds); - await this.apiService.putCipherCollectionsAdmin(cipher.id, request); + const response = await this.apiService.putCipherCollectionsAdmin(cipher.id, request); + const data = new CipherData(response); + return new Cipher(data); } /** diff --git a/libs/tools/generator/core/src/policies/constraints.ts b/libs/tools/generator/core/src/policies/constraints.ts index 6071b57048f..d320329938f 100644 --- a/libs/tools/generator/core/src/policies/constraints.ts +++ b/libs/tools/generator/core/src/policies/constraints.ts @@ -2,6 +2,7 @@ import { Constraint } from "@bitwarden/common/tools/types"; import { sum } from "../util"; +const Zero: Constraint = { min: 0, max: 0 }; const AtLeastOne: Constraint = { min: 1 }; const RequiresTrue: Constraint = { requiredValue: true }; @@ -159,6 +160,7 @@ export { enforceConstant, readonlyTrueWhen, fitLength, + Zero, AtLeastOne, RequiresTrue, }; diff --git a/libs/tools/generator/core/src/policies/dynamic-password-policy-constraints.spec.ts b/libs/tools/generator/core/src/policies/dynamic-password-policy-constraints.spec.ts index 96f590f8ed6..d05d75ffb76 100644 --- a/libs/tools/generator/core/src/policies/dynamic-password-policy-constraints.spec.ts +++ b/libs/tools/generator/core/src/policies/dynamic-password-policy-constraints.spec.ts @@ -1,6 +1,6 @@ import { DefaultPasswordBoundaries, DefaultPasswordGenerationOptions, Policies } from "../data"; -import { AtLeastOne } from "./constraints"; +import { AtLeastOne, Zero } from "./constraints"; import { DynamicPasswordPolicyConstraints } from "./dynamic-password-policy-constraints"; describe("DynamicPasswordPolicyConstraints", () => { @@ -207,7 +207,7 @@ describe("DynamicPasswordPolicyConstraints", () => { expect(calibrated.constraints.minNumber).toEqual(dynamic.constraints.minNumber); }); - it("disables the minNumber constraint when the state's number flag is false", () => { + it("outputs the zero constraint when the state's number flag is false", () => { const dynamic = new DynamicPasswordPolicyConstraints(Policies.Password.disabledValue); const state = { ...DefaultPasswordGenerationOptions, @@ -216,7 +216,7 @@ describe("DynamicPasswordPolicyConstraints", () => { const calibrated = dynamic.calibrate(state); - expect(calibrated.constraints.minNumber).toBeUndefined(); + expect(calibrated.constraints.minNumber).toEqual(Zero); }); it("outputs the minSpecial constraint when the state's special flag is true", () => { @@ -231,7 +231,7 @@ describe("DynamicPasswordPolicyConstraints", () => { expect(calibrated.constraints.minSpecial).toEqual(dynamic.constraints.minSpecial); }); - it("disables the minSpecial constraint when the state's special flag is false", () => { + it("outputs the zero constraint when the state's special flag is false", () => { const dynamic = new DynamicPasswordPolicyConstraints(Policies.Password.disabledValue); const state = { ...DefaultPasswordGenerationOptions, @@ -240,23 +240,7 @@ describe("DynamicPasswordPolicyConstraints", () => { const calibrated = dynamic.calibrate(state); - expect(calibrated.constraints.minSpecial).toBeUndefined(); - }); - - it("copies the minimum length constraint", () => { - const dynamic = new DynamicPasswordPolicyConstraints(Policies.Password.disabledValue); - - const calibrated = dynamic.calibrate(DefaultPasswordGenerationOptions); - - expect(calibrated.constraints.minSpecial).toBeUndefined(); - }); - - it("overrides the minimum length constraint when it is less than the sum of the state's minimums", () => { - const dynamic = new DynamicPasswordPolicyConstraints(Policies.Password.disabledValue); - - const calibrated = dynamic.calibrate(DefaultPasswordGenerationOptions); - - expect(calibrated.constraints.minSpecial).toBeUndefined(); + expect(calibrated.constraints.minSpecial).toEqual(Zero); }); }); }); diff --git a/libs/tools/generator/core/src/policies/dynamic-password-policy-constraints.ts b/libs/tools/generator/core/src/policies/dynamic-password-policy-constraints.ts index daff9882547..7fe76061885 100644 --- a/libs/tools/generator/core/src/policies/dynamic-password-policy-constraints.ts +++ b/libs/tools/generator/core/src/policies/dynamic-password-policy-constraints.ts @@ -7,7 +7,7 @@ import { import { DefaultPasswordBoundaries } from "../data"; import { PasswordGeneratorPolicy, PasswordGeneratorSettings } from "../types"; -import { atLeast, atLeastSum, maybe, readonlyTrueWhen, AtLeastOne } from "./constraints"; +import { atLeast, atLeastSum, maybe, readonlyTrueWhen, AtLeastOne, Zero } from "./constraints"; import { PasswordPolicyConstraints } from "./password-policy-constraints"; /** Creates state constraints by blending policy and password settings. */ @@ -68,8 +68,8 @@ export class DynamicPasswordPolicyConstraints ...this.constraints, minLowercase: maybe(lowercase, this.constraints.minLowercase ?? AtLeastOne), minUppercase: maybe(uppercase, this.constraints.minUppercase ?? AtLeastOne), - minNumber: maybe(number, this.constraints.minNumber), - minSpecial: maybe(special, this.constraints.minSpecial), + minNumber: maybe(number, this.constraints.minNumber) ?? Zero, + minSpecial: maybe(special, this.constraints.minSpecial) ?? Zero, }; // lower bound of length must always at least fit its sub-lengths diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts index b62557a4329..93229bda6c3 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts @@ -87,7 +87,12 @@ describe("ItemDetailsSectionComponent", () => { component.config.allowPersonalOwnership = true; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ - { id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView, + { + id: "col1", + name: "Collection 1", + organizationId: "org1", + canEditItems: (_org) => true, + } as CollectionView, ]; component.originalCipherView = { name: "cipher1", @@ -116,8 +121,18 @@ describe("ItemDetailsSectionComponent", () => { component.config.allowPersonalOwnership = true; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ - { id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView, - { id: "col2", name: "Collection 2", organizationId: "org1" } as CollectionView, + { + id: "col1", + name: "Collection 1", + organizationId: "org1", + canEditItems: (_org) => false, + } as CollectionView, + { + id: "col2", + name: "Collection 2", + organizationId: "org1", + canEditItems: (_org) => true, + } as CollectionView, ]; component.originalCipherView = { name: "cipher1", @@ -367,9 +382,24 @@ describe("ItemDetailsSectionComponent", () => { } as CipherView; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ - { id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView, - { id: "col2", name: "Collection 2", organizationId: "org1" } as CollectionView, - { id: "col3", name: "Collection 3", organizationId: "org1" } as CollectionView, + { + id: "col1", + name: "Collection 1", + organizationId: "org1", + canEditItems: (_org) => true, + } as CollectionView, + { + id: "col2", + name: "Collection 2", + organizationId: "org1", + canEditItems: (_org) => true, + } as CollectionView, + { + id: "col3", + name: "Collection 3", + organizationId: "org1", + canEditItems: (_org) => true, + } as CollectionView, ]; fixture.detectChanges(); @@ -387,7 +417,12 @@ describe("ItemDetailsSectionComponent", () => { component.config.allowPersonalOwnership = true; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ - { id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView, + { + id: "col1", + name: "Collection 1", + organizationId: "org1", + canEditItems: (_org) => true, + } as CollectionView, ]; fixture.detectChanges(); @@ -414,13 +449,24 @@ describe("ItemDetailsSectionComponent", () => { } as CipherView; component.config.organizations = [{ id: "org1" } as Organization]; component.config.collections = [ - { id: "col1", name: "Collection 1", organizationId: "org1" } as CollectionView, - { id: "col2", name: "Collection 2", organizationId: "org1" } as CollectionView, + { + id: "col1", + name: "Collection 1", + organizationId: "org1", + canEditItems: (_org) => true, + } as CollectionView, + { + id: "col2", + name: "Collection 2", + organizationId: "org1", + canEditItems: (_org) => true, + } as CollectionView, { id: "col3", name: "Collection 3", organizationId: "org1", readOnly: true, + canEditItems: (_org) => true, } as CollectionView, ]; @@ -433,5 +479,94 @@ describe("ItemDetailsSectionComponent", () => { expect(collectionHint).not.toBeNull(); }); + + it("should allow all collections to be altered when `config.admin` is true", async () => { + component.config.admin = true; + component.config.allowPersonalOwnership = true; + component.config.organizations = [{ id: "org1" } as Organization]; + component.config.collections = [ + { + id: "col1", + name: "Collection 1", + organizationId: "org1", + readOnly: true, + canEditItems: (_org) => false, + } as CollectionView, + { + id: "col2", + name: "Collection 2", + organizationId: "org1", + readOnly: true, + canEditItems: (_org) => false, + } as CollectionView, + { + id: "col3", + name: "Collection 3", + organizationId: "org1", + readOnly: false, + canEditItems: (_org) => false, + } as CollectionView, + ]; + + fixture.detectChanges(); + await fixture.whenStable(); + + component.itemDetailsForm.controls.organizationId.setValue("org1"); + + expect(component["collectionOptions"].map((c) => c.id)).toEqual(["col1", "col2", "col3"]); + }); + }); + + describe("readonlyCollections", () => { + beforeEach(() => { + component.config.mode = "edit"; + component.config.admin = true; + component.config.collections = [ + { + id: "col1", + name: "Collection 1", + organizationId: "org1", + readOnly: true, + canEditItems: (_org) => false, + } as CollectionView, + { + id: "col2", + name: "Collection 2", + organizationId: "org1", + canEditItems: (_org) => false, + } as CollectionView, + { + id: "col3", + name: "Collection 3", + organizationId: "org1", + readOnly: true, + canEditItems: (_org) => false, + } as CollectionView, + ]; + component.originalCipherView = { + name: "cipher1", + organizationId: "org1", + folderId: "folder1", + collectionIds: ["col1", "col2", "col3"], + favorite: true, + } as CipherView; + component.config.organizations = [{ id: "org1" } as Organization]; + }); + + it("should not show collections as readonly when `config.admin` is true", async () => { + await component.ngOnInit(); + fixture.detectChanges(); + + // Filters out all collections + expect(component["readOnlyCollections"]).toEqual([]); + + // Non-admin, keep readonly collections + component.config.admin = false; + + await component.ngOnInit(); + fixture.detectChanges(); + + expect(component["readOnlyCollections"]).toEqual(["Collection 1", "Collection 3"]); + }); }); }); diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts index 86a8818bbe3..ea82aa0cae4 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts @@ -240,7 +240,11 @@ export class ItemDetailsSectionComponent implements OnInit { } else if (this.config.mode === "edit") { this.readOnlyCollections = this.collections .filter( - (c) => c.readOnly && this.originalCipherView.collectionIds.includes(c.id as CollectionId), + // When the configuration is set up for admins, they can alter read only collections + (c) => + c.readOnly && + !this.config.admin && + this.originalCipherView.collectionIds.includes(c.id as CollectionId), ) .map((c) => c.name); } @@ -262,12 +266,24 @@ export class ItemDetailsSectionComponent implements OnInit { collectionsControl.disable(); this.showCollectionsControl = false; return; + } else { + collectionsControl.enable(); + this.showCollectionsControl = true; } + const organization = this.organizations.find((o) => o.id === orgId); + this.collectionOptions = this.collections .filter((c) => { - // If partial edit mode, show all org collections because the control is disabled. - return c.organizationId === orgId && (this.partialEdit || !c.readOnly); + // Filter criteria: + // - The collection belongs to the organization + // - When in partial edit mode, show all org collections because the control is disabled. + // - The user can edit items within the collection + // - When viewing as an admin, all collections should be shown, even readonly. When non-admin, filter out readonly collections + return ( + c.organizationId === orgId && + (this.partialEdit || c.canEditItems(organization) || this.config.admin) + ); }) .map((c) => ({ id: c.id, diff --git a/libs/vault/src/cipher-form/services/default-cipher-form.service.ts b/libs/vault/src/cipher-form/services/default-cipher-form.service.ts index 8e73d9edd40..1b7e86f82a7 100644 --- a/libs/vault/src/cipher-form/services/default-cipher-form.service.ts +++ b/libs/vault/src/cipher-form/services/default-cipher-form.service.ts @@ -1,6 +1,7 @@ import { inject, Injectable } from "@angular/core"; import { firstValueFrom, map } from "rxjs"; +import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { Cipher } from "@bitwarden/common/vault/models/domain/cipher"; @@ -17,6 +18,7 @@ function isSetEqual(a: Set, b: Set) { export class DefaultCipherFormService implements CipherFormService { private cipherService: CipherService = inject(CipherService); private accountService: AccountService = inject(AccountService); + private apiService: ApiService = inject(ApiService); async decryptCipher(cipher: Cipher): Promise { const activeUserId = await firstValueFrom( @@ -66,11 +68,21 @@ export class DefaultCipherFormService implements CipherFormService { // Updating a cipher with collection changes is not supported with a single request currently // First update the cipher with the original collectionIds encryptedCipher.collectionIds = config.originalCipher.collectionIds; - await this.cipherService.updateWithServer(encryptedCipher, config.admin); + await this.cipherService.updateWithServer( + encryptedCipher, + config.admin || originalCollectionIds.size === 0, + config.mode !== "clone", + ); // Then save the new collection changes separately encryptedCipher.collectionIds = cipher.collectionIds; - savedCipher = await this.cipherService.saveCollectionsWithServer(encryptedCipher); + + if (config.admin || originalCollectionIds.size === 0) { + // When using an admin config or the cipher was unassigned, update collections as an admin + savedCipher = await this.cipherService.saveCollectionsWithServerAdmin(encryptedCipher); + } else { + savedCipher = await this.cipherService.saveCollectionsWithServer(encryptedCipher); + } } // Its possible the cipher was made no longer available due to collection assignment changes diff --git a/libs/vault/src/cipher-view/cipher-view.component.ts b/libs/vault/src/cipher-view/cipher-view.component.ts index 5d61caf52f3..0871fd8e788 100644 --- a/libs/vault/src/cipher-view/cipher-view.component.ts +++ b/libs/vault/src/cipher-view/cipher-view.component.ts @@ -98,6 +98,7 @@ export class CipherViewComponent implements OnChanges, OnDestroy { async loadCipherData() { // Load collections if not provided and the cipher has collectionIds if ( + this.cipher.collectionIds && this.cipher.collectionIds.length > 0 && (!this.collections || this.collections.length === 0) ) {