Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PM-15506] Wire up vNextOrganizationService for web client #12810

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
d2370da
Wire up vNextOrganizationService in PolicyService
BTreston Dec 30, 2024
59564ee
Wire vNextOrganizationService in SyncService
BTreston Dec 30, 2024
937176b
wire vNextOrganizationService for EventCollectionService
BTreston Dec 30, 2024
feb1854
wire vNextOrganizationService for KeyConnectorService
BTreston Dec 30, 2024
5111b78
wire up vNextOrganizationService for CipherAuthorizationService
BTreston Dec 31, 2024
54b4a77
Wire up vNextOrganizationService in PolicyService
BTreston Dec 30, 2024
bf7132c
Wire vNextOrganizationService in SyncService
BTreston Dec 30, 2024
42c9b7f
wire vNextOrganizationService for EventCollectionService
BTreston Dec 30, 2024
24520f1
wire vNextOrganizationService for KeyConnectorService
BTreston Dec 30, 2024
4f5f522
wire up vNextOrganizationService for CipherAuthorizationService
BTreston Dec 31, 2024
3f2c736
wire vNextOrganizationService for share.component
BTreston Jan 2, 2025
d79eeff
wire vNextOrganizationService for collections.component
BTreston Jan 2, 2025
c33470a
Merge branch 'ac/pm-15506-Wire-up-vNext-OrganizationService' of githuโ€ฆ
BTreston Jan 2, 2025
a6697cb
wire vNextOrganizationServcie for add-account-credit-dialog
BTreston Jan 2, 2025
bfcfba4
wire vNextOrganizationService for vault-filter.service
BTreston Jan 3, 2025
d42fc73
fix browser errors for vNextOrganizationService implementation in libs
BTreston Jan 3, 2025
385d14b
fix desktop errors for vNextOrganizationService implementation for libs
BTreston Jan 3, 2025
61dc7c7
fix linter errors
BTreston Jan 3, 2025
4c607ae
fix CLI errors on vNextOrganizationServcie implementations for libs
BTreston Jan 3, 2025
0d90a8b
Refactor for libs
BTreston Jan 10, 2025
5a3115e
refactor org service calls in cli
BTreston Jan 10, 2025
9433431
refactor calls in extension
BTreston Jan 10, 2025
6e6cf38
refactor call in desktop
BTreston Jan 10, 2025
1ba6d90
wire up vNextOrganizationService in web client
BTreston Jan 10, 2025
cc21be8
Merge branch 'ac/pm-15506-vNextOrganizationService' into ac/pm-15506-โ€ฆ
BTreston Jan 10, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
wire up vNextOrganizationService for CipherAuthorizationService
  • Loading branch information
BTreston committed Jan 2, 2025
commit 4f5f5226713e160898b59e25ebc53ca05fbde071
2 changes: 1 addition & 1 deletion libs/angular/src/services/jslib-services.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1411,7 +1411,7 @@ const safeProviders: SafeProvider[] = [
safeProvider({
provide: CipherAuthorizationService,
useClass: DefaultCipherAuthorizationService,
deps: [CollectionService, OrganizationServiceAbstraction],
deps: [CollectionService, vNextOrganizationServiceAbstraction, AccountServiceAbstraction],
}),
safeProvider({
provide: AuthRequestApiService,
Expand Down
59 changes: 39 additions & 20 deletions libs/common/src/vault/services/cipher-authorization.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { mock } from "jest-mock-extended";
import { firstValueFrom, of } from "rxjs";
import { Observable, firstValueFrom, of } from "rxjs";

import { CollectionService, CollectionView } from "@bitwarden/admin-console/common";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { vNextOrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/vnext.organization.service.abstraction";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { CollectionId } from "@bitwarden/common/types/guid";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { CollectionId, UserId } from "@bitwarden/common/types/guid";

import { FakeAccountService, mockAccountServiceWith } from "../../../spec";
import { CipherView } from "../models/view/cipher.view";

import {
Expand All @@ -17,7 +19,9 @@ describe("CipherAuthorizationService", () => {
let cipherAuthorizationService: CipherAuthorizationService;

const mockCollectionService = mock<CollectionService>();
const mockOrganizationService = mock<OrganizationService>();
const mockOrganizationService = mock<vNextOrganizationService>();
const mockUserId = Utils.newGuid() as UserId;
let mockAccountService: FakeAccountService;

// Mock factories
const createMockCipher = (
Expand All @@ -42,6 +46,7 @@ describe("CipherAuthorizationService", () => {
isAdmin = false,
editAnyCollection = false,
} = {}) => ({
id: "org1",
allowAdminAccessToAllCollectionItems,
canEditAllCiphers,
canEditUnassignedCiphers,
Expand All @@ -53,9 +58,11 @@ describe("CipherAuthorizationService", () => {

beforeEach(() => {
jest.clearAllMocks();
mockAccountService = mockAccountServiceWith(mockUserId);
cipherAuthorizationService = new DefaultCipherAuthorizationService(
mockCollectionService,
mockOrganizationService,
mockAccountService,
);
});

Expand All @@ -72,7 +79,9 @@ describe("CipherAuthorizationService", () => {
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.get$.mockReturnValue(of(organization as Organization));
mockOrganizationService.organizations$.mockReturnValue(
of([organization]) as Observable<Organization[]>,
);

cipherAuthorizationService.canDeleteCipher$(cipher, [], true).subscribe((result) => {
expect(result).toBe(true);
Expand All @@ -83,19 +92,21 @@ describe("CipherAuthorizationService", () => {
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.get$.mockReturnValue(of(organization as Organization));
mockOrganizationService.organizations$.mockReturnValue(
of([organization]) as Observable<Organization[]>,
);

cipherAuthorizationService.canDeleteCipher$(cipher, [], true).subscribe((result) => {
expect(result).toBe(true);
expect(mockOrganizationService.get$).toHaveBeenCalledWith("org1");
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.get$.mockReturnValue(of(organization as Organization));
mockOrganizationService.organizations$.mockReturnValue(of([organization] as Organization[]));

cipherAuthorizationService.canDeleteCipher$(cipher, [], true).subscribe((result) => {
expect(result).toBe(false);
Expand All @@ -106,8 +117,8 @@ describe("CipherAuthorizationService", () => {
it("should return true if activeCollectionId is provided and has manage permission", (done) => {
const cipher = createMockCipher("org1", ["col1", "col2"]) as CipherView;
const activeCollectionId = "col1" as CollectionId;
const org = createMockOrganization();
mockOrganizationService.get$.mockReturnValue(of(org as Organization));
const organization = createMockOrganization();
mockOrganizationService.organizations$.mockReturnValue(of([organization] as Organization[]));

const allCollections = [
createMockCollection("col1", true),
Expand All @@ -132,8 +143,8 @@ describe("CipherAuthorizationService", () => {
it("should return false if activeCollectionId is provided and manage permission is not present", (done) => {
const cipher = createMockCipher("org1", ["col1", "col2"]) as CipherView;
const activeCollectionId = "col1" as CollectionId;
const org = createMockOrganization();
mockOrganizationService.get$.mockReturnValue(of(org as Organization));
const organization = createMockOrganization();
mockOrganizationService.organizations$.mockReturnValue(of([organization] as Organization[]));

const allCollections = [
createMockCollection("col1", false),
Expand All @@ -157,8 +168,8 @@ describe("CipherAuthorizationService", () => {

it("should return true if any collection has manage permission", (done) => {
const cipher = createMockCipher("org1", ["col1", "col2", "col3"]) as CipherView;
const org = createMockOrganization();
mockOrganizationService.get$.mockReturnValue(of(org as Organization));
const organization = createMockOrganization();
mockOrganizationService.organizations$.mockReturnValue(of([organization] as Organization[]));

const allCollections = [
createMockCollection("col1", false),
Expand All @@ -182,8 +193,8 @@ describe("CipherAuthorizationService", () => {

it("should return false if no collection has manage permission", (done) => {
const cipher = createMockCipher("org1", ["col1", "col2"]) as CipherView;
const org = createMockOrganization();
mockOrganizationService.get$.mockReturnValue(of(org as Organization));
const organization = createMockOrganization();
mockOrganizationService.organizations$.mockReturnValue(of([organization] as Organization[]));

const allCollections = [
createMockCollection("col1", false),
Expand Down Expand Up @@ -216,7 +227,9 @@ describe("CipherAuthorizationService", () => {
it("should return true for admin users", async () => {
const cipher = createMockCipher("org1", []) as CipherView;
const organization = createMockOrganization({ isAdmin: true });
mockOrganizationService.get$.mockReturnValue(of(organization as Organization));
mockOrganizationService.organizations$.mockReturnValue(
of([organization] as Organization[]),
);

const result = await firstValueFrom(
cipherAuthorizationService.canCloneCipher$(cipher, true),
Expand All @@ -227,7 +240,9 @@ describe("CipherAuthorizationService", () => {
it("should return true for custom user with canEditAnyCollection", async () => {
const cipher = createMockCipher("org1", []) as CipherView;
const organization = createMockOrganization({ editAnyCollection: true });
mockOrganizationService.get$.mockReturnValue(of(organization as Organization));
mockOrganizationService.organizations$.mockReturnValue(
of([organization] as Organization[]),
);

const result = await firstValueFrom(
cipherAuthorizationService.canCloneCipher$(cipher, true),
Expand All @@ -240,7 +255,9 @@ describe("CipherAuthorizationService", () => {
it("should return true if at least one cipher collection has manage permission", async () => {
const cipher = createMockCipher("org1", ["col1", "col2"]) as CipherView;
const organization = createMockOrganization();
mockOrganizationService.get$.mockReturnValue(of(organization as Organization));
mockOrganizationService.organizations$.mockReturnValue(
of([organization] as Organization[]),
);

const allCollections = [
createMockCollection("col1", true),
Expand All @@ -257,7 +274,9 @@ describe("CipherAuthorizationService", () => {
it("should return false if no collection has manage permission", async () => {
const cipher = createMockCipher("org1", ["col1", "col2"]) as CipherView;
const organization = createMockOrganization();
mockOrganizationService.get$.mockReturnValue(of(organization as Organization));
mockOrganizationService.organizations$.mockReturnValue(
of([organization] as Organization[]),
);

const allCollections = [
createMockCollection("col1", false),
Expand Down
15 changes: 11 additions & 4 deletions libs/common/src/vault/services/cipher-authorization.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
import { map, Observable, of, shareReplay, switchMap } from "rxjs";

import { CollectionService } from "@bitwarden/admin-console/common";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { vNextOrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/vnext.organization.service.abstraction";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { CollectionId } from "@bitwarden/common/types/guid";

import { Cipher } from "../models/domain/cipher";
Expand Down Expand Up @@ -50,9 +51,15 @@ export abstract class CipherAuthorizationService {
export class DefaultCipherAuthorizationService implements CipherAuthorizationService {
constructor(
private collectionService: CollectionService,
private organizationService: OrganizationService,
private organizationService: vNextOrganizationService,
private accountService: AccountService,
) {}

private organization$ = (cipher: CipherLike) =>
this.accountService.activeAccount$.pipe(
switchMap((account) => this.organizationService.organizations$(account?.id)),
map((orgs) => orgs.find((org) => org.id === cipher.organizationId)),
);
/**
*
* {@link CipherAuthorizationService.canDeleteCipher$}
Expand All @@ -66,7 +73,7 @@ export class DefaultCipherAuthorizationService implements CipherAuthorizationSer
return of(true);
}

return this.organizationService.get$(cipher.organizationId).pipe(
return this.organization$(cipher).pipe(
switchMap((organization) => {
if (isAdminConsoleAction) {
// If the user is an admin, they can delete an unassigned cipher
Expand Down Expand Up @@ -104,7 +111,7 @@ export class DefaultCipherAuthorizationService implements CipherAuthorizationSer
return of(true);
}

return this.organizationService.get$(cipher.organizationId).pipe(
return this.organization$(cipher).pipe(
switchMap((organization) => {
// Admins and custom users can always clone when in the Admin Console
if (
Expand Down