Skip to content

Commit 764d091

Browse files
tyler-daneclaude
andauthored
fix: google revocation handling (#1484)
* refactor(issue-templates): simplify feature request and bug report templates - Removed unnecessary emoji from template names for a cleaner presentation. - Eliminated the priority dropdown from the feature request template to streamline the submission process. - Updated the bug report template to remove the emoji, enhancing consistency across issue templates. - Adjusted the configuration file to reflect these changes, ensuring clarity in the issue submission process. * test(socket): enhance SocketProvider tests with async act calls - Updated SocketProvider tests to utilize `act` for asynchronous callback invocations, ensuring proper handling of state updates during testing. - Improved test reliability by wrapping callback executions in `act`, aligning with React's testing best practices. * test(socket): add race condition handling test for useGcalSync - Introduced a new test case to verify the correct handling of import end events when the awaitingImportResults state changes mid-render. - Utilized a ref to prevent stale closures, ensuring that the correct state is referenced during socket event processing. - Enhanced the reliability of the useGcalSync hook by ensuring it properly processes events even when state changes occur asynchronously. * delete(svg): remove unused circle.svg file - Deleted the circle.svg file from the public SVG assets as it is no longer needed in the project. - This cleanup helps streamline the asset management and reduces unnecessary file clutter. * fix(socket): address PR review comments for useGcalSync - Update ref synchronously during render instead of in useEffect to fully eliminate race condition window - Rename misleading test name (timeout -> successfully) - Fix test assertions to validate action creator calls directly instead of using unused variable and dispatch-based assertions Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor(sync): rename awaitingImportResults to isImportPending - Updated state management and selectors to reflect the new naming convention for clarity. - Adjusted related components and tests to ensure consistency with the updated state name. - This change enhances code readability and aligns with the overall naming strategy in the project. * refactor(socket): simplify reconnect logic and update test case - Removed the 1-second delay in the reconnect function, allowing for immediate reconnection after disconnection. - Updated the corresponding test case to reflect the change in behavior, ensuring it accurately tests the immediate reconnection functionality. - This change enhances the responsiveness of the socket client and improves the clarity of the test case. * refactor(tests): update useGcalSync tests to remove fake timers - Removed the use of fake timers in the tests for useGcalSync, simplifying the test setup. - Updated type definitions for event handler variables to allow for undefined values, enhancing type safety. - This change improves test clarity and aligns with best practices for handling asynchronous events in React testing. * test(socket): add integration tests for Google Calendar re-authentication flow - Introduced a new test suite for the Google Calendar re-authentication process, validating user experience during import operations. - Implemented tests to ensure the spinner visibility during import initiation, handling of socket events, and correct state updates upon import completion. - Enhanced test reliability by utilizing `act` for asynchronous operations and capturing socket event callbacks. - This addition improves coverage for the re-authentication flow and ensures a seamless user experience during Google Calendar synchronization. * refactor(sync): rename setAwaitingImportResults to setIsImportPending - Updated the Redux action and corresponding function names to improve clarity and consistency in the import state management. - Adjusted all related components and tests to reflect the new naming convention, ensuring seamless integration across the codebase. - This change enhances code readability and aligns with the overall naming strategy in the project. * feat(errors): enhance Google token error handling and add corresponding tests - Updated the error handling for invalid Google tokens to prune user data and notify clients via WebSocket. - Modified the response to include a specific payload indicating Google access has been revoked. - Added unit tests to verify the new error handling behavior, ensuring proper response and state management during Google token invalidation. - This change improves user experience by providing clearer feedback on session status and data management. * feat(user): implement pruneGoogleData method and corresponding tests - Added the pruneGoogleData method to UserService, which stops Google Calendar sync and removes the Google field from the user document. - Introduced a new test suite for pruneGoogleData, verifying that user data is correctly pruned and associated events are deleted. - This enhancement improves data management and user experience by ensuring that outdated Google data is effectively removed. * feat(sync): implement Google revoked access handling and notifications - Added functionality to handle Google access revocation by pruning user data and notifying clients via WebSocket. - Updated SyncController to respond with a structured message when access is revoked, improving user feedback. - Introduced tests to verify the new behavior, ensuring proper handling of revoked access scenarios. - This enhancement improves data management and user experience by effectively managing Google Calendar access changes. * feat(api): implement Google revoked access handling in CompassApi - Added handling for Google access revocation in CompassApi, ensuring users remain logged in while displaying a toast notification. - Introduced a new utility function to extract error codes from Axios errors, enhancing error management. - Updated tests to verify the new behavior, including scenarios for Google revoked access. - This enhancement improves user experience by providing clear feedback and managing Google Calendar data effectively. * chore: fix conflicts in useGcalSync * refactor: revert try/catch in event.controller * test(compass): enhance Google revoked access handling tests - Updated the test for handling Google access revocation to ensure proper behavior on 401/410 responses. - Introduced a new payload structure for the GOOGLE_REVOKED error code and verified that the correct toast notification is displayed. - Improved test coverage by iterating over multiple status codes to validate consistent error handling and user feedback. * test(socket): enhance useGcalSync tests with timer management - Added beforeEach and afterEach hooks to manage fake timers in the useGcalSync test suite. - This change improves test reliability by ensuring proper timer handling during asynchronous operations. * refactor(errors): improve Google error handling type safety - Updated error handling functions to accept `unknown` type instead of specific error types, enhancing type safety and flexibility. - Adjusted the `handleExpressError` and `isFullSyncRequired` functions to work with the new type definitions, ensuring consistent error management across the application. - This change improves code robustness and prepares the error handling system for a wider range of error scenarios. * feat(sync): implement deleteWatchesByUser and enhance Google error handling - Introduced the deleteWatchesByUser method in SyncService to remove watch records for a specific user, improving data management. - Added createGoogleError utility for generating mock Google errors, enhancing test coverage and error handling. - Updated gcal.utils to include getGoogleErrorStatus for better error status retrieval from Google errors. - Enhanced tests for SyncService to validate behavior when handling Google errors, ensuring robust error management. * feat(auth): implement handleGoogleRevoked utility for improved Google access management - Introduced the handleGoogleRevoked function to centralize handling of Google access revocation, including toast notifications and event management. - Updated CompassApi and useGcalSync to utilize the new utility, ensuring consistent behavior across the application. - Enhanced tests for handleGoogleRevoked to verify correct toast display and event dispatching, improving test coverage and reliability. - This change enhances user experience by providing clear feedback and managing Google Calendar data effectively upon access revocation. * fix(event-test-utils): update keyboard shortcut for saving event forms - Changed the keyboard shortcut for saving event forms from "Enter" to "Meta+Enter" to accommodate different operating systems (Cmd+Enter on Mac, Win+Enter on Windows/Linux). - This update ensures consistent behavior across platforms when saving event data. * refactor(event-form): unify keyboard shortcuts for form submission - Updated keyboard shortcuts for submitting event forms to use "mod+enter" for consistency across platforms (Cmd+Enter on Mac, Ctrl+Enter on Windows/Linux). - Adjusted related tests and utility functions to reflect the new shortcut, ensuring reliable behavior in form submissions. - This change enhances user experience by providing a consistent interaction model across different operating systems. --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent ddd0437 commit 764d091

30 files changed

Lines changed: 801 additions & 50 deletions

e2e/utils/event-test-utils.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,9 @@ export const fillTitleAndSaveWithKeyboard = async (
212212
const titleInput = getFormTitleInput(page);
213213
await expect(titleInput).toBeVisible({ timeout: FORM_TIMEOUT });
214214
await titleInput.fill(title);
215-
await page.keyboard.press("Enter");
215+
// EventForm saves on Cmd+Enter (Mac) or Ctrl+Enter (Linux/Windows)
216+
// ControlOrMeta maps to the platform-appropriate modifier
217+
await page.keyboard.press("ControlOrMeta+Enter");
216218
// Wait for form to close, confirming the save completed
217219
await titleInput.waitFor({ state: "hidden", timeout: FORM_TIMEOUT });
218220
};
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import { GaxiosError } from "gaxios";
2+
3+
export const createGoogleError = (
4+
overrides: {
5+
code?: string | number;
6+
responseStatus?: number;
7+
message?: string;
8+
} = {},
9+
) => {
10+
const url = new URL("https://www.googleapis.com/calendar/v3");
11+
const headers = new Headers();
12+
13+
const error = new GaxiosError(
14+
overrides.message ?? "test google error",
15+
{
16+
headers,
17+
url,
18+
},
19+
overrides.responseStatus
20+
? {
21+
config: {
22+
headers,
23+
url,
24+
},
25+
status: overrides.responseStatus,
26+
statusText: "ERROR",
27+
headers,
28+
data: {},
29+
ok: false,
30+
redirected: false,
31+
type: "error" as ResponseType,
32+
url: url.toString(),
33+
body: null,
34+
bodyUsed: false,
35+
clone: () => {
36+
throw new Error("Not implemented");
37+
},
38+
arrayBuffer: async () => {
39+
throw new Error("Not implemented");
40+
},
41+
blob: async () => {
42+
throw new Error("Not implemented");
43+
},
44+
formData: async () => {
45+
throw new Error("Not implemented");
46+
},
47+
json: async () => ({}),
48+
text: async () => "",
49+
bytes: async () => {
50+
throw new Error("Not implemented");
51+
},
52+
}
53+
: undefined,
54+
);
55+
56+
error.code = overrides.code;
57+
58+
return error;
59+
};

packages/backend/src/common/errors/handlers/error.express.handler.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { Request } from "express";
22
import { GaxiosError } from "gaxios";
33
import { SessionRequest } from "supertokens-node/framework/express";
4+
import { GOOGLE_REVOKED } from "@core/constants/websocket.constants";
45
import { BaseError } from "@core/errors/errors.base";
56
import { Status } from "@core/errors/status.codes";
67
import { Logger } from "@core/logger/winston.logger";
@@ -19,6 +20,7 @@ import {
1920
} from "@backend/common/services/gcal/gcal.utils";
2021
import { CompassError, Info_Error } from "@backend/common/types/error.types";
2122
import { SessionResponse } from "@backend/common/types/express.types";
23+
import { webSocketServer } from "@backend/servers/websocket/websocket.server";
2224
import { getSyncByToken } from "@backend/sync/util/sync.queries";
2325
import { findCompassUserBy } from "@backend/user/queries/user.queries";
2426
import userService from "@backend/user/services/user.service";
@@ -91,7 +93,7 @@ export const handleExpressError = async (
9193
}
9294

9395
if (isGoogleError(e)) {
94-
await handleGoogleError(req, res, userId, e);
96+
await handleGoogleError(req, res, userId, e as GaxiosError);
9597
} else {
9698
const errInfo = assembleErrorInfo(e);
9799
res.status(e.status || Status.INTERNAL_SERVER).send(errInfo);
@@ -104,20 +106,23 @@ export const handleExpressError = async (
104106
};
105107

106108
const handleGoogleError = async (
107-
req: Request | SessionRequest,
109+
_req: Request | SessionRequest,
108110
res: SessionResponse,
109111
userId: string,
110112
e: GaxiosError,
111113
) => {
112114
if (isInvalidGoogleToken(e)) {
113-
await req.session?.revokeSession();
115+
await userService.pruneGoogleData(userId);
116+
webSocketServer.handleGoogleRevoked(userId);
114117

115-
// revoke specific sessions for this user
116-
logger.debug(
117-
`Invalid Google token for user: ${userId}\n\tsession revoked as result`,
118+
logger.warn(
119+
`Invalid Google token for user: ${userId}. Google data pruned and client notified.`,
118120
);
119121

120-
res.status(Status.UNAUTHORIZED).send();
122+
res.status(Status.UNAUTHORIZED).send({
123+
code: GOOGLE_REVOKED,
124+
message: "Google access revoked. Your Google data has been removed.",
125+
});
121126
return;
122127
}
123128

packages/backend/src/common/errors/handlers/error.handler.test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
1+
import { GOOGLE_REVOKED } from "@core/constants/websocket.constants";
12
import { BaseError } from "@core/errors/errors.base";
23
import { Status } from "@core/errors/status.codes";
4+
import { invalidGrant400Error } from "@backend/__tests__/mocks.gcal/errors/error.google.invalidGrant";
5+
import { handleExpressError } from "@backend/common/errors/handlers/error.express.handler";
36
import {
47
error,
8+
errorHandler,
59
toClientErrorPayload,
610
} from "@backend/common/errors/handlers/error.handler";
711
import { UserError } from "@backend/common/errors/user/user.errors";
12+
import { webSocketServer } from "@backend/servers/websocket/websocket.server";
13+
import userService from "@backend/user/services/user.service";
814

915
describe("error.handler", () => {
1016
describe("toClientErrorPayload", () => {
@@ -38,4 +44,38 @@ describe("error.handler", () => {
3844
expect(Object.keys(payload)).toEqual(["result", "message"]);
3945
});
4046
});
47+
48+
describe("handleExpressError", () => {
49+
it("returns 401 with GOOGLE_REVOKED payload when Google token is invalid", async () => {
50+
const userId = "507f1f77bcf86cd799439011";
51+
jest.spyOn(userService, "pruneGoogleData").mockResolvedValue();
52+
const handleGoogleRevokedSpy = jest.spyOn(
53+
webSocketServer,
54+
"handleGoogleRevoked",
55+
);
56+
handleGoogleRevokedSpy.mockImplementation(() => undefined);
57+
jest.spyOn(errorHandler, "isOperational").mockReturnValue(true);
58+
59+
const send = jest.fn();
60+
const res = {
61+
header: jest.fn().mockReturnThis(),
62+
status: jest.fn().mockReturnThis(),
63+
send,
64+
} as unknown as Parameters<typeof handleExpressError>[1];
65+
const req = {
66+
session: { getUserId: () => userId },
67+
} as Parameters<typeof handleExpressError>[0];
68+
(res as { req?: typeof req }).req = req;
69+
70+
await handleExpressError(req, res, invalidGrant400Error);
71+
72+
expect(res.status).toHaveBeenCalledWith(Status.UNAUTHORIZED);
73+
expect(send).toHaveBeenCalledWith({
74+
code: GOOGLE_REVOKED,
75+
message: "Google access revoked. Your Google data has been removed.",
76+
});
77+
expect(handleGoogleRevokedSpy).toHaveBeenCalledWith(userId);
78+
handleGoogleRevokedSpy.mockRestore();
79+
});
80+
});
4181
});

packages/backend/src/common/services/gcal/gcal.util.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1+
import { createGoogleError } from "../../../__tests__/mocks.gcal/errors/error.google.factory";
12
import { invalidGrant400Error } from "../../../__tests__/mocks.gcal/errors/error.google.invalidGrant";
23
import { invalidValueError } from "../../../__tests__/mocks.gcal/errors/error.google.invalidValue";
34
import { invalidSyncTokenError } from "../../../__tests__/mocks.gcal/errors/error.invalidSyncToken";
45
import {
56
getEmailFromUrl,
7+
getGoogleErrorStatus,
68
isFullSyncRequired,
79
isInvalidGoogleToken,
810
isInvalidValue,
@@ -18,6 +20,19 @@ describe("Google Error Parsing", () => {
1820
it("recognizes expired refresh token", () => {
1921
expect(isInvalidGoogleToken(invalidGrant400Error)).toBe(true);
2022
});
23+
it("returns response status when present", () => {
24+
expect(
25+
getGoogleErrorStatus(
26+
createGoogleError({ code: "500", responseStatus: 401 }),
27+
),
28+
).toBe(401);
29+
});
30+
it("falls back to the parsed gaxios code", () => {
31+
expect(getGoogleErrorStatus(createGoogleError({ code: "410" }))).toBe(410);
32+
});
33+
it("returns undefined for non-google errors", () => {
34+
expect(getGoogleErrorStatus(new Error("nope"))).toBeUndefined();
35+
});
2136
});
2237

2338
describe("Gaxios response parsing", () => {

packages/backend/src/common/services/gcal/gcal.utils.ts

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -42,27 +42,40 @@ export const getEmailFromUrl = (url: string) => {
4242
};
4343

4444
// occurs when token expired or revoked
45-
export const isInvalidGoogleToken = (e: GaxiosError | Error) => {
46-
const is400 = "code" in e && e.code === "400";
47-
const hasInvalidMsg = "message" in e && e.message === "invalid_grant";
48-
const isInvalid = is400 && hasInvalidMsg;
45+
export const isInvalidGoogleToken = (e: unknown) => {
46+
if (!isGoogleError(e)) return false;
4947

50-
return isGoogleError(e) && isInvalid;
48+
const err = e as GaxiosError;
49+
const is400 = err.code === "400" || err.response?.status === 400;
50+
const hasInvalidMsg = err.message === "invalid_grant";
51+
const hasInvalidData = err.response?.data?.error === "invalid_grant";
52+
53+
return is400 && (hasInvalidMsg || hasInvalidData);
5154
};
5255

5356
export const isGoogleError = (e: unknown) => {
54-
return e instanceof GaxiosError;
57+
return e instanceof GaxiosError || (e as any)?.name === "GaxiosError";
5558
};
5659

57-
export const isFullSyncRequired = (e: GaxiosError | Error) => {
58-
if (isGoogleError(e) && e.code) {
59-
const codeStr = typeof e.code === "string" ? e.code : String(e.code);
60-
if (parseInt(codeStr) === 410) {
61-
return true;
62-
}
60+
export const getGoogleErrorStatus = (e: unknown) => {
61+
if (!isGoogleError(e)) return undefined;
62+
63+
const err = e as GaxiosError;
64+
const responseStatus = err.response?.status;
65+
66+
if (typeof responseStatus === "number") return responseStatus;
67+
if (typeof err.code === "number") return err.code;
68+
69+
if (typeof err.code === "string") {
70+
const code = Number.parseInt(err.code, 10);
71+
if (!Number.isNaN(code)) return code;
6372
}
6473

65-
return false;
74+
return undefined;
75+
};
76+
77+
export const isFullSyncRequired = (e: unknown) => {
78+
return getGoogleErrorStatus(e) === 410;
6679
};
6780

6881
export const isInvalidValue = (e: GaxiosError) => {

packages/backend/src/servers/websocket/websocket.server.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
EVENT_CHANGED,
77
EVENT_CHANGE_PROCESSED,
88
FETCH_USER_METADATA,
9+
GOOGLE_REVOKED,
910
IMPORT_GCAL_END,
1011
IMPORT_GCAL_START,
1112
RESULT_IGNORED,
@@ -236,6 +237,10 @@ class WebSocketServer {
236237
handleBackgroundSomedayChange(userId: string) {
237238
return this.notifyUser(userId, SOMEDAY_EVENT_CHANGED);
238239
}
240+
241+
handleGoogleRevoked(userId: string) {
242+
return this.notifyUser(userId, GOOGLE_REVOKED);
243+
}
239244
}
240245

241246
export const webSocketServer = new WebSocketServer();

packages/backend/src/sync/controllers/sync.controller.test.ts

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ import { randomUUID } from "node:crypto";
33
import { DefaultEventsMap } from "socket.io";
44
import { Socket } from "socket.io-client";
55
import { faker } from "@faker-js/faker";
6-
import { EVENT_CHANGED } from "@core/constants/websocket.constants";
6+
import {
7+
EVENT_CHANGED,
8+
GOOGLE_REVOKED,
9+
} from "@core/constants/websocket.constants";
710
import { Status } from "@core/errors/status.codes";
811
import { Resource_Sync, XGoogleResourceState } from "@core/types/sync.types";
912
import { Schema_User } from "@core/types/user.types";
@@ -23,12 +26,16 @@ import {
2326
cleanupTestDb,
2427
setupTestDb,
2528
} from "@backend/__tests__/helpers/mock.db.setup";
29+
import { invalidGrant400Error } from "@backend/__tests__/mocks.gcal/errors/error.google.invalidGrant";
2630
import { WatchError } from "@backend/common/errors/sync/watch.errors";
2731
import gcalService from "@backend/common/services/gcal/gcal.service";
2832
import mongoService from "@backend/common/services/mongo.service";
33+
import { webSocketServer } from "@backend/servers/websocket/websocket.server";
34+
import syncService from "@backend/sync/services/sync.service";
2935
import * as syncQueries from "@backend/sync/util/sync.queries";
3036
import { updateSync } from "@backend/sync/util/sync.queries";
3137
import userMetadataService from "@backend/user/services/user-metadata.service";
38+
import userService from "@backend/user/services/user.service";
3239

3340
describe("SyncController", () => {
3441
const baseDriver = new BaseDriver();
@@ -184,6 +191,54 @@ describe("SyncController", () => {
184191

185192
expect(response.text).toEqual("IGNORED");
186193
});
194+
195+
it("should prune Google data, notify client via websocket, and return structured response when user revokes access", async () => {
196+
const { user } = await UtilDriver.setupTestUser();
197+
const userId = user._id.toString();
198+
199+
const watch = await mongoService.watch.findOne({
200+
user: userId,
201+
gCalendarId: { $ne: Resource_Sync.CALENDAR },
202+
});
203+
204+
expect(watch).toBeDefined();
205+
expect(watch).not.toBeNull();
206+
207+
const handleGcalNotificationSpy = jest
208+
.spyOn(syncService, "handleGcalNotification")
209+
.mockRejectedValue(invalidGrant400Error);
210+
211+
const pruneGoogleDataSpy = jest
212+
.spyOn(userService, "pruneGoogleData")
213+
.mockResolvedValue();
214+
215+
const handleGoogleRevokedSpy = jest.spyOn(
216+
webSocketServer,
217+
"handleGoogleRevoked",
218+
);
219+
220+
const response = await syncDriver.handleGoogleNotification(
221+
{
222+
resource: Resource_Sync.EVENTS,
223+
channelId: watch!._id,
224+
resourceId: watch!.resourceId,
225+
resourceState: XGoogleResourceState.EXISTS,
226+
expiration: watch!.expiration,
227+
},
228+
Status.GONE,
229+
);
230+
231+
expect(response.body).toEqual({
232+
code: GOOGLE_REVOKED,
233+
message: "User revoked access, pruned Google data",
234+
});
235+
expect(pruneGoogleDataSpy).toHaveBeenCalledWith(userId);
236+
expect(handleGoogleRevokedSpy).toHaveBeenCalledWith(userId);
237+
238+
handleGcalNotificationSpy.mockRestore();
239+
pruneGoogleDataSpy.mockRestore();
240+
handleGoogleRevokedSpy.mockRestore();
241+
});
187242
});
188243

189244
describe("importGCal: ", () => {

0 commit comments

Comments
 (0)