Skip to content

Commit dd8b6b4

Browse files
authored
Recover expired Google connect callbacks (#1834)
* fix(web): recover expired google connect callbacks * test(web): cover google callback session recovery * refactor(web): name google auth api error shape
1 parent 2799c91 commit dd8b6b4

16 files changed

Lines changed: 334 additions & 102 deletions

File tree

CONTEXT.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ An authenticated user with usable Google credentials stored by the backend.
4545

4646
**Google authorization**:
4747
The Google approval step that lets Compass sign a user in with Google or connect
48-
Google Calendar to an existing Compass session.
48+
Google Calendar to an active existing Compass session.
4949
_Avoid_: Google login mode
5050

5151
**Google authorization intent**:
@@ -256,6 +256,10 @@ during Import or Public watch notification handling.
256256
event changes to Google.
257257
- A **Google authorization** must preserve its **Google authorization intent**
258258
instead of inferring the user's goal from the later session state.
259+
- A Google Calendar connect/reconnect **Google authorization intent** requires
260+
an active **Authenticated user** session. If that session is missing or
261+
expired at callback time, Compass should recover through Google sign-in
262+
instead of calling the authenticated Google connect endpoint.
259263
- **Public watch notifications** are separate from browser API and **SSE**
260264
traffic; browser traffic can be local, but Google webhook posts need public
261265
HTTPS when continuous sync is expected.

bun.lock

Lines changed: 0 additions & 22 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

e2e/oauth/google-auth-callback.spec.ts

Lines changed: 72 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,23 @@ const getCallbackUrl = (state: string, scope = REQUIRED_SCOPES.join(" ")) =>
4040
state,
4141
)}&code=auth-code&scope=${encodeURIComponent(scope)}`;
4242

43+
const expectGoogleAuthRequestBody = (
44+
request: CapturedAuthRequest | undefined,
45+
state: string,
46+
) => {
47+
expect(request?.body).toMatchObject({
48+
thirdPartyId: "google",
49+
clientType: "web",
50+
redirectURIInfo: {
51+
redirectURIOnProviderDashboard: expect.stringContaining(CALLBACK_PATH),
52+
redirectURIQueryParams: {
53+
code: "auth-code",
54+
state,
55+
},
56+
},
57+
});
58+
};
59+
4360
const writeGoogleAuthorizationIntent = async ({
4461
intent,
4562
page,
@@ -67,6 +84,34 @@ const writeGoogleAuthorizationIntent = async ({
6784
);
6885
};
6986

87+
const setActiveCompassSession = async (page: Page) => {
88+
const now = Date.now();
89+
const pageOrigin = new URL(page.url()).origin;
90+
const expires = Math.floor(now / 1000) + 60 * 60;
91+
const frontToken = Buffer.from(
92+
JSON.stringify({
93+
ate: now + 60 * 60 * 1000,
94+
uid: "test-user-id",
95+
up: {},
96+
}),
97+
).toString("base64");
98+
99+
await page.context().addCookies([
100+
{
101+
expires,
102+
name: "st-last-access-token-update",
103+
url: pageOrigin,
104+
value: now.toString(),
105+
},
106+
{
107+
expires,
108+
name: "sFrontToken",
109+
url: pageOrigin,
110+
value: frontToken,
111+
},
112+
]);
113+
};
114+
70115
const prepareGoogleAuthCallbackPage = async (
71116
page: Page,
72117
options: ApiMockOptions = {},
@@ -186,17 +231,7 @@ test.describe("Google auth callback", () => {
186231
expect(apiMocks.loginOrSignup).toHaveLength(1);
187232
expect(apiMocks.connectGoogle).toHaveLength(0);
188233
expect(apiMocks.loginOrSignup[0]?.headers.rid).toBe("thirdparty");
189-
expect(apiMocks.loginOrSignup[0]?.body).toMatchObject({
190-
thirdPartyId: "google",
191-
clientType: "web",
192-
redirectURIInfo: {
193-
redirectURIOnProviderDashboard: expect.stringContaining(CALLBACK_PATH),
194-
redirectURIQueryParams: {
195-
code: "auth-code",
196-
state,
197-
},
198-
},
199-
});
234+
expectGoogleAuthRequestBody(apiMocks.loginOrSignup[0], state);
200235
expect(
201236
await page.evaluate(
202237
(key) => sessionStorage.getItem(key),
@@ -205,7 +240,9 @@ test.describe("Google auth callback", () => {
205240
).toBeNull();
206241
});
207242

208-
test("finishes a saved Google Calendar connect intent", async ({ page }) => {
243+
test("finishes a saved Google Calendar connect intent when the Compass session is active", async ({
244+
page,
245+
}) => {
209246
const state = "connect-calendar-state";
210247
const apiMocks = await prepareGoogleAuthCallbackPage(page);
211248

@@ -215,23 +252,36 @@ test.describe("Google auth callback", () => {
215252
returnPath: "/week",
216253
state,
217254
});
255+
await setActiveCompassSession(page);
218256

219257
await page.goto(getCallbackUrl(state));
220258

221259
await expect(page).toHaveURL(/\/week$/);
222260
expect(apiMocks.loginOrSignup).toHaveLength(0);
223261
expect(apiMocks.connectGoogle).toHaveLength(1);
224-
expect(apiMocks.connectGoogle[0]?.body).toMatchObject({
225-
thirdPartyId: "google",
226-
clientType: "web",
227-
redirectURIInfo: {
228-
redirectURIOnProviderDashboard: expect.stringContaining(CALLBACK_PATH),
229-
redirectURIQueryParams: {
230-
code: "auth-code",
231-
state,
232-
},
233-
},
262+
expectGoogleAuthRequestBody(apiMocks.connectGoogle[0], state);
263+
});
264+
265+
test("recovers a saved Google Calendar connect intent through Google sign-in when the Compass session is missing", async ({
266+
page,
267+
}) => {
268+
const state = "connect-calendar-session-missing-state";
269+
const apiMocks = await prepareGoogleAuthCallbackPage(page);
270+
271+
await writeGoogleAuthorizationIntent({
272+
intent: "connectCalendar",
273+
page,
274+
returnPath: "/week",
275+
state,
234276
});
277+
278+
await page.goto(getCallbackUrl(state));
279+
280+
await expect(page).toHaveURL(/\/week$/);
281+
expect(apiMocks.connectGoogle).toHaveLength(0);
282+
expect(apiMocks.loginOrSignup).toHaveLength(1);
283+
expect(apiMocks.loginOrSignup[0]?.headers.rid).toBe("thirdparty");
284+
expectGoogleAuthRequestBody(apiMocks.loginOrSignup[0], state);
235285
});
236286

237287
test("rejects callbacks that are missing required Google Calendar scopes", async ({

packages/backend/package.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
"helmet": "^7.0.0",
1414
"lodash.mergewith": "^4.6.2",
1515
"mongodb": "^7.2.0",
16-
"morgan": "^1.10.0",
1716
"rrule": "^2.7.2",
1817
"saslprep": "^1.0.3",
1918
"supertokens-node": "^23.0.1",
@@ -26,7 +25,6 @@
2625
"@types/express": "^4.17.13",
2726
"@types/jest": "^29.0.0",
2827
"@types/lodash.mergewith": "^4.6.9",
29-
"@types/morgan": "^1.9.4",
3028
"@types/node": "^22.13.10",
3129
"@types/supertest": "^6.0.3",
3230
"jest-environment-node": "^29.7.0",
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import { type Request, type Response } from "express";
2+
import { EventEmitter } from "node:events";
3+
4+
const { httpLoggingMiddleware } = jest.requireActual<
5+
typeof import("@backend/common/middleware/http.logger.middleware")
6+
>("@backend/common/middleware/http.logger.middleware");
7+
8+
describe("httpLoggingMiddleware", () => {
9+
it("logs completed requests without reading the request IP address", () => {
10+
const req = {
11+
method: "GET",
12+
originalUrl: "/api/health",
13+
get ip() {
14+
throw new Error("request IP should not be read");
15+
},
16+
} as unknown as Request;
17+
const res = Object.assign(new EventEmitter(), {
18+
statusCode: 200,
19+
}) as unknown as Response;
20+
const next = jest.fn();
21+
const logSpy = jest.spyOn(console, "log").mockImplementation(() => {});
22+
23+
try {
24+
httpLoggingMiddleware(req, res, next);
25+
res.emit("finish");
26+
27+
expect(next).toHaveBeenCalledTimes(1);
28+
expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("GET"));
29+
expect(logSpy).toHaveBeenCalledWith(
30+
expect.stringContaining("/api/health"),
31+
);
32+
} finally {
33+
logSpy.mockRestore();
34+
}
35+
});
36+
});
Lines changed: 22 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,8 @@
1-
import morgan from "morgan";
2-
import { type IncomingMessage, type ServerResponse } from "node:http";
1+
import { type RequestHandler } from "express";
32
import { styleText } from "node:util";
43

54
type HttpLogColor = "cyanBright" | "yellow" | "red" | "magentaBright";
65

7-
const get = (
8-
key: string,
9-
tokens: morgan.TokenIndexer<IncomingMessage, ServerResponse<IncomingMessage>>,
10-
req?: IncomingMessage,
11-
res?: ServerResponse<IncomingMessage>,
12-
): string => {
13-
const token = tokens[key];
14-
if (token === undefined) {
15-
return "unknown";
16-
}
17-
if (req && res) {
18-
if (typeof token === "function") {
19-
const val = token(req, res);
20-
return val === undefined ? "unknown" : String(val);
21-
}
22-
return String(token);
23-
}
24-
return "unknown";
25-
};
26-
276
const getStatusColor = (status: string): HttpLogColor => {
287
switch (status[0]) {
298
case "1":
@@ -43,18 +22,26 @@ const getStatusColor = (status: string): HttpLogColor => {
4322
}
4423
};
4524

46-
export const httpLoggingMiddleware = morgan((tokens, req, res) => {
47-
const responseTime =
48-
get("response-time", tokens, req, res)?.toString() || "unknown";
25+
export const httpLoggingMiddleware: RequestHandler = (req, res, next) => {
26+
const startTime = process.hrtime.bigint();
27+
28+
res.once("finish", () => {
29+
const elapsedMs = Number(process.hrtime.bigint() - startTime) / 1_000_000;
30+
const status = String(res.statusCode);
31+
const statusColor = getStatusColor(status);
32+
const method = req.method || "unknown";
33+
const url = req.originalUrl || req.url || "unknown";
4934

50-
const status = get("status", tokens, req, res);
51-
const statusColor = getStatusColor(status);
35+
console.log(
36+
[
37+
styleText(["bold", statusColor], status),
38+
styleText(["bold", "whiteBright"], method),
39+
styleText(["bold", "cyanBright"], url),
40+
styleText(["bold", "blueBright"], `${elapsedMs.toFixed(3)}ms`),
41+
styleText(["bold", "magentaBright"], new Date().toUTCString()),
42+
].join(" "),
43+
);
44+
});
5245

53-
return [
54-
styleText(["bold", statusColor], status),
55-
styleText(["bold", "whiteBright"], get("method", tokens, req, res)),
56-
styleText(["bold", "cyanBright"], get("url", tokens, req, res)),
57-
styleText(["bold", "blueBright"], `${responseTime}ms`),
58-
styleText(["bold", "magentaBright"], get("date", tokens, req, res)),
59-
].join(" ");
60-
});
46+
next();
47+
};

0 commit comments

Comments
 (0)