feat: Google Calendar access revoked and webhook removed #530
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request primarily involve the integration with Google Calendar services across several files. Key modifications include the removal of the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (6)
apps/backend/src/routers/integration/calendar.route.js (1)
2-2: Consider breaking down the import statement for better readability.The import statement is becoming quite long. Consider restructuring it into multiple lines for improved readability and maintainability.
-import { getGoogleCalendarAccessTokenController, getGoogleCalendarEventsController, addGoogleCalendarEventController, updateGoogleCalendarEventController, deleteGoogleCalendarEventController, getGoogleCalendarMeetingsController, getGoogleCalendarupComingMeetingsController, revokeGoogleCalendarAccessController } from "../../controllers/integration/calendar.controller.js"; +import { + getGoogleCalendarAccessTokenController, + getGoogleCalendarEventsController, + addGoogleCalendarEventController, + updateGoogleCalendarEventController, + deleteGoogleCalendarEventController, + getGoogleCalendarMeetingsController, + getGoogleCalendarupComingMeetingsController, + revokeGoogleCalendarAccessController +} from "../../controllers/integration/calendar.controller.js";apps/backend/src/models/core/user.model.js (2)
101-103: Consider documenting the metadata field structure and purpose.While Schema.Types.Mixed provides flexibility, it can make the codebase harder to maintain and migrate. Consider:
- Adding JSDoc comments describing the expected structure and purpose of the metadata field
- Adding a default value (e.g.,
default: {}) to ensure consistency for existing documentsgoogleCalendar: { accessToken: String, refreshToken: String, + // Stores additional Google Calendar integration data such as webhook configurations metadata: { type: Schema.Types.Mixed, + default: {} }, connected: { type: Boolean, default: false } },
101-103: Consider data sanitization for the metadata field.Since Schema.Types.Mixed allows any type of data, consider implementing middleware to sanitize and validate the metadata content before saving to prevent security issues or data inconsistencies.
apps/backend/src/controllers/integration/calendar.controller.js (1)
172-178: Consider transaction-like behavior for webhook and access revocation.The current implementation could leave the system in an inconsistent state if webhook removal succeeds but access revocation fails. Consider implementing a transaction-like pattern.
try { + const errors = []; if (metadata && metadata.channelId && metadata.resourceId) { - await removeGoogleCalendarWebhook(metadata.channelId, metadata.resourceId, accessToken); + try { + await removeGoogleCalendarWebhook(metadata.channelId, metadata.resourceId, accessToken); + } catch (webhookError) { + errors.push(`Failed to remove webhook: ${webhookError.message}`); + } } if (accessToken) { - await revokeGoogleCalendarAccess(user); + try { + await revokeGoogleCalendarAccess(user); + } catch (accessError) { + errors.push(`Failed to revoke access: ${accessError.message}`); + } } + + if (errors.length > 0) { + throw new Error(`Revocation partially failed: ${errors.join(', ')}`); + }🧰 Tools
🪛 Biome
[error] 172-172: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/backend/src/services/integration/calendar.service.js (2)
Line range hint
233-255: Add error handling and improve metadata initialization.While the webhook setup logic is correct, consider adding error handling and making the metadata initialization more defensive.
const setUpCalendarWatch = async (accessToken, calendarId, webhookUrl, user) => { const auth = new google.auth.OAuth2(); auth.setCredentials({ access_token: accessToken }); const requestBody = { id: uuid(), type: 'web_hook', address: webhookUrl, token: environment.CALENDAR_WEBHOOK_SECRET }; const calendar = google.calendar({ version: 'v3' }); + try { const response = await calendar.events.watch({ auth, calendarId, requestBody }); - user.integration.googleCalendar.metadata = user.integration.googleCalendar.metadata || {}; + // Ensure all required objects exist + user.integration = user.integration || {}; + user.integration.googleCalendar = user.integration.googleCalendar || {}; + user.integration.googleCalendar.metadata = user.integration.googleCalendar.metadata || {}; user.integration.googleCalendar.metadata.channelId = response.data.id; user.integration.googleCalendar.metadata.resourceId = response.data.resourceId; await user.save(); return response.data; + } catch (error) { + console.error('Failed to set up calendar watch:', error); + throw new Error('Failed to set up calendar webhook'); + } };
324-356: Consider implementing atomic operations for access revocation.The revocation process involves multiple steps (token revocation, webhook removal, and user data cleanup). Consider implementing these operations atomically to maintain data consistency in case of failures.
Suggestions:
- Create a higher-level function that coordinates these operations
- Implement rollback mechanisms for partial failures
- Consider using database transactions for user data updates
Example implementation:
const revokeGoogleCalendarAccessAndCleanup = async (user) => { try { // 1. Remove webhook first (if exists) if (user?.integration?.googleCalendar?.metadata?.channelId) { await removeGoogleCalendarWebhook( user.integration.googleCalendar.metadata.channelId, user.integration.googleCalendar.metadata.resourceId, user.integration.googleCalendar.accessToken ); } // 2. Revoke access await revokeGoogleCalendarAccess(user); return true; } catch (error) { console.error('Failed to revoke access and cleanup:', error); // Implement rollback logic here if needed throw new Error('Failed to complete calendar cleanup'); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
apps/backend/src/controllers/integration/calendar.controller.js(3 hunks)apps/backend/src/models/core/user.model.js(1 hunks)apps/backend/src/routers/integration/calendar.route.js(1 hunks)apps/backend/src/services/integration/calendar.service.js(4 hunks)
🧰 Additional context used
🪛 Biome
apps/backend/src/controllers/integration/calendar.controller.js
[error] 172-172: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (6)
apps/backend/src/routers/integration/calendar.route.js (1)
Line range hint 1-17: Verify the impact of removing the Google Calendar connect route.
The removal of the "/connect/" route (previously using redirectGoogleCalendarOAuthLoginController) might affect:
- Client applications that depend on this endpoint
- Documentation that references this endpoint
- User onboarding flows
✅ Verification successful
The removal of the /connect route appears to be a safe refactoring
The codebase analysis shows that:
- The OAuth flow is handled through the Google OAuth2 client configuration in
google.loader.js - The calendar service manages connection state internally via user.integration.googleCalendar.connected
- No direct references to a
/connectendpoint were found in frontend components or hooks - Other integration routes (email, notion, linear) have their own OAuth flows
The removal of the /connect route seems to be part of a refactoring where the OAuth flow is likely handled through the /getAccessToken endpoint instead, maintaining backward compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for references to the removed connect route
echo "Searching for references to the connect route in the codebase..."
rg -l "\/connect\/" --type js --type ts --type jsx --type tsx
echo "Searching for documentation files that might reference the endpoint..."
fd -e md -e mdx | xargs rg -l "\/connect\/"
echo "Searching for potential client-side API calls..."
rg -l "googleCalendar.*connect" --type js --type ts --type jsx --type tsx
Length of output: 766
Script:
#!/bin/bash
# Search for references to the connect route in the codebase with fixed regex
echo "Searching for references to the connect route in the codebase..."
rg -l "/connect/" --type js --type ts
echo "Searching for documentation files that might reference the endpoint..."
fd -e md -e mdx | xargs rg -l "connect"
echo "Searching for the old controller in the codebase..."
rg -l "redirectGoogleCalendarOAuthLoginController" --type js --type ts
# Search for any Google Calendar connect/OAuth related code
echo "Searching for Google Calendar OAuth related code..."
rg -l "googleCalendar.*[Cc]onnect|[Oo]auth" --type js --type ts
Length of output: 1619
Script:
#!/bin/bash
# Let's check the content of relevant calendar-related files to understand the OAuth flow
echo "Checking calendar service implementation..."
rg -A 5 "OAuth|connect" apps/backend/src/services/integration/calendar.service.js
echo "Checking calendar-related frontend components..."
rg -A 5 "OAuth|connect" apps/frontend/src/components/atoms/GoogleLogin.tsx
echo "Checking calendar hook implementation..."
rg -A 5 "OAuth|connect" apps/frontend/src/hooks/useCalendar.ts
echo "Checking Google loader implementation..."
rg -A 5 "OAuth|connect" apps/backend/src/loaders/google.loader.js
Length of output: 1590
apps/backend/src/models/core/user.model.js (1)
101-103: Verify the impact of metadata field on existing documents.
The addition of an unstructured metadata field without a default value might affect existing documents or queries.
✅ Verification successful
Let me check the content of the calendar service to understand how the metadata field is being used.
Let me check the Meeting model to understand how the metadata field is being used there.
The metadata field addition is safe to proceed
The verification shows that:
- The metadata field in the Meeting model is already being used consistently in the calendar service for storing meeting-related data (status, location, attendees, etc.)
- For Google Calendar integration, the metadata field is properly initialized with an empty object when revoking access
- No migrations are needed as the field is of type Mixed, which is flexible and won't affect existing documents
- The code handles the case where metadata might not exist by using the OR operator (
metadata = metadata || {})
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for existing documents that might be affected by the schema change
# Test 1: Check if there are any documents with existing metadata field
# Expect: No documents should have this field yet
echo "Checking for existing metadata usage..."
ast-grep --pattern 'integration.googleCalendar.metadata'
# Test 2: Check for queries that might be affected
# Expect: No direct queries on this field
echo "Checking for potential query impacts..."
rg -l "googleCalendar.*metadata" --type js
Length of output: 337
Script:
#!/bin/bash
# Check the calendar service implementation
echo "Checking calendar service implementation..."
rg "metadata" -A 5 -B 5 apps/backend/src/services/integration/calendar.service.js
# Check if this is a new file or existing one
echo -e "\nChecking file history..."
git log --follow -p apps/backend/src/services/integration/calendar.service.js | rg "metadata" -C 5
Length of output: 5078
Script:
#!/bin/bash
# Check the Meeting model implementation and its usage
echo "Checking Meeting model..."
fd "meeting.*model" --type f --exec rg "metadata" -A 5 -B 5 {}
# Check for any existing migrations
echo -e "\nChecking for migrations..."
fd "migration" --type f --exec rg "metadata" -A 5 -B 5 {}
Length of output: 421
apps/backend/src/controllers/integration/calendar.controller.js (3)
12-14: LGTM: Import statements align with PR objectives.
The new imports support the Google Calendar access revocation and webhook removal functionality.
197-198: LGTM: Export changes are consistent.
The exports correctly reflect the addition of the new controller and removal of the OAuth redirect controller.
27-27: Verify setUpCalendarWatch parameter change across codebase.
The setUpCalendarWatch function now accepts a user parameter. Let's verify this change is consistently implemented.
✅ Verification successful
Let me search for the implementation of setUpCalendarWatch to verify the parameter changes.
Parameter change in setUpCalendarWatch is consistently implemented
The function is only called once in calendar.controller.js and its implementation in calendar.service.js correctly defines all four parameters including the new user parameter. The change is consistent across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all calls to setUpCalendarWatch to ensure consistent parameter usage
ast-grep --pattern 'setUpCalendarWatch($$$)'
Length of output: 193
Script:
#!/bin/bash
# Search for the implementation of setUpCalendarWatch
ast-grep --pattern 'function setUpCalendarWatch($$$)'
# Also search with rg to catch any other definitions
rg "setUpCalendarWatch.*=.*" -g "*.js" -A 5
Length of output: 677
apps/backend/src/services/integration/calendar.service.js (1)
370-372: LGTM!
The new functions are correctly exported.
| router.route("/getAccessToken/").get(getGoogleCalendarAccessTokenController) | ||
| router.route("/events/").get(getGoogleCalendarEventsController) | ||
| router.route("/events/").post(addGoogleCalendarEventController) | ||
| router.route("/events/:eventId/").patch(updateGoogleCalendarEventController) | ||
| router.route("/events/:eventId/").delete(deleteGoogleCalendarEventController) | ||
| router.route("/revoke-access/").get(revokeGoogleCalendarAccessController) |
There was a problem hiding this comment.
Use POST/DELETE instead of GET for revocation endpoint.
The revocation endpoint should use POST or DELETE instead of GET since it's a state-changing operation. Using GET for such operations:
- Doesn't follow RESTful principles
- Could be accidentally triggered (e.g., by prefetching)
- May be cached by browsers/proxies
-router.route("/revoke-access/").get(revokeGoogleCalendarAccessController)
+router.route("/revoke-access/").post(revokeGoogleCalendarAccessController)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| router.route("/revoke-access/").get(revokeGoogleCalendarAccessController) | |
| router.route("/revoke-access/").post(revokeGoogleCalendarAccessController) |
| const { accessToken, metadata } = user.integration.googleCalendar || {}; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for user integration.
Consider adding explicit validation for the Google Calendar integration before proceeding with revocation.
- const { accessToken, metadata } = user.integration.googleCalendar || {};
+ if (!user.integration?.googleCalendar) {
+ return res.status(400).json({
+ message: 'No Google Calendar integration found for this user.'
+ });
+ }
+ const { accessToken, metadata } = user.integration.googleCalendar;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { accessToken, metadata } = user.integration.googleCalendar || {}; | |
| if (!user.integration?.googleCalendar) { | |
| return res.status(400).json({ | |
| message: 'No Google Calendar integration found for this user.' | |
| }); | |
| } | |
| const { accessToken, metadata } = user.integration.googleCalendar; |
| res.status(200).json({ | ||
| message: 'Google Calendar access revoked and webhook removed successfully.' | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add metadata cleanup after successful revocation.
The user's Google Calendar metadata should be cleaned up after successful revocation.
+ // Clean up user metadata
+ user.integration.googleCalendar = {};
+ await user.save();
+
res.status(200).json({
message: 'Google Calendar access revoked and webhook removed successfully.'
});Committable suggestion skipped: line range outside the PR's diff.
| const removeGoogleCalendarWebhook = async (channelId, resourceId, accessToken) => { | ||
| const stopWebhookUrl = 'https://www.googleapis.com/calendar/v3/channels/stop'; | ||
|
|
||
| await axios.post(stopWebhookUrl, { | ||
| id: channelId, | ||
| resourceId | ||
| }, { | ||
| headers: { | ||
| Authorization: `Bearer ${accessToken}`, | ||
| 'Content-Type': 'application/json' | ||
| } | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and parameter validation for webhook removal.
The webhook removal implementation should validate inputs and handle potential errors.
const removeGoogleCalendarWebhook = async (channelId, resourceId, accessToken) => {
+ if (!channelId || !resourceId || !accessToken) {
+ throw new Error('Missing required parameters for webhook removal');
+ }
+
const stopWebhookUrl = 'https://www.googleapis.com/calendar/v3/channels/stop';
+ try {
await axios.post(stopWebhookUrl, {
id: channelId,
resourceId
}, {
headers: {
Authorization: `Bearer ${accessToken}`,
'Content-Type': 'application/json'
}
});
+ } catch (error) {
+ console.error('Failed to remove calendar webhook:', error);
+ throw new Error('Failed to remove calendar webhook');
+ }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const removeGoogleCalendarWebhook = async (channelId, resourceId, accessToken) => { | |
| const stopWebhookUrl = 'https://www.googleapis.com/calendar/v3/channels/stop'; | |
| await axios.post(stopWebhookUrl, { | |
| id: channelId, | |
| resourceId | |
| }, { | |
| headers: { | |
| Authorization: `Bearer ${accessToken}`, | |
| 'Content-Type': 'application/json' | |
| } | |
| }); | |
| }; | |
| const removeGoogleCalendarWebhook = async (channelId, resourceId, accessToken) => { | |
| if (!channelId || !resourceId || !accessToken) { | |
| throw new Error('Missing required parameters for webhook removal'); | |
| } | |
| const stopWebhookUrl = 'https://www.googleapis.com/calendar/v3/channels/stop'; | |
| try { | |
| await axios.post(stopWebhookUrl, { | |
| id: channelId, | |
| resourceId | |
| }, { | |
| headers: { | |
| Authorization: `Bearer ${accessToken}`, | |
| 'Content-Type': 'application/json' | |
| } | |
| }); | |
| } catch (error) { | |
| console.error('Failed to remove calendar webhook:', error); | |
| throw new Error('Failed to remove calendar webhook'); | |
| } | |
| }; |
| const revokeGoogleCalendarAccess = async (user) => { | ||
| const revokeTokenUrl = 'https://oauth2.googleapis.com/revoke'; | ||
| const accessToken = user.integration.googleCalendar.accessToken; | ||
|
|
||
| await axios.post(revokeTokenUrl, null, { | ||
| params: { | ||
| token: accessToken | ||
| }, | ||
| headers: { | ||
| 'Content-Type': 'application/x-www-form-urlencoded' | ||
| } | ||
| }); | ||
|
|
||
| user.integration.googleCalendar.accessToken = null; | ||
| user.integration.googleCalendar.refreshToken = null; | ||
| user.integration.googleCalendar.connected = false; | ||
| user.integration.googleCalendar.metadata = {}; | ||
| await user.save(); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add comprehensive error handling for token revocation.
The token revocation logic should handle various edge cases and potential failures.
const revokeGoogleCalendarAccess = async (user) => {
+ try {
+ // Validate user has required data
+ if (!user?.integration?.googleCalendar?.accessToken) {
+ console.log('No access token found to revoke');
+ return;
+ }
+
const revokeTokenUrl = 'https://oauth2.googleapis.com/revoke';
const accessToken = user.integration.googleCalendar.accessToken;
await axios.post(revokeTokenUrl, null, {
params: {
token: accessToken
},
headers: {
'Content-Type': 'application/x-www-form-urlencoded'
}
});
+ // Ensure integration object exists
+ user.integration = user.integration || {};
+ user.integration.googleCalendar = user.integration.googleCalendar || {};
+
user.integration.googleCalendar.accessToken = null;
user.integration.googleCalendar.refreshToken = null;
user.integration.googleCalendar.connected = false;
user.integration.googleCalendar.metadata = {};
await user.save();
+ } catch (error) {
+ console.error('Failed to revoke Google Calendar access:', error);
+ throw new Error('Failed to revoke calendar access');
+ }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const revokeGoogleCalendarAccess = async (user) => { | |
| const revokeTokenUrl = 'https://oauth2.googleapis.com/revoke'; | |
| const accessToken = user.integration.googleCalendar.accessToken; | |
| await axios.post(revokeTokenUrl, null, { | |
| params: { | |
| token: accessToken | |
| }, | |
| headers: { | |
| 'Content-Type': 'application/x-www-form-urlencoded' | |
| } | |
| }); | |
| user.integration.googleCalendar.accessToken = null; | |
| user.integration.googleCalendar.refreshToken = null; | |
| user.integration.googleCalendar.connected = false; | |
| user.integration.googleCalendar.metadata = {}; | |
| await user.save(); | |
| }; | |
| const revokeGoogleCalendarAccess = async (user) => { | |
| try { | |
| // Validate user has required data | |
| if (!user?.integration?.googleCalendar?.accessToken) { | |
| console.log('No access token found to revoke'); | |
| return; | |
| } | |
| const revokeTokenUrl = 'https://oauth2.googleapis.com/revoke'; | |
| const accessToken = user.integration.googleCalendar.accessToken; | |
| await axios.post(revokeTokenUrl, null, { | |
| params: { | |
| token: accessToken | |
| }, | |
| headers: { | |
| 'Content-Type': 'application/x-www-form-urlencoded' | |
| } | |
| }); | |
| // Ensure integration object exists | |
| user.integration = user.integration || {}; | |
| user.integration.googleCalendar = user.integration.googleCalendar || {}; | |
| user.integration.googleCalendar.accessToken = null; | |
| user.integration.googleCalendar.refreshToken = null; | |
| user.integration.googleCalendar.connected = false; | |
| user.integration.googleCalendar.metadata = {}; | |
| await user.save(); | |
| } catch (error) { | |
| console.error('Failed to revoke Google Calendar access:', error); | |
| throw new Error('Failed to revoke calendar access'); | |
| } | |
| }; |
What does this PR do?
Google Calendar access revoked and webhook removed
Fixes:
Mandatory Tasks:
How should this be tested?
Environment Setup: ( if applicable)
Checklist: