From 91beeeb525fce2904700d5e9c1da748dc17ae253 Mon Sep 17 00:00:00 2001 From: Joe Au-Yeung <65426560+joeauyeung@users.noreply.github.com> Date: Thu, 29 Feb 2024 15:01:13 -0500 Subject: [PATCH] fix: Google Profile Picture Update Race Condition (#13711) * Minor fixes * Discard changes to packages/app-store/ics-feedcalendar/config.json * Add comment * refactor: denesting and conditional profile update --------- Co-authored-by: zomars --- packages/app-store/googlecalendar/api/add.ts | 21 +-- .../app-store/googlecalendar/api/callback.ts | 124 +++++++++--------- .../app-store/googlecalendar/lib/constants.ts | 7 + 3 files changed, 73 insertions(+), 79 deletions(-) create mode 100644 packages/app-store/googlecalendar/lib/constants.ts diff --git a/packages/app-store/googlecalendar/api/add.ts b/packages/app-store/googlecalendar/api/add.ts index a898fed283..e4e841f19b 100644 --- a/packages/app-store/googlecalendar/api/add.ts +++ b/packages/app-store/googlecalendar/api/add.ts @@ -2,34 +2,21 @@ import { google } from "googleapis"; import type { NextApiRequest, NextApiResponse } from "next"; import { WEBAPP_URL_FOR_OAUTH } from "@calcom/lib/constants"; -import { HttpError } from "@calcom/lib/http-error"; import { defaultHandler, defaultResponder } from "@calcom/lib/server"; -import getAppKeysFromSlug from "../../_utils/getAppKeysFromSlug"; import { encodeOAuthState } from "../../_utils/oauth/encodeOAuthState"; - -export const scopes = [ - "https://www.googleapis.com/auth/calendar.readonly", - "https://www.googleapis.com/auth/calendar.events", - "https://www.googleapis.com/auth/userinfo.profile", -]; - -let client_id = ""; -let client_secret = ""; +import { SCOPES } from "../lib/constants"; +import { getGoogleAppKeys } from "../lib/getGoogleAppKeys"; async function getHandler(req: NextApiRequest, res: NextApiResponse) { // Get token from Google Calendar API - const appKeys = await getAppKeysFromSlug("google-calendar"); - if (typeof appKeys.client_id === "string") client_id = appKeys.client_id; - if (typeof appKeys.client_secret === "string") client_secret = appKeys.client_secret; - if (!client_id) throw new HttpError({ statusCode: 400, message: "Google client_id missing." }); - if (!client_secret) throw new HttpError({ statusCode: 400, message: "Google client_secret missing." }); + const { client_id, client_secret } = await getGoogleAppKeys(); const redirect_uri = `${WEBAPP_URL_FOR_OAUTH}/api/integrations/googlecalendar/callback`; const oAuth2Client = new google.auth.OAuth2(client_id, client_secret, redirect_uri); const authUrl = oAuth2Client.generateAuthUrl({ access_type: "offline", - scope: scopes, + scope: SCOPES, // A refresh token is only returned the first time the user // consents to providing access. For illustration purposes, // setting the prompt to 'consent' will force this consent diff --git a/packages/app-store/googlecalendar/api/callback.ts b/packages/app-store/googlecalendar/api/callback.ts index cebf818d61..16e8a712ca 100644 --- a/packages/app-store/googlecalendar/api/callback.ts +++ b/packages/app-store/googlecalendar/api/callback.ts @@ -3,7 +3,7 @@ import { google } from "googleapis"; import type { NextApiRequest, NextApiResponse } from "next"; import { renewSelectedCalendarCredentialId } from "@calcom/lib/connectedCalendar"; -import { WEBAPP_URL_FOR_OAUTH, WEBAPP_URL } from "@calcom/lib/constants"; +import { WEBAPP_URL, WEBAPP_URL_FOR_OAUTH } from "@calcom/lib/constants"; import { getSafeRedirectUrl } from "@calcom/lib/getSafeRedirectUrl"; import { HttpError } from "@calcom/lib/http-error"; import logger from "@calcom/lib/logger"; @@ -11,13 +11,10 @@ import { defaultHandler, defaultResponder } from "@calcom/lib/server"; import prisma from "@calcom/prisma"; import { Prisma } from "@calcom/prisma/client"; -import getAppKeysFromSlug from "../../_utils/getAppKeysFromSlug"; import getInstalledAppPath from "../../_utils/getInstalledAppPath"; import { decodeOAuthState } from "../../_utils/oauth/decodeOAuthState"; -import { scopes } from "./add"; - -let client_id = ""; -let client_secret = ""; +import { REQUIRED_SCOPES, SCOPE_USERINFO_PROFILE } from "../lib/constants"; +import { getGoogleAppKeys } from "../lib/getGoogleAppKeys"; async function getHandler(req: NextApiRequest, res: NextApiResponse) { const { code } = req.query; @@ -39,11 +36,7 @@ async function getHandler(req: NextApiRequest, res: NextApiResponse) { throw new HttpError({ statusCode: 401, message: "You must be logged in to do this" }); } - const appKeys = await getAppKeysFromSlug("google-calendar"); - if (typeof appKeys.client_id === "string") client_id = appKeys.client_id; - if (typeof appKeys.client_secret === "string") client_secret = appKeys.client_secret; - if (!client_id) throw new HttpError({ statusCode: 400, message: "Google client_id missing." }); - if (!client_secret) throw new HttpError({ statusCode: 400, message: "Google client_secret missing." }); + const { client_id, client_secret } = await getGoogleAppKeys(); const redirect_uri = `${WEBAPP_URL_FOR_OAUTH}/api/integrations/googlecalendar/callback`; @@ -53,33 +46,27 @@ async function getHandler(req: NextApiRequest, res: NextApiResponse) { if (code) { const token = await oAuth2Client.getToken(code); - key = token.res?.data; - - // Check that the has granted all permissions - const grantedScopes = key.scope; - for (const scope of scopes) { - if (!grantedScopes.includes(scope)) { - if (!state?.fromApp) { - throw new HttpError({ - statusCode: 400, - message: "You must grant all permissions to use this integration", - }); - } else { - res.redirect( - getSafeRedirectUrl(state.onErrorReturnTo) ?? - getSafeRedirectUrl(state?.returnTo) ?? - `${WEBAPP_URL}/apps/installed` - ); - return; - } + key = token.tokens; + const grantedScopes = token.tokens.scope?.split(" ") ?? []; + // Check if we have granted all required permissions + const hasMissingRequiredScopes = REQUIRED_SCOPES.some((scope) => !grantedScopes.includes(scope)); + if (hasMissingRequiredScopes) { + if (!state?.fromApp) { + throw new HttpError({ + statusCode: 400, + message: "You must grant all permissions to use this integration", + }); } + res.redirect( + getSafeRedirectUrl(state.onErrorReturnTo) ?? + getSafeRedirectUrl(state?.returnTo) ?? + `${WEBAPP_URL}/apps/installed` + ); + return; } // Set the primary calendar as the first selected calendar - // We can ignore this type error because we just validated the key when we init oAuth2Client - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - //@ts-ignore oAuth2Client.setCredentials(key); const calendar = google.calendar({ @@ -94,9 +81,10 @@ async function getHandler(req: NextApiRequest, res: NextApiResponse) { throw new HttpError({ message: "Internal Error", statusCode: 500 }); } - // Update the user's profile photo with google profile photo if it's null - // Since we don't want to block the user from using the app if this fails, we don't await this - updateProfilePhoto(oAuth2Client, req.session.user.id); + // Only attempt to update the user's profile photo if the user has granted the required scope + if (grantedScopes.includes(SCOPE_USERINFO_PROFILE)) { + await updateProfilePhoto(oAuth2Client, req.session.user.id); + } const credential = await prisma.credential.create({ data: { @@ -148,33 +136,44 @@ async function getHandler(req: NextApiRequest, res: NextApiResponse) { } } - if (state?.installGoogleVideo) { - const existingGoogleMeetCredential = await prisma.credential.findFirst({ - where: { - userId: req.session.user.id, - type: "google_video", - }, - }); - - if (!existingGoogleMeetCredential) { - await prisma.credential.create({ - data: { - type: "google_video", - key: {}, - userId: req.session.user.id, - appId: "google-meet", - }, - }); - - res.redirect( - getSafeRedirectUrl(`${WEBAPP_URL}/apps/installed/conferencing?hl=google-meet`) ?? - getInstalledAppPath({ variant: "conferencing", slug: "google-meet" }) - ); - } + // No need to install? Redirect to the returnTo URL + if (!state?.installGoogleVideo) { + res.redirect( + getSafeRedirectUrl(state?.returnTo) ?? + getInstalledAppPath({ variant: "calendar", slug: "google-calendar" }) + ); + return; } + + const existingGoogleMeetCredential = await prisma.credential.findFirst({ + where: { + userId: req.session.user.id, + type: "google_video", + }, + }); + + // If the user already has a google meet credential, there's nothing to do in here + if (existingGoogleMeetCredential) { + res.redirect( + getSafeRedirectUrl(`${WEBAPP_URL}/apps/installed/conferencing?hl=google-meet`) ?? + getInstalledAppPath({ variant: "conferencing", slug: "google-meet" }) + ); + return; + } + + // Create a new google meet credential + await prisma.credential.create({ + data: { + type: "google_video", + key: {}, + userId: req.session.user.id, + appId: "google-meet", + }, + }); + res.redirect( - getSafeRedirectUrl(state?.returnTo) ?? - getInstalledAppPath({ variant: "calendar", slug: "google-calendar" }) + getSafeRedirectUrl(`${WEBAPP_URL}/apps/installed/conferencing?hl=google-meet`) ?? + getInstalledAppPath({ variant: "conferencing", slug: "google-meet" }) ); } @@ -183,7 +182,8 @@ async function updateProfilePhoto(oAuth2Client: Auth.OAuth2Client, userId: numbe const oauth2 = google.oauth2({ version: "v2", auth: oAuth2Client }); const userDetails = await oauth2.userinfo.get(); if (userDetails.data?.picture) { - await prisma.user.update({ + // Using updateMany here since if the user already has a profile it would throw an error because no records were found to update the profile picture + await prisma.user.updateMany({ where: { id: userId, avatarUrl: null, avatar: null }, data: { avatarUrl: userDetails.data.picture, diff --git a/packages/app-store/googlecalendar/lib/constants.ts b/packages/app-store/googlecalendar/lib/constants.ts new file mode 100644 index 0000000000..5c285ce803 --- /dev/null +++ b/packages/app-store/googlecalendar/lib/constants.ts @@ -0,0 +1,7 @@ +export const SCOPE_USERINFO_PROFILE = "https://www.googleapis.com/auth/userinfo.profile"; +export const SCOPE_CALENDAR_READONLY = "https://www.googleapis.com/auth/calendar.readonly"; +export const SCOPE_CALENDAR_EVENT = "https://www.googleapis.com/auth/calendar.events"; + +export const REQUIRED_SCOPES = [SCOPE_CALENDAR_READONLY, SCOPE_CALENDAR_EVENT]; + +export const SCOPES = [...REQUIRED_SCOPES, SCOPE_USERINFO_PROFILE];