fix: deliver a plugin OAuth callback only to the plugin that started the flow

This commit is contained in:
Linus Rath
2026-10-14 03:04:17 +02:00
parent bf1d8fd2c3
commit 8ef7ec022f
6 changed files with 133 additions and 14 deletions
@@ -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<void>;
let disposable: { dispose: () => void };
@@ -35,6 +38,7 @@ describe("PluginOAuthCallbackListener", () => {
mockUsePathname.mockReturnValue("/en/mail");
handler = vi.fn<(payload: unknown) => Promise<void>>().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(<PluginOAuthCallbackListener />);
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(<PluginOAuthCallbackListener />);
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(<PluginOAuthCallbackListener />);
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(<PluginOAuthCallbackListener />);
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(<PluginOAuthCallbackListener />);
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(<PluginOAuthCallbackListener />);
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<void>>().mockResolvedValue(undefined);
const otherDisposable = authHooks.onOAuthCallback.register("other-plugin", other);
const { unmount } = render(<PluginOAuthCallbackListener />);
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(<PluginOAuthCallbackListener />);
emitStorageEvent(JSON.stringify({ code: "abc", state: "made-up", receivedAt: 1 }));
expect(handler).not.toHaveBeenCalled();
warn.mockRestore();
unmount();
});
});
@@ -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 */ }
};
+12
View File
@@ -193,6 +193,18 @@ export class HookBus<T extends (...args: any[]) => any> {
}
}
/** Observer emit to one plugin's handlers only. */
async emitTo(pluginId: string, ...args: Parameters<T>): Promise<void> {
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<T>): Promise<boolean> {
return (await this.interceptOutcome(...args)) === 'allowed';
+53
View File
@@ -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<string, { pluginId: string; created: number }>;
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;
}
+12 -1
View File
@@ -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<string, Permission | null> = {
// 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();
+7
View File
@@ -488,6 +488,13 @@ function buildPluginApi(manifest: PluginManifest) {
run: (query: unknown, options?: { label?: string }) =>
callApi('search.run', [query, options]) as Promise<void>,
},
// 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<string>,
getRedirectUri: () => callApi('oauth.getRedirectUri', []) as Promise<string>,
},
// Sieve integration for delivery-time classification (permissions:
// filters:read / filters:write). Plugins never write scripts directly -
// they register an onSieveScriptGenerate transform hook and call