diff --git a/apps/web/src/app/admin-console/organizations/collections/vault.component.ts b/apps/web/src/app/admin-console/organizations/collections/vault.component.ts index 65cb6739887..073d73f6a50 100644 --- a/apps/web/src/app/admin-console/organizations/collections/vault.component.ts +++ b/apps/web/src/app/admin-console/organizations/collections/vault.component.ts @@ -920,14 +920,9 @@ export class VaultComponent implements OnInit, OnDestroy { cipher?: CipherView, activeCollectionId?: CollectionId, ) { - const organization = await firstValueFrom(this.organization$); - const disableForm = cipher ? !cipher.edit && !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, + mode, formConfig, - disableForm, activeCollectionId, isAdminConsoleAction: true, restore: this.restore, diff --git a/apps/web/src/app/dirt/reports/pages/cipher-report.component.ts b/apps/web/src/app/dirt/reports/pages/cipher-report.component.ts index bd061bf34d3..c1955b0678b 100644 --- a/apps/web/src/app/dirt/reports/pages/cipher-report.component.ts +++ b/apps/web/src/app/dirt/reports/pages/cipher-report.component.ts @@ -186,13 +186,10 @@ export abstract class CipherReportComponent implements OnDestroy { cipher: CipherView, activeCollectionId?: CollectionId, ) { - const disableForm = cipher ? !cipher.edit && !this.organization?.canEditAllCiphers : false; - this.vaultItemDialogRef = VaultItemDialogComponent.open(this.dialogService, { mode, formConfig, activeCollectionId, - disableForm, isAdminConsoleAction: this.organization != null, }); diff --git a/apps/web/src/app/dirt/reports/pages/organizations/exposed-passwords-report.component.ts b/apps/web/src/app/dirt/reports/pages/organizations/exposed-passwords-report.component.ts index 6c81cbd9986..603c01bd2ab 100644 --- a/apps/web/src/app/dirt/reports/pages/organizations/exposed-passwords-report.component.ts +++ b/apps/web/src/app/dirt/reports/pages/organizations/exposed-passwords-report.component.ts @@ -1,17 +1,13 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { Component, OnInit } from "@angular/core"; import { ActivatedRoute } from "@angular/router"; -import { firstValueFrom } from "rxjs"; +import { firstValueFrom, takeUntil, tap } from "rxjs"; import { AuditService } from "@bitwarden/common/abstractions/audit.service"; -import { - getOrganizationById, - OrganizationService, -} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { getById } from "@bitwarden/common/platform/misc"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { Cipher } from "@bitwarden/common/vault/models/domain/cipher"; @@ -51,7 +47,7 @@ export class ExposedPasswordsReportComponent extends BaseExposedPasswordsReportComponent implements OnInit { - manageableCiphers: Cipher[]; + private manageableCiphers: Cipher[] = []; constructor( cipherService: CipherService, @@ -82,20 +78,25 @@ export class ExposedPasswordsReportComponent async ngOnInit() { this.isAdminConsoleActive = true; - // eslint-disable-next-line rxjs-angular/prefer-takeuntil, rxjs/no-async-subscribe - this.route.parent.parent.params.subscribe(async (params) => { - const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); - this.organization = await firstValueFrom( - this.organizationService - .organizations$(userId) - .pipe(getOrganizationById(params.organizationId)), - ); - this.manageableCiphers = await this.cipherService.getAll(userId); - }); + this.route.parent?.parent?.params + .pipe( + tap(async (params) => { + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + this.organization = await firstValueFrom( + this.organizationService.organizations$(userId).pipe(getById(params.organizationId)), + ); + this.manageableCiphers = await this.cipherService.getAll(userId); + }), + takeUntil(this.destroyed$), + ) + .subscribe(); } - getAllCiphers(): Promise { - return this.cipherService.getAllFromApiForOrganization(this.organization.id, true); + async getAllCiphers(): Promise { + if (this.organization) { + return this.cipherService.getAllFromApiForOrganization(this.organization.id, true); + } + return []; } canManageCipher(c: CipherView): boolean { diff --git a/apps/web/src/app/dirt/reports/pages/organizations/inactive-two-factor-report.component.ts b/apps/web/src/app/dirt/reports/pages/organizations/inactive-two-factor-report.component.ts index 6b93b289df9..4104e16b3b5 100644 --- a/apps/web/src/app/dirt/reports/pages/organizations/inactive-two-factor-report.component.ts +++ b/apps/web/src/app/dirt/reports/pages/organizations/inactive-two-factor-report.component.ts @@ -1,9 +1,10 @@ import { ChangeDetectorRef, Component, OnInit, ChangeDetectionStrategy } from "@angular/core"; import { ActivatedRoute } from "@angular/router"; -import { firstValueFrom, map, takeUntil } from "rxjs"; +import { firstValueFrom, takeUntil, tap } from "rxjs"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { getById } from "@bitwarden/common/platform/misc"; @@ -81,27 +82,24 @@ export class InactiveTwoFactorReportComponent this.isAdminConsoleActive = true; this.route.parent?.parent?.params - ?.pipe(takeUntil(this.destroyed$)) - // eslint-disable-next-line rxjs/no-async-subscribe - .subscribe(async (params) => { - const userId = await firstValueFrom( - this.accountService.activeAccount$.pipe(map((a) => a?.id)), - ); - - if (userId) { + .pipe( + tap(async (params) => { + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); this.organization = await firstValueFrom( this.organizationService.organizations$(userId).pipe(getById(params.organizationId)), ); this.manageableCiphers = await this.cipherService.getAll(userId); await super.ngOnInit(); - } - this.changeDetectorRef.markForCheck(); - }); + this.changeDetectorRef.markForCheck(); + }), + takeUntil(this.destroyed$), + ) + .subscribe(); } async getAllCiphers(): Promise { if (this.organization) { - return await this.cipherService.getAllFromApiForOrganization(this.organization.id, true); + return this.cipherService.getAllFromApiForOrganization(this.organization.id, true); } return []; } diff --git a/apps/web/src/app/dirt/reports/pages/organizations/reused-passwords-report.component.ts b/apps/web/src/app/dirt/reports/pages/organizations/reused-passwords-report.component.ts index 0ae9ecad0cb..683b195b271 100644 --- a/apps/web/src/app/dirt/reports/pages/organizations/reused-passwords-report.component.ts +++ b/apps/web/src/app/dirt/reports/pages/organizations/reused-passwords-report.component.ts @@ -1,16 +1,12 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { Component, OnInit } from "@angular/core"; import { ActivatedRoute } from "@angular/router"; -import { firstValueFrom } from "rxjs"; +import { firstValueFrom, takeUntil, tap } from "rxjs"; -import { - getOrganizationById, - OrganizationService, -} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { getById } from "@bitwarden/common/platform/misc"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { Cipher } from "@bitwarden/common/vault/models/domain/cipher"; @@ -50,7 +46,7 @@ export class ReusedPasswordsReportComponent extends BaseReusedPasswordsReportComponent implements OnInit { - manageableCiphers: Cipher[]; + manageableCiphers: Cipher[] = []; constructor( cipherService: CipherService, @@ -79,21 +75,27 @@ export class ReusedPasswordsReportComponent async ngOnInit() { this.isAdminConsoleActive = true; - // eslint-disable-next-line rxjs-angular/prefer-takeuntil, rxjs/no-async-subscribe - this.route.parent.parent.params.subscribe(async (params) => { - const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); - this.organization = await firstValueFrom( - this.organizationService - .organizations$(userId) - .pipe(getOrganizationById(params.organizationId)), - ); - this.manageableCiphers = await this.cipherService.getAll(userId); - await super.ngOnInit(); - }); + + this.route.parent?.parent?.params + .pipe( + tap(async (params) => { + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + this.organization = await firstValueFrom( + this.organizationService.organizations$(userId).pipe(getById(params.organizationId)), + ); + this.manageableCiphers = await this.cipherService.getAll(userId); + await super.ngOnInit(); + }), + takeUntil(this.destroyed$), + ) + .subscribe(); } - getAllCiphers(): Promise { - return this.cipherService.getAllFromApiForOrganization(this.organization.id, true); + async getAllCiphers(): Promise { + if (this.organization) { + return this.cipherService.getAllFromApiForOrganization(this.organization.id, true); + } + return []; } canManageCipher(c: CipherView): boolean { diff --git a/apps/web/src/app/dirt/reports/pages/organizations/unsecured-websites-report.component.ts b/apps/web/src/app/dirt/reports/pages/organizations/unsecured-websites-report.component.ts index 0b7cd3bfe7c..893a5058bd2 100644 --- a/apps/web/src/app/dirt/reports/pages/organizations/unsecured-websites-report.component.ts +++ b/apps/web/src/app/dirt/reports/pages/organizations/unsecured-websites-report.component.ts @@ -1,16 +1,13 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { Component, OnInit } from "@angular/core"; import { ActivatedRoute } from "@angular/router"; -import { firstValueFrom, map } from "rxjs"; +import { firstValueFrom, takeUntil, tap } from "rxjs"; import { CollectionService } from "@bitwarden/admin-console/common"; -import { - getOrganizationById, - OrganizationService, -} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { getById } from "@bitwarden/common/platform/misc"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { Cipher } from "@bitwarden/common/vault/models/domain/cipher"; @@ -51,7 +48,7 @@ export class UnsecuredWebsitesReportComponent implements OnInit { // Contains a list of ciphers, the user running the report, can manage - private manageableCiphers: Cipher[]; + private manageableCiphers: Cipher[] = []; constructor( cipherService: CipherService, @@ -82,23 +79,26 @@ export class UnsecuredWebsitesReportComponent async ngOnInit() { this.isAdminConsoleActive = true; - // eslint-disable-next-line rxjs-angular/prefer-takeuntil, rxjs/no-async-subscribe - this.route.parent.parent.params.subscribe(async (params) => { - const userId = await firstValueFrom( - this.accountService.activeAccount$.pipe(map((a) => a?.id)), - ); - this.organization = await firstValueFrom( - this.organizationService - .organizations$(userId) - .pipe(getOrganizationById(params.organizationId)), - ); - this.manageableCiphers = await this.cipherService.getAll(userId); - await super.ngOnInit(); - }); + this.route.parent?.parent?.params + .pipe( + tap(async (params) => { + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + this.organization = await firstValueFrom( + this.organizationService.organizations$(userId).pipe(getById(params.organizationId)), + ); + this.manageableCiphers = await this.cipherService.getAll(userId); + await super.ngOnInit(); + }), + takeUntil(this.destroyed$), + ) + .subscribe(); } - getAllCiphers(): Promise { - return this.cipherService.getAllFromApiForOrganization(this.organization.id, true); + async getAllCiphers(): Promise { + if (this.organization) { + return this.cipherService.getAllFromApiForOrganization(this.organization.id, true); + } + return []; } protected canManageCipher(c: CipherView): boolean { diff --git a/apps/web/src/app/dirt/reports/pages/organizations/weak-passwords-report.component.ts b/apps/web/src/app/dirt/reports/pages/organizations/weak-passwords-report.component.ts index 411295ceb2a..aadd015e29d 100644 --- a/apps/web/src/app/dirt/reports/pages/organizations/weak-passwords-report.component.ts +++ b/apps/web/src/app/dirt/reports/pages/organizations/weak-passwords-report.component.ts @@ -1,16 +1,12 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { Component, OnInit } from "@angular/core"; import { ActivatedRoute } from "@angular/router"; -import { firstValueFrom } from "rxjs"; +import { firstValueFrom, takeUntil, tap } from "rxjs"; -import { - getOrganizationById, - OrganizationService, -} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { getById } from "@bitwarden/common/platform/misc"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; @@ -51,7 +47,7 @@ export class WeakPasswordsReportComponent extends BaseWeakPasswordsReportComponent implements OnInit { - manageableCiphers: Cipher[]; + private manageableCiphers: Cipher[] = []; constructor( cipherService: CipherService, @@ -82,22 +78,26 @@ export class WeakPasswordsReportComponent async ngOnInit() { this.isAdminConsoleActive = true; - // eslint-disable-next-line rxjs-angular/prefer-takeuntil, rxjs/no-async-subscribe - this.route.parent.parent.params.subscribe(async (params) => { - const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); - - this.organization = await firstValueFrom( - this.organizationService - .organizations$(userId) - .pipe(getOrganizationById(params.organizationId)), - ); - this.manageableCiphers = await this.cipherService.getAll(userId); - await super.ngOnInit(); - }); + this.route.parent?.parent?.params + .pipe( + tap(async (params) => { + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + this.organization = await firstValueFrom( + this.organizationService.organizations$(userId).pipe(getById(params.organizationId)), + ); + this.manageableCiphers = await this.cipherService.getAll(userId); + await super.ngOnInit(); + }), + takeUntil(this.destroyed$), + ) + .subscribe(); } - getAllCiphers(): Promise { - return this.cipherService.getAllFromApiForOrganization(this.organization.id, true); + async getAllCiphers(): Promise { + if (this.organization) { + return this.cipherService.getAllFromApiForOrganization(this.organization.id, true); + } + return []; } canManageCipher(c: CipherView): boolean { 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 df73aacfdde..0fe63ed43bd 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 @@ -87,11 +87,6 @@ export interface VaultItemDialogParams { */ formConfig: CipherFormConfig; - /** - * If true, the "edit" button will be disabled in the dialog. - */ - disableForm?: boolean; - /** * The ID of the active collection. This is know the collection filter selected by the user. */ @@ -273,7 +268,7 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy { } protected get disableEdit() { - return this.params.disableForm; + return !this.canEdit; } protected get showEdit() { @@ -314,6 +309,8 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy { protected canDelete = false; + protected canEdit = false; + protected attachmentsButtonDisabled = false; protected confirmedPremiumUpgrade = false; @@ -372,6 +369,20 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy { ), ); + this.canEdit = await firstValueFrom( + this.cipherAuthorizationService.canEditCipher$( + this.cipher, + this.params.isAdminConsoleAction, + ), + ); + + // If user cannot edit and dialog opened in form mode, force to view mode + if (!this.canEdit && this.params.mode === "form") { + this.params.mode = "view"; + this.loadForm = false; + this.updateTitle(); + } + await this.eventCollectionService.collect( EventType.Cipher_ClientViewed, this.cipher.id, diff --git a/libs/common/src/vault/services/cipher-authorization.service.spec.ts b/libs/common/src/vault/services/cipher-authorization.service.spec.ts index f1cc8743492..0490fba3d90 100644 --- a/libs/common/src/vault/services/cipher-authorization.service.spec.ts +++ b/libs/common/src/vault/services/cipher-authorization.service.spec.ts @@ -205,6 +205,70 @@ describe("CipherAuthorizationService", () => { }); }); + describe("canEditCipher$", () => { + it("should return true if isAdminConsoleAction is true and cipher is unassigned", (done) => { + const cipher = createMockCipher("org1", []) as CipherView; + const organization = createMockOrganization({ canEditUnassignedCiphers: true }); + mockOrganizationService.organizations$.mockReturnValue( + of([organization]) as Observable, + ); + + cipherAuthorizationService.canEditCipher$(cipher, true).subscribe((result) => { + expect(result).toBe(true); + done(); + }); + }); + + it("should return true if isAdminConsoleAction is true and user can edit all ciphers in the org", (done) => { + const cipher = createMockCipher("org1", ["col1"]) as CipherView; + const organization = createMockOrganization({ canEditAllCiphers: true }); + mockOrganizationService.organizations$.mockReturnValue( + of([organization]) as Observable, + ); + + cipherAuthorizationService.canEditCipher$(cipher, true).subscribe((result) => { + expect(result).toBe(true); + expect(mockOrganizationService.organizations$).toHaveBeenCalledWith(mockUserId); + done(); + }); + }); + + it("should return false if isAdminConsoleAction is true but user does not have permission to edit unassigned ciphers", (done) => { + const cipher = createMockCipher("org1", []) as CipherView; + const organization = createMockOrganization({ canEditUnassignedCiphers: false }); + mockOrganizationService.organizations$.mockReturnValue(of([organization] as Organization[])); + + cipherAuthorizationService.canEditCipher$(cipher, true).subscribe((result) => { + expect(result).toBe(false); + done(); + }); + }); + + it("should return true if cipher.edit is true and is not an admin action", (done) => { + const cipher = createMockCipher("org1", [], true) as CipherView; + const organization = createMockOrganization(); + mockOrganizationService.organizations$.mockReturnValue(of([organization] as Organization[])); + + cipherAuthorizationService.canEditCipher$(cipher, false).subscribe((result) => { + expect(result).toBe(true); + expect(mockCollectionService.decryptedCollections$).not.toHaveBeenCalled(); + done(); + }); + }); + + it("should return false if cipher.edit is false and is not an admin action", (done) => { + const cipher = createMockCipher("org1", [], false) as CipherView; + const organization = createMockOrganization(); + mockOrganizationService.organizations$.mockReturnValue(of([organization] as Organization[])); + + cipherAuthorizationService.canEditCipher$(cipher, false).subscribe((result) => { + expect(result).toBe(false); + expect(mockCollectionService.decryptedCollections$).not.toHaveBeenCalled(); + done(); + }); + }); + }); + describe("canCloneCipher$", () => { it("should return true if cipher has no organizationId", async () => { const cipher = createMockCipher(null, []) as CipherView; diff --git a/libs/common/src/vault/services/cipher-authorization.service.ts b/libs/common/src/vault/services/cipher-authorization.service.ts index 7f7e2c3f531..eb89819a05e 100644 --- a/libs/common/src/vault/services/cipher-authorization.service.ts +++ b/libs/common/src/vault/services/cipher-authorization.service.ts @@ -53,6 +53,19 @@ export abstract class CipherAuthorizationService { cipher: CipherLike, isAdminConsoleAction?: boolean, ) => Observable; + + /** + * Determines if the user can edit the specified cipher. + * + * @param {CipherLike} cipher - The cipher object to evaluate for edit permissions. + * @param {boolean} isAdminConsoleAction - Optional. A flag indicating if the action is being performed from the admin console. + * + * @returns {Observable} - An observable that emits a boolean value indicating if the user can edit the cipher. + */ + abstract canEditCipher$: ( + cipher: CipherLike, + isAdminConsoleAction?: boolean, + ) => Observable; } /** @@ -118,6 +131,29 @@ export class DefaultCipherAuthorizationService implements CipherAuthorizationSer ); } + /** + * + * {@link CipherAuthorizationService.canEditCipher$} + */ + canEditCipher$(cipher: CipherLike, isAdminConsoleAction?: boolean): Observable { + return this.organization$(cipher).pipe( + map((organization) => { + if (isAdminConsoleAction) { + // If the user is an admin, they can edit an unassigned cipher + if (!cipher.collectionIds || cipher.collectionIds.length === 0) { + return organization?.canEditUnassignedCiphers === true; + } + + if (organization?.canEditAllCiphers) { + return true; + } + } + + return !!cipher.edit; + }), + ); + } + /** * {@link CipherAuthorizationService.canCloneCipher$} */