[EC-850] ProviderUser permissions should prevail over member permissions (#5162)

* Apply provider permissions even if also member

* Add org.isMember

* Refactor: extract syncProfileOrganizations method

* Change isNotProvider logic to isMember

* Fix cascading org permissions

* Add memberOrganizations$ observable

apps/browser/src/vault/popup/components/vault/vault-select.component.ts

@@ -17,11 +17,9 @@ import { BehaviorSubject, concatMap, map, merge, Observable, Subject, takeUntil
 
 import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
 import { PlatformUtilsService } from "@bitwarden/common/abstractions/platformUtils.service";
-import {
-  isNotProviderUser,
-  OrganizationService,
-} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
+import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
+import { Utils } from "@bitwarden/common/misc/utils";
 
 import { VaultFilterService } from "../../../services/vault-filter.service";
 
@@ -101,11 +99,9 @@ export class VaultSelectComponent implements OnInit, OnDestroy {
   }
 
   async ngOnInit() {
-    this.organizations$ = this.organizationService.organizations$
+    this.organizations$ = this.organizationService.memberOrganizations$
       .pipe(takeUntil(this._destroy))
-      .pipe(
-        map((orgs) => orgs.filter(isNotProviderUser).sort((a, b) => a.name.localeCompare(b.name)))
-      );
+      .pipe(map((orgs) => orgs.sort(Utils.getSortFunction(this.i18nService, "name"))));
 
     this.organizations$
       .pipe(

apps/web/src/app/admin-console/components/organization-switcher.component.ts

@@ -4,7 +4,6 @@ import { map, Observable } from "rxjs";
 import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
 import {
   canAccessAdmin,
-  isNotProviderUser,
   OrganizationService,
 } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
@@ -23,8 +22,7 @@ export class OrganizationSwitcherComponent implements OnInit {
   loaded = false;
 
   async ngOnInit() {
-    this.organizations$ = this.organizationService.organizations$.pipe(
-      map((orgs) => orgs.filter(isNotProviderUser)),
+    this.organizations$ = this.organizationService.memberOrganizations$.pipe(
       canAccessAdmin(this.i18nService),
       map((orgs) => orgs.sort(Utils.getSortFunction(this.i18nService, "name")))
     );

apps/web/src/app/layouts/navbar.component.ts

@@ -1,12 +1,11 @@
 import { Component, OnInit } from "@angular/core";
-import { map, Observable } from "rxjs";
+import { Observable } from "rxjs";
 
 import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
 import { MessagingService } from "@bitwarden/common/abstractions/messaging.service";
 import { PlatformUtilsService } from "@bitwarden/common/abstractions/platformUtils.service";
 import {
   canAccessAdmin,
-  isNotProviderUser,
   OrganizationService,
 } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { ProviderService } from "@bitwarden/common/admin-console/abstractions/provider.service";
@@ -54,8 +53,7 @@ export class NavbarComponent implements OnInit {
     }
     this.providers = await this.providerService.getAll();
 
-    this.organizations$ = this.organizationService.organizations$.pipe(
-      map((orgs) => orgs.filter(isNotProviderUser)),
+    this.organizations$ = this.organizationService.memberOrganizations$.pipe(
       canAccessAdmin(this.i18nService)
     );
   }

apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.spec.ts

@@ -251,6 +251,7 @@ describe("vault filter service", () => {
     org.id = id;
     org.name = name;
     org.identifier = name;
+    org.isMember = true;
     return org;
   }
 

apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.ts

@@ -14,7 +14,7 @@ import {
 import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
 import { StateService } from "@bitwarden/common/abstractions/state.service";
 import {
-  isNotProviderUser,
+  isMember,
   OrganizationService,
 } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
@@ -139,7 +139,7 @@ export class VaultFilterService implements VaultFilterServiceAbstraction {
     }
     if (orgs) {
       const orgNodes: TreeNode<OrganizationFilter>[] = [];
-      orgs.filter(isNotProviderUser).forEach((org) => {
+      orgs.filter(isMember).forEach((org) => {
         const orgCopy = org as OrganizationFilter;
         orgCopy.icon = "bwi-business";
         const node = new TreeNode<OrganizationFilter>(orgCopy, headNode, orgCopy.name);

libs/angular/src/components/share.component.ts

@@ -5,10 +5,7 @@ import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
 import { LogService } from "@bitwarden/common/abstractions/log.service";
 import { PlatformUtilsService } from "@bitwarden/common/abstractions/platformUtils.service";
 import { CollectionService } from "@bitwarden/common/admin-console/abstractions/collection.service";
-import {
-  isNotProviderUser,
-  OrganizationService,
-} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
+import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { OrganizationUserStatusType } from "@bitwarden/common/admin-console/enums";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
 import { CollectionView } from "@bitwarden/common/admin-console/models/view/collection.view";
@@ -54,13 +51,10 @@ export class ShareComponent implements OnInit, OnDestroy {
     const allCollections = await this.collectionService.getAllDecrypted();
     this.writeableCollections = allCollections.map((c) => c).filter((c) => !c.readOnly);
 
-    this.organizations$ = this.organizationService.organizations$.pipe(
+    this.organizations$ = this.organizationService.memberOrganizations$.pipe(
       map((orgs) => {
         return orgs
-          .filter(
-            (o) =>
-              o.enabled && o.status === OrganizationUserStatusType.Confirmed && isNotProviderUser(o)
-          )
+          .filter((o) => o.enabled && o.status === OrganizationUserStatusType.Confirmed)
           .sort(Utils.getSortFunction(this.i18nService, "name"));
       })
     );

libs/angular/src/vault/components/add-edit.component.ts

@@ -10,7 +10,7 @@ import { PlatformUtilsService } from "@bitwarden/common/abstractions/platformUti
 import { StateService } from "@bitwarden/common/abstractions/state.service";
 import { CollectionService } from "@bitwarden/common/admin-console/abstractions/collection.service";
 import {
-  isNotProviderUser,
+  isMember,
   OrganizationService,
 } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
@@ -192,7 +192,7 @@ export class AddEditComponent implements OnInit, OnDestroy {
 
     const orgs = await this.organizationService.getAll();
     orgs
-      .filter(isNotProviderUser)
+      .filter(isMember)
       .sort(Utils.getSortFunction(this.i18nService, "name"))
       .forEach((o) => {
         if (o.enabled && o.status === OrganizationUserStatusType.Confirmed) {

libs/angular/src/vault/vault-filter/services/vault-filter.service.ts

@@ -4,7 +4,7 @@ import { firstValueFrom, from, mergeMap, Observable } from "rxjs";
 import { StateService } from "@bitwarden/common/abstractions/state.service";
 import { CollectionService } from "@bitwarden/common/admin-console/abstractions/collection.service";
 import {
-  isNotProviderUser,
+  isMember,
   OrganizationService,
 } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
@@ -44,9 +44,7 @@ export class VaultFilterService implements DeprecatedVaultFilterServiceAbstracti
   async buildOrganizations(): Promise<Organization[]> {
     let organizations = await this.organizationService.getAll();
     if (organizations != null) {
-      organizations = organizations
-        .filter(isNotProviderUser)
-        .sort((a, b) => a.name.localeCompare(b.name));
+      organizations = organizations.filter(isMember).sort((a, b) => a.name.localeCompare(b.name));
     }
 
     return organizations;

libs/common/spec/services/organization/organization.service.spec.ts

@@ -160,7 +160,7 @@ describe("Organization Service", () => {
   });
 
   function organizationData(id: string, name: string) {
-    const data = new OrganizationData({} as any);
+    const data = new OrganizationData({} as any, {} as any);
     data.id = id;
     data.name = name;
     data.identifier = "test";

libs/common/spec/services/policy.service.spec.ts

@@ -401,7 +401,7 @@ describe("PolicyService", () => {
     status: OrganizationUserStatusType,
     managePolicies: boolean
   ) {
-    const organizationData = new OrganizationData({} as any);
+    const organizationData = new OrganizationData({} as any, {} as any);
     organizationData.id = id;
     organizationData.enabled = enabled;
     organizationData.usePolicies = usePolicies;

libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts

@@ -56,13 +56,22 @@ export function canAccessAdmin(i18nService: I18nService) {
   );
 }
 
-export function isNotProviderUser(org: Organization): boolean {
-  return !org.isProviderUser;
+/**
+ * Returns `true` if a user is a member of an organization (rather than only being a ProviderUser)
+ * @deprecated Use organizationService.memberOrganizations$ instead
+ */
+export function isMember(org: Organization): boolean {
+  return org.isMember;
 }
 
 export abstract class OrganizationService {
   organizations$: Observable<Organization[]>;
 
+  /**
+   * Organizations that the user is a member of (excludes organizations that they only have access to via a provider)
+   */
+  memberOrganizations$: Observable<Organization[]>;
+
   get$: (id: string) => Observable<Organization | undefined>;
   get: (id: string) => Organization;
   getByIdentifier: (identifier: string) => Organization;

libs/common/src/admin-console/models/data/organization.data.ts

@@ -38,6 +38,7 @@ export class OrganizationData {
   providerName: string;
   providerType?: ProviderType;
   isProviderUser: boolean;
+  isMember: boolean;
   familySponsorshipFriendlyName: string;
   familySponsorshipAvailable: boolean;
   planProductType: ProductType;
@@ -48,7 +49,13 @@ export class OrganizationData {
   familySponsorshipToDelete?: boolean;
   accessSecretsManager: boolean;
 
-  constructor(response: ProfileOrganizationResponse) {
+  constructor(
+    response: ProfileOrganizationResponse,
+    options: {
+      isMember: boolean;
+      isProviderUser: boolean;
+    }
+  ) {
     this.id = response.id;
     this.name = response.name;
     this.status = response.status;
@@ -91,5 +98,8 @@ export class OrganizationData {
     this.familySponsorshipValidUntil = response.familySponsorshipValidUntil;
     this.familySponsorshipToDelete = response.familySponsorshipToDelete;
     this.accessSecretsManager = response.accessSecretsManager;
+
+    this.isMember = options.isMember;
+    this.isProviderUser = options.isProviderUser;
   }
 }

libs/common/src/admin-console/models/domain/organization.ts

@@ -9,7 +9,14 @@ export class Organization {
   id: string;
   name: string;
   status: OrganizationUserStatusType;
+
+  /**
+   * The member's role in the organization.
+   * Avoid using this for permission checks - use the getters instead (e.g. isOwner, isAdmin, canManageX), because they
+   * properly handle permission inheritance and relationships.
+   */
   type: OrganizationUserType;
+
   enabled: boolean;
   usePolicies: boolean;
   useGroups: boolean;
@@ -39,7 +46,14 @@ export class Organization {
   providerId: string;
   providerName: string;
   providerType?: ProviderType;
+  /**
+   * Indicates that a user is a ProviderUser for the organization
+   */
   isProviderUser: boolean;
+  /**
+   * Indicates that a user is a member for the organization (may be `false` if they have access via a Provider only)
+   */
+  isMember: boolean;
   familySponsorshipFriendlyName: string;
   familySponsorshipAvailable: boolean;
   planProductType: ProductType;
@@ -89,6 +103,7 @@ export class Organization {
     this.providerName = obj.providerName;
     this.providerType = obj.providerType;
     this.isProviderUser = obj.isProviderUser;
+    this.isMember = obj.isMember;
     this.familySponsorshipFriendlyName = obj.familySponsorshipFriendlyName;
     this.familySponsorshipAvailable = obj.familySponsorshipAvailable;
     this.planProductType = obj.planProductType;
@@ -101,24 +116,29 @@ export class Organization {
   }
 
   get canAccess() {
-    if (this.type === OrganizationUserType.Owner) {
+    if (this.isOwner) {
       return true;
     }
     return this.enabled && this.status === OrganizationUserStatusType.Confirmed;
   }
 
+  /**
+   * Whether a user has Manager permissions or greater
+   */
   get isManager() {
-    return (
-      this.type === OrganizationUserType.Manager ||
-      this.type === OrganizationUserType.Owner ||
-      this.type === OrganizationUserType.Admin
-    );
+    return this.type === OrganizationUserType.Manager || this.isAdmin;
   }
 
+  /**
+   * Whether a user has Admin permissions or greater
+   */
   get isAdmin() {
-    return this.type === OrganizationUserType.Owner || this.type === OrganizationUserType.Admin;
+    return this.type === OrganizationUserType.Admin || this.isOwner;
   }
 
+  /**
+   * Whether a user has Owner permissions (including ProviderUsers)
+   */
   get isOwner() {
     return this.type === OrganizationUserType.Owner || this.isProviderUser;
   }

libs/common/src/admin-console/services/organization/organization.service.ts

@@ -1,7 +1,10 @@
 import { BehaviorSubject, concatMap, map, Observable } from "rxjs";
 
 import { StateService } from "../../../abstractions/state.service";
-import { InternalOrganizationService as InternalOrganizationServiceAbstraction } from "../../abstractions/organization/organization.service.abstraction";
+import {
+  InternalOrganizationService as InternalOrganizationServiceAbstraction,
+  isMember,
+} from "../../abstractions/organization/organization.service.abstraction";
 import { OrganizationData } from "../../models/data/organization.data";
 import { Organization } from "../../models/domain/organization";
 
@@ -9,6 +12,7 @@ export class OrganizationService implements InternalOrganizationServiceAbstracti
   protected _organizations = new BehaviorSubject<Organization[]>([]);
 
   organizations$ = this._organizations.asObservable();
+  memberOrganizations$ = this.organizations$.pipe(map((orgs) => orgs.filter(isMember)));
 
   constructor(private stateService: StateService) {
     this.stateService.activeAccountUnlocked$

libs/common/src/admin-console/services/policy/policy.service.ts

@@ -260,7 +260,7 @@ export class PolicyService implements InternalPolicyServiceAbstraction {
     await this.stateService.setEncryptedPolicies(null, { userId: userId });
   }
 
-  private isExcemptFromPolicies(organization: Organization, policyType: PolicyType) {
+  private isExemptFromPolicies(organization: Organization, policyType: PolicyType) {
     if (policyType === PolicyType.MaximumVaultTimeout) {
       return organization.type === OrganizationUserType.Owner;
     }
@@ -291,7 +291,7 @@ export class PolicyService implements InternalPolicyServiceAbstraction {
         o.status >= OrganizationUserStatusType.Accepted &&
         o.usePolicies &&
         policySet.has(o.id) &&
-        !this.isExcemptFromPolicies(o, policyType)
+        !this.isExemptFromPolicies(o, policyType)
     );
   }
 }

libs/common/src/vault/services/sync/sync.service.ts

@@ -314,24 +314,13 @@ export class SyncService implements SyncServiceAbstraction {
     await this.stateService.setForcePasswordReset(response.forcePasswordReset);
     await this.keyConnectorService.setUsesKeyConnector(response.usesKeyConnector);
 
-    const organizations: { [id: string]: OrganizationData } = {};
-    response.organizations.forEach((o) => {
-      organizations[o.id] = new OrganizationData(o);
-    });
+    await this.syncProfileOrganizations(response);
 
     const providers: { [id: string]: ProviderData } = {};
     response.providers.forEach((p) => {
       providers[p.id] = new ProviderData(p);
     });
 
-    response.providerOrganizations.forEach((o) => {
-      if (organizations[o.id] == null) {
-        organizations[o.id] = new OrganizationData(o);
-        organizations[o.id].isProviderUser = true;
-      }
-    });
-
-    await this.organizationService.replace(organizations);
     await this.providerService.save(providers);
 
     if (await this.keyConnectorService.userNeedsMigration()) {
@@ -342,6 +331,29 @@ export class SyncService implements SyncServiceAbstraction {
     }
   }
 
+  private async syncProfileOrganizations(response: ProfileResponse) {
+    const organizations: { [id: string]: OrganizationData } = {};
+    response.organizations.forEach((o) => {
+      organizations[o.id] = new OrganizationData(o, {
+        isMember: true,
+        isProviderUser: false,
+      });
+    });
+
+    response.providerOrganizations.forEach((o) => {
+      if (organizations[o.id] == null) {
+        organizations[o.id] = new OrganizationData(o, {
+          isMember: false,
+          isProviderUser: true,
+        });
+      } else {
+        organizations[o.id].isProviderUser = true;
+      }
+    });
+
+    await this.organizationService.replace(organizations);
+  }
+
   private async syncFolders(response: FolderResponse[]) {
     const folders: { [id: string]: FolderData } = {};
     response.forEach((f) => {