From a5fff6d7dcccd108d565f7a9f3cb9be6825c0286 Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 6 Feb 2025 14:45:23 -0500 Subject: [PATCH 01/21] Create `createUser` method in `userCreationService` --- .../lib/server/service/userCreationService.ts | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 packages/lib/server/service/userCreationService.ts diff --git a/packages/lib/server/service/userCreationService.ts b/packages/lib/server/service/userCreationService.ts new file mode 100644 index 00000000000000..3732974abbdadc --- /dev/null +++ b/packages/lib/server/service/userCreationService.ts @@ -0,0 +1,43 @@ +import { createHash } from "crypto"; + +import { checkIfEmailIsBlockedInWatchlistController } from "@calcom/features/watchlist/operations/check-if-email-in-watchlist.controller"; +import type { CreationSource } from "@calcom/prisma/enums"; + +import { UserRepository } from "../repository/user"; + +interface CreateUserInput { + email: string; + username: string; + password?: string; + brandColor?: string; + darkBrandColor?: string; + hideBranding?: boolean; + weekStart?: string; + timeZone?: string; + theme?: string | null; + timeFormat?: number; + locale?: string; + avatar?: string; + organizationId?: number | null; + creationSource: CreationSource; +} + +export class UserCreationService { + static async createUser(data: CreateUserInput) { + const { email, password } = data; + + const shouldLockByDefault = await checkIfEmailIsBlockedInWatchlistController(email); + + const hashedPassword = + password ?? createHash("md5").update(`${email}${process.env.CALENDSO_ENCRYPTION_KEY}`).digest("hex"); + + const user = await UserRepository.create({ + ...data, + hashedPassword, + organizationId: data?.organizationId ?? null, + locked: shouldLockByDefault, + }); + + return user; + } +} From 898377beca07e21e92d275dfa2b0d8e87702b1a2 Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 6 Feb 2025 14:46:29 -0500 Subject: [PATCH 02/21] Refactor `UserRepository` create method to accept prisma input and remove business logic --- packages/lib/server/repository/user.ts | 47 +++++++++++++------------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/packages/lib/server/repository/user.ts b/packages/lib/server/repository/user.ts index 0449d05f2d1e6c..1eef99a11ebfdc 100644 --- a/packages/lib/server/repository/user.ts +++ b/packages/lib/server/repository/user.ts @@ -1,7 +1,4 @@ -import { createHash } from "crypto"; - import { whereClauseForOrgWithSlugOrRequestedSlug } from "@calcom/ee/organizations/lib/orgDomains"; -import { hashPassword } from "@calcom/features/auth/lib/hashPassword"; import logger from "@calcom/lib/logger"; import { safeStringify } from "@calcom/lib/safeStringify"; import { getTranslation } from "@calcom/lib/server/i18n"; @@ -581,20 +578,19 @@ export class UserRepository { }); } - static async create({ - email, - username, - organizationId, - creationSource, - }: { - email: string; - username: string; - organizationId: number | null; - creationSource: CreationSource; - }) { + static async create( + data: Omit & { + username: string; + hashedPassword: string; + organizationId: number | null; + creationSource: CreationSource; + locked: boolean; + } + ) { + const organizationId = data.organizationId; + const { email, username, hashedPassword, creationSource, ...rest } = data; + console.log("create user", { email, username, organizationId }); - const password = createHash("md5").update(`${email}${process.env.CALENDSO_ENCRYPTION_KEY}`).digest("hex"); - const hashedPassword = await hashPassword(password); const t = await getTranslation("en", "common"); const availability = getAvailabilityFromSchedule(DEFAULT_SCHEDULE); @@ -618,17 +614,20 @@ export class UserRepository { }, }, }, - organizationId: organizationId, - profiles: organizationId + creationSource, + ...(!!organizationId ? { - create: { - username: slugify(username), - organizationId: organizationId, - uid: ProfileRepository.generateProfileUid(), + organizationId: organizationId, + profiles: { + create: { + username: slugify(username), + organizationId: organizationId, + uid: ProfileRepository.generateProfileUid(), + }, }, } - : undefined, - creationSource, + : {}), + ...rest, }, }); } From a3a758376ee1a575a89a24b6321f9442d62159a6 Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 6 Feb 2025 14:47:32 -0500 Subject: [PATCH 03/21] API use `UserCreationService` --- apps/api/v1/pages/api/users/_post.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/api/v1/pages/api/users/_post.ts b/apps/api/v1/pages/api/users/_post.ts index 8331005acd1eb2..90dfb5378f5303 100644 --- a/apps/api/v1/pages/api/users/_post.ts +++ b/apps/api/v1/pages/api/users/_post.ts @@ -2,7 +2,7 @@ import type { NextApiRequest } from "next"; import { HttpError } from "@calcom/lib/http-error"; import { defaultResponder } from "@calcom/lib/server"; -import prisma from "@calcom/prisma"; +import { UserCreationService } from "@calcom/lib/server/service/userCreationService"; import { CreationSource } from "@calcom/prisma/enums"; import { schemaUserCreateBodyParams } from "~/lib/validations/user"; @@ -93,7 +93,7 @@ async function postHandler(req: NextApiRequest) { // If user is not ADMIN, return unauthorized. if (!isSystemWideAdmin) throw new HttpError({ statusCode: 401, message: "You are not authorized" }); const data = await schemaUserCreateBodyParams.parseAsync(req.body); - const user = await prisma.user.create({ data: { ...data, creationSource: CreationSource.API_V1 } }); + const user = await UserCreationService.createUser({ ...data, creationSource: CreationSource.API_V1 }); req.statusCode = 201; return { user }; } From 852dad73c0ec59796716a04fb8f0fb2f442a6715 Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 6 Feb 2025 14:51:52 -0500 Subject: [PATCH 04/21] Move slugify to service --- packages/lib/server/repository/user.ts | 5 ++--- packages/lib/server/service/userCreationService.ts | 4 +++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/lib/server/repository/user.ts b/packages/lib/server/repository/user.ts index 1eef99a11ebfdc..08622cb2332c14 100644 --- a/packages/lib/server/repository/user.ts +++ b/packages/lib/server/repository/user.ts @@ -13,7 +13,6 @@ import { userMetadata } from "@calcom/prisma/zod-utils"; import type { UpId, UserProfile } from "@calcom/types/UserProfile"; import { DEFAULT_SCHEDULE, getAvailabilityFromSchedule } from "../../availability"; -import slugify from "../../slugify"; import { ProfileRepository } from "./profile"; import { getParsedTeam } from "./teamUtils"; @@ -596,7 +595,7 @@ export class UserRepository { return await prisma.user.create({ data: { - username: slugify(username), + username, email: email, password: { create: { hash: hashedPassword } }, // Default schedule @@ -620,7 +619,7 @@ export class UserRepository { organizationId: organizationId, profiles: { create: { - username: slugify(username), + username, organizationId: organizationId, uid: ProfileRepository.generateProfileUid(), }, diff --git a/packages/lib/server/service/userCreationService.ts b/packages/lib/server/service/userCreationService.ts index 3732974abbdadc..0ca7ce398274cd 100644 --- a/packages/lib/server/service/userCreationService.ts +++ b/packages/lib/server/service/userCreationService.ts @@ -3,6 +3,7 @@ import { createHash } from "crypto"; import { checkIfEmailIsBlockedInWatchlistController } from "@calcom/features/watchlist/operations/check-if-email-in-watchlist.controller"; import type { CreationSource } from "@calcom/prisma/enums"; +import slugify from "../../slugify"; import { UserRepository } from "../repository/user"; interface CreateUserInput { @@ -24,7 +25,7 @@ interface CreateUserInput { export class UserCreationService { static async createUser(data: CreateUserInput) { - const { email, password } = data; + const { email, password, username } = data; const shouldLockByDefault = await checkIfEmailIsBlockedInWatchlistController(email); @@ -33,6 +34,7 @@ export class UserCreationService { const user = await UserRepository.create({ ...data, + username: slugify(username), hashedPassword, organizationId: data?.organizationId ?? null, locked: shouldLockByDefault, From ec9bc6c91743cef759fd41f5bb4f9bcc94eaec86 Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 6 Feb 2025 15:23:31 -0500 Subject: [PATCH 05/21] Use hashPassword instead --- packages/lib/server/repository/user.ts | 6 +++--- packages/lib/server/service/userCreationService.ts | 14 ++++++++------ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/lib/server/repository/user.ts b/packages/lib/server/repository/user.ts index 08622cb2332c14..9d92ac724c63ff 100644 --- a/packages/lib/server/repository/user.ts +++ b/packages/lib/server/repository/user.ts @@ -580,14 +580,14 @@ export class UserRepository { static async create( data: Omit & { username: string; - hashedPassword: string; + hashedPassword?: string; organizationId: number | null; creationSource: CreationSource; locked: boolean; } ) { const organizationId = data.organizationId; - const { email, username, hashedPassword, creationSource, ...rest } = data; + const { email, username, creationSource, ...rest } = data; console.log("create user", { email, username, organizationId }); const t = await getTranslation("en", "common"); @@ -597,7 +597,7 @@ export class UserRepository { data: { username, email: email, - password: { create: { hash: hashedPassword } }, + ...(data.hashedPassword && { password: { create: { hash: hashedPassword } } }), // Default schedule schedules: { create: { diff --git a/packages/lib/server/service/userCreationService.ts b/packages/lib/server/service/userCreationService.ts index 0ca7ce398274cd..80ed77b3bf9dcf 100644 --- a/packages/lib/server/service/userCreationService.ts +++ b/packages/lib/server/service/userCreationService.ts @@ -1,7 +1,6 @@ -import { createHash } from "crypto"; - +import { hashPassword } from "@calcom/features/auth/lib/hashPassword"; import { checkIfEmailIsBlockedInWatchlistController } from "@calcom/features/watchlist/operations/check-if-email-in-watchlist.controller"; -import type { CreationSource } from "@calcom/prisma/enums"; +import type { CreationSource, UserPermissionRole, IdentityProvider } from "@calcom/prisma/enums"; import slugify from "../../slugify"; import { UserRepository } from "../repository/user"; @@ -9,6 +8,7 @@ import { UserRepository } from "../repository/user"; interface CreateUserInput { email: string; username: string; + name?: string; password?: string; brandColor?: string; darkBrandColor?: string; @@ -21,6 +21,9 @@ interface CreateUserInput { avatar?: string; organizationId?: number | null; creationSource: CreationSource; + role?: UserPermissionRole; + emailVerified?: Date; + identityProvider?: IdentityProvider; } export class UserCreationService { @@ -29,13 +32,12 @@ export class UserCreationService { const shouldLockByDefault = await checkIfEmailIsBlockedInWatchlistController(email); - const hashedPassword = - password ?? createHash("md5").update(`${email}${process.env.CALENDSO_ENCRYPTION_KEY}`).digest("hex"); + const hashedPassword = password ? await hashPassword(password) : null; const user = await UserRepository.create({ ...data, username: slugify(username), - hashedPassword, + ...(hashedPassword && { hashedPassword }), organizationId: data?.organizationId ?? null, locked: shouldLockByDefault, }); From d486212fab86d60fc2f84ee26256f0e1cab2e59b Mon Sep 17 00:00:00 2001 From: Joe Au-Yeung Date: Fri, 7 Feb 2025 10:29:57 -0500 Subject: [PATCH 06/21] Type fixes in `UserCreationService` --- packages/lib/server/service/userCreationService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/lib/server/service/userCreationService.ts b/packages/lib/server/service/userCreationService.ts index 80ed77b3bf9dcf..88a4f86b54f120 100644 --- a/packages/lib/server/service/userCreationService.ts +++ b/packages/lib/server/service/userCreationService.ts @@ -8,7 +8,7 @@ import { UserRepository } from "../repository/user"; interface CreateUserInput { email: string; username: string; - name?: string; + name?: string | null; password?: string; brandColor?: string; darkBrandColor?: string; @@ -27,7 +27,7 @@ interface CreateUserInput { } export class UserCreationService { - static async createUser(data: CreateUserInput) { + static async createUser({ data }: { data: CreateUserInput }) { const { email, password, username } = data; const shouldLockByDefault = await checkIfEmailIsBlockedInWatchlistController(email); From f8230303aecb4df57b5663c2de7a2e3b7cace89a Mon Sep 17 00:00:00 2001 From: Joe Au-Yeung Date: Fri, 7 Feb 2025 10:32:36 -0500 Subject: [PATCH 07/21] Add `userCreationService` tests --- .../service/userCreationService.test.ts | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 packages/lib/server/service/userCreationService.test.ts diff --git a/packages/lib/server/service/userCreationService.test.ts b/packages/lib/server/service/userCreationService.test.ts new file mode 100644 index 00000000000000..8aa0a83fa21bed --- /dev/null +++ b/packages/lib/server/service/userCreationService.test.ts @@ -0,0 +1,86 @@ +import { describe, test, expect, vi, beforeEach } from "vitest"; + +import { hashPassword } from "@calcom/features/auth/lib/hashPassword"; +import { checkIfEmailIsBlockedInWatchlistController } from "@calcom/features/watchlist/operations/check-if-email-in-watchlist.controller"; +import { CreationSource } from "@calcom/prisma/enums"; + +import { UserRepository } from "../repository/user"; +import { UserCreationService } from "./userCreationService"; + +vi.mock("@calcom/lib/server/i18n", () => { + return { + getTranslation: (key: string) => { + return () => key; + }, + }; +}); + +vi.mock("@calcom/features/auth/lib/hashPassword", () => ({ + hashPassword: vi.fn().mockResolvedValue("hashed-password"), +})); + +vi.mock("../repository/user", async () => { + return { + UserRepository: { + create: vi.fn(), + }, + }; +}); + +vi.mock("@calcom/features/watchlist/operations/check-if-email-in-watchlist.controller", (async) => ({ + checkIfEmailIsBlockedInWatchlistController: vi.fn(() => false), +})); + +const mockUserData = { + email: "test@example.com", + username: "test", + creationSource: CreationSource.WEBAPP, +}; + +vi.stubEnv("CALCOM_LICENSE_KEY", undefined); + +describe("UserCreationService", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + test("should create user with transformed fields", async () => { + await UserCreationService.createUser({ data: mockUserData }); + + expect(UserRepository.create).toHaveBeenCalledWith( + expect.objectContaining({ + username: "test", + locked: false, + organizationId: null, + }) + ); + }); + + test("should lock user when email is in watchlist", async () => { + vi.mocked(checkIfEmailIsBlockedInWatchlistController).mockResolvedValue(true); + + await UserCreationService.createUser({ data: mockUserData }); + + expect(UserRepository.create).toHaveBeenCalledWith( + expect.objectContaining({ + locked: true, + }) + ); + }); + + test("should hash password when provided", async () => { + const mockPassword = "password"; + vi.mocked(hashPassword).mockResolvedValue("hashed_password"); + + await UserCreationService.createUser({ + data: { ...mockUserData, password: mockPassword }, + }); + + expect(hashPassword).toHaveBeenCalledWith(mockPassword); + expect(UserRepository.create).toHaveBeenCalledWith( + expect.objectContaining({ + hashedPassword: "hashed_password", + }) + ); + }); +}); From 01637f4d422185705dcdaf2c4f8681f1a074d2db Mon Sep 17 00:00:00 2001 From: Joe Au-Yeung Date: Fri, 7 Feb 2025 16:05:42 -0500 Subject: [PATCH 08/21] API accept data object --- apps/api/v1/pages/api/users/_post.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/api/v1/pages/api/users/_post.ts b/apps/api/v1/pages/api/users/_post.ts index 90dfb5378f5303..80fa92e2452081 100644 --- a/apps/api/v1/pages/api/users/_post.ts +++ b/apps/api/v1/pages/api/users/_post.ts @@ -93,7 +93,9 @@ async function postHandler(req: NextApiRequest) { // If user is not ADMIN, return unauthorized. if (!isSystemWideAdmin) throw new HttpError({ statusCode: 401, message: "You are not authorized" }); const data = await schemaUserCreateBodyParams.parseAsync(req.body); - const user = await UserCreationService.createUser({ ...data, creationSource: CreationSource.API_V1 }); + const user = await UserCreationService.createUser({ + data: { ...data, creationSource: CreationSource.API_V1 }, + }); req.statusCode = 201; return { user }; } From a88878100c84db4b5abecaa39a8e091f5c8678d7 Mon Sep 17 00:00:00 2001 From: Joe Au-Yeung Date: Fri, 7 Feb 2025 16:13:02 -0500 Subject: [PATCH 09/21] Type fixes --- packages/lib/server/repository/user.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/lib/server/repository/user.ts b/packages/lib/server/repository/user.ts index 9d92ac724c63ff..d2a0f5c5786700 100644 --- a/packages/lib/server/repository/user.ts +++ b/packages/lib/server/repository/user.ts @@ -586,18 +586,18 @@ export class UserRepository { locked: boolean; } ) { - const organizationId = data.organizationId; - const { email, username, creationSource, ...rest } = data; + const organizationIdValue = data.organizationId; + const { email, username, creationSource, hashedPassword, organizationId, ...rest } = data; - console.log("create user", { email, username, organizationId }); + console.log("create user", { email, username, organizationIdValue }); const t = await getTranslation("en", "common"); const availability = getAvailabilityFromSchedule(DEFAULT_SCHEDULE); - return await prisma.user.create({ + const user = await prisma.user.create({ data: { username, email: email, - ...(data.hashedPassword && { password: { create: { hash: hashedPassword } } }), + ...(data.hashedPassword && { password: { create: { hash: data.hashedPassword } } }), // Default schedule schedules: { create: { @@ -614,13 +614,13 @@ export class UserRepository { }, }, creationSource, - ...(!!organizationId + ...(organizationIdValue ? { - organizationId: organizationId, + organizationId: organizationIdValue, profiles: { create: { username, - organizationId: organizationId, + organizationId: organizationIdValue, uid: ProfileRepository.generateProfileUid(), }, }, @@ -629,6 +629,9 @@ export class UserRepository { ...rest, }, }); + console.log("🚀 ~ UserRepository ~ user:", user); + + return user; } static async getUserAdminTeams(userId: number) { return prisma.user.findFirst({ From 4e0d12e9a0db16d9e1e23188c5150bb906c11fd3 Mon Sep 17 00:00:00 2001 From: Joe Au-Yeung Date: Fri, 7 Feb 2025 16:14:20 -0500 Subject: [PATCH 10/21] Add user _post test --- apps/api/v1/test/lib/users/_post.test.ts | 90 ++++++++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 apps/api/v1/test/lib/users/_post.test.ts diff --git a/apps/api/v1/test/lib/users/_post.test.ts b/apps/api/v1/test/lib/users/_post.test.ts new file mode 100644 index 00000000000000..c907848ba121dc --- /dev/null +++ b/apps/api/v1/test/lib/users/_post.test.ts @@ -0,0 +1,90 @@ +import prismock from "../../../../../../tests/libs/__mocks__/prisma"; + +import type { Request, Response } from "express"; +import type { NextApiRequest, NextApiResponse } from "next"; +import { createMocks } from "node-mocks-http"; +import { describe, test, expect, vi } from "vitest"; + +import handler from "../../../pages/api/users/_post"; + +type CustomNextApiRequest = NextApiRequest & Request; +type CustomNextApiResponse = NextApiResponse & Response; + +vi.mock("@calcom/lib/server/i18n", () => { + return { + getTranslation: (key: string) => { + return () => key; + }, + }; +}); + +vi.stubEnv("CALCOM_LICENSE_KEY", undefined); + +describe("POST /api/users", () => { + test("should throw 401 if not system-wide admin", async () => { + const { req, res } = createMocks({ + method: "POST", + body: { + email: "test@example.com", + username: "test", + }, + }); + req.isSystemWideAdmin = false; + + await handler(req, res); + + expect(res.statusCode).toBe(401); + }); + test("should throw a 400 if no email is provided", async () => { + const { req, res } = createMocks({ + method: "POST", + body: { + username: "test", + }, + }); + req.isSystemWideAdmin = true; + + await handler(req, res); + + expect(res.statusCode).toBe(400); + }); + test("should throw a 400 if no username is provided", async () => { + const { req, res } = createMocks({ + method: "POST", + body: { + email: "test@example.com", + }, + }); + req.isSystemWideAdmin = true; + + await handler(req, res); + + expect(res.statusCode).toBe(400); + }); + test("should create user successfully", async () => { + const { req, res } = createMocks({ + method: "POST", + body: { + email: "test@example.com", + username: "test", + }, + prisma: prismock, + }); + req.isSystemWideAdmin = true; + + await handler(req, res); + + expect(res.statusCode).toBe(200); + + const response = JSON.parse(res._getData()); + + expect(response.user).toEqual( + expect.objectContaining({ + email: "test@example.com", + username: "test", + locked: false, + organizationId: null, + }) + ); + }); +}); From 3e18a615369be8155a63a9ddea621feaa63bae18 Mon Sep 17 00:00:00 2001 From: Joe Au-Yeung Date: Sat, 8 Feb 2025 21:04:50 -0500 Subject: [PATCH 11/21] Add test for locked user --- apps/api/v1/test/lib/users/_post.test.ts | 36 ++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/apps/api/v1/test/lib/users/_post.test.ts b/apps/api/v1/test/lib/users/_post.test.ts index c907848ba121dc..c3b1a784ea04af 100644 --- a/apps/api/v1/test/lib/users/_post.test.ts +++ b/apps/api/v1/test/lib/users/_post.test.ts @@ -87,4 +87,40 @@ describe("POST /api/users", () => { }) ); }); + + test("should auto lock user if email is in watchlist", async () => { + const { req, res } = createMocks({ + method: "POST", + body: { + email: "test@example.com", + username: "test", + }, + prisma: prismock, + }); + req.isSystemWideAdmin = true; + + await prismock.watchlist.create({ + data: { + type: "EMAIL", + value: "test@example.com", + severity: "CRITICAL", + createdById: 1, + }, + }); + + await handler(req, res); + + expect(res.statusCode).toBe(200); + + const response = JSON.parse(res._getData()); + + expect(response.user).toEqual( + expect.objectContaining({ + email: "test@example.com", + username: "test", + locked: true, + organizationId: null, + }) + ); + }); }); From a679abfba9d2499d66258e55c91b50a98e0b936d Mon Sep 17 00:00:00 2001 From: Joe Au-Yeung Date: Sat, 8 Feb 2025 21:10:55 -0500 Subject: [PATCH 12/21] Add locked param to log --- packages/lib/server/repository/user.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/lib/server/repository/user.ts b/packages/lib/server/repository/user.ts index d2a0f5c5786700..e6b69882652a5c 100644 --- a/packages/lib/server/repository/user.ts +++ b/packages/lib/server/repository/user.ts @@ -587,9 +587,9 @@ export class UserRepository { } ) { const organizationIdValue = data.organizationId; - const { email, username, creationSource, hashedPassword, organizationId, ...rest } = data; + const { email, username, creationSource, locked, ...rest } = data; - console.log("create user", { email, username, organizationIdValue }); + console.log("create user", { email, username, organizationIdValue, locked }); const t = await getTranslation("en", "common"); const availability = getAvailabilityFromSchedule(DEFAULT_SCHEDULE); @@ -629,7 +629,6 @@ export class UserRepository { ...rest, }, }); - console.log("🚀 ~ UserRepository ~ user:", user); return user; } From cf319e984a511710ae035713c765ffa80a733953 Mon Sep 17 00:00:00 2001 From: Joe Au-Yeung Date: Sat, 8 Feb 2025 21:13:48 -0500 Subject: [PATCH 13/21] Add user repository tests --- packages/lib/server/repository/user.test.ts | 101 ++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 packages/lib/server/repository/user.test.ts diff --git a/packages/lib/server/repository/user.test.ts b/packages/lib/server/repository/user.test.ts new file mode 100644 index 00000000000000..282d00a6dead2d --- /dev/null +++ b/packages/lib/server/repository/user.test.ts @@ -0,0 +1,101 @@ +// eslint-disable-next-line @typescript-eslint/no-unused-vars +import prismock from "../../../../tests/libs/__mocks__/prisma"; + +import { describe, test, vi, expect, beforeEach } from "vitest"; + +import { CreationSource } from "@calcom/prisma/enums"; + +import { UserRepository } from "./user"; + +vi.mock("@calcom/lib/server/i18n", () => { + return { + getTranslation: (key: string) => { + return () => key; + }, + }; +}); + +describe("UserRepository", () => { + beforeEach(() => { + prismock; + }); + + describe("create", () => { + test("Should create a user without a password", async () => { + const user = await UserRepository.create({ + username: "test", + email: "test@example.com", + organizationId: null, + creationSource: CreationSource.WEBAPP, + locked: false, + }); + + expect(user).toEqual( + expect.objectContaining({ + username: "test", + email: "test@example.com", + organizationId: null, + creationSource: CreationSource.WEBAPP, + locked: false, + }) + ); + + const password = await prismock.userPassword.findUnique({ + where: { + userId: user.id, + }, + }); + + expect(password).toBeNull(); + }); + + test("If locked param is passed, user should be locked", async () => { + const user = await UserRepository.create({ + username: "test", + email: "test@example.com", + organizationId: null, + creationSource: CreationSource.WEBAPP, + locked: true, + }); + + expect(user).toEqual( + expect.objectContaining({ + locked: true, + }) + ); + }); + + test("If organizationId is passed, user should be associated with the organization", async () => { + const organizationId = 123; + const username = "test"; + + const user = await UserRepository.create({ + username, + email: "test@example.com", + organizationId, + creationSource: CreationSource.WEBAPP, + locked: true, + }); + + expect(user).toEqual( + expect.objectContaining({ + organizationId, + }) + ); + + const profile = await prismock.profile.findFirst({ + where: { + organizationId, + username, + }, + }); + + expect(profile).toEqual( + expect.objectContaining({ + organizationId, + username, + }) + ); + }); + }); +}); From 4aa04d43c8b79cef3a6698c3bddd5538333ef27a Mon Sep 17 00:00:00 2001 From: Joe Au-Yeung Date: Sat, 8 Feb 2025 21:39:41 -0500 Subject: [PATCH 14/21] Do not return locked status --- packages/lib/server/service/userCreationService.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/lib/server/service/userCreationService.ts b/packages/lib/server/service/userCreationService.ts index 88a4f86b54f120..5056965348511d 100644 --- a/packages/lib/server/service/userCreationService.ts +++ b/packages/lib/server/service/userCreationService.ts @@ -1,5 +1,6 @@ import { hashPassword } from "@calcom/features/auth/lib/hashPassword"; import { checkIfEmailIsBlockedInWatchlistController } from "@calcom/features/watchlist/operations/check-if-email-in-watchlist.controller"; +import logger from "@calcom/lib/logger"; import type { CreationSource, UserPermissionRole, IdentityProvider } from "@calcom/prisma/enums"; import slugify from "../../slugify"; @@ -26,6 +27,8 @@ interface CreateUserInput { identityProvider?: IdentityProvider; } +const log = logger.getSubLogger({ prefix: ["[userCreationService]"] }); + export class UserCreationService { static async createUser({ data }: { data: CreateUserInput }) { const { email, password, username } = data; @@ -42,6 +45,10 @@ export class UserCreationService { locked: shouldLockByDefault, }); - return user; + log.info(`Created user: ${user.id} with locked status of ${user.locked}`); + + const { locked, ...rest } = user; + + return rest; } } From 86200eceb327304cca727cf7adb5fcd8f58716ea Mon Sep 17 00:00:00 2001 From: Joe Au-Yeung Date: Sat, 8 Feb 2025 21:42:34 -0500 Subject: [PATCH 15/21] Explicitly pass `locked` prop --- packages/lib/server/repository/user.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/lib/server/repository/user.ts b/packages/lib/server/repository/user.ts index e6b69882652a5c..5cfe242377c003 100644 --- a/packages/lib/server/repository/user.ts +++ b/packages/lib/server/repository/user.ts @@ -614,6 +614,7 @@ export class UserRepository { }, }, creationSource, + locked, ...(organizationIdValue ? { organizationId: organizationIdValue, From ea94100de1b1e288b6da7ab14bd6c1955cba3428 Mon Sep 17 00:00:00 2001 From: Joe Au-Yeung Date: Sat, 8 Feb 2025 21:50:05 -0500 Subject: [PATCH 16/21] Fix tests when locked isn't returned --- packages/lib/server/repository/user.test.ts | 11 +++++++++- .../service/userCreationService.test.ts | 21 ++++++++++++++++--- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/packages/lib/server/repository/user.test.ts b/packages/lib/server/repository/user.test.ts index 282d00a6dead2d..78886a2819fd05 100644 --- a/packages/lib/server/repository/user.test.ts +++ b/packages/lib/server/repository/user.test.ts @@ -58,7 +58,16 @@ describe("UserRepository", () => { locked: true, }); - expect(user).toEqual( + const userQuery = await prismock.user.findUnique({ + where: { + email: "test@example.com", + }, + select: { + locked: true, + }, + }); + + expect(userQuery).toEqual( expect.objectContaining({ locked: true, }) diff --git a/packages/lib/server/service/userCreationService.test.ts b/packages/lib/server/service/userCreationService.test.ts index 8aa0a83fa21bed..b067d7262d6f5b 100644 --- a/packages/lib/server/service/userCreationService.test.ts +++ b/packages/lib/server/service/userCreationService.test.ts @@ -1,3 +1,5 @@ +import prismock from "../../../../tests/libs/__mocks__/prisma"; + import { describe, test, expect, vi, beforeEach } from "vitest"; import { hashPassword } from "@calcom/features/auth/lib/hashPassword"; @@ -41,11 +43,18 @@ vi.stubEnv("CALCOM_LICENSE_KEY", undefined); describe("UserCreationService", () => { beforeEach(() => { + prismock; vi.clearAllMocks(); }); test("should create user with transformed fields", async () => { - await UserCreationService.createUser({ data: mockUserData }); + vi.spyOn(UserRepository, "create").mockResolvedValue({ + username: "test", + locked: false, + organizationId: null, + } as any); + + const user = await UserCreationService.createUser({ data: mockUserData }); expect(UserRepository.create).toHaveBeenCalledWith( expect.objectContaining({ @@ -54,25 +63,29 @@ describe("UserCreationService", () => { organizationId: null, }) ); + + expect(user).not.toHaveProperty("locked"); }); test("should lock user when email is in watchlist", async () => { vi.mocked(checkIfEmailIsBlockedInWatchlistController).mockResolvedValue(true); - await UserCreationService.createUser({ data: mockUserData }); + const user = await UserCreationService.createUser({ data: mockUserData }); expect(UserRepository.create).toHaveBeenCalledWith( expect.objectContaining({ locked: true, }) ); + + expect(user).not.toHaveProperty("locked"); }); test("should hash password when provided", async () => { const mockPassword = "password"; vi.mocked(hashPassword).mockResolvedValue("hashed_password"); - await UserCreationService.createUser({ + const user = await UserCreationService.createUser({ data: { ...mockUserData, password: mockPassword }, }); @@ -82,5 +95,7 @@ describe("UserCreationService", () => { hashedPassword: "hashed_password", }) ); + + expect(user).not.toHaveProperty("locked"); }); }); From 8e56c25aec1a2245d4445cdd3990926b047ce20d Mon Sep 17 00:00:00 2001 From: Joe Au-Yeung Date: Sun, 9 Feb 2025 16:41:25 -0500 Subject: [PATCH 17/21] Fix tests --- apps/api/v1/test/lib/users/_post.test.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/apps/api/v1/test/lib/users/_post.test.ts b/apps/api/v1/test/lib/users/_post.test.ts index c3b1a784ea04af..7aabe2fd03e85d 100644 --- a/apps/api/v1/test/lib/users/_post.test.ts +++ b/apps/api/v1/test/lib/users/_post.test.ts @@ -76,9 +76,13 @@ describe("POST /api/users", () => { expect(res.statusCode).toBe(200); - const response = JSON.parse(res._getData()); + const userQuery = await prismock.user.findFirst({ + where: { + email: "test@example.com", + }, + }); - expect(response.user).toEqual( + expect(userQuery).toEqual( expect.objectContaining({ email: "test@example.com", username: "test", @@ -112,9 +116,13 @@ describe("POST /api/users", () => { expect(res.statusCode).toBe(200); - const response = JSON.parse(res._getData()); + const userQuery = await prismock.user.findFirst({ + where: { + email: "test@example.com", + }, + }); - expect(response.user).toEqual( + expect(userQuery).toEqual( expect.objectContaining({ email: "test@example.com", username: "test", From 1eae797dd0ff90f37e4ca49099494bdddf25b768 Mon Sep 17 00:00:00 2001 From: Joe Au-Yeung Date: Sun, 9 Feb 2025 17:42:32 -0500 Subject: [PATCH 18/21] Pass locked prop --- packages/lib/server/repository/organization.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/lib/server/repository/organization.ts b/packages/lib/server/repository/organization.ts index 752e19b1f16cb7..809c91df00eb3f 100644 --- a/packages/lib/server/repository/organization.ts +++ b/packages/lib/server/repository/organization.ts @@ -90,6 +90,7 @@ export class OrganizationRepository { email: owner.email, username: ownerUsernameInOrg, organizationId: organization.id, + locked: false, creationSource, }); From 571ae020cb7c44197c742dc556114e84736f9ce3 Mon Sep 17 00:00:00 2001 From: Joe Au-Yeung Date: Sun, 9 Feb 2025 23:14:18 -0500 Subject: [PATCH 19/21] Edit test name --- packages/lib/server/service/userCreationService.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/lib/server/service/userCreationService.test.ts b/packages/lib/server/service/userCreationService.test.ts index b067d7262d6f5b..39e35fe1de3d8f 100644 --- a/packages/lib/server/service/userCreationService.test.ts +++ b/packages/lib/server/service/userCreationService.test.ts @@ -47,7 +47,7 @@ describe("UserCreationService", () => { vi.clearAllMocks(); }); - test("should create user with transformed fields", async () => { + test("should create user", async () => { vi.spyOn(UserRepository, "create").mockResolvedValue({ username: "test", locked: false, From a6bbd69a6621d9caff8cee6644a5f3d11808c807 Mon Sep 17 00:00:00 2001 From: Joe Au-Yeung Date: Mon, 10 Feb 2025 16:14:18 -0500 Subject: [PATCH 20/21] Use logger --- packages/lib/server/repository/user.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/lib/server/repository/user.ts b/packages/lib/server/repository/user.ts index 5cfe242377c003..09d77ccee652bf 100644 --- a/packages/lib/server/repository/user.ts +++ b/packages/lib/server/repository/user.ts @@ -589,7 +589,7 @@ export class UserRepository { const organizationIdValue = data.organizationId; const { email, username, creationSource, locked, ...rest } = data; - console.log("create user", { email, username, organizationIdValue, locked }); + logger.info("create user", { email, username, organizationIdValue, locked }); const t = await getTranslation("en", "common"); const availability = getAvailabilityFromSchedule(DEFAULT_SCHEDULE); From 527539e85f389883abbd70aa28db89bb85fce978 Mon Sep 17 00:00:00 2001 From: Joe Au-Yeung Date: Thu, 13 Feb 2025 23:17:03 -0500 Subject: [PATCH 21/21] Fix passing hashed password --- packages/lib/server/repository/user.ts | 4 ++-- packages/lib/server/service/userCreationService.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/lib/server/repository/user.ts b/packages/lib/server/repository/user.ts index 09d77ccee652bf..8bfab16f6f3cac 100644 --- a/packages/lib/server/repository/user.ts +++ b/packages/lib/server/repository/user.ts @@ -587,7 +587,7 @@ export class UserRepository { } ) { const organizationIdValue = data.organizationId; - const { email, username, creationSource, locked, ...rest } = data; + const { email, username, creationSource, locked, hashedPassword, ...rest } = data; logger.info("create user", { email, username, organizationIdValue, locked }); const t = await getTranslation("en", "common"); @@ -597,7 +597,7 @@ export class UserRepository { data: { username, email: email, - ...(data.hashedPassword && { password: { create: { hash: data.hashedPassword } } }), + ...(hashedPassword && { password: { create: { hash: hashedPassword } } }), // Default schedule schedules: { create: { diff --git a/packages/lib/server/service/userCreationService.ts b/packages/lib/server/service/userCreationService.ts index 5056965348511d..3b138aca71b967 100644 --- a/packages/lib/server/service/userCreationService.ts +++ b/packages/lib/server/service/userCreationService.ts @@ -47,8 +47,8 @@ export class UserCreationService { log.info(`Created user: ${user.id} with locked status of ${user.locked}`); - const { locked, ...rest } = user; + const { locked, ...restUser } = user; - return rest; + return restUser; } }