CodeCow/clients
1
0
Fork 0
mirror of https://github.com/bitwarden/clients.git synced 2026-07-21 21:17:06 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

PM-38298 - Make DefaultOrganizationInviteService type-safe

Browse Source 
Switch getInvitePolicies to return undefined instead of null (per the
strict-mode ADR), plumb the user ID through the accept path so we no
longer have to read it from a nullable observable, and add defensive
guards around encryption outputs that the request constructors require.
Drops the now-unused AccountService dependency.
This commit is contained in:
Jared Snider
2026-06-02 19:22:23 -04:00
parent 7225dbeb79
commit c475c541be
7 changed files with 40 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/web/src/app/auth/core/services/login/web-login-component.service.spec.ts
+2 -2
View File
@@ -101,9 +101,9 @@ describe("WebLoginComponentService", () => {
expect(result).toBeUndefined();
});
it("returns undefined if getInvitePolicies returns null", async () => {
it("returns undefined if getInvitePolicies returns undefined", async () => {
organizationInviteService.getOrganizationInvite.mockResolvedValue(orgInvite);
organizationInviteService.getInvitePolicies.mockResolvedValue(null);
organizationInviteService.getInvitePolicies.mockResolvedValue(undefined);
const result = await service.getOrgPoliciesFromOrgInvite(mockEmail);
expect(result).toBeUndefined();
});
apps/web/src/app/auth/core/services/registration/web-registration-finish.service.spec.ts
+2 -2
View File
@@ -111,9 +111,9 @@ describe("WebRegistrationFinishService", () => {
expect(organizationInviteService.getOrganizationInvite).toHaveBeenCalled();
});
it("returns null when the policies are null", async () => {
it("returns null when the policies are undefined", async () => {
organizationInviteService.getOrganizationInvite.mockResolvedValue(orgInvite);
organizationInviteService.getInvitePolicies.mockResolvedValue(null);
organizationInviteService.getInvitePolicies.mockResolvedValue(undefined);
const result = await service.getMasterPasswordPolicyOptsFromOrgInvite();
apps/web/src/app/billing/trial-initiation/complete-trial-initiation/complete-trial-initiation.component.ts
+2 -2
View File
@@ -169,13 +169,13 @@ export class CompleteTrialInitiationComponent implements OnInit, OnDestroy {
});
const invite = await this.organizationInviteService.getOrganizationInvite();
let policies: Policy[] | undefined | null = null;
let policies: Policy[] | undefined;
if (invite != null) {
policies = await this.organizationInviteService.getInvitePolicies(invite);
}
if (policies !== null) {
if (policies != null) {
this.accountService.activeAccount$
.pipe(
getUserId,
libs/angular/src/services/jslib-services.module.ts
-1
View File
@@ -1770,7 +1770,6 @@ const safeProviders: SafeProvider[] = [
OrganizationApiServiceAbstraction,
OrganizationUserApiService,
I18nServiceAbstraction,
AccountService,
GlobalStateProvider,
],
}),
libs/common/src/auth/organization-invite/default-organization-invite.service.spec.ts
+2 -7
View File
@@ -12,7 +12,6 @@ import { PolicyType } from "@bitwarden/common/admin-console/enums";
import { Policy } from "@bitwarden/common/admin-console/models/domain/policy";
import { ResetPasswordPolicyOptions } from "@bitwarden/common/admin-console/models/domain/reset-password-policy-options";
import { OrganizationKeysResponse } from "@bitwarden/common/admin-console/models/response/organization-keys.response";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { OrganizationInvite } from "@bitwarden/common/auth/organization-invite/organization-invite";
import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
@@ -42,7 +41,6 @@ describe("DefaultOrganizationInviteService", () => {
let organizationApiService: MockProxy<OrganizationApiServiceAbstraction>;
let organizationUserApiService: MockProxy<OrganizationUserApiService>;
let i18nService: MockProxy<I18nService>;
let accountService: MockProxy<AccountService>;
let globalStateProvider: FakeGlobalStateProvider;
beforeEach(() => {
@@ -56,7 +54,6 @@ describe("DefaultOrganizationInviteService", () => {
organizationApiService = mock();
organizationUserApiService = mock();
i18nService = mock();
accountService = mock();
globalStateProvider = new FakeGlobalStateProvider();
sut = new DefaultOrganizationInviteService(
@@ -70,7 +67,6 @@ describe("DefaultOrganizationInviteService", () => {
organizationApiService,
organizationUserApiService,
i18nService,
accountService,
globalStateProvider,
);
});
@@ -232,7 +228,6 @@ describe("DefaultOrganizationInviteService", () => {
publicKey: "publicKey",
}),
);
accountService.activeAccount$ = new BehaviorSubject({ id: "activeUserId" }) as any;
keyService.userKey$.mockReturnValue(new BehaviorSubject({ key: "userKey" } as any));
encryptService.encapsulateKeyUnsigned.mockResolvedValue({
encryptedString: "encryptedString",
@@ -277,14 +272,14 @@ describe("DefaultOrganizationInviteService", () => {
);
});
it("returns null and logs when the policy fetch throws", async () => {
it("returns undefined and logs when the policy fetch throws", async () => {
const invite = createOrgInvite();
const error = new Error("fetch failed");
policyApiService.getPoliciesByToken.mockRejectedValue(error);
const result = await sut.getInvitePolicies(invite);
expect(result).toBeNull();
expect(result).toBeUndefined();
expect(logService.error).toHaveBeenCalledWith(error);
});
libs/common/src/auth/organization-invite/default-organization-invite.service.ts
+29 -21
View File
@@ -1,5 +1,3 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { firstValueFrom } from "rxjs";
// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
@@ -16,7 +14,6 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli
import { PolicyType } from "@bitwarden/common/admin-console/enums";
import { Policy } from "@bitwarden/common/admin-console/models/domain/policy";
import { OrganizationKeysRequest } from "@bitwarden/common/admin-console/models/request/organization-keys.request";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { OrganizationInvite } from "@bitwarden/common/auth/organization-invite/organization-invite";
import { ORGANIZATION_INVITE } from "@bitwarden/common/auth/organization-invite/organization-invite-state";
@@ -47,7 +44,6 @@ export class DefaultOrganizationInviteService implements OrganizationInviteServi
private readonly organizationApiService: OrganizationApiServiceAbstraction,
private readonly organizationUserApiService: OrganizationUserApiService,
private readonly i18nService: I18nService,
private readonly accountService: AccountService,
private readonly globalStateProvider: GlobalStateProvider,
) {
this.organizationInvitationState = this.globalStateProvider.get(ORGANIZATION_INVITE);
@@ -72,16 +68,13 @@ export class DefaultOrganizationInviteService implements OrganizationInviteServi
* Note: Users might need to pass a MP policy check before accepting an invite to an existing organization. If the user
* has not passed this check, they will be logged out and the invite will be stored for later use.
* @param invite an organization invite
* @param activeUserId the user ID of the active user accepting the invite
* @param userId the user ID of the active user accepting the invite
* @returns a promise that resolves a boolean indicating if the invite was accepted.
*/
async validateAndAcceptInvite(
invite: OrganizationInvite,
activeUserId: UserId,
): Promise<boolean> {
async validateAndAcceptInvite(invite: OrganizationInvite, userId: UserId): Promise<boolean> {
// Creation of a new org
if (invite.initOrganization) {
await this.acceptAndInitOrganization(invite, activeUserId);
await this.acceptAndInitOrganization(invite, userId);
return true;
}
@@ -95,11 +88,11 @@ export class DefaultOrganizationInviteService implements OrganizationInviteServi
}
// We know the user has already logged in and passed a MP policy check
await this.accept(invite);
await this.accept(invite, userId);
return true;
}
async getInvitePolicies(invite: OrganizationInvite): Promise<Policy[] | null> {
async getInvitePolicies(invite: OrganizationInvite): Promise<Policy[] | undefined> {
const cached = this.policyCache.get(invite.token);
if (cached != null) {
return cached;
@@ -118,15 +111,15 @@ export class DefaultOrganizationInviteService implements OrganizationInviteServi
return policies;
} catch (e) {
this.logService.error(e);
return null;
return undefined;
}
}
private async acceptAndInitOrganization(
invite: OrganizationInvite,
activeUserId: UserId,
userId: UserId,
): Promise<void> {
await this.prepareAcceptAndInitRequest(invite, activeUserId).then((request) =>
await this.prepareAcceptAndInitRequest(invite, userId).then((request) =>
this.organizationUserApiService.postOrganizationUserAcceptInit(
invite.organizationId,
invite.organizationUserId,
@@ -139,15 +132,23 @@ export class DefaultOrganizationInviteService implements OrganizationInviteServi
private async prepareAcceptAndInitRequest(
invite: OrganizationInvite,
activeUserId: UserId,
userId: UserId,
): Promise<OrganizationUserAcceptInitRequest> {
const [encryptedOrgKey, orgKey] = await this.keyService.makeOrgKey<OrgKey>(activeUserId);
const [encryptedOrgKey, orgKey] = await this.keyService.makeOrgKey<OrgKey>(userId);
const [orgPublicKey, encryptedOrgPrivateKey] = await this.keyService.makeKeyPair(orgKey);
const collection = await this.encryptService.encryptString(
this.i18nService.t("defaultCollection"),
orgKey,
);
if (
encryptedOrgKey.encryptedString == null ||
encryptedOrgPrivateKey.encryptedString == null ||
collection.encryptedString == null
) {
throw new Error("Failed to encrypt organization init data.");
}
return new OrganizationUserAcceptInitRequest(
invite.token,
encryptedOrgKey.encryptedString,
@@ -156,8 +157,8 @@ export class DefaultOrganizationInviteService implements OrganizationInviteServi
);
}
private async accept(invite: OrganizationInvite): Promise<void> {
await this.prepareAcceptRequest(invite).then((request) =>
private async accept(invite: OrganizationInvite, userId: UserId): Promise<void> {
await this.prepareAcceptRequest(invite, userId).then((request) =>
this.organizationUserApiService.postOrganizationUserAccept(
invite.organizationId,
invite.organizationUserId,
@@ -171,6 +172,7 @@ export class DefaultOrganizationInviteService implements OrganizationInviteServi
private async prepareAcceptRequest(
invite: OrganizationInvite,
userId: UserId,
): Promise<OrganizationUserAcceptRequest> {
const request = new OrganizationUserAcceptRequest();
request.token = invite.token;
@@ -184,10 +186,16 @@ export class DefaultOrganizationInviteService implements OrganizationInviteServi
const publicKey = Utils.fromB64ToArray(response.publicKey);
const activeUserId = (await firstValueFrom(this.accountService.activeAccount$)).id;
const userKey = await firstValueFrom(this.keyService.userKey$(activeUserId));
const userKey = await firstValueFrom(this.keyService.userKey$(userId));
if (userKey == null) {
throw new Error("User key is required to enroll in password reset.");
}
// RSA Encrypt user's encKey.key with organization public key
const encryptedKey = await this.encryptService.encapsulateKeyUnsigned(userKey, publicKey);
if (encryptedKey.encryptedString == null) {
throw new Error("Failed to encrypt user key for password reset enrollment.");
}
// Add reset password key to accept request
request.resetPasswordKey = encryptedKey.encryptedString;
libs/common/src/auth/organization-invite/organization-invite.service.ts
+3 -6
View File
@@ -25,10 +25,7 @@ export abstract class OrganizationInviteService {
* the user returns after re-authenticating with a compliant master password.
* @returns true if the invite was accepted; false if it was stashed pending re-auth.
*/
abstract validateAndAcceptInvite(
invite: OrganizationInvite,
activeUserId: UserId,
): Promise<boolean>;
abstract validateAndAcceptInvite(invite: OrganizationInvite, userId: UserId): Promise<boolean>;
/**
* Fetches all enabled policies for the inviting organization, authenticated via the invite token
@@ -36,7 +33,7 @@ export abstract class OrganizationInviteService {
* `ResetPassword`). Results are cached on the service instance keyed by invite token; the cache
* is cleared on `setOrganizationInvitation` and `clearOrganizationInvitation` so state transitions
* never leave stale entries behind.
* @returns all enabled policies for the org, or null on fetch error.
* @returns all enabled policies for the org, or undefined on fetch error.
*/
abstract getInvitePolicies(invite: OrganizationInvite): Promise<Policy[] | null>;
abstract getInvitePolicies(invite: OrganizationInvite): Promise<Policy[] | undefined>;
}