From 97581e860ae06f60db37439f9d3e90ae77f4aa41 Mon Sep 17 00:00:00 2001 From: d-klotz Date: Thu, 17 Sep 2026 08:36:53 -0300 Subject: [PATCH 1/3] fix(salesforce): persist rotated refresh tokens and route jsforce refreshes through core MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With refresh-token rotation enabled on the connected app, Salesforce returns a new refresh_token on every refresh and invalidates the old one. jsforce's default oauthRefreshFn never updates conn.refreshToken (connection.js _establish keeps the old value), and the module persisted this.conn.refreshToken — the consumed token — over the good one. Presenting a rotated-out token later trips Salesforce's reuse detection, which revokes the current token and every access token in the family. Observed 2026-09-16: four worker Lambdas overwrote the daemon's correct write, then 765 refreshes failed with "expired access/refresh token" while the credential stayed authIsValid=true. Hand jsforce a custom refreshFn that delegates to core's _refreshAuthOnce(). Core (frigg#636, @friggframework/core 2.0.0-next.110) supplies adopt-before-refresh, definitive-vs-transport classification, adopt-after-invalid_grant with backoff, INVALID_AUTH, telemetry and single-flight. The module adds only: - _buildConnection(): the one place that constructs the jsforce Connection, with the refreshFn. resetToSandbox() previously built its connection without any refresh listener, so sandbox connections never persisted a refresh. - _adoptNewerCredential(): mirrors adopted tokens onto conn. - refreshAccessToken(): accepts both core's { refresh_token } and a token response the caller already obtained (the CDC daemon's call shape). Persists res.refresh_token. Leaves instanceUrl alone on purpose: getCredentialDetails keys the credential upsert on it, and moving it would fork the Credential row. - _normalizeTokenError(): jsforce reports the OAuth code in err.name with no HTTP status; core classifies on statusCode first. ERROR_HTTP_5xx keeps that status (transport); invalid_grant, invalid_client, invalid_client_id, invalid_app_access, inactive_user and inactive_org become 400 (definitive). - _refreshRejected memo: after a definitive rejection, later 401s only re-read the store. Without it every 401 on a warm instance repeats the full cycle (4 reads, 1 POST, 3 s backoff, 3 writes) and re-fires CREDENTIAL_INVALIDATED. The dead token is never replayed. The refresh event handler is deleted: a custom refreshFn still emits 'refresh', and the old handler was fire-and-forget and logged the live access token. Bump @friggframework/core from the exact 2.0.0-next.79 pin to ^2.0.0-next.110, matching the other v1-ready modules. Every primitive above is absent in next.79. Test fixture: baseParams now carries scope, as definition.env does at runtime. The two getAuthorizationUri scope assertions were already failing on next. Co-Authored-By: Claude Fable 5.1 --- package-lock.json | 46 ++- packages/v1-ready/salesforce/api.js | 106 ++++-- packages/v1-ready/salesforce/package.json | 2 +- packages/v1-ready/salesforce/test/api.test.js | 308 ++++++++++++++++++ 4 files changed, 422 insertions(+), 40 deletions(-) diff --git a/package-lock.json b/package-lock.json index 0df07ee..c73a4a2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -56652,7 +56652,7 @@ "version": "1.0.2", "license": "MIT", "dependencies": { - "@friggframework/core": "2.0.0-next.79", + "@friggframework/core": "^2.0.0-next.110", "jsforce": "^3.8.1" }, "devDependencies": { @@ -56666,36 +56666,46 @@ } }, "packages/v1-ready/salesforce/node_modules/@friggframework/core": { - "version": "2.0.0-next.79", - "resolved": "https://registry.npmjs.org/@friggframework/core/-/core-2.0.0-next.79.tgz", - "integrity": "sha512-tw2Hh33PHR0XxcGjgUYt2ssdgmmN8cI2Bz8KTiSI2affWJQZuC4Np4mPG97hhDWmKWHEOYl0KciaqYlt22HaOw==", + "version": "2.0.0-next.110", + "resolved": "https://registry.npmjs.org/@friggframework/core/-/core-2.0.0-next.110.tgz", + "integrity": "sha512-T8MGYKxseuwzb1MNMAs8+Tlm0j+4yZGTUDQ1tUTiOcEfx9pmYKtg/7cW6dq9+s0h8zutpbDKxG+c2hFWBwuGfg==", "license": "MIT", "dependencies": { "@aws-sdk/client-apigatewaymanagementapi": "^3.588.0", "@aws-sdk/client-kms": "^3.588.0", "@aws-sdk/client-lambda": "^3.714.0", + "@aws-sdk/client-s3": "^3.588.0", "@aws-sdk/client-sqs": "^3.588.0", + "@aws-sdk/client-ssm": "^3.588.0", + "@aws-sdk/s3-request-presigner": "^3.588.0", "@hapi/boom": "^10.0.1", + "@opentelemetry/api": "^1.9.1", + "@opentelemetry/context-async-hooks": "^2.9.0", + "@opentelemetry/exporter-metrics-otlp-http": "^0.220.0", + "@opentelemetry/exporter-trace-otlp-http": "^0.220.0", + "@opentelemetry/resources": "^2.9.0", + "@opentelemetry/sdk-metrics": "^2.9.0", + "@opentelemetry/sdk-trace-base": "^2.9.0", "bcryptjs": "^2.4.3", - "body-parser": "^1.20.2", + "body-parser": "^1.20.5", "bson": "^4.7.2", "chalk": "^4.1.2", "common-tags": "^1.8.2", "cors": "^2.8.5", "dotenv": "^16.4.7", - "express": "^4.19.2", + "express": "^4.22.2", "express-async-handler": "^1.2.0", - "form-data": "^4.0.0", + "form-data": "^4.0.6", "fs-extra": "^11.2.0", - "lodash": "4.17.21", + "lodash": "4.18.1", "lodash.get": "^4.4.2", "node-fetch": "^2.6.7", "serverless-http": "^2.7.0", - "uuid": "^9.0.1" + "uuid": "^11.1.1" }, "peerDependencies": { - "@prisma/client": "^6.16.3", - "prisma": "^6.16.3" + "@prisma/client": "^6.19.3", + "prisma": "^6.19.3" }, "peerDependenciesMeta": { "@prisma/client": { @@ -56718,17 +56728,23 @@ "url": "https://dotenvx.com" } }, + "packages/v1-ready/salesforce/node_modules/lodash": { + "version": "4.18.1", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.18.1.tgz", + "integrity": "sha512-dMInicTPVE8d1e5otfwmmjlxkZoUpiVLwyeTdUsi/Caj/gfzzblBcCE5sRHV/AsjuCmxWrte2TNGSYuCeCq+0Q==", + "license": "MIT" + }, "packages/v1-ready/salesforce/node_modules/uuid": { - "version": "9.0.1", - "resolved": "https://registry.npmjs.org/uuid/-/uuid-9.0.1.tgz", - "integrity": "sha512-b+1eJOlsR9K8HJpow9Ok3fiWOWSIcIzXodvv0rQjVoOVNpWMpxf1wZNpt4y9h10odCNrqnYp1OBzRktckBe3sA==", + "version": "11.1.1", + "resolved": "https://registry.npmjs.org/uuid/-/uuid-11.1.1.tgz", + "integrity": "sha512-vIYxrBCC/N/K+Js3qSN88go7kIfNPssr/hHCesKCQNAjmgvYS2oqr69kIufEG+O4+PfezOH4EbIeHCfFov8ZgQ==", "funding": [ "https://github.com/sponsors/broofa", "https://github.com/sponsors/ctavan" ], "license": "MIT", "bin": { - "uuid": "dist/bin/uuid" + "uuid": "dist/esm/bin/uuid" } }, "packages/v1-ready/stripe": { diff --git a/packages/v1-ready/salesforce/api.js b/packages/v1-ready/salesforce/api.js index 0ba369f..1b8e9eb 100644 --- a/packages/v1-ready/salesforce/api.js +++ b/packages/v1-ready/salesforce/api.js @@ -6,6 +6,15 @@ class Api extends OAuth2Requester { // URL-unreserved and outside the base64url alphabet. static STATE_VERIFIER_DELIMITER = '~'; + static DEFINITIVE_TOKEN_ERRORS = new Set([ + 'invalid_grant', + 'invalid_client', + 'invalid_client_id', + 'invalid_app_access', + 'inactive_user', + 'inactive_org', + ]); + constructor(params) { super(params); this.jsforce = jsforce; @@ -24,21 +33,21 @@ class Api extends OAuth2Requester { redirectUri: this.redirect_uri, loginUrl: this.loginUrl, }); - this.conn = new jsforce.Connection({ + this.conn = this._buildConnection(); + } + + _buildConnection() { + const conn = new jsforce.Connection({ oauth2: this.oauth2, accessToken: this.access_token, refreshToken: this.refresh_token, instanceUrl: this.instanceUrl, + refreshFn: (_conn, callback) => this._jsforceRefreshFn(callback), }); - this.conn.on('refresh', (accessToken, res) => { - console.log(accessToken); - this.refreshAccessToken(res).then(() => { - console.log('Refreshed'); - }); - }); - this.conn.on('error', (error) => { + conn.on('error', (error) => { console.log(error); }); + return conn; } getAuthorizationUri() { @@ -111,12 +120,7 @@ class Api extends OAuth2Requester { loginUrl: 'https://test.salesforce.com', }); - this.conn = new jsforce.Connection({ - oauth2: this.oauth2, - accessToken: this.access_token, - refreshToken: this.refresh_token, - instanceUrl: this.instanceUrl, - }); + this.conn = this._buildConnection(); this.isSandbox = true; } @@ -198,17 +202,71 @@ class Api extends OAuth2Requester { return response; } - async refreshAccessToken(res) { - const OAuthDetails = { + async _jsforceRefreshFn(callback) { + if (this._refreshRejected) { + if (!(await this._adoptNewerCredential())) { + return callback( + new Error('Salesforce rejected the refresh token') + ); + } + this._refreshRejected = false; + return callback(undefined, this.access_token); + } + let refreshed; + try { + refreshed = await this._refreshAuthOnce(); + } catch (err) { + return callback(err); + } + if (!refreshed) { + this._refreshRejected = true; + return callback(new Error('Salesforce rejected the refresh token')); + } + callback(undefined, this.access_token); + } + + async _adoptNewerCredential() { + const adopted = await super._adoptNewerCredential(); + if (adopted) { + this.conn.accessToken = this.access_token; + this.conn.refreshToken = this.refresh_token; + } + return adopted; + } + + async refreshAccessToken(tokenOrResponse) { + let res = tokenOrResponse; + if (!res.access_token) { + try { + res = await this.oauth2.refreshToken(res.refresh_token); + } catch (err) { + throw this._normalizeTokenError(err); + } + } + await this._applyTokenResponse(res); + return res; + } + + _normalizeTokenError(err) { + if (!err || typeof err !== 'object' || err.statusCode !== undefined) { + return err; + } + const httpStatus = /^ERROR_HTTP_(\d{3})$/.exec(err.name || ''); + if (httpStatus) { + err.statusCode = Number(httpStatus[1]); + } else if (Api.DEFINITIVE_TOKEN_ERRORS.has(err.name)) { + err.statusCode = 400; + } + return err; + } + + async _applyTokenResponse(res) { + this.conn.accessToken = res.access_token; + if (res.refresh_token) this.conn.refreshToken = res.refresh_token; + await this.setTokens({ access_token: res.access_token, - refresh_token: this.conn.refreshToken, - instanceUrl: this.conn.instanceUrl, - }; - // Set the instance URL because I'm not sure this gets set... Access and Refresh get set by setTokens, - // which then invokes `notify` to do the token update in the DB. The idea, though, is that auth and refresh - // automatically re-set the access token for future requests of the instance of the class and tells the - // delegate to update the DB for future requests. - await this.setTokens(OAuthDetails); + refresh_token: res.refresh_token, + }); } } diff --git a/packages/v1-ready/salesforce/package.json b/packages/v1-ready/salesforce/package.json index 1edb694..d36e593 100644 --- a/packages/v1-ready/salesforce/package.json +++ b/packages/v1-ready/salesforce/package.json @@ -21,7 +21,7 @@ "prettier": "^2.7.1" }, "dependencies": { - "@friggframework/core": "2.0.0-next.79", + "@friggframework/core": "^2.0.0-next.110", "jsforce": "^3.8.1" } } \ No newline at end of file diff --git a/packages/v1-ready/salesforce/test/api.test.js b/packages/v1-ready/salesforce/test/api.test.js index ddf75da..c4b07c0 100644 --- a/packages/v1-ready/salesforce/test/api.test.js +++ b/packages/v1-ready/salesforce/test/api.test.js @@ -11,9 +11,12 @@ jest.mock('jsforce', () => { instanceUrl: 'https://test.salesforce.com', }; + const mockRefreshToken = jest.fn(); + return { OAuth2: jest.fn().mockImplementation((params) => ({ getAuthorizationUrl: mockGetAuthorizationUrl, + refreshToken: mockRefreshToken, codeVerifier: params?.useVerifier ? 'test-code-verifier' : undefined, _params: params, })), @@ -27,6 +30,7 @@ const baseParams = { client_id: 'test-client-id', client_secret: 'test-client-secret', redirect_uri: 'http://localhost/redirect/salesforce', + scope: 'full refresh_token', delegate: { notify: jest.fn(), addDelegate: jest.fn(), delegateTypes: [] }, }; @@ -118,6 +122,13 @@ describe('Salesforce Api', () => { expect(lastCall[0].refreshToken).toBe('my-refresh-token'); expect(lastCall[0].instanceUrl).toBe('https://myorg.salesforce.com'); }); + + it('does not subscribe to the jsforce refresh event', () => { + expect(api.conn.on).not.toHaveBeenCalledWith( + 'refresh', + expect.any(Function) + ); + }); }); }); @@ -160,3 +171,300 @@ describe('getAuthorizationUri state handling', () => { expect(api.oauth2.codeVerifier).toBe('test-code-verifier'); }); }); + +describe('Salesforce Api token refresh', () => { + const rotated = { + access_token: 'at-new', + refresh_token: 'rt-new', + instance_url: 'https://new.my.salesforce.com', + }; + + const tokenEndpointError = (name, message) => + Object.assign(new Error(message), { name }); + const invalidGrant = () => + tokenEndpointError('invalid_grant', 'expired access/refresh token'); + const invalidClientId = () => + tokenEndpointError('invalid_client_id', 'client identifier invalid'); + const http503 = () => + tokenEndpointError('ERROR_HTTP_503', 'maintenance'); + + function makeApi({ stored, backoff = [] } = {}) { + const store = { current: stored }; + const receiveNotification = jest.fn(async (_notifier, type) => + type === 'CREDENTIAL_RELOAD' ? store.current : undefined + ); + const api = new Api({ + ...baseParams, + access_token: 'at-old', + refresh_token: 'rt-old', + instanceUrl: 'https://old.my.salesforce.com', + credentialReloadBackoffMs: backoff, + delegate: { ...baseParams.delegate, receiveNotification }, + }); + Object.assign(api.conn, { + accessToken: 'at-old', + refreshToken: 'rt-old', + instanceUrl: 'https://old.my.salesforce.com', + }); + api.oauth2.refreshToken.mockReset(); + return { api, store, receiveNotification }; + } + + const jsforceRefresh = (api) => { + const { refreshFn } = + require('jsforce').Connection.mock.calls.at(-1)[0]; + return new Promise((resolve) => + refreshFn(api.conn, (err, accessToken) => + resolve({ err, accessToken }) + ) + ); + }; + + const notified = (spy, type) => + spy.mock.calls.filter(([, t]) => t === type); + + let logSpy; + let warnSpy; + let errorSpy; + const consoleOutput = () => + JSON.stringify( + [logSpy, warnSpy, errorSpy].map((spy) => spy.mock.calls) + ); + + beforeEach(() => { + jest.clearAllMocks(); + logSpy = jest.spyOn(console, 'log').mockImplementation(() => {}); + warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + logSpy.mockRestore(); + warnSpy.mockRestore(); + errorSpy.mockRestore(); + }); + + it('persists the rotated refresh token on a jsforce refresh', async () => { + const { api, receiveNotification } = makeApi(); + api.oauth2.refreshToken.mockResolvedValue(rotated); + + const { err, accessToken } = await jsforceRefresh(api); + + expect(err).toBeUndefined(); + expect(accessToken).toBe('at-new'); + expect(api.refresh_token).toBe('rt-new'); + expect(api.access_token).toBe('at-new'); + expect(api.conn.refreshToken).toBe('rt-new'); + expect(api.conn.accessToken).toBe('at-new'); + expect(notified(receiveNotification, 'TOKEN_UPDATE')).toHaveLength(1); + }); + + it('adopts a newer stored credential instead of calling the token endpoint', async () => { + const { api, receiveNotification } = makeApi({ + stored: { access_token: 'at-db', refresh_token: 'rt-db' }, + }); + + const { err, accessToken } = await jsforceRefresh(api); + + expect(err).toBeUndefined(); + expect(accessToken).toBe('at-db'); + expect(api.oauth2.refreshToken).not.toHaveBeenCalled(); + expect(api.conn.refreshToken).toBe('rt-db'); + expect(api.conn.accessToken).toBe('at-db'); + expect(notified(receiveNotification, 'TOKEN_UPDATE')).toHaveLength(0); + }); + + it('notifies INVALID_AUTH when Salesforce rejects the grant and nothing newer is stored', async () => { + const { api, receiveNotification } = makeApi({ + stored: { access_token: 'at-old', refresh_token: 'rt-old' }, + }); + api.oauth2.refreshToken.mockRejectedValue(invalidGrant()); + + const { err } = await jsforceRefresh(api); + + expect(err).toBeDefined(); + expect(err.message).toBe('Salesforce rejected the refresh token'); + expect(notified(receiveNotification, 'INVALID_AUTH')).toHaveLength(1); + expect(notified(receiveNotification, 'TOKEN_UPDATE')).toHaveLength(0); + expect(api.conn.refreshToken).toBe('rt-old'); + }); + + it('fails the refresh without invalidating on a token-endpoint transport error', async () => { + const { api, receiveNotification } = makeApi(); + api.oauth2.refreshToken.mockRejectedValue(http503()); + + const { err } = await jsforceRefresh(api); + + expect(err.isTokenRefreshTransportFailure).toBe(true); + expect(err.statusCode).toBe(503); + expect(notified(receiveNotification, 'INVALID_AUTH')).toHaveLength(0); + }); + + it('classifies invalid_client_id as a definitive rejection', async () => { + const { api, receiveNotification } = makeApi({ + stored: { access_token: 'at-old', refresh_token: 'rt-old' }, + }); + api.oauth2.refreshToken.mockRejectedValue(invalidClientId()); + + await jsforceRefresh(api); + + const invalidations = notified(receiveNotification, 'INVALID_AUTH'); + expect(invalidations).toHaveLength(1); + expect(invalidations[0][2]).toEqual({ statusCode: 400 }); + expect(notified(receiveNotification, 'TOKEN_UPDATE')).toHaveLength(0); + }); + + it('adopts the credential another worker wrote after Salesforce rejects the grant', async () => { + const { api, store, receiveNotification } = makeApi({ + stored: { access_token: 'at-old', refresh_token: 'rt-old' }, + backoff: [0], + }); + api.oauth2.refreshToken.mockImplementation(async () => { + store.current = { access_token: 'at-db', refresh_token: 'rt-db' }; + throw invalidGrant(); + }); + + const { err, accessToken } = await jsforceRefresh(api); + + expect(err).toBeUndefined(); + expect(accessToken).toBe('at-db'); + expect(api.conn.refreshToken).toBe('rt-db'); + expect(notified(receiveNotification, 'INVALID_AUTH')).toHaveLength(0); + }); + + it('after a definitive rejection only re-reads the store on later refreshes', async () => { + const { api, store, receiveNotification } = makeApi({ + stored: { access_token: 'at-old', refresh_token: 'rt-old' }, + }); + api.oauth2.refreshToken.mockRejectedValue(invalidGrant()); + await jsforceRefresh(api); + api.oauth2.refreshToken.mockClear(); + receiveNotification.mockClear(); + + const second = await jsforceRefresh(api); + + expect(second.err).toBeDefined(); + expect(api.oauth2.refreshToken).not.toHaveBeenCalled(); + expect(notified(receiveNotification, 'CREDENTIAL_RELOAD')).toHaveLength( + 1 + ); + expect(notified(receiveNotification, 'INVALID_AUTH')).toHaveLength(0); + + store.current = { access_token: 'at-db', refresh_token: 'rt-db' }; + const third = await jsforceRefresh(api); + + expect(third.err).toBeUndefined(); + expect(third.accessToken).toBe('at-db'); + expect(api.conn.refreshToken).toBe('rt-db'); + expect(api.oauth2.refreshToken).not.toHaveBeenCalled(); + }); + + it('resetToSandbox keeps the refresh hook on the new connection', async () => { + const { api } = makeApi(); + api.resetToSandbox(); + Object.assign(api.conn, { + accessToken: 'at-old', + refreshToken: 'rt-old', + }); + api.oauth2.refreshToken.mockResolvedValue(rotated); + + const { err, accessToken } = await jsforceRefresh(api); + + expect(err).toBeUndefined(); + expect(accessToken).toBe('at-new'); + expect(api.refresh_token).toBe('rt-new'); + expect(api.conn.refreshToken).toBe('rt-new'); + }); + + it('refreshes through jsforce when core calls refreshAuth()', async () => { + const { api, receiveNotification } = makeApi(); + api.oauth2.refreshToken.mockResolvedValue(rotated); + + await expect(api.refreshAuth()).resolves.toBe(true); + + expect(api.oauth2.refreshToken).toHaveBeenCalledWith('rt-old'); + expect(api.access_token).toBe('at-new'); + expect(api.conn.accessToken).toBe('at-new'); + expect(notified(receiveNotification, 'TOKEN_UPDATE')).toHaveLength(1); + }); + + it('persists a token response the caller already obtained without refreshing', async () => { + const { api, receiveNotification } = makeApi(); + + await api.refreshAccessToken(rotated); + + expect(api.oauth2.refreshToken).not.toHaveBeenCalled(); + expect(api.refresh_token).toBe('rt-new'); + expect(api.conn.refreshToken).toBe('rt-new'); + expect(api.instanceUrl).toBe('https://old.my.salesforce.com'); + expect(notified(receiveNotification, 'TOKEN_UPDATE')).toHaveLength(1); + }); + + it('does not move the credential instance URL on refresh', async () => { + const { api } = makeApi(); + api.oauth2.refreshToken.mockResolvedValue(rotated); + + await jsforceRefresh(api); + + expect(api.instanceUrl).toBe('https://old.my.salesforce.com'); + expect(api.conn.instanceUrl).toBe('https://old.my.salesforce.com'); + }); + + it('hands jsforce the token only after the credential write has been awaited', async () => { + let release; + const gate = new Promise((resolve) => { + release = resolve; + }); + const { api, receiveNotification } = makeApi(); + receiveNotification.mockImplementation(async (_notifier, type) => { + if (type === 'TOKEN_UPDATE') await gate; + return undefined; + }); + api.oauth2.refreshToken.mockResolvedValue(rotated); + const callback = jest.fn(); + const { refreshFn } = + require('jsforce').Connection.mock.calls.at(-1)[0]; + + refreshFn(api.conn, callback); + await new Promise(setImmediate); + expect(callback).not.toHaveBeenCalled(); + + release(); + await new Promise(setImmediate); + expect(callback).toHaveBeenCalledWith(undefined, 'at-new'); + }); + + it('keeps the stored refresh token when Salesforce does not rotate', async () => { + const { api } = makeApi(); + api.oauth2.refreshToken.mockResolvedValue({ + access_token: 'at-new', + instance_url: rotated.instance_url, + }); + + const { accessToken } = await jsforceRefresh(api); + + expect(accessToken).toBe('at-new'); + expect(api.refresh_token).toBe('rt-old'); + expect(api.conn.refreshToken).toBe('rt-old'); + }); + + it('does not write tokens to the console on a successful refresh', async () => { + const { api } = makeApi(); + api.oauth2.refreshToken.mockResolvedValue(rotated); + + await jsforceRefresh(api); + + expect(consoleOutput()).not.toMatch(/at-new|rt-new|at-old|rt-old/); + }); + + it('does not write tokens to the console on a rejected refresh', async () => { + const { api } = makeApi({ + stored: { access_token: 'at-old', refresh_token: 'rt-old' }, + }); + api.oauth2.refreshToken.mockRejectedValue(invalidGrant()); + + await jsforceRefresh(api); + + expect(consoleOutput()).not.toMatch(/at-new|rt-new|at-old|rt-old/); + }); +}); From b8889ff7cea9d456e78c1cb94068b6e226ba2368 Mon Sep 17 00:00:00 2001 From: d-klotz Date: Thu, 17 Sep 2026 10:03:00 -0300 Subject: [PATCH 2/3] fix(salesforce): refuse a tokenless refresh and log a failed token persist Review follow-ups on the rotated-refresh-token change. - refreshAccessToken() with neither access_token nor refresh_token used to POST an empty refresh_token; Salesforce answers invalid_grant, which the normaliser classifies as definitive and core turns into INVALID_AUTH. A caller passing a partial response could brick a healthy credential. It now throws before the request. - When the token POST succeeds but the credential write fails, the rotated pair stays in memory on purpose (replaying the consumed token revokes the family), but the stored token is now consumed and nothing said so. Log it at error level before rethrowing. - Initialise _refreshRejected in the constructor. - Tests: a transport failure must not arm the rejection memo (the 503 test now refreshes a second time and expects a token POST); the tokenless call throws without a POST or an INVALID_AUTH; the persist-failure ordering and its log. Dropped the "does not subscribe to the jsforce refresh event" test: it pinned a mechanism, not a behaviour. One TOKEN_UPDATE per refresh and no tokens in the console already cover what it stood for. Left as-is, deliberately: invalid_client_id / inactive_* are classified definitive (extends core's existing invalid_client policy); a missing client_id now surfaces on the first 401 rather than at construction. Co-Authored-By: Claude Fable 5.1 --- packages/v1-ready/salesforce/api.js | 22 ++++++++-- packages/v1-ready/salesforce/test/api.test.js | 42 +++++++++++++++---- 2 files changed, 51 insertions(+), 13 deletions(-) diff --git a/packages/v1-ready/salesforce/api.js b/packages/v1-ready/salesforce/api.js index 1b8e9eb..a23127a 100644 --- a/packages/v1-ready/salesforce/api.js +++ b/packages/v1-ready/salesforce/api.js @@ -18,6 +18,7 @@ class Api extends OAuth2Requester { constructor(params) { super(params); this.jsforce = jsforce; + this._refreshRejected = false; this.key = get(params, 'client_id', null); this.secret = get(params, 'client_secret', null); this.instanceUrl = get(params, 'instanceUrl', null); @@ -237,6 +238,11 @@ class Api extends OAuth2Requester { async refreshAccessToken(tokenOrResponse) { let res = tokenOrResponse; if (!res.access_token) { + if (!res.refresh_token) { + throw new Error( + 'refreshAccessToken requires an access_token or a refresh_token' + ); + } try { res = await this.oauth2.refreshToken(res.refresh_token); } catch (err) { @@ -263,10 +269,18 @@ class Api extends OAuth2Requester { async _applyTokenResponse(res) { this.conn.accessToken = res.access_token; if (res.refresh_token) this.conn.refreshToken = res.refresh_token; - await this.setTokens({ - access_token: res.access_token, - refresh_token: res.refresh_token, - }); + try { + await this.setTokens({ + access_token: res.access_token, + refresh_token: res.refresh_token, + }); + } catch (err) { + console.error( + '[salesforce] rotated refresh token was not persisted; the stored token is now consumed', + { message: err?.message } + ); + throw err; + } } } diff --git a/packages/v1-ready/salesforce/test/api.test.js b/packages/v1-ready/salesforce/test/api.test.js index c4b07c0..38d6c7c 100644 --- a/packages/v1-ready/salesforce/test/api.test.js +++ b/packages/v1-ready/salesforce/test/api.test.js @@ -121,15 +121,7 @@ describe('Salesforce Api', () => { expect(lastCall[0].accessToken).toBe('my-access-token'); expect(lastCall[0].refreshToken).toBe('my-refresh-token'); expect(lastCall[0].instanceUrl).toBe('https://myorg.salesforce.com'); - }); - - it('does not subscribe to the jsforce refresh event', () => { - expect(api.conn.on).not.toHaveBeenCalledWith( - 'refresh', - expect.any(Function) - ); - }); - }); + }); }); }); describe('getAuthorizationUri state handling', () => { @@ -298,6 +290,10 @@ describe('Salesforce Api token refresh', () => { expect(err.isTokenRefreshTransportFailure).toBe(true); expect(err.statusCode).toBe(503); expect(notified(receiveNotification, 'INVALID_AUTH')).toHaveLength(0); + + api.oauth2.refreshToken.mockClear(); + await jsforceRefresh(api); + expect(api.oauth2.refreshToken).toHaveBeenCalledTimes(1); }); it('classifies invalid_client_id as a definitive rejection', async () => { @@ -467,4 +463,32 @@ describe('Salesforce Api token refresh', () => { expect(consoleOutput()).not.toMatch(/at-new|rt-new|at-old|rt-old/); }); + + it('refuses to refresh without a token to refresh with', async () => { + const { api, receiveNotification } = makeApi(); + + await expect(api.refreshAccessToken({})).rejects.toThrow( + /access_token or a refresh_token/ + ); + + expect(api.oauth2.refreshToken).not.toHaveBeenCalled(); + expect(notified(receiveNotification, 'INVALID_AUTH')).toHaveLength(0); + }); + + it('keeps the rotated pair in memory when the credential write fails', async () => { + const { api, receiveNotification } = makeApi(); + receiveNotification.mockImplementation(async (_notifier, type) => { + if (type === 'TOKEN_UPDATE') throw new Error('db unavailable'); + return undefined; + }); + api.oauth2.refreshToken.mockResolvedValue(rotated); + + const { err } = await jsforceRefresh(api); + + expect(err).toBeDefined(); + expect(api.conn.refreshToken).toBe('rt-new'); + expect(api.conn.accessToken).toBe('at-new'); + expect(consoleOutput()).toMatch(/not persisted/); + expect(consoleOutput()).not.toMatch(/at-new|rt-new|at-old|rt-old/); + }); }); From 46601ad4df2de0e8b5145b10cf966ea88231c484 Mon Sep 17 00:00:00 2001 From: d-klotz Date: Thu, 17 Sep 2026 11:09:55 -0300 Subject: [PATCH 3/3] fix(salesforce): let a fresh token install clear the rejection memo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Codex findings on the memo introduced in 97581e8. - After a definitive rejection armed _refreshRejected, a re-authorization on the same Api instance — getAccessToken(), or the persist-only refreshAccessToken(response) shape — installed fresh tokens but left the memo set. On the next 401 the instance only re-read the store; the stored refresh token equalled the in-memory one, adoption returned false, and the perfectly valid token was never submitted. The instance stayed read-only until recreated. Every successful setTokens() now clears the memo. - The memo branch called _adoptNewerCredential() outside the try/catch that guards _refreshAuthOnce(). A throwing reload escaped the async refreshFn adapter, leaving jsforce's refresh promise unsettled and the originating request hanging. Both branches now share one try/catch. Co-Authored-By: Claude Fable 5.1 --- packages/v1-ready/salesforce/api.js | 19 +++-- packages/v1-ready/salesforce/test/api.test.js | 69 +++++++++++++++++++ 2 files changed, 78 insertions(+), 10 deletions(-) diff --git a/packages/v1-ready/salesforce/api.js b/packages/v1-ready/salesforce/api.js index a23127a..fdfb64e 100644 --- a/packages/v1-ready/salesforce/api.js +++ b/packages/v1-ready/salesforce/api.js @@ -204,18 +204,11 @@ class Api extends OAuth2Requester { } async _jsforceRefreshFn(callback) { - if (this._refreshRejected) { - if (!(await this._adoptNewerCredential())) { - return callback( - new Error('Salesforce rejected the refresh token') - ); - } - this._refreshRejected = false; - return callback(undefined, this.access_token); - } let refreshed; try { - refreshed = await this._refreshAuthOnce(); + refreshed = this._refreshRejected + ? await this._adoptNewerCredential() + : await this._refreshAuthOnce(); } catch (err) { return callback(err); } @@ -223,6 +216,7 @@ class Api extends OAuth2Requester { this._refreshRejected = true; return callback(new Error('Salesforce rejected the refresh token')); } + this._refreshRejected = false; callback(undefined, this.access_token); } @@ -235,6 +229,11 @@ class Api extends OAuth2Requester { return adopted; } + async setTokens(params) { + await super.setTokens(params); + this._refreshRejected = false; + } + async refreshAccessToken(tokenOrResponse) { let res = tokenOrResponse; if (!res.access_token) { diff --git a/packages/v1-ready/salesforce/test/api.test.js b/packages/v1-ready/salesforce/test/api.test.js index 38d6c7c..eedccd3 100644 --- a/packages/v1-ready/salesforce/test/api.test.js +++ b/packages/v1-ready/salesforce/test/api.test.js @@ -6,6 +6,8 @@ jest.mock('jsforce', () => { const mockConnection = { on: jest.fn(), + authorize: jest.fn(), + oauth2: {}, accessToken: 'test-access-token', refreshToken: 'test-refresh-token', instanceUrl: 'https://test.salesforce.com', @@ -491,4 +493,71 @@ describe('Salesforce Api token refresh', () => { expect(consoleOutput()).toMatch(/not persisted/); expect(consoleOutput()).not.toMatch(/at-new|rt-new|at-old|rt-old/); }); + + it('clears the rejection memo when fresh tokens are persisted', async () => { + const { api, store } = makeApi({ + stored: { access_token: 'at-old', refresh_token: 'rt-old' }, + }); + api.oauth2.refreshToken.mockRejectedValue(invalidGrant()); + await jsforceRefresh(api); + + await api.refreshAccessToken(rotated); + store.current = { access_token: 'at-new', refresh_token: 'rt-new' }; + api.oauth2.refreshToken.mockReset(); + api.oauth2.refreshToken.mockResolvedValue({ + access_token: 'at-newer', + refresh_token: 'rt-newer', + }); + + const { err, accessToken } = await jsforceRefresh(api); + + expect(err).toBeUndefined(); + expect(accessToken).toBe('at-newer'); + expect(api.oauth2.refreshToken).toHaveBeenCalledWith('rt-new'); + }); + + it('clears the rejection memo when an authorization code is exchanged', async () => { + const { api, store } = makeApi({ + stored: { access_token: 'at-old', refresh_token: 'rt-old' }, + }); + api.oauth2.refreshToken.mockRejectedValue(invalidGrant()); + await jsforceRefresh(api); + + api.conn.authorize.mockImplementation(async () => { + Object.assign(api.conn, { + accessToken: 'at-new', + refreshToken: 'rt-new', + }); + }); + await api.getAccessToken('auth-code'); + store.current = { access_token: 'at-new', refresh_token: 'rt-new' }; + api.oauth2.refreshToken.mockReset(); + api.oauth2.refreshToken.mockResolvedValue({ + access_token: 'at-newer', + refresh_token: 'rt-newer', + }); + + const { err } = await jsforceRefresh(api); + + expect(err).toBeUndefined(); + expect(api.oauth2.refreshToken).toHaveBeenCalledWith('rt-new'); + }); + + it('reports a failed credential reload through the callback while the memo is set', async () => { + const { api } = makeApi({ + stored: { access_token: 'at-old', refresh_token: 'rt-old' }, + }); + api.oauth2.refreshToken.mockRejectedValue(invalidGrant()); + await jsforceRefresh(api); + jest.spyOn(api, '_adoptNewerCredential').mockRejectedValue( + new Error('db down') + ); + const { refreshFn } = + require('jsforce').Connection.mock.calls.at(-1)[0]; + const callback = jest.fn(); + + await refreshFn(api.conn, callback); + + expect(callback).toHaveBeenCalledWith(expect.any(Error)); + }); });