diff --git a/packages/junior-evals/evals/core/oauth-workflows.eval.ts b/packages/junior-evals/evals/core/oauth-workflows.eval.ts index 58c1a75d1..eba2cea03 100644 --- a/packages/junior-evals/evals/core/oauth-workflows.eval.ts +++ b/packages/junior-evals/evals/core/oauth-workflows.eval.ts @@ -1,6 +1,7 @@ import { assistantMessages, describeEval, toolCalls } from "vitest-evals"; import type { HarnessRun } from "vitest-evals/harness"; import { expect } from "vitest"; +import { readEvalOAuthRefreshTokens } from "@junior-tests/msw/handlers/eval-oauth"; import { authorizationCompletions, rubric, @@ -193,6 +194,44 @@ describeEval("OAuth Workflows", slackEvals, (it) => { expectFinalThreadReply(result, oauthResumeThread, /eval-oauth-user/i); }); + const oauthRefreshThread = { + id: "thread-oauth-refresh", + channel_id: "COAUTHREFRESH", + thread_ts: "17000000.1004", + }; + + it("refreshes an expired generic OAuth credential during a normal turn", async ({ + run, + }) => { + const result = await run({ + overrides: { + expired_oauth_tokens: ["eval-oauth"], + plugin_dirs: ["fixtures/plugins"], + }, + initialEvents: [ + threadMessage( + "/eval-oauth Tell me which eval identity is currently active.", + { thread: oauthRefreshThread, is_mention: true }, + ), + ], + criteria: rubric({ + pass: [ + "The response identifies the active account as eval-oauth-user.", + "The request completes without asking the user to authorize or reconnect.", + ], + fail: [ + "Do not post a generic failure message.", + "Do not ask the user to authorize, connect, or reconnect the account.", + ], + }), + }); + + expectEvalOauthIdentityCheck(result); + expect(authorizationCompletions(result)).toEqual([]); + expect(readEvalOAuthRefreshTokens()).toEqual(["eval-oauth-refresh-token"]); + expectFinalThreadReply(result, oauthRefreshThread, /eval-oauth-user/i); + }); + const oauthReconnectThread = { id: "thread-oauth-reconnect", channel_id: "COAUTHRECONNECT", diff --git a/packages/junior-evals/src/behavior-harness.ts b/packages/junior-evals/src/behavior-harness.ts index e9fa209ce..a4b9a38f7 100644 --- a/packages/junior-evals/src/behavior-harness.ts +++ b/packages/junior-evals/src/behavior-harness.ts @@ -287,6 +287,7 @@ export interface EvalOverrides { auto_complete_mcp_oauth?: string[]; auto_complete_oauth?: string[]; credential_providers?: Array<"github" | "sentry">; + expired_oauth_tokens?: string[]; fail_reply_call?: number; mock_image_generation?: boolean; plugin_dirs?: string[]; @@ -670,7 +671,8 @@ function isSandboxReachableBaseUrl(value: string): boolean { function scenarioNeedsEvalEgress(scenario: EvalScenario): boolean { return Boolean( scenario.overrides?.credential_providers?.length || - scenario.overrides?.auto_complete_oauth?.length, + scenario.overrides?.auto_complete_oauth?.length || + scenario.overrides?.expired_oauth_tokens?.length, ); } @@ -1327,6 +1329,28 @@ async function seedCredentialProviderTokens(input: { } } +async function seedExpiredOAuthTokens(input: { + providers: Set; + userIds: Iterable; +}): Promise { + const userTokenStore = createUserTokenStore(); + for (const provider of input.providers) { + if (provider !== EVAL_OAUTH_PROVIDER) { + throw new Error( + `No expired OAuth eval fixture for provider "${provider}"`, + ); + } + for (const userId of input.userIds) { + await userTokenStore.set(userId, provider, { + accessToken: "expired-eval-oauth-access-token", + refreshToken: "eval-oauth-refresh-token", + expiresAt: Date.now() - 1, + scope: "read", + }); + } + } +} + function getDefaultAuthCode( type: "mcp-oauth" | "oauth", provider: string, @@ -1570,6 +1594,7 @@ interface HarnessEnvironment { autoCompleteMcpOauthProviders: Set; autoCompleteOauthProviders: Set; credentialProviders: Set<"github" | "sentry">; + expiredOauthProviders: Set; configuredPluginDirs: string[]; configuredSkillDirs: string[]; envSnapshot: EnvSnapshot; @@ -1603,6 +1628,11 @@ async function setupHarnessEnvironment( const credentialProviders = new Set( scenario.overrides?.credential_providers ?? [], ); + const expiredOauthProviders = new Set( + scenario.overrides?.expired_oauth_tokens?.map((provider) => + provider.trim(), + ) ?? [], + ); const authActorUsers = new Set( scenarioEvents(scenario).flatMap((event) => "message" in event @@ -1643,16 +1673,22 @@ async function setupHarnessEnvironment( await cleanupMcpAuthState(authActorUsers, autoCompleteMcpOauthProviders); await cleanupOAuthTokens(authActorUsers, autoCompleteOauthProviders); await cleanupOAuthTokens(authActorUsers, credentialProviders); + await cleanupOAuthTokens(authActorUsers, expiredOauthProviders); await seedCredentialProviderTokens({ providers: credentialProviders, userIds: authActorUsers, }); + await seedExpiredOAuthTokens({ + providers: expiredOauthProviders, + userIds: authActorUsers, + }); return { authActorUsers, autoCompleteMcpOauthProviders, autoCompleteOauthProviders, credentialProviders, + expiredOauthProviders, configuredPluginDirs, configuredSkillDirs, envSnapshot, @@ -1683,6 +1719,7 @@ async function teardownHarnessEnvironment( ); await cleanupOAuthTokens(env.authActorUsers, env.autoCompleteOauthProviders); await cleanupOAuthTokens(env.authActorUsers, env.credentialProviders); + await cleanupOAuthTokens(env.authActorUsers, env.expiredOauthProviders); await env.egressServer?.close(); env.envSnapshot.restore(); await env.pluginApp?.cleanup(); diff --git a/packages/junior/src/chat/plugins/auth/oauth-bearer-broker.ts b/packages/junior/src/chat/plugins/auth/oauth-bearer-broker.ts index 8f28a3da8..8459ee092 100644 --- a/packages/junior/src/chat/plugins/auth/oauth-bearer-broker.ts +++ b/packages/junior/src/chat/plugins/auth/oauth-bearer-broker.ts @@ -99,6 +99,7 @@ async function refreshAccessToken( const data = (await response.json()) as Record; return parseOAuthTokenResponse(data, requestedScope, { treatEmptyScopeAsUnreported: oauth.treatEmptyScopeAsUnreported, + refreshToken, }); } diff --git a/packages/junior/src/chat/plugins/auth/oauth-request.ts b/packages/junior/src/chat/plugins/auth/oauth-request.ts index c0e8078ca..88a013b3f 100644 --- a/packages/junior/src/chat/plugins/auth/oauth-request.ts +++ b/packages/junior/src/chat/plugins/auth/oauth-request.ts @@ -91,14 +91,24 @@ export function buildOAuthTokenRequest(input: OAuthTokenRequestInput): { }; } +/** Parse provider tokens, retaining the request token when a refresh response omits rotation. */ export function parseOAuthTokenResponse( data: unknown, requestedScope?: string, - options?: { treatEmptyScopeAsUnreported?: boolean }, + options?: { + treatEmptyScopeAsUnreported?: boolean; + refreshToken?: string; + }, ): OAuthTokenResponse { const response = requireTokenResponseObject(data); const accessToken = requireNonEmptyTokenField(response, "access_token"); - const refreshToken = requireNonEmptyTokenField(response, "refresh_token"); + const resolvedRefreshToken = + response.refresh_token === undefined + ? options?.refreshToken + : requireNonEmptyTokenField(response, "refresh_token"); + if (!resolvedRefreshToken) { + throw new Error("OAuth token response missing refresh_token"); + } const expiresIn = response.expires_in; const refreshTokenExpiresIn = response.refresh_token_expires_in; const responseScope = response.scope; @@ -126,7 +136,11 @@ export function parseOAuthTokenResponse( expiresAt?: number; refreshTokenExpiresAt?: number; scope?: string; - } = { accessToken, refreshToken, ...(scope ? { scope } : {}) }; + } = { + accessToken, + refreshToken: resolvedRefreshToken, + ...(scope ? { scope } : {}), + }; if (expiresIn !== undefined) { if ( diff --git a/packages/junior/tests/msw/handlers/eval-oauth.ts b/packages/junior/tests/msw/handlers/eval-oauth.ts index 466235c45..e770f6a3b 100644 --- a/packages/junior/tests/msw/handlers/eval-oauth.ts +++ b/packages/junior/tests/msw/handlers/eval-oauth.ts @@ -5,13 +5,37 @@ export const EVAL_OAUTH_CODE = "eval-oauth-code"; export const EVAL_OAUTH_ORIGIN = "https://example.com"; const EVAL_OAUTH_TOKEN_ENDPOINT = `${EVAL_OAUTH_ORIGIN}/junior-eval-oauth/oauth/token`; const EVAL_OAUTH_ACCESS_TOKEN = "eval-oauth-access-token"; +const EVAL_OAUTH_REFRESH_TOKEN = "eval-oauth-refresh-token"; +const refreshTokens: string[] = []; -export function resetEvalOAuthMockState(): void {} +/** Clear captured eval OAuth refresh requests between scenarios. */ +export function resetEvalOAuthMockState(): void { + refreshTokens.length = 0; +} + +/** Return refresh tokens submitted to the eval OAuth token endpoint. */ +export function readEvalOAuthRefreshTokens(): string[] { + return [...refreshTokens]; +} export const evalOAuthHandlers = [ http.post(EVAL_OAUTH_TOKEN_ENDPOINT, async ({ request }) => { const bodyText = await request.text(); const params = new URLSearchParams(bodyText); + if (params.get("grant_type") === "refresh_token") { + const refreshToken = params.get("refresh_token") ?? ""; + refreshTokens.push(refreshToken); + if (refreshToken !== EVAL_OAUTH_REFRESH_TOKEN) { + return HttpResponse.json({ error: "invalid_grant" }, { status: 400 }); + } + return HttpResponse.json({ + access_token: EVAL_OAUTH_ACCESS_TOKEN, + token_type: "Bearer", + expires_in: 3600, + scope: "read", + }); + } + const code = params.get("code"); if (code !== EVAL_OAUTH_CODE) { return HttpResponse.json( @@ -27,7 +51,7 @@ export const evalOAuthHandlers = [ access_token: EVAL_OAUTH_ACCESS_TOKEN, token_type: "Bearer", expires_in: 3600, - refresh_token: "eval-oauth-refresh-token", + refresh_token: EVAL_OAUTH_REFRESH_TOKEN, scope: "read", }); }), diff --git a/packages/junior/tests/unit/plugins/oauth-token-response.test.ts b/packages/junior/tests/unit/plugins/oauth-token-response.test.ts index 6eb545381..77caa8e8a 100644 --- a/packages/junior/tests/unit/plugins/oauth-token-response.test.ts +++ b/packages/junior/tests/unit/plugins/oauth-token-response.test.ts @@ -54,6 +54,39 @@ describe("parseOAuthTokenResponse", () => { expect(result.scope).toBe("gist repo"); }); + it("keeps the current refresh token when a refresh response omits refresh_token", () => { + const result = parseOAuthTokenResponse({ access_token: "access" }, "repo", { + refreshToken: "current-refresh", + }); + expect(result.refreshToken).toBe("current-refresh"); + expect(result.accessToken).toBe("access"); + }); + + it("prefers a rotated refresh token over the fallback", () => { + const result = parseOAuthTokenResponse( + { access_token: "access", refresh_token: "rotated" }, + "repo", + { refreshToken: "current-refresh" }, + ); + expect(result.refreshToken).toBe("rotated"); + }); + + it("rejects a missing refresh_token when no fallback is provided", () => { + expect(() => + parseOAuthTokenResponse({ access_token: "access" }, "repo"), + ).toThrow("OAuth token response missing refresh_token"); + }); + + it("rejects an invalid refresh_token even when a fallback is provided", () => { + expect(() => + parseOAuthTokenResponse( + { access_token: "access", refresh_token: "" }, + "repo", + { refreshToken: "current-refresh" }, + ), + ).toThrow("OAuth token response missing refresh_token"); + }); + it("records refresh token expiry separately from access token expiry", () => { vi.useFakeTimers(); vi.setSystemTime(new Date("2026-06-24T00:00:00Z")); diff --git a/packages/junior/tests/unit/plugins/sentry-broker.test.ts b/packages/junior/tests/unit/plugins/sentry-broker.test.ts index 9e35b381f..fa8399338 100644 --- a/packages/junior/tests/unit/plugins/sentry-broker.test.ts +++ b/packages/junior/tests/unit/plugins/sentry-broker.test.ts @@ -185,7 +185,7 @@ describe("sentry credential broker (oauth-bearer plugin)", () => { ).rejects.toThrow(CredentialUnavailableError); }); - it("refreshes tokens that are near expiry", async () => { + it("retains the refresh token when refreshing near expiry without rotation", async () => { process.env.SENTRY_CLIENT_ID = "client-id"; process.env.SENTRY_CLIENT_SECRET = "client-secret"; const refreshTokenExpiresAt = Date.now() + 30 * 24 * 60 * 60 * 1000; @@ -204,7 +204,6 @@ describe("sentry credential broker (oauth-bearer plugin)", () => { ok: true, json: async () => ({ access_token: "new-access-token", - refresh_token: "new-refresh-token", expires_in: 3600, }), })) as unknown as typeof fetch; @@ -228,7 +227,7 @@ describe("sentry credential broker (oauth-bearer plugin)", () => { const stored = await tokenStore.get("U123", "sentry"); expect(stored?.accessToken).toBe("new-access-token"); - expect(stored?.refreshToken).toBe("new-refresh-token"); + expect(stored?.refreshToken).toBe("old-refresh-token"); expect(stored?.refreshTokenExpiresAt).toBe(refreshTokenExpiresAt); expect(globalThis.fetch).toHaveBeenCalledWith( "https://sentry.io/oauth/token/",