diff --git a/e2e/oauth/google-auth-callback.spec.ts b/e2e/oauth/google-auth-callback.spec.ts index 848d25632..229345da3 100644 --- a/e2e/oauth/google-auth-callback.spec.ts +++ b/e2e/oauth/google-auth-callback.spec.ts @@ -21,6 +21,10 @@ type ApiMocks = { type ApiMockOptions = { beforeConnectGoogleResponse?: Promise; beforeLoginOrSignupResponse?: Promise; + connectGoogleResponse?: { + body?: unknown; + status: number; + }; }; const createDeferred = () => { @@ -158,9 +162,9 @@ const prepareGoogleAuthCallbackPage = async ( }); await options.beforeConnectGoogleResponse; return route.fulfill({ - status: 200, + status: options.connectGoogleResponse?.status ?? 200, contentType: "application/json", - body: JSON.stringify({}), + body: JSON.stringify(options.connectGoogleResponse?.body ?? {}), }); } @@ -284,6 +288,35 @@ test.describe("Google auth callback", () => { expectGoogleAuthRequestBody(apiMocks.loginOrSignup[0], state); }); + test("recovers through Google sign-in when Google connect rejects an expired Compass session", async ({ + page, + }) => { + const state = "connect-calendar-session-expired-state"; + const apiMocks = await prepareGoogleAuthCallbackPage(page, { + connectGoogleResponse: { + status: 401, + body: { message: "unauthorized" }, + }, + }); + + await writeGoogleAuthorizationIntent({ + intent: "connectCalendar", + page, + returnPath: "/week", + state, + }); + await setActiveCompassSession(page); + + await page.goto(getCallbackUrl(state)); + + await expect(page).toHaveURL(/\/week$/); + expect(apiMocks.connectGoogle).toHaveLength(1); + expect(apiMocks.loginOrSignup).toHaveLength(1); + expect(apiMocks.loginOrSignup[0]?.headers.rid).toBe("thirdparty"); + expectGoogleAuthRequestBody(apiMocks.connectGoogle[0], state); + expectGoogleAuthRequestBody(apiMocks.loginOrSignup[0], state); + }); + test("rejects callbacks that are missing required Google Calendar scopes", async ({ page, }) => { diff --git a/packages/web/src/auth/google/authorization/complete-google-authorization.ts b/packages/web/src/auth/google/authorization/complete-google-authorization.ts index f7eaf4af4..1c71b8798 100644 --- a/packages/web/src/auth/google/authorization/complete-google-authorization.ts +++ b/packages/web/src/auth/google/authorization/complete-google-authorization.ts @@ -3,10 +3,7 @@ import { type GoogleAuthCodeRequest, GoogleConnectErrorResponseSchema, } from "@core/types/auth.types"; -import { - type ApiError, - type ApiMethodConfig, -} from "@web/common/apis/api.types"; +import { type ApiError } from "@web/common/apis/api.types"; import { ROOT_ROUTES } from "@web/common/constants/routes"; import { GOOGLE_AUTH_SCOPES_REQUIRED, @@ -28,10 +25,7 @@ type CompleteAuthentication = (input: { }) => Promise; export type GoogleAuthorizationAuthAdapter = { - connectGoogle( - data: GoogleAuthCodeRequest, - config?: ApiMethodConfig, - ): Promise; + connectGoogle(data: GoogleAuthCodeRequest): Promise; loginOrSignup(data: GoogleAuthCodeRequest): Promise<{ user: { emails?: string[] }; }>; @@ -149,7 +143,7 @@ export async function completeGoogleAuthorization({ await completeGoogleSignIn(); } else { try { - await authApi.connectGoogle(payload, { skipSessionRecovery: true }); + await authApi.connectGoogle(payload); await refreshUserMetadata(); requestEventFetch?.(); } catch (error) { diff --git a/packages/web/src/auth/google/authorization/google-auth-callback.api.test.ts b/packages/web/src/auth/google/authorization/google-auth-callback.api.test.ts new file mode 100644 index 000000000..16915f1f2 --- /dev/null +++ b/packages/web/src/auth/google/authorization/google-auth-callback.api.test.ts @@ -0,0 +1,97 @@ +import { Status } from "@core/errors/status.codes"; +import { type GoogleAuthCodeRequest } from "@core/types/auth.types"; +import { BaseApi } from "@web/common/apis/base/base.api"; +import { session } from "@web/common/classes/Session"; +import { GoogleAuthCallbackApi } from "./google-auth-callback.api"; +import { + afterEach, + beforeEach, + describe, + expect, + it, + mock, + spyOn, +} from "bun:test"; + +const originalFetch = globalThis.fetch; + +const payload: GoogleAuthCodeRequest = { + clientType: "web", + redirectURIInfo: { + redirectURIOnProviderDashboard: + "http://localhost:9080/auth/google/callback", + redirectURIQueryParams: { + code: "auth-code", + scope: "https://www.googleapis.com/auth/calendar", + state: "state-1", + }, + }, + thirdPartyId: "google", +}; + +describe("GoogleAuthCallbackApi", () => { + beforeEach(() => { + BaseApi.defaults.adapter = undefined; + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + BaseApi.defaults.adapter = undefined; + }); + + it("lets the callback flow handle an expired connect session locally", async () => { + const signOutSpy = spyOn(session, "signOut").mockResolvedValue(undefined); + globalThis.fetch = mock(async () => + Promise.resolve( + new Response(JSON.stringify({ message: "unauthorised" }), { + status: Status.UNAUTHORIZED, + }), + ), + ) as unknown as typeof fetch; + + await expect( + GoogleAuthCallbackApi.connectGoogle(payload), + ).rejects.toMatchObject({ + name: "ApiError", + response: { + status: Status.UNAUTHORIZED, + }, + }); + + expect(signOutSpy).not.toHaveBeenCalled(); + expect(globalThis.fetch).toHaveBeenCalledWith( + expect.stringContaining("/auth/google/connect"), + expect.objectContaining({ + credentials: "include", + method: "POST", + }), + ); + + signOutSpy.mockRestore(); + }); + + it("uses shared session handling for non-recoverable connect session errors", async () => { + window.history.pushState({}, "", "/day"); + const signOutSpy = spyOn(session, "signOut").mockResolvedValue(undefined); + globalThis.fetch = mock(async () => + Promise.resolve( + new Response(JSON.stringify({ message: "not found" }), { + status: Status.NOT_FOUND, + }), + ), + ) as unknown as typeof fetch; + + await expect( + GoogleAuthCallbackApi.connectGoogle(payload), + ).rejects.toMatchObject({ + name: "ApiError", + response: { + status: Status.NOT_FOUND, + }, + }); + + expect(signOutSpy).toHaveBeenCalledTimes(1); + + signOutSpy.mockRestore(); + }); +}); diff --git a/packages/web/src/auth/google/authorization/google-auth-callback.api.ts b/packages/web/src/auth/google/authorization/google-auth-callback.api.ts new file mode 100644 index 000000000..a89f6ec10 --- /dev/null +++ b/packages/web/src/auth/google/authorization/google-auth-callback.api.ts @@ -0,0 +1,51 @@ +import { GOOGLE_REVOKED } from "@core/constants/sse.constants"; +import { Status } from "@core/errors/status.codes"; +import { + type GoogleAuthCodeRequest, + type GoogleConnectResponse, +} from "@core/types/auth.types"; +import { type ApiError, type ApiResponse } from "@web/common/apis/api.types"; +import { AuthApi } from "@web/common/apis/auth.api"; +import { sendApiRequestWithoutSharedErrorRecovery } from "@web/common/apis/base/base.api"; +import { + getApiErrorCode, + handleErrorResponse, + isApiError, +} from "@web/common/apis/util/api.util"; +import { type GoogleAuthorizationAuthAdapter } from "./complete-google-authorization"; + +const isRecoverableConnectSessionError = (error: ApiError): boolean => { + return ( + error.response?.status === Status.UNAUTHORIZED && + getApiErrorCode(error) !== GOOGLE_REVOKED + ); +}; + +export const GoogleAuthCallbackApi = { + async connectGoogle( + data: GoogleAuthCodeRequest, + ): Promise { + try { + const response = + await sendApiRequestWithoutSharedErrorRecovery( + "POST", + "/auth/google/connect", + data, + ); + + return response.data; + } catch (error) { + if (!isApiError(error)) { + throw error; + } + + if (isRecoverableConnectSessionError(error)) { + throw error; + } + + await handleErrorResponse>(error); + throw error; + } + }, + loginOrSignup: AuthApi.loginOrSignup, +} satisfies GoogleAuthorizationAuthAdapter; diff --git a/packages/web/src/auth/google/authorization/google-authorization.test.ts b/packages/web/src/auth/google/authorization/google-authorization.test.ts index ca2d62f89..ef43ad5d2 100644 --- a/packages/web/src/auth/google/authorization/google-authorization.test.ts +++ b/packages/web/src/auth/google/authorization/google-authorization.test.ts @@ -82,6 +82,7 @@ describe("completeGoogleAuthorization", () => { ).resolves.toEqual({ status: "completed", returnPath: "/day" }); expect(deps.authApi.connectGoogle).toHaveBeenCalledTimes(1); + expect(deps.authApi.connectGoogle.mock.calls[0]).toHaveLength(1); expect(deps.refreshUserMetadata).toHaveBeenCalledTimes(1); expect(deps.requestEventFetch).toHaveBeenCalledTimes(1); expect(deps.completeAuthentication).not.toHaveBeenCalled(); @@ -136,6 +137,7 @@ describe("completeGoogleAuthorization", () => { ).resolves.toEqual({ status: "completed", returnPath: "/day" }); expect(deps.authApi.connectGoogle).toHaveBeenCalledTimes(1); + expect(deps.authApi.connectGoogle.mock.calls[0]).toHaveLength(1); expect(deps.authApi.loginOrSignup).toHaveBeenCalledTimes(1); expect(deps.completeAuthentication).toHaveBeenCalledWith({ email: "user@example.com", diff --git a/packages/web/src/common/apis/api.types.ts b/packages/web/src/common/apis/api.types.ts index ccbee5b72..e1650dab3 100644 --- a/packages/web/src/common/apis/api.types.ts +++ b/packages/web/src/common/apis/api.types.ts @@ -12,7 +12,6 @@ export interface ApiError extends Error { export interface ApiRequestConfig { headers?: HeadersInit; method?: string; - skipSessionRecovery?: boolean; url?: string; } @@ -24,10 +23,7 @@ export interface ApiResponse { statusText: string; } -export type ApiMethodConfig = Pick< - ApiRequestConfig, - "headers" | "skipSessionRecovery" ->; +export type ApiMethodConfig = Pick; export type SignoutStatus = | Status.UNAUTHORIZED diff --git a/packages/web/src/common/apis/auth.api.ts b/packages/web/src/common/apis/auth.api.ts index 3095840aa..17059fb58 100644 --- a/packages/web/src/common/apis/auth.api.ts +++ b/packages/web/src/common/apis/auth.api.ts @@ -3,7 +3,6 @@ import { type GoogleConnectResponse, type Result_Auth_Compass, } from "@core/types/auth.types"; -import { type ApiMethodConfig } from "@web/common/apis/api.types"; import { BaseApi } from "@web/common/apis/base/base.api"; const AuthApi = { @@ -21,12 +20,10 @@ const AuthApi = { async connectGoogle( data: GoogleAuthCodeRequest, - config?: ApiMethodConfig, ): Promise { const response = await BaseApi.post( `/auth/google/connect`, data, - config, ); return response.data; diff --git a/packages/web/src/common/apis/base/base.api.ts b/packages/web/src/common/apis/base/base.api.ts index 54f96785e..570b785cf 100644 --- a/packages/web/src/common/apis/base/base.api.ts +++ b/packages/web/src/common/apis/base/base.api.ts @@ -26,11 +26,32 @@ const request = async ( url: string, body?: unknown, config: ApiMethodConfig = {}, +): Promise> => { + try { + return await sendApiRequestWithoutSharedErrorRecovery( + method, + url, + body, + config, + ); + } catch (error) { + if (isApiError(error)) { + return handleErrorResponse>(error); + } + + throw error; + } +}; + +export const sendApiRequestWithoutSharedErrorRecovery = async ( + method: string, + url: string, + body?: unknown, + config: ApiMethodConfig = {}, ): Promise> => { const requestConfig = { headers: config.headers, method, - skipSessionRecovery: config.skipSessionRecovery, url, } satisfies ApiRequestConfig; @@ -74,7 +95,7 @@ const request = async ( } if (isApiError(error)) { - return handleErrorResponse>(error); + throw error; } throw createApiError(requestConfig); diff --git a/packages/web/src/common/apis/util/api.util.test.ts b/packages/web/src/common/apis/util/api.util.test.ts index 36fb9e721..5562aa3e6 100644 --- a/packages/web/src/common/apis/util/api.util.test.ts +++ b/packages/web/src/common/apis/util/api.util.test.ts @@ -1,31 +1,15 @@ -import { Status } from "@core/errors/status.codes"; -import { - ApiErrorResponseSchema, - GoogleConnectErrorResponseSchema, -} from "@core/types/auth.types"; -import { session } from "@web/common/classes/Session"; -import { - type ApiError, - type ApiRequestConfig, - type ApiResponse, -} from "../api.types"; -import { - getApiErrorCode, - handleErrorResponse, - parseApiError, - parseGoogleConnectError, -} from "./api.util"; -import { describe, expect, it, spyOn } from "bun:test"; +import { ApiErrorResponseSchema } from "@core/types/auth.types"; +import { type ApiError, type ApiResponse } from "../api.types"; +import { getApiErrorCode, parseApiError } from "./api.util"; +import { describe, expect, it } from "bun:test"; const createApiError = ( response: { data?: unknown; status?: number } | null, - config: ApiRequestConfig = {}, ): ApiError => Object.assign(new Error("Request failed"), { - config, response: response ? ({ - config, + config: {}, data: response.data, headers: new Headers(), status: response.status ?? 400, @@ -113,63 +97,3 @@ describe("parseApiError", () => { expect(parseApiError(error, ApiErrorResponseSchema)).toBeUndefined(); }); }); - -describe("parseGoogleConnectError", () => { - it("parses typed Google connect errors", () => { - const error = createApiError({ - data: { - code: "GOOGLE_ACCOUNT_ALREADY_CONNECTED", - message: "Google account is already connected", - }, - }); - - expect(parseGoogleConnectError(error)).toEqual({ - code: "GOOGLE_ACCOUNT_ALREADY_CONNECTED", - message: "Google account is already connected", - }); - }); - - it("parses Google not configured connect errors", () => { - const error = createApiError({ - data: { - code: "GOOGLE_NOT_CONFIGURED", - message: "Google is not configured for this Compass instance", - }, - }); - - expect(parseGoogleConnectError(error)).toEqual({ - code: "GOOGLE_NOT_CONFIGURED", - message: "Google is not configured for this Compass instance", - }); - }); - - it("returns undefined for non-Google-connect error codes", () => { - const error = createApiError({ - data: { - code: "ANY_ERROR", - message: "Something went wrong", - }, - }); - - expect(parseGoogleConnectError(error)).toBeUndefined(); - expect( - parseApiError(error, GoogleConnectErrorResponseSchema), - ).toBeUndefined(); - }); -}); - -describe("handleErrorResponse", () => { - it("keeps skipSessionRecovery local to unauthorized responses", async () => { - window.history.pushState({}, "", "/day"); - const signOutSpy = spyOn(session, "signOut").mockResolvedValue(undefined); - const error = createApiError( - { status: Status.NOT_FOUND }, - { skipSessionRecovery: true, url: "/event" }, - ); - - await expect(handleErrorResponse(error)).rejects.toBe(error); - - expect(signOutSpy).toHaveBeenCalledTimes(1); - signOutSpy.mockRestore(); - }); -}); diff --git a/packages/web/src/common/apis/util/api.util.ts b/packages/web/src/common/apis/util/api.util.ts index 4f7d68182..2919ed149 100644 --- a/packages/web/src/common/apis/util/api.util.ts +++ b/packages/web/src/common/apis/util/api.util.ts @@ -1,10 +1,6 @@ import { type ZodType } from "zod"; import { GOOGLE_REVOKED } from "@core/constants/sse.constants"; import { Status } from "@core/errors/status.codes"; -import { - type GoogleConnectErrorResponse, - GoogleConnectErrorResponseSchema, -} from "@core/types/auth.types"; import { handleGoogleRevoked } from "@web/auth/google/util/google.auth.util"; import { session } from "@web/common/classes/Session"; import { ENV_WEB } from "../../constants/env.constants"; @@ -65,12 +61,6 @@ export const parseApiError = ( return parsed.success ? parsed.data : undefined; }; -export const parseGoogleConnectError = ( - error: ApiError, -): GoogleConnectErrorResponse | undefined => { - return parseApiError(error, GoogleConnectErrorResponseSchema); -}; - export const signOut = async (status: SignoutStatus) => { // since there are currently duplicate event fetches, // this prevents triggering a separate alert for each fetch @@ -138,10 +128,6 @@ export const handleErrorResponse = async (error: ApiError) => { throw error; } - if (error.config?.skipSessionRecovery && status === Status.UNAUTHORIZED) { - throw error; - } - const isAuthEndpoint = requestUrl?.includes("/signinup"); if ( diff --git a/packages/web/src/views/GoogleAuthCallback/GoogleAuthCallback.tsx b/packages/web/src/views/GoogleAuthCallback/GoogleAuthCallback.tsx index 916be43e9..9e5e0853c 100644 --- a/packages/web/src/views/GoogleAuthCallback/GoogleAuthCallback.tsx +++ b/packages/web/src/views/GoogleAuthCallback/GoogleAuthCallback.tsx @@ -3,7 +3,7 @@ import { useLocation, useNavigate } from "react-router-dom"; import { useCompleteAuthentication } from "@web/auth/compass/hooks/useCompleteAuthentication"; import { refreshUserMetadata } from "@web/auth/compass/user/util/user-metadata.util"; import { completeGoogleAuthorization } from "@web/auth/google/authorization/complete-google-authorization"; -import { AuthApi } from "@web/common/apis/auth.api"; +import { GoogleAuthCallbackApi } from "@web/auth/google/authorization/google-auth-callback.api"; import { session } from "@web/common/classes/Session"; import { showErrorToast } from "@web/common/utils/toast/error-toast.util"; import { OverlayPanel } from "@web/components/OverlayPanel/OverlayPanel"; @@ -27,7 +27,7 @@ export async function completeGoogleAuthCallback({ search, }: CompleteGoogleAuthCallbackOptions): Promise { const result = await completeGoogleAuthorization({ - authApi: AuthApi, + authApi: GoogleAuthCallbackApi, completeAuthentication, doesSessionExist: () => session.doesSessionExist(), refreshUserMetadata,