Skip to content

Commit cb68bdb

Browse files
committed
refactor(web): localize google callback recovery
1 parent 4987de9 commit cb68bdb

11 files changed

Lines changed: 219 additions & 118 deletions

File tree

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

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ type ApiMocks = {
2121
type ApiMockOptions = {
2222
beforeConnectGoogleResponse?: Promise<void>;
2323
beforeLoginOrSignupResponse?: Promise<void>;
24+
connectGoogleResponse?: {
25+
body?: unknown;
26+
status: number;
27+
};
2428
};
2529

2630
const createDeferred = () => {
@@ -158,9 +162,9 @@ const prepareGoogleAuthCallbackPage = async (
158162
});
159163
await options.beforeConnectGoogleResponse;
160164
return route.fulfill({
161-
status: 200,
165+
status: options.connectGoogleResponse?.status ?? 200,
162166
contentType: "application/json",
163-
body: JSON.stringify({}),
167+
body: JSON.stringify(options.connectGoogleResponse?.body ?? {}),
164168
});
165169
}
166170

@@ -284,6 +288,35 @@ test.describe("Google auth callback", () => {
284288
expectGoogleAuthRequestBody(apiMocks.loginOrSignup[0], state);
285289
});
286290

291+
test("recovers through Google sign-in when Google connect rejects an expired Compass session", async ({
292+
page,
293+
}) => {
294+
const state = "connect-calendar-session-expired-state";
295+
const apiMocks = await prepareGoogleAuthCallbackPage(page, {
296+
connectGoogleResponse: {
297+
status: 401,
298+
body: { message: "unauthorized" },
299+
},
300+
});
301+
302+
await writeGoogleAuthorizationIntent({
303+
intent: "connectCalendar",
304+
page,
305+
returnPath: "/week",
306+
state,
307+
});
308+
await setActiveCompassSession(page);
309+
310+
await page.goto(getCallbackUrl(state));
311+
312+
await expect(page).toHaveURL(/\/week$/);
313+
expect(apiMocks.connectGoogle).toHaveLength(1);
314+
expect(apiMocks.loginOrSignup).toHaveLength(1);
315+
expect(apiMocks.loginOrSignup[0]?.headers.rid).toBe("thirdparty");
316+
expectGoogleAuthRequestBody(apiMocks.connectGoogle[0], state);
317+
expectGoogleAuthRequestBody(apiMocks.loginOrSignup[0], state);
318+
});
319+
287320
test("rejects callbacks that are missing required Google Calendar scopes", async ({
288321
page,
289322
}) => {

packages/web/src/auth/google/authorization/complete-google-authorization.ts

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,7 @@ import {
33
type GoogleAuthCodeRequest,
44
GoogleConnectErrorResponseSchema,
55
} from "@core/types/auth.types";
6-
import {
7-
type ApiError,
8-
type ApiMethodConfig,
9-
} from "@web/common/apis/api.types";
6+
import { type ApiError } from "@web/common/apis/api.types";
107
import { ROOT_ROUTES } from "@web/common/constants/routes";
118
import {
129
GOOGLE_AUTH_SCOPES_REQUIRED,
@@ -28,10 +25,7 @@ type CompleteAuthentication = (input: {
2825
}) => Promise<void>;
2926

3027
export type GoogleAuthorizationAuthAdapter = {
31-
connectGoogle(
32-
data: GoogleAuthCodeRequest,
33-
config?: ApiMethodConfig,
34-
): Promise<unknown>;
28+
connectGoogle(data: GoogleAuthCodeRequest): Promise<unknown>;
3529
loginOrSignup(data: GoogleAuthCodeRequest): Promise<{
3630
user: { emails?: string[] };
3731
}>;
@@ -149,7 +143,7 @@ export async function completeGoogleAuthorization({
149143
await completeGoogleSignIn();
150144
} else {
151145
try {
152-
await authApi.connectGoogle(payload, { skipSessionRecovery: true });
146+
await authApi.connectGoogle(payload);
153147
await refreshUserMetadata();
154148
requestEventFetch?.();
155149
} catch (error) {
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import { Status } from "@core/errors/status.codes";
2+
import { type GoogleAuthCodeRequest } from "@core/types/auth.types";
3+
import { BaseApi } from "@web/common/apis/base/base.api";
4+
import { session } from "@web/common/classes/Session";
5+
import { GoogleAuthCallbackApi } from "./google-auth-callback.api";
6+
import {
7+
afterEach,
8+
beforeEach,
9+
describe,
10+
expect,
11+
it,
12+
mock,
13+
spyOn,
14+
} from "bun:test";
15+
16+
const originalFetch = globalThis.fetch;
17+
18+
const payload: GoogleAuthCodeRequest = {
19+
clientType: "web",
20+
redirectURIInfo: {
21+
redirectURIOnProviderDashboard:
22+
"http://localhost:9080/auth/google/callback",
23+
redirectURIQueryParams: {
24+
code: "auth-code",
25+
scope: "https://www.googleapis.com/auth/calendar",
26+
state: "state-1",
27+
},
28+
},
29+
thirdPartyId: "google",
30+
};
31+
32+
describe("GoogleAuthCallbackApi", () => {
33+
beforeEach(() => {
34+
BaseApi.defaults.adapter = undefined;
35+
});
36+
37+
afterEach(() => {
38+
globalThis.fetch = originalFetch;
39+
BaseApi.defaults.adapter = undefined;
40+
});
41+
42+
it("lets the callback flow handle an expired connect session locally", async () => {
43+
const signOutSpy = spyOn(session, "signOut").mockResolvedValue(undefined);
44+
globalThis.fetch = mock(async () =>
45+
Promise.resolve(
46+
new Response(JSON.stringify({ message: "unauthorised" }), {
47+
status: Status.UNAUTHORIZED,
48+
}),
49+
),
50+
) as unknown as typeof fetch;
51+
52+
await expect(
53+
GoogleAuthCallbackApi.connectGoogle(payload),
54+
).rejects.toMatchObject({
55+
name: "ApiError",
56+
response: {
57+
status: Status.UNAUTHORIZED,
58+
},
59+
});
60+
61+
expect(signOutSpy).not.toHaveBeenCalled();
62+
expect(globalThis.fetch).toHaveBeenCalledWith(
63+
expect.stringContaining("/auth/google/connect"),
64+
expect.objectContaining({
65+
credentials: "include",
66+
method: "POST",
67+
}),
68+
);
69+
70+
signOutSpy.mockRestore();
71+
});
72+
73+
it("uses shared session handling for non-recoverable connect session errors", async () => {
74+
window.history.pushState({}, "", "/day");
75+
const signOutSpy = spyOn(session, "signOut").mockResolvedValue(undefined);
76+
globalThis.fetch = mock(async () =>
77+
Promise.resolve(
78+
new Response(JSON.stringify({ message: "not found" }), {
79+
status: Status.NOT_FOUND,
80+
}),
81+
),
82+
) as unknown as typeof fetch;
83+
84+
await expect(
85+
GoogleAuthCallbackApi.connectGoogle(payload),
86+
).rejects.toMatchObject({
87+
name: "ApiError",
88+
response: {
89+
status: Status.NOT_FOUND,
90+
},
91+
});
92+
93+
expect(signOutSpy).toHaveBeenCalledTimes(1);
94+
95+
signOutSpy.mockRestore();
96+
});
97+
});
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import { GOOGLE_REVOKED } from "@core/constants/sse.constants";
2+
import { Status } from "@core/errors/status.codes";
3+
import {
4+
type GoogleAuthCodeRequest,
5+
type GoogleConnectResponse,
6+
} from "@core/types/auth.types";
7+
import { type ApiError, type ApiResponse } from "@web/common/apis/api.types";
8+
import { AuthApi } from "@web/common/apis/auth.api";
9+
import { sendApiRequestWithoutSharedErrorRecovery } from "@web/common/apis/base/base.api";
10+
import {
11+
getApiErrorCode,
12+
handleErrorResponse,
13+
isApiError,
14+
} from "@web/common/apis/util/api.util";
15+
import { type GoogleAuthorizationAuthAdapter } from "./complete-google-authorization";
16+
17+
const isRecoverableConnectSessionError = (error: ApiError): boolean => {
18+
return (
19+
error.response?.status === Status.UNAUTHORIZED &&
20+
getApiErrorCode(error) !== GOOGLE_REVOKED
21+
);
22+
};
23+
24+
export const GoogleAuthCallbackApi = {
25+
async connectGoogle(
26+
data: GoogleAuthCodeRequest,
27+
): Promise<GoogleConnectResponse> {
28+
try {
29+
const response =
30+
await sendApiRequestWithoutSharedErrorRecovery<GoogleConnectResponse>(
31+
"POST",
32+
"/auth/google/connect",
33+
data,
34+
);
35+
36+
return response.data;
37+
} catch (error) {
38+
if (!isApiError(error)) {
39+
throw error;
40+
}
41+
42+
if (isRecoverableConnectSessionError(error)) {
43+
throw error;
44+
}
45+
46+
await handleErrorResponse<ApiResponse<GoogleConnectResponse>>(error);
47+
throw error;
48+
}
49+
},
50+
loginOrSignup: AuthApi.loginOrSignup,
51+
} satisfies GoogleAuthorizationAuthAdapter;

packages/web/src/auth/google/authorization/google-authorization.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ describe("completeGoogleAuthorization", () => {
8282
).resolves.toEqual({ status: "completed", returnPath: "/day" });
8383

8484
expect(deps.authApi.connectGoogle).toHaveBeenCalledTimes(1);
85+
expect(deps.authApi.connectGoogle.mock.calls[0]).toHaveLength(1);
8586
expect(deps.refreshUserMetadata).toHaveBeenCalledTimes(1);
8687
expect(deps.requestEventFetch).toHaveBeenCalledTimes(1);
8788
expect(deps.completeAuthentication).not.toHaveBeenCalled();
@@ -136,6 +137,7 @@ describe("completeGoogleAuthorization", () => {
136137
).resolves.toEqual({ status: "completed", returnPath: "/day" });
137138

138139
expect(deps.authApi.connectGoogle).toHaveBeenCalledTimes(1);
140+
expect(deps.authApi.connectGoogle.mock.calls[0]).toHaveLength(1);
139141
expect(deps.authApi.loginOrSignup).toHaveBeenCalledTimes(1);
140142
expect(deps.completeAuthentication).toHaveBeenCalledWith({
141143
email: "user@example.com",

packages/web/src/common/apis/api.types.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ export interface ApiError extends Error {
1212
export interface ApiRequestConfig {
1313
headers?: HeadersInit;
1414
method?: string;
15-
skipSessionRecovery?: boolean;
1615
url?: string;
1716
}
1817

@@ -24,10 +23,7 @@ export interface ApiResponse<T> {
2423
statusText: string;
2524
}
2625

27-
export type ApiMethodConfig = Pick<
28-
ApiRequestConfig,
29-
"headers" | "skipSessionRecovery"
30-
>;
26+
export type ApiMethodConfig = Pick<ApiRequestConfig, "headers">;
3127

3228
export type SignoutStatus =
3329
| Status.UNAUTHORIZED

packages/web/src/common/apis/auth.api.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import {
33
type GoogleConnectResponse,
44
type Result_Auth_Compass,
55
} from "@core/types/auth.types";
6-
import { type ApiMethodConfig } from "@web/common/apis/api.types";
76
import { BaseApi } from "@web/common/apis/base/base.api";
87

98
const AuthApi = {
@@ -21,12 +20,10 @@ const AuthApi = {
2120

2221
async connectGoogle(
2322
data: GoogleAuthCodeRequest,
24-
config?: ApiMethodConfig,
2523
): Promise<GoogleConnectResponse> {
2624
const response = await BaseApi.post<GoogleConnectResponse>(
2725
`/auth/google/connect`,
2826
data,
29-
config,
3027
);
3128

3229
return response.data;

packages/web/src/common/apis/base/base.api.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,32 @@ const request = async <T>(
2626
url: string,
2727
body?: unknown,
2828
config: ApiMethodConfig = {},
29+
): Promise<ApiResponse<T>> => {
30+
try {
31+
return await sendApiRequestWithoutSharedErrorRecovery<T>(
32+
method,
33+
url,
34+
body,
35+
config,
36+
);
37+
} catch (error) {
38+
if (isApiError(error)) {
39+
return handleErrorResponse<ApiResponse<T>>(error);
40+
}
41+
42+
throw error;
43+
}
44+
};
45+
46+
export const sendApiRequestWithoutSharedErrorRecovery = async <T>(
47+
method: string,
48+
url: string,
49+
body?: unknown,
50+
config: ApiMethodConfig = {},
2951
): Promise<ApiResponse<T>> => {
3052
const requestConfig = {
3153
headers: config.headers,
3254
method,
33-
skipSessionRecovery: config.skipSessionRecovery,
3455
url,
3556
} satisfies ApiRequestConfig;
3657

@@ -74,7 +95,7 @@ const request = async <T>(
7495
}
7596

7697
if (isApiError(error)) {
77-
return handleErrorResponse<ApiResponse<T>>(error);
98+
throw error;
7899
}
79100

80101
throw createApiError(requestConfig);

0 commit comments

Comments
 (0)