Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(oauth): bubble up new token when refreshing it #1163

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

cristiand391
Copy link
Member

@cristiand391 cristiand391 commented Jan 16, 2025

What does this PR do?

Fixes AuthInfo.refreshFn passing the expired access token instead of the new fresh one when refreshing the session.
Others:

  • makes Org/Connection refreshAuth do a HEAD request (small improvement)
  • add UT to cover AuthInfo.refreshFn always passing the right token.

Context

sfdx-core sets a custom refreshFn for oauth refresh (jsforce has a default one but ours also updates the AuthInfo instance with the new token):

refreshFn: this.refreshFn.bind(this),

private async refreshFn(

jsforce calls it here, expecting the callback to send back the new, refreshed token to set it in the Connection instance:
https://github.com/jsforce/jsforce/blob/73a6d0a174050b370b4248925e2e474bba27091d/src/session-refresh-delegate.ts#L53

AuthInfo.refreshFn was getting a new access token when calling this.initAuthOptions here:
https://github.com/forcedotcom/sfdx-core/blob/0919f2844544983b7635f9d25eb544c2f4aa7cdd/src/org/authInfo.ts#L949C1-L952C55

but it was sending back the expired access token from the previously decrypted auth fields to the callback, causing 2 refresh calls every time sfdx-core was doing a request with an expired token.

See the following examples, we expire the current token and call api request reset (with this PR linked: salesforcecli/plugin-api#62) which tries to refresh the token before doing the limits call with got.

`AuthInfo.refreshFn` passes the expired AT to the callback, fires an additional refresh
➜  plugin-api git:(cd/refresh-token) ✗ SF_AT=$(sf org display --json | jq -r .result.accessToken) &&
echo "token=$SF_AT&token_type_hint=access_token" > token.txt &&
curl "https://customer-customization-3903-dev-ed.scratch.my.salesforce.com/services/oauth2/revoke" -d "@token.txt" -H 'Content-Type: application/x-www-form-urlencoded' -i
 && JSFORCE_LOG_LEVEL=DEBUG sf api request rest services/data/v62.0/limits
 ›   Warning: @salesforce/plugin-api is a linked ESM module and cannot be auto-transpiled. Existing compiled source will be used instead.
HTTP/2 200
...
                                                                                                                                                                          
 ›   Warning: @salesforce/plugin-api is a linked ESM module and cannot be auto-transpiled. Existing compiled source will be used instead.
Warning: This command is currently in beta. Any aspect of this command can change without advanced notice. Don't use beta commands in your scripts.
DEBUG   [http-api]  <request> method=GET, url=https://customer-customization-3903-dev-ed.scratch.my.salesforce.com/services/data/v62.0
DEBUG   [http-api]  elapsed time: 355 msec
DEBUG   [http-api]  <response> status=401, url=https://customer-customization-3903-dev-ed.scratch.my.salesforce.com/services/data/v62.0
INFO    [session-refresh-delegate]  <refresh token>
DEBUG   [session-refresh-delegate]  Connection refresh completed.
INFO    [session-refresh-delegate]  <refresh complete>
DEBUG   [http-api]  <request> method=GET, url=https://customer-customization-3903-dev-ed.scratch.my.salesforce.com/services/data/v62.0
DEBUG   [http-api]  elapsed time: 202 msec
DEBUG   [http-api]  <response> status=401, url=https://customer-customization-3903-dev-ed.scratch.my.salesforce.com/services/data/v62.0
INFO    [session-refresh-delegate]  <refresh token>
DEBUG   [session-refresh-delegate]  Connection refresh completed.
INFO    [session-refresh-delegate]  <refresh complete>
DEBUG   [http-api]  <request> method=GET, url=https://customer-customization-3903-dev-ed.scratch.my.salesforce.com/services/data/v62.0
DEBUG   [http-api]  elapsed time: 198 msec
DEBUG   [http-api]  <response> status=200, url=https://customer-customization-3903-dev-ed.scratch.my.salesforce.com/services/data/v62.0
{
  "AnalyticsExternalDataSizeMB": {
    "Max": 40960,
    "Remaining": 40960
  },
`AuthInfo.refreshFn` passes the new AT to the callback, next request uses it successfully
➜  plugin-api git:(cd/refresh-token) ✗ SF_AT=$(sf org display --json | jq -r .result.accessToken) &&
echo "token=$SF_AT&token_type_hint=access_token" > token.txt &&
curl "https://customer-customization-3903-dev-ed.scratch.my.salesforce.com/services/oauth2/revoke" -d "@token.txt" -H 'Content-Type: application/x-www-form-urlencoded' -i
 && JSFORCE_LOG_LEVEL=DEBUG sf api request rest services/data/v62.0/limits
 ›   Warning: @salesforce/plugin-api is a linked ESM module and cannot be auto-transpiled. Existing compiled source will be used instead.
HTTP/2 200
...

 ›   Warning: @salesforce/plugin-api is a linked ESM module and cannot be auto-transpiled. Existing compiled source will be used instead.
Warning: This command is currently in beta. Any aspect of this command can change without advanced notice. Don't use beta commands in your scripts.
DEBUG   [http-api]  <request> method=HEAD, url=https://customer-customization-3903-dev-ed.scratch.my.salesforce.com/services/data/v62.0
DEBUG   [http-api]  elapsed time: 329 msec
DEBUG   [http-api]  <response> status=401, url=https://customer-customization-3903-dev-ed.scratch.my.salesforce.com/services/data/v62.0
INFO    [session-refresh-delegate]  <refresh token>
DEBUG   [session-refresh-delegate]  Connection refresh completed.
INFO    [session-refresh-delegate]  <refresh complete>
DEBUG   [http-api]  <request> method=HEAD, url=https://customer-customization-3903-dev-ed.scratch.my.salesforce.com/services/data/v62.0
DEBUG   [http-api]  elapsed time: 423 msec
DEBUG   [http-api]  <response> status=200, url=https://customer-customization-3903-dev-ed.scratch.my.salesforce.com/services/data/v62.0
DEBUG   [http-api]  Failed to parse body of content-type: application/json;charset=UTF-8. Error: Unexpected end of JSON input
{
  "AnalyticsExternalDataSizeMB": {
    "Max": 40960,
    "Remaining": 40960
  },

Testing

  1. checkout this PR fix: refresh access token before issuing request salesforcecli/plugin-api#62
  2. checkout this PR and link it to plugin-api

See repro steps in plugin-api PR ⬆️
With both PRs you should see only one refresh call when running api request rest with an expired token.

What issues does this PR fix or reference?

forcedotcom/cli#3176
@W-17605467@

Updated UT to ensure `AuthInfo.refreshFn` always bubbles up the new,
refreshed access token instead of the old one.
jsforce only needs the response status code to refresh the session.
@cristiand391 cristiand391 marked this pull request as ready for review January 16, 2025 18:48
@cristiand391 cristiand391 requested a review from a team as a code owner January 16, 2025 18:48
privateKey: context.getFields().privateKey,
loginUrl: testOrg.loginUrl,
clientId: testOrg.clientId,
privateKey: testOrg.privateKey,
accessToken: testOrg.accessToken,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed these context.getFields() calls to simplify the stubbing (see the new stub only cares about the first and second call)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants