Merge pull request #346 from trails-cool/stigi/wahoo-revoke

Revoke Wahoo tokens on disconnect; surface OAuth errors

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<typeof vi.fn>;
+  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<typeof vi.fn>;
+  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();
+  });
+});

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<void> {
+    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;

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<PushRouteResult>;
+
+  /** Optional: revoke the given access token at the provider. Best-effort — failures should be swallowed by callers. */
+  revoke?: (tokens: TokenSet) => Promise<void>;
 }
 
 export function providerSupportsPush(

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) {

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");
 }

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 (
     <section>
       <h2 className="text-lg font-semibold text-gray-900">{t("settings.services.title")}</h2>
+      {errorKey && (
+        <div
+          role="alert"
+          className="mt-4 rounded-md border border-red-200 bg-red-50 px-4 py-3 text-sm text-red-800"
+        >
+          {t(`settings.services.errors.${errorKey}`)}
+        </div>
+      )}
       <div className="mt-4 space-y-3">
         {providers.map((p) => (
           <div

packages/i18n/src/locales/de.ts

@@ -389,6 +389,12 @@ export default {
         connect: "Verbinden",
         disconnect: "Trennen",
         connectedAs: "Verbunden (ID: {{id}})",
+        errors: {
+          too_many_tokens:
+            "Für diesen Dienst gibt es zu viele aktive Autorisierungen für dein Konto. Öffne die App des Anbieters, widerrufe dort die trails.cool-Autorisierung und versuche es dann erneut.",
+          sync_failed: "Verbindung fehlgeschlagen — der Anbieter hat die Anfrage abgelehnt. Bitte versuche es erneut.",
+          generic: "Verbindung fehlgeschlagen — der Anbieter hat die Anfrage abgelehnt. Bitte versuche es erneut.",
+        },
       },
     },
     sync: {

packages/i18n/src/locales/en.ts

@@ -389,6 +389,12 @@ export default {
         connect: "Connect",
         disconnect: "Disconnect",
         connectedAs: "Connected (ID: {{id}})",
+        errors: {
+          too_many_tokens:
+            "This provider has too many active authorizations for your account. Open the provider's app, revoke the trails.cool authorization there, then try connecting again.",
+          sync_failed: "Couldn't connect — the provider rejected the request. Please try again.",
+          generic: "Couldn't connect — the provider rejected the request. Please try again.",
+        },
       },
     },
     sync: {