[AC-1041] My Vault filter on first login fix (#10301)

* [AC-1041] Ensure organizationTree$ updates whenever the policy observables emit

* [AC-1041] Ensure enforcePersonalOwnership updates whenever the policy observable emits

* [AC-1041] Do not attempt to pre-select null filter values or read-only collections
This commit is contained in:
Shane Melton 2024-08-13 10:32:02 -07:00 committed by GitHub
parent fe9d44af6d
commit 1b22320dc5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 64 additions and 28 deletions

View File

@ -5,17 +5,28 @@ import {
Component, Component,
ElementRef, ElementRef,
EventEmitter, EventEmitter,
HostListener,
OnDestroy,
OnInit, OnInit,
Output, Output,
TemplateRef, TemplateRef,
ViewChild, ViewChild,
ViewContainerRef, ViewContainerRef,
HostListener,
OnDestroy,
} from "@angular/core"; } from "@angular/core";
import { BehaviorSubject, concatMap, map, merge, Observable, Subject, takeUntil } from "rxjs"; import {
BehaviorSubject,
combineLatest,
concatMap,
map,
merge,
Observable,
Subject,
takeUntil,
} from "rxjs";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.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 { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
@ -88,6 +99,7 @@ export class VaultSelectComponent implements OnInit, OnDestroy {
private viewContainerRef: ViewContainerRef, private viewContainerRef: ViewContainerRef,
private platformUtilsService: PlatformUtilsService, private platformUtilsService: PlatformUtilsService,
private organizationService: OrganizationService, private organizationService: OrganizationService,
private policyService: PolicyService,
) {} ) {}
@HostListener("document:keydown.escape", ["$event"]) @HostListener("document:keydown.escape", ["$event"])
@ -103,11 +115,13 @@ export class VaultSelectComponent implements OnInit, OnDestroy {
.pipe(takeUntil(this._destroy)) .pipe(takeUntil(this._destroy))
.pipe(map((orgs) => orgs.sort(Utils.getSortFunction(this.i18nService, "name")))); .pipe(map((orgs) => orgs.sort(Utils.getSortFunction(this.i18nService, "name"))));
this.organizations$ combineLatest([
this.organizations$,
this.policyService.policyAppliesToActiveUser$(PolicyType.PersonalOwnership),
])
.pipe( .pipe(
concatMap(async (organizations) => { concatMap(async ([organizations, enforcePersonalOwnership]) => {
this.enforcePersonalOwnership = this.enforcePersonalOwnership = enforcePersonalOwnership;
await this.vaultFilterService.checkForPersonalOwnershipPolicy();
if (this.shouldShow(organizations)) { if (this.shouldShow(organizations)) {
if (this.enforcePersonalOwnership && !this.vaultFilterService.vaultFilter.myVaultOnly) { if (this.enforcePersonalOwnership && !this.vaultFilterService.vaultFilter.myVaultOnly) {

View File

@ -36,6 +36,8 @@ describe("vault filter service", () => {
let organizations: ReplaySubject<Organization[]>; let organizations: ReplaySubject<Organization[]>;
let folderViews: ReplaySubject<FolderView[]>; let folderViews: ReplaySubject<FolderView[]>;
let collectionViews: ReplaySubject<CollectionView[]>; let collectionViews: ReplaySubject<CollectionView[]>;
let personalOwnershipPolicy: ReplaySubject<boolean>;
let singleOrgPolicy: ReplaySubject<boolean>;
let stateProvider: FakeStateProvider; let stateProvider: FakeStateProvider;
const mockUserId = Utils.newGuid() as UserId; const mockUserId = Utils.newGuid() as UserId;
@ -56,10 +58,18 @@ describe("vault filter service", () => {
organizations = new ReplaySubject<Organization[]>(1); organizations = new ReplaySubject<Organization[]>(1);
folderViews = new ReplaySubject<FolderView[]>(1); folderViews = new ReplaySubject<FolderView[]>(1);
collectionViews = new ReplaySubject<CollectionView[]>(1); collectionViews = new ReplaySubject<CollectionView[]>(1);
personalOwnershipPolicy = new ReplaySubject<boolean>(1);
singleOrgPolicy = new ReplaySubject<boolean>(1);
organizationService.memberOrganizations$ = organizations; organizationService.memberOrganizations$ = organizations;
folderService.folderViews$ = folderViews; folderService.folderViews$ = folderViews;
collectionService.decryptedCollections$ = collectionViews; collectionService.decryptedCollections$ = collectionViews;
policyService.policyAppliesToActiveUser$
.calledWith(PolicyType.PersonalOwnership)
.mockReturnValue(personalOwnershipPolicy);
policyService.policyAppliesToActiveUser$
.calledWith(PolicyType.SingleOrg)
.mockReturnValue(singleOrgPolicy);
vaultFilterService = new VaultFilterService( vaultFilterService = new VaultFilterService(
organizationService, organizationService,
@ -100,6 +110,8 @@ describe("vault filter service", () => {
beforeEach(() => { beforeEach(() => {
const storedOrgs = [createOrganization("1", "org1"), createOrganization("2", "org2")]; const storedOrgs = [createOrganization("1", "org1"), createOrganization("2", "org2")];
organizations.next(storedOrgs); organizations.next(storedOrgs);
personalOwnershipPolicy.next(false);
singleOrgPolicy.next(false);
}); });
it("returns a nested tree", async () => { it("returns a nested tree", async () => {
@ -111,9 +123,7 @@ describe("vault filter service", () => {
}); });
it("hides My Vault if personal ownership policy is enabled", async () => { it("hides My Vault if personal ownership policy is enabled", async () => {
policyService.policyAppliesToUser personalOwnershipPolicy.next(true);
.calledWith(PolicyType.PersonalOwnership)
.mockResolvedValue(true);
const tree = await firstValueFrom(vaultFilterService.organizationTree$); const tree = await firstValueFrom(vaultFilterService.organizationTree$);
@ -122,7 +132,7 @@ describe("vault filter service", () => {
}); });
it("returns 1 organization and My Vault if single organization policy is enabled", async () => { it("returns 1 organization and My Vault if single organization policy is enabled", async () => {
policyService.policyAppliesToUser.calledWith(PolicyType.SingleOrg).mockResolvedValue(true); singleOrgPolicy.next(true);
const tree = await firstValueFrom(vaultFilterService.organizationTree$); const tree = await firstValueFrom(vaultFilterService.organizationTree$);
@ -132,10 +142,8 @@ describe("vault filter service", () => {
}); });
it("returns 1 organization if both single organization and personal ownership policies are enabled", async () => { it("returns 1 organization if both single organization and personal ownership policies are enabled", async () => {
policyService.policyAppliesToUser.calledWith(PolicyType.SingleOrg).mockResolvedValue(true); singleOrgPolicy.next(true);
policyService.policyAppliesToUser personalOwnershipPolicy.next(true);
.calledWith(PolicyType.PersonalOwnership)
.mockResolvedValue(true);
const tree = await firstValueFrom(vaultFilterService.organizationTree$); const tree = await firstValueFrom(vaultFilterService.organizationTree$);

View File

@ -1,6 +1,7 @@
import { Injectable } from "@angular/core"; import { Injectable } from "@angular/core";
import { import {
BehaviorSubject, BehaviorSubject,
combineLatest,
combineLatestWith, combineLatestWith,
firstValueFrom, firstValueFrom,
map, map,
@ -39,10 +40,15 @@ const NestingDelimiter = "/";
@Injectable() @Injectable()
export class VaultFilterService implements VaultFilterServiceAbstraction { export class VaultFilterService implements VaultFilterServiceAbstraction {
organizationTree$: Observable<TreeNode<OrganizationFilter>> = organizationTree$: Observable<TreeNode<OrganizationFilter>> = combineLatest([
this.organizationService.memberOrganizations$.pipe( this.organizationService.memberOrganizations$,
switchMap((orgs) => this.buildOrganizationTree(orgs)), this.policyService.policyAppliesToActiveUser$(PolicyType.SingleOrg),
); this.policyService.policyAppliesToActiveUser$(PolicyType.PersonalOwnership),
]).pipe(
switchMap(([orgs, singleOrgPolicy, personalOwnershipPolicy]) =>
this.buildOrganizationTree(orgs, singleOrgPolicy, personalOwnershipPolicy),
),
);
protected _organizationFilter = new BehaviorSubject<Organization>(null); protected _organizationFilter = new BehaviorSubject<Organization>(null);
@ -125,14 +131,16 @@ export class VaultFilterService implements VaultFilterServiceAbstraction {
} }
protected async buildOrganizationTree( protected async buildOrganizationTree(
orgs?: Organization[], orgs: Organization[],
singleOrgPolicy: boolean,
personalOwnershipPolicy: boolean,
): Promise<TreeNode<OrganizationFilter>> { ): Promise<TreeNode<OrganizationFilter>> {
const headNode = this.getOrganizationFilterHead(); const headNode = this.getOrganizationFilterHead();
if (!(await this.policyService.policyAppliesToUser(PolicyType.PersonalOwnership))) { if (!personalOwnershipPolicy) {
const myVaultNode = this.getOrganizationFilterMyVault(); const myVaultNode = this.getOrganizationFilterMyVault();
headNode.children.push(myVaultNode); headNode.children.push(myVaultNode);
} }
if (await this.policyService.policyAppliesToUser(PolicyType.SingleOrg)) { if (singleOrgPolicy) {
orgs = orgs.slice(0, 1); orgs = orgs.slice(0, 1);
} }
if (orgs) { if (orgs) {

View File

@ -586,18 +586,24 @@ export class VaultComponent implements OnInit, OnDestroy {
async addCipher(cipherType?: CipherType) { async addCipher(cipherType?: CipherType) {
const component = await this.editCipher(null); const component = await this.editCipher(null);
component.type = cipherType || this.activeFilter.cipherType; component.type = cipherType || this.activeFilter.cipherType;
if (this.activeFilter.organizationId !== "MyVault") { if (
this.activeFilter.organizationId !== "MyVault" &&
this.activeFilter.organizationId != null
) {
component.organizationId = this.activeFilter.organizationId; component.organizationId = this.activeFilter.organizationId;
component.collections = ( component.collections = (
await firstValueFrom(this.vaultFilterService.filteredCollections$) await firstValueFrom(this.vaultFilterService.filteredCollections$)
).filter((c) => !c.readOnly && c.id != null); ).filter((c) => !c.readOnly && c.id != null);
} }
const selectedColId = this.activeFilter.collectionId; const selectedColId = this.activeFilter.collectionId;
if (selectedColId !== "AllCollections") { if (selectedColId !== "AllCollections" && selectedColId != null) {
component.organizationId = component.collections.find( const selectedCollection = (
(collection) => collection.id === selectedColId, await firstValueFrom(this.vaultFilterService.filteredCollections$)
)?.organizationId; ).find((c) => c.id === selectedColId);
component.collectionIds = [selectedColId]; component.organizationId = selectedCollection?.organizationId;
if (!selectedCollection.readOnly) {
component.collectionIds = [selectedColId];
}
} }
component.folderId = this.activeFilter.folderId; component.folderId = this.activeFilter.folderId;
} }