fix(auth): clear OAuth callback timer and reorder EMA scope persist
Clear the loopback callback timeout in runner-interactive-oauth finally so already_authorized and error paths do not hang the CLI/TUI. Check EMA step-up satisfaction against minted token scope before persisting the requested union, matching the standard OAuth path. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
0635d09e83
commit
52023ff1c3
4 changed files with 92 additions and 10 deletions
|
|
@ -84,6 +84,34 @@ describe("runRunnerInteractiveOAuth", () => {
|
||||||
expect(client.completeOAuthFlow).not.toHaveBeenCalled();
|
expect(client.completeOAuthFlow).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("clears the callback timeout on already_authorized", async () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
try {
|
||||||
|
const client = mockClient({
|
||||||
|
authenticate: vi.fn(async () => undefined),
|
||||||
|
});
|
||||||
|
const redirectUrlProvider = { redirectUrl: "" };
|
||||||
|
|
||||||
|
const result = await runRunnerInteractiveOAuth({
|
||||||
|
client,
|
||||||
|
redirectUrlProvider,
|
||||||
|
callbackListen: {
|
||||||
|
hostname: "127.0.0.1",
|
||||||
|
port: 6276,
|
||||||
|
pathname: "/oauth/callback",
|
||||||
|
},
|
||||||
|
callbackTimeoutMs: 60_000,
|
||||||
|
createCallbackServer: () => createMockCallbackServer(handlers),
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result).toEqual({ kind: "already_authorized" });
|
||||||
|
expect(vi.getTimerCount()).toBe(0);
|
||||||
|
await vi.advanceTimersByTimeAsync(60_000);
|
||||||
|
} finally {
|
||||||
|
vi.useRealTimers();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
it("completes connect-time OAuth via authenticate and callback", async () => {
|
it("completes connect-time OAuth via authenticate and callback", async () => {
|
||||||
const redirectUrlProvider = { redirectUrl: "" };
|
const redirectUrlProvider = { redirectUrl: "" };
|
||||||
const client = mockClient({
|
const client = mockClient({
|
||||||
|
|
|
||||||
|
|
@ -1215,9 +1215,6 @@ describe("OAuthManager", () => {
|
||||||
});
|
});
|
||||||
|
|
||||||
it("returns satisfied for EMA insufficient_scope after user confirms", async () => {
|
it("returns satisfied for EMA insufficient_scope after user confirms", async () => {
|
||||||
const silentSpy = vi
|
|
||||||
.spyOn(emaFlow, "trySilentEmaAuth")
|
|
||||||
.mockResolvedValue({ status: "success" });
|
|
||||||
const params = createMockParams({
|
const params = createMockParams({
|
||||||
enterpriseManagedAuth: {
|
enterpriseManagedAuth: {
|
||||||
idp: {
|
idp: {
|
||||||
|
|
@ -1237,6 +1234,14 @@ describe("OAuthManager", () => {
|
||||||
token_type: "Bearer",
|
token_type: "Bearer",
|
||||||
scope: "mcp",
|
scope: "mcp",
|
||||||
});
|
});
|
||||||
|
const silentSpy = vi.spyOn(emaFlow, "trySilentEmaAuth").mockImplementation(async () => {
|
||||||
|
storageOf(params).getTokens.mockResolvedValue({
|
||||||
|
access_token: "tok",
|
||||||
|
token_type: "Bearer",
|
||||||
|
scope: "mcp weather:read",
|
||||||
|
});
|
||||||
|
return { status: "success" };
|
||||||
|
});
|
||||||
const manager = new OAuthManager(params);
|
const manager = new OAuthManager(params);
|
||||||
manager.setOAuthConfig({ enterpriseManaged: true });
|
manager.setOAuthConfig({ enterpriseManaged: true });
|
||||||
|
|
||||||
|
|
@ -1257,6 +1262,51 @@ describe("OAuthManager", () => {
|
||||||
silentSpy.mockRestore();
|
silentSpy.mockRestore();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("does not persist union scope or return satisfied when silent EMA mint is down-scoped", async () => {
|
||||||
|
const silentSpy = vi
|
||||||
|
.spyOn(emaFlow, "trySilentEmaAuth")
|
||||||
|
.mockResolvedValue({ status: "success" });
|
||||||
|
const authUrl = new URL("https://idp.example.com/authorize?state=ema");
|
||||||
|
const startSpy = vi
|
||||||
|
.spyOn(emaFlow, "startEmaIdpAuthorization")
|
||||||
|
.mockResolvedValue(authUrl);
|
||||||
|
const params = createMockParams({
|
||||||
|
enterpriseManagedAuth: {
|
||||||
|
idp: {
|
||||||
|
issuer: "https://idp.example.com",
|
||||||
|
clientId: "app-client",
|
||||||
|
clientSecret: "secret",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
storageOf(params).getScope.mockReturnValue("mcp");
|
||||||
|
storageOf(params).getTokens.mockResolvedValue({
|
||||||
|
access_token: "tok",
|
||||||
|
token_type: "Bearer",
|
||||||
|
scope: "mcp",
|
||||||
|
});
|
||||||
|
const manager = new OAuthManager(params);
|
||||||
|
manager.setOAuthConfig({ enterpriseManaged: true });
|
||||||
|
|
||||||
|
const outcome = await manager.handleAuthChallenge(
|
||||||
|
{
|
||||||
|
reason: "insufficient_scope",
|
||||||
|
requiredScopes: ["weather:read"],
|
||||||
|
},
|
||||||
|
{ confirmedStepUp: true },
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(outcome).toEqual(
|
||||||
|
expect.objectContaining({
|
||||||
|
kind: "interactive",
|
||||||
|
authorizationUrl: authUrl,
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
expect(storageOf(params).saveScope).not.toHaveBeenCalled();
|
||||||
|
silentSpy.mockRestore();
|
||||||
|
startSpy.mockRestore();
|
||||||
|
});
|
||||||
|
|
||||||
it("completeOAuthFlow mints EMA tokens with pending step-up union scope", async () => {
|
it("completeOAuthFlow mints EMA tokens with pending step-up union scope", async () => {
|
||||||
const silentSpy = vi
|
const silentSpy = vi
|
||||||
.spyOn(emaFlow, "trySilentEmaAuth")
|
.spyOn(emaFlow, "trySilentEmaAuth")
|
||||||
|
|
|
||||||
|
|
@ -58,6 +58,8 @@ export async function runRunnerInteractiveOAuth(
|
||||||
flowReject = reject;
|
flowReject = reject;
|
||||||
});
|
});
|
||||||
|
|
||||||
|
let timeoutId: ReturnType<typeof setTimeout> | undefined;
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const { redirectUrl } = await server.start({
|
const { redirectUrl } = await server.start({
|
||||||
hostname: options.callbackListen.hostname,
|
hostname: options.callbackListen.hostname,
|
||||||
|
|
@ -85,7 +87,6 @@ export async function runRunnerInteractiveOAuth(
|
||||||
|
|
||||||
const timeoutMs =
|
const timeoutMs =
|
||||||
options.callbackTimeoutMs ?? DEFAULT_RUNNER_INTERACTIVE_OAUTH_TIMEOUT_MS;
|
options.callbackTimeoutMs ?? DEFAULT_RUNNER_INTERACTIVE_OAUTH_TIMEOUT_MS;
|
||||||
let timeoutId: ReturnType<typeof setTimeout> | undefined;
|
|
||||||
const waitForCallback = Promise.race([
|
const waitForCallback = Promise.race([
|
||||||
flowDone.finally(() => {
|
flowDone.finally(() => {
|
||||||
if (timeoutId !== undefined) {
|
if (timeoutId !== undefined) {
|
||||||
|
|
@ -131,6 +132,9 @@ export async function runRunnerInteractiveOAuth(
|
||||||
|
|
||||||
return { kind: "success" };
|
return { kind: "success" };
|
||||||
} finally {
|
} finally {
|
||||||
|
if (timeoutId !== undefined) {
|
||||||
|
clearTimeout(timeoutId);
|
||||||
|
}
|
||||||
await server.stop().catch(() => {});
|
await server.stop().catch(() => {});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -602,13 +602,13 @@ export class OAuthManager {
|
||||||
if (enriched.reason === "insufficient_scope") {
|
if (enriched.reason === "insufficient_scope") {
|
||||||
const silent = await trySilentEmaAuth(config);
|
const silent = await trySilentEmaAuth(config);
|
||||||
if (silent.status === "success") {
|
if (silent.status === "success") {
|
||||||
if (enriched.authorizationScopes?.length) {
|
|
||||||
await this.oauthConfig.storage!.saveScope(
|
|
||||||
this.getServerUrl(),
|
|
||||||
enriched.authorizationScopes.join(" "),
|
|
||||||
);
|
|
||||||
}
|
|
||||||
if (await this.checkAuthChallengeSatisfied(enriched)) {
|
if (await this.checkAuthChallengeSatisfied(enriched)) {
|
||||||
|
if (enriched.authorizationScopes?.length) {
|
||||||
|
await this.oauthConfig.storage!.saveScope(
|
||||||
|
this.getServerUrl(),
|
||||||
|
enriched.authorizationScopes.join(" "),
|
||||||
|
);
|
||||||
|
}
|
||||||
return { kind: "satisfied" };
|
return { kind: "satisfied" };
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue