fix(oauth): keep a token expiry when refresh omits expires_in (oomol-lab#266)

`expires_in` is optional on a refresh response and plenty of providers
only send it on the initial grant. When it is missing the stored
credential ends up with no `expiresAt`, and a credential with no expiry
is never considered expired, so it is never refreshed again. There is no
retry on a 401 either, so once the access token actually lapses every
call through that connection fails until someone reconnects by hand. One
refresh is enough to get there.

The refreshed credential now falls back to the lifetime the provider
last reported, which is already kept in the credential metadata. If no
lifetime was ever reported the expiry stays unset, same as today.

Worth noting why the fallback is not the previous `expiresAt`: a refresh
only runs once that timestamp is in the past, so reusing it would mark
the new token expired immediately and trigger a refresh on every
request.

---------

Co-authored-by: l1shen <648952316@qq.com>

Author: DevMello

src/oauth/oauth-credential-refresh-service.test.ts

@@ -0,0 +1,121 @@
+import type { ResolvedCredential } from "../core/types.ts";
+import type { OAuthClientConfigService } from "./oauth-client-config-service.ts";
+
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { OAuthCredentialRefreshService } from "./oauth-credential-refresh-service.ts";
+
+type OAuthCredential = Extract<ResolvedCredential, { authType: "oauth2" }>;
+
+const clientConfigs = {
+  getOAuthDefinition: () => ({ type: "oauth2", tokenUrl: "https://provider.example.com/oauth/token" }),
+  getConfig: async () => ({ clientId: "client-id", clientSecret: "client-secret", extra: {} }),
+  resolveEndpointUrl: (_service: string, endpointUrl: string) => endpointUrl,
+} as unknown as OAuthClientConfigService;
+
+/** A stored credential whose access token has already lapsed, which is when a refresh runs. */
+function expiredCredential(metadata: Record<string, unknown>): OAuthCredential {
+  return {
+    authType: "oauth2",
+    accessToken: "old-access-token",
+    tokenType: "Bearer",
+    expiresAt: new Date(Date.now() - 60_000).toISOString(),
+    refreshToken: "refresh-token",
+    profile: { accountId: "oauth2", displayName: "OAuth Credential", grantedScopes: [] },
+    metadata,
+  };
+}
+
+function stubRefreshResponse(payload: Record<string, unknown>): void {
+  vi.stubGlobal(
+    "fetch",
+    vi.fn(async () => Response.json({ access_token: "new-access-token", ...payload })),
+  );
+}
+
+describe("OAuthCredentialRefreshService", () => {
+  afterEach(() => {
+    vi.unstubAllGlobals();
+    vi.restoreAllMocks();
+  });
+
+  it("keeps an expiry when the refresh response omits expires_in", async () => {
+    const now = Date.now();
+    vi.spyOn(Date, "now").mockReturnValue(now);
+    stubRefreshResponse({});
+
+    const refreshed = await new OAuthCredentialRefreshService(clientConfigs).refresh(
+      "example",
+      expiredCredential({ expires_in: 3600 }),
+    );
+
+    expect(refreshed.expiresAt).toBe(new Date(now + 3600_000).toISOString());
+  });
+
+  it("never carries the lapsed expiry forward, which would refresh on every call", async () => {
+    const credential = expiredCredential({ expires_in: 3600 });
+    stubRefreshResponse({});
+
+    const refreshed = await new OAuthCredentialRefreshService(clientConfigs).refresh("example", credential);
+
+    expect(refreshed.expiresAt).not.toBe(credential.expiresAt);
+    expect(Date.parse(refreshed.expiresAt!)).toBeGreaterThan(Date.now());
+  });
+
+  it("prefers the expiry the refresh response reports", async () => {
+    const now = Date.now();
+    vi.spyOn(Date, "now").mockReturnValue(now);
+    stubRefreshResponse({ expires_in: 120 });
+
+    const refreshed = await new OAuthCredentialRefreshService(clientConfigs).refresh(
+      "example",
+      expiredCredential({ expires_in: 3600 }),
+    );
+
+    expect(refreshed.expiresAt).toBe(new Date(now + 120_000).toISOString());
+  });
+
+  it("leaves the expiry unset when no lifetime was ever reported", async () => {
+    stubRefreshResponse({});
+
+    const refreshed = await new OAuthCredentialRefreshService(clientConfigs).refresh("example", expiredCredential({}));
+
+    expect(refreshed.expiresAt).toBeUndefined();
+  });
+
+  it("carries a reported lifetime through a later refresh that omits it", async () => {
+    const service = new OAuthCredentialRefreshService(clientConfigs);
+    stubRefreshResponse({ expires_in: 3600 });
+    const first = await service.refresh("example", expiredCredential({}));
+
+    const now = Date.now();
+    vi.spyOn(Date, "now").mockReturnValue(now);
+    stubRefreshResponse({});
+    const second = await service.refresh("example", { ...first, expiresAt: new Date(now - 60_000).toISOString() });
+
+    expect(second.expiresAt).toBe(new Date(now + 3600_000).toISOString());
+  });
+
+  it("keeps the last usable lifetime when a refresh reports an unusable value", async () => {
+    const now = Date.now();
+    vi.spyOn(Date, "now").mockReturnValue(now);
+    const service = new OAuthCredentialRefreshService(clientConfigs);
+    stubRefreshResponse({ expires_in: 0 });
+    const first = await service.refresh("example", expiredCredential({ expires_in: 3600 }));
+
+    stubRefreshResponse({});
+    const second = await service.refresh("example", { ...first, expiresAt: new Date(now - 60_000).toISOString() });
+
+    expect(second.expiresAt).toBe(new Date(now + 3600_000).toISOString());
+  });
+
+  it("keeps the stored refresh token when the response omits a new one", async () => {
+    stubRefreshResponse({});
+
+    const refreshed = await new OAuthCredentialRefreshService(clientConfigs).refresh(
+      "example",
+      expiredCredential({ expires_in: 3600 }),
+    );
+
+    expect(refreshed.refreshToken).toBe("refresh-token");
+  });
+});

src/oauth/oauth-credential-refresh-service.ts

@@ -2,7 +2,7 @@ import type { ResolvedCredential } from "../core/types.ts";
 import type { OAuthClientConfigService } from "./oauth-client-config-service.ts";
 
 import { ConnectionError } from "../connection-service.ts";
-import { requestRefreshToken } from "./oauth-token.ts";
+import { expiresAtFromLifetime, requestRefreshToken } from "./oauth-token.ts";
 
 type OAuthCredential = Extract<ResolvedCredential, { authType: "oauth2" }>;
 
@@ -41,14 +41,24 @@ export class OAuthCredentialRefreshService implements IOAuthCredentialRefresher
       tokenUrl: this.clientConfigs.resolveEndpointUrl(service, auth.refreshTokenUrl ?? auth.tokenUrl, config),
       createError: (message) => new ConnectionError("oauth_token_refresh_failed", message),
     });
+    const expiresIn =
+      refreshed.expiresAt === undefined ? credential.metadata.expires_in : refreshed.metadata.expires_in;
 
     return {
       ...refreshed,
       refreshToken: refreshed.refreshToken ?? credential.refreshToken,
+      // `expires_in` is optional on a refresh response, and a credential without an
+      // expiry is never treated as expired again, so the token silently stops being
+      // refreshed and every later call fails once it lapses. Reuse the lifetime the
+      // provider last reported instead. Carrying `credential.expiresAt` forward is
+      // not an option: a refresh only runs once that timestamp is already past, so
+      // the stored token would look expired immediately and refresh on every call.
+      expiresAt: refreshed.expiresAt ?? expiresAtFromLifetime(expiresIn),
       profile: credential.profile,
       metadata: {
         ...credential.metadata,
         ...refreshed.metadata,
+        expires_in: expiresIn,
         refreshedAt: new Date().toISOString(),
       },
     };

src/oauth/oauth-token.ts

@@ -115,13 +115,12 @@ async function requestToken(input: TokenRequest): Promise<Extract<ResolvedCreden
 
   const accessToken = requiredString(payload.access_token ?? payload.token, "access_token", input.createError);
   const tokenType = optionalString(payload.token_type) ?? "Bearer";
-  const expiresIn = readExpiresInSeconds(payload.expires_in);
   return {
     authType: "oauth2",
     accessToken,
     tokenType,
     refreshToken: optionalString(payload.refresh_token),
-    expiresAt: expiresIn === undefined ? undefined : new Date(Date.now() + expiresIn * 1000).toISOString(),
+    expiresAt: expiresAtFromLifetime(payload.expires_in),
     profile: {
       accountId: "oauth2",
       displayName: "OAuth Credential",
@@ -166,6 +165,15 @@ function createTokenMetadata(payload: Record<string, unknown>): Record<string, u
   return metadata;
 }
 
+/**
+ * Build the absolute expiry for an OAuth `expires_in` lifetime, or undefined when
+ * the provider did not report a usable one.
+ */
+export function expiresAtFromLifetime(value: unknown): string | undefined {
+  const seconds = readExpiresInSeconds(value);
+  return seconds === undefined ? undefined : new Date(Date.now() + seconds * 1000).toISOString();
+}
+
 /**
  * Parse OAuth `expires_in` lifetimes. Providers commonly return a JSON number,
  * but some return the same value as a string, which used to be dropped so the