Skip to content

Commit f8bd206

Browse files
authored
[PM-28045] - Validate Organization Key Population (#19954)
* Adding validation to ensure organizations are not being created without keys unintentionally * Added request test class. * Removing request test. * removing duplicate check * Keys is different than key
1 parent 26200c7 commit f8bd206

12 files changed

Lines changed: 365 additions & 79 deletions

File tree

apps/web/src/app/auth/organization-invite/accept-organization.service.ts

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ import { Injectable } from "@angular/core";
44
import { BehaviorSubject, firstValueFrom, map } from "rxjs";
55

66
import {
7-
OrganizationUserApiService,
8-
OrganizationUserAcceptRequest,
97
OrganizationUserAcceptInitRequest,
8+
OrganizationUserAcceptRequest,
9+
OrganizationUserApiService,
1010
} from "@bitwarden/admin-console/common";
1111
import { ApiService } from "@bitwarden/common/abstractions/api.service";
1212
import { OrganizationApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization-api.service.abstraction";
@@ -105,24 +105,19 @@ export class AcceptOrganizationInviteService {
105105
invite: OrganizationInvite,
106106
activeUserId: UserId,
107107
): Promise<OrganizationUserAcceptInitRequest> {
108-
const request = new OrganizationUserAcceptInitRequest();
109-
request.token = invite.token;
110-
111108
const [encryptedOrgKey, orgKey] = await this.keyService.makeOrgKey<OrgKey>(activeUserId);
112109
const [orgPublicKey, encryptedOrgPrivateKey] = await this.keyService.makeKeyPair(orgKey);
113110
const collection = await this.encryptService.encryptString(
114111
this.i18nService.t("defaultCollection"),
115112
orgKey,
116113
);
117114

118-
request.key = encryptedOrgKey.encryptedString;
119-
request.keys = new OrganizationKeysRequest(
120-
orgPublicKey,
121-
encryptedOrgPrivateKey.encryptedString,
115+
return new OrganizationUserAcceptInitRequest(
116+
invite.token,
117+
encryptedOrgKey.encryptedString,
118+
new OrganizationKeysRequest(orgPublicKey, encryptedOrgPrivateKey.encryptedString),
119+
collection.encryptedString,
122120
);
123-
request.collectionName = collection.encryptedString;
124-
125-
return request;
126121
}
127122

128123
private async accept(invite: OrganizationInvite): Promise<void> {

apps/web/src/app/billing/organizations/organization-plans.component.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,16 +1023,17 @@ export class OrganizationPlansComponent implements OnInit, OnDestroy {
10231023
orgKey: SymmetricCryptoKey;
10241024
activeUserId: UserId;
10251025
}): Promise<string> {
1026-
const request = new OrganizationCreateRequest();
1027-
request.key = encryptionData.key;
1028-
request.collectionName = encryptionData.collectionCt;
1026+
const request = new OrganizationCreateRequest(
1027+
encryptionData.key,
1028+
new OrganizationKeysRequest(
1029+
encryptionData.orgKeys[0],
1030+
encryptionData.orgKeys[1].encryptedString as string,
1031+
),
1032+
encryptionData.collectionCt,
1033+
);
10291034
request.name = this.formGroup.controls.name.value ?? "";
10301035
request.billingEmail = this.formGroup.controls.billingEmail.value ?? "";
10311036
request.initiationPath = "New organization creation in-product";
1032-
request.keys = new OrganizationKeysRequest(
1033-
encryptionData.orgKeys[0],
1034-
encryptionData.orgKeys[1].encryptedString as string,
1035-
);
10361037

10371038
if (this.selectedPlan()!.type === PlanType.Free) {
10381039
request.planType = PlanType.Free;

bitwarden_license/bit-web/src/app/admin-console/providers/services/web-provider.service.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -81,15 +81,15 @@ export class WebProviderService {
8181
providerKey,
8282
);
8383

84-
const request = new CreateProviderOrganizationRequest();
85-
request.name = name;
86-
request.ownerEmail = ownerEmail;
87-
request.planType = planType;
88-
request.seats = seats;
89-
90-
request.key = encryptedProviderKey.encryptedString;
91-
request.keyPair = new OrganizationKeysRequest(publicKey, encryptedPrivateKey.encryptedString);
92-
request.collectionName = encryptedCollectionName.encryptedString;
84+
const request = new CreateProviderOrganizationRequest(
85+
name,
86+
ownerEmail,
87+
planType,
88+
seats,
89+
encryptedProviderKey.encryptedString,
90+
new OrganizationKeysRequest(publicKey, encryptedPrivateKey.encryptedString),
91+
encryptedCollectionName.encryptedString,
92+
);
9393

9494
await this.providerApiService.createProviderOrganization(providerId, request);
9595

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { OrganizationKeysRequest } from "@bitwarden/common/admin-console/models/request/organization-keys.request";
2+
3+
import { OrganizationUserAcceptInitRequest } from "./organization-user-accept-init.request";
4+
5+
describe("OrganizationUserAcceptInitRequest", () => {
6+
const validToken = "invite-token";
7+
const validKey = "encrypted-org-key";
8+
const validKeys = new OrganizationKeysRequest("public-key", "encrypted-private-key");
9+
const validCollectionName = "encrypted-collection-name";
10+
11+
it("should create a request with all required parameters", () => {
12+
const request = new OrganizationUserAcceptInitRequest(
13+
validToken,
14+
validKey,
15+
validKeys,
16+
validCollectionName,
17+
);
18+
19+
expect(request.token).toBe(validToken);
20+
expect(request.key).toBe(validKey);
21+
expect(request.keys).toBe(validKeys);
22+
expect(request.collectionName).toBe(validCollectionName);
23+
});
24+
25+
it("should throw when token is empty", () => {
26+
expect(
27+
() => new OrganizationUserAcceptInitRequest("", validKey, validKeys, validCollectionName),
28+
).toThrow("Token is required");
29+
});
30+
31+
it("should throw when key is empty", () => {
32+
expect(
33+
() => new OrganizationUserAcceptInitRequest(validToken, "", validKeys, validCollectionName),
34+
).toThrow("Organization key is required");
35+
});
36+
37+
it("should throw when keys is null", () => {
38+
expect(
39+
() => new OrganizationUserAcceptInitRequest(validToken, validKey, null!, validCollectionName),
40+
).toThrow("Organization keys are required");
41+
});
42+
43+
it("should throw when keys is undefined", () => {
44+
expect(
45+
() =>
46+
new OrganizationUserAcceptInitRequest(
47+
validToken,
48+
validKey,
49+
undefined!,
50+
validCollectionName,
51+
),
52+
).toThrow("Organization keys are required");
53+
});
54+
55+
it("should throw when collectionName is empty", () => {
56+
expect(
57+
() => new OrganizationUserAcceptInitRequest(validToken, validKey, validKeys, ""),
58+
).toThrow("Collection name is required");
59+
});
60+
});
Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,27 @@
1-
// FIXME: Update this file to be type safe and remove this and next line
2-
// @ts-strict-ignore
31
import { OrganizationKeysRequest } from "@bitwarden/common/admin-console/models/request/organization-keys.request";
42

53
export class OrganizationUserAcceptInitRequest {
64
token: string;
75
key: string;
86
keys: OrganizationKeysRequest;
97
collectionName: string;
8+
9+
constructor(token: string, key: string, keys: OrganizationKeysRequest, collectionName: string) {
10+
if (!token) {
11+
throw new Error("Token is required");
12+
}
13+
if (!key) {
14+
throw new Error("Organization key is required");
15+
}
16+
if (!keys) {
17+
throw new Error("Organization keys are required");
18+
}
19+
if (!collectionName) {
20+
throw new Error("Collection name is required");
21+
}
22+
this.token = token;
23+
this.key = key;
24+
this.keys = keys;
25+
this.collectionName = collectionName;
26+
}
1027
}
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import { PlanType } from "../../../billing/enums";
2+
3+
import { CreateProviderOrganizationRequest } from "./create-provider-organization.request";
4+
import { OrganizationKeysRequest } from "./organization-keys.request";
5+
6+
describe("CreateProviderOrganizationRequest", () => {
7+
const validName = "Test Org";
8+
const validOwnerEmail = "owner@example.com";
9+
const validPlanType = PlanType.TeamsAnnually;
10+
const validSeats = 10;
11+
const validKey = "encrypted-org-key";
12+
const validKeyPair = new OrganizationKeysRequest("public-key", "encrypted-private-key");
13+
const validCollectionName = "encrypted-collection-name";
14+
15+
const createRequest = (
16+
overrides: Partial<{
17+
name: string;
18+
ownerEmail: string;
19+
planType: PlanType;
20+
seats: number;
21+
key: string;
22+
keyPair: OrganizationKeysRequest;
23+
collectionName: string;
24+
}> = {},
25+
) => {
26+
return new CreateProviderOrganizationRequest(
27+
"name" in overrides ? overrides.name! : validName,
28+
"ownerEmail" in overrides ? overrides.ownerEmail! : validOwnerEmail,
29+
"planType" in overrides ? overrides.planType! : validPlanType,
30+
"seats" in overrides ? overrides.seats! : validSeats,
31+
"key" in overrides ? overrides.key! : validKey,
32+
"keyPair" in overrides ? overrides.keyPair! : validKeyPair,
33+
"collectionName" in overrides ? overrides.collectionName! : validCollectionName,
34+
);
35+
};
36+
37+
it("should create a request with all required parameters", () => {
38+
const request = createRequest();
39+
40+
expect(request.name).toBe(validName);
41+
expect(request.ownerEmail).toBe(validOwnerEmail);
42+
expect(request.planType).toBe(validPlanType);
43+
expect(request.seats).toBe(validSeats);
44+
expect(request.key).toBe(validKey);
45+
expect(request.keyPair).toBe(validKeyPair);
46+
expect(request.collectionName).toBe(validCollectionName);
47+
});
48+
49+
it("should throw when name is empty", () => {
50+
expect(() => createRequest({ name: "" })).toThrow("Name is required");
51+
});
52+
53+
it("should throw when ownerEmail is empty", () => {
54+
expect(() => createRequest({ ownerEmail: "" })).toThrow("Owner email is required");
55+
});
56+
57+
it("should throw when planType is null", () => {
58+
expect(() => createRequest({ planType: null! })).toThrow("Plan type is required");
59+
});
60+
61+
it("should throw when seats is null", () => {
62+
expect(() => createRequest({ seats: null! })).toThrow("Seats is required");
63+
});
64+
65+
it("should throw when key is empty", () => {
66+
expect(() => createRequest({ key: "" })).toThrow("Organization key is required");
67+
});
68+
69+
it("should throw when keyPair is null", () => {
70+
expect(() => createRequest({ keyPair: null! })).toThrow("Organization key pair is required");
71+
});
72+
73+
it("should throw when keyPair is undefined", () => {
74+
expect(() => createRequest({ keyPair: undefined })).toThrow(
75+
"Organization key pair is required",
76+
);
77+
});
78+
79+
it("should throw when collectionName is empty", () => {
80+
expect(() => createRequest({ collectionName: "" })).toThrow("Collection name is required");
81+
});
82+
});

libs/common/src/admin-console/models/request/create-provider-organization.request.ts

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
// FIXME: Update this file to be type safe and remove this and next line
2-
// @ts-strict-ignore
31
import { PlanType } from "../../../billing/enums";
42

53
import { OrganizationKeysRequest } from "./organization-keys.request";
@@ -12,4 +10,43 @@ export class CreateProviderOrganizationRequest {
1210
key: string;
1311
keyPair: OrganizationKeysRequest;
1412
collectionName: string;
13+
14+
constructor(
15+
name: string,
16+
ownerEmail: string,
17+
planType: PlanType,
18+
seats: number,
19+
key: string,
20+
keyPair: OrganizationKeysRequest,
21+
collectionName: string,
22+
) {
23+
if (!name) {
24+
throw new Error("Name is required");
25+
}
26+
if (!ownerEmail) {
27+
throw new Error("Owner email is required");
28+
}
29+
if (planType == null) {
30+
throw new Error("Plan type is required");
31+
}
32+
if (seats == null) {
33+
throw new Error("Seats is required");
34+
}
35+
if (!key) {
36+
throw new Error("Organization key is required");
37+
}
38+
if (!keyPair) {
39+
throw new Error("Organization key pair is required");
40+
}
41+
if (!collectionName) {
42+
throw new Error("Collection name is required");
43+
}
44+
this.name = name;
45+
this.ownerEmail = ownerEmail;
46+
this.planType = planType;
47+
this.seats = seats;
48+
this.key = key;
49+
this.keyPair = keyPair;
50+
this.collectionName = collectionName;
51+
}
1552
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { OrganizationCreateRequest } from "./organization-create.request";
2+
import { OrganizationKeysRequest } from "./organization-keys.request";
3+
4+
describe("OrganizationCreateRequest", () => {
5+
const validKey = "encrypted-org-key";
6+
const validKeys = new OrganizationKeysRequest("public-key", "encrypted-private-key");
7+
const validCollectionName = "encrypted-collection-name";
8+
9+
it("should create a request with valid key parameters", () => {
10+
const request = new OrganizationCreateRequest(validKey, validKeys, validCollectionName);
11+
12+
expect(request.key).toBe(validKey);
13+
expect(request.keys).toBe(validKeys);
14+
expect(request.collectionName).toBe(validCollectionName);
15+
});
16+
17+
it("should inherit validation from parent class", () => {
18+
expect(() => new OrganizationCreateRequest("", validKeys, validCollectionName)).toThrow(
19+
"Organization key is required",
20+
);
21+
22+
expect(() => new OrganizationCreateRequest(validKey, null!, validCollectionName)).toThrow(
23+
"Organization keys are required",
24+
);
25+
26+
expect(() => new OrganizationCreateRequest(validKey, validKeys, "")).toThrow(
27+
"Collection name is required",
28+
);
29+
});
30+
31+
it("should allow setting payment fields after construction", () => {
32+
const request = new OrganizationCreateRequest(validKey, validKeys, validCollectionName);
33+
request.paymentToken = "token";
34+
35+
expect(request.paymentToken).toBe("token");
36+
});
37+
});
Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
1-
// FIXME: Update this file to be type safe and remove this and next line
2-
// @ts-strict-ignore
31
import { PaymentMethodType } from "../../../billing/enums";
42
import { OrganizationNoPaymentMethodCreateRequest } from "../../../billing/models/request/organization-no-payment-method-create-request";
53

64
export class OrganizationCreateRequest extends OrganizationNoPaymentMethodCreateRequest {
7-
paymentMethodType: PaymentMethodType;
8-
paymentToken: string;
5+
paymentMethodType!: PaymentMethodType;
6+
paymentToken: string = "";
97
skipTrial?: boolean;
108
coupons?: string[];
119
}

0 commit comments

Comments
 (0)