diff --git a/apps/calendar-web/src/auth/posthog/oauth-signup-funnel.test.ts b/apps/calendar-web/src/auth/posthog/oauth-signup-funnel.test.ts index 530ea3d90f..74cff52ab9 100644 --- a/apps/calendar-web/src/auth/posthog/oauth-signup-funnel.test.ts +++ b/apps/calendar-web/src/auth/posthog/oauth-signup-funnel.test.ts @@ -13,7 +13,7 @@ describe("oauth-signup-funnel", () => { "missing_code", ); expect(oauthFailedReasonFromSignupFailure("oauth_exchange_failed")).toBe( - "token_exchange_error", + "token_exchange_failed", ); }); }); diff --git a/apps/calendar-web/src/auth/posthog/oauth-signup-funnel.ts b/apps/calendar-web/src/auth/posthog/oauth-signup-funnel.ts index 58f2e2c93e..65133523d7 100644 --- a/apps/calendar-web/src/auth/posthog/oauth-signup-funnel.ts +++ b/apps/calendar-web/src/auth/posthog/oauth-signup-funnel.ts @@ -17,7 +17,7 @@ const SIGNUP_FAILURE_TO_OAUTH_FAILED: Partial< oauth_missing_state: "invalid_state", oauth_missing_intent: "invalid_state", oauth_missing_code: "missing_code", - oauth_exchange_failed: "token_exchange_error", + oauth_exchange_failed: "token_exchange_failed", oauth_missing_scopes: "other", oauth_callback_crashed: "other", }; diff --git a/apps/calendar-web/src/auth/providers/authorization/useStartProviderAuthorization.impl.ts b/apps/calendar-web/src/auth/providers/authorization/useStartProviderAuthorization.impl.ts index 13e6cf7909..5040fe8dad 100644 --- a/apps/calendar-web/src/auth/providers/authorization/useStartProviderAuthorization.impl.ts +++ b/apps/calendar-web/src/auth/providers/authorization/useStartProviderAuthorization.impl.ts @@ -55,7 +55,8 @@ type AuthorizationSession = { type CreateOAuthState = ( redirectUri: string, - posthogDistinctId?: string, + posthogDistinctId: string | undefined, + signupFlow: boolean | undefined, ) => string; /** @@ -67,11 +68,16 @@ type CreateOAuthState = ( function useAuthorizationSession( provider: ProviderKind, createState: CreateOAuthState, + signupFlow?: boolean, ): AuthorizationSession { const [session] = useState(() => { const redirectUri = buildProviderAuthCallbackUrl(provider); return { - state: createState(redirectUri, readPosthogDistinctIdForOAuth()), + state: createState( + redirectUri, + readPosthogDistinctIdForOAuth(), + signupFlow, + ), redirectUri, }; }); @@ -151,7 +157,7 @@ function createRedirectAuthorizationStrategy({ }: RedirectAuthorizationSpec): ProviderAuthorizationStrategy { return ({ intent, signupFlow, onStart, onError, prompt }) => { const [loading, setLoading] = useState(false); - const session = useAuthorizationSession(provider, createState); + const session = useAuthorizationSession(provider, createState, signupFlow); return { loading, @@ -190,8 +196,13 @@ const useGoogleProviderAuthorizationStrategy: ProviderAuthorizationStrategy = ({ const [loading, setLoading] = useState(false); const { state, redirectUri } = useAuthorizationSession( "google", - (_redirectUri, posthogDistinctId) => - buildSignInOAuthState({ isDesktopClient: false, posthogDistinctId }), + (_redirectUri, posthogDistinctId, signupFlow) => + buildSignInOAuthState({ + isDesktopClient: false, + posthogDistinctId, + flow: signupFlow ? "signup" : "login", + }), + signupFlow, ); const loginOptions = useMemo< @@ -234,8 +245,12 @@ const useMicrosoftProviderAuthorizationStrategy = provider: "microsoft", getClientId: () => getMicrosoftSignInClientId(), notConfiguredMessage: "Microsoft sign-in is not configured", - createState: (_redirectUri, posthogDistinctId) => - buildSignInOAuthState({ isDesktopClient: false, posthogDistinctId }), + createState: (_redirectUri, posthogDistinctId, signupFlow) => + buildSignInOAuthState({ + isDesktopClient: false, + posthogDistinctId, + flow: signupFlow ? "signup" : "login", + }), buildAuthorizationUrl: ({ clientId, redirectUri, state, prompt }) => buildMicrosoftAuthorizationUrl({ clientId, @@ -253,12 +268,13 @@ const useAppleProviderAuthorizationStrategy = notConfiguredMessage: "Apple sign-in is not configured", // Apple returns to the backend form_post route, so the frontend callback it // should land on afterwards rides along inside the state. - createState: (redirectUri, posthogDistinctId) => + createState: (redirectUri, posthogDistinctId, signupFlow) => btoa( JSON.stringify({ frontendRedirectURI: redirectUri, nonce: crypto.randomUUID(), ...(posthogDistinctId ? { ph: posthogDistinctId } : {}), + fl: signupFlow ? "signup" : "login", }), ), buildAuthorizationUrl: ({ clientId, state }) => diff --git a/docs/frontend/frontend-runtime-flow.md b/docs/frontend/frontend-runtime-flow.md index a4990f7eaf..32414f34b9 100644 --- a/docs/frontend/frontend-runtime-flow.md +++ b/docs/frontend/frontend-runtime-flow.md @@ -231,15 +231,17 @@ Apple (`packages/core/src/analytics/oauth-signup.events.ts`): 1. `oauth_redirect_started` (`provider`, `intent`) when the browser leaves for the provider consent screen. -2. `oauth_callback_received` (`provider`) when the provider redirect lands. The - SPA relays the query string to the backend route - `GET /auth/:provider/callback` with `X-Compass-OAuth-Telemetry: 1` so the - event is captured server-side even when JavaScript fails later. OAuth `state` - carries the PostHog anon distinct id when available. -3. `oauth_failed` (`provider`, `reason`, optional `provider_error`) for consent - denial, invalid state, missing code, token exchange errors, and other - failures. Legacy `signup_failed` still fires with `reason_code` for existing - insights. +2. `oauth_callback_received` (`provider`) on the client when the provider + redirect lands, plus server `oauth_callback` (`environment`, `provider`, + `flow`, `outcome`, optional `reason`) from the backend route + `GET /auth/:provider/callback`. The SPA relays the query string with + `X-Compass-OAuth-Telemetry: 1` so the server event is captured even when + JavaScript fails later. OAuth `state` carries the PostHog anon distinct id and + signup vs login flow when available. +3. Server `oauth_callback` failures cover consent denial (`access_denied`), + invalid state, missing code, token exchange failures, and user creation + failures. Client `oauth_failed` and legacy `signup_failed` still fire for + existing insights. 4. `signup_completed` or `login_completed` (`method`) after a successful SuperTokens exchange (unchanged). diff --git a/packages/backend/src/auth/auth.routes.config.ts b/packages/backend/src/auth/auth.routes.config.ts index 523fa201bc..287e6c42ff 100644 --- a/packages/backend/src/auth/auth.routes.config.ts +++ b/packages/backend/src/auth/auth.routes.config.ts @@ -94,14 +94,14 @@ export class AuthRoutes extends CommonRoutesConfig { appleSignInCallbackLimiter, urlencoded({ extended: false }), (req, res) => { - authController.appleSignInCallback(req, res); + void authController.appleSignInCallback(req, res); }, ); this.app .route("/auth/:provider/callback") .get(providerOAuthCallbackLimiter, (req, res) => { - authController.providerOAuthCallback(req, res); + void authController.providerOAuthCallback(req, res); }); return this.app; diff --git a/packages/backend/src/auth/controllers/auth.controller.ts b/packages/backend/src/auth/controllers/auth.controller.ts index 64a58063bd..427279e6fe 100644 --- a/packages/backend/src/auth/controllers/auth.controller.ts +++ b/packages/backend/src/auth/controllers/auth.controller.ts @@ -204,14 +204,14 @@ class AuthController { ); }; - appleSignInCallback = ( + appleSignInCallback = async ( req: ReqBody>, res: Res_Promise, - ): void => { + ): Promise => { const body = req.body ?? {}; const state = typeof body["state"] === "string" ? body["state"].trim() : ""; if (state) { - appleFormPostAnalytics({ + await appleFormPostAnalytics({ state, code: typeof body["code"] === "string" ? body["code"] : undefined, error: typeof body["error"] === "string" ? body["error"] : undefined, @@ -226,10 +226,10 @@ class AuthController { res.redirect(302, result.location); }; - providerOAuthCallback = ( + providerOAuthCallback = async ( req: express.Request, res: express.Response, - ): void => { + ): Promise => { const provider = parseProviderOAuthCallbackProvider( String(req.params["provider"] ?? ""), ); @@ -240,7 +240,7 @@ class AuthController { const telemetryOnly = req.header("x-compass-oauth-telemetry") === "1"; - const resolution = handleProviderOAuthCallback({ + const resolution = await handleProviderOAuthCallback({ provider, query: req.query as Record, telemetryOnly, diff --git a/packages/backend/src/auth/services/provider-oauth-callback.service.test.ts b/packages/backend/src/auth/services/provider-oauth-callback.service.test.ts index f87d99d802..17cc5134be 100644 --- a/packages/backend/src/auth/services/provider-oauth-callback.service.test.ts +++ b/packages/backend/src/auth/services/provider-oauth-callback.service.test.ts @@ -13,36 +13,30 @@ describe("provider-oauth-callback.service", () => { expect(parseProviderOAuthCallbackProvider("unknown")).toBeNull(); }); - it("captures callback and failure then redirects to the SPA", () => { - const received = spyOn( + it("captures oauth_callback then redirects to the SPA", async () => { + const capture = spyOn( analytics.providerOAuthSignupAnalytics, - "captureCallbackReceived", - ).mockImplementation(() => undefined); - const failed = spyOn( - analytics.providerOAuthSignupAnalytics, - "captureFailed", - ).mockImplementation(() => undefined); + "captureCallbackQuery", + ).mockResolvedValue(true); const state = buildSignInOAuthState({ isDesktopClient: false, posthogDistinctId: "anon-abc", }); - const result = handleProviderOAuthCallback({ + const result = await handleProviderOAuthCallback({ provider: "google", query: { state, error: "access_denied" }, telemetryOnly: false, frontendUrl: "http://localhost:9080", }); - expect(received).toHaveBeenCalledWith({ provider: "google", state }); - expect(failed).toHaveBeenCalledWith( - expect.objectContaining({ - provider: "google", - reason: "access_denied", - providerError: "access_denied", - }), - ); + expect(capture).toHaveBeenCalledWith({ + provider: "google", + state, + code: undefined, + providerError: "access_denied", + }); expect(result).toEqual({ mode: "redirect", location: expect.stringContaining( @@ -50,59 +44,44 @@ describe("provider-oauth-callback.service", () => { ), }); - received.mockRestore(); - failed.mockRestore(); + capture.mockRestore(); }); - it("returns telemetry-only without redirect", () => { - const received = spyOn( + it("returns telemetry-only without redirect", async () => { + const capture = spyOn( analytics.providerOAuthSignupAnalytics, - "captureCallbackReceived", - ).mockImplementation(() => undefined); - spyOn( - analytics.providerOAuthSignupAnalytics, - "captureFailed", - ).mockImplementation(() => undefined); + "captureCallbackQuery", + ).mockResolvedValue(true); const state = buildSignInOAuthState({ isDesktopClient: false }); - const result = handleProviderOAuthCallback({ + const result = await handleProviderOAuthCallback({ provider: "microsoft", query: { state, code: "auth-code" }, telemetryOnly: true, }); expect(result).toEqual({ mode: "telemetry" }); - received.mockRestore(); + capture.mockRestore(); }); - it("captures Apple form-post callback and failure on the same analytics path", () => { - const received = spyOn( - analytics.providerOAuthSignupAnalytics, - "captureCallbackReceived", - ).mockImplementation(() => undefined); - const failed = spyOn( + it("captures Apple form-post callback on the same analytics path", async () => { + const capture = spyOn( analytics.providerOAuthSignupAnalytics, - "captureFailed", - ).mockImplementation(() => undefined); + "captureCallbackQuery", + ).mockResolvedValue(true); - appleFormPostAnalytics({ + await appleFormPostAnalytics({ state: "apple-state", error: "access_denied", }); - expect(received).toHaveBeenCalledWith({ + expect(capture).toHaveBeenCalledWith({ provider: "apple", state: "apple-state", + code: undefined, + providerError: "access_denied", }); - expect(failed).toHaveBeenCalledWith( - expect.objectContaining({ - provider: "apple", - reason: "access_denied", - providerError: "access_denied", - }), - ); - received.mockRestore(); - failed.mockRestore(); + capture.mockRestore(); }); }); diff --git a/packages/backend/src/auth/services/provider-oauth-callback.service.ts b/packages/backend/src/auth/services/provider-oauth-callback.service.ts index 2343ffc082..0e2d680243 100644 --- a/packages/backend/src/auth/services/provider-oauth-callback.service.ts +++ b/packages/backend/src/auth/services/provider-oauth-callback.service.ts @@ -1,4 +1,3 @@ -import { oauthFailedReasonFromCallbackQuery } from "@core/analytics/oauth-signup.events"; import { type ProviderKind, ProviderKindSchema, @@ -30,28 +29,16 @@ function queryField( return typeof value === "string" && value.length > 0 ? value : undefined; } -function captureOAuthCallbackQueryAnalytics( +async function captureOAuthCallbackQueryAnalytics( provider: ProviderKind, query: { state?: string; code?: string; providerError?: string }, -): void { - providerOAuthSignupAnalytics.captureCallbackReceived({ +): Promise { + await providerOAuthSignupAnalytics.captureCallbackQuery({ provider, state: query.state, - }); - - const failureReason = oauthFailedReasonFromCallbackQuery({ - state: query.state, code: query.code, - providerError: query.providerError, + providerError: query.providerError ?? null, }); - if (failureReason) { - providerOAuthSignupAnalytics.captureFailed({ - provider, - state: query.state, - reason: failureReason, - providerError: query.providerError, - }); - } } function spaCallbackUrl( @@ -76,17 +63,17 @@ export function parseProviderOAuthCallbackProvider( return parsed.success ? parsed.data : null; } -export function handleProviderOAuthCallback(input: { +export async function handleProviderOAuthCallback(input: { provider: ProviderKind; query: ProviderOAuthCallbackQuery; telemetryOnly: boolean; frontendUrl?: string; -}): ProviderOAuthCallbackResolution { +}): Promise { const state = queryField(input.query, "state"); const code = queryField(input.query, "code"); const providerError = queryField(input.query, "error"); - captureOAuthCallbackQueryAnalytics(input.provider, { + await captureOAuthCallbackQueryAnalytics(input.provider, { state, code, providerError, @@ -102,12 +89,12 @@ export function handleProviderOAuthCallback(input: { }; } -export function appleFormPostAnalytics(input: { +export async function appleFormPostAnalytics(input: { state: string; code?: string; error?: string; -}): void { - captureOAuthCallbackQueryAnalytics("apple", { +}): Promise { + await captureOAuthCallbackQueryAnalytics("apple", { state: input.state, code: input.code, providerError: input.error, diff --git a/packages/backend/src/auth/services/provider-oauth-signup.analytics.test.ts b/packages/backend/src/auth/services/provider-oauth-signup.analytics.test.ts new file mode 100644 index 0000000000..b7dcb3fc1e --- /dev/null +++ b/packages/backend/src/auth/services/provider-oauth-signup.analytics.test.ts @@ -0,0 +1,92 @@ +import { OAUTH_CALLBACK_EVENT } from "@core/analytics/oauth-signup.events"; +import { buildSignInOAuthState } from "@core/auth/signin-oauth-state.util"; +import { mockEnv } from "@backend/__tests__/helpers/mock.setup"; +import { providerOAuthSignupAnalytics } from "@backend/auth/services/provider-oauth-signup.analytics"; +import { describe, expect, it } from "bun:test"; + +describe("provider-oauth-signup.analytics", () => { + it("captures oauth_callback with anon distinct id and signup flow", async () => { + const captured: unknown[] = []; + using _env = mockEnv({ + POSTHOG_KEY: "phc_test", + POSTHOG_HOST: "https://ph.example.test", + NODE_ENV: "staging", + }); + + const originalFetch = globalThis.fetch; + globalThis.fetch = (async (_input, init) => { + captured.push(JSON.parse(String(init?.body))); + return new Response(null, { status: 200 }); + }) as typeof fetch; + + const state = buildSignInOAuthState({ + isDesktopClient: false, + posthogDistinctId: "anon-xyz", + flow: "signup", + }); + + const ok = await providerOAuthSignupAnalytics.captureCallbackQuery({ + provider: "google", + state, + code: "auth-code", + providerError: null, + }); + + globalThis.fetch = originalFetch; + + expect(ok).toBe(true); + expect(captured).toHaveLength(1); + const body = captured[0] as { + event: string; + distinct_id: string; + properties: Record; + }; + expect(body.event).toBe(OAUTH_CALLBACK_EVENT); + expect(body.distinct_id).toBe("anon-xyz"); + expect(body.properties).toEqual({ + environment: "staging", + provider: "google", + flow: "signup", + outcome: "success", + $lib: "compass-backend", + }); + }); + + it("captures access_denied failures on the same oauth_callback event", async () => { + const captured: unknown[] = []; + using _env = mockEnv({ + POSTHOG_KEY: "phc_test", + POSTHOG_HOST: "https://ph.example.test", + NODE_ENV: "production", + }); + + const originalFetch = globalThis.fetch; + globalThis.fetch = (async (_input, init) => { + captured.push(JSON.parse(String(init?.body))); + return new Response(null, { status: 200 }); + }) as typeof fetch; + + const state = buildSignInOAuthState({ + isDesktopClient: false, + posthogDistinctId: "anon-deny", + flow: "signup", + }); + + await providerOAuthSignupAnalytics.captureCallbackQuery({ + provider: "google", + state, + code: null, + providerError: "access_denied", + }); + + globalThis.fetch = originalFetch; + + const body = captured[0] as { properties: Record }; + expect(body.properties).toMatchObject({ + outcome: "failure", + reason: "access_denied", + flow: "signup", + environment: "production", + }); + }); +}); diff --git a/packages/backend/src/auth/services/provider-oauth-signup.analytics.ts b/packages/backend/src/auth/services/provider-oauth-signup.analytics.ts index ed2f2434b1..2ab204156e 100644 --- a/packages/backend/src/auth/services/provider-oauth-signup.analytics.ts +++ b/packages/backend/src/auth/services/provider-oauth-signup.analytics.ts @@ -1,9 +1,14 @@ import { - OAUTH_CALLBACK_RECEIVED_EVENT, - OAUTH_FAILED_EVENT, + type AuthOAuthCallbackOutcome, + type AuthOAuthFlow, + authOAuthCallbackOutcomeFromQuery, + OAUTH_CALLBACK_EVENT, type OAuthFailedReason, } from "@core/analytics/oauth-signup.events"; -import { posthogDistinctIdFromSignInOAuthState } from "@core/auth/signin-oauth-state.util"; +import { + posthogDistinctIdFromSignInOAuthState, + signInOAuthFlowFromState, +} from "@core/auth/signin-oauth-state.util"; import { captureSafely } from "@core/logger/posthog-capture"; import { type ProviderKind } from "@core/types/sync/identity.contracts"; import { CONFIG } from "@backend/common/constants/config.constants"; @@ -15,41 +20,59 @@ function distinctIdFromOAuthState(state: string | null | undefined): string { return posthogDistinctIdFromSignInOAuthState(state) ?? FALLBACK_DISTINCT_ID; } -function captureOAuthSignupEvent( - event: typeof OAUTH_CALLBACK_RECEIVED_EVENT | typeof OAUTH_FAILED_EVENT, - input: { - provider: ProviderKind; - state: string | null | undefined; - reason?: OAuthFailedReason; - providerError?: string; - }, -): void { - void captureSafely(getBackendPostHogClient(), { - event, +async function captureAuthOAuthCallback(input: { + provider: ProviderKind; + state: string | null | undefined; + flow?: AuthOAuthFlow; + outcome: AuthOAuthCallbackOutcome; + reason?: OAuthFailedReason; +}): Promise { + return captureSafely(getBackendPostHogClient(), { + event: OAUTH_CALLBACK_EVENT, distinctId: distinctIdFromOAuthState(input.state), properties: { environment: CONFIG.NODE_ENV, provider: input.provider, + flow: input.flow ?? signInOAuthFlowFromState(input.state), + outcome: input.outcome, ...(input.reason ? { reason: input.reason } : {}), - ...(input.providerError ? { provider_error: input.providerError } : {}), }, }); } export const providerOAuthSignupAnalytics = { - captureCallbackReceived(input: { + async captureCallbackQuery(input: { provider: ProviderKind; state: string | null | undefined; - }): void { - captureOAuthSignupEvent(OAUTH_CALLBACK_RECEIVED_EVENT, input); + code: string | null | undefined; + providerError: string | null | undefined; + }): Promise { + const { outcome, reason } = authOAuthCallbackOutcomeFromQuery({ + state: input.state, + code: input.code, + providerError: input.providerError, + }); + return captureAuthOAuthCallback({ + provider: input.provider, + state: input.state, + outcome, + reason, + }); }, - captureFailed(input: { + async captureExchangeFailure(input: { provider: ProviderKind; state: string | null | undefined; - reason: OAuthFailedReason; - providerError?: string; - }): void { - captureOAuthSignupEvent(OAUTH_FAILED_EVENT, input); + reason: Extract< + OAuthFailedReason, + "token_exchange_failed" | "user_create_failed" | "other" + >; + }): Promise { + return captureAuthOAuthCallback({ + provider: input.provider, + state: input.state, + outcome: "failure", + reason: input.reason, + }); }, }; diff --git a/packages/backend/src/common/middleware/supertokens.middleware.handlers.ts b/packages/backend/src/common/middleware/supertokens.middleware.handlers.ts index 08ec78d5d3..ea35d4296d 100644 --- a/packages/backend/src/common/middleware/supertokens.middleware.handlers.ts +++ b/packages/backend/src/common/middleware/supertokens.middleware.handlers.ts @@ -238,10 +238,10 @@ export async function handleThirdPartySignInUp( provider: kind, error: err instanceof Error ? err.message : String(err), }); - providerOAuthSignupAnalytics.captureFailed({ + void providerOAuthSignupAnalytics.captureExchangeFailure({ provider: kind, state: oauthState, - reason: "token_exchange_error", + reason: "token_exchange_failed", }); throw err; } @@ -251,10 +251,10 @@ export async function handleThirdPartySignInUp( provider: kind, status: response.status, }); - providerOAuthSignupAnalytics.captureFailed({ + void providerOAuthSignupAnalytics.captureExchangeFailure({ provider: kind, state: oauthState, - reason: "token_exchange_error", + reason: "user_create_failed", }); } @@ -325,7 +325,7 @@ export async function handleThirdPartySignInUp( provider: kind, error: err instanceof Error ? err.message : String(err), }); - providerOAuthSignupAnalytics.captureFailed({ + void providerOAuthSignupAnalytics.captureExchangeFailure({ provider: kind, state: oauthState, reason: "other", diff --git a/packages/core/src/analytics/auth-oauth-callback.analytics.test.ts b/packages/core/src/analytics/auth-oauth-callback.analytics.test.ts new file mode 100644 index 0000000000..9ffb37882f --- /dev/null +++ b/packages/core/src/analytics/auth-oauth-callback.analytics.test.ts @@ -0,0 +1,31 @@ +import { authOAuthCallbackOutcomeFromQuery } from "@core/analytics/oauth-signup.events"; +import { describe, expect, it } from "bun:test"; + +describe("auth-oauth-callback.analytics", () => { + it("marks a code+state return as success", () => { + expect( + authOAuthCallbackOutcomeFromQuery({ + state: "state-token", + code: "auth-code", + providerError: null, + }), + ).toEqual({ outcome: "success" }); + }); + + it("maps provider and query failures to reasons", () => { + expect( + authOAuthCallbackOutcomeFromQuery({ + state: "state-token", + code: null, + providerError: "access_denied", + }), + ).toEqual({ outcome: "failure", reason: "access_denied" }); + expect( + authOAuthCallbackOutcomeFromQuery({ + state: null, + code: "auth-code", + providerError: null, + }), + ).toEqual({ outcome: "failure", reason: "invalid_state" }); + }); +}); diff --git a/packages/core/src/analytics/oauth-callback.analytics.test.ts b/packages/core/src/analytics/oauth-callback.analytics.test.ts index 9b64738b6f..501d122a42 100644 --- a/packages/core/src/analytics/oauth-callback.analytics.test.ts +++ b/packages/core/src/analytics/oauth-callback.analytics.test.ts @@ -38,7 +38,7 @@ describe("oauth-callback.analytics", () => { ).toEqual({ reason: "missing_code" }); }); - it("maps token exchange failures to token_exchange_error", () => { + it("maps token exchange failures to token_exchange_failed", () => { expect( oauthCallbackFailureReason({ outcome: "error", @@ -51,7 +51,7 @@ describe("oauth-callback.analytics", () => { }), }), ).toEqual({ - reason: "token_exchange_error", + reason: "token_exchange_failed", provider_error: "exchangeFailed", }); }); @@ -67,7 +67,7 @@ describe("oauth-callback.analytics", () => { name: "ProviderAuthError", }), }), - ).toEqual({ reason: "token_exchange_error" }); + ).toEqual({ reason: "token_exchange_failed" }); }); it("ignores a non-provider error on the exchange path", () => { diff --git a/packages/core/src/analytics/oauth-callback.analytics.ts b/packages/core/src/analytics/oauth-callback.analytics.ts index c1c2cc774c..b716a3c196 100644 --- a/packages/core/src/analytics/oauth-callback.analytics.ts +++ b/packages/core/src/analytics/oauth-callback.analytics.ts @@ -88,7 +88,7 @@ export function oauthCallbackFailureReason(input: { } const exchange = tokenExchangeFailure(input.exchangeError); if (exchange) { - return withProviderError("token_exchange_error", exchange.providerError); + return withProviderError("token_exchange_failed", exchange.providerError); } return withProviderError("other", mergedProviderError); } diff --git a/packages/core/src/analytics/oauth-signup.events.ts b/packages/core/src/analytics/oauth-signup.events.ts index 7b0d5cf7ba..a21d238280 100644 --- a/packages/core/src/analytics/oauth-signup.events.ts +++ b/packages/core/src/analytics/oauth-signup.events.ts @@ -2,11 +2,19 @@ export const OAUTH_CALLBACK_RECEIVED_EVENT = "oauth_callback_received" as const; export const OAUTH_FAILED_EVENT = "oauth_failed" as const; +/** Shared with sync calendar connect callbacks (`connection.routes.ts`). */ +export const OAUTH_CALLBACK_EVENT = "oauth_callback" as const; + +export type AuthOAuthFlow = "signup" | "login"; + +export type AuthOAuthCallbackOutcome = "success" | "failure"; + export type OAuthFailedReason = | "access_denied" | "invalid_state" - | "token_exchange_error" + | "token_exchange_failed" | "missing_code" + | "user_create_failed" | "other"; export function oauthFailedReasonFromCallbackQuery(input: { @@ -20,3 +28,18 @@ export function oauthFailedReasonFromCallbackQuery(input: { if (!input.code) return "missing_code"; return undefined; } + +export function authOAuthCallbackOutcomeFromQuery(input: { + state: string | null | undefined; + code: string | null | undefined; + providerError: string | null | undefined; +}): { + outcome: AuthOAuthCallbackOutcome; + reason?: OAuthFailedReason; +} { + const reason = oauthFailedReasonFromCallbackQuery(input); + if (reason) { + return { outcome: "failure", reason }; + } + return { outcome: "success" }; +} diff --git a/packages/core/src/auth/signin-oauth-state.util.test.ts b/packages/core/src/auth/signin-oauth-state.util.test.ts index cb6e7b15e4..fccbf75f2e 100644 --- a/packages/core/src/auth/signin-oauth-state.util.test.ts +++ b/packages/core/src/auth/signin-oauth-state.util.test.ts @@ -16,6 +16,7 @@ describe("signin-oauth-state", () => { rawState: state, nonce: expect.any(String), posthogDistinctId: "anon-123", + flow: "login", isDesktop: false, }); }); @@ -37,6 +38,7 @@ describe("signin-oauth-state", () => { expect(parseSignInOAuthState(legacy)).toEqual({ rawState: legacy, nonce: legacy, + flow: "login", isDesktop: false, }); expect(posthogDistinctIdFromSignInOAuthState(legacy)).toBeUndefined(); diff --git a/packages/core/src/auth/signin-oauth-state.util.ts b/packages/core/src/auth/signin-oauth-state.util.ts index 6502b49814..646d09604e 100644 --- a/packages/core/src/auth/signin-oauth-state.util.ts +++ b/packages/core/src/auth/signin-oauth-state.util.ts @@ -1,3 +1,4 @@ +import { type AuthOAuthFlow } from "@core/analytics/oauth-signup.events"; import { DESKTOP_OAUTH_STATE_PREFIX } from "@core/desktop/desktop-oauth-state.util"; const SIGNIN_OAUTH_STATE_VERSION_PREFIX = "v1."; @@ -9,12 +10,14 @@ export type ParsedSignInOAuthState = { rawState: string; nonce: string; posthogDistinctId?: string; + flow: AuthOAuthFlow; isDesktop: boolean; }; type SignInOAuthStatePayload = { n: string; ph?: string; + fl?: AuthOAuthFlow; }; function base64UrlEncode(value: string): string { @@ -56,6 +59,13 @@ function decodePayload(body: string): SignInOAuthStatePayload | null { if (parsed.ph !== undefined && typeof parsed.ph !== "string") { return null; } + if ( + parsed.fl !== undefined && + parsed.fl !== "signup" && + parsed.fl !== "login" + ) { + return null; + } return parsed; } catch { return null; @@ -66,10 +76,12 @@ function decodePayload(body: string): SignInOAuthStatePayload | null { export function buildSignInOAuthState(input: { isDesktopClient: boolean; posthogDistinctId?: string; + flow?: AuthOAuthFlow; }): string { const payload: SignInOAuthStatePayload = { n: crypto.randomUUID(), ...(input.posthogDistinctId ? { ph: input.posthogDistinctId } : {}), + ...(input.flow ? { fl: input.flow } : {}), }; const body = encodePayload(payload); return input.isDesktopClient ? `${DESKTOP_OAUTH_STATE_PREFIX}${body}` : body; @@ -89,6 +101,7 @@ export function parseSignInOAuthState( return { rawState: state, nonce: body, + flow: "login", isDesktop, }; } @@ -105,25 +118,43 @@ export function parseSignInOAuthState( rawState: state, nonce: payload.n, posthogDistinctId, + flow: payload.fl ?? "login", isDesktop, }; } +export function signInOAuthFlowFromState( + state: string | null | undefined, +): AuthOAuthFlow { + if (!state) return "login"; + const parsed = parseSignInOAuthState(state); + if (parsed) return parsed.flow; + return parseAppleRelayOAuthState(state).flow; +} + /** * Apple relays `state` back as its own base64 JSON blob rather than the * version-prefixed payload above. base64UrlDecode reads both alphabets (it * only rewrites `-`/`_` and re-pads), so one pass covers plain base64 too. */ -function posthogDistinctIdFromAppleOAuthState( - state: string, -): string | undefined { +function parseAppleRelayOAuthState(state: string): { + posthogDistinctId?: string; + flow: AuthOAuthFlow; +} { try { - const parsed = JSON.parse(base64UrlDecode(state)) as { ph?: unknown }; - return typeof parsed.ph === "string" && parsed.ph.length > 0 - ? parsed.ph - : undefined; + const parsed = JSON.parse(base64UrlDecode(state)) as { + ph?: unknown; + fl?: unknown; + }; + const posthogDistinctId = + typeof parsed.ph === "string" && parsed.ph.length > 0 + ? parsed.ph + : undefined; + const flow = + parsed.fl === "signup" || parsed.fl === "login" ? parsed.fl : "login"; + return { posthogDistinctId, flow }; } catch { - return undefined; + return { flow: "login" }; } } @@ -133,6 +164,6 @@ export function posthogDistinctIdFromSignInOAuthState( if (!state) return undefined; return ( parseSignInOAuthState(state)?.posthogDistinctId ?? - posthogDistinctIdFromAppleOAuthState(state) + parseAppleRelayOAuthState(state).posthogDistinctId ); } diff --git a/packages/sync/src/server/connection.routes.db.test.ts b/packages/sync/src/server/connection.routes.db.test.ts index fbe9cab9e3..cd2d97964f 100644 --- a/packages/sync/src/server/connection.routes.db.test.ts +++ b/packages/sync/src/server/connection.routes.db.test.ts @@ -1377,7 +1377,7 @@ describe("GET /sync/google", () => { expect(captured).toHaveLength(1); expect(captured[0]!.distinctId).toBe(principalId); expect(captured[0]!.properties["outcome"]).toBe("error"); - expect(captured[0]!.properties["reason"]).toBe("token_exchange_error"); + expect(captured[0]!.properties["reason"]).toBe("token_exchange_failed"); expect(captured[0]!.properties["provider_error"]).toBe("exchangeFailed"); expect(captured[0]!.properties["errorClass"]).toBe("ProviderAuthError"); expect(captured[0]!.properties).not.toHaveProperty("message");