fix: DelegationCredential - Duplicate GoogleMeet and unnecessary serviceAccount key access (calcom#21269)

* Remove unused return statement

* Fix return from fn instead of continuing the loop

* Ensure userId is set for existing records on update

* ensure that serviceAccountKey is only retrieved whenb assked

Author: hariombalhara

packages/app-store/server.ts

@@ -125,7 +125,10 @@ export async function getLocationGroupedOptions(
             : {}),
         };
         if (apps[groupByCategory]) {
-          apps[groupByCategory] = [...apps[groupByCategory], option];
+          const existingOption = apps[groupByCategory].find((o) => o.value === option.value);
+          if (!existingOption) {
+            apps[groupByCategory] = [...apps[groupByCategory], option];
+          }
         } else {
           apps[groupByCategory] = [option];
         }

packages/features/bookings/lib/getAllCredentialsForUsersOnEvent/getAllCredentials.test.ts

@@ -15,13 +15,14 @@ import { UserRepository } from "@calcom/lib/server/repository/user";
 //   }
 // })
 
-describe("getAllCredentials", () => {
+describe("getAllCredentialsIncludeServiceAccountKey", () => {
   test("Get an individual's credentials", async () => {
     vi.spyOn(UserRepository, "enrichUserWithItsProfile").mockReturnValue({
       profile: null,
     });
 
-    const getAllCredentials = (await import("./getAllCredentials")).getAllCredentials;
+    const getAllCredentialsIncludeServiceAccountKey = (await import("./getAllCredentials"))
+      .getAllCredentialsIncludeServiceAccountKey;
 
     const userCredential = {
       id: 1,
@@ -38,7 +39,7 @@ describe("getAllCredentials", () => {
       { type: "team-credential", teamId: 1, key: {} },
     ]);
 
-    const credentials = await getAllCredentials(
+    const credentials = await getAllCredentialsIncludeServiceAccountKey(
       {
         id: 1,
         username: "test",
@@ -65,7 +66,8 @@ describe("getAllCredentials", () => {
             profile: null,
           });
 
-          const getAllCredentials = (await import("./getAllCredentials")).getAllCredentials;
+          const getAllCredentialsIncludeServiceAccountKey = (await import("./getAllCredentials"))
+            .getAllCredentialsIncludeServiceAccountKey;
 
           const crmCredential = {
             id: 1,
@@ -101,7 +103,7 @@ describe("getAllCredentials", () => {
             },
           ]);
 
-          const credentials = await getAllCredentials(
+          const credentials = await getAllCredentialsIncludeServiceAccountKey(
             {
               id: 1,
               username: "test",
@@ -138,7 +140,8 @@ describe("getAllCredentials", () => {
             profile: null,
           });
 
-          const getAllCredentials = (await import("./getAllCredentials")).getAllCredentials;
+          const getAllCredentialsIncludeServiceAccountKey = (await import("./getAllCredentials"))
+            .getAllCredentialsIncludeServiceAccountKey;
 
           const crmCredential = {
             id: 1,
@@ -170,7 +173,7 @@ describe("getAllCredentials", () => {
             },
           ]);
 
-          const credentials = await getAllCredentials(
+          const credentials = await getAllCredentialsIncludeServiceAccountKey(
             {
               id: 1,
               username: "test",
@@ -204,7 +207,8 @@ describe("getAllCredentials", () => {
             profile: null,
           });
 
-          const getAllCredentials = (await import("./getAllCredentials")).getAllCredentials;
+          const getAllCredentialsIncludeServiceAccountKey = (await import("./getAllCredentials"))
+            .getAllCredentialsIncludeServiceAccountKey;
 
           const teamId = 1;
           const crmCredential = {
@@ -261,7 +265,7 @@ describe("getAllCredentials", () => {
 
           console.log(testEventType);
 
-          const credentials = await getAllCredentials(
+          const credentials = await getAllCredentialsIncludeServiceAccountKey(
             {
               id: 1,
               username: "test",
@@ -291,7 +295,8 @@ describe("getAllCredentials", () => {
           expect(credentials).toContainEqual(expect.objectContaining({ teamId, type: "salesforce_crm" }));
         });
         test("For an org user", async () => {
-          const getAllCredentials = (await import("./getAllCredentials")).getAllCredentials;
+          const getAllCredentialsIncludeServiceAccountKey = (await import("./getAllCredentials"))
+            .getAllCredentialsIncludeServiceAccountKey;
           const orgId = 3;
           vi.spyOn(UserRepository, "enrichUserWithItsProfile").mockReturnValue({
             profile: { organizationId: orgId },
@@ -357,7 +362,7 @@ describe("getAllCredentials", () => {
             },
           ]);
 
-          const credentials = await getAllCredentials(
+          const credentials = await getAllCredentialsIncludeServiceAccountKey(
             {
               id: 1,
               username: "test",
@@ -394,7 +399,8 @@ describe("getAllCredentials", () => {
       });
       describe("Default with _other_calendar credentials", () => {
         test("For users", async () => {
-          const getAllCredentials = (await import("./getAllCredentials")).getAllCredentials;
+          const getAllCredentialsIncludeServiceAccountKey = (await import("./getAllCredentials"))
+            .getAllCredentialsIncludeServiceAccountKey;
 
           const crmCredential = {
             id: 1,
@@ -430,7 +436,7 @@ describe("getAllCredentials", () => {
             },
           ]);
 
-          const credentials = await getAllCredentials(
+          const credentials = await getAllCredentialsIncludeServiceAccountKey(
             {
               id: 1,
               username: "test",
@@ -459,7 +465,8 @@ describe("getAllCredentials", () => {
           );
         });
         test("For teams", async () => {
-          const getAllCredentials = (await import("./getAllCredentials")).getAllCredentials;
+          const getAllCredentialsIncludeServiceAccountKey = (await import("./getAllCredentials"))
+            .getAllCredentialsIncludeServiceAccountKey;
 
           const crmCredential = {
             id: 1,
@@ -491,7 +498,7 @@ describe("getAllCredentials", () => {
             },
           ]);
 
-          const credentials = await getAllCredentials(
+          const credentials = await getAllCredentialsIncludeServiceAccountKey(
             {
               id: 1,
               username: "test",
@@ -517,7 +524,8 @@ describe("getAllCredentials", () => {
           );
         });
         test("For child of managed event type", async () => {
-          const getAllCredentials = (await import("./getAllCredentials")).getAllCredentials;
+          const getAllCredentialsIncludeServiceAccountKey = (await import("./getAllCredentials"))
+            .getAllCredentialsIncludeServiceAccountKey;
 
           const teamId = 1;
           const crmCredential = {
@@ -574,7 +582,7 @@ describe("getAllCredentials", () => {
 
           console.log(testEventType);
 
-          const credentials = await getAllCredentials(
+          const credentials = await getAllCredentialsIncludeServiceAccountKey(
             {
               id: 1,
               username: "test",
@@ -600,7 +608,8 @@ describe("getAllCredentials", () => {
           );
         });
         test("For an org user", async () => {
-          const getAllCredentials = (await import("./getAllCredentials")).getAllCredentials;
+          const getAllCredentialsIncludeServiceAccountKey = (await import("./getAllCredentials"))
+            .getAllCredentialsIncludeServiceAccountKey;
           const orgId = 3;
           vi.spyOn(UserRepository, "enrichUserWithItsProfile").mockReturnValue({
             profile: { organizationId: orgId },
@@ -666,7 +675,7 @@ describe("getAllCredentials", () => {
             },
           ]);
 
-          const credentials = await getAllCredentials(
+          const credentials = await getAllCredentialsIncludeServiceAccountKey(
             {
               id: 1,
               username: "test",

packages/features/bookings/lib/getAllCredentialsForUsersOnEvent/getAllCredentials.ts

@@ -1,6 +1,6 @@
 import type z from "zod";
 
-import { enrichUserWithDelegationCredentialsWithoutOrgId } from "@calcom/lib/delegationCredential/server";
+import { enrichUserWithDelegationCredentialsIncludeServiceAccountKey } from "@calcom/lib/delegationCredential/server";
 import { UserRepository } from "@calcom/lib/server/repository/user";
 import prisma from "@calcom/prisma";
 import { credentialForCalendarServiceSelect } from "@calcom/prisma/selects/credential";
@@ -19,7 +19,7 @@ export type EventType = {
  * Gets credentials from the user, team, and org if applicable
  *
  */
-export const getAllCredentials = async (
+export const getAllCredentialsIncludeServiceAccountKey = async (
   user: { id: number; username: string | null; email: string; credentials: CredentialPayload[] },
   eventType: EventType
 ) => {
@@ -120,7 +120,7 @@ export const getAllCredentials = async (
     }
   });
 
-  const userWithDelegationCredentials = await enrichUserWithDelegationCredentialsWithoutOrgId({
+  const userWithDelegationCredentials = await enrichUserWithDelegationCredentialsIncludeServiceAccountKey({
     user: { ...user, credentials: allCredentials },
   });
 

packages/features/bookings/lib/handleCancelBooking.ts

@@ -37,7 +37,7 @@ import type { EventTypeMetadata } from "@calcom/prisma/zod-utils";
 import { getAllWorkflowsFromEventType } from "@calcom/trpc/server/routers/viewer/workflows/util";
 import type { CalendarEvent } from "@calcom/types/Calendar";
 
-import { getAllCredentials } from "./getAllCredentialsForUsersOnEvent/getAllCredentials";
+import { getAllCredentialsIncludeServiceAccountKey } from "./getAllCredentialsForUsersOnEvent/getAllCredentials";
 import { getBookingToDelete } from "./getBookingToDelete";
 import { handleInternalNote } from "./handleInternalNote";
 import cancelAttendeeSeat from "./handleSeats/cancel/cancelAttendeeSeat";
@@ -467,7 +467,7 @@ async function handler(input: CancelBookingInput) {
 
     const bookingToDeleteEventTypeMetadata = bookingToDeleteEventTypeMetadataParsed.data;
 
-    const credentials = await getAllCredentials(bookingToDelete.user, {
+    const credentials = await getAllCredentialsIncludeServiceAccountKey(bookingToDelete.user, {
       ...bookingToDelete.eventType,
       metadata: bookingToDeleteEventTypeMetadata,
     });

packages/features/bookings/lib/handleNewBooking.ts

@@ -81,7 +81,7 @@ import type { CredentialForCalendarService } from "@calcom/types/Credential";
 import type { EventResult, PartialReference } from "@calcom/types/EventManager";
 
 import type { EventPayloadType, EventTypeInfo } from "../../webhooks/lib/sendPayload";
-import { getAllCredentials } from "./getAllCredentialsForUsersOnEvent/getAllCredentials";
+import { getAllCredentialsIncludeServiceAccountKey } from "./getAllCredentialsForUsersOnEvent/getAllCredentials";
 import { refreshCredentials } from "./getAllCredentialsForUsersOnEvent/refreshCredentials";
 import getBookingDataSchema from "./getBookingDataSchema";
 import { addVideoCallDataToEvent } from "./handleNewBooking/addVideoCallDataToEvent";
@@ -820,7 +820,7 @@ async function handler(
     : users[0];
 
   const tOrganizer = await getTranslation(organizerUser?.locale ?? "en", "common");
-  const allCredentials = await getAllCredentials(organizerUser, eventType);
+  const allCredentials = await getAllCredentialsIncludeServiceAccountKey(organizerUser, eventType);
 
   // If the Organizer himself is rescheduling, the booker should be sent the communication in his timezone and locale.
   const attendeeInfoOnReschedule =

packages/features/bookings/lib/handleSeats/cancel/cancelAttendeeSeat.ts

@@ -2,7 +2,7 @@ import { getCalendar } from "@calcom/app-store/_utils/getCalendar";
 import { sendCancelledSeatEmailsAndSMS } from "@calcom/emails";
 import sendPayload from "@calcom/features/webhooks/lib/sendOrSchedulePayload";
 import type { EventPayloadType, EventTypeInfo } from "@calcom/features/webhooks/lib/sendPayload";
-import { getAllDelegationCredentialsForUser } from "@calcom/lib/delegationCredential/server";
+import { getAllDelegationCredentialsForUserIncludeServiceAccountKey } from "@calcom/lib/delegationCredential/server";
 import { getDelegationCredentialOrFindRegularCredential } from "@calcom/lib/delegationCredential/server";
 import { HttpError } from "@calcom/lib/http-error";
 import logger from "@calcom/lib/logger";
@@ -71,7 +71,8 @@ async function cancelAttendeeSeat(
   const attendee = bookingToDelete?.attendees.find((attendee) => attendee.id === seatReference.attendeeId);
   const bookingToDeleteUser = bookingToDelete.user ?? null;
   const delegationCredentials = bookingToDeleteUser
-    ? await getAllDelegationCredentialsForUser({
+    ? // We fetch delegation credentials with ServiceAccount key as CalendarService instance created later in the flow needs it
+      await getAllDelegationCredentialsForUserIncludeServiceAccountKey({
         user: { email: bookingToDeleteUser.email, id: bookingToDeleteUser.id },
       })
     : [];

packages/features/bookings/lib/handleSeats/lib/lastAttendeeDeleteBooking.ts

@@ -2,7 +2,7 @@ import type { Attendee } from "@prisma/client";
 
 // eslint-disable-next-line no-restricted-imports
 import { getCalendar } from "@calcom/app-store/_utils/getCalendar";
-import { getAllDelegationCredentialsForUser } from "@calcom/lib/delegationCredential/server";
+import { getAllDelegationCredentialsForUserIncludeServiceAccountKey } from "@calcom/lib/delegationCredential/server";
 import { getDelegationCredentialOrFindRegularCredential } from "@calcom/lib/delegationCredential/server";
 import { deleteMeeting } from "@calcom/lib/videoClient";
 import prisma from "@calcom/prisma";
@@ -21,7 +21,8 @@ const lastAttendeeDeleteBooking = async (
   let deletedReferences = false;
   const bookingUser = originalRescheduledBooking?.user;
   const delegationCredentials = bookingUser
-    ? await getAllDelegationCredentialsForUser({
+    ? // We fetch delegation credentials with ServiceAccount key as CalendarService instance created later in the flow needs it
+      await getAllDelegationCredentialsForUserIncludeServiceAccountKey({
         user: { email: bookingUser.email, id: bookingUser.id },
       })
     : [];

packages/features/bookings/lib/handleSeats/types.d.ts

@@ -18,7 +18,7 @@ export type NewSeatedBookingObject = {
     bookerUrl: string;
   };
   invitee: Invitee;
-  allCredentials: Awaited<ReturnType<typeof getAllCredentials>>;
+  allCredentials: Awaited<ReturnType<typeof getAllCredentialsIncludeServiceAccountKey>>;
   organizerUser: OrganizerUser;
   originalRescheduledBooking: OriginalRescheduledBooking;
   bookerEmail: string;

…tion-credential/delegation-credential.md …eatures/delegation-credentials/README.mdapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/delegation-credential/delegation-credential.md renamed to packages/features/delegation-credentials/README.md apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/delegation-credential/delegation-credential.md renamed to packages/features/delegation-credentials/README.md

@@ -95,8 +95,7 @@ Step 6: Enable Delegation Credential(To Be taken By Cal.com organization Owner/A
 ### Terminology
 
 - Delegation Credential: A Delegation Credential service account key along with user's email becomes the Delegation Credential which is an alternative to regular Credential in DB.
-- DWD: Domain Wide Delegation
-- non-dwd credential: Regular credentials that are stored in Credentials table
+- Delegation User Credential: A Delegation User Credential is a Credential record in DB that uses DelegationCredential record to actually access the user's calendar. A Credential record with delegationCredentialId set is a Delegation User Credential.
 
 ### How Delegation Credential works
 
@@ -108,6 +107,13 @@ Step 6: Enable Delegation Credential(To Be taken By Cal.com organization Owner/A
 - A Delegation Credential service account key along with user's email becomes the Delegation Credential which is an alternative to regular Credential in DB.
 - Delegation Credential doesn't completely replace the regular credentials. Delegation Credential gives access to the cal.com user's email in Google Calendar. So, if the user needs to connect to some other email's calendar, we need to use the regular credentials.
 
+### Cron Jobs
+
+Cron jobs ensure that for each and every member of the organization that has Delegation Credential enabled, corresponding SelectedCalendar records are there. These crons currently run every 5 minutes, look at vercel.json for the up-to-date schedule.
+
+- `credentials` cron job creates Delegation User Credential records for all the members of the organization who don't have Delegation User Credentials yet. It also ensures that on disabling Delegation Credential, the Delegation User Credentials are deleted which automatically deletes the SelectedCalendars through DB cascade.
+- `selected-calendars` cron job creates SelectedCalendar records for all the Delegation User Credentials of the organization who don't have Selected Calendars yet.
+
 ### Important Points
 
 - No Credential table entry is created when enabling Delegation Credential. The workspace platform's related apps will be considered as "installed" for the users with email matching dwd domain. An in-memory credential like object is created for this purpose. It allows avoiding creation of thousands of records for all the members of the organization when Delegation Credential is enabled.
@@ -122,27 +128,26 @@ Step 6: Enable Delegation Credential(To Be taken By Cal.com organization Owner/A
 1. Identify the logged in user's email
 2. Identify the domainWideDelegations for that email's domain
 3. Build in-memory credentials for the domainWideDelegations and use them along with the actual credentials(that user might have connected) of the user
-4. We don't show the non-dwd connected calendar(if there is a corresponding dwd connected calendar). Though we use the non-dwd credentials to identify the selected calendars, for the dwd connected calendar.
+4. We don't show the non DelegationCredential connected calendar(if there is a corresponding DelegationCredential connected calendar). Though we use the non DelegationCredential credentials to identify the selected calendars, for the DelegationCredential connected calendar.
 
 ### Impact of disabling Delegation Credential
 
-Disabling effectively stops generating in-memory delegation user credentials. So, any members who haven't manually connected their Calendar and thus their calendar connections were working only because of Delegation Credential, would have their connections broken.
-
-#### What would not work correctly ?
-
-- Calendar won't be checked for conflicts. So, they could get booked at a time when they are marked busy in their calendar.
-  - Cal.com bookings would still be checked for conflicts.
-- Bookings might not appear in the attendee and host's Google Calendar. Because we would be unable to use the API to create the events in calendar and Google doesn't always add events to calendar automatically based on .ics file alone.
-- Cal Video would be used as the booking location instead of Google Meet.
+Disabling effectively stops generating in-memory delegation user credentials. So, any members who haven't manually connected their Calendar and thus their calendar connections were working only because of Delegation Credential, would have their calendar connections broken.
 
-#### What would work correctly ?
-
-- Bookings would still go through. People relying on Salesforce for booking details, would face no issues.
-- Cal.com bookings would still be checked for conflicts.
+### Impact of enabling Delegation Credential
+- Existing calendar-cache records are re-used as we identify the relevant record by userId and key of CalendarCache record.
+  - Any updates to those calendar-cache records keep on working by using the non-delegation credential attached with the SelectedCalendar record.
+- For any new members, we create Credential records and SelectedCalendar records through cron jobs and thus their calendar-cache records will also be created.
 
 ### Notes when testing locally
 
 - You need to enable the feature through feature flag.
 - You could use Acme org and login as <owner1-acme@example.com>
 - Make sure to change the email of the user above to your workspace owner's email(other member's email might also work). This is necessary otherwise you won't be able to enable Delegation Credential for the organization.
   - Note: After changing the email, you would have to logout and login again
+
+
+
+## TODO
+- Test what happens when credential expires that was used in CalendarCache/SelectedCalendar
+  - It seems that if refresh token is valid then it would still be refreshed but if it becomes invalid then it ends up causing the calendar-cache updates to break because it isn't able to renew the access token. How do you fix it?