From 8ef7ec022feef5ff32ae18d930947ad7e05c66c4 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 14 Oct 2026 03:04:17 +0200 Subject: [PATCH] fix: deliver a plugin OAuth callback only to the plugin that started the flow --- .../plugin-oauth-callback-listener.test.tsx | 53 ++++++++++++++----- .../plugin-oauth-callback-listener.tsx | 9 +++- lib/plugin-hooks.ts | 12 +++++ lib/plugin-oauth.ts | 53 +++++++++++++++++++ lib/plugin-sandbox/host-api.ts | 13 ++++- lib/plugin-sandbox/runtime.tsx | 7 +++ 6 files changed, 133 insertions(+), 14 deletions(-) diff --git a/components/__tests__/plugin-oauth-callback-listener.test.tsx b/components/__tests__/plugin-oauth-callback-listener.test.tsx index 6b738810c..2c84da6d9 100644 --- a/components/__tests__/plugin-oauth-callback-listener.test.tsx +++ b/components/__tests__/plugin-oauth-callback-listener.test.tsx @@ -2,6 +2,7 @@ import { authHooks } from "@/lib/plugin-hooks"; import { PLUGIN_OAUTH_EVENT, PLUGIN_OAUTH_STORAGE_KEY, + createOAuthState, } from "@/lib/plugin-oauth"; import { act, render } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -25,6 +26,8 @@ function emitStorageEvent(newValue: string | null) { }); } +let STATE = ""; + describe("PluginOAuthCallbackListener", () => { let handler: (payload: unknown) => Promise; let disposable: { dispose: () => void }; @@ -35,6 +38,7 @@ describe("PluginOAuthCallbackListener", () => { mockUsePathname.mockReturnValue("/en/mail"); handler = vi.fn<(payload: unknown) => Promise>().mockResolvedValue(undefined); disposable = authHooks.onOAuthCallback.register("test-plugin", handler); + STATE = createOAuthState("test-plugin"); }); afterEach(() => { @@ -43,14 +47,14 @@ describe("PluginOAuthCallbackListener", () => { it("delivers a success payload via the storage event", () => { const { unmount } = render(); - const payload = { code: "abc", state: "xyz", receivedAt: 1 }; + const payload = { code: "abc", state: STATE, receivedAt: 1 }; localStorage.setItem(PLUGIN_OAUTH_STORAGE_KEY, JSON.stringify(payload)); emitStorageEvent(JSON.stringify(payload)); expect(handler).toHaveBeenCalledOnce(); expect(handler).toHaveBeenCalledWith( - expect.objectContaining({ code: "abc", state: "xyz" }), + expect.objectContaining({ code: "abc", state: STATE }), ); // Best-effort cleanup after delivery expect(localStorage.getItem(PLUGIN_OAUTH_STORAGE_KEY)).toBeNull(); @@ -59,14 +63,14 @@ describe("PluginOAuthCallbackListener", () => { it("delivers an error payload (denial) with state", () => { const { unmount } = render(); - const payload = { state: "xyz", error: "access_denied", receivedAt: 1 }; + const payload = { state: STATE, error: "access_denied", receivedAt: 1 }; localStorage.setItem(PLUGIN_OAUTH_STORAGE_KEY, JSON.stringify(payload)); emitStorageEvent(JSON.stringify(payload)); expect(handler).toHaveBeenCalledOnce(); expect(handler).toHaveBeenCalledWith( - expect.objectContaining({ state: "xyz", error: "access_denied" }), + expect.objectContaining({ state: STATE, error: "access_denied" }), ); unmount(); }); @@ -74,7 +78,7 @@ describe("PluginOAuthCallbackListener", () => { it("does nothing on the callback route", () => { mockUsePathname.mockReturnValue("/en/plugins/oauth/callback"); const { unmount } = render(); - const payload = { code: "abc", state: "xyz", receivedAt: 1 }; + const payload = { code: "abc", state: STATE, receivedAt: 1 }; localStorage.setItem(PLUGIN_OAUTH_STORAGE_KEY, JSON.stringify(payload)); emitStorageEvent(JSON.stringify(payload)); @@ -95,7 +99,7 @@ describe("PluginOAuthCallbackListener", () => { emitStorageEvent("not json"); emitStorageEvent(JSON.stringify({ foo: "bar" })); - emitStorageEvent(JSON.stringify({ code: 123, state: "xyz" })); + emitStorageEvent(JSON.stringify({ code: 123, state: STATE })); emitStorageEvent(null); expect(handler).not.toHaveBeenCalled(); @@ -119,7 +123,7 @@ describe("PluginOAuthCallbackListener", () => { try { vi.setSystemTime(0); const { unmount } = render(); - const payload = { code: "abc", state: "xyz", receivedAt: 1 }; + const payload = { code: "abc", state: STATE, receivedAt: 1 }; localStorage.setItem(PLUGIN_OAUTH_STORAGE_KEY, JSON.stringify(payload)); emitStorageEvent(JSON.stringify(payload)); @@ -127,10 +131,10 @@ describe("PluginOAuthCallbackListener", () => { expect(handler).toHaveBeenCalledOnce(); - // Outside the dedup window the same payload is delivered again + // A state is used once: replaying it later delivers nothing either. vi.setSystemTime(61_000); emitStorageEvent(JSON.stringify(payload)); - expect(handler).toHaveBeenCalledTimes(2); + expect(handler).toHaveBeenCalledOnce(); unmount(); } finally { @@ -140,7 +144,7 @@ describe("PluginOAuthCallbackListener", () => { it("delivers same-tab custom events", () => { const { unmount } = render(); - const payload = { code: "abc", state: "xyz", receivedAt: 1 }; + const payload = { code: "abc", state: STATE, receivedAt: 1 }; act(() => { window.dispatchEvent( @@ -150,7 +154,7 @@ describe("PluginOAuthCallbackListener", () => { expect(handler).toHaveBeenCalledOnce(); expect(handler).toHaveBeenCalledWith( - expect.objectContaining({ code: "abc", state: "xyz" }), + expect.objectContaining({ code: "abc", state: STATE }), ); unmount(); }); @@ -159,7 +163,7 @@ describe("PluginOAuthCallbackListener", () => { const { unmount } = render(); unmount(); - const payload = { code: "abc", state: "xyz", receivedAt: 1 }; + const payload = { code: "abc", state: STATE, receivedAt: 1 }; localStorage.setItem(PLUGIN_OAUTH_STORAGE_KEY, JSON.stringify(payload)); emitStorageEvent(JSON.stringify(payload)); act(() => { @@ -170,4 +174,29 @@ describe("PluginOAuthCallbackListener", () => { expect(handler).not.toHaveBeenCalled(); }); + + it("delivers the code only to the plugin that started the flow", () => { + // Every plugin with http:fetch used to receive every code. + const other = vi.fn<(payload: unknown) => Promise>().mockResolvedValue(undefined); + const otherDisposable = authHooks.onOAuthCallback.register("other-plugin", other); + const { unmount } = render(); + + emitStorageEvent(JSON.stringify({ code: "abc", state: STATE, receivedAt: 1 })); + + expect(handler).toHaveBeenCalledOnce(); + expect(other).not.toHaveBeenCalled(); + otherDisposable.dispose(); + unmount(); + }); + + it("drops a callback whose state no plugin created", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const { unmount } = render(); + + emitStorageEvent(JSON.stringify({ code: "abc", state: "made-up", receivedAt: 1 })); + + expect(handler).not.toHaveBeenCalled(); + warn.mockRestore(); + unmount(); + }); }); diff --git a/components/providers/plugin-oauth-callback-listener.tsx b/components/providers/plugin-oauth-callback-listener.tsx index 3b4299257..e610c9665 100644 --- a/components/providers/plugin-oauth-callback-listener.tsx +++ b/components/providers/plugin-oauth-callback-listener.tsx @@ -20,6 +20,7 @@ import { authHooks } from '@/lib/plugin-hooks'; import { PLUGIN_OAUTH_EVENT as EVENT_NAME, PLUGIN_OAUTH_STORAGE_KEY as STORAGE_KEY, + takeOAuthStateOwner, type OAuthCallbackPayload, } from '@/lib/plugin-oauth'; import { usePathname } from 'next/navigation'; @@ -83,7 +84,13 @@ export function PluginOAuthCallbackListener() { lastCode = code; lastState = payload.state ?? null; lastAt = now; - void authHooks.onOAuthCallback.emit(payload); + // Only the plugin that created this state gets the code. + const pluginId = takeOAuthStateOwner(payload.state); + if (pluginId) { + void authHooks.onOAuthCallback.emitTo(pluginId, payload); + } else { + console.warn('[plugin-oauth] callback with a state no plugin asked for; ignored'); + } // Best-effort cleanup so a reload doesn't replay an old payload. try { localStorage.removeItem(STORAGE_KEY); } catch { /* ignore */ } }; diff --git a/lib/plugin-hooks.ts b/lib/plugin-hooks.ts index 58b2ffd38..3c6c48dcf 100644 --- a/lib/plugin-hooks.ts +++ b/lib/plugin-hooks.ts @@ -193,6 +193,18 @@ export class HookBus any> { } } + /** Observer emit to one plugin's handlers only. */ + async emitTo(pluginId: string, ...args: Parameters): Promise { + for (const { pluginId: owner, handler } of this.handlers) { + if (owner !== pluginId || pluginErrorTracker.isDisabled(owner)) continue; + try { + await withTimeout(handler(...args), this.timeoutMs); + } catch (err) { + this.recordFailure(owner, err); + } + } + } + /** Fire handlers as interceptors - any returning false cancels the operation */ async intercept(...args: Parameters): Promise { return (await this.interceptOutcome(...args)) === 'allowed'; diff --git a/lib/plugin-oauth.ts b/lib/plugin-oauth.ts index 16149e104..d5fb09fe1 100644 --- a/lib/plugin-oauth.ts +++ b/lib/plugin-oauth.ts @@ -30,3 +30,56 @@ export type OAuthCallbackPayload = error_description?: string; receivedAt?: number; }; + +// ─── State values bound to the plugin that started the flow ─── + +const STATES_KEY = "plugin-oauth-states"; +/** A flow that takes longer than this is abandoned. */ +const STATE_TTL_MS = 15 * 60_000; + +type StateTable = Record; + +function readStates(): StateTable { + try { + const raw = localStorage.getItem(STATES_KEY); + const parsed = raw ? JSON.parse(raw) as StateTable : {}; + const now = Date.now(); + for (const [state, entry] of Object.entries(parsed)) { + if (!entry || typeof entry.pluginId !== "string" || now - entry.created > STATE_TTL_MS) delete parsed[state]; + } + return parsed; + } catch { + return {}; + } +} + +function writeStates(table: StateTable): void { + try { localStorage.setItem(STATES_KEY, JSON.stringify(table)); } catch { /* storage blocked */ } +} + +/** + * A fresh `state` for a plugin's OAuth redirect. The callback is delivered + * only to the plugin that asked for this value; before, every plugin with + * http:fetch received every code. Kept in localStorage so the tab that gets + * the callback event finds it even if another tab started the flow. + */ +export function createOAuthState(pluginId: string): string { + const bytes = new Uint8Array(24); + crypto.getRandomValues(bytes); + const state = Array.from(bytes, (b) => b.toString(16).padStart(2, "0")).join(""); + const table = readStates(); + table[state] = { pluginId, created: Date.now() }; + writeStates(table); + return state; +} + +/** The plugin a callback belongs to, consumed so it can't be delivered twice. */ +export function takeOAuthStateOwner(state: string | undefined): string | null { + if (!state) return null; + const table = readStates(); + const entry = table[state]; + if (!entry) return null; + delete table[state]; + writeStates(table); + return entry.pluginId; +} diff --git a/lib/plugin-sandbox/host-api.ts b/lib/plugin-sandbox/host-api.ts index b421f94f6..bd0db432d 100644 --- a/lib/plugin-sandbox/host-api.ts +++ b/lib/plugin-sandbox/host-api.ts @@ -19,7 +19,8 @@ import { type KeywordVisibility, } from '@/stores/settings-store'; import type { MessageListTabsConfig } from '../plugin-types'; -import { apiFetch, getPathPrefix } from '../browser-navigation'; +import { apiFetch, getPathPrefix, withBasePath } from '../browser-navigation'; +import { createOAuthState } from '../plugin-oauth'; import { reportUploadProgress } from '../upload-progress'; import { DEFAULT_KEYWORD_SCAN_LIMIT } from '../jmap/client'; import { suggestKeywordColor } from '../keyword-discovery'; @@ -164,6 +165,10 @@ const PERM_PER_METHOD: Record = { // message-list search: the plugin never sees the results, but it decides // which messages the list shows. 'search.run': 'email:read', + // OAuth with a third party: a state value bound to this plugin, and the + // redirect URI the provider sends the user back to. + 'oauth.createState': 'http:fetch', + 'oauth.getRedirectUri': 'http:fetch', // sieve (delivery-time classification) 'sieve.isSupported': 'filters:read', 'sieve.getActiveScript': 'filters:read', @@ -1810,6 +1815,12 @@ export async function dispatchApiCall( case 'search.run': return doSearchRun(plugin, args[0], args[1]); + case 'oauth.createState': return createOAuthState(plugin.id); + case 'oauth.getRedirectUri': { + const locale = (globalThis as unknown as { __APP_LOCALE__?: string }).__APP_LOCALE__ || 'en'; + return `${window.location.origin}${withBasePath(`/${encodeURIComponent(locale)}/plugins/oauth/callback`)}`; + } + case 'sieve.isSupported': { const { client } = useAuthStore.getState(); return !!client?.supportsSieve(); diff --git a/lib/plugin-sandbox/runtime.tsx b/lib/plugin-sandbox/runtime.tsx index 21607f4c1..557c896fa 100644 --- a/lib/plugin-sandbox/runtime.tsx +++ b/lib/plugin-sandbox/runtime.tsx @@ -488,6 +488,13 @@ function buildPluginApi(manifest: PluginManifest) { run: (query: unknown, options?: { label?: string }) => callApi('search.run', [query, options]) as Promise, }, + // OAuth with a third party (permission: http:fetch). Put createState()'s + // value in the authorize URL; the code comes back through the + // onOAuthCallback hook, to this plugin only. + oauth: { + createState: () => callApi('oauth.createState', []) as Promise, + getRedirectUri: () => callApi('oauth.getRedirectUri', []) as Promise, + }, // Sieve integration for delivery-time classification (permissions: // filters:read / filters:write). Plugins never write scripts directly - // they register an onSieveScriptGenerate transform hook and call