diff --git a/apps/journal/app/lib/connected-services/oauth-flow.server.test.ts b/apps/journal/app/lib/connected-services/oauth-flow.server.test.ts new file mode 100644 index 0000000..f627ebb --- /dev/null +++ b/apps/journal/app/lib/connected-services/oauth-flow.server.test.ts @@ -0,0 +1,162 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { createHash } from "node:crypto"; + +const link = vi.fn(); +vi.mock("./manager.ts", () => ({ + link: (...args: unknown[]) => link(...args), +})); +vi.mock("../config.server.ts", () => ({ + getOrigin: () => "https://journal.test", +})); + +import { initiateOAuthFlow, completeOAuthFlow } from "./oauth-flow.server.ts"; +import { decodeOAuthState } from "./oauth-state.server.ts"; +import type { ProviderManifest } from "./registry.ts"; + +function fakeManifest(overrides: Partial = {}): ProviderManifest { + return { + id: "fakeprov", + displayName: "Fake Provider", + credentialKind: "oauth", + credentialAdapter: { isExpired: () => false, refresh: vi.fn() }, + buildAuthUrl: vi.fn( + (redirectUri: string, state: string, extras?: { codeChallenge?: string }) => + `https://provider.test/authorize?redirect_uri=${encodeURIComponent(redirectUri)}&state=${state}` + + (extras?.codeChallenge ? `&code_challenge=${extras.codeChallenge}` : ""), + ), + exchangeCode: vi.fn(), + ...overrides, + } as unknown as ProviderManifest; +} + +function callbackRequest(params: Record, cookie?: string): Request { + const url = new URL("https://journal.test/api/sync/callback/fakeprov"); + for (const [k, v] of Object.entries(params)) url.searchParams.set(k, v); + return new Request(url, { headers: cookie ? { Cookie: cookie } : {} }); +} + +beforeEach(() => vi.clearAllMocks()); + +describe("initiateOAuthFlow", () => { + it("redirects to the provider with the intent in the state, no cookie for non-PKCE", () => { + const manifest = fakeManifest(); + const resp = initiateOAuthFlow(manifest, { returnTo: "/settings/connections" }); + + expect(resp.status).toBe(302); + const location = new URL(resp.headers.get("Location")!); + expect(location.origin).toBe("https://provider.test"); + expect(location.searchParams.get("redirect_uri")).toBe( + "https://journal.test/api/sync/callback/fakeprov", + ); + expect(decodeOAuthState(location.searchParams.get("state"))).toEqual({ + returnTo: "/settings/connections", + }); + expect(resp.headers.get("Set-Cookie")).toBeNull(); + }); + + it("PKCE: sets the verifier cookie and sends its S256 challenge", () => { + const manifest = fakeManifest({ pkce: true }); + const resp = initiateOAuthFlow(manifest, { pushAfter: { routeId: "r-1" }, returnTo: "/r" }); + + const cookie = resp.headers.get("Set-Cookie")!; + expect(cookie).toContain("__oauth_pkce="); + expect(cookie).toContain("HttpOnly"); + const verifier = cookie.match(/__oauth_pkce=([^;]+)/)![1]!; + + const location = new URL(resp.headers.get("Location")!); + const expectedChallenge = createHash("sha256").update(verifier).digest("base64url"); + expect(location.searchParams.get("code_challenge")).toBe(expectedChallenge); + // push intent rides the state even on the PKCE path + expect(decodeOAuthState(location.searchParams.get("state"))).toEqual({ + pushAfter: { routeId: "r-1" }, + returnTo: "/r", + }); + }); +}); + +describe("completeOAuthFlow", () => { + it("maps access_denied to denied with decoded state", async () => { + const manifest = fakeManifest(); + const state = new URL( + initiateOAuthFlow(manifest, { returnTo: "/x" }).headers.get("Location")!, + ).searchParams.get("state")!; + + const result = await completeOAuthFlow( + manifest, + callbackRequest({ error: "access_denied", state }), + "user-1", + ); + expect(result).toEqual({ status: "denied", state: { returnTo: "/x" } }); + expect(link).not.toHaveBeenCalled(); + }); + + it("returns missing_code when no code param", async () => { + const result = await completeOAuthFlow(fakeManifest(), callbackRequest({}), "user-1"); + expect(result.status).toBe("missing_code"); + }); + + it("PKCE: returns missing_verifier when the cookie is gone", async () => { + const result = await completeOAuthFlow( + fakeManifest({ pkce: true }), + callbackRequest({ code: "abc" }), + "user-1", + ); + expect(result.status).toBe("missing_verifier"); + }); + + it("exchanges the code and links the connection", async () => { + const manifest = fakeManifest(); + (manifest.exchangeCode as ReturnType).mockResolvedValue({ + credentials: { accessToken: "t" }, + providerUserId: "prov-9", + grantedScopes: ["routes_write"], + }); + + const result = await completeOAuthFlow(manifest, callbackRequest({ code: "abc" }), "user-1"); + + expect(result.status).toBe("linked"); + expect(manifest.exchangeCode).toHaveBeenCalledWith( + "abc", + "https://journal.test/api/sync/callback/fakeprov", + undefined, + ); + expect(link).toHaveBeenCalledWith({ + userId: "user-1", + provider: "fakeprov", + credentialKind: "oauth", + credentials: { accessToken: "t" }, + providerUserId: "prov-9", + grantedScopes: ["routes_write"], + }); + }); + + it("PKCE: passes the cookie verifier to exchangeCode", async () => { + const manifest = fakeManifest({ pkce: true }); + (manifest.exchangeCode as ReturnType).mockResolvedValue({ + credentials: {}, + providerUserId: "p", + grantedScopes: [], + }); + + await completeOAuthFlow( + manifest, + callbackRequest({ code: "abc" }, "__oauth_pkce=my-verifier"), + "user-1", + ); + expect(manifest.exchangeCode).toHaveBeenCalledWith( + "abc", + expect.any(String), + { codeVerifier: "my-verifier" }, + ); + }); + + it("maps exchange failures to error with the provider code", async () => { + const manifest = fakeManifest(); + (manifest.exchangeCode as ReturnType).mockRejectedValue( + Object.assign(new Error("nope"), { code: "rate_limited" }), + ); + + const result = await completeOAuthFlow(manifest, callbackRequest({ code: "x" }), "user-1"); + expect(result).toMatchObject({ status: "error", code: "rate_limited" }); + }); +}); diff --git a/apps/journal/app/lib/connected-services/oauth-flow.server.ts b/apps/journal/app/lib/connected-services/oauth-flow.server.ts new file mode 100644 index 0000000..b530b12 --- /dev/null +++ b/apps/journal/app/lib/connected-services/oauth-flow.server.ts @@ -0,0 +1,122 @@ +// The OAuth connect → callback lifecycle as one module. Routes stay +// thin adapters: they look up the manifest, call initiate/complete, +// and map the result to redirects. Everything protocol-shaped — +// state encoding, PKCE pair generation, the verifier cookie's +// lifecycle, redirect-URI construction, code exchange, linking the +// connection — lives here, so the next OAuth provider (Coros, Strava) +// reuses the flow instead of the pattern. + +import { redirect } from "react-router"; +import { getOrigin } from "../config.server.ts"; +import { link } from "./manager.ts"; +import type { ProviderManifest } from "./registry.ts"; +import { + clearPkceCookieHeader, + decodeOAuthState, + encodeOAuthState, + generatePkcePair, + pkceCookieHeader, + readPkceVerifier, + type PushOAuthState, +} from "./oauth-state.server.ts"; + +function callbackUri(manifest: ProviderManifest): string { + return `${getOrigin()}/api/sync/callback/${manifest.id}`; +} + +/** + * Build the provider authorization redirect. Encodes post-callback + * intent into the state param and, for PKCE providers, sets the + * httpOnly verifier cookie — including when the flow is initiated + * from a push-resume (the old inline code only handled PKCE on the + * plain connect path). + * + * Caller must have verified `manifest.buildAuthUrl` exists. + */ +export function initiateOAuthFlow( + manifest: ProviderManifest, + intent: PushOAuthState, +): Response { + if (!manifest.buildAuthUrl) { + throw new Error(`Provider ${manifest.id} has no buildAuthUrl`); + } + const state = encodeOAuthState(intent); + const redirectUri = callbackUri(manifest); + + if (manifest.pkce) { + const { verifier, challenge } = generatePkcePair(); + return redirect(manifest.buildAuthUrl(redirectUri, state, { codeChallenge: challenge }), { + headers: { "Set-Cookie": pkceCookieHeader(verifier) }, + }); + } + return redirect(manifest.buildAuthUrl(redirectUri, state)); +} + +export type OAuthCompletion = + /** Provider sent the user back with access_denied. */ + | { status: "denied"; state: PushOAuthState } + /** No authorization code in the callback URL. */ + | { status: "missing_code"; state: PushOAuthState } + /** PKCE provider but the verifier cookie is gone (expired / cleared). */ + | { status: "missing_verifier"; state: PushOAuthState } + /** Code exchange or linking failed; `code` is provider-specific or "sync_failed". */ + | { status: "error"; code: string; state: PushOAuthState } + /** Connection linked. */ + | { status: "linked"; state: PushOAuthState }; + +/** + * Consume the provider callback: decode state, recover the PKCE + * verifier, exchange the code, and link the connection. Never throws — + * every outcome is a variant the route maps to a redirect. The spent + * verifier cookie should be cleared on whatever response the route + * returns (`clearPkceCookieHeader`). + */ +export async function completeOAuthFlow( + manifest: ProviderManifest, + request: Request, + userId: string, +): Promise { + if (!manifest.exchangeCode) { + throw new Error(`Provider ${manifest.id} has no exchangeCode`); + } + const url = new URL(request.url); + const state = decodeOAuthState(url.searchParams.get("state")); + + if (url.searchParams.get("error") === "access_denied") { + return { status: "denied", state }; + } + + const code = url.searchParams.get("code"); + if (!code) return { status: "missing_code", state }; + + const codeVerifier = manifest.pkce ? readPkceVerifier(request) : null; + if (manifest.pkce && !codeVerifier) { + return { status: "missing_verifier", state }; + } + + try { + const exchange = await manifest.exchangeCode( + code, + callbackUri(manifest), + codeVerifier ? { codeVerifier } : undefined, + ); + await link({ + userId, + provider: manifest.id, + credentialKind: manifest.credentialKind, + credentials: exchange.credentials as Record, + providerUserId: exchange.providerUserId, + grantedScopes: exchange.grantedScopes, + }); + return { status: "linked", state }; + } catch (e) { + console.error(`OAuth callback failed for ${manifest.id}:`, e); + const code = + typeof (e as { code?: string }).code === "string" + ? (e as { code: string }).code + : "sync_failed"; + return { status: "error", code, state }; + } +} + +export { clearPkceCookieHeader }; diff --git a/apps/journal/app/routes/api.sync.callback.$provider.ts b/apps/journal/app/routes/api.sync.callback.$provider.ts index e9c0db0..f935ced 100644 --- a/apps/journal/app/routes/api.sync.callback.$provider.ts +++ b/apps/journal/app/routes/api.sync.callback.$provider.ts @@ -1,13 +1,11 @@ import { redirect, data } from "react-router"; -import { getOrigin } from "~/lib/config.server"; import type { Route } from "./+types/api.sync.callback.$provider"; import { requireSessionUser } from "~/lib/auth/session.server"; -import { getManifest, link } from "~/lib/connected-services"; +import { getManifest } from "~/lib/connected-services"; import { - decodeOAuthState, - readPkceVerifier, + completeOAuthFlow, clearPkceCookieHeader, -} from "~/lib/connected-services/oauth-state.server"; +} from "~/lib/connected-services/oauth-flow.server"; import { pushRouteToProvider } from "~/lib/connected-services/push-action.server"; export async function loader({ params, request }: Route.LoaderArgs) { @@ -18,51 +16,28 @@ export async function loader({ params, request }: Route.LoaderArgs) { return data({ error: "Unknown provider" }, { status: 404 }); } - const url = new URL(request.url); - const state = decodeOAuthState(url.searchParams.get("state")); + const result = await completeOAuthFlow(manifest, request, user.id); + const state = result.state; const fallbackReturn = state.returnTo ?? "/settings"; + // Spent (or irrelevant) verifier — clear it on every outcome. + const headers = { "Set-Cookie": clearPkceCookieHeader() }; - // User denied the new scope at Wahoo. Send them back to the originating - // page with a notice instead of looping them through OAuth again. - if (url.searchParams.get("error") === "access_denied") { - return redirect(`${fallbackReturn}?push=needs_permission`); - } - - const code = url.searchParams.get("code"); - if (!code) return data({ error: "Missing authorization code" }, { status: 400 }); - - const origin = getOrigin(); - const redirectUri = `${origin}/api/sync/callback/${params.provider}`; - - // PKCE providers: recover the verifier from the connect-time cookie. - const codeVerifier = manifest.pkce ? readPkceVerifier(request) : null; - if (manifest.pkce && !codeVerifier) { - return redirect(`${fallbackReturn}?error=sync_failed`); - } - - try { - const exchange = await manifest.exchangeCode( - code, - redirectUri, - codeVerifier ? { codeVerifier } : undefined, - ); - await link({ - userId: user.id, - provider: manifest.id, - credentialKind: manifest.credentialKind, - credentials: exchange.credentials as Record, - providerUserId: exchange.providerUserId, - grantedScopes: exchange.grantedScopes, - }); - } catch (e) { - console.error(`OAuth callback failed for ${params.provider}:`, e); - const errCode = - typeof (e as { code?: string }).code === "string" - ? (e as { code: string }).code - : "sync_failed"; - return redirect(`${fallbackReturn}?error=${errCode}`); + switch (result.status) { + case "denied": + // User declined at the provider. Back to the originating page + // with a notice instead of looping them through OAuth again. + return redirect(`${fallbackReturn}?push=needs_permission`, { headers }); + case "missing_code": + return data({ error: "Missing authorization code" }, { status: 400 }); + case "missing_verifier": + return redirect(`${fallbackReturn}?error=sync_failed`, { headers }); + case "error": + return redirect(`${fallbackReturn}?error=${result.code}`, { headers }); + case "linked": + break; } + // Resume an interrupted push now that the connection has the scope. if (state.pushAfter?.routeId) { const outcome = await pushRouteToProvider({ userId: user.id, @@ -70,16 +45,12 @@ export async function loader({ params, request }: Route.LoaderArgs) { routeId: state.pushAfter.routeId, }); const target = state.returnTo ?? `/routes/${state.pushAfter.routeId}`; - if (outcome.status === "success") return redirect(`${target}?push=success`); - if (outcome.status === "scope_missing") return redirect(`${target}?push=needs_permission`); - if (outcome.status === "needs_relink") return redirect(`${target}?push=needs_permission`); - if (outcome.status === "error") return redirect(`${target}?push=error&code=${outcome.code}`); - return redirect(`${target}?push=${outcome.status}`); + if (outcome.status === "success") return redirect(`${target}?push=success`, { headers }); + if (outcome.status === "scope_missing") return redirect(`${target}?push=needs_permission`, { headers }); + if (outcome.status === "needs_relink") return redirect(`${target}?push=needs_permission`, { headers }); + if (outcome.status === "error") return redirect(`${target}?push=error&code=${outcome.code}`, { headers }); + return redirect(`${target}?push=${outcome.status}`, { headers }); } - return redirect(state.returnTo ?? "/settings", { - // Spent verifier — clear it regardless of which provider this was - // (harmless no-op for non-PKCE providers without the cookie). - headers: { "Set-Cookie": clearPkceCookieHeader() }, - }); + return redirect(state.returnTo ?? "/settings", { headers }); } diff --git a/apps/journal/app/routes/api.sync.connect.$provider.ts b/apps/journal/app/routes/api.sync.connect.$provider.ts index 6e27d09..8118f96 100644 --- a/apps/journal/app/routes/api.sync.connect.$provider.ts +++ b/apps/journal/app/routes/api.sync.connect.$provider.ts @@ -1,13 +1,8 @@ -import { redirect, data } from "react-router"; -import { getOrigin } from "~/lib/config.server"; +import { data } from "react-router"; import type { Route } from "./+types/api.sync.connect.$provider"; import { requireSessionUser } from "~/lib/auth/session.server"; import { getManifest } from "~/lib/connected-services"; -import { - encodeOAuthState, - generatePkcePair, - pkceCookieHeader, -} from "~/lib/connected-services/oauth-state.server"; +import { initiateOAuthFlow } from "~/lib/connected-services/oauth-flow.server"; export async function loader({ params, request }: Route.LoaderArgs) { await requireSessionUser(request); @@ -17,19 +12,5 @@ export async function loader({ params, request }: Route.LoaderArgs) { return data({ error: "Unknown provider" }, { status: 404 }); } - const origin = getOrigin(); - const redirectUri = `${origin}/api/sync/callback/${params.provider}`; - const state = encodeOAuthState({ returnTo: "/settings/connections" }); - - // PKCE providers (Garmin): the verifier crosses the redirect in an - // httpOnly cookie; only the S256 challenge goes to the provider. - if (manifest.pkce) { - const { verifier, challenge } = generatePkcePair(); - return redirect( - manifest.buildAuthUrl(redirectUri, state, { codeChallenge: challenge }), - { headers: { "Set-Cookie": pkceCookieHeader(verifier) } }, - ); - } - - return redirect(manifest.buildAuthUrl(redirectUri, state)); + return initiateOAuthFlow(manifest, { returnTo: "/settings/connections" }); } diff --git a/apps/journal/app/routes/api.sync.push.$provider.$routeId.ts b/apps/journal/app/routes/api.sync.push.$provider.$routeId.ts index bb4d7e3..9bb93ea 100644 --- a/apps/journal/app/routes/api.sync.push.$provider.$routeId.ts +++ b/apps/journal/app/routes/api.sync.push.$provider.$routeId.ts @@ -1,10 +1,9 @@ import { redirect, data } from "react-router"; -import { getOrigin } from "~/lib/config.server"; import type { Route } from "./+types/api.sync.push.$provider.$routeId"; import { requireSessionUser } from "~/lib/auth/session.server"; import { getManifest } from "~/lib/connected-services"; import { pushRouteToProvider } from "~/lib/connected-services/push-action.server"; -import { encodeOAuthState } from "~/lib/connected-services/oauth-state.server"; +import { initiateOAuthFlow } from "~/lib/connected-services/oauth-flow.server"; export async function action({ params, request }: Route.ActionArgs) { const user = await requireSessionUser(request); @@ -26,13 +25,12 @@ export async function action({ params, request }: Route.ActionArgs) { if (!manifest.buildAuthUrl) { return redirect(`${returnTo}?push=needs_permission`); } - const origin = getOrigin(); - const redirectUri = `${origin}/api/sync/callback/${manifest.id}`; - const state = encodeOAuthState({ + // Re-authorize with the push intent in the state; the callback + // resumes the push once the scope is granted. + return initiateOAuthFlow(manifest, { pushAfter: { routeId: params.routeId }, returnTo, }); - return redirect(manifest.buildAuthUrl(redirectUri, state)); } case "needs_relink": return redirect(`${returnTo}?push=needs_permission`);