diff --git a/apps/journal/app/lib/sync/providers/wahoo.test.ts b/apps/journal/app/lib/sync/providers/wahoo.test.ts index 443b0ba..74b33d3 100644 --- a/apps/journal/app/lib/sync/providers/wahoo.test.ts +++ b/apps/journal/app/lib/sync/providers/wahoo.test.ts @@ -2,7 +2,7 @@ import { readFileSync } from "node:fs"; import { resolve } from "node:path"; import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { wahooProvider } from "./wahoo"; -import { PushError } from "../types.ts"; +import { PushError, OAuthError } from "../types.ts"; const fixturePath = resolve(__dirname, "__fixtures__/wahoo-ride.fit"); const fitBuffer = readFileSync(fixturePath); @@ -146,3 +146,62 @@ describe("wahooProvider.pushRoute", () => { await expect(wahooProvider.pushRoute!(tokens, basePayload)).rejects.toBeInstanceOf(PushError); }); }); + +describe("wahooProvider.exchangeCode", () => { + let fetchMock: ReturnType; + beforeEach(() => { + fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + }); + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("maps Wahoo's 'too many unrevoked access tokens' 400 to OAuthError(too_many_tokens)", async () => { + fetchMock.mockResolvedValueOnce({ + ok: false, + status: 400, + text: async () => + '{"error":"Too many unrevoked access tokens exist for this app and user. ..."}', + }); + await expect(wahooProvider.exchangeCode("code", "https://x/cb")).rejects.toMatchObject({ + name: "OAuthError", + code: "too_many_tokens", + status: 400, + }); + }); +}); + +describe("wahooProvider.revoke", () => { + let fetchMock: ReturnType; + beforeEach(() => { + fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + }); + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("DELETEs /v1/permissions with the bearer token", async () => { + fetchMock.mockResolvedValueOnce({ ok: true, status: 204, text: async () => "" }); + await wahooProvider.revoke!({ + accessToken: "tok", + refreshToken: "r", + expiresAt: new Date(), + }); + expect(fetchMock).toHaveBeenCalledOnce(); + const [url, init] = fetchMock.mock.calls[0]!; + expect(url).toBe("https://api.wahooligan.com/v1/permissions"); + expect(init.method).toBe("DELETE"); + expect(init.headers.Authorization).toBe("Bearer tok"); + }); + + it("throws on non-OK response", async () => { + fetchMock.mockResolvedValueOnce({ ok: false, status: 401, text: async () => "bad" }); + await expect( + wahooProvider.revoke!({ accessToken: "tok", refreshToken: "r", expiresAt: new Date() }), + ).rejects.toThrow(); + // Reference OAuthError to keep the import used even if future tests trim above blocks. + expect(OAuthError).toBeDefined(); + }); +}); diff --git a/apps/journal/app/lib/sync/providers/wahoo.ts b/apps/journal/app/lib/sync/providers/wahoo.ts index 210132a..c3c283a 100644 --- a/apps/journal/app/lib/sync/providers/wahoo.ts +++ b/apps/journal/app/lib/sync/providers/wahoo.ts @@ -9,7 +9,7 @@ import type { PushRoutePayload, PushRouteResult, } from "../types.ts"; -import { PushError } from "../types.ts"; +import { PushError, OAuthError } from "../types.ts"; const WAHOO_API = "https://api.wahooligan.com"; const WAHOO_AUTH = "https://api.wahooligan.com/oauth"; @@ -47,7 +47,10 @@ export const wahooProvider: SyncProvider = { }); if (!resp.ok) { const text = await resp.text().catch(() => ""); - throw new Error(`Wahoo token exchange failed: ${resp.status} ${text}`); + if (text.includes("Too many unrevoked access tokens")) { + throw new OAuthError("too_many_tokens", text, resp.status); + } + throw new OAuthError("generic", `Wahoo token exchange failed: ${resp.status} ${text}`, resp.status); } const data = await resp.json() as { access_token: string; refresh_token: string; expires_in: number }; @@ -217,6 +220,17 @@ export const wahooProvider: SyncProvider = { throw new PushError("generic", `Wahoo route push failed: ${resp.status} ${text}`, resp.status); }, + async revoke(tokens: TokenSet): Promise { + const resp = await fetch(`${WAHOO_API}/v1/permissions`, { + method: "DELETE", + headers: { Authorization: `Bearer ${tokens.accessToken}` }, + }); + if (!resp.ok) { + const text = await resp.text().catch(() => ""); + throw new Error(`Wahoo revoke failed: ${resp.status} ${text}`); + } + }, + parseWebhook(body: unknown): WebhookEvent | null { const payload = body as { event_type?: string; diff --git a/apps/journal/app/lib/sync/types.ts b/apps/journal/app/lib/sync/types.ts index fdccf82..d3ac0c9 100644 --- a/apps/journal/app/lib/sync/types.ts +++ b/apps/journal/app/lib/sync/types.ts @@ -58,6 +58,20 @@ export type PushErrorCode = | "rate_limit" | "generic"; +export type OAuthErrorCode = "too_many_tokens" | "generic"; + +export class OAuthError extends Error { + code: OAuthErrorCode; + status?: number; + + constructor(code: OAuthErrorCode, message: string, status?: number) { + super(message); + this.name = "OAuthError"; + this.code = code; + this.status = status; + } +} + export class PushError extends Error { code: PushErrorCode; status?: number; @@ -87,6 +101,9 @@ export interface SyncProvider { /** Optional: providers that can accept routes implement this. UI hides the action when undefined. */ pushRoute?: (tokens: TokenSet, payload: PushRoutePayload) => Promise; + + /** Optional: revoke the given access token at the provider. Best-effort — failures should be swallowed by callers. */ + revoke?: (tokens: TokenSet) => Promise; } export function providerSupportsPush( diff --git a/apps/journal/app/routes/api.sync.callback.$provider.ts b/apps/journal/app/routes/api.sync.callback.$provider.ts index 4ad85c5..340532d 100644 --- a/apps/journal/app/routes/api.sync.callback.$provider.ts +++ b/apps/journal/app/routes/api.sync.callback.$provider.ts @@ -4,6 +4,7 @@ import { getSessionUser } from "~/lib/auth.server"; import { getProvider } from "~/lib/sync/registry"; import { saveConnection } from "~/lib/sync/connections.server"; import { decodeOAuthState, pushRouteToProvider } from "~/lib/sync/pushes.server"; +import { OAuthError } from "~/lib/sync/types"; export async function loader({ params, request }: Route.LoaderArgs) { const user = await getSessionUser(request); @@ -35,7 +36,8 @@ export async function loader({ params, request }: Route.LoaderArgs) { await saveConnection(user.id, provider.id, tokens, provider.scopes); } catch (e) { console.error(`OAuth callback failed for ${params.provider}:`, e); - return redirect(`${fallbackReturn}?error=sync_failed`); + const code = e instanceof OAuthError ? e.code : "sync_failed"; + return redirect(`${fallbackReturn}?error=${code}`); } if (state.pushAfter?.routeId) { diff --git a/apps/journal/app/routes/api.sync.disconnect.$provider.ts b/apps/journal/app/routes/api.sync.disconnect.$provider.ts index 1021587..83ff544 100644 --- a/apps/journal/app/routes/api.sync.disconnect.$provider.ts +++ b/apps/journal/app/routes/api.sync.disconnect.$provider.ts @@ -2,7 +2,7 @@ import { redirect, data } from "react-router"; import type { Route } from "./+types/api.sync.disconnect.$provider"; import { getSessionUser } from "~/lib/auth.server"; import { getProvider } from "~/lib/sync/registry"; -import { deleteConnection } from "~/lib/sync/connections.server"; +import { deleteConnection, getConnection } from "~/lib/sync/connections.server"; export async function action({ params, request }: Route.ActionArgs) { const user = await getSessionUser(request); @@ -11,6 +11,22 @@ export async function action({ params, request }: Route.ActionArgs) { const provider = getProvider(params.provider); if (!provider) return data({ error: "Unknown provider" }, { status: 404 }); + if (provider.revoke) { + const conn = await getConnection(user.id, provider.id); + if (conn) { + try { + await provider.revoke({ + accessToken: conn.accessToken, + refreshToken: conn.refreshToken, + expiresAt: conn.expiresAt, + }); + } catch (e) { + // Best-effort: token may already be expired/revoked. Drop the local row regardless. + console.warn(`Failed to revoke ${provider.id} token (continuing with disconnect):`, e); + } + } + } + await deleteConnection(user.id, provider.id); return redirect("/settings"); } diff --git a/apps/journal/app/routes/settings.connections.tsx b/apps/journal/app/routes/settings.connections.tsx index 682e5a7..db046df 100644 --- a/apps/journal/app/routes/settings.connections.tsx +++ b/apps/journal/app/routes/settings.connections.tsx @@ -1,4 +1,4 @@ -import { data, redirect } from "react-router"; +import { data, redirect, useSearchParams } from "react-router"; import { useTranslation } from "react-i18next"; import { eq } from "drizzle-orm"; import type { Route } from "./+types/settings.connections"; @@ -7,6 +7,12 @@ import { getDb } from "~/lib/db"; import { syncConnections } from "@trails-cool/db/schema/journal"; import { getAllProviders } from "~/lib/sync/registry"; +const KNOWN_ERRORS = ["too_many_tokens", "sync_failed", "generic"] as const; +type KnownError = (typeof KNOWN_ERRORS)[number]; +function isKnownError(value: string | null): value is KnownError { + return value !== null && (KNOWN_ERRORS as readonly string[]).includes(value); +} + export function meta() { return [{ title: "Connected services — Settings — trails.cool" }]; } @@ -35,10 +41,21 @@ export async function loader({ request }: Route.LoaderArgs) { export default function ConnectionsSettings({ loaderData }: Route.ComponentProps) { const { providers } = loaderData; const { t } = useTranslation(["journal"]); + const [searchParams] = useSearchParams(); + const errorParam = searchParams.get("error"); + const errorKey: KnownError | null = isKnownError(errorParam) ? errorParam : errorParam ? "generic" : null; return (

{t("settings.services.title")}

+ {errorKey && ( +
+ {t(`settings.services.errors.${errorKey}`)} +
+ )}
{providers.map((p) => (