fix(auth): reject load() when persist read fails
This commit is contained in:
parent
a24091484c
commit
512f45d357
4 changed files with 91 additions and 10 deletions
|
|
@ -2191,7 +2191,15 @@ function App() {
|
|||
}
|
||||
|
||||
void (async () => {
|
||||
await webOAuthStorage.load();
|
||||
try {
|
||||
await webOAuthStorage.load();
|
||||
} catch (err) {
|
||||
connectStartRef.current = undefined;
|
||||
queueMicrotask(() => {
|
||||
showReAuthBanner(server.id, err instanceof Error ? err : String(err));
|
||||
});
|
||||
return;
|
||||
}
|
||||
const client = setupClientForServer(server, sessionId);
|
||||
setActiveServerId(server.id);
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,9 @@
|
|||
import { describe, it, expect, beforeEach, vi } from "vitest";
|
||||
import { RemoteOAuthStorage } from "@inspector/core/auth/remote/storage-remote.js";
|
||||
import {
|
||||
type OAuthStore,
|
||||
waitForOAuthStorePersistLoad,
|
||||
} from "@inspector/core/auth/store.js";
|
||||
|
||||
const NOOP_FETCH = vi.fn(
|
||||
async () =>
|
||||
|
|
@ -121,6 +125,12 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => {
|
|||
expect(s).toBeInstanceOf(RemoteOAuthStorage);
|
||||
});
|
||||
|
||||
it("waitForOAuthStorePersistLoad resolves for an unregistered store handle", async () => {
|
||||
await expect(
|
||||
waitForOAuthStorePersistLoad({} as OAuthStore),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("load() waits for remote GET before sync reads see persisted state", async () => {
|
||||
const persisted = {
|
||||
state: {
|
||||
|
|
@ -155,4 +165,20 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => {
|
|||
"persisted-verifier",
|
||||
);
|
||||
});
|
||||
|
||||
it("load() rejects when remote GET fails instead of hanging", async () => {
|
||||
const failingFetch = vi.fn(
|
||||
async () => new Response("server error", { status: 500 }),
|
||||
) as unknown as typeof fetch;
|
||||
|
||||
const failingStorage = new RemoteOAuthStorage({
|
||||
baseUrl: "http://remote.example",
|
||||
storeId: `fail-${Math.random().toString(36).slice(2)}`,
|
||||
fetchFn: failingFetch,
|
||||
});
|
||||
|
||||
await expect(failingStorage.load()).rejects.toThrow(
|
||||
"Failed to read store: 500",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -8,7 +8,11 @@ import {
|
|||
OAuthTokensSchema,
|
||||
} from "@modelcontextprotocol/sdk/shared/auth.js";
|
||||
import type { OAuthStorage } from "./storage.js";
|
||||
import { type createOAuthStore, type ServerOAuthState } from "./store.js";
|
||||
import {
|
||||
type OAuthStore,
|
||||
type ServerOAuthState,
|
||||
waitForOAuthStorePersistLoad,
|
||||
} from "./store.js";
|
||||
import type {
|
||||
IdpSessionState,
|
||||
OAuthClientRegistrationKind,
|
||||
|
|
@ -16,10 +20,9 @@ import type {
|
|||
SaveTokensOptions,
|
||||
} from "./storage.js";
|
||||
|
||||
type OAuthStoreWithPersist = ReturnType<typeof createOAuthStore> & {
|
||||
type OAuthStoreWithPersist = OAuthStore & {
|
||||
persist: {
|
||||
hasHydrated: () => boolean;
|
||||
onFinishHydration: (fn: () => void) => void;
|
||||
};
|
||||
};
|
||||
|
||||
|
|
@ -32,19 +35,18 @@ export class OAuthStorageBase implements OAuthStorage {
|
|||
private readonly store: OAuthStoreWithPersist;
|
||||
private loadedPromise: Promise<void> | undefined;
|
||||
|
||||
constructor(store: ReturnType<typeof createOAuthStore>) {
|
||||
constructor(store: OAuthStore) {
|
||||
this.store = store as OAuthStoreWithPersist;
|
||||
}
|
||||
|
||||
load(): Promise<void> {
|
||||
if (!this.loadedPromise) {
|
||||
this.loadedPromise = new Promise((resolve) => {
|
||||
this.loadedPromise = (async () => {
|
||||
if (this.store.persist.hasHydrated()) {
|
||||
resolve();
|
||||
return;
|
||||
}
|
||||
this.store.persist.onFinishHydration(() => resolve());
|
||||
});
|
||||
await waitForOAuthStorePersistLoad(this.store);
|
||||
})();
|
||||
}
|
||||
return this.loadedPromise;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -43,6 +43,23 @@ export interface OAuthStoreState {
|
|||
clearEnterpriseManagedResourceServers: () => void;
|
||||
}
|
||||
|
||||
export type OAuthStore = ReturnType<typeof createOAuthStore>;
|
||||
|
||||
const persistLoadByStore = new WeakMap<OAuthStore, Promise<void>>();
|
||||
|
||||
/**
|
||||
* Wait until the store's initial persist read has finished (success or failure).
|
||||
* Used by {@link OAuthStorageBase.load}; resolves on success, rejects when the
|
||||
* storage adapter's getItem fails (Zustand does not fire onFinishHydration on error).
|
||||
*/
|
||||
export function waitForOAuthStorePersistLoad(store: OAuthStore): Promise<void> {
|
||||
const promise = persistLoadByStore.get(store);
|
||||
if (!promise) {
|
||||
return Promise.resolve();
|
||||
}
|
||||
return promise;
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a Zustand store for OAuth state with the given storage adapter.
|
||||
* The storage adapter handles persistence (file, remote HTTP, sessionStorage, etc.).
|
||||
|
|
@ -53,7 +70,29 @@ export interface OAuthStoreState {
|
|||
export function createOAuthStore(
|
||||
storage: ReturnType<typeof createJSONStorage>,
|
||||
) {
|
||||
return createStore<OAuthStoreState>()(
|
||||
let resolvePersistLoad!: () => void;
|
||||
let rejectPersistLoad!: (error: unknown) => void;
|
||||
let persistLoadSettled = false;
|
||||
|
||||
const persistLoadPromise = new Promise<void>((resolve, reject) => {
|
||||
resolvePersistLoad = resolve;
|
||||
rejectPersistLoad = reject;
|
||||
});
|
||||
|
||||
const settlePersistLoad = (error?: unknown) => {
|
||||
if (persistLoadSettled) {
|
||||
/* v8 ignore next -- idempotent if persist signals completion more than once */
|
||||
return;
|
||||
}
|
||||
persistLoadSettled = true;
|
||||
if (error) {
|
||||
rejectPersistLoad(error);
|
||||
return;
|
||||
}
|
||||
resolvePersistLoad();
|
||||
};
|
||||
|
||||
const store = createStore<OAuthStoreState>()(
|
||||
persist(
|
||||
(set, get) => ({
|
||||
servers: {},
|
||||
|
|
@ -118,7 +157,13 @@ export function createOAuthStore(
|
|||
{
|
||||
name: "mcp-inspector-oauth",
|
||||
storage,
|
||||
onRehydrateStorage: () => (_state, error) => {
|
||||
settlePersistLoad(error);
|
||||
},
|
||||
},
|
||||
),
|
||||
);
|
||||
|
||||
persistLoadByStore.set(store, persistLoadPromise);
|
||||
return store;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue