From 59fca85d927d18ec35e95f2a22c61be192e7c33a Mon Sep 17 00:00:00 2001 From: Syed Ali Shahbaz <52925846+alishaz-polymath@users.noreply.github.com> Date: Thu, 8 Jan 2026 15:30:54 +0400 Subject: [PATCH] fix: use graceful filtering for previously hard failing blocked users in team events (#26446) * init * typefix * -- * fix test * update index * cleanup * rm unnecessary comments * improvements * add tests * test fixes * fixes * fixes * fixes * add try catch to make booking fail-open * Update packages/features/watchlist/operations/check-user-blocking.ts Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> * fix type --------- Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> --- apps/api/v1/test/lib/bookings/_post.test.ts | 7 + .../handleNewBooking/loadAndValidateUsers.ts | 38 +- .../repositories/eventTypeRepository.ts | 3 + .../profile/repositories/ProfileRepository.ts | 16 + .../users/repositories/UserRepository.ts | 1 + .../lib/interface/IBlockingService.ts | 15 + .../lib/interface/IWatchlistRepositories.ts | 19 + .../repository/GlobalWatchlistRepository.ts | 35 ++ .../OrganizationWatchlistRepository.ts | 35 ++ .../lib/service/GlobalBlockingService.ts | 63 ++- .../service/OrganizationBlockingService.ts | 64 ++- .../operations/check-user-blocking.test.ts | 394 ++++++++++++++++++ .../operations/check-user-blocking.ts | 148 +++++++ .../filter-blocked-hosts.controller.test.ts | 229 ++++++++++ .../filter-blocked-hosts.controller.ts | 49 +++ .../filter-blocked-users.controller.test.ts | 370 ++++++++++++++++ .../filter-blocked-users.controller.ts | 56 +++ .../migration.sql | 5 + packages/prisma/schema.prisma | 2 +- .../trpc/server/routers/viewer/slots/util.ts | 61 ++- 20 files changed, 1573 insertions(+), 37 deletions(-) create mode 100644 packages/features/watchlist/operations/check-user-blocking.test.ts create mode 100644 packages/features/watchlist/operations/check-user-blocking.ts create mode 100644 packages/features/watchlist/operations/filter-blocked-hosts.controller.test.ts create mode 100644 packages/features/watchlist/operations/filter-blocked-hosts.controller.ts create mode 100644 packages/features/watchlist/operations/filter-blocked-users.controller.test.ts create mode 100644 packages/features/watchlist/operations/filter-blocked-users.controller.ts create mode 100644 packages/prisma/migrations/20260105071846_update_watchlist_index_with_is_global/migration.sql diff --git a/apps/api/v1/test/lib/bookings/_post.test.ts b/apps/api/v1/test/lib/bookings/_post.test.ts index 4d46a65ac4..1de9042d09 100644 --- a/apps/api/v1/test/lib/bookings/_post.test.ts +++ b/apps/api/v1/test/lib/bookings/_post.test.ts @@ -76,6 +76,13 @@ vi.mock("@calcom/features/watchlist/operations/check-if-users-are-blocked.contro checkIfUsersAreBlocked: vi.fn().mockResolvedValue(false), })); +vi.mock("@calcom/features/watchlist/operations/filter-blocked-users.controller", () => ({ + filterBlockedUsers: vi.fn().mockImplementation(async (users) => ({ + eligibleUsers: users, + blockedCount: 0, + })), +})); + vi.mock("@calcom/features/di/containers/QualifiedHosts", () => ({ getQualifiedHostsService: vi.fn().mockReturnValue({ findQualifiedHostsWithDelegationCredentials: vi.fn().mockResolvedValue({ diff --git a/packages/features/bookings/lib/handleNewBooking/loadAndValidateUsers.ts b/packages/features/bookings/lib/handleNewBooking/loadAndValidateUsers.ts index 6342cfa8bd..30da714f29 100644 --- a/packages/features/bookings/lib/handleNewBooking/loadAndValidateUsers.ts +++ b/packages/features/bookings/lib/handleNewBooking/loadAndValidateUsers.ts @@ -1,22 +1,21 @@ -import type { Logger } from "tslog"; - import { enrichUsersWithDelegationCredentials } from "@calcom/app-store/delegationCredential"; import type { RoutingFormResponse } from "@calcom/features/bookings/lib/getLuckyUser"; import { getQualifiedHostsService } from "@calcom/features/di/containers/QualifiedHosts"; +import { ProfileRepository } from "@calcom/features/profile/repositories/ProfileRepository"; import { withSelectedCalendars } from "@calcom/features/users/repositories/UserRepository"; import { sentrySpan } from "@calcom/features/watchlist/lib/telemetry"; -import { checkIfUsersAreBlocked } from "@calcom/features/watchlist/operations/check-if-users-are-blocked.controller"; +import { filterBlockedUsers } from "@calcom/features/watchlist/operations/filter-blocked-users.controller"; import getOrgIdFromMemberOrTeamId from "@calcom/lib/getOrgIdFromMemberOrTeamId"; import { HttpError } from "@calcom/lib/http-error"; import { getPiiFreeUser } from "@calcom/lib/piiFreeData"; import { safeStringify } from "@calcom/lib/safeStringify"; import { withReporting } from "@calcom/lib/sentryWrapper"; -import { userSelect } from "@calcom/prisma"; -import prisma from "@calcom/prisma"; +import prisma, { userSelect } from "@calcom/prisma"; import type { Prisma } from "@calcom/prisma/client"; import { SchedulingType } from "@calcom/prisma/enums"; import { credentialForCalendarServiceSelect } from "@calcom/prisma/selects/credential"; import type { CredentialForCalendarService } from "@calcom/types/Credential"; +import type { Logger } from "tslog"; import type { NewBookingEventType } from "./getEventTypesFromDB"; import { loadUsers } from "./loadUsers"; @@ -46,6 +45,7 @@ type EventType = Pick< | "schedulingType" | "maxLeadThreshold" | "team" + | "parent" | "assignAllTeamMembers" | "assignRRMembersUsingSegment" | "rrSegmentQueryValue" @@ -133,14 +133,28 @@ const _loadAndValidateUsers = async ({ if (!users) throw new HttpError({ statusCode: 404, message: "eventTypeUser.notFound" }); - // Determine if users are locked - const containsBlockedUser = await checkIfUsersAreBlocked({ - users, - organizationId: null, - span: sentrySpan, - }); + // Get organizationId from eventType (handles org teams and managed events) + let organizationId: number | null = eventType.parent?.team?.parentId ?? eventType.team?.parentId ?? null; - if (containsBlockedUser) throw new HttpError({ statusCode: 404, message: "eventTypeUser.notFound" }); + // Fallback: For personal events, use the user's first org membership for org-specific blocking + // TODO: When we support multiple orgs, revisit the logic + if (!organizationId && eventType.userId) { + organizationId = await ProfileRepository.findFirstOrganizationIdForUser({ userId: eventType.userId }); + } + + const { eligibleUsers, blockedCount } = await filterBlockedUsers(users, organizationId, sentrySpan); + + if (blockedCount > 0) { + logger.info(`Filtered out ${blockedCount} blocked user(s) from booking`); + } + + // If all users are blocked, throw 404 + // For team events with some eligible users, continue with graceful degradation + if (eligibleUsers.length === 0) { + throw new HttpError({ statusCode: 404, message: "eventTypeUser.notFound" }); + } + + users = eligibleUsers; // map fixed users users = users.map((user) => ({ diff --git a/packages/features/eventtypes/repositories/eventTypeRepository.ts b/packages/features/eventtypes/repositories/eventTypeRepository.ts index 2e1da121c0..a4b5413fcd 100644 --- a/packages/features/eventtypes/repositories/eventTypeRepository.ts +++ b/packages/features/eventtypes/repositories/eventTypeRepository.ts @@ -1325,6 +1325,7 @@ export class EventTypeRepository { team: { select: { id: true, + parentId: true, bookingLimits: true, includeManagedEventsInLimits: true, }, @@ -1362,6 +1363,7 @@ export class EventTypeRepository { groupId: true, user: { select: { + locked: true, credentials: { select: credentialForCalendarServiceSelect }, ...availabilityUserSelect, }, @@ -1384,6 +1386,7 @@ export class EventTypeRepository { }, users: { select: { + locked: true, credentials: { select: credentialForCalendarServiceSelect }, ...availabilityUserSelect, }, diff --git a/packages/features/profile/repositories/ProfileRepository.ts b/packages/features/profile/repositories/ProfileRepository.ts index 43c7bc0470..5a7e71d957 100644 --- a/packages/features/profile/repositories/ProfileRepository.ts +++ b/packages/features/profile/repositories/ProfileRepository.ts @@ -911,6 +911,22 @@ export class ProfileRepository { }); } + /** + * Returns the first organization ID the user belongs to, or null if none. + * Used for org-specific blocking on personal events. + * + * TODO: When we support checking against multiple orgs, update this to return + * all org IDs and check if user is blocked in ANY of them. + */ + static async findFirstOrganizationIdForUser({ userId }: { userId: number }): Promise { + const profile = await prisma.profile.findFirst({ + where: { userId }, + select: { organizationId: true }, + }); + + return profile?.organizationId ?? null; + } + static async findManyForOrg({ organizationId }: { organizationId: number }) { return await prisma.profile.findMany({ where: { diff --git a/packages/features/users/repositories/UserRepository.ts b/packages/features/users/repositories/UserRepository.ts index 6af638fa99..1fad91aaea 100644 --- a/packages/features/users/repositories/UserRepository.ts +++ b/packages/features/users/repositories/UserRepository.ts @@ -1178,6 +1178,7 @@ export class UserRepository { return this.prismaClient.user.findMany({ where, select: { + locked: true, allowDynamicBooking: true, ...availabilityUserSelect, credentials: { diff --git a/packages/features/watchlist/lib/interface/IBlockingService.ts b/packages/features/watchlist/lib/interface/IBlockingService.ts index dabf67ab96..1634c2dcb7 100644 --- a/packages/features/watchlist/lib/interface/IBlockingService.ts +++ b/packages/features/watchlist/lib/interface/IBlockingService.ts @@ -6,6 +6,21 @@ export interface BlockingResult { watchlistEntry?: Record | null; } +/** + * Result of bulk blocking check. + * Maps email (lowercase) -> BlockingResult + */ +export type BulkBlockingResult = Map; + export interface IBlockingService { + /** + * Check if a single email is blocked. + */ isBlocked(email: string, organizationId?: number | null): Promise; + + /** + * Bulk check multiple emails in a single query. + * Returns Map for efficient lookup. + */ + areBlocked(emails: string[], organizationId?: number | null): Promise; } diff --git a/packages/features/watchlist/lib/interface/IWatchlistRepositories.ts b/packages/features/watchlist/lib/interface/IWatchlistRepositories.ts index b2baccfddb..d1cc304338 100644 --- a/packages/features/watchlist/lib/interface/IWatchlistRepositories.ts +++ b/packages/features/watchlist/lib/interface/IWatchlistRepositories.ts @@ -13,6 +13,15 @@ export interface IGlobalWatchlistRepository { findById(id: string): Promise; listBlockedEntries(): Promise; + /** + * Bulk find blocking entries for multiple emails and domains. + * Single query for N emails and domains - eliminates N+1. + */ + findBlockingEntriesForEmailsAndDomains(params: { + emails: string[]; + domains: string[]; + }): Promise; + // Write operations createEntry(data: { type: WatchlistType; @@ -52,6 +61,16 @@ export interface IOrganizationWatchlistRepository { listBlockedEntries(organizationId: number): Promise; listAllOrganizationEntries(): Promise; + /** + * Bulk find blocking entries for multiple emails and domains within an organization. + * Single query for N emails and domains - eliminates N+1. + */ + findBlockingEntriesForEmailsAndDomains(params: { + emails: string[]; + domains: string[]; + organizationId: number; + }): Promise; + // Write operations createEntry( organizationId: number, diff --git a/packages/features/watchlist/lib/repository/GlobalWatchlistRepository.ts b/packages/features/watchlist/lib/repository/GlobalWatchlistRepository.ts index a6c64343f9..2a73c72e2a 100644 --- a/packages/features/watchlist/lib/repository/GlobalWatchlistRepository.ts +++ b/packages/features/watchlist/lib/repository/GlobalWatchlistRepository.ts @@ -85,6 +85,41 @@ export class GlobalWatchlistRepository implements IGlobalWatchlistRepository { }); } + /** + * Bulk find blocking entries for multiple emails and domains. + * Single query for N emails and domains - eliminates N+1. + */ + async findBlockingEntriesForEmailsAndDomains(params: { + emails: string[]; + domains: string[]; + }): Promise { + const { emails, domains } = params; + + if (emails.length === 0 && domains.length === 0) { + return []; + } + + const conditions: Array<{ type: WatchlistType; value: { in: string[] } }> = []; + + if (emails.length > 0) { + conditions.push({ type: WatchlistType.EMAIL, value: { in: emails } }); + } + + if (domains.length > 0) { + conditions.push({ type: WatchlistType.DOMAIN, value: { in: domains } }); + } + + return this.prisma.watchlist.findMany({ + select: this.selectFields, + where: { + organizationId: null, + isGlobal: true, + action: WatchlistAction.BLOCK, + OR: conditions, + }, + }); + } + // Write operations for global entries async createEntry(data: { type: WatchlistType; diff --git a/packages/features/watchlist/lib/repository/OrganizationWatchlistRepository.ts b/packages/features/watchlist/lib/repository/OrganizationWatchlistRepository.ts index 43cd6add53..8ec2c70c3d 100644 --- a/packages/features/watchlist/lib/repository/OrganizationWatchlistRepository.ts +++ b/packages/features/watchlist/lib/repository/OrganizationWatchlistRepository.ts @@ -76,6 +76,41 @@ export class OrganizationWatchlistRepository implements IOrganizationWatchlistRe }); } + /** + * Batch find blocking entries for multiple emails and domains within an organization. + * Single query for N users - eliminates N+1. + */ + async findBlockingEntriesForEmailsAndDomains(params: { + emails: string[]; + domains: string[]; + organizationId: number; + }): Promise { + const { emails, domains, organizationId } = params; + + if (emails.length === 0 && domains.length === 0) { + return []; + } + + const conditions: Array<{ type: WatchlistType; value: { in: string[] } }> = []; + + if (emails.length > 0) { + conditions.push({ type: WatchlistType.EMAIL, value: { in: emails } }); + } + + if (domains.length > 0) { + conditions.push({ type: WatchlistType.DOMAIN, value: { in: domains } }); + } + + return this.prisma.watchlist.findMany({ + select: this.selectFields, + where: { + organizationId, + action: WatchlistAction.BLOCK, + OR: conditions, + }, + }); + } + async findById(id: string, organizationId: number): Promise { return this.prisma.watchlist.findFirst({ select: this.selectFields, diff --git a/packages/features/watchlist/lib/service/GlobalBlockingService.ts b/packages/features/watchlist/lib/service/GlobalBlockingService.ts index 13ba8f28d6..0a9924e02c 100644 --- a/packages/features/watchlist/lib/service/GlobalBlockingService.ts +++ b/packages/features/watchlist/lib/service/GlobalBlockingService.ts @@ -1,7 +1,7 @@ -import type { IBlockingService, BlockingResult } from "../interface/IBlockingService"; +import type { BulkBlockingResult, BlockingResult, IBlockingService } from "../interface/IBlockingService"; import type { IGlobalWatchlistRepository } from "../interface/IWatchlistRepositories"; import { WatchlistType } from "../types"; -import { normalizeEmail, extractDomainFromEmail, normalizeDomain } from "../utils/normalization"; +import { extractDomainFromEmail, normalizeDomain, normalizeEmail } from "../utils/normalization"; type Deps = { globalRepo: IGlobalWatchlistRepository; @@ -42,6 +42,65 @@ export class GlobalBlockingService implements IBlockingService { return { isBlocked: false }; } + /** + * Bulk check multiple emails in a single query. + * Returns Map for efficient lookup. + */ + async areBlocked(emails: string[]): Promise { + const result: BulkBlockingResult = new Map(); + + if (emails.length === 0) { + return result; + } + + // Normalize and extract domains + const normalizedEmails = emails.map((e) => normalizeEmail(e)); + const uniqueDomains = Array.from(new Set(normalizedEmails.map((e) => extractDomainFromEmail(e)))); + + // Single DB query for all emails and domains + const blockingEntries = await this.deps.globalRepo.findBlockingEntriesForEmailsAndDomains({ + emails: normalizedEmails, + domains: uniqueDomains, + }); + + // Build lookup maps for O(1) checks (Maps serve as both lookup and entry storage) + const emailEntries = new Map(); + const domainEntries = new Map(); + + for (const entry of blockingEntries) { + if (entry.type === WatchlistType.EMAIL) { + emailEntries.set(entry.value.toLowerCase(), entry); + } else if (entry.type === WatchlistType.DOMAIN) { + domainEntries.set(entry.value.toLowerCase(), entry); + } + } + + // Map results for each email + for (const email of normalizedEmails) { + const domain = extractDomainFromEmail(email); + const emailEntry = emailEntries.get(email); + const domainEntry = domainEntries.get(domain); + + if (emailEntry) { + result.set(email, { + isBlocked: true, + reason: WatchlistType.EMAIL, + watchlistEntry: emailEntry, + }); + } else if (domainEntry) { + result.set(email, { + isBlocked: true, + reason: WatchlistType.DOMAIN, + watchlistEntry: domainEntry, + }); + } else { + result.set(email, { isBlocked: false }); + } + } + + return result; + } + async isFreeEmailDomain(domain: string): Promise { const normalizedDomain = normalizeDomain(domain); return this.deps.globalRepo.findFreeEmailDomain(normalizedDomain).then((entry) => !!entry); diff --git a/packages/features/watchlist/lib/service/OrganizationBlockingService.ts b/packages/features/watchlist/lib/service/OrganizationBlockingService.ts index 2321aab4f9..6f8fa64dcf 100644 --- a/packages/features/watchlist/lib/service/OrganizationBlockingService.ts +++ b/packages/features/watchlist/lib/service/OrganizationBlockingService.ts @@ -1,7 +1,7 @@ -import type { IBlockingService, BlockingResult } from "../interface/IBlockingService"; +import type { BulkBlockingResult, BlockingResult, IBlockingService } from "../interface/IBlockingService"; import type { IOrganizationWatchlistRepository } from "../interface/IWatchlistRepositories"; import { WatchlistType } from "../types"; -import { normalizeEmail, extractDomainFromEmail, normalizeDomain } from "../utils/normalization"; +import { extractDomainFromEmail, normalizeDomain, normalizeEmail } from "../utils/normalization"; type Deps = { orgRepo: IOrganizationWatchlistRepository; @@ -42,6 +42,66 @@ export class OrganizationBlockingService implements IBlockingService { return { isBlocked: false }; } + /** + * Bulk check multiple emails in a single query for an organization. + * Returns Map for efficient lookup. + */ + async areBlocked(emails: string[], organizationId: number): Promise { + const result: BulkBlockingResult = new Map(); + + if (emails.length === 0 || !organizationId) { + return result; + } + + // Normalize and extract domains + const normalizedEmails = emails.map((e) => normalizeEmail(e)); + const uniqueDomains = Array.from(new Set(normalizedEmails.map((e) => extractDomainFromEmail(e)))); + + // Single DB query for all emails and domains + const blockingEntries = await this.deps.orgRepo.findBlockingEntriesForEmailsAndDomains({ + emails: normalizedEmails, + domains: uniqueDomains, + organizationId, + }); + + // Build lookup maps for O(1) checks (Maps serve as both lookup and entry storage) + const emailEntries = new Map(); + const domainEntries = new Map(); + + for (const entry of blockingEntries) { + if (entry.type === WatchlistType.EMAIL) { + emailEntries.set(entry.value.toLowerCase(), entry); + } else if (entry.type === WatchlistType.DOMAIN) { + domainEntries.set(entry.value.toLowerCase(), entry); + } + } + + // Map results for each email + for (const email of normalizedEmails) { + const domain = extractDomainFromEmail(email); + const emailEntry = emailEntries.get(email); + const domainEntry = domainEntries.get(domain); + + if (emailEntry) { + result.set(email, { + isBlocked: true, + reason: WatchlistType.EMAIL, + watchlistEntry: emailEntry, + }); + } else if (domainEntry) { + result.set(email, { + isBlocked: true, + reason: WatchlistType.DOMAIN, + watchlistEntry: domainEntry, + }); + } else { + result.set(email, { isBlocked: false }); + } + } + + return result; + } + async isDomainBlocked(domain: string, organizationId: number): Promise { const normalizedDomain = normalizeDomain(domain); diff --git a/packages/features/watchlist/operations/check-user-blocking.test.ts b/packages/features/watchlist/operations/check-user-blocking.test.ts new file mode 100644 index 0000000000..5f1aa4a04e --- /dev/null +++ b/packages/features/watchlist/operations/check-user-blocking.test.ts @@ -0,0 +1,394 @@ +import { describe, test, expect, vi, beforeEach } from "vitest"; + +import { + getBlockedUsersMap, + isUserBlocked, + checkWatchlistBlocking, + type BlockableUser, + type BlockingInfo, +} from "./check-user-blocking"; + + +vi.mock("@calcom/features/di/watchlist/containers/watchlist", () => ({ + getWatchlistFeature: vi.fn(), +})); + +const mockGlobalBlocking: { areBlocked: ReturnType } = { + areBlocked: vi.fn(), +}; + +const mockOrgBlocking: { areBlocked: ReturnType } = { + areBlocked: vi.fn(), +}; + +const mockWatchlistFeature: { + globalBlocking: typeof mockGlobalBlocking; + orgBlocking: typeof mockOrgBlocking; + watchlist: Record; + audit: Record; +} = { + globalBlocking: mockGlobalBlocking, + orgBlocking: mockOrgBlocking, + watchlist: {}, + audit: {}, +}; + +describe("check-user-blocking", () => { + beforeEach(async () => { + vi.clearAllMocks(); + const { getWatchlistFeature } = await import("@calcom/features/di/watchlist/containers/watchlist"); + vi.mocked(getWatchlistFeature).mockResolvedValue(mockWatchlistFeature as never); + }); + + describe("getBlockedUsersMap", () => { + describe("Empty input handling", () => { + test("should return empty result for empty users array", async () => { + const result = await getBlockedUsersMap([]); + + expect(result).toEqual({ + blockingMap: new Map(), + blockedCount: 0, + lockedCount: 0, + watchlistBlockedCount: 0, + }); + expect(mockGlobalBlocking.areBlocked).not.toHaveBeenCalled(); + }); + + test("should skip users with empty emails", async () => { + mockGlobalBlocking.areBlocked.mockResolvedValue(new Map([["valid@example.com", { isBlocked: false }]])); + + const users: BlockableUser[] = [ + { email: "", locked: false }, + { email: " ", locked: false }, + { email: "valid@example.com", locked: false }, + ]; + + const result = await getBlockedUsersMap(users); + + expect(result.blockedCount).toBe(0); + expect(mockGlobalBlocking.areBlocked).toHaveBeenCalledWith(["valid@example.com"]); + }); + + test("should return empty result if all users have empty emails", async () => { + const users: BlockableUser[] = [ + { email: "", locked: false }, + { email: " ", locked: false }, + ]; + + const result = await getBlockedUsersMap(users); + + expect(result).toEqual({ + blockingMap: new Map(), + blockedCount: 0, + lockedCount: 0, + watchlistBlockedCount: 0, + }); + expect(mockGlobalBlocking.areBlocked).not.toHaveBeenCalled(); + }); + }); + + describe("Locked user blocking", () => { + test("should block locked users without calling watchlist", async () => { + const users: BlockableUser[] = [ + { email: "locked@example.com", locked: true }, + { email: "unlocked@example.com", locked: false }, + ]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["unlocked@example.com", { isBlocked: false }]]) + ); + + const result = await getBlockedUsersMap(users); + + expect(result.blockedCount).toBe(1); + expect(result.lockedCount).toBe(1); + expect(result.watchlistBlockedCount).toBe(0); + + const lockedInfo = result.blockingMap.get("locked@example.com"); + expect(lockedInfo).toEqual({ isBlocked: true, reason: "locked" }); + + // Should only check unlocked user in watchlist + expect(mockGlobalBlocking.areBlocked).toHaveBeenCalledWith(["unlocked@example.com"]); + }); + + test("should not call watchlist if all users are locked", async () => { + const users: BlockableUser[] = [ + { email: "locked1@example.com", locked: true }, + { email: "locked2@example.com", locked: true }, + ]; + + const result = await getBlockedUsersMap(users); + + expect(result.blockedCount).toBe(2); + expect(result.lockedCount).toBe(2); + expect(result.watchlistBlockedCount).toBe(0); + expect(mockGlobalBlocking.areBlocked).not.toHaveBeenCalled(); + }); + + test("should correctly count multiple locked users", async () => { + const users: BlockableUser[] = [ + { email: "locked1@example.com", locked: true }, + { email: "locked2@example.com", locked: true }, + { email: "locked3@example.com", locked: true }, + ]; + + const result = await getBlockedUsersMap(users); + + expect(result.lockedCount).toBe(3); + expect(result.blockedCount).toBe(3); + }); + }); + + describe("Watchlist blocking", () => { + test("should block users on global watchlist", async () => { + const users: BlockableUser[] = [ + { email: "blocked@spam.com", locked: false }, + { email: "clean@example.com", locked: false }, + ]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([ + ["blocked@spam.com", { isBlocked: true }], + ["clean@example.com", { isBlocked: false }], + ]) + ); + + const result = await getBlockedUsersMap(users); + + expect(result.blockedCount).toBe(1); + expect(result.lockedCount).toBe(0); + expect(result.watchlistBlockedCount).toBe(1); + + const blockedInfo = result.blockingMap.get("blocked@spam.com"); + expect(blockedInfo).toEqual({ isBlocked: true, reason: "watchlist" }); + + const cleanInfo = result.blockingMap.get("clean@example.com"); + expect(cleanInfo).toEqual({ isBlocked: false }); + }); + + test("should check org-level blocking when organizationId is provided", async () => { + const users: BlockableUser[] = [{ email: "user@example.com", locked: false }]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["user@example.com", { isBlocked: false }]]) + ); + mockOrgBlocking.areBlocked.mockResolvedValue(new Map([["user@example.com", { isBlocked: true }]])); + + const result = await getBlockedUsersMap(users, 123); + + expect(result.blockedCount).toBe(1); + expect(result.watchlistBlockedCount).toBe(1); + expect(mockOrgBlocking.areBlocked).toHaveBeenCalledWith(["user@example.com"], 123); + }); + + test("should not check org-level blocking when organizationId is not provided", async () => { + const users: BlockableUser[] = [{ email: "user@example.com", locked: false }]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["user@example.com", { isBlocked: false }]]) + ); + + await getBlockedUsersMap(users); + + expect(mockOrgBlocking.areBlocked).not.toHaveBeenCalled(); + }); + + test("should not check org-level blocking when organizationId is null", async () => { + const users: BlockableUser[] = [{ email: "user@example.com", locked: false }]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["user@example.com", { isBlocked: false }]]) + ); + + await getBlockedUsersMap(users, null); + + expect(mockOrgBlocking.areBlocked).not.toHaveBeenCalled(); + }); + }); + + describe("Combined blocking (locked + watchlist)", () => { + test("should correctly count both locked and watchlist-blocked users", async () => { + const users: BlockableUser[] = [ + { email: "locked@example.com", locked: true }, + { email: "watchlist-blocked@spam.com", locked: false }, + { email: "clean@example.com", locked: false }, + ]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([ + ["watchlist-blocked@spam.com", { isBlocked: true }], + ["clean@example.com", { isBlocked: false }], + ]) + ); + + const result = await getBlockedUsersMap(users); + + expect(result.blockedCount).toBe(2); + expect(result.lockedCount).toBe(1); + expect(result.watchlistBlockedCount).toBe(1); + }); + + test("should prioritize locked status over watchlist (locked users not checked in watchlist)", async () => { + const users: BlockableUser[] = [ + { email: "locked-and-would-be-blocked@spam.com", locked: true }, + { email: "clean@example.com", locked: false }, + ]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["clean@example.com", { isBlocked: false }]]) + ); + + const result = await getBlockedUsersMap(users); + + // The locked user's email should NOT be sent to watchlist check + expect(mockGlobalBlocking.areBlocked).toHaveBeenCalledWith(["clean@example.com"]); + expect(mockGlobalBlocking.areBlocked).not.toHaveBeenCalledWith( + expect.arrayContaining(["locked-and-would-be-blocked@spam.com"]) + ); + + // But the locked user should still be marked as blocked + const lockedInfo = result.blockingMap.get("locked-and-would-be-blocked@spam.com"); + expect(lockedInfo).toEqual({ isBlocked: true, reason: "locked" }); + }); + }); + + describe("Email normalization", () => { + test("should normalize emails to lowercase", async () => { + const users: BlockableUser[] = [{ email: "USER@EXAMPLE.COM", locked: false }]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["user@example.com", { isBlocked: false }]]) + ); + + const result = await getBlockedUsersMap(users); + + expect(result.blockingMap.has("user@example.com")).toBe(true); + expect(result.blockingMap.has("USER@EXAMPLE.COM")).toBe(false); + }); + + test("should trim whitespace from emails", async () => { + const users: BlockableUser[] = [{ email: " user@example.com ", locked: false }]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["user@example.com", { isBlocked: false }]]) + ); + + const result = await getBlockedUsersMap(users); + + expect(result.blockingMap.has("user@example.com")).toBe(true); + }); + }); + }); + + describe("isUserBlocked", () => { + test("should return true for blocked user", () => { + const blockingMap = new Map([ + ["blocked@example.com", { isBlocked: true, reason: "locked" }], + ]); + + expect(isUserBlocked("blocked@example.com", blockingMap)).toBe(true); + }); + + test("should return false for non-blocked user", () => { + const blockingMap = new Map([ + ["clean@example.com", { isBlocked: false }], + ]); + + expect(isUserBlocked("clean@example.com", blockingMap)).toBe(false); + }); + + test("should return false for user not in map", () => { + const blockingMap = new Map(); + + expect(isUserBlocked("unknown@example.com", blockingMap)).toBe(false); + }); + + test("should return false for empty email", () => { + const blockingMap = new Map([ + ["blocked@example.com", { isBlocked: true, reason: "locked" }], + ]); + + expect(isUserBlocked("", blockingMap)).toBe(false); + expect(isUserBlocked(" ", blockingMap)).toBe(false); + }); + + test("should normalize email before lookup", () => { + const blockingMap = new Map([ + ["blocked@example.com", { isBlocked: true, reason: "locked" }], + ]); + + expect(isUserBlocked("BLOCKED@EXAMPLE.COM", blockingMap)).toBe(true); + expect(isUserBlocked(" blocked@example.com ", blockingMap)).toBe(true); + }); + }); + + describe("checkWatchlistBlocking", () => { + test("should batch check all emails against global watchlist", async () => { + const emails = ["user1@example.com", "user2@example.com"]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([ + ["user1@example.com", { isBlocked: false }], + ["user2@example.com", { isBlocked: true }], + ]) + ); + + const result = await checkWatchlistBlocking(emails); + + expect(mockGlobalBlocking.areBlocked).toHaveBeenCalledWith(emails); + expect(result.get("user1@example.com")).toBe(false); + expect(result.get("user2@example.com")).toBe(true); + }); + + test("should combine global and org blocking results", async () => { + const emails = ["user@example.com"]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["user@example.com", { isBlocked: false }]]) + ); + mockOrgBlocking.areBlocked.mockResolvedValue(new Map([["user@example.com", { isBlocked: true }]])); + + const result = await checkWatchlistBlocking(emails, 123); + + expect(result.get("user@example.com")).toBe(true); + }); + + test("should block if either global or org blocks", async () => { + const emails = ["global-blocked@example.com", "org-blocked@example.com", "clean@example.com"]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([ + ["global-blocked@example.com", { isBlocked: true }], + ["org-blocked@example.com", { isBlocked: false }], + ["clean@example.com", { isBlocked: false }], + ]) + ); + mockOrgBlocking.areBlocked.mockResolvedValue( + new Map([ + ["global-blocked@example.com", { isBlocked: false }], + ["org-blocked@example.com", { isBlocked: true }], + ["clean@example.com", { isBlocked: false }], + ]) + ); + + const result = await checkWatchlistBlocking(emails, 123); + + expect(result.get("global-blocked@example.com")).toBe(true); + expect(result.get("org-blocked@example.com")).toBe(true); + expect(result.get("clean@example.com")).toBe(false); + }); + + test("should normalize emails in result map", async () => { + const emails = ["USER@EXAMPLE.COM"]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["user@example.com", { isBlocked: true }]]) + ); + + const result = await checkWatchlistBlocking(emails); + + expect(result.get("user@example.com")).toBe(true); + }); + }); +}); + diff --git a/packages/features/watchlist/operations/check-user-blocking.ts b/packages/features/watchlist/operations/check-user-blocking.ts new file mode 100644 index 0000000000..d65db02dbb --- /dev/null +++ b/packages/features/watchlist/operations/check-user-blocking.ts @@ -0,0 +1,148 @@ +import { getWatchlistFeature } from "@calcom/features/di/watchlist/containers/watchlist"; +import logger from "@calcom/lib/logger"; + +const log: ReturnType = logger.getSubLogger({ + prefix: ["watchlist", "check-user-blocking"], +}); + +/** + * Minimal user shape required for blocking checks. + * Users must have email and locked status. + */ +export interface BlockableUser { + email: string; + locked: boolean; +} + +export interface BlockingInfo { + isBlocked: boolean; + reason?: "locked" | "watchlist"; +} + +export interface CheckUserBlockingResult { + /** Map of normalized email -> blocking info */ + blockingMap: Map; + /** Count of blocked users */ + blockedCount: number; + /** Count of locked users */ + lockedCount: number; + /** Count of watchlist-blocked users */ + watchlistBlockedCount: number; +} + +export async function checkWatchlistBlocking( + emails: string[], + organizationId?: number | null +): Promise> { + try { + const watchlist = await getWatchlistFeature(); + + // Batch check - single DB query for all emails + const globalBlockedMap = await watchlist.globalBlocking.areBlocked(emails); + + let orgBlockedMap: Map | null = null; + if (organizationId) { + orgBlockedMap = await watchlist.orgBlocking.areBlocked(emails, organizationId); + } + + const result = new Map(); + for (const email of emails) { + const normalizedEmail = email.trim().toLowerCase(); + const globalResult = globalBlockedMap.get(normalizedEmail); + const orgResult = orgBlockedMap?.get(normalizedEmail); + + result.set(normalizedEmail, !!(globalResult?.isBlocked || orgResult?.isBlocked)); + } + + return result; + } catch (error) { + // Fail-open: If watchlist check fails, allow booking to proceed + // This ensures availability even if watchlist service is down + let errorMessage = "Unknown error"; + if (error instanceof Error) { + errorMessage = error.message; + } + log.error("Watchlist check failed, allowing users through (fail-open)", { + error: errorMessage, + emailCount: emails.length, + organizationId, + }); + return new Map(emails.map((e) => [e.trim().toLowerCase(), false])); + } +} + +/** + * Core utility to check which users are blocked. + * Checks both: + * 1. Locked users (from user.locked field) + * 2. Watchlist-blocked emails (global and org-level) + * + * Single DB query for N emails - eliminates N+1. + * + * @param users - List of users to check (must have email and locked fields) + * @param organizationId - Optional organization ID for org-specific blocking + * @returns Map of email -> blocking info and counts + */ +export async function getBlockedUsersMap( + users: T[], + organizationId?: number | null +): Promise { + // Filter out users with empty/invalid emails + const validUsers = users.filter((u) => u.email && u.email.trim().length > 0); + + if (validUsers.length === 0) { + return { blockingMap: new Map(), blockedCount: 0, lockedCount: 0, watchlistBlockedCount: 0 }; + } + + const blockingMap = new Map(); + let lockedCount = 0; + + // First pass: check locked users (immediate block, no DB needed) + const nonLockedUsers: T[] = []; + for (const user of validUsers) { + const normalizedEmail = user.email.trim().toLowerCase(); + + if (user.locked) { + blockingMap.set(normalizedEmail, { isBlocked: true, reason: "locked" }); + lockedCount++; + } else { + nonLockedUsers.push(user); + } + } + + // Second pass: check watchlist for non-locked users + let watchlistBlockedCount = 0; + if (nonLockedUsers.length > 0) { + const emails = nonLockedUsers.map((u) => u.email); + const watchlistResults = await checkWatchlistBlocking(emails, organizationId); + + for (const user of nonLockedUsers) { + const normalizedEmail = user.email.trim().toLowerCase(); + const isWatchlistBlocked = watchlistResults.get(normalizedEmail) ?? false; + + if (isWatchlistBlocked) { + blockingMap.set(normalizedEmail, { isBlocked: true, reason: "watchlist" }); + watchlistBlockedCount++; + } else { + blockingMap.set(normalizedEmail, { isBlocked: false }); + } + } + } + + return { + blockingMap, + blockedCount: lockedCount + watchlistBlockedCount, + lockedCount, + watchlistBlockedCount, + }; +} + +/** + * Check if a single user is blocked (convenience wrapper). + */ +export function isUserBlocked(email: string, blockingMap: Map): boolean { + if (!email || email.trim().length === 0) { + return false; // Empty emails can't be blocked + } + return blockingMap.get(email.trim().toLowerCase())?.isBlocked ?? false; +} diff --git a/packages/features/watchlist/operations/filter-blocked-hosts.controller.test.ts b/packages/features/watchlist/operations/filter-blocked-hosts.controller.test.ts new file mode 100644 index 0000000000..555bcf9f6e --- /dev/null +++ b/packages/features/watchlist/operations/filter-blocked-hosts.controller.test.ts @@ -0,0 +1,229 @@ +import { describe, test, expect, vi, beforeEach } from "vitest"; + +import { filterBlockedHosts, type HostWithEmail } from "./filter-blocked-hosts.controller"; + + +vi.mock("@calcom/features/di/watchlist/containers/watchlist", () => ({ + getWatchlistFeature: vi.fn(), +})); + +const mockGlobalBlocking: { areBlocked: ReturnType } = { + areBlocked: vi.fn(), +}; + +const mockOrgBlocking: { areBlocked: ReturnType } = { + areBlocked: vi.fn(), +}; + +const mockWatchlistFeature: { + globalBlocking: typeof mockGlobalBlocking; + orgBlocking: typeof mockOrgBlocking; + watchlist: Record; + audit: Record; +} = { + globalBlocking: mockGlobalBlocking, + orgBlocking: mockOrgBlocking, + watchlist: {}, + audit: {}, +}; + +// Helper to create host objects +function createHost(id: number, email: string, locked: boolean): HostWithEmail { + return { + user: { id, email, locked }, + }; +} + +describe("filterBlockedHosts", () => { + beforeEach(async () => { + vi.clearAllMocks(); + const { getWatchlistFeature } = await import("@calcom/features/di/watchlist/containers/watchlist"); + vi.mocked(getWatchlistFeature).mockResolvedValue(mockWatchlistFeature as never); + }); + + describe("Empty input handling", () => { + test("should return empty array for empty hosts array", async () => { + const result = await filterBlockedHosts([]); + + expect(result).toEqual({ eligibleHosts: [], blockedCount: 0 }); + expect(mockGlobalBlocking.areBlocked).not.toHaveBeenCalled(); + }); + }); + + describe("Locked host filtering", () => { + test("should filter out locked hosts", async () => { + const hosts = [ + createHost(1, "locked@example.com", true), + createHost(2, "unlocked@example.com", false), + ]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["unlocked@example.com", { isBlocked: false }]]) + ); + + const result = await filterBlockedHosts(hosts); + + expect(result.eligibleHosts).toHaveLength(1); + expect(result.eligibleHosts[0].user.id).toBe(2); + expect(result.blockedCount).toBe(1); + }); + + test("should filter out all locked hosts", async () => { + const hosts = [ + createHost(1, "locked1@example.com", true), + createHost(2, "locked2@example.com", true), + ]; + + const result = await filterBlockedHosts(hosts); + + expect(result.eligibleHosts).toHaveLength(0); + expect(result.blockedCount).toBe(2); + expect(mockGlobalBlocking.areBlocked).not.toHaveBeenCalled(); + }); + }); + + describe("Watchlist blocking", () => { + test("should filter out watchlist-blocked hosts", async () => { + const hosts = [ + createHost(1, "blocked@spam.com", false), + createHost(2, "clean@example.com", false), + ]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([ + ["blocked@spam.com", { isBlocked: true }], + ["clean@example.com", { isBlocked: false }], + ]) + ); + + const result = await filterBlockedHosts(hosts); + + expect(result.eligibleHosts).toHaveLength(1); + expect(result.eligibleHosts[0].user.id).toBe(2); + expect(result.blockedCount).toBe(1); + }); + + test("should check org-level blocking when organizationId provided", async () => { + const hosts = [createHost(1, "user@example.com", false)]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["user@example.com", { isBlocked: false }]]) + ); + mockOrgBlocking.areBlocked.mockResolvedValue(new Map([["user@example.com", { isBlocked: true }]])); + + const result = await filterBlockedHosts(hosts, 123); + + expect(result.eligibleHosts).toHaveLength(0); + expect(result.blockedCount).toBe(1); + expect(mockOrgBlocking.areBlocked).toHaveBeenCalledWith(["user@example.com"], 123); + }); + }); + + describe("Combined blocking (locked + watchlist)", () => { + test("should filter out both locked and watchlist-blocked hosts", async () => { + const hosts = [ + createHost(1, "locked@example.com", true), + createHost(2, "watchlist-blocked@spam.com", false), + createHost(3, "clean@example.com", false), + ]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([ + ["watchlist-blocked@spam.com", { isBlocked: true }], + ["clean@example.com", { isBlocked: false }], + ]) + ); + + const result = await filterBlockedHosts(hosts); + + expect(result.eligibleHosts).toHaveLength(1); + expect(result.eligibleHosts[0].user.id).toBe(3); + expect(result.blockedCount).toBe(2); + }); + }); + + describe("Preserves host data", () => { + test("should preserve all host properties in returned eligible hosts", async () => { + interface ExtendedHost extends HostWithEmail { + isFixed: boolean; + priority: number; + } + + const hosts: ExtendedHost[] = [ + { + user: { id: 1, email: "host@example.com", locked: false }, + isFixed: true, + priority: 10, + }, + ]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["host@example.com", { isBlocked: false }]]) + ); + + const result = await filterBlockedHosts(hosts); + + expect(result.eligibleHosts).toHaveLength(1); + expect(result.eligibleHosts[0]).toEqual({ + user: { id: 1, email: "host@example.com", locked: false }, + isFixed: true, + priority: 10, + }); + }); + }); + + describe("Email normalization", () => { + test("should handle emails with different cases", async () => { + const hosts = [createHost(1, "USER@EXAMPLE.COM", false)]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["user@example.com", { isBlocked: true }]]) + ); + + const result = await filterBlockedHosts(hosts); + + expect(result.eligibleHosts).toHaveLength(0); + expect(result.blockedCount).toBe(1); + }); + }); + + describe("Performance: batched checks", () => { + test("should make single batch call for multiple hosts", async () => { + const hosts = [ + createHost(1, "user1@example.com", false), + createHost(2, "user2@example.com", false), + createHost(3, "user3@example.com", false), + ]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([ + ["user1@example.com", { isBlocked: false }], + ["user2@example.com", { isBlocked: false }], + ["user3@example.com", { isBlocked: false }], + ]) + ); + + await filterBlockedHosts(hosts); + + // Should be called once with all emails, not 3 times + expect(mockGlobalBlocking.areBlocked).toHaveBeenCalledTimes(1); + expect(mockGlobalBlocking.areBlocked).toHaveBeenCalledWith([ + "user1@example.com", + "user2@example.com", + "user3@example.com", + ]); + }); + + test("should not call watchlist for locked-only hosts", async () => { + const hosts = [ + createHost(1, "locked1@example.com", true), + createHost(2, "locked2@example.com", true), + ]; + + await filterBlockedHosts(hosts); + + expect(mockGlobalBlocking.areBlocked).not.toHaveBeenCalled(); + }); + }); +}); + diff --git a/packages/features/watchlist/operations/filter-blocked-hosts.controller.ts b/packages/features/watchlist/operations/filter-blocked-hosts.controller.ts new file mode 100644 index 0000000000..3e88a8a3af --- /dev/null +++ b/packages/features/watchlist/operations/filter-blocked-hosts.controller.ts @@ -0,0 +1,49 @@ +import type { BlockableUser } from "./check-user-blocking"; +import { getBlockedUsersMap, isUserBlocked } from "./check-user-blocking"; + +/** + * Host type expected from slots service. + * Minimal shape needed for filtering. + */ +export interface HostWithEmail { + user: BlockableUser & { + id: number; + }; +} + +export interface FilterBlockedHostsResult { + eligibleHosts: T[]; + blockedCount: number; +} + +/** + * Filter out blocked hosts from a list using batched blocking check. + * Checks both locked users and watchlist-blocked emails. + * Single DB query for N hosts - eliminates N+1. + * + * @param hosts - List of hosts to filter + * @param organizationId - Optional organization ID for org-specific blocking + * @returns Object with eligibleHosts and blockedCount + */ +export async function filterBlockedHosts( + hosts: T[], + organizationId?: number | null +): Promise> { + if (hosts.length === 0) { + return { eligibleHosts: [], blockedCount: 0 }; + } + + // Map hosts to blockable users for the core utility + const usersToCheck = hosts.map((h) => h.user); + + // Get blocking map (handles locked users and watchlist in one call) + const { blockingMap, blockedCount } = await getBlockedUsersMap(usersToCheck, organizationId); + + // Filter out blocked hosts + const eligibleHosts = hosts.filter((host) => !isUserBlocked(host.user.email, blockingMap)); + + return { + eligibleHosts, + blockedCount, + }; +} diff --git a/packages/features/watchlist/operations/filter-blocked-users.controller.test.ts b/packages/features/watchlist/operations/filter-blocked-users.controller.test.ts new file mode 100644 index 0000000000..987f3fdb14 --- /dev/null +++ b/packages/features/watchlist/operations/filter-blocked-users.controller.test.ts @@ -0,0 +1,370 @@ +import { describe, test, expect, vi, beforeEach } from "vitest"; + +import type { SpanFn } from "@calcom/features/watchlist/lib/telemetry"; + +import { filterBlockedUsers, type UserWithEmail } from "./filter-blocked-users.controller"; + + +vi.mock("@calcom/features/di/watchlist/containers/watchlist", () => ({ + getWatchlistFeature: vi.fn(), +})); + +const mockGlobalBlocking: { areBlocked: ReturnType } = { + areBlocked: vi.fn(), +}; + +const mockOrgBlocking: { areBlocked: ReturnType } = { + areBlocked: vi.fn(), +}; + +const mockWatchlistFeature: { + globalBlocking: typeof mockGlobalBlocking; + orgBlocking: typeof mockOrgBlocking; + watchlist: Record; + audit: Record; +} = { + globalBlocking: mockGlobalBlocking, + orgBlocking: mockOrgBlocking, + watchlist: {}, + audit: {}, +}; + +// Helper to create user objects +function createUser(email: string, locked: boolean, id?: number, username?: string | null): UserWithEmail { + return { + id, + email, + locked, + username: username ?? email.split("@")[0], + }; +} + +describe("filterBlockedUsers", () => { + beforeEach(async () => { + vi.clearAllMocks(); + const { getWatchlistFeature } = await import("@calcom/features/di/watchlist/containers/watchlist"); + vi.mocked(getWatchlistFeature).mockResolvedValue(mockWatchlistFeature as never); + }); + + describe("Empty input handling", () => { + test("should return empty array for empty users array", async () => { + const result = await filterBlockedUsers([]); + + expect(result).toEqual({ eligibleUsers: [], blockedCount: 0 }); + expect(mockGlobalBlocking.areBlocked).not.toHaveBeenCalled(); + }); + }); + + describe("Locked user filtering", () => { + test("should filter out locked users", async () => { + const users = [ + createUser("locked@example.com", true, 1), + createUser("unlocked@example.com", false, 2), + ]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["unlocked@example.com", { isBlocked: false }]]) + ); + + const result = await filterBlockedUsers(users); + + expect(result.eligibleUsers).toHaveLength(1); + expect(result.eligibleUsers[0].email).toBe("unlocked@example.com"); + expect(result.blockedCount).toBe(1); + }); + + test("should filter out all locked users", async () => { + const users = [ + createUser("locked1@example.com", true, 1), + createUser("locked2@example.com", true, 2), + ]; + + const result = await filterBlockedUsers(users); + + expect(result.eligibleUsers).toHaveLength(0); + expect(result.blockedCount).toBe(2); + expect(mockGlobalBlocking.areBlocked).not.toHaveBeenCalled(); + }); + + test("should correctly identify locked users regardless of position", async () => { + const users = [ + createUser("first@example.com", false, 1), + createUser("locked@example.com", true, 2), + createUser("last@example.com", false, 3), + ]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([ + ["first@example.com", { isBlocked: false }], + ["last@example.com", { isBlocked: false }], + ]) + ); + + const result = await filterBlockedUsers(users); + + expect(result.eligibleUsers).toHaveLength(2); + expect(result.blockedCount).toBe(1); + }); + }); + + describe("Watchlist blocking", () => { + test("should filter out watchlist-blocked users", async () => { + const users = [ + createUser("blocked@spam.com", false, 1), + createUser("clean@example.com", false, 2), + ]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([ + ["blocked@spam.com", { isBlocked: true }], + ["clean@example.com", { isBlocked: false }], + ]) + ); + + const result = await filterBlockedUsers(users); + + expect(result.eligibleUsers).toHaveLength(1); + expect(result.eligibleUsers[0].email).toBe("clean@example.com"); + expect(result.blockedCount).toBe(1); + }); + + test("should check org-level blocking when organizationId provided", async () => { + const users = [createUser("user@example.com", false, 1)]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["user@example.com", { isBlocked: false }]]) + ); + mockOrgBlocking.areBlocked.mockResolvedValue(new Map([["user@example.com", { isBlocked: true }]])); + + const result = await filterBlockedUsers(users, 123); + + expect(result.eligibleUsers).toHaveLength(0); + expect(result.blockedCount).toBe(1); + expect(mockOrgBlocking.areBlocked).toHaveBeenCalledWith(["user@example.com"], 123); + }); + + test("should not check org-level blocking when organizationId is null", async () => { + const users = [createUser("user@example.com", false, 1)]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["user@example.com", { isBlocked: false }]]) + ); + + await filterBlockedUsers(users, null); + + expect(mockOrgBlocking.areBlocked).not.toHaveBeenCalled(); + }); + }); + + describe("Combined blocking (locked + watchlist)", () => { + test("should filter out both locked and watchlist-blocked users", async () => { + const users = [ + createUser("locked@example.com", true, 1), + createUser("watchlist-blocked@spam.com", false, 2), + createUser("clean@example.com", false, 3), + ]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([ + ["watchlist-blocked@spam.com", { isBlocked: true }], + ["clean@example.com", { isBlocked: false }], + ]) + ); + + const result = await filterBlockedUsers(users); + + expect(result.eligibleUsers).toHaveLength(1); + expect(result.eligibleUsers[0].email).toBe("clean@example.com"); + expect(result.blockedCount).toBe(2); + }); + + test("should handle complex scenario with mixed blocking reasons", async () => { + const users = [ + createUser("locked1@example.com", true, 1), + createUser("global-blocked@spam.com", false, 2), + createUser("org-blocked@competitor.com", false, 3), + createUser("clean1@example.com", false, 4), + createUser("locked2@example.com", true, 5), + createUser("clean2@example.com", false, 6), + ]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([ + ["global-blocked@spam.com", { isBlocked: true }], + ["org-blocked@competitor.com", { isBlocked: false }], + ["clean1@example.com", { isBlocked: false }], + ["clean2@example.com", { isBlocked: false }], + ]) + ); + mockOrgBlocking.areBlocked.mockResolvedValue( + new Map([ + ["global-blocked@spam.com", { isBlocked: false }], + ["org-blocked@competitor.com", { isBlocked: true }], + ["clean1@example.com", { isBlocked: false }], + ["clean2@example.com", { isBlocked: false }], + ]) + ); + + const result = await filterBlockedUsers(users, 123); + + expect(result.eligibleUsers).toHaveLength(2); + expect(result.eligibleUsers.map((u) => u.email)).toEqual(["clean1@example.com", "clean2@example.com"]); + expect(result.blockedCount).toBe(4); // 2 locked + 1 global + 1 org + }); + }); + + describe("Preserves user data", () => { + test("should preserve all user properties in returned eligible users", async () => { + interface ExtendedUser extends UserWithEmail { + metadata: Record; + credentials: unknown[]; + } + + const users: ExtendedUser[] = [ + { + id: 1, + email: "user@example.com", + locked: false, + username: "testuser", + metadata: { key: "value" }, + credentials: [{ type: "google" }], + }, + ]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["user@example.com", { isBlocked: false }]]) + ); + + const result = await filterBlockedUsers(users); + + expect(result.eligibleUsers).toHaveLength(1); + expect(result.eligibleUsers[0]).toEqual({ + id: 1, + email: "user@example.com", + locked: false, + username: "testuser", + metadata: { key: "value" }, + credentials: [{ type: "google" }], + }); + }); + + test("should handle users with null username", async () => { + const users: UserWithEmail[] = [ + { + id: 1, + email: "user@example.com", + locked: false, + username: null, + }, + ]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["user@example.com", { isBlocked: false }]]) + ); + + const result = await filterBlockedUsers(users); + + expect(result.eligibleUsers).toHaveLength(1); + expect(result.eligibleUsers[0].username).toBeNull(); + }); + }); + + describe("Email edge cases", () => { + test("should handle emails with different cases", async () => { + const users = [createUser("USER@EXAMPLE.COM", false, 1)]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["user@example.com", { isBlocked: true }]]) + ); + + const result = await filterBlockedUsers(users); + + expect(result.eligibleUsers).toHaveLength(0); + expect(result.blockedCount).toBe(1); + }); + + test("should handle emails with whitespace", async () => { + const users = [createUser(" user@example.com ", false, 1)]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["user@example.com", { isBlocked: false }]]) + ); + + const result = await filterBlockedUsers(users); + + expect(result.eligibleUsers).toHaveLength(1); + }); + }); + + describe("Performance: batched checks", () => { + test("should make single batch call for multiple users", async () => { + const users = [ + createUser("user1@example.com", false, 1), + createUser("user2@example.com", false, 2), + createUser("user3@example.com", false, 3), + ]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([ + ["user1@example.com", { isBlocked: false }], + ["user2@example.com", { isBlocked: false }], + ["user3@example.com", { isBlocked: false }], + ]) + ); + + await filterBlockedUsers(users); + + // Should be called once with all emails, not 3 times + expect(mockGlobalBlocking.areBlocked).toHaveBeenCalledTimes(1); + expect(mockGlobalBlocking.areBlocked).toHaveBeenCalledWith([ + "user1@example.com", + "user2@example.com", + "user3@example.com", + ]); + }); + + test("should not call watchlist for locked-only users", async () => { + const users = [ + createUser("locked1@example.com", true, 1), + createUser("locked2@example.com", true, 2), + ]; + + await filterBlockedUsers(users); + + expect(mockGlobalBlocking.areBlocked).not.toHaveBeenCalled(); + }); + }); + + describe("Telemetry (span)", () => { + test("should call span when provided", async () => { + const users = [createUser("user@example.com", false, 1)]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["user@example.com", { isBlocked: false }]]) + ); + + const mockSpan: SpanFn = vi.fn((_options, callback) => { + return Promise.resolve(callback()); + }); + + const result = await filterBlockedUsers(users, null, mockSpan); + + expect(result.eligibleUsers).toHaveLength(1); + expect(mockSpan).toHaveBeenCalledWith({ name: "filterBlockedUsers Controller" }, expect.any(Function)); + }); + + test("should work without span", async () => { + const users = [createUser("user@example.com", false, 1)]; + + mockGlobalBlocking.areBlocked.mockResolvedValue( + new Map([["user@example.com", { isBlocked: false }]]) + ); + + // Should not throw when span is not provided + const result = await filterBlockedUsers(users); + + expect(result.eligibleUsers).toHaveLength(1); + }); + }); +}); + diff --git a/packages/features/watchlist/operations/filter-blocked-users.controller.ts b/packages/features/watchlist/operations/filter-blocked-users.controller.ts new file mode 100644 index 0000000000..335bcbf55b --- /dev/null +++ b/packages/features/watchlist/operations/filter-blocked-users.controller.ts @@ -0,0 +1,56 @@ +import type { SpanFn } from "@calcom/features/watchlist/lib/telemetry"; + +import type { BlockableUser } from "./check-user-blocking"; +import { getBlockedUsersMap, isUserBlocked } from "./check-user-blocking"; + +/** + * User type expected from loadAndValidateUsers. + */ +export interface UserWithEmail extends BlockableUser { + id?: number; + username: string | null; +} + +export interface FilterBlockedUsersResult { + eligibleUsers: T[]; + blockedCount: number; +} + +/** + * Filter out blocked users from a list using batched blocking check. + * Checks both locked users and watchlist-blocked emails. + * Single DB query for N users - eliminates N+1. + * + * @param users - List of users to filter + * @param organizationId - Optional organization ID for org-specific blocking + * @param span - Optional tracing span + * @returns Object with eligibleUsers and blockedCount + */ +export async function filterBlockedUsers( + users: T[], + organizationId?: number | null, + span?: SpanFn +): Promise> { + const execute = async (): Promise> => { + if (users.length === 0) { + return { eligibleUsers: [], blockedCount: 0 }; + } + + // Get blocking map (handles locked users and watchlist in one call) + const { blockingMap, blockedCount } = await getBlockedUsersMap(users, organizationId); + + // Filter out blocked users + const eligibleUsers = users.filter((user) => !isUserBlocked(user.email, blockingMap)); + + return { + eligibleUsers, + blockedCount, + }; + }; + + if (!span) { + return execute(); + } + + return span({ name: "filterBlockedUsers Controller" }, execute); +} diff --git a/packages/prisma/migrations/20260105071846_update_watchlist_index_with_is_global/migration.sql b/packages/prisma/migrations/20260105071846_update_watchlist_index_with_is_global/migration.sql new file mode 100644 index 0000000000..401fc9a633 --- /dev/null +++ b/packages/prisma/migrations/20260105071846_update_watchlist_index_with_is_global/migration.sql @@ -0,0 +1,5 @@ +-- DropIndex +DROP INDEX "public"."Watchlist_type_value_organizationId_action_idx"; + +-- CreateIndex +CREATE INDEX "Watchlist_isGlobal_action_organizationId_type_value_idx" ON "public"."Watchlist"("isGlobal", "action", "organizationId", "type", "value"); diff --git a/packages/prisma/schema.prisma b/packages/prisma/schema.prisma index a355345d15..2098c6b5b6 100644 --- a/packages/prisma/schema.prisma +++ b/packages/prisma/schema.prisma @@ -2412,7 +2412,7 @@ model Watchlist { audits WatchlistAudit[] @@unique([type, value, organizationId]) - @@index([type, value, organizationId, action]) + @@index([isGlobal, action, organizationId, type, value]) } model WatchlistAudit { diff --git a/packages/trpc/server/routers/viewer/slots/util.ts b/packages/trpc/server/routers/viewer/slots/util.ts index 6a564705bb..92bd6b70e1 100644 --- a/packages/trpc/server/routers/viewer/slots/util.ts +++ b/packages/trpc/server/routers/viewer/slots/util.ts @@ -1,6 +1,4 @@ -import type { Logger } from "tslog"; -import { v4 as uuid } from "uuid"; - +import process from "node:process"; import type { Dayjs } from "@calcom/dayjs"; import dayjs from "@calcom/dayjs"; import { orgDomainConfig } from "@calcom/ee/organizations/lib/orgDomains"; @@ -9,9 +7,9 @@ import type { CurrentSeats, EventType, GetAvailabilityUser, - UserAvailabilityService, IFromUser, IToUser, + UserAvailabilityService, } from "@calcom/features/availability/lib/getUserAvailability"; import type { CheckBookingLimitsService } from "@calcom/features/bookings/lib/checkBookingLimits"; import { checkForConflicts } from "@calcom/features/bookings/lib/conflictChecker/checkForConflicts"; @@ -33,6 +31,7 @@ import type { ISelectedSlotRepository } from "@calcom/features/selectedSlots/rep import type { NoSlotsNotificationService } from "@calcom/features/slots/handleNotificationWhenNoSlots"; import type { UserRepository } from "@calcom/features/users/repositories/UserRepository"; import { withSelectedCalendars } from "@calcom/features/users/repositories/UserRepository"; +import { filterBlockedHosts } from "@calcom/features/watchlist/operations/filter-blocked-hosts.controller"; import { shouldIgnoreContactOwner } from "@calcom/lib/bookings/routing/utils"; import { RESERVED_SUBDOMAINS } from "@calcom/lib/constants"; import { getUTCOffsetByTimezone } from "@calcom/lib/dayjs"; @@ -43,21 +42,21 @@ import { parseDurationLimit } from "@calcom/lib/intervalLimits/isDurationLimits" import LimitManager from "@calcom/lib/intervalLimits/limitManager"; import { isBookingWithinPeriod } from "@calcom/lib/intervalLimits/utils"; import { + BookingDateInPastError, calculatePeriodLimits, isTimeOutOfBounds, isTimeViolatingFutureLimit, - BookingDateInPastError, } from "@calcom/lib/isOutOfBounds"; import logger from "@calcom/lib/logger"; import { safeStringify } from "@calcom/lib/safeStringify"; import { withReporting } from "@calcom/lib/sentryWrapper"; import type { RoutingFormResponseRepository } from "@calcom/lib/server/repository/formResponse"; -import { SchedulingType, PeriodType } from "@calcom/prisma/enums"; +import { PeriodType, SchedulingType } from "@calcom/prisma/enums"; import type { EventBusyDate, EventBusyDetails } from "@calcom/types/Calendar"; import type { CredentialForCalendarService } from "@calcom/types/Credential"; - import { TRPCError } from "@trpc/server"; - +import type { Logger } from "tslog"; +import { v4 as uuid } from "uuid"; import type { TGetScheduleInputSchema } from "./getSchedule.schema"; import type { GetScheduleOptions } from "./types"; @@ -200,8 +199,8 @@ export class AvailableSlotsService { usernameList: Array.isArray(input.usernameList) ? input.usernameList : input.usernameList - ? [input.usernameList] - : [], + ? [input.usernameList] + : [], }); const usersWithOldSelectedCalendars = usersForDynamicEventType.map((user) => withSelectedCalendars(user)); @@ -280,7 +279,7 @@ export class AvailableSlotsService { * cause slots from adjacent days to leak into the response. */ private _filterSlotsByRequestedDateRange< - T extends Record + T extends Record, >({ slotsMappedToDate, startTime, @@ -871,7 +870,7 @@ export class AvailableSlotsService { }); } - let busyTimesFromLimitsMap: Map | undefined = undefined; + let busyTimesFromLimitsMap: Map | undefined; if (eventType && (bookingLimits || durationLimits)) { const usersForLimits = usersWithCredentials.map((user) => ({ id: user.id, email: user.email })); const eventTimeZone = eventType.schedule?.timeZone ?? usersWithCredentials[0]?.timeZone ?? "UTC"; @@ -894,7 +893,7 @@ export class AvailableSlotsService { const teamBookingLimits = parseBookingLimit(teamForBookingLimits?.bookingLimits); - let teamBookingLimitsMap: Map | undefined = undefined; + let teamBookingLimitsMap: Map | undefined; if (teamForBookingLimits && teamBookingLimits) { const usersForTeamLimits = usersWithCredentials.map((user) => ({ id: user.id, email: user.email })); const eventTimeZone = eventType.schedule?.timeZone ?? usersWithCredentials[0]?.timeZone ?? "UTC"; @@ -1097,11 +1096,32 @@ export class AvailableSlotsService { rrHostSubsetIds: input.rrHostSubsetIds ?? undefined, }); - const allHosts = [...qualifiedRRHosts, ...fixedHosts]; + // Filter out blocked hosts BEFORE calculating availability (batched - single DB query) + const organizationId = eventType.parent?.team?.parentId ?? eventType.team?.parentId ?? null; + + const { eligibleHosts: eligibleQualifiedRRHosts } = await filterBlockedHosts( + qualifiedRRHosts, + organizationId + ); + const { eligibleHosts: eligibleFixedHosts } = await filterBlockedHosts(fixedHosts, organizationId); + const { eligibleHosts: eligibleFallbackRRHosts } = allFallbackRRHosts + ? await filterBlockedHosts(allFallbackRRHosts, organizationId) + : { eligibleHosts: [] }; + + const allHosts = [...eligibleQualifiedRRHosts, ...eligibleFixedHosts]; + + // If all hosts are blocked, return empty slots + if (allHosts.length === 0) { + loggerWithEventDetails.info("All hosts are blocked by watchlist, returning empty slots"); + return { + slots: {}, + }; + } const twoWeeksFromNow = dayjs().add(2, "week"); - const hasFallbackRRHosts = allFallbackRRHosts && allFallbackRRHosts.length > qualifiedRRHosts.length; + const hasFallbackRRHosts = + eligibleFallbackRRHosts.length > 0 && eligibleFallbackRRHosts.length > eligibleQualifiedRRHosts.length; let { allUsersAvailability, usersWithCredentials, currentSeats } = await this.calculateHostsAndAvailabilities({ @@ -1142,7 +1162,7 @@ export class AvailableSlotsService { const firstTwoWeeksAvailabilities = await this.calculateHostsAndAvailabilities({ input, eventType, - hosts: [...qualifiedRRHosts, ...fixedHosts], + hosts: [...eligibleQualifiedRRHosts, ...eligibleFixedHosts], loggerWithEventDetails, startTime: dayjs(), endTime: twoWeeksFromNow, @@ -1165,8 +1185,9 @@ export class AvailableSlotsService { loggerWithEventDetails.info({ email: input.email, contactOwnerEmail, - qualifiedRRHosts: qualifiedRRHosts.map((host) => host.user.id), - fallbackRRHosts: allFallbackRRHosts.map((host) => host.user.id), + eligibleQualifiedRRHosts: eligibleQualifiedRRHosts.map((host) => host.user.id), + eligibleFallbackRRHosts: eligibleFallbackRRHosts.map((host) => host.user.id), + blockedHostsCount: qualifiedRRHosts.length - eligibleQualifiedRRHosts.length, fallBackActive: diff > 0, }); } @@ -1177,7 +1198,7 @@ export class AvailableSlotsService { await this.calculateHostsAndAvailabilities({ input, eventType, - hosts: [...allFallbackRRHosts, ...fixedHosts], + hosts: [...eligibleFallbackRRHosts, ...eligibleFixedHosts], loggerWithEventDetails, startTime, endTime, @@ -1413,7 +1434,7 @@ export class AvailableSlotsService { eventType.timeZone || eventType?.schedule?.timeZone || allUsersAvailability?.[0]?.timeZone; const eventUtcOffset = getUTCOffsetByTimezone(eventTimeZone) ?? 0; - const bookerUtcOffset = input.timeZone ? getUTCOffsetByTimezone(input.timeZone) ?? 0 : 0; + const bookerUtcOffset = input.timeZone ? (getUTCOffsetByTimezone(input.timeZone) ?? 0) : 0; const periodLimits = calculatePeriodLimits({ periodType: eventType.periodType, periodDays: eventType.periodDays,