From fa7817a69c0cbc6796fd6b48582320f07e331dc5 Mon Sep 17 00:00:00 2001 From: Cristian Dominguez Date: Thu, 16 Jan 2025 12:37:04 -0300 Subject: [PATCH 1/3] test(oauth): UT ensures new token is bubbled up Updated UT to ensure `AuthInfo.refreshFn` always bubbles up the new, refreshed access token instead of the old one. --- test/unit/org/authInfo.test.ts | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/test/unit/org/authInfo.test.ts b/test/unit/org/authInfo.test.ts index dbd3ce709..3697c87d7 100644 --- a/test/unit/org/authInfo.test.ts +++ b/test/unit/org/authInfo.test.ts @@ -1096,14 +1096,25 @@ describe('AuthInfo', () => { describe('refreshFn', () => { it('should call init() and save()', async () => { + const refreshedToken = '123456789abc'; + const context = { getUsername: () => '', - getFields: (decrypt = false) => ({ - loginUrl: testOrg.loginUrl, - clientId: testOrg.clientId, - privateKey: 'authInfoTest/jwt/server.key', - accessToken: decrypt ? testOrg.accessToken : testOrg.encryptedAccessToken, - }), + getFields: $$.SANDBOX.stub() + .onFirstCall() + .callsFake((decrypt = false) => ({ + loginUrl: testOrg.loginUrl, + clientId: testOrg.clientId, + privateKey: 'authInfoTest/jwt/server.key', + accessToken: decrypt ? testOrg.accessToken : testOrg.encryptedAccessToken, + })) + .onSecondCall() + .callsFake((decrypt = false) => ({ + loginUrl: testOrg.loginUrl, + clientId: testOrg.clientId, + privateKey: 'authInfoTest/jwt/server.key', + accessToken: decrypt ? refreshedToken : testOrg.encryptedAccessToken, + })), initAuthOptions: $$.SANDBOX.stub(), save: $$.SANDBOX.stub(), logger: $$.TEST_LOGGER, @@ -1119,15 +1130,18 @@ describe('AuthInfo', () => { expect(context.initAuthOptions.called, 'Should have called AuthInfo.initAuthOptions() during refreshFn()').to.be .true; const expectedInitArgs = { - loginUrl: context.getFields().loginUrl, - clientId: context.getFields().clientId, - privateKey: context.getFields().privateKey, + loginUrl: testOrg.loginUrl, + clientId: testOrg.clientId, + privateKey: testOrg.privateKey, accessToken: testOrg.accessToken, }; expect(context.initAuthOptions.firstCall.args[0]).to.deep.equal(expectedInitArgs); expect(context.save.called, 'Should have called AuthInfo.save() during refreshFn()').to.be.true; expect(testCallback.called, 'Should have called the callback passed to refreshFn()').to.be.true; - expect(testCallback.firstCall.args[1]).to.equal(testOrg.accessToken); + expect( + testCallback.firstCall.args[1], + 'Should have passed the new access token to the refreshFn callback' + ).to.equal(refreshedToken); }); it('should path.resolve jwtkeyfilepath', async () => { From 1170cd70566a34c361a57caba66d6046ae3745d8 Mon Sep 17 00:00:00 2001 From: Cristian Dominguez Date: Thu, 16 Jan 2025 12:42:06 -0300 Subject: [PATCH 2/3] chore: refreshAuth methods do HEAD request jsforce only needs the response status code to refresh the session. --- LICENSE.txt | 2 +- src/org/connection.ts | 14 +++++++++----- src/org/org.ts | 9 +++++++-- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/LICENSE.txt b/LICENSE.txt index 6cfd87342..fa1e65c74 100644 --- a/LICENSE.txt +++ b/LICENSE.txt @@ -1,4 +1,4 @@ -Copyright (c) 2024, Salesforce.com, Inc. +Copyright (c) 2025, Salesforce.com, Inc. All rights reserved. Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met: diff --git a/src/org/connection.ts b/src/org/connection.ts index d411a1b98..a437fd370 100644 --- a/src/org/connection.ts +++ b/src/org/connection.ts @@ -14,7 +14,6 @@ import { asString, ensure, isString, JsonMap, Optional } from '@salesforce/ts-ty import { Connection as JSForceConnection, ConnectionConfig, - HttpMethods, HttpRequest, QueryOptions, QueryResult, @@ -414,14 +413,19 @@ export class Connection extends JSForceConnection } /** - * Executes a get request on the baseUrl to force an auth refresh - * Useful for the raw methods (request, requestRaw) that use the accessToken directly and don't handle refreshes + * Executes a HEAD request on the baseUrl to force an auth refresh. + * This is useful for the raw methods (request, requestRaw) that use the accessToken directly and don't handle refreshes. + * + * This method issues a request using the current access token to check if it is still valid. + * If the request returns 200, no refresh happens, and we keep the token. + * If it returns 401, jsforce will request a new token and set it in the connection instance. */ + public async refreshAuth(): Promise { this.logger.debug('Refreshing auth for org.'); - const requestInfo = { + const requestInfo: HttpRequest = { url: this.baseUrl(), - method: 'GET' as HttpMethods, + method: 'HEAD', }; await this.request(requestInfo); } diff --git a/src/org/org.ts b/src/org/org.ts index d23507be6..f1a62a746 100644 --- a/src/org/org.ts +++ b/src/org/org.ts @@ -792,13 +792,18 @@ export class Org extends AsyncOptionalCreatable { } /** - * Refreshes the auth for this org's instance by calling HTTP GET on the baseUrl of the connection object. + * Executes a HEAD request on the baseUrl to force an auth refresh. + * This is useful for the raw methods (request, requestRaw) that use the accessToken directly and don't handle refreshes. + * + * This method issues a request using the current access token to check if it is still valid. + * If the request returns 200, no refresh happens, and we keep the token. + * If it returns 401, jsforce will request a new token and set it in the connection instance. */ public async refreshAuth(): Promise { this.logger.debug('Refreshing auth for org.'); const requestInfo: HttpRequest = { url: this.getConnection().baseUrl(), - method: 'GET', + method: 'HEAD', }; await this.getConnection().request(requestInfo); From b6bdd36c9dffbdd3d234bc66e8b867e339159901 Mon Sep 17 00:00:00 2001 From: Cristian Dominguez Date: Thu, 16 Jan 2025 12:44:07 -0300 Subject: [PATCH 3/3] fix(oauth): bubble up new token when refreshing it --- src/org/authInfo.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/org/authInfo.ts b/src/org/authInfo.ts index dda82cb5d..7171fe937 100644 --- a/src/org/authInfo.ts +++ b/src/org/authInfo.ts @@ -940,16 +940,23 @@ export class AuthInfo extends AsyncOptionalCreatable { // A callback function for a connection to refresh an access token. This is used // both for a JWT connection and an OAuth connection. private async refreshFn( - conn: Connection, + _conn: Connection, callback: (err: Nullable, accessToken?: string, res?: Record) => Promise ): Promise { this.logger.info('Access token has expired. Updating...'); try { const fields = this.getFields(true); + + // This method will request the new access token and save to the current AuthInfo instance (but don't persist them!). await this.initAuthOptions(fields); + // Persist fields with refreshed access token to auth file. await this.save(); - return await callback(null, fields.accessToken); + + // Pass new access token to the jsforce's session-refresh callback for proper propagation: + // https://jsforce.github.io/jsforce/types/session_refresh_delegate.SessionRefreshFunc.html + const { accessToken } = this.getFields(true); + return await callback(null, accessToken); } catch (err) { const error = err as Error; if (error?.message?.includes('Data Not Available')) {