test: build a real OAuthStorage in the EMA leak test (#2018)
Copilot round 1: the new EMA regression test constructed its inner provider's storage with `as unknown as OAuthStorage`, an unjustified double cast — the rule in AGENTS.md is that such a cast must carry a justification or be replaced. Replaced with the genuine article: `OAuthStorageBase` over an `OAuthMemoryStore` and a no-op persist backend, so the stub type-checks against the real interface the provider is handed and a future `OAuthStorage` contract change cannot pass silently. Reusing `emaConfig.storage`, as suggested, would not have addressed the objection — that value is itself a cast, and typed as `EmaFlowConfig["storage"]` rather than `OAuthStorage`. The test's assertion is unchanged and was re-verified to fail against pre-fix code (`expected 'corp-idp' to be null`). Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
866c32f629
commit
362e2f419d
1 changed files with 21 additions and 3 deletions
|
|
@ -6,6 +6,9 @@ import {
|
|||
MutableRedirectUrlProvider,
|
||||
} from "@inspector/core/auth/providers.js";
|
||||
import type { OAuthStorage } from "@inspector/core/auth/storage.js";
|
||||
import { OAuthStorageBase } from "@inspector/core/auth/oauth-storage.js";
|
||||
import { OAuthMemoryStore } from "@inspector/core/auth/store.js";
|
||||
import type { OAuthPersistBackend } from "@inspector/core/auth/oauth-persist.js";
|
||||
import type { EmaFlowConfig } from "@inspector/core/auth/ema/emaFlow.js";
|
||||
import { EmaTransportOAuthProvider } from "@inspector/core/auth/ema/transportProvider.js";
|
||||
import {
|
||||
|
|
@ -23,6 +26,20 @@ const startIdpMock = vi.mocked(startEmaIdpAuthorization);
|
|||
|
||||
const SERVER_URL = "http://127.0.0.1:9999/mcp";
|
||||
|
||||
/**
|
||||
* A real `OAuthStorage` — in-memory state over a no-op persist backend — for the
|
||||
* one test below that drives an actual `BaseOAuthClientProvider`. Building the
|
||||
* genuine article costs two lines and avoids an `as unknown as` stub that would
|
||||
* stop type-checking against the interface the provider is handed.
|
||||
*/
|
||||
function makeRealStorage(): OAuthStorage {
|
||||
const backend: OAuthPersistBackend = {
|
||||
read: async () => null,
|
||||
write: async () => {},
|
||||
};
|
||||
return new OAuthStorageBase(new OAuthMemoryStore(), backend);
|
||||
}
|
||||
|
||||
function jwtWithExp(expSec: number): string {
|
||||
const payload = btoa(JSON.stringify({ exp: expSec }))
|
||||
.replace(/\+/g, "-")
|
||||
|
|
@ -200,9 +217,10 @@ describe("EmaTransportOAuthProvider", () => {
|
|||
it("does not leak custom authorization params onto the IdP authorize URL", async () => {
|
||||
const navCallback = vi.fn();
|
||||
const realInner = new BaseOAuthClientProvider(SERVER_URL, {
|
||||
storage: {
|
||||
getScope: vi.fn().mockResolvedValue(undefined),
|
||||
} as unknown as OAuthStorage,
|
||||
// A real OAuthStorage rather than a cast stub: the redirect path never
|
||||
// reads it, and a genuine implementation keeps this test honest if the
|
||||
// storage contract changes.
|
||||
storage: makeRealStorage(),
|
||||
redirectUrlProvider: new MutableRedirectUrlProvider(),
|
||||
navigation: new CallbackNavigation(navCallback),
|
||||
authorizationParams: { kc_idp_hint: "corp-idp", audience: "api://mcp" },
|
||||
|
|
|
|||
Loading…
Reference in a new issue