From c21841a2df54e02b5ddafa962f33f5c577726a78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Mon, 9 Feb 2026 11:38:00 +0000 Subject: [PATCH] [PM-26485] Add member status validation to CLI confirm command (#18557) * Add validation for organization user status in CLI Confirm command - Implemented a new method to validate the status of an organization user before confirmation. - Added checks for various user states: invited, confirmed, revoked, and accepted. - Enhanced error handling to provide clearer feedback based on user status. * Refactor validation logic in ConfirmCommand to remove unnecessary user ID check - Removed the check for null userId in the validateOrganizationUserStatus method. - Simplified the validation process for organization user status before confirmation. * Add unit tests for ConfirmCommand in CLI - Created a new test suite for the ConfirmCommand to validate its functionality. - Implemented tests for various scenarios including bad requests, user status validations, and successful confirmations. - Enhanced error handling tests to ensure proper responses for missing organization keys and API failures. --- .../commands/confirm.command.spec.ts | 250 ++++++++++++++++++ .../admin-console/commands/confirm.command.ts | 23 ++ 2 files changed, 273 insertions(+) create mode 100644 apps/cli/src/admin-console/commands/confirm.command.spec.ts diff --git a/apps/cli/src/admin-console/commands/confirm.command.spec.ts b/apps/cli/src/admin-console/commands/confirm.command.spec.ts new file mode 100644 index 00000000000..69eb1cf6f39 --- /dev/null +++ b/apps/cli/src/admin-console/commands/confirm.command.spec.ts @@ -0,0 +1,250 @@ +import { mock } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { + OrganizationUserApiService, + OrganizationUserDetailsResponse, +} from "@bitwarden/admin-console/common"; +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { OrganizationUserStatusType } from "@bitwarden/common/admin-console/enums"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { UserId } from "@bitwarden/common/types/guid"; +import { OrgKey } from "@bitwarden/common/types/key"; +import { KeyService } from "@bitwarden/key-management"; + +import { Response } from "../../models/response"; + +import { ConfirmCommand } from "./confirm.command"; + +describe("ConfirmCommand", () => { + let command: ConfirmCommand; + let apiService: jest.Mocked; + let keyService: jest.Mocked; + let encryptService: jest.Mocked; + let organizationUserApiService: jest.Mocked; + let accountService: jest.Mocked; + let i18nService: jest.Mocked; + + const userId = "test-user-id" as UserId; + const organizationId = "bf61e571-fb70-4113-b305-b331004d0f19"; + const organizationUserId = "6aa431fa-7ea1-4852-907e-b36b0030a87d"; + const mockOrgKey = {} as OrgKey; + const mockPublicKey = "mockPublicKey"; + + beforeEach(() => { + apiService = mock(); + keyService = mock(); + encryptService = mock(); + organizationUserApiService = mock(); + accountService = mock(); + i18nService = mock(); + + command = new ConfirmCommand( + apiService, + keyService, + encryptService, + organizationUserApiService, + accountService, + i18nService, + ); + + // Default mocks + accountService.activeAccount$ = of({ id: userId } as any); + keyService.orgKeys$ = jest.fn().mockReturnValue(of({ [organizationId]: mockOrgKey })); + i18nService.t.mockReturnValue("My Items"); + encryptService.encryptString.mockResolvedValue({ encryptedString: "encrypted" } as any); + encryptService.encapsulateKeyUnsigned.mockResolvedValue({ encryptedString: "key" } as any); + apiService.getUserPublicKey.mockResolvedValue({ publicKey: mockPublicKey } as any); + organizationUserApiService.postOrganizationUserConfirm.mockResolvedValue(); + }); + + describe("run", () => { + it("should return bad request for unknown object", async () => { + const response = await command.run("unknown-object", organizationUserId, { + organizationid: organizationId, + }); + + expect(response).toBeInstanceOf(Response); + expect(response.success).toBe(false); + expect(response.message).toBe("Unknown object."); + }); + + it("should return bad request when organizationId is missing", async () => { + const response = await command.run("org-member", organizationUserId, {}); + + expect(response).toBeInstanceOf(Response); + expect(response.success).toBe(false); + expect(response.message).toBe("--organizationid required."); + }); + + it("should return bad request when id is not a GUID", async () => { + const response = await command.run("org-member", "not-a-guid", { + organizationid: organizationId, + }); + + expect(response).toBeInstanceOf(Response); + expect(response.success).toBe(false); + expect(response.message).toContain("is not a GUID"); + }); + + it("should return bad request when organizationId is not a GUID", async () => { + const response = await command.run("org-member", organizationUserId, { + organizationid: "not-a-guid", + }); + + expect(response).toBeInstanceOf(Response); + expect(response.success).toBe(false); + expect(response.message).toContain("is not a GUID"); + }); + }); + + describe("confirmOrganizationMember - status validation", () => { + it("should reject user with Invited status", async () => { + const invitedUser = { + id: organizationUserId, + userId: null, + status: OrganizationUserStatusType.Invited, + } as unknown as OrganizationUserDetailsResponse; + + organizationUserApiService.getOrganizationUser.mockResolvedValue(invitedUser); + + const response = await command.run("org-member", organizationUserId, { + organizationid: organizationId, + }); + + expect(response).toBeInstanceOf(Response); + expect(response.success).toBe(false); + expect(response.message).toContain( + "User must accept the invitation before they can be confirmed.", + ); + }); + + it("should reject user with Confirmed status", async () => { + const confirmedUser = { + id: organizationUserId, + userId: userId, + status: OrganizationUserStatusType.Confirmed, + } as unknown as OrganizationUserDetailsResponse; + + organizationUserApiService.getOrganizationUser.mockResolvedValue(confirmedUser); + + const response = await command.run("org-member", organizationUserId, { + organizationid: organizationId, + }); + + expect(response).toBeInstanceOf(Response); + expect(response.success).toBe(false); + expect(response.message).toContain("User is already confirmed."); + }); + + it("should reject user with Revoked status", async () => { + const revokedUser = { + id: organizationUserId, + userId: userId, + status: OrganizationUserStatusType.Revoked, + } as unknown as OrganizationUserDetailsResponse; + + organizationUserApiService.getOrganizationUser.mockResolvedValue(revokedUser); + + const response = await command.run("org-member", organizationUserId, { + organizationid: organizationId, + }); + + expect(response).toBeInstanceOf(Response); + expect(response.success).toBe(false); + expect(response.message).toContain("User is revoked and cannot be confirmed."); + }); + + it("should reject user with unexpected status", async () => { + const invalidUser = { + id: organizationUserId, + userId: userId, + status: 999 as OrganizationUserStatusType, // Invalid status + } as unknown as OrganizationUserDetailsResponse; + + organizationUserApiService.getOrganizationUser.mockResolvedValue(invalidUser); + + const response = await command.run("org-member", organizationUserId, { + organizationid: organizationId, + }); + + expect(response).toBeInstanceOf(Response); + expect(response.success).toBe(false); + expect(response.message).toContain("User is not in a valid state to be confirmed."); + }); + + it("should successfully confirm user with Accepted status", async () => { + const acceptedUser = { + id: organizationUserId, + userId: userId, + status: OrganizationUserStatusType.Accepted, + } as unknown as OrganizationUserDetailsResponse; + + organizationUserApiService.getOrganizationUser.mockResolvedValue(acceptedUser); + + const response = await command.run("org-member", organizationUserId, { + organizationid: organizationId, + }); + + expect(response).toBeInstanceOf(Response); + expect(response.success).toBe(true); + expect(apiService.getUserPublicKey).toHaveBeenCalledWith(userId); + expect(organizationUserApiService.postOrganizationUserConfirm).toHaveBeenCalledWith( + organizationId, + organizationUserId.toLowerCase(), + expect.objectContaining({ + key: "key", + defaultUserCollectionName: "encrypted", + }), + ); + }); + }); + + describe("error handling", () => { + it("should return error when organization key is not found", async () => { + keyService.orgKeys$ = jest.fn().mockReturnValue(of({})); + + const response = await command.run("org-member", organizationUserId, { + organizationid: organizationId, + }); + + expect(response).toBeInstanceOf(Response); + expect(response.success).toBe(false); + expect(response.message).toContain("No encryption key for this organization"); + }); + + it("should return error when organization user is not found", async () => { + organizationUserApiService.getOrganizationUser.mockResolvedValue(null); + + const response = await command.run("org-member", organizationUserId, { + organizationid: organizationId, + }); + + expect(response).toBeInstanceOf(Response); + expect(response.success).toBe(false); + expect(response.message).toContain("Member id does not exist for this organization"); + }); + + it("should return error when API call fails", async () => { + const acceptedUser = { + id: organizationUserId, + userId: userId, + status: OrganizationUserStatusType.Accepted, + } as unknown as OrganizationUserDetailsResponse; + + organizationUserApiService.getOrganizationUser.mockResolvedValue(acceptedUser); + organizationUserApiService.postOrganizationUserConfirm.mockRejectedValue( + new Error("API Error"), + ); + + const response = await command.run("org-member", organizationUserId, { + organizationid: organizationId, + }); + + expect(response).toBeInstanceOf(Response); + expect(response.success).toBe(false); + }); + }); +}); diff --git a/apps/cli/src/admin-console/commands/confirm.command.ts b/apps/cli/src/admin-console/commands/confirm.command.ts index 4458054e244..a387df25443 100644 --- a/apps/cli/src/admin-console/commands/confirm.command.ts +++ b/apps/cli/src/admin-console/commands/confirm.command.ts @@ -5,8 +5,10 @@ import { firstValueFrom, map, switchMap } from "rxjs"; import { OrganizationUserApiService, OrganizationUserConfirmRequest, + OrganizationUserDetailsResponse, } from "@bitwarden/admin-console/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { OrganizationUserStatusType } from "@bitwarden/common/admin-console/enums"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; @@ -72,6 +74,9 @@ export class ConfirmCommand { if (orgUser == null) { throw new Error("Member id does not exist for this organization."); } + + this.validateOrganizationUserStatus(orgUser); + const publicKeyResponse = await this.apiService.getUserPublicKey(orgUser.userId); const publicKey = Utils.fromB64ToArray(publicKeyResponse.publicKey); const key = await this.encryptService.encapsulateKeyUnsigned(orgKey, publicKey); @@ -94,6 +99,24 @@ export class ConfirmCommand { const encrypted = await this.encryptService.encryptString(defaultCollectionName, orgKey); return encrypted.encryptedString; } + + private validateOrganizationUserStatus(orgUser: OrganizationUserDetailsResponse): void { + if (orgUser.status === OrganizationUserStatusType.Invited) { + throw new Error("User must accept the invitation before they can be confirmed."); + } + + if (orgUser.status === OrganizationUserStatusType.Confirmed) { + throw new Error("User is already confirmed."); + } + + if (orgUser.status === OrganizationUserStatusType.Revoked) { + throw new Error("User is revoked and cannot be confirmed."); + } + + if (orgUser.status !== OrganizationUserStatusType.Accepted) { + throw new Error("User is not in a valid state to be confirmed."); + } + } } class Options {