From e21dbb704efff0782d9d73c02d15eaa6b97e929d Mon Sep 17 00:00:00 2001 From: Hariom Balhara Date: Fri, 23 May 2025 09:07:24 +0530 Subject: [PATCH] fix error in selected-calendars cron for delegation credential (#21465) --- .../selected-calendars/__tests__/cron.test.ts | 93 +++++++++++++++---- .../app/api/cron/selected-calendars/route.ts | 63 ++++++++++--- 2 files changed, 127 insertions(+), 29 deletions(-) diff --git a/apps/web/app/api/cron/selected-calendars/__tests__/cron.test.ts b/apps/web/app/api/cron/selected-calendars/__tests__/cron.test.ts index d00c72ad0e..2d2405cad3 100644 --- a/apps/web/app/api/cron/selected-calendars/__tests__/cron.test.ts +++ b/apps/web/app/api/cron/selected-calendars/__tests__/cron.test.ts @@ -5,7 +5,7 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { CalendarAppDelegationCredentialInvalidGrantError } from "@calcom/lib/CalendarAppError"; -import { handleCreateSelectedCalendars } from "../route"; +import { handleCreateSelectedCalendars, isSameEmail } from "../route"; // Mock GoogleCalendarService const fetchPrimaryCalendarMock = vi.fn(); @@ -148,23 +148,50 @@ describe("handleCreateSelectedCalendars integration", () => { ]); }); - it("creates a Selected Calendar even if the user's primary calendar ID does not match their email address", async () => { - await createOrg({ id: 1 }); - await createWorkspacePlatform({ id: 1 }); - const user = await createUser({ id: 1, email: "user1@example.com" }); - const delegationCredential = await createDelegationCredential({ - id: "delegation-credential-1", - orgId: 1, - domain: "example.com", - }); - await createCredential({ id: 1, userId: user.id, delegationCredentialId: delegationCredential.id }); - fetchPrimaryCalendarMock.mockResolvedValue({ id: "notuser@example.com" }); + describe("when the user's primary calendar ID does not match their email address", () => { + it("creates a Selected Calendar with the primaryCalendarId as the externalId if user's email is not a plus based variant of primaryCalendarId", async () => { + await createOrg({ id: 1 }); + await createWorkspacePlatform({ id: 1 }); + const user = await createUser({ id: 1, email: "user1@example.com" }); + const delegationCredential = await createDelegationCredential({ + id: "delegation-credential-1", + orgId: 1, + domain: "example.com", + }); + await createCredential({ id: 1, userId: user.id, delegationCredentialId: delegationCredential.id }); + fetchPrimaryCalendarMock.mockResolvedValue({ id: "notuser@example.com" }); - const result = await handleCreateSelectedCalendars(); - expect(result.success).toBe(1); - await expectSelectedCalendars([ - { userId: user.id, externalId: "notuser@example.com", delegationCredentialId: delegationCredential.id }, - ]); + const result = await handleCreateSelectedCalendars(); + expect(result.success).toBe(1); + expect(result.failures).toBe(0); + await expectSelectedCalendars([ + { + userId: user.id, + externalId: "notuser@example.com", + delegationCredentialId: delegationCredential.id, + }, + ]); + }); + + it("creates a Selected Calendar with the user's email as the externalId if user's email is a plus based variant of primaryCalendarId", async () => { + await createOrg({ id: 1 }); + await createWorkspacePlatform({ id: 1 }); + const user = await createUser({ id: 1, email: "user1+test@example.com" }); + const delegationCredential = await createDelegationCredential({ + id: "delegation-credential-1", + orgId: 1, + domain: "example.com", + }); + await createCredential({ id: 1, userId: user.id, delegationCredentialId: delegationCredential.id }); + fetchPrimaryCalendarMock.mockResolvedValue({ id: "user1@example.com" }); + + const result = await handleCreateSelectedCalendars(); + expect(result.success).toBe(1); + expect(result.failures).toBe(0); + await expectSelectedCalendars([ + { userId: user.id, externalId: user.email, delegationCredentialId: delegationCredential.id }, + ]); + }); }); it("does not create duplicate Selected Calendars for users who already have a valid one", async () => { @@ -274,3 +301,35 @@ describe("handleCreateSelectedCalendars integration", () => { }); }); }); + +describe("isSameEmail", () => { + it("returns true if variant email is a plus based variant of main email", () => { + expect( + isSameEmail({ + mainEmail: "user1@example.com", + variantEmail: "user1+test@example.com", + emailProvider: "google", + }) + ).toBe(true); + }); + + it("returns false if variant email is different from main email", () => { + expect( + isSameEmail({ + mainEmail: "user1@example.com", + variantEmail: "user1@domain.com", + emailProvider: "google", + }) + ).toBe(false); + }); + + it("returns true if variant email is same as main email but with different casing", () => { + expect( + isSameEmail({ + mainEmail: "User1@example.com", + variantEmail: "user1+test@Example.com", + emailProvider: "google", + }) + ).toBe(true); + }); +}); diff --git a/apps/web/app/api/cron/selected-calendars/route.ts b/apps/web/app/api/cron/selected-calendars/route.ts index 0523b82f22..a0ddb4c3d9 100644 --- a/apps/web/app/api/cron/selected-calendars/route.ts +++ b/apps/web/app/api/cron/selected-calendars/route.ts @@ -29,6 +29,25 @@ const validateRequest = (req: NextRequest) => { } }; +export function isSameEmail({ + mainEmail, + variantEmail, + emailProvider, +}: { + mainEmail: string; + variantEmail: string; + emailProvider: "google"; +}) { + if (emailProvider !== "google") { + return mainEmail.toLowerCase() === variantEmail.toLowerCase(); + } + const variantEmailParts = variantEmail.split("@"); + const variantEmailUserName = variantEmailParts[0]; + const variantEmailDomain = variantEmailParts[1]; + const variantEmailUserNameWithoutPlus = variantEmailUserName.split("+")[0]; + return mainEmail.toLowerCase() === `${variantEmailUserNameWithoutPlus}@${variantEmailDomain}`.toLowerCase(); +} + type DelegationUserCredential = Awaited< ReturnType >[number]; @@ -156,6 +175,9 @@ async function fetchPrimaryCalendarId({ return primaryCalendarId; } +/** + * Creates a SelectedCalendar record corresponding to Delegation User Credential - that would normally use primary calendar Id as the externalId but in some cases, it would use user's email as the externalId + */ async function processDelegationUserCredential( delegationUserCredential: DelegationUserCredentialWithEnsuredUser ) { @@ -166,30 +188,47 @@ async function processDelegationUserCredential( return; } - const externalCalendarId = await fetchPrimaryCalendarId({ + const primaryCalendarId = await fetchPrimaryCalendarId({ googleCalendarService, delegationUserCredential, }); - if (!externalCalendarId) { + if (!primaryCalendarId) { log.error( `External calendar not found for delegationCredentialId: ${delegationUserCredential.delegationCredentialId}, credentialId: ${delegationUserCredential.id} and userId: ${delegationUserCredential.userId}` ); throw new Error("externalCalendarId could not be determined"); } - if (externalCalendarId !== userEmail) { - // We don't know the scenario when Delegation Credential allows access to an email that is not the primary calendar email - // So log a warning for further investigation - log.warn( - `External calendar id mismatch for delegationCredentialId: ${delegationUserCredential.delegationCredentialId}, credentialId: ${delegationUserCredential.id} and userId: ${delegationUserCredential.userId}`, - safeStringify({ - externalCalendarId, - userEmail, - }) - ); + let externalCalendarId = primaryCalendarId; + + if (primaryCalendarId !== userEmail) { + const areEmailsSame = isSameEmail({ + emailProvider: "google", + mainEmail: primaryCalendarId, + variantEmail: userEmail, + }); + + if (areEmailsSame) { + log.info( + `Email mismatch for delegationCredentialId: ${delegationUserCredential.delegationCredentialId}, credentialId: ${delegationUserCredential.id} and userId: ${delegationUserCredential.userId}. Using userEmail as the externalCalendarId as it is a plus based variant of primaryCalendarId` + ); + // We have found that for some plus based email addresses(like john+test@acme.com) fetching primary calendar is successful and the primaryCalendarId comes back as john@acme.com + // Using userEmail as the externalCalendarId would ensure that the SelectedCalendar isn't attempted to be re-created and the SelectedCalendar record is still logically correct + externalCalendarId = userEmail; + } else { + // Unknown reason for mismatch, so we log it. + log.warn( + `Email mismatch for delegationCredentialId: ${delegationUserCredential.delegationCredentialId}, credentialId: ${delegationUserCredential.id} and userId: ${delegationUserCredential.userId}`, + safeStringify({ + externalCalendarId, + userEmail, + }) + ); + } } + // This creation call isn't expected to fail as we have already checked for the existence of the SelectedCalendar by userEmail await SelectedCalendarRepository.create({ integration: "google_calendar", externalId: externalCalendarId,