From 5fe54fd3f2be5d4df8f4f71a070ee82ad76d431f Mon Sep 17 00:00:00 2001 From: Pedro Castro Date: Sun, 28 Dec 2025 23:47:08 -0300 Subject: [PATCH] fix(schedules): add missing await on async permission check (#26218) The hasReadPermissionsForUserId function is async but was being called without await in two locations. This aligns with the existing correct usage in ScheduleRepository.findManyDetailedScheduleByUserId - Add await to getAllSchedulesByUserId.handler.ts - Add await to ScheduleRepository.findDetailedScheduleById - Refactor ScheduleRepository tests to use shared prismaMock - Add new handler test with authorization coverage Co-authored-by: Keith Williams --- .../repositories/ScheduleRepository.test.ts | 196 ++++++++++++------ .../repositories/ScheduleRepository.ts | 2 +- .../getAllSchedulesByUserId.handler.test.ts | 158 ++++++++++++++ .../getAllSchedulesByUserId.handler.ts | 2 +- 4 files changed, 291 insertions(+), 67 deletions(-) create mode 100644 packages/trpc/server/routers/viewer/availability/schedule/getAllSchedulesByUserId.handler.test.ts diff --git a/packages/features/schedules/repositories/ScheduleRepository.test.ts b/packages/features/schedules/repositories/ScheduleRepository.test.ts index 9d383e99b3..495bc03a71 100644 --- a/packages/features/schedules/repositories/ScheduleRepository.test.ts +++ b/packages/features/schedules/repositories/ScheduleRepository.test.ts @@ -1,52 +1,39 @@ +import prismaMock from "../../../../tests/libs/__mocks__/prismaMock"; + import { describe, expect, it, vi, beforeEach } from "vitest"; -import { PrismaClient } from "@calcom/prisma"; +import type { Schedule, User } from "@calcom/prisma/client"; import { ScheduleRepository } from "./ScheduleRepository"; -vi.mock("@calcom/prisma", () => { - const mockPrisma = { - user: { - findUnique: vi.fn(), - update: vi.fn(), - }, - schedule: { - findFirst: vi.fn(), - }, - }; - return { - __esModule: true, - default: mockPrisma, - PrismaClient: vi.fn(() => mockPrisma), - }; -}); - vi.mock("@calcom/lib/hasEditPermissionForUser", () => ({ - hasReadPermissionsForUserId: vi.fn().mockResolvedValue(true), + hasReadPermissionsForUserId: vi.fn(), })); +import { hasReadPermissionsForUserId } from "@calcom/lib/hasEditPermissionForUser"; + +const mockHasReadPermissions = vi.mocked(hasReadPermissionsForUserId); + describe("ScheduleRepository", () => { - let prisma: PrismaClient; let scheduleRepository: ScheduleRepository; beforeEach(() => { - prisma = new PrismaClient(); - scheduleRepository = new ScheduleRepository(prisma); vi.clearAllMocks(); + scheduleRepository = new ScheduleRepository(prismaMock); }); describe("constructor", () => { it("should throw error if prismaClient is not provided", () => { - expect(() => new ScheduleRepository(null as any)).toThrow( - "PrismaClient is required for ScheduleRepository" - ); - expect(() => new ScheduleRepository(undefined as any)).toThrow( + // @ts-expect-error - testing invalid input + expect(() => new ScheduleRepository(null)).toThrow("PrismaClient is required for ScheduleRepository"); + // @ts-expect-error - testing invalid input + expect(() => new ScheduleRepository(undefined)).toThrow( "PrismaClient is required for ScheduleRepository" ); }); it("should create instance successfully with valid prismaClient", () => { - const repo = new ScheduleRepository(prisma); + const repo = new ScheduleRepository(prismaMock); expect(repo).toBeInstanceOf(ScheduleRepository); }); }); @@ -55,14 +42,12 @@ describe("ScheduleRepository", () => { it("should return defaultScheduleId if user has one", async () => { const userId = 1; const defaultScheduleId = 123; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (prisma.user.findUnique as any).mockResolvedValue({ - defaultScheduleId, - }); + + prismaMock.user.findUnique.mockResolvedValue({ defaultScheduleId } as User); const result = await scheduleRepository.getDefaultScheduleId(userId); - expect(prisma.user.findUnique).toHaveBeenCalledWith({ + expect(prismaMock.user.findUnique).toHaveBeenCalledWith({ where: { id: userId }, select: { defaultScheduleId: true }, }); @@ -72,22 +57,13 @@ describe("ScheduleRepository", () => { it("should find and return first schedule if user has no defaultScheduleId", async () => { const userId = 1; const scheduleId = 456; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (prisma.user.findUnique as any).mockResolvedValue({ - defaultScheduleId: null, - }); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (prisma.schedule.findFirst as any).mockResolvedValue({ - id: scheduleId, - }); + + prismaMock.user.findUnique.mockResolvedValue({ defaultScheduleId: null } as User); + prismaMock.schedule.findFirst.mockResolvedValue({ id: scheduleId } as Schedule); const result = await scheduleRepository.getDefaultScheduleId(userId); - expect(prisma.user.findUnique).toHaveBeenCalledWith({ - where: { id: userId }, - select: { defaultScheduleId: true }, - }); - expect(prisma.schedule.findFirst).toHaveBeenCalledWith({ + expect(prismaMock.schedule.findFirst).toHaveBeenCalledWith({ where: { userId }, select: { id: true }, }); @@ -96,12 +72,9 @@ describe("ScheduleRepository", () => { it("should throw error if no schedules found", async () => { const userId = 1; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (prisma.user.findUnique as any).mockResolvedValue({ - defaultScheduleId: null, - }); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (prisma.schedule.findFirst as any).mockResolvedValue(null); + + prismaMock.user.findUnique.mockResolvedValue({ defaultScheduleId: null } as User); + prismaMock.schedule.findFirst.mockResolvedValue(null); await expect(scheduleRepository.getDefaultScheduleId(userId)).rejects.toThrow( "No schedules found for user" @@ -120,14 +93,12 @@ describe("ScheduleRepository", () => { it("should return true if user has a schedule", async () => { const user = { id: 1, defaultScheduleId: null }; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (prisma.schedule.findFirst as any).mockResolvedValue({ - id: 456, - }); + + prismaMock.schedule.findFirst.mockResolvedValue({ id: 456 } as Schedule); const result = await scheduleRepository.hasDefaultSchedule(user); - expect(prisma.schedule.findFirst).toHaveBeenCalledWith({ + expect(prismaMock.schedule.findFirst).toHaveBeenCalledWith({ where: { userId: user.id }, }); expect(result).toBe(true); @@ -135,14 +106,11 @@ describe("ScheduleRepository", () => { it("should return false if user has no defaultScheduleId and no schedules", async () => { const user = { id: 1, defaultScheduleId: null }; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (prisma.schedule.findFirst as any).mockResolvedValue(null); + + prismaMock.schedule.findFirst.mockResolvedValue(null); const result = await scheduleRepository.hasDefaultSchedule(user); - expect(prisma.schedule.findFirst).toHaveBeenCalledWith({ - where: { userId: user.id }, - }); expect(result).toBe(false); }); }); @@ -151,17 +119,115 @@ describe("ScheduleRepository", () => { it("should update user with new defaultScheduleId", async () => { const userId = 1; const scheduleId = 123; - const updatedUser = { id: userId, defaultScheduleId: scheduleId }; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (prisma.user.update as any).mockResolvedValue(updatedUser); + const updatedUser = { id: userId, defaultScheduleId: scheduleId } as User; + + prismaMock.user.update.mockResolvedValue(updatedUser); const result = await scheduleRepository.setupDefaultSchedule(userId, scheduleId); - expect(prisma.user.update).toHaveBeenCalledWith({ + expect(prismaMock.user.update).toHaveBeenCalledWith({ where: { id: userId }, data: { defaultScheduleId: scheduleId }, }); expect(result).toEqual(updatedUser); }); }); + + describe("findDetailedScheduleById", () => { + const createMockSchedule = (overrides: Partial = {}): Partial => ({ + id: 100, + userId: 2, + name: "Working Hours", + availability: [], + timeZone: "America/New_York", + ...overrides, + }); + + beforeEach(() => { + prismaMock.schedule.count.mockResolvedValue(1); + }); + + it("should allow access when user is the schedule owner", async () => { + const ownerId = 2; + const mockSchedule = createMockSchedule({ userId: ownerId }); + + prismaMock.schedule.findUnique.mockResolvedValue(mockSchedule as Schedule); + mockHasReadPermissions.mockResolvedValue(false); + + const result = await scheduleRepository.findDetailedScheduleById({ + scheduleId: 100, + userId: ownerId, + timeZone: "UTC", + defaultScheduleId: 100, + }); + + expect(result).toMatchObject({ + id: 100, + name: "Working Hours", + userId: ownerId, + }); + }); + + it("should allow access when user is part of the same team", async () => { + const scheduleOwnerId = 2; + const teamMemberId = 3; + const mockSchedule = createMockSchedule({ userId: scheduleOwnerId }); + + prismaMock.schedule.findUnique.mockResolvedValue(mockSchedule as Schedule); + mockHasReadPermissions.mockResolvedValue(true); + + const result = await scheduleRepository.findDetailedScheduleById({ + scheduleId: 100, + userId: teamMemberId, + timeZone: "UTC", + defaultScheduleId: null, + }); + + expect(mockHasReadPermissions).toHaveBeenCalledWith({ + memberId: scheduleOwnerId, + userId: teamMemberId, + }); + expect(result).toMatchObject({ + id: 100, + name: "Working Hours", + userId: scheduleOwnerId, + }); + }); + + it("should deny access when user is not owner and not part of team", async () => { + const scheduleOwnerId = 2; + const unauthorizedUserId = 999; + const mockSchedule = createMockSchedule({ userId: scheduleOwnerId }); + + prismaMock.schedule.findUnique.mockResolvedValue(mockSchedule as Schedule); + mockHasReadPermissions.mockResolvedValue(false); + + await expect( + scheduleRepository.findDetailedScheduleById({ + scheduleId: 100, + userId: unauthorizedUserId, + timeZone: "UTC", + defaultScheduleId: null, + }) + ).rejects.toThrow("UNAUTHORIZED"); + + expect(mockHasReadPermissions).toHaveBeenCalledWith({ + memberId: scheduleOwnerId, + userId: unauthorizedUserId, + }); + }); + + it("should throw error when schedule is not found", async () => { + prismaMock.schedule.findUnique.mockResolvedValue(null); + + await expect( + scheduleRepository.findDetailedScheduleById({ + scheduleId: 999, + userId: 1, + timeZone: "UTC", + defaultScheduleId: null, + }) + ).rejects.toThrow("Schedule not found"); + }); + }); }); diff --git a/packages/features/schedules/repositories/ScheduleRepository.ts b/packages/features/schedules/repositories/ScheduleRepository.ts index 41de2f09af..eee2ca3d67 100644 --- a/packages/features/schedules/repositories/ScheduleRepository.ts +++ b/packages/features/schedules/repositories/ScheduleRepository.ts @@ -112,7 +112,7 @@ export class ScheduleRepository { if (!schedule) { throw new Error("Schedule not found"); } - const isCurrentUserPartOfTeam = hasReadPermissionsForUserId({ memberId: schedule?.userId, userId }); + const isCurrentUserPartOfTeam = await hasReadPermissionsForUserId({ memberId: schedule?.userId, userId }); const isCurrentUserOwner = schedule?.userId === userId; diff --git a/packages/trpc/server/routers/viewer/availability/schedule/getAllSchedulesByUserId.handler.test.ts b/packages/trpc/server/routers/viewer/availability/schedule/getAllSchedulesByUserId.handler.test.ts new file mode 100644 index 0000000000..185aaaba0e --- /dev/null +++ b/packages/trpc/server/routers/viewer/availability/schedule/getAllSchedulesByUserId.handler.test.ts @@ -0,0 +1,158 @@ +import prismaMock from "../../../../../../../tests/libs/__mocks__/prismaMock"; + +import { describe, it, beforeEach, vi, expect } from "vitest"; + +import type { Schedule } from "@calcom/prisma/client"; + +import type { TrpcSessionUser } from "../../../../types"; +import { getAllSchedulesByUserIdHandler } from "./getAllSchedulesByUserId.handler"; + +const DEFAULT_SCHEDULE_ID = 1; + +vi.mock("@calcom/lib/hasEditPermissionForUser", () => ({ + hasReadPermissionsForUserId: vi.fn(), +})); + +vi.mock("@calcom/features/schedules/repositories/ScheduleRepository", () => ({ + ScheduleRepository: vi.fn().mockImplementation(() => ({ + getDefaultScheduleId: vi.fn().mockResolvedValue(DEFAULT_SCHEDULE_ID), + })), +})); + +import { hasReadPermissionsForUserId } from "@calcom/lib/hasEditPermissionForUser"; + +const mockHasReadPermissions = vi.mocked(hasReadPermissionsForUserId); + +describe("getAllSchedulesByUserIdHandler", () => { + const createMockUser = (id: number): NonNullable => + ({ + id, + username: `user-${id}`, + email: `user-${id}@example.com`, + }) as NonNullable; + + const createMockSchedule = (overrides: Partial = {}): Partial => ({ + id: 1, + userId: 10, + name: "Working Hours", + ...overrides, + }); + + beforeEach(() => { + vi.clearAllMocks(); + prismaMock.schedule.findMany.mockReset(); + }); + + describe("authorization", () => { + it("returns schedules when user is the owner", async () => { + const ownerId = 10; + const user = createMockUser(ownerId); + const mockSchedules = [createMockSchedule({ userId: ownerId })]; + + mockHasReadPermissions.mockResolvedValue(false); + prismaMock.schedule.findMany.mockResolvedValue(mockSchedules as Schedule[]); + + const result = await getAllSchedulesByUserIdHandler({ + ctx: { user }, + input: { userId: ownerId }, + }); + + expect(result.schedules).toHaveLength(1); + expect(result.schedules[0]).toMatchObject({ + id: 1, + userId: ownerId, + name: "Working Hours", + readOnly: false, + }); + }); + + it("returns schedules when user is part of the same team", async () => { + const scheduleOwnerId = 10; + const teamMemberId = 20; + const user = createMockUser(teamMemberId); + const mockSchedules = [createMockSchedule({ userId: scheduleOwnerId })]; + + mockHasReadPermissions.mockResolvedValue(true); + prismaMock.schedule.findMany.mockResolvedValue(mockSchedules as Schedule[]); + + const result = await getAllSchedulesByUserIdHandler({ + ctx: { user }, + input: { userId: scheduleOwnerId }, + }); + + expect(mockHasReadPermissions).toHaveBeenCalledWith({ + memberId: scheduleOwnerId, + userId: teamMemberId, + }); + expect(result.schedules).toHaveLength(1); + expect(result.schedules[0].readOnly).toBe(true); + }); + + it("throws UNAUTHORIZED when user is not owner and not part of team", async () => { + const scheduleOwnerId = 10; + const unauthorizedUserId = 999; + const user = createMockUser(unauthorizedUserId); + + mockHasReadPermissions.mockResolvedValue(false); + + await expect( + getAllSchedulesByUserIdHandler({ + ctx: { user }, + input: { userId: scheduleOwnerId }, + }) + ).rejects.toMatchObject({ + code: "UNAUTHORIZED", + }); + + expect(mockHasReadPermissions).toHaveBeenCalledWith({ + memberId: scheduleOwnerId, + userId: unauthorizedUserId, + }); + expect(prismaMock.schedule.findMany).not.toHaveBeenCalled(); + }); + }); + + describe("schedule retrieval", () => { + it("marks exactly one schedule as default", async () => { + const ownerId = 10; + const user = createMockUser(ownerId); + const mockSchedules = [ + createMockSchedule({ id: DEFAULT_SCHEDULE_ID, userId: ownerId }), + createMockSchedule({ id: 2, userId: ownerId, name: "Evening Hours" }), + ]; + + mockHasReadPermissions.mockResolvedValue(false); + prismaMock.schedule.findMany.mockResolvedValue(mockSchedules as Schedule[]); + + const result = await getAllSchedulesByUserIdHandler({ + ctx: { user }, + input: { userId: ownerId }, + }); + + const defaultSchedules = result.schedules.filter((s) => s.isDefault); + expect(defaultSchedules).toHaveLength(1); + expect(defaultSchedules[0].id).toBe(DEFAULT_SCHEDULE_ID); + }); + + it("returns multiple schedules for user", async () => { + const ownerId = 10; + const user = createMockUser(ownerId); + const mockSchedules = [ + createMockSchedule({ id: 1, userId: ownerId, name: "Morning" }), + createMockSchedule({ id: 2, userId: ownerId, name: "Afternoon" }), + createMockSchedule({ id: 3, userId: ownerId, name: "Evening" }), + ]; + + mockHasReadPermissions.mockResolvedValue(false); + prismaMock.schedule.findMany.mockResolvedValue(mockSchedules as Schedule[]); + + const result = await getAllSchedulesByUserIdHandler({ + ctx: { user }, + input: { userId: ownerId }, + }); + + expect(result.schedules).toHaveLength(3); + expect(result.schedules.map((s) => s.name)).toEqual(["Morning", "Afternoon", "Evening"]); + }); + }); +}); diff --git a/packages/trpc/server/routers/viewer/availability/schedule/getAllSchedulesByUserId.handler.ts b/packages/trpc/server/routers/viewer/availability/schedule/getAllSchedulesByUserId.handler.ts index 01e5f251f7..cc77e21ef0 100644 --- a/packages/trpc/server/routers/viewer/availability/schedule/getAllSchedulesByUserId.handler.ts +++ b/packages/trpc/server/routers/viewer/availability/schedule/getAllSchedulesByUserId.handler.ts @@ -20,7 +20,7 @@ type GetOptions = { export const getAllSchedulesByUserIdHandler = async ({ ctx, input }: GetOptions) => { const { user } = ctx; - const isCurrentUserPartOfTeam = hasReadPermissionsForUserId({ memberId: input?.userId, userId: user.id }); + const isCurrentUserPartOfTeam = await hasReadPermissionsForUserId({ memberId: input?.userId, userId: user.id }); const isCurrentUserOwner = input?.userId === user.id;